Refactored nonce checking.

Fixed tests.

Author: rohe

src/oidcrp/oidc/authorization.py

@@ -2,6 +2,7 @@
 
 from oidcmsg import oauth2
 from oidcmsg import oidc
+from oidcmsg.exception import MissingRequiredAttribute
 from oidcmsg.oidc import make_openid_request
 from oidcmsg.oidc import verified_claim_name
 from oidcmsg.time_util import time_sans_frac
@@ -49,26 +50,32 @@ def set_state(self, request_args, **kwargs):
     def update_service_context(self, resp, key='', **kwargs):
         _context = self.client_get("service_context")
 
-        _idt = resp.get(verified_claim_name('id_token'))
-        if _idt:
-            # If there is a verified ID Token then we have to do nonce
-            # verification
-            item = _context.state.get_item(oauth2.AuthorizationRequest, 'auth_request', key)
-            if item['nonce'] != _idt['nonce']:
-                raise ValueError('Invalid nonce')
-
-            # try:
-            #     if _context.state.get_state_by_nonce(_idt['nonce']) != key:
-            #         raise ParameterError('Someone has messed with "nonce"')
-            # except KeyError:
-            #     raise ValueError('Invalid nonce')
-
-            _context.state.store_sub2state(_idt['sub'], key)
-
         if 'expires_in' in resp:
             resp['__expires_at'] = time_sans_frac() + int(resp['expires_in'])
         _context.state.store_item(resp.to_json(), 'auth_response', key)
 
+    def get_request_from_response(self, response):
+        _context = self.client_get("service_context")
+        return _context.state.get_item(oauth2.AuthorizationRequest, 'auth_request',
+                                       response["state"])
+
+    def post_parse_response(self, response, **kwargs):
+        response = authorization.Authorization.post_parse_response(self, response, **kwargs)
+
+        _idt = response.get(verified_claim_name('id_token'))
+        if _idt:
+            # If there is a verified ID Token then we have to do nonce
+            # verification.
+            _request = self.get_request_from_response(response)
+            _req_nonce = _request.get('nonce')
+            if _req_nonce:
+                _id_token_nonce = _idt.get('nonce')
+                if not _id_token_nonce:
+                    raise MissingRequiredAttribute('nonce')
+                elif _req_nonce != _id_token_nonce:
+                    raise ValueError('Invalid nonce')
+        return response
+
     def oidc_pre_construct(self, request_args=None, post_args=None, **kwargs):
         _context = self.client_get("service_context")
         if request_args is None:
@@ -272,10 +279,6 @@ def gather_verify_arguments(self):
             'skew': _context.clock_skew
         }
 
-        _nonce = _context.state.get_item(oauth2.AuthorizationRequest, 'auth_request', 'nonce')
-        if _nonce:
-            kwargs["nonce"] = _nonce
-
         _client_id = _context.client_id
         if _client_id:
             kwargs['client_id'] = _client_id

src/oidcrp/service.py

@@ -557,8 +557,7 @@ def parse_response(self, info, sformat="", state="", **kwargs):
             vargs = self.gather_verify_arguments()
             LOGGER.debug("Verify response with %s", vargs)
             try:
-                # verify the message. If something is wrong an exception is
-                # thrown
+                # verify the message. If something is wrong an exception is thrown
                 resp.verify(**vargs)
             except Exception as err:
                 LOGGER.error(

tests/request123456.jwt

@@ -1 +1 @@
-eyJhbGciOiJSUzI1NiIsImtpZCI6IlNVc3dOaTFNUkZsRFQwWTJZalUxWjFSZlFsbzJTM2RFYTNGVFRrVjNMVGhGY25oRFRIRjVlbGsyVlEifQ.eyJyZXNwb25zZV90eXBlIjogImNvZGUiLCAic3RhdGUiOiAic3RhdGUiLCAicmVkaXJlY3RfdXJpIjogImh0dHBzOi8vZXhhbXBsZS5jb20vY2xpL2F1dGh6X2NiIiwgInNjb3BlIjogIm9wZW5pZCIsICJub25jZSI6ICI4Tkc4VWt1cm9qWEE4azJ4SEJhYXQyRnpCcFVpbDRXNyIsICJjbGllbnRfaWQiOiAiY2xpZW50X2lkIiwgImlzcyI6ICJjbGllbnRfaWQiLCAiaWF0IjogMTYzMzM1Mjk1NywgImF1ZCI6IFsiaHR0cHM6Ly9leGFtcGxlLmNvbSJdfQ.KEY0WdzULSWr7M4guCOArwRWPpY6S8_ldckWYDvihTIy-V59zqQxucNrxEtSh8nkCr_dzZB4ZyHX836g77UOCtFh8TYMARpADWGV-m83OBR0_nwiohMlZJL1H4sH-ovID7CUHebr0yXu0NzEI_Fx_hr1PUeQru2UcoQBZg9g8uGpfW9GXy9H_rLg_l2T3xtpPTQfE057q9KREmQmc0XDXVi1XXUyUSGNeXJUlVLu14sWO5y92b5rWjujk9tUTS2wAiVOtH3qNCP8BorSkiG6pvCHDejYZYRG037pKiUJSN3z8McXgTSSIOxW4zfpP9D5FSkYn-re9kKh5jMWpv9w_Q
+eyJhbGciOiJSUzI1NiIsImtpZCI6IlNVc3dOaTFNUkZsRFQwWTJZalUxWjFSZlFsbzJTM2RFYTNGVFRrVjNMVGhGY25oRFRIRjVlbGsyVlEifQ.eyJyZXNwb25zZV90eXBlIjogImNvZGUiLCAic3RhdGUiOiAic3RhdGUiLCAicmVkaXJlY3RfdXJpIjogImh0dHBzOi8vZXhhbXBsZS5jb20vY2xpL2F1dGh6X2NiIiwgInNjb3BlIjogIm9wZW5pZCIsICJub25jZSI6ICJCZ25yS0daMHZNQ3lFRlU1U0d5ekZmVlNkVGxHZmhIaSIsICJjbGllbnRfaWQiOiAiY2xpZW50X2lkIiwgImlzcyI6ICJjbGllbnRfaWQiLCAiaWF0IjogMTYzMzU5MjI4OCwgImF1ZCI6IFsiaHR0cHM6Ly9leGFtcGxlLmNvbSJdfQ.xYAc40jcNNioyQ_FbbhrBectUkhxX62rPmf8whkH-7FBkrzAdjYIM7PmyDfJXRmXz0mw54EOriq3aXS-CPZqcSfRAYf7e-Shw2Ve1-138o307l6x7LmvLpK8EnUealcO5fEs-aLEwVre1ZOXNHchWKt-Lj_eL5cVA-FNQj09IzKTlDv_1gh_bSJJKELW0BsK9f3JYf-pM4EoCqoajbt_jw2WakzF0Phg2mc_wolpUPPZigjgQj8AGeAcDVf6y74E9j9csSVpB8YlnFwyUyJ0Yh-zRnIK0EInufdHu7qo1rJS6UpcTb2eS354Zm3cd1YDcfvPfsT__YV8Rb32Uo2_WQ

tests/test_13_oidc_service.py

@@ -7,6 +7,7 @@
 from cryptojwt.jwt import JWT
 from cryptojwt.key_jar import build_keyjar
 from cryptojwt.key_jar import init_key_jar
+from oidcmsg.exception import MissingRequiredAttribute
 from oidcmsg.oidc import AccessTokenRequest
 from oidcmsg.oidc import AccessTokenResponse
 from oidcmsg.oidc import AuthorizationRequest
@@ -202,7 +203,7 @@ def test_update_service_context_with_idtoken_wrong_nonce(self):
         idt = JWT(ISS_KEY, iss=ISS, lifetime=3600)
         payload = {
             'sub': '123456789', 'aud': ['client_id'],
-            'nonce': 'nonce'
+            'nonce': 'noice'
         }
         # have to calculate c_hash
         alg = 'RS256'
@@ -211,9 +212,8 @@ def test_update_service_context_with_idtoken_wrong_nonce(self):
 
         _idt = idt.pack(payload)
         resp = AuthorizationResponse(state='state', code='code', id_token=_idt)
-        resp = self.service.parse_response(resp.to_urlencoded())
-        with pytest.raises(ParameterError):
-            self.service.update_service_context(resp, 'state2')
+        with pytest.raises(ValueError):
+            self.service.parse_response(resp.to_urlencoded())
 
     def test_update_service_context_with_idtoken_missing_nonce(self):
         req_args = {'response_type': 'code', 'state': 'state', 'nonce': 'nonce'}
@@ -229,9 +229,8 @@ def test_update_service_context_with_idtoken_missing_nonce(self):
 
         _idt = idt.pack(payload)
         resp = AuthorizationResponse(state='state', code='code', id_token=_idt)
-        resp = self.service.parse_response(resp.to_urlencoded())
-        with pytest.raises(ValueError):
-            self.service.update_service_context(resp, 'state')
+        with pytest.raises(MissingRequiredAttribute):
+            self.service.parse_response(resp.to_urlencoded())
 
     @pytest.mark.parametrize("allow_sign_alg_none", [True, False])
     def test_allow_unsigned_idtoken(self, allow_sign_alg_none):
@@ -240,14 +239,14 @@ def test_allow_unsigned_idtoken(self, allow_sign_alg_none):
         self.service.get_request_parameters(request_args=req_args)
         # Build an ID Token
         idt = JWT(ISS_KEY, iss=ISS, lifetime=3600, sign_alg='none')
-        payload = {'sub': '123456789', 'aud': ['client_id']}
+        payload = {'sub': '123456789', 'aud': ['client_id'], 'nonce': req_args['nonce']}
         _idt = idt.pack(payload)
         self.service.client_get("service_context").behaviour["verify_args"] = {
             "allow_sign_alg_none": allow_sign_alg_none
         }
         resp = AuthorizationResponse(state='state', code='code', id_token=_idt)
         if allow_sign_alg_none:
-            resp = self.service.parse_response(resp.to_urlencoded())
+            self.service.parse_response(resp.to_urlencoded())
         else:
             with pytest.raises(UnsupportedAlgorithm):
                 self.service.parse_response(resp.to_urlencoded())