diff --git a/common/djangoapps/external_auth/login_and_register.py b/common/djangoapps/external_auth/login_and_register.py new file mode 100644 index 0000000000..997b2deb70 --- /dev/null +++ b/common/djangoapps/external_auth/login_and_register.py @@ -0,0 +1,92 @@ +"""Intercept login and registration requests. + +This module contains legacy code originally from `student.views`. +""" +import re + +from django.conf import settings +from django.shortcuts import redirect +from django.core.urlresolvers import reverse +import external_auth.views + +from xmodule.modulestore.django import modulestore +from opaque_keys.edx.locations import SlashSeparatedCourseKey + + +# pylint: disable=fixme +# TODO: This function is kind of gnarly/hackish/etc and is only used in one location. +# It'd be awesome if we could get rid of it; manually parsing course_id strings form larger strings +# seems Probably Incorrect +def _parse_course_id_from_string(input_str): + """ + Helper function to determine if input_str (typically the queryparam 'next') contains a course_id. + @param input_str: + @return: the course_id if found, None if not + """ + m_obj = re.match(r'^/courses/{}'.format(settings.COURSE_ID_PATTERN), input_str) + if m_obj: + return SlashSeparatedCourseKey.from_deprecated_string(m_obj.group('course_id')) + return None + + +def _get_course_enrollment_domain(course_id): + """ + Helper function to get the enrollment domain set for a course with id course_id + @param course_id: + @return: + """ + course = modulestore().get_course(course_id) + if course is None: + return None + + return course.enrollment_domain + + +def login(request): + """Allow external auth to intercept and handle a login request. + + Arguments: + request (Request): A request for the login page. + + Returns: + Response or None + + """ + # Default to a `None` response, indicating that external auth + # is not handling the request. + response = None + + if settings.FEATURES['AUTH_USE_CERTIFICATES'] and external_auth.views.ssl_get_cert_from_request(request): + # SSL login doesn't require a view, so redirect + # branding and allow that to process the login if it + # is enabled and the header is in the request. + response = external_auth.views.redirect_with_get('root', request.GET) + elif settings.FEATURES.get('AUTH_USE_CAS'): + # If CAS is enabled, redirect auth handling to there + response = redirect(reverse('cas-login')) + elif settings.FEATURES.get('AUTH_USE_SHIB'): + redirect_to = request.GET.get('next') + if redirect_to: + course_id = _parse_course_id_from_string(redirect_to) + if course_id and _get_course_enrollment_domain(course_id): + response = external_auth.views.course_specific_login(request, course_id.to_deprecated_string()) + + return response + + +def register(request): + """Allow external auth to intercept and handle a registration request. + + Arguments: + request (Request): A request for the registration page. + + Returns: + Response or None + + """ + response = None + if settings.FEATURES.get('AUTH_USE_CERTIFICATES_IMMEDIATE_SIGNUP'): + # Redirect to branding to process their certificate if SSL is enabled + # and registration is disabled. + response = external_auth.views.redirect_with_get('root', request.GET) + return response diff --git a/common/djangoapps/external_auth/tests/test_ssl.py b/common/djangoapps/external_auth/tests/test_ssl.py index 18cee1e35b..ba47942b6e 100644 --- a/common/djangoapps/external_auth/tests/test_ssl.py +++ b/common/djangoapps/external_auth/tests/test_ssl.py @@ -220,8 +220,7 @@ class SSLClientTest(ModuleStoreTestCase): # Test that they do signin if they don't have a cert response = self.client.get(reverse('signin_user')) self.assertEqual(200, response.status_code) - self.assertTrue('login_form' in response.content - or 'login-form' in response.content) + self.assertTrue('login-and-registration-container' in response.content) # And get directly logged in otherwise response = self.client.get( diff --git a/common/djangoapps/student/helpers.py b/common/djangoapps/student/helpers.py index d874595af9..4c7e2c5e58 100644 --- a/common/djangoapps/student/helpers.py +++ b/common/djangoapps/student/helpers.py @@ -4,78 +4,10 @@ from datetime import datetime from pytz import UTC from django.utils.http import cookie_date from django.conf import settings -from django.core.urlresolvers import reverse -from opaque_keys.edx.keys import CourseKey -from course_modes.models import CourseMode -from third_party_auth import ( # pylint: disable=unused-import - pipeline, provider, - is_enabled as third_party_auth_enabled -) + from verify_student.models import SoftwareSecurePhotoVerification # pylint: disable=F0401 - - -def auth_pipeline_urls(auth_entry, redirect_url=None, course_id=None): - """Retrieve URLs for each enabled third-party auth provider. - - These URLs are used on the "sign up" and "sign in" buttons - on the login/registration forms to allow users to begin - authentication with a third-party provider. - - Optionally, we can redirect the user to an arbitrary - url after auth completes successfully. We use this - to redirect the user to a page that required login, - or to send users to the payment flow when enrolling - in a course. - - Args: - auth_entry (string): Either `pipeline.AUTH_ENTRY_LOGIN` or `pipeline.AUTH_ENTRY_REGISTER` - - Keyword Args: - redirect_url (unicode): If provided, send users to this URL - after they successfully authenticate. - - course_id (unicode): The ID of the course the user is enrolling in. - We use this to send users to the track selection page - if the course has a payment option. - Note that `redirect_url` takes precedence over the redirect - to the track selection page. - - Returns: - dict mapping provider names to URLs - - """ - if not third_party_auth_enabled(): - return {} - - if redirect_url is not None: - pipeline_redirect = redirect_url - elif course_id is not None: - # If the course is white-label (paid), then we send users - # to the shopping cart. (There is a third party auth pipeline - # step that will add the course to the cart.) - if CourseMode.is_white_label(CourseKey.from_string(course_id)): - pipeline_redirect = reverse("shoppingcart.views.show_cart") - - # Otherwise, send the user to the track selection page. - # The track selection page may redirect the user to the dashboard - # (if the only available mode is honor), or directly to verification - # (for professional ed). - else: - pipeline_redirect = reverse( - "course_modes_choose", - kwargs={'course_id': unicode(course_id)} - ) - else: - pipeline_redirect = None - - return { - provider.NAME: pipeline.get_login_url( - provider.NAME, auth_entry, - enroll_course_id=course_id, - redirect_url=pipeline_redirect - ) - for provider in provider.Registry.enabled() - } +from course_modes.models import CourseMode +from student_account.helpers import auth_pipeline_urls # pylint: disable=unused-import,import-error def set_logged_in_cookie(request, response): diff --git a/common/djangoapps/student/tests/test_login.py b/common/djangoapps/student/tests/test_login.py index 828e951cbb..c205f056c8 100644 --- a/common/djangoapps/student/tests/test_login.py +++ b/common/djangoapps/student/tests/test_login.py @@ -14,18 +14,13 @@ from django.http import HttpResponseBadRequest, HttpResponse from external_auth.models import ExternalAuthMap import httpretty from mock import patch -from opaque_keys.edx.locations import SlashSeparatedCourseKey from social.apps.django_app.default.models import UserSocialAuth -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE from student.tests.factories import UserFactory, RegistrationFactory, UserProfileFactory -from student.views import ( - _parse_course_id_from_string, - _get_course_enrollment_domain, - login_oauth_token, -) +from student.views import login_oauth_token + from xmodule.modulestore.tests.factories import CourseFactory -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, TEST_DATA_MOCK_MODULESTORE class LoginTest(TestCase): @@ -324,24 +319,6 @@ class LoginTest(TestCase): self.assertNotIn(log_string, format_string) -class UtilFnTest(TestCase): - """ - Tests for utility functions in student.views - """ - def test__parse_course_id_from_string(self): - """ - Tests the _parse_course_id_from_string util function - """ - COURSE_ID = u'org/num/run' # pylint: disable=invalid-name - COURSE_URL = u'/courses/{}/otherstuff'.format(COURSE_ID) # pylint: disable=invalid-name - NON_COURSE_URL = u'/blahblah' # pylint: disable=invalid-name - self.assertEqual( - _parse_course_id_from_string(COURSE_URL), - SlashSeparatedCourseKey.from_deprecated_string(COURSE_ID) - ) - self.assertIsNone(_parse_course_id_from_string(NON_COURSE_URL)) - - @override_settings(MODULESTORE=TEST_DATA_MOCK_MODULESTORE) class ExternalAuthShibTest(ModuleStoreTestCase): """ @@ -387,15 +364,6 @@ class ExternalAuthShibTest(ModuleStoreTestCase): 'redirect': reverse('shib-login'), }) - @unittest.skipUnless(settings.FEATURES.get('AUTH_USE_SHIB'), "AUTH_USE_SHIB not set") - def test__get_course_enrollment_domain(self): - """ - Tests the _get_course_enrollment_domain utility function - """ - self.assertIsNone(_get_course_enrollment_domain(SlashSeparatedCourseKey("I", "DONT", "EXIST"))) - self.assertIsNone(_get_course_enrollment_domain(self.course.id)) - self.assertEqual(self.shib_course.enrollment_domain, _get_course_enrollment_domain(self.shib_course.id)) - @unittest.skipUnless(settings.FEATURES.get('AUTH_USE_SHIB'), "AUTH_USE_SHIB not set") def test_login_required_dashboard(self): """ @@ -416,7 +384,7 @@ class ExternalAuthShibTest(ModuleStoreTestCase): noshib_response = self.client.get(TARGET_URL, follow=True) self.assertEqual(noshib_response.redirect_chain[-1], ('http://testserver/accounts/login?next={url}'.format(url=TARGET_URL), 302)) - self.assertContains(noshib_response, ("Log into your {platform_name} Account | {platform_name}" + self.assertContains(noshib_response, ("Log in or Register | {platform_name}" .format(platform_name=settings.PLATFORM_NAME))) self.assertEqual(noshib_response.status_code, 200) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 8fdc96c2fb..d995a72001 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -75,6 +75,10 @@ from django_comment_common.models import Role from external_auth.models import ExternalAuthMap import external_auth.views +from external_auth.login_and_register import ( + login as external_auth_login, + register as external_auth_register +) from bulk_email.models import Optout, CourseAuthorization import shoppingcart @@ -348,18 +352,14 @@ def _cert_info(user, course, cert_status): @ensure_csrf_cookie def signin_user(request): + """This view will display the non-modal login form + + DEPRECATION WARNING: This view will eventually be deprecated and replaced + with the combined login/registration page in `student_account.views`. """ - This view will display the non-modal login form - """ - if (settings.FEATURES['AUTH_USE_CERTIFICATES'] and - external_auth.views.ssl_get_cert_from_request(request)): - # SSL login doesn't require a view, so redirect - # branding and allow that to process the login if it - # is enabled and the header is in the request. - return external_auth.views.redirect_with_get('root', request.GET) - if settings.FEATURES.get('AUTH_USE_CAS'): - # If CAS is enabled, redirect auth handling to there - return redirect(reverse('cas-login')) + external_auth_response = external_auth_login(request) + if external_auth_response is not None: + return external_auth_response if request.user.is_authenticated(): return redirect(reverse('dashboard')) @@ -383,15 +383,17 @@ def signin_user(request): @ensure_csrf_cookie def register_user(request, extra_context=None): - """ - This view will display the non-modal registration form + """This view will display the non-modal registration form + + DEPRECATION WARNING: This view will eventually be deprecated and replaced + with the combined login/registration page in `student_account.views`. """ if request.user.is_authenticated(): return redirect(reverse('dashboard')) - if settings.FEATURES.get('AUTH_USE_CERTIFICATES_IMMEDIATE_SIGNUP'): - # Redirect to branding to process their certificate if SSL is enabled - # and registration is disabled. - return external_auth.views.redirect_with_get('root', request.GET) + + external_auth_response = external_auth_register(request) + if external_auth_response is not None: + return external_auth_response course_id = request.GET.get('course_id') @@ -897,55 +899,21 @@ def change_enrollment(request, check_access=True): return HttpResponseBadRequest(_("Enrollment action is invalid")) -# TODO: This function is kind of gnarly/hackish/etc and is only used in one location. -# It'd be awesome if we could get rid of it; manually parsing course_id strings form larger strings -# seems Probably Incorrect -def _parse_course_id_from_string(input_str): - """ - Helper function to determine if input_str (typically the queryparam 'next') contains a course_id. - @param input_str: - @return: the course_id if found, None if not - """ - m_obj = re.match(r'^/courses/{}'.format(settings.COURSE_ID_PATTERN), input_str) - if m_obj: - return SlashSeparatedCourseKey.from_deprecated_string(m_obj.group('course_id')) - return None - - -def _get_course_enrollment_domain(course_id): - """ - Helper function to get the enrollment domain set for a course with id course_id - @param course_id: - @return: - """ - course = modulestore().get_course(course_id) - if course is None: - return None - - return course.enrollment_domain - - @never_cache @ensure_csrf_cookie def accounts_login(request): """ This view is mainly used as the redirect from the @login_required decorator. I don't believe that the login path linked from the homepage uses it. - """ - if settings.FEATURES.get('AUTH_USE_CAS'): - return redirect(reverse('cas-login')) - if settings.FEATURES['AUTH_USE_CERTIFICATES']: - # SSL login doesn't require a view, so login - # directly here - return external_auth.views.ssl_login(request) - # see if the "next" parameter has been set, whether it has a course context, and if so, whether - # there is a course-specific place to redirect - redirect_to = request.GET.get('next') - if redirect_to: - course_id = _parse_course_id_from_string(redirect_to) - if course_id and _get_course_enrollment_domain(course_id): - return external_auth.views.course_specific_login(request, course_id.to_deprecated_string()) + DEPRECATION WARNING: This view will eventually be deprecated and replaced + with the combined login/registration page in `student_account.views`. + """ + external_auth_response = external_auth_login(request) + if external_auth_response is not None: + return external_auth_response + + redirect_to = request.GET.get('next') context = { 'pipeline_running': 'false', 'pipeline_url': auth_pipeline_urls(pipeline.AUTH_ENTRY_LOGIN, redirect_url=redirect_to), diff --git a/common/djangoapps/third_party_auth/pipeline.py b/common/djangoapps/third_party_auth/pipeline.py index 4564f5533a..3eadf28ca9 100644 --- a/common/djangoapps/third_party_auth/pipeline.py +++ b/common/djangoapps/third_party_auth/pipeline.py @@ -111,10 +111,10 @@ AUTH_ENTRY_LOGIN = 'login' AUTH_ENTRY_PROFILE = 'profile' AUTH_ENTRY_REGISTER = 'register' -# pylint: disable=fixme -# TODO (ECOM-369): Replace `AUTH_ENTRY_LOGIN` and `AUTH_ENTRY_REGISTER` -# with these values once the A/B test completes, then delete -# these constants. +# This is left-over from an A/B test +# of the new combined login/registration page (ECOM-369) +# We need to keep both the old and new entry points +# until every session from before the test ended has expired. AUTH_ENTRY_LOGIN_2 = 'account_login' AUTH_ENTRY_REGISTER_2 = 'account_register' @@ -129,12 +129,13 @@ AUTH_ENTRY_API = 'api' # to load that depend on this module. AUTH_DISPATCH_URLS = { AUTH_ENTRY_DASHBOARD: '/dashboard', - AUTH_ENTRY_LOGIN: '/login', - AUTH_ENTRY_REGISTER: '/register', + AUTH_ENTRY_LOGIN: '/account/login', + AUTH_ENTRY_REGISTER: '/account/register', - # TODO (ECOM-369): Replace the dispatch URLs - # for `AUTH_ENTRY_LOGIN` and `AUTH_ENTRY_REGISTER` - # with these values, but DO NOT DELETE THESE KEYS. + # This is left-over from an A/B test + # of the new combined login/registration page (ECOM-369) + # We need to keep both the old and new entry points + # until every session from before the test ended has expired. AUTH_ENTRY_LOGIN_2: '/account/login/', AUTH_ENTRY_REGISTER_2: '/account/register/', @@ -150,11 +151,10 @@ _AUTH_ENTRY_CHOICES = frozenset([ AUTH_ENTRY_PROFILE, AUTH_ENTRY_REGISTER, - # TODO (ECOM-369): For the A/B test of the combined - # login/registration, we needed to introduce two - # additional end-points. Once the test completes, - # delete these constants from the choices list. - # pylint: disable=fixme + # This is left-over from an A/B test + # of the new combined login/registration page (ECOM-369) + # We need to keep both the old and new entry points + # until every session from before the test ended has expired. AUTH_ENTRY_LOGIN_2, AUTH_ENTRY_REGISTER_2, @@ -437,31 +437,16 @@ def parse_query_params(strategy, response, *args, **kwargs): # Whether the auth pipeline entered from /dashboard. 'is_dashboard': auth_entry == AUTH_ENTRY_DASHBOARD, # Whether the auth pipeline entered from /login. - 'is_login': auth_entry == AUTH_ENTRY_LOGIN, + 'is_login': auth_entry in [AUTH_ENTRY_LOGIN, AUTH_ENTRY_LOGIN_2], # Whether the auth pipeline entered from /register. - 'is_register': auth_entry == AUTH_ENTRY_REGISTER, + 'is_register': auth_entry in [AUTH_ENTRY_REGISTER, AUTH_ENTRY_REGISTER_2], # Whether the auth pipeline entered from /profile. 'is_profile': auth_entry == AUTH_ENTRY_PROFILE, # Whether the auth pipeline entered from an API 'is_api': auth_entry == AUTH_ENTRY_API, - - # TODO (ECOM-369): Delete these once the A/B test - # for the combined login/registration form completes. - # pylint: disable=fixme - 'is_login_2': auth_entry == AUTH_ENTRY_LOGIN_2, - 'is_register_2': auth_entry == AUTH_ENTRY_REGISTER_2, } -# TODO (ECOM-369): Once the A/B test of the combined login/registration -# form completes, we will be able to remove the extra login/registration -# end-points. HOWEVER, users who used the new forms during the A/B -# test may still have values for "is_login_2" and "is_register_2" -# in their sessions. For this reason, we need to continue accepting -# these kwargs in `redirect_to_supplementary_form`, but -# these should redirect to the same location as "is_login" and "is_register" -# (whichever login/registration end-points win in the test). -# pylint: disable=fixme @partial.partial def ensure_user_information( strategy, @@ -497,36 +482,23 @@ def ensure_user_information( # invariants have been violated and future misbehavior is likely. user_inactive = user and not user.is_active user_unset = user is None - dispatch_to_login = is_login and (user_unset or user_inactive) + dispatch_to_login = (is_login or is_login_2) and (user_unset or user_inactive) + dispatch_to_register = (is_register or is_register_2) and user_unset reject_api_request = is_api and (user_unset or user_inactive) if reject_api_request: # Content doesn't matter; we just want to exit the pipeline return HttpResponseBadRequest() - # TODO (ECOM-369): Consolidate this with `dispatch_to_login` - # once the A/B test completes. # pylint: disable=fixme - dispatch_to_login_2 = is_login_2 and (user_unset or user_inactive) - if is_dashboard or is_profile: return if dispatch_to_login: return redirect(AUTH_DISPATCH_URLS[AUTH_ENTRY_LOGIN], name='signin_user') - # TODO (ECOM-369): Consolidate this with `dispatch_to_login` - # once the A/B test completes. # pylint: disable=fixme - if dispatch_to_login_2: - return redirect(AUTH_DISPATCH_URLS[AUTH_ENTRY_LOGIN_2]) - - if is_register and user_unset: + if dispatch_to_register: return redirect(AUTH_DISPATCH_URLS[AUTH_ENTRY_REGISTER], name='register_user') - # TODO (ECOM-369): Consolidate this with `is_register` - # once the A/B test completes. # pylint: disable=fixme - if is_register_2 and user_unset: - return redirect(AUTH_DISPATCH_URLS[AUTH_ENTRY_REGISTER_2]) - @partial.partial def set_logged_in_cookie(backend=None, user=None, request=None, is_api=None, *args, **kwargs): diff --git a/common/djangoapps/third_party_auth/tests/specs/base.py b/common/djangoapps/third_party_auth/tests/specs/base.py index 3c34754931..1b6da1739d 100644 --- a/common/djangoapps/third_party_auth/tests/specs/base.py +++ b/common/djangoapps/third_party_auth/tests/specs/base.py @@ -198,13 +198,6 @@ class IntegrationTest(testutil.TestCase, test.TestCase): self.assertFalse(payload.get('success')) self.assertIn('incorrect', payload.get('value')) - def assert_javascript_would_submit_login_form(self, boolean, response): - """Asserts we pass form submit JS the right boolean string.""" - argument_string = re.search( - r'function\ post_form_if_pipeline_running.*\(([a-z]+)\)', response.content, re.DOTALL).groups()[0] - self.assertIn(argument_string, ['true', 'false']) - self.assertEqual(boolean, True if argument_string == 'true' else False) - def assert_json_failure_response_is_inactive_account(self, response): """Asserts failure on /login for inactive account looks right.""" self.assertEqual(200, response.status_code) # Yes, it's a 200 even though it's a failure. @@ -234,15 +227,14 @@ class IntegrationTest(testutil.TestCase, test.TestCase): def assert_login_response_before_pipeline_looks_correct(self, response): """Asserts a GET of /login not in the pipeline looks correct.""" self.assertEqual(200, response.status_code) - self.assertIn('Sign in with ' + self.PROVIDER_CLASS.NAME, response.content) - self.assert_javascript_would_submit_login_form(False, response) - self.assert_signin_button_looks_functional(response.content, pipeline.AUTH_ENTRY_LOGIN) + # The combined login/registration page dynamically generates the login button, + # but we can still check that the provider name is passed in the data attribute + # for the container element. + self.assertIn(self.PROVIDER_CLASS.NAME, response.content) def assert_login_response_in_pipeline_looks_correct(self, response): """Asserts a GET of /login in the pipeline looks correct.""" self.assertEqual(200, response.status_code) - # Make sure the form submit JS is told to submit the form: - self.assert_javascript_would_submit_login_form(True, response) def assert_password_overridden_by_pipeline(self, username, password): """Verifies that the given password is not correct. @@ -262,27 +254,22 @@ class IntegrationTest(testutil.TestCase, test.TestCase): self.assertEqual(auth_settings._SOCIAL_AUTH_LOGIN_REDIRECT_URL, response.get('Location')) def assert_redirect_to_login_looks_correct(self, response): - """Asserts a response would redirect to /login.""" + """Asserts a response would redirect to /account/login.""" self.assertEqual(302, response.status_code) - self.assertEqual('/' + pipeline.AUTH_ENTRY_LOGIN, response.get('Location')) + self.assertEqual('/account/login', response.get('Location')) def assert_redirect_to_register_looks_correct(self, response): - """Asserts a response would redirect to /register.""" + """Asserts a response would redirect to /account/register.""" self.assertEqual(302, response.status_code) - self.assertEqual('/' + pipeline.AUTH_ENTRY_REGISTER, response.get('Location')) + self.assertEqual('/account/register', response.get('Location')) def assert_register_response_before_pipeline_looks_correct(self, response): """Asserts a GET of /register not in the pipeline looks correct.""" self.assertEqual(200, response.status_code) - self.assertIn('Sign up with ' + self.PROVIDER_CLASS.NAME, response.content) - self.assert_signin_button_looks_functional(response.content, pipeline.AUTH_ENTRY_REGISTER) - - def assert_signin_button_looks_functional(self, content, auth_entry): - """Asserts JS is available to signin buttons and has the right args.""" - self.assertTrue(re.search(r'function thirdPartySignin', content)) - self.assertEqual( - pipeline.get_login_url(self.PROVIDER_CLASS.NAME, auth_entry), - re.search(r"thirdPartySignin\(event, '([^']+)", content).groups()[0]) + # The combined login/registration page dynamically generates the register button, + # but we can still check that the provider name is passed in the data attribute + # for the container element. + self.assertIn(self.PROVIDER_CLASS.NAME, response.content) def assert_social_auth_does_not_exist_for_user(self, user, strategy): """Asserts a user does not have an auth with the expected provider.""" @@ -404,10 +391,10 @@ class IntegrationTest(testutil.TestCase, test.TestCase): # Actual tests, executed once per child. def test_canceling_authentication_redirects_to_login_when_auth_entry_login(self): - self.assert_exception_redirect_looks_correct('/login', auth_entry=pipeline.AUTH_ENTRY_LOGIN) + self.assert_exception_redirect_looks_correct('/account/login', auth_entry=pipeline.AUTH_ENTRY_LOGIN) def test_canceling_authentication_redirects_to_register_when_auth_entry_register(self): - self.assert_exception_redirect_looks_correct('/register', auth_entry=pipeline.AUTH_ENTRY_REGISTER) + self.assert_exception_redirect_looks_correct('/account/register', auth_entry=pipeline.AUTH_ENTRY_REGISTER) def test_canceling_authentication_redirects_to_login_when_auth_login_2(self): self.assert_exception_redirect_looks_correct('/account/login/', auth_entry=pipeline.AUTH_ENTRY_LOGIN_2) diff --git a/common/test/acceptance/tests/lms/test_lms.py b/common/test/acceptance/tests/lms/test_lms.py index 09853250bf..499d5768c2 100644 --- a/common/test/acceptance/tests/lms/test_lms.py +++ b/common/test/acceptance/tests/lms/test_lms.py @@ -11,8 +11,6 @@ from bok_choy.web_app_test import WebAppTest from ..helpers import UniqueCourseTest, load_data_str from ...pages.lms.auto_auth import AutoAuthPage from ...pages.common.logout import LogoutPage -from ...pages.lms.find_courses import FindCoursesPage -from ...pages.lms.course_about import CourseAboutPage from ...pages.lms.course_info import CourseInfoPage from ...pages.lms.tab_nav import TabNavPage from ...pages.lms.course_nav import CourseNavPage @@ -25,48 +23,6 @@ from ...pages.lms.login_and_register import CombinedLoginAndRegisterPage from ...fixtures.course import CourseFixture, XBlockFixtureDesc, CourseUpdateDesc -class RegistrationTest(UniqueCourseTest): - """ - Test the registration process. - """ - - def setUp(self): - """ - Initialize pages and install a course fixture. - """ - super(RegistrationTest, self).setUp() - - self.find_courses_page = FindCoursesPage(self.browser) - self.course_about_page = CourseAboutPage(self.browser, self.course_id) - - # Create a course to register for - CourseFixture( - self.course_info['org'], self.course_info['number'], - self.course_info['run'], self.course_info['display_name'] - ).install() - - def test_register(self): - - # Visit the main page with the list of courses - self.find_courses_page.visit() - - # Go to the course about page and click the register button - self.course_about_page.visit() - register_page = self.course_about_page.register() - - # Fill in registration info and submit - username = "test_" + self.unique_id[0:6] - register_page.provide_info( - username + "@example.com", "test", username, "Test User" - ) - dashboard = register_page.submit() - - # We should end up at the dashboard - # Check that we're registered for the course - course_names = dashboard.available_courses - self.assertIn(self.course_info['display_name'], course_names) - - @attr('shard_1') class LoginFromCombinedPageTest(UniqueCourseTest): """Test that we can log in using the combined login/registration page. diff --git a/lms/djangoapps/courseware/features/login.feature b/lms/djangoapps/courseware/features/login.feature deleted file mode 100644 index 3576a1adaf..0000000000 --- a/lms/djangoapps/courseware/features/login.feature +++ /dev/null @@ -1,58 +0,0 @@ -@shard_1 -Feature: LMS.Login in as a registered user - As a registered user - In order to access my content - I want to be able to login in to edX - - Scenario: Login to an unactivated account - Given I am an edX user - And I am an unactivated user - And I visit the homepage - When I click the link with the text "Log in" - And I submit my credentials on the login form - Then I should see the login error message "This account has not been activated" - - # firefox will not redirect properly when the whole suite is run - @skip_firefox - Scenario: Login to an activated account - Given I am an edX user - And I am an activated user - And I visit the homepage - When I click the link with the text "Log in" - And I submit my credentials on the login form - Then I should be on the dashboard page - - Scenario: Logout of a signed in account - Given I am logged in - When I click the dropdown arrow - And I click the link with the text "Log Out" - Then I should see a link with the text "Log in" - And I should see that the path is "/" - - Scenario: Login with valid redirect - Given I am an edX user - And The course "6.002x" exists - And I am registered for the course "6.002x" - And I am not logged in - And I visit the url "/courses/{}/courseware" - And I should see that the path is "/accounts/login?next=/courses/{}/courseware" - When I submit my credentials on the login form - And I wait for "2" seconds - Then the page title should contain "6.002x Courseware" - - Scenario: Login with an invalid redirect - Given I am an edX user - And I am not logged in - And I visit the url "/login?next=http://www.google.com/" - When I submit my credentials on the login form - Then I should be on the dashboard page - - Scenario: Login with a redirect with parameters - Given I am an edX user - And I am not logged in - And I visit the url "/debug/show_parameters?foo=hello&bar=world" - And I should see that the path is "/accounts/login?next=/debug/show_parameters%3Ffoo%3Dhello%26bar%3Dworld" - When I submit my credentials on the login form - And I wait for "2" seconds - Then I should see "foo: u'hello'" somewhere on the page - And I should see "bar: u'world'" somewhere on the page diff --git a/lms/djangoapps/courseware/features/login.py b/lms/djangoapps/courseware/features/login.py deleted file mode 100644 index 0881a81eea..0000000000 --- a/lms/djangoapps/courseware/features/login.py +++ /dev/null @@ -1,59 +0,0 @@ -# pylint: disable=missing-docstring -# pylint: disable=redefined-outer-name - -from lettuce import step, world -from django.contrib.auth.models import User - - -@step('I am an unactivated user$') -def i_am_an_unactivated_user(step): - user_is_an_unactivated_user('robot') - - -@step('I am an activated user$') -def i_am_an_activated_user(step): - user_is_an_activated_user('robot') - - -@step('I submit my credentials on the login form') -def i_submit_my_credentials_on_the_login_form(step): - fill_in_the_login_form('email', 'robot@edx.org') - fill_in_the_login_form('password', 'test') - - def submit_login_form(): - login_form = world.browser.find_by_css('form#login-form') - login_form.find_by_name('submit').click() - world.retry_on_exception(submit_login_form) - - -@step(u'I should see the login error message "([^"]*)"$') -def i_should_see_the_login_error_message(step, msg): - login_error_div = world.browser.find_by_css('.submission-error.is-shown') - assert (msg in login_error_div.text) - - -@step(u'click the dropdown arrow$') -def click_the_dropdown(step): - world.css_click('.dropdown') - -#### helper functions - - -def user_is_an_unactivated_user(uname): - u = User.objects.get(username=uname) - u.is_active = False - u.save() - - -def user_is_an_activated_user(uname): - u = User.objects.get(username=uname) - u.is_active = True - u.save() - - -def fill_in_the_login_form(field, value): - def fill_login_form(): - login_form = world.browser.find_by_css('form#login-form') - form_field = login_form.find_by_name(field) - form_field.fill(value) - world.retry_on_exception(fill_login_form) diff --git a/lms/djangoapps/courseware/features/signup.feature b/lms/djangoapps/courseware/features/signup.feature deleted file mode 100644 index 60067d5b20..0000000000 --- a/lms/djangoapps/courseware/features/signup.feature +++ /dev/null @@ -1,19 +0,0 @@ -@shard_2 -Feature: LMS.Sign in - In order to use the edX content - As a new user - I want to signup for a student account - - # firefox will not redirect properly - @skip_firefox - Scenario: Sign up from the homepage - Given I visit the homepage - When I click the link with the text "Register Now" - And I fill in "email" on the registration form with "robot2@edx.org" - And I fill in "password" on the registration form with "test" - And I fill in "username" on the registration form with "robot2" - And I fill in "name" on the registration form with "Robot Two" - And I check the checkbox named "terms_of_service" - And I check the checkbox named "honor_code" - And I submit the registration form - Then I should see "Thanks for Registering!" in the dashboard banner diff --git a/lms/djangoapps/courseware/features/signup.py b/lms/djangoapps/courseware/features/signup.py deleted file mode 100644 index 5f35a40e9a..0000000000 --- a/lms/djangoapps/courseware/features/signup.py +++ /dev/null @@ -1,33 +0,0 @@ -# pylint: disable=missing-docstring -# pylint: disable=redefined-outer-name - -from lettuce import world, step - - -@step('I fill in "([^"]*)" on the registration form with "([^"]*)"$') -def when_i_fill_in_field_on_the_registration_form_with_value(step, field, value): - def fill_in_registration(): - register_form = world.browser.find_by_css('form#register-form') - form_field = register_form.find_by_name(field) - form_field.fill(value) - world.retry_on_exception(fill_in_registration) - - -@step('I submit the registration form$') -def i_press_the_button_on_the_registration_form(step): - def submit_registration(): - register_form = world.browser.find_by_css('form#register-form') - register_form.find_by_name('submit').click() - world.retry_on_exception(submit_registration) - - -@step('I check the checkbox named "([^"]*)"$') -def i_check_checkbox(step, checkbox): - css_selector = 'input[name={}]'.format(checkbox) - world.css_check(css_selector) - - -@step('I should see "([^"]*)" in the dashboard banner$') -def i_should_see_text_in_the_dashboard_banner_section(step, text): - css_selector = "section.dashboard-banner h2" - assert (text in world.css_text(css_selector)) diff --git a/lms/djangoapps/courseware/tests/test_registration_extra_vars.py b/lms/djangoapps/courseware/tests/test_registration_extra_vars.py deleted file mode 100644 index 174ec66440..0000000000 --- a/lms/djangoapps/courseware/tests/test_registration_extra_vars.py +++ /dev/null @@ -1,293 +0,0 @@ -# -*- coding: utf-8 -""" -Tests for extra registration variables -""" -import json -from django.conf import settings -from django.test import TestCase -from django.core.urlresolvers import reverse -from mock import patch -from bs4 import BeautifulSoup -from django.utils import translation - - -class TestSortedCountryList(TestCase): - """ - Test that country list is always sorted alphabetically - """ - def setUp(self): - super(TestSortedCountryList, self).setUp() - self.url = reverse('register_user') - - def find_option_by_code(self, options, code): - for index, option in enumerate(options): - if option.attrs['value'] == code: - return (index, option) - - @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'country': 'required'}) - def test_country_sorting_english(self): - """ - Test that country list is always sorted alphabetically in English - """ - response = self.client.get(self.url) - soup = BeautifulSoup(response.content) - country = soup.find(id="country") - options = country.findAll("option") - (af_index, af_option) = self.find_option_by_code(options, 'AF') - self.assertEqual( - af_option.text, - u'Afghanistan', - ) - (us_index, us_option) = self.find_option_by_code(options, 'US') - self.assertEqual( - us_option.text, - u'United States', - ) - # testing that the Afghan entry is always before the US entry - self.assertLess(af_index, us_index) - # testing two option elements to be in alphabetical order - self.assertLess(options[1].text, options[10].text) - - @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'country': 'required'}) - def test_country_sorting_french(self): - """ - Test that country list is always sorted alphabetically in French - """ - user_language = 'fr' - with translation.override(user_language): - self.client.cookies['django_language'] = user_language - response = self.client.get(self.url) - soup = BeautifulSoup(response.content) - country = soup.find(id="country") - options = country.findAll("option") - (af_index, af_option) = self.find_option_by_code(options, 'AF') - self.assertEqual( - af_option.text, - u'Afghanistan', - ) - (us_index, us_option) = self.find_option_by_code(options, 'US') - self.assertEqual( - us_option.text, - u'États-Unis', - ) - # testing that the Afghan entry is always before the US entry - self.assertLess(af_index, us_index) - # testing two option elements to be in alphabetical order - self.assertLess(options[1].text, options[10].text) - - -class TestExtraRegistrationVariables(TestCase): - """ - Test that extra registration variables are properly checked according to settings - """ - def setUp(self): - super(TestExtraRegistrationVariables, self).setUp() - self.url = reverse('create_account') - - self.url_params = { - 'username': 'username', - 'name': 'name', - 'email': 'foo_bar@bar.com', - 'password': 'password', - 'terms_of_service': 'true', - 'honor_code': 'true', - } - - def test_default_missing_honor(self): - """ - By default, the honor code must be required - """ - self.url_params['honor_code'] = '' - response = self.client.post(self.url, self.url_params) - self.assertEqual(response.status_code, 400) - obj = json.loads(response.content) - self.assertEqual( - obj['value'], - u'To enroll, you must follow the honor code.', - ) - - @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'honor_code': 'optional'}) - def test_optional_honor(self): - """ - With the honor code is made optional, should pass without extra vars - """ - self.url_params['honor_code'] = '' - response = self.client.post(self.url, self.url_params) - self.assertEqual(response.status_code, 200) - obj = json.loads(response.content) - self.assertEqual(obj['success'], True) - - @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, { - 'level_of_education': 'hidden', - 'gender': 'hidden', - 'year_of_birth': 'hidden', - 'mailing_address': 'hidden', - 'goals': 'hidden', - 'honor_code': 'hidden', - 'city': 'hidden', - 'country': 'hidden'}) - def test_all_hidden(self): - """ - When the fields are all hidden, should pass without extra vars - """ - response = self.client.post(self.url, self.url_params) - self.assertEqual(response.status_code, 200) - obj = json.loads(response.content) - self.assertTrue(obj['success']) - - @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'city': 'required'}) - def test_required_city_missing(self): - """ - Should require the city if configured as 'required' but missing - """ - self.url_params['city'] = '' - response = self.client.post(self.url, self.url_params) - self.assertEqual(response.status_code, 400) - obj = json.loads(response.content) - self.assertEqual( - obj['value'], - u'A city is required', - ) - - @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'city': 'required'}) - def test_required_city(self): - """ - Should require the city if configured as 'required' but missing - """ - self.url_params['city'] = 'New York' - response = self.client.post(self.url, self.url_params) - self.assertEqual(response.status_code, 200) - obj = json.loads(response.content) - self.assertTrue(obj['success']) - - @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'country': 'required'}) - def test_required_country_missing(self): - """ - Should require the country if configured as 'required' but missing - """ - self.url_params['country'] = '' - response = self.client.post(self.url, self.url_params) - self.assertEqual(response.status_code, 400) - obj = json.loads(response.content) - self.assertEqual( - obj['value'], - u'A country is required', - ) - - @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'country': 'required'}) - def test_required_country(self): - self.url_params['country'] = 'New York' - response = self.client.post(self.url, self.url_params) - self.assertEqual(response.status_code, 200) - obj = json.loads(response.content) - self.assertTrue(obj['success']) - - @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'level_of_education': 'required'}) - def test_required_level_of_education_missing(self): - """ - Should require the level_of_education if configured as 'required' but missing - """ - self.url_params['level_of_education'] = '' - response = self.client.post(self.url, self.url_params) - self.assertEqual(response.status_code, 400) - obj = json.loads(response.content) - self.assertEqual( - obj['value'], - u'A level of education is required', - ) - - @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'level_of_education': 'required'}) - def test_required_level_of_education(self): - self.url_params['level_of_education'] = 'p' - response = self.client.post(self.url, self.url_params) - self.assertEqual(response.status_code, 200) - obj = json.loads(response.content) - self.assertTrue(obj['success']) - - @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'gender': 'required'}) - def test_required_gender_missing(self): - """ - Should require the gender if configured as 'required' but missing - """ - self.url_params['gender'] = '' - response = self.client.post(self.url, self.url_params) - self.assertEqual(response.status_code, 400) - obj = json.loads(response.content) - self.assertEqual( - obj['value'], - u'Your gender is required', - ) - - @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'gender': 'required'}) - def test_required_gender(self): - self.url_params['gender'] = 'm' - response = self.client.post(self.url, self.url_params) - self.assertEqual(response.status_code, 200) - obj = json.loads(response.content) - self.assertTrue(obj['success']) - - @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'year_of_birth': 'required'}) - def test_required_year_of_birth_missing(self): - """ - Should require the year_of_birth if configured as 'required' but missing - """ - self.url_params['year_of_birth'] = '' - response = self.client.post(self.url, self.url_params) - self.assertEqual(response.status_code, 400) - obj = json.loads(response.content) - self.assertEqual( - obj['value'], - u'Your year of birth is required', - ) - - @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'year_of_birth': 'required'}) - def test_required_year_of_birth(self): - self.url_params['year_of_birth'] = '1982' - response = self.client.post(self.url, self.url_params) - self.assertEqual(response.status_code, 200) - obj = json.loads(response.content) - self.assertTrue(obj['success']) - - @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'mailing_address': 'required'}) - def test_required_mailing_address_missing(self): - """ - Should require the mailing_address if configured as 'required' but missing - """ - self.url_params['mailing_address'] = '' - response = self.client.post(self.url, self.url_params) - self.assertEqual(response.status_code, 400) - obj = json.loads(response.content) - self.assertEqual( - obj['value'], - u'Your mailing address is required', - ) - - @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'mailing_address': 'required'}) - def test_required_mailing_address(self): - self.url_params['mailing_address'] = 'my address' - response = self.client.post(self.url, self.url_params) - self.assertEqual(response.status_code, 200) - obj = json.loads(response.content) - self.assertTrue(obj['success']) - - @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'goals': 'required'}) - def test_required_goals_missing(self): - """ - Should require the goals if configured as 'required' but missing - """ - self.url_params['goals'] = '' - response = self.client.post(self.url, self.url_params) - self.assertEqual(response.status_code, 400) - obj = json.loads(response.content) - self.assertEqual( - obj['value'], - u'A description of your goals is required', - ) - - @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'goals': 'required'}) - def test_required_goals(self): - self.url_params['goals'] = 'my goals' - response = self.client.post(self.url, self.url_params) - self.assertEqual(response.status_code, 200) - obj = json.loads(response.content) - self.assertTrue(obj['success']) diff --git a/lms/djangoapps/instructor/enrollment.py b/lms/djangoapps/instructor/enrollment.py index 1e9735ee11..b16581f6b4 100644 --- a/lms/djangoapps/instructor/enrollment.py +++ b/lms/djangoapps/instructor/enrollment.py @@ -241,7 +241,7 @@ def get_email_params(course, auto_enroll, secure=True): registration_url = u'{proto}://{site}{path}'.format( proto=protocol, site=stripped_site_name, - path=reverse('student.views.register_user') + path=reverse('register_user') ) course_url = u'{proto}://{site}{path}'.format( proto=protocol, diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index 8e8a48513a..600edac6a5 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -1470,7 +1470,7 @@ def _do_enroll_students(course, course_key, students, secure=False, overload=Fal registration_url = '{proto}://{site}{path}'.format( proto=protocol, site=stripped_site_name, - path=reverse('student.views.register_user') + path=reverse('register_user') ) course_url = '{proto}://{site}{path}'.format( proto=protocol, diff --git a/lms/djangoapps/student_account/helpers.py b/lms/djangoapps/student_account/helpers.py index cd2e8d947f..faa5416f72 100644 --- a/lms/djangoapps/student_account/helpers.py +++ b/lms/djangoapps/student_account/helpers.py @@ -1,4 +1,72 @@ """Helper functions for the student account app. """ +from django.core.urlresolvers import reverse +from opaque_keys.edx.keys import CourseKey +from course_modes.models import CourseMode +from third_party_auth import ( # pylint: disable=W0611 + pipeline, provider, + is_enabled as third_party_auth_enabled +) -# TODO: move this function here instead of importing it from student # pylint: disable=fixme -from student.helpers import auth_pipeline_urls # pylint: disable=unused-import + +def auth_pipeline_urls(auth_entry, redirect_url=None, course_id=None): + """Retrieve URLs for each enabled third-party auth provider. + + These URLs are used on the "sign up" and "sign in" buttons + on the login/registration forms to allow users to begin + authentication with a third-party provider. + + Optionally, we can redirect the user to an arbitrary + url after auth completes successfully. We use this + to redirect the user to a page that required login, + or to send users to the payment flow when enrolling + in a course. + + Args: + auth_entry (string): Either `pipeline.AUTH_ENTRY_LOGIN` or `pipeline.AUTH_ENTRY_REGISTER` + + Keyword Args: + redirect_url (unicode): If provided, send users to this URL + after they successfully authenticate. + + course_id (unicode): The ID of the course the user is enrolling in. + We use this to send users to the track selection page + if the course has a payment option. + Note that `redirect_url` takes precedence over the redirect + to the track selection page. + + Returns: + dict mapping provider names to URLs + + """ + if not third_party_auth_enabled(): + return {} + + if redirect_url is not None: + pipeline_redirect = redirect_url + elif course_id is not None: + # If the course is white-label (paid), then we send users + # to the shopping cart. (There is a third party auth pipeline + # step that will add the course to the cart.) + if CourseMode.is_white_label(CourseKey.from_string(course_id)): + pipeline_redirect = reverse("shoppingcart.views.show_cart") + + # Otherwise, send the user to the track selection page. + # The track selection page may redirect the user to the dashboard + # (if the only available mode is honor), or directly to verification + # (for professional ed). + else: + pipeline_redirect = reverse( + "course_modes_choose", + kwargs={'course_id': unicode(course_id)} + ) + else: + pipeline_redirect = None + + return { + provider.NAME: pipeline.get_login_url( + provider.NAME, auth_entry, + enroll_course_id=course_id, + redirect_url=pipeline_redirect + ) + for provider in provider.Registry.enabled() + } diff --git a/lms/djangoapps/student_account/test/test_views.py b/lms/djangoapps/student_account/test/test_views.py index b293292f3f..6fe8456bc4 100644 --- a/lms/djangoapps/student_account/test/test_views.py +++ b/lms/djangoapps/student_account/test/test_views.py @@ -434,14 +434,14 @@ class StudentAccountLoginAndRegistrationTest(ModuleStoreTestCase): { "name": "Facebook", "iconClass": "icon-facebook", - "loginUrl": self._third_party_login_url("facebook", "account_login"), - "registerUrl": self._third_party_login_url("facebook", "account_register") + "loginUrl": self._third_party_login_url("facebook", "login"), + "registerUrl": self._third_party_login_url("facebook", "register") }, { "name": "Google", "iconClass": "icon-google-plus", - "loginUrl": self._third_party_login_url("google-oauth2", "account_login"), - "registerUrl": self._third_party_login_url("google-oauth2", "account_register") + "loginUrl": self._third_party_login_url("google-oauth2", "login"), + "registerUrl": self._third_party_login_url("google-oauth2", "register") } ] self._assert_third_party_auth_data(response, current_provider, expected_providers) @@ -468,12 +468,12 @@ class StudentAccountLoginAndRegistrationTest(ModuleStoreTestCase): "name": "Facebook", "iconClass": "icon-facebook", "loginUrl": self._third_party_login_url( - "facebook", "account_login", + "facebook", "login", course_id=unicode(course.id), redirect_url=course_modes_choose_url ), "registerUrl": self._third_party_login_url( - "facebook", "account_register", + "facebook", "register", course_id=unicode(course.id), redirect_url=course_modes_choose_url ) @@ -482,12 +482,12 @@ class StudentAccountLoginAndRegistrationTest(ModuleStoreTestCase): "name": "Google", "iconClass": "icon-google-plus", "loginUrl": self._third_party_login_url( - "google-oauth2", "account_login", + "google-oauth2", "login", course_id=unicode(course.id), redirect_url=course_modes_choose_url ), "registerUrl": self._third_party_login_url( - "google-oauth2", "account_register", + "google-oauth2", "register", course_id=unicode(course.id), redirect_url=course_modes_choose_url ) @@ -516,12 +516,12 @@ class StudentAccountLoginAndRegistrationTest(ModuleStoreTestCase): "name": "Facebook", "iconClass": "icon-facebook", "loginUrl": self._third_party_login_url( - "facebook", "account_login", + "facebook", "login", course_id=unicode(course.id), redirect_url=shoppingcart_url ), "registerUrl": self._third_party_login_url( - "facebook", "account_register", + "facebook", "register", course_id=unicode(course.id), redirect_url=shoppingcart_url ) @@ -530,12 +530,12 @@ class StudentAccountLoginAndRegistrationTest(ModuleStoreTestCase): "name": "Google", "iconClass": "icon-google-plus", "loginUrl": self._third_party_login_url( - "google-oauth2", "account_login", + "google-oauth2", "login", course_id=unicode(course.id), redirect_url=shoppingcart_url ), "registerUrl": self._third_party_login_url( - "google-oauth2", "account_register", + "google-oauth2", "register", course_id=unicode(course.id), redirect_url=shoppingcart_url ) @@ -546,6 +546,27 @@ class StudentAccountLoginAndRegistrationTest(ModuleStoreTestCase): response = self.client.get(reverse("account_login"), {"course_id": unicode(course.id)}) self._assert_third_party_auth_data(response, None, expected_providers) + @override_settings(SITE_NAME=settings.MICROSITE_TEST_HOSTNAME) + def test_microsite_uses_old_login_page(self): + # Retrieve the login page from a microsite domain + # and verify that we're served the old page. + resp = self.client.get( + reverse("account_login"), + HTTP_HOST=settings.MICROSITE_TEST_HOSTNAME + ) + self.assertContains(resp, "Log into your Test Microsite Account") + self.assertContains(resp, "login-form") + + def test_microsite_uses_old_register_page(self): + # Retrieve the register page from a microsite domain + # and verify that we're served the old page. + resp = self.client.get( + reverse("account_register"), + HTTP_HOST=settings.MICROSITE_TEST_HOSTNAME + ) + self.assertContains(resp, "Register for Test Microsite") + self.assertContains(resp, "register-form") + def _assert_third_party_auth_data(self, response, current_provider, providers): """Verify that third party auth info is rendered correctly in a DOM data attribute. """ expected_data = u"data-third-party-auth='{auth_info}'".format( diff --git a/lms/djangoapps/student_account/views.py b/lms/djangoapps/student_account/views.py index 5239f8f27a..647427a27a 100644 --- a/lms/djangoapps/student_account/views.py +++ b/lms/djangoapps/student_account/views.py @@ -15,6 +15,14 @@ from django.views.decorators.http import require_http_methods from edxmako.shortcuts import render_to_response, render_to_string from microsite_configuration import microsite import third_party_auth +from external_auth.login_and_register import ( + login as external_auth_login, + register as external_auth_register +) +from student.views import ( + signin_user as old_login_view, + register_user as old_register_view +) from user_api.api import account as account_api from user_api.api import profile as profile_api @@ -60,13 +68,26 @@ def login_and_registration_form(request, initial_mode="login"): the user_api. Keyword Args: - initial_mode (string): Either "login" or "registration". + initial_mode (string): Either "login" or "register". """ # If we're already logged in, redirect to the dashboard if request.user.is_authenticated(): return redirect(reverse('dashboard')) + # If this is a microsite, revert to the old login/registration pages. + # We need to do this for now to support existing themes. + if microsite.is_request_in_microsite(): + if initial_mode == "login": + return old_login_view(request) + elif initial_mode == "register": + return old_register_view(request) + + # Allow external auth to intercept and handle the request + ext_auth_response = _external_auth_intercept(request, initial_mode) + if ext_auth_response is not None: + return ext_auth_response + # Otherwise, render the combined login/registration page context = { 'disable_courseware_js': True, @@ -285,12 +306,14 @@ def _third_party_auth_context(request): } course_id = request.GET.get("course_id") + redirect_to = request.GET.get("next") login_urls = auth_pipeline_urls( - third_party_auth.pipeline.AUTH_ENTRY_LOGIN_2, - course_id=course_id + third_party_auth.pipeline.AUTH_ENTRY_LOGIN, + course_id=course_id, + redirect_url=redirect_to ) register_urls = auth_pipeline_urls( - third_party_auth.pipeline.AUTH_ENTRY_REGISTER_2, + third_party_auth.pipeline.AUTH_ENTRY_REGISTER, course_id=course_id ) @@ -313,3 +336,20 @@ def _third_party_auth_context(request): context["currentProvider"] = current_provider.NAME return context + + +def _external_auth_intercept(request, mode): + """Allow external auth to intercept a login/registration request. + + Arguments: + request (Request): The original request. + mode (str): Either "login" or "register" + + Returns: + Response or None + + """ + if mode == "login": + return external_auth_login(request) + elif mode == "register": + return external_auth_register(request) diff --git a/lms/templates/courseware/mktg_course_about.html b/lms/templates/courseware/mktg_course_about.html index e1f71a2af2..3186dd8e81 100644 --- a/lms/templates/courseware/mktg_course_about.html +++ b/lms/templates/courseware/mktg_course_about.html @@ -41,7 +41,11 @@ } else if (xhr.status == 403) { var email_opt_in = $("input[name='email_opt_in']").val(); ## Ugh. + % if settings.FEATURES.get("ENABLE_COMBINED_LOGIN_REGISTRATION"): + window.top.location.href = $("a.register").attr("href") || "${reverse('accounts_login')}?course_id=${course.id | u}&enrollment_action=enroll&email_opt_in=" + email_opt_in; + % else: window.top.location.href = $("a.register").attr("href") || "${reverse('register_user')}?course_id=${course.id | u}&enrollment_action=enroll&email_opt_in=" + email_opt_in; + % endif } else { $('#register_error').html( (xhr.responseText ? xhr.responseText : "${_("An error occurred. Please try again later.")}") @@ -66,7 +70,11 @@ %elif allow_registration: ${_("Enroll in")} ${course.display_number_with_default | h} %if len(course_modes) > 1: diff --git a/lms/urls.py b/lms/urls.py index 69bc7beaaf..4730514479 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -16,8 +16,8 @@ urlpatterns = ('', # nopep8 url(r'^request_certificate$', 'certificates.views.request_certificate'), url(r'^$', 'branding.views.index', name="root"), # Main marketing page, or redirect to courseware url(r'^dashboard$', 'student.views.dashboard', name="dashboard"), - url(r'^login$', 'student.views.signin_user', name="signin_user"), - url(r'^register$', 'student.views.register_user', name="register_user"), + url(r'^login_ajax$', 'student.views.login_user', name="login"), + url(r'^login_ajax/(?P[^/]*)$', 'student.views.login_user'), url(r'^admin_dashboard$', 'dashboard.views.dashboard'), @@ -31,14 +31,11 @@ urlpatterns = ('', # nopep8 url(r'^segmentio/event$', 'track.views.segmentio.segmentio_event'), url(r'^t/(?P