Merge pull request #34 from its-dirg/oidc-frontend-fix

Update OIDCFrontend

Author: RebeckaG

src/satosa/base.py

@@ -105,7 +105,6 @@ def _auth_req_callback_func(self, context, internal_request):
         context.state[consent.STATE_KEY] = {"filter": internal_request.approved_attributes or []}
         satosa_logging(logger, logging.INFO,
                        "Requesting provider: {}".format(internal_request.requester), state)
-        context.request = None
 
         UserIdHasher.save_state(internal_request, state)
         if self.request_micro_services:
@@ -115,6 +114,7 @@ def _auth_req_callback_func(self, context, internal_request):
 
     def _auth_req_finish(self, context, internal_request):
         backend = self.module_router.backend_routing(context)
+        context.request = None
         return backend.start_auth(context, internal_request)
 
     def _auth_resp_finish(self, context, internal_response):

src/satosa/frontends/openid_connect.py

@@ -3,7 +3,7 @@
 """
 import json
 import logging
-from urllib.parse import urlencode
+from urllib.parse import urlencode, urlparse
 
 from jwkest.jwk import rsa_load, RSAKey
 from oic.oic.message import (AuthorizationRequest, AuthorizationErrorResponse, TokenErrorResponse,
@@ -57,6 +57,7 @@ def _create_provider(self, endpoint_baseurl):
         capabilities = {
             "issuer": self.base_url,
             "authorization_endpoint": "{}/{}".format(endpoint_baseurl, AuthorizationEndpoint.url),
+            "jwks_uri": "{}/jwks".format(endpoint_baseurl),
             "response_types_supported": response_types_supported,
             "id_token_signing_alg_values_supported": [self.signing_key.alg],
             "response_modes_supported": ["fragment", "query"],
@@ -102,7 +103,7 @@ def _init_authorization_state(self):
         return AuthorizationState(HashBasedSubjectIdentifierFactory(sub_hash_salt), authz_code_db, access_token_db,
                                   refresh_token_db, sub_db, **token_lifetimes)
 
-    def handle_authn_response(self, context, internal_resp):
+    def handle_authn_response(self, context, internal_resp, extra_id_token_claims=None):
         """
         See super class method satosa.frontends.base.FrontendModule#handle_authn_response
         :type context: satosa.context.Context
@@ -114,7 +115,7 @@ def handle_authn_response(self, context, internal_resp):
 
         attributes = self.converter.from_internal("openid", internal_resp.attributes)
         self.user_db[internal_resp.user_id] = {k: v[0] for k, v in attributes.items()}
-        auth_resp = self.provider.authorize(auth_req, internal_resp.user_id)
+        auth_resp = self.provider.authorize(auth_req, internal_resp.user_id, extra_id_token_claims)
 
         del context.state[self.name]
         http_response = auth_resp.request(auth_req["redirect_uri"], should_fragment_encode(auth_req))
@@ -138,32 +139,46 @@ 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
         """
+        backend_name = None
         if len(backend_names) != 1:
             # only supports one backend since there currently is no way to publish multiple authorization endpoints
             # in configuration information and there is no other standard way of authorization_endpoint discovery
             # similar to SAML entity discovery
-            raise ValueError("OpenID Connect frontend only supports one backend.")
-        backend = backend_names[0]
-        endpoint_baseurl = "{}/{}".format(self.base_url, backend)
+            # this can be circumvented with a custom RequestMicroService which handles the routing based on something
+            # in the authentication request
+            logger.warn("More than one backend is configured, make sure to provide a custom routing micro service to "
+                        "determine which backend should be used per request.")
+        else:
+            backend_name = backend_names[0]
+
+        endpoint_baseurl = "{}/{}".format(self.base_url, self.name)
         self._create_provider(endpoint_baseurl)
 
         provider_config = ("^.well-known/openid-configuration$", self.provider_config)
-        jwks_uri = ("^jwks$", self.jwks)
-        authentication = ("^{}/{}".format(backend, AuthorizationEndpoint.url), self.handle_authn_request)
+        jwks_uri = ("^{}/jwks$".format(self.name), self.jwks)
+
+        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)
+            self.provider.configuration_information["authorization_endpoint"] = auth_endpoint
+            auth_path = urlparse(auth_endpoint).path.lstrip("/")
+        else:
+            auth_path = "{}/{}".format(self.name, AuthorizationEndpoint.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(endpoint_baseurl,
                                                                                        TokenEndpoint.url)
-            token_endpoint = ("^{}/{}".format(backend, TokenEndpoint.url), self.token_endpoint)
+            token_endpoint = ("^{}/{}".format(self.name, TokenEndpoint.url), self.token_endpoint)
             url_map.append(token_endpoint)
 
             self.provider.configuration_information["userinfo_endpoint"] = "{}/{}".format(endpoint_baseurl,
                                                                                           UserinfoEndpoint.url)
-            userinfo_endpoint = ("^{}/{}".format(backend, UserinfoEndpoint.url), self.userinfo_endpoint)
+            userinfo_endpoint = ("^{}/{}".format(self.name, UserinfoEndpoint.url), self.userinfo_endpoint)
             url_map.append(userinfo_endpoint)
         if "registration_endpoint" in self.provider.configuration_information:
-            client_registration = ("^{}/{}".format(backend, RegistrationEndpoint.url), self.client_registration)
+            client_registration = ("^{}/{}".format(self.name, RegistrationEndpoint.url), self.client_registration)
             url_map.append(client_registration)
 
         return url_map
@@ -247,8 +262,17 @@ def handle_authn_request(self, context):
         client_id = authn_req["client_id"]
         context.state[self.name] = {"oidc_request": request}
         hash_type = oidc_subject_type_to_hash_type(self.provider.clients[client_id].get("subject_type", "pairwise"))
-        internal_req = InternalRequest(hash_type, client_id, self.provider.clients[client_id].get("client_name"))
+        client_name = self.provider.clients[client_id].get("client_name")
+        if client_name:
+            # TODO should process client names for all languages, see OIDC Registration, Section 2.1
+            requester_name = [{"lang": "en", "text": client_name}]
+        else:
+            requester_name = None
+        internal_req = InternalRequest(hash_type, client_id, requester_name)
 
+        internal_req.approved_attributes = self.converter.to_internal_filter("openid",
+                                                                             self.provider.configuration_information[
+                                                                                 "claims_supported"])
         return self.auth_req_callback_func(context, internal_req)
 
     def jwks(self, context):

tests/satosa/frontends/test_openid_connect.py

@@ -21,7 +21,7 @@
 from tests.users import USERS
 
 INTERNAL_ATTRIBUTES = {
-    'attributes': {"mail": {"saml": ["email"], "openid": ["email"]}}
+    "attributes": {"mail": {"saml": ["email"], "openid": ["email"]}}
 }
 BASE_URL = "https://op.example.com"
 CLIENT_ID = "client1"
@@ -60,11 +60,12 @@ def authn_req(self):
                                    nonce=nonce, claims=claims_req)
         return req
 
-    def insert_client_in_client_db(self, frontend, redirect_uri):
+    def insert_client_in_client_db(self, frontend, redirect_uri, extra_metadata={}):
         frontend.provider.clients = {
             CLIENT_ID: {"response_types": ["code", "id_token"],
                         "redirect_uris": [redirect_uri],
                         "client_secret": CLIENT_SECRET}}
+        frontend.provider.clients[CLIENT_ID].update(extra_metadata)
 
     def insert_user_in_user_db(self, frontend, user_id):
         frontend.user_db[user_id] = {"email": "[email protected]"}
@@ -104,15 +105,18 @@ def test_handle_authn_response(self, context, frontend, authn_req):
     def test_handle_authn_request(self, context, frontend, authn_req):
         mock_callback = Mock()
         frontend.auth_req_callback_func = mock_callback
-        self.insert_client_in_client_db(frontend, authn_req["redirect_uri"])
+        client_name = "test client"
+        self.insert_client_in_client_db(frontend, authn_req["redirect_uri"], {"client_name": client_name})
 
         context.request = dict(parse_qsl(authn_req.to_urlencoded()))
         frontend.handle_authn_request(context)
 
         assert mock_callback.call_count == 1
         context, internal_req = mock_callback.call_args[0]
         assert internal_req.requester == authn_req["client_id"]
+        assert internal_req.requester_name == [{"lang": "en", "text": client_name}]
         assert internal_req.user_id_hash_type == UserIdHashType.pairwise
+        assert internal_req.approved_attributes == ["mail"]
 
     def test_handle_backend_error(self, context, frontend):
         redirect_uri = "https://client.example.com"
@@ -158,22 +162,23 @@ def test_register_client_with_wrong_response_type(self, context, frontend):
     def test_provider_configuration_endpoint(self, context, frontend):
         expected_capabilities = {
             "response_types_supported": ["code", "id_token", "code id_token token"],
-            "token_endpoint": BASE_URL + "/foo_backend/token",
+            "jwks_uri": "{}/{}/jwks".format(BASE_URL, frontend.name),
+            "authorization_endpoint": "{}/foo_backend/{}/authorization".format(BASE_URL, frontend.name),
+            "token_endpoint": "{}/{}/token".format(BASE_URL, frontend.name),
+            "userinfo_endpoint": "{}/{}/userinfo".format(BASE_URL, frontend.name),
             "id_token_signing_alg_values_supported": ["RS256"],
             "response_modes_supported": ["fragment", "query"],
             "subject_types_supported": ["pairwise"],
             "claim_types_supported": ["normal"],
             "claims_parameter_supported": True,
             "request_parameter_supported": False,
             "request_uri_parameter_supported": False,
-            "authorization_endpoint": "{}/foo_backend/authorization".format(BASE_URL),
             "scopes_supported": ["openid", "email"],
             "claims_supported": ["email"],
             "grant_types_supported": ["authorization_code", "implicit"],
             "issuer": BASE_URL,
             "require_request_uri_registration": True,
             "token_endpoint_auth_methods_supported": ["client_secret_basic"],
-            "userinfo_endpoint": "{}/foo_backend/userinfo".format(BASE_URL),
             "version": "3.0"
         }
 
@@ -189,8 +194,8 @@ def test_jwks(self, context, frontend):
 
     def test_register_endpoints_token_and_userinfo_endpoint_is_published_if_necessary(self, frontend):
         urls = frontend.register_endpoints(["test"])
-        assert ("^{}/{}".format("test", TokenEndpoint.url), frontend.token_endpoint) in urls
-        assert ("^{}/{}".format("test", UserinfoEndpoint.url), frontend.userinfo_endpoint) in urls
+        assert ("^{}/{}".format(frontend.name, TokenEndpoint.url), frontend.token_endpoint) in urls
+        assert ("^{}/{}".format(frontend.name, 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):
@@ -215,7 +220,7 @@ def test_register_endpoints_dynamic_client_registration_is_configurable(
         frontend = self.frontend(frontend_config)
 
         urls = frontend.register_endpoints(["test"])
-        assert (("^{}/{}".format("test", RegistrationEndpoint.url),
+        assert (("^{}/{}".format(frontend.name, 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