Merge pull request #667 from Abhishek8394/sanitize-get

Fix Issue #666: ban 'client_secret' and 'code_verifier' from url query params

oauthlib/oauth2/rfc6749/endpoints/base.py

@@ -15,6 +15,8 @@
                       TemporarilyUnavailableError, InvalidRequestError,
                       InvalidClientError, UnsupportedTokenTypeError)
 
+from oauthlib.common import CaseInsensitiveDict, urldecode
+
 log = logging.getLogger(__name__)
 
 
@@ -23,14 +25,26 @@ class BaseEndpoint(object):
     def __init__(self):
         self._available = True
         self._catch_errors = False
+        self._valid_request_methods = None
+
+    @property
+    def valid_request_methods(self):
+        return self._valid_request_methods
+
+    @valid_request_methods.setter
+    def valid_request_methods(self, valid_request_methods):
+        if valid_request_methods is not None:
+            valid_request_methods = [x.upper() for x in valid_request_methods]
+        self._valid_request_methods = valid_request_methods
+
 
     @property
     def available(self):
         return self._available
 
     @available.setter
     def available(self, available):
-        self._available = available
+        self._available = available       
 
     @property
     def catch_errors(self):
@@ -62,6 +76,21 @@ def _raise_on_unsupported_token(self, request):
             request.token_type_hint not in self.supported_token_types):
             raise UnsupportedTokenTypeError(request=request)
 
+    def _raise_on_bad_method(self, request):
+        if self.valid_request_methods is None:
+            raise ValueError('Configure "valid_request_methods" property first')
+        if request.http_method.upper() not in self.valid_request_methods:
+            raise InvalidRequestError(request=request,
+                                      description=('Unsupported request method %s' % request.http_method.upper()))
+
+    def _raise_on_bad_post_request(self, request):
+        """Raise if invalid POST request received
+        """
+        if request.http_method.upper() == 'POST':
+            query_params = request.uri_query or ""
+            if query_params:
+                raise InvalidRequestError(request=request,
+                                          description=('URL query parameters are not allowed'))
 
 def catch_errors_and_unavailability(f):
     @functools.wraps(f)

oauthlib/oauth2/rfc6749/endpoints/introspect.py

@@ -39,6 +39,7 @@ class IntrospectEndpoint(BaseEndpoint):
    """
 
     valid_token_types = ('access_token', 'refresh_token')
+    valid_request_methods = ('POST',)
 
     def __init__(self, request_validator, supported_token_types=None):
         BaseEndpoint.__init__(self)
@@ -117,6 +118,8 @@ def validate_introspect_request(self, request):
         .. _`section 1.5`: http://tools.ietf.org/html/rfc6749#section-1.5
         .. _`RFC6749`: http://tools.ietf.org/html/rfc6749
         """
+        self._raise_on_bad_method(request)
+        self._raise_on_bad_post_request(request)
         self._raise_on_missing_token(request)
         self._raise_on_invalid_client(request)
         self._raise_on_unsupported_token(request)

oauthlib/oauth2/rfc6749/endpoints/revocation.py

@@ -28,6 +28,7 @@ class RevocationEndpoint(BaseEndpoint):
     """
 
     valid_token_types = ('access_token', 'refresh_token')
+    valid_request_methods = ('POST',)
 
     def __init__(self, request_validator, supported_token_types=None,
             enable_jsonp=False):
@@ -121,6 +122,8 @@ def validate_revocation_request(self, request):
         .. _`Section 4.1.2`: https://tools.ietf.org/html/draft-ietf-oauth-revocation-11#section-4.1.2
         .. _`RFC6749`: https://tools.ietf.org/html/rfc6749
         """
+        self._raise_on_bad_method(request)
+        self._raise_on_bad_post_request(request)
         self._raise_on_missing_token(request)
         self._raise_on_invalid_client(request)
         self._raise_on_unsupported_token(request)

oauthlib/oauth2/rfc6749/endpoints/token.py

@@ -62,6 +62,8 @@ class TokenEndpoint(BaseEndpoint):
     .. _`Appendix B`: https://tools.ietf.org/html/rfc6749#appendix-B
     """
 
+    valid_request_methods = ('POST',)
+
     def __init__(self, default_grant_type, default_token_type, grant_types):
         BaseEndpoint.__init__(self)
         self._grant_types = grant_types
@@ -85,13 +87,13 @@ def default_token_type(self):
         return self._default_token_type
 
     @catch_errors_and_unavailability
-    def create_token_response(self, uri, http_method='GET', body=None,
+    def create_token_response(self, uri, http_method='POST', body=None,
                               headers=None, credentials=None, grant_type_for_scope=None,
                               claims=None):
         """Extract grant_type and route to the designated handler."""
         request = Request(
             uri, http_method=http_method, body=body, headers=headers)
-
+        self.validate_token_request(request)
         # 'scope' is an allowed Token Request param in both the "Resource Owner Password Credentials Grant"
         # and "Client Credentials Grant" flows
         # https://tools.ietf.org/html/rfc6749#section-4.3.2
@@ -115,3 +117,7 @@ def create_token_response(self, uri, http_method='GET', body=None,
                   request.grant_type, grant_type_handler)
         return grant_type_handler.create_token_response(
             request, self.default_token_type)
+
+    def validate_token_request(self, request):
+        self._raise_on_bad_method(request)
+        self._raise_on_bad_post_request(request)

tests/oauth2/rfc6749/endpoints/test_base_endpoint.py

@@ -25,7 +25,7 @@ def test_error_catching(self):
         server = Server(validator)
         server.catch_errors = True
         h, b, s = server.create_token_response(
-            'https://example.com?grant_type=authorization_code&code=abc'
+            'https://example.com', body='grant_type=authorization_code&code=abc'
         )
         self.assertIn("server_error", b)
         self.assertEqual(s, 500)

tests/oauth2/rfc6749/endpoints/test_error_responses.py

@@ -6,11 +6,11 @@
 
 import mock
 
+from oauthlib.common import urlencode
 from oauthlib.oauth2 import (BackendApplicationServer, LegacyApplicationServer,
                              MobileApplicationServer, RequestValidator,
                              WebApplicationServer)
 from oauthlib.oauth2.rfc6749 import errors
-
 from ....unittest import TestCase
 
 
@@ -437,3 +437,56 @@ def test_unsupported_grant_type(self):
         _, body, _ = self.backend.create_token_response('https://i.b/token',
                 body='grant_type=bar')
         self.assertEqual('unsupported_grant_type', json.loads(body)['error'])
+
+    def test_invalid_request_method(self):
+        test_methods = ['GET', 'pUt', 'dEleTe', 'paTcH']
+        test_methods = test_methods + [x.lower() for x in test_methods] + [x.upper() for x in test_methods]
+        for method in test_methods:
+            self.validator.authenticate_client.side_effect = self.set_client
+
+            uri = "http://i/b/token/"
+            try:
+                _, body, s = self.web.create_token_response(uri,
+                        body='grant_type=access_token&code=123', http_method=method)
+                self.fail('This should have failed with InvalidRequestError')
+            except errors.InvalidRequestError as ire:
+                self.assertIn('Unsupported request method', ire.description)
+
+            try:
+                _, body, s = self.legacy.create_token_response(uri,
+                        body='grant_type=access_token&code=123', http_method=method)
+                self.fail('This should have failed with InvalidRequestError')
+            except errors.InvalidRequestError as ire:
+                self.assertIn('Unsupported request method', ire.description)
+
+            try:
+                _, body, s = self.backend.create_token_response(uri,
+                        body='grant_type=access_token&code=123', http_method=method)
+                self.fail('This should have failed with InvalidRequestError')
+            except errors.InvalidRequestError as ire:
+                self.assertIn('Unsupported request method', ire.description)
+
+    def test_invalid_post_request(self):
+        self.validator.authenticate_client.side_effect = self.set_client
+        for param in ['token', 'secret', 'code', 'foo']:
+            uri = 'https://i/b/token?' + urlencode([(param, 'secret')])
+            try:
+                _, body, s = self.web.create_token_response(uri,
+                        body='grant_type=access_token&code=123')
+                self.fail('This should have failed with InvalidRequestError')
+            except errors.InvalidRequestError as ire:
+                self.assertIn('URL query parameters are not allowed', ire.description)
+
+            try:
+                _, body, s = self.legacy.create_token_response(uri,
+                        body='grant_type=access_token&code=123')
+                self.fail('This should have failed with InvalidRequestError')
+            except errors.InvalidRequestError as ire:
+                self.assertIn('URL query parameters are not allowed', ire.description)
+
+            try:
+                _, body, s = self.backend.create_token_response(uri,
+                        body='grant_type=access_token&code=123')
+                self.fail('This should have failed with InvalidRequestError')
+            except errors.InvalidRequestError as ire:
+                self.assertIn('URL query parameters are not allowed', ire.description)

tests/oauth2/rfc6749/endpoints/test_introspect_endpoint.py

@@ -139,3 +139,33 @@ def test_introspect_unsupported_token(self):
         self.assertEqual(h, self.resp_h)
         self.assertEqual(loads(b)['error'], 'invalid_request')
         self.assertEqual(s, 400)
+
+    def test_introspect_invalid_request_method(self):
+        endpoint = IntrospectEndpoint(self.validator,
+                                      supported_token_types=['access_token'])
+        test_methods = ['GET', 'pUt', 'dEleTe', 'paTcH']
+        test_methods = test_methods + [x.lower() for x in test_methods] + [x.upper() for x in test_methods]
+        for method in test_methods:
+            body = urlencode([('token', 'foo'),
+                              ('token_type_hint', 'refresh_token')])
+            h, b, s = endpoint.create_introspect_response(self.uri,
+                    http_method = method, headers=self.headers, body=body)
+            self.assertEqual(h, self.resp_h)
+            self.assertEqual(loads(b)['error'], 'invalid_request')
+            self.assertIn('Unsupported request method', loads(b)['error_description'])
+            self.assertEqual(s, 400)
+
+    def test_introspect_bad_post_request(self):
+        endpoint = IntrospectEndpoint(self.validator,
+                                      supported_token_types=['access_token'])
+        for param in ['token', 'secret', 'code', 'foo']:
+            uri = 'http://some.endpoint?' + urlencode([(param, 'secret')])
+            body = urlencode([('token', 'foo'),
+                              ('token_type_hint', 'access_token')])
+            h, b, s = endpoint.create_introspect_response(
+                uri,
+                headers=self.headers, body=body)
+            self.assertEqual(h, self.resp_h)
+            self.assertEqual(loads(b)['error'], 'invalid_request')
+            self.assertIn('query parameters are not allowed', loads(b)['error_description'])
+            self.assertEqual(s, 400)

tests/oauth2/rfc6749/endpoints/test_revocation_endpoint.py

@@ -120,3 +120,32 @@ def test_revoke_unsupported_token(self):
         self.assertEqual(h, self.resp_h)
         self.assertEqual(loads(b)['error'], 'invalid_request')
         self.assertEqual(s, 400)
+
+    def test_revoke_invalid_request_method(self):
+        endpoint = RevocationEndpoint(self.validator,
+                                      supported_token_types=['access_token'])
+        test_methods = ['GET', 'pUt', 'dEleTe', 'paTcH']
+        test_methods = test_methods + [x.lower() for x in test_methods] + [x.upper() for x in test_methods]
+        for method in test_methods:
+            body = urlencode([('token', 'foo'),
+                              ('token_type_hint', 'refresh_token')])
+            h, b, s = endpoint.create_revocation_response(self.uri,
+                    http_method = method, headers=self.headers, body=body)
+            self.assertEqual(h, self.resp_h)
+            self.assertEqual(loads(b)['error'], 'invalid_request')
+            self.assertIn('Unsupported request method', loads(b)['error_description'])
+            self.assertEqual(s, 400)
+
+    def test_revoke_bad_post_request(self):
+        endpoint = RevocationEndpoint(self.validator,
+                                      supported_token_types=['access_token'])
+        for param in ['token', 'secret', 'code', 'foo']:
+            uri = 'http://some.endpoint?' + urlencode([(param, 'secret')])
+            body = urlencode([('token', 'foo'),
+                              ('token_type_hint', 'access_token')])
+            h, b, s = endpoint.create_revocation_response(uri,
+                    headers=self.headers, body=body)
+            self.assertEqual(h, self.resp_h)
+            self.assertEqual(loads(b)['error'], 'invalid_request')
+            self.assertIn('query parameters are not allowed', loads(b)['error_description'])
+            self.assertEqual(s, 400)

tox.ini

@@ -28,6 +28,7 @@ commands=
         echo setup.py/long description is syntaxly correct
 
 [testenv:bandit]
+basepython=python2.7
 skipsdist=True
 deps=bandit
 commands=bandit -b bandit.json -r oauthlib/