From 5e86a64729e435d025d618b4871eb22a7c8cbed7 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Tue, 23 Jun 2015 12:55:35 -0700 Subject: [PATCH] User info cookie * Add a new cookie for user information * Make marketing cookie names configurable. * Handle URL reversal when URLs don't exist (in Studio) * Move cookie code from student/helpers.py into its own module. --- cms/envs/aws.py | 6 + cms/envs/common.py | 5 +- common/djangoapps/student/cookies.py | 130 ++++++++++++++++++ common/djangoapps/student/helpers.py | 43 +----- .../student/tests/test_create_account.py | 3 +- common/djangoapps/student/tests/test_login.py | 37 +++++ common/djangoapps/student/views.py | 17 +-- .../djangoapps/third_party_auth/pipeline.py | 7 +- .../djangoapps/third_party_auth/settings.py | 2 +- .../third_party_auth/tests/specs/base.py | 18 ++- lms/envs/aws.py | 6 + lms/envs/common.py | 5 +- .../djangoapps/user_api/tests/test_views.py | 3 +- openedx/core/djangoapps/user_api/views.py | 6 +- 14 files changed, 222 insertions(+), 66 deletions(-) create mode 100644 common/djangoapps/student/cookies.py diff --git a/cms/envs/aws.py b/cms/envs/aws.py index b14c726e40..85396530fe 100644 --- a/cms/envs/aws.py +++ b/cms/envs/aws.py @@ -153,6 +153,12 @@ if ENV_TOKENS.get('SESSION_COOKIE_NAME', None): # NOTE, there's a bug in Django (http://bugs.python.org/issue18012) which necessitates this being a str() SESSION_COOKIE_NAME = str(ENV_TOKENS.get('SESSION_COOKIE_NAME')) +# Set the names of cookies shared with the marketing site +# These have the same cookie domain as the session, which in production +# usually includes subdomains. +EDXMKTG_LOGGED_IN_COOKIE_NAME = ENV_TOKENS.get('EDXMKTG_LOGGED_IN_COOKIE_NAME', EDXMKTG_LOGGED_IN_COOKIE_NAME) +EDXMKTG_USER_INFO_COOKIE_NAME = ENV_TOKENS.get('EDXMKTG_USER_INFO_COOKIE_NAME', EDXMKTG_USER_INFO_COOKIE_NAME) + #Email overrides DEFAULT_FROM_EMAIL = ENV_TOKENS.get('DEFAULT_FROM_EMAIL', DEFAULT_FROM_EMAIL) DEFAULT_FEEDBACK_EMAIL = ENV_TOKENS.get('DEFAULT_FEEDBACK_EMAIL', DEFAULT_FEEDBACK_EMAIL) diff --git a/cms/envs/common.py b/cms/envs/common.py index 084eb4103c..f7485589cf 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -764,7 +764,10 @@ INSTALLED_APPS = ( ################# EDX MARKETING SITE ################################## -EDXMKTG_COOKIE_NAME = 'edxloggedin' +EDXMKTG_LOGGED_IN_COOKIE_NAME = 'edxloggedin' +EDXMKTG_USER_INFO_COOKIE_NAME = 'edx-user-info' +EDXMKTG_USER_INFO_COOKIE_VERSION = 1 + MKTG_URLS = {} MKTG_URL_LINK_MAP = { diff --git a/common/djangoapps/student/cookies.py b/common/djangoapps/student/cookies.py new file mode 100644 index 0000000000..81a7b9a6c9 --- /dev/null +++ b/common/djangoapps/student/cookies.py @@ -0,0 +1,130 @@ +""" +Utility functions for setting "logged in" cookies used by subdomains. +""" + +import time +import json + +from django.utils.http import cookie_date +from django.conf import settings +from django.core.urlresolvers import reverse, NoReverseMatch + + +def set_logged_in_cookies(request, response, user): + """ + Set cookies indicating that the user is logged in. + + Some installations have an external marketing site configured + that displays a different UI when the user is logged in + (e.g. a link to the student dashboard instead of to the login page) + + Currently, two cookies are set: + + * EDXMKTG_LOGGED_IN_COOKIE_NAME: Set to 'true' if the user is logged in. + * EDXMKTG_USER_INFO_COOKIE_VERSION: JSON-encoded dictionary with user information (see below). + + The user info cookie has the following format: + { + "version": 1, + "username": "test-user", + "email": "test-user@example.com", + "header_urls": { + "account_settings": "https://example.com/account/settings", + "learner_profile": "https://example.com/u/test-user", + "logout": "https://example.com/logout" + } + } + + Arguments: + request (HttpRequest): The request to the view, used to calculate + the cookie's expiration date based on the session expiration date. + response (HttpResponse): The response on which the cookie will be set. + user (User): The currently logged in user. + + Returns: + HttpResponse + + """ + if request.session.get_expire_at_browser_close(): + max_age = None + expires = None + else: + max_age = request.session.get_expiry_age() + expires_time = time.time() + max_age + expires = cookie_date(expires_time) + + cookie_settings = { + 'max_age': max_age, + 'expires': expires, + 'domain': settings.SESSION_COOKIE_DOMAIN, + 'path': '/', + 'secure': None, + 'httponly': None, + } + + # 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, 'true', **cookie_settings) + + # Set a cookie with user info. This can be used by external sites + # to customize content based on user information. Currently, + # we include information that's used to customize the "account" + # links in the header of subdomain sites (such as the marketing site). + header_urls = {'logout': reverse('logout')} + + # Unfortunately, this app is currently used by both the LMS and Studio login pages. + # If we're in Studio, we won't be able to reverse the account/profile URLs. + # To handle this, we don't add the URLs if we can't reverse them. + # External sites will need to have fallback mechanisms to handle this case + # (most likely just hiding the links). + try: + header_urls['account_settings'] = reverse('account_settings') + header_urls['learner_profile'] = reverse('learner_profile', kwargs={'username': user.username}) + except NoReverseMatch: + pass + + # Convert relative URL paths to absolute URIs + for url_name, url_path in header_urls.iteritems(): + header_urls[url_name] = request.build_absolute_uri(url_path) + + user_info = { + 'version': settings.EDXMKTG_USER_INFO_COOKIE_VERSION, + 'username': user.username, + 'email': user.email, + 'header_urls': header_urls, + } + + response.set_cookie( + settings.EDXMKTG_USER_INFO_COOKIE_NAME, + json.dumps(user_info), + **cookie_settings + ) + + return response + + +def delete_logged_in_cookies(response): + """ + Delete cookies indicating that the user is logged in. + + Arguments: + response (HttpResponse): The response sent to the client. + + Returns: + HttpResponse + + """ + for cookie_name in [settings.EDXMKTG_LOGGED_IN_COOKIE_NAME, settings.EDXMKTG_USER_INFO_COOKIE_NAME]: + response.delete_cookie(cookie_name, path='/', domain=settings.SESSION_COOKIE_DOMAIN) + + return response + + +def is_logged_in_cookie_set(request): + """Check whether the request has logged in cookies set. """ + return ( + settings.EDXMKTG_LOGGED_IN_COOKIE_NAME in request.COOKIES and + settings.EDXMKTG_USER_INFO_COOKIE_NAME in request.COOKIES + ) diff --git a/common/djangoapps/student/helpers.py b/common/djangoapps/student/helpers.py index 5efa8f7cec..9ddfea424c 100644 --- a/common/djangoapps/student/helpers.py +++ b/common/djangoapps/student/helpers.py @@ -1,54 +1,19 @@ """Helpers for the student app. """ import time from datetime import datetime +import urllib + from pytz import UTC + from django.utils.http import cookie_date from django.conf import settings from django.core.urlresolvers import reverse, NoReverseMatch + import third_party_auth -import urllib from verify_student.models import SoftwareSecurePhotoVerification # pylint: disable=F0401 from course_modes.models import CourseMode -def set_logged_in_cookie(request, response): - """Set a cookie indicating that the user is logged in. - - Some installations have an external marketing site configured - that displays a different UI when the user is logged in - (e.g. a link to the student dashboard instead of to the login page) - - Arguments: - request (HttpRequest): The request to the view, used to calculate - the cookie's expiration date based on the session expiration date. - response (HttpResponse): The response on which the cookie will be set. - - Returns: - HttpResponse - - """ - if request.session.get_expire_at_browser_close(): - max_age = None - expires = None - else: - max_age = request.session.get_expiry_age() - expires_time = time.time() + max_age - expires = cookie_date(expires_time) - - response.set_cookie( - settings.EDXMKTG_COOKIE_NAME, 'true', max_age=max_age, - expires=expires, domain=settings.SESSION_COOKIE_DOMAIN, - path='/', secure=None, httponly=None, - ) - - return response - - -def is_logged_in_cookie_set(request): - """Check whether the request has the logged in cookie set. """ - return settings.EDXMKTG_COOKIE_NAME in request.COOKIES - - # Enumeration of per-course verification statuses # we display on the student dashboard. VERIFY_STATUS_NEED_TO_VERIFY = "verify_need_to_verify" diff --git a/common/djangoapps/student/tests/test_create_account.py b/common/djangoapps/student/tests/test_create_account.py index f21e19bf47..e5318208ef 100644 --- a/common/djangoapps/student/tests/test_create_account.py +++ b/common/djangoapps/student/tests/test_create_account.py @@ -86,7 +86,8 @@ class TestCreateAccount(TestCase): def test_marketing_cookie(self): response = self.client.post(self.url, self.params) self.assertEqual(response.status_code, 200) - self.assertIn(settings.EDXMKTG_COOKIE_NAME, self.client.cookies) + self.assertIn(settings.EDXMKTG_LOGGED_IN_COOKIE_NAME, self.client.cookies) + self.assertIn(settings.EDXMKTG_USER_INFO_COOKIE_NAME, self.client.cookies) @unittest.skipUnless( "microsite_configuration.middleware.MicrositeMiddleware" in settings.MIDDLEWARE_CLASSES, diff --git a/common/djangoapps/student/tests/test_login.py b/common/djangoapps/student/tests/test_login.py index 2c648be6ed..1f4a8bcb06 100644 --- a/common/djangoapps/student/tests/test_login.py +++ b/common/djangoapps/student/tests/test_login.py @@ -158,6 +158,43 @@ class LoginTest(TestCase): self.assertEqual(response.status_code, 302) self._assert_audit_log(mock_audit_log, 'info', [u'Logout', u'test']) + def test_login_user_info_cookie(self): + response, _ = self._login_response('test@edx.org', 'test_password') + self._assert_response(response, success=True) + + # Verify the format of the "user info" cookie set on login + cookie = self.client.cookies[settings.EDXMKTG_USER_INFO_COOKIE_NAME] + user_info = json.loads(cookie.value) + + # Check that the version is set + self.assertEqual(user_info["version"], settings.EDXMKTG_USER_INFO_COOKIE_VERSION) + + # Check that the username and email are set + self.assertEqual(user_info["username"], self.user.username) + self.assertEqual(user_info["email"], self.user.email) + + # Check that the URLs are absolute + for url in user_info["header_urls"].values(): + self.assertIn("http://testserver/", url) + + def test_logout_deletes_mktg_cookies(self): + response, _ = self._login_response('test@edx.org', 'test_password') + self._assert_response(response, success=True) + + # Check that the marketing site cookies have been set + self.assertIn(settings.EDXMKTG_LOGGED_IN_COOKIE_NAME, self.client.cookies) + self.assertIn(settings.EDXMKTG_USER_INFO_COOKIE_NAME, self.client.cookies) + + # Log out + logout_url = reverse('logout') + response = self.client.post(logout_url) + + # Check that the marketing site cookies have been deleted + # (cookies are deleted by setting an expiration date in 1970) + for cookie_name in [settings.EDXMKTG_LOGGED_IN_COOKIE_NAME, settings.EDXMKTG_USER_INFO_COOKIE_NAME]: + cookie = self.client.cookies[cookie_name] + self.assertIn("01-Jan-1970", cookie.get('expires')) + @patch.dict("django.conf.settings.FEATURES", {'SQUELCH_PII_IN_LOGS': True}) def test_logout_logging_no_pii(self): response, _ = self._login_response('test@edx.org', 'test_password') diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 5cc906ad23..ed1e486a89 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -106,9 +106,10 @@ from util.password_policy_validators import ( import third_party_auth from third_party_auth import pipeline, provider from student.helpers import ( - set_logged_in_cookie, check_verify_status_by_course, + check_verify_status_by_course, auth_pipeline_urls, get_next_url_for_login_page ) +from student.cookies import set_logged_in_cookies, delete_logged_in_cookies from student.models import anonymous_id_for_user from xmodule.error_module import ErrorDescriptor from shoppingcart.models import DonationConfiguration, CourseRegistrationCode @@ -1064,7 +1065,7 @@ def login_user(request, error=""): # pylint: disable-msg=too-many-statements,un # Ensure that the external marketing site can # detect that the user is logged in. - return set_logged_in_cookie(request, response) + return set_logged_in_cookies(request, response, user) if settings.FEATURES['SQUELCH_PII_IN_LOGS']: AUDIT_LOG.warning(u"Login failed - Account not active for user.id: {0}, resending activation".format(user.id)) @@ -1128,10 +1129,8 @@ def logout_user(request): else: target = '/' response = redirect(target) - response.delete_cookie( - settings.EDXMKTG_COOKIE_NAME, - path='/', domain=settings.SESSION_COOKIE_DOMAIN, - ) + + delete_logged_in_cookies(response) return response @@ -1549,6 +1548,8 @@ def create_account_with_params(request, params): new_user.save() AUDIT_LOG.info(u"Login activated on extauth account - {0} ({1})".format(new_user.username, new_user.email)) + return new_user + @csrf_exempt def create_account(request, post_override=None): @@ -1559,7 +1560,7 @@ def create_account(request, post_override=None): warnings.warn("Please use RegistrationView instead.", DeprecationWarning) try: - create_account_with_params(request, post_override or request.POST) + user = create_account_with_params(request, post_override or request.POST) except AccountValidationError as exc: return JsonResponse({'success': False, 'value': exc.message, 'field': exc.field}, status=400) except ValidationError as exc: @@ -1584,7 +1585,7 @@ def create_account(request, post_override=None): 'success': True, 'redirect_url': redirect_url, }) - set_logged_in_cookie(request, response) + 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 4d4ba9ba99..7c0ad27c08 100644 --- a/common/djangoapps/third_party_auth/pipeline.py +++ b/common/djangoapps/third_party_auth/pipeline.py @@ -61,7 +61,6 @@ import random import string # pylint: disable-msg=deprecated-module from collections import OrderedDict import urllib -from ipware.ip import get_ip import analytics from eventtracking import tracker @@ -534,7 +533,7 @@ def ensure_user_information(strategy, auth_entry, backend=None, user=None, socia @partial.partial -def set_logged_in_cookie(backend=None, user=None, strategy=None, auth_entry=None, *args, **kwargs): +def set_logged_in_cookies(backend=None, user=None, strategy=None, auth_entry=None, *args, **kwargs): """This pipeline step sets the "logged in" cookie for authenticated users. Some installations have a marketing site front-end separate from @@ -566,7 +565,7 @@ def set_logged_in_cookie(backend=None, user=None, strategy=None, auth_entry=None # 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 = student.helpers.is_logged_in_cookie_set(request) + has_cookie = student.cookies.is_logged_in_cookie_set(request) if not has_cookie: try: redirect_url = get_complete_url(backend.name) @@ -577,7 +576,7 @@ def set_logged_in_cookie(backend=None, user=None, strategy=None, auth_entry=None pass else: response = redirect(redirect_url) - return student.helpers.set_logged_in_cookie(request, response) + return student.cookies.set_logged_in_cookies(request, response, user) @partial.partial diff --git a/common/djangoapps/third_party_auth/settings.py b/common/djangoapps/third_party_auth/settings.py index 12b362759c..e9468b7ce8 100644 --- a/common/djangoapps/third_party_auth/settings.py +++ b/common/djangoapps/third_party_auth/settings.py @@ -111,7 +111,7 @@ def _set_global_settings(django_settings): 'social.pipeline.social_auth.associate_user', 'social.pipeline.social_auth.load_extra_data', 'social.pipeline.user.user_details', - 'third_party_auth.pipeline.set_logged_in_cookie', + 'third_party_auth.pipeline.set_logged_in_cookies', 'third_party_auth.pipeline.login_analytics', ) diff --git a/common/djangoapps/third_party_auth/tests/specs/base.py b/common/djangoapps/third_party_auth/tests/specs/base.py index 0db38295e1..25e060c099 100644 --- a/common/djangoapps/third_party_auth/tests/specs/base.py +++ b/common/djangoapps/third_party_auth/tests/specs/base.py @@ -372,11 +372,15 @@ class IntegrationTest(testutil.TestCase, test.TestCase): response["Location"], pipeline.get_complete_url(self.PROVIDER_CLASS.BACKEND_CLASS.name) ) - self.assertEqual(response.cookies[django_settings.EDXMKTG_COOKIE_NAME].value, 'true') + self.assertEqual(response.cookies[django_settings.EDXMKTG_LOGGED_IN_COOKIE_NAME].value, 'true') + self.assertIn(django_settings.EDXMKTG_USER_INFO_COOKIE_NAME, response.cookies) - def set_logged_in_cookie(self, request): + def set_logged_in_cookies(self, request): """Simulate setting the marketing site cookie on the request. """ - request.COOKIES[django_settings.EDXMKTG_COOKIE_NAME] = 'true' + request.COOKIES[django_settings.EDXMKTG_LOGGED_IN_COOKIE_NAME] = 'true' + request.COOKIES[django_settings.EDXMKTG_USER_INFO_COOKIE_NAME] = json.dumps({ + 'version': django_settings.EDXMKTG_USER_INFO_COOKIE_VERSION, + }) # Actual tests, executed once per child. @@ -434,7 +438,7 @@ class IntegrationTest(testutil.TestCase, test.TestCase): )) # Set the cookie and try again - self.set_logged_in_cookie(request) + self.set_logged_in_cookies(request) # Fire off the auth pipeline to link. self.assert_redirect_to_dashboard_looks_correct(actions.do_complete( @@ -456,7 +460,7 @@ class IntegrationTest(testutil.TestCase, test.TestCase): self.assert_social_auth_exists_for_user(user, strategy) # We're already logged in, so simulate that the cookie is set correctly - self.set_logged_in_cookie(request) + self.set_logged_in_cookies(request) # Instrument the pipeline to get to the dashboard with the full # expected state. @@ -582,7 +586,7 @@ class IntegrationTest(testutil.TestCase, test.TestCase): )) # Set the cookie and try again - self.set_logged_in_cookie(request) + self.set_logged_in_cookies(request) self.assert_redirect_to_dashboard_looks_correct( actions.do_complete(request.backend, social_views._do_login, user=user)) @@ -683,7 +687,7 @@ class IntegrationTest(testutil.TestCase, test.TestCase): )) # Set the cookie and try again - self.set_logged_in_cookie(request) + self.set_logged_in_cookies(request) self.assert_redirect_to_dashboard_looks_correct( actions.do_complete(strategy.request.backend, social_views._do_login, user=created_user)) # Now the user has been redirected to the dashboard. Their third party account should now be linked. diff --git a/lms/envs/aws.py b/lms/envs/aws.py index 1f113305a6..58fab6cd8c 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -155,6 +155,12 @@ SESSION_COOKIE_HTTPONLY = ENV_TOKENS.get('SESSION_COOKIE_HTTPONLY', True) REGISTRATION_EXTRA_FIELDS = ENV_TOKENS.get('REGISTRATION_EXTRA_FIELDS', REGISTRATION_EXTRA_FIELDS) SESSION_COOKIE_SECURE = ENV_TOKENS.get('SESSION_COOKIE_SECURE', SESSION_COOKIE_SECURE) +# Set the names of cookies shared with the marketing site +# These have the same cookie domain as the session, which in production +# usually includes subdomains. +EDXMKTG_LOGGED_IN_COOKIE_NAME = ENV_TOKENS.get('EDXMKTG_LOGGED_IN_COOKIE_NAME', EDXMKTG_LOGGED_IN_COOKIE_NAME) +EDXMKTG_USER_INFO_COOKIE_NAME = ENV_TOKENS.get('EDXMKTG_USER_INFO_COOKIE_NAME', EDXMKTG_USER_INFO_COOKIE_NAME) + CMS_BASE = ENV_TOKENS.get('CMS_BASE', 'studio.edx.org') # allow for environments to specify what cookie name our login subsystem should use diff --git a/lms/envs/common.py b/lms/envs/common.py index 1aa40c9cf5..bfefcd10d5 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1916,7 +1916,10 @@ CSRF_COOKIE_AGE = 60 * 60 * 24 * 7 * 52 ######################### MARKETING SITE ############################### -EDXMKTG_COOKIE_NAME = 'edxloggedin' +EDXMKTG_LOGGED_IN_COOKIE_NAME = 'edxloggedin' +EDXMKTG_USER_INFO_COOKIE_NAME = 'edx-user-info' +EDXMKTG_USER_INFO_COOKIE_VERSION = 1 + MKTG_URLS = {} MKTG_URL_LINK_MAP = { 'ABOUT': 'about', diff --git a/openedx/core/djangoapps/user_api/tests/test_views.py b/openedx/core/djangoapps/user_api/tests/test_views.py index 34f3b780a6..6034a9cc2a 100644 --- a/openedx/core/djangoapps/user_api/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/tests/test_views.py @@ -1264,7 +1264,8 @@ class RegistrationViewTest(ApiTestCase): "honor_code": "true", }) self.assertHttpOK(response) - self.assertIn(settings.EDXMKTG_COOKIE_NAME, self.client.cookies) + self.assertIn(settings.EDXMKTG_LOGGED_IN_COOKIE_NAME, self.client.cookies) + self.assertIn(settings.EDXMKTG_USER_INFO_COOKIE_NAME, self.client.cookies) user = User.objects.get(username=self.USERNAME) account_settings = get_account_settings(user) diff --git a/openedx/core/djangoapps/user_api/views.py b/openedx/core/djangoapps/user_api/views.py index e357cd97cb..79a119378e 100644 --- a/openedx/core/djangoapps/user_api/views.py +++ b/openedx/core/djangoapps/user_api/views.py @@ -25,7 +25,7 @@ import third_party_auth from django_comment_common.models import Role from edxmako.shortcuts import marketing_link from student.views import create_account_with_params -from student.helpers import set_logged_in_cookie +from student.cookies import set_logged_in_cookies from openedx.core.lib.api.authentication import SessionAuthenticationAllowInactiveUser from util.json_request import JsonResponse from .preferences.api import update_email_opt_in @@ -295,7 +295,7 @@ class RegistrationView(APIView): data["terms_of_service"] = data["honor_code"] try: - create_account_with_params(request, data) + user = create_account_with_params(request, data) except ValidationError as err: # Should only get non-field errors from this function assert NON_FIELD_ERRORS not in err.message_dict @@ -307,7 +307,7 @@ class RegistrationView(APIView): return JsonResponse(errors, status=400) response = JsonResponse({"success": True}) - set_logged_in_cookie(request, response) + set_logged_in_cookies(request, response, user) return response def _add_email_field(self, form_desc, required=True):