diff --git a/common/djangoapps/enrollment/tests/test_views.py b/common/djangoapps/enrollment/tests/test_views.py index f353dd8ff0..69b4451b9c 100644 --- a/common/djangoapps/enrollment/tests/test_views.py +++ b/common/djangoapps/enrollment/tests/test_views.py @@ -147,7 +147,7 @@ class EnrollmentTest(ModuleStoreTestCase, APITestCase): self.client.logout() # Try to enroll, this should fail. - self._create_enrollment(expected_status=status.HTTP_403_FORBIDDEN) + self._create_enrollment(expected_status=status.HTTP_401_UNAUTHORIZED) def test_user_not_activated(self): # Log out the default user, Bob. diff --git a/common/djangoapps/enrollment/views.py b/common/djangoapps/enrollment/views.py index a1f655c5b8..194b1c19dc 100644 --- a/common/djangoapps/enrollment/views.py +++ b/common/djangoapps/enrollment/views.py @@ -9,7 +9,6 @@ from opaque_keys import InvalidKeyError from opaque_keys.edx.locator import CourseLocator from openedx.core.djangoapps.user_api import api as user_api from rest_framework import status -from rest_framework.authentication import OAuth2Authentication from rest_framework import permissions from rest_framework.response import Response from rest_framework.throttling import UserRateThrottle @@ -19,7 +18,7 @@ from opaque_keys import InvalidKeyError from enrollment import api from enrollment.errors import CourseNotFoundError, CourseEnrollmentError, CourseModeNotFoundError from embargo import api as embargo_api -from util.authentication import SessionAuthenticationAllowInactiveUser +from util.authentication import SessionAuthenticationAllowInactiveUser, OAuth2AuthenticationAllowInactiveUser from util.disable_rate_limit import can_disable_rate_limit @@ -71,7 +70,7 @@ class EnrollmentView(APIView): * user: The ID of the user. """ - authentication_classes = OAuth2Authentication, SessionAuthenticationAllowInactiveUser + authentication_classes = OAuth2AuthenticationAllowInactiveUser, SessionAuthenticationAllowInactiveUser permission_classes = permissions.IsAuthenticated, throttle_classes = EnrollmentUserThrottle, @@ -243,7 +242,7 @@ class EnrollmentListView(APIView): * user: The ID of the user. """ - authentication_classes = OAuth2Authentication, SessionAuthenticationAllowInactiveUser + authentication_classes = OAuth2AuthenticationAllowInactiveUser, SessionAuthenticationAllowInactiveUser permission_classes = permissions.IsAuthenticated, throttle_classes = EnrollmentUserThrottle, diff --git a/common/djangoapps/util/authentication.py b/common/djangoapps/util/authentication.py index 901f3ad6e8..92f46861c4 100644 --- a/common/djangoapps/util/authentication.py +++ b/common/djangoapps/util/authentication.py @@ -1,5 +1,7 @@ """ Common Authentication Handlers used across projects. """ from rest_framework import authentication +from rest_framework.exceptions import AuthenticationFailed +from rest_framework.compat import oauth2_provider, provider_now class SessionAuthenticationAllowInactiveUser(authentication.SessionAuthentication): @@ -39,10 +41,42 @@ class SessionAuthenticationAllowInactiveUser(authentication.SessionAuthenticatio # Unauthenticated, CSRF validation not required # This is where regular `SessionAuthentication` checks that the user is active. # We have removed that check in this implementation. - if not user: + # But we added a check to prevent anonymous users since we require a logged-in account. + if not user or user.is_anonymous(): return None self.enforce_csrf(request) # CSRF passed with authenticated user return (user, None) + + +class OAuth2AuthenticationAllowInactiveUser(authentication.OAuth2Authentication): + """ + This is a temporary workaround while the is_active field on the user is coupled + with whether or not the user has verified ownership of their claimed email address. + Once is_active is decoupled from verified_email, we will no longer need this + class override. + + But until then, this authentication class ensures that the user is logged in, + but does not require that their account "is_active". + + 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_credentials(self, request, access_token): + """ + Authenticate the request, given the access token. + Override 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 diff --git a/common/djangoapps/util/tests/test_authentication.py b/common/djangoapps/util/tests/test_authentication.py new file mode 100644 index 0000000000..fe9a1dbd47 --- /dev/null +++ b/common/djangoapps/util/tests/test_authentication.py @@ -0,0 +1,63 @@ +"""Tests for util.authentication module.""" + +from mock import patch +from django.conf import settings +from rest_framework import permissions +from rest_framework.compat import patterns, url +from rest_framework.tests import test_authentication +from provider import scope, constants +from unittest import skipUnless + +from ..authentication import OAuth2AuthenticationAllowInactiveUser + + +class OAuth2AuthAllowInactiveUserDebug(OAuth2AuthenticationAllowInactiveUser): + """ + A debug class analogous to the OAuth2AuthenticationDebug class that tests + the OAuth2 flow with the access token sent in a query param.""" + allow_query_params_token = True + + +# The following patch overrides the URL patterns for the MockView class used in +# rest_framework.tests.test_authentication so that the corresponding AllowInactiveUser +# classes are tested instead. +@skipUnless(settings.FEATURES.get('ENABLE_OAUTH2_PROVIDER'), 'OAuth2 not enabled') +@patch.object( + test_authentication, + 'urlpatterns', + patterns( + '', + url( + r'^oauth2-test/$', + test_authentication.MockView.as_view(authentication_classes=[OAuth2AuthenticationAllowInactiveUser]) + ), + url( + r'^oauth2-test-debug/$', + test_authentication.MockView.as_view(authentication_classes=[OAuth2AuthAllowInactiveUserDebug]) + ), + url( + r'^oauth2-with-scope-test/$', + test_authentication.MockView.as_view( + authentication_classes=[OAuth2AuthenticationAllowInactiveUser], + permission_classes=[permissions.TokenHasReadWriteScope] + ) + ) + ) +) +class OAuth2AuthenticationAllowInactiveUserTestCase(test_authentication.OAuth2Tests): + """ + Tests the OAuth2AuthenticationAllowInactiveUser class by running all the existing tests in + OAuth2Tests but with the is_active flag on the user set to False. + """ + def setUp(self): + super(OAuth2AuthenticationAllowInactiveUserTestCase, self).setUp() + + # set the user's is_active flag to False. + self.user.is_active = False + self.user.save() + + # Override the SCOPE_NAME_DICT setting for tests for oauth2-with-scope-test. This is + # needed to support READ and WRITE scopes as they currently aren't supported by the + # edx-auth2-provider, and their scope values collide with other scopes defined in the + # edx-auth2-provider. + scope.SCOPE_NAME_DICT = {'read': constants.READ, 'write': constants.WRITE} diff --git a/lms/djangoapps/mobile_api/utils.py b/lms/djangoapps/mobile_api/utils.py index cd7720f8a3..7cb71702cd 100644 --- a/lms/djangoapps/mobile_api/utils.py +++ b/lms/djangoapps/mobile_api/utils.py @@ -4,10 +4,9 @@ Common utility methods and decorators for Mobile APIs. import functools - from rest_framework import permissions -from rest_framework.authentication import OAuth2Authentication, SessionAuthentication +from util.authentication import SessionAuthenticationAllowInactiveUser, OAuth2AuthenticationAllowInactiveUser from opaque_keys.edx.keys import CourseKey from xmodule.modulestore.django import modulestore from courseware.courses import get_course_with_access @@ -55,7 +54,10 @@ def mobile_view(is_user=False): Requires either OAuth2 or Session-based authentication. If is_user is True, also requires username in URL matches the request user. """ - func_or_class.authentication_classes = (OAuth2Authentication, SessionAuthentication) + func_or_class.authentication_classes = ( + OAuth2AuthenticationAllowInactiveUser, + SessionAuthenticationAllowInactiveUser + ) func_or_class.permission_classes = (permissions.IsAuthenticated,) if is_user: func_or_class.permission_classes += (IsUser,) diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 0403cd7254..9011639ece 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -33,7 +33,7 @@ git+https://github.com/mitocw/django-cas.git@60a5b8e5a62e63e0d5d224a87f0b489201a -e git+https://github.com/edx/opaque-keys.git@1254ed4d615a428591850656f39f26509b86d30a#egg=opaque-keys -e git+https://github.com/edx/ease.git@97de68448e5495385ba043d3091f570a699d5b5f#egg=ease -e git+https://github.com/edx/i18n-tools.git@193cebd9aa784f8899ef496f2aa050b08eff402b#egg=i18n-tools --e git+https://github.com/edx/edx-oauth2-provider.git@0.4.0#egg=oauth2-provider +-e git+https://github.com/edx/edx-oauth2-provider.git@0.4.1#egg=oauth2-provider -e git+https://github.com/edx/edx-val.git@9ceddb4944d0a1264b345947bf486340c5774a00#egg=edx-val -e git+https://github.com/pmitros/RecommenderXBlock.git@9b07e807c89ba5761827d0387177f71aa57ef056#egg=recommender-xblock -e git+https://github.com/edx/edx-milestones.git@547f2250ee49e73ce8d7ff4e78ecf1b049892510#egg=edx-milestones