Refactor auth code client_id fallback auth

3 files changed, 46 insertions, 14 deletions

diff --git a/oauthlib/oauth2/draft25/grant_types.py b/oauthlib/oauth2/draft25/grant_types.py
index 43bddb1..17e80e1 100644
--- a/oauthlib/oauth2/draft25/grant_types.py
+++ b/oauthlib/oauth2/draft25/grant_types.py
@@ -45,6 +45,24 @@ class RequestValidator(object):
         """
         raise NotImplementedError('Subclasses must implement this method.')
 
+    def authenticate_client_id(self, client_id, request, *args, **kwargs):
+        """Ensure client_id belong to a non-confidential client.
+
+        A non-confidential client is one that is not required to authenticate
+        through other means, such as using HTTP Basic.
+
+        Note, while not strictly necessary it can often be very convenient
+        to set request.client to the client object associated with the
+        given client_id.
+
+        :param request: oauthlib.common.Request
+        :rtype: True or False
+
+        Method is used by:
+            - Authorization Code Grant
+        """
+        raise NotImplementedError('Subclasses must implement this method.')
+
     def confirm_redirect_uri(self, client_id, code, redirect_uri, client, *args, **kwargs):
         """Ensure client is authorized to redirect to the redirect_uri requested.
 
@@ -517,24 +535,19 @@ class AuthorizationCodeGrant(GrantTypeBase):
         # in Section 3.2.1.
         # http://tools.ietf.org/html/rfc6749#section-3.2.1
         if not self.request_validator.authenticate_client(request):
-            log.debug('Could not authenticate client, %r.', request)
-            raise errors.InvalidClientError()
+            # REQUIRED, if the client is not authenticating with the
+            # authorization server as described in Section 3.2.1.
+            # http://tools.ietf.org/html/rfc6749#section-3.2.1
+            if not self.request_validator.authenticate_client_id(
+                    request.client_id, request):
+                log.debug('Client authentication failed, %r.', request)
+                raise errors.InvalidClientError()
         else:
             if not hasattr(request.client, 'client_id'):
                 raise NotImplementedError('Authenticate client must set the '
                                           'request.client.client_id attribute '
                                           'in authenticate_client.')
 
-        # REQUIRED, if the client is not authenticating with the
-        # authorization server as described in Section 3.2.1.
-        # http://tools.ietf.org/html/rfc6749#section-3.2.1
-        if (not hasattr(request.client, 'client_id') and not
-                self.request_validator.validate_client_id(
-                    request.client_id, request)):
-            log.debug('Client_id not provided for unauthenticated client, %r.',
-                      request)
-            raise errors.InvalidClientError()
-
         # Ensure client is authorized use of this grant type
         self.validate_grant_type(request)
 
diff --git a/tests/oauth2/draft25/test_grant_types.py b/tests/oauth2/draft25/test_grant_types.py
index fc2b2b8..549644d 100644
--- a/tests/oauth2/draft25/test_grant_types.py
+++ b/tests/oauth2/draft25/test_grant_types.py
@@ -88,7 +88,7 @@ class AuthorizationCodeGrantTest(TestCase):
                 auth.validate_token_request, request)
 
         mock_validator.authenticate_client.return_value = False
-        mock_validator.validate_client_id.return_value = False
+        mock_validator.authenticate_client_id.return_value = False
         request.code = 'waffles'
         self.assertRaises(InvalidClientError,
                 auth.validate_token_request, request)
diff --git a/tests/oauth2/draft25/test_servers.py b/tests/oauth2/draft25/test_servers.py
index 64e1fb2..3f07e48 100644
--- a/tests/oauth2/draft25/test_servers.py
+++ b/tests/oauth2/draft25/test_servers.py
@@ -284,10 +284,25 @@ class ClientAuthenticationTest(TestCase):
         return True
 
     def test_client_id_authentication(self):
-        pass
+        token_uri = 'http://example.com/path'
+
+        # authorization code grant
+        self.validator.authenticate_client.return_value = False
+        self.validator.authenticate_client_id.return_value = False
+        _, _, body, _ = self.web.create_token_response(token_uri,
+                body='grant_type=authorization_code&code=mock')
+        self.assertEqual(json.loads(body)['error'], 'invalid_client')
+
+        self.validator.authenticate_client_id.return_value = True
+        self.validator.authenticate_client.side_effect = self.set_client
+        _, _, body, _ = self.web.create_token_response(token_uri,
+                body='grant_type=authorization_code&code=mock')
+        self.assertIn('access_token', json.loads(body))
 
     def test_custom_authentication(self):
         token_uri = 'http://example.com/path'
+
+        # authorization code grant
         self.assertRaises(NotImplementedError,
                 self.web.create_token_response, token_uri,
                 body='grant_type=authorization_code&code=mock')
@@ -297,6 +312,10 @@ class ClientAuthenticationTest(TestCase):
                 body='grant_type=authorization_code&code=mock')
         self.assertIn('access_token', json.loads(body))
 
+        # password grant
+
+        # client credentials grant
+
 
 class ResourceOwnerAssociationTest(TestCase):