From bbc7929a60ca6e2a4f1dee5315ed7271caf7985a Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Tue, 31 Aug 2021 09:58:09 +0200 Subject: [PATCH 01/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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/40] 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 3abbb14d4a77f63997d637dce2b620d46d1a2563 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Tue, 7 Sep 2021 13:44:00 +0200 Subject: [PATCH 40/40] 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)