From f091c20780f6f9dadc708485897eebc34f18b709 Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Mon, 13 Sep 2021 18:17:41 +0300 Subject: [PATCH 1/4] Refactor scopes --- docs/source/contents/conf.rst | 59 +++-- src/oidcop/authz/__init__.py | 6 +- src/oidcop/configure.py | 41 +++- src/oidcop/endpoint_context.py | 19 +- src/oidcop/oauth2/authorization.py | 19 +- src/oidcop/oidc/add_on/custom_scopes.py | 7 +- src/oidcop/scopes.py | 67 ++++-- src/oidcop/server.py | 7 +- src/oidcop/session/claims.py | 6 +- tests/test_07_userinfo.py | 217 ++++++++++++------ .../test_22_oidc_provider_config_endpoint.py | 40 +++- tests/test_24_oauth2_token_endpoint.py | 6 +- tests/test_26_oidc_userinfo_endpoint.py | 144 ++++++++++-- tests/test_50_persistence.py | 33 ++- 14 files changed, 484 insertions(+), 187 deletions(-) diff --git a/docs/source/contents/conf.rst b/docs/source/contents/conf.rst index 9799e8bc..f8a97b1c 100644 --- a/docs/source/contents/conf.rst +++ b/docs/source/contents/conf.rst @@ -53,6 +53,28 @@ sub_funcs Optional. Functions involved in *sub*ject value creation. + + +scopes_mapping +############## + +A dict defining the scopes that are allowed to be used per client and the claims +they map to (defaults to the scopes mapping described in the spec). If we want +to define a scope that doesn't map to claims (e.g. offline_access) then we +simply map it to an empty list. E.g.:: + { + "scope_a": ["claim1", "claim2"], + "scope_b": [] + } +*Note*: For OIDC the `openid` scope must be present in this mapping. + + +allowed_scopes +############## + +A list with the scopes that are allowed to be used (defaults to the keys in scopes_mapping). + + ------ add_on ------ @@ -67,21 +89,6 @@ An example:: "code_challenge_method": "S256 S384 S512" } }, - "claims": { - "function": "oidcop.oidc.add_on.custom_scopes.add_custom_scopes", - "kwargs": { - "research_and_scholarship": [ - "name", - "given_name", - "family_name", - "email", - "email_verified", - "sub", - "iss", - "eduperson_scoped_affiliation" - ] - } - } } The provided add-ons can be seen in the following sections. @@ -176,6 +183,8 @@ An example:: backchannel_logout_supported: True backchannel_logout_session_supported: True check_session_iframe: https://127.0.0.1:5000/check_session_iframe + scopes_supported: ["openid", "profile", "random"] + claims_supported: ["sub", "given_name", "birthdate"] --------- client_db @@ -720,3 +729,23 @@ grant_types_supported --------------------- Configure the allowed grant types on the token endpoint. + +-------------- +scopes_mapping +-------------- + +A dict defining the scopes that are allowed to be used per client and the claims +they map to (defaults to the scopes mapping described in the spec). If we want +to define a scope that doesn't map to claims (e.g. offline_access) then we +simply map it to an empty list. E.g.:: + { + "scope_a": ["claim1", "claim2"], + "scope_b": [] + } + +-------------- +allowed_scopes +-------------- + +A list with the scopes that are allowed to be used (defaults to the keys in the +clients scopes_mapping). diff --git a/src/oidcop/authz/__init__.py b/src/oidcop/authz/__init__.py index 04203cff..a22a11bb 100755 --- a/src/oidcop/authz/__init__.py +++ b/src/oidcop/authz/__init__.py @@ -80,8 +80,10 @@ def __call__( grant.resources = resources # After this is where user consent should be handled - scopes = request.get("scope", []) - grant.scope = scopes + scopes = grant.scope + if not scopes: + scopes = request.get("scope", []) + grant.scope = scopes grant.claims = self.server_get("endpoint_context").claims_interface.get_claims_all_usage( session_id=session_id, scopes=scopes ) diff --git a/src/oidcop/configure.py b/src/oidcop/configure.py index 87591022..bde65666 100755 --- a/src/oidcop/configure.py +++ b/src/oidcop/configure.py @@ -10,6 +10,7 @@ from typing import Union from oidcop.logging import configure_logging +from oidcop.scopes import SCOPE2CLAIMS from oidcop.utils import load_yaml_config DEFAULT_FILE_ATTRIBUTE_NAMES = [ @@ -78,6 +79,7 @@ "refresh": {"class": "oidcop.token.jwt_token.JWTToken", "kwargs": {"lifetime": 86400}, }, "id_token": {"class": "oidcop.token.id_token.IDToken", "kwargs": {}}, }, + "scopes_mapping": SCOPE2CLAIMS, } AS_DEFAULT_CONFIG = copy.deepcopy(OP_DEFAULT_CONFIG) @@ -274,12 +276,39 @@ class OPConfiguration(EntityConfiguration): "Provider configuration" default_config = OP_DEFAULT_CONFIG parameter = EntityConfiguration.parameter.copy() - parameter.update({ - "id_token": None, - "login_hint2acrs": {}, - "login_hint_lookup": None, - "sub_func": {} - }) + parameter.update( + { + "id_token": None, + "login_hint2acrs": {}, + "login_hint_lookup": None, + "sub_func": {}, + "scopes_mapping": {}, + "scopes_supported": None, + "advertised_scopes": None, + } + ) + + def __init__( + self, + conf: Dict, + base_path: Optional[str] = "", + entity_conf: Optional[List[dict]] = None, + domain: Optional[str] = "", + port: Optional[int] = 0, + file_attributes: Optional[List[str]] = None, + ): + super().__init__( + conf=conf, + base_path=base_path, + entity_conf=entity_conf, + domain=domain, + port=port, + file_attributes=file_attributes, + ) + scopes_mapping = self.scopes_mapping + if "advertised_scopes" not in self: + self["advertised_scopes"] = list(scopes_mapping.keys()) + class ASConfiguration(EntityConfiguration): "Authorization server configuration" diff --git a/src/oidcop/endpoint_context.py b/src/oidcop/endpoint_context.py index 9a683f2b..6285abe7 100755 --- a/src/oidcop/endpoint_context.py +++ b/src/oidcop/endpoint_context.py @@ -1,6 +1,7 @@ import json import logging from typing import Any +from typing import Callable from typing import Optional from typing import Union @@ -121,6 +122,7 @@ class EndpointContext(OidcContext): def __init__( self, conf: Union[dict, OPConfiguration], + server_get: Callable, keyjar: Optional[KeyJar] = None, cwd: Optional[str] = "", cookie_handler: Optional[Any] = None, @@ -128,6 +130,7 @@ def __init__( ): OidcContext.__init__(self, conf, keyjar, entity_id=conf.get("issuer", "")) self.conf = conf + self.server_get = server_get _client_db = conf.get("client_db") if _client_db: @@ -248,10 +251,14 @@ def set_scopes_handler(self): _spec = self.conf.get("scopes_handler") if _spec: _kwargs = _spec.get("kwargs", {}) - _cls = importer(_spec["class"])(**_kwargs) - self.scopes_handler = _cls(_kwargs) + _cls = importer(_spec["class"]) + self.scopes_handler = _cls(self.server_get, **_kwargs) else: - self.scopes_handler = Scopes() + self.scopes_handler = Scopes( + self.server_get, + allowed_scopes=self.conf.get("allowed_scopes"), + scopes_mapping=self.conf.get("scopes_mapping"), + ) def do_add_on(self, endpoints): _add_on_conf = self.conf.get("add_on") @@ -325,8 +332,10 @@ def create_providerinfo(self, capabilities): _provider_info["jwks_uri"] = self.jwks_uri if "scopes_supported" not in _provider_info: - _provider_info["scopes_supported"] = [s for s in self.scope2claims.keys()] + _provider_info["scopes_supported"] = self.scopes_handler.get_allowed_scopes() if "claims_supported" not in _provider_info: - _provider_info["claims_supported"] = STANDARD_CLAIMS[:] + _provider_info["claims_supported"] = list( + self.scopes_handler.scopes_to_claims(_provider_info["scopes_supported"]).keys() + ) return _provider_info diff --git a/src/oidcop/oauth2/authorization.py b/src/oidcop/oauth2/authorization.py index 41339e98..c225a73a 100755 --- a/src/oidcop/oauth2/authorization.py +++ b/src/oidcop/oauth2/authorization.py @@ -244,15 +244,17 @@ def authn_args_gather( return authn_args -def check_unknown_scopes_policy(request_info, cinfo, endpoint_context): - op_capabilities = endpoint_context.conf["capabilities"] - client_allowed_scopes = cinfo.get("allowed_scopes") or op_capabilities["scopes_supported"] +def check_unknown_scopes_policy(request_info, client_id, endpoint_context): + if not endpoint_context.conf["capabilities"].get("deny_unknown_scopes"): + return + + allowed_scopes = endpoint_context.scopes_handler.get_allowed_scopes(client_id=client_id) # this prevents that authz would be released for unavailable scopes for scope in request_info["scope"]: - if op_capabilities.get("deny_unknown_scopes") and scope not in client_allowed_scopes: + if scope not in allowed_scopes: _msg = "{} requested an unauthorized scope ({})" - logger.warning(_msg.format(cinfo["client_id"], scope)) + logger.warning(_msg.format(client_id, scope)) raise UnAuthorizedClientScope() @@ -681,7 +683,9 @@ def create_authn_response(self, request: Union[dict, Message], sid: str) -> dict _sinfo = _mngr.get_session_info(sid, grant=True) if request.get("scope"): - aresp["scope"] = request["scope"] + aresp["scope"] = _context.scopes_handler.filter_scopes( + request["scope"], _sinfo["client_id"] + ) rtype = set(request["response_type"][:]) handled_response_type = [] @@ -903,8 +907,7 @@ def process_request( # logger.debug("client {}: {}".format(_cid, cinfo)) # this apply the default optionally deny_unknown_scopes policy - if cinfo: - check_unknown_scopes_policy(request, cinfo, _context) + check_unknown_scopes_policy(request, _cid, _context) if http_info is None: http_info = {} diff --git a/src/oidcop/oidc/add_on/custom_scopes.py b/src/oidcop/oidc/add_on/custom_scopes.py index 1366548f..2fbc6a32 100644 --- a/src/oidcop/oidc/add_on/custom_scopes.py +++ b/src/oidcop/oidc/add_on/custom_scopes.py @@ -10,17 +10,22 @@ def add_custom_scopes(endpoint, **kwargs): :param endpoint: A dictionary with endpoint instances as values """ # Just need an endpoint, anyone will do + LOGGER.warning( + "The custom_scopes add on is deprecated. The `scopes_mapping` config " + "option should be used instead." + ) _endpoint = list(endpoint.values())[0] _scopes2claims = SCOPE2CLAIMS.copy() _scopes2claims.update(kwargs) _context = _endpoint.server_get("endpoint_context") - _context.scope2claims = _scopes2claims + _context.scopes_handler.scopes_mapping = _scopes2claims pi = _context.provider_info _scopes = set(pi.get("scopes_supported", [])) _scopes.update(set(kwargs.keys())) pi["scopes_supported"] = list(_scopes) + _context.scopes_handler.allowed_scopes = pi["scopes_supported"] _claims = set(pi.get("claims_supported", [])) for vals in kwargs.values(): diff --git a/src/oidcop/scopes.py b/src/oidcop/scopes.py index ec772040..87d2fe80 100644 --- a/src/oidcop/scopes.py +++ b/src/oidcop/scopes.py @@ -25,14 +25,6 @@ } -def available_scopes(endpoint_context): - _supported = endpoint_context.provider_info.get("scopes_supported") - if _supported: - return [s for s in endpoint_context.scope2claims.keys() if s in _supported] - else: - return [s for s in endpoint_context.scope2claims.keys()] - - def convert_scopes2claims(scopes, allowed_claims=None, scope2claim_map=None): scope2claim_map = scope2claim_map or SCOPE2CLAIMS @@ -53,26 +45,55 @@ def convert_scopes2claims(scopes, allowed_claims=None, scope2claim_map=None): class Scopes: - def __init__(self): - pass + def __init__(self, server_get, allowed_scopes=None, scopes_mapping=None): + self.server_get = server_get + if not scopes_mapping: + scopes_mapping = dict(SCOPE2CLAIMS) + self.scopes_mapping = scopes_mapping + if not allowed_scopes: + allowed_scopes = list(scopes_mapping.keys()) + self.allowed_scopes = allowed_scopes - def allowed_scopes(self, client_id, endpoint_context): + def get_allowed_scopes(self, client_id=None): """ Returns the set of scopes that a specific client can use. :param client_id: The client identifier - :param endpoint_context: A EndpointContext instance :returns: List of scope names. Can be empty. """ - _cli = endpoint_context.cdb.get(client_id) - if _cli is not None: - _scopes = _cli.get("allowed_scopes") - if _scopes: - return _scopes - else: - return available_scopes(endpoint_context) - return [] - - def filter_scopes(self, client_id, endpoint_context, scopes): - allowed_scopes = self.allowed_scopes(client_id, endpoint_context) + allowed_scopes = self.allowed_scopes + if client_id: + client = self.server_get("endpoint_context").cdb.get(client_id) + if client is not None: + if "allowed_scopes" in client: + allowed_scopes = client.get("allowed_scopes") + elif "scopes_mapping" in client: + allowed_scopes = list(client.get("scopes_mapping").keys()) + + return allowed_scopes + + def get_scopes_mapping(self, client_id=None): + """ + Returns the mapping of scopes to claims fora specific client. + + :param client_id: The client identifier + :returns: Dict of scopes to claims. Can be empty. + """ + scopes_mapping = self.scopes_mapping + if client_id: + client = self.server_get("endpoint_context").cdb.get(client_id) + if client is not None: + scopes_mapping = client.get("scopes_mapping", scopes_mapping) + return scopes_mapping + + def filter_scopes(self, scopes, client_id=None): + allowed_scopes = self.get_allowed_scopes(client_id) return [s for s in scopes if s in allowed_scopes] + + def scopes_to_claims(self, scopes, scopes_mapping=None, client_id=None): + if not scopes_mapping: + scopes_mapping = self.get_scopes_mapping(client_id) + + scopes = self.filter_scopes(scopes, client_id) + + return convert_scopes2claims(scopes, scope2claim_map=scopes_mapping) diff --git a/src/oidcop/server.py b/src/oidcop/server.py index 99349fbe..c7d698ea 100644 --- a/src/oidcop/server.py +++ b/src/oidcop/server.py @@ -67,7 +67,12 @@ def __init__( ImpExp.__init__(self) self.conf = conf self.endpoint_context = EndpointContext( - conf=conf, keyjar=keyjar, cwd=cwd, cookie_handler=cookie_handler, httpc=httpc, + conf=conf, + server_get=self.server_get, + keyjar=keyjar, + cwd=cwd, + cookie_handler=cookie_handler, + httpc=httpc, ) self.endpoint_context.authz = self.do_authz() diff --git a/src/oidcop/session/claims.py b/src/oidcop/session/claims.py index 5ac1991c..3c82fc73 100755 --- a/src/oidcop/session/claims.py +++ b/src/oidcop/session/claims.py @@ -4,8 +4,8 @@ from oidcmsg.oidc import OpenIDSchema -from oidcop.exception import ServiceError from oidcop.exception import ImproperlyConfigured +from oidcop.exception import ServiceError from oidcop.scopes import convert_scopes2claims logger = logging.getLogger(__name__) @@ -113,9 +113,7 @@ def get_claims(self, session_id: str, scopes: str, claims_release_point: str) -> if add_claims_by_scope: if scopes: - _scopes = _context.scopes_handler.filter_scopes(client_id, _context, scopes) - - _claims = convert_scopes2claims(_scopes, scope2claim_map=_context.scope2claims) + _claims = _context.scopes_handler.scopes_to_claims(scopes, client_id=client_id) claims.update(_claims) # Bring in claims specification from the authorization request diff --git a/tests/test_07_userinfo.py b/tests/test_07_userinfo.py index 486614a2..5d6f821d 100644 --- a/tests/test_07_userinfo.py +++ b/tests/test_07_userinfo.py @@ -1,11 +1,11 @@ import json import os -from oidcop.configure import OPConfiguration import pytest from oidcmsg.oidc import OpenIDRequest from oidcop.authn_event import create_authn_event +from oidcop.configure import OPConfiguration from oidcop.oidc import userinfo from oidcop.oidc.authorization import Authorization from oidcop.oidc.provider_config import ProviderConfiguration @@ -388,83 +388,89 @@ def test_collect_user_info_enable_claims_per_client(self): class TestCollectUserInfoCustomScopes: - @pytest.fixture(autouse=True) - def create_endpoint_context(self): - self.server = Server( - { - "userinfo": {"class": UserInfo, "kwargs": {"db": USERINFO_DB}}, - "password": "we didn't start the fire", - "issuer": "https://example.com/op", - "claims_interface": {"class": "oidcop.session.claims.OAuth2ClaimsInterface", - "kwargs": {}}, - "endpoint": { - "provider_config": { - "path": "{}/.well-known/openid-configuration", - "class": ProviderConfiguration, - "kwargs": {}, - }, - "registration": { - "path": "{}/registration", - "class": Registration, - "kwargs": {}, - }, - "authorization": { - "path": "{}/authorization", - "class": Authorization, - "kwargs": { - "response_types_supported": [ - " ".join(x) for x in RESPONSE_TYPES_SUPPORTED - ], - "response_modes_supported": ["query", "fragment", "form_post",], - "claims_parameter_supported": True, - "request_parameter_supported": True, - "request_uri_parameter_supported": True, - }, - }, - "userinfo": { - "path": "userinfo", - "class": userinfo.UserInfo, - "kwargs": { - "claim_types_supported": ["normal", "aggregated", "distributed",], - "client_authn_method": ["bearer_header"], - "base_claims": {"eduperson_scoped_affiliation": None, "email": None,}, - "add_claims_by_scope": True, - "enable_claims_per_client": True, - }, - }, + @pytest.fixture + def conf(self): + return { + "userinfo": {"class": UserInfo, "kwargs": {"db": USERINFO_DB}}, + "password": "we didn't start the fire", + "issuer": "https://example.com/op", + "claims_interface": {"class": "oidcop.session.claims.ClaimsInterface", "kwargs": {}}, + "endpoint": { + "provider_config": { + "path": "{}/.well-known/openid-configuration", + "class": ProviderConfiguration, + "kwargs": {}, }, - "add_on": { - "custom_scopes": { - "function": "oidcop.oidc.add_on.custom_scopes.add_custom_scopes", - "kwargs": { - "research_and_scholarship": [ - "name", - "given_name", - "family_name", - "email", - "email_verified", - "sub", - "iss", - "eduperson_scoped_affiliation", - ] - }, - } + "registration": { + "path": "{}/registration", + "class": Registration, + "kwargs": {}, }, - "keys": { - "public_path": "jwks.json", - "key_defs": KEYDEFS, - "uri_path": "static/jwks.json", + "authorization": { + "path": "{}/authorization", + "class": Authorization, + "kwargs": { + "response_types_supported": [" ".join(x) for x in RESPONSE_TYPES_SUPPORTED], + "response_modes_supported": [ + "query", + "fragment", + "form_post", + ], + "claims_parameter_supported": True, + "request_parameter_supported": True, + "request_uri_parameter_supported": True, + }, }, - "authentication": { - "anon": { - "acr": INTERNETPROTOCOLPASSWORD, - "class": "oidcop.user_authn.user.NoAuthn", - "kwargs": {"user": "diana"}, - } + "userinfo": { + "path": "userinfo", + "class": userinfo.UserInfo, + "kwargs": { + "claim_types_supported": [ + "normal", + "aggregated", + "distributed", + ], + "client_authn_method": ["bearer_header"], + "base_claims": { + "eduperson_scoped_affiliation": None, + "email": None, + }, + "add_claims_by_scope": True, + "enable_claims_per_client": True, + }, }, - "template_dir": "template", - } - ) + }, + "scopes_mapping": { + "openid": ["sub"], + "research_and_scholarship": [ + "name", + "given_name", + "family_name", + "email", + "email_verified", + "sub", + "iss", + "eduperson_scoped_affiliation", + ], + }, + "keys": { + "public_path": "jwks.json", + "key_defs": KEYDEFS, + "uri_path": "static/jwks.json", + }, + "authentication": { + "anon": { + "acr": INTERNETPROTOCOLPASSWORD, + "class": "oidcop.user_authn.user.NoAuthn", + "kwargs": {"user": "diana"}, + } + }, + "template_dir": "template", + } + + @pytest.fixture(autouse=True) + def create_endpoint_context(self, conf): + self.server = Server(conf) self.endpoint_context = self.server.endpoint_context self.endpoint_context.cdb["client1"] = {} self.session_manager = self.endpoint_context.session_manager @@ -513,3 +519,66 @@ def test_collect_user_info_custom_scope(self): "given_name": "Diana", "name": "Diana Krall", } + + def test_collect_user_info_scope_mapping_per_client(self, conf): + conf["scopes_mapping"] = SCOPE2CLAIMS + server = Server(conf) + endpoint_context = server.endpoint_context + self.session_manager = endpoint_context.session_manager + claims_interface = endpoint_context.claims_interface + endpoint_context.cdb["client1"] = { + "scopes_mapping": { + "openid": ["sub"], + "research_and_scholarship": [ + "name", + "given_name", + "family_name", + "email", + "email_verified", + "sub", + "iss", + "eduperson_scoped_affiliation", + ], + } + } + + _req = OIDR.copy() + _req["scope"] = "openid research_and_scholarship" + del _req["claims"] + + session_id = self._create_session(_req) + + _restriction = claims_interface.get_claims( + session_id=session_id, scopes=_req["scope"], claims_release_point="userinfo" + ) + + res = claims_interface.get_user_claims("diana", _restriction) + + assert res == { + "eduperson_scoped_affiliation": ["staff@example.org"], + "email": "diana@example.org", + "email_verified": False, + "family_name": "Krall", + "given_name": "Diana", + "name": "Diana Krall", + } + + def test_collect_user_info_allowed_scopes_per_client(self): + self.endpoint_context.cdb["client1"] = {"allowed_scopes": {"openid"}} + + _req = OIDR.copy() + _req["scope"] = "openid research_and_scholarship" + del _req["claims"] + + session_id = self._create_session(_req) + + _restriction = self.claims_interface.get_claims( + session_id=session_id, scopes=_req["scope"], claims_release_point="userinfo" + ) + + res = self.claims_interface.get_user_claims("diana", _restriction) + + assert res == { + "eduperson_scoped_affiliation": ["staff@example.org"], + "email": "diana@example.org", + } diff --git a/tests/test_22_oidc_provider_config_endpoint.py b/tests/test_22_oidc_provider_config_endpoint.py index df950023..96424e8a 100755 --- a/tests/test_22_oidc_provider_config_endpoint.py +++ b/tests/test_22_oidc_provider_config_endpoint.py @@ -1,9 +1,9 @@ import json import os -from oidcop.configure import OPConfiguration import pytest +from oidcop.configure import OPConfiguration from oidcop.oidc.provider_config import ProviderConfiguration from oidcop.oidc.token import Token from oidcop.server import Server @@ -50,9 +50,9 @@ class TestEndpoint(object): - @pytest.fixture(autouse=True) - def create_endpoint(self): - conf = { + @pytest.fixture + def conf(self): + return { "issuer": "https://example.com/", "password": "mycket hemligt", "verify_ssl": False, @@ -68,6 +68,9 @@ def create_endpoint(self): }, "template_dir": "template", } + + @pytest.fixture(autouse=True) + def create_endpoint(self, conf): server = Server(OPConfiguration(conf=conf, base_path=BASEDIR), cwd=BASEDIR) self.endpoint_context = server.endpoint_context @@ -104,3 +107,32 @@ def test_do_response(self): "birthdate", } assert ("Content-type", "application/json; charset=utf-8") in msg["http_headers"] + + def test_advertised_scopes(self, conf): + scopes_supported = ["openid", "random", "profile"] + conf["capabilities"]["scopes_supported"] = scopes_supported + + server = Server(OPConfiguration(conf=conf, base_path=BASEDIR), cwd=BASEDIR) + endpoint = server.server_get("endpoint", "provider_config") + args = endpoint.process_request() + msg = endpoint.do_response(args["response_args"]) + assert isinstance(msg, dict) + _msg = json.loads(msg["response"]) + assert set(_msg["scopes_supported"]) == set(scopes_supported) + assert set(_msg["claims_supported"]) == { + "zoneinfo", + "gender", + "sub", + "middle_name", + "given_name", + "nickname", + "preferred_username", + "name", + "updated_at", + "birthdate", + "locale", + "profile", + "family_name", + "picture", + "website", + } diff --git a/tests/test_24_oauth2_token_endpoint.py b/tests/test_24_oauth2_token_endpoint.py index 0f2bf2fc..66e605c6 100644 --- a/tests/test_24_oauth2_token_endpoint.py +++ b/tests/test_24_oauth2_token_endpoint.py @@ -1,6 +1,7 @@ import json import os +import pytest from cryptojwt import JWT from cryptojwt.key_jar import build_keyjar from oidcmsg.oidc import AccessTokenRequest @@ -8,7 +9,6 @@ from oidcmsg.oidc import RefreshAccessTokenRequest from oidcmsg.oidc import TokenErrorResponse from oidcmsg.time_util import utc_time_sans_frac -import pytest from oidcop import JWT_BEARER from oidcop.authn_event import create_authn_event @@ -361,9 +361,7 @@ def test_do_refresh_access_token(self): def test_refresh_grant_disallowed_per_client(self): areq = AUTH_REQ.copy() areq["scope"] = ["email"] - self.endpoint_context.cdb["client_1"]["grant_types_supported"] = [ - "authorization_code" - ] + self.endpoint_context.cdb["client_1"]["grant_types_supported"] = ["authorization_code"] session_id = self._create_session(areq) grant = self.endpoint_context.authz(session_id, areq) diff --git a/tests/test_26_oidc_userinfo_endpoint.py b/tests/test_26_oidc_userinfo_endpoint.py index 03db905a..2577a450 100755 --- a/tests/test_26_oidc_userinfo_endpoint.py +++ b/tests/test_26_oidc_userinfo_endpoint.py @@ -1,11 +1,11 @@ import json import os +import pytest from oidcmsg.oauth2 import ResponseMessage from oidcmsg.oidc import AccessTokenRequest from oidcmsg.oidc import AuthorizationRequest from oidcmsg.time_util import time_sans_frac -import pytest from oidcop import user_info from oidcop.authn_event import create_authn_event @@ -17,6 +17,7 @@ from oidcop.oidc.provider_config import ProviderConfiguration from oidcop.oidc.registration import Registration from oidcop.oidc.token import Token +from oidcop.scopes import SCOPE2CLAIMS from oidcop.server import Server from oidcop.user_authn.authn_context import INTERNETPROTOCOLPASSWORD from oidcop.user_info import UserInfo @@ -148,35 +149,31 @@ def create_endpoint(self): }, "template_dir": "template", - "add_on": { - "custom_scopes": { - "function": "oidcop.oidc.add_on.custom_scopes.add_custom_scopes", - "kwargs": { - "research_and_scholarship": [ - "name", - "given_name", - "family_name", - "email", - "email_verified", - "sub", - "eduperson_scoped_affiliation", - ] - }, - } + "scopes_mapping": { + **SCOPE2CLAIMS, + "research_and_scholarship": [ + "name", + "given_name", + "family_name", + "email", + "email_verified", + "sub", + "eduperson_scoped_affiliation", + ], }, } - server = Server(OPConfiguration(conf=conf, base_path=BASEDIR), cwd=BASEDIR) + self.server = Server(OPConfiguration(conf=conf, base_path=BASEDIR), cwd=BASEDIR) - endpoint_context = server.endpoint_context - endpoint_context.cdb["client_1"] = { + self.endpoint_context = self.server.endpoint_context + self.endpoint_context.cdb["client_1"] = { "client_secret": "hemligt", "redirect_uris": [("https://example.com/cb", None)], "client_salt": "salted", "token_endpoint_auth_method": "client_secret_post", "response_types": ["code", "token", "code id_token", "id_token"], } - self.endpoint = server.server_get("endpoint", "userinfo") - self.session_manager = endpoint_context.session_manager + self.endpoint = self.server.server_get("endpoint", "userinfo") + self.session_manager = self.endpoint_context.session_manager self.user_id = "diana" def _create_session(self, auth_req, sub_type="public", sector_identifier="", @@ -320,7 +317,7 @@ def test_do_signed_response(self): res = self.endpoint.do_response(request=_req, **args) assert res - def test_custom_scope(self): + def test_scopes_mapping(self): _auth_req = AUTH_REQ.copy() _auth_req["scope"] = ["openid", "research_and_scholarship"] @@ -350,6 +347,109 @@ def test_custom_scope(self): "sub", } + def test_scopes_mapping_per_client(self): + self.endpoint_context.cdb["client_1"]["scopes_mapping"] = { + **SCOPE2CLAIMS, + "research_and_scholarship_2": [ + "name", + "given_name", + "family_name", + "email", + "email_verified", + "sub", + "eduperson_scoped_affiliation", + ], + } + + _auth_req = AUTH_REQ.copy() + _auth_req["scope"] = ["openid", "research_and_scholarship_2"] + + session_id = self._create_session(_auth_req) + grant = self.session_manager[session_id] + access_token = self._mint_token("access_token", grant, session_id) + + self.endpoint.kwargs["add_claims_by_scope"] = True + self.endpoint.server_get("endpoint_context").claims_interface.add_claims_by_scope = True + grant.claims = { + "userinfo": self.endpoint.server_get("endpoint_context").claims_interface.get_claims( + session_id=session_id, scopes=_auth_req["scope"], claims_release_point="userinfo" + ) + } + + http_info = {"headers": {"authorization": "Bearer {}".format(access_token.value)}} + _req = self.endpoint.parse_request({}, http_info=http_info) + args = self.endpoint.process_request(_req, http_info=http_info) + + assert set(args["response_args"].keys()) == { + "eduperson_scoped_affiliation", + "given_name", + "email_verified", + "email", + "family_name", + "name", + "sub", + } + + def test_allowed_scopes(self): + self.endpoint_context.scopes_handler.allowed_scopes = list(SCOPE2CLAIMS.keys()) + + _auth_req = AUTH_REQ.copy() + _auth_req["scope"] = ["openid", "research_and_scholarship"] + + session_id = self._create_session(_auth_req) + grant = self.session_manager[session_id] + access_token = self._mint_token("access_token", grant, session_id) + + self.endpoint.kwargs["add_claims_by_scope"] = True + self.endpoint.server_get("endpoint_context").claims_interface.add_claims_by_scope = True + grant.claims = { + "userinfo": self.endpoint.server_get("endpoint_context").claims_interface.get_claims( + session_id=session_id, scopes=_auth_req["scope"], claims_release_point="userinfo" + ) + } + + http_info = {"headers": {"authorization": "Bearer {}".format(access_token.value)}} + _req = self.endpoint.parse_request({}, http_info=http_info) + args = self.endpoint.process_request(_req, http_info=http_info) + + assert set(args["response_args"].keys()) == {"sub"} + + def test_allowed_scopes_per_client(self): + self.endpoint_context.cdb["client_1"]["scopes_mapping"] = { + **SCOPE2CLAIMS, + "research_and_scholarship_2": [ + "name", + "given_name", + "family_name", + "email", + "email_verified", + "sub", + "eduperson_scoped_affiliation", + ], + } + self.endpoint_context.cdb["client_1"]["allowed_scopes"] = list(SCOPE2CLAIMS.keys()) + + _auth_req = AUTH_REQ.copy() + _auth_req["scope"] = ["openid", "research_and_scholarship_2"] + + session_id = self._create_session(_auth_req) + grant = self.session_manager[session_id] + access_token = self._mint_token("access_token", grant, session_id) + + self.endpoint.kwargs["add_claims_by_scope"] = True + self.endpoint.server_get("endpoint_context").claims_interface.add_claims_by_scope = True + grant.claims = { + "userinfo": self.endpoint.server_get("endpoint_context").claims_interface.get_claims( + session_id=session_id, scopes=_auth_req["scope"], claims_release_point="userinfo" + ) + } + + http_info = {"headers": {"authorization": "Bearer {}".format(access_token.value)}} + _req = self.endpoint.parse_request({}, http_info=http_info) + args = self.endpoint.process_request(_req, http_info=http_info) + + assert set(args["response_args"].keys()) == {"sub"} + def test_wrong_type_of_token(self): _auth_req = AUTH_REQ.copy() _auth_req["scope"] = ["openid", "research_and_scholarship"] diff --git a/tests/test_50_persistence.py b/tests/test_50_persistence.py index 9052dd27..89f5c2ef 100644 --- a/tests/test_50_persistence.py +++ b/tests/test_50_persistence.py @@ -2,10 +2,10 @@ import os import shutil +import pytest from cryptojwt.jwt import utc_time_sans_frac from oidcmsg.oidc import AccessTokenRequest from oidcmsg.oidc import AuthorizationRequest -import pytest from oidcop import user_info from oidcop.authn_event import create_authn_event @@ -16,6 +16,7 @@ from oidcop.oidc.provider_config import ProviderConfiguration from oidcop.oidc.registration import Registration from oidcop.oidc.token import Token +from oidcop.scopes import SCOPE2CLAIMS from oidcop.server import Server from oidcop.user_authn.authn_context import INTERNETPROTOCOLPASSWORD from oidcop.user_info import UserInfo @@ -136,21 +137,17 @@ def full_path(local_file): } }, "template_dir": "template", - "add_on": { - "custom_scopes": { - "function": "oidcop.oidc.add_on.custom_scopes.add_custom_scopes", - "kwargs": { - "research_and_scholarship": [ - "name", - "given_name", - "family_name", - "email", - "email_verified", - "sub", - "eduperson_scoped_affiliation", - ] - }, - } + "scopes_mapping": { + **SCOPE2CLAIMS, + "research_and_scholarship": [ + "name", + "given_name", + "family_name", + "email", + "email_verified", + "sub", + "eduperson_scoped_affiliation", + ], }, "authz": { "class": AuthzHandling, @@ -401,8 +398,8 @@ def test_custom_scope(self): _auth_req = AUTH_REQ.copy() _auth_req["scope"] = ["openid", "research_and_scholarship"] - session_id = self._create_session(AUTH_REQ, index=2) - grant = self.endpoint[2].server_get("endpoint_context").authz(session_id, AUTH_REQ) + session_id = self._create_session(_auth_req, index=2) + grant = self.endpoint[2].server_get("endpoint_context").authz(session_id, _auth_req) self._dump_restore(2, 1) From fbd1c624e08bbb605a460b13cbaad06404e460f2 Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Mon, 20 Sep 2021 11:28:07 +0300 Subject: [PATCH 2/4] Add get_claims_from_request --- src/oidcop/session/claims.py | 91 +++++++++++++++----- tests/test_01_claims.py | 19 ++-- tests/test_06_session_manager.py | 5 +- tests/test_24_oidc_authorization_endpoint.py | 16 ++-- 4 files changed, 87 insertions(+), 44 deletions(-) diff --git a/src/oidcop/session/claims.py b/src/oidcop/session/claims.py index 3c82fc73..72f65cd7 100755 --- a/src/oidcop/session/claims.py +++ b/src/oidcop/session/claims.py @@ -31,12 +31,13 @@ class ClaimsInterface: def __init__(self, server_get): self.server_get = server_get - def authorization_request_claims(self, - session_id: str, - claims_release_point: Optional[str] = "") -> dict: - _grant = self.server_get("endpoint_context").session_manager.get_grant(session_id) - if _grant.authorization_request and "claims" in _grant.authorization_request: - return _grant.authorization_request["claims"].get(claims_release_point, {}) + def authorization_request_claims( + self, + authorization_request: dict, + claims_release_point: Optional[str] = "", + ) -> dict: + if authorization_request and "claims" in authorization_request: + return authorization_request["claims"].get(claims_release_point, {}) return {} @@ -70,16 +71,13 @@ def _get_module(self, usage, endpoint_context): return module - def get_claims(self, session_id: str, scopes: str, claims_release_point: str) -> dict: - """ - - :param session_id: Session identifier - :param scopes: Scopes - :param claims_release_point: Where to release the claims. One of - "userinfo"/"id_token"/"introspection"/"access_token" - :return: Claims specification as a dictionary. - """ - + def get_claims_from_request( + self, + auth_req: dict, + claims_release_point: str, + scopes: str = None, + client_id: str = None, + ): _context = self.server_get("endpoint_context") # which endpoint module configuration to get the base claims from module = self._get_module(claims_release_point, _context) @@ -89,7 +87,8 @@ def get_claims(self, session_id: str, scopes: str, claims_release_point: str) -> else: return {} - user_id, client_id, grant_id = _context.session_manager.decrypt_session_id(session_id) + if not client_id: + client_id = auth_req.get("client_id") # Can there be per client specification of which claims to use. if module.kwargs.get("enable_claims_per_client"): @@ -112,28 +111,76 @@ def get_claims(self, session_id: str, scopes: str, claims_release_point: str) -> add_claims_by_scope = module.kwargs.get("add_claims_by_scope") if add_claims_by_scope: + if scopes is None: + scopes = auth_req.get("scopes") if scopes: _claims = _context.scopes_handler.scopes_to_claims(scopes, client_id=client_id) claims.update(_claims) # Bring in claims specification from the authorization request # This only goes for ID Token and user info - request_claims = self.authorization_request_claims(session_id=session_id, - claims_release_point=claims_release_point) + request_claims = self.authorization_request_claims( + authorization_request=auth_req, + claims_release_point=claims_release_point + ) # This will add claims that has not be added before and - # set filters on those claims that also appears in one of the sources above + # set filters on those claims that also appears in one of the sources + # above if request_claims: claims.update(request_claims) return claims - def get_claims_all_usage(self, session_id: str, scopes: str) -> dict: + def get_claims(self, session_id: str, scopes: str, claims_release_point: str) -> dict: + """ + + :param session_id: Session identifier + :param scopes: Scopes + :param claims_release_point: Where to release the claims. One of + "userinfo"/"id_token"/"introspection"/"access_token" + :return: Claims specification as a dictionary. + """ + _context = self.server_get("endpoint_context") + session_info = _context.session_manager.get_session_info( + session_id, grant=True + ) + client_id = session_info["client_id"] + grant = session_info["grant"] + + if grant.authorization_request: + auth_req = grant.authorization_request + else: + auth_req = {} + claims = self.get_claims_from_request( + auth_req=auth_req, + claims_release_point=claims_release_point, + scopes=scopes, + client_id=client_id, + ) + + return claims + + def get_claims_all_usage_from_request( + self, auth_req: dict, scopes: str + ) -> dict: _claims = {} for usage in self.claims_release_points: - _claims[usage] = self.get_claims(session_id, scopes, usage) + _claims[usage] = self.get_claims_from_request( + auth_req, usage, scopes + ) return _claims + def get_claims_all_usage(self, session_id: str, scopes: str) -> dict: + grant = self.server_get( + "endpoint_context" + ).session_manager.get_grant(session_id) + if grant.authorization_request: + auth_req = grant.authorization_request + else: + auth_req = {} + return self.get_claims_all_usage_from_request(auth_req, scopes) + def get_user_claims(self, user_id: str, claims_restriction: dict) -> dict: """ diff --git a/tests/test_01_claims.py b/tests/test_01_claims.py index 4b027c45..ca60643c 100644 --- a/tests/test_01_claims.py +++ b/tests/test_01_claims.py @@ -141,33 +141,24 @@ def _create_session(self, auth_req, sub_type="public", sector_identifier=""): ) def test_authorization_request_id_token_claims(self): - session_id = self._create_session(AREQ) - - claims = self.claims_interface.authorization_request_claims(session_id, "id_token") + claims = self.claims_interface.authorization_request_claims(AREQ, "id_token") assert claims == {} def test_authorization_request_id_token_claims_2(self): - session_id = self._create_session(AREQ_2) - claims = self.claims_interface.authorization_request_claims(session_id, "id_token") + claims = self.claims_interface.authorization_request_claims(AREQ_2, "id_token") assert claims assert set(claims.keys()) == {"nickname"} def test_authorization_request_userinfo_claims(self): - session_id = self._create_session(AREQ) - - claims = self.claims_interface.authorization_request_claims(session_id, "userinfo") + claims = self.claims_interface.authorization_request_claims(AREQ, "userinfo") assert claims == {} def test_authorization_request_userinfo_claims_2(self): - session_id = self._create_session(AREQ_2) - - claims = self.claims_interface.authorization_request_claims(session_id, "userinfo") + claims = self.claims_interface.authorization_request_claims(AREQ_2, "userinfo") assert claims == {} def test_authorization_request_userinfo_claims_3(self): - session_id = self._create_session(AREQ_3) - - claims = self.claims_interface.authorization_request_claims(session_id, "userinfo") + claims = self.claims_interface.authorization_request_claims(AREQ_3, "userinfo") assert set(claims.keys()) == {"name", "email", "email_verified"} @pytest.mark.parametrize("usage", ["id_token", "userinfo", "introspection", "token"]) diff --git a/tests/test_06_session_manager.py b/tests/test_06_session_manager.py index 0fae6fa1..eaa189e8 100644 --- a/tests/test_06_session_manager.py +++ b/tests/test_06_session_manager.py @@ -200,11 +200,12 @@ def _mint_token(self, token_class, grant, session_id, based_on=None): ) def test_grant(self): - grant = Grant() + sid = self._create_session(AUTH_REQ) + grant = self.session_manager.get_grant(sid) assert grant.issued_token == [] assert grant.is_active() is True - code = self._mint_token("authorization_code", grant, self.dummy_session_id) + code = self._mint_token("authorization_code", grant, sid) assert isinstance(code, AuthorizationCode) assert code.is_active() assert len(grant.issued_token) == 1 diff --git a/tests/test_24_oidc_authorization_endpoint.py b/tests/test_24_oidc_authorization_endpoint.py index c877a069..a13aa0c2 100755 --- a/tests/test_24_oidc_authorization_endpoint.py +++ b/tests/test_24_oidc_authorization_endpoint.py @@ -825,14 +825,18 @@ def test_verify_response_type(self): @pytest.mark.parametrize("exp_in", [360, "360", 0]) def test_mint_token_exp_at(self, exp_in): - grant = Grant() - grant.usage_rules = {"authorization_code": {"expires_in": exp_in}} - - DUMMY_SESSION_ID = self.session_manager.encrypted_session_id( - "user_id", "client_id", "grant.id" + request = AuthorizationRequest( + client_id="client_1", + response_type=["code"], + redirect_uri="https://example.com/cb", + state="state", + scope="openid", ) + sid = self._create_session(request) + grant = self.session_manager.get_grant(sid) + grant.usage_rules = {"authorization_code": {"expires_in": exp_in}} - code = self.endpoint.mint_token("authorization_code", grant, DUMMY_SESSION_ID) + code = self.endpoint.mint_token("authorization_code", grant, sid) if exp_in in [360, "360"]: assert code.expires_at else: From 76623c003a22eee0f34fa683571cb519bda518c7 Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Mon, 20 Sep 2021 12:57:06 +0300 Subject: [PATCH 3/4] Minor fixes --- src/oidcop/session/claims.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/oidcop/session/claims.py b/src/oidcop/session/claims.py index 72f65cd7..2251b4bf 100755 --- a/src/oidcop/session/claims.py +++ b/src/oidcop/session/claims.py @@ -112,7 +112,7 @@ def get_claims_from_request( if add_claims_by_scope: if scopes is None: - scopes = auth_req.get("scopes") + scopes = auth_req.get("scope") if scopes: _claims = _context.scopes_handler.scopes_to_claims(scopes, client_id=client_id) claims.update(_claims) @@ -162,12 +162,12 @@ def get_claims(self, session_id: str, scopes: str, claims_release_point: str) -> return claims def get_claims_all_usage_from_request( - self, auth_req: dict, scopes: str + self, auth_req: dict, scopes: str = None, client_id: str = None ) -> dict: _claims = {} for usage in self.claims_release_points: _claims[usage] = self.get_claims_from_request( - auth_req, usage, scopes + auth_req, usage, scopes=scopes, client_id=client_id ) return _claims From fcb48905872b7734ef678bb451b89d4c6e1eaec4 Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Mon, 20 Sep 2021 14:47:26 +0300 Subject: [PATCH 4/4] Add allowed_scopes to README --- docs/source/contents/conf.rst | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/source/contents/conf.rst b/docs/source/contents/conf.rst index f8a97b1c..8ccf4c47 100644 --- a/docs/source/contents/conf.rst +++ b/docs/source/contents/conf.rst @@ -54,7 +54,6 @@ sub_funcs Optional. Functions involved in *sub*ject value creation. - scopes_mapping ############## @@ -75,6 +74,12 @@ allowed_scopes A list with the scopes that are allowed to be used (defaults to the keys in scopes_mapping). +advertised_scopes +################# + +A list with the scopes that will be advertised in the well-known endpoint (defaults to allowed_scopes). + + ------ add_on ------