OIDC: Raise error=invalid_request when nonce is mandatory

Until now, only OIDC implicit was raising an error, but OIDC hybrid contain a couple of mandatory nonce, too.

4 files changed, 99 insertions, 32 deletions

diff --git a/oauthlib/openid/connect/core/grant_types/base.py b/oauthlib/openid/connect/core/grant_types/base.py
index 05cdd37..4f5c944 100644
--- a/oauthlib/openid/connect/core/grant_types/base.py
+++ b/oauthlib/openid/connect/core/grant_types/base.py
@@ -247,28 +247,5 @@ class GrantTypeBase(object):
 
         return request_info
 
-    def openid_implicit_authorization_validator(self, request):
-        """Additional validation when following the implicit flow.
-        """
-        # Undefined in OpenID Connect, fall back to OAuth2 definition.
-        if request.response_type == 'token':
-            return {}
-
-        # 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 {}
-
-        # REQUIRED. String value used to associate a Client session with an ID
-        # Token, and to mitigate replay attacks. The value is passed through
-        # unmodified from the Authentication Request to the ID Token.
-        # Sufficient entropy MUST be present in the nonce values used to
-        # prevent attackers from guessing values. For implementation notes, see
-        # Section 15.5.2.
-        if not request.nonce:
-            desc = 'Request is missing mandatory nonce parameter.'
-            raise InvalidRequestError(request=request, description=desc)
-
-        return {}
-
 
 OpenIDConnectBase = GrantTypeBase
diff --git a/oauthlib/openid/connect/core/grant_types/hybrid.py b/oauthlib/openid/connect/core/grant_types/hybrid.py
index 54669ae..685fa08 100644
--- a/oauthlib/openid/connect/core/grant_types/hybrid.py
+++ b/oauthlib/openid/connect/core/grant_types/hybrid.py
@@ -8,6 +8,7 @@ from __future__ import absolute_import, unicode_literals
 import logging
 
 from oauthlib.oauth2.rfc6749.grant_types.authorization_code import AuthorizationCodeGrant as OAuth2AuthorizationCodeGrant
+from oauthlib.oauth2.rfc6749.errors import InvalidRequestError
 
 from .base import GrantTypeBase
 from ..request_validator import RequestValidator
@@ -34,3 +35,27 @@ class HybridGrant(GrantTypeBase):
         self.register_code_modifier(self.add_token)
         self.register_code_modifier(self.add_id_token)
         self.register_token_modifier(self.add_id_token)
+
+    def openid_authorization_validator(self, request):
+        """Additional validation when following the Authorization Code flow.
+        """
+        request_info = super(HybridGrant, self).openid_authorization_validator(request)
+        if not request_info:  # returns immediately if OAuth2.0
+            return request_info
+
+        # REQUIRED if the Response Type of the request is `code
+        # id_token` or `code id_token token` and OPTIONAL when the
+        # Response Type of the request is `code token`. It is a string
+        # value used to associate a Client session with an ID Token,
+        # and to mitigate replay attacks. The value is passed through
+        # unmodified from the Authentication Request to the ID
+        # Token. Sufficient entropy MUST be present in the `nonce`
+        # values used to prevent attackers from guessing values. For
+        # implementation notes, see Section 15.5.2.
+        if request.response_type in ["code id_token", "code id_token token"]:
+            if not request.nonce:
+                raise InvalidRequestError(
+                    request=request,
+                    description='Request is missing mandatory nonce parameter.'
+                )
+        return request_info
diff --git a/oauthlib/openid/connect/core/grant_types/implicit.py b/oauthlib/openid/connect/core/grant_types/implicit.py
index 0a6fcb7..d3797b2 100644
--- a/oauthlib/openid/connect/core/grant_types/implicit.py
+++ b/oauthlib/openid/connect/core/grant_types/implicit.py
@@ -10,6 +10,7 @@ import logging
 from .base import GrantTypeBase
 
 from oauthlib.oauth2.rfc6749.grant_types.implicit import ImplicitGrant as OAuth2ImplicitGrant
+from oauthlib.oauth2.rfc6749.errors import InvalidRequestError
 
 log = logging.getLogger(__name__)
 
@@ -23,11 +24,29 @@ class ImplicitGrant(GrantTypeBase):
         self.register_response_type('id_token token')
         self.custom_validators.post_auth.append(
             self.openid_authorization_validator)
-        self.custom_validators.post_auth.append(
-            self.openid_implicit_authorization_validator)
         self.register_token_modifier(self.add_id_token)
 
     def add_id_token(self, token, token_handler, request):
         if 'state' not in token:
             token['state'] = request.state
         return super(ImplicitGrant, self).add_id_token(token, token_handler, request)
+
+    def openid_authorization_validator(self, request):
+        """Additional validation when following the implicit flow.
+        """
+        request_info = super(ImplicitGrant, self).openid_authorization_validator(request)
+        if not request_info:  # returns immediately if OAuth2.0
+            return request_info
+
+        # REQUIRED. String value used to associate a Client session with an ID
+        # Token, and to mitigate replay attacks. The value is passed through
+        # unmodified from the Authentication Request to the ID Token.
+        # Sufficient entropy MUST be present in the nonce values used to
+        # prevent attackers from guessing values. For implementation notes, see
+        # Section 15.5.2.
+        if not request.nonce:
+            raise InvalidRequestError(
+                request=request,
+                description='Request is missing mandatory nonce parameter.'
+            )
+        return request_info
diff --git a/tests/openid/connect/core/grant_types/test_implicit.py b/tests/openid/connect/core/grant_types/test_implicit.py
index 7ab198a..54fd8b9 100644
--- a/tests/openid/connect/core/grant_types/test_implicit.py
+++ b/tests/openid/connect/core/grant_types/test_implicit.py
@@ -4,6 +4,7 @@ from __future__ import absolute_import, unicode_literals
 import mock
 
 from oauthlib.common import Request
+from oauthlib.oauth2.rfc6749 import errors
 from oauthlib.oauth2.rfc6749.tokens import BearerToken
 from oauthlib.openid.connect.core.grant_types.exceptions import OIDCNoPrompt
 from oauthlib.openid.connect.core.grant_types.hybrid import HybridGrant
@@ -30,8 +31,8 @@ class OpenIDImplicitTest(TestCase):
         self.request.client_id = 'abcdef'
         self.request.response_type = 'id_token token'
         self.request.redirect_uri = 'https://a.b/cb'
-        self.request.nonce = 'zxc'
         self.request.state = 'abc'
+        self.request.nonce = 'xyz'
 
         self.mock_validator = mock.MagicMock()
         self.mock_validator.get_id_token.side_effect = get_id_token_mock
@@ -61,12 +62,6 @@ class OpenIDImplicitTest(TestCase):
         self.assertEqual(b, None)
         self.assertEqual(s, 302)
 
-        self.request.nonce = None
-        h, b, s = self.auth.create_authorization_response(self.request, bearer)
-        self.assertIn('error=invalid_request', h['Location'])
-        self.assertEqual(b, None)
-        self.assertEqual(s, 302)
-
     @mock.patch('oauthlib.common.generate_token')
     def test_no_prompt_authorization(self, generate_token):
         generate_token.return_value = 'abc'
@@ -105,16 +100,41 @@ class OpenIDImplicitTest(TestCase):
         h, b, s = self.auth.create_authorization_response(self.request, bearer)
         self.assertIn('error=login_required', h['Location'])
 
+    @mock.patch('oauthlib.common.generate_token')
+    def test_required_nonce(self, generate_token):
+        generate_token.return_value = 'abc'
+        self.request.nonce = None
+        self.assertRaises(errors.InvalidRequestError, self.auth.validate_authorization_request, self.request)
+
+        bearer = BearerToken(self.mock_validator)
+        h, b, s = self.auth.create_authorization_response(self.request, bearer)
+        self.assertIn('error=invalid_request', h['Location'])
+        self.assertEqual(b, None)
+        self.assertEqual(s, 302)
+
 
 class OpenIDHybridCodeTokenTest(OpenIDAuthCodeTest):
 
     def setUp(self):
         super(OpenIDHybridCodeTokenTest, self).setUp()
         self.request.response_type = 'code token'
+        self.request.nonce = None
         self.auth = HybridGrant(request_validator=self.mock_validator)
         self.url_query = 'https://a.b/cb?code=abc&state=abc&token_type=Bearer&expires_in=3600&scope=hello+openid&access_token=abc'
         self.url_fragment = 'https://a.b/cb#code=abc&state=abc&token_type=Bearer&expires_in=3600&scope=hello+openid&access_token=abc'
 
+    @mock.patch('oauthlib.common.generate_token')
+    def test_optional_nonce(self, generate_token):
+        generate_token.return_value = 'abc'
+        self.request.nonce = 'xyz'
+        scope, info = self.auth.validate_authorization_request(self.request)
+
+        bearer = BearerToken(self.mock_validator)
+        h, b, s = self.auth.create_authorization_response(self.request, bearer)
+        self.assertURLEqual(h['Location'], self.url_fragment, parse_fragment=True)
+        self.assertEqual(b, None)
+        self.assertEqual(s, 302)
+
 
 class OpenIDHybridCodeIdTokenTest(OpenIDAuthCodeTest):
 
@@ -122,11 +142,24 @@ class OpenIDHybridCodeIdTokenTest(OpenIDAuthCodeTest):
         super(OpenIDHybridCodeIdTokenTest, self).setUp()
         self.mock_validator.get_code_challenge.return_value = None
         self.request.response_type = 'code id_token'
+        self.request.nonce = 'zxc'
         self.auth = HybridGrant(request_validator=self.mock_validator)
         token = 'MOCKED_TOKEN'
         self.url_query = 'https://a.b/cb?code=abc&state=abc&id_token=%s' % token
         self.url_fragment = 'https://a.b/cb#code=abc&state=abc&id_token=%s' % token
 
+    @mock.patch('oauthlib.common.generate_token')
+    def test_required_nonce(self, generate_token):
+        generate_token.return_value = 'abc'
+        self.request.nonce = None
+        self.assertRaises(errors.InvalidRequestError, self.auth.validate_authorization_request, self.request)
+
+        bearer = BearerToken(self.mock_validator)
+        h, b, s = self.auth.create_authorization_response(self.request, bearer)
+        self.assertIn('error=invalid_request', h['Location'])
+        self.assertEqual(b, None)
+        self.assertEqual(s, 302)
+
 
 class OpenIDHybridCodeIdTokenTokenTest(OpenIDAuthCodeTest):
 
@@ -134,7 +167,20 @@ class OpenIDHybridCodeIdTokenTokenTest(OpenIDAuthCodeTest):
         super(OpenIDHybridCodeIdTokenTokenTest, self).setUp()
         self.mock_validator.get_code_challenge.return_value = None
         self.request.response_type = 'code id_token token'
+        self.request.nonce = 'xyz'
         self.auth = HybridGrant(request_validator=self.mock_validator)
         token = 'MOCKED_TOKEN'
         self.url_query = 'https://a.b/cb?code=abc&state=abc&token_type=Bearer&expires_in=3600&scope=hello+openid&access_token=abc&id_token=%s' % token
         self.url_fragment = 'https://a.b/cb#code=abc&state=abc&token_type=Bearer&expires_in=3600&scope=hello+openid&access_token=abc&id_token=%s' % token
+
+    @mock.patch('oauthlib.common.generate_token')
+    def test_required_nonce(self, generate_token):
+        generate_token.return_value = 'abc'
+        self.request.nonce = None
+        self.assertRaises(errors.InvalidRequestError, self.auth.validate_authorization_request, self.request)
+
+        bearer = BearerToken(self.mock_validator)
+        h, b, s = self.auth.create_authorization_response(self.request, bearer)
+        self.assertIn('error=invalid_request', h['Location'])
+        self.assertEqual(b, None)
+        self.assertEqual(s, 302)