diff --git a/lms/envs/common.py b/lms/envs/common.py index 9a280b6afefaec8c58af994d76442ad5c296d266..c125e9e6f21605268bb1fe9e2bab627ae0a830aa 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -521,7 +521,7 @@ OAUTH_EXPIRE_PUBLIC_CLIENT_DAYS = 30 ################################## DJANGO OAUTH TOOLKIT ####################################### OAUTH2_PROVIDER = { - 'OAUTH2_VALIDATOR_CLASS': 'openedx.core.djangoapps.oauth_dispatch.dot_overrides.EdxOAuth2Validator', + 'OAUTH2_VALIDATOR_CLASS': 'openedx.core.djangoapps.oauth_dispatch.dot_overrides.validators.EdxOAuth2Validator', 'REFRESH_TOKEN_EXPIRE_SECONDS': 20160, 'SCOPES': { 'read': 'Read access', @@ -531,6 +531,7 @@ OAUTH2_PROVIDER = { # to lms/templates/provider/authorize.html. This may be revised later. 'profile': 'Know your name and username', }, + 'REQUEST_APPROVAL_PROMPT': 'auto_even_if_expired', } # This is required for the migrations in oauth_dispatch.models # otherwise it fails saying this attribute is not present in Settings diff --git a/openedx/core/djangoapps/oauth_dispatch/dot_overrides/__init__.py b/openedx/core/djangoapps/oauth_dispatch/dot_overrides/__init__.py new file mode 100644 index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 diff --git a/openedx/core/djangoapps/oauth_dispatch/dot_overrides.py b/openedx/core/djangoapps/oauth_dispatch/dot_overrides/validators.py similarity index 98% rename from openedx/core/djangoapps/oauth_dispatch/dot_overrides.py rename to openedx/core/djangoapps/oauth_dispatch/dot_overrides/validators.py index 0e8ff6ffdcda1deafdb53536aaa6dd5c6d53aa1c..702d0b4e06dea9146d1f6a33bdb6e9ccc36d69ac 100644 --- a/openedx/core/djangoapps/oauth_dispatch/dot_overrides.py +++ b/openedx/core/djangoapps/oauth_dispatch/dot_overrides/validators.py @@ -12,7 +12,7 @@ from oauth2_provider.models import AccessToken from oauth2_provider.oauth2_validators import OAuth2Validator from pytz import utc -from .models import RestrictedApplication +from ..models import RestrictedApplication @receiver(pre_save, sender=AccessToken) diff --git a/openedx/core/djangoapps/oauth_dispatch/dot_overrides/views.py b/openedx/core/djangoapps/oauth_dispatch/dot_overrides/views.py new file mode 100644 index 0000000000000000000000000000000000000000..363e6e86ebd4d38ec280319f2d8d8d5d466624dc --- /dev/null +++ b/openedx/core/djangoapps/oauth_dispatch/dot_overrides/views.py @@ -0,0 +1,89 @@ +""" +Classes that override default django-oauth-toolkit behavior +""" +from __future__ import unicode_literals + +from oauth2_provider.exceptions import OAuthToolkitError +from oauth2_provider.http import HttpResponseUriRedirect +from oauth2_provider.models import get_application_model +from oauth2_provider.scopes import get_scopes_backend +from oauth2_provider.settings import oauth2_settings +from oauth2_provider.views import AuthorizationView + + +# TODO (ARCH-83) remove once we have full support of OAuth Scopes +class EdxOAuth2AuthorizationView(AuthorizationView): + """ + Override the AuthorizationView's GET method so the user isn't + prompted to approve the application if they have already in + the past, even if their access token is expired. + + This is a temporary override of the base implementation + in order to accommodate our Restricted Applications support + until OAuth Scopes are fully supported. + """ + def get(self, request, *args, **kwargs): + # Note: This code is copied from https://github.com/evonove/django-oauth-toolkit/blob/34f3b7b3511c15686039079026165feaadb1b87d/oauth2_provider/views/base.py#L111 + # Places that we have changed are noted with ***. + try: + # *** Moved code to get the require_approval value earlier on so we can + # circumvent our custom code in the case when auto_even_if_expired + # isn't required. + require_approval = request.GET.get( + "approval_prompt", + oauth2_settings.REQUEST_APPROVAL_PROMPT, + ) + if require_approval != 'auto_even_if_expired': + return super(EdxOAuth2AuthorizationView, self).get(request, *args, **kwargs) + + scopes, credentials = self.validate_authorization_request(request) + all_scopes = get_scopes_backend().get_all_scopes() + kwargs["scopes_descriptions"] = [all_scopes[scope] for scope in scopes] + kwargs['scopes'] = scopes + + # at this point we know an Application instance with such client_id exists in the database + application = get_application_model().objects.get(client_id=credentials['client_id']) + kwargs['application'] = application + kwargs['client_id'] = credentials['client_id'] + kwargs['redirect_uri'] = credentials['redirect_uri'] + kwargs['response_type'] = credentials['response_type'] + kwargs['state'] = credentials['state'] + + self.oauth2_data = kwargs + # following two loc are here only because of https://code.djangoproject.com/ticket/17795 + form = self.get_form(self.get_form_class()) + kwargs['form'] = form + + # If skip_authorization field is True, skip the authorization screen even + # if this is the first use of the application and there was no previous authorization. + # This is useful for in-house applications-> assume an in-house applications + # are already approved. + if application.skip_authorization: + uri, headers, body, status = self.create_authorization_response( + request=self.request, scopes=" ".join(scopes), + credentials=credentials, allow=True) + return HttpResponseUriRedirect(uri) + + # *** Changed the if statement that checked for require_approval to an assert. + assert require_approval == 'auto_even_if_expired' + tokens = request.user.accesstoken_set.filter( + application=kwargs['application'], + # *** Purposefully keeping this commented out code to highlight that + # our version of the implementation does NOT filter by expiration date. + # expires__gt=timezone.now(), + ).all() + + # check past authorizations regarded the same scopes as the current one + for token in tokens: + if token.allow_scopes(scopes): + uri, headers, body, status = self.create_authorization_response( + request=self.request, scopes=" ".join(scopes), + credentials=credentials, allow=True) + return HttpResponseUriRedirect(uri) + + # render an authorization prompt so the user can approve + # the application's requested scopes + return self.render_to_response(self.get_context_data(**kwargs)) + + except OAuthToolkitError as error: + return self.error_response(error) diff --git a/openedx/core/djangoapps/oauth_dispatch/tests/test_dot_overrides.py b/openedx/core/djangoapps/oauth_dispatch/tests/test_dot_overrides.py index 4e5472d7fe64b1dc4f3135b0d88680dc53f66a58..16f4510740b34b130698b09e0657eb74069e1d98 100644 --- a/openedx/core/djangoapps/oauth_dispatch/tests/test_dot_overrides.py +++ b/openedx/core/djangoapps/oauth_dispatch/tests/test_dot_overrides.py @@ -3,15 +3,23 @@ Test of custom django-oauth-toolkit behavior """ # pylint: disable=protected-access +import datetime import unittest from django.conf import settings from django.contrib.auth.models import User +from django.utils import timezone from django.test import TestCase, RequestFactory +from student.tests.factories import UserFactory # oauth_dispatch is not in CMS' INSTALLED_APPS so these imports will error during test collection if settings.ROOT_URLCONF == 'lms.urls': - from ..dot_overrides import EdxOAuth2Validator + from oauth2_provider import models as dot_models + + from .. import adapters + from .. import models + from ..dot_overrides.validators import EdxOAuth2Validator + from .constants import DUMMY_REDIRECT_URL @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') @@ -71,3 +79,66 @@ class CustomValidationTestCase(TestCase): self.user.save() request = self.request_factory.get('/') self.assertTrue(self.validator.validate_user('darkhelmet', '12345', client=None, request=request)) + + +@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') +class CustomAuthorizationViewTestCase(TestCase): + """ + Test custom authorization view works. + + In particular, users should not be re-prompted to approve + an application even if the access token is expired. + (This is a temporary override until Auth Scopes is implemented.) + """ + def setUp(self): + super(CustomAuthorizationViewTestCase, self).setUp() + self.dot_adapter = adapters.DOTAdapter() + self.user = UserFactory() + self.client.login(username=self.user.username, password='test') + + self.restricted_dot_app = self._create_restricted_app() + self._create_expired_token(self.restricted_dot_app) + + def _create_restricted_app(self): + restricted_app = self.dot_adapter.create_confidential_client( + name='test restricted dot application', + user=self.user, + redirect_uri=DUMMY_REDIRECT_URL, + client_id='dot-restricted-app-client-id', + ) + models.RestrictedApplication.objects.create(application=restricted_app) + return restricted_app + + def _create_expired_token(self, application): + date_in_the_past = timezone.now() + datetime.timedelta(days=-100) + dot_models.AccessToken.objects.create( + user=self.user, + token='1234567890', + application=application, + expires=date_in_the_past, + scope='profile', + ) + + def _get_authorize(self, scope): + authorize_url = '/oauth2/authorize/' + return self.client.get( + authorize_url, + { + 'client_id': self.restricted_dot_app.client_id, + 'response_type': 'code', + 'state': 'random_state_string', + 'redirect_uri': DUMMY_REDIRECT_URL, + 'scope': scope, + }, + ) + + def test_no_reprompting(self): + response = self._get_authorize(scope='profile') + self.assertEqual(response.status_code, 302) + self.assertTrue(response.url.startswith(DUMMY_REDIRECT_URL)) + + def test_prompting_with_new_scope(self): + response = self._get_authorize(scope='email') + self.assertEqual(response.status_code, 200) + self.assertContains(response, settings.OAUTH2_PROVIDER['SCOPES']['email']) + self.assertNotContains(response, settings.OAUTH2_PROVIDER['SCOPES']['profile']) diff --git a/openedx/core/djangoapps/oauth_dispatch/views.py b/openedx/core/djangoapps/oauth_dispatch/views.py index 8eae354de13e87dd3ce29f0fa3f30cc55debaf55..910a6d724b0694c557409fedb1e0bc65533da012 100644 --- a/openedx/core/djangoapps/oauth_dispatch/views.py +++ b/openedx/core/djangoapps/oauth_dispatch/views.py @@ -24,6 +24,7 @@ from openedx.core.djangoapps.auth_exchange import views as auth_exchange_views from openedx.core.lib.token_utils import JwtBuilder from . import adapters +from .dot_overrides import views as dot_overrides_views class _DispatchingView(View): @@ -129,7 +130,7 @@ class AuthorizationView(_DispatchingView): Part of the authorization flow. """ dop_view = dop_views.Capture - dot_view = dot_views.AuthorizationView + dot_view = dot_overrides_views.EdxOAuth2AuthorizationView class AccessTokenExchangeView(_DispatchingView):