Add `reqval.fill_id_token` with technicals OIDC fields into `id_token` (#660)

Add `reqval.fill_id_token` with technicals OIDC fields into `id_token`

8 files changed, 303 insertions, 22 deletions

diff --git a/docs/oauth2/oidc/id_tokens.rst b/docs/oauth2/oidc/id_tokens.rst
index 999cfa7..a1bf7cf 100644
--- a/docs/oauth2/oidc/id_tokens.rst
+++ b/docs/oauth2/oidc/id_tokens.rst
@@ -1,9 +1,9 @@
 ID Tokens
 =========
 
-The creation of `ID Tokens`_ is ultimately done not by OAuthLib but by your ``RequestValidator`` subclass.  This is because their
+The creation of `ID Tokens`_ is ultimately not done by OAuthLib but by your ``RequestValidator`` subclass.  This is because their
 content is dependent on your implementation of users, their attributes, any claims you may wish to support, as well as the
-details of how you model the notion of a Client Application.  As such OAuthLib simply calls your validator's ``get_id_token``
+details of how you model the notion of a Client Application. As such OAuthLib simply calls your validator's ``finalize_id_token``
 method at the appropriate times during the authorization flow, depending on the grant type requested (Authorization Code, Implicit,
 Hybrid, etc.).
 
@@ -12,7 +12,7 @@ See examples below.
 .. _`ID Tokens`: http://openid.net/specs/openid-connect-core-1_0.html#IDToken
 
 .. autoclass:: oauthlib.oauth2.RequestValidator
-   :members: get_id_token
+   :members: finalize_id_token
 
 
 JWT/JWS example with pyjwt library
@@ -38,12 +38,13 @@ You can switch to jwcrypto library if you want to return JWE instead.
 
         super().__init__(self, **kwargs)
 
-    def get_id_token(self, token, token_handler, request):
+    def finalize_id_token(self, id_token, token, token_handler, request):
         import jwt
 
-        data = {"nonce": request.nonce} if request.nonce is not None else {}
-
+        id_token["iss"] = "https://my.cool.app.com"
+        id_token["sub"] = request.user.id
+        id_token["exp"] = id_token["iat"] + 3600 * 24  # keep it valid for 24hours
         for claim_key in request.claims:
-            data[claim_key] = request.userattributes[claim_key]  # this must be set in another callback
+            id_token[claim_key] = request.userattributes[claim_key]  # this must be set in another callback
 
-        return jwt.encode(data, self.private_pem, 'RS256')
+        return jwt.encode(id_token, self.private_pem, 'RS256')
diff --git a/oauthlib/oauth2/rfc6749/request_validator.py b/oauthlib/oauth2/rfc6749/request_validator.py
index d6ec2ab..86509b6 100644
--- a/oauthlib/oauth2/rfc6749/request_validator.py
+++ b/oauthlib/oauth2/rfc6749/request_validator.py
@@ -271,6 +271,9 @@ class RequestValidator(object):
             - Code Challenge (``request.code_challenge``) and
             - Code Challenge Method (``request.code_challenge_method``)
 
+        To support OIDC, you MUST associate the code with:
+            - nonce, if present (``code["nonce"]``)
+
         The ``code`` argument is actually a dictionary, containing at least a
         ``code`` key with the actual authorization code:
 
diff --git a/oauthlib/openid/connect/core/grant_types/authorization_code.py b/oauthlib/openid/connect/core/grant_types/authorization_code.py
index b0b1015..becfcfa 100644
--- a/oauthlib/openid/connect/core/grant_types/authorization_code.py
+++ b/oauthlib/openid/connect/core/grant_types/authorization_code.py
@@ -22,3 +22,23 @@ class AuthorizationCodeGrant(GrantTypeBase):
         self.custom_validators.post_auth.append(
             self.openid_authorization_validator)
         self.register_token_modifier(self.add_id_token)
+
+    def add_id_token(self, token, token_handler, request):
+        """
+        Construct an initial version of id_token, and let the
+        request_validator sign or encrypt it.
+
+        The authorization_code version of this method is used to
+        retrieve the nonce accordingly to the code storage.
+        """
+        # Treat it as normal OAuth 2 auth code request if openid is not present
+        if not request.scopes or 'openid' not in request.scopes:
+            return token
+
+        nonce = self.request_validator.get_authorization_code_nonce(
+            request.client_id,
+            request.code,
+            request.redirect_uri,
+            request
+        )
+        return super(AuthorizationCodeGrant, self).add_id_token(token, token_handler, request, nonce=nonce)
diff --git a/oauthlib/openid/connect/core/grant_types/base.py b/oauthlib/openid/connect/core/grant_types/base.py
index 4f5c944..32a21b6 100644
--- a/oauthlib/openid/connect/core/grant_types/base.py
+++ b/oauthlib/openid/connect/core/grant_types/base.py
@@ -1,7 +1,9 @@
 from .exceptions import OIDCNoPrompt
 
-import datetime
+import base64
+import hashlib
 import logging
+import time
 from json import loads
 
 from oauthlib.oauth2.rfc6749.errors import ConsentRequired, InvalidRequestError, LoginRequired
@@ -49,7 +51,45 @@ class GrantTypeBase(object):
                 raise InvalidRequestError(description="Malformed claims parameter",
                                           uri="http://openid.net/specs/openid-connect-core-1_0.html#ClaimsParameter")
 
-    def add_id_token(self, token, token_handler, request):
+    def id_token_hash(self, value, hashfunc=hashlib.sha256):
+        """
+        Its value is the base64url encoding of the left-most half of the
+        hash of the octets of the ASCII representation of the access_token
+        value, where the hash algorithm used is the hash algorithm used in
+        the alg Header Parameter of the ID Token's JOSE Header.
+
+        For instance, if the alg is RS256, hash the access_token value
+        with SHA-256, then take the left-most 128 bits and
+        base64url-encode them.
+        For instance, if the alg is HS512, hash the code value with
+        SHA-512, then take the left-most 256 bits and base64url-encode
+        them. The c_hash value is a case-sensitive string.
+
+        Example of hash from OIDC specification (bound to a JWS using RS256):
+
+        code:
+        Qcb0Orv1zh30vL1MPRsbm-diHiMwcLyZvn1arpZv-Jxf_11jnpEX3Tgfvk
+
+        c_hash:
+        LDktKdoQak3Pk0cnXxCltA
+        """
+        digest = hashfunc(value.encode()).digest()
+        left_most = len(digest) // 2
+        return base64.urlsafe_b64encode(digest[:left_most]).decode().rstrip("=")
+
+    def add_id_token(self, token, token_handler, request, nonce=None):
+        """
+        Construct an initial version of id_token, and let the
+        request_validator sign or encrypt it.
+
+        The initial version can contain the fields below, accordingly
+        to the spec:
+        - aud
+        - iat
+        - nonce
+        - at_hash
+        - c_hash
+        """
         # Treat it as normal OAuth 2 auth code request if openid is not present
         if not request.scopes or 'openid' not in request.scopes:
             return token
@@ -58,13 +98,54 @@ class GrantTypeBase(object):
         if request.response_type and 'id_token' not in request.response_type:
             return token
 
-        if request.max_age:
-            d = datetime.datetime.utcnow()
-            token['auth_time'] = d.isoformat("T") + "Z"
-
-        # TODO: acr claims (probably better handled by server code using oauthlib in get_id_token)
+        # Implementation mint its own id_token without help.
+        id_token = self.request_validator.get_id_token(token, token_handler, request)
+        if id_token:
+            token['id_token'] = id_token
+            return token
 
-        token['id_token'] = self.request_validator.get_id_token(token, token_handler, request)
+        # Fallback for asking some help from oauthlib framework.
+        # Start with technicals fields bound to the specification.
+        id_token = {}
+        id_token['aud'] = request.client_id
+        id_token['iat'] = int(time.time())
+
+        # nonce is REQUIRED when response_type value is:
+        # - id_token token (Implicit)
+        # - id_token (Implicit)
+        # - code id_token (Hybrid)
+        # - code id_token token (Hybrid)
+        #
+        # nonce is OPTIONAL when response_type value is:
+        # - code (Authorization Code)
+        # - code token (Hybrid)
+        if nonce is not None:
+            id_token["nonce"] = nonce
+
+        # at_hash is REQUIRED when response_type value is:
+        # - id_token token (Implicit)
+        # - code id_token token (Hybrid)
+        #
+        # at_hash is OPTIONAL when:
+        # - code (Authorization code)
+        # - code id_token (Hybrid)
+        # - code token (Hybrid)
+        #
+        # at_hash MAY NOT be used when:
+        # - id_token (Implicit)
+        if "access_token" in token:
+            id_token["at_hash"] = self.id_token_hash(token["access_token"])
+
+        # c_hash is REQUIRED when response_type value is:
+        # - code id_token (Hybrid)
+        # - code id_token token (Hybrid)
+        #
+        # c_hash is OPTIONAL for others.
+        if "code" in token:
+            id_token["c_hash"] = self.id_token_hash(token["code"])
+
+        # Call request_validator to complete/sign/encrypt id_token
+        token['id_token'] = self.request_validator.finalize_id_token(id_token, token, token_handler, request)
 
         return token
 
diff --git a/oauthlib/openid/connect/core/grant_types/implicit.py b/oauthlib/openid/connect/core/grant_types/implicit.py
index d3797b2..c2dbc27 100644
--- a/oauthlib/openid/connect/core/grant_types/implicit.py
+++ b/oauthlib/openid/connect/core/grant_types/implicit.py
@@ -27,9 +27,9 @@ class ImplicitGrant(GrantTypeBase):
         self.register_token_modifier(self.add_id_token)
 
     def add_id_token(self, token, token_handler, request):
-        if 'state' not in token:
+        if 'state' not in token and request.state:
             token['state'] = request.state
-        return super(ImplicitGrant, self).add_id_token(token, token_handler, request)
+        return super(ImplicitGrant, self).add_id_token(token, token_handler, request, nonce=request.nonce)
 
     def openid_authorization_validator(self, request):
         """Additional validation when following the implicit flow.
diff --git a/oauthlib/openid/connect/core/request_validator.py b/oauthlib/openid/connect/core/request_validator.py
index 1587754..d96c9ef 100644
--- a/oauthlib/openid/connect/core/request_validator.py
+++ b/oauthlib/openid/connect/core/request_validator.py
@@ -24,7 +24,7 @@ class RequestValidator(OAuth2RequestValidator):
         id_token in token response only based on authorization code scopes.
 
         Only code param should be sufficient to retrieve grant code from
-        any storage you are using, `client_id` and `redirect_uri` can gave a
+        any storage you are using, `client_id` and `redirect_uri` can have a
         blank value `""` don't forget to check it before using those values
         in a select query if a database is used.
 
@@ -38,6 +38,30 @@ class RequestValidator(OAuth2RequestValidator):
         """
         raise NotImplementedError('Subclasses must implement this method.')
 
+    def get_authorization_code_nonce(self, client_id, code, redirect_uri, request):
+        """ Extracts nonce from saved authorization code.
+
+        If present in the Authentication Request, Authorization
+        Servers MUST include a nonce Claim in the ID Token with the
+        Claim Value being the nonce value sent in the Authentication
+        Request. Authorization Servers SHOULD perform no other
+        processing on nonce values used. The nonce value is a
+        case-sensitive string.
+
+        Only code param should be sufficient to retrieve grant code from
+        any storage you are using. However, `client_id` and `redirect_uri`
+        have been validated and can be used also.
+
+        :param client_id: Unicode client identifier
+        :param code: Unicode authorization code grant
+        :param redirect_uri: Unicode absolute URI
+        :return: Unicode nonce
+
+        Method is used by:
+            - Authorization Token Grant Dispatcher
+        """
+        raise NotImplementedError('Subclasses must implement this method.')
+
     def get_jwt_bearer_token(self, token, token_handler, request):
         """Get JWT Bearer token or OpenID Connect ID token
 
@@ -57,6 +81,12 @@ class RequestValidator(OAuth2RequestValidator):
     def get_id_token(self, token, token_handler, request):
         """Get OpenID Connect ID token
 
+        This method is OPTIONAL and is NOT RECOMMENDED.
+        `finalize_id_token` SHOULD be implemented instead. However, if you
+        want a full control over the minting of the `id_token`, you
+        MAY want to override `get_id_token` instead of using
+        `finalize_id_token`.
+
         In the OpenID Connect workflows when an ID Token is requested this method is called.
         Subclasses should implement the construction, signing and optional encryption of the
         ID Token as described in the OpenID Connect spec.
@@ -85,7 +115,49 @@ class RequestValidator(OAuth2RequestValidator):
         :type request: oauthlib.common.Request
         :return: The ID Token (a JWS signed JWT)
         """
-        # the request.scope should be used by the get_id_token() method to determine which claims to include in the resulting id_token
+        return None
+
+    def finalize_id_token(self, id_token, token, token_handler, request):
+        """Finalize OpenID Connect ID token & Sign or Encrypt.
+
+        In the OpenID Connect workflows when an ID Token is requested
+        this method is called.  Subclasses should implement the
+        construction, signing and optional encryption of the ID Token
+        as described in the OpenID Connect spec.
+
+        The `id_token` parameter is a dict containing a couple of OIDC
+        technical fields related to the specification. Prepopulated
+        attributes are:
+
+        - `aud`, equals to `request.client_id`.
+        - `iat`, equals to current time.
+        - `nonce`, if present, is equals to the `nonce` from the
+          authorization request.
+        - `at_hash`, hash of `access_token`, if relevant.
+        - `c_hash`, hash of `code`, if relevant.
+
+        This method MUST provide required fields as below:
+
+        - `iss`, REQUIRED. Issuer Identifier for the Issuer of the response.
+        - `sub`, REQUIRED. Subject Identifier
+        - `exp`, REQUIRED. Expiration time on or after which the ID
+          Token MUST NOT be accepted by the RP when performing
+          authentication with the OP.
+
+        Additionals claims must be added, note that `request.scope`
+        should be used to determine the list of claims.
+
+        More information can be found at `OpenID Connect Core#Claims`_
+
+        .. _`OpenID Connect Core#Claims`: https://openid.net/specs/openid-connect-core-1_0.html#Claims
+
+        :param id_token: A dict containing technical fields of id_token
+        :param token: A Bearer token dict
+        :param token_handler: the token handler (BearerToken class)
+        :param request: OAuthlib request.
+        :type request: oauthlib.common.Request
+        :return: The ID Token (a JWS signed JWT or JWE encrypted JWT)
+        """
         raise NotImplementedError('Subclasses must implement this method.')
 
     def validate_jwt_bearer_token(self, token, scopes, request):
diff --git a/tests/openid/connect/core/grant_types/test_base.py b/tests/openid/connect/core/grant_types/test_base.py
new file mode 100644
index 0000000..76e017f
--- /dev/null
+++ b/tests/openid/connect/core/grant_types/test_base.py
@@ -0,0 +1,104 @@
+# -*- coding: utf-8 -*-
+import mock
+import time
+
+from oauthlib.common import Request
+from oauthlib.openid.connect.core.grant_types.base import GrantTypeBase
+
+from tests.unittest import TestCase
+
+
+class GrantBase(GrantTypeBase):
+    """Class to test GrantTypeBase"""
+    def __init__(self, request_validator=None, **kwargs):
+        self.request_validator = request_validator
+
+
+class IDTokenTest(TestCase):
+
+    def setUp(self):
+        self.request = Request('http://a.b/path')
+        self.request.scopes = ('hello', 'openid')
+        self.request.expires_in = 1800
+        self.request.client_id = 'abcdef'
+        self.request.code = '1234'
+        self.request.response_type = 'id_token'
+        self.request.grant_type = 'authorization_code'
+        self.request.redirect_uri = 'https://a.b/cb'
+        self.request.state = 'abc'
+        self.request.nonce = None
+
+        self.mock_validator = mock.MagicMock()
+        self.mock_validator.get_id_token.return_value = None
+        self.mock_validator.finalize_id_token.return_value = "eyJ.body.signature"
+        self.token = {}
+
+        self.grant = GrantBase(request_validator=self.mock_validator)
+
+        self.url_query = 'https://a.b/cb?code=abc&state=abc'
+        self.url_fragment = 'https://a.b/cb#code=abc&state=abc'
+
+    def test_id_token_hash(self):
+        self.assertEqual(self.grant.id_token_hash(
+            "Qcb0Orv1zh30vL1MPRsbm-diHiMwcLyZvn1arpZv-Jxf_11jnpEX3Tgfvk",
+        ), "LDktKdoQak3Pk0cnXxCltA", "hash differs from RFC")
+
+    def test_get_id_token_no_openid(self):
+        self.request.scopes = ('hello')
+        token = self.grant.add_id_token(self.token, "token_handler_mock", self.request)
+        self.assertNotIn("id_token", token)
+
+        self.request.scopes = None
+        token = self.grant.add_id_token(self.token, "token_handler_mock", self.request)
+        self.assertNotIn("id_token", token)
+
+        self.request.scopes = ()
+        token = self.grant.add_id_token(self.token, "token_handler_mock", self.request)
+        self.assertNotIn("id_token", token)
+
+    def test_get_id_token(self):
+        self.mock_validator.get_id_token.return_value = "toto"
+        token = self.grant.add_id_token(self.token, "token_handler_mock", self.request)
+        self.assertIn("id_token", token)
+        self.assertEqual(token["id_token"], "toto")
+
+    def test_finalize_id_token(self):
+        token = self.grant.add_id_token(self.token, "token_handler_mock", self.request)
+        self.assertIn("id_token", token)
+        self.assertEqual(token["id_token"], "eyJ.body.signature")
+        id_token = self.mock_validator.finalize_id_token.call_args[0][0]
+        self.assertEqual(id_token['aud'], 'abcdef')
+        self.assertGreaterEqual(id_token['iat'], int(time.time()))
+
+    def test_finalize_id_token_with_nonce(self):
+        token = self.grant.add_id_token(self.token, "token_handler_mock", self.request, "my_nonce")
+        self.assertIn("id_token", token)
+        self.assertEqual(token["id_token"], "eyJ.body.signature")
+        id_token = self.mock_validator.finalize_id_token.call_args[0][0]
+        self.assertEqual(id_token['nonce'], 'my_nonce')
+
+    def test_finalize_id_token_with_at_hash(self):
+        self.token["access_token"] = "Qcb0Orv1zh30vL1MPRsbm-diHiMwcLyZvn1arpZv-Jxf_11jnpEX3Tgfvk"
+        token = self.grant.add_id_token(self.token, "token_handler_mock", self.request)
+        self.assertIn("id_token", token)
+        self.assertEqual(token["id_token"], "eyJ.body.signature")
+        id_token = self.mock_validator.finalize_id_token.call_args[0][0]
+        self.assertEqual(id_token['at_hash'], 'LDktKdoQak3Pk0cnXxCltA')
+
+    def test_finalize_id_token_with_c_hash(self):
+        self.token["code"] = "Qcb0Orv1zh30vL1MPRsbm-diHiMwcLyZvn1arpZv-Jxf_11jnpEX3Tgfvk"
+        token = self.grant.add_id_token(self.token, "token_handler_mock", self.request)
+        self.assertIn("id_token", token)
+        self.assertEqual(token["id_token"], "eyJ.body.signature")
+        id_token = self.mock_validator.finalize_id_token.call_args[0][0]
+        self.assertEqual(id_token['c_hash'], 'LDktKdoQak3Pk0cnXxCltA')
+
+    def test_finalize_id_token_with_c_and_at_hash(self):
+        self.token["code"] = "Qcb0Orv1zh30vL1MPRsbm-diHiMwcLyZvn1arpZv-Jxf_11jnpEX3Tgfvk"
+        self.token["access_token"] = "Qcb0Orv1zh30vL1MPRsbm-diHiMwcLyZvn1arpZv-Jxf_11jnpEX3Tgfvk"
+        token = self.grant.add_id_token(self.token, "token_handler_mock", self.request)
+        self.assertIn("id_token", token)
+        self.assertEqual(token["id_token"], "eyJ.body.signature")
+        id_token = self.mock_validator.finalize_id_token.call_args[0][0]
+        self.assertEqual(id_token['at_hash'], 'LDktKdoQak3Pk0cnXxCltA')
+        self.assertEqual(id_token['c_hash'], 'LDktKdoQak3Pk0cnXxCltA')
diff --git a/tests/openid/connect/core/test_request_validator.py b/tests/openid/connect/core/test_request_validator.py
index e20e88f..ebe0aeb 100644
--- a/tests/openid/connect/core/test_request_validator.py
+++ b/tests/openid/connect/core/test_request_validator.py
@@ -22,8 +22,8 @@ class RequestValidatorTest(TestCase):
         )
         self.assertRaises(
             NotImplementedError,
-            v.get_id_token,
-            'token', 'token_handler', 'request'
+            v.finalize_id_token,
+            'id_token', 'token', 'token_handler', 'request'
         )
         self.assertRaises(
             NotImplementedError,