From bbc7929a60ca6e2a4f1dee5315ed7271caf7985a Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Tue, 31 Aug 2021 09:58:09 +0200 Subject: [PATCH 01/58] Client registration endpoint should return a 201 HTTP response code on successful registration. --- example/flask_op/views.py | 6 ++++-- src/oidcop/configure.py | 10 ++++++++-- src/oidcop/endpoint.py | 5 +++++ src/oidcop/oidc/registration.py | 2 +- src/oidcop/token/__init__.py | 5 ++--- tests/test_23_oidc_registration_endpoint.py | 3 +++ 6 files changed, 23 insertions(+), 8 deletions(-) diff --git a/example/flask_op/views.py b/example/flask_op/views.py index a10d41fc..81b82674 100644 --- a/example/flask_op/views.py +++ b/example/flask_op/views.py @@ -78,14 +78,16 @@ def do_response(endpoint, req_args, error='', **args): if error: if _response_placement == 'body': _log.info('Error Response: {}'.format(info['response'])) - resp = make_response(info['response'], 400) + _http_response_code = info.get('response_code', 400) + resp = make_response(info['response'], _http_response_code) else: # _response_placement == 'url': _log.info('Redirect to: {}'.format(info['response'])) resp = redirect(info['response']) else: if _response_placement == 'body': _log.info('Response: {}'.format(info['response'])) - resp = make_response(info['response'], 200) + _http_response_code = info.get('response_code', 200) + resp = make_response(info['response'], _http_response_code) else: # _response_placement == 'url': _log.info('Redirect to: {}'.format(info['response'])) resp = redirect(info['response']) diff --git a/src/oidcop/configure.py b/src/oidcop/configure.py index 042593fa..87591022 100755 --- a/src/oidcop/configure.py +++ b/src/oidcop/configure.py @@ -59,7 +59,10 @@ "max_usage": 1, }, "access_token": {}, - "refresh_token": {"supports_minting": ["access_token", "refresh_token"]}, + "refresh_token": { + "supports_minting": ["access_token", "refresh_token"], + "expires_in": -1 + }, }, "expires_in": 43200, } @@ -380,7 +383,10 @@ def __init__( "max_usage": 1, }, "access_token": {}, - "refresh_token": {"supports_minting": ["access_token", "refresh_token"]}, + "refresh_token": { + "supports_minting": ["access_token", "refresh_token"], + "expires_in": -1 + }, }, "expires_in": 43200, } diff --git a/src/oidcop/endpoint.py b/src/oidcop/endpoint.py index 3152cbb3..2e0c0d12 100755 --- a/src/oidcop/endpoint.py +++ b/src/oidcop/endpoint.py @@ -405,6 +405,11 @@ def do_response( except KeyError: pass + try: + _resp["response_code"] = kwargs["response_code"] + except KeyError: + pass + return _resp def allowed_target_uris(self): diff --git a/src/oidcop/oidc/registration.py b/src/oidcop/oidc/registration.py index 71aa7374..cc67dcd1 100755 --- a/src/oidcop/oidc/registration.py +++ b/src/oidcop/oidc/registration.py @@ -473,4 +473,4 @@ def process_request(self, request=None, new_id=True, set_secret=True, **kwargs): name=_context.cookie_handler.name["register"], client_id=reg_resp["client_id"], ) - return {"response_args": reg_resp, "cookie": _cookie} + return {"response_args": reg_resp, "cookie": _cookie, "response_code": 201} diff --git a/src/oidcop/token/__init__.py b/src/oidcop/token/__init__.py index a9bcd791..9134bf55 100755 --- a/src/oidcop/token/__init__.py +++ b/src/oidcop/token/__init__.py @@ -54,11 +54,10 @@ def __call__(self, session_id: Optional[str] = "", ttype: Optional[str] = "", ** def info(self, token): """ - Return type of Token (A=Access code, T=Token, R=Refresh token) and - the session id. + Return dictionary with token information. :param token: A token - :return: tuple of token type and session id + :return: Dictionary with information about the token """ raise NotImplementedError() diff --git a/tests/test_23_oidc_registration_endpoint.py b/tests/test_23_oidc_registration_endpoint.py index eb2c23f2..f55e2bc8 100755 --- a/tests/test_23_oidc_registration_endpoint.py +++ b/tests/test_23_oidc_registration_endpoint.py @@ -178,6 +178,8 @@ def test_process_request(self): _reg_resp = _resp["response_args"] assert isinstance(_reg_resp, RegistrationResponse) assert "client_id" in _reg_resp and "client_secret" in _reg_resp + assert "response_code" in _resp + assert _resp["response_code"] == 201 def test_do_response(self): _req = self.endpoint.parse_request(CLI_REQ.to_json()) @@ -194,6 +196,7 @@ def test_do_response(self): assert isinstance(msg, dict) _msg = json.loads(msg["response"]) assert _msg + assert "response_code" in msg def test_register_unsupported_algo(self): _msg = MSG.copy() From 178ca74b5334cedf19deda0266d0e73bb25fd384 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Wed, 1 Sep 2021 11:16:37 +0200 Subject: [PATCH 02/58] Default token lifetime should not be 0 (zero). Changed to be 30 minutes (1800 seconds). --- src/oidcop/constant.py | 2 ++ src/oidcop/oauth2/token.py | 3 ++- src/oidcop/token/jwt_token.py | 6 ++---- tests/test_35_oidc_token_endpoint.py | 1 + 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/oidcop/constant.py b/src/oidcop/constant.py index c0527b74..99a0f06d 100644 --- a/src/oidcop/constant.py +++ b/src/oidcop/constant.py @@ -1 +1,3 @@ DIVIDER = ";;" + +DEFAULT_TOKEN_LIFETIME = 1800 diff --git a/src/oidcop/oauth2/token.py b/src/oidcop/oauth2/token.py index e84ea887..75c4efc8 100755 --- a/src/oidcop/oauth2/token.py +++ b/src/oidcop/oauth2/token.py @@ -13,6 +13,7 @@ from oidcmsg.time_util import time_sans_frac from oidcop import sanitize +from oidcop.constant import DEFAULT_TOKEN_LIFETIME from oidcop.endpoint import Endpoint from oidcop.exception import ProcessError from oidcop.session.grant import AuthorizationCode @@ -62,7 +63,7 @@ def _mint_token( if usage_rules: _exp_in = usage_rules.get("expires_in") else: - _exp_in = 0 + _exp_in = DEFAULT_TOKEN_LIFETIME token_args = token_args or {} for meth in _context.token_args_methods: diff --git a/src/oidcop/token/jwt_token.py b/src/oidcop/token/jwt_token.py index 1329f64d..b24d5238 100644 --- a/src/oidcop/token/jwt_token.py +++ b/src/oidcop/token/jwt_token.py @@ -7,12 +7,10 @@ from oidcop.exception import ToOld from oidcop.token import Crypt from oidcop.token.exception import WrongTokenClass - from . import Token from . import is_expired from .exception import UnknownToken - -# TYPE_MAP = {"A": "code", "T": "access_token", "R": "refresh_token"} +from ..constant import DEFAULT_TOKEN_LIFETIME class JWTToken(Token): @@ -23,7 +21,7 @@ def __init__( issuer: str = None, aud: Optional[list] = None, alg: str = "ES256", - lifetime: int = 300, + lifetime: int = DEFAULT_TOKEN_LIFETIME, server_get: Callable = None, token_type: str = "Bearer", password: str = "", diff --git a/tests/test_35_oidc_token_endpoint.py b/tests/test_35_oidc_token_endpoint.py index 908966bb..918a8e0a 100755 --- a/tests/test_35_oidc_token_endpoint.py +++ b/tests/test_35_oidc_token_endpoint.py @@ -283,6 +283,7 @@ def test_process_request(self): assert _resp assert set(_resp.keys()) == {"cookie", "http_headers", "response_args"} + assert "expires_in" in _resp["response_args"] def test_process_request_using_code_twice(self): session_id = self._create_session(AUTH_REQ) From ce142a830918026a1e1c51b053965cc28f8dcd24 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 2 Sep 2021 09:57:33 +0200 Subject: [PATCH 03/58] Userinfo endpoint should support POST. --- example/flask_op/views.py | 10 +++++++--- src/oidcop/oidc/userinfo.py | 2 +- tests/test_26_oidc_userinfo_endpoint.py | 19 ++++++++++++++++++- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/example/flask_op/views.py b/example/flask_op/views.py index 81b82674..82e4f679 100644 --- a/example/flask_op/views.py +++ b/example/flask_op/views.py @@ -168,10 +168,14 @@ def registration(): current_app.server.server_get("endpoint", 'registration')) -@oidc_op_views.route('/registration_api', methods=['GET']) +@oidc_op_views.route('/registration_api', methods=['GET', 'DELETE']) def registration_api(): - return service_endpoint( - current_app.server.server_get("endpoint", 'registration_read')) + if request.method == "DELETE": + return service_endpoint( + current_app.server.server_get("endpoint", 'registration_delete')) + else: + return service_endpoint( + current_app.server.server_get("endpoint", 'registration_read')) @oidc_op_views.route('/authorization') diff --git a/src/oidcop/oidc/userinfo.py b/src/oidcop/oidc/userinfo.py index 1b514c01..aabc94d8 100755 --- a/src/oidcop/oidc/userinfo.py +++ b/src/oidcop/oidc/userinfo.py @@ -32,7 +32,7 @@ class UserInfo(Endpoint): "userinfo_signing_alg_values_supported": None, "userinfo_encryption_alg_values_supported": None, "userinfo_encryption_enc_values_supported": None, - "client_authn_method": ["bearer_header"], + "client_authn_method": ["bearer_header", "bearer_body"], } def __init__(self, server_get: Callable, add_claims_by_scope: Optional[bool] = True, **kwargs): diff --git a/tests/test_26_oidc_userinfo_endpoint.py b/tests/test_26_oidc_userinfo_endpoint.py index 41ebc65c..4a270d3b 100755 --- a/tests/test_26_oidc_userinfo_endpoint.py +++ b/tests/test_26_oidc_userinfo_endpoint.py @@ -124,7 +124,7 @@ def create_endpoint(self): "class": userinfo.UserInfo, "kwargs": { "claim_types_supported": ["normal", "aggregated", "distributed", ], - "client_authn_method": ["bearer_header"], + "client_authn_method": ["bearer_header", "bearer_body"], }, }, }, @@ -439,3 +439,20 @@ def test_userinfo_claims_acr_none(self): res = self.endpoint.do_response(request=_req, **args) _response = json.loads(res["response"]) assert _response["acr"] == _acr + + def test_userinfo_claims_post(self): + _acr = "https://refeds.org/profile/mfa" + _auth_req = AUTH_REQ.copy() + _auth_req["claims"] = {"userinfo": {"acr": {"value": _acr}}} + + session_id = self._create_session(_auth_req, authn_info=_acr) + grant = self.session_manager[session_id] + code = self._mint_code(grant, session_id) + access_token = self._mint_token("access_token", grant, session_id, code) + + _req = self.endpoint.parse_request({"access_token": access_token.value}) + args = self.endpoint.process_request(_req) + assert args + res = self.endpoint.do_response(request=_req, **args) + _response = json.loads(res["response"]) + assert _response["acr"] == _acr From 6f87892779025dfaba1e5dfeb38d8a8e60be22d6 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 2 Sep 2021 11:05:35 +0200 Subject: [PATCH 04/58] Authorization error response MUST contain 'state' if it is present in the request. --- src/oidcop/oauth2/authorization.py | 134 +++++++++++-------- tests/test_24_oidc_authorization_endpoint.py | 2 +- 2 files changed, 80 insertions(+), 56 deletions(-) diff --git a/src/oidcop/oauth2/authorization.py b/src/oidcop/oauth2/authorization.py index 2a68230e..6a0a3917 100755 --- a/src/oidcop/oauth2/authorization.py +++ b/src/oidcop/oauth2/authorization.py @@ -45,9 +45,11 @@ # For the time being. This is JAR specific and should probably be configurable. ALG_PARAMS = { - "sign": ["request_object_signing_alg", "request_object_signing_alg_values_supported",], - "enc_alg": ["request_object_encryption_alg", "request_object_encryption_alg_values_supported",], - "enc_enc": ["request_object_encryption_enc", "request_object_encryption_enc_values_supported",], + "sign": ["request_object_signing_alg", "request_object_signing_alg_values_supported", ], + "enc_alg": ["request_object_encryption_alg", + "request_object_encryption_alg_values_supported", ], + "enc_enc": ["request_object_encryption_enc", + "request_object_encryption_enc_values_supported", ], } FORM_POST = """ @@ -79,10 +81,10 @@ def max_age(request): def verify_uri( - endpoint_context: EndpointContext, - request: Union[dict, Message], - uri_type: str, - client_id: Optional[str] = None, + endpoint_context: EndpointContext, + request: Union[dict, Message], + uri_type: str, + client_id: Optional[str] = None, ): """ A redirect URI @@ -207,7 +209,7 @@ def get_uri(endpoint_context, request, uri_type): def authn_args_gather( - request: Union[AuthorizationRequest, dict], authn_class_ref: str, cinfo: dict, **kwargs, + request: Union[AuthorizationRequest, dict], authn_class_ref: str, cinfo: dict, **kwargs, ): """ Gather information to be used by the authentication method @@ -291,6 +293,13 @@ def filter_request(self, endpoint_context, req): def extra_response_args(self, aresp): return aresp + def authentication_error_response(self, request, error, error_description, **kwargs): + _error_msg = self.error_cls(error=error, error_description=error_description) + _state = request.get("state") + if _state: + _error_msg["state"] = _state + return _error_msg + def verify_response_type(self, request: Union[Message, dict], cinfo: dict) -> bool: # Checking response types _registered = [set(rt.split(" ")) for rt in cinfo.get("response_types", [])] @@ -392,20 +401,23 @@ def _post_parse_request(self, request, client_id, endpoint_context, **kwargs): """ if not request: logger.debug("No AuthzRequest") - return self.error_cls( - error="invalid_request", error_description="Can not parse AuthzRequest" - ) + return self.authentication_error_response(request, + error="invalid_request", + error_description="Can not parse AuthzRequest" + ) request = self.filter_request(endpoint_context, request) _cinfo = endpoint_context.cdb.get(client_id) if not _cinfo: logger.error("Client ID ({}) not in client database".format(request["client_id"])) - return self.error_cls(error="unauthorized_client", error_description="unknown client") + return self.authentication_error_response(request, error="unauthorized_client", + error_description="unknown client") # Is the asked for response_type among those that are permitted if not self.verify_response_type(request, _cinfo): - return self.error_cls( + return self.authentication_error_response( + request, error="invalid_request", error_description="Trying to use unregistered response_type", ) @@ -414,7 +426,8 @@ def _post_parse_request(self, request, client_id, endpoint_context, **kwargs): try: redirect_uri = get_uri(endpoint_context, request, "redirect_uri") except (RedirectURIError, ParameterError) as err: - return self.error_cls( + return self.authentication_error_response( + request, error="invalid_request", error_description="{}:{}".format(err.__class__.__name__, err), ) @@ -452,7 +465,7 @@ def pick_authn_method(self, request, redirect_uri, acr=None, **kwargs): def create_session(self, request, user_id, acr, time_stamp, authn_method): _context = self.server_get("endpoint_context") _mngr = _context.session_manager - authn_event = create_authn_event(user_id, authn_info=acr, time_stamp=time_stamp,) + authn_event = create_authn_event(user_id, authn_info=acr, time_stamp=time_stamp, ) _exp_in = authn_method.kwargs.get("expires_in") if _exp_in and "valid_until" in authn_event: authn_event["valid_until"] = utc_time_sans_frac() + _exp_in @@ -466,14 +479,25 @@ def create_session(self, request, user_id, acr, time_stamp, authn_method): token_usage_rules=_token_usage_rules, ) + def _login_required_error(self, redirect_uri, request): + _res = { + "error": "login_required", + "return_uri": redirect_uri, + "return_type": request["response_type"], + } + _state = request.get("state") + if _state: + _res["state"] = _state + return _res + def setup_auth( - self, - request: Optional[Union[Message, dict]], - redirect_uri: str, - cinfo: dict, - cookie: List[dict] = None, - acr: str = None, - **kwargs, + self, + request: Optional[Union[Message, dict]], + redirect_uri: str, + cinfo: dict, + cookie: List[dict] = None, + acr: str = None, + **kwargs, ): """ @@ -541,11 +565,7 @@ def setup_auth( if "prompt" in request and "none" in request["prompt"]: # Need to authenticate but not allowed - return { - "error": "login_required", - "return_uri": redirect_uri, - "return_type": request["response_type"], - } + return self._login_required_error(redirect_uri, request) else: return {"function": authn, "args": authn_args} else: @@ -560,11 +580,7 @@ def setup_auth( if user != kwargs["req_user"]: logger.debug("Wanted to be someone else!") if "prompt" in request and "none" in request["prompt"]: - # Need to authenticate but not allowed - return { - "error": "login_required", - "return_uri": redirect_uri, - } + return self._login_required_error(redirect_uri, request) else: return {"function": authn, "args": authn_args} @@ -605,12 +621,12 @@ def aresp_check(self, aresp, request): return "" def response_mode( - self, - request: Union[dict, AuthorizationRequest], - response_args: Optional[AuthorizationResponse] = None, - return_uri: Optional[str] = "", - fragment_enc: Optional[bool] = None, - **kwargs, + self, + request: Union[dict, AuthorizationRequest], + response_args: Optional[AuthorizationResponse] = None, + return_uri: Optional[str] = "", + fragment_enc: Optional[bool] = None, + **kwargs, ) -> dict: resp_mode = request["response_mode"] if resp_mode == "form_post": @@ -618,9 +634,9 @@ def response_mode( _args = response_args.to_dict() else: _args = response_args - msg = FORM_POST.format(inputs=inputs(_args), action=return_uri,) + msg = FORM_POST.format(inputs=inputs(_args), action=return_uri, ) kwargs.update( - {"response_msg": msg, "content_type": "text/html", "response_placement": "body",} + {"response_msg": msg, "content_type": "text/html", "response_placement": "body", } ) elif resp_mode == "fragment": if fragment_enc is False: @@ -640,8 +656,10 @@ def response_mode( return kwargs - def error_response(self, response_info, error, error_description): - resp = self.error_cls(error=error, error_description=str(error_description)) + def error_response(self, response_info, request, error, error_description): + resp = self.authentication_error_response(request, + error=error, + error_description=str(error_description)) response_info["response_args"] = resp return response_info @@ -715,7 +733,8 @@ def create_authn_response(self, request: Union[dict, Message], sid: str) -> dict # id_token = _context.idtoken.make(sid, **kwargs) except (JWEException, NoSuitableSigningKeys) as err: logger.warning(str(err)) - resp = self.error_cls( + resp = self.authentication_error_response( + request, error="invalid_request", error_description="Could not sign/encrypt id_token", ) @@ -726,7 +745,8 @@ def create_authn_response(self, request: Union[dict, Message], sid: str) -> dict not_handled = rtype.difference(handled_response_type) if not_handled: - resp = self.error_cls( + resp = self.authentication_error_response( + request, error="invalid_request", error_description="unsupported_response_type", ) return {"response_args": resp, "fragment_enc": fragment_enc} @@ -753,13 +773,14 @@ def post_authentication(self, request: Union[dict, Message], session_id: str, ** grant = _context.authz(session_id, request=request) if grant.is_active() is False: - return self.error_response(response_info, "server_error", "Grant not usable") + return self.error_response(response_info, request, "server_error", "Grant not usable") user_id, client_id, grant_id = _mngr.decrypt_session_id(session_id) try: _mngr.set([user_id, client_id, grant_id], grant) except Exception as err: - return self.error_response(response_info, "server_error", "{}".format(err.args)) + return self.error_response(response_info, request, "server_error", + "{}".format(err.args)) logger.debug("response type: %s" % request["response_type"]) @@ -771,7 +792,8 @@ def post_authentication(self, request: Union[dict, Message], session_id: str, ** try: redirect_uri = get_uri(_context, request, "redirect_uri") except (RedirectURIError, ParameterError) as err: - return self.error_response(response_info, "invalid_request", "{}".format(err.args)) + return self.error_response(response_info, request, "invalid_request", + "{}".format(err.args)) else: response_info["return_uri"] = redirect_uri @@ -783,7 +805,8 @@ def post_authentication(self, request: Union[dict, Message], session_id: str, ** try: response_info = self.response_mode(request, **response_info) except InvalidRequest as err: - return self.error_response(response_info, "invalid_request", "{}".format(err.args)) + return self.error_response(response_info, request, "invalid_request", + "{}".format(err.args)) _cookie_info = _context.new_cookie( name=_context.cookie_handler.name["session"], @@ -807,7 +830,7 @@ def authz_part2(self, request, session_id, **kwargs): try: resp_info = self.post_authentication(request, session_id, **kwargs) except Exception as err: - return self.error_response({}, "server_error", err) + return self.error_response({}, request, "server_error", err) _context = self.server_get("endpoint_context") @@ -816,10 +839,11 @@ def authz_part2(self, request, session_id, **kwargs): try: authn_event = _context.session_manager.get_authentication_event(session_id) except KeyError: - return self.error_response({}, "server_error", "No such session") + return self.error_response({}, request, "server_error", "No such session") else: if authn_event.is_valid() is False: - return self.error_response({}, "server_error", "Authentication has timed out") + return self.error_response({}, request, "server_error", + "Authentication has timed out") _state = b64e(as_bytes(json.dumps({"authn_time": authn_event["authn_time"]}))) @@ -859,10 +883,10 @@ def do_request_user(self, request_info, **kwargs): return kwargs def process_request( - self, - request: Optional[Union[Message, dict]] = None, - http_info: Optional[dict] = None, - **kwargs, + self, + request: Optional[Union[Message, dict]] = None, + http_info: Optional[dict] = None, + **kwargs, ): """ The AuthorizationRequest endpoint diff --git a/tests/test_24_oidc_authorization_endpoint.py b/tests/test_24_oidc_authorization_endpoint.py index fa671abd..c877a069 100755 --- a/tests/test_24_oidc_authorization_endpoint.py +++ b/tests/test_24_oidc_authorization_endpoint.py @@ -894,7 +894,7 @@ def test_do_request_uri(self): def test_post_parse_request(self): endpoint_context = self.endpoint.server_get("endpoint_context") - msg = self.endpoint._post_parse_request(None, "client_1", endpoint_context) + msg = self.endpoint._post_parse_request({}, "client_1", endpoint_context) assert "error" in msg request = AuthorizationRequest( From 32640c168a442f1578d33ab9b32c54d63a695510 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 2 Sep 2021 11:42:11 +0200 Subject: [PATCH 05/58] Authorization error response MUST contain 'state' if it is present in the request. --- src/oidcop/endpoint.py | 7 +++---- src/oidcop/oauth2/authorization.py | 1 + 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/oidcop/endpoint.py b/src/oidcop/endpoint.py index 2e0c0d12..fcb0f7f2 100755 --- a/src/oidcop/endpoint.py +++ b/src/oidcop/endpoint.py @@ -330,10 +330,9 @@ def do_response( resp = None if error: _response = ResponseMessage(error=error) - try: - _response["error_description"] = kwargs["error_description"] - except KeyError: - pass + for attr in ["error_description", "error_uri", "state"]: + if attr in kwargs: + _response[attr] = kwargs[attr] elif "response_msg" in kwargs: resp = kwargs["response_msg"] _response_placement = kwargs.get("response_placement") diff --git a/src/oidcop/oauth2/authorization.py b/src/oidcop/oauth2/authorization.py index 6a0a3917..237cbfe4 100755 --- a/src/oidcop/oauth2/authorization.py +++ b/src/oidcop/oauth2/authorization.py @@ -488,6 +488,7 @@ def _login_required_error(self, redirect_uri, request): _state = request.get("state") if _state: _res["state"] = _state + logger.debug("Login required error: {}".format(_res)) return _res def setup_auth( From a15874160cf499bc7958007693a156a701d82624 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 2 Sep 2021 15:58:46 +0200 Subject: [PATCH 06/58] Cookie handling - bug, wrong name. --- src/oidcop/oauth2/authorization.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/oidcop/oauth2/authorization.py b/src/oidcop/oauth2/authorization.py index 237cbfe4..f46bafe7 100755 --- a/src/oidcop/oauth2/authorization.py +++ b/src/oidcop/oauth2/authorization.py @@ -102,8 +102,6 @@ def verify_uri( if not _cid: logger.error("No client id found") raise UnknownClient("No client_id provided") - else: - logger.debug("Client ID: {}".format(_cid)) _uri = request.get(uri_type) if _uri is None: @@ -124,7 +122,6 @@ def verify_uri( if client_info is None: raise KeyError("No such client") - logger.debug("Client info: {}".format(client_info)) redirect_uris = client_info.get("{}s".format(uri_type)) if redirect_uris is None: raise ValueError(f"No registered {uri_type} for {_cid}") @@ -902,7 +899,7 @@ def process_request( _cid = request["client_id"] _context = self.server_get("endpoint_context") cinfo = _context.cdb[_cid] - logger.debug("client {}: {}".format(_cid, cinfo)) + # logger.debug("client {}: {}".format(_cid, cinfo)) # this apply the default optionally deny_unknown_scopes policy if cinfo: @@ -913,7 +910,8 @@ def process_request( _cookies = http_info.get("cookie") if _cookies: - _cookies = _context.cookie_handler.parse_cookie("oidcop", _cookies) + _session_cookie_name = _context.cookie_handler.name["session"] + _cookies = _context.cookie_handler.parse_cookie(_session_cookie_name, _cookies) kwargs = self.do_request_user(request_info=request, **kwargs) From e7303bc31a54ef09aa5a763344204cfc2a277c56 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 2 Sep 2021 16:07:56 +0200 Subject: [PATCH 07/58] Cookie handling - bug, wrong name. --- src/oidcop/cookie_handler.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/oidcop/cookie_handler.py b/src/oidcop/cookie_handler.py index b36a0c33..262dfaae 100755 --- a/src/oidcop/cookie_handler.py +++ b/src/oidcop/cookie_handler.py @@ -167,6 +167,7 @@ def _ver_dec_content(self, parts): try: msg = decrypter.decrypt(ciphertext, iv, tag=tag) except InvalidTag: + LOGGER.debug("Decryption failed") return None p = lv_unpack(msg.decode("utf-8")) @@ -180,6 +181,8 @@ def _ver_dec_content(self, parts): self.sign_key.key, ): return payload, timestamp + else: + LOGGER.debug("Could not verify signature") else: return payload, timestamp return None @@ -250,9 +253,13 @@ def parse_cookie(self, name: str, cookies: List[dict]) -> Optional[List[dict]]: res = [] for _cookie in cookies: if _cookie["name"] == name: - payload, timestamp = self._ver_dec_content(_cookie["value"].split("|")) - value, typ = payload.split("::") - res.append({"value": value, "type": typ, "timestamp": timestamp}) + _content = self._ver_dec_content(_cookie["value"].split("|")) + if _content: + payload, timestamp = self._ver_dec_content(_cookie["value"].split("|")) + value, typ = payload.split("::") + res.append({"value": value, "type": typ, "timestamp": timestamp}) + else: + LOGGER.debug(f"Could not verify {name} cookie") return res From f2b8ea2e2658e62fe52b15e5155cc67ca40d68e7 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 2 Sep 2021 16:10:43 +0200 Subject: [PATCH 08/58] Cookie handling - bug, wrong name. --- src/oidcop/cookie_handler.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/oidcop/cookie_handler.py b/src/oidcop/cookie_handler.py index 262dfaae..0c456f4b 100755 --- a/src/oidcop/cookie_handler.py +++ b/src/oidcop/cookie_handler.py @@ -252,6 +252,7 @@ def parse_cookie(self, name: str, cookies: List[dict]) -> Optional[List[dict]]: res = [] for _cookie in cookies: + LOGGER.debug('Cookie: {}'.format(_cookie)) if _cookie["name"] == name: _content = self._ver_dec_content(_cookie["value"].split("|")) if _content: From 0003078b2c5aa30b9dfc0f0f3f51ee2bf28acfbf Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 2 Sep 2021 16:15:54 +0200 Subject: [PATCH 09/58] Cookie handling - bug, wrong name. --- src/oidcop/cookie_handler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/oidcop/cookie_handler.py b/src/oidcop/cookie_handler.py index 0c456f4b..3500fe3f 100755 --- a/src/oidcop/cookie_handler.py +++ b/src/oidcop/cookie_handler.py @@ -253,7 +253,7 @@ def parse_cookie(self, name: str, cookies: List[dict]) -> Optional[List[dict]]: res = [] for _cookie in cookies: LOGGER.debug('Cookie: {}'.format(_cookie)) - if _cookie["name"] == name: + if "name" in _cookie and _cookie["name"] == name: _content = self._ver_dec_content(_cookie["value"].split("|")) if _content: payload, timestamp = self._ver_dec_content(_cookie["value"].split("|")) From fcfd63bdf76cbadcdd4b8e2e6157426ae83617fe Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 2 Sep 2021 16:18:36 +0200 Subject: [PATCH 10/58] Cookie handling - bug, wrong name. --- src/oidcop/cookie_handler.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/oidcop/cookie_handler.py b/src/oidcop/cookie_handler.py index 3500fe3f..7812f825 100755 --- a/src/oidcop/cookie_handler.py +++ b/src/oidcop/cookie_handler.py @@ -250,6 +250,7 @@ def parse_cookie(self, name: str, cookies: List[dict]) -> Optional[List[dict]]: if not cookies: return None + LOGGER.debug("Looking for '{}' cookies".format(name)) res = [] for _cookie in cookies: LOGGER.debug('Cookie: {}'.format(_cookie)) From 66594599e540bf1205de8a3eb6364cabe8878e34 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 2 Sep 2021 16:37:23 +0200 Subject: [PATCH 11/58] Cookie handling - bug, wrong name. --- src/oidcop/endpoint.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/oidcop/endpoint.py b/src/oidcop/endpoint.py index fcb0f7f2..48abd454 100755 --- a/src/oidcop/endpoint.py +++ b/src/oidcop/endpoint.py @@ -128,9 +128,9 @@ def __init__(self, server_get: Callable, **kwargs): self.allowed_targets = [self.name] self.client_verification_method = [] - def parse_cookies(self, cookies: List[dict], context: EndpointContext, name: str): - res = context.cookie_handler.parse_cookie(name, cookies) - return res + # def parse_cookies(self, cookies: List[dict], context: EndpointContext, name: str): + # res = context.cookie_handler.parse_cookie(name, cookies) + # return res def parse_request( self, request: Union[Message, dict, str], http_info: Optional[dict] = None, **kwargs From 71c77b603ba7fc20c9da8a6b290177425027117c Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 2 Sep 2021 16:45:15 +0200 Subject: [PATCH 12/58] Cookie handling - bug, wrong name. --- src/oidcop/endpoint.py | 4 ---- src/oidcop/oauth2/authorization.py | 9 ++++++--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/oidcop/endpoint.py b/src/oidcop/endpoint.py index 48abd454..f5ea2bf8 100755 --- a/src/oidcop/endpoint.py +++ b/src/oidcop/endpoint.py @@ -128,10 +128,6 @@ def __init__(self, server_get: Callable, **kwargs): self.allowed_targets = [self.name] self.client_verification_method = [] - # def parse_cookies(self, cookies: List[dict], context: EndpointContext, name: str): - # res = context.cookie_handler.parse_cookie(name, cookies) - # return res - def parse_request( self, request: Union[Message, dict, str], http_info: Optional[dict] = None, **kwargs ): diff --git a/src/oidcop/oauth2/authorization.py b/src/oidcop/oauth2/authorization.py index f46bafe7..8ed4c880 100755 --- a/src/oidcop/oauth2/authorization.py +++ b/src/oidcop/oauth2/authorization.py @@ -910,12 +910,15 @@ def process_request( _cookies = http_info.get("cookie") if _cookies: + logger.debug("parse_cookie@process_request") _session_cookie_name = _context.cookie_handler.name["session"] - _cookies = _context.cookie_handler.parse_cookie(_session_cookie_name, _cookies) + _my_cookies = _context.cookie_handler.parse_cookie(_session_cookie_name, _cookies) + else: + _my_cookies = {} kwargs = self.do_request_user(request_info=request, **kwargs) - info = self.setup_auth(request, request["redirect_uri"], cinfo, _cookies, **kwargs) + info = self.setup_auth(request, request["redirect_uri"], cinfo, _my_cookies, **kwargs) if "error" in info: return info @@ -924,7 +927,7 @@ def process_request( if not _function: logger.debug("- authenticated -") logger.debug("AREQ keys: %s" % request.keys()) - return self.authz_part2(request=request, cookie=_cookies, **info) + return self.authz_part2(request=request, cookie=_my_cookies, **info) try: # Run the authentication function From 5cfa28906fb9cd7f7a4ea781df8256a79a7ac77b Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 2 Sep 2021 16:51:45 +0200 Subject: [PATCH 13/58] parse_cookie twice. --- src/oidcop/oidc/session.py | 1 + src/oidcop/user_authn/user.py | 1 + 2 files changed, 2 insertions(+) diff --git a/src/oidcop/oidc/session.py b/src/oidcop/oidc/session.py index b3db0e42..4e35141e 100644 --- a/src/oidcop/oidc/session.py +++ b/src/oidcop/oidc/session.py @@ -240,6 +240,7 @@ def process_request( _session_info = None if _cookies: + logger.debug("parse_cookie@session") _cookie_name = _context.cookie_handler.name["session"] try: _cookie_infos = _context.cookie_handler.parse_cookie( diff --git a/src/oidcop/user_authn/user.py b/src/oidcop/user_authn/user.py index b8baba08..b5e1d611 100755 --- a/src/oidcop/user_authn/user.py +++ b/src/oidcop/user_authn/user.py @@ -93,6 +93,7 @@ def done(self, areq): def cookie_info(self, cookie: List[dict], client_id: str) -> dict: _context = self.server_get("endpoint_context") try: + logger.debug("parse_cookie@UserAuthnMethod") vals = _context.cookie_handler.parse_cookie( cookies=cookie, name=_context.cookie_handler.name["session"] ) From f384d202d6c880db2b6b3ee1084359d6b64f8534 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 2 Sep 2021 16:57:34 +0200 Subject: [PATCH 14/58] parse_cookie twice. --- src/oidcop/user_authn/user.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/oidcop/user_authn/user.py b/src/oidcop/user_authn/user.py index b5e1d611..dd291d3d 100755 --- a/src/oidcop/user_authn/user.py +++ b/src/oidcop/user_authn/user.py @@ -92,14 +92,17 @@ def done(self, areq): def cookie_info(self, cookie: List[dict], client_id: str) -> dict: _context = self.server_get("endpoint_context") - try: - logger.debug("parse_cookie@UserAuthnMethod") - vals = _context.cookie_handler.parse_cookie( - cookies=cookie, name=_context.cookie_handler.name["session"] - ) - except (InvalidCookieSign, AssertionError, AttributeError) as err: - logger.warning(err) - vals = None + if cookie and "value" in cookie[0]: + vals = cookie + else: + try: + logger.debug("parse_cookie@UserAuthnMethod") + vals = _context.cookie_handler.parse_cookie( + cookies=cookie, name=_context.cookie_handler.name["session"] + ) + except (InvalidCookieSign, AssertionError, AttributeError) as err: + logger.warning(err) + vals = None if vals is None: pass From 514601043298ba2dd372e0e4bafb2bd29d44a5e3 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 2 Sep 2021 17:00:59 +0200 Subject: [PATCH 15/58] parse_cookie twice. --- src/oidcop/user_authn/user.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/oidcop/user_authn/user.py b/src/oidcop/user_authn/user.py index dd291d3d..809ca9df 100755 --- a/src/oidcop/user_authn/user.py +++ b/src/oidcop/user_authn/user.py @@ -104,6 +104,8 @@ def cookie_info(self, cookie: List[dict], client_id: str) -> dict: logger.warning(err) vals = None + logger.debug("Value cookies: {}".format(vals)) + if vals is None: pass else: From c57bd80b6fcc78ada0f84ee09e2778983ac9387f Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 2 Sep 2021 17:05:19 +0200 Subject: [PATCH 16/58] parse_cookie twice. --- src/oidcop/user_authn/user.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/oidcop/user_authn/user.py b/src/oidcop/user_authn/user.py index 809ca9df..5f308368 100755 --- a/src/oidcop/user_authn/user.py +++ b/src/oidcop/user_authn/user.py @@ -111,6 +111,8 @@ def cookie_info(self, cookie: List[dict], client_id: str) -> dict: else: for val in vals: _info = json.loads(val["value"]) + session_id = _context.session_manager.decrypt_session_id(_info["sid"]) + logger.debug("session id: {}".format(session_id)) _, cid, _ = _context.session_manager.decrypt_session_id(_info["sid"]) if cid != client_id: continue From 031cd28929fd053c5c0a92fa944c30f2058e9d0d Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 2 Sep 2021 17:08:03 +0200 Subject: [PATCH 17/58] parse_cookie twice. --- src/oidcop/user_authn/user.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/oidcop/user_authn/user.py b/src/oidcop/user_authn/user.py index 5f308368..c6684762 100755 --- a/src/oidcop/user_authn/user.py +++ b/src/oidcop/user_authn/user.py @@ -113,10 +113,12 @@ def cookie_info(self, cookie: List[dict], client_id: str) -> dict: _info = json.loads(val["value"]) session_id = _context.session_manager.decrypt_session_id(_info["sid"]) logger.debug("session id: {}".format(session_id)) - _, cid, _ = _context.session_manager.decrypt_session_id(_info["sid"]) - if cid != client_id: + # _, cid, _ = _context.session_manager.decrypt_session_id(_info["sid"]) + if session_id[1] != client_id: continue else: + _info["uid"] = session_id[0] + _info["grant_id"] = session_id[2] return _info return {} From 93e95895c2d9c7aaac2e987971771f58233f157c Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Fri, 3 Sep 2021 08:58:12 +0200 Subject: [PATCH 18/58] More logging --- src/oidcop/oauth2/authorization.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/oidcop/oauth2/authorization.py b/src/oidcop/oauth2/authorization.py index 8ed4c880..d13464f6 100755 --- a/src/oidcop/oauth2/authorization.py +++ b/src/oidcop/oauth2/authorization.py @@ -521,6 +521,7 @@ def setup_auth( _max_age = 0 else: _max_age = max_age(request) + logger.debug(f'Max age: {max_age}') identity, _ts = authn.authenticated_as( client_id, cookie, authorization=_auth_info, max_age=_max_age ) From cc6337a8340b71ad4afc93047ee884a5657cc453 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Fri, 3 Sep 2021 09:00:32 +0200 Subject: [PATCH 19/58] More logging --- src/oidcop/oauth2/authorization.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/oidcop/oauth2/authorization.py b/src/oidcop/oauth2/authorization.py index d13464f6..c24e54b5 100755 --- a/src/oidcop/oauth2/authorization.py +++ b/src/oidcop/oauth2/authorization.py @@ -521,7 +521,7 @@ def setup_auth( _max_age = 0 else: _max_age = max_age(request) - logger.debug(f'Max age: {max_age}') + logger.debug(f'Max age: {_max_age}') identity, _ts = authn.authenticated_as( client_id, cookie, authorization=_auth_info, max_age=_max_age ) From bf595db64c873ad33c4ff72935ee268a46e53099 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Fri, 3 Sep 2021 09:06:41 +0200 Subject: [PATCH 20/58] Too old authentication --- src/oidcop/user_authn/user.py | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/oidcop/user_authn/user.py b/src/oidcop/user_authn/user.py index c6684762..86d532ab 100755 --- a/src/oidcop/user_authn/user.py +++ b/src/oidcop/user_authn/user.py @@ -5,12 +5,13 @@ import logging import sys import time -import warnings from typing import List from urllib.parse import unquote +import warnings from cryptography.hazmat.primitives.ciphers.aead import AESGCM from cryptojwt.jwt import JWT +from oidcmsg.time_util import utc_time_sans_frac from oidcop.exception import FailedAuthentication from oidcop.exception import ImproperlyConfigured @@ -65,6 +66,13 @@ def authenticated_as(self, client_id, cookie=None, **kwargs): return None, 0 else: _info = self.cookie_info(cookie, client_id) + if 'max_age' in kwargs: + _max_age = kwargs["max_age"] + _now = utc_time_sans_frac() + if _now > _info["timestamp"] + _max_age: + logger.debug( + "Too old by {} seconds".format(_now - (_info["timestamp"] + _max_age))) + return None, 0 return _info, time.time() def verify(self, *args, **kwargs): @@ -140,13 +148,13 @@ class UserPassJinja2(UserAuthnMethod): url_endpoint = "/verify/user_pass_jinja" def __init__( - self, - db, - template_handler, - template="user_pass.jinja2", - server_get=None, - verify_endpoint="", - **kwargs, + self, + db, + template_handler, + template="user_pass.jinja2", + server_get=None, + verify_endpoint="", + **kwargs, ): super(UserPassJinja2, self).__init__(server_get=server_get) From c0607d5838acb3df658dc7368ffd957c2206efb0 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Fri, 3 Sep 2021 09:08:28 +0200 Subject: [PATCH 21/58] Too old authentication - logging --- src/oidcop/user_authn/user.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/oidcop/user_authn/user.py b/src/oidcop/user_authn/user.py index 86d532ab..96d4a78e 100755 --- a/src/oidcop/user_authn/user.py +++ b/src/oidcop/user_authn/user.py @@ -66,6 +66,7 @@ def authenticated_as(self, client_id, cookie=None, **kwargs): return None, 0 else: _info = self.cookie_info(cookie, client_id) + logger.debug('Cookie info: {}'.format(_info)) if 'max_age' in kwargs: _max_age = kwargs["max_age"] _now = utc_time_sans_frac() From bef128e84555a44df22225d00409f06528393392 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Fri, 3 Sep 2021 09:10:01 +0200 Subject: [PATCH 22/58] Too old authentication --- src/oidcop/user_authn/user.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/oidcop/user_authn/user.py b/src/oidcop/user_authn/user.py index 96d4a78e..fcd231b4 100755 --- a/src/oidcop/user_authn/user.py +++ b/src/oidcop/user_authn/user.py @@ -67,13 +67,14 @@ def authenticated_as(self, client_id, cookie=None, **kwargs): else: _info = self.cookie_info(cookie, client_id) logger.debug('Cookie info: {}'.format(_info)) - if 'max_age' in kwargs: - _max_age = kwargs["max_age"] - _now = utc_time_sans_frac() - if _now > _info["timestamp"] + _max_age: - logger.debug( - "Too old by {} seconds".format(_now - (_info["timestamp"] + _max_age))) - return None, 0 + if _info: + if 'max_age' in kwargs: + _max_age = kwargs["max_age"] + _now = utc_time_sans_frac() + if _now > _info["timestamp"] + _max_age: + logger.debug( + "Too old by {} seconds".format(_now - (_info["timestamp"] + _max_age))) + return None, 0 return _info, time.time() def verify(self, *args, **kwargs): From f7cc53a4869be2abd54dce25a1847f22d24173eb Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Fri, 3 Sep 2021 09:13:10 +0200 Subject: [PATCH 23/58] Cookie info --- src/oidcop/user_authn/user.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/oidcop/user_authn/user.py b/src/oidcop/user_authn/user.py index fcd231b4..574be57c 100755 --- a/src/oidcop/user_authn/user.py +++ b/src/oidcop/user_authn/user.py @@ -121,6 +121,7 @@ def cookie_info(self, cookie: List[dict], client_id: str) -> dict: else: for val in vals: _info = json.loads(val["value"]) + _info["timestamp"] = val["timestamp"] session_id = _context.session_manager.decrypt_session_id(_info["sid"]) logger.debug("session id: {}".format(session_id)) # _, cid, _ = _context.session_manager.decrypt_session_id(_info["sid"]) From 0ea0cea75c2572375e3c4546b8896257ef506585 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Fri, 3 Sep 2021 09:14:43 +0200 Subject: [PATCH 24/58] Cookie info --- src/oidcop/user_authn/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/oidcop/user_authn/user.py b/src/oidcop/user_authn/user.py index 574be57c..e9b6c88d 100755 --- a/src/oidcop/user_authn/user.py +++ b/src/oidcop/user_authn/user.py @@ -121,7 +121,7 @@ def cookie_info(self, cookie: List[dict], client_id: str) -> dict: else: for val in vals: _info = json.loads(val["value"]) - _info["timestamp"] = val["timestamp"] + _info["timestamp"] = int(val["timestamp"]) session_id = _context.session_manager.decrypt_session_id(_info["sid"]) logger.debug("session id: {}".format(session_id)) # _, cid, _ = _context.session_manager.decrypt_session_id(_info["sid"]) From ff35728d6be0944c11beeba941c20e12a0a3d893 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Fri, 3 Sep 2021 09:25:46 +0200 Subject: [PATCH 25/58] Wrong error code. --- src/oidcop/oidc/token.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/oidcop/oidc/token.py b/src/oidcop/oidc/token.py index a88f45c8..8e6f8485 100755 --- a/src/oidcop/oidc/token.py +++ b/src/oidcop/oidc/token.py @@ -168,7 +168,7 @@ def post_parse_request( return self.error_cls(error="invalid_request", error_description="Wrong token type") if code.is_active() is False: - return self.error_cls(error="invalid_request", error_description="Code inactive") + return self.error_cls(error="invalid_grant", error_description="Code inactive") _auth_req = grant.authorization_request From 93eb7a75e111bea4d0fc28df5553db2fb4aa8962 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Fri, 3 Sep 2021 10:12:07 +0200 Subject: [PATCH 26/58] Revoke tokens that has been minted using a code that then is used once more. --- example/flask_op/views.py | 8 ++++++-- src/oidcop/oidc/token.py | 9 +++++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/example/flask_op/views.py b/example/flask_op/views.py index 82e4f679..d17b7347 100644 --- a/example/flask_op/views.py +++ b/example/flask_op/views.py @@ -251,10 +251,14 @@ def service_endpoint(endpoint): err_msg = ResponseMessage(error='invalid_request', error_description=str(err)) return make_response(err_msg.to_json(), 400) - _log.info('request: {}'.format(req_args)) if isinstance(req_args, ResponseMessage) and 'error' in req_args: - return make_response(req_args.to_json(), 400) + _log.info('Error response: {}'.format(req_args)) + _resp = make_response(req_args.to_json(), 400) + if request.method == "POST": + _resp.headers["Content-type"] = "application/json" + return _resp try: + _log.info('request: {}'.format(req_args)) if isinstance(endpoint, Token): args = endpoint.process_request(AccessTokenRequest(**req_args), http_info=http_info) else: diff --git a/src/oidcop/oidc/token.py b/src/oidcop/oidc/token.py index 8e6f8485..730f97b3 100755 --- a/src/oidcop/oidc/token.py +++ b/src/oidcop/oidc/token.py @@ -145,7 +145,7 @@ def process_request(self, req: Union[Message, dict], **kwargs): return _response def post_parse_request( - self, request: Union[Message, dict], client_id: Optional[str] = "", **kwargs + self, request: Union[Message, dict], client_id: Optional[str] = "", **kwargs ): """ This is where clients come to get their access tokens @@ -167,6 +167,11 @@ def post_parse_request( if not isinstance(code, AuthorizationCode): return self.error_cls(error="invalid_request", error_description="Wrong token type") + if code.used: # Has been used already + # invalidate all tokens that has been minted using this code + grant.revoke_token(based_on=request["code"], recursive=True) + return self.error_cls(error="invalid_grant", error_description="Code inactive") + if code.is_active() is False: return self.error_cls(error="invalid_grant", error_description="Code inactive") @@ -267,7 +272,7 @@ def process_request(self, req: Union[Message, dict], **kwargs): return _resp def post_parse_request( - self, request: Union[Message, dict], client_id: Optional[str] = "", **kwargs + self, request: Union[Message, dict], client_id: Optional[str] = "", **kwargs ): """ This is where clients come to refresh their access tokens From fc59033b51349b46c522ecf6c644413135e7c2cf Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Fri, 3 Sep 2021 10:36:01 +0200 Subject: [PATCH 27/58] Undefined max age --- src/oidcop/user_authn/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/oidcop/user_authn/user.py b/src/oidcop/user_authn/user.py index e9b6c88d..dc3eec98 100755 --- a/src/oidcop/user_authn/user.py +++ b/src/oidcop/user_authn/user.py @@ -68,7 +68,7 @@ def authenticated_as(self, client_id, cookie=None, **kwargs): _info = self.cookie_info(cookie, client_id) logger.debug('Cookie info: {}'.format(_info)) if _info: - if 'max_age' in kwargs: + if 'max_age' in kwargs and kwargs["max_age"] != 0: _max_age = kwargs["max_age"] _now = utc_time_sans_frac() if _now > _info["timestamp"] + _max_age: From 007fc0eac1e8025a5c9a25b1fbac6fbf9895104e Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Fri, 3 Sep 2021 16:14:07 +0200 Subject: [PATCH 28/58] Correct user. --- src/oidcop/oauth2/token.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/oidcop/oauth2/token.py b/src/oidcop/oauth2/token.py index 75c4efc8..c976bed4 100755 --- a/src/oidcop/oauth2/token.py +++ b/src/oidcop/oauth2/token.py @@ -226,6 +226,12 @@ def process_request(self, req: Union[Message, dict], **kwargs): token_value = req["refresh_token"] _session_info = _mngr.get_session_info_by_token(token_value, grant=True) + + if _session_info["client_id"] != req["client_id"]: + logger.debug("{} owner of token".format(_session_info["client_id"])) + logger.warning("Client using token it was not given") + return self.error_cls(error="invalid_grant", error_description="Wrong client") + _grant = _session_info["grant"] token_type = "Bearer" From 9188cac4379a52269c1c26d114cd1e0509b2d082 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Fri, 3 Sep 2021 16:17:04 +0200 Subject: [PATCH 29/58] logging --- src/oidcop/oauth2/token.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/oidcop/oauth2/token.py b/src/oidcop/oauth2/token.py index c976bed4..3f8c9138 100755 --- a/src/oidcop/oauth2/token.py +++ b/src/oidcop/oauth2/token.py @@ -226,6 +226,7 @@ def process_request(self, req: Union[Message, dict], **kwargs): token_value = req["refresh_token"] _session_info = _mngr.get_session_info_by_token(token_value, grant=True) + logger.debug("Session info: {}".format(_session_info)) if _session_info["client_id"] != req["client_id"]: logger.debug("{} owner of token".format(_session_info["client_id"])) From 88da05a10334739f6c0b1467d6e37b8f5f72102f Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Fri, 3 Sep 2021 16:21:21 +0200 Subject: [PATCH 30/58] logging --- src/oidcop/oauth2/token.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/oidcop/oauth2/token.py b/src/oidcop/oauth2/token.py index 3f8c9138..b38518c2 100755 --- a/src/oidcop/oauth2/token.py +++ b/src/oidcop/oauth2/token.py @@ -107,9 +107,8 @@ def process_request(self, req: Union[Message, dict], **kwargs): :return: """ _context = self.endpoint.server_get("endpoint_context") - _mngr = _context.session_manager - _log_debug = logger.debug + logger.debug("Access Token") if req["grant_type"] != "authorization_code": return self.error_cls(error="invalid_request", error_description="Unknown grant_type") @@ -135,7 +134,7 @@ def process_request(self, req: Union[Message, dict], **kwargs): error="invalid_request", error_description="redirect_uri mismatch" ) - _log_debug("All checks OK") + logger.debug("All checks OK") issue_refresh = kwargs.get("issue_refresh", False) _response = { @@ -220,6 +219,7 @@ class RefreshTokenHelper(TokenEndpointHelper): def process_request(self, req: Union[Message, dict], **kwargs): _context = self.endpoint.server_get("endpoint_context") _mngr = _context.session_manager + logger.debug("Refresh Token") if req["grant_type"] != "refresh_token": return self.error_cls(error="invalid_request", error_description="Wrong grant_type") From 5879a9ab07960d00a0c87adcd59cad60513a4466 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Fri, 3 Sep 2021 16:37:03 +0200 Subject: [PATCH 31/58] Verify correct user --- src/oidcop/oidc/token.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/oidcop/oidc/token.py b/src/oidcop/oidc/token.py index 730f97b3..61c9e878 100755 --- a/src/oidcop/oidc/token.py +++ b/src/oidcop/oidc/token.py @@ -32,7 +32,7 @@ def process_request(self, req: Union[Message, dict], **kwargs): _context = self.endpoint.server_get("endpoint_context") _mngr = _context.session_manager - _log_debug = logger.debug + logger.debug("OIDC Access Token") if req["grant_type"] != "authorization_code": return self.error_cls(error="invalid_request", error_description="Unknown grant_type") @@ -43,6 +43,13 @@ def process_request(self, req: Union[Message, dict], **kwargs): return self.error_cls(error="invalid_request", error_description="Missing code") _session_info = _mngr.get_session_info_by_token(_access_code, grant=True) + logger.debug(f"Session info: {_session_info}") + + if _session_info["client_id"] != req["client_id"]: + logger.debug("{} owner of token".format(_session_info["client_id"])) + logger.warning("{} using token it was not given".format(req["client_id"])) + return self.error_cls(error="invalid_grant", error_description="Wrong client") + grant = _session_info["grant"] token_type = "Bearer" @@ -72,7 +79,7 @@ def process_request(self, req: Union[Message, dict], **kwargs): error="invalid_request", error_description="redirect_uri mismatch" ) - _log_debug("All checks OK") + logger.debug("All checks OK") issue_refresh = False if "issue_refresh" in kwargs: @@ -195,6 +202,11 @@ def process_request(self, req: Union[Message, dict], **kwargs): token_value = req["refresh_token"] _session_info = _mngr.get_session_info_by_token(token_value, grant=True) + if _session_info["client_id"] != req["client_id"]: + logger.debug("{} owner of token".format(_session_info["client_id"])) + logger.warning("{} using token it was not given".format(req["client_id"])) + return self.error_cls(error="invalid_grant", error_description="Wrong client") + _grant = _session_info["grant"] token_type = "Bearer" From f061a88528b6bcd1c7a95834e7ca0fce89e1d729 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Fri, 3 Sep 2021 16:37:33 +0200 Subject: [PATCH 32/58] Verify correct user --- src/oidcop/oauth2/token.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/oidcop/oauth2/token.py b/src/oidcop/oauth2/token.py index b38518c2..c2d32703 100755 --- a/src/oidcop/oauth2/token.py +++ b/src/oidcop/oauth2/token.py @@ -119,6 +119,11 @@ def process_request(self, req: Union[Message, dict], **kwargs): return self.error_cls(error="invalid_request", error_description="Missing code") _session_info = _mngr.get_session_info_by_token(_access_code, grant=True) + if _session_info["client_id"] != req["client_id"]: + logger.debug("{} owner of token".format(_session_info["client_id"])) + logger.warning("Client using token it was not given") + return self.error_cls(error="invalid_grant", error_description="Wrong client") + grant = _session_info["grant"] _based_on = grant.get_token(_access_code) From 2c8b44c514b4da873a007cdb3250ccd722c947cb Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Mon, 6 Sep 2021 08:57:21 +0200 Subject: [PATCH 33/58] prompt==login forces reauthentication. --- src/oidcop/oauth2/authorization.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/oidcop/oauth2/authorization.py b/src/oidcop/oauth2/authorization.py index c24e54b5..dc1c5694 100755 --- a/src/oidcop/oauth2/authorization.py +++ b/src/oidcop/oauth2/authorization.py @@ -968,4 +968,7 @@ def re_authenticate(request, authn) -> bool: :param authn: :return: """ + if "prompt" in request and request["prompt"] == "login": + return True + return False From de49d674b8307e75fcc6ca5f18b2db43d661c2a2 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Mon, 6 Sep 2021 08:59:20 +0200 Subject: [PATCH 34/58] prompt==login forces reauthentication. --- src/oidcop/oauth2/authorization.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/oidcop/oauth2/authorization.py b/src/oidcop/oauth2/authorization.py index dc1c5694..c1cefbca 100755 --- a/src/oidcop/oauth2/authorization.py +++ b/src/oidcop/oauth2/authorization.py @@ -969,6 +969,7 @@ def re_authenticate(request, authn) -> bool: :return: """ if "prompt" in request and request["prompt"] == "login": + logger.debug("Reauthenticate due to prompt=login") return True return False From f0d2d8c2de3637e277575b7da04abba0682cf5d7 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Mon, 6 Sep 2021 09:04:56 +0200 Subject: [PATCH 35/58] prompt==login forces reauthentication. --- src/oidcop/oauth2/authorization.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/oidcop/oauth2/authorization.py b/src/oidcop/oauth2/authorization.py index c1cefbca..4d6bd884 100755 --- a/src/oidcop/oauth2/authorization.py +++ b/src/oidcop/oauth2/authorization.py @@ -961,13 +961,15 @@ def __call__(self, client_id, endpoint_context, alg, alg_type): def re_authenticate(request, authn) -> bool: """ - This is where you can demand reauthentication even though the authentication in use + This is where you can demand re-authentication even though the authentication in use is still valid. :param request: :param authn: :return: """ + logger.debug("Re-authenticate ??") + if "prompt" in request and request["prompt"] == "login": logger.debug("Reauthenticate due to prompt=login") return True From 4ee4b3f48b73f0c214fde488e3c7cf35b03cb019 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Mon, 6 Sep 2021 09:09:52 +0200 Subject: [PATCH 36/58] prompt==login forces reauthentication. --- src/oidcop/oauth2/authorization.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/oidcop/oauth2/authorization.py b/src/oidcop/oauth2/authorization.py index 4d6bd884..69b0d55c 100755 --- a/src/oidcop/oauth2/authorization.py +++ b/src/oidcop/oauth2/authorization.py @@ -968,7 +968,7 @@ def re_authenticate(request, authn) -> bool: :param authn: :return: """ - logger.debug("Re-authenticate ??") + logger.debug("Re-authenticate ??: {}".format(request)) if "prompt" in request and request["prompt"] == "login": logger.debug("Reauthenticate due to prompt=login") From 22e3b7db7b049176e03dd27e87f888e401867d33 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Mon, 6 Sep 2021 09:12:27 +0200 Subject: [PATCH 37/58] prompt==login forces reauthentication. --- src/oidcop/oauth2/authorization.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/oidcop/oauth2/authorization.py b/src/oidcop/oauth2/authorization.py index 69b0d55c..c7929c17 100755 --- a/src/oidcop/oauth2/authorization.py +++ b/src/oidcop/oauth2/authorization.py @@ -970,7 +970,9 @@ def re_authenticate(request, authn) -> bool: """ logger.debug("Re-authenticate ??: {}".format(request)) - if "prompt" in request and request["prompt"] == "login": + _prompt = request.get("prompt") + logger.debug(f"Prompt={_prompt}") + if _prompt == "login": logger.debug("Reauthenticate due to prompt=login") return True From a61e27ea17bb4f2dea97dbcc89968fadd8c2db39 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Mon, 6 Sep 2021 09:14:37 +0200 Subject: [PATCH 38/58] Prompt is a list. --- src/oidcop/oauth2/authorization.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/oidcop/oauth2/authorization.py b/src/oidcop/oauth2/authorization.py index c7929c17..c3689abb 100755 --- a/src/oidcop/oauth2/authorization.py +++ b/src/oidcop/oauth2/authorization.py @@ -972,7 +972,7 @@ def re_authenticate(request, authn) -> bool: _prompt = request.get("prompt") logger.debug(f"Prompt={_prompt}") - if _prompt == "login": + if "login" in _prompt: logger.debug("Reauthenticate due to prompt=login") return True From 984eb5a31a049756533f4937a06211217a9e5c30 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Mon, 6 Sep 2021 10:03:54 +0200 Subject: [PATCH 39/58] Prompt is a list. --- src/oidcop/oauth2/authorization.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/oidcop/oauth2/authorization.py b/src/oidcop/oauth2/authorization.py index c3689abb..41339e98 100755 --- a/src/oidcop/oauth2/authorization.py +++ b/src/oidcop/oauth2/authorization.py @@ -970,7 +970,7 @@ def re_authenticate(request, authn) -> bool: """ logger.debug("Re-authenticate ??: {}".format(request)) - _prompt = request.get("prompt") + _prompt = request.get("prompt", []) logger.debug(f"Prompt={_prompt}") if "login" in _prompt: logger.debug("Reauthenticate due to prompt=login") From ba3b9d59ee720e24afd173c1bb049cc447e86b7e Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Mon, 6 Sep 2021 19:51:05 +0300 Subject: [PATCH 40/58] Add grant_types_supported per client --- src/oidcop/oauth2/token.py | 12 ++++-- tests/test_24_oauth2_token_endpoint.py | 56 ++++++++++++++++++++++++++ tests/test_35_oidc_token_endpoint.py | 3 +- 3 files changed, 67 insertions(+), 4 deletions(-) diff --git a/src/oidcop/oauth2/token.py b/src/oidcop/oauth2/token.py index e84ea887..2d4d8cf9 100755 --- a/src/oidcop/oauth2/token.py +++ b/src/oidcop/oauth2/token.py @@ -4,7 +4,6 @@ from cryptojwt.jwe.exception import JWEException from cryptojwt.jwt import utc_time_sans_frac - from oidcmsg.message import Message from oidcmsg.oauth2 import AccessTokenResponse from oidcmsg.oauth2 import ResponseMessage @@ -390,13 +389,20 @@ def configure_grant_types(self, grant_types_supported): def _post_parse_request( self, request: Union[Message, dict], client_id: Optional[str] = "", **kwargs ): - _helper = self.helper.get(request["grant_type"]) + grant_type = request["grant_type"] + _helper = self.helper.get(grant_type) + client = kwargs["endpoint_context"].cdb[client_id] + if "grant_types_supported" in client and grant_type not in client["grant_types_supported"]: + return self.error_cls( + error="invalid_request", + error_description=f"Unsupported grant_type: {grant_type}", + ) if _helper: return _helper.post_parse_request(request, client_id, **kwargs) else: return self.error_cls( error="invalid_request", - error_description=f"Unsupported grant_type: {request['grant_type']}", + error_description=f"Unsupported grant_type: {grant_type}", ) def process_request(self, request: Optional[Union[Message, dict]] = None, **kwargs): diff --git a/tests/test_24_oauth2_token_endpoint.py b/tests/test_24_oauth2_token_endpoint.py index e3627be5..1d7a6104 100644 --- a/tests/test_24_oauth2_token_endpoint.py +++ b/tests/test_24_oauth2_token_endpoint.py @@ -234,6 +234,28 @@ def test_parse(self): assert set(_req.keys()) == set(_token_request.keys()) + def test_auth_code_grant_disallowed_per_client(self): + areq = AUTH_REQ.copy() + areq["scope"] = ["email"] + self.endpoint_context.cdb["client_1"]["grant_types_supported"] = [] + + session_id = self._create_session(areq) + grant = self.endpoint_context.authz(session_id, areq) + code = self._mint_code(grant, areq["client_id"]) + + _cntx = self.endpoint_context + + _token_request = TOKEN_REQ_DICT.copy() + _token_request["code"] = code.value + _req = self.token_endpoint.parse_request(_token_request) + _resp = self.token_endpoint.process_request(request=_req, issue_refresh=True) + + assert isinstance(_req, TokenErrorResponse) + assert _req.to_dict() == { + "error": "invalid_request", + "error_description": "Unsupported grant_type: authorization_code", + } + def test_process_request(self): session_id = self._create_session(AUTH_REQ) grant = self.session_manager[session_id] @@ -336,6 +358,40 @@ def test_do_refresh_access_token(self): msg = self.token_endpoint.do_response(request=_req, **_resp) assert isinstance(msg, dict) + def test_refresh_grant_disallowed_per_client(self): + areq = AUTH_REQ.copy() + areq["scope"] = ["email"] + self.endpoint_context.cdb["client_1"]["grant_types_supported"] = [ + "authorization_code" + ] + + session_id = self._create_session(areq) + grant = self.endpoint_context.authz(session_id, areq) + code = self._mint_code(grant, areq["client_id"]) + + _cntx = self.endpoint_context + + _token_request = TOKEN_REQ_DICT.copy() + _token_request["code"] = code.value + _req = self.token_endpoint.parse_request(_token_request) + _resp = self.token_endpoint.process_request(request=_req, issue_refresh=True) + + _request = REFRESH_TOKEN_REQ.copy() + _request["refresh_token"] = _resp["response_args"]["refresh_token"] + + _token_value = _resp["response_args"]["refresh_token"] + _session_info = self.session_manager.get_session_info_by_token(_token_value) + _token = self.session_manager.find_token(_session_info["session_id"], _token_value) + _token.usage_rules["supports_minting"] = ["access_token", "refresh_token"] + + _req = self.token_endpoint.parse_request(_request.to_json()) + + assert isinstance(_req, TokenErrorResponse) + assert _req.to_dict() == { + "error": "invalid_request", + "error_description": "Unsupported grant_type: refresh_token", + } + def test_do_2nd_refresh_access_token(self): areq = AUTH_REQ.copy() areq["scope"] = ["email"] diff --git a/tests/test_35_oidc_token_endpoint.py b/tests/test_35_oidc_token_endpoint.py index 998fbfab..f8336310 100755 --- a/tests/test_35_oidc_token_endpoint.py +++ b/tests/test_35_oidc_token_endpoint.py @@ -29,6 +29,7 @@ from oidcop.user_authn.authn_context import INTERNETPROTOCOLPASSWORD from oidcop.user_info import UserInfo from oidcop.util import lv_pack +from tests.test_24_oauth2_token_endpoint import TestEndpoint as _TestEndpoint KEYDEFS = [ {"type": "RSA", "key": "", "use": ["sig"]}, @@ -183,7 +184,7 @@ def conf(): } -class TestEndpoint(object): +class TestEndpoint(_TestEndpoint): @pytest.fixture(autouse=True) def create_endpoint(self, conf): server = Server(OPConfiguration(conf=conf, base_path=BASEDIR), cwd=BASEDIR) From 675be1318de45d6c4f032bcb8d1c1fc1d489259a Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Mon, 6 Sep 2021 19:56:41 +0300 Subject: [PATCH 41/58] Add docs --- docs/source/contents/conf.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/source/contents/conf.rst b/docs/source/contents/conf.rst index 483a7d4c..9799e8bc 100644 --- a/docs/source/contents/conf.rst +++ b/docs/source/contents/conf.rst @@ -712,3 +712,11 @@ How to configure the release of the user claims per clients:: "id_token": False, }, }, + +Some of the allowed client configurations are (this is an ongoing work): + +--------------------- +grant_types_supported +--------------------- + +Configure the allowed grant types on the token endpoint. From 3abbb14d4a77f63997d637dce2b620d46d1a2563 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Tue, 7 Sep 2021 13:44:00 +0200 Subject: [PATCH 42/58] Fixed cookie_info(). Added tests and fixed a test result. --- src/oidcop/user_authn/user.py | 19 +++++----- tests/test_12_user_authn.py | 3 +- tests/test_24_oauth2_token_endpoint.py | 50 ++++++++++++++++++++++++++ tests/test_35_oidc_token_endpoint.py | 49 +++++++++++++++++++++++++ 4 files changed, 109 insertions(+), 12 deletions(-) diff --git a/src/oidcop/user_authn/user.py b/src/oidcop/user_authn/user.py index dc3eec98..5b5bdded 100755 --- a/src/oidcop/user_authn/user.py +++ b/src/oidcop/user_authn/user.py @@ -102,17 +102,14 @@ def done(self, areq): def cookie_info(self, cookie: List[dict], client_id: str) -> dict: _context = self.server_get("endpoint_context") - if cookie and "value" in cookie[0]: - vals = cookie - else: - try: - logger.debug("parse_cookie@UserAuthnMethod") - vals = _context.cookie_handler.parse_cookie( - cookies=cookie, name=_context.cookie_handler.name["session"] - ) - except (InvalidCookieSign, AssertionError, AttributeError) as err: - logger.warning(err) - vals = None + try: + logger.debug("parse_cookie@UserAuthnMethod") + vals = _context.cookie_handler.parse_cookie( + cookies=cookie, name=_context.cookie_handler.name["session"] + ) + except (InvalidCookieSign, AssertionError, AttributeError) as err: + logger.warning(err) + vals = [] logger.debug("Value cookies: {}".format(vals)) diff --git a/tests/test_12_user_authn.py b/tests/test_12_user_authn.py index 14585267..3cf89fcc 100644 --- a/tests/test_12_user_authn.py +++ b/tests/test_12_user_authn.py @@ -100,7 +100,8 @@ def test_authenticated_as_with_cookie(self): ) _info, _time_stamp = method.authenticated_as("client 12345", [_cookie]) - assert set(_info.keys()) == {"sub", "sid", "state", "client_id"} + assert set(_info.keys()) == {'sub', 'uid', 'state', 'grant_id', 'timestamp', 'sid', + 'client_id'} assert _info["sub"] == "diana" def test_userpassjinja2(self): diff --git a/tests/test_24_oauth2_token_endpoint.py b/tests/test_24_oauth2_token_endpoint.py index 1d7a6104..0f2bf2fc 100644 --- a/tests/test_24_oauth2_token_endpoint.py +++ b/tests/test_24_oauth2_token_endpoint.py @@ -648,3 +648,53 @@ def test_configure_grant_types(self): assert len(self.token_endpoint.helper) == 1 assert "access_token" in self.token_endpoint.helper assert "refresh_token" not in self.token_endpoint.helper + + def test_token_request_other_client(self): + _context = self.endpoint_context + _context.cdb["client_2"] = _context.cdb["client_1"] + session_id = self._create_session(AUTH_REQ) + grant = self.session_manager[session_id] + code = self._mint_code(grant, AUTH_REQ["client_id"]) + + _token_request = TOKEN_REQ_DICT.copy() + _token_request["client_id"] = "client_2" + _token_request["code"] = code.value + + _req = self.token_endpoint.parse_request(_token_request) + _resp = self.token_endpoint.process_request(request=_req) + + assert isinstance(_resp, TokenErrorResponse) + assert _resp.to_dict() == { + "error": "invalid_grant", "error_description": "Wrong client" + } + + def test_refresh_token_request_other_client(self): + _context = self.endpoint_context + _context.cdb["client_2"] = _context.cdb["client_1"] + session_id = self._create_session(AUTH_REQ) + grant = self.session_manager[session_id] + code = self._mint_code(grant, AUTH_REQ["client_id"]) + + _token_request = TOKEN_REQ_DICT.copy() + _token_request["code"] = code.value + + _req = self.token_endpoint.parse_request(_token_request) + _resp = self.token_endpoint.process_request( + request=_req, issue_refresh=True + ) + + _request = REFRESH_TOKEN_REQ.copy() + _request["client_id"] = "client_2" + _request["refresh_token"] = _resp["response_args"]["refresh_token"] + + _token_value = _resp["response_args"]["refresh_token"] + _session_info = self.session_manager.get_session_info_by_token(_token_value) + _token = self.session_manager.find_token(_session_info["session_id"], _token_value) + _token.usage_rules["supports_minting"] = ["access_token", "refresh_token"] + + _req = self.token_endpoint.parse_request(_request.to_json()) + _resp = self.token_endpoint.process_request(request=_req, ) + assert isinstance(_resp, TokenErrorResponse) + assert _resp.to_dict() == { + "error": "invalid_grant", "error_description": "Wrong client" + } \ No newline at end of file diff --git a/tests/test_35_oidc_token_endpoint.py b/tests/test_35_oidc_token_endpoint.py index f82066a6..e4c203ea 100755 --- a/tests/test_35_oidc_token_endpoint.py +++ b/tests/test_35_oidc_token_endpoint.py @@ -838,6 +838,55 @@ def test_access_token_lifetime(self): assert access_token["exp"] - access_token["iat"] == lifetime + def test_token_request_other_client(self): + _context = self.endpoint_context + _context.cdb["client_2"] = _context.cdb["client_1"] + session_id = self._create_session(AUTH_REQ) + grant = self.session_manager[session_id] + code = self._mint_code(grant, AUTH_REQ["client_id"]) + + _token_request = TOKEN_REQ_DICT.copy() + _token_request["client_id"] = "client_2" + _token_request["code"] = code.value + + _req = self.token_endpoint.parse_request(_token_request) + _resp = self.token_endpoint.process_request(request=_req) + + assert isinstance(_resp, TokenErrorResponse) + assert _resp.to_dict() == { + "error": "invalid_grant", "error_description": "Wrong client" + } + + def test_refresh_token_request_other_client(self): + _context = self.endpoint_context + _context.cdb["client_2"] = _context.cdb["client_1"] + session_id = self._create_session(AUTH_REQ) + grant = self.session_manager[session_id] + code = self._mint_code(grant, AUTH_REQ["client_id"]) + + _token_request = TOKEN_REQ_DICT.copy() + _token_request["code"] = code.value + + _req = self.token_endpoint.parse_request(_token_request) + _resp = self.token_endpoint.process_request( + request=_req, issue_refresh=True + ) + + _request = REFRESH_TOKEN_REQ.copy() + _request["client_id"] = "client_2" + _request["refresh_token"] = _resp["response_args"]["refresh_token"] + + _token_value = _resp["response_args"]["refresh_token"] + _session_info = self.session_manager.get_session_info_by_token(_token_value) + _token = self.session_manager.find_token(_session_info["session_id"], _token_value) + _token.usage_rules["supports_minting"] = ["access_token", "refresh_token"] + + _req = self.token_endpoint.parse_request(_request.to_json()) + _resp = self.token_endpoint.process_request(request=_req,) + assert isinstance(_resp, TokenErrorResponse) + assert _resp.to_dict() == { + "error": "invalid_grant", "error_description": "Wrong client" + } class TestOldTokens(object): @pytest.fixture(autouse=True) From f091c20780f6f9dadc708485897eebc34f18b709 Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Mon, 13 Sep 2021 18:17:41 +0300 Subject: [PATCH 43/58] Refactor scopes --- docs/source/contents/conf.rst | 59 +++-- src/oidcop/authz/__init__.py | 6 +- src/oidcop/configure.py | 41 +++- src/oidcop/endpoint_context.py | 19 +- src/oidcop/oauth2/authorization.py | 19 +- src/oidcop/oidc/add_on/custom_scopes.py | 7 +- src/oidcop/scopes.py | 67 ++++-- src/oidcop/server.py | 7 +- src/oidcop/session/claims.py | 6 +- tests/test_07_userinfo.py | 217 ++++++++++++------ .../test_22_oidc_provider_config_endpoint.py | 40 +++- tests/test_24_oauth2_token_endpoint.py | 6 +- tests/test_26_oidc_userinfo_endpoint.py | 144 ++++++++++-- tests/test_50_persistence.py | 33 ++- 14 files changed, 484 insertions(+), 187 deletions(-) diff --git a/docs/source/contents/conf.rst b/docs/source/contents/conf.rst index 9799e8bc..f8a97b1c 100644 --- a/docs/source/contents/conf.rst +++ b/docs/source/contents/conf.rst @@ -53,6 +53,28 @@ sub_funcs Optional. Functions involved in *sub*ject value creation. + + +scopes_mapping +############## + +A dict defining the scopes that are allowed to be used per client and the claims +they map to (defaults to the scopes mapping described in the spec). If we want +to define a scope that doesn't map to claims (e.g. offline_access) then we +simply map it to an empty list. E.g.:: + { + "scope_a": ["claim1", "claim2"], + "scope_b": [] + } +*Note*: For OIDC the `openid` scope must be present in this mapping. + + +allowed_scopes +############## + +A list with the scopes that are allowed to be used (defaults to the keys in scopes_mapping). + + ------ add_on ------ @@ -67,21 +89,6 @@ An example:: "code_challenge_method": "S256 S384 S512" } }, - "claims": { - "function": "oidcop.oidc.add_on.custom_scopes.add_custom_scopes", - "kwargs": { - "research_and_scholarship": [ - "name", - "given_name", - "family_name", - "email", - "email_verified", - "sub", - "iss", - "eduperson_scoped_affiliation" - ] - } - } } The provided add-ons can be seen in the following sections. @@ -176,6 +183,8 @@ An example:: backchannel_logout_supported: True backchannel_logout_session_supported: True check_session_iframe: https://127.0.0.1:5000/check_session_iframe + scopes_supported: ["openid", "profile", "random"] + claims_supported: ["sub", "given_name", "birthdate"] --------- client_db @@ -720,3 +729,23 @@ grant_types_supported --------------------- Configure the allowed grant types on the token endpoint. + +-------------- +scopes_mapping +-------------- + +A dict defining the scopes that are allowed to be used per client and the claims +they map to (defaults to the scopes mapping described in the spec). If we want +to define a scope that doesn't map to claims (e.g. offline_access) then we +simply map it to an empty list. E.g.:: + { + "scope_a": ["claim1", "claim2"], + "scope_b": [] + } + +-------------- +allowed_scopes +-------------- + +A list with the scopes that are allowed to be used (defaults to the keys in the +clients scopes_mapping). diff --git a/src/oidcop/authz/__init__.py b/src/oidcop/authz/__init__.py index 04203cff..a22a11bb 100755 --- a/src/oidcop/authz/__init__.py +++ b/src/oidcop/authz/__init__.py @@ -80,8 +80,10 @@ def __call__( grant.resources = resources # After this is where user consent should be handled - scopes = request.get("scope", []) - grant.scope = scopes + scopes = grant.scope + if not scopes: + scopes = request.get("scope", []) + grant.scope = scopes grant.claims = self.server_get("endpoint_context").claims_interface.get_claims_all_usage( session_id=session_id, scopes=scopes ) diff --git a/src/oidcop/configure.py b/src/oidcop/configure.py index 87591022..bde65666 100755 --- a/src/oidcop/configure.py +++ b/src/oidcop/configure.py @@ -10,6 +10,7 @@ from typing import Union from oidcop.logging import configure_logging +from oidcop.scopes import SCOPE2CLAIMS from oidcop.utils import load_yaml_config DEFAULT_FILE_ATTRIBUTE_NAMES = [ @@ -78,6 +79,7 @@ "refresh": {"class": "oidcop.token.jwt_token.JWTToken", "kwargs": {"lifetime": 86400}, }, "id_token": {"class": "oidcop.token.id_token.IDToken", "kwargs": {}}, }, + "scopes_mapping": SCOPE2CLAIMS, } AS_DEFAULT_CONFIG = copy.deepcopy(OP_DEFAULT_CONFIG) @@ -274,12 +276,39 @@ class OPConfiguration(EntityConfiguration): "Provider configuration" default_config = OP_DEFAULT_CONFIG parameter = EntityConfiguration.parameter.copy() - parameter.update({ - "id_token": None, - "login_hint2acrs": {}, - "login_hint_lookup": None, - "sub_func": {} - }) + parameter.update( + { + "id_token": None, + "login_hint2acrs": {}, + "login_hint_lookup": None, + "sub_func": {}, + "scopes_mapping": {}, + "scopes_supported": None, + "advertised_scopes": None, + } + ) + + def __init__( + self, + conf: Dict, + base_path: Optional[str] = "", + entity_conf: Optional[List[dict]] = None, + domain: Optional[str] = "", + port: Optional[int] = 0, + file_attributes: Optional[List[str]] = None, + ): + super().__init__( + conf=conf, + base_path=base_path, + entity_conf=entity_conf, + domain=domain, + port=port, + file_attributes=file_attributes, + ) + scopes_mapping = self.scopes_mapping + if "advertised_scopes" not in self: + self["advertised_scopes"] = list(scopes_mapping.keys()) + class ASConfiguration(EntityConfiguration): "Authorization server configuration" diff --git a/src/oidcop/endpoint_context.py b/src/oidcop/endpoint_context.py index 9a683f2b..6285abe7 100755 --- a/src/oidcop/endpoint_context.py +++ b/src/oidcop/endpoint_context.py @@ -1,6 +1,7 @@ import json import logging from typing import Any +from typing import Callable from typing import Optional from typing import Union @@ -121,6 +122,7 @@ class EndpointContext(OidcContext): def __init__( self, conf: Union[dict, OPConfiguration], + server_get: Callable, keyjar: Optional[KeyJar] = None, cwd: Optional[str] = "", cookie_handler: Optional[Any] = None, @@ -128,6 +130,7 @@ def __init__( ): OidcContext.__init__(self, conf, keyjar, entity_id=conf.get("issuer", "")) self.conf = conf + self.server_get = server_get _client_db = conf.get("client_db") if _client_db: @@ -248,10 +251,14 @@ def set_scopes_handler(self): _spec = self.conf.get("scopes_handler") if _spec: _kwargs = _spec.get("kwargs", {}) - _cls = importer(_spec["class"])(**_kwargs) - self.scopes_handler = _cls(_kwargs) + _cls = importer(_spec["class"]) + self.scopes_handler = _cls(self.server_get, **_kwargs) else: - self.scopes_handler = Scopes() + self.scopes_handler = Scopes( + self.server_get, + allowed_scopes=self.conf.get("allowed_scopes"), + scopes_mapping=self.conf.get("scopes_mapping"), + ) def do_add_on(self, endpoints): _add_on_conf = self.conf.get("add_on") @@ -325,8 +332,10 @@ def create_providerinfo(self, capabilities): _provider_info["jwks_uri"] = self.jwks_uri if "scopes_supported" not in _provider_info: - _provider_info["scopes_supported"] = [s for s in self.scope2claims.keys()] + _provider_info["scopes_supported"] = self.scopes_handler.get_allowed_scopes() if "claims_supported" not in _provider_info: - _provider_info["claims_supported"] = STANDARD_CLAIMS[:] + _provider_info["claims_supported"] = list( + self.scopes_handler.scopes_to_claims(_provider_info["scopes_supported"]).keys() + ) return _provider_info diff --git a/src/oidcop/oauth2/authorization.py b/src/oidcop/oauth2/authorization.py index 41339e98..c225a73a 100755 --- a/src/oidcop/oauth2/authorization.py +++ b/src/oidcop/oauth2/authorization.py @@ -244,15 +244,17 @@ def authn_args_gather( return authn_args -def check_unknown_scopes_policy(request_info, cinfo, endpoint_context): - op_capabilities = endpoint_context.conf["capabilities"] - client_allowed_scopes = cinfo.get("allowed_scopes") or op_capabilities["scopes_supported"] +def check_unknown_scopes_policy(request_info, client_id, endpoint_context): + if not endpoint_context.conf["capabilities"].get("deny_unknown_scopes"): + return + + allowed_scopes = endpoint_context.scopes_handler.get_allowed_scopes(client_id=client_id) # this prevents that authz would be released for unavailable scopes for scope in request_info["scope"]: - if op_capabilities.get("deny_unknown_scopes") and scope not in client_allowed_scopes: + if scope not in allowed_scopes: _msg = "{} requested an unauthorized scope ({})" - logger.warning(_msg.format(cinfo["client_id"], scope)) + logger.warning(_msg.format(client_id, scope)) raise UnAuthorizedClientScope() @@ -681,7 +683,9 @@ def create_authn_response(self, request: Union[dict, Message], sid: str) -> dict _sinfo = _mngr.get_session_info(sid, grant=True) if request.get("scope"): - aresp["scope"] = request["scope"] + aresp["scope"] = _context.scopes_handler.filter_scopes( + request["scope"], _sinfo["client_id"] + ) rtype = set(request["response_type"][:]) handled_response_type = [] @@ -903,8 +907,7 @@ def process_request( # logger.debug("client {}: {}".format(_cid, cinfo)) # this apply the default optionally deny_unknown_scopes policy - if cinfo: - check_unknown_scopes_policy(request, cinfo, _context) + check_unknown_scopes_policy(request, _cid, _context) if http_info is None: http_info = {} diff --git a/src/oidcop/oidc/add_on/custom_scopes.py b/src/oidcop/oidc/add_on/custom_scopes.py index 1366548f..2fbc6a32 100644 --- a/src/oidcop/oidc/add_on/custom_scopes.py +++ b/src/oidcop/oidc/add_on/custom_scopes.py @@ -10,17 +10,22 @@ def add_custom_scopes(endpoint, **kwargs): :param endpoint: A dictionary with endpoint instances as values """ # Just need an endpoint, anyone will do + LOGGER.warning( + "The custom_scopes add on is deprecated. The `scopes_mapping` config " + "option should be used instead." + ) _endpoint = list(endpoint.values())[0] _scopes2claims = SCOPE2CLAIMS.copy() _scopes2claims.update(kwargs) _context = _endpoint.server_get("endpoint_context") - _context.scope2claims = _scopes2claims + _context.scopes_handler.scopes_mapping = _scopes2claims pi = _context.provider_info _scopes = set(pi.get("scopes_supported", [])) _scopes.update(set(kwargs.keys())) pi["scopes_supported"] = list(_scopes) + _context.scopes_handler.allowed_scopes = pi["scopes_supported"] _claims = set(pi.get("claims_supported", [])) for vals in kwargs.values(): diff --git a/src/oidcop/scopes.py b/src/oidcop/scopes.py index ec772040..87d2fe80 100644 --- a/src/oidcop/scopes.py +++ b/src/oidcop/scopes.py @@ -25,14 +25,6 @@ } -def available_scopes(endpoint_context): - _supported = endpoint_context.provider_info.get("scopes_supported") - if _supported: - return [s for s in endpoint_context.scope2claims.keys() if s in _supported] - else: - return [s for s in endpoint_context.scope2claims.keys()] - - def convert_scopes2claims(scopes, allowed_claims=None, scope2claim_map=None): scope2claim_map = scope2claim_map or SCOPE2CLAIMS @@ -53,26 +45,55 @@ def convert_scopes2claims(scopes, allowed_claims=None, scope2claim_map=None): class Scopes: - def __init__(self): - pass + def __init__(self, server_get, allowed_scopes=None, scopes_mapping=None): + self.server_get = server_get + if not scopes_mapping: + scopes_mapping = dict(SCOPE2CLAIMS) + self.scopes_mapping = scopes_mapping + if not allowed_scopes: + allowed_scopes = list(scopes_mapping.keys()) + self.allowed_scopes = allowed_scopes - def allowed_scopes(self, client_id, endpoint_context): + def get_allowed_scopes(self, client_id=None): """ Returns the set of scopes that a specific client can use. :param client_id: The client identifier - :param endpoint_context: A EndpointContext instance :returns: List of scope names. Can be empty. """ - _cli = endpoint_context.cdb.get(client_id) - if _cli is not None: - _scopes = _cli.get("allowed_scopes") - if _scopes: - return _scopes - else: - return available_scopes(endpoint_context) - return [] - - def filter_scopes(self, client_id, endpoint_context, scopes): - allowed_scopes = self.allowed_scopes(client_id, endpoint_context) + allowed_scopes = self.allowed_scopes + if client_id: + client = self.server_get("endpoint_context").cdb.get(client_id) + if client is not None: + if "allowed_scopes" in client: + allowed_scopes = client.get("allowed_scopes") + elif "scopes_mapping" in client: + allowed_scopes = list(client.get("scopes_mapping").keys()) + + return allowed_scopes + + def get_scopes_mapping(self, client_id=None): + """ + Returns the mapping of scopes to claims fora specific client. + + :param client_id: The client identifier + :returns: Dict of scopes to claims. Can be empty. + """ + scopes_mapping = self.scopes_mapping + if client_id: + client = self.server_get("endpoint_context").cdb.get(client_id) + if client is not None: + scopes_mapping = client.get("scopes_mapping", scopes_mapping) + return scopes_mapping + + def filter_scopes(self, scopes, client_id=None): + allowed_scopes = self.get_allowed_scopes(client_id) return [s for s in scopes if s in allowed_scopes] + + def scopes_to_claims(self, scopes, scopes_mapping=None, client_id=None): + if not scopes_mapping: + scopes_mapping = self.get_scopes_mapping(client_id) + + scopes = self.filter_scopes(scopes, client_id) + + return convert_scopes2claims(scopes, scope2claim_map=scopes_mapping) diff --git a/src/oidcop/server.py b/src/oidcop/server.py index 99349fbe..c7d698ea 100644 --- a/src/oidcop/server.py +++ b/src/oidcop/server.py @@ -67,7 +67,12 @@ def __init__( ImpExp.__init__(self) self.conf = conf self.endpoint_context = EndpointContext( - conf=conf, keyjar=keyjar, cwd=cwd, cookie_handler=cookie_handler, httpc=httpc, + conf=conf, + server_get=self.server_get, + keyjar=keyjar, + cwd=cwd, + cookie_handler=cookie_handler, + httpc=httpc, ) self.endpoint_context.authz = self.do_authz() diff --git a/src/oidcop/session/claims.py b/src/oidcop/session/claims.py index 5ac1991c..3c82fc73 100755 --- a/src/oidcop/session/claims.py +++ b/src/oidcop/session/claims.py @@ -4,8 +4,8 @@ from oidcmsg.oidc import OpenIDSchema -from oidcop.exception import ServiceError from oidcop.exception import ImproperlyConfigured +from oidcop.exception import ServiceError from oidcop.scopes import convert_scopes2claims logger = logging.getLogger(__name__) @@ -113,9 +113,7 @@ def get_claims(self, session_id: str, scopes: str, claims_release_point: str) -> if add_claims_by_scope: if scopes: - _scopes = _context.scopes_handler.filter_scopes(client_id, _context, scopes) - - _claims = convert_scopes2claims(_scopes, scope2claim_map=_context.scope2claims) + _claims = _context.scopes_handler.scopes_to_claims(scopes, client_id=client_id) claims.update(_claims) # Bring in claims specification from the authorization request diff --git a/tests/test_07_userinfo.py b/tests/test_07_userinfo.py index 486614a2..5d6f821d 100644 --- a/tests/test_07_userinfo.py +++ b/tests/test_07_userinfo.py @@ -1,11 +1,11 @@ import json import os -from oidcop.configure import OPConfiguration import pytest from oidcmsg.oidc import OpenIDRequest from oidcop.authn_event import create_authn_event +from oidcop.configure import OPConfiguration from oidcop.oidc import userinfo from oidcop.oidc.authorization import Authorization from oidcop.oidc.provider_config import ProviderConfiguration @@ -388,83 +388,89 @@ def test_collect_user_info_enable_claims_per_client(self): class TestCollectUserInfoCustomScopes: - @pytest.fixture(autouse=True) - def create_endpoint_context(self): - self.server = Server( - { - "userinfo": {"class": UserInfo, "kwargs": {"db": USERINFO_DB}}, - "password": "we didn't start the fire", - "issuer": "https://example.com/op", - "claims_interface": {"class": "oidcop.session.claims.OAuth2ClaimsInterface", - "kwargs": {}}, - "endpoint": { - "provider_config": { - "path": "{}/.well-known/openid-configuration", - "class": ProviderConfiguration, - "kwargs": {}, - }, - "registration": { - "path": "{}/registration", - "class": Registration, - "kwargs": {}, - }, - "authorization": { - "path": "{}/authorization", - "class": Authorization, - "kwargs": { - "response_types_supported": [ - " ".join(x) for x in RESPONSE_TYPES_SUPPORTED - ], - "response_modes_supported": ["query", "fragment", "form_post",], - "claims_parameter_supported": True, - "request_parameter_supported": True, - "request_uri_parameter_supported": True, - }, - }, - "userinfo": { - "path": "userinfo", - "class": userinfo.UserInfo, - "kwargs": { - "claim_types_supported": ["normal", "aggregated", "distributed",], - "client_authn_method": ["bearer_header"], - "base_claims": {"eduperson_scoped_affiliation": None, "email": None,}, - "add_claims_by_scope": True, - "enable_claims_per_client": True, - }, - }, + @pytest.fixture + def conf(self): + return { + "userinfo": {"class": UserInfo, "kwargs": {"db": USERINFO_DB}}, + "password": "we didn't start the fire", + "issuer": "https://example.com/op", + "claims_interface": {"class": "oidcop.session.claims.ClaimsInterface", "kwargs": {}}, + "endpoint": { + "provider_config": { + "path": "{}/.well-known/openid-configuration", + "class": ProviderConfiguration, + "kwargs": {}, }, - "add_on": { - "custom_scopes": { - "function": "oidcop.oidc.add_on.custom_scopes.add_custom_scopes", - "kwargs": { - "research_and_scholarship": [ - "name", - "given_name", - "family_name", - "email", - "email_verified", - "sub", - "iss", - "eduperson_scoped_affiliation", - ] - }, - } + "registration": { + "path": "{}/registration", + "class": Registration, + "kwargs": {}, }, - "keys": { - "public_path": "jwks.json", - "key_defs": KEYDEFS, - "uri_path": "static/jwks.json", + "authorization": { + "path": "{}/authorization", + "class": Authorization, + "kwargs": { + "response_types_supported": [" ".join(x) for x in RESPONSE_TYPES_SUPPORTED], + "response_modes_supported": [ + "query", + "fragment", + "form_post", + ], + "claims_parameter_supported": True, + "request_parameter_supported": True, + "request_uri_parameter_supported": True, + }, }, - "authentication": { - "anon": { - "acr": INTERNETPROTOCOLPASSWORD, - "class": "oidcop.user_authn.user.NoAuthn", - "kwargs": {"user": "diana"}, - } + "userinfo": { + "path": "userinfo", + "class": userinfo.UserInfo, + "kwargs": { + "claim_types_supported": [ + "normal", + "aggregated", + "distributed", + ], + "client_authn_method": ["bearer_header"], + "base_claims": { + "eduperson_scoped_affiliation": None, + "email": None, + }, + "add_claims_by_scope": True, + "enable_claims_per_client": True, + }, }, - "template_dir": "template", - } - ) + }, + "scopes_mapping": { + "openid": ["sub"], + "research_and_scholarship": [ + "name", + "given_name", + "family_name", + "email", + "email_verified", + "sub", + "iss", + "eduperson_scoped_affiliation", + ], + }, + "keys": { + "public_path": "jwks.json", + "key_defs": KEYDEFS, + "uri_path": "static/jwks.json", + }, + "authentication": { + "anon": { + "acr": INTERNETPROTOCOLPASSWORD, + "class": "oidcop.user_authn.user.NoAuthn", + "kwargs": {"user": "diana"}, + } + }, + "template_dir": "template", + } + + @pytest.fixture(autouse=True) + def create_endpoint_context(self, conf): + self.server = Server(conf) self.endpoint_context = self.server.endpoint_context self.endpoint_context.cdb["client1"] = {} self.session_manager = self.endpoint_context.session_manager @@ -513,3 +519,66 @@ def test_collect_user_info_custom_scope(self): "given_name": "Diana", "name": "Diana Krall", } + + def test_collect_user_info_scope_mapping_per_client(self, conf): + conf["scopes_mapping"] = SCOPE2CLAIMS + server = Server(conf) + endpoint_context = server.endpoint_context + self.session_manager = endpoint_context.session_manager + claims_interface = endpoint_context.claims_interface + endpoint_context.cdb["client1"] = { + "scopes_mapping": { + "openid": ["sub"], + "research_and_scholarship": [ + "name", + "given_name", + "family_name", + "email", + "email_verified", + "sub", + "iss", + "eduperson_scoped_affiliation", + ], + } + } + + _req = OIDR.copy() + _req["scope"] = "openid research_and_scholarship" + del _req["claims"] + + session_id = self._create_session(_req) + + _restriction = claims_interface.get_claims( + session_id=session_id, scopes=_req["scope"], claims_release_point="userinfo" + ) + + res = claims_interface.get_user_claims("diana", _restriction) + + assert res == { + "eduperson_scoped_affiliation": ["staff@example.org"], + "email": "diana@example.org", + "email_verified": False, + "family_name": "Krall", + "given_name": "Diana", + "name": "Diana Krall", + } + + def test_collect_user_info_allowed_scopes_per_client(self): + self.endpoint_context.cdb["client1"] = {"allowed_scopes": {"openid"}} + + _req = OIDR.copy() + _req["scope"] = "openid research_and_scholarship" + del _req["claims"] + + session_id = self._create_session(_req) + + _restriction = self.claims_interface.get_claims( + session_id=session_id, scopes=_req["scope"], claims_release_point="userinfo" + ) + + res = self.claims_interface.get_user_claims("diana", _restriction) + + assert res == { + "eduperson_scoped_affiliation": ["staff@example.org"], + "email": "diana@example.org", + } diff --git a/tests/test_22_oidc_provider_config_endpoint.py b/tests/test_22_oidc_provider_config_endpoint.py index df950023..96424e8a 100755 --- a/tests/test_22_oidc_provider_config_endpoint.py +++ b/tests/test_22_oidc_provider_config_endpoint.py @@ -1,9 +1,9 @@ import json import os -from oidcop.configure import OPConfiguration import pytest +from oidcop.configure import OPConfiguration from oidcop.oidc.provider_config import ProviderConfiguration from oidcop.oidc.token import Token from oidcop.server import Server @@ -50,9 +50,9 @@ class TestEndpoint(object): - @pytest.fixture(autouse=True) - def create_endpoint(self): - conf = { + @pytest.fixture + def conf(self): + return { "issuer": "https://example.com/", "password": "mycket hemligt", "verify_ssl": False, @@ -68,6 +68,9 @@ def create_endpoint(self): }, "template_dir": "template", } + + @pytest.fixture(autouse=True) + def create_endpoint(self, conf): server = Server(OPConfiguration(conf=conf, base_path=BASEDIR), cwd=BASEDIR) self.endpoint_context = server.endpoint_context @@ -104,3 +107,32 @@ def test_do_response(self): "birthdate", } assert ("Content-type", "application/json; charset=utf-8") in msg["http_headers"] + + def test_advertised_scopes(self, conf): + scopes_supported = ["openid", "random", "profile"] + conf["capabilities"]["scopes_supported"] = scopes_supported + + server = Server(OPConfiguration(conf=conf, base_path=BASEDIR), cwd=BASEDIR) + endpoint = server.server_get("endpoint", "provider_config") + args = endpoint.process_request() + msg = endpoint.do_response(args["response_args"]) + assert isinstance(msg, dict) + _msg = json.loads(msg["response"]) + assert set(_msg["scopes_supported"]) == set(scopes_supported) + assert set(_msg["claims_supported"]) == { + "zoneinfo", + "gender", + "sub", + "middle_name", + "given_name", + "nickname", + "preferred_username", + "name", + "updated_at", + "birthdate", + "locale", + "profile", + "family_name", + "picture", + "website", + } diff --git a/tests/test_24_oauth2_token_endpoint.py b/tests/test_24_oauth2_token_endpoint.py index 0f2bf2fc..66e605c6 100644 --- a/tests/test_24_oauth2_token_endpoint.py +++ b/tests/test_24_oauth2_token_endpoint.py @@ -1,6 +1,7 @@ import json import os +import pytest from cryptojwt import JWT from cryptojwt.key_jar import build_keyjar from oidcmsg.oidc import AccessTokenRequest @@ -8,7 +9,6 @@ from oidcmsg.oidc import RefreshAccessTokenRequest from oidcmsg.oidc import TokenErrorResponse from oidcmsg.time_util import utc_time_sans_frac -import pytest from oidcop import JWT_BEARER from oidcop.authn_event import create_authn_event @@ -361,9 +361,7 @@ def test_do_refresh_access_token(self): def test_refresh_grant_disallowed_per_client(self): areq = AUTH_REQ.copy() areq["scope"] = ["email"] - self.endpoint_context.cdb["client_1"]["grant_types_supported"] = [ - "authorization_code" - ] + self.endpoint_context.cdb["client_1"]["grant_types_supported"] = ["authorization_code"] session_id = self._create_session(areq) grant = self.endpoint_context.authz(session_id, areq) diff --git a/tests/test_26_oidc_userinfo_endpoint.py b/tests/test_26_oidc_userinfo_endpoint.py index 03db905a..2577a450 100755 --- a/tests/test_26_oidc_userinfo_endpoint.py +++ b/tests/test_26_oidc_userinfo_endpoint.py @@ -1,11 +1,11 @@ import json import os +import pytest from oidcmsg.oauth2 import ResponseMessage from oidcmsg.oidc import AccessTokenRequest from oidcmsg.oidc import AuthorizationRequest from oidcmsg.time_util import time_sans_frac -import pytest from oidcop import user_info from oidcop.authn_event import create_authn_event @@ -17,6 +17,7 @@ from oidcop.oidc.provider_config import ProviderConfiguration from oidcop.oidc.registration import Registration from oidcop.oidc.token import Token +from oidcop.scopes import SCOPE2CLAIMS from oidcop.server import Server from oidcop.user_authn.authn_context import INTERNETPROTOCOLPASSWORD from oidcop.user_info import UserInfo @@ -148,35 +149,31 @@ def create_endpoint(self): }, "template_dir": "template", - "add_on": { - "custom_scopes": { - "function": "oidcop.oidc.add_on.custom_scopes.add_custom_scopes", - "kwargs": { - "research_and_scholarship": [ - "name", - "given_name", - "family_name", - "email", - "email_verified", - "sub", - "eduperson_scoped_affiliation", - ] - }, - } + "scopes_mapping": { + **SCOPE2CLAIMS, + "research_and_scholarship": [ + "name", + "given_name", + "family_name", + "email", + "email_verified", + "sub", + "eduperson_scoped_affiliation", + ], }, } - server = Server(OPConfiguration(conf=conf, base_path=BASEDIR), cwd=BASEDIR) + self.server = Server(OPConfiguration(conf=conf, base_path=BASEDIR), cwd=BASEDIR) - endpoint_context = server.endpoint_context - endpoint_context.cdb["client_1"] = { + self.endpoint_context = self.server.endpoint_context + self.endpoint_context.cdb["client_1"] = { "client_secret": "hemligt", "redirect_uris": [("https://example.com/cb", None)], "client_salt": "salted", "token_endpoint_auth_method": "client_secret_post", "response_types": ["code", "token", "code id_token", "id_token"], } - self.endpoint = server.server_get("endpoint", "userinfo") - self.session_manager = endpoint_context.session_manager + self.endpoint = self.server.server_get("endpoint", "userinfo") + self.session_manager = self.endpoint_context.session_manager self.user_id = "diana" def _create_session(self, auth_req, sub_type="public", sector_identifier="", @@ -320,7 +317,7 @@ def test_do_signed_response(self): res = self.endpoint.do_response(request=_req, **args) assert res - def test_custom_scope(self): + def test_scopes_mapping(self): _auth_req = AUTH_REQ.copy() _auth_req["scope"] = ["openid", "research_and_scholarship"] @@ -350,6 +347,109 @@ def test_custom_scope(self): "sub", } + def test_scopes_mapping_per_client(self): + self.endpoint_context.cdb["client_1"]["scopes_mapping"] = { + **SCOPE2CLAIMS, + "research_and_scholarship_2": [ + "name", + "given_name", + "family_name", + "email", + "email_verified", + "sub", + "eduperson_scoped_affiliation", + ], + } + + _auth_req = AUTH_REQ.copy() + _auth_req["scope"] = ["openid", "research_and_scholarship_2"] + + session_id = self._create_session(_auth_req) + grant = self.session_manager[session_id] + access_token = self._mint_token("access_token", grant, session_id) + + self.endpoint.kwargs["add_claims_by_scope"] = True + self.endpoint.server_get("endpoint_context").claims_interface.add_claims_by_scope = True + grant.claims = { + "userinfo": self.endpoint.server_get("endpoint_context").claims_interface.get_claims( + session_id=session_id, scopes=_auth_req["scope"], claims_release_point="userinfo" + ) + } + + http_info = {"headers": {"authorization": "Bearer {}".format(access_token.value)}} + _req = self.endpoint.parse_request({}, http_info=http_info) + args = self.endpoint.process_request(_req, http_info=http_info) + + assert set(args["response_args"].keys()) == { + "eduperson_scoped_affiliation", + "given_name", + "email_verified", + "email", + "family_name", + "name", + "sub", + } + + def test_allowed_scopes(self): + self.endpoint_context.scopes_handler.allowed_scopes = list(SCOPE2CLAIMS.keys()) + + _auth_req = AUTH_REQ.copy() + _auth_req["scope"] = ["openid", "research_and_scholarship"] + + session_id = self._create_session(_auth_req) + grant = self.session_manager[session_id] + access_token = self._mint_token("access_token", grant, session_id) + + self.endpoint.kwargs["add_claims_by_scope"] = True + self.endpoint.server_get("endpoint_context").claims_interface.add_claims_by_scope = True + grant.claims = { + "userinfo": self.endpoint.server_get("endpoint_context").claims_interface.get_claims( + session_id=session_id, scopes=_auth_req["scope"], claims_release_point="userinfo" + ) + } + + http_info = {"headers": {"authorization": "Bearer {}".format(access_token.value)}} + _req = self.endpoint.parse_request({}, http_info=http_info) + args = self.endpoint.process_request(_req, http_info=http_info) + + assert set(args["response_args"].keys()) == {"sub"} + + def test_allowed_scopes_per_client(self): + self.endpoint_context.cdb["client_1"]["scopes_mapping"] = { + **SCOPE2CLAIMS, + "research_and_scholarship_2": [ + "name", + "given_name", + "family_name", + "email", + "email_verified", + "sub", + "eduperson_scoped_affiliation", + ], + } + self.endpoint_context.cdb["client_1"]["allowed_scopes"] = list(SCOPE2CLAIMS.keys()) + + _auth_req = AUTH_REQ.copy() + _auth_req["scope"] = ["openid", "research_and_scholarship_2"] + + session_id = self._create_session(_auth_req) + grant = self.session_manager[session_id] + access_token = self._mint_token("access_token", grant, session_id) + + self.endpoint.kwargs["add_claims_by_scope"] = True + self.endpoint.server_get("endpoint_context").claims_interface.add_claims_by_scope = True + grant.claims = { + "userinfo": self.endpoint.server_get("endpoint_context").claims_interface.get_claims( + session_id=session_id, scopes=_auth_req["scope"], claims_release_point="userinfo" + ) + } + + http_info = {"headers": {"authorization": "Bearer {}".format(access_token.value)}} + _req = self.endpoint.parse_request({}, http_info=http_info) + args = self.endpoint.process_request(_req, http_info=http_info) + + assert set(args["response_args"].keys()) == {"sub"} + def test_wrong_type_of_token(self): _auth_req = AUTH_REQ.copy() _auth_req["scope"] = ["openid", "research_and_scholarship"] diff --git a/tests/test_50_persistence.py b/tests/test_50_persistence.py index 9052dd27..89f5c2ef 100644 --- a/tests/test_50_persistence.py +++ b/tests/test_50_persistence.py @@ -2,10 +2,10 @@ import os import shutil +import pytest from cryptojwt.jwt import utc_time_sans_frac from oidcmsg.oidc import AccessTokenRequest from oidcmsg.oidc import AuthorizationRequest -import pytest from oidcop import user_info from oidcop.authn_event import create_authn_event @@ -16,6 +16,7 @@ from oidcop.oidc.provider_config import ProviderConfiguration from oidcop.oidc.registration import Registration from oidcop.oidc.token import Token +from oidcop.scopes import SCOPE2CLAIMS from oidcop.server import Server from oidcop.user_authn.authn_context import INTERNETPROTOCOLPASSWORD from oidcop.user_info import UserInfo @@ -136,21 +137,17 @@ def full_path(local_file): } }, "template_dir": "template", - "add_on": { - "custom_scopes": { - "function": "oidcop.oidc.add_on.custom_scopes.add_custom_scopes", - "kwargs": { - "research_and_scholarship": [ - "name", - "given_name", - "family_name", - "email", - "email_verified", - "sub", - "eduperson_scoped_affiliation", - ] - }, - } + "scopes_mapping": { + **SCOPE2CLAIMS, + "research_and_scholarship": [ + "name", + "given_name", + "family_name", + "email", + "email_verified", + "sub", + "eduperson_scoped_affiliation", + ], }, "authz": { "class": AuthzHandling, @@ -401,8 +398,8 @@ def test_custom_scope(self): _auth_req = AUTH_REQ.copy() _auth_req["scope"] = ["openid", "research_and_scholarship"] - session_id = self._create_session(AUTH_REQ, index=2) - grant = self.endpoint[2].server_get("endpoint_context").authz(session_id, AUTH_REQ) + session_id = self._create_session(_auth_req, index=2) + grant = self.endpoint[2].server_get("endpoint_context").authz(session_id, _auth_req) self._dump_restore(2, 1) From fbd1c624e08bbb605a460b13cbaad06404e460f2 Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Mon, 20 Sep 2021 11:28:07 +0300 Subject: [PATCH 44/58] Add get_claims_from_request --- src/oidcop/session/claims.py | 91 +++++++++++++++----- tests/test_01_claims.py | 19 ++-- tests/test_06_session_manager.py | 5 +- tests/test_24_oidc_authorization_endpoint.py | 16 ++-- 4 files changed, 87 insertions(+), 44 deletions(-) diff --git a/src/oidcop/session/claims.py b/src/oidcop/session/claims.py index 3c82fc73..72f65cd7 100755 --- a/src/oidcop/session/claims.py +++ b/src/oidcop/session/claims.py @@ -31,12 +31,13 @@ class ClaimsInterface: def __init__(self, server_get): self.server_get = server_get - def authorization_request_claims(self, - session_id: str, - claims_release_point: Optional[str] = "") -> dict: - _grant = self.server_get("endpoint_context").session_manager.get_grant(session_id) - if _grant.authorization_request and "claims" in _grant.authorization_request: - return _grant.authorization_request["claims"].get(claims_release_point, {}) + def authorization_request_claims( + self, + authorization_request: dict, + claims_release_point: Optional[str] = "", + ) -> dict: + if authorization_request and "claims" in authorization_request: + return authorization_request["claims"].get(claims_release_point, {}) return {} @@ -70,16 +71,13 @@ def _get_module(self, usage, endpoint_context): return module - def get_claims(self, session_id: str, scopes: str, claims_release_point: str) -> dict: - """ - - :param session_id: Session identifier - :param scopes: Scopes - :param claims_release_point: Where to release the claims. One of - "userinfo"/"id_token"/"introspection"/"access_token" - :return: Claims specification as a dictionary. - """ - + def get_claims_from_request( + self, + auth_req: dict, + claims_release_point: str, + scopes: str = None, + client_id: str = None, + ): _context = self.server_get("endpoint_context") # which endpoint module configuration to get the base claims from module = self._get_module(claims_release_point, _context) @@ -89,7 +87,8 @@ def get_claims(self, session_id: str, scopes: str, claims_release_point: str) -> else: return {} - user_id, client_id, grant_id = _context.session_manager.decrypt_session_id(session_id) + if not client_id: + client_id = auth_req.get("client_id") # Can there be per client specification of which claims to use. if module.kwargs.get("enable_claims_per_client"): @@ -112,28 +111,76 @@ def get_claims(self, session_id: str, scopes: str, claims_release_point: str) -> add_claims_by_scope = module.kwargs.get("add_claims_by_scope") if add_claims_by_scope: + if scopes is None: + scopes = auth_req.get("scopes") if scopes: _claims = _context.scopes_handler.scopes_to_claims(scopes, client_id=client_id) claims.update(_claims) # Bring in claims specification from the authorization request # This only goes for ID Token and user info - request_claims = self.authorization_request_claims(session_id=session_id, - claims_release_point=claims_release_point) + request_claims = self.authorization_request_claims( + authorization_request=auth_req, + claims_release_point=claims_release_point + ) # This will add claims that has not be added before and - # set filters on those claims that also appears in one of the sources above + # set filters on those claims that also appears in one of the sources + # above if request_claims: claims.update(request_claims) return claims - def get_claims_all_usage(self, session_id: str, scopes: str) -> dict: + def get_claims(self, session_id: str, scopes: str, claims_release_point: str) -> dict: + """ + + :param session_id: Session identifier + :param scopes: Scopes + :param claims_release_point: Where to release the claims. One of + "userinfo"/"id_token"/"introspection"/"access_token" + :return: Claims specification as a dictionary. + """ + _context = self.server_get("endpoint_context") + session_info = _context.session_manager.get_session_info( + session_id, grant=True + ) + client_id = session_info["client_id"] + grant = session_info["grant"] + + if grant.authorization_request: + auth_req = grant.authorization_request + else: + auth_req = {} + claims = self.get_claims_from_request( + auth_req=auth_req, + claims_release_point=claims_release_point, + scopes=scopes, + client_id=client_id, + ) + + return claims + + def get_claims_all_usage_from_request( + self, auth_req: dict, scopes: str + ) -> dict: _claims = {} for usage in self.claims_release_points: - _claims[usage] = self.get_claims(session_id, scopes, usage) + _claims[usage] = self.get_claims_from_request( + auth_req, usage, scopes + ) return _claims + def get_claims_all_usage(self, session_id: str, scopes: str) -> dict: + grant = self.server_get( + "endpoint_context" + ).session_manager.get_grant(session_id) + if grant.authorization_request: + auth_req = grant.authorization_request + else: + auth_req = {} + return self.get_claims_all_usage_from_request(auth_req, scopes) + def get_user_claims(self, user_id: str, claims_restriction: dict) -> dict: """ diff --git a/tests/test_01_claims.py b/tests/test_01_claims.py index 4b027c45..ca60643c 100644 --- a/tests/test_01_claims.py +++ b/tests/test_01_claims.py @@ -141,33 +141,24 @@ def _create_session(self, auth_req, sub_type="public", sector_identifier=""): ) def test_authorization_request_id_token_claims(self): - session_id = self._create_session(AREQ) - - claims = self.claims_interface.authorization_request_claims(session_id, "id_token") + claims = self.claims_interface.authorization_request_claims(AREQ, "id_token") assert claims == {} def test_authorization_request_id_token_claims_2(self): - session_id = self._create_session(AREQ_2) - claims = self.claims_interface.authorization_request_claims(session_id, "id_token") + claims = self.claims_interface.authorization_request_claims(AREQ_2, "id_token") assert claims assert set(claims.keys()) == {"nickname"} def test_authorization_request_userinfo_claims(self): - session_id = self._create_session(AREQ) - - claims = self.claims_interface.authorization_request_claims(session_id, "userinfo") + claims = self.claims_interface.authorization_request_claims(AREQ, "userinfo") assert claims == {} def test_authorization_request_userinfo_claims_2(self): - session_id = self._create_session(AREQ_2) - - claims = self.claims_interface.authorization_request_claims(session_id, "userinfo") + claims = self.claims_interface.authorization_request_claims(AREQ_2, "userinfo") assert claims == {} def test_authorization_request_userinfo_claims_3(self): - session_id = self._create_session(AREQ_3) - - claims = self.claims_interface.authorization_request_claims(session_id, "userinfo") + claims = self.claims_interface.authorization_request_claims(AREQ_3, "userinfo") assert set(claims.keys()) == {"name", "email", "email_verified"} @pytest.mark.parametrize("usage", ["id_token", "userinfo", "introspection", "token"]) diff --git a/tests/test_06_session_manager.py b/tests/test_06_session_manager.py index 0fae6fa1..eaa189e8 100644 --- a/tests/test_06_session_manager.py +++ b/tests/test_06_session_manager.py @@ -200,11 +200,12 @@ def _mint_token(self, token_class, grant, session_id, based_on=None): ) def test_grant(self): - grant = Grant() + sid = self._create_session(AUTH_REQ) + grant = self.session_manager.get_grant(sid) assert grant.issued_token == [] assert grant.is_active() is True - code = self._mint_token("authorization_code", grant, self.dummy_session_id) + code = self._mint_token("authorization_code", grant, sid) assert isinstance(code, AuthorizationCode) assert code.is_active() assert len(grant.issued_token) == 1 diff --git a/tests/test_24_oidc_authorization_endpoint.py b/tests/test_24_oidc_authorization_endpoint.py index c877a069..a13aa0c2 100755 --- a/tests/test_24_oidc_authorization_endpoint.py +++ b/tests/test_24_oidc_authorization_endpoint.py @@ -825,14 +825,18 @@ def test_verify_response_type(self): @pytest.mark.parametrize("exp_in", [360, "360", 0]) def test_mint_token_exp_at(self, exp_in): - grant = Grant() - grant.usage_rules = {"authorization_code": {"expires_in": exp_in}} - - DUMMY_SESSION_ID = self.session_manager.encrypted_session_id( - "user_id", "client_id", "grant.id" + request = AuthorizationRequest( + client_id="client_1", + response_type=["code"], + redirect_uri="https://example.com/cb", + state="state", + scope="openid", ) + sid = self._create_session(request) + grant = self.session_manager.get_grant(sid) + grant.usage_rules = {"authorization_code": {"expires_in": exp_in}} - code = self.endpoint.mint_token("authorization_code", grant, DUMMY_SESSION_ID) + code = self.endpoint.mint_token("authorization_code", grant, sid) if exp_in in [360, "360"]: assert code.expires_at else: From 76623c003a22eee0f34fa683571cb519bda518c7 Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Mon, 20 Sep 2021 12:57:06 +0300 Subject: [PATCH 45/58] Minor fixes --- src/oidcop/session/claims.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/oidcop/session/claims.py b/src/oidcop/session/claims.py index 72f65cd7..2251b4bf 100755 --- a/src/oidcop/session/claims.py +++ b/src/oidcop/session/claims.py @@ -112,7 +112,7 @@ def get_claims_from_request( if add_claims_by_scope: if scopes is None: - scopes = auth_req.get("scopes") + scopes = auth_req.get("scope") if scopes: _claims = _context.scopes_handler.scopes_to_claims(scopes, client_id=client_id) claims.update(_claims) @@ -162,12 +162,12 @@ def get_claims(self, session_id: str, scopes: str, claims_release_point: str) -> return claims def get_claims_all_usage_from_request( - self, auth_req: dict, scopes: str + self, auth_req: dict, scopes: str = None, client_id: str = None ) -> dict: _claims = {} for usage in self.claims_release_points: _claims[usage] = self.get_claims_from_request( - auth_req, usage, scopes + auth_req, usage, scopes=scopes, client_id=client_id ) return _claims From fcb48905872b7734ef678bb451b89d4c6e1eaec4 Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Mon, 20 Sep 2021 14:47:26 +0300 Subject: [PATCH 46/58] Add allowed_scopes to README --- docs/source/contents/conf.rst | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/source/contents/conf.rst b/docs/source/contents/conf.rst index f8a97b1c..8ccf4c47 100644 --- a/docs/source/contents/conf.rst +++ b/docs/source/contents/conf.rst @@ -54,7 +54,6 @@ sub_funcs Optional. Functions involved in *sub*ject value creation. - scopes_mapping ############## @@ -75,6 +74,12 @@ allowed_scopes A list with the scopes that are allowed to be used (defaults to the keys in scopes_mapping). +advertised_scopes +################# + +A list with the scopes that will be advertised in the well-known endpoint (defaults to allowed_scopes). + + ------ add_on ------ From c0bb0cae4a4fdfdd59b0544a11af56c06834c559 Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Thu, 9 Sep 2021 12:44:01 +0300 Subject: [PATCH 47/58] Don't issue refresh token if not configured --- src/oidcop/oauth2/token.py | 15 ++++++++++++--- src/oidcop/oidc/token.py | 15 ++++++++++++--- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/oidcop/oauth2/token.py b/src/oidcop/oauth2/token.py index c06199fc..0a39dd21 100755 --- a/src/oidcop/oauth2/token.py +++ b/src/oidcop/oauth2/token.py @@ -118,11 +118,16 @@ def process_request(self, req: Union[Message, dict], **kwargs): return self.error_cls(error="invalid_request", error_description="Missing code") _session_info = _mngr.get_session_info_by_token(_access_code, grant=True) - if _session_info["client_id"] != req["client_id"]: - logger.debug("{} owner of token".format(_session_info["client_id"])) + client_id = _session_info["client_id"] + if client_id != req["client_id"]: + logger.debug("{} owner of token".format(client_id)) logger.warning("Client using token it was not given") return self.error_cls(error="invalid_grant", error_description="Wrong client") + if "grant_types_supported" in _context.cdb[client_id]: + grant_types_supported = _context.cdb[client_id].get("grant_types_supported") + else: + grant_types_supported = _context.provider_info["grant_types_supported"] grant = _session_info["grant"] _based_on = grant.get_token(_access_code) @@ -162,7 +167,11 @@ def process_request(self, req: Union[Message, dict], **kwargs): if token.expires_at: _response["expires_in"] = token.expires_at - utc_time_sans_frac() - if issue_refresh and "refresh_token" in _supports_minting: + if ( + issue_refresh + and "refresh_token" in _supports_minting + and "refresh_token" in grant_types_supported + ): try: refresh_token = self._mint_token( token_class="refresh_token", diff --git a/src/oidcop/oidc/token.py b/src/oidcop/oidc/token.py index 61c9e878..bf4f3e2b 100755 --- a/src/oidcop/oidc/token.py +++ b/src/oidcop/oidc/token.py @@ -45,11 +45,16 @@ def process_request(self, req: Union[Message, dict], **kwargs): _session_info = _mngr.get_session_info_by_token(_access_code, grant=True) logger.debug(f"Session info: {_session_info}") - if _session_info["client_id"] != req["client_id"]: - logger.debug("{} owner of token".format(_session_info["client_id"])) + client_id = _session_info["client_id"] + if client_id != req["client_id"]: + logger.debug("{} owner of token".format(client_id)) logger.warning("{} using token it was not given".format(req["client_id"])) return self.error_cls(error="invalid_grant", error_description="Wrong client") + if "grant_types_supported" in _context.cdb[client_id]: + grant_types_supported = _context.cdb[client_id].get("grant_types_supported") + else: + grant_types_supported = _context.provider_info["grant_types_supported"] grant = _session_info["grant"] token_type = "Bearer" @@ -110,7 +115,11 @@ def process_request(self, req: Union[Message, dict], **kwargs): if token.expires_at: _response["expires_in"] = token.expires_at - utc_time_sans_frac() - if issue_refresh and "refresh_token" in _supports_minting: + if ( + issue_refresh + and "refresh_token" in _supports_minting + and "refresh_token" in grant_types_supported + ): try: refresh_token = self._mint_token( token_class="refresh_token", From 5228f178137adb211cab9192cd0dc782b765b223 Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Fri, 10 Sep 2021 14:42:36 +0300 Subject: [PATCH 48/58] Fix tests --- tests/test_24_oauth2_token_endpoint.py | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/tests/test_24_oauth2_token_endpoint.py b/tests/test_24_oauth2_token_endpoint.py index 0f2bf2fc..bab5ad46 100644 --- a/tests/test_24_oauth2_token_endpoint.py +++ b/tests/test_24_oauth2_token_endpoint.py @@ -376,21 +376,7 @@ def test_refresh_grant_disallowed_per_client(self): _req = self.token_endpoint.parse_request(_token_request) _resp = self.token_endpoint.process_request(request=_req, issue_refresh=True) - _request = REFRESH_TOKEN_REQ.copy() - _request["refresh_token"] = _resp["response_args"]["refresh_token"] - - _token_value = _resp["response_args"]["refresh_token"] - _session_info = self.session_manager.get_session_info_by_token(_token_value) - _token = self.session_manager.find_token(_session_info["session_id"], _token_value) - _token.usage_rules["supports_minting"] = ["access_token", "refresh_token"] - - _req = self.token_endpoint.parse_request(_request.to_json()) - - assert isinstance(_req, TokenErrorResponse) - assert _req.to_dict() == { - "error": "invalid_request", - "error_description": "Unsupported grant_type: refresh_token", - } + assert "refresh_token" not in _resp def test_do_2nd_refresh_access_token(self): areq = AUTH_REQ.copy() From 1a9f4581ade4eab821cffb1bd6a224414cf8c6f3 Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Mon, 20 Sep 2021 18:40:10 +0300 Subject: [PATCH 49/58] Fix form post bug --- src/oidcop/oauth2/authorization.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/oidcop/oauth2/authorization.py b/src/oidcop/oauth2/authorization.py index c225a73a..dd689f60 100755 --- a/src/oidcop/oauth2/authorization.py +++ b/src/oidcop/oauth2/authorization.py @@ -71,6 +71,8 @@ def inputs(form_args): element = [] html_field = '' for name, value in form_args.items(): + if name == "scope" and isinstance(value, list): + value = " ".join(value) element.append(html_field.format(name, value)) return "\n".join(element) @@ -877,8 +879,9 @@ def authz_part2(self, request, session_id, **kwargs): resp_info["response_args"]["session_state"] = _session_state # Mix-Up mitigation - resp_info["response_args"]["iss"] = _context.issuer - resp_info["response_args"]["client_id"] = request["client_id"] + if "response_args" in resp_info: + resp_info["response_args"]["iss"] = _context.issuer + resp_info["response_args"]["client_id"] = request["client_id"] return resp_info From 9cb37c41220cca41f10d6f1d371d12b3e5bbfa8f Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Tue, 21 Sep 2021 13:25:24 +0300 Subject: [PATCH 50/58] Catch unhandled exception `get_session_info_by_token` might throw an exception, we catch it and return the appropriate error. --- src/oidcop/oidc/userinfo.py | 8 +++++++- tests/test_26_oidc_userinfo_endpoint.py | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/oidcop/oidc/userinfo.py b/src/oidcop/oidc/userinfo.py index aabc94d8..1abbbaa0 100755 --- a/src/oidcop/oidc/userinfo.py +++ b/src/oidcop/oidc/userinfo.py @@ -107,7 +107,13 @@ def do_response( def process_request(self, request=None, **kwargs): _mngr = self.server_get("endpoint_context").session_manager - _session_info = _mngr.get_session_info_by_token(request["access_token"], grant=True) + try: + _session_info = _mngr.get_session_info_by_token( + request["access_token"], grant=True + ) + except (KeyError, ValueError): + return self.error_cls(error="invalid_token", error_description="Invalid Token") + _grant = _session_info["grant"] token = _grant.get_token(request["access_token"]) # should be an access token diff --git a/tests/test_26_oidc_userinfo_endpoint.py b/tests/test_26_oidc_userinfo_endpoint.py index 2577a450..53d8d12c 100755 --- a/tests/test_26_oidc_userinfo_endpoint.py +++ b/tests/test_26_oidc_userinfo_endpoint.py @@ -482,6 +482,22 @@ def test_invalid_token(self): assert isinstance(args, ResponseMessage) assert args["error_description"] == "Invalid Token" + def test_invalid_token_2(self): + _auth_req = AUTH_REQ.copy() + _auth_req["scope"] = ["openid", "research_and_scholarship"] + + session_id = self._create_session(_auth_req) + grant = self.session_manager[session_id] + access_token = self._mint_token("access_token", grant, session_id) + self.session_manager.flush() + + http_info = {"headers": {"authorization": "Bearer {}".format(access_token.value)}} + _req = self.endpoint.parse_request({}, http_info=http_info) + args = self.endpoint.process_request(_req) + + assert isinstance(args, ResponseMessage) + assert args["error_description"] == "Invalid Token" + def test_expired_token(self, monkeypatch): _auth_req = AUTH_REQ.copy() _auth_req["scope"] = ["openid", "research_and_scholarship"] From 946f20f56ccb67a29b6ef2f39456963c2e7eb05c Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Tue, 21 Sep 2021 13:39:30 +0300 Subject: [PATCH 51/58] Rename advertised_scopes to scopes_supported --- docs/source/contents/conf.rst | 4 ++-- src/oidcop/configure.py | 4 ---- tests/test_22_oidc_provider_config_endpoint.py | 2 +- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/docs/source/contents/conf.rst b/docs/source/contents/conf.rst index 8ccf4c47..0696b88b 100644 --- a/docs/source/contents/conf.rst +++ b/docs/source/contents/conf.rst @@ -74,8 +74,8 @@ allowed_scopes A list with the scopes that are allowed to be used (defaults to the keys in scopes_mapping). -advertised_scopes -################# +scopes_supported +################ A list with the scopes that will be advertised in the well-known endpoint (defaults to allowed_scopes). diff --git a/src/oidcop/configure.py b/src/oidcop/configure.py index bde65666..6a17cb82 100755 --- a/src/oidcop/configure.py +++ b/src/oidcop/configure.py @@ -283,8 +283,6 @@ class OPConfiguration(EntityConfiguration): "login_hint_lookup": None, "sub_func": {}, "scopes_mapping": {}, - "scopes_supported": None, - "advertised_scopes": None, } ) @@ -306,8 +304,6 @@ def __init__( file_attributes=file_attributes, ) scopes_mapping = self.scopes_mapping - if "advertised_scopes" not in self: - self["advertised_scopes"] = list(scopes_mapping.keys()) class ASConfiguration(EntityConfiguration): diff --git a/tests/test_22_oidc_provider_config_endpoint.py b/tests/test_22_oidc_provider_config_endpoint.py index 96424e8a..a124cfa9 100755 --- a/tests/test_22_oidc_provider_config_endpoint.py +++ b/tests/test_22_oidc_provider_config_endpoint.py @@ -108,7 +108,7 @@ def test_do_response(self): } assert ("Content-type", "application/json; charset=utf-8") in msg["http_headers"] - def test_advertised_scopes(self, conf): + def test_scopes_supported(self, conf): scopes_supported = ["openid", "random", "profile"] conf["capabilities"]["scopes_supported"] = scopes_supported From ddb861b536ea07716441ffb6e00656f49d2c8751 Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Tue, 21 Sep 2021 13:46:57 +0300 Subject: [PATCH 52/58] Rename scopes_mapping to scopes_to_claims --- docs/source/contents/conf.rst | 8 +++---- src/oidcop/configure.py | 6 +++--- src/oidcop/endpoint_context.py | 2 +- src/oidcop/oidc/add_on/custom_scopes.py | 4 ++-- src/oidcop/scopes.py | 28 ++++++++++++------------- tests/test_07_userinfo.py | 6 +++--- tests/test_26_oidc_userinfo_endpoint.py | 10 ++++----- tests/test_50_persistence.py | 2 +- 8 files changed, 33 insertions(+), 33 deletions(-) diff --git a/docs/source/contents/conf.rst b/docs/source/contents/conf.rst index 0696b88b..19cd854d 100644 --- a/docs/source/contents/conf.rst +++ b/docs/source/contents/conf.rst @@ -54,7 +54,7 @@ sub_funcs Optional. Functions involved in *sub*ject value creation. -scopes_mapping +scopes_to_claims ############## A dict defining the scopes that are allowed to be used per client and the claims @@ -71,7 +71,7 @@ simply map it to an empty list. E.g.:: allowed_scopes ############## -A list with the scopes that are allowed to be used (defaults to the keys in scopes_mapping). +A list with the scopes that are allowed to be used (defaults to the keys in scopes_to_claims). scopes_supported @@ -736,7 +736,7 @@ grant_types_supported Configure the allowed grant types on the token endpoint. -------------- -scopes_mapping +scopes_to_claims -------------- A dict defining the scopes that are allowed to be used per client and the claims @@ -753,4 +753,4 @@ allowed_scopes -------------- A list with the scopes that are allowed to be used (defaults to the keys in the -clients scopes_mapping). +clients scopes_to_claims). diff --git a/src/oidcop/configure.py b/src/oidcop/configure.py index 6a17cb82..fb637827 100755 --- a/src/oidcop/configure.py +++ b/src/oidcop/configure.py @@ -79,7 +79,7 @@ "refresh": {"class": "oidcop.token.jwt_token.JWTToken", "kwargs": {"lifetime": 86400}, }, "id_token": {"class": "oidcop.token.id_token.IDToken", "kwargs": {}}, }, - "scopes_mapping": SCOPE2CLAIMS, + "scopes_to_claims": SCOPE2CLAIMS, } AS_DEFAULT_CONFIG = copy.deepcopy(OP_DEFAULT_CONFIG) @@ -282,7 +282,7 @@ class OPConfiguration(EntityConfiguration): "login_hint2acrs": {}, "login_hint_lookup": None, "sub_func": {}, - "scopes_mapping": {}, + "scopes_to_claims": {}, } ) @@ -303,7 +303,7 @@ def __init__( port=port, file_attributes=file_attributes, ) - scopes_mapping = self.scopes_mapping + scopes_to_claims = self.scopes_to_claims class ASConfiguration(EntityConfiguration): diff --git a/src/oidcop/endpoint_context.py b/src/oidcop/endpoint_context.py index 6285abe7..51629b67 100755 --- a/src/oidcop/endpoint_context.py +++ b/src/oidcop/endpoint_context.py @@ -257,7 +257,7 @@ def set_scopes_handler(self): self.scopes_handler = Scopes( self.server_get, allowed_scopes=self.conf.get("allowed_scopes"), - scopes_mapping=self.conf.get("scopes_mapping"), + scopes_to_claims=self.conf.get("scopes_to_claims"), ) def do_add_on(self, endpoints): diff --git a/src/oidcop/oidc/add_on/custom_scopes.py b/src/oidcop/oidc/add_on/custom_scopes.py index 2fbc6a32..e3981e18 100644 --- a/src/oidcop/oidc/add_on/custom_scopes.py +++ b/src/oidcop/oidc/add_on/custom_scopes.py @@ -11,7 +11,7 @@ def add_custom_scopes(endpoint, **kwargs): """ # Just need an endpoint, anyone will do LOGGER.warning( - "The custom_scopes add on is deprecated. The `scopes_mapping` config " + "The custom_scopes add on is deprecated. The `scopes_to_claims` config " "option should be used instead." ) _endpoint = list(endpoint.values())[0] @@ -19,7 +19,7 @@ def add_custom_scopes(endpoint, **kwargs): _scopes2claims = SCOPE2CLAIMS.copy() _scopes2claims.update(kwargs) _context = _endpoint.server_get("endpoint_context") - _context.scopes_handler.scopes_mapping = _scopes2claims + _context.scopes_handler.scopes_to_claims = _scopes2claims pi = _context.provider_info _scopes = set(pi.get("scopes_supported", [])) diff --git a/src/oidcop/scopes.py b/src/oidcop/scopes.py index 87d2fe80..a42a93ce 100644 --- a/src/oidcop/scopes.py +++ b/src/oidcop/scopes.py @@ -45,13 +45,13 @@ def convert_scopes2claims(scopes, allowed_claims=None, scope2claim_map=None): class Scopes: - def __init__(self, server_get, allowed_scopes=None, scopes_mapping=None): + def __init__(self, server_get, allowed_scopes=None, scopes_to_claims=None): self.server_get = server_get - if not scopes_mapping: - scopes_mapping = dict(SCOPE2CLAIMS) - self.scopes_mapping = scopes_mapping + if not scopes_to_claims: + scopes_to_claims = dict(SCOPE2CLAIMS) + self._scopes_to_claims = scopes_to_claims if not allowed_scopes: - allowed_scopes = list(scopes_mapping.keys()) + allowed_scopes = list(scopes_to_claims.keys()) self.allowed_scopes = allowed_scopes def get_allowed_scopes(self, client_id=None): @@ -67,8 +67,8 @@ def get_allowed_scopes(self, client_id=None): if client is not None: if "allowed_scopes" in client: allowed_scopes = client.get("allowed_scopes") - elif "scopes_mapping" in client: - allowed_scopes = list(client.get("scopes_mapping").keys()) + elif "scopes_to_claims" in client: + allowed_scopes = list(client.get("scopes_to_claims").keys()) return allowed_scopes @@ -79,21 +79,21 @@ def get_scopes_mapping(self, client_id=None): :param client_id: The client identifier :returns: Dict of scopes to claims. Can be empty. """ - scopes_mapping = self.scopes_mapping + scopes_to_claims = self._scopes_to_claims if client_id: client = self.server_get("endpoint_context").cdb.get(client_id) if client is not None: - scopes_mapping = client.get("scopes_mapping", scopes_mapping) - return scopes_mapping + scopes_to_claims = client.get("scopes_to_claims", scopes_to_claims) + return scopes_to_claims def filter_scopes(self, scopes, client_id=None): allowed_scopes = self.get_allowed_scopes(client_id) return [s for s in scopes if s in allowed_scopes] - def scopes_to_claims(self, scopes, scopes_mapping=None, client_id=None): - if not scopes_mapping: - scopes_mapping = self.get_scopes_mapping(client_id) + def scopes_to_claims(self, scopes, scopes_to_claims=None, client_id=None): + if not scopes_to_claims: + scopes_to_claims = self.get_scopes_mapping(client_id) scopes = self.filter_scopes(scopes, client_id) - return convert_scopes2claims(scopes, scope2claim_map=scopes_mapping) + return convert_scopes2claims(scopes, scope2claim_map=scopes_to_claims) diff --git a/tests/test_07_userinfo.py b/tests/test_07_userinfo.py index 5d6f821d..9f1142e6 100644 --- a/tests/test_07_userinfo.py +++ b/tests/test_07_userinfo.py @@ -440,7 +440,7 @@ def conf(self): }, }, }, - "scopes_mapping": { + "scopes_to_claims": { "openid": ["sub"], "research_and_scholarship": [ "name", @@ -521,13 +521,13 @@ def test_collect_user_info_custom_scope(self): } def test_collect_user_info_scope_mapping_per_client(self, conf): - conf["scopes_mapping"] = SCOPE2CLAIMS + conf["scopes_to_claims"] = SCOPE2CLAIMS server = Server(conf) endpoint_context = server.endpoint_context self.session_manager = endpoint_context.session_manager claims_interface = endpoint_context.claims_interface endpoint_context.cdb["client1"] = { - "scopes_mapping": { + "scopes_to_claims": { "openid": ["sub"], "research_and_scholarship": [ "name", diff --git a/tests/test_26_oidc_userinfo_endpoint.py b/tests/test_26_oidc_userinfo_endpoint.py index 2577a450..5a4453c0 100755 --- a/tests/test_26_oidc_userinfo_endpoint.py +++ b/tests/test_26_oidc_userinfo_endpoint.py @@ -149,7 +149,7 @@ def create_endpoint(self): }, "template_dir": "template", - "scopes_mapping": { + "scopes_to_claims": { **SCOPE2CLAIMS, "research_and_scholarship": [ "name", @@ -317,7 +317,7 @@ def test_do_signed_response(self): res = self.endpoint.do_response(request=_req, **args) assert res - def test_scopes_mapping(self): + def test_scopes_to_claims(self): _auth_req = AUTH_REQ.copy() _auth_req["scope"] = ["openid", "research_and_scholarship"] @@ -347,8 +347,8 @@ def test_scopes_mapping(self): "sub", } - def test_scopes_mapping_per_client(self): - self.endpoint_context.cdb["client_1"]["scopes_mapping"] = { + def test_scopes_to_claims_per_client(self): + self.endpoint_context.cdb["client_1"]["scopes_to_claims"] = { **SCOPE2CLAIMS, "research_and_scholarship_2": [ "name", @@ -415,7 +415,7 @@ def test_allowed_scopes(self): assert set(args["response_args"].keys()) == {"sub"} def test_allowed_scopes_per_client(self): - self.endpoint_context.cdb["client_1"]["scopes_mapping"] = { + self.endpoint_context.cdb["client_1"]["scopes_to_claims"] = { **SCOPE2CLAIMS, "research_and_scholarship_2": [ "name", diff --git a/tests/test_50_persistence.py b/tests/test_50_persistence.py index 89f5c2ef..d53148fc 100644 --- a/tests/test_50_persistence.py +++ b/tests/test_50_persistence.py @@ -137,7 +137,7 @@ def full_path(local_file): } }, "template_dir": "template", - "scopes_mapping": { + "scopes_to_claims": { **SCOPE2CLAIMS, "research_and_scholarship": [ "name", From 16e99e6f26acff81f1bd624a097346c3029310cf Mon Sep 17 00:00:00 2001 From: Kostis Triantafyllakis Date: Wed, 22 Sep 2021 14:36:03 +0300 Subject: [PATCH 53/58] Add parameter to revoke old refresh token upon issuing new --- docs/source/contents/conf.rst | 10 +++- src/oidcop/oauth2/token.py | 12 ++++ src/oidcop/oidc/token.py | 33 +++++++---- tests/test_24_oauth2_token_endpoint.py | 82 +++++++++++++++++++++++++- tests/test_35_oidc_token_endpoint.py | 82 +++++++++++++++++++++++++- 5 files changed, 200 insertions(+), 19 deletions(-) diff --git a/docs/source/contents/conf.rst b/docs/source/contents/conf.rst index 19cd854d..4a7121a7 100644 --- a/docs/source/contents/conf.rst +++ b/docs/source/contents/conf.rst @@ -339,8 +339,9 @@ An example:: "client_secret_post", "client_secret_basic", "client_secret_jwt", - "private_key_jwt" - ] + "private_key_jwt", + ], + "revoke_refresh_on_issue": True } }, "userinfo": { @@ -754,3 +755,8 @@ allowed_scopes A list with the scopes that are allowed to be used (defaults to the keys in the clients scopes_to_claims). + +----------------------- +revoke_refresh_on_issue +----------------------- +Configure whether to revoke the refresh token that was used to issue a new refresh token diff --git a/src/oidcop/oauth2/token.py b/src/oidcop/oauth2/token.py index c06199fc..cbd8b30f 100755 --- a/src/oidcop/oauth2/token.py +++ b/src/oidcop/oauth2/token.py @@ -287,6 +287,17 @@ def process_request(self, req: Union[Message, dict], **kwargs): token.register_usage() + if ("client_id" in req + and req["client_id"] in _context.cdb + and "revoke_refresh_on_issue" in _context.cdb[req["client_id"]] + ): + revoke_refresh = _context.cdb[req["client_id"]].get("revoke_refresh_on_issue") + else: + revoke_refresh = self.endpoint.revoke_refresh_on_issue + + if revoke_refresh: + token.revoke() + return _resp def post_parse_request( @@ -365,6 +376,7 @@ def __init__(self, server_get, new_refresh_token=False, **kwargs): self.allow_refresh = False self.new_refresh_token = new_refresh_token self.configure_grant_types(kwargs.get("grant_types_supported")) + self.revoke_refresh_on_issue = kwargs.get("revoke_refresh_on_issue", False) def configure_grant_types(self, grant_types_supported): if grant_types_supported is None: diff --git a/src/oidcop/oidc/token.py b/src/oidcop/oidc/token.py index 61c9e878..19977f5e 100755 --- a/src/oidcop/oidc/token.py +++ b/src/oidcop/oidc/token.py @@ -81,12 +81,10 @@ def process_request(self, req: Union[Message, dict], **kwargs): logger.debug("All checks OK") - issue_refresh = False - if "issue_refresh" in kwargs: - issue_refresh = kwargs["issue_refresh"] - else: - if "offline_access" in grant.scope: - issue_refresh = True + issue_refresh = kwargs.get("issue_refresh", None) + # The existence of offline_access scope overwrites issue_refresh + if issue_refresh is None and "offline_access" in grant.scope: + issue_refresh = True _response = { "token_type": token_type, @@ -242,12 +240,12 @@ def process_request(self, req: Union[Message, dict], **kwargs): _resp["expires_in"] = access_token.expires_at - utc_time_sans_frac() _mints = token.usage_rules.get("supports_minting") - issue_refresh = False - if "issue_refresh" in kwargs: - issue_refresh = kwargs["issue_refresh"] - else: - if "offline_access" in scope: - issue_refresh = True + + issue_refresh = kwargs.get("issue_refresh", None) + # The existence of offline_access scope overwrites issue_refresh + if issue_refresh is None and "offline_access" in scope: + issue_refresh = True + if "refresh_token" in _mints and issue_refresh: refresh_token = self._mint_token( token_class="refresh_token", @@ -281,6 +279,17 @@ def process_request(self, req: Union[Message, dict], **kwargs): token.register_usage() + if ("client_id" in req + and req["client_id"] in _context.cdb + and "revoke_refresh_on_issue" in _context.cdb[req["client_id"]] + ): + revoke_refresh = _context.cdb[req["client_id"]].get("revoke_refresh_on_issue") + else: + revoke_refresh = revoke_refresh = self.endpoint.revoke_refresh_on_issue + + if revoke_refresh: + token.revoke() + return _resp def post_parse_request( diff --git a/tests/test_24_oauth2_token_endpoint.py b/tests/test_24_oauth2_token_endpoint.py index 66e605c6..9f72fe0d 100644 --- a/tests/test_24_oauth2_token_endpoint.py +++ b/tests/test_24_oauth2_token_endpoint.py @@ -398,6 +398,7 @@ def test_do_2nd_refresh_access_token(self): grant = self.endpoint_context.authz(session_id, areq) code = self._mint_code(grant, areq["client_id"]) + self.token_endpoint.revoke_refresh_on_issue = False _cntx = self.endpoint_context _token_request = TOKEN_REQ_DICT.copy() @@ -423,8 +424,7 @@ def test_do_2nd_refresh_access_token(self): _2nd_request = REFRESH_TOKEN_REQ.copy() _2nd_request["refresh_token"] = _resp["response_args"]["refresh_token"] _2nd_req = self.token_endpoint.parse_request(_request.to_json()) - _2nd_resp = self.token_endpoint.process_request(request=_req, issue_refresh=True) - + _2nd_resp = self.token_endpoint.process_request(request=_2nd_req, issue_refresh=True) assert set(_2nd_resp.keys()) == {"cookie", "response_args", "http_headers"} assert set(_2nd_resp["response_args"].keys()) == { "access_token", @@ -475,6 +475,82 @@ def test_new_refresh_token(self, conf): assert first_refresh_token != second_refresh_token + def test_revoke_on_issue_refresh_token(self, conf): + self.endpoint_context.cdb["client_1"] = { + "client_secret": "hemligt", + "redirect_uris": [("https://example.com/cb", None)], + "client_salt": "salted", + "endpoint_auth_method": "client_secret_post", + "response_types": ["code", "token", "code id_token", "id_token"], + } + + self.token_endpoint.revoke_refresh_on_issue = True + areq = AUTH_REQ.copy() + areq["scope"] = ["email"] + + session_id = self._create_session(areq) + grant = self.endpoint_context.authz(session_id, areq) + code = self._mint_code(grant, areq["client_id"]) + + _token_request = TOKEN_REQ_DICT.copy() + _token_request["code"] = code.value + _req = self.token_endpoint.parse_request(_token_request) + _resp = self.token_endpoint.process_request(request=_req, issue_refresh=True) + assert "refresh_token" in _resp["response_args"] + first_refresh_token = _resp["response_args"]["refresh_token"] + + _refresh_request = REFRESH_TOKEN_REQ.copy() + _refresh_request["refresh_token"] = first_refresh_token + _2nd_req = self.token_endpoint.parse_request(_refresh_request.to_json()) + _2nd_resp = self.token_endpoint.process_request(request=_2nd_req, issue_refresh=True) + assert "refresh_token" in _2nd_resp["response_args"] + second_refresh_token = _2nd_resp["response_args"]["refresh_token"] + + assert first_refresh_token != second_refresh_token + first_refresh_token = grant.get_token(first_refresh_token) + second_refresh_token = grant.get_token(second_refresh_token) + assert first_refresh_token.revoked is True + assert second_refresh_token.revoked is False + + def test_revoke_on_issue_refresh_token_per_client(self, conf): + self.endpoint_context.cdb["client_1"] = { + "client_secret": "hemligt", + "redirect_uris": [("https://example.com/cb", None)], + "client_salt": "salted", + "endpoint_auth_method": "client_secret_post", + "response_types": ["code", "token", "code id_token", "id_token"], + } + self.endpoint_context.cdb[AUTH_REQ["client_id"]]["revoke_refresh_on_issue"] = True + areq = AUTH_REQ.copy() + areq["scope"] = ["openid", "offline_access"] + + session_id = self._create_session(areq) + grant = self.endpoint_context.authz(session_id, areq) + code = self._mint_code(grant, areq["client_id"]) + + _token_request = TOKEN_REQ_DICT.copy() + _token_request["code"] = code.value + _req = self.token_endpoint.parse_request(_token_request) + _resp = self.token_endpoint.process_request(request=_req, issue_refresh=True) + assert "refresh_token" in _resp["response_args"] + first_refresh_token = _resp["response_args"]["refresh_token"] + + _refresh_request = REFRESH_TOKEN_REQ.copy() + _refresh_request["refresh_token"] = first_refresh_token + _2nd_req = self.token_endpoint.parse_request(_refresh_request.to_json()) + _2nd_resp = self.token_endpoint.process_request(request=_2nd_req, issue_refresh=True) + assert "refresh_token" in _2nd_resp["response_args"] + second_refresh_token = _2nd_resp["response_args"]["refresh_token"] + + _2d_refresh_request = REFRESH_TOKEN_REQ.copy() + _2d_refresh_request["refresh_token"] = second_refresh_token + + assert first_refresh_token != second_refresh_token + first_refresh_token = grant.get_token(first_refresh_token) + second_refresh_token = grant.get_token(second_refresh_token) + assert first_refresh_token.revoked is True + assert second_refresh_token.revoked is False + def test_refresh_scopes(self): areq = AUTH_REQ.copy() areq["scope"] = ["email", "profile"] @@ -695,4 +771,4 @@ def test_refresh_token_request_other_client(self): assert isinstance(_resp, TokenErrorResponse) assert _resp.to_dict() == { "error": "invalid_grant", "error_description": "Wrong client" - } \ No newline at end of file + } diff --git a/tests/test_35_oidc_token_endpoint.py b/tests/test_35_oidc_token_endpoint.py index e4c203ea..66dd6ec0 100755 --- a/tests/test_35_oidc_token_endpoint.py +++ b/tests/test_35_oidc_token_endpoint.py @@ -117,7 +117,7 @@ def conf(): }, "refresh": { "class": "oidcop.token.jwt_token.JWTToken", - "kwargs": {"lifetime": 3600, "aud": ["https://example.org/appl"], }, + "kwargs": {"lifetime": 3600, "aud": ["https://example.org/appl"]}, }, "id_token": {"class": "oidcop.token.id_token.IDToken", "kwargs": {}}, }, @@ -390,7 +390,7 @@ def test_do_2nd_refresh_access_token(self): session_id = self._create_session(areq) grant = self.endpoint_context.authz(session_id, areq) code = self._mint_code(grant, areq["client_id"]) - + self.token_endpoint.revoke_refresh_on_issue = False _cntx = self.endpoint_context _token_request = TOKEN_REQ_DICT.copy() @@ -761,6 +761,84 @@ def test_new_refresh_token(self, conf): assert first_refresh_token != second_refresh_token + def test_revoke_on_issue_refresh_token(self, conf): + self.endpoint_context.cdb["client_1"] = { + "client_secret": "hemligt", + "redirect_uris": [("https://example.com/cb", None)], + "client_salt": "salted", + "endpoint_auth_method": "client_secret_post", + "response_types": ["code", "token", "code id_token", "id_token"], + } + self.token_endpoint.revoke_refresh_on_issue = True + areq = AUTH_REQ.copy() + areq["scope"] = ["openid", "offline_access"] + + session_id = self._create_session(areq) + grant = self.endpoint_context.authz(session_id, areq) + code = self._mint_code(grant, areq["client_id"]) + + _token_request = TOKEN_REQ_DICT.copy() + _token_request["code"] = code.value + _req = self.token_endpoint.parse_request(_token_request) + _resp = self.token_endpoint.process_request(request=_req, issue_refresh=True) + assert "refresh_token" in _resp["response_args"] + first_refresh_token = _resp["response_args"]["refresh_token"] + + _refresh_request = REFRESH_TOKEN_REQ.copy() + _refresh_request["refresh_token"] = first_refresh_token + _2nd_req = self.token_endpoint.parse_request(_refresh_request.to_json()) + _2nd_resp = self.token_endpoint.process_request(request=_2nd_req, issue_refresh=True) + assert "refresh_token" in _2nd_resp["response_args"] + second_refresh_token = _2nd_resp["response_args"]["refresh_token"] + + _2d_refresh_request = REFRESH_TOKEN_REQ.copy() + _2d_refresh_request["refresh_token"] = second_refresh_token + + assert first_refresh_token != second_refresh_token + first_refresh_token = grant.get_token(first_refresh_token) + second_refresh_token = grant.get_token(second_refresh_token) + assert first_refresh_token.revoked is True + assert second_refresh_token.revoked is False + + def test_revoke_on_issue_refresh_token_per_client(self, conf): + self.endpoint_context.cdb["client_1"] = { + "client_secret": "hemligt", + "redirect_uris": [("https://example.com/cb", None)], + "client_salt": "salted", + "endpoint_auth_method": "client_secret_post", + "response_types": ["code", "token", "code id_token", "id_token"], + } + self.endpoint_context.cdb[AUTH_REQ["client_id"]]["revoke_refresh_on_issue"] = True + areq = AUTH_REQ.copy() + areq["scope"] = ["openid", "offline_access"] + + session_id = self._create_session(areq) + grant = self.endpoint_context.authz(session_id, areq) + code = self._mint_code(grant, areq["client_id"]) + + _token_request = TOKEN_REQ_DICT.copy() + _token_request["code"] = code.value + _req = self.token_endpoint.parse_request(_token_request) + _resp = self.token_endpoint.process_request(request=_req, issue_refresh=True) + assert "refresh_token" in _resp["response_args"] + first_refresh_token = _resp["response_args"]["refresh_token"] + + _refresh_request = REFRESH_TOKEN_REQ.copy() + _refresh_request["refresh_token"] = first_refresh_token + _2nd_req = self.token_endpoint.parse_request(_refresh_request.to_json()) + _2nd_resp = self.token_endpoint.process_request(request=_2nd_req, issue_refresh=True) + assert "refresh_token" in _2nd_resp["response_args"] + second_refresh_token = _2nd_resp["response_args"]["refresh_token"] + + _2d_refresh_request = REFRESH_TOKEN_REQ.copy() + _2d_refresh_request["refresh_token"] = second_refresh_token + + assert first_refresh_token != second_refresh_token + first_refresh_token = grant.get_token(first_refresh_token) + second_refresh_token = grant.get_token(second_refresh_token) + assert first_refresh_token.revoked is True + assert second_refresh_token.revoked is False + def test_do_refresh_access_token_not_allowed(self): areq = AUTH_REQ.copy() areq["scope"] = ["openid", "offline_access"] From ed7e8497177bd4f2fb90bf81d75529297006e477 Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Thu, 9 Sep 2021 13:04:38 +0300 Subject: [PATCH 54/58] Add README --- docs/source/contents/conf.rst | 40 +++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/docs/source/contents/conf.rst b/docs/source/contents/conf.rst index 4a7121a7..6c64ca6e 100644 --- a/docs/source/contents/conf.rst +++ b/docs/source/contents/conf.rst @@ -759,4 +759,44 @@ clients scopes_to_claims). ----------------------- revoke_refresh_on_issue ----------------------- + Configure whether to revoke the refresh token that was used to issue a new refresh token + +---------- +add_claims +---------- + +A dictionary with the following keys + +always +###### + +A dictionary with the following keys: `userinfo`, `id_token`, `introspection`, `access_token`. +The keys are used to describe the claims we want to add to the corresponding interface. +The keys can be a list of claims to be added or a dict in the format described +in https://openid.net/specs/openid-connect-core-1_0.html#IndividualClaimsRequests +E.g.:: + + { + "add_claims": { + "always": { + "userinfo": ["email", "phone"], # Always add "email" and "phone" in the userinfo response if such claims exists + "id_token": {"email": null}, # Always add "email" in the id_token if such a claim exists + "introspection": {"email": {"value": "a@a.com"}}, # Add "email" in the introspection response only if its value is "a@a.com" + } + } + } + +by_scope +######## + +A dictionary with the following keys: `userinfo`, `id_token`, `introspection`, `access_token`. +The keys are boolean values that describe whether the scopes should be mapped +to claims and added to the response. +E.g.:: + + { + "add_claims": { + "by_scope": { + id_token: True, # Map the requested scopes to claims and add them to the id token + } From 8387e9d507b229ded47c31b6c9e210f6079ec736 Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Wed, 6 Oct 2021 12:40:49 +0300 Subject: [PATCH 55/58] Add more client configurations --- docs/source/contents/conf.rst | 246 ++++++++++++++++++++++++++-------- 1 file changed, 191 insertions(+), 55 deletions(-) diff --git a/docs/source/contents/conf.rst b/docs/source/contents/conf.rst index 6c64ca6e..92e97711 100644 --- a/docs/source/contents/conf.rst +++ b/docs/source/contents/conf.rst @@ -51,20 +51,22 @@ Optional. Salt, value or filename, used in sub_funcs (pairwise, public) for crea sub_funcs ######### -Optional. Functions involved in *sub*ject value creation. +Optional. Functions involved in subject value creation. scopes_to_claims -############## +################ A dict defining the scopes that are allowed to be used per client and the claims they map to (defaults to the scopes mapping described in the spec). If we want to define a scope that doesn't map to claims (e.g. offline_access) then we simply map it to an empty list. E.g.:: - { - "scope_a": ["claim1", "claim2"], - "scope_b": [] - } + + { + "scope_a": ["claim1", "claim2"], + "scope_b": [] + } + *Note*: For OIDC the `openid` scope must be present in this mapping. @@ -664,57 +666,14 @@ the following:: Clients ======= -In this section there are some client configuration examples. - -A common configuration:: - - endpoint_context.cdb['jbxedfmfyc'] = { - client_id: 'jbxedfmfyc', - client_salt: '6flfsj0Z', - registration_access_token: 'z3PCMmC1HZ1QmXeXGOQMJpWQNQynM4xY', - registration_client_uri: 'https://127.0.0.1:8000/registration_api?client_id=jbxedfmfyc', - client_id_issued_at: 1630256902, - client_secret: '19cc69b70d0108f630e52f72f7a3bd37ba4e11678ad1a7434e9818e1', - client_secret_expires_at: 1929727754, - application_type: 'web', - contacts: [ - 'rp@example.com' - ], - token_endpoint_auth_method: 'client_secret_basic', - redirect_uris: [ - [ - 'https://127.0.0.1:8090/authz_cb/satosa', - {} - ] - ], - post_logout_redirect_uris: [ - [ - 'https://127.0.0.1:8090/session_logout/satosa', - null - ] - ], - response_types: [ - 'code' - ], - grant_types: [ - 'authorization_code' - ], - allowed_scopes: [ - 'openid', - 'profile', - 'email', - 'offline_access' - ] - } - +In this section there are some client configuration examples. That can be used +to override the global configuration of the OP. How to configure the release of the user claims per clients:: endpoint_context.cdb["client_1"] = { "client_secret": "hemligt", "redirect_uris": [("https://example.com/cb", None)], - "client_salt": "salted", - "token_endpoint_auth_method": "client_secret_post", "response_types": ["code", "token", "code id_token", "id_token"], "add_claims": { "always": { @@ -728,7 +687,53 @@ How to configure the release of the user claims per clients:: }, }, -Some of the allowed client configurations are (this is an ongoing work): +The available configuration options are: + +------------- +client_secret +------------- + +The client secret. This parameter is required. + +------------------------ +client_secret_expires_at +------------------------ + +When the client_secret expires. + +------------- +redirect_uris +------------- + +The client's redirect uris. + +----------- +auth_method +----------- + +The auth_method that can be used per endpoint. +E.g:: + + { + "AccessTokenRequest": "client_secret_basic", + ... + } + +------------ +request_uris +------------ + +A list of `request_uris`. + +See https://openid.net/specs/openid-connect-registration-1_0-29.html#ClientMetadata. + +-------------- +response_types +-------------- + +The allowed `response_types` for this client. + +See https://openid.net/specs/openid-connect-registration-1_0-29.html#ClientMetadata. --------------------- grant_types_supported @@ -736,14 +741,17 @@ grant_types_supported Configure the allowed grant types on the token endpoint. --------------- +See https://openid.net/specs/openid-connect-registration-1_0-29.html#ClientMetadata. + +---------------- scopes_to_claims --------------- +---------------- A dict defining the scopes that are allowed to be used per client and the claims they map to (defaults to the scopes mapping described in the spec). If we want to define a scope that doesn't map to claims (e.g. offline_access) then we simply map it to an empty list. E.g.:: + { "scope_a": ["claim1", "claim2"], "scope_b": [] @@ -760,7 +768,7 @@ clients scopes_to_claims). revoke_refresh_on_issue ----------------------- -Configure whether to revoke the refresh token that was used to issue a new refresh token +Configure whether to revoke the refresh token that was used to issue a new refresh token. ---------- add_claims @@ -800,3 +808,131 @@ E.g.:: "by_scope": { id_token: True, # Map the requested scopes to claims and add them to the id token } + +----------------- +token_usage_rules +----------------- + +The usage rules for each token type. E.g.:: + + { + "usage_rules": { + "authorization_code": { + "expires_in": 3600, + "supports_minting": [ + "access_token", + "id_token", + ], + "max_usage": 1, + }, + "access_token": { + "expires_in": self.params["access_token_lifetime"], + }, + } + } + +-------------- +pkce_essential +-------------- + +Whether pkce is essential for this client. + +------------------------ +post_logout_redirect_uri +------------------------ + +The client's post logout redirect uris. + +See https://openid.net/specs/openid-connect-rpinitiated-1_0.html#RPLogout. + +---------------------- +backchannel_logout_uri +---------------------- + +The client's `backchannel_logout_uri`. + +See https://openid.net/specs/openid-connect-backchannel-1_0.html#BCRegistration + +----------------------- +frontchannel_logout_uri +----------------------- + +The client's `frontchannel_logout_uri`. + +See https://openid.net/specs/openid-connect-frontchannel-1_0.html#RPLogout + +-------------------------- +request_object_signing_alg +-------------------------- + +A list with the allowed algorithms for signing the request object. + +See https://openid.net/specs/openid-connect-registration-1_0-29.html#ClientMetadata + +----------------------------- +request_object_encryption_alg +----------------------------- + +A list with the allowed alg algorithms for encrypting the request object. + +See https://openid.net/specs/openid-connect-registration-1_0-29.html#ClientMetadata + +----------------------------- +request_object_encryption_enc +----------------------------- + +A list with the allowed enc algorithms for signing the request object. + +See https://openid.net/specs/openid-connect-registration-1_0-29.html#ClientMetadata + +---------------------------- +userinfo_signed_response_alg +---------------------------- + +JWS alg algorithm [JWA] REQUIRED for signing UserInfo Responses. + +See https://openid.net/specs/openid-connect-registration-1_0-29.html#ClientMetadata + +------------------------------- +userinfo_encrypted_response_enc +------------------------------- + +The alg algorithm [JWA] REQUIRED for encrypting UserInfo Responses. + +See https://openid.net/specs/openid-connect-registration-1_0-29.html#ClientMetadata + +------------------------------- +userinfo_encrypted_response_alg +------------------------------- + +JWE enc algorithm [JWA] REQUIRED for encrypting UserInfo Responses. + +See https://openid.net/specs/openid-connect-registration-1_0-29.html#ClientMetadata + +---------------------------- +id_token_signed_response_alg +---------------------------- + +JWS alg algorithm [JWA] REQUIRED for signing ID Token issued to this Client. + +See https://openid.net/specs/openid-connect-registration-1_0-29.html#ClientMetadata + +------------------------------- +id_token_encrypted_response_enc +------------------------------- + +The alg algorithm [JWA] REQUIRED for encrypting ID Token issued to this Client. + +See https://openid.net/specs/openid-connect-registration-1_0-29.html#ClientMetadata + +------------------------------- +id_token_encrypted_response_alg +------------------------------- + +JWE enc algorithm [JWA] REQUIRED for encrypting ID Token issued to this Client. + +See https://openid.net/specs/openid-connect-registration-1_0-29.html#ClientMetadata + +-------- +dpop_jkt +-------- From cdfa5e5326b240bc9c3b1330bd29e41b22ce4708 Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Wed, 6 Oct 2021 14:11:51 +0300 Subject: [PATCH 56/58] Use filter_scopes in check_unknown_scopes_policy --- src/oidcop/oauth2/authorization.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/oidcop/oauth2/authorization.py b/src/oidcop/oauth2/authorization.py index dd689f60..23625261 100755 --- a/src/oidcop/oauth2/authorization.py +++ b/src/oidcop/oauth2/authorization.py @@ -253,11 +253,12 @@ def check_unknown_scopes_policy(request_info, client_id, endpoint_context): allowed_scopes = endpoint_context.scopes_handler.get_allowed_scopes(client_id=client_id) # this prevents that authz would be released for unavailable scopes - for scope in request_info["scope"]: - if scope not in allowed_scopes: - _msg = "{} requested an unauthorized scope ({})" - logger.warning(_msg.format(client_id, scope)) - raise UnAuthorizedClientScope() + if set(request_info["scope"]) != set( + endpoint_context.scopes_handler.filter_scopes(request_info["scope"], client_id=client_id) + ): + _msg = "{} requested an unauthorized scope ({})" + logger.warning(_msg.format(client_id, scope)) + raise UnAuthorizedClientScope() class Authorization(Endpoint): From 82cb13aecc4022638c8a50a2589d478c956c8327 Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Wed, 6 Oct 2021 15:33:54 +0300 Subject: [PATCH 57/58] Fix log --- src/oidcop/oauth2/authorization.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/oidcop/oauth2/authorization.py b/src/oidcop/oauth2/authorization.py index 23625261..2d13ad50 100755 --- a/src/oidcop/oauth2/authorization.py +++ b/src/oidcop/oauth2/authorization.py @@ -250,14 +250,15 @@ def check_unknown_scopes_policy(request_info, client_id, endpoint_context): if not endpoint_context.conf["capabilities"].get("deny_unknown_scopes"): return - allowed_scopes = endpoint_context.scopes_handler.get_allowed_scopes(client_id=client_id) - + scope = request_info["scope"] + filtered_scopes = set( + endpoint_context.scopes_handler.filter_scopes(scope, client_id=client_id) + ) + scopes = set(scope) # this prevents that authz would be released for unavailable scopes - if set(request_info["scope"]) != set( - endpoint_context.scopes_handler.filter_scopes(request_info["scope"], client_id=client_id) - ): - _msg = "{} requested an unauthorized scope ({})" - logger.warning(_msg.format(client_id, scope)) + if scopes != filtered_scopes: + diff = " ".join(scopes - filtered_scopes) + logger.warning(f"{client_id} requested unauthorized scopes: {diff}") raise UnAuthorizedClientScope() From 49c6cec0b0a28942619875be5221ca02ba491a1d Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Wed, 22 Sep 2021 11:51:56 +0300 Subject: [PATCH 58/58] Don't require a scope to be defined Scopes that don't map to claims shouldn't have to be defined in the scopes to claims mapping --- src/oidcop/scopes.py | 4 ++-- tests/test_26_oidc_userinfo_endpoint.py | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/oidcop/scopes.py b/src/oidcop/scopes.py index a42a93ce..8a15bf97 100644 --- a/src/oidcop/scopes.py +++ b/src/oidcop/scopes.py @@ -31,12 +31,12 @@ def convert_scopes2claims(scopes, allowed_claims=None, scope2claim_map=None): res = {} if allowed_claims is None: for scope in scopes: - claims = {name: None for name in scope2claim_map[scope]} + claims = {name: None for name in scope2claim_map.get(scope, [])} res.update(claims) else: for scope in scopes: try: - claims = {name: None for name in scope2claim_map[scope] if name in allowed_claims} + claims = {name: None for name in scope2claim_map.get(scope, []) if name in allowed_claims} res.update(claims) except KeyError: continue diff --git a/tests/test_26_oidc_userinfo_endpoint.py b/tests/test_26_oidc_userinfo_endpoint.py index 288530e8..372d5261 100755 --- a/tests/test_26_oidc_userinfo_endpoint.py +++ b/tests/test_26_oidc_userinfo_endpoint.py @@ -360,9 +360,10 @@ def test_scopes_to_claims_per_client(self): "eduperson_scoped_affiliation", ], } + self.endpoint_context.cdb["client_1"]["allowed_scopes"] = list(self.endpoint_context.cdb["client_1"]["scopes_to_claims"].keys()) + ["aba"] _auth_req = AUTH_REQ.copy() - _auth_req["scope"] = ["openid", "research_and_scholarship_2"] + _auth_req["scope"] = ["openid", "research_and_scholarship_2", "aba"] session_id = self._create_session(_auth_req) grant = self.session_manager[session_id]