diff --git a/common/djangoapps/student/views/dashboard.py b/common/djangoapps/student/views/dashboard.py index 722b717fbd867c7212a9815082200bd0d54ade9d..e4a0028693dd7eca3863ba9e3233bba74f07c96e 100644 --- a/common/djangoapps/student/views/dashboard.py +++ b/common/djangoapps/student/views/dashboard.py @@ -44,7 +44,7 @@ from openedx.features.enterprise_support.api import get_dashboard_consent_notifi from openedx.features.journals.api import journals_enabled from shoppingcart.api import order_history from shoppingcart.models import CourseRegistrationCode, DonationConfiguration -from openedx.core.djangoapps.user_authn.cookies import _set_deprecated_user_info_cookie +from openedx.core.djangoapps.user_authn.cookies import set_deprecated_user_info_cookie from student.helpers import cert_info, check_verify_status_by_course from student.models import ( CourseEnrollment, @@ -848,5 +848,5 @@ def student_dashboard(request): }) response = render_to_response('dashboard.html', context) - _set_deprecated_user_info_cookie(response, request, user) # pylint: disable=protected-access + set_deprecated_user_info_cookie(response, request, user) # pylint: disable=protected-access return response diff --git a/openedx/core/djangoapps/auth_exchange/tests/mixins.py b/openedx/core/djangoapps/auth_exchange/tests/mixins.py index 70ce191b8df6026ebd8ce8c736fcb9b14ad8784d..671c542517254af6113429c4b2dd5ccd8ebb5f09 100644 --- a/openedx/core/djangoapps/auth_exchange/tests/mixins.py +++ b/openedx/core/djangoapps/auth_exchange/tests/mixins.py @@ -103,7 +103,3 @@ class DOTAdapterMixin(object): def test_single_access_token(self): # TODO: Single access tokens not supported yet for DOT (See MA-2122) super(DOTAdapterMixin, self).test_single_access_token() - - @skip("Not supported yet (See MA-2123)") - def test_scopes(self): - super(DOTAdapterMixin, self).test_scopes() diff --git a/openedx/core/djangoapps/auth_exchange/tests/test_views.py b/openedx/core/djangoapps/auth_exchange/tests/test_views.py index f9ce0ab76020fecfeb1321eeb6c2e839098c4c5e..c757000d0b790213c64fff63648d4fdd3c59035f 100644 --- a/openedx/core/djangoapps/auth_exchange/tests/test_views.py +++ b/openedx/core/djangoapps/auth_exchange/tests/test_views.py @@ -60,7 +60,7 @@ class AccessTokenExchangeViewTest(AccessTokenExchangeTestMixin): timedelta(seconds=int(content["expires_in"])), provider.constants.EXPIRE_DELTA_PUBLIC ) - self.assertEqual(content["scope"], self.oauth2_adapter.normalize_scopes(expected_scopes)) + self.assertEqual(content["scope"], ' '.join(expected_scopes)) token = self.oauth2_adapter.get_access_token(token_string=content["access_token"]) self.assertEqual(token.user, self.user) self.assertEqual(self.oauth2_adapter.get_client_for_token(token), self.oauth_client) diff --git a/openedx/core/djangoapps/auth_exchange/views.py b/openedx/core/djangoapps/auth_exchange/views.py index 63634ad2e236de23133b0f2a76a75c01a6c6e40a..0fd37122e72dbd3f0a76be32da511c22dcba6b64 100644 --- a/openedx/core/djangoapps/auth_exchange/views.py +++ b/openedx/core/djangoapps/auth_exchange/views.py @@ -21,6 +21,7 @@ from oauth2_provider.settings import oauth2_settings from oauth2_provider.views.base import TokenView as DOTAccessTokenView from oauthlib.oauth2.rfc6749.tokens import BearerToken from provider import constants +from provider import scope as dop_scope from provider.oauth2.views import AccessTokenView as DOPAccessTokenView from rest_framework import permissions from rest_framework.exceptions import AuthenticationFailed @@ -111,7 +112,8 @@ class DOTAccessTokenExchangeView(AccessTokenExchangeBase, DOTAccessTokenView): """ Create and return a new access token. """ - return create_dot_access_token(request, user, client, scope=scope) + scopes = dop_scope.to_names(scope) + return create_dot_access_token(request, user, client, scopes=scopes) def access_token_response(self, token): """ diff --git a/openedx/core/djangoapps/oauth_dispatch/adapters/dop.py b/openedx/core/djangoapps/oauth_dispatch/adapters/dop.py index ec8afa11f526081033023290fdf8cdef21cd3d79..e9a1eee5daaf33b17a8c3c183ed0590b7e671a08 100644 --- a/openedx/core/djangoapps/oauth_dispatch/adapters/dop.py +++ b/openedx/core/djangoapps/oauth_dispatch/adapters/dop.py @@ -69,12 +69,6 @@ class DOPAdapter(object): expires=expires, ) - def normalize_scopes(self, scopes): - """ - Given a list of scopes, return a space-separated list of those scopes. - """ - return ' '.join(scopes) - def get_token_scope_names(self, token): """ Given an access token object, return its scopes. diff --git a/openedx/core/djangoapps/oauth_dispatch/adapters/dot.py b/openedx/core/djangoapps/oauth_dispatch/adapters/dot.py index 18ab3f4491c53d8cdd7f2dcd0f3e3f16a926f081..c83e1c4cff1677e88cd9f8f55782d4a2f1cffee1 100644 --- a/openedx/core/djangoapps/oauth_dispatch/adapters/dot.py +++ b/openedx/core/djangoapps/oauth_dispatch/adapters/dot.py @@ -79,14 +79,6 @@ class DOTAdapter(object): expires=expires, ) - def normalize_scopes(self, scopes): - """ - Given a list of scopes, return a space-separated list of those scopes. - """ - if not scopes: - scopes = ['default'] - return ' '.join(scopes) - def get_token_scope_names(self, token): """ Given an access token object, return its scopes. diff --git a/openedx/core/djangoapps/oauth_dispatch/api.py b/openedx/core/djangoapps/oauth_dispatch/api.py index 0c56bf901c83ee211f05245ed776ffdc06e18246..786c0664869fcdc4edfa44d922688dd0b53c0ee5 100644 --- a/openedx/core/djangoapps/oauth_dispatch/api.py +++ b/openedx/core/djangoapps/oauth_dispatch/api.py @@ -2,7 +2,6 @@ import json from django.conf import settings -from edx_oauth2_provider.constants import SCOPE_VALUE_DICT from oauthlib.oauth2.rfc6749.errors import OAuth2Error from oauthlib.oauth2.rfc6749.tokens import BearerToken from oauth2_provider.models import AccessToken as dot_access_token @@ -22,7 +21,7 @@ def destroy_oauth_tokens(user): dot_refresh_token.objects.filter(user=user.id).delete() -def create_dot_access_token(request, user, client, expires_in=None, scope=None): +def create_dot_access_token(request, user, client, expires_in=None, scopes=None): """ Create and return a new (persisted) access token, including a refresh token. The token is returned in the form of a Dict: @@ -31,17 +30,15 @@ def create_dot_access_token(request, user, client, expires_in=None, scope=None): u'refresh_token': u'another string', u'token_type': u'Bearer', u'expires_in': 36000, - u'scope': u'default', + u'scope': u'profile email', }, """ - # TODO (ARCH-204) the 'scope' argument may not really be needed by callers. - expires_in = _get_expires_in_value(expires_in) token_generator = BearerToken( expires_in=expires_in, request_validator=dot_settings.OAUTH2_VALIDATOR_CLASS(), ) - _populate_create_access_token_request(request, user, client, scope) + _populate_create_access_token_request(request, user, client, scopes) return token_generator.create_token(request, refresh_token=True) @@ -72,17 +69,15 @@ def _get_expires_in_value(expires_in): return expires_in or dot_settings.ACCESS_TOKEN_EXPIRE_SECONDS -def _populate_create_access_token_request(request, user, client, scope=None): +def _populate_create_access_token_request(request, user, client, scopes): """ django-oauth-toolkit expects certain non-standard attributes to be present on the request object. This function modifies the request object to match these expectations """ - if scope is None: - scope = 0 request.user = user - request.scopes = [SCOPE_VALUE_DICT[scope]] request.client = client + request.scopes = scopes or '' request.state = None request.refresh_token = None request.extra_credentials = None diff --git a/openedx/core/djangoapps/oauth_dispatch/tests/test_api.py b/openedx/core/djangoapps/oauth_dispatch/tests/test_api.py index aba40a15c85a8642d36924ecff1a0d2f1868a70c..81150a2b2c0598a6e57371fff9a36395dca37a8f 100644 --- a/openedx/core/djangoapps/oauth_dispatch/tests/test_api.py +++ b/openedx/core/djangoapps/oauth_dispatch/tests/test_api.py @@ -45,7 +45,7 @@ class TestOAuthDispatchAPI(TestCase): { u'token_type': u'Bearer', u'expires_in': EXPECTED_DEFAULT_EXPIRES_IN, - u'scope': u'default', + u'scope': u'', }, token, ) @@ -58,7 +58,9 @@ class TestOAuthDispatchAPI(TestCase): def test_create_token_overrides(self): expires_in = 4800 - token = api.create_dot_access_token(HttpRequest(), self.user, self.client, expires_in=expires_in, scope=2) + token = api.create_dot_access_token( + HttpRequest(), self.user, self.client, expires_in=expires_in, scopes=['profile'], + ) self.assertDictContainsSubset({u'scope': u'profile'}, token) self.assertDictContainsSubset({u'expires_in': expires_in}, token) @@ -69,7 +71,7 @@ class TestOAuthDispatchAPI(TestCase): { u'token_type': u'Bearer', u'expires_in': EXPECTED_DEFAULT_EXPIRES_IN, - u'scope': u'default', + u'scope': u'', }, new_token, ) diff --git a/openedx/core/djangoapps/user_authn/cookies.py b/openedx/core/djangoapps/user_authn/cookies.py index 06a2725bfd8ce0fe9dbc5659622a7a5608d05097..242edf0aeb9a70540503339eac8f3a5e90f1d78a 100644 --- a/openedx/core/djangoapps/user_authn/cookies.py +++ b/openedx/core/djangoapps/user_authn/cookies.py @@ -135,9 +135,13 @@ def set_logged_in_cookies(request, response, user): # that is passed in when needed. if user.is_authenticated and not user.is_anonymous: - _set_deprecated_logged_in_cookie(response, request) - _set_deprecated_user_info_cookie(response, request, user) - _create_and_set_jwt_cookies(response, request, user) + # JWT cookies expire at the same time as other login-related cookies + # so that cookie-based login determination remains consistent. + cookie_settings = standard_cookie_settings(request) + + _set_deprecated_logged_in_cookie(response, cookie_settings) + set_deprecated_user_info_cookie(response, request, user, cookie_settings) + _create_and_set_jwt_cookies(response, request, cookie_settings, user=user) CREATE_LOGON_COOKIE.send(sender=None, user=user, response=response) return response @@ -153,29 +157,14 @@ def refresh_jwt_cookies(request, response): refresh_token = request.COOKIES[jwt_cookies.jwt_refresh_cookie_name()] except KeyError: raise AuthFailedError(u"JWT Refresh Cookie not found in request.") - _create_and_set_jwt_cookies(response, request, refresh_token=refresh_token) - return response - - -def _set_deprecated_logged_in_cookie(response, request): - """ Sets the logged in cookie on the response. """ - - # Backwards compatibility: set the cookie indicating that the user - # is logged in. This is just a boolean value, so it's not very useful. - # In the future, we should be able to replace this with the "user info" - # cookie set below. - cookie_settings = standard_cookie_settings(request) - - response.set_cookie( - settings.EDXMKTG_LOGGED_IN_COOKIE_NAME.encode('utf-8'), - 'true', - **cookie_settings - ) + # TODO don't extend the cookie expiration - reuse value from existing cookie + cookie_settings = standard_cookie_settings(request) + _create_and_set_jwt_cookies(response, request, cookie_settings, refresh_token=refresh_token) return response -def _set_deprecated_user_info_cookie(response, request, user): +def set_deprecated_user_info_cookie(response, request, user, cookie_settings=None): """ Sets the user info cookie on the response. @@ -192,8 +181,7 @@ def _set_deprecated_user_info_cookie(response, request, user): } } """ - cookie_settings = standard_cookie_settings(request) - + cookie_settings = cookie_settings or standard_cookie_settings(request) user_info = _get_user_info_cookie_data(request, user) response.set_cookie( settings.EDXMKTG_USER_INFO_COOKIE_NAME.encode('utf-8'), @@ -202,6 +190,22 @@ def _set_deprecated_user_info_cookie(response, request, user): ) +def _set_deprecated_logged_in_cookie(response, cookie_settings): + """ Sets the logged in cookie on the response. """ + + # Backwards compatibility: set the cookie indicating that the user + # is logged in. This is just a boolean value, so it's not very useful. + # In the future, we should be able to replace this with the "user info" + # cookie set below. + response.set_cookie( + settings.EDXMKTG_LOGGED_IN_COOKIE_NAME.encode('utf-8'), + 'true', + **cookie_settings + ) + + return response + + def _get_user_info_cookie_data(request, user): """ Returns information that will populate the user info cookie. """ @@ -242,15 +246,11 @@ def _get_user_info_cookie_data(request, user): return user_info -def _create_and_set_jwt_cookies(response, request, user=None, refresh_token=None): +def _create_and_set_jwt_cookies(response, request, cookie_settings, user=None, refresh_token=None): """ Sets a cookie containing a JWT on the response. """ if not JWT_COOKIES_FLAG.is_enabled(): return - # JWT cookies expire at the same time as other login-related cookies - # so that cookie-based login determination remains consistent. - cookie_settings = standard_cookie_settings(request) - # For security reasons, the JWT that is embedded inside the cookie expires # much sooner than the cookie itself, per the following setting. expires_in = settings.JWT_AUTH['JWT_IN_COOKIE_EXPIRATION'] @@ -262,7 +262,7 @@ def _create_and_set_jwt_cookies(response, request, user=None, refresh_token=None ) else: access_token = create_dot_access_token( - request, user, oauth_application, expires_in=expires_in, + request, user, oauth_application, expires_in=expires_in, scopes=['email', 'profile'], ) jwt = create_jwt_from_token(access_token, DOTAdapter(), use_asymmetric_key=True) jwt_header_and_payload, jwt_signature = _parse_jwt(jwt) diff --git a/openedx/core/djangoapps/user_authn/tests/test_cookies.py b/openedx/core/djangoapps/user_authn/tests/test_cookies.py index 4bc0154ea48e628dc05b42dba2c9d70617d77a85..ab0d3ce32cdd4437bf146f291cc457d016dc9079 100644 --- a/openedx/core/djangoapps/user_authn/tests/test_cookies.py +++ b/openedx/core/djangoapps/user_authn/tests/test_cookies.py @@ -8,6 +8,7 @@ from django.http import HttpResponse from django.urls import reverse from django.test import RequestFactory, TestCase +from edx_rest_framework_extensions.auth.jwt.decoder import jwt_decode_handler from edx_rest_framework_extensions.auth.jwt.middleware import JwtAuthCookieMiddleware from openedx.core.djangoapps.user_authn import cookies as cookies_api from openedx.core.djangoapps.user_authn.tests.utils import setup_login_oauth_client @@ -57,8 +58,10 @@ class CookieTests(TestCase): def _assert_recreate_jwt_from_cookies(self, response, can_recreate): """ - Verifies that a JWT can be properly recreated from the 2 separate - JWT-related cookies using the JwtAuthCookieMiddleware middleware. + If can_recreate is True, verifies that a JWT can be properly recreated + from the 2 separate JWT-related cookies using the + JwtAuthCookieMiddleware middleware and returns the recreated JWT. + If can_recreate is False, verifies that a JWT cannot be recreated. """ self._copy_cookies_to_request(response, self.request) JwtAuthCookieMiddleware().process_request(self.request) @@ -66,6 +69,10 @@ class CookieTests(TestCase): cookies_api.jwt_cookies.jwt_cookie_name() in self.request.COOKIES, can_recreate, ) + if can_recreate: + jwt_string = self.request.COOKIES[cookies_api.jwt_cookies.jwt_cookie_name()] + jwt = jwt_decode_handler(jwt_string) + self.assertEqual(jwt['scopes'], ['email', 'profile']) def _assert_cookies_present(self, response, expected_cookies): """ Verify all expected_cookies are present in the response. """