diff --git a/common/djangoapps/student/cookies.py b/common/djangoapps/student/cookies.py index 508da869f9..b508f93339 100644 --- a/common/djangoapps/student/cookies.py +++ b/common/djangoapps/student/cookies.py @@ -2,43 +2,18 @@ Utility functions for setting "logged in" cookies used by subdomains. """ -import json import time +import json -import six -import urllib +from django.dispatch import Signal + +from django.utils.http import cookie_date from django.conf import settings from django.core.urlresolvers import reverse, NoReverseMatch -from django.dispatch import Signal -from django.utils.http import cookie_date - -from student.models import CourseEnrollment CREATE_LOGON_COOKIE = Signal(providing_args=["user", "response"]) -def _get_cookie_settings(request): - """ Returns the common cookie settings (e.g. expiration time). """ - - 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': '/', - 'httponly': None, - } - - return cookie_settings - - def set_logged_in_cookies(request, response, user): """ Set cookies indicating that the user is logged in. @@ -74,7 +49,21 @@ def set_logged_in_cookies(request, response, user): HttpResponse """ - cookie_settings = _get_cookie_settings(request) + 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': '/', + '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. @@ -87,41 +76,6 @@ def set_logged_in_cookies(request, response, user): **cookie_settings ) - set_user_info_cookie(response, request, user) - - # give signal receivers a chance to add cookies - CREATE_LOGON_COOKIE.send(sender=None, user=user, response=response) - - return response - - -def set_user_info_cookie(response, request, user): - """ Sets the user info cookie on the response. """ - cookie_settings = _get_cookie_settings(request) - - # In production, TLS should be enabled so that this cookie is encrypted - # when we send it. We also need to set "secure" to True so that the browser - # will transmit it only over secure connections. - # - # 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 - # whether it needs to set the cookie or continue to the next pipeline stage. - user_info_cookie_is_secure = request.is_secure() - user_info = get_user_info_cookie_data(request, user) - - response.set_cookie( - settings.EDXMKTG_USER_INFO_COOKIE_NAME.encode('utf-8'), - urllib.quote(json.dumps(user_info)), - secure=user_info_cookie_is_secure, - **cookie_settings - ) - - -def get_user_info_cookie_data(request, user): - """ Returns information that wil populate the user info cookie. """ - # 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" @@ -140,24 +94,38 @@ def get_user_info_cookie_data(request, user): pass # Convert relative URL paths to absolute URIs - for url_name, url_path in six.iteritems(header_urls): + for url_name, url_path in header_urls.iteritems(): header_urls[url_name] = request.build_absolute_uri(url_path) - enrollments = [] - for enrollment in CourseEnrollment.enrollments_for_user(user): - enrollments.append({ - 'course_run_id': six.text_type(enrollment.course_id), - 'seat_type': enrollment.mode - }) - user_info = { 'version': settings.EDXMKTG_USER_INFO_COOKIE_VERSION, 'username': user.username, + 'email': user.email, 'header_urls': header_urls, - 'enrollments': enrollments, } - return user_info + # In production, TLS should be enabled so that this cookie is encrypted + # when we send it. We also need to set "secure" to True so that the browser + # will transmit it only over secure connections. + # + # 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 + # whether it needs to set the cookie or continue to the next pipeline stage. + user_info_cookie_is_secure = request.is_secure() + + response.set_cookie( + settings.EDXMKTG_USER_INFO_COOKIE_NAME.encode('utf-8'), + json.dumps(user_info), + secure=user_info_cookie_is_secure, + **cookie_settings + ) + + # give signal receivers a chance to add cookies + CREATE_LOGON_COOKIE.send(sender=None, user=user, response=response) + + return response def delete_logged_in_cookies(response): diff --git a/common/djangoapps/student/tests/factories.py b/common/djangoapps/student/tests/factories.py index 129b22fc2c..6f246dc413 100644 --- a/common/djangoapps/student/tests/factories.py +++ b/common/djangoapps/student/tests/factories.py @@ -1,31 +1,28 @@ """Provides factories for student models.""" import random -from datetime import datetime -from uuid import uuid4 -import factory +from student.models import (User, UserProfile, Registration, + CourseEnrollmentAllowed, CourseEnrollment, + PendingEmailChange, UserStanding, + CourseAccessRole) +from course_modes.models import CourseMode from django.contrib.auth.models import Group, AnonymousUser +from datetime import datetime +import factory from factory import lazy_attribute from factory.django import DjangoModelFactory -from opaque_keys.edx.locations import SlashSeparatedCourseKey +from uuid import uuid4 from pytz import UTC - -from course_modes.models import CourseMode -from student.models import ( - User, UserProfile, Registration, CourseEnrollmentAllowed, CourseEnrollment, PendingEmailChange, UserStanding, - CourseAccessRole -) +from opaque_keys.edx.locations import SlashSeparatedCourseKey # Factories are self documenting # pylint: disable=missing-docstring -USER_PASSWORD = 'test' - class GroupFactory(DjangoModelFactory): class Meta(object): model = Group - django_get_or_create = ('name',) + django_get_or_create = ('name', ) name = factory.Sequence(u'group{0}'.format) @@ -42,7 +39,7 @@ class UserStandingFactory(DjangoModelFactory): class UserProfileFactory(DjangoModelFactory): class Meta(object): model = UserProfile - django_get_or_create = ('user',) + django_get_or_create = ('user', ) user = None name = factory.LazyAttribute(u'{0.user.first_name} {0.user.last_name}'.format) @@ -86,7 +83,7 @@ class UserFactory(DjangoModelFactory): username = factory.Sequence(u'robot{0}'.format) email = factory.Sequence(u'robot+test+{0}@edx.org'.format) - password = factory.PostGenerationMethodCall('set_password', USER_PASSWORD) + password = factory.PostGenerationMethodCall('set_password', 'test') first_name = factory.Sequence(u'Robot{0}'.format) last_name = 'Test' is_staff = False @@ -158,7 +155,6 @@ class PendingEmailChangeFactory(DjangoModelFactory): new_email: sequence of new+email+{}@edx.org activation_key: sequence of integers, padded to 30 characters """ - class Meta(object): model = PendingEmailChange diff --git a/common/djangoapps/student/tests/test_cookies.py b/common/djangoapps/student/tests/test_cookies.py deleted file mode 100644 index fc4ba276dc..0000000000 --- a/common/djangoapps/student/tests/test_cookies.py +++ /dev/null @@ -1,66 +0,0 @@ -# pylint: disable=missing-docstring -from __future__ import unicode_literals - -import six -from django.conf import settings -from django.core.urlresolvers import reverse -from django.test import RequestFactory -from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory - -from student.cookies import get_user_info_cookie_data -from student.tests.factories import UserFactory, CourseEnrollmentFactory - - -class CookieTests(SharedModuleStoreTestCase): - @classmethod - def setUpClass(cls): - super(CookieTests, cls).setUpClass() - cls.course = CourseFactory() - - def setUp(self): - super(CookieTests, self).setUp() - self.user = UserFactory.create() - - def _get_expected_header_urls(self, request): - expected_header_urls = { - 'logout': reverse('logout'), - } - - # Studio (CMS) does not have the URLs below - if settings.ROOT_URLCONF == 'lms.urls': - expected_header_urls.update({ - 'account_settings': reverse('account_settings'), - 'learner_profile': reverse('learner_profile', kwargs={'username': self.user.username}), - }) - - # Convert relative URL paths to absolute URIs - for url_name, url_path in six.iteritems(expected_header_urls): - expected_header_urls[url_name] = request.build_absolute_uri(url_path) - - return expected_header_urls - - def test_get_user_info_cookie_data(self): - """ Verify the function returns data that """ - request = RequestFactory().get('/') - request.user = self.user - - enrollment_mode = 'verified' - course_id = self.course.id # pylint: disable=no-member - CourseEnrollmentFactory.create(user=self.user, course_id=course_id, mode=enrollment_mode) - - actual = get_user_info_cookie_data(request, self.user) - - expected_enrollments = [{ - 'course_run_id': six.text_type(course_id), - 'seat_type': enrollment_mode, - }] - - expected = { - 'version': settings.EDXMKTG_USER_INFO_COOKIE_VERSION, - 'username': self.user.username, - 'header_urls': self._get_expected_header_urls(request), - 'enrollments': expected_enrollments, - } - - self.assertDictEqual(actual, expected) diff --git a/common/djangoapps/student/tests/test_login.py b/common/djangoapps/student/tests/test_login.py index c833cd6699..6f4896516f 100644 --- a/common/djangoapps/student/tests/test_login.py +++ b/common/djangoapps/student/tests/test_login.py @@ -4,7 +4,6 @@ Tests for student activation and login import json import unittest -import urllib from django.test import TestCase from django.test.client import Client from django.test.utils import override_settings @@ -170,10 +169,14 @@ class LoginTest(CacheIsolationTestCase): # 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(urllib.unquote(cookie.value)) + 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(): diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 92c84a2e52..46e54b2bfa 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -106,7 +106,7 @@ from student.helpers import ( auth_pipeline_urls, get_next_url_for_login_page, DISABLE_UNENROLL_CERT_STATES, ) -from student.cookies import set_logged_in_cookies, delete_logged_in_cookies, set_user_info_cookie +from student.cookies import set_logged_in_cookies, delete_logged_in_cookies from student.models import anonymous_id_for_user, UserAttribute, EnrollStatusChange from shoppingcart.models import DonationConfiguration, CourseRegistrationCode @@ -749,9 +749,7 @@ def dashboard(request): 'ecommerce_payment_page': ecommerce_service.payment_page_url(), }) - response = render_to_response('dashboard.html', context) - set_user_info_cookie(response, request, user) - return response + return render_to_response('dashboard.html', context) def _create_recent_enrollment_message(course_enrollments, course_modes): # pylint: disable=invalid-name diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index fac09f2971..47e5b894f7 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -94,7 +94,6 @@ requests-oauthlib==0.4.1 scipy==0.14.0 Shapely==1.2.16 singledispatch==3.4.0.2 -six>=1.10.0,<2.0.0 sorl-thumbnail==12.3 sortedcontainers==0.9.2 stevedore==1.10.0