Merge pull request #358 from garciasolero/develop

Fix management of rfc6749 errors

AUTHORS

@@ -22,3 +22,4 @@ Tyler Jones (squirly)
 Massimiliano Pippi
 Josh Turmel
 David Baumgold
+Juan Fabio García Solero

oauthlib/oauth2/rfc6749/errors.py

@@ -122,24 +122,32 @@ class FatalClientError(OAuth2Error):
     pass
 
 
-class InvalidRedirectURIError(FatalClientError):
-    error = 'invalid_redirect_uri'
+class InvalidRequestFatalError(FatalClientError):
+    """For fatal errors, the request is missing a required parameter, includes
+    an invalid parameter value, includes a parameter more than once, or is
+    otherwise malformed.
+    """
+    error = 'invalid_request'
+
 
+class InvalidRedirectURIError(InvalidRequestFatalError):
+    description = 'Invalid redirect URI.'
 
-class MissingRedirectURIError(FatalClientError):
-    error = 'missing_redirect_uri'
 
+class MissingRedirectURIError(InvalidRequestFatalError):
+    description = 'Missing redirect URI.'
 
-class MismatchingRedirectURIError(FatalClientError):
-    error = 'mismatching_redirect_uri'
 
+class MismatchingRedirectURIError(InvalidRequestFatalError):
+    description = 'Mismatching redirect URI.'
 
-class MissingClientIdError(FatalClientError):
-    error = 'invalid_client_id'
 
+class InvalidClientIdError(InvalidRequestFatalError):
+    description = 'Invalid client_id parameter value.'
 
-class InvalidClientIdError(FatalClientError):
-    error = 'invalid_client_id'
+
+class MissingClientIdError(InvalidRequestFatalError):
+    description = 'Missing client_id parameter.'
 
 
 class InvalidRequestError(OAuth2Error):
@@ -151,6 +159,10 @@ class InvalidRequestError(OAuth2Error):
     error = 'invalid_request'
 
 
+class MissingResponseTypeError(InvalidRequestError):
+    description = 'Missing response_type parameter.'
+
+
 class AccessDeniedError(OAuth2Error):
 
     """The resource owner or authorization server denied the request."""

oauthlib/oauth2/rfc6749/grant_types/authorization_code.py

@@ -264,6 +264,15 @@ def validate_authorization_request(self, request):
         # error and MUST NOT automatically redirect the user-agent to the
         # invalid redirection URI.
 
+        # First check duplicate parameters
+        for param in ('client_id', 'response_type', 'redirect_uri', 'scope', 'state'):
+            try:
+                duplicate_params = request.duplicate_params
+            except ValueError:
+                raise errors.InvalidRequestFatalError(description='Unable to parse query string', request=request)
+            if param in duplicate_params:
+                raise errors.InvalidRequestFatalError(description='Duplicate %s parameter.' % param, request=request)
+
         # REQUIRED. The client identifier as described in Section 2.2.
         # http://tools.ietf.org/html/rfc6749#section-2.2
         if not request.client_id:
@@ -304,27 +313,22 @@ def validate_authorization_request(self, request):
 
         # Note that the correct parameters to be added are automatically
         # populated through the use of specific exceptions.
-        if request.response_type is None:
-            raise errors.InvalidRequestError(description='Missing response_type parameter.', request=request)
 
-        for param in ('client_id', 'response_type', 'redirect_uri', 'scope', 'state'):
-            try:
-                duplicate_params = request.duplicate_params
-            except ValueError:
-                raise errors.InvalidRequestError(description='Unable to parse query string', request=request)
-            if param in duplicate_params:
-                raise errors.InvalidRequestError(description='Duplicate %s parameter.' % param, request=request)
+        # REQUIRED.
+        if request.response_type is None:
+            raise errors.MissingResponseTypeError(request=request)
+        # Value MUST be set to "code".
+        elif request.response_type != 'code':
+            raise errors.UnsupportedResponseTypeError(request=request)
 
         if not self.request_validator.validate_response_type(request.client_id,
-                                                             request.response_type, request.client, request):
+                                                             request.response_type,
+                                                             request.client, request):
+
             log.debug('Client %s is not authorized to use response_type %s.',
                       request.client_id, request.response_type)
             raise errors.UnauthorizedClientError(request=request)
 
-        # REQUIRED. Value MUST be set to "code".
-        if request.response_type != 'code':
-            raise errors.UnsupportedResponseTypeError(request=request)
-
         # OPTIONAL. The scope of the access request as described by Section 3.3
         # http://tools.ietf.org/html/rfc6749#section-3.3
         self.validate_scopes(request)

oauthlib/oauth2/rfc6749/grant_types/implicit.py

@@ -260,6 +260,15 @@ def validate_token_request(self, request):
         # error and MUST NOT automatically redirect the user-agent to the
         # invalid redirection URI.
 
+        # First check duplicate parameters
+        for param in ('client_id', 'response_type', 'redirect_uri', 'scope', 'state'):
+            try:
+                duplicate_params = request.duplicate_params
+            except ValueError:
+                raise errors.InvalidRequestFatalError(description='Unable to parse query string', request=request)
+            if param in duplicate_params:
+                raise errors.InvalidRequestFatalError(description='Duplicate %s parameter.' % param, request=request)
+
         # REQUIRED. The client identifier as described in Section 2.2.
         # http://tools.ietf.org/html/rfc6749#section-2.2
         if not request.client_id:
@@ -304,27 +313,21 @@ def validate_token_request(self, request):
         # http://tools.ietf.org/html/rfc6749#appendix-B
 
         # Note that the correct parameters to be added are automatically
-        # populated through the use of specific exceptions.
-        if request.response_type is None:
-            raise errors.InvalidRequestError(description='Missing response_type parameter.',
-                                             request=request)
+        # populated through the use of specific exceptions
 
-        for param in ('client_id', 'response_type', 'redirect_uri', 'scope', 'state'):
-            try:
-                duplicate_params = request.duplicate_params
-            except ValueError:
-                raise errors.InvalidRequestError(description='Unable to parse query string', request=request)
-            if param in duplicate_params:
-                raise errors.InvalidRequestError(description='Duplicate %s parameter.' % param, request=request)
-
-        # REQUIRED. Value MUST be set to "token".
-        if request.response_type != 'token':
+        # REQUIRED.
+        if request.response_type is None:
+            raise errors.MissingResponseTypeError(request=request)
+        # Value MUST be set to "token".
+        elif request.response_type != 'token':
             raise errors.UnsupportedResponseTypeError(request=request)
 
         log.debug('Validating use of response_type token for client %r (%r).',
                   request.client_id, request.client)
         if not self.request_validator.validate_response_type(request.client_id,
-                                                             request.response_type, request.client, request):
+                                                             request.response_type,
+                                                             request.client, request):
+
             log.debug('Client %s is not authorized to use response_type %s.',
                       request.client_id, request.response_type)
             raise errors.UnauthorizedClientError(request=request)

tests/oauth2/rfc6749/endpoints/test_error_responses.py

@@ -28,109 +28,98 @@ def setUp(self):
         self.backend = BackendApplicationServer(self.validator)
 
     def test_invalid_redirect_uri(self):
-        uri = 'https://example.com/authorize?client_id=foo&redirect_uri=wrong'
+        uri = 'https://example.com/authorize?response_type={0}&client_id=foo&redirect_uri=wrong'
+
         # Authorization code grant
         self.assertRaises(errors.InvalidRedirectURIError,
-                self.web.validate_authorization_request, uri)
+                self.web.validate_authorization_request, uri.format('code'))
         self.assertRaises(errors.InvalidRedirectURIError,
-                self.web.create_authorization_response, uri, scopes=['foo'])
+                self.web.create_authorization_response, uri.format('code'), scopes=['foo'])
 
         # Implicit grant
         self.assertRaises(errors.InvalidRedirectURIError,
-                self.mobile.validate_authorization_request, uri)
+                self.mobile.validate_authorization_request, uri.format('token'))
         self.assertRaises(errors.InvalidRedirectURIError,
-                self.mobile.create_authorization_response, uri, scopes=['foo'])
+                self.mobile.create_authorization_response, uri.format('token'), scopes=['foo'])
 
     def test_missing_redirect_uri(self):
-        uri = 'https://example.com/authorize?client_id=foo'
+        uri = 'https://example.com/authorize?response_type={0}&client_id=foo'
+
         # Authorization code grant
         self.assertRaises(errors.MissingRedirectURIError,
-                self.web.validate_authorization_request, uri)
+                self.web.validate_authorization_request, uri.format('code'))
         self.assertRaises(errors.MissingRedirectURIError,
-                self.web.create_authorization_response, uri, scopes=['foo'])
+                self.web.create_authorization_response, uri.format('code'), scopes=['foo'])
 
         # Implicit grant
         self.assertRaises(errors.MissingRedirectURIError,
-                self.mobile.validate_authorization_request, uri)
+                self.mobile.validate_authorization_request, uri.format('token'))
         self.assertRaises(errors.MissingRedirectURIError,
-                self.mobile.create_authorization_response, uri, scopes=['foo'])
+                self.mobile.create_authorization_response, uri.format('token'), scopes=['foo'])
 
     def test_mismatching_redirect_uri(self):
-        uri = 'https://example.com/authorize?client_id=foo&redirect_uri=https%3A%2F%2Fi.b%2Fback'
+        uri = 'https://example.com/authorize?response_type={0}&client_id=foo&redirect_uri=https%3A%2F%2Fi.b%2Fback'
+
         # Authorization code grant
         self.validator.validate_redirect_uri.return_value = False
         self.assertRaises(errors.MismatchingRedirectURIError,
-                self.web.validate_authorization_request, uri)
+                self.web.validate_authorization_request, uri.format('code'))
         self.assertRaises(errors.MismatchingRedirectURIError,
-                self.web.create_authorization_response, uri, scopes=['foo'])
+                self.web.create_authorization_response, uri.format('code'), scopes=['foo'])
 
         # Implicit grant
         self.assertRaises(errors.MismatchingRedirectURIError,
-                self.mobile.validate_authorization_request, uri)
+                self.mobile.validate_authorization_request, uri.format('token'))
         self.assertRaises(errors.MismatchingRedirectURIError,
-                self.mobile.create_authorization_response, uri, scopes=['foo'])
+                self.mobile.create_authorization_response, uri.format('token'), scopes=['foo'])
 
     def test_missing_client_id(self):
-        uri = 'https://example.com/authorize?redirect_uri=https%3A%2F%2Fi.b%2Fback'
+        uri = 'https://example.com/authorize?response_type={0}&redirect_uri=https%3A%2F%2Fi.b%2Fback'
+
         # Authorization code grant
         self.validator.validate_redirect_uri.return_value = False
         self.assertRaises(errors.MissingClientIdError,
-                self.web.validate_authorization_request, uri)
+                self.web.validate_authorization_request, uri.format('code'))
         self.assertRaises(errors.MissingClientIdError,
-                self.web.create_authorization_response, uri, scopes=['foo'])
+                self.web.create_authorization_response, uri.format('code'), scopes=['foo'])
 
         # Implicit grant
         self.assertRaises(errors.MissingClientIdError,
-                self.mobile.validate_authorization_request, uri)
+                self.mobile.validate_authorization_request, uri.format('token'))
         self.assertRaises(errors.MissingClientIdError,
-                self.mobile.create_authorization_response, uri, scopes=['foo'])
+                self.mobile.create_authorization_response, uri.format('token'), scopes=['foo'])
 
     def test_invalid_client_id(self):
-        uri = 'https://example.com/authorize?client_id=foo&redirect_uri=https%3A%2F%2Fi.b%2Fback'
+        uri = 'https://example.com/authorize?response_type={0}&client_id=foo&redirect_uri=https%3A%2F%2Fi.b%2Fback'
+
         # Authorization code grant
         self.validator.validate_client_id.return_value = False
         self.assertRaises(errors.InvalidClientIdError,
-                self.web.validate_authorization_request, uri)
+                self.web.validate_authorization_request, uri.format('code'))
         self.assertRaises(errors.InvalidClientIdError,
-                self.web.create_authorization_response, uri, scopes=['foo'])
+                self.web.create_authorization_response, uri.format('code'), scopes=['foo'])
 
         # Implicit grant
         self.assertRaises(errors.InvalidClientIdError,
-                self.mobile.validate_authorization_request, uri)
+                self.mobile.validate_authorization_request, uri.format('token'))
         self.assertRaises(errors.InvalidClientIdError,
-                self.mobile.create_authorization_response, uri, scopes=['foo'])
+                self.mobile.create_authorization_response, uri.format('token'), scopes=['foo'])
 
     def test_empty_parameter(self):
         uri = 'https://example.com/authorize?client_id=foo&redirect_uri=https%3A%2F%2Fi.b%2Fback&response_type=code&'
 
         # Authorization code grant
-        self.assertRaises(errors.InvalidRequestError,
+        self.assertRaises(errors.InvalidRequestFatalError,
                 self.web.validate_authorization_request, uri)
 
         # Implicit grant
-        self.assertRaises(errors.InvalidRequestError,
+        self.assertRaises(errors.InvalidRequestFatalError,
                 self.mobile.validate_authorization_request, uri)
 
     def test_invalid_request(self):
         self.validator.get_default_redirect_uri.return_value = 'https://i.b/cb'
         token_uri = 'https://i.b/token'
-        invalid_uris = [
-            # Duplicate parameters
-            'https://i.b/auth?client_id=foo&client_id=bar&response_type={0}',
-            # Missing response type
-            'https://i.b/auth?client_id=foo',
-        ]
 
-        # Authorization code grant
-        for uri in invalid_uris:
-            self.assertRaises(errors.InvalidRequestError,
-                    self.web.validate_authorization_request,
-                    uri.format('code'))
-            h, _, s = self.web.create_authorization_response(
-                    uri.format('code'), scopes=['foo'])
-            self.assertEqual(s, 302)
-            self.assertIn('Location', h)
-            self.assertIn('error=invalid_request', h['Location'])
         invalid_bodies = [
             # duplicate params
             'grant_type=authorization_code&client_id=nope&client_id=nope&code=foo'
@@ -140,17 +129,6 @@ def test_invalid_request(self):
                     body=body)
             self.assertEqual('invalid_request', json.loads(body)['error'])
 
-        # Implicit grant
-        for uri in invalid_uris:
-            self.assertRaises(errors.InvalidRequestError,
-                    self.mobile.validate_authorization_request,
-                    uri.format('token'))
-            h, _, s = self.mobile.create_authorization_response(
-                    uri.format('token'), scopes=['foo'])
-            self.assertEqual(s, 302)
-            self.assertIn('Location', h)
-            self.assertIn('error=invalid_request', h['Location'])
-
         # Password credentials grant
         invalid_bodies = [
             # duplicate params
@@ -176,6 +154,55 @@ def test_invalid_request(self):
                     body=body)
             self.assertEqual('invalid_request', json.loads(body)['error'])
 
+    def test_invalid_request_duplicate_params(self):
+        self.validator.get_default_redirect_uri.return_value = 'https://i.b/cb'
+        uri = 'https://i.b/auth?client_id=foo&client_id=bar&response_type={0}'
+        description = 'Duplicate client_id parameter.'
+
+        # Authorization code
+        self.assertRaisesRegexp(errors.InvalidRequestFatalError,
+                              description,
+                              self.web.validate_authorization_request,
+                              uri.format('code'))
+        self.assertRaisesRegexp(errors.InvalidRequestFatalError,
+                              description,
+                              self.web.create_authorization_response,
+                              uri.format('code'), scopes=['foo'])
+
+        # Implicit grant
+        self.assertRaisesRegexp(errors.InvalidRequestFatalError,
+                              description,
+                              self.mobile.validate_authorization_request,
+                              uri.format('token'))
+        self.assertRaisesRegexp(errors.InvalidRequestFatalError,
+                              description,
+                              self.mobile.create_authorization_response,
+                              uri.format('token'), scopes=['foo'])
+
+    def test_invalid_request_missing_response_type(self):
+
+        self.validator.get_default_redirect_uri.return_value = 'https://i.b/cb'
+
+        uri = 'https://i.b/auth?client_id=foo'
+
+        # Authorization code
+        self.assertRaises(errors.MissingResponseTypeError,
+                          self.web.validate_authorization_request,
+                          uri.format('code'))
+        h, _, s = self.web.create_authorization_response(uri, scopes=['foo'])
+        self.assertEqual(s, 302)
+        self.assertIn('Location', h)
+        self.assertIn('error=invalid_request', h['Location'])
+
+        # Implicit grant
+        self.assertRaises(errors.MissingResponseTypeError,
+                          self.mobile.validate_authorization_request,
+                          uri.format('token'))
+        h, _, s = self.mobile.create_authorization_response(uri, scopes=['foo'])
+        self.assertEqual(s, 302)
+        self.assertIn('Location', h)
+        self.assertIn('error=invalid_request', h['Location'])
+
     def test_unauthorized_client(self):
         self.validator.get_default_redirect_uri.return_value = 'https://i.b/cb'
         self.validator.validate_grant_type.return_value = False

tests/oauth2/rfc6749/endpoints/test_extra_credentials.py

@@ -41,7 +41,7 @@ def save_token(token, request):
 
         # Implicit grant
         self.validator.save_bearer_token.side_effect = save_token
-        self.web.create_authorization_response(
+        self.mobile.create_authorization_response(
                 'https://i.b/auth?client_id=foo&response_type=token',
                 scopes=['foo'],
                 credentials={'extra': 'creds'})