Fix reversed values, add config options

The values of the protocol ID and identity provider ID are
reversed in the charm's code. This causes the mod_auth_openidc
config to render incorrectly in the Apache template. This change
fixes the reversed values and adds a config option for
defining the identity provider ID. This change also adds new
config options for enabling using mod_auth_openidc with
multiple Keystone units and with proxies.

Closes-Bug: #2065590
Func-Test-Pr: openstack-charmers/zaza-openstack-tests#1241
Change-Id: Ie7da5f9a85027a287aad9958c54848a948f8495c
(cherry picked from commit da043cb)

Author: jadonn

config.yaml

@@ -66,6 +66,34 @@ options:
       (e.g. https://example.com/idp/userinfo.openid). Used when
       oidc-provider-metadata-url is not set or the metadata obtained from that
       URL does not set it.
+  oidc-x-forwarded-headers:
+    default: ''
+    type: string
+    description: |
+      X-Forwarded-* headers from reverse proxies that mod_auth_openidc should
+      look for. Must be one or more of "X-Forwarded-Host", "X-Forwarded-Port",
+      "X-Forwarded-Proto", "Forwarded", or "none". mod_auth_openidc ignores
+      this setting if it is "none" or undefined. Use this setting when using
+      a proxy that changes the protocol, host, or port when handling the
+      authentication workflow.
+  oidc-state-input-headers:
+    default: 'user-agent'
+    type: string
+    description: |
+      Define the headers mod_auth_openidc uses to calculate the browser
+      fingerprint during authentication. Set to "none" if using multiple
+      units of Keystone behind a load balancer or proxy.
+  oidc-session-type:
+    default: 'server-cache'
+    type: string
+    description: |
+      Set where OpenID Connect session cookies are stored. BY default cookies
+      are stored on the web server. Can be one of 'server-cache',
+      'server-cache:persistent', 'client-cookie', 'client-cookie:persistent',
+      'client-cookie:store_id_token', or
+      'client-cookie:persistent:store_id_token'. When using multiple units
+      of Keystone behind a proxy, use 'client-cookie:persistent' if you are
+      not using shared session storage for Keystone.
   auth-type:
     default: 'auth-openidc'
     type: string
@@ -74,6 +102,11 @@ options:
       by applications that do not adopt the browser flow, such the OpenStack
       CLI, the auth-type must be set to auth-openidc (the default) otherwise
       to openid-connect.
+  idp_id:
+    default: ''
+    type: string
+    description: |
+      The ID of the Identity Provider defined in Keystone.
   protocol_id:
     default: 'openid'
     type: string

src/charm.py

@@ -94,11 +94,6 @@ def oidc_auth_path(self) -> str:
         return (f'/v3/OS-FEDERATION/identity_providers/{self.idp_id}'
                 f'/protocols/openid/auth')
 
-    @property
-    def idp_id(self) -> str:
-        """Identity provider name to use for URL generation."""
-        return 'openid'
-
     @property
     def scheme(self) -> Optional[str]:
         data = self._get_principal_data()
@@ -186,7 +181,7 @@ class KeystoneOpenIDCCharm(ops_openstack.core.OSBaseCharm):
                           'cluster']
 
     REQUIRED_KEYS = ['oidc_crypto_passphrase', 'oidc_client_id',
-                     'hostname', 'port', 'scheme']
+                     'hostname', 'port', 'scheme', 'idp_id', 'protocol_id']
     APACHE2_MODULE = 'auth_openidc'
 
     CONFIG_FILE_OWNER = 'root'
@@ -261,7 +256,7 @@ def update_principal_data(self):
         # When (if) this patch is merged, we can use auth-method
         # https://review.opendev.org/c/openstack/charm-keystone/+/852601
         # data['auth-method'] = json.dumps(self.auth_method)
-        data['protocol-name'] = json.dumps(self.options.idp_id)
+        data['protocol-name'] = json.dumps(self.options.protocol_id)
         data['remote-id-attribute'] = json.dumps(
             self.options.remote_id_attribute)
 
@@ -283,8 +278,8 @@ def _update_websso_data(self):
             return
 
         data = relation.data[self.unit]
-        data['protocol-name'] = json.dumps(self.options.idp_id)
-        data['idp-name'] = json.dumps(self.options.protocol_id)
+        data['protocol-name'] = json.dumps(self.options.protocol_id)
+        data['idp-name'] = json.dumps(self.options.idp_id)
         data['user-facing-name'] = json.dumps(self.options.user_facing_name)
 
     def _on_config_changed(self, event):

templates/apache-openidc-location.conf

@@ -4,6 +4,15 @@ OIDCClaimDelimiter ";"
 OIDCResponseType "id_token"
 OIDCScope "openid email profile"
 
+{% if options.oidc_session_type -%}
+OIDCSessionType {{ options.oidc_session_type }}
+{% endif -%}
+{% if options.oidc_state_input_headers -%}
+OIDCStateInputHeaders {{ options.oidc_state_input_headers }}
+{% endif -%}
+{% if options.oidc_x_forwarded_headers -%}
+OIDCXForwardedHeaders {{ options.oidc_x_forwarded_headers }}
+{% endif -%}
 {% if options.oidc_provider_metadata_url -%}
 OIDCProviderMetadataURL {{ options.oidc_provider_metadata_url }}
 {% endif -%}
@@ -58,10 +67,6 @@ OIDCOAuthClientSecret {{ options.oidc_client_secret }}
 {%- endif %}
 </LocationMatch>
 
-# Support for websso from Horizon
-OIDCRedirectURI "{{ options.scheme }}://{{ options.hostname }}:{{ options.port }}/v3/auth/OS-FEDERATION/identity_providers/{{ options.idp_id }}/protocols/{{ options.protocol_id }}/websso"
-OIDCRedirectURI "{{ options.scheme }}://{{ options.hostname }}:{{ options.port }}/v3/auth/OS-FEDERATION/websso/{{ options.protocol_id }}"
-
 <Location /v3/auth/OS-FEDERATION/websso/{{ options.protocol_id }}>
   Require valid-user
   AuthType openid-connect

tests/tests.yaml

@@ -28,7 +28,7 @@ tests:
 target_deploy_status:
   keystone-openidc:
     workload-status: blocked
-    workload-status-message-prefix: 'required keys: oidc_client_id, oidc_provider_metadata_url'
+    workload-status-message-prefix: 'required keys: oidc_client_id, idp_id, oidc_provider_metadata_url'
   openidc-test-fixture:
     workload-status: active
     workload-status-message-prefix: 'ready'

unit_tests/test_charm.py

@@ -128,7 +128,9 @@ def test_find_missing_keys_no_metadata_url(self):
         missing_keys = self.harness.charm.find_missing_keys()
         missing_keys.sort()
 
-        expected = ['oidc_client_id', 'oidc_provider_metadata_url']
+        expected = ['idp_id',
+                    'oidc_client_id',
+                    'oidc_provider_metadata_url']
         expected.sort()
         self.assertEqual(missing_keys, expected)
 
@@ -142,7 +144,8 @@ def test_find_missing_keys_manual_configuration(self):
         missing_keys = self.harness.charm.find_missing_keys()
         missing_keys.sort()
 
-        expected = ['oidc_provider_auth_endpoint',
+        expected = ['idp_id',
+                    'oidc_provider_auth_endpoint',
                     'oidc_provider_token_endpoint',
                     'oidc_provider_token_endpoint_auth',
                     'oidc_provider_user_info_endpoint',
@@ -186,8 +189,8 @@ def test_update_websso_data(self):
         self.harness.charm._update_websso_data()
         data = self.harness.get_relation_data(rid, 'keystone-openidc/0')
         options = self.harness.charm.options
-        expected = {'protocol-name': json.dumps(options.idp_id),
-                    'idp-name': json.dumps(options.protocol_id),
+        expected = {'protocol-name': json.dumps(options.protocol_id),
+                    'idp-name': json.dumps(options.idp_id),
                     'user-facing-name': json.dumps(options.user_facing_name)}
         self.assertDictEqual(data, expected)