Merge pull request #116 from nsklikas/fix-refresh-id-token

Fix refresh tokens

Author: peppelinux

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",

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:
@@ -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"],
@@ -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",

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

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)
 

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()