diff --git a/common/djangoapps/student/tests/test_views.py b/common/djangoapps/student/tests/test_views.py index 5fbc296c23..3a32eba078 100644 --- a/common/djangoapps/student/tests/test_views.py +++ b/common/djangoapps/student/tests/test_views.py @@ -175,7 +175,8 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin, MOCK_SETTINGS = { 'FEATURES': { 'DISABLE_START_DATES': False, - 'ENABLE_MKTG_SITE': True + 'ENABLE_MKTG_SITE': True, + 'DISABLE_SET_JWT_COOKIES_FOR_TESTS': True, }, 'SOCIAL_SHARING_SETTINGS': { 'CUSTOM_COURSE_URLS': True, @@ -186,6 +187,7 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin, MOCK_SETTINGS_HIDE_COURSES = { 'FEATURES': { 'HIDE_DASHBOARD_COURSES_UNTIL_ACTIVATED': True, + 'DISABLE_SET_JWT_COOKIES_FOR_TESTS': True, } } diff --git a/common/djangoapps/student/views/dashboard.py b/common/djangoapps/student/views/dashboard.py index 1884245080..4705fd6529 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_logged_in_cookies from student.helpers import cert_info, check_verify_status_by_course from student.models import ( CourseEnrollment, @@ -851,5 +851,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_logged_in_cookies(request, response, user) return response diff --git a/common/djangoapps/third_party_auth/pipeline.py b/common/djangoapps/third_party_auth/pipeline.py index e18525e4ba..4a40148634 100644 --- a/common/djangoapps/third_party_auth/pipeline.py +++ b/common/djangoapps/third_party_auth/pipeline.py @@ -632,7 +632,7 @@ def set_logged_in_cookies(backend=None, user=None, strategy=None, auth_entry=Non # Check that the cookie isn't already set. # This ensures that we allow the user to continue to the next # pipeline step once he/she has the cookie set by this step. - has_cookie = user_authn_cookies.is_logged_in_cookie_set(request) + has_cookie = user_authn_cookies.are_logged_in_cookies_set(request) if not has_cookie: try: redirect_url = get_complete_url(current_partial.backend) diff --git a/openedx/core/djangoapps/user_authn/cookies.py b/openedx/core/djangoapps/user_authn/cookies.py index 38d9e012a4..865994281f 100644 --- a/openedx/core/djangoapps/user_authn/cookies.py +++ b/openedx/core/djangoapps/user_authn/cookies.py @@ -55,12 +55,17 @@ DEPRECATED_LOGGED_IN_COOKIE_NAMES = ( ALL_LOGGED_IN_COOKIE_NAMES = JWT_COOKIE_NAMES + DEPRECATED_LOGGED_IN_COOKIE_NAMES -def is_logged_in_cookie_set(request): +def are_logged_in_cookies_set(request): """ Check whether the request has logged in cookies set. """ - return ( - settings.EDXMKTG_LOGGED_IN_COOKIE_NAME in request.COOKIES and - request.COOKIES[settings.EDXMKTG_LOGGED_IN_COOKIE_NAME] - ) + if settings.FEATURES.get('DISABLE_SET_JWT_COOKIES_FOR_TESTS', False): + cookies_that_should_exist = DEPRECATED_LOGGED_IN_COOKIE_NAMES + else: + cookies_that_should_exist = ALL_LOGGED_IN_COOKIE_NAMES + + return all( + cookie_name in request.COOKIES + for cookie_name in cookies_that_should_exist + ) and request.COOKIES[settings.EDXMKTG_LOGGED_IN_COOKIE_NAME] def delete_logged_in_cookies(response): @@ -107,7 +112,7 @@ def standard_cookie_settings(request): # In non-production environments (acceptance tests, devstack, and sandboxes), # we still want to set this cookie. However, we do NOT want to set it to "secure" # because the browser won't send it back to us. This can cause an infinite redirect - # loop in the third-party auth flow, which calls `is_logged_in_cookie_set` to determine + # loop in the third-party auth flow, which calls `are_logged_in_cookies_set` to determine # whether it needs to set the cookie or continue to the next pipeline stage. cookie_settings['secure'] = request.is_secure() @@ -139,7 +144,7 @@ def set_logged_in_cookies(request, response, user): cookie_settings = standard_cookie_settings(request) _set_deprecated_logged_in_cookie(response, cookie_settings) - set_deprecated_user_info_cookie(response, request, user, 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) @@ -162,7 +167,7 @@ def refresh_jwt_cookies(request, response): return response -def set_deprecated_user_info_cookie(response, request, user, cookie_settings=None): +def _set_deprecated_user_info_cookie(response, request, user, cookie_settings=None): """ Sets the user info cookie on the response. diff --git a/openedx/core/djangoapps/user_authn/tests/test_cookies.py b/openedx/core/djangoapps/user_authn/tests/test_cookies.py index ce903d3708..3d0e1fcdcb 100644 --- a/openedx/core/djangoapps/user_authn/tests/test_cookies.py +++ b/openedx/core/djangoapps/user_authn/tests/test_cookies.py @@ -117,14 +117,16 @@ class CookieTests(TestCase): self._assert_consistent_expires(response) self._assert_recreate_jwt_from_cookies(response, can_recreate=True) - def test_delete_and_is_logged_in_cookie_set(self): + @patch.dict("django.conf.settings.FEATURES", {"DISABLE_SET_JWT_COOKIES_FOR_TESTS": False}) + def test_delete_and_are_logged_in_cookies_set(self): + setup_login_oauth_client() response = cookies_api.set_logged_in_cookies(self.request, HttpResponse(), self.user) self._copy_cookies_to_request(response, self.request) - self.assertTrue(cookies_api.is_logged_in_cookie_set(self.request)) + self.assertTrue(cookies_api.are_logged_in_cookies_set(self.request)) cookies_api.delete_logged_in_cookies(response) self._copy_cookies_to_request(response, self.request) - self.assertFalse(cookies_api.is_logged_in_cookie_set(self.request)) + self.assertFalse(cookies_api.are_logged_in_cookies_set(self.request)) @patch.dict("django.conf.settings.FEATURES", {"DISABLE_SET_JWT_COOKIES_FOR_TESTS": False}) def test_refresh_jwt_cookies(self): diff --git a/openedx/core/djangoapps/user_authn/views/login_form.py b/openedx/core/djangoapps/user_authn/views/login_form.py index 07aa5cc433..076240a759 100644 --- a/openedx/core/djangoapps/user_authn/views/login_form.py +++ b/openedx/core/djangoapps/user_authn/views/login_form.py @@ -24,6 +24,7 @@ from openedx.core.djangoapps.user_api.api import ( get_login_session_form, get_password_reset_form ) +from openedx.core.djangoapps.user_authn.cookies import are_logged_in_cookies_set from openedx.features.enterprise_support.api import enterprise_customer_for_request from openedx.features.enterprise_support.utils import ( handle_enterprise_cookies_for_logistration, @@ -53,8 +54,12 @@ def login_and_registration_form(request, initial_mode="login"): """ # Determine the URL to redirect to following login/registration/third_party_auth redirect_to = get_next_url_for_login_page(request) + # If we're already logged in, redirect to the dashboard - if request.user.is_authenticated: + # Note: We check for the existence of login-related cookies in addition to is_authenticated + # since Django's SessionAuthentication middleware auto-updates session cookies but not + # the other login-related cookies. See ARCH-282. + if request.user.is_authenticated and are_logged_in_cookies_set(request): return redirect(redirect_to) # Retrieve the form descriptions from the user API diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_views.py b/openedx/core/djangoapps/user_authn/views/tests/test_views.py index 343dc82f84..6544f431db 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_views.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_views.py @@ -327,9 +327,19 @@ class LoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMixin, ModuleSto @ddt.data("signin_user", "register_user") def test_login_and_registration_form_already_authenticated(self, url_name): - # Create/activate a new account and log in - activation_key = create_account(self.USERNAME, self.PASSWORD, self.EMAIL) - activate_account(activation_key) + # call the account registration api that sets the login cookies + url = reverse('user_api_registration') + request_data = { + 'username': self.USERNAME, + 'password': self.PASSWORD, + 'email': self.EMAIL, + 'name': self.USERNAME, + 'terms_of_service': 'true', + 'honor_code': 'true', + } + result = self.client.post(url, data=request_data) + self.assertEqual(result.status_code, 200) + result = self.client.login(username=self.USERNAME, password=self.PASSWORD) self.assertTrue(result)