Merge pull request #6077 from edx/will/enable-logistration

Login/Registration A/B test cleanup

common/djangoapps/external_auth/login_and_register.py

+"""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

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(

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 verify_student.models import SoftwareSecurePhotoVerification  # pylint: disable=F0401
+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):

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):
     """
@@ -388,15 +365,6 @@ class ExternalAuthShibTest(ModuleStoreTestCase):
         })
 
     @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):
         """
         Tests redirects to when @login_required to dashboard, which should always be the normal login,
@@ -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)
 

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.
+
+    DEPRECATION WARNING: This view will eventually be deprecated and replaced
+    with the combined login/registration page in `student_account.views`.
     """
-    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())
+    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),

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):

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)

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.

lms/djangoapps/courseware/features/login.feature

-@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

lms/djangoapps/courseware/features/login.py

-# 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)

lms/djangoapps/courseware/features/signup.feature

-@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

lms/djangoapps/courseware/features/signup.py

-# 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))

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,

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,

lms/djangoapps/student_account/helpers.py

 """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()
+    }

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(

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)

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:
         <a class="action action-register register ${'has-option-verified' if len(course_modes) > 1 else ''}"
             %if not user.is_authenticated():
+                % if settings.FEATURES.get("ENABLE_COMBINED_LOGIN_REGISTRATION"):
+                href="${reverse('accounts_login')}?course_id=${course.id | u}&enrollment_action=enroll"
+                % else:
                 href="${reverse('register_user')}?course_id=${course.id | u}&enrollment_action=enroll"
+                % endif
             %endif
         >${_("Enroll in")} <strong>${course.display_number_with_default | h}</strong>
             %if len(course_modes) > 1:

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<error>[^/]*)$', '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<template>[^/]*)$', 'static_template_view.views.index'),   # TODO: Is this used anymore? What is STATIC_GRAB?
 
-    url(r'^accounts/login$', 'student.views.accounts_login', name="accounts_login"),
     url(r'^accounts/manage_user_standing', 'student.views.manage_user_standing',
         name='manage_user_standing'),
     url(r'^accounts/disable_account_ajax$', 'student.views.disable_account_ajax',
         name="disable_account_ajax"),
 
-    url(r'^login_ajax$', 'student.views.login_user', name="login"),
-    url(r'^login_ajax/(?P<error>[^/]*)$', 'student.views.login_user'),
     url(r'^logout$', 'student.views.logout_user', name='logout'),
     url(r'^create_account$', 'student.views.create_account', name='create_account'),
     url(r'^activate/(?P<key>[^/]*)$', 'student.views.activate_account', name="activate"),
@@ -78,6 +75,21 @@ urlpatterns = ('',  # nopep8
 
 )
 
+if settings.FEATURES["ENABLE_COMBINED_LOGIN_REGISTRATION"]:
+    # Backwards compatibility with old URL structure, but serve the new views
+    urlpatterns += (
+        url(r'^login$', 'student_account.views.login_and_registration_form', {'initial_mode': 'login'}, name="signin_user"),
+        url(r'^register$', 'student_account.views.login_and_registration_form', {'initial_mode': 'register'}, name="register_user"),
+        url(r'^accounts/login$', 'student_account.views.login_and_registration_form', {'initial_mode': 'login'}, name="accounts_login"),
+    )
+else:
+    # Serve the old views
+    urlpatterns += (
+        url(r'^login$', 'student.views.signin_user', name="signin_user"),
+        url(r'^register$', 'student.views.register_user', name="register_user"),
+        url(r'^accounts/login$', 'student.views.accounts_login', name="accounts_login"),
+    )
+
 if settings.FEATURES["ENABLE_MOBILE_REST_API"]:
     urlpatterns += (
         url(r'^api/mobile/v0.5/', include('mobile_api.urls')),