From 2f12e4c230c93b8bd51b09f4320ba437abe7f1ab Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Fri, 24 Sep 2021 09:10:19 +0200 Subject: [PATCH 01/40] First set of changes. Mostly of the use of the request/request_uri parameters. --- example/flask_rp/templates/opresult.html | 4 + example/flask_rp/views.py | 13 +- src/oidcrp/oidc/authorization.py | 34 +++-- src/oidcrp/oidc/userinfo.py | 4 + src/oidcrp/oidc/utils.py | 2 +- src/oidcrp/rp_handler.py | 85 ++++++++---- src/oidcrp/service.py | 8 +- src/oidcrp/service_context.py | 2 +- tests/test_14_oidc.py | 12 +- tests/test_20_rp_handler_oidc.py | 170 ++++++++++++++++++----- 10 files changed, 241 insertions(+), 93 deletions(-) diff --git a/example/flask_rp/templates/opresult.html b/example/flask_rp/templates/opresult.html index 34bd7be..86c6789 100644 --- a/example/flask_rp/templates/opresult.html +++ b/example/flask_rp/templates/opresult.html @@ -18,6 +18,10 @@

Endpoints

{{ url }}
{% endfor %} +
+
ID Token
+
{{ id_token }}
+

User information

{% for key, value in userinfo.items() %} diff --git a/example/flask_rp/views.py b/example/flask_rp/views.py index 5af68ae..7efbfe9 100644 --- a/example/flask_rp/views.py +++ b/example/flask_rp/views.py @@ -53,10 +53,14 @@ def rp(): uid = '' if iss or uid: + args = { + 'req_args': { + "claims": {"id_token": {"acr": {"value": "https://refeds.org/profile/mfa"}}} + } + } + if uid: - args = {'user_id': uid} - else: - args = {} + args['user_id'] = uid session['op_identifier'] = iss try: @@ -145,6 +149,7 @@ def finalize(op_identifier, request_args): return render_template('opresult.html', endpoints=endpoints, userinfo=res['userinfo'], access_token=res['token'], + id_token=res["id_token"], **kwargs) else: return make_response(res['error'], 400) @@ -152,7 +157,7 @@ def finalize(op_identifier, request_args): def get_op_identifier_by_cb_uri(url: str): uri = splitquery(url)[0] - for k,v in current_app.rph.issuer2rp.items(): + for k, v in current_app.rph.issuer2rp.items(): _cntx = v.get_service_context() for endpoint in ("redirect_uris", "post_logout_redirect_uris", diff --git a/src/oidcrp/oidc/authorization.py b/src/oidcrp/oidc/authorization.py index b95601b..2f123c6 100644 --- a/src/oidcrp/oidc/authorization.py +++ b/src/oidcrp/oidc/authorization.py @@ -148,9 +148,9 @@ def store_request_on_file(self, req, **kwargs): fid.close() return _webname - def construct_request_parameter(self, req, request_method, audience=None, expires_in=0, + def construct_request_parameter(self, req, request_param, audience=None, expires_in=0, **kwargs): - """Construct a request parameter""" + """ Construct a request parameter """ alg = self.get_request_object_signing_alg(**kwargs) kwargs["request_object_signing_alg"] = alg @@ -176,12 +176,15 @@ def construct_request_parameter(self, req, request_method, audience=None, expire if expires_in: req['exp'] = utc_time_sans_frac() + int(expires_in) - _req = make_openid_request(req, **kwargs) + _mor_args = {k: kwargs[k] for k in ["keys", "issuer", "request_object_signing_alg", "recv", + "with_jti", "lifetime"] if k in kwargs} + + _req = make_openid_request(req, **_mor_args) # Should the request be encrypted _req = request_object_encryption(_req, _context, **kwargs) - if request_method == "request": + if request_param == "request": req["request"] = _req else: # MUST be request_uri req["request_uri"] = self.store_request_on_file(_req, **kwargs) @@ -204,16 +207,23 @@ def oidc_post_construct(self, req, **kwargs): if 'prompt' not in req: req['prompt'] = 'consent' - try: - _request_method = kwargs['request_param'] - except KeyError: - pass - else: - del kwargs['request_param'] + _context.state.store_item(req, 'auth_request', req['state']) - self.construct_request_parameter(req, _request_method, **kwargs) + _request_param = kwargs.get('request_param') + if _request_param: + del kwargs['request_param'] + # local_dir, base_path + _config = _context.get('config') + kwargs["local_dir"] = _config.get('local_dir', '/tmp') + kwargs["base_path"] = _context.get('base_url') + self.construct_request_parameter(req, _request_param, **kwargs) + # removed all arguments except request/request_uri and the required + _leave = ['request', 'request_uri'] + _leave.extend(req.required_parameters()) + _keys = [k for k in req.keys() if k not in _leave] + for k in _keys: + del req[k] - _context.state.store_item(req, 'auth_request', req['state']) return req def gather_verify_arguments(self): diff --git a/src/oidcrp/oidc/userinfo.py b/src/oidcrp/oidc/userinfo.py index 2b5852d..a3eddcb 100644 --- a/src/oidcrp/oidc/userinfo.py +++ b/src/oidcrp/oidc/userinfo.py @@ -105,6 +105,10 @@ def post_parse_response(self, response, **kwargs): "url": spec["endpoint"] } + # Extension point + for meth in self.post_parse_process: + response = meth(response, _state_interface, kwargs['state']) + _state_interface.store_item(response, 'user_info', kwargs['state']) return response diff --git a/src/oidcrp/oidc/utils.py b/src/oidcrp/oidc/utils.py index 40b9b90..674e103 100644 --- a/src/oidcrp/oidc/utils.py +++ b/src/oidcrp/oidc/utils.py @@ -83,5 +83,5 @@ def construct_request_uri(local_dir, base_path, **kwargs): while os.path.exists(filename): _name = rndstr(10) filename = os.path.join(_filedir, _name) - _webname = "%s%s" % (_webpath, _name) + _webname = f"{_webpath}{_name}" return filename, _webname diff --git a/src/oidcrp/rp_handler.py b/src/oidcrp/rp_handler.py index e359939..4d588d3 100644 --- a/src/oidcrp/rp_handler.py +++ b/src/oidcrp/rp_handler.py @@ -85,7 +85,7 @@ def __init__(self, base_url, client_configs=None, services=None, keyjar=None, else: self.client_configs = client_configs - # keep track on which RP instance that serves with OP + # keep track on which RP instance that serves which OP self.issuer2rp = {} self.hash2issuer = {} self.httplib = http_lib @@ -252,31 +252,52 @@ def do_client_registration(self, client=None, iss_id='', state=''): _context = client.client_get("service_context") _iss = _context.get('issuer') - if not _context.get('redirect_uris'): - # Create the necessary callback URLs - # as a side effect self.hash2issuer is set - callbacks = self.create_callbacks(_iss) - - _context.set('redirect_uris', [ - v for k, v in callbacks.items() if not k.startswith('__')]) - _context.set('callbacks', callbacks) - else: - self.hash2issuer[iss_id] = _iss + self.hash2issuer[iss_id] = _iss # This should only be interesting if the client supports Single Log Out if _context.post_logout_redirect_uris is None: _context.post_logout_redirect_uris = [self.base_url] - if not _context.client_id: + if not _context.client_id: # means I have to do dynamic client registration + if not _context.get('redirect_uris'): + # Create the necessary callback URLs + # as a side effect self.hash2issuer is set + if 'require_request_uri_registration' in _context.get('provider_info'): + callbacks = self.create_callbacks(_iss, request_uri=True) + else: + callbacks = self.create_callbacks(_iss) + + _context.set('redirect_uris', [ + v for k, v in callbacks.items() if not k.startswith('__')]) + _context.set('callbacks', callbacks) + load_registration_response(client) def add_callbacks(self, service_context): - _callbacks = self.create_callbacks(service_context.get('provider_info')['issuer']) + _iss = service_context.get('issuer') + + if 'require_request_uri_registration' in service_context.get('provider_info'): + _callbacks = self.create_callbacks(_iss, request_uri=True) + else: + _callbacks = self.create_callbacks(_iss) + service_context.set('redirect_uris', [ v for k, v in _callbacks.items() if not k.startswith('__')]) service_context.set('callbacks', _callbacks) return _callbacks + def do_webfinger(self, user): + """ + Does OpenID Provider Issuer discovery using webfinger. + + :param user: Identifier for the target End-User that is the subject of the discovery + request. + :return: A Client instance + """ + temporary_client = self.init_client('') + temporary_client.do_request('webfinger', resource=user) + return temporary_client + def client_setup(self, iss_id='', user=''): """ First if no issuer ID is given then the identifier for the user is @@ -298,8 +319,7 @@ def client_setup(self, iss_id='', user=''): raise ValueError('Need issuer or user') logger.debug("Connecting to previously unknown OP") - temporary_client = self.init_client('') - temporary_client.do_request('webfinger', resource=user) + temporary_client = self.do_webfinger(user) else: temporary_client = None @@ -323,7 +343,7 @@ def client_setup(self, iss_id='', user=''): self.issuer2rp[issuer] = client return client - def create_callbacks(self, issuer): + def create_callbacks(self, issuer, request_uri=False): """ To mitigate some security issues the redirect_uris should be OP/AS specific. This method creates a set of redirect_uris unique to the @@ -337,14 +357,18 @@ def create_callbacks(self, issuer): _hash.update(as_bytes(issuer)) _hex = _hash.hexdigest() self.hash2issuer[_hex] = issuer - return { + res = { 'code': "{}/authz_cb/{}".format(self.base_url, _hex), 'implicit': "{}/authz_im_cb/{}".format(self.base_url, _hex), 'form_post': "{}/authz_fp_cb/{}".format(self.base_url, _hex), '__hex': _hex } + if request_uri: + res["request_uri"] = f"{self.base_url}/req_uri/{_hex}" - def init_authorization(self, client=None, state='', req_args=None): + return res + + def init_authorization(self, client=None, state='', req_args=None, behaviour_args=None): """ Constructs the URL that will redirect the user to the authorization endpoint of the OP/AS. @@ -387,12 +411,16 @@ def init_authorization(self, client=None, state='', req_args=None): logger.debug('Authorization request args: {}'.format(request_args)) + if "request_param" not in behaviour_args: + _pi = _context.get("provider_info") + _srv = client.get_service('authorization') - _info = _srv.get_request_parameters(request_args=request_args) + _info = _srv.get_request_parameters(request_args=request_args, + behaviour_args=behaviour_args) logger.debug('Authorization info: {}'.format(_info)) return {'url': _info['url'], 'state': _state} - def begin(self, issuer_id='', user_id=''): + def begin(self, issuer_id='', user_id='', req_args=None, behaviour_args=None): """ This is the first of the 3 high level methods that most users of this library should confine them self to use. @@ -412,7 +440,7 @@ def begin(self, issuer_id='', user_id=''): client = self.client_setup(issuer_id, user_id) try: - res = self.init_authorization(client) + res = self.init_authorization(client, req_args=req_args, behaviour_args=behaviour_args) except Exception: message = traceback.format_exception(*sys.exc_info()) logger.error(message) @@ -447,7 +475,8 @@ def get_client_authn_method(client, endpoint): """ if endpoint == 'token_endpoint': try: - am = client.client_get("service_context").get('behaviour')['token_endpoint_auth_method'] + am = client.client_get("service_context").get('behaviour')[ + 'token_endpoint_auth_method'] except KeyError: return '' else: @@ -711,6 +740,8 @@ def finalize(self, issuer, response): _state = authorization_response['state'] token = self.get_access_and_id_token(authorization_response, state=_state, client=client) + _id_token = token.get("id_token") + logger.debug(f"ID Token: {_id_token}") if client.client_get("service", "userinfo") and token['access_token']: inforesp = self.get_user_info( @@ -723,8 +754,8 @@ def finalize(self, issuer, response): 'state': _state } - elif token['id_token']: # look for it in the ID Token - inforesp = self.userinfo_in_id_token(token['id_token']) + elif _id_token: # look for it in the ID Token + inforesp = self.userinfo_in_id_token(_id_token) else: inforesp = {} @@ -743,19 +774,19 @@ def finalize(self, issuer, response): if _sid_support: try: - sid = token['id_token']['sid'] + sid = _id_token['sid'] except KeyError: pass else: _context.state.store_sid2state(sid, _state) - _context.state.store_sub2state(token['id_token']['sub'], _state) + _context.state.store_sub2state(_id_token['sub'], _state) return { 'userinfo': inforesp, 'state': authorization_response['state'], 'token': token['access_token'], - 'id_token': token['id_token'] + 'id_token': _id_token } def has_active_authentication(self, state): diff --git a/src/oidcrp/service.py b/src/oidcrp/service.py index 0e350e2..29d19a1 100644 --- a/src/oidcrp/service.py +++ b/src/oidcrp/service.py @@ -88,6 +88,7 @@ def __init__(self, self.pre_construct = [] self.post_construct = [] self.construct_extra_headers = [] + self.post_parse_process = [] def gather_request_args(self, **kwargs): """ @@ -204,8 +205,7 @@ def construct(self, request_args=None, **kwargs): # run the pre_construct methods. Will return a possibly new # set of request arguments but also a set of arguments to # be used by the post_construct methods. - request_args, post_args = self.do_pre_construct(request_args, - **kwargs) + request_args, post_args = self.do_pre_construct(request_args, **kwargs) # If 'state' appears among the keyword argument and is not # expected to appear in the request, remove it. @@ -222,6 +222,10 @@ def construct(self, request_args=None, **kwargs): # message type request = self.msg_type(**_args) + _behaviour_args = kwargs.get("behaviour_args") + if _behaviour_args: + post_args.update(_behaviour_args) + return self.do_post_construct(request, **post_args) def init_authentication_method(self, request, authn_method, diff --git a/src/oidcrp/service_context.py b/src/oidcrp/service_context.py index 0247162..51cb75e 100644 --- a/src/oidcrp/service_context.py +++ b/src/oidcrp/service_context.py @@ -145,7 +145,7 @@ def __init__(self, base_url="", keyjar=None, config=None, state=None, **kwargs): 'behaviour', 'callback', 'issuer']: _val = config.get(param, _def_value[param]) self.set(param, _val) - if param == 'client_secret': + if param == 'client_secret' and _val: self.keyjar.add_symmetric('', _val) if not self.issuer: diff --git a/tests/test_14_oidc.py b/tests/test_14_oidc.py index 5ebc41c..42de74b 100755 --- a/tests/test_14_oidc.py +++ b/tests/test_14_oidc.py @@ -108,8 +108,7 @@ def test_construct_refresh_token_request(self): token_response = AccessTokenResponse(refresh_token="refresh_with_me", access_token="access") - _context.state.store_item(token_response, - 'token_response', 'ABCDE') + _context.state.store_item(token_response, 'token_response', 'ABCDE') req_args = {} msg = self.client.client_get("service",'refresh_token').construct( @@ -131,17 +130,14 @@ def test_do_userinfo_request_init(self): state='state' ) - _context.state.store_item(auth_request, - 'auth_request', 'ABCDE') + _context.state.store_item(auth_request, 'auth_request', 'ABCDE') auth_response = AuthorizationResponse(code='access_code') - _context.state.store_item(auth_response, - 'auth_response', 'ABCDE') + _context.state.store_item(auth_response, 'auth_response', 'ABCDE') token_response = AccessTokenResponse(refresh_token="refresh_with_me", access_token="access") - _context.state.store_item(token_response, - 'token_response', 'ABCDE') + _context.state.store_item(token_response, 'token_response', 'ABCDE') _srv = self.client.client_get("service",'userinfo') _srv.endpoint = "https://example.com/userinfo" diff --git a/tests/test_20_rp_handler_oidc.py b/tests/test_20_rp_handler_oidc.py index b74b588..54a4d5c 100644 --- a/tests/test_20_rp_handler_oidc.py +++ b/tests/test_20_rp_handler_oidc.py @@ -29,7 +29,8 @@ "code id_token token", "code token"], "scope": ["openid", "profile", "email", "address", "phone"], "token_endpoint_auth_method": "client_secret_basic", - "verify_args": {"allow_sign_alg_none": True} + "verify_args": {"allow_sign_alg_none": True}, + "request_parameter_preference": ["request_uri", "request"] } CLIENT_CONFIG = { @@ -143,6 +144,40 @@ "userinfo_endpoint": "https://api.github.com/user" }, + 'services': { + 'authorization': { + 'class': 'oidcrp.oidc.authorization.Authorization', + }, + 'access_token': { + 'class': 'oidcrp.oidc.access_token.AccessToken' + }, + 'userinfo': { + 'class': 'oidcrp.oidc.userinfo.UserInfo', + 'kwargs': {'conf': {'default_authn_method': ''}} + }, + 'refresh_access_token': { + 'class': 'oidcrp.oidc.refresh_access_token.RefreshAccessToken' + } + } + }, + 'github2': { + "issuer": "https://github.com/login/oauth/authorize", + 'client_id': 'eeeeeeeee', + 'client_secret': 'aaaaaaaaaaaaaaaaaaaa', + "redirect_uris": ["{}/authz_cb/github".format(BASE_URL)], + "behaviour": { + "response_types": ["code"], + "scope": ["user", "public_repo"], + "token_endpoint_auth_method": '', + "verify_args": {"allow_sign_alg_none": True} + }, + "provider_info": { + "authorization_endpoint": "https://github.com/login/oauth/authorize", + "token_endpoint": "https://github.com/login/oauth/access_token", + "userinfo_endpoint": "https://api.github.com/user", + "request_parameter_supported": True, + "request_uri_parameter_supported": True + }, 'services': { 'authorization': { 'class': 'oidcrp.oidc.authorization.Authorization' @@ -155,8 +190,7 @@ 'kwargs': {'conf': {'default_authn_method': ''}} }, 'refresh_access_token': { - 'class': 'oidcrp.oidc.refresh_access_token' - '.RefreshAccessToken' + 'class': 'oidcrp.oidc.refresh_access_token.RefreshAccessToken' } } } @@ -265,7 +299,7 @@ def test_do_provider_info(self): # Make sure the service endpoints are set for service_type in ['authorization', 'accesstoken', 'userinfo']: - _srv = client.client_get("service",service_type) + _srv = client.client_get("service", service_type) _endp = client.client_get("service_context").get('provider_info')[_srv.endpoint_name] assert _srv.endpoint == _endp @@ -296,7 +330,7 @@ def test_do_client_setup(self): assert len(keys) == 2 for service_type in ['authorization', 'accesstoken', 'userinfo']: - _srv = client.client_get("service",service_type) + _srv = client.client_get("service", service_type) _endp = _srv.client_get("service_context").get('provider_info')[_srv.endpoint_name] assert _srv.endpoint == _endp @@ -367,8 +401,9 @@ def test_finalize_auth(self): state=res['state']) resp = self.rph.finalize_auth(client, _session['iss'], auth_response.to_dict()) assert set(resp.keys()) == {'state', 'code'} - aresp = client.client_get("service_context").state.get_item(AuthorizationResponse, 'auth_response', - res['state']) + aresp = client.client_get("service_context").state.get_item(AuthorizationResponse, + 'auth_response', + res['state']) assert set(aresp.keys()) == {'state', 'code'} def test_get_client_authn_method(self): @@ -417,7 +452,7 @@ def test_get_access_token(self): with responses.RequestsMock() as rsps: rsps.add("POST", _url, body=at.to_json(), adding_headers={"Content-Type": "application/json"}, status=200) - client.client_get("service",'accesstoken').endpoint = _url + client.client_get("service", 'accesstoken').endpoint = _url auth_response = AuthorizationResponse(code='access_code', state=res['state']) @@ -466,7 +501,7 @@ def test_access_and_id_token(self): with responses.RequestsMock() as rsps: rsps.add("POST", _url, body=at.to_json(), adding_headers={"Content-Type": "application/json"}, status=200) - client.client_get("service",'accesstoken').endpoint = _url + client.client_get("service", 'accesstoken').endpoint = _url _response = AuthorizationResponse(code='access_code', state=res['state']) @@ -507,7 +542,7 @@ def test_access_and_id_token_by_reference(self): with responses.RequestsMock() as rsps: rsps.add("POST", _url, body=at.to_json(), adding_headers={"Content-Type": "application/json"}, status=200) - client.client_get("service",'accesstoken').endpoint = _url + client.client_get("service", 'accesstoken').endpoint = _url _response = AuthorizationResponse(code='access_code', state=res['state']) @@ -548,7 +583,7 @@ def test_get_user_info(self): with responses.RequestsMock() as rsps: rsps.add("POST", _url, body=at.to_json(), adding_headers={"Content-Type": "application/json"}, status=200) - client.client_get("service",'accesstoken').endpoint = _url + client.client_get("service", 'accesstoken').endpoint = _url _response = AuthorizationResponse(code='access_code', state=res['state']) @@ -562,7 +597,7 @@ def test_get_user_info(self): with responses.RequestsMock() as rsps: rsps.add("GET", _url, body='{"sub":"EndUserSubject"}', adding_headers={"Content-Type": "application/json"}, status=200) - client.client_get("service",'userinfo').endpoint = _url + client.client_get("service", 'userinfo').endpoint = _url userinfo_resp = self.rph.get_user_info(res['state'], client, token_resp['access_token']) @@ -595,7 +630,7 @@ def test_get_provider_specific_service(): } } entity = Entity(services=srv_desc) - assert entity.client_get("service",'accesstoken').response_body_type == 'urlencoded' + assert entity.client_get("service", 'accesstoken').response_body_type == 'urlencoded' class TestRPHandlerTier2(object): @@ -634,7 +669,7 @@ def rphandler_setup(self): rsps.add("POST", _url, body=at.to_json(), adding_headers={"Content-Type": "application/json"}, status=200) - client.client_get("service",'accesstoken').endpoint = _url + client.client_get("service", 'accesstoken').endpoint = _url _response = AuthorizationResponse(code='access_code', state=res['state']) @@ -649,7 +684,7 @@ def rphandler_setup(self): rsps.add("GET", _url, body='{"sub":"EndUserSubject"}', adding_headers={"Content-Type": "application/json"}, status=200) - client.client_get("service",'userinfo').endpoint = _url + client.client_get("service", 'userinfo').endpoint = _url self.rph.get_user_info(res['state'], client, token_resp['access_token']) self.state = res['state'] @@ -677,7 +712,7 @@ def test_refresh_access_token(self): rsps.add("POST", _url, body=at.to_json(), adding_headers={"Content-Type": "application/json"}, status=200) - client.client_get("service",'refresh_token').endpoint = _url + client.client_get("service", 'refresh_token').endpoint = _url res = self.rph.refresh_access_token(self.state, client, 'openid email') assert res['access_token'] == '2nd_accessTok' @@ -689,7 +724,7 @@ def test_get_user_info(self): with responses.RequestsMock() as rsps: rsps.add("GET", _url, body='{"sub":"EndUserSubject", "mail":"foo@example.com"}', adding_headers={"Content-Type": "application/json"}, status=200) - client.client_get("service",'userinfo').endpoint = _url + client.client_get("service", 'userinfo').endpoint = _url resp = self.rph.get_user_info(self.state, client) assert set(resp.keys()) == {'sub', 'mail'} @@ -722,8 +757,7 @@ def __init__(self, issuer, keyjar=None): self.post_response = {} self.register_post_response('default', 'OK', 200) - def register_get_response(self, path, data, status_code=200, - headers=None): + def register_get_response(self, path, data, status_code=200, headers=None): _headers = headers or {} self.get_response[path] = MockResponse(status_code, data, _headers) @@ -787,6 +821,24 @@ def registration_callback(data): return json.dumps(_req) +def test_rphandler_request_uri(): + rph = RPHandler(BASE_URL, CLIENT_CONFIG, keyjar=CLI_KEY) + res = rph.begin(issuer_id='github2', behaviour_args={"request_param": "request_uri"}) + _session = rph.get_session_information(res['state']) + _url = res["url"] + _qp = parse_qs(urlparse(_url).query) + assert 'request_uri' in _qp + + +def test_rphandler_request(): + rph = RPHandler(BASE_URL, CLIENT_CONFIG, keyjar=CLI_KEY) + res = rph.begin(issuer_id='github2', behaviour_args={"request_param": "request"}) + _session = rph.get_session_information(res['state']) + _url = res["url"] + _qp = parse_qs(urlparse(_url).query) + assert 'request' in _qp + + class TestRPHandlerWithMockOP(object): @pytest.fixture(autouse=True) def rphandler_setup(self): @@ -863,32 +915,22 @@ def test_dynamic_setup(self): "issuer": "https://server.example.com", "subject_types_supported": ['public'], "token_endpoint": "https://server.example.com/connect/token", - "token_endpoint_auth_methods_supported": ["client_secret_basic", - "private_key_jwt"], + "token_endpoint_auth_methods_supported": ["client_secret_basic", "private_key_jwt"], "userinfo_endpoint": "https://server.example.com/connect/user", "check_id_endpoint": "https://server.example.com/connect/check_id", - "refresh_session_endpoint": - "https://server.example.com/connect/refresh_session", - "end_session_endpoint": - "https://server.example.com/connect/end_session", + "refresh_session_endpoint": "https://server.example.com/connect/refresh_session", + "end_session_endpoint": "https://server.example.com/connect/end_session", "jwks_uri": "https://server.example.com/jwk.json", - "registration_endpoint": - "https://server.example.com/connect/register", - "scopes_supported": ["openid", "profile", "email", "address", - "phone"], - "response_types_supported": ["code", "code id_token", - "token id_token"], - "acrs_supported": ["1", "2", - "http://id.incommon.org/assurance/bronze"], + "registration_endpoint": "https://server.example.com/connect/register", + "scopes_supported": ["openid", "profile", "email", "address", "phone"], + "response_types_supported": ["code", "code id_token", "token id_token"], + "acrs_supported": ["1", "2", "http://id.incommon.org/assurance/bronze"], "user_id_types_supported": ["public", "pairwise"], - "userinfo_algs_supported": ["HS256", "RS256", "A128CBC", "A128KW", - "RSA1_5"], + "userinfo_algs_supported": ["HS256", "RS256", "A128CBC", "A128KW", "RSA1_5"], "id_token_signing_alg_values_supported": ["HS256", "RS256", "A128CBC", "A128KW", "RSA1_5"], - "request_object_algs_supported": ["HS256", "RS256", "A128CBC", - "A128KW", - "RSA1_5"] + "request_object_algs_supported": ["HS256", "RS256", "A128CBC", "A128KW", "RSA1_5"] } pcr = ProviderConfigurationResponse(**resp) @@ -902,3 +944,55 @@ def test_dynamic_setup(self): auth_query = self.rph.begin(user_id=user_id) assert auth_query + + def test_dynamic_setup_redirect_uri(self): + user_id = 'acct:foobar@example.com' + _link = Link(rel="http://openid.net/specs/connect/1.0/issuer", + href="https://server.example.com") + webfinger_response = JRD(subject=user_id, links=[_link]) + self.mock_op.register_get_response( + '/.well-known/webfinger', webfinger_response.to_json(), 200, + {'content-type': "application/json"}) + + resp = { + "authorization_endpoint": + "https://server.example.com/connect/authorize", + "issuer": "https://server.example.com", + "subject_types_supported": ['public'], + "token_endpoint": "https://server.example.com/connect/token", + "token_endpoint_auth_methods_supported": ["client_secret_basic", "private_key_jwt"], + "userinfo_endpoint": "https://server.example.com/connect/user", + "check_id_endpoint": "https://server.example.com/connect/check_id", + "refresh_session_endpoint": "https://server.example.com/connect/refresh_session", + "end_session_endpoint": "https://server.example.com/connect/end_session", + "jwks_uri": "https://server.example.com/jwk.json", + "registration_endpoint": "https://server.example.com/connect/register", + "scopes_supported": ["openid", "profile", "email", "address", "phone"], + "response_types_supported": ["code", "code id_token", "token id_token"], + "acrs_supported": ["1", "2", "http://id.incommon.org/assurance/bronze"], + "user_id_types_supported": ["public", "pairwise"], + "userinfo_algs_supported": ["HS256", "RS256", "A128CBC", "A128KW", "RSA1_5"], + "id_token_signing_alg_values_supported": ["HS256", "RS256", + "A128CBC", "A128KW", + "RSA1_5"], + "request_object_algs_supported": ["HS256", "RS256", "A128CBC", "A128KW", "RSA1_5"], + "request_parameter_supported": True, + "request_uri_parameter_supported": True, + "require_request_uri_registration": True + } + + pcr = ProviderConfigurationResponse(**resp) + self.mock_op.register_get_response( + '/.well-known/openid-configuration', pcr.to_json(), 200, + {'content-type': "application/json"}) + + self.mock_op.register_post_response( + '/connect/register', registration_callback, 200, + {'content-type': "application/json"}) + + res = self.rph.begin(user_id=user_id) + assert res + + _url = res["url"] + _qp = parse_qs(urlparse(_url).query) + assert 'request' in _qp From dc0354251cb45ac37bd793b4803d85ab3b67ba7a Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Fri, 24 Sep 2021 10:32:06 +0200 Subject: [PATCH 02/40] Make sure there is a '/' between path and name. --- src/oidcrp/oidc/utils.py | 5 ++++- src/oidcrp/rp_handler.py | 2 ++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/oidcrp/oidc/utils.py b/src/oidcrp/oidc/utils.py index 674e103..9605733 100644 --- a/src/oidcrp/oidc/utils.py +++ b/src/oidcrp/oidc/utils.py @@ -83,5 +83,8 @@ def construct_request_uri(local_dir, base_path, **kwargs): while os.path.exists(filename): _name = rndstr(10) filename = os.path.join(_filedir, _name) - _webname = f"{_webpath}{_name}" + if _webpath.endswith("/"): + _webname = f"{_webpath}{_name}" + else: + _webname = f"{_webpath}/{_name}" return filename, _webname diff --git a/src/oidcrp/rp_handler.py b/src/oidcrp/rp_handler.py index 4d588d3..49669c9 100644 --- a/src/oidcrp/rp_handler.py +++ b/src/oidcrp/rp_handler.py @@ -429,6 +429,8 @@ def begin(self, issuer_id='', user_id='', req_args=None, behaviour_args=None): Once it has the client it will construct an Authorization request. + :param behaviour_args: + :param req_args: :param issuer_id: Issuer ID :param user_id: A user identifier :return: A dictionary containing **url** the URL that will redirect the From f9711ada80c08dee1a4fa32262b3652da2b14674 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Fri, 24 Sep 2021 11:02:34 +0200 Subject: [PATCH 03/40] Allow behaviour_args for do_provider_info and do_client_registration too. --- src/oidcrp/rp_handler.py | 22 +++++++++++++++------- src/oidcrp/util.py | 4 ++-- tests/test_20_rp_handler_oidc.py | 8 ++++++-- 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/src/oidcrp/rp_handler.py b/src/oidcrp/rp_handler.py index 49669c9..e5c0627 100644 --- a/src/oidcrp/rp_handler.py +++ b/src/oidcrp/rp_handler.py @@ -18,6 +18,7 @@ from oidcmsg.oidc import AuthorizationResponse from oidcmsg.oidc import Claims from oidcmsg.oidc import OpenIDSchema +from oidcmsg.oidc import RegistrationRequest from oidcmsg.oidc.session import BackChannelLogoutRequest from oidcmsg.time_util import time_sans_frac @@ -180,7 +181,7 @@ def init_client(self, issuer): _context.jwks_uri = self.jwks_uri return client - def do_provider_info(self, client=None, state=''): + def do_provider_info(self, client=None, state='', behaviour_args=None): """ Either get the provider info from configuration or through dynamic discovery. @@ -235,10 +236,14 @@ def do_provider_info(self, client=None, state=''): except KeyError: return _context.get('issuer') - def do_client_registration(self, client=None, iss_id='', state=''): + def do_client_registration(self, client=None, + iss_id: Optional[str] = '', + state: Optional[str] = '', + behaviour_args: Optional[dict] = None): """ Prepare for and do client registration if configured to do so + :param behaviour_args: To fine tune behaviour :param client: A Client instance :param state: A key by which the state of the session can be retrieved @@ -271,7 +276,9 @@ def do_client_registration(self, client=None, iss_id='', state=''): v for k, v in callbacks.items() if not k.startswith('__')]) _context.set('callbacks', callbacks) - load_registration_response(client) + _params = RegistrationRequest().parameters() + request_args = {k:v for k, v in behaviour_args.items() if k in _params} + load_registration_response(client, request_args=request_args) def add_callbacks(self, service_context): _iss = service_context.get('issuer') @@ -298,7 +305,7 @@ def do_webfinger(self, user): temporary_client.do_request('webfinger', resource=user) return temporary_client - def client_setup(self, iss_id='', user=''): + def client_setup(self, iss_id='', user='', behaviour_args=None): """ First if no issuer ID is given then the identifier for the user is used by the webfinger service to try to find the issuer ID. @@ -307,6 +314,7 @@ def client_setup(self, iss_id='', user=''): the necessary information for the client to be able to communicate with the OP/AS that has the provided issuer ID. + :param behaviour_args: To fine tune behaviour :param iss_id: The issuer ID :param user: A user identifier :return: A :py:class:`oidcrp.oidc.Client` instance @@ -335,10 +343,10 @@ def client_setup(self, iss_id='', user=''): return client logger.debug("Get provider info") - issuer = self.do_provider_info(client) + issuer = self.do_provider_info(client, behaviour_args=behaviour_args) logger.debug("Do client registration") - self.do_client_registration(client, iss_id) + self.do_client_registration(client, iss_id, behaviour_args=behaviour_args) self.issuer2rp[issuer] = client return client @@ -439,7 +447,7 @@ def begin(self, issuer_id='', user_id='', req_args=None, behaviour_args=None): """ # Get the client instance that has been assigned to this issuer - client = self.client_setup(issuer_id, user_id) + client = self.client_setup(issuer_id, user_id, behaviour_args=behaviour_args) try: res = self.init_authorization(client, req_args=req_args, behaviour_args=behaviour_args) diff --git a/src/oidcrp/util.py b/src/oidcrp/util.py index eb774cb..c6e6759 100755 --- a/src/oidcrp/util.py +++ b/src/oidcrp/util.py @@ -534,7 +534,7 @@ def add_path(url, path): return '{}/{}'.format(url, path) -def load_registration_response(client): +def load_registration_response(client, request_args=None): """ If the client has been statically registered that information must be provided during the configuration. If expected to be @@ -544,7 +544,7 @@ def load_registration_response(client): """ if not client.client_get("service_context").get('client_id'): try: - response = client.do_request('registration') + response = client.do_request('registration', request_args=request_args) except KeyError: raise ConfigurationError('No registration info') except Exception as err: diff --git a/tests/test_20_rp_handler_oidc.py b/tests/test_20_rp_handler_oidc.py index 54a4d5c..983d352 100644 --- a/tests/test_20_rp_handler_oidc.py +++ b/tests/test_20_rp_handler_oidc.py @@ -832,7 +832,8 @@ def test_rphandler_request_uri(): def test_rphandler_request(): rph = RPHandler(BASE_URL, CLIENT_CONFIG, keyjar=CLI_KEY) - res = rph.begin(issuer_id='github2', behaviour_args={"request_param": "request"}) + res = rph.begin(issuer_id='github2', + behaviour_args={"request_param": "request"}) _session = rph.get_session_information(res['state']) _url = res["url"] _qp = parse_qs(urlparse(_url).query) @@ -990,7 +991,10 @@ def test_dynamic_setup_redirect_uri(self): '/connect/register', registration_callback, 200, {'content-type': "application/json"}) - res = self.rph.begin(user_id=user_id) + res = self.rph.begin(user_id=user_id, + behaviour_args={ + "request_param": "request", + "request_object_signing_alg": "RS256"}) assert res _url = res["url"] From befb1ed2abc6fff1ca61f7a82c1a03a8f86ddbe2 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Fri, 24 Sep 2021 11:19:35 +0200 Subject: [PATCH 04/40] Change default names for where to store signed requests. To not collide with other packages. --- src/oidcrp/oidc/authorization.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/oidcrp/oidc/authorization.py b/src/oidcrp/oidc/authorization.py index 2f123c6..be6444a 100644 --- a/src/oidcrp/oidc/authorization.py +++ b/src/oidcrp/oidc/authorization.py @@ -214,8 +214,8 @@ def oidc_post_construct(self, req, **kwargs): del kwargs['request_param'] # local_dir, base_path _config = _context.get('config') - kwargs["local_dir"] = _config.get('local_dir', '/tmp') - kwargs["base_path"] = _context.get('base_url') + kwargs["local_dir"] = _config.get('local_dir', './requests') + kwargs["base_path"] = _context.get('base_url') + '/' + "requests" self.construct_request_parameter(req, _request_param, **kwargs) # removed all arguments except request/request_uri and the required _leave = ['request', 'request_uri'] From 76c3fdc5fe73be240d96e9a9312054a7993bb4a3 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Fri, 24 Sep 2021 15:59:50 +0200 Subject: [PATCH 05/40] Allow alg == "none" --- src/oidcrp/oidc/authorization.py | 3 +++ src/oidcrp/rp_handler.py | 6 ++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/oidcrp/oidc/authorization.py b/src/oidcrp/oidc/authorization.py index be6444a..106ce89 100644 --- a/src/oidcrp/oidc/authorization.py +++ b/src/oidcrp/oidc/authorization.py @@ -158,6 +158,9 @@ def construct_request_parameter(self, req, request_param, audience=None, expires if "keys" not in kwargs and alg and alg != "none": kwargs["keys"] = _context.keyjar + if alg == "none": + kwargs["keys"] = [] + _srv_cntx = _context # This is the issuer of the JWT, that is me ! diff --git a/src/oidcrp/rp_handler.py b/src/oidcrp/rp_handler.py index e5c0627..9625e99 100644 --- a/src/oidcrp/rp_handler.py +++ b/src/oidcrp/rp_handler.py @@ -277,7 +277,7 @@ def do_client_registration(self, client=None, _context.set('callbacks', callbacks) _params = RegistrationRequest().parameters() - request_args = {k:v for k, v in behaviour_args.items() if k in _params} + request_args = {k: v for k, v in behaviour_args.items() if k in _params} load_registration_response(client, request_args=request_args) def add_callbacks(self, service_context): @@ -618,10 +618,12 @@ def userinfo_in_id_token(id_token): res.update(id_token.extra()) return res - def finalize_auth(self, client, issuer, response): + def finalize_auth(self, client, issuer: str, response: dict, + behaviour_args: Optional[dict] = None): """ Given the response returned to the redirect_uri, parse and verify it. + :param behaviour_args: For fine tuning behaviour :param client: A Client instance :param issuer: An Issuer ID :param response: The authorization response as a dictionary From c68bcc709d761a24ff8c4d6bc63d65ceca8f1087 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Fri, 24 Sep 2021 16:04:05 +0200 Subject: [PATCH 06/40] Might not be any behaviour args. --- src/oidcrp/rp_handler.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/oidcrp/rp_handler.py b/src/oidcrp/rp_handler.py index 9625e99..0236869 100644 --- a/src/oidcrp/rp_handler.py +++ b/src/oidcrp/rp_handler.py @@ -276,8 +276,12 @@ def do_client_registration(self, client=None, v for k, v in callbacks.items() if not k.startswith('__')]) _context.set('callbacks', callbacks) - _params = RegistrationRequest().parameters() - request_args = {k: v for k, v in behaviour_args.items() if k in _params} + if behaviour_args: + _params = RegistrationRequest().parameters() + request_args = {k: v for k, v in behaviour_args.items() if k in _params} + else: + request_args = {} + load_registration_response(client, request_args=request_args) def add_callbacks(self, service_context): From 4e6ba5543359efc65638c80b51cb7b65726ad714 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Fri, 24 Sep 2021 16:06:13 +0200 Subject: [PATCH 07/40] Might not be any behaviour args. --- src/oidcrp/rp_handler.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/oidcrp/rp_handler.py b/src/oidcrp/rp_handler.py index 0236869..9468d2f 100644 --- a/src/oidcrp/rp_handler.py +++ b/src/oidcrp/rp_handler.py @@ -423,8 +423,8 @@ def init_authorization(self, client=None, state='', req_args=None, behaviour_arg logger.debug('Authorization request args: {}'.format(request_args)) - if "request_param" not in behaviour_args: - _pi = _context.get("provider_info") + # if behaviour_args and "request_param" not in behaviour_args: + # _pi = _context.get("provider_info") _srv = client.get_service('authorization') _info = _srv.get_request_parameters(request_args=request_args, From 208cdc07bca0d68a03cd615755765e8e65680312 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Fri, 24 Sep 2021 16:22:34 +0200 Subject: [PATCH 08/40] Added argument. --- src/oidcrp/rp_handler.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/oidcrp/rp_handler.py b/src/oidcrp/rp_handler.py index 9468d2f..db22cc6 100644 --- a/src/oidcrp/rp_handler.py +++ b/src/oidcrp/rp_handler.py @@ -727,7 +727,7 @@ def get_access_and_id_token(self, authorization_response=None, state='', return {'access_token': access_token, 'id_token': id_token} # noinspection PyUnusedLocal - def finalize(self, issuer, response): + def finalize(self, issuer, response, behaviour_args: Optional[dict] = None): """ The third of the high level methods that a user of this Class should know about. @@ -735,6 +735,7 @@ def finalize(self, issuer, response): callback URL there might be a number of services that the client should use. Which one those are are defined by the client configuration. + :param behaviour_args: For fine tuning :param issuer: Who sent the response :param response: The Authorization response as a dictionary :returns: A dictionary with two claims: From 70a8e71a633862f187c0a6fc02766882930bd5c7 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Sat, 25 Sep 2021 09:27:34 +0200 Subject: [PATCH 09/40] Fixed tests. --- tests/request123456.jwt | 2 +- tests/test_13_oidc_service.py | 33 ++++++++++++++++---------------- tests/test_20_rp_handler_oidc.py | 2 +- 3 files changed, 18 insertions(+), 19 deletions(-) diff --git a/tests/request123456.jwt b/tests/request123456.jwt index f72cdd1..84b7793 100644 --- a/tests/request123456.jwt +++ b/tests/request123456.jwt @@ -1 +1 @@ -eyJhbGciOiJSUzI1NiIsImtpZCI6IlNVc3dOaTFNUkZsRFQwWTJZalUxWjFSZlFsbzJTM2RFYTNGVFRrVjNMVGhGY25oRFRIRjVlbGsyVlEifQ.eyJyZXNwb25zZV90eXBlIjogImNvZGUiLCAic3RhdGUiOiAic3RhdGUiLCAicmVkaXJlY3RfdXJpIjogImh0dHBzOi8vZXhhbXBsZS5jb20vY2xpL2F1dGh6X2NiIiwgInNjb3BlIjogIm9wZW5pZCIsICJub25jZSI6ICJWNVFGVUxMWGpBS0lOS3V3Y2JlVU43OHRuWkZ2MGloYyIsICJjbGllbnRfaWQiOiAiY2xpZW50X2lkIiwgImlzcyI6ICJjbGllbnRfaWQiLCAiaWF0IjogMTYxNzg3MDI0NiwgImF1ZCI6IFsiaHR0cHM6Ly9leGFtcGxlLmNvbSJdfQ.itHjMHec6T2py2zDxaAS11tFAYdsTZ-SY3AV2j5SCTHPfk9CkXa6g6s_t0oVvcdzLVXaqQ1iU9WqOeirwj4UDxCTHulPzBOcBuC3WbCki2HT9EPI88Lov-kCuz_4juw97lIyU1BkaofaZJSjRcEW_fOY0KuP7BKIDmthTLTWxEGMoMXmXcs_QD13tz0IWjrkqjwbcKjbUxrvGHJbOnOBGwgHPPm46otDMO-hQrtvTOGz4PbdD5XZ4imDV0bJkx72ITpAfM8iODmd9sKrOWkEZEhsRG1ugXa8RgOPNLsLTLjTpzWiVeczJHOiE5H4EC-uwzXiRDiShq-q7VbTKocyYw \ No newline at end of file +eyJhbGciOiJSUzI1NiIsImtpZCI6IlNVc3dOaTFNUkZsRFQwWTJZalUxWjFSZlFsbzJTM2RFYTNGVFRrVjNMVGhGY25oRFRIRjVlbGsyVlEifQ.eyJyZXNwb25zZV90eXBlIjogImNvZGUiLCAic3RhdGUiOiAic3RhdGUiLCAicmVkaXJlY3RfdXJpIjogImh0dHBzOi8vZXhhbXBsZS5jb20vY2xpL2F1dGh6X2NiIiwgInNjb3BlIjogIm9wZW5pZCIsICJub25jZSI6ICI4TjRoNW1DMXlUZ2FSZlM4YjhFQUNKREFSZTNFWjRDciIsICJjbGllbnRfaWQiOiAiY2xpZW50X2lkIiwgImlzcyI6ICJjbGllbnRfaWQiLCAiaWF0IjogMTYzMjU1NDEyNywgImF1ZCI6IFsiaHR0cHM6Ly9leGFtcGxlLmNvbSJdfQ.i41o35EdqXxjd_po9tL3PssPLhHime9coujYsnDcfFG3Fhc0AllQSO_cMPe_mKfqCfkpYg--ibohGS5OEIK_h2i9j7IILpCeXf9NWqesWastyUkuAEjrYOhmTx2UfWw5SOVRs3wi2FIsOiaf64gEe1T6dgmU9T0NzGWAa5pk9GAftv3RoOsz16riLgMSl8xBsP1ReSQINsbO6pmnlAY2xqEmJNprs0Vsb63Rt0NiiNe5e36RNkDzV-uhRZY6wfNKgcn5ygcwPvcJJErAbP5k-kkgQRwiBsz5E-rdH69jXpc9WJJddO9hA9DkcrVVTSi7wJa0oqLR1XxBTspBQNbt4w \ No newline at end of file diff --git a/tests/test_13_oidc_service.py b/tests/test_13_oidc_service.py index c3a30dc..dcecbec 100644 --- a/tests/test_13_oidc_service.py +++ b/tests/test_13_oidc_service.py @@ -68,7 +68,7 @@ def create_request(self): } entity = Entity(services=DEFAULT_OIDC_SERVICES, keyjar=CLI_KEY, config=client_config) entity.client_get("service_context").issuer = 'https://example.com' - self.service = entity.client_get("service",'authorization') + self.service = entity.client_get("service", 'authorization') def test_construct(self): req_args = { @@ -137,13 +137,12 @@ def test_request_init(self): def test_request_init_request_method(self): req_args = {'response_type': 'code', 'state': 'state'} self.service.endpoint = 'https://example.com/authorize' - _info = self.service.get_request_parameters(request_args=req_args, - request_method='value') + _info = self.service.get_request_parameters(request_args=req_args, request_method='value') assert set(_info.keys()) == {'url', 'method', 'request'} msg = AuthorizationRequest().from_urlencoded( self.service.get_urlinfo(_info['url'])) - assert set(msg.to_dict()) == {'client_id', 'redirect_uri', 'request', - 'response_type', 'state', 'scope', 'nonce'} + assert set(msg.to_dict()) == {'client_id', 'redirect_uri', 'request', 'response_type', + 'scope'} _jws = jws.factory(msg['request']) assert _jws _resp = _jws.verify_compact( @@ -267,7 +266,7 @@ def create_request(self): } entity = Entity(keyjar=CLI_KEY, config=client_config, services=DEFAULT_OIDC_SERVICES) entity.client_get("service_context").issuer = 'https://example.com' - self.service = entity.client_get("service",'authorization') + self.service = entity.client_get("service", 'authorization') def test_construct_code(self): req_args = { @@ -318,7 +317,7 @@ def create_request(self): } entity = Entity(keyjar=CLI_KEY, config=client_config, services=DEFAULT_OIDC_SERVICES) entity.client_get("service_context").issuer = 'https://example.com' - self.service = entity.client_get("service",'accesstoken') + self.service = entity.client_get("service", 'accesstoken') # add some history auth_request = AuthorizationRequest( @@ -409,7 +408,7 @@ def create_service(self): } entity = Entity(keyjar=CLI_KEY, config=client_config, services=DEFAULT_OIDC_SERVICES) entity.client_get("service_context").issuer = 'https://example.com' - self.service = entity.client_get("service",'provider_info') + self.service = entity.client_get("service", 'provider_info') def test_construct(self): _req = self.service.construct() @@ -601,7 +600,7 @@ def create_request(self): } entity = Entity(keyjar=CLI_KEY, config=client_config, services=DEFAULT_OIDC_SERVICES) entity.client_get("service_context").issuer = 'https://example.com' - self.service = entity.client_get("service",'registration') + self.service = entity.client_get("service", 'registration') def test_construct(self): _req = self.service.construct() @@ -638,7 +637,7 @@ def create_request(self): } entity = Entity(keyjar=CLI_KEY, config=client_config, services=DEFAULT_OIDC_SERVICES) entity.client_get("service_context").issuer = 'https://example.com' - self.service = entity.client_get("service",'userinfo') + self.service = entity.client_get("service", 'userinfo') entity.client_get("service_context").behaviour = { 'userinfo_signed_response_alg': 'RS256', @@ -780,7 +779,7 @@ def create_request(self): }} entity = Entity(keyjar=CLI_KEY, config=client_config, services=services) entity.client_get("service_context").issuer = 'https://example.com' - self.service = entity.client_get("service",'check_session') + self.service = entity.client_get("service", 'check_session') def test_construct(self): _state_interface = self.service.client_get("service_context").state @@ -809,7 +808,7 @@ def create_request(self): }} entity = Entity(keyjar=CLI_KEY, config=client_config, services=services) entity.client_get("service_context").issuer = 'https://example.com' - self.service = entity.client_get("service",'check_id') + self.service = entity.client_get("service", 'check_id') def test_construct(self): _state_interface = self.service.client_get("service_context").state @@ -839,7 +838,7 @@ def create_request(self): }} entity = Entity(keyjar=CLI_KEY, config=client_config, services=services) entity.client_get("service_context").issuer = 'https://example.com' - self.service = entity.client_get("service",'end_session') + self.service = entity.client_get("service", 'end_session') def test_construct(self): self.service.client_get("service_context").state.store_item( @@ -880,7 +879,7 @@ def test_authz_service_conf(): } entity = Entity(keyjar=CLI_KEY, config=client_config, services=services) entity.client_get("service_context").issuer = 'https://example.com' - service = entity.client_get("service",'authorization') + service = entity.client_get("service", 'authorization') req = service.construct() assert 'claims' in req @@ -900,7 +899,7 @@ def test_add_jwks_uri_or_jwks_0(): } entity = Entity(keyjar=CLI_KEY, config=client_config, services=DEFAULT_OIDC_SERVICES) entity.client_get("service_context").issuer = 'https://example.com' - service = entity.client_get("service",'registration') + service = entity.client_get("service", 'registration') req_args, post_args = add_jwks_uri_or_jwks({}, service) assert req_args['jwks_uri'] == 'https://example.com/jwks/jwks.json' @@ -919,7 +918,7 @@ def test_add_jwks_uri_or_jwks_1(): } } entity = Entity(keyjar=CLI_KEY, config=client_config, services=DEFAULT_OIDC_SERVICES) - service = entity.client_get("service",'registration') + service = entity.client_get("service", 'registration') req_args, post_args = add_jwks_uri_or_jwks({}, service) assert req_args['jwks_uri'] == 'https://example.com/jwks/jwks.json' @@ -938,7 +937,7 @@ def test_add_jwks_uri_or_jwks_2(): } entity = Entity(keyjar=CLI_KEY, config=client_config, jwks_uri='https://example.com/jwks/jwks.json', services=DEFAULT_OIDC_SERVICES) - service = entity.client_get("service",'registration') + service = entity.client_get("service", 'registration') req_args, post_args = add_jwks_uri_or_jwks({}, service) assert req_args['jwks_uri'] == 'https://example.com/jwks/jwks.json' diff --git a/tests/test_20_rp_handler_oidc.py b/tests/test_20_rp_handler_oidc.py index 983d352..b5168db 100644 --- a/tests/test_20_rp_handler_oidc.py +++ b/tests/test_20_rp_handler_oidc.py @@ -846,7 +846,7 @@ def rphandler_setup(self): self.issuer = 'https://github.com/login/oauth/authorize' self.mock_op = MockOP(issuer=self.issuer) self.rph = RPHandler(BASE_URL, client_configs=CLIENT_CONFIG, - http_lib=self.mock_op, keyjar=KeyJar()) + http_lib=self.mock_op, keyjar=CLI_KEY) def test_finalize(self): auth_query = self.rph.begin(issuer_id='github') From bbdab145684673c74ccd380132f643df0045391c Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Sun, 26 Sep 2021 09:07:33 +0200 Subject: [PATCH 10/40] Set requirement on oidcmsg. --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 75a0fe5..c8b8625 100755 --- a/setup.py +++ b/setup.py @@ -67,7 +67,7 @@ def run_tests(self): "Programming Language :: Python :: 3.9", "Topic :: Software Development :: Libraries :: Python Modules"], install_requires=[ - 'oidcmsg==1.3.3-1', + 'oidcmsg==1.4.1', 'pyyaml>=5.1.2', 'responses' ], From d334f733ebce66506600e48c09ccc220f04e9d72 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Mon, 27 Sep 2021 10:02:41 +0200 Subject: [PATCH 11/40] Make repsonse_type actually matter. --- src/oidcrp/rp_handler.py | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/oidcrp/rp_handler.py b/src/oidcrp/rp_handler.py index db22cc6..caff746 100644 --- a/src/oidcrp/rp_handler.py +++ b/src/oidcrp/rp_handler.py @@ -380,6 +380,21 @@ def create_callbacks(self, issuer, request_uri=False): return res + def _get_response_type(self, context, req_args: Optional[dict] =None): + if req_args: + return req_args.get("response_type", context.get('behaviour')['response_types'][0]) + else: + return context.get('behaviour')['response_types'][0] + + def _pick_redirect_uri(self, context, response_type: str): + _callbacks = context.get("callbacks") + if response_type == ["code"]: + return _callbacks["code"] + elif response_type == ["form_post"]: + return _callbacks["formpost"] + else: + return _callbacks["implicit"] + def init_authorization(self, client=None, state='', req_args=None, behaviour_args=None): """ Constructs the URL that will redirect the user to the authorization @@ -400,10 +415,11 @@ def init_authorization(self, client=None, state='', req_args=None, behaviour_arg _context = client.client_get("service_context") _nonce = rndstr(24) + _response_type = self._get_response_type(_context, req_args) request_args = { - 'redirect_uri': _context.get('redirect_uris')[0], + 'redirect_uri': self._pick_redirect_uri(_context, _response_type), 'scope': _context.get('behaviour')['scope'], - 'response_type': _context.get('behaviour')['response_types'][0], + 'response_type': _response_type, 'nonce': _nonce } From 41a8e9c0a1d7fedaa13cb4f51dfd87fd2cb9565e Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Tue, 28 Sep 2021 09:32:55 +0200 Subject: [PATCH 12/40] Might not have an ID Token. Changed name to reflect reality. --- example/flask_rp/views.py | 6 +++--- src/oidcrp/rp_handler.py | 5 ++++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/example/flask_rp/views.py b/example/flask_rp/views.py index 7efbfe9..caa7141 100644 --- a/example/flask_rp/views.py +++ b/example/flask_rp/views.py @@ -185,10 +185,10 @@ def repost_fragment(): return finalize(op_identifier, args) -@oidc_rp_views.route('/ihf_cb') -def ihf_cb(self, op_identifier='', **kwargs): +@oidc_rp_views.route('/authz_im_cb') +def authz_im_cb(op_identifier='', **kwargs): logger.debug('implicit_hybrid_flow kwargs: {}'.format(kwargs)) - return render_template('repost_fragment.html') + return render_template('repost_fragment.html', op_identifier=op_identifier) @oidc_rp_views.route('/session_iframe') diff --git a/src/oidcrp/rp_handler.py b/src/oidcrp/rp_handler.py index caff746..ebe39cb 100644 --- a/src/oidcrp/rp_handler.py +++ b/src/oidcrp/rp_handler.py @@ -813,7 +813,10 @@ def finalize(self, issuer, response, behaviour_args: Optional[dict] = None): else: _context.state.store_sid2state(sid, _state) - _context.state.store_sub2state(_id_token['sub'], _state) + if _id_token: + _context.state.store_sub2state(_id_token['sub'], _state) + else: + _context.state.store_sub2state(inforesp['sub'], _state) return { 'userinfo': inforesp, From d941becd6a5d0c36300c86ee3abbda66d7854598 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Tue, 28 Sep 2021 10:52:05 +0200 Subject: [PATCH 13/40] Collect and ID Token if it's not collected as part of the normal stream. --- src/oidcrp/rp_handler.py | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/src/oidcrp/rp_handler.py b/src/oidcrp/rp_handler.py index ebe39cb..fc0e8cc 100644 --- a/src/oidcrp/rp_handler.py +++ b/src/oidcrp/rp_handler.py @@ -682,13 +682,14 @@ def finalize_auth(self, client, issuer: str, response: dict, authorization_response['state']) return authorization_response - def get_access_and_id_token(self, authorization_response=None, state='', - client=None): + def get_access_and_id_token(self, authorization_response=None, state='', client=None, + behaviour_args: Optionl[dict] =None): """ There are a number of services where access tokens and ID tokens can occur in the response. This method goes through the possible places based on the response_type the client uses. + :param behaviour_args: For fine tuning behaviour :param authorization_response: The Authorization response :param state: The state key (the state parameter in the authorization request) @@ -726,19 +727,26 @@ def get_access_and_id_token(self, authorization_response=None, state='', if _resp_type in [{'token'}, {'id_token', 'token'}, {'code', 'token'}, {'code', 'id_token', 'token'}]: access_token = authorization_response["access_token"] - elif _resp_type in [{'code'}, {'code', 'id_token'}]: + if behaviour_args and behaviour_args.get("collect_id_token", False): + if "id_token" not in _resp_type: + # get the access token + token_resp = self.get_access_token(state, client=client) + if is_error_message(token_resp): + return False, "Invalid response %s." % token_resp["error"] + # Now which access_token should I use + access_token = token_resp["access_token"] + # May or may not get an ID Token + id_token = token_resp.get('__verified_id_token') + elif _resp_type in [{'code'}, {'code', 'id_token'}]: # get the access token token_resp = self.get_access_token(state, client=client) if is_error_message(token_resp): return False, "Invalid response %s." % token_resp["error"] access_token = token_resp["access_token"] - - try: - id_token = token_resp['__verified_id_token'] - except KeyError: - pass + # May or may not get an ID Token + id_token = token_resp.get('__verified_id_token') return {'access_token': access_token, 'id_token': id_token} @@ -771,8 +779,8 @@ def finalize(self, issuer, response, behaviour_args: Optional[dict] = None): } _state = authorization_response['state'] - token = self.get_access_and_id_token(authorization_response, - state=_state, client=client) + token = self.get_access_and_id_token(authorization_response, state=_state, client=client, + behaviour_args=behaviour_args) _id_token = token.get("id_token") logger.debug(f"ID Token: {_id_token}") From 9a5426e94187062beace948f900e3730d4406321 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Tue, 28 Sep 2021 10:54:45 +0200 Subject: [PATCH 14/40] Collect and ID Token if it's not collected as part of the normal stream. --- src/oidcrp/rp_handler.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/oidcrp/rp_handler.py b/src/oidcrp/rp_handler.py index fc0e8cc..113613d 100644 --- a/src/oidcrp/rp_handler.py +++ b/src/oidcrp/rp_handler.py @@ -380,7 +380,7 @@ def create_callbacks(self, issuer, request_uri=False): return res - def _get_response_type(self, context, req_args: Optional[dict] =None): + def _get_response_type(self, context, req_args: Optional[dict] = None): if req_args: return req_args.get("response_type", context.get('behaviour')['response_types'][0]) else: @@ -682,8 +682,10 @@ def finalize_auth(self, client, issuer: str, response: dict, authorization_response['state']) return authorization_response - def get_access_and_id_token(self, authorization_response=None, state='', client=None, - behaviour_args: Optionl[dict] =None): + def get_access_and_id_token(self, authorization_response=None, + state: Optional[str] = '', + client: Optional[object] =None, + behaviour_args: Optional[dict] = None): """ There are a number of services where access tokens and ID tokens can occur in the response. This method goes through the possible places From 381fcf1874daf33a19c9c0d693ba1a681e0bf2a4 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Tue, 28 Sep 2021 10:59:25 +0200 Subject: [PATCH 15/40] Log --- src/oidcrp/rp_handler.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/oidcrp/rp_handler.py b/src/oidcrp/rp_handler.py index 113613d..73fd886 100644 --- a/src/oidcrp/rp_handler.py +++ b/src/oidcrp/rp_handler.py @@ -731,6 +731,7 @@ def get_access_and_id_token(self, authorization_response=None, access_token = authorization_response["access_token"] if behaviour_args and behaviour_args.get("collect_id_token", False): if "id_token" not in _resp_type: + logger.debug("Collect ID Token") # get the access token token_resp = self.get_access_token(state, client=client) if is_error_message(token_resp): @@ -773,6 +774,9 @@ def finalize(self, issuer, response, behaviour_args: Optional[dict] = None): client = self.issuer2rp[issuer] + if behaviour_args: + logger.debug(f"Behaviour args: {behaviour_args}") + authorization_response = self.finalize_auth(client, issuer, response) if is_error_message(authorization_response): return { From 816498cc0c7e5664b5af2fe7cea7dafc83d11903 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 30 Sep 2021 09:26:32 +0200 Subject: [PATCH 16/40] Log --- src/oidcrp/oauth2/utils.py | 20 +++++++++++--------- src/oidcrp/rp_handler.py | 2 +- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/oidcrp/oauth2/utils.py b/src/oidcrp/oauth2/utils.py index 2393766..2969bfc 100644 --- a/src/oidcrp/oauth2/utils.py +++ b/src/oidcrp/oauth2/utils.py @@ -1,5 +1,9 @@ +import logging + from oidcmsg.exception import MissingParameter +logger = logging.getLogger(__name__) + def get_state_parameter(request_args, kwargs): """Find a state value from a set of possible places.""" @@ -23,16 +27,10 @@ def pick_redirect_uris(request_args=None, service=None, **kwargs): _callback = _context.callback if _callback: - try: - _response_type = request_args['response_type'] - except KeyError: - _response_type = _context.behaviour['response_types'][0] - request_args['response_type'] = _response_type + _response_type = request_args.get('response_type', _context.behaviour['response_types'][0]) + request_args['response_type'] = _response_type - try: - _response_mode = request_args['response_mode'] - except KeyError: - _response_mode = '' + _response_mode = request_args.get('response_mode') if _response_mode == 'form_post': request_args['redirect_uri'] = _callback['form_post'] @@ -40,6 +38,10 @@ def pick_redirect_uris(request_args=None, service=None, **kwargs): request_args['redirect_uri'] = _callback['code'] else: request_args['redirect_uri'] = _callback['implicit'] + + logger.debug( + f"pick_redirect_uris: response_type={_response_type}, response_mode={_response_mode}, " + f"redirect_uri={request_args['redirect_uri']}") else: request_args['redirect_uri'] = _context.redirect_uris[0] diff --git a/src/oidcrp/rp_handler.py b/src/oidcrp/rp_handler.py index 73fd886..991e4f6 100644 --- a/src/oidcrp/rp_handler.py +++ b/src/oidcrp/rp_handler.py @@ -775,7 +775,7 @@ def finalize(self, issuer, response, behaviour_args: Optional[dict] = None): client = self.issuer2rp[issuer] if behaviour_args: - logger.debug(f"Behaviour args: {behaviour_args}") + logger.debug(f"Finalize behaviour args: {behaviour_args}") authorization_response = self.finalize_auth(client, issuer, response) if is_error_message(authorization_response): From 6029656a6c6c7e7f5c8ccd1f81f45a1e02a48737 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 30 Sep 2021 09:46:40 +0200 Subject: [PATCH 17/40] Refactoring --- src/oidcrp/oauth2/authorization.py | 4 ++-- src/oidcrp/oauth2/utils.py | 23 ++++++++++++++++------- src/oidcrp/oidc/authorization.py | 4 ++-- src/oidcrp/rp_handler.py | 12 ++---------- 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/oidcrp/oauth2/authorization.py b/src/oidcrp/oauth2/authorization.py index 366b223..d82c8e1 100644 --- a/src/oidcrp/oauth2/authorization.py +++ b/src/oidcrp/oauth2/authorization.py @@ -7,7 +7,7 @@ from oidcmsg.time_util import time_sans_frac from oidcrp.oauth2.utils import get_state_parameter -from oidcrp.oauth2.utils import pick_redirect_uris +from oidcrp.oauth2.utils import pre_construct_pick_redirect_uri from oidcrp.oauth2.utils import set_state_parameter from oidcrp.service import Service @@ -32,7 +32,7 @@ class Authorization(Service): def __init__(self, client_get, client_authn_factory=None, conf=None): Service.__init__(self, client_get, client_authn_factory=client_authn_factory, conf=conf) - self.pre_construct.extend([pick_redirect_uris, set_state_parameter]) + self.pre_construct.extend([pre_construct_pick_redirect_uri, set_state_parameter]) self.post_construct.append(self.store_auth_request) def update_service_context(self, resp, key='', **kwargs): diff --git a/src/oidcrp/oauth2/utils.py b/src/oidcrp/oauth2/utils.py index 2969bfc..ac5ad07 100644 --- a/src/oidcrp/oauth2/utils.py +++ b/src/oidcrp/oauth2/utils.py @@ -1,6 +1,11 @@ import logging +from typing import Optional +from typing import Union from oidcmsg.exception import MissingParameter +from oidcmsg.message import Message + +from oidcrp.service import Service logger = logging.getLogger(__name__) @@ -18,16 +23,13 @@ def get_state_parameter(request_args, kwargs): return _state -def pick_redirect_uris(request_args=None, service=None, **kwargs): - """Pick one redirect_uri base on response_mode out of a list of such.""" - _context = service.client_get("service_context") - +def pick_redirect_uri(context, request_args=None): if 'redirect_uri' in request_args: return request_args, {} - _callback = _context.callback + _callback = context.callback if _callback: - _response_type = request_args.get('response_type', _context.behaviour['response_types'][0]) + _response_type = request_args.get('response_type', context.behaviour['response_types'][0]) request_args['response_type'] = _response_type _response_mode = request_args.get('response_mode') @@ -43,11 +45,18 @@ def pick_redirect_uris(request_args=None, service=None, **kwargs): f"pick_redirect_uris: response_type={_response_type}, response_mode={_response_mode}, " f"redirect_uri={request_args['redirect_uri']}") else: - request_args['redirect_uri'] = _context.redirect_uris[0] + request_args['redirect_uri'] = context.redirect_uris[0] return request_args, {} +def pre_construct_pick_redirect_uri(request_args: Optional[Union[Message, dict]] = None, + service: Optional[Service] = None, **kwargs): + _context = service.client_get("service_context") + request_args["redirect_uri"] = pick_redirect_uri(_context, request_args) + return request_args, {} + + def set_state_parameter(request_args=None, **kwargs): """Assigned a state value.""" request_args['state'] = get_state_parameter(request_args, kwargs) diff --git a/src/oidcrp/oidc/authorization.py b/src/oidcrp/oidc/authorization.py index 106ce89..1e858ef 100644 --- a/src/oidcrp/oidc/authorization.py +++ b/src/oidcrp/oidc/authorization.py @@ -8,7 +8,7 @@ from oidcrp.exception import ParameterError from oidcrp.oauth2 import authorization -from oidcrp.oauth2.utils import pick_redirect_uris +from oidcrp.oauth2.utils import pre_construct_pick_redirect_uri from oidcrp.oidc import IDT2REG from oidcrp.oidc.utils import construct_request_uri from oidcrp.oidc.utils import request_object_encryption @@ -28,7 +28,7 @@ def __init__(self, client_get, client_authn_factory=None, conf=None): authorization.Authorization.__init__(self, client_get, client_authn_factory, conf=conf) self.default_request_args = {'scope': ['openid']} - self.pre_construct = [self.set_state, pick_redirect_uris, + self.pre_construct = [self.set_state, pre_construct_pick_redirect_uri, self.oidc_pre_construct] self.post_construct = [self.oidc_post_construct] diff --git a/src/oidcrp/rp_handler.py b/src/oidcrp/rp_handler.py index 991e4f6..7a1f485 100644 --- a/src/oidcrp/rp_handler.py +++ b/src/oidcrp/rp_handler.py @@ -28,6 +28,7 @@ from .defaults import DEFAULT_RP_KEY_DEFS from .exception import OidcServiceError from .oauth2 import Client +from .oauth2.utils import pick_redirect_uri from .util import add_path from .util import dynamic_provider_info_discovery from .util import load_registration_response @@ -386,15 +387,6 @@ def _get_response_type(self, context, req_args: Optional[dict] = None): else: return context.get('behaviour')['response_types'][0] - def _pick_redirect_uri(self, context, response_type: str): - _callbacks = context.get("callbacks") - if response_type == ["code"]: - return _callbacks["code"] - elif response_type == ["form_post"]: - return _callbacks["formpost"] - else: - return _callbacks["implicit"] - def init_authorization(self, client=None, state='', req_args=None, behaviour_args=None): """ Constructs the URL that will redirect the user to the authorization @@ -417,7 +409,7 @@ def init_authorization(self, client=None, state='', req_args=None, behaviour_arg _nonce = rndstr(24) _response_type = self._get_response_type(_context, req_args) request_args = { - 'redirect_uri': self._pick_redirect_uri(_context, _response_type), + 'redirect_uri': pick_redirect_uri(_context, _response_type), 'scope': _context.get('behaviour')['scope'], 'response_type': _response_type, 'nonce': _nonce From b5926ef02f6a55022691e0e9e2c73315c588374c Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 30 Sep 2021 09:58:34 +0200 Subject: [PATCH 18/40] Refactoring --- src/oidcrp/oauth2/utils.py | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/oidcrp/oauth2/utils.py b/src/oidcrp/oauth2/utils.py index ac5ad07..e3f0c0c 100644 --- a/src/oidcrp/oauth2/utils.py +++ b/src/oidcrp/oauth2/utils.py @@ -25,29 +25,32 @@ def get_state_parameter(request_args, kwargs): def pick_redirect_uri(context, request_args=None): if 'redirect_uri' in request_args: - return request_args, {} + return request_args["redirect_uri"] + + if context.callback: + _conf_resp_types = context.behaviour.get('response_types', []) + _response_type = request_args.get('response_type') + if not _response_type and _conf_resp_types: + _response_type = _conf_resp_types[0] - _callback = context.callback - if _callback: - _response_type = request_args.get('response_type', context.behaviour['response_types'][0]) request_args['response_type'] = _response_type _response_mode = request_args.get('response_mode') if _response_mode == 'form_post': - request_args['redirect_uri'] = _callback['form_post'] + redirect_uri = context.callback['form_post'] elif _response_type == 'code': - request_args['redirect_uri'] = _callback['code'] + redirect_uri = context.callback['code'] else: - request_args['redirect_uri'] = _callback['implicit'] + redirect_uri = context.callback['implicit'] logger.debug( f"pick_redirect_uris: response_type={_response_type}, response_mode={_response_mode}, " - f"redirect_uri={request_args['redirect_uri']}") + f"redirect_uri={redirect_uri}") else: - request_args['redirect_uri'] = context.redirect_uris[0] + redirect_uri = context.redirect_uris[0] - return request_args, {} + return redirect_uri def pre_construct_pick_redirect_uri(request_args: Optional[Union[Message, dict]] = None, From f8d04149819d41c9302b70170789a879e9952662 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 30 Sep 2021 10:11:23 +0200 Subject: [PATCH 19/40] Bumped version. --- src/oidcrp/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/oidcrp/__init__.py b/src/oidcrp/__init__.py index 23161e0..823f282 100644 --- a/src/oidcrp/__init__.py +++ b/src/oidcrp/__init__.py @@ -1,7 +1,7 @@ import logging __author__ = 'Roland Hedberg' -__version__ = '2.0.1' +__version__ = '2.1.0' logger = logging.getLogger(__name__) From 7d898748a1f7050df1829a1e6b9fae92b93eea96 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 30 Sep 2021 11:25:02 +0200 Subject: [PATCH 20/40] Fixed pick_redirect_uri --- src/oidcrp/oauth2/utils.py | 25 +++++++++++++++---------- src/oidcrp/rp_handler.py | 6 +++--- tests/pub_client.jwks | 2 +- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/oidcrp/oauth2/utils.py b/src/oidcrp/oauth2/utils.py index e3f0c0c..76a4c53 100644 --- a/src/oidcrp/oauth2/utils.py +++ b/src/oidcrp/oauth2/utils.py @@ -23,29 +23,33 @@ def get_state_parameter(request_args, kwargs): return _state -def pick_redirect_uri(context, request_args=None): +def pick_redirect_uri(context, + request_args: Optional[Union[Message, dict]] = None, + response_type: Optional[str] = ''): + if request_args is None: + request_args = {} + if 'redirect_uri' in request_args: return request_args["redirect_uri"] if context.callback: - _conf_resp_types = context.behaviour.get('response_types', []) - _response_type = request_args.get('response_type') - if not _response_type and _conf_resp_types: - _response_type = _conf_resp_types[0] - - request_args['response_type'] = _response_type + if not response_type: + _conf_resp_types = context.behaviour.get('response_types', []) + response_type = request_args.get('response_type') + if not response_type and _conf_resp_types: + response_type = _conf_resp_types[0] _response_mode = request_args.get('response_mode') if _response_mode == 'form_post': redirect_uri = context.callback['form_post'] - elif _response_type == 'code': + elif response_type == 'code': redirect_uri = context.callback['code'] else: redirect_uri = context.callback['implicit'] logger.debug( - f"pick_redirect_uris: response_type={_response_type}, response_mode={_response_mode}, " + f"pick_redirect_uris: response_type={response_type}, response_mode={_response_mode}, " f"redirect_uri={redirect_uri}") else: redirect_uri = context.redirect_uris[0] @@ -56,7 +60,8 @@ def pick_redirect_uri(context, request_args=None): def pre_construct_pick_redirect_uri(request_args: Optional[Union[Message, dict]] = None, service: Optional[Service] = None, **kwargs): _context = service.client_get("service_context") - request_args["redirect_uri"] = pick_redirect_uri(_context, request_args) + request_args["redirect_uri"] = pick_redirect_uri(_context, + request_args=request_args) return request_args, {} diff --git a/src/oidcrp/rp_handler.py b/src/oidcrp/rp_handler.py index 7a1f485..be6eb95 100644 --- a/src/oidcrp/rp_handler.py +++ b/src/oidcrp/rp_handler.py @@ -275,7 +275,7 @@ def do_client_registration(self, client=None, _context.set('redirect_uris', [ v for k, v in callbacks.items() if not k.startswith('__')]) - _context.set('callbacks', callbacks) + _context.set('callback', callbacks) if behaviour_args: _params = RegistrationRequest().parameters() @@ -295,7 +295,7 @@ def add_callbacks(self, service_context): service_context.set('redirect_uris', [ v for k, v in _callbacks.items() if not k.startswith('__')]) - service_context.set('callbacks', _callbacks) + service_context.set('callback', _callbacks) return _callbacks def do_webfinger(self, user): @@ -409,7 +409,7 @@ def init_authorization(self, client=None, state='', req_args=None, behaviour_arg _nonce = rndstr(24) _response_type = self._get_response_type(_context, req_args) request_args = { - 'redirect_uri': pick_redirect_uri(_context, _response_type), + 'redirect_uri': pick_redirect_uri(_context, response_type=_response_type), 'scope': _context.get('behaviour')['scope'], 'response_type': _response_type, 'nonce': _nonce diff --git a/tests/pub_client.jwks b/tests/pub_client.jwks index a57e904..d16a636 100644 --- a/tests/pub_client.jwks +++ b/tests/pub_client.jwks @@ -1 +1 @@ -{"keys": [{"kty": "RSA", "use": "sig", "kid": "SUswNi1MRFlDT0Y2YjU1Z1RfQlo2S3dEa3FTTkV3LThFcnhDTHF5elk2VQ", "n": "0UkUx2ewKyc-XJ1o0ToyGjws_JybAMZj2oYjsPyyvQ_T5dhZ2VmRRRkhsaVJ2xE_GGc7mSG0IjmGFyXp5y0w4mJBcsAEE5-8eBTvQdYIryjW74r3jt6Fi4Hlm1yFMTie3apv8mw79BUj-jT0kh3_m-FiKKUvLsq45DcLtTJ4cx7Ize37dl1sFSpQcoYMk7eiUEM8fiNboiVwvBYNAWVMkUM-LnVUPm3UjvKp0LihYEkZFWOxmuQmj2x25SFUkjus38ERrRqJQBZduxdBHFrWtWg8yOA53BkMU0FFg_r0H3ctl-5GaKw-BWlogU4qXnsq85xy0EoenRk7FPV8g_ulJw", "e": "AQAB"}, {"kty": "EC", "use": "sig", "kid": "NC1pdGRQN002bWM3bk1xX2R0SktscElqbFdtN29ITDV2WVd2b0hOYzREVQ", "crv": "P-256", "x": "kK7Qp1woSerI7rUOAwW_4sU6ZmwV3wwXKX3VU-v2fMI", "y": "iPWd_Pjq6EjxYy08KNFZ3PxhEwgWHgAQTTknlKMKJA0"}]} \ No newline at end of file +{"keys": [{"kty": "RSA", "use": "sig", "kid": "SUswNi1MRFlDT0Y2YjU1Z1RfQlo2S3dEa3FTTkV3LThFcnhDTHF5elk2VQ", "e": "AQAB", "n": "0UkUx2ewKyc-XJ1o0ToyGjws_JybAMZj2oYjsPyyvQ_T5dhZ2VmRRRkhsaVJ2xE_GGc7mSG0IjmGFyXp5y0w4mJBcsAEE5-8eBTvQdYIryjW74r3jt6Fi4Hlm1yFMTie3apv8mw79BUj-jT0kh3_m-FiKKUvLsq45DcLtTJ4cx7Ize37dl1sFSpQcoYMk7eiUEM8fiNboiVwvBYNAWVMkUM-LnVUPm3UjvKp0LihYEkZFWOxmuQmj2x25SFUkjus38ERrRqJQBZduxdBHFrWtWg8yOA53BkMU0FFg_r0H3ctl-5GaKw-BWlogU4qXnsq85xy0EoenRk7FPV8g_ulJw"}, {"kty": "EC", "use": "sig", "kid": "NC1pdGRQN002bWM3bk1xX2R0SktscElqbFdtN29ITDV2WVd2b0hOYzREVQ", "crv": "P-256", "x": "kK7Qp1woSerI7rUOAwW_4sU6ZmwV3wwXKX3VU-v2fMI", "y": "iPWd_Pjq6EjxYy08KNFZ3PxhEwgWHgAQTTknlKMKJA0"}]} \ No newline at end of file From bdaffb648b13cdcf018c39e7f88e17f136abeec8 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Fri, 1 Oct 2021 08:51:46 +0200 Subject: [PATCH 21/40] Missed request args. Necessary !! --- src/oidcrp/rp_handler.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/oidcrp/rp_handler.py b/src/oidcrp/rp_handler.py index be6eb95..9fb4b99 100644 --- a/src/oidcrp/rp_handler.py +++ b/src/oidcrp/rp_handler.py @@ -409,7 +409,9 @@ def init_authorization(self, client=None, state='', req_args=None, behaviour_arg _nonce = rndstr(24) _response_type = self._get_response_type(_context, req_args) request_args = { - 'redirect_uri': pick_redirect_uri(_context, response_type=_response_type), + 'redirect_uri': pick_redirect_uri(_context, + request_args=req_args, + response_type=_response_type), 'scope': _context.get('behaviour')['scope'], 'response_type': _response_type, 'nonce': _nonce From 7bce59ca21c93f280c63976e316868c4bf56cfdd Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Mon, 4 Oct 2021 08:54:49 +0200 Subject: [PATCH 22/40] Extended add_callbacks to deal with more callback uris. --- src/oidcrp/rp_handler.py | 83 +++++++++++++++++++++----------- src/oidcrp/service.py | 2 +- tests/pub_client.jwks | 2 +- tests/test_20_rp_handler_oidc.py | 2 +- 4 files changed, 59 insertions(+), 30 deletions(-) diff --git a/src/oidcrp/rp_handler.py b/src/oidcrp/rp_handler.py index 9fb4b99..851c82d 100644 --- a/src/oidcrp/rp_handler.py +++ b/src/oidcrp/rp_handler.py @@ -237,6 +237,30 @@ def do_provider_info(self, client=None, state='', behaviour_args=None): except KeyError: return _context.get('issuer') + def add_callbacks(self, context): + _iss = context.get('issuer') + # Create the necessary callback URLs + # as a side effect self.hash2issuer is set + _extra_uris = { + "request_uri": False, + "backchannel_logout_uri": False, + "frontchannel_logout_uri": False + } + _pi = context.get('provider_info') + _cp = context.config.get("client_preferences") + if 'require_request_uri_registration' in _pi and "request_uri_usable" in _cp: + _extra_uris['request_uri'] = True + if 'frontchannel_logout_supported' in _pi and "frontchannel_logout_usable" in _cp: + _extra_uris["frontchannel_logout_uri"] = True + if 'backchannel_logout_supported' in _pi and "backchannel_logout_usable" in _cp: + _extra_uris["backchannel_logout_uri"] = True + + callbacks = self.create_callbacks(_iss, **_extra_uris) + + context.set('redirect_uris', [ + v for k, v in callbacks.items() if not k.startswith('__')]) + context.set('callback', callbacks) + def do_client_registration(self, client=None, iss_id: Optional[str] = '', state: Optional[str] = '', @@ -266,16 +290,7 @@ def do_client_registration(self, client=None, if not _context.client_id: # means I have to do dynamic client registration if not _context.get('redirect_uris'): - # Create the necessary callback URLs - # as a side effect self.hash2issuer is set - if 'require_request_uri_registration' in _context.get('provider_info'): - callbacks = self.create_callbacks(_iss, request_uri=True) - else: - callbacks = self.create_callbacks(_iss) - - _context.set('redirect_uris', [ - v for k, v in callbacks.items() if not k.startswith('__')]) - _context.set('callback', callbacks) + self.add_callbacks(_context) if behaviour_args: _params = RegistrationRequest().parameters() @@ -285,19 +300,6 @@ def do_client_registration(self, client=None, load_registration_response(client, request_args=request_args) - def add_callbacks(self, service_context): - _iss = service_context.get('issuer') - - if 'require_request_uri_registration' in service_context.get('provider_info'): - _callbacks = self.create_callbacks(_iss, request_uri=True) - else: - _callbacks = self.create_callbacks(_iss) - - service_context.set('redirect_uris', [ - v for k, v in _callbacks.items() if not k.startswith('__')]) - service_context.set('callback', _callbacks) - return _callbacks - def do_webfinger(self, user): """ Does OpenID Provider Issuer discovery using webfinger. @@ -356,12 +358,16 @@ def client_setup(self, iss_id='', user='', behaviour_args=None): self.issuer2rp[issuer] = client return client - def create_callbacks(self, issuer, request_uri=False): + def create_callbacks(self, issuer, request_uri=False, backchannel_logout_uri=False, + frontchannel_logout_uri=False): """ To mitigate some security issues the redirect_uris should be OP/AS specific. This method creates a set of redirect_uris unique to the OP/AS. + :param frontchannel_logout_uri: Whether a front-channel logout uri should be constructed + :param backchannel_logout_uri: Whether a back-channel logout uri should be constructed + :param request_uri: Whether a request_uri should be constructed :param issuer: Issuer ID :return: A set of redirect_uris """ @@ -379,6 +385,15 @@ def create_callbacks(self, issuer, request_uri=False): if request_uri: res["request_uri"] = f"{self.base_url}/req_uri/{_hex}" + if backchannel_logout_uri or frontchannel_logout_uri: + res["post_logout_redirect_uris"] = f"{self.base_url}/session_logout/{_hex}" + + if backchannel_logout_uri: + res["backchannel_logout_uri"] = f"{self.base_url}/bc_logout/{_hex}" + if frontchannel_logout_uri: + res["frontchannel_logout_uri"] = f"{self.base_url}/fc_logout/{_hex}" + + logger.debug(f"Created callback URIs: {res}") return res def _get_response_type(self, context, req_args: Optional[dict] = None): @@ -678,7 +693,7 @@ def finalize_auth(self, client, issuer: str, response: dict, def get_access_and_id_token(self, authorization_response=None, state: Optional[str] = '', - client: Optional[object] =None, + client: Optional[object] = None, behaviour_args: Optional[dict] = None): """ There are a number of services where access tokens and ID tokens can @@ -830,7 +845,8 @@ def finalize(self, issuer, response, behaviour_args: Optional[dict] = None): 'userinfo': inforesp, 'state': authorization_response['state'], 'token': token['access_token'], - 'id_token': _id_token + 'id_token': _id_token, + 'session_state': authorization_response.get('session_state', '') } def has_active_authentication(self, state): @@ -898,7 +914,9 @@ def get_valid_access_token(self, state): else: raise OidcServiceError('No valid access token') - def logout(self, state, client=None, post_logout_redirect_uri=''): + def logout(self, state: str, + client: Optional[Client] = None, + post_logout_redirect_uri: Optional[str] = '') -> dict: """ Does a RP initiated logout from an OP. After logout the user will be redirect by the OP to a URL of choice (post_logout_redirect_uri). @@ -929,6 +947,17 @@ def logout(self, state, client=None, post_logout_redirect_uri=''): return resp + def close(self, state: str, + issuer: Optional[str] = '', + post_logout_redirect_uri: Optional[str] = '') -> dict: + if issuer: + client = self.issuer2rp[issuer] + else: + client = self.get_client_from_session_key(state) + + return self.logout(state=state, client=client, + post_logout_redirect_uri=post_logout_redirect_uri) + def clear_session(self, state): client = self.get_client_from_session_key(state) client.client_get("service_context").state.remove_state(state) diff --git a/src/oidcrp/service.py b/src/oidcrp/service.py index 29d19a1..ea3bc63 100644 --- a/src/oidcrp/service.py +++ b/src/oidcrp/service.py @@ -344,7 +344,7 @@ def get_headers(self, return _headers def get_request_parameters(self, request_args=None, method="", - request_body_type="", authn_method='', **kwargs): + request_body_type="", authn_method='', **kwargs) -> dict: """ Builds the request message and constructs the HTTP headers. diff --git a/tests/pub_client.jwks b/tests/pub_client.jwks index d16a636..a57e904 100644 --- a/tests/pub_client.jwks +++ b/tests/pub_client.jwks @@ -1 +1 @@ -{"keys": [{"kty": "RSA", "use": "sig", "kid": "SUswNi1MRFlDT0Y2YjU1Z1RfQlo2S3dEa3FTTkV3LThFcnhDTHF5elk2VQ", "e": "AQAB", "n": "0UkUx2ewKyc-XJ1o0ToyGjws_JybAMZj2oYjsPyyvQ_T5dhZ2VmRRRkhsaVJ2xE_GGc7mSG0IjmGFyXp5y0w4mJBcsAEE5-8eBTvQdYIryjW74r3jt6Fi4Hlm1yFMTie3apv8mw79BUj-jT0kh3_m-FiKKUvLsq45DcLtTJ4cx7Ize37dl1sFSpQcoYMk7eiUEM8fiNboiVwvBYNAWVMkUM-LnVUPm3UjvKp0LihYEkZFWOxmuQmj2x25SFUkjus38ERrRqJQBZduxdBHFrWtWg8yOA53BkMU0FFg_r0H3ctl-5GaKw-BWlogU4qXnsq85xy0EoenRk7FPV8g_ulJw"}, {"kty": "EC", "use": "sig", "kid": "NC1pdGRQN002bWM3bk1xX2R0SktscElqbFdtN29ITDV2WVd2b0hOYzREVQ", "crv": "P-256", "x": "kK7Qp1woSerI7rUOAwW_4sU6ZmwV3wwXKX3VU-v2fMI", "y": "iPWd_Pjq6EjxYy08KNFZ3PxhEwgWHgAQTTknlKMKJA0"}]} \ No newline at end of file +{"keys": [{"kty": "RSA", "use": "sig", "kid": "SUswNi1MRFlDT0Y2YjU1Z1RfQlo2S3dEa3FTTkV3LThFcnhDTHF5elk2VQ", "n": "0UkUx2ewKyc-XJ1o0ToyGjws_JybAMZj2oYjsPyyvQ_T5dhZ2VmRRRkhsaVJ2xE_GGc7mSG0IjmGFyXp5y0w4mJBcsAEE5-8eBTvQdYIryjW74r3jt6Fi4Hlm1yFMTie3apv8mw79BUj-jT0kh3_m-FiKKUvLsq45DcLtTJ4cx7Ize37dl1sFSpQcoYMk7eiUEM8fiNboiVwvBYNAWVMkUM-LnVUPm3UjvKp0LihYEkZFWOxmuQmj2x25SFUkjus38ERrRqJQBZduxdBHFrWtWg8yOA53BkMU0FFg_r0H3ctl-5GaKw-BWlogU4qXnsq85xy0EoenRk7FPV8g_ulJw", "e": "AQAB"}, {"kty": "EC", "use": "sig", "kid": "NC1pdGRQN002bWM3bk1xX2R0SktscElqbFdtN29ITDV2WVd2b0hOYzREVQ", "crv": "P-256", "x": "kK7Qp1woSerI7rUOAwW_4sU6ZmwV3wwXKX3VU-v2fMI", "y": "iPWd_Pjq6EjxYy08KNFZ3PxhEwgWHgAQTTknlKMKJA0"}]} \ No newline at end of file diff --git a/tests/test_20_rp_handler_oidc.py b/tests/test_20_rp_handler_oidc.py index b5168db..55a6d73 100644 --- a/tests/test_20_rp_handler_oidc.py +++ b/tests/test_20_rp_handler_oidc.py @@ -898,7 +898,7 @@ def test_finalize(self): # assume code flow resp = self.rph.finalize(_session['iss'], auth_response.to_dict()) - assert set(resp.keys()) == {'userinfo', 'state', 'token', 'id_token'} + assert set(resp.keys()) == {'userinfo', 'state', 'token', 'id_token', 'session_state'} def test_dynamic_setup(self): user_id = 'acct:foobar@example.com' From a84270ba82b5f55df24e0c3fc92655d7aa08df60 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Tue, 5 Oct 2021 12:55:56 +0200 Subject: [PATCH 23/40] Add callback uris handled by the registration service instead of the RP handler. --- src/oidcrp/oauth2/utils.py | 8 +- src/oidcrp/oidc/registration.py | 157 ++++++++++++++++++++++--- src/oidcrp/rp_handler.py | 117 ++++++++---------- src/oidcrp/service_context.py | 10 ++ src/oidcrp/util.py | 2 +- tests/request123456.jwt | 2 +- tests/test_13_oidc_service.py | 61 ++++++++-- tests/test_20_rp_handler_oidc.py | 19 +-- tests/test_21_rph_defaults.py | 2 - tests/test_40_rp_handler_persistent.py | 17 --- 10 files changed, 274 insertions(+), 121 deletions(-) diff --git a/src/oidcrp/oauth2/utils.py b/src/oidcrp/oauth2/utils.py index 76a4c53..eb11816 100644 --- a/src/oidcrp/oauth2/utils.py +++ b/src/oidcrp/oauth2/utils.py @@ -3,6 +3,7 @@ from typing import Union from oidcmsg.exception import MissingParameter +from oidcmsg.exception import MissingRequiredAttribute from oidcmsg.message import Message from oidcrp.service import Service @@ -32,7 +33,9 @@ def pick_redirect_uri(context, if 'redirect_uri' in request_args: return request_args["redirect_uri"] - if context.callback: + if context.redirect_uris: + redirect_uri = context.redirect_uris[0] + elif context.callback: if not response_type: _conf_resp_types = context.behaviour.get('response_types', []) response_type = request_args.get('response_type') @@ -52,7 +55,8 @@ def pick_redirect_uri(context, f"pick_redirect_uris: response_type={response_type}, response_mode={_response_mode}, " f"redirect_uri={redirect_uri}") else: - redirect_uri = context.redirect_uris[0] + logger.error("No redirect_uri") + raise MissingRequiredAttribute('redirect_uri') return redirect_uri diff --git a/src/oidcrp/oidc/registration.py b/src/oidcrp/oidc/registration.py index 925cc66..9b1c367 100644 --- a/src/oidcrp/oidc/registration.py +++ b/src/oidcrp/oidc/registration.py @@ -1,5 +1,9 @@ +import hashlib import logging +from typing import List +from typing import Optional +from cryptojwt.utils import as_bytes from oidcmsg import oidc from oidcmsg.oauth2 import ResponseMessage @@ -37,19 +41,133 @@ def response_types_to_grant_types(response_types): return list(_res) -def add_request_uri(request_args=None, service=None, **kwargs): - _context = service.client_get("service_context") - if _context.requests_dir: - _pi = _context.provider_info - if _pi: - _req = _pi.get('require_request_uri_registration', False) - if _req is True: - request_args['request_uris'] = _context.generate_request_uris(_context.requests_dir) +def create_callbacks(issuer: str, + hash_seed: str, + base_url: str, + code: Optional[bool] = False, + implicit: Optional[bool] = False, + form_post: Optional[bool] = False, + request_uris: Optional[bool] = False, + backchannel_logout_uri: Optional[bool] = False, + frontchannel_logout_uri: Optional[bool] = False): + """ + To mitigate some security issues the redirect_uris should be OP/AS + specific. This method creates a set of redirect_uris unique to the + OP/AS. + + :param frontchannel_logout_uri: Whether a front-channel logout uri should be constructed + :param backchannel_logout_uri: Whether a back-channel logout uri should be constructed + :param request_uri: Whether a request_uri should be constructed + :param issuer: Issuer ID + :return: A set of redirect_uris + """ + _hash = hashlib.sha256() + _hash.update(hash_seed) + _hash.update(as_bytes(issuer)) + _hex = _hash.hexdigest() + + res = {'__hex': _hex} + + if code: + res['code'] = f"{base_url}/authz_cb/{_hex}" + + if implicit: + res['implicit'] = f"{base_url}/authz_im_cb/{_hex}" + + if form_post: + res['form_post'] = f"{base_url}/authz_fp_cb/{_hex}" + + if request_uris: + res["request_uris"] = f"{base_url}/req_uri/{_hex}" + + if backchannel_logout_uri or frontchannel_logout_uri: + res["post_logout_redirect_uris"] = f"{base_url}/session_logout/{_hex}" + + if backchannel_logout_uri: + res["backchannel_logout_uri"] = f"{base_url}/bc_logout/{_hex}" + + if frontchannel_logout_uri: + res["frontchannel_logout_uri"] = f"{base_url}/fc_logout/{_hex}" + + logger.debug(f"Created callback URIs: {res}") + return res + + +def _cmp(a, b): + if b is None: # Don't care about the value as long as there is one + return True + elif isinstance(a, str) and a == b: + return True + elif isinstance(a, list) and b in a: + return True + + return a == b - return request_args, {} +def _in_config_or_client_preferences(config, attr, val): + _val = config.get("client_preferences", {}).get(attr) + if _cmp(_val, val): + return True + _val = config.get(attr) + return _cmp(_val, val) -def add_post_logout_redirect_uris(request_args=None, service=None, **kwargs): + +def add_callbacks(context, ignore: Optional[List[str]] = None): + if ignore is None: + ignore = [] + _iss = context.get('issuer') + + _uris = {} + + _pi = context.get('provider_info') + _cp = context.config.get("client_preferences") + + if "redirect_uris" not in ignore: + # code and/or implicit + if _in_config_or_client_preferences(context.config, "response_types", "code"): + _uris['code'] = True + for rt in ["id_token", "id_token token", "code id_token token", "code idtoken", + "code token"]: + if _in_config_or_client_preferences(context.config, "response_types", rt): + _uris["implicit"] = True + break + + if "form_post" not in ignore: + if _in_config_or_client_preferences(context.config, "form_post_usable", True): + _uris["form_post"] = True + + if "request_uris" not in ignore: + if 'require_request_uri_registration' in _pi and _in_config_or_client_preferences( + context.config, "request_uri_usable", True): + _uris['request_uris'] = True + + if "frontchannel_logout_uri" not in ignore: + if 'frontchannel_logout_supported' in _pi and _in_config_or_client_preferences( + context.config, "frontchannel_logout_usable", True): + _uris["frontchannel_logout_uri"] = True + + if "backchannel_logout_uri" not in ignore: + if 'backchannel_logout_supported' in _pi and _in_config_or_client_preferences( + context.config, "backchannel_logout_usable", True): + _uris["backchannel_logout_uri"] = True + + callbacks = create_callbacks(_iss, + hash_seed=context.get('hash_seed'), + base_url=context.get("base_url"), + **_uris) + context.hash2issuer[callbacks['__hex']] = _iss + + if "redirect_uris" not in ignore: + _redirect_uris = [v for k, v in callbacks.items() if k in ["code", "implicit", "form_post"]] + callbacks["redirect_uris"] = _redirect_uris + context.set('callback', callbacks) + + +CALLBACK_URIS = ["post_logout_redirect_uris", "backchannel_logout_uri", "frontchannel_logout_uri", + "request_uris", 'redirect_uris'] + + +def add_callback_uris(request_args=None, service=None, **kwargs): """ :param request_args: @@ -59,10 +177,17 @@ def add_post_logout_redirect_uris(request_args=None, service=None, **kwargs): :return: """ - if "post_logout_redirect_uris" not in request_args: - _uris = service.client_get("service_context").register_args.get("post_logout_redirect_uris") - if _uris: - request_args["post_logout_redirect_uris"] = _uris + _context = service.client_get("service_context") + _ignore = [k for k in list(request_args.keys()) if k in CALLBACK_URIS] + add_callbacks(_context, ignore=_ignore) + for _key in CALLBACK_URIS: + _req_val = request_args.get(_key) + if not _req_val: + _uri = _context.register_args.get(_key) + if _uri is None: + _uri = _context.callback.get(_key) + if _uri: + request_args[_key] = _uri return request_args, {} @@ -107,8 +232,8 @@ def __init__(self, client_get, client_authn_factory=None, conf=None): client_authn_factory=client_authn_factory, conf=conf) self.pre_construct = [self.add_client_behaviour_preference, - add_redirect_uris, add_request_uri, - add_post_logout_redirect_uris, + #add_redirect_uris, + add_callback_uris, add_jwks_uri_or_jwks] self.post_construct = [self.oidc_post_construct] diff --git a/src/oidcrp/rp_handler.py b/src/oidcrp/rp_handler.py index 851c82d..8ae357f 100644 --- a/src/oidcrp/rp_handler.py +++ b/src/oidcrp/rp_handler.py @@ -1,4 +1,3 @@ -import hashlib import logging import sys import traceback @@ -29,6 +28,8 @@ from .exception import OidcServiceError from .oauth2 import Client from .oauth2.utils import pick_redirect_uri +from .oidc.registration import CALLBACK_URIS +from .oidc.registration import add_callbacks from .util import add_path from .util import dynamic_provider_info_discovery from .util import load_registration_response @@ -237,33 +238,10 @@ def do_provider_info(self, client=None, state='', behaviour_args=None): except KeyError: return _context.get('issuer') - def add_callbacks(self, context): - _iss = context.get('issuer') - # Create the necessary callback URLs - # as a side effect self.hash2issuer is set - _extra_uris = { - "request_uri": False, - "backchannel_logout_uri": False, - "frontchannel_logout_uri": False - } - _pi = context.get('provider_info') - _cp = context.config.get("client_preferences") - if 'require_request_uri_registration' in _pi and "request_uri_usable" in _cp: - _extra_uris['request_uri'] = True - if 'frontchannel_logout_supported' in _pi and "frontchannel_logout_usable" in _cp: - _extra_uris["frontchannel_logout_uri"] = True - if 'backchannel_logout_supported' in _pi and "backchannel_logout_usable" in _cp: - _extra_uris["backchannel_logout_uri"] = True - - callbacks = self.create_callbacks(_iss, **_extra_uris) - - context.set('redirect_uris', [ - v for k, v in callbacks.items() if not k.startswith('__')]) - context.set('callback', callbacks) - def do_client_registration(self, client=None, iss_id: Optional[str] = '', state: Optional[str] = '', + request_args: Optional[dict] = None, behaviour_args: Optional[dict] = None): """ Prepare for and do client registration if configured to do so @@ -289,14 +267,19 @@ def do_client_registration(self, client=None, _context.post_logout_redirect_uris = [self.base_url] if not _context.client_id: # means I have to do dynamic client registration - if not _context.get('redirect_uris'): - self.add_callbacks(_context) + if request_args is None: + request_args = {} if behaviour_args: _params = RegistrationRequest().parameters() - request_args = {k: v for k, v in behaviour_args.items() if k in _params} - else: - request_args = {} + request_args.update({k: v for k, v in behaviour_args.items() if k in _params}) + + # _ignore = [k for k in list(request_args.keys()) if k in CALLBACK_URIS] + # if _context.get('redirect_uris'): + # if 'redirect_uris' not in _ignore: + # _ignore.append('redirect_uris') + # + # add_callbacks(_context, _ignore) load_registration_response(client, request_args=request_args) @@ -358,43 +341,43 @@ def client_setup(self, iss_id='', user='', behaviour_args=None): self.issuer2rp[issuer] = client return client - def create_callbacks(self, issuer, request_uri=False, backchannel_logout_uri=False, - frontchannel_logout_uri=False): - """ - To mitigate some security issues the redirect_uris should be OP/AS - specific. This method creates a set of redirect_uris unique to the - OP/AS. - - :param frontchannel_logout_uri: Whether a front-channel logout uri should be constructed - :param backchannel_logout_uri: Whether a back-channel logout uri should be constructed - :param request_uri: Whether a request_uri should be constructed - :param issuer: Issuer ID - :return: A set of redirect_uris - """ - _hash = hashlib.sha256() - _hash.update(self.hash_seed) - _hash.update(as_bytes(issuer)) - _hex = _hash.hexdigest() - self.hash2issuer[_hex] = issuer - res = { - 'code': "{}/authz_cb/{}".format(self.base_url, _hex), - 'implicit': "{}/authz_im_cb/{}".format(self.base_url, _hex), - 'form_post': "{}/authz_fp_cb/{}".format(self.base_url, _hex), - '__hex': _hex - } - if request_uri: - res["request_uri"] = f"{self.base_url}/req_uri/{_hex}" - - if backchannel_logout_uri or frontchannel_logout_uri: - res["post_logout_redirect_uris"] = f"{self.base_url}/session_logout/{_hex}" - - if backchannel_logout_uri: - res["backchannel_logout_uri"] = f"{self.base_url}/bc_logout/{_hex}" - if frontchannel_logout_uri: - res["frontchannel_logout_uri"] = f"{self.base_url}/fc_logout/{_hex}" - - logger.debug(f"Created callback URIs: {res}") - return res + # def create_callbacks(self, issuer, request_uri=False, backchannel_logout_uri=False, + # frontchannel_logout_uri=False): + # """ + # To mitigate some security issues the redirect_uris should be OP/AS + # specific. This method creates a set of redirect_uris unique to the + # OP/AS. + # + # :param frontchannel_logout_uri: Whether a front-channel logout uri should be constructed + # :param backchannel_logout_uri: Whether a back-channel logout uri should be constructed + # :param request_uri: Whether a request_uri should be constructed + # :param issuer: Issuer ID + # :return: A set of redirect_uris + # """ + # _hash = hashlib.sha256() + # _hash.update(self.hash_seed) + # _hash.update(as_bytes(issuer)) + # _hex = _hash.hexdigest() + # self.hash2issuer[_hex] = issuer + # res = { + # 'code': "{}/authz_cb/{}".format(self.base_url, _hex), + # 'implicit': "{}/authz_im_cb/{}".format(self.base_url, _hex), + # 'form_post': "{}/authz_fp_cb/{}".format(self.base_url, _hex), + # '__hex': _hex + # } + # if request_uri: + # res["request_uri"] = f"{self.base_url}/req_uri/{_hex}" + # + # if backchannel_logout_uri or frontchannel_logout_uri: + # res["post_logout_redirect_uris"] = f"{self.base_url}/session_logout/{_hex}" + # + # if backchannel_logout_uri: + # res["backchannel_logout_uri"] = f"{self.base_url}/bc_logout/{_hex}" + # if frontchannel_logout_uri: + # res["frontchannel_logout_uri"] = f"{self.base_url}/fc_logout/{_hex}" + # + # logger.debug(f"Created callback URIs: {res}") + # return res def _get_response_type(self, context, req_args: Optional[dict] = None): if req_args: diff --git a/src/oidcrp/service_context.py b/src/oidcrp/service_context.py index 51cb75e..62b07c0 100644 --- a/src/oidcrp/service_context.py +++ b/src/oidcrp/service_context.py @@ -11,6 +11,7 @@ from cryptojwt.utils import as_bytes from oidcmsg.context import OidcContext from oidcmsg.oidc import RegistrationRequest +from oidcmsg.util import rndstr from oidcrp.state_interface import StateInterface @@ -97,6 +98,8 @@ class ServiceContext(OidcContext): "client_secret_expires_at": 0, 'clock_skew': None, "config": None, + "hash_seed": b'', + "hash2issuer": None, "httpc_params": None, 'issuer': None, "kid": None, @@ -126,6 +129,7 @@ def __init__(self, base_url="", keyjar=None, config=None, state=None, **kwargs): self.client_preferences = {} self.args = {} self.add_on = {} + self.hash2issuer = {} self.httpc_params = {} self.issuer = "" self.client_id = "" @@ -156,6 +160,12 @@ def __init__(self, base_url="", keyjar=None, config=None, state=None, **kwargs): except KeyError: self.clock_skew = 15 + _seed = config.get("hash_seed") + if _seed: + self.hash_seed = as_bytes(_seed) + else: + self.hash_seed = as_bytes(rndstr(32)) + for key, val in kwargs.items(): setattr(self, key, val) diff --git a/src/oidcrp/util.py b/src/oidcrp/util.py index c6e6759..51591d2 100755 --- a/src/oidcrp/util.py +++ b/src/oidcrp/util.py @@ -538,7 +538,7 @@ def load_registration_response(client, request_args=None): """ If the client has been statically registered that information must be provided during the configuration. If expected to be - done dynamically. This method will do dynamic client registration. + done dynamically this method will do dynamic client registration. :param client: A :py:class:`oidcrp.oidc.Client` instance """ diff --git a/tests/request123456.jwt b/tests/request123456.jwt index 84b7793..c0d4c1c 100644 --- a/tests/request123456.jwt +++ b/tests/request123456.jwt @@ -1 +1 @@ -eyJhbGciOiJSUzI1NiIsImtpZCI6IlNVc3dOaTFNUkZsRFQwWTJZalUxWjFSZlFsbzJTM2RFYTNGVFRrVjNMVGhGY25oRFRIRjVlbGsyVlEifQ.eyJyZXNwb25zZV90eXBlIjogImNvZGUiLCAic3RhdGUiOiAic3RhdGUiLCAicmVkaXJlY3RfdXJpIjogImh0dHBzOi8vZXhhbXBsZS5jb20vY2xpL2F1dGh6X2NiIiwgInNjb3BlIjogIm9wZW5pZCIsICJub25jZSI6ICI4TjRoNW1DMXlUZ2FSZlM4YjhFQUNKREFSZTNFWjRDciIsICJjbGllbnRfaWQiOiAiY2xpZW50X2lkIiwgImlzcyI6ICJjbGllbnRfaWQiLCAiaWF0IjogMTYzMjU1NDEyNywgImF1ZCI6IFsiaHR0cHM6Ly9leGFtcGxlLmNvbSJdfQ.i41o35EdqXxjd_po9tL3PssPLhHime9coujYsnDcfFG3Fhc0AllQSO_cMPe_mKfqCfkpYg--ibohGS5OEIK_h2i9j7IILpCeXf9NWqesWastyUkuAEjrYOhmTx2UfWw5SOVRs3wi2FIsOiaf64gEe1T6dgmU9T0NzGWAa5pk9GAftv3RoOsz16riLgMSl8xBsP1ReSQINsbO6pmnlAY2xqEmJNprs0Vsb63Rt0NiiNe5e36RNkDzV-uhRZY6wfNKgcn5ygcwPvcJJErAbP5k-kkgQRwiBsz5E-rdH69jXpc9WJJddO9hA9DkcrVVTSi7wJa0oqLR1XxBTspBQNbt4w \ No newline at end of file +eyJhbGciOiJSUzI1NiIsImtpZCI6IlNVc3dOaTFNUkZsRFQwWTJZalUxWjFSZlFsbzJTM2RFYTNGVFRrVjNMVGhGY25oRFRIRjVlbGsyVlEifQ.eyJyZXNwb25zZV90eXBlIjogImNvZGUiLCAic3RhdGUiOiAic3RhdGUiLCAicmVkaXJlY3RfdXJpIjogImh0dHBzOi8vZXhhbXBsZS5jb20vY2xpL2F1dGh6X2NiIiwgInNjb3BlIjogIm9wZW5pZCIsICJub25jZSI6ICI4Tkc4VWt1cm9qWEE4azJ4SEJhYXQyRnpCcFVpbDRXNyIsICJjbGllbnRfaWQiOiAiY2xpZW50X2lkIiwgImlzcyI6ICJjbGllbnRfaWQiLCAiaWF0IjogMTYzMzM1Mjk1NywgImF1ZCI6IFsiaHR0cHM6Ly9leGFtcGxlLmNvbSJdfQ.KEY0WdzULSWr7M4guCOArwRWPpY6S8_ldckWYDvihTIy-V59zqQxucNrxEtSh8nkCr_dzZB4ZyHX836g77UOCtFh8TYMARpADWGV-m83OBR0_nwiohMlZJL1H4sH-ovID7CUHebr0yXu0NzEI_Fx_hr1PUeQru2UcoQBZg9g8uGpfW9GXy9H_rLg_l2T3xtpPTQfE057q9KREmQmc0XDXVi1XXUyUSGNeXJUlVLu14sWO5y92b5rWjujk9tUTS2wAiVOtH3qNCP8BorSkiG6pvCHDejYZYRG037pKiUJSN3z8McXgTSSIOxW4zfpP9D5FSkYn-re9kKh5jMWpv9w_Q \ No newline at end of file diff --git a/tests/test_13_oidc_service.py b/tests/test_13_oidc_service.py index dcecbec..2e995ee 100644 --- a/tests/test_13_oidc_service.py +++ b/tests/test_13_oidc_service.py @@ -615,14 +615,59 @@ def test_config_with_post_logout(self): assert len(_req) == 5 assert 'post_logout_redirect_uris' in _req - def test_config_with_required_request_uri(self): - _pi = self.service.client_get("service_context").provider_info - _pi['require_request_uri_registration'] = True - self.service.client_get("service_context").provider_info = _pi - _req = self.service.construct() - assert isinstance(_req, RegistrationRequest) - assert len(_req) == 5 - assert 'request_uris' in _req + +def test_config_with_required_request_uri(): + client_config = { + 'client_id': 'client_id', 'client_secret': 'a longesh password', + 'redirect_uris': ['https://example.com/cli/authz_cb'], + 'issuer': ISS, + 'requests_dir': 'requests', + 'base_url': 'https://example.com/cli/', + 'client_preferences': { + "request_uri_usable": True + } + } + entity = Entity(keyjar=CLI_KEY, config=client_config, services=DEFAULT_OIDC_SERVICES) + entity.client_get("service_context").issuer = 'https://example.com' + service = entity.client_get("service", 'registration') + _context = service.client_get("service_context") + + _pi = _context.provider_info + _pi['require_request_uri_registration'] = True + _context.config["client_preferences"]["request_uri_usable"] = True + _req = service.construct() + assert isinstance(_req, RegistrationRequest) + assert len(_req) == 5 + assert 'request_uris' in _req + + +def test_config_logout_uri(): + client_config = { + 'client_id': 'client_id', 'client_secret': 'a longesh password', + 'redirect_uris': ['https://example.com/cli/authz_cb'], + 'issuer': ISS, + 'requests_dir': 'requests', + 'base_url': 'https://example.com/cli/', + 'client_preferences': { + "request_uri_usable": True + } + } + entity = Entity(keyjar=CLI_KEY, config=client_config, services=DEFAULT_OIDC_SERVICES) + entity.client_get("service_context").issuer = 'https://example.com' + service = entity.client_get("service", 'registration') + _context = service.client_get("service_context") + + _pi = _context.provider_info + _pi['require_request_uri_registration'] = True + _pi['frontchannel_logout_supported'] = True + _context.config["client_preferences"]["request_uri_usable"] = True + _context.config["client_preferences"]["frontchannel_logout_usable"] = True + _req = service.construct() + assert isinstance(_req, RegistrationRequest) + assert len(_req) == 7 + assert 'request_uris' in _req + assert 'frontchannel_logout_uri' in _req + assert 'post_logout_redirect_uris' in _req class TestUserInfo(object): diff --git a/tests/test_20_rp_handler_oidc.py b/tests/test_20_rp_handler_oidc.py index 55a6d73..34a6090 100644 --- a/tests/test_20_rp_handler_oidc.py +++ b/tests/test_20_rp_handler_oidc.py @@ -17,6 +17,7 @@ import responses from oidcrp.entity import Entity +from oidcrp.oidc.registration import add_callbacks from oidcrp.rp_handler import RPHandler BASE_URL = 'https://example.com/rp' @@ -337,18 +338,22 @@ def test_do_client_setup(self): assert self.rph.hash2issuer['github'] == _context.get('issuer') def test_create_callbacks(self): - cb = self.rph.create_callbacks('https://op.example.com/') + client = self.rph.init_client('https://op.example.com/') + _srv = client.client_get("service", "registration") + _context = _srv.client_get("service_context") + add_callbacks(_context, []) - assert set(cb.keys()) == {'code', 'implicit', 'form_post', '__hex'} + cb = _srv.client_get("service_context").callback + + assert set(cb.keys()) == {'redirect_uris', 'code', 'implicit', '__hex'} _hash = cb['__hex'] - assert cb['code'] == 'https://example.com/rp/authz_cb/{}'.format(_hash) - assert cb['implicit'] == 'https://example.com/rp/authz_im_cb/{}'.format(_hash) - assert cb['form_post'] == 'https://example.com/rp/authz_fp_cb/{}'.format(_hash) + assert cb['code'] == f'https://example.com/rp/authz_cb/{_hash}' + assert cb['implicit'] == f'https://example.com/rp/authz_im_cb/{_hash}' - assert list(self.rph.hash2issuer.keys()) == [_hash] + assert list(_context.hash2issuer.keys()) == [_hash] - assert self.rph.hash2issuer[_hash] == 'https://op.example.com/' + assert _context.hash2issuer[_hash] == 'https://op.example.com/' def test_begin(self): res = self.rph.begin(issuer_id='github') diff --git a/tests/test_21_rph_defaults.py b/tests/test_21_rph_defaults.py index ac07e72..ced2cad 100644 --- a/tests/test_21_rph_defaults.py +++ b/tests/test_21_rph_defaults.py @@ -72,7 +72,6 @@ def test_begin(self): _context = client.client_get("service_context") # Calculating request so I can build a reasonable response - self.rph.add_callbacks(_context) _req = client.client_get("service",'registration').construct_request() with responses.RequestsMock() as rsps: @@ -129,7 +128,6 @@ def test_begin_2(self): _context = client.client_get("service_context") # Calculating request so I can build a reasonable response - self.rph.add_callbacks(_context) # Publishing a JWKS instead of a JWKS_URI _context.jwks_uri = '' _context.jwks = _context.keyjar.export_jwks() diff --git a/tests/test_40_rp_handler_persistent.py b/tests/test_40_rp_handler_persistent.py index 2dd6faf..f226481 100644 --- a/tests/test_40_rp_handler_persistent.py +++ b/tests/test_40_rp_handler_persistent.py @@ -275,23 +275,6 @@ def test_do_client_setup(self): assert rph_1.hash2issuer['github'] == _context.get('issuer') - def test_create_callbacks(self): - rph_1 = RPHandler(BASE_URL, client_configs=CLIENT_CONFIG, - keyjar=CLI_KEY, module_dirs=['oidc']) - - cb = rph_1.create_callbacks('https://op.example.com/') - - assert set(cb.keys()) == {'code', 'implicit', 'form_post', '__hex'} - _hash = cb['__hex'] - - assert cb['code'] == 'https://example.com/rp/authz_cb/{}'.format(_hash) - assert cb['implicit'] == 'https://example.com/rp/authz_im_cb/{}'.format(_hash) - assert cb['form_post'] == 'https://example.com/rp/authz_fp_cb/{}'.format(_hash) - - assert list(rph_1.hash2issuer.keys()) == [_hash] - - assert rph_1.hash2issuer[_hash] == 'https://op.example.com/' - def test_begin(self): rph_1 = RPHandler(BASE_URL, client_configs=CLIENT_CONFIG, keyjar=CLI_KEY, module_dirs=['oidc']) From b9c8570dba0d388cdcc2c43aa3c07c790fc46c48 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Tue, 5 Oct 2021 13:39:34 +0200 Subject: [PATCH 24/40] log --- src/oidcrp/oidc/registration.py | 2 +- src/oidcrp/rp_handler.py | 73 +++++++++++++++------------------ 2 files changed, 33 insertions(+), 42 deletions(-) diff --git a/src/oidcrp/oidc/registration.py b/src/oidcrp/oidc/registration.py index 9b1c367..4cd87d1 100644 --- a/src/oidcrp/oidc/registration.py +++ b/src/oidcrp/oidc/registration.py @@ -184,7 +184,7 @@ def add_callback_uris(request_args=None, service=None, **kwargs): _req_val = request_args.get(_key) if not _req_val: _uri = _context.register_args.get(_key) - if _uri is None: + if not _uri: _uri = _context.callback.get(_key) if _uri: request_args[_key] = _uri diff --git a/src/oidcrp/rp_handler.py b/src/oidcrp/rp_handler.py index 8ae357f..adcc95f 100644 --- a/src/oidcrp/rp_handler.py +++ b/src/oidcrp/rp_handler.py @@ -28,8 +28,6 @@ from .exception import OidcServiceError from .oauth2 import Client from .oauth2.utils import pick_redirect_uri -from .oidc.registration import CALLBACK_URIS -from .oidc.registration import add_callbacks from .util import add_path from .util import dynamic_provider_info_discovery from .util import load_registration_response @@ -152,6 +150,9 @@ def init_client(self, issuer): :param issuer: An issuer ID :return: A Client instance """ + + logger.debug(20 * "*" + " init_client " + 20 * "*") + try: _cnf = self.pick_config(issuer) except KeyError: @@ -193,6 +194,7 @@ def do_provider_info(self, client=None, state='', behaviour_args=None): retrieved :return: issuer ID """ + logger.debug(20 * "*" + " do_provider_info " + 20 * "*") if not client: if state: @@ -252,6 +254,8 @@ def do_client_registration(self, client=None, retrieved """ + logger.debug(20 * "*" + " do_client_registration " + 20 * "*") + if not client: if state: client = self.get_client_from_session_key(state) @@ -291,6 +295,9 @@ def do_webfinger(self, user): request. :return: A Client instance """ + + logger.debug(20 * "*" + " do_webfinger " + 20 * "*") + temporary_client = self.init_client('') temporary_client.do_request('webfinger', resource=user) return temporary_client @@ -310,6 +317,8 @@ def client_setup(self, iss_id='', user='', behaviour_args=None): :return: A :py:class:`oidcrp.oidc.Client` instance """ + logger.debug(20 * "*" + " client_setup " + 20 * "*") + logger.info('client_setup: iss_id={}, user={}'.format(iss_id, user)) if not iss_id: @@ -341,44 +350,6 @@ def client_setup(self, iss_id='', user='', behaviour_args=None): self.issuer2rp[issuer] = client return client - # def create_callbacks(self, issuer, request_uri=False, backchannel_logout_uri=False, - # frontchannel_logout_uri=False): - # """ - # To mitigate some security issues the redirect_uris should be OP/AS - # specific. This method creates a set of redirect_uris unique to the - # OP/AS. - # - # :param frontchannel_logout_uri: Whether a front-channel logout uri should be constructed - # :param backchannel_logout_uri: Whether a back-channel logout uri should be constructed - # :param request_uri: Whether a request_uri should be constructed - # :param issuer: Issuer ID - # :return: A set of redirect_uris - # """ - # _hash = hashlib.sha256() - # _hash.update(self.hash_seed) - # _hash.update(as_bytes(issuer)) - # _hex = _hash.hexdigest() - # self.hash2issuer[_hex] = issuer - # res = { - # 'code': "{}/authz_cb/{}".format(self.base_url, _hex), - # 'implicit': "{}/authz_im_cb/{}".format(self.base_url, _hex), - # 'form_post': "{}/authz_fp_cb/{}".format(self.base_url, _hex), - # '__hex': _hex - # } - # if request_uri: - # res["request_uri"] = f"{self.base_url}/req_uri/{_hex}" - # - # if backchannel_logout_uri or frontchannel_logout_uri: - # res["post_logout_redirect_uris"] = f"{self.base_url}/session_logout/{_hex}" - # - # if backchannel_logout_uri: - # res["backchannel_logout_uri"] = f"{self.base_url}/bc_logout/{_hex}" - # if frontchannel_logout_uri: - # res["frontchannel_logout_uri"] = f"{self.base_url}/fc_logout/{_hex}" - # - # logger.debug(f"Created callback URIs: {res}") - # return res - def _get_response_type(self, context, req_args: Optional[dict] = None): if req_args: return req_args.get("response_type", context.get('behaviour')['response_types'][0]) @@ -396,6 +367,9 @@ def init_authorization(self, client=None, state='', req_args=None, behaviour_arg URL and **state** the key to the session information in the state data store. """ + + logger.debug(20 * "*" + " init_authorization " + 20 * "*") + if not client: if state: client = self.get_client_from_session_key(state) @@ -517,7 +491,7 @@ def get_access_token(self, state, client: Optional[Client] = None): :return: A :py:class:`oidcmsg.oidc.AccessTokenResponse` or :py:class:`oidcmsg.oauth2.AuthorizationResponse` """ - logger.debug('get_accesstoken') + logger.debug(20 * "*" + " get_access_token " + 20 * "*") if client is None: client = self.get_client_from_session_key(state) @@ -563,6 +537,9 @@ def refresh_access_token(self, state, client=None, scope=''): :param scope: What the returned token should be valid for. :return: A :py:class:`oidcmsg.oidc.AccessTokenResponse` instance """ + + logger.debug(20 * "*" + " refresh_access_token " + 20 * "*") + if scope: req_args = {'scope': scope} else: @@ -599,6 +576,9 @@ def get_user_info(self, state, client=None, access_token='', :param kwargs: Extra keyword arguments :return: A :py:class:`oidcmsg.oidc.OpenIDSchema` instance """ + + logger.debug(20 * "*" + " get_user_info " + 20 * "*") + if client is None: client = self.get_client_from_session_key(state) @@ -642,6 +622,9 @@ def finalize_auth(self, client, issuer: str, response: dict, :return: An :py:class:`oidcmsg.oidc.AuthorizationResponse` or :py:class:`oidcmsg.oauth2.AuthorizationResponse` instance. """ + + logger.debug(20 * "*" + " finalize_auth " + 20 * "*") + _srv = client.get_service('authorization') try: authorization_response = _srv.parse_response(response, @@ -692,6 +675,8 @@ def get_access_and_id_token(self, authorization_response=None, was returned otherwise None. """ + logger.debug(20 * "*" + " get_access_and_id_token " + 20 * "*") + if client is None: client = self.get_client_from_session_key(state) @@ -910,6 +895,9 @@ def logout(self, state: str, should be used :return: A US """ + + logger.debug(20 * "*" + " logout " + 20 * "*") + if client is None: client = self.get_client_from_session_key(state) @@ -933,6 +921,9 @@ def logout(self, state: str, def close(self, state: str, issuer: Optional[str] = '', post_logout_redirect_uri: Optional[str] = '') -> dict: + + logger.debug(20 * "*" + " close " + 20 * "*") + if issuer: client = self.issuer2rp[issuer] else: From be2871c9fbc3f476fd35c613c69afe91bbd36251 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Tue, 5 Oct 2021 13:55:07 +0200 Subject: [PATCH 25/40] log --- src/oidcrp/rp_handler.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/oidcrp/rp_handler.py b/src/oidcrp/rp_handler.py index adcc95f..c0aca99 100644 --- a/src/oidcrp/rp_handler.py +++ b/src/oidcrp/rp_handler.py @@ -916,6 +916,7 @@ def logout(self, state: str, resp = srv.get_request_parameters(state=state, request_args=request_args) + logger.debug(f"EndSession Request: {resp}") return resp def close(self, state: str, From eb7fd14130d6e8b575b5f50b242b0cdd92e779aa Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Tue, 5 Oct 2021 17:18:32 +0200 Subject: [PATCH 26/40] post_logout_redirect_uri singleton. --- example/flask_rp/views.py | 2 +- src/oidcrp/oidc/registration.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/example/flask_rp/views.py b/example/flask_rp/views.py index caa7141..9e2a127 100644 --- a/example/flask_rp/views.py +++ b/example/flask_rp/views.py @@ -160,7 +160,7 @@ def get_op_identifier_by_cb_uri(url: str): for k, v in current_app.rph.issuer2rp.items(): _cntx = v.get_service_context() for endpoint in ("redirect_uris", - "post_logout_redirect_uris", + "post_logout_redirect_uri", "frontchannel_logout_uri", "backchannel_logout_uri"): if uri in _cntx.get(endpoint, []): diff --git a/src/oidcrp/oidc/registration.py b/src/oidcrp/oidc/registration.py index 4cd87d1..219fdc7 100644 --- a/src/oidcrp/oidc/registration.py +++ b/src/oidcrp/oidc/registration.py @@ -81,7 +81,7 @@ def create_callbacks(issuer: str, res["request_uris"] = f"{base_url}/req_uri/{_hex}" if backchannel_logout_uri or frontchannel_logout_uri: - res["post_logout_redirect_uris"] = f"{base_url}/session_logout/{_hex}" + res["post_logout_redirect_uri"] = f"{base_url}/session_logout/{_hex}" if backchannel_logout_uri: res["backchannel_logout_uri"] = f"{base_url}/bc_logout/{_hex}" @@ -163,7 +163,7 @@ def add_callbacks(context, ignore: Optional[List[str]] = None): context.set('callback', callbacks) -CALLBACK_URIS = ["post_logout_redirect_uris", "backchannel_logout_uri", "frontchannel_logout_uri", +CALLBACK_URIS = ["post_logout_redirect_uri", "backchannel_logout_uri", "frontchannel_logout_uri", "request_uris", 'redirect_uris'] From 731b7d5f5d346f6b399ec0582c5c5bdc9d7e5b12 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Wed, 6 Oct 2021 10:06:22 +0200 Subject: [PATCH 27/40] Callback uris are stored in context.callback . --- src/oidcrp/configure.py | 2 +- src/oidcrp/oauth2/__init__.py | 1 + src/oidcrp/oidc/__init__.py | 10 ++++++++++ src/oidcrp/oidc/end_session.py | 15 ++++++++------- src/oidcrp/oidc/registration.py | 5 ++--- src/oidcrp/rp_handler.py | 5 +++-- src/oidcrp/service_context.py | 4 ++-- tests/pub_client.jwks | 2 +- tests/test_13_oidc_service.py | 5 ++--- tests/test_20_rp_handler_oidc.py | 2 +- tests/test_40_rp_handler_persistent.py | 2 +- 11 files changed, 32 insertions(+), 21 deletions(-) diff --git a/src/oidcrp/configure.py b/src/oidcrp/configure.py index d38cc76..84ac367 100755 --- a/src/oidcrp/configure.py +++ b/src/oidcrp/configure.py @@ -14,7 +14,7 @@ try: from secrets import token_urlsafe as rnd_token except ImportError: - from oidcendpoint import rndstr as rnd_token + from cryptojwt import rndstr as rnd_token DEFAULT_FILE_ATTRIBUTE_NAMES = ['server_key', 'server_cert', 'filename', 'template_dir', 'private_path', 'public_path', 'db_file'] diff --git a/src/oidcrp/oauth2/__init__.py b/src/oidcrp/oauth2/__init__.py index b195c8d..8d1b483 100755 --- a/src/oidcrp/oauth2/__init__.py +++ b/src/oidcrp/oauth2/__init__.py @@ -3,6 +3,7 @@ from oidcmsg.exception import FormatError +from oidcrp.configure import URIS from oidcrp.entity import Entity from oidcrp.exception import OidcServiceError from oidcrp.exception import ParseError diff --git a/src/oidcrp/oidc/__init__.py b/src/oidcrp/oidc/__init__.py index cb000e4..df9db5e 100755 --- a/src/oidcrp/oidc/__init__.py +++ b/src/oidcrp/oidc/__init__.py @@ -2,6 +2,7 @@ import logging from oidcrp.client_auth import BearerHeader +from oidcrp.oidc.registration import CALLBACK_URIS try: from json import JSONDecodeError @@ -112,6 +113,15 @@ def __init__(self, client_authn_factory=None, keyjar=keyjar, verify_ssl=verify_ssl, config=config, httplib=httplib, services=_srvs, httpc_params=httpc_params) + _context = self.get_service_context() + if _context.callback is None: + _context.callback = {} + + for _cb in CALLBACK_URIS: + _uri = config.get(_cb) + if _uri: + _context.callback[_cb] = _uri + def fetch_distributed_claims(self, userinfo, callback=None): """ diff --git a/src/oidcrp/oidc/end_session.py b/src/oidcrp/oidc/end_session.py index a58c91d..91030f4 100644 --- a/src/oidcrp/oidc/end_session.py +++ b/src/oidcrp/oidc/end_session.py @@ -54,13 +54,14 @@ def get_id_token_hint(self, request_args=None, **kwargs): def add_post_logout_redirect_uri(self, request_args=None, **kwargs): if 'post_logout_redirect_uri' not in request_args: - try: - request_args[ - 'post_logout_redirect_uri' - ] = self.client_get("service_context").register_args[ - 'post_logout_redirect_uris'][0] - except KeyError: - pass + _context = self.client_get("service_context") + _uri = _context.register_args.get('post_logout_redirect_uri') + if _uri: + request_args['post_logout_redirect_uri'] = _uri + else: + _uris = _context.callback.get("post_logout_redirect_uris", []) + if _uris: + request_args['post_logout_redirect_uri'] = _uris[0] return request_args, {} diff --git a/src/oidcrp/oidc/registration.py b/src/oidcrp/oidc/registration.py index 219fdc7..06a3ea9 100644 --- a/src/oidcrp/oidc/registration.py +++ b/src/oidcrp/oidc/registration.py @@ -7,7 +7,6 @@ from oidcmsg import oidc from oidcmsg.oauth2 import ResponseMessage -from oidcrp.oidc.provider_info_discovery import add_redirect_uris from oidcrp.service import Service __author__ = 'Roland Hedberg' @@ -81,7 +80,7 @@ def create_callbacks(issuer: str, res["request_uris"] = f"{base_url}/req_uri/{_hex}" if backchannel_logout_uri or frontchannel_logout_uri: - res["post_logout_redirect_uri"] = f"{base_url}/session_logout/{_hex}" + res["post_logout_redirect_uris"] = [f"{base_url}/session_logout/{_hex}"] if backchannel_logout_uri: res["backchannel_logout_uri"] = f"{base_url}/bc_logout/{_hex}" @@ -163,7 +162,7 @@ def add_callbacks(context, ignore: Optional[List[str]] = None): context.set('callback', callbacks) -CALLBACK_URIS = ["post_logout_redirect_uri", "backchannel_logout_uri", "frontchannel_logout_uri", +CALLBACK_URIS = ["post_logout_redirect_uris", "backchannel_logout_uri", "frontchannel_logout_uri", "request_uris", 'redirect_uris'] diff --git a/src/oidcrp/rp_handler.py b/src/oidcrp/rp_handler.py index c0aca99..7e81d0f 100644 --- a/src/oidcrp/rp_handler.py +++ b/src/oidcrp/rp_handler.py @@ -248,6 +248,7 @@ def do_client_registration(self, client=None, """ Prepare for and do client registration if configured to do so + :param iss_id: Issuer ID :param behaviour_args: To fine tune behaviour :param client: A Client instance :param state: A key by which the state of the session can be @@ -267,8 +268,8 @@ def do_client_registration(self, client=None, self.hash2issuer[iss_id] = _iss # This should only be interesting if the client supports Single Log Out - if _context.post_logout_redirect_uris is None: - _context.post_logout_redirect_uris = [self.base_url] + # if _context.callback.get("post_logout_redirect_uris") is None: + # _context.callback["post_logout_redirect_uris"] = [self.base_url] if not _context.client_id: # means I have to do dynamic client registration if request_args is None: diff --git a/src/oidcrp/service_context.py b/src/oidcrp/service_context.py index 62b07c0..8771fb9 100644 --- a/src/oidcrp/service_context.py +++ b/src/oidcrp/service_context.py @@ -103,7 +103,7 @@ class ServiceContext(OidcContext): "httpc_params": None, 'issuer': None, "kid": None, - "post_logout_redirect_uris": [], + "post_logout_redirect_uri": '', 'provider_info': None, 'redirect_uris': None, "requests_dir": None, @@ -137,7 +137,7 @@ def __init__(self, base_url="", keyjar=None, config=None, state=None, **kwargs): self.client_secret_expires_at = 0 self.behaviour = {} self.provider_info = {} - self.post_logout_redirect_uris = [] + self.post_logout_redirect_uri = '' self.redirect_uris = [] self.register_args = {} self.registration_response = {} diff --git a/tests/pub_client.jwks b/tests/pub_client.jwks index a57e904..d16a636 100644 --- a/tests/pub_client.jwks +++ b/tests/pub_client.jwks @@ -1 +1 @@ -{"keys": [{"kty": "RSA", "use": "sig", "kid": "SUswNi1MRFlDT0Y2YjU1Z1RfQlo2S3dEa3FTTkV3LThFcnhDTHF5elk2VQ", "n": "0UkUx2ewKyc-XJ1o0ToyGjws_JybAMZj2oYjsPyyvQ_T5dhZ2VmRRRkhsaVJ2xE_GGc7mSG0IjmGFyXp5y0w4mJBcsAEE5-8eBTvQdYIryjW74r3jt6Fi4Hlm1yFMTie3apv8mw79BUj-jT0kh3_m-FiKKUvLsq45DcLtTJ4cx7Ize37dl1sFSpQcoYMk7eiUEM8fiNboiVwvBYNAWVMkUM-LnVUPm3UjvKp0LihYEkZFWOxmuQmj2x25SFUkjus38ERrRqJQBZduxdBHFrWtWg8yOA53BkMU0FFg_r0H3ctl-5GaKw-BWlogU4qXnsq85xy0EoenRk7FPV8g_ulJw", "e": "AQAB"}, {"kty": "EC", "use": "sig", "kid": "NC1pdGRQN002bWM3bk1xX2R0SktscElqbFdtN29ITDV2WVd2b0hOYzREVQ", "crv": "P-256", "x": "kK7Qp1woSerI7rUOAwW_4sU6ZmwV3wwXKX3VU-v2fMI", "y": "iPWd_Pjq6EjxYy08KNFZ3PxhEwgWHgAQTTknlKMKJA0"}]} \ No newline at end of file +{"keys": [{"kty": "RSA", "use": "sig", "kid": "SUswNi1MRFlDT0Y2YjU1Z1RfQlo2S3dEa3FTTkV3LThFcnhDTHF5elk2VQ", "e": "AQAB", "n": "0UkUx2ewKyc-XJ1o0ToyGjws_JybAMZj2oYjsPyyvQ_T5dhZ2VmRRRkhsaVJ2xE_GGc7mSG0IjmGFyXp5y0w4mJBcsAEE5-8eBTvQdYIryjW74r3jt6Fi4Hlm1yFMTie3apv8mw79BUj-jT0kh3_m-FiKKUvLsq45DcLtTJ4cx7Ize37dl1sFSpQcoYMk7eiUEM8fiNboiVwvBYNAWVMkUM-LnVUPm3UjvKp0LihYEkZFWOxmuQmj2x25SFUkjus38ERrRqJQBZduxdBHFrWtWg8yOA53BkMU0FFg_r0H3ctl-5GaKw-BWlogU4qXnsq85xy0EoenRk7FPV8g_ulJw"}, {"kty": "EC", "use": "sig", "kid": "NC1pdGRQN002bWM3bk1xX2R0SktscElqbFdtN29ITDV2WVd2b0hOYzREVQ", "crv": "P-256", "x": "kK7Qp1woSerI7rUOAwW_4sU6ZmwV3wwXKX3VU-v2fMI", "y": "iPWd_Pjq6EjxYy08KNFZ3PxhEwgWHgAQTTknlKMKJA0"}]} \ No newline at end of file diff --git a/tests/test_13_oidc_service.py b/tests/test_13_oidc_service.py index 2e995ee..d5c737a 100644 --- a/tests/test_13_oidc_service.py +++ b/tests/test_13_oidc_service.py @@ -891,9 +891,8 @@ def test_construct(self): 'token_response', 'abcde') _req = self.service.construct(state='abcde') assert isinstance(_req, EndSessionRequest) - assert len(_req) == 3 - assert set(_req.keys()) == {'state', 'id_token_hint', - 'post_logout_redirect_uri'} + assert len(_req) == 2 + assert set(_req.keys()) == {'state', 'id_token_hint'} def test_authz_service_conf(): diff --git a/tests/test_20_rp_handler_oidc.py b/tests/test_20_rp_handler_oidc.py index 34a6090..c2769df 100644 --- a/tests/test_20_rp_handler_oidc.py +++ b/tests/test_20_rp_handler_oidc.py @@ -312,7 +312,7 @@ def test_do_client_registration(self): # only 2 things should have happened assert self.rph.hash2issuer['github'] == issuer - assert client.client_get("service_context").post_logout_redirect_uris == [] + assert client.client_get("service_context").callback.get("post_logout_redirect_uris") is None def test_do_client_setup(self): client = self.rph.client_setup('github') diff --git a/tests/test_40_rp_handler_persistent.py b/tests/test_40_rp_handler_persistent.py index f226481..b9567c7 100644 --- a/tests/test_40_rp_handler_persistent.py +++ b/tests/test_40_rp_handler_persistent.py @@ -247,7 +247,7 @@ def test_do_client_registration(self): # only 2 things should have happened assert rph_1.hash2issuer['github'] == issuer - assert not client.client_get("service_context").post_logout_redirect_uris + assert not client.client_get("service_context").callback.get('post_logout_redirect_uri') def test_do_client_setup(self): rph_1 = RPHandler(BASE_URL, client_configs=CLIENT_CONFIG, From 37d5a24122ff47e5176c35a47811945ea8587349 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Wed, 6 Oct 2021 10:15:59 +0200 Subject: [PATCH 28/40] If no args then I should try to use them. --- src/oidcrp/rp_handler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/oidcrp/rp_handler.py b/src/oidcrp/rp_handler.py index 7e81d0f..8dc5a54 100644 --- a/src/oidcrp/rp_handler.py +++ b/src/oidcrp/rp_handler.py @@ -948,7 +948,7 @@ def backchannel_logout(client, request='', request_args=None): if request: req = BackChannelLogoutRequest().from_urlencoded(as_unicode(request)) else: - req = BackChannelLogoutRequest(**request_args) + req = BackChannelLogoutRequest() _context = client.client_get("service_context") kwargs = { From 6b59b2a101b0f425d22434f2f8a95427380ad5ae Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 7 Oct 2021 08:41:36 +0200 Subject: [PATCH 29/40] Check if nonce values are the same. Deal with missing attribute. --- src/oidcrp/oidc/authorization.py | 54 ++++++++++++++++++++++++++------ src/oidcrp/rp_handler.py | 14 ++++++--- 2 files changed, 53 insertions(+), 15 deletions(-) diff --git a/src/oidcrp/oidc/authorization.py b/src/oidcrp/oidc/authorization.py index 1e858ef..9a7066f 100644 --- a/src/oidcrp/oidc/authorization.py +++ b/src/oidcrp/oidc/authorization.py @@ -1,5 +1,6 @@ import logging +from oidcmsg import oauth2 from oidcmsg import oidc from oidcmsg.oidc import make_openid_request from oidcmsg.oidc import verified_claim_name @@ -47,18 +48,20 @@ def set_state(self, request_args, **kwargs): def update_service_context(self, resp, key='', **kwargs): _context = self.client_get("service_context") - try: - _idt = resp[verified_claim_name('id_token')] - except KeyError: - pass - else: + + _idt = resp.get(verified_claim_name('id_token')) + if _idt: # If there is a verified ID Token then we have to do nonce # verification - try: - if _context.state.get_state_by_nonce(_idt['nonce']) != key: - raise ParameterError('Someone has messed with "nonce"') - except KeyError: - raise ValueError('Missing nonce value') + item = _context.state.get_item(oauth2.AuthorizationRequest, 'auth_request', key) + if item['nonce'] != _idt['nonce']: + raise ValueError('Invalid nonce') + + # try: + # if _context.state.get_state_by_nonce(_idt['nonce']) != key: + # raise ParameterError('Someone has messed with "nonce"') + # except KeyError: + # raise ValueError('Invalid nonce') _context.state.store_sub2state(_idt['sub'], key) @@ -229,6 +232,33 @@ def oidc_post_construct(self, req, **kwargs): return req + # def post_parse_response(self, response, **kwargs): + # """ + # Add scope claim to response, from the request, if not present in the + # response + # + # :param response: The response + # :param kwargs: Extra Keyword arguments + # :return: A possibly augmented response + # """ + # + # authorization.Authorization.parse_response(self, response, **kwargs) + # + # if "id_token" not in response: + # try: + # _key = kwargs['state'] + # except KeyError: + # pass + # else: + # if _key: + # item = self.client_get("service_context").state.get_item(oauth2.AuthorizationRequest, + # 'auth_request', _key) + # try: + # response["scope"] = item["scope"] + # except KeyError: + # pass + # return response + def gather_verify_arguments(self): """ Need to add some information before running verify() @@ -242,6 +272,10 @@ def gather_verify_arguments(self): 'skew': _context.clock_skew } + _nonce = _context.state.get_item(oauth2.AuthorizationRequest, 'auth_request', 'nonce') + if _nonce: + kwargs["nonce"] = _nonce + _client_id = _context.client_id if _client_id: kwargs['client_id'] = _client_id diff --git a/src/oidcrp/rp_handler.py b/src/oidcrp/rp_handler.py index 8dc5a54..c6449d4 100644 --- a/src/oidcrp/rp_handler.py +++ b/src/oidcrp/rp_handler.py @@ -9,6 +9,7 @@ from cryptojwt.utils import as_bytes from oidcmsg import verified_claim_name from oidcmsg.exception import MessageException +from oidcmsg.exception import MissingRequiredAttribute from oidcmsg.exception import NotForMe from oidcmsg.oauth2 import ResponseMessage from oidcmsg.oauth2 import is_error_message @@ -628,16 +629,14 @@ def finalize_auth(self, client, issuer: str, response: dict, _srv = client.get_service('authorization') try: - authorization_response = _srv.parse_response(response, - sformat='dict') + authorization_response = _srv.parse_response(response, sformat='dict') except Exception as err: logger.error('Parsing authorization_response: {}'.format(err)) message = traceback.format_exception(*sys.exc_info()) logger.error(message) raise else: - logger.debug( - 'Authz response: {}'.format(authorization_response.to_dict())) + logger.debug('Authz response: {}'.format(authorization_response.to_dict())) if is_error_message(authorization_response): return authorization_response @@ -947,8 +946,10 @@ def backchannel_logout(client, request='', request_args=None): """ if request: req = BackChannelLogoutRequest().from_urlencoded(as_unicode(request)) + elif request_args: + req = BackChannelLogoutRequest(**request_args) else: - req = BackChannelLogoutRequest() + raise MissingRequiredAttribute('logout_token') _context = client.client_get("service_context") kwargs = { @@ -959,10 +960,13 @@ def backchannel_logout(client, request='', request_args=None): "id_token_signed_response_alg", "RS256") } + logger.debug(f"(backchannel_logout) Verifying request using: {kwargs}") try: req.verify(**kwargs) except (MessageException, ValueError, NotForMe) as err: raise MessageException('Bogus logout request: {}'.format(err)) + else: + logger.debug("Request verified OK") # Find the subject through 'sid' or 'sub' sub = req[verified_claim_name('logout_token')].get('sub') From 9ec3b08fb5f6a03b0643fb56c0b89e33b17555de Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 7 Oct 2021 09:44:38 +0200 Subject: [PATCH 30/40] Refactored nonce checking. Fixed tests. --- src/oidcrp/oidc/authorization.py | 43 +++++++++++++++++--------------- src/oidcrp/service.py | 3 +-- tests/request123456.jwt | 2 +- tests/test_13_oidc_service.py | 17 ++++++------- 4 files changed, 33 insertions(+), 32 deletions(-) diff --git a/src/oidcrp/oidc/authorization.py b/src/oidcrp/oidc/authorization.py index 9a7066f..5c06bd1 100644 --- a/src/oidcrp/oidc/authorization.py +++ b/src/oidcrp/oidc/authorization.py @@ -2,6 +2,7 @@ from oidcmsg import oauth2 from oidcmsg import oidc +from oidcmsg.exception import MissingRequiredAttribute from oidcmsg.oidc import make_openid_request from oidcmsg.oidc import verified_claim_name from oidcmsg.time_util import time_sans_frac @@ -49,26 +50,32 @@ def set_state(self, request_args, **kwargs): def update_service_context(self, resp, key='', **kwargs): _context = self.client_get("service_context") - _idt = resp.get(verified_claim_name('id_token')) - if _idt: - # If there is a verified ID Token then we have to do nonce - # verification - item = _context.state.get_item(oauth2.AuthorizationRequest, 'auth_request', key) - if item['nonce'] != _idt['nonce']: - raise ValueError('Invalid nonce') - - # try: - # if _context.state.get_state_by_nonce(_idt['nonce']) != key: - # raise ParameterError('Someone has messed with "nonce"') - # except KeyError: - # raise ValueError('Invalid nonce') - - _context.state.store_sub2state(_idt['sub'], key) - if 'expires_in' in resp: resp['__expires_at'] = time_sans_frac() + int(resp['expires_in']) _context.state.store_item(resp.to_json(), 'auth_response', key) + def get_request_from_response(self, response): + _context = self.client_get("service_context") + return _context.state.get_item(oauth2.AuthorizationRequest, 'auth_request', + response["state"]) + + def post_parse_response(self, response, **kwargs): + response = authorization.Authorization.post_parse_response(self, response, **kwargs) + + _idt = response.get(verified_claim_name('id_token')) + if _idt: + # If there is a verified ID Token then we have to do nonce + # verification. + _request = self.get_request_from_response(response) + _req_nonce = _request.get('nonce') + if _req_nonce: + _id_token_nonce = _idt.get('nonce') + if not _id_token_nonce: + raise MissingRequiredAttribute('nonce') + elif _req_nonce != _id_token_nonce: + raise ValueError('Invalid nonce') + return response + def oidc_pre_construct(self, request_args=None, post_args=None, **kwargs): _context = self.client_get("service_context") if request_args is None: @@ -272,10 +279,6 @@ def gather_verify_arguments(self): 'skew': _context.clock_skew } - _nonce = _context.state.get_item(oauth2.AuthorizationRequest, 'auth_request', 'nonce') - if _nonce: - kwargs["nonce"] = _nonce - _client_id = _context.client_id if _client_id: kwargs['client_id'] = _client_id diff --git a/src/oidcrp/service.py b/src/oidcrp/service.py index ea3bc63..21b519d 100644 --- a/src/oidcrp/service.py +++ b/src/oidcrp/service.py @@ -557,8 +557,7 @@ def parse_response(self, info, sformat="", state="", **kwargs): vargs = self.gather_verify_arguments() LOGGER.debug("Verify response with %s", vargs) try: - # verify the message. If something is wrong an exception is - # thrown + # verify the message. If something is wrong an exception is thrown resp.verify(**vargs) except Exception as err: LOGGER.error( diff --git a/tests/request123456.jwt b/tests/request123456.jwt index c0d4c1c..e1feb7a 100644 --- a/tests/request123456.jwt +++ b/tests/request123456.jwt @@ -1 +1 @@ -eyJhbGciOiJSUzI1NiIsImtpZCI6IlNVc3dOaTFNUkZsRFQwWTJZalUxWjFSZlFsbzJTM2RFYTNGVFRrVjNMVGhGY25oRFRIRjVlbGsyVlEifQ.eyJyZXNwb25zZV90eXBlIjogImNvZGUiLCAic3RhdGUiOiAic3RhdGUiLCAicmVkaXJlY3RfdXJpIjogImh0dHBzOi8vZXhhbXBsZS5jb20vY2xpL2F1dGh6X2NiIiwgInNjb3BlIjogIm9wZW5pZCIsICJub25jZSI6ICI4Tkc4VWt1cm9qWEE4azJ4SEJhYXQyRnpCcFVpbDRXNyIsICJjbGllbnRfaWQiOiAiY2xpZW50X2lkIiwgImlzcyI6ICJjbGllbnRfaWQiLCAiaWF0IjogMTYzMzM1Mjk1NywgImF1ZCI6IFsiaHR0cHM6Ly9leGFtcGxlLmNvbSJdfQ.KEY0WdzULSWr7M4guCOArwRWPpY6S8_ldckWYDvihTIy-V59zqQxucNrxEtSh8nkCr_dzZB4ZyHX836g77UOCtFh8TYMARpADWGV-m83OBR0_nwiohMlZJL1H4sH-ovID7CUHebr0yXu0NzEI_Fx_hr1PUeQru2UcoQBZg9g8uGpfW9GXy9H_rLg_l2T3xtpPTQfE057q9KREmQmc0XDXVi1XXUyUSGNeXJUlVLu14sWO5y92b5rWjujk9tUTS2wAiVOtH3qNCP8BorSkiG6pvCHDejYZYRG037pKiUJSN3z8McXgTSSIOxW4zfpP9D5FSkYn-re9kKh5jMWpv9w_Q \ No newline at end of file +eyJhbGciOiJSUzI1NiIsImtpZCI6IlNVc3dOaTFNUkZsRFQwWTJZalUxWjFSZlFsbzJTM2RFYTNGVFRrVjNMVGhGY25oRFRIRjVlbGsyVlEifQ.eyJyZXNwb25zZV90eXBlIjogImNvZGUiLCAic3RhdGUiOiAic3RhdGUiLCAicmVkaXJlY3RfdXJpIjogImh0dHBzOi8vZXhhbXBsZS5jb20vY2xpL2F1dGh6X2NiIiwgInNjb3BlIjogIm9wZW5pZCIsICJub25jZSI6ICJCZ25yS0daMHZNQ3lFRlU1U0d5ekZmVlNkVGxHZmhIaSIsICJjbGllbnRfaWQiOiAiY2xpZW50X2lkIiwgImlzcyI6ICJjbGllbnRfaWQiLCAiaWF0IjogMTYzMzU5MjI4OCwgImF1ZCI6IFsiaHR0cHM6Ly9leGFtcGxlLmNvbSJdfQ.xYAc40jcNNioyQ_FbbhrBectUkhxX62rPmf8whkH-7FBkrzAdjYIM7PmyDfJXRmXz0mw54EOriq3aXS-CPZqcSfRAYf7e-Shw2Ve1-138o307l6x7LmvLpK8EnUealcO5fEs-aLEwVre1ZOXNHchWKt-Lj_eL5cVA-FNQj09IzKTlDv_1gh_bSJJKELW0BsK9f3JYf-pM4EoCqoajbt_jw2WakzF0Phg2mc_wolpUPPZigjgQj8AGeAcDVf6y74E9j9csSVpB8YlnFwyUyJ0Yh-zRnIK0EInufdHu7qo1rJS6UpcTb2eS354Zm3cd1YDcfvPfsT__YV8Rb32Uo2_WQ \ No newline at end of file diff --git a/tests/test_13_oidc_service.py b/tests/test_13_oidc_service.py index d5c737a..e3f4450 100644 --- a/tests/test_13_oidc_service.py +++ b/tests/test_13_oidc_service.py @@ -7,6 +7,7 @@ from cryptojwt.jwt import JWT from cryptojwt.key_jar import build_keyjar from cryptojwt.key_jar import init_key_jar +from oidcmsg.exception import MissingRequiredAttribute from oidcmsg.oidc import AccessTokenRequest from oidcmsg.oidc import AccessTokenResponse from oidcmsg.oidc import AuthorizationRequest @@ -202,7 +203,7 @@ def test_update_service_context_with_idtoken_wrong_nonce(self): idt = JWT(ISS_KEY, iss=ISS, lifetime=3600) payload = { 'sub': '123456789', 'aud': ['client_id'], - 'nonce': 'nonce' + 'nonce': 'noice' } # have to calculate c_hash alg = 'RS256' @@ -211,9 +212,8 @@ def test_update_service_context_with_idtoken_wrong_nonce(self): _idt = idt.pack(payload) resp = AuthorizationResponse(state='state', code='code', id_token=_idt) - resp = self.service.parse_response(resp.to_urlencoded()) - with pytest.raises(ParameterError): - self.service.update_service_context(resp, 'state2') + with pytest.raises(ValueError): + self.service.parse_response(resp.to_urlencoded()) def test_update_service_context_with_idtoken_missing_nonce(self): req_args = {'response_type': 'code', 'state': 'state', 'nonce': 'nonce'} @@ -229,9 +229,8 @@ def test_update_service_context_with_idtoken_missing_nonce(self): _idt = idt.pack(payload) resp = AuthorizationResponse(state='state', code='code', id_token=_idt) - resp = self.service.parse_response(resp.to_urlencoded()) - with pytest.raises(ValueError): - self.service.update_service_context(resp, 'state') + with pytest.raises(MissingRequiredAttribute): + self.service.parse_response(resp.to_urlencoded()) @pytest.mark.parametrize("allow_sign_alg_none", [True, False]) def test_allow_unsigned_idtoken(self, allow_sign_alg_none): @@ -240,14 +239,14 @@ def test_allow_unsigned_idtoken(self, allow_sign_alg_none): self.service.get_request_parameters(request_args=req_args) # Build an ID Token idt = JWT(ISS_KEY, iss=ISS, lifetime=3600, sign_alg='none') - payload = {'sub': '123456789', 'aud': ['client_id']} + payload = {'sub': '123456789', 'aud': ['client_id'], 'nonce': req_args['nonce']} _idt = idt.pack(payload) self.service.client_get("service_context").behaviour["verify_args"] = { "allow_sign_alg_none": allow_sign_alg_none } resp = AuthorizationResponse(state='state', code='code', id_token=_idt) if allow_sign_alg_none: - resp = self.service.parse_response(resp.to_urlencoded()) + self.service.parse_response(resp.to_urlencoded()) else: with pytest.raises(UnsupportedAlgorithm): self.service.parse_response(resp.to_urlencoded()) From 90c3a085c4f05d63377c75dccd49b764a111b76d Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 7 Oct 2021 20:20:19 +0200 Subject: [PATCH 31/40] Added 2 new parameters to gather_verify_arguments. Allows for extended functionality. --- src/oidcrp/oidc/access_token.py | 6 +++++- src/oidcrp/oidc/authorization.py | 35 ++++++-------------------------- src/oidcrp/oidc/userinfo.py | 6 +++++- src/oidcrp/service.py | 18 ++++++++++------ tests/pub_client.jwks | 2 +- tests/request123456.jwt | 2 +- 6 files changed, 30 insertions(+), 39 deletions(-) diff --git a/src/oidcrp/oidc/access_token.py b/src/oidcrp/oidc/access_token.py index 03a87f1..7828b69 100644 --- a/src/oidcrp/oidc/access_token.py +++ b/src/oidcrp/oidc/access_token.py @@ -1,7 +1,9 @@ import logging from typing import Optional +from typing import Union from oidcmsg import oidc +from oidcmsg.message import Message from oidcmsg.oidc import verified_claim_name from oidcmsg.time_util import time_sans_frac @@ -26,7 +28,9 @@ def __init__(self, access_token.AccessToken.__init__(self, client_get, client_authn_factory=client_authn_factory, conf=conf) - def gather_verify_arguments(self): + def gather_verify_arguments(self, + response: Optional[Union[dict, Message]] = None, + behaviour_args: Optional[dict] = None): """ Need to add some information before running verify() diff --git a/src/oidcrp/oidc/authorization.py b/src/oidcrp/oidc/authorization.py index 5c06bd1..5845640 100644 --- a/src/oidcrp/oidc/authorization.py +++ b/src/oidcrp/oidc/authorization.py @@ -1,14 +1,16 @@ import logging +from typing import Optional +from typing import Union from oidcmsg import oauth2 from oidcmsg import oidc from oidcmsg.exception import MissingRequiredAttribute +from oidcmsg.message import Message from oidcmsg.oidc import make_openid_request from oidcmsg.oidc import verified_claim_name from oidcmsg.time_util import time_sans_frac from oidcmsg.time_util import utc_time_sans_frac -from oidcrp.exception import ParameterError from oidcrp.oauth2 import authorization from oidcrp.oauth2.utils import pre_construct_pick_redirect_uri from oidcrp.oidc import IDT2REG @@ -239,34 +241,9 @@ def oidc_post_construct(self, req, **kwargs): return req - # def post_parse_response(self, response, **kwargs): - # """ - # Add scope claim to response, from the request, if not present in the - # response - # - # :param response: The response - # :param kwargs: Extra Keyword arguments - # :return: A possibly augmented response - # """ - # - # authorization.Authorization.parse_response(self, response, **kwargs) - # - # if "id_token" not in response: - # try: - # _key = kwargs['state'] - # except KeyError: - # pass - # else: - # if _key: - # item = self.client_get("service_context").state.get_item(oauth2.AuthorizationRequest, - # 'auth_request', _key) - # try: - # response["scope"] = item["scope"] - # except KeyError: - # pass - # return response - - def gather_verify_arguments(self): + def gather_verify_arguments(self, + response: Optional[Union[dict, Message]] = None, + behaviour_args: Optional[dict] = None): """ Need to add some information before running verify() diff --git a/src/oidcrp/oidc/userinfo.py b/src/oidcrp/oidc/userinfo.py index a3eddcb..6c93b80 100644 --- a/src/oidcrp/oidc/userinfo.py +++ b/src/oidcrp/oidc/userinfo.py @@ -1,4 +1,6 @@ import logging +from typing import Optional +from typing import Union from oidcmsg import oidc from oidcmsg.exception import MissingSigningKey @@ -112,7 +114,9 @@ def post_parse_response(self, response, **kwargs): _state_interface.store_item(response, 'user_info', kwargs['state']) return response - def gather_verify_arguments(self): + def gather_verify_arguments(self, + response: Optional[Union[dict, Message]] = None, + behaviour_args: Optional[dict] = None): """ Need to add some information before running verify() diff --git a/src/oidcrp/service.py b/src/oidcrp/service.py index 21b519d..3eaa025 100644 --- a/src/oidcrp/service.py +++ b/src/oidcrp/service.py @@ -90,7 +90,7 @@ def __init__(self, self.construct_extra_headers = [] self.post_parse_process = [] - def gather_request_args(self, **kwargs): + def gather_request_args(self,**kwargs): """ Go through the attributes that the message class can contain and add values if they are missing but exists in the client info or @@ -444,7 +444,9 @@ def post_parse_response(self, response, **kwargs): """ return response - def gather_verify_arguments(self): + def gather_verify_arguments(self, + response: Optional[Union[dict, Message]] = None, + behaviour_args: Optional[dict] = None): """ Need to add some information before running verify() @@ -501,7 +503,11 @@ def _do_response(self, info, sformat, **kwargs): raise return resp - def parse_response(self, info, sformat="", state="", **kwargs): + def parse_response(self, info, + sformat: Optional[str] = "", + state: Optional[str] = "", + behaviour_args: Optional[dict] = None, + **kwargs): """ This the start of a pipeline that will: @@ -513,8 +519,8 @@ def parse_response(self, info, sformat="", state="", **kwargs): 3 runs the do_post_parse_response method iff the response was not an error response. - :param info: The response, can be either in a JSON or an urlencoded - format + :param behaviour_args: + :param info: The response, can be either in a JSON or an urlencoded format :param sformat: Which serialization that was used :param state: The state :param kwargs: Extra key word arguments @@ -554,7 +560,7 @@ def parse_response(self, info, sformat="", state="", **kwargs): if is_error_message(resp): LOGGER.debug('Error response: %s', resp) else: - vargs = self.gather_verify_arguments() + vargs = self.gather_verify_arguments(response=resp, behaviour_args=behaviour_args) LOGGER.debug("Verify response with %s", vargs) try: # verify the message. If something is wrong an exception is thrown diff --git a/tests/pub_client.jwks b/tests/pub_client.jwks index d16a636..a57e904 100644 --- a/tests/pub_client.jwks +++ b/tests/pub_client.jwks @@ -1 +1 @@ -{"keys": [{"kty": "RSA", "use": "sig", "kid": "SUswNi1MRFlDT0Y2YjU1Z1RfQlo2S3dEa3FTTkV3LThFcnhDTHF5elk2VQ", "e": "AQAB", "n": "0UkUx2ewKyc-XJ1o0ToyGjws_JybAMZj2oYjsPyyvQ_T5dhZ2VmRRRkhsaVJ2xE_GGc7mSG0IjmGFyXp5y0w4mJBcsAEE5-8eBTvQdYIryjW74r3jt6Fi4Hlm1yFMTie3apv8mw79BUj-jT0kh3_m-FiKKUvLsq45DcLtTJ4cx7Ize37dl1sFSpQcoYMk7eiUEM8fiNboiVwvBYNAWVMkUM-LnVUPm3UjvKp0LihYEkZFWOxmuQmj2x25SFUkjus38ERrRqJQBZduxdBHFrWtWg8yOA53BkMU0FFg_r0H3ctl-5GaKw-BWlogU4qXnsq85xy0EoenRk7FPV8g_ulJw"}, {"kty": "EC", "use": "sig", "kid": "NC1pdGRQN002bWM3bk1xX2R0SktscElqbFdtN29ITDV2WVd2b0hOYzREVQ", "crv": "P-256", "x": "kK7Qp1woSerI7rUOAwW_4sU6ZmwV3wwXKX3VU-v2fMI", "y": "iPWd_Pjq6EjxYy08KNFZ3PxhEwgWHgAQTTknlKMKJA0"}]} \ No newline at end of file +{"keys": [{"kty": "RSA", "use": "sig", "kid": "SUswNi1MRFlDT0Y2YjU1Z1RfQlo2S3dEa3FTTkV3LThFcnhDTHF5elk2VQ", "n": "0UkUx2ewKyc-XJ1o0ToyGjws_JybAMZj2oYjsPyyvQ_T5dhZ2VmRRRkhsaVJ2xE_GGc7mSG0IjmGFyXp5y0w4mJBcsAEE5-8eBTvQdYIryjW74r3jt6Fi4Hlm1yFMTie3apv8mw79BUj-jT0kh3_m-FiKKUvLsq45DcLtTJ4cx7Ize37dl1sFSpQcoYMk7eiUEM8fiNboiVwvBYNAWVMkUM-LnVUPm3UjvKp0LihYEkZFWOxmuQmj2x25SFUkjus38ERrRqJQBZduxdBHFrWtWg8yOA53BkMU0FFg_r0H3ctl-5GaKw-BWlogU4qXnsq85xy0EoenRk7FPV8g_ulJw", "e": "AQAB"}, {"kty": "EC", "use": "sig", "kid": "NC1pdGRQN002bWM3bk1xX2R0SktscElqbFdtN29ITDV2WVd2b0hOYzREVQ", "crv": "P-256", "x": "kK7Qp1woSerI7rUOAwW_4sU6ZmwV3wwXKX3VU-v2fMI", "y": "iPWd_Pjq6EjxYy08KNFZ3PxhEwgWHgAQTTknlKMKJA0"}]} \ No newline at end of file diff --git a/tests/request123456.jwt b/tests/request123456.jwt index e1feb7a..2e8824e 100644 --- a/tests/request123456.jwt +++ b/tests/request123456.jwt @@ -1 +1 @@ -eyJhbGciOiJSUzI1NiIsImtpZCI6IlNVc3dOaTFNUkZsRFQwWTJZalUxWjFSZlFsbzJTM2RFYTNGVFRrVjNMVGhGY25oRFRIRjVlbGsyVlEifQ.eyJyZXNwb25zZV90eXBlIjogImNvZGUiLCAic3RhdGUiOiAic3RhdGUiLCAicmVkaXJlY3RfdXJpIjogImh0dHBzOi8vZXhhbXBsZS5jb20vY2xpL2F1dGh6X2NiIiwgInNjb3BlIjogIm9wZW5pZCIsICJub25jZSI6ICJCZ25yS0daMHZNQ3lFRlU1U0d5ekZmVlNkVGxHZmhIaSIsICJjbGllbnRfaWQiOiAiY2xpZW50X2lkIiwgImlzcyI6ICJjbGllbnRfaWQiLCAiaWF0IjogMTYzMzU5MjI4OCwgImF1ZCI6IFsiaHR0cHM6Ly9leGFtcGxlLmNvbSJdfQ.xYAc40jcNNioyQ_FbbhrBectUkhxX62rPmf8whkH-7FBkrzAdjYIM7PmyDfJXRmXz0mw54EOriq3aXS-CPZqcSfRAYf7e-Shw2Ve1-138o307l6x7LmvLpK8EnUealcO5fEs-aLEwVre1ZOXNHchWKt-Lj_eL5cVA-FNQj09IzKTlDv_1gh_bSJJKELW0BsK9f3JYf-pM4EoCqoajbt_jw2WakzF0Phg2mc_wolpUPPZigjgQj8AGeAcDVf6y74E9j9csSVpB8YlnFwyUyJ0Yh-zRnIK0EInufdHu7qo1rJS6UpcTb2eS354Zm3cd1YDcfvPfsT__YV8Rb32Uo2_WQ \ No newline at end of file +eyJhbGciOiJSUzI1NiIsImtpZCI6IlNVc3dOaTFNUkZsRFQwWTJZalUxWjFSZlFsbzJTM2RFYTNGVFRrVjNMVGhGY25oRFRIRjVlbGsyVlEifQ.eyJyZXNwb25zZV90eXBlIjogImNvZGUiLCAic3RhdGUiOiAic3RhdGUiLCAicmVkaXJlY3RfdXJpIjogImh0dHBzOi8vZXhhbXBsZS5jb20vY2xpL2F1dGh6X2NiIiwgInNjb3BlIjogIm9wZW5pZCIsICJub25jZSI6ICJvWkpBNTRnZTVaUndNalkwOVVLVnpwYkx5MEdNUEwwaCIsICJjbGllbnRfaWQiOiAiY2xpZW50X2lkIiwgImlzcyI6ICJjbGllbnRfaWQiLCAiaWF0IjogMTYzMzU5NTc4OSwgImF1ZCI6IFsiaHR0cHM6Ly9leGFtcGxlLmNvbSJdfQ.KVMPK6leJ5pEXnJ0jXiXu21U176IU9iwkT4FkQV_33jGYTsgdqCqXw5XHR1ciixdcH2cWf0SzTPOgIzGsI4NJiPNdR9xOusYRyYKZciXHq85nrM7fr7dEPaVntWCU6uadH0MNHWCcq2FyBdz2YYDuiFPUXoxkFbfWZoo_jVMAWLxGQtGEitniI49qo0zbeSFck4hBmEtQTUOrGQvg_CjkSZb5oNb5rt_X5T-ZSK9y3AeKru4HLSQRkWj-oD-Fgd60Sm3XqfLQXrx26lk4a8ORah01BMmMsi5jeIUbOTthhhglZhMwoI9xCZ57I4SF7870-PrinIByW8d2keA1-LipQ \ No newline at end of file From 58f0ec4d1d9e722d14cebbb544d511cf31de4bb1 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Fri, 8 Oct 2021 10:11:29 +0200 Subject: [PATCH 32/40] Typing --- src/oidcrp/client_auth.py | 2 +- src/oidcrp/oauth2/__init__.py | 54 +++++++++++++++++++++++++++++++---- src/oidcrp/rp_handler.py | 38 +++++++++++++----------- src/oidcrp/util.py | 24 +--------------- 4 files changed, 72 insertions(+), 46 deletions(-) diff --git a/src/oidcrp/client_auth.py b/src/oidcrp/client_auth.py index f585ec7..571f428 100755 --- a/src/oidcrp/client_auth.py +++ b/src/oidcrp/client_auth.py @@ -12,8 +12,8 @@ from oidcmsg.oauth2 import SINGLE_OPTIONAL_STRING from oidcmsg.oidc import AuthnToken from oidcmsg.time_util import utc_time_sans_frac +from oidcmsg.util import rndstr -from oidcrp.util import rndstr from oidcrp.util import sanitize from .defaults import DEF_SIGN_ALG from .defaults import JWT_BEARER diff --git a/src/oidcrp/oauth2/__init__.py b/src/oidcrp/oauth2/__init__.py index 8d1b483..6b7612e 100755 --- a/src/oidcrp/oauth2/__init__.py +++ b/src/oidcrp/oauth2/__init__.py @@ -1,15 +1,19 @@ from json import JSONDecodeError import logging +from typing import Optional from oidcmsg.exception import FormatError +from oidcmsg.message import Message +from oidcmsg.oauth2 import is_error_message -from oidcrp.configure import URIS from oidcrp.entity import Entity +from oidcrp.exception import ConfigurationError from oidcrp.exception import OidcServiceError from oidcrp.exception import ParseError from oidcrp.http import HTTPLib from oidcrp.service import REQUEST_INFO from oidcrp.service import SUCCESSFUL +from oidcrp.service import Service from oidcrp.util import do_add_ons from oidcrp.util import get_deserialization_method @@ -73,7 +77,11 @@ def __init__(self, client_authn_factory=None, keyjar=None, verify_ssl=True, conf # just ignore verify_ssl until it goes away self.verify_ssl = self.httpc_params.get("verify", True) - def do_request(self, request_type, response_body_type="", request_args=None, **kwargs): + def do_request(self, + request_type: str, + response_body_type: Optional[str] = "", + request_args: Optional[dict] = None, + behaviour_args: Optional[dict] = None, **kwargs): _srv = self._service[request_type] _info = _srv.get_request_parameters(request_args=request_args, **kwargs) @@ -94,8 +102,13 @@ def set_client_id(self, client_id): self.client_id = client_id self._service_context.set('client_id', client_id) - def get_response(self, service, url, method="GET", body=None, response_body_type="", - headers=None, **kwargs): + def get_response(self, + service: Service, + url: str, + method: Optional[str] = "GET", + body: Optional[dict] = None, + response_body_type: Optional[str] = "", + headers: Optional[dict] = None, **kwargs): """ :param url: @@ -130,8 +143,13 @@ def get_response(self, service, url, method="GET", body=None, response_body_type return self.parse_request_response(service, resp, response_body_type, **kwargs) - def service_request(self, service, url, method="GET", body=None, - response_body_type="", headers=None, **kwargs): + def service_request(self, + service: Service, + url: str, + method: Optional[str] = "GET", + body: Optional[dict] = None, + response_body_type: Optional[str] = "", + headers: Optional[dict] = None, **kwargs) -> Message: """ The method that sends the request and handles the response returned. This assumes that the response arrives in the HTTP response. @@ -250,3 +268,27 @@ def parse_request_response(self, service, reqresp, response_body_type='', reqresp.text)) raise OidcServiceError("HTTP ERROR: %s [%s] on %s" % ( reqresp.text, reqresp.status_code, reqresp.url)) + + +def dynamic_provider_info_discovery(client: Client, behaviour_args: Optional[dict]=None): + """ + This is about performing dynamic Provider Info discovery + + :param behaviour_args: + :param client: A :py:class:`oidcrp.oidc.Client` instance + """ + try: + client.get_service('provider_info') + except KeyError: + raise ConfigurationError( + 'Can not do dynamic provider info discovery') + else: + _context = client.client_get("service_context") + try: + _context.set('issuer', _context.config['srv_discovery_url']) + except KeyError: + pass + + response = client.do_request('provider_info', behaviour_args=behaviour_args) + if is_error_message(response): + raise OidcServiceError(response['error']) diff --git a/src/oidcrp/rp_handler.py b/src/oidcrp/rp_handler.py index c6449d4..609145b 100644 --- a/src/oidcrp/rp_handler.py +++ b/src/oidcrp/rp_handler.py @@ -28,9 +28,9 @@ from .defaults import DEFAULT_RP_KEY_DEFS from .exception import OidcServiceError from .oauth2 import Client +from .oauth2 import dynamic_provider_info_discovery from .oauth2.utils import pick_redirect_uri from .util import add_path -from .util import dynamic_provider_info_discovery from .util import load_registration_response from .util import rndstr @@ -185,11 +185,15 @@ def init_client(self, issuer): _context.jwks_uri = self.jwks_uri return client - def do_provider_info(self, client=None, state='', behaviour_args=None): + def do_provider_info(self, + client: Optional[Client]=None, + state: Optional[str]='', + behaviour_args: Optional[dict]=None) -> str: """ Either get the provider info from configuration or through dynamic discovery. + :param behaviour_args: :param client: A Client instance :param state: A key by which the state of the session can be retrieved @@ -205,7 +209,7 @@ def do_provider_info(self, client=None, state='', behaviour_args=None): _context = client.client_get("service_context") if not _context.get('provider_info'): - dynamic_provider_info_discovery(client) + dynamic_provider_info_discovery(client, behaviour_args=behaviour_args) return _context.get('provider_info')['issuer'] else: _pi = _context.get('provider_info') @@ -280,16 +284,9 @@ def do_client_registration(self, client=None, _params = RegistrationRequest().parameters() request_args.update({k: v for k, v in behaviour_args.items() if k in _params}) - # _ignore = [k for k in list(request_args.keys()) if k in CALLBACK_URIS] - # if _context.get('redirect_uris'): - # if 'redirect_uris' not in _ignore: - # _ignore.append('redirect_uris') - # - # add_callbacks(_context, _ignore) - load_registration_response(client, request_args=request_args) - def do_webfinger(self, user): + def do_webfinger(self, user: str) -> Client: """ Does OpenID Provider Issuer discovery using webfinger. @@ -304,7 +301,10 @@ def do_webfinger(self, user): temporary_client.do_request('webfinger', resource=user) return temporary_client - def client_setup(self, iss_id='', user='', behaviour_args=None): + def client_setup(self, + iss_id: Optional[str] = '', + user: Optional[str] = '', + behaviour_args: Optional[dict] = None) -> Client: """ First if no issuer ID is given then the identifier for the user is used by the webfinger service to try to find the issuer ID. @@ -358,11 +358,17 @@ def _get_response_type(self, context, req_args: Optional[dict] = None): else: return context.get('behaviour')['response_types'][0] - def init_authorization(self, client=None, state='', req_args=None, behaviour_args=None): + def init_authorization(self, + client: Optional[Client] = None, + state: Optional[str] = '', + req_args: Optional[dict] = None, + behaviour_args: Optional[dict] = None) -> dict: """ Constructs the URL that will redirect the user to the authorization endpoint of the OP/AS. + :param behaviour_args: + :param state: :param client: A Client instance :param req_args: Non-default Request arguments :return: A dictionary with 2 keys: **url** The authorization redirect @@ -607,8 +613,7 @@ def userinfo_in_id_token(id_token): :param id_token: An :py:class:`oidcmsg.oidc.IDToken` instance :return: A dictionary with user information """ - res = dict([(k, id_token[k]) for k in OpenIDSchema.c_param.keys() if - k in id_token]) + res = dict([(k, id_token[k]) for k in OpenIDSchema.c_param.keys() if k in id_token]) res.update(id_token.extra()) return res @@ -629,7 +634,8 @@ def finalize_auth(self, client, issuer: str, response: dict, _srv = client.get_service('authorization') try: - authorization_response = _srv.parse_response(response, sformat='dict') + authorization_response = _srv.parse_response(response, sformat='dict', + behaviour_args=behaviour_args) except Exception as err: logger.error('Parsing authorization_response: {}'.format(err)) message = traceback.format_exception(*sys.exc_info()) diff --git a/src/oidcrp/util.py b/src/oidcrp/util.py index 51591d2..17e0d51 100755 --- a/src/oidcrp/util.py +++ b/src/oidcrp/util.py @@ -9,6 +9,7 @@ import ssl import string import sys +from typing import Optional from urllib.parse import parse_qs from urllib.parse import urlsplit from urllib.parse import urlunsplit @@ -553,26 +554,3 @@ def load_registration_response(client, request_args=None): else: if 'error' in response: raise OidcServiceError(response.to_json()) - - -def dynamic_provider_info_discovery(client): - """ - This is about performing dynamic Provider Info discovery - - :param client: A :py:class:`oidcrp.oidc.Client` instance - """ - try: - client.get_service('provider_info') - except KeyError: - raise ConfigurationError( - 'Can not do dynamic provider info discovery') - else: - _context = client.client_get("service_context") - try: - _context.set('issuer', _context.config['srv_discovery_url']) - except KeyError: - pass - - response = client.do_request('provider_info') - if is_error_message(response): - raise OidcServiceError(response['error']) From fac61ff876227f95bdafdaaff6e1cd2b07aeeb9d Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Sun, 10 Oct 2021 08:47:49 +0200 Subject: [PATCH 33/40] This method is about getting all the tokens the token endpoint provides not just the access token. --- src/oidcrp/rp_handler.py | 18 ++++++++---------- tests/test_20_rp_handler_oidc.py | 4 ++-- tests/test_40_rp_handler_persistent.py | 4 ++-- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/oidcrp/rp_handler.py b/src/oidcrp/rp_handler.py index 609145b..f5ac634 100644 --- a/src/oidcrp/rp_handler.py +++ b/src/oidcrp/rp_handler.py @@ -489,7 +489,7 @@ def get_client_authn_method(client, endpoint): else: # a list return am[0] - def get_access_token(self, state, client: Optional[Client] = None): + def get_tokens(self, state, client: Optional[Client] = None): """ Use the 'accesstoken' service to get an access token from the OP/AS. @@ -499,7 +499,7 @@ def get_access_token(self, state, client: Optional[Client] = None): :return: A :py:class:`oidcmsg.oidc.AccessTokenResponse` or :py:class:`oidcmsg.oauth2.AuthorizationResponse` """ - logger.debug(20 * "*" + " get_access_token " + 20 * "*") + logger.debug(20 * "*" + " get_tokens " + 20 * "*") if client is None: client = self.get_client_from_session_key(state) @@ -699,8 +699,7 @@ def get_access_and_id_token(self, authorization_response=None, if not state: state = authorization_response['state'] - authreq = _context.state.get_item( - AuthorizationRequest, 'auth_request', state) + authreq = _context.state.get_item(AuthorizationRequest, 'auth_request', state) _resp_type = set(authreq['response_type']) access_token = None @@ -712,11 +711,10 @@ def get_access_and_id_token(self, authorization_response=None, if _resp_type in [{'token'}, {'id_token', 'token'}, {'code', 'token'}, {'code', 'id_token', 'token'}]: access_token = authorization_response["access_token"] - if behaviour_args and behaviour_args.get("collect_id_token", False): - if "id_token" not in _resp_type: - logger.debug("Collect ID Token") - # get the access token - token_resp = self.get_access_token(state, client=client) + if behaviour_args: + if behaviour_args.get("collect_tokens", False): + # get what you can from the token endpoint + token_resp = self.get_tokens(state, client=client) if is_error_message(token_resp): return False, "Invalid response %s." % token_resp["error"] # Now which access_token should I use @@ -726,7 +724,7 @@ def get_access_and_id_token(self, authorization_response=None, elif _resp_type in [{'code'}, {'code', 'id_token'}]: # get the access token - token_resp = self.get_access_token(state, client=client) + token_resp = self.get_tokens(state, client=client) if is_error_message(token_resp): return False, "Invalid response %s." % token_resp["error"] diff --git a/tests/test_20_rp_handler_oidc.py b/tests/test_20_rp_handler_oidc.py index c2769df..1191cd9 100644 --- a/tests/test_20_rp_handler_oidc.py +++ b/tests/test_20_rp_handler_oidc.py @@ -425,7 +425,7 @@ def test_get_client_authn_method(self): 'token_endpoint') assert authn_method == 'client_secret_post' - def test_get_access_token(self): + def test_get_tokens(self): res = self.rph.begin(issuer_id='github') _session = self.rph.get_session_information(res['state']) client = self.rph.issuer2rp[_session['iss']] @@ -464,7 +464,7 @@ def test_get_access_token(self): resp = self.rph.finalize_auth(client, _session['iss'], auth_response.to_dict()) - resp = self.rph.get_access_token(res['state'], client) + resp = self.rph.get_tokens(res['state'], client) assert set(resp.keys()) == {'access_token', 'expires_in', 'id_token', 'token_type', '__verified_id_token', '__expires_at'} diff --git a/tests/test_40_rp_handler_persistent.py b/tests/test_40_rp_handler_persistent.py index b9567c7..e01de6d 100644 --- a/tests/test_40_rp_handler_persistent.py +++ b/tests/test_40_rp_handler_persistent.py @@ -359,7 +359,7 @@ def test_get_client_authn_method(self): 'token_endpoint') assert authn_method == 'client_secret_post' - def test_get_access_token(self): + def test_get_tokens(self): rph_1 = RPHandler(BASE_URL, client_configs=CLIENT_CONFIG, keyjar=CLI_KEY, module_dirs=['oidc']) @@ -401,7 +401,7 @@ def test_get_access_token(self): resp = rph_1.finalize_auth(client, _session['iss'], auth_response.to_dict()) - resp = rph_1.get_access_token(res['state'], client) + resp = rph_1.get_tokens(res['state'], client) assert set(resp.keys()) == {'access_token', 'expires_in', 'id_token', 'token_type', '__verified_id_token', '__expires_at'} From 28f87513b260c280537d4de29522d3a85996f8dc Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Wed, 13 Oct 2021 12:16:03 +0200 Subject: [PATCH 34/40] post_logout_redirect_uri not post_logout_redirect_uris --- example/flask_rp/conf.json | 8 ++------ src/oidcrp/configure.py | 2 +- src/oidcrp/oidc/end_session.py | 2 +- src/oidcrp/oidc/registration.py | 4 ++-- src/oidcrp/rp_handler.py | 4 ++-- tests/test_13_oidc_service.py | 8 ++++---- tests/test_20_rp_handler_oidc.py | 2 +- 7 files changed, 13 insertions(+), 17 deletions(-) diff --git a/example/flask_rp/conf.json b/example/flask_rp/conf.json index b7cd241..6d7203b 100644 --- a/example/flask_rp/conf.json +++ b/example/flask_rp/conf.json @@ -162,9 +162,7 @@ "redirect_uris": [ "https://{domain}:{port}/authz_cb/local" ], - "post_logout_redirect_uris": [ - "https://{domain}:{port}/session_logout/local" - ], + "post_logout_redirect_uri": "https://{domain}:{port}/session_logout/local", "frontchannel_logout_uri": "https://{domain}:{port}/fc_logout/local", "frontchannel_logout_session_required": true, "backchannel_logout_uri": "https://{domain}:{port}/bc_logout/local", @@ -231,9 +229,7 @@ "redirect_uris": [ "https://{domain}:{port}/authz_cb/django" ], - "post_logout_redirect_uris": [ - "https://{domain}:{port}/session_logout/django" - ], + "post_logout_redirect_uris": "https://{domain}:{port}/session_logout/django", "frontchannel_logout_uri": "https://{domain}:{port}/fc_logout/django", "frontchannel_logout_session_required": true, "backchannel_logout_uri": "https://{domain}:{port}/bc_logout/django", diff --git a/src/oidcrp/configure.py b/src/oidcrp/configure.py index 84ac367..05da08f 100755 --- a/src/oidcrp/configure.py +++ b/src/oidcrp/configure.py @@ -100,7 +100,7 @@ def extend(self, entity_conf, conf, base_path, file_attributes, domain, port): URIS = [ - "redirect_uris", 'post_logout_redirect_uris', 'frontchannel_logout_uri', + "redirect_uris", 'post_logout_redirect_uri', 'frontchannel_logout_uri', 'backchannel_logout_uri', 'issuer', 'base_url'] diff --git a/src/oidcrp/oidc/end_session.py b/src/oidcrp/oidc/end_session.py index 91030f4..0d7277d 100644 --- a/src/oidcrp/oidc/end_session.py +++ b/src/oidcrp/oidc/end_session.py @@ -59,7 +59,7 @@ def add_post_logout_redirect_uri(self, request_args=None, **kwargs): if _uri: request_args['post_logout_redirect_uri'] = _uri else: - _uris = _context.callback.get("post_logout_redirect_uris", []) + _uris = _context.callback.get("post_logout_redirect_uri", []) if _uris: request_args['post_logout_redirect_uri'] = _uris[0] diff --git a/src/oidcrp/oidc/registration.py b/src/oidcrp/oidc/registration.py index 06a3ea9..2bf6fe9 100644 --- a/src/oidcrp/oidc/registration.py +++ b/src/oidcrp/oidc/registration.py @@ -80,7 +80,7 @@ def create_callbacks(issuer: str, res["request_uris"] = f"{base_url}/req_uri/{_hex}" if backchannel_logout_uri or frontchannel_logout_uri: - res["post_logout_redirect_uris"] = [f"{base_url}/session_logout/{_hex}"] + res["post_logout_redirect_uri"] = [f"{base_url}/session_logout/{_hex}"] if backchannel_logout_uri: res["backchannel_logout_uri"] = f"{base_url}/bc_logout/{_hex}" @@ -162,7 +162,7 @@ def add_callbacks(context, ignore: Optional[List[str]] = None): context.set('callback', callbacks) -CALLBACK_URIS = ["post_logout_redirect_uris", "backchannel_logout_uri", "frontchannel_logout_uri", +CALLBACK_URIS = ["post_logout_redirect_uri", "backchannel_logout_uri", "frontchannel_logout_uri", "request_uris", 'redirect_uris'] diff --git a/src/oidcrp/rp_handler.py b/src/oidcrp/rp_handler.py index f5ac634..19f69e0 100644 --- a/src/oidcrp/rp_handler.py +++ b/src/oidcrp/rp_handler.py @@ -273,8 +273,8 @@ def do_client_registration(self, client=None, self.hash2issuer[iss_id] = _iss # This should only be interesting if the client supports Single Log Out - # if _context.callback.get("post_logout_redirect_uris") is None: - # _context.callback["post_logout_redirect_uris"] = [self.base_url] + # if _context.callback.get("post_logout_redirect_uri") is None: + # _context.callback["post_logout_redirect_uri"] = [self.base_url] if not _context.client_id: # means I have to do dynamic client registration if request_args is None: diff --git a/tests/test_13_oidc_service.py b/tests/test_13_oidc_service.py index e3f4450..4ef52bd 100644 --- a/tests/test_13_oidc_service.py +++ b/tests/test_13_oidc_service.py @@ -608,11 +608,11 @@ def test_construct(self): def test_config_with_post_logout(self): self.service.client_get("service_context").register_args[ - 'post_logout_redirect_uris'] = ['https://example.com/post_logout'] + 'post_logout_redirect_uri'] = 'https://example.com/post_logout' _req = self.service.construct() assert isinstance(_req, RegistrationRequest) assert len(_req) == 5 - assert 'post_logout_redirect_uris' in _req + assert 'post_logout_redirect_uri' in _req def test_config_with_required_request_uri(): @@ -666,7 +666,7 @@ def test_config_logout_uri(): assert len(_req) == 7 assert 'request_uris' in _req assert 'frontchannel_logout_uri' in _req - assert 'post_logout_redirect_uris' in _req + assert 'post_logout_redirect_uri' in _req class TestUserInfo(object): @@ -874,7 +874,7 @@ def create_request(self): 'redirect_uris': ['https://example.com/cli/authz_cb'], 'issuer': self._iss, 'requests_dir': 'requests', 'base_url': 'https://example.com/cli/', - 'post_logout_redirect_uris': ['https://example.com/post_logout'] + 'post_logout_redirect_uri': 'https://example.com/post_logout' } services = { "checksession": { diff --git a/tests/test_20_rp_handler_oidc.py b/tests/test_20_rp_handler_oidc.py index 1191cd9..4a942be 100644 --- a/tests/test_20_rp_handler_oidc.py +++ b/tests/test_20_rp_handler_oidc.py @@ -312,7 +312,7 @@ def test_do_client_registration(self): # only 2 things should have happened assert self.rph.hash2issuer['github'] == issuer - assert client.client_get("service_context").callback.get("post_logout_redirect_uris") is None + assert client.client_get("service_context").callback.get("post_logout_redirect_uri") is None def test_do_client_setup(self): client = self.rph.client_setup('github') From 57330a1abd6d1db50a2a6ee4121b21b63f3ba448 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Wed, 13 Oct 2021 12:24:09 +0200 Subject: [PATCH 35/40] post_logout_redirect_uri not post_logout_redirect_uris --- src/oidcrp/oidc/registration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/oidcrp/oidc/registration.py b/src/oidcrp/oidc/registration.py index 2bf6fe9..9ebf280 100644 --- a/src/oidcrp/oidc/registration.py +++ b/src/oidcrp/oidc/registration.py @@ -80,7 +80,7 @@ def create_callbacks(issuer: str, res["request_uris"] = f"{base_url}/req_uri/{_hex}" if backchannel_logout_uri or frontchannel_logout_uri: - res["post_logout_redirect_uri"] = [f"{base_url}/session_logout/{_hex}"] + res["post_logout_redirect_uri"] = f"{base_url}/session_logout/{_hex}" if backchannel_logout_uri: res["backchannel_logout_uri"] = f"{base_url}/bc_logout/{_hex}" From 59f81702b0ea4d4a2a1c0b51bacf12bea3ab66cb Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 14 Oct 2021 09:07:22 +0200 Subject: [PATCH 36/40] post_logout_redirect_uri not post_logout_redirect_uris --- src/oidcrp/oidc/end_session.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/oidcrp/oidc/end_session.py b/src/oidcrp/oidc/end_session.py index 0d7277d..6aa6bbb 100644 --- a/src/oidcrp/oidc/end_session.py +++ b/src/oidcrp/oidc/end_session.py @@ -59,9 +59,9 @@ def add_post_logout_redirect_uri(self, request_args=None, **kwargs): if _uri: request_args['post_logout_redirect_uri'] = _uri else: - _uris = _context.callback.get("post_logout_redirect_uri", []) - if _uris: - request_args['post_logout_redirect_uri'] = _uris[0] + _uri = _context.callback.get("post_logout_redirect_uri") + if _uri: + request_args['post_logout_redirect_uri'] = _uri return request_args, {} From 813b9d96f654f8632fbd122eeb2f812361044a20 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 14 Oct 2021 09:33:47 +0200 Subject: [PATCH 37/40] post_logout_redirect_uri in logout request. post_logout_redirect_uris in registration request. --- example/flask_rp/views.py | 2 +- src/oidcrp/configure.py | 2 +- src/oidcrp/oidc/end_session.py | 11 +++++++---- src/oidcrp/oidc/registration.py | 4 ++-- src/oidcrp/service_context.py | 4 ++-- tests/test_13_oidc_service.py | 8 ++++---- tests/test_20_rp_handler_oidc.py | 2 +- tests/test_40_rp_handler_persistent.py | 2 +- 8 files changed, 19 insertions(+), 16 deletions(-) diff --git a/example/flask_rp/views.py b/example/flask_rp/views.py index 9e2a127..caa7141 100644 --- a/example/flask_rp/views.py +++ b/example/flask_rp/views.py @@ -160,7 +160,7 @@ def get_op_identifier_by_cb_uri(url: str): for k, v in current_app.rph.issuer2rp.items(): _cntx = v.get_service_context() for endpoint in ("redirect_uris", - "post_logout_redirect_uri", + "post_logout_redirect_uris", "frontchannel_logout_uri", "backchannel_logout_uri"): if uri in _cntx.get(endpoint, []): diff --git a/src/oidcrp/configure.py b/src/oidcrp/configure.py index 05da08f..84ac367 100755 --- a/src/oidcrp/configure.py +++ b/src/oidcrp/configure.py @@ -100,7 +100,7 @@ def extend(self, entity_conf, conf, base_path, file_attributes, domain, port): URIS = [ - "redirect_uris", 'post_logout_redirect_uri', 'frontchannel_logout_uri', + "redirect_uris", 'post_logout_redirect_uris', 'frontchannel_logout_uri', 'backchannel_logout_uri', 'issuer', 'base_url'] diff --git a/src/oidcrp/oidc/end_session.py b/src/oidcrp/oidc/end_session.py index 6aa6bbb..df4d2d4 100644 --- a/src/oidcrp/oidc/end_session.py +++ b/src/oidcrp/oidc/end_session.py @@ -55,13 +55,16 @@ def get_id_token_hint(self, request_args=None, **kwargs): def add_post_logout_redirect_uri(self, request_args=None, **kwargs): if 'post_logout_redirect_uri' not in request_args: _context = self.client_get("service_context") - _uri = _context.register_args.get('post_logout_redirect_uri') + _uri = _context.register_args.get('post_logout_redirect_uris') if _uri: - request_args['post_logout_redirect_uri'] = _uri + if isinstance(_uri, str): + request_args['post_logout_redirect_uri'] = _uri + else: # assume list + request_args['post_logout_redirect_uri'] = _uri[0] else: - _uri = _context.callback.get("post_logout_redirect_uri") + _uri = _context.callback.get("post_logout_redirect_uris") if _uri: - request_args['post_logout_redirect_uri'] = _uri + request_args['post_logout_redirect_uri'] = _uri[0] return request_args, {} diff --git a/src/oidcrp/oidc/registration.py b/src/oidcrp/oidc/registration.py index 9ebf280..06a3ea9 100644 --- a/src/oidcrp/oidc/registration.py +++ b/src/oidcrp/oidc/registration.py @@ -80,7 +80,7 @@ def create_callbacks(issuer: str, res["request_uris"] = f"{base_url}/req_uri/{_hex}" if backchannel_logout_uri or frontchannel_logout_uri: - res["post_logout_redirect_uri"] = f"{base_url}/session_logout/{_hex}" + res["post_logout_redirect_uris"] = [f"{base_url}/session_logout/{_hex}"] if backchannel_logout_uri: res["backchannel_logout_uri"] = f"{base_url}/bc_logout/{_hex}" @@ -162,7 +162,7 @@ def add_callbacks(context, ignore: Optional[List[str]] = None): context.set('callback', callbacks) -CALLBACK_URIS = ["post_logout_redirect_uri", "backchannel_logout_uri", "frontchannel_logout_uri", +CALLBACK_URIS = ["post_logout_redirect_uris", "backchannel_logout_uri", "frontchannel_logout_uri", "request_uris", 'redirect_uris'] diff --git a/src/oidcrp/service_context.py b/src/oidcrp/service_context.py index 8771fb9..7a6753c 100644 --- a/src/oidcrp/service_context.py +++ b/src/oidcrp/service_context.py @@ -103,7 +103,7 @@ class ServiceContext(OidcContext): "httpc_params": None, 'issuer': None, "kid": None, - "post_logout_redirect_uri": '', + "post_logout_redirect_uris": None, 'provider_info': None, 'redirect_uris': None, "requests_dir": None, @@ -137,7 +137,7 @@ def __init__(self, base_url="", keyjar=None, config=None, state=None, **kwargs): self.client_secret_expires_at = 0 self.behaviour = {} self.provider_info = {} - self.post_logout_redirect_uri = '' + self.post_logout_redirect_uris = [] self.redirect_uris = [] self.register_args = {} self.registration_response = {} diff --git a/tests/test_13_oidc_service.py b/tests/test_13_oidc_service.py index 4ef52bd..e3f4450 100644 --- a/tests/test_13_oidc_service.py +++ b/tests/test_13_oidc_service.py @@ -608,11 +608,11 @@ def test_construct(self): def test_config_with_post_logout(self): self.service.client_get("service_context").register_args[ - 'post_logout_redirect_uri'] = 'https://example.com/post_logout' + 'post_logout_redirect_uris'] = ['https://example.com/post_logout'] _req = self.service.construct() assert isinstance(_req, RegistrationRequest) assert len(_req) == 5 - assert 'post_logout_redirect_uri' in _req + assert 'post_logout_redirect_uris' in _req def test_config_with_required_request_uri(): @@ -666,7 +666,7 @@ def test_config_logout_uri(): assert len(_req) == 7 assert 'request_uris' in _req assert 'frontchannel_logout_uri' in _req - assert 'post_logout_redirect_uri' in _req + assert 'post_logout_redirect_uris' in _req class TestUserInfo(object): @@ -874,7 +874,7 @@ def create_request(self): 'redirect_uris': ['https://example.com/cli/authz_cb'], 'issuer': self._iss, 'requests_dir': 'requests', 'base_url': 'https://example.com/cli/', - 'post_logout_redirect_uri': 'https://example.com/post_logout' + 'post_logout_redirect_uris': ['https://example.com/post_logout'] } services = { "checksession": { diff --git a/tests/test_20_rp_handler_oidc.py b/tests/test_20_rp_handler_oidc.py index 4a942be..1191cd9 100644 --- a/tests/test_20_rp_handler_oidc.py +++ b/tests/test_20_rp_handler_oidc.py @@ -312,7 +312,7 @@ def test_do_client_registration(self): # only 2 things should have happened assert self.rph.hash2issuer['github'] == issuer - assert client.client_get("service_context").callback.get("post_logout_redirect_uri") is None + assert client.client_get("service_context").callback.get("post_logout_redirect_uris") is None def test_do_client_setup(self): client = self.rph.client_setup('github') diff --git a/tests/test_40_rp_handler_persistent.py b/tests/test_40_rp_handler_persistent.py index e01de6d..737365b 100644 --- a/tests/test_40_rp_handler_persistent.py +++ b/tests/test_40_rp_handler_persistent.py @@ -247,7 +247,7 @@ def test_do_client_registration(self): # only 2 things should have happened assert rph_1.hash2issuer['github'] == issuer - assert not client.client_get("service_context").callback.get('post_logout_redirect_uri') + assert not client.client_get("service_context").callback.get('post_logout_redirect_uris') def test_do_client_setup(self): rph_1 = RPHandler(BASE_URL, client_configs=CLIENT_CONFIG, From 3088dec3b998a5b5c114bb734c12b275fe8a17a7 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 14 Oct 2021 10:05:48 +0200 Subject: [PATCH 38/40] Spelling error. --- src/oidcrp/state_interface.py | 2 +- tests/pub_client.jwks | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/oidcrp/state_interface.py b/src/oidcrp/state_interface.py index a377da1..bfc7100 100644 --- a/src/oidcrp/state_interface.py +++ b/src/oidcrp/state_interface.py @@ -382,7 +382,7 @@ def remove_state(self, state): :param state: Key to the state """ del self._db[state] - refs = json.loads(self._db_db.get("ref{}ref".format(state))) + refs = json.loads(self._db.get("ref{}ref".format(state))) if refs: for xtyp, _val in refs.items(): del self._db[KEY_PATTERN[xtyp].format(_val)] diff --git a/tests/pub_client.jwks b/tests/pub_client.jwks index a57e904..d16a636 100644 --- a/tests/pub_client.jwks +++ b/tests/pub_client.jwks @@ -1 +1 @@ -{"keys": [{"kty": "RSA", "use": "sig", "kid": "SUswNi1MRFlDT0Y2YjU1Z1RfQlo2S3dEa3FTTkV3LThFcnhDTHF5elk2VQ", "n": "0UkUx2ewKyc-XJ1o0ToyGjws_JybAMZj2oYjsPyyvQ_T5dhZ2VmRRRkhsaVJ2xE_GGc7mSG0IjmGFyXp5y0w4mJBcsAEE5-8eBTvQdYIryjW74r3jt6Fi4Hlm1yFMTie3apv8mw79BUj-jT0kh3_m-FiKKUvLsq45DcLtTJ4cx7Ize37dl1sFSpQcoYMk7eiUEM8fiNboiVwvBYNAWVMkUM-LnVUPm3UjvKp0LihYEkZFWOxmuQmj2x25SFUkjus38ERrRqJQBZduxdBHFrWtWg8yOA53BkMU0FFg_r0H3ctl-5GaKw-BWlogU4qXnsq85xy0EoenRk7FPV8g_ulJw", "e": "AQAB"}, {"kty": "EC", "use": "sig", "kid": "NC1pdGRQN002bWM3bk1xX2R0SktscElqbFdtN29ITDV2WVd2b0hOYzREVQ", "crv": "P-256", "x": "kK7Qp1woSerI7rUOAwW_4sU6ZmwV3wwXKX3VU-v2fMI", "y": "iPWd_Pjq6EjxYy08KNFZ3PxhEwgWHgAQTTknlKMKJA0"}]} \ No newline at end of file +{"keys": [{"kty": "RSA", "use": "sig", "kid": "SUswNi1MRFlDT0Y2YjU1Z1RfQlo2S3dEa3FTTkV3LThFcnhDTHF5elk2VQ", "e": "AQAB", "n": "0UkUx2ewKyc-XJ1o0ToyGjws_JybAMZj2oYjsPyyvQ_T5dhZ2VmRRRkhsaVJ2xE_GGc7mSG0IjmGFyXp5y0w4mJBcsAEE5-8eBTvQdYIryjW74r3jt6Fi4Hlm1yFMTie3apv8mw79BUj-jT0kh3_m-FiKKUvLsq45DcLtTJ4cx7Ize37dl1sFSpQcoYMk7eiUEM8fiNboiVwvBYNAWVMkUM-LnVUPm3UjvKp0LihYEkZFWOxmuQmj2x25SFUkjus38ERrRqJQBZduxdBHFrWtWg8yOA53BkMU0FFg_r0H3ctl-5GaKw-BWlogU4qXnsq85xy0EoenRk7FPV8g_ulJw"}, {"kty": "EC", "use": "sig", "kid": "NC1pdGRQN002bWM3bk1xX2R0SktscElqbFdtN29ITDV2WVd2b0hOYzREVQ", "crv": "P-256", "x": "kK7Qp1woSerI7rUOAwW_4sU6ZmwV3wwXKX3VU-v2fMI", "y": "iPWd_Pjq6EjxYy08KNFZ3PxhEwgWHgAQTTknlKMKJA0"}]} \ No newline at end of file From 02cfc1edaa317878b62f3c719795ca8d6eb52b76 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Wed, 20 Oct 2021 11:33:24 +0200 Subject: [PATCH 39/40] If no ID Token then no sid. --- src/oidcrp/rp_handler.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/oidcrp/rp_handler.py b/src/oidcrp/rp_handler.py index 19f69e0..957ab2d 100644 --- a/src/oidcrp/rp_handler.py +++ b/src/oidcrp/rp_handler.py @@ -1,3 +1,4 @@ +from inspect import currentframe import logging import sys import traceback @@ -377,7 +378,6 @@ def init_authorization(self, """ logger.debug(20 * "*" + " init_authorization " + 20 * "*") - if not client: if state: client = self.get_client_from_session_key(state) @@ -791,8 +791,7 @@ def finalize(self, issuer, response, behaviour_args: Optional[dict] = None): _context = client.client_get("service_context") try: - _sid_support = _context.get('provider_info')[ - 'backchannel_logout_session_supported'] + _sid_support = _context.get('provider_info')['backchannel_logout_session_supported'] except KeyError: try: _sid_support = _context.get('provider_info')[ @@ -800,7 +799,7 @@ def finalize(self, issuer, response, behaviour_args: Optional[dict] = None): except: _sid_support = False - if _sid_support: + if _sid_support and _id_token: try: sid = _id_token['sid'] except KeyError: From de794e2351c849b3d69a91bfb883771e502234a4 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Wed, 20 Oct 2021 16:14:40 +0200 Subject: [PATCH 40/40] response_type can be list or singelton. --- src/oidcrp/oauth2/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/oidcrp/oauth2/utils.py b/src/oidcrp/oauth2/utils.py index eb11816..d1c4038 100644 --- a/src/oidcrp/oauth2/utils.py +++ b/src/oidcrp/oauth2/utils.py @@ -44,9 +44,9 @@ def pick_redirect_uri(context, _response_mode = request_args.get('response_mode') - if _response_mode == 'form_post': + if _response_mode == 'form_post' or response_type == ["form_post"]: redirect_uri = context.callback['form_post'] - elif response_type == 'code': + elif response_type == 'code' or response_type == ["code"]: redirect_uri = context.callback['code'] else: redirect_uri = context.callback['implicit']