From 692a7f2c4c897a7673b0cc7cf921760820197217 Mon Sep 17 00:00:00 2001 From: Giuseppe Date: Mon, 5 Jul 2021 09:19:23 +0200 Subject: [PATCH 01/17] feat: added pypi gh action CD --- .github/workflows/pypi.yml | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 .github/workflows/pypi.yml diff --git a/.github/workflows/pypi.yml b/.github/workflows/pypi.yml new file mode 100644 index 00000000..4a916526 --- /dev/null +++ b/.github/workflows/pypi.yml @@ -0,0 +1,35 @@ +name: Publish Python distribution to PyPI +on: + release: + types: + - created + +jobs: + build-n-publish: + name: Publish Python distribution to PyPI + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@master + - name: Setup Python 3.8 + uses: actions/setup-python@v1 + with: + python-version: 3.8 + - name: Install pypa/build + run: >- + python -m + pip install + build + --user + - name: Build a binary wheel and a source tarball + run: >- + python -m + build + --sdist + --wheel + --outdir dist/ + . + - name: Publish distribution to PyPI + uses: pypa/gh-action-pypi-publish@master + with: + user: __token__ + password: ${{ secrets.PYPI_API_TOKEN }} From 6dd6b4b133feeda805f331b1946ca6b704b9dec6 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 8 Jul 2021 11:52:56 +0200 Subject: [PATCH 02/17] For debugging purpose nice to know what was put in the ID Token and also what was in a received ID Token. --- example/flask_op/views.py | 1 + requirements.txt | 2 +- src/oidcop/token/id_token.py | 5 +++++ tests/test_05_id_token.py | 17 +++++++++++++++++ 4 files changed, 24 insertions(+), 1 deletion(-) diff --git a/example/flask_op/views.py b/example/flask_op/views.py index f8e485b6..a10d41fc 100644 --- a/example/flask_op/views.py +++ b/example/flask_op/views.py @@ -32,6 +32,7 @@ def _add_cookie(resp, cookie_spec): for k,v in cookie_spec.items() if k not in ('name',)} kwargs["path"] = "/" + kwargs["samesite"] = "Lax" resp.set_cookie(cookie_spec["name"], **kwargs) diff --git a/requirements.txt b/requirements.txt index 9126daf7..d7aa79dd 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,4 @@ -oidcmsg>=1.3.0 +oidcmsg>=1.4.0 pyyaml jinja2>=2.11.3 responses>=0.13.0 diff --git a/src/oidcop/token/id_token.py b/src/oidcop/token/id_token.py index 6d08ed0d..bf044f68 100755 --- a/src/oidcop/token/id_token.py +++ b/src/oidcop/token/id_token.py @@ -134,6 +134,7 @@ def payload( self, session_id, alg="RS256", code=None, access_token=None, extra_claims=None, ): """ + Collect payload for the ID Token. :param session_id: Session identifier :param alg: Which signing algorithm to use for the IdToken @@ -197,6 +198,8 @@ def payload( except KeyError: pass + logger.debug(f"Constructed ID Token payload: {_args}") + return _args def sign_encrypt( @@ -297,6 +300,8 @@ def info(self, token): except JWSException: raise UnknownToken() + logger.debug(f"Received ID Token payload: {_payload}") + if is_expired(_payload["exp"]): raise ToOld("Token has expired") # All the token metadata diff --git a/tests/test_05_id_token.py b/tests/test_05_id_token.py index 8bac2025..38dfbe18 100644 --- a/tests/test_05_id_token.py +++ b/tests/test_05_id_token.py @@ -609,3 +609,20 @@ def test_id_token_acr_claim(self): _jwt = factory(id_token.value) _id_token_content = _jwt.jwt.payload() assert _id_token_content["acr"] == "https://refeds.org/profile/mfa" + + def test_id_token_acr_none(self): + _req = AREQS.copy() + _req["claims"] = {"id_token": {"acr": None}} + + session_id = self._create_session(_req,authn_info="https://refeds.org/profile/mfa") + grant = self.session_manager[session_id] + code = self._mint_code(grant, session_id) + access_token = self._mint_access_token(grant, session_id, code) + + id_token = self._mint_id_token( + grant, session_id, token_ref=code, access_token=access_token.value + ) + + _jwt = factory(id_token.value) + _id_token_content = _jwt.jwt.payload() + assert _id_token_content["acr"] == "https://refeds.org/profile/mfa" From 7ca060d64641955c9ff64a2c07a2cfbfe844a3a2 Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Tue, 27 Jul 2021 13:55:09 +0300 Subject: [PATCH 03/17] Don't create a new client session every time A new client session was created every time create_session was called, this resulted in losing the client subordinates --- src/oidcop/session/manager.py | 9 ++++++--- tests/test_34_oidc_sso.py | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/oidcop/session/manager.py b/src/oidcop/session/manager.py index b8129904..12875293 100644 --- a/src/oidcop/session/manager.py +++ b/src/oidcop/session/manager.py @@ -12,6 +12,7 @@ from oidcop.exception import ConfigurationError from oidcop.token import handler from oidcop.util import Crypt +from oidcop.session.database import NoSuchClientSession from .database import Database from .grant import Grant from .grant import SessionToken @@ -226,9 +227,11 @@ def create_session( if not client_id: client_id = auth_req["client_id"] - client_info = ClientSessionInfo(client_id=client_id) - - self.set([user_id, client_id], client_info) + try: + self.get([user_id, client_id]) + except (NoSuchClientSession, ValueError): + client_info = ClientSessionInfo(client_id=client_id) + self.set([user_id, client_id], client_info) return self.create_grant( auth_req=auth_req, diff --git a/tests/test_34_oidc_sso.py b/tests/test_34_oidc_sso.py index dca54792..92e2d9be 100755 --- a/tests/test_34_oidc_sso.py +++ b/tests/test_34_oidc_sso.py @@ -255,4 +255,4 @@ def test_sso(self): assert len(csi1.subordinate) == 2 assert len(csi2.subordinate) == 1 - assert len(csi3.subordinate) == 1 + assert len(csi3.subordinate) == 2 From 9c0f7db7c4fcac95902d41802961f482a0c43203 Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Wed, 4 Aug 2021 13:06:22 +0300 Subject: [PATCH 04/17] Add pkce per client --- src/oidcop/oidc/add_on/pkce.py | 15 ++++++++------ tests/test_33_oauth2_pkce.py | 37 +++++++++++++++++++++++++++++++++- 2 files changed, 45 insertions(+), 7 deletions(-) diff --git a/src/oidcop/oidc/add_on/pkce.py b/src/oidcop/oidc/add_on/pkce.py index 75b541d6..065e3e75 100644 --- a/src/oidcop/oidc/add_on/pkce.py +++ b/src/oidcop/oidc/add_on/pkce.py @@ -3,11 +3,9 @@ from typing import Dict from cryptojwt.utils import b64e -from oidcmsg.oauth2 import ( - AuthorizationErrorResponse, - RefreshAccessTokenRequest, - TokenExchangeRequest, -) +from oidcmsg.oauth2 import AuthorizationErrorResponse +from oidcmsg.oauth2 import RefreshAccessTokenRequest +from oidcmsg.oauth2 import TokenExchangeRequest from oidcmsg.oidc import TokenErrorResponse from oidcop.endpoint import Endpoint @@ -41,7 +39,12 @@ def post_authn_parse(request, client_id, endpoint_context, **kwargs): :param kwargs: :return: """ - if endpoint_context.args["pkce"]["essential"] and "code_challenge" not in request: + client = endpoint_context.cdb[client_id] + if "pkce_essential" in client: + essential = client["pkce_essential"] + else: + essential = endpoint_context.args["pkce"]["essential"] + if essential and "code_challenge" not in request: return AuthorizationErrorResponse( error="invalid_request", error_description="Missing required code_challenge", ) diff --git a/tests/test_33_oauth2_pkce.py b/tests/test_33_oauth2_pkce.py index 08c24d7d..95fad43c 100644 --- a/tests/test_33_oauth2_pkce.py +++ b/tests/test_33_oauth2_pkce.py @@ -4,7 +4,6 @@ import secrets import string -from oidcop.configure import ASConfiguration import pytest import yaml from oidcmsg.message import Message @@ -15,6 +14,7 @@ from oidcmsg.oidc import TokenErrorResponse import oidcop.oauth2.introspection +from oidcop.configure import ASConfiguration from oidcop.configure import OPConfiguration from oidcop.cookie_handler import CookieHandler from oidcop.endpoint import Endpoint @@ -285,6 +285,41 @@ def test_not_essential(self, conf): assert isinstance(_req, Message) + def test_essential_per_client(self, conf): + conf["add_on"]["pkce"]["kwargs"]["essential"] = False + server = create_server(conf) + authn_endpoint = server.server_get("endpoint", "authorization") + token_endpoint = server.server_get("endpoint", "token") + _authn_req = AUTH_REQ.copy() + endpoint_context = server.server_get("endpoint_context") + endpoint_context.cdb[AUTH_REQ["client_id"]]["pkce_essential"] = True + + _pr_resp = authn_endpoint.parse_request(_authn_req.to_dict()) + + assert isinstance(_pr_resp, AuthorizationErrorResponse) + assert _pr_resp["error"] == "invalid_request" + assert _pr_resp["error_description"] == "Missing required code_challenge" + + def test_not_essential_per_client(self, conf): + conf["add_on"]["pkce"]["kwargs"]["essential"] = True + server = create_server(conf) + authn_endpoint = server.server_get("endpoint", "authorization") + token_endpoint = server.server_get("endpoint", "token") + _authn_req = AUTH_REQ.copy() + endpoint_context = server.server_get("endpoint_context") + endpoint_context.cdb[AUTH_REQ["client_id"]]["pkce_essential"] = False + + _pr_resp = authn_endpoint.parse_request(_authn_req.to_dict()) + resp = authn_endpoint.process_request(_pr_resp) + + assert isinstance(resp["response_args"], AuthorizationResponse) + + _token_request = TOKEN_REQ.copy() + _token_request["code"] = resp["response_args"]["code"] + _req = token_endpoint.parse_request(_token_request) + + assert isinstance(_req, Message) + def test_unknown_code_challenge_method(self): _authn_req = AUTH_REQ.copy() _authn_req["code_challenge"] = "aba" From ccd7234b6ef9568310d6faab9146786304a0923d Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Wed, 4 Aug 2021 17:48:29 +0300 Subject: [PATCH 05/17] Fix bug with refresh id tokens --- src/oidcop/oidc/token.py | 2 +- tests/test_35_oidc_token_endpoint.py | 26 +++++++++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/oidcop/oidc/token.py b/src/oidcop/oidc/token.py index 0a4aeca9..3a46a834 100755 --- a/src/oidcop/oidc/token.py +++ b/src/oidcop/oidc/token.py @@ -246,7 +246,7 @@ def process_request(self, req: Union[Message, dict], **kwargs): if "id_token" in _mints and "openid" in scope: try: _idtoken = self._mint_token( - token_class="refresh_token", + token_class="id_token", grant=_grant, session_id=_session_info["session_id"], client_id=_session_info["client_id"], diff --git a/tests/test_35_oidc_token_endpoint.py b/tests/test_35_oidc_token_endpoint.py index b6bdad7f..5c0e7edc 100755 --- a/tests/test_35_oidc_token_endpoint.py +++ b/tests/test_35_oidc_token_endpoint.py @@ -2,14 +2,15 @@ import json import os +import pytest from cryptojwt import JWT from cryptojwt.key_jar import build_keyjar from oidcmsg.oidc import AccessTokenRequest from oidcmsg.oidc import AuthorizationRequest +from oidcmsg.oidc import AuthorizationResponse 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 @@ -372,6 +373,10 @@ def test_do_refresh_access_token(self): "id_token", "scope", } + AuthorizationResponse().from_jwt( + _resp["response_args"]["id_token"], _cntx.keyjar, sender="" + ) + msg = self.token_endpoint.do_response(request=_req, **_resp) assert isinstance(msg, dict) @@ -420,6 +425,10 @@ def test_do_2nd_refresh_access_token(self): "id_token", "scope", } + AuthorizationResponse().from_jwt( + _2nd_resp["response_args"]["id_token"], _cntx.keyjar, sender="" + ) + msg = self.token_endpoint.do_response(request=_req, **_resp) assert isinstance(msg, dict) @@ -460,6 +469,11 @@ def test_refresh_scopes(self): "id_token", "scope", } + AuthorizationResponse().from_jwt( + _resp["response_args"]["id_token"], + self.endpoint_context.keyjar, + sender="", + ) _token_value = _resp["response_args"]["access_token"] _session_info = self.session_manager.get_session_info_by_token(_token_value) @@ -560,6 +574,11 @@ def test_refresh_more_scopes_2(self): "id_token", "scope", } + AuthorizationResponse().from_jwt( + _resp["response_args"]["id_token"], + self.endpoint_context.keyjar, + sender="", + ) _token_value = _resp["response_args"]["access_token"] _session_info = self.session_manager.get_session_info_by_token(_token_value) @@ -647,6 +666,11 @@ def test_refresh_no_offline_access_scope(self): "id_token", "scope", } + AuthorizationResponse().from_jwt( + _resp["response_args"]["id_token"], + self.endpoint_context.keyjar, + sender="", + ) def test_new_refresh_token(self, conf): self.endpoint_context.cdb["client_1"] = { From fca92e7fb555e1ef9a8e9ee6f8c26ce5156971d2 Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Thu, 5 Aug 2021 12:33:01 +0300 Subject: [PATCH 06/17] Fix id tokens claims --- src/oidcop/token/id_token.py | 76 ++++++++++++++++++---------- tests/test_05_id_token.py | 63 +++++++++++++++-------- tests/test_35_oidc_token_endpoint.py | 45 ++++++++++++++++ 3 files changed, 137 insertions(+), 47 deletions(-) diff --git a/src/oidcop/token/id_token.py b/src/oidcop/token/id_token.py index bf044f68..0cec1c86 100755 --- a/src/oidcop/token/id_token.py +++ b/src/oidcop/token/id_token.py @@ -12,9 +12,10 @@ from oidcop.session.claims import claims_match from oidcop.token import is_expired from oidcop.token.exception import InvalidToken + +from ..util import get_logout_id from . import Token from . import UnknownToken -from ..util import get_logout_id logger = logging.getLogger(__name__) @@ -131,7 +132,13 @@ def __init__( self.provider_info = construct_endpoint_info(self.default_capabilities, **kwargs) def payload( - self, session_id, alg="RS256", code=None, access_token=None, extra_claims=None, + self, + session_id, + alg="RS256", + code=None, + access_token=None, + extra_claims=None, + user_info=None, ): """ Collect payload for the ID Token. @@ -155,16 +162,18 @@ def payload( if _val: _args[attr] = _val - _claims_restriction = grant.claims.get("id_token") - if _claims_restriction == {}: - user_info = None - else: - user_info = _context.claims_interface.get_user_claims( - user_id=session_information["user_id"], claims_restriction=_claims_restriction, - ) - if _claims_restriction and "acr" in _claims_restriction and "acr" in _args: - if claims_match(_args["acr"], _claims_restriction["acr"]) is False: - raise ValueError("Could not match expected 'acr'") + if not user_info: + _claims_restriction = grant.claims.get("id_token") + if _claims_restriction == {}: + user_info = None + else: + user_info = _context.claims_interface.get_user_claims( + user_id=session_information["user_id"], + claims_restriction=_claims_restriction, + ) + if _claims_restriction and "acr" in _claims_restriction and "acr" in _args: + if claims_match(_args["acr"], _claims_restriction["acr"]) is False: + raise ValueError("Could not match expected 'acr'") if user_info: try: @@ -203,15 +212,16 @@ def payload( return _args def sign_encrypt( - self, - session_id, - client_id, - code=None, - access_token=None, - sign=True, - encrypt=False, - lifetime=None, - extra_claims=None, + self, + session_id, + client_id, + code=None, + access_token=None, + sign=True, + encrypt=False, + lifetime=None, + extra_claims=None, + user_info=None, ) -> str: """ Signed and or encrypt a IDToken @@ -240,6 +250,7 @@ def sign_encrypt( code=code, access_token=access_token, extra_claims=extra_claims, + user_info=user_info, ) if lifetime is None: @@ -249,7 +260,15 @@ def sign_encrypt( return _jwt.pack(_payload, recv=client_id) - def __call__(self, session_id: Optional[str] = "", ttype: Optional[str] = "", **kwargs) -> str: + def __call__( + self, + session_id: Optional[str] = "", + ttype: Optional[str] = "", + encrypt=False, + code=None, + access_token=None, + **kwargs, + ) -> str: _context = self.server_get("endpoint_context") user_id, client_id, grant_id = _context.session_manager.decrypt_session_id(session_id) @@ -265,11 +284,16 @@ def __call__(self, session_id: Optional[str] = "", ttype: Optional[str] = "", ** lifetime = self.lifetime - # Weed out stuff that doesn't belong here - kwargs = {k: v for k, v in kwargs.items() if k in ["encrypt", "code", "access_token"]} - id_token = self.sign_encrypt( - session_id, client_id, sign=True, lifetime=lifetime, extra_claims=xargs, **kwargs + session_id, + client_id, + sign=True, + lifetime=lifetime, + extra_claims=xargs, + encrypt=encrypt, + code=code, + access_token=access_token, + user_info=kwargs, ) return id_token diff --git a/tests/test_05_id_token.py b/tests/test_05_id_token.py index 38dfbe18..3afc1899 100644 --- a/tests/test_05_id_token.py +++ b/tests/test_05_id_token.py @@ -235,6 +235,11 @@ def test_id_token_payload_0(self): "nonce", "iat", "exp", + "email", + "email_verified", + "jti", + "scope", + "client_id", "iss", } @@ -252,6 +257,11 @@ def test_id_token_payload_with_code(self): "auth_time", "aud", "exp", + "email", + "email_verified", + "jti", + "scope", + "client_id", "c_hash", "iss", "iat", @@ -277,6 +287,11 @@ def test_id_token_payload_with_access_token(self): "auth_time", "aud", "exp", + "email", + "email_verified", + "jti", + "scope", + "client_id", "iss", "iat", "nonce", @@ -300,6 +315,11 @@ def test_id_token_payload_with_code_and_access_token(self): "auth_time", "aud", "exp", + "email", + "email_verified", + "jti", + "scope", + "client_id", "iss", "iat", "nonce", @@ -308,9 +328,10 @@ def test_id_token_payload_with_code_and_access_token(self): } def test_id_token_payload_with_userinfo(self): - session_id = self._create_session(AREQ) + req = dict(AREQ) + req["claims"] = {"id_token": {"given_name": None}} + session_id = self._create_session(req) grant = self.session_manager[session_id] - grant.claims = {"id_token": {"given_name": None}} id_token = self._mint_id_token(grant, session_id) @@ -320,6 +341,11 @@ def test_id_token_payload_with_userinfo(self): "nonce", "iat", "iss", + "email", + "email_verified", + "jti", + "scope", + "client_id", "given_name", "aud", "exp", @@ -328,9 +354,10 @@ def test_id_token_payload_with_userinfo(self): } def test_id_token_payload_many_0(self): - session_id = self._create_session(AREQ) + req = dict(AREQ) + req["claims"] = {"id_token": {"given_name": None}} + session_id = self._create_session(req) grant = self.session_manager[session_id] - grant.claims = {"id_token": {"given_name": None}} code = self._mint_code(grant, session_id) access_token = self._mint_access_token(grant, session_id, code) @@ -344,6 +371,11 @@ def test_id_token_payload_many_0(self): "nonce", "c_hash", "at_hash", + "email", + "email_verified", + "jti", + "scope", + "client_id", "sub", "auth_time", "given_name", @@ -391,9 +423,10 @@ def test_get_sign_algorithm(self): } def test_available_claims(self): - session_id = self._create_session(AREQ) + req = dict(AREQ) + req["claims"] = {"id_token": {"nickname": {"essential": True}}} + session_id = self._create_session(req) grant = self.session_manager[session_id] - grant.claims = {"id_token": {"nickname": {"essential": True}}} id_token = self._mint_id_token(grant, session_id) @@ -497,11 +530,7 @@ def test_client_claims_scopes(self): grant = self.session_manager[session_id] self.session_manager.token_handler["id_token"].kwargs["add_claims_by_scope"] = True - - _claims = self.endpoint_context.claims_interface.get_claims( - session_id=session_id, scopes=AREQS["scope"], claims_release_point="id_token" - ) - grant.claims = {"id_token": _claims} + grant.scope = AREQS["scope"] id_token = self._mint_id_token(grant, session_id) @@ -519,11 +548,7 @@ def test_client_claims_scopes_and_request_claims_no_match(self): grant = self.session_manager[session_id] self.session_manager.token_handler["id_token"].kwargs["add_claims_by_scope"] = True - - _claims = self.endpoint_context.claims_interface.get_claims( - session_id=session_id, scopes=AREQRC["scope"], claims_release_point="id_token" - ) - grant.claims = {"id_token": _claims} + grant.scope = AREQRC["scope"] id_token = self._mint_id_token(grant, session_id) @@ -546,11 +571,7 @@ def test_client_claims_scopes_and_request_claims_one_match(self): grant = self.session_manager[session_id] self.session_manager.token_handler["id_token"].kwargs["add_claims_by_scope"] = True - - _claims = self.endpoint_context.claims_interface.get_claims( - session_id=session_id, scopes=_req["scope"], claims_release_point="id_token" - ) - grant.claims = {"id_token": _claims} + grant.scope = _req["scope"] id_token = self._mint_id_token(grant, session_id) diff --git a/tests/test_35_oidc_token_endpoint.py b/tests/test_35_oidc_token_endpoint.py index 5c0e7edc..b1717474 100755 --- a/tests/test_35_oidc_token_endpoint.py +++ b/tests/test_35_oidc_token_endpoint.py @@ -197,6 +197,7 @@ def create_endpoint(self, conf): "response_types": ["code", "token", "code id_token", "id_token"], } endpoint_context.keyjar.import_jwks(CLIENT_KEYJAR.export_jwks(), "client_1") + endpoint_context.userinfo = USERINFO self.session_manager = endpoint_context.session_manager self.token_endpoint = server.server_get("endpoint", "token") self.user_id = "diana" @@ -591,6 +592,50 @@ def test_refresh_more_scopes_2(self): assert at.scope == rt.scope == _request["scope"] + def test_refresh_less_scopes(self): + areq = AUTH_REQ.copy() + areq["scope"] = ["openid", "offline_access", "email"] + + self.session_manager.token_handler.handler["id_token"].kwargs["add_claims_by_scope"] = True + session_id = self._create_session(areq) + grant = self.endpoint_context.authz(session_id, areq) + code = self._mint_code(grant, areq["client_id"]) + + _token_request = TOKEN_REQ_DICT.copy() + _token_request["code"] = code.value + _req = self.token_endpoint.parse_request(_token_request) + _resp = self.token_endpoint.process_request(request=_req) + idtoken = AuthorizationResponse().from_jwt( + _resp["response_args"]["id_token"], + self.endpoint_context.keyjar, + sender="", + ) + + assert "email" in idtoken + + _request = REFRESH_TOKEN_REQ.copy() + _request["refresh_token"] = _resp["response_args"]["refresh_token"] + _request["scope"] = ["openid", "offline_access"] + + _token_value = _resp["response_args"]["refresh_token"] + _session_info = self.session_manager.get_session_info_by_token(_token_value) + _token = self.session_manager.find_token(_session_info["session_id"], _token_value) + _token.usage_rules["supports_minting"] = [ + "access_token", + "refresh_token", + "id_token", + ] + + _req = self.token_endpoint.parse_request(_request.to_json()) + _resp = self.token_endpoint.process_request(request=_req) + idtoken = AuthorizationResponse().from_jwt( + _resp["response_args"]["id_token"], + self.endpoint_context.keyjar, + sender="", + ) + + assert "email" not in idtoken + def test_refresh_no_openid_scope(self): areq = AUTH_REQ.copy() areq["scope"] = ["openid", "offline_access"] From bf8a63bbd78dffcd77e2a4428ffb88d8b8fa9eb4 Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Thu, 5 Aug 2021 14:39:55 +0300 Subject: [PATCH 07/17] Add documentation --- docs/source/contents/conf.rst | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/docs/source/contents/conf.rst b/docs/source/contents/conf.rst index 34493edc..53287fa8 100644 --- a/docs/source/contents/conf.rst +++ b/docs/source/contents/conf.rst @@ -84,6 +84,27 @@ An example:: } } +The provided add-ons can be seen in the following sections. + +pkce +#### + +The pkce add on is activated using the ``oidcop.oidc.add_on.pkce.add_pkce_support`` +function. The possible configuration options can be found below. + +essential +--------- + +Whether pkce is mandatory, authentication requests without a ``code_challenge`` +will fail if this is True. This option can be overridden per client by defining +``pkce_essential`` in the client metadata. + +code_challenge_method +--------------------- + +The allowed code_challenge methods. The supported code challenge methods are: +``plain, S256, S384, S512`` + -------------- authentication -------------- From ee87c96ea941db402e3c320100caed6f444f3fd2 Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Wed, 4 Aug 2021 17:48:29 +0300 Subject: [PATCH 08/17] Fix bugs with refresh tokens --- src/oidcop/oauth2/token.py | 4 ++-- src/oidcop/oidc/token.py | 4 ++-- tests/test_24_oauth2_token_endpoint.py | 6 +++--- tests/test_35_oidc_token_endpoint.py | 7 +++++-- 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/oidcop/oauth2/token.py b/src/oidcop/oauth2/token.py index 87724793..e84ea887 100755 --- a/src/oidcop/oauth2/token.py +++ b/src/oidcop/oauth2/token.py @@ -253,7 +253,7 @@ def process_request(self, req: Union[Message, dict], **kwargs): _resp = { "access_token": access_token.value, "token_type": access_token.token_type, - "scope": _grant.scope, + "scope": scope, } if access_token.expires_at: @@ -318,7 +318,7 @@ def post_parse_request( if "scope" in request: req_scopes = set(request["scope"]) scopes = set(grant.find_scope(token.based_on)) - if scopes < req_scopes: + if not req_scopes.issubset(scopes): return self.error_cls( error="invalid_request", error_description="Invalid refresh scopes", diff --git a/src/oidcop/oidc/token.py b/src/oidcop/oidc/token.py index 3a46a834..a88f45c8 100755 --- a/src/oidcop/oidc/token.py +++ b/src/oidcop/oidc/token.py @@ -218,7 +218,7 @@ def process_request(self, req: Union[Message, dict], **kwargs): _resp = { "access_token": access_token.value, "token_type": token_type, - "scope": _grant.scope, + "scope": scope, } if access_token.expires_at: @@ -307,7 +307,7 @@ def post_parse_request( if "scope" in request: req_scopes = set(request["scope"]) scopes = set(grant.find_scope(token.based_on)) - if scopes < req_scopes: + if not req_scopes.issubset(scopes): return self.error_cls( error="invalid_request", error_description="Invalid refresh scopes", diff --git a/tests/test_24_oauth2_token_endpoint.py b/tests/test_24_oauth2_token_endpoint.py index 6fe115da..e3627be5 100644 --- a/tests/test_24_oauth2_token_endpoint.py +++ b/tests/test_24_oauth2_token_endpoint.py @@ -458,7 +458,7 @@ def test_refresh_scopes(self): _session_info["session_id"], _resp["response_args"]["refresh_token"] ) - assert at.scope == rt.scope == _request["scope"] + assert at.scope == rt.scope == _request["scope"] == _resp["response_args"]["scope"] def test_refresh_more_scopes(self): areq = AUTH_REQ.copy() @@ -475,7 +475,7 @@ def test_refresh_more_scopes(self): _request = REFRESH_TOKEN_REQ.copy() _request["refresh_token"] = _resp["response_args"]["refresh_token"] - _request["scope"] = ["email", "profile"] + _request["scope"] = ["ema"] _req = self.token_endpoint.parse_request(_request.to_json()) assert isinstance(_req, TokenErrorResponse) @@ -534,7 +534,7 @@ def test_refresh_more_scopes_2(self): _session_info["session_id"], _resp["response_args"]["refresh_token"] ) - assert at.scope == rt.scope == _request["scope"] + assert at.scope == rt.scope == _request["scope"] == _resp["response_args"]["scope"] def test_do_refresh_access_token_not_allowed(self): areq = AUTH_REQ.copy() diff --git a/tests/test_35_oidc_token_endpoint.py b/tests/test_35_oidc_token_endpoint.py index b1717474..908966bb 100755 --- a/tests/test_35_oidc_token_endpoint.py +++ b/tests/test_35_oidc_token_endpoint.py @@ -485,7 +485,7 @@ def test_refresh_scopes(self): _session_info["session_id"], _resp["response_args"]["refresh_token"] ) - assert at.scope == rt.scope == _request["scope"] + assert at.scope == rt.scope == _request["scope"] == _resp["response_args"]["scope"] def test_refresh_more_scopes(self): areq = AUTH_REQ.copy() @@ -590,7 +590,7 @@ def test_refresh_more_scopes_2(self): _session_info["session_id"], _resp["response_args"]["refresh_token"] ) - assert at.scope == rt.scope == _request["scope"] + assert at.scope == rt.scope == _request["scope"] == _resp["response_args"]["scope"] def test_refresh_less_scopes(self): areq = AUTH_REQ.copy() @@ -635,6 +635,7 @@ def test_refresh_less_scopes(self): ) assert "email" not in idtoken + assert _resp["response_args"]["scope"] == ["openid", "offline_access"] def test_refresh_no_openid_scope(self): areq = AUTH_REQ.copy() @@ -673,6 +674,7 @@ def test_refresh_no_openid_scope(self): "refresh_token", "scope", } + assert _resp["response_args"]["scope"] == ["offline_access"] def test_refresh_no_offline_access_scope(self): areq = AUTH_REQ.copy() @@ -716,6 +718,7 @@ def test_refresh_no_offline_access_scope(self): self.endpoint_context.keyjar, sender="", ) + assert _resp["response_args"]["scope"] == ["openid"] def test_new_refresh_token(self, conf): self.endpoint_context.cdb["client_1"] = { From 9429d57f1c0965874c5dd2984debb0da16192f82 Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Mon, 16 Aug 2021 13:49:11 +0300 Subject: [PATCH 09/17] Handle ToOld token exception --- src/oidcop/client_authn.py | 3 +++ tests/test_26_oidc_userinfo_endpoint.py | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/src/oidcop/client_authn.py b/src/oidcop/client_authn.py index f02868d9..7678419a 100755 --- a/src/oidcop/client_authn.py +++ b/src/oidcop/client_authn.py @@ -22,6 +22,7 @@ from oidcop.exception import InvalidClient from oidcop.exception import MultipleUsage from oidcop.exception import NotForMe +from oidcop.exception import ToOld from oidcop.exception import UnknownClient from oidcop.util import importer @@ -409,6 +410,8 @@ def verify_client( try: # get_client_id_from_token is a callback... Do not abuse for code readability. auth_info["client_id"] = get_client_id_from_token(endpoint_context, _token, request) + except ToOld: + raise ValueError("Expired token") except KeyError: raise ValueError("Unknown token") diff --git a/tests/test_26_oidc_userinfo_endpoint.py b/tests/test_26_oidc_userinfo_endpoint.py index e61262a8..41ebc65c 100755 --- a/tests/test_26_oidc_userinfo_endpoint.py +++ b/tests/test_26_oidc_userinfo_endpoint.py @@ -381,6 +381,27 @@ def test_invalid_token(self): assert isinstance(args, ResponseMessage) assert args["error_description"] == "Invalid Token" + def test_expired_token(self, monkeypatch): + _auth_req = AUTH_REQ.copy() + _auth_req["scope"] = ["openid", "research_and_scholarship"] + + session_id = self._create_session(_auth_req) + grant = self.session_manager[session_id] + access_token = self._mint_token("access_token", grant, session_id) + + http_info = {"headers": {"authorization": "Bearer {}".format(access_token.value)}} + + def mock(): + return time_sans_frac() + access_token.expires_at + 1 + + monkeypatch.setattr("oidcop.token.time_sans_frac", mock) + + _req = self.endpoint.parse_request({}, http_info=http_info) + + assert _req.to_dict() == { + "error": "invalid_token", "error_description": "Expired token" + } + def test_userinfo_claims(self): _acr = "https://refeds.org/profile/mfa" _auth_req = AUTH_REQ.copy() From 230d04ecd68c81ac25b34c928cf22a24f7c66098 Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Mon, 16 Aug 2021 14:17:38 +0300 Subject: [PATCH 10/17] Don't set default value to pkce essential --- src/oidcop/oidc/add_on/pkce.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/oidcop/oidc/add_on/pkce.py b/src/oidcop/oidc/add_on/pkce.py index 065e3e75..6825c38c 100644 --- a/src/oidcop/oidc/add_on/pkce.py +++ b/src/oidcop/oidc/add_on/pkce.py @@ -43,7 +43,9 @@ def post_authn_parse(request, client_id, endpoint_context, **kwargs): if "pkce_essential" in client: essential = client["pkce_essential"] else: - essential = endpoint_context.args["pkce"]["essential"] + essential = endpoint_context.args["pkce"].get( + "essential", False + ) if essential and "code_challenge" not in request: return AuthorizationErrorResponse( error="invalid_request", error_description="Missing required code_challenge", @@ -134,9 +136,6 @@ def add_pkce_support(endpoint: Dict[str, Endpoint], **kwargs): authn_endpoint.post_parse_request.append(post_authn_parse) token_endpoint.post_parse_request.append(post_token_parse) - if "essential" not in kwargs: - kwargs["essential"] = False - code_challenge_methods = kwargs.get("code_challenge_methods", CC_METHOD.keys()) kwargs["code_challenge_methods"] = {} From ac9d69f70fbb550767edfbbcfd5e326a6250af1c Mon Sep 17 00:00:00 2001 From: Giuseppe Date: Thu, 19 Aug 2021 10:52:11 +0200 Subject: [PATCH 11/17] chore: Exception when configuration lacks of userinfo definition --- src/oidcop/session/claims.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/oidcop/session/claims.py b/src/oidcop/session/claims.py index edcb1076..d9560343 100755 --- a/src/oidcop/session/claims.py +++ b/src/oidcop/session/claims.py @@ -5,6 +5,7 @@ from oidcmsg.oidc import OpenIDSchema from oidcop.exception import ServiceError +from oidcop.exception import ImproperlyConfigured from oidcop.scopes import convert_scopes2claims logger = logging.getLogger(__name__) @@ -129,7 +130,12 @@ def get_user_claims(self, user_id: str, claims_restriction: dict) -> dict: """ if claims_restriction: # Get all possible claims - user_info = self.server_get("endpoint_context").userinfo(user_id, client_id=None) + meth = self.server_get("endpoint_context").userinfo + if not meth: + raise ImproperlyConfigured( + "userinfo MUST be defined in the configuration" + ) + user_info = meth(user_id, client_id=None) # Filter out the claims that can be returned return { k: user_info.get(k) From 600c03ab86ec5c16def7073efcc7e72dd98c4abb Mon Sep 17 00:00:00 2001 From: Giuseppe Date: Thu, 19 Aug 2021 16:25:00 +0200 Subject: [PATCH 12/17] test: missing userinfo in configuration --- src/oidcop/session/claims.py | 12 ++++++------ tests/test_26_oidc_userinfo_endpoint.py | 12 ++++++++++++ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/oidcop/session/claims.py b/src/oidcop/session/claims.py index d9560343..13af6399 100755 --- a/src/oidcop/session/claims.py +++ b/src/oidcop/session/claims.py @@ -128,13 +128,13 @@ def get_user_claims(self, user_id: str, claims_restriction: dict) -> dict: :param claims_restriction: Specifies the upper limit of which claims can be returned :return: """ + # Get all possible claims + meth = self.server_get("endpoint_context").userinfo + if not meth: + raise ImproperlyConfigured( + "userinfo MUST be defined in the configuration" + ) if claims_restriction: - # Get all possible claims - meth = self.server_get("endpoint_context").userinfo - if not meth: - raise ImproperlyConfigured( - "userinfo MUST be defined in the configuration" - ) user_info = meth(user_id, client_id=None) # Filter out the claims that can be returned return { diff --git a/tests/test_26_oidc_userinfo_endpoint.py b/tests/test_26_oidc_userinfo_endpoint.py index 41ebc65c..cc7c082a 100755 --- a/tests/test_26_oidc_userinfo_endpoint.py +++ b/tests/test_26_oidc_userinfo_endpoint.py @@ -11,6 +11,7 @@ from oidcop.authn_event import create_authn_event from oidcop.configure import OPConfiguration from oidcop.cookie_handler import CookieHandler +from oidcop.exception import ImproperlyConfigured from oidcop.oidc import userinfo from oidcop.oidc.authorization import Authorization from oidcop.oidc.provider_config import ProviderConfiguration @@ -439,3 +440,14 @@ def test_userinfo_claims_acr_none(self): res = self.endpoint.do_response(request=_req, **args) _response = json.loads(res["response"]) assert _response["acr"] == _acr + + def test_process_request_absent_userinfo_conf(self): + # consider to have a configuration without userinfo defined in + ec = self.endpoint.server_get('endpoint_context') + ec.userinfo = None + + session_id = self._create_session(AUTH_REQ) + grant = self.session_manager[session_id] + + with pytest.raises(ImproperlyConfigured): + code = self._mint_code(grant, session_id) From d0eb303b81229fae9595734eb38a1677fb86c3ad Mon Sep 17 00:00:00 2001 From: Giuseppe Date: Thu, 19 Aug 2021 16:40:46 +0200 Subject: [PATCH 13/17] chore: fixed userinfo configuration in unit tests --- src/oidcop/session/claims.py | 2 +- tests/__init__.py | 7 +++++++ tests/test_01_grant.py | 6 ++++++ tests/test_06_session_manager.py | 5 +++++ tests/test_08_session_life.py | 5 +++++ tests/test_33_oauth2_pkce.py | 5 +++++ tests/test_34_oidc_sso.py | 9 +++++++-- 7 files changed, 36 insertions(+), 3 deletions(-) create mode 100644 tests/__init__.py diff --git a/src/oidcop/session/claims.py b/src/oidcop/session/claims.py index 13af6399..2151e76d 100755 --- a/src/oidcop/session/claims.py +++ b/src/oidcop/session/claims.py @@ -128,13 +128,13 @@ def get_user_claims(self, user_id: str, claims_restriction: dict) -> dict: :param claims_restriction: Specifies the upper limit of which claims can be returned :return: """ - # Get all possible claims meth = self.server_get("endpoint_context").userinfo if not meth: raise ImproperlyConfigured( "userinfo MUST be defined in the configuration" ) if claims_restriction: + # Get all possible claims user_info = meth(user_id, client_id=None) # Filter out the claims that can be returned return { diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 00000000..c0125ddb --- /dev/null +++ b/tests/__init__.py @@ -0,0 +1,7 @@ +import os + +BASEDIR = os.path.abspath(os.path.dirname(__file__)) + + +def full_path(local_file): + return os.path.join(BASEDIR, local_file) diff --git a/tests/test_01_grant.py b/tests/test_01_grant.py index ddccb7bf..67199372 100644 --- a/tests/test_01_grant.py +++ b/tests/test_01_grant.py @@ -2,6 +2,7 @@ from cryptojwt.key_jar import build_keyjar from oidcmsg.oidc import AuthorizationRequest +from . import full_path from oidcop.authn_event import create_authn_event from oidcop.server import Server from oidcop.session.grant import TOKEN_MAP @@ -20,6 +21,7 @@ KEYJAR = build_keyjar(KEYDEFS) + conf = { "issuer": "https://example.com/", "template_dir": "template", @@ -40,6 +42,10 @@ } }, "claims_interface": {"class": "oidcop.session.claims.ClaimsInterface", "kwargs": {}}, + "userinfo": { + "class": "oidcop.user_info.UserInfo", + "kwargs": {"db_file": full_path("users.json")}, + }, } USER_ID = "diana" diff --git a/tests/test_06_session_manager.py b/tests/test_06_session_manager.py index acc85371..0fae6fa1 100644 --- a/tests/test_06_session_manager.py +++ b/tests/test_06_session_manager.py @@ -2,6 +2,7 @@ from oidcmsg.time_util import time_sans_frac import pytest +from . import full_path from oidcop.authn_event import AuthnEvent from oidcop.authn_event import create_authn_event from oidcop.authz import AuthzHandling @@ -74,6 +75,10 @@ def create_session_manager(self): }, "template_dir": "template", "claims_interface": {"class": "oidcop.session.claims.ClaimsInterface", "kwargs": {}}, + "userinfo": { + "class": "oidcop.user_info.UserInfo", + "kwargs": {"db_file": full_path("users.json")}, + }, } server = Server(conf) self.server = server diff --git a/tests/test_08_session_life.py b/tests/test_08_session_life.py index 4c962469..bb29fdff 100644 --- a/tests/test_08_session_life.py +++ b/tests/test_08_session_life.py @@ -8,6 +8,7 @@ from oidcmsg.oidc import RefreshAccessTokenRequest from oidcmsg.time_util import time_sans_frac +from . import full_path from oidcop import user_info from oidcop.authn_event import create_authn_event from oidcop.client_authn import verify_client @@ -50,6 +51,10 @@ def setup_token_handler(self): "token_endpoint": {"path": "{}/token", "class": Token, "kwargs": {}}, }, "template_dir": "template", + "userinfo": { + "class": "oidcop.user_info.UserInfo", + "kwargs": {"db_file": full_path("users.json")}, + }, } server = Server(OPConfiguration(conf=conf, base_path=BASEDIR), cwd=BASEDIR) diff --git a/tests/test_33_oauth2_pkce.py b/tests/test_33_oauth2_pkce.py index 08c24d7d..b2d2fb47 100644 --- a/tests/test_33_oauth2_pkce.py +++ b/tests/test_33_oauth2_pkce.py @@ -4,6 +4,7 @@ import secrets import string +from . import full_path from oidcop.configure import ASConfiguration import pytest import yaml @@ -161,6 +162,10 @@ def conf(): }, }, }, + "userinfo": { + "class": "oidcop.user_info.UserInfo", + "kwargs": {"db_file": full_path("users.json")}, + }, } diff --git a/tests/test_34_oidc_sso.py b/tests/test_34_oidc_sso.py index 92e2d9be..fedf301d 100755 --- a/tests/test_34_oidc_sso.py +++ b/tests/test_34_oidc_sso.py @@ -2,6 +2,7 @@ import json import os +from . import full_path from oidcop.configure import OPConfiguration import pytest import yaml @@ -89,11 +90,11 @@ def full_path(local_file): client_1: client_secret: hemligtkodord, client_id: client_1, - "redirect_uris": + "redirect_uris": - ['https://example.com/cb', ''] "client_salt": "salted" 'token_endpoint_auth_method': 'client_secret_post' - 'response_types': + 'response_types': - 'code' - 'token' - 'code id_token' @@ -158,6 +159,10 @@ def create_endpoint_context(self): }, }, "template_dir": "template", + "userinfo": { + "class": "oidcop.user_info.UserInfo", + "kwargs": {"db_file": full_path("users.json")}, + }, } server = Server(OPConfiguration(conf=conf, base_path=BASEDIR), cwd=BASEDIR) From 4c4e1e06fb71c1a312105f7353943369e7df5443 Mon Sep 17 00:00:00 2001 From: Nikos Sklikas Date: Mon, 16 Aug 2021 13:33:15 +0300 Subject: [PATCH 14/17] Fix JWT access token lifetime --- src/oidcop/session/grant.py | 4 +++- src/oidcop/token/id_token.py | 1 + src/oidcop/token/jwt_token.py | 18 +++++++++++++++--- tests/test_35_oidc_token_endpoint.py | 20 ++++++++++++++++++++ 4 files changed, 39 insertions(+), 4 deletions(-) diff --git a/src/oidcop/session/grant.py b/src/oidcop/session/grant.py index 29a83dd7..25b66692 100644 --- a/src/oidcop/session/grant.py +++ b/src/oidcop/session/grant.py @@ -316,7 +316,9 @@ def mint_token( scope=scope, extra_payload=handler_args, ) - item.value = token_handler(session_id=session_id, **token_payload) + item.value = token_handler( + session_id=session_id, usage_rules=usage_rules, **token_payload + ) else: raise ValueError("Can not mint that kind of token") diff --git a/src/oidcop/token/id_token.py b/src/oidcop/token/id_token.py index 0cec1c86..5f8bc1f4 100755 --- a/src/oidcop/token/id_token.py +++ b/src/oidcop/token/id_token.py @@ -267,6 +267,7 @@ def __call__( encrypt=False, code=None, access_token=None, + usage_rules: Optional[dict] = None, **kwargs, ) -> str: _context = self.server_get("endpoint_context") diff --git a/src/oidcop/token/jwt_token.py b/src/oidcop/token/jwt_token.py index 1329f64d..d19024c9 100644 --- a/src/oidcop/token/jwt_token.py +++ b/src/oidcop/token/jwt_token.py @@ -48,8 +48,13 @@ def load_custom_claims(self, payload: dict = None): # inherit me and do your things here return payload - def __call__(self, session_id: Optional[str] = "", token_class: Optional[str] = "", - **payload) -> str: + def __call__( + self, + session_id: Optional[str] = "", + token_class: Optional[str] = "", + usage_rules: Optional[dict] = None, + **payload + ) -> str: """ Return a token. @@ -70,8 +75,15 @@ def __call__(self, session_id: Optional[str] = "", token_class: Optional[str] = # payload.update(kwargs) _context = self.server_get("endpoint_context") + if usage_rules and "expires_in" in usage_rules: + lifetime = usage_rules.get("expires_in") + else: + lifetime = self.lifetime signer = JWT( - key_jar=_context.keyjar, iss=self.issuer, lifetime=self.lifetime, sign_alg=self.alg, + key_jar=_context.keyjar, + iss=self.issuer, + lifetime=lifetime, + sign_alg=self.alg, ) return signer.pack(payload) diff --git a/tests/test_35_oidc_token_endpoint.py b/tests/test_35_oidc_token_endpoint.py index 908966bb..998fbfab 100755 --- a/tests/test_35_oidc_token_endpoint.py +++ b/tests/test_35_oidc_token_endpoint.py @@ -816,6 +816,26 @@ def test_configure_grant_types(self): assert "access_token" in self.token_endpoint.helper assert "refresh_token" not in self.token_endpoint.helper + def test_access_token_lifetime(self): + lifetime = 100 + session_id = self._create_session(AUTH_REQ) + grant = self.session_manager[session_id] + code = self._mint_code(grant, AUTH_REQ["client_id"]) + grant.usage_rules["access_token"] = {"expires_in": lifetime} + + _token_request = TOKEN_REQ_DICT.copy() + _token_request["code"] = code.value + _req = self.token_endpoint.parse_request(_token_request) + _resp = self.token_endpoint.process_request(request=_req) + + access_token = AccessTokenRequest().from_jwt( + _resp["response_args"]["access_token"], + self.endpoint_context.keyjar, + sender="", + ) + + assert access_token["exp"] - access_token["iat"] == lifetime + class TestOldTokens(object): @pytest.fixture(autouse=True) From a84eadf1a3509c1c3061fe3cfd11bdb3cf1f5d01 Mon Sep 17 00:00:00 2001 From: Kostis Triantafyllakis Date: Tue, 27 Jul 2021 13:49:58 +0300 Subject: [PATCH 15/17] Introduce add_claims_by_scope per client configuration --- src/oidcop/session/claims.py | 21 +++++++++++++++++--- tests/test_01_claims.py | 23 ++++++++++++---------- tests/test_05_id_token.py | 28 ++++++++++++++++++++++++++- tests/test_05_jwt_token.py | 6 +++++- tests/test_07_userinfo.py | 9 +++++++-- tests/test_31_oauth2_introspection.py | 7 ++++++- 6 files changed, 76 insertions(+), 18 deletions(-) diff --git a/src/oidcop/session/claims.py b/src/oidcop/session/claims.py index edcb1076..e7670280 100755 --- a/src/oidcop/session/claims.py +++ b/src/oidcop/session/claims.py @@ -41,7 +41,11 @@ def authorization_request_claims(self, def _get_client_claims(self, client_id, usage): client_info = self.server_get("endpoint_context").cdb.get(client_id, {}) - client_claims = client_info.get("{}_claims".format(usage), {}) + client_claims = ( + client_info.get("add_claims", {}) + .get("always", {}) + .get(usage, {}) + ) if isinstance(client_claims, list): client_claims = {k: None for k in client_claims} return client_claims @@ -94,8 +98,19 @@ def get_claims(self, session_id: str, scopes: str, claims_release_point: str) -> claims.update(base_claims) - # Scopes can in some cases equate to set of claims, is that used here ? - if module.kwargs.get("add_claims_by_scope"): + # If specific client configuration exists overwrite add_claims_by_scope + if client_id in _context.cdb: + add_claims_by_scope = ( + _context.cdb[client_id].get("add_claims", {}) + .get("by_scope", {}) + .get(claims_release_point, {}) + ) + if isinstance(add_claims_by_scope, dict) and not add_claims_by_scope: + add_claims_by_scope = module.kwargs.get("add_claims_by_scope") + else: + add_claims_by_scope = module.kwargs.get("add_claims_by_scope") + + if add_claims_by_scope: if scopes: _scopes = _context.scopes_handler.filter_scopes(client_id, _context, scopes) diff --git a/tests/test_01_claims.py b/tests/test_01_claims.py index fa5f8dae..4b027c45 100644 --- a/tests/test_01_claims.py +++ b/tests/test_01_claims.py @@ -114,6 +114,9 @@ def create_idtoken(self): "client_salt": "salted", "token_endpoint_auth_method": "client_secret_post", "response_types": ["code", "token", "code id_token", "id_token"], + "add_claims": { + "always": {}, + }, } server.endpoint_context.keyjar.add_symmetric( "client_1", "hemligtochintekort", ["sig", "enc"] @@ -173,17 +176,17 @@ def test_get_client_claims_0(self, usage): assert claims == {} def test_get_client_claims_id_token_1(self): - self.endpoint_context.cdb["client_1"]["id_token_claims"] = ["name", "email"] + self.endpoint_context.cdb["client_1"]["add_claims"]["always"]["id_token"] = ["name", "email"] claims = self.claims_interface._get_client_claims("client_1", "id_token") assert set(claims.keys()) == {"name", "email"} def test_get_client_claims_userinfo_1(self): - self.endpoint_context.cdb["client_1"]["userinfo_claims"] = ["email", "address"] + self.endpoint_context.cdb["client_1"]["add_claims"]["always"]["userinfo"] = ["email", "address"] claims = self.claims_interface._get_client_claims("client_1", "userinfo") assert set(claims.keys()) == {"address", "email"} def test_get_client_claims_introspection_1(self): - self.endpoint_context.cdb["client_1"]["introspection_claims"] = ["email"] + self.endpoint_context.cdb["client_1"]["add_claims"]["always"]["introspection"] = ["email"] claims = self.claims_interface._get_client_claims("client_1", "introspection") assert set(claims.keys()) == {"email"} @@ -207,7 +210,7 @@ def test_get_claims_id_token_2(self): "base_claims": {"email": None, "email_verified": None}, "enable_claims_per_client": True, } - self.endpoint_context.cdb["client_1"]["id_token_claims"] = ["name", "email"] + self.endpoint_context.cdb["client_1"]["add_claims"]["always"]["id_token"] = ["name", "email"] claims = self.claims_interface.get_claims(session_id, [], "id_token") assert set(claims.keys()) == {"name", "email", "email_verified"} @@ -219,7 +222,7 @@ def test_get_claims_id_token_3(self): "enable_claims_per_client": True, "add_claims_by_scope": True, } - self.endpoint_context.cdb["client_1"]["id_token_claims"] = ["name", "email"] + self.endpoint_context.cdb["client_1"]["add_claims"]["always"]["id_token"] = ["name", "email"] claims = self.claims_interface.get_claims(session_id, ["openid", "address"], "id_token") assert set(claims.keys()) == { @@ -238,7 +241,7 @@ def test_get_claims_userinfo_3(self): "enable_claims_per_client": True, "add_claims_by_scope": True, } - self.endpoint_context.cdb["client_1"]["userinfo_claims"] = ["name", "email"] + self.endpoint_context.cdb["client_1"]["add_claims"]["always"]["userinfo"] = ["name", "email"] claims = self.claims_interface.get_claims(session_id, ["openid", "address"], "userinfo") assert set(claims.keys()) == { @@ -256,7 +259,7 @@ def test_get_claims_introspection_3(self): "enable_claims_per_client": True, "add_claims_by_scope": True, } - self.endpoint_context.cdb["client_1"]["introspection_claims"] = [ + self.endpoint_context.cdb["client_1"]["add_claims"]["always"]["introspection"] = [ "name", "email", ] @@ -280,7 +283,7 @@ def test_get_claims_access_token_3(self): "enable_claims_per_client": True, "add_claims_by_scope": True, } - self.endpoint_context.cdb["client_1"]["access_token_claims"] = ["name", "email"] + self.endpoint_context.cdb["client_1"]["add_claims"]["always"]["access_token"] = ["name", "email"] session_id = self._create_session(AREQ) claims = self.claims_interface.get_claims(session_id, ["openid", "address"], "access_token") @@ -320,7 +323,7 @@ def test_get_claims_all_usage_2(self): self.server.server_get("endpoint", "userinfo").kwargs = { "enable_claims_per_client": True, } - self.endpoint_context.cdb["client_1"]["userinfo_claims"] = ["name", "email"] + self.endpoint_context.cdb["client_1"]["add_claims"]["always"]["userinfo"] = ["name", "email"] self.server.server_get("endpoint", "introspection").kwargs = {"add_claims_by_scope": True} @@ -349,7 +352,7 @@ def test_get_user_claims(self): self.server.server_get("endpoint", "userinfo").kwargs = { "enable_claims_per_client": True, } - self.endpoint_context.cdb["client_1"]["userinfo_claims"] = ["name", "email"] + self.endpoint_context.cdb["client_1"]["add_claims"]["always"]["userinfo"] = ["name", "email"] self.server.server_get("endpoint", "introspection").kwargs = {"add_claims_by_scope": True} diff --git a/tests/test_05_id_token.py b/tests/test_05_id_token.py index 3afc1899..c0bffdc4 100644 --- a/tests/test_05_id_token.py +++ b/tests/test_05_id_token.py @@ -170,6 +170,10 @@ def create_session_manager(self): "client_salt": "salted", "token_endpoint_auth_method": "client_secret_post", "response_types": ["code", "token", "code id_token", "id_token"], + "add_claims": { + "always": {}, + "by_scope": {}, + }, } self.endpoint_context.keyjar.add_symmetric("client_1", "hemligtochintekort", ["sig", "enc"]) self.session_manager = self.endpoint_context.session_manager @@ -487,7 +491,7 @@ def test_client_claims(self): grant = self.session_manager[session_id] self.session_manager.token_handler["id_token"].kwargs["enable_claims_per_client"] = True - self.endpoint_context.cdb["client_1"]["id_token_claims"] = {"address": None} + self.endpoint_context.cdb["client_1"]["add_claims"]["always"]["id_token"] = {"address": None} _claims = self.endpoint_context.claims_interface.get_claims( session_id=session_id, scopes=AREQ["scope"], claims_release_point="id_token" @@ -543,6 +547,28 @@ def test_client_claims_scopes(self): assert "email" in res assert "nickname" not in res + def test_client_claims_scopes_per_client(self): + session_id = self._create_session(AREQS) + grant = self.session_manager[session_id] + self.session_manager.token_handler["id_token"].kwargs["add_claims_by_scope"] = True + self.endpoint_context.cdb[AREQS["client_id"]]["add_claims"]["by_scope"]["id_token"] = False + + _claims = self.endpoint_context.claims_interface.get_claims( + session_id=session_id, scopes=AREQS["scope"], claims_release_point="id_token" + ) + grant.claims = {"id_token": _claims} + + id_token = self._mint_id_token(grant, session_id) + + client_keyjar = KeyJar() + _jwks = self.endpoint_context.keyjar.export_jwks() + client_keyjar.import_jwks(_jwks, self.endpoint_context.issuer) + _jwt = JWT(key_jar=client_keyjar, iss="client_1") + res = _jwt.unpack(id_token.value) + assert "address" not in res + assert "email" in res + assert "nickname" not in res + def test_client_claims_scopes_and_request_claims_no_match(self): session_id = self._create_session(AREQRC) grant = self.session_manager[session_id] diff --git a/tests/test_05_jwt_token.py b/tests/test_05_jwt_token.py index 78f5f5e3..d5bb1240 100644 --- a/tests/test_05_jwt_token.py +++ b/tests/test_05_jwt_token.py @@ -186,6 +186,10 @@ def create_endpoint(self): "client_salt": "salted", "token_endpoint_auth_method": "client_secret_post", "response_types": ["code", "token", "code id_token", "id_token"], + "add_claims": { + "always": {}, + "by_scope": {}, + }, } self.session_manager = self.endpoint_context.session_manager self.user_id = "diana" @@ -247,7 +251,7 @@ def test_info(self): @pytest.mark.parametrize("enable_claims_per_client", [True, False]) def test_enable_claims_per_client(self, enable_claims_per_client): # Set up configuration - self.endpoint_context.cdb["client_1"]["access_token_claims"] = {"address": None} + self.endpoint_context.cdb["client_1"]["add_claims"]["always"]["access_token"] = {"address": None} self.endpoint_context.session_manager.token_handler.handler["access_token"].kwargs[ "enable_claims_per_client" ] = enable_claims_per_client diff --git a/tests/test_07_userinfo.py b/tests/test_07_userinfo.py index f556fcca..486614a2 100644 --- a/tests/test_07_userinfo.py +++ b/tests/test_07_userinfo.py @@ -255,7 +255,12 @@ def create_endpoint_context(self): server = Server(OPConfiguration(conf=conf, base_path=BASEDIR), cwd=BASEDIR) self.endpoint_context = server.endpoint_context # Just has to be there - self.endpoint_context.cdb["client1"] = {} + self.endpoint_context.cdb["client1"] = { + "add_claims": { + "always": {}, + "by_scope": {}, + }, + } self.session_manager = self.endpoint_context.session_manager self.claims_interface = ClaimsInterface(server.server_get) self.user_id = "diana" @@ -371,7 +376,7 @@ def test_collect_user_info_enable_claims_per_client(self): _userinfo_endpoint.kwargs["enable_claims_per_client"] = True del _userinfo_endpoint.kwargs["base_claims"] - self.endpoint_context.cdb[_req["client_id"]]["userinfo_claims"] = {"phone_number": None} + self.endpoint_context.cdb[_req["client_id"]]["add_claims"]["always"]["userinfo"] = {"phone_number": None} _userinfo_restriction = self.claims_interface.get_claims( session_id=session_id, scopes=_req["scope"], claims_release_point="userinfo" diff --git a/tests/test_31_oauth2_introspection.py b/tests/test_31_oauth2_introspection.py index 39e94b67..e2902b70 100644 --- a/tests/test_31_oauth2_introspection.py +++ b/tests/test_31_oauth2_introspection.py @@ -192,7 +192,12 @@ def create_endpoint(self, jwt_token): "client_salt": "salted", "token_endpoint_auth_method": "client_secret_post", "response_types": ["code", "token", "code id_token", "id_token"], - "introspection_claims": {"nickname": None, "eduperson_scoped_affiliation": None,}, + "add_claims": { + "always": { + "introspection": ["nickname", "eduperson_scoped_affiliation"], + }, + "by_scope": {}, + }, } endpoint_context.keyjar.import_jwks_as_json( endpoint_context.keyjar.export_jwks_as_json(private=True), endpoint_context.issuer, From 78cae684e16fb6e49349ff831b953533d1d1cc52 Mon Sep 17 00:00:00 2001 From: Giuseppe Date: Fri, 3 Sep 2021 01:32:09 +0200 Subject: [PATCH 16/17] chore: Documentation on CDB and minor changes --- docs/source/contents/conf.rst | 69 +++++++++++++++++++++++++++++++++++ docs/source/contents/usage.md | 16 ++++---- 2 files changed, 77 insertions(+), 8 deletions(-) diff --git a/docs/source/contents/conf.rst b/docs/source/contents/conf.rst index 34493edc..2378e0e3 100644 --- a/docs/source/contents/conf.rst +++ b/docs/source/contents/conf.rst @@ -622,3 +622,72 @@ the following:: } } } + + +======= +Clients +======= + +In this section there are some client configuration examples. + +A common configuration:: + + endpoint_context.cdb['jbxedfmfyc'] = { + client_id: 'jbxedfmfyc', + client_salt: '6flfsj0Z', + registration_access_token: 'z3PCMmC1HZ1QmXeXGOQMJpWQNQynM4xY', + registration_client_uri: 'https://127.0.0.1:8000/registration_api?client_id=jbxedfmfyc', + client_id_issued_at: 1630256902, + client_secret: '19cc69b70d0108f630e52f72f7a3bd37ba4e11678ad1a7434e9818e1', + client_secret_expires_at: 1929727754, + application_type: 'web', + contacts: [ + 'rp@example.com' + ], + token_endpoint_auth_method: 'client_secret_basic', + redirect_uris: [ + [ + 'https://127.0.0.1:8090/authz_cb/satosa', + {} + ] + ], + post_logout_redirect_uris: [ + [ + 'https://127.0.0.1:8090/session_logout/satosa', + null + ] + ], + response_types: [ + 'code' + ], + grant_types: [ + 'authorization_code' + ], + allowed_scopes: [ + 'openid', + 'profile', + 'email', + 'offline_access' + ] + } + + +How to configure the release of the user claims per clients:: + + endpoint_context.cdb["client_1"] = { + "client_secret": "hemligt", + "redirect_uris": [("https://example.com/cb", None)], + "client_salt": "salted", + "token_endpoint_auth_method": "client_secret_post", + "response_types": ["code", "token", "code id_token", "id_token"], + "add_claims": { + "always": { + "introspection": ["nickname", "eduperson_scoped_affiliation"], + "userinfo": ["picture", "phone_number"], + }, + # this overload the general endpoint configuration for this client + # self.server.server_get("endpoint", "id_token").kwargs = {"add_claims_by_scope": True} + "by_scope": { + "id_token": False, + }, + }, diff --git a/docs/source/contents/usage.md b/docs/source/contents/usage.md index 5fb8ab70..795cc0d3 100644 --- a/docs/source/contents/usage.md +++ b/docs/source/contents/usage.md @@ -1,7 +1,7 @@ Usage ----- -Some examples, how to run flask_op and django_op, but also some typical configuration in relation to common use cases. +Some examples, how to run [flask_op](https://github.com/IdentityPython/oidc-op/tree/master/example/flask_op) and [django_op](https://github.com/peppelinux/django-oidc-op) but also some typical configuration in relation to common use cases. @@ -34,7 +34,7 @@ Get to the RP landing page to choose your authentication endpoint. The first opt ![OP Auth](../_images/2.png) -AS/OP accepted our authentication request and prompt to us the login form. Read passwd.json file to get credentials. +The AS/OP supports dynamic client registration, it accepts the authentication request and prompt to us the login form. Read [passwd.json](https://github.com/IdentityPython/oidc-op/blob/master/example/flask_op/passwd.json) file to get credentials. ---------------------------------- @@ -75,12 +75,12 @@ It is important to consider that only scope=offline_access will get a usable ref oidc-op will return a json response like this:: -{ - 'access_token': 'eyJhbGc ... CIOH_09tT_YVa_gyTqg', - 'token_type': 'Bearer', - 'scope': 'openid profile email address phone offline_access', - 'refresh_token': 'Z0FBQ ... 1TE16cm1Tdg==' -} + { + 'access_token': 'eyJhbGc ... CIOH_09tT_YVa_gyTqg', + 'token_type': 'Bearer', + 'scope': 'openid profile email address phone offline_access', + 'refresh_token': 'Z0FBQ ... 1TE16cm1Tdg==' + } From 906a1be9d3d9c2417a0ea67e838f9b9ba66d42c9 Mon Sep 17 00:00:00 2001 From: Giuseppe Date: Fri, 3 Sep 2021 01:58:06 +0200 Subject: [PATCH 17/17] v2.1.1 --- src/oidcop/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/oidcop/__init__.py b/src/oidcop/__init__.py index 9b05f0cf..353bd1f8 100644 --- a/src/oidcop/__init__.py +++ b/src/oidcop/__init__.py @@ -1,6 +1,6 @@ import secrets -__version__ = "2.1.0" +__version__ = "2.1.1" DEF_SIGN_ALG = { "id_token": "RS256",