Merge pull request #11182 from edx/mobile/access-token-messages-MA-1900

Provide useful response messages with access token errors

openedx/core/lib/api/authentication.py

-""" Common Authentication Handlers used across projects. """
+"""
+Common Authentication Handlers used across projects.
+"""
+
+
 from rest_framework.authentication import SessionAuthentication
+from rest_framework import exceptions as drf_exceptions
 from rest_framework_oauth.authentication import OAuth2Authentication
-from rest_framework.exceptions import AuthenticationFailed
+from .exceptions import AuthenticationFailed
 from rest_framework_oauth.compat import oauth2_provider, provider_now
 
+OAUTH2_TOKEN_ERROR = u'token_error'
+OAUTH2_TOKEN_ERROR_EXPIRED = u'token_expired'
+OAUTH2_TOKEN_ERROR_MALFORMED = u'token_malformed'
+OAUTH2_TOKEN_ERROR_NONEXISTENT = u'token_nonexistent'
+OAUTH2_TOKEN_ERROR_NOT_PROVIDED = u'token_not_provided'
+
 
 class SessionAuthenticationAllowInactiveUser(SessionAuthentication):
     """Ensure that the user is logged in, but do not require the account to be active.
@@ -65,19 +76,55 @@ class OAuth2AuthenticationAllowInactiveUser(OAuth2Authentication):
     This class can be used for an OAuth2-accessible endpoint that allows users to access
     that endpoint without having their email verified.  For example, this is used
     for mobile endpoints.
-
     """
+
+    def authenticate(self, *args, **kwargs):
+        """
+        Returns two-tuple of (user, token) if access token authentication
+        succeeds, raises an AuthenticationFailed (HTTP 401) if authentication
+        fails or None if the user did not try to authenticate using an access
+        token.
+
+        Overrides base class implementation to return edX-style error
+        responses.
+        """
+
+        try:
+            return super(OAuth2AuthenticationAllowInactiveUser, self).authenticate(*args, **kwargs)
+        except AuthenticationFailed:
+            # AuthenticationFailed is a subclass of drf_exceptions.AuthenticationFailed,
+            # but we don't want to post-process the exception detail for our own class.
+            raise
+        except drf_exceptions.AuthenticationFailed as exc:
+            if 'No credentials provided' in exc.detail:
+                error_code = OAUTH2_TOKEN_ERROR_NOT_PROVIDED
+            elif 'Token string should not contain spaces' in exc.detail:
+                error_code = OAUTH2_TOKEN_ERROR_MALFORMED
+            else:
+                error_code = OAUTH2_TOKEN_ERROR
+            raise AuthenticationFailed({
+                u'error_code': error_code,
+                u'developer_message': exc.detail
+            })
+
     def authenticate_credentials(self, request, access_token):
         """
         Authenticate the request, given the access token.
-        Override base class implementation to discard failure if user is inactive.
+        Overrides base class implementation to discard failure if user is inactive.
         """
-        try:
-            token = oauth2_provider.oauth2.models.AccessToken.objects.select_related('user')
-            # provider_now switches to timezone aware datetime when
-            # the oauth2_provider version supports to it.
-            token = token.get(token=access_token, expires__gt=provider_now())
-        except oauth2_provider.oauth2.models.AccessToken.DoesNotExist:
-            raise AuthenticationFailed('Invalid token')
-
-        return token.user, token
+        token_query = oauth2_provider.oauth2.models.AccessToken.objects.select_related('user')
+        token = token_query.filter(token=access_token).first()
+        if not token:
+            raise AuthenticationFailed({
+                u'error_code': OAUTH2_TOKEN_ERROR_NONEXISTENT,
+                u'developer_message': u'The provided access token does not match any valid tokens.'
+            })
+        # provider_now switches to timezone aware datetime when
+        # the oauth2_provider version supports it.
+        elif token.expires < provider_now():
+            raise AuthenticationFailed({
+                u'error_code': OAUTH2_TOKEN_ERROR_EXPIRED,
+                u'developer_message': u'The provided access token has expired and is no longer valid.',
+            })
+        else:
+            return token.user, token

openedx/core/lib/api/exceptions.py

+"""
+Custom exceptions, that allow details to be passed as dict values (which can be
+converted to JSON, like other API responses.
+"""
+
+from rest_framework import exceptions
+
+
+# TODO: Override Throttled, UnsupportedMediaType, ValidationError.  These types require
+# more careful handling of arguments.
+
+
+class _DictAPIException(exceptions.APIException):
+    """
+    Intermediate class to allow exceptions to pass dict detail values.  Use by
+    subclassing this along with another subclass of `exceptions.APIException`.
+    """
+    def __init__(self, detail):
+        if isinstance(detail, dict):
+            self.detail = detail
+        else:
+            super(_DictAPIException, self).__init__(detail)
+
+
+class AuthenticationFailed(exceptions.AuthenticationFailed, _DictAPIException):
+    """
+    Override of DRF's AuthenticationFailed exception to allow dictionary responses.
+    """
+    pass
+
+
+class MethodNotAllowed(exceptions.MethodNotAllowed, _DictAPIException):
+    """
+    Override of DRF's MethodNotAllowed exception to allow dictionary responses.
+    """
+    def __init__(self, method, detail=None):
+        if isinstance(detail, dict):
+            self.detail = detail
+        else:
+            super(MethodNotAllowed, self).__init__(method, detail)
+
+
+class NotAcceptable(exceptions.NotAcceptable, _DictAPIException):
+    """
+    Override of DRF's NotAcceptable exception to allow dictionary responses.
+    """
+
+    def __init__(self, detail=None, available_renderers=None):
+        self.available_renderers = available_renderers
+        if isinstance(detail, dict):
+            self.detail = detail
+        else:
+            super(NotAcceptable, self).__init__(detail, available_renderers)
+
+
+class NotAuthenticated(exceptions.NotAuthenticated, _DictAPIException):
+    """
+    Override of DRF's NotAuthenticated exception to allow dictionary responses.
+    """
+    pass
+
+
+class NotFound(exceptions.NotFound, _DictAPIException):
+    """
+    Override of DRF's NotFound exception to allow dictionary responses.
+    """
+    pass
+
+
+class ParseError(exceptions.ParseError, _DictAPIException):
+    """
+    Override of DRF's ParseError exception to allow dictionary responses.
+    """
+    pass
+
+
+class PermissionDenied(exceptions.PermissionDenied, _DictAPIException):
+    """
+    Override of DRF's PermissionDenied exception to allow dictionary responses.
+    """
+    pass

openedx/core/lib/api/tests/test_exceptions.py

+"""
+Test Custom Exceptions
+"""
+import ddt
+from django.test import TestCase
+from rest_framework import exceptions as drf_exceptions
+
+from .. import exceptions
+
+
+@ddt.ddt
+class TestDictExceptionsAllowDictDetails(TestCase):
+    """
+    Standard DRF exceptions coerce detail inputs to strings.  We want to use
+    dicts to allow better customization of error messages.  Demonstrate that
+    we can provide dictionaries as exception details, and that custom
+    classes subclass the relevant DRF exceptions, to provide consistent
+    exception catching behavior.
+    """
+
+    def test_drf_errors_coerce_strings(self):
+        # Demonstrate the base issue we are trying to solve.
+        exc = drf_exceptions.AuthenticationFailed({u'error_code': -1})
+        self.assertEqual(exc.detail, u"{u'error_code': -1}")
+
+    @ddt.data(
+        exceptions.AuthenticationFailed,
+        exceptions.NotAuthenticated,
+        exceptions.NotFound,
+        exceptions.ParseError,
+        exceptions.PermissionDenied,
+    )
+    def test_exceptions_allows_dict_detail(self, exception_class):
+        exc = exception_class({u'error_code': -1})
+        self.assertEqual(exc.detail, {u'error_code': -1})
+
+    def test_method_not_allowed_allows_dict_detail(self):
+        exc = exceptions.MethodNotAllowed(u'POST', {u'error_code': -1})
+        self.assertEqual(exc.detail, {u'error_code': -1})
+
+    def test_not_acceptable_allows_dict_detail(self):
+        exc = exceptions.NotAcceptable({u'error_code': -1}, available_renderers=['application/json'])
+        self.assertEqual(exc.detail, {u'error_code': -1})
+        self.assertEqual(exc.available_renderers, ['application/json'])
+
+
+@ddt.ddt
+class TestDictExceptionSubclassing(TestCase):
+    """
+    Custom exceptions should subclass standard DRF exceptions, so code that
+    catches the DRF exceptions also catches ours.
+    """
+
+    @ddt.data(
+        (exceptions.AuthenticationFailed, drf_exceptions.AuthenticationFailed),
+        (exceptions.NotAcceptable, drf_exceptions.NotAcceptable),
+        (exceptions.NotAuthenticated, drf_exceptions.NotAuthenticated),
+        (exceptions.NotFound, drf_exceptions.NotFound),
+        (exceptions.ParseError, drf_exceptions.ParseError),
+        (exceptions.PermissionDenied, drf_exceptions.PermissionDenied),
+    )
+    @ddt.unpack
+    def test_exceptions_subclass_drf_exceptions(self, exception_class, drf_exception_class):
+        exc = exception_class({u'error_code': -1})
+        self.assertIsInstance(exc, drf_exception_class)
+
+    def test_method_not_allowed_subclasses_drf_exception(self):
+        exc = exceptions.MethodNotAllowed(u'POST', {u'error_code': -1})
+        self.assertIsInstance(exc, drf_exceptions.MethodNotAllowed)