fixup! ModuleRouter: support paths in BASE

Rebased to current master. When composing the paths, use os.path.join
primarily, since it handles empty strings and duplicate separators
logically.
As long as we use the BASE_URL in the OpenID Connect frontend as an
issuer, it's not possible to create multiple provider discovery URLs.
Add documentation and a comment to explain this limitation.

Author: bajnokk

doc/README.md

@@ -78,7 +78,8 @@ bind_password: !ENVFILE LDAP_BIND_PASSWORD_FILE
 
 | Parameter name | Data type | Example value | Description |
 | -------------- | --------- | ------------- | ----------- |
-| `BASE` | string | `https://proxy.example.com` | base url of the proxy |
+| `BASE` | string | `https://proxy.example.com` | The base url of the proxy. For the OIDC Frontend, this is used to set the issuer as well, and due to implementation constraints, avoid
+using trailing slashes in this case.  |
 | `COOKIE_STATE_NAME` | string | `satosa_state` | name of the cookie SATOSA uses for preserving state between requests |
 | `CONTEXT_STATE_DELETE` | bool | `True` | controls whether SATOSA will delete the state cookie after receiving the authentication response from the upstream IdP|
 | `STATE_ENCRYPTION_KEY` | string | `52fddd3528a44157` | key used for encrypting the state cookie, will be overridden by the environment variable `SATOSA_STATE_ENCRYPTION_KEY` if it is set |

src/satosa/frontends/base.py

@@ -17,16 +17,19 @@ def __init__(self, auth_req_callback_func, internal_attributes, base_url, name):
         :type auth_req_callback_func:
         (satosa.context.Context, satosa.internal.InternalData) -> satosa.response.Response
         :type internal_attributes: dict[str, dict[str, str | list[str]]]
+        :type base_url: str
         :type name: str
 
         :param auth_req_callback_func: Callback should be called by the module after the
         authorization response has been processed.
+        :param internal_attributes: attribute mapping
+        :param base_url: base url of the proxy
         :param name: name of the plugin
         """
         self.auth_req_callback_func = auth_req_callback_func
         self.internal_attributes = internal_attributes
         self.converter = AttributeMapper(internal_attributes)
-        self.base_url = base_url.rstrip("/") if base_url else ""
+        self.base_url = base_url or ""
         self.name = name
         self.endpoint_baseurl = os.path.join(self.base_url, self.name)
         self.endpoint_basepath = urlparse(self.endpoint_baseurl).path.lstrip("/")

src/satosa/frontends/openid_connect.py

@@ -4,6 +4,7 @@
 
 import json
 import logging
+import os.path
 from collections import defaultdict
 from urllib.parse import urlencode, urlparse
 
@@ -172,7 +173,16 @@ def register_endpoints(self, backend_names):
         :rtype: list[(str, ((satosa.context.Context, Any) -> satosa.response.Response, Any))]
         :raise ValueError: if more than one backend is configured
         """
-        provider_config = ("^.well-known/openid-configuration$", self.provider_config)
+        # See https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfig
+        # Unfortunately since the issuer is always `base_url` for all OIDC frontend instances,
+        # the discovery endpoint will be the same for every instance.
+        # This means that only one frontend will be usable for autodiscovery.
+        autoconf_path = ".well-known/openid-configuration"
+        base_path = urlparse(self.base_url).path.lstrip("/")
+        provider_config = (
+            "^{}$".format(os.path.join(base_path, autoconf_path)),
+            self.provider_config,
+        )
         jwks_uri = ("^{}/jwks$".format(self.endpoint_basepath), self.jwks)
 
         backend_name = None
@@ -193,35 +203,47 @@ def register_endpoints(self, backend_names):
 
         if backend_name:
             # if there is only one backend, include its name in the path so the default routing can work
-            auth_endpoint = "{}/{}/{}/{}".format(self.base_url, backend_name, self.name, AuthorizationEndpoint.url)
+            auth_endpoint = os.path.join(
+                self.base_url,
+                backend_name,
+                self.name,
+                AuthorizationEndpoint.url,
+            )
             self.provider.configuration_information["authorization_endpoint"] = auth_endpoint
             auth_path = urlparse(auth_endpoint).path.lstrip("/")
         else:
-            auth_path = "{}/{}".format(self.endpoint_basepath, AuthorizationEndpoint.url)
+            auth_path = os.path.join(self.endpoint_basepath, AuthorizationRequest.url)
 
         authentication = ("^{}$".format(auth_path), self.handle_authn_request)
         url_map = [provider_config, jwks_uri, authentication]
 
         if any("code" in v for v in self.provider.configuration_information["response_types_supported"]):
-            self.provider.configuration_information["token_endpoint"] = "{}/{}".format(
-                self.endpoint_baseurl, TokenEndpoint.url
+            self.provider.configuration_information["token_endpoint"] = os.path.join(
+                self.endpoint_baseurl,
+                TokenEndpoint.url,
             )
             token_endpoint = (
-                "^{}/{}".format(self.endpoint_basepath, TokenEndpoint.url), self.token_endpoint
+                "^{}".format(os.path.join(self.endpoint_basepath, TokenEndpoint.url)),
+                self.token_endpoint,
             )
             url_map.append(token_endpoint)
 
             self.provider.configuration_information["userinfo_endpoint"] = (
-                "{}/{}".format(self.endpoint_baseurl, UserinfoEndpoint.url)
+                os.path.join(self.endpoint_baseurl, UserinfoEndpoint.url)
             )
             userinfo_endpoint = (
-                "^{}/{}".format(self.endpoint_basepath, UserinfoEndpoint.url), self.userinfo_endpoint
+                "^{}".format(
+                    os.path.join(self.endpoint_basepath, UserinfoEndpoint.url)
+                ),
+                self.userinfo_endpoint,
             )
             url_map.append(userinfo_endpoint)
 
         if "registration_endpoint" in self.provider.configuration_information:
             client_registration = (
-                "^{}/{}".format(self.endpoint_basepath, RegistrationEndpoint.url),
+                "^{}".format(
+                    os.path.join(self.endpoint_basepath, RegistrationEndpoint.url)
+                ),
                 self.client_registration,
             )
             url_map.append(client_registration)

tests/flows/test_oidc-saml.py

@@ -52,7 +52,6 @@ def oidc_frontend_config(signing_key_path):
         "module": "satosa.frontends.openid_connect.OpenIDConnectFrontend",
         "name": "OIDCFrontend",
         "config": {
-            "issuer": "https://proxy-op.example.com",
             "signing_key_path": signing_key_path,
             "provider": {"response_types_supported": ["id_token"]},
             "client_db_uri": DB_URI,  # use mongodb for integration testing
@@ -69,7 +68,6 @@ def oidc_stateless_frontend_config(signing_key_path, client_db_path):
         "module": "satosa.frontends.openid_connect.OpenIDConnectFrontend",
         "name": "OIDCFrontend",
         "config": {
-            "issuer": "https://proxy-op.example.com",
             "signing_key_path": signing_key_path,
             "client_db_path": client_db_path,
             "db_uri": "stateless://user:abc123@localhost",
@@ -94,6 +92,12 @@ def _client_setup(self):
             "response_types": ["id_token"]
         }
 
+    def _discover_provider(self, client, provider):
+        discovery_path = (
+            os.path.join(urlparse(provider).path, ".well-known/openid-configuration")
+        )
+        return json.loads(client.get(discovery_path).data.decode("utf-8"))
+
     def test_full_flow(self, satosa_config_dict, oidc_frontend_config, saml_backend_config, idp_conf):
         self._client_setup()
         subject_id = "testuser1"
@@ -110,7 +114,8 @@ def test_full_flow(self, satosa_config_dict, oidc_frontend_config, saml_backend_
         test_client = Client(make_app(SATOSAConfig(satosa_config_dict)), Response)
 
         # get frontend OP config info
-        provider_config = json.loads(test_client.get("/.well-known/openid-configuration").data.decode("utf-8"))
+        issuer = satosa_config_dict["BASE"]
+        provider_config = self._discover_provider(test_client, issuer)
 
         # create auth req
         claims_request = ClaimsRequest(id_token=Claims(**{k: None for k in USERS[subject_id]}))
@@ -168,7 +173,8 @@ def test_full_stateless_id_token_flow(self, satosa_config_dict, oidc_stateless_f
         test_client = Client(make_app(SATOSAConfig(satosa_config_dict)), Response)
 
         # get frontend OP config info
-        provider_config = json.loads(test_client.get("/.well-known/openid-configuration").data.decode("utf-8"))
+        issuer = satosa_config_dict["BASE"]
+        provider_config = self._discover_provider(test_client, issuer)
 
         # create auth req
         claims_request = ClaimsRequest(id_token=Claims(**{k: None for k in USERS[subject_id]}))
@@ -226,7 +232,8 @@ def test_full_stateless_code_flow(self, satosa_config_dict, oidc_stateless_front
         test_client = Client(make_app(SATOSAConfig(satosa_config_dict)), Response)
 
         # get frontend OP config info
-        provider_config = json.loads(test_client.get("/.well-known/openid-configuration").data.decode("utf-8"))
+        issuer = satosa_config_dict["BASE"]
+        provider_config = self._discover_provider(test_client, issuer)
 
         # create auth req
         claims_request = ClaimsRequest(id_token=Claims(**{k: None for k in USERS[subject_id]}))

tests/satosa/frontends/test_openid_connect.py

@@ -28,7 +28,7 @@
 INTERNAL_ATTRIBUTES = {
     "attributes": {"mail": {"saml": ["email"], "openid": ["email"]}}
 }
-BASE_URL = "https://op.example.com"
+BASE_URL = "https://op.example.com/satosa"
 CLIENT_ID = "client1"
 CLIENT_SECRET = "client_secret"
 EXTRA_CLAIMS = {
@@ -86,10 +86,10 @@ def frontend_config_with_extra_id_token_claims(self, signing_key_path):
 
         return config
 
-    def create_frontend(self, frontend_config):
+    def create_frontend(self, frontend_config, issuer=BASE_URL):
         # will use in-memory storage
         instance = OpenIDConnectFrontend(lambda ctx, req: None, INTERNAL_ATTRIBUTES,
-                                         frontend_config, BASE_URL, "oidc_frontend")
+                                         frontend_config, issuer, "oidc_frontend")
         instance.register_endpoints(["foo_backend"])
         return instance
 
@@ -369,19 +369,45 @@ def test_jwks(self, context, frontend):
         jwks = json.loads(http_response.message)
         assert jwks == {"keys": [frontend.signing_key.serialize()]}
 
-    def test_register_endpoints_token_and_userinfo_endpoint_is_published_if_necessary(self, frontend):
+    @pytest.mark.parametrize("issuer", [
+        "https://example.com",
+        "https://example.com/some/op",
+        "https://example.com/"
+    ])
+    def test_register_endpoints_handles_path_in_issuer(self, frontend_config, issuer):
+        frontend = self.create_frontend(frontend_config, issuer)
+        issuer_path = urlparse(issuer).path[1:]
+        if issuer_path:
+            issuer_path += "/"
         urls = frontend.register_endpoints(["test"])
-        assert ("^{}/{}".format(frontend.name, TokenEndpoint.url), frontend.token_endpoint) in urls
-        assert ("^{}/{}".format(frontend.name, UserinfoEndpoint.url), frontend.userinfo_endpoint) in urls
+        assert (
+            "^{}{}$".format(issuer_path, ".well-known/openid-configuration"),
+            frontend.provider_config,
+        ) in urls
+
+        assert (
+            "^{}/{}".format(frontend.endpoint_basepath, TokenEndpoint.url),
+            frontend.token_endpoint,
+        ) in urls
+        assert (
+            "^{}/{}".format(frontend.endpoint_basepath, UserinfoEndpoint.url),
+            frontend.userinfo_endpoint,
+        ) in urls
 
     def test_register_endpoints_token_and_userinfo_endpoint_is_not_published_if_only_implicit_flow(
             self, frontend_config, context):
         frontend_config["provider"]["response_types_supported"] = ["id_token", "id_token token"]
         frontend = self.create_frontend(frontend_config)
 
         urls = frontend.register_endpoints(["test"])
-        assert ("^{}/{}".format("test", TokenEndpoint.url), frontend.token_endpoint) not in urls
-        assert ("^{}/{}".format("test", UserinfoEndpoint.url), frontend.userinfo_endpoint) not in urls
+        assert (
+            "^{}/{}".format(frontend.endpoint_basepath, TokenEndpoint.url),
+            frontend.token_endpoint,
+        ) not in urls
+        assert (
+            "^{}/{}".format(frontend.endpoint_basepath, UserinfoEndpoint.url),
+            frontend.userinfo_endpoint,
+        ) not in urls
 
         http_response = frontend.provider_config(context)
         provider_config = ProviderConfigurationResponse().deserialize(http_response.message, "json")
@@ -397,8 +423,13 @@ def test_register_endpoints_dynamic_client_registration_is_configurable(
         frontend = self.create_frontend(frontend_config)
 
         urls = frontend.register_endpoints(["test"])
-        assert (("^{}/{}".format(frontend.name, RegistrationEndpoint.url),
-                 frontend.client_registration) in urls) == client_registration_enabled
+        assert (
+            (
+                "^{}/{}".format(frontend.endpoint_basepath, RegistrationEndpoint.url),
+                frontend.client_registration,
+            )
+            in urls
+        ) == client_registration_enabled
         provider_info = ProviderConfigurationResponse().deserialize(frontend.provider_config(None).message, "json")
         assert ("registration_endpoint" in provider_info) == client_registration_enabled