Revert "If feature flag is enabled, replace the old login/registration pages…

Revert "If feature flag is enabled, replace the old login/registration pages with the new combined login/registration page."

This reverts commit f40447b3.

Conflicts:
	lms/urls.py

Revert "Add support for external auth on the combined login/registration page"

This reverts commit 98875339.

Conflicts:
	lms/templates/courseware/mktg_course_about.html
	lms/urls.py

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,7 +220,8 @@ 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-and-registration-container' in response.content)
+        self.assertTrue('login_form' in response.content
+                        or 'login-form' in response.content)
 
         # And get directly logged in otherwise
         response = self.client.get(

common/djangoapps/student/helpers.py

@@ -4,10 +4,78 @@ from datetime import datetime
 from pytz import UTC
 from django.utils.http import cookie_date
 from django.conf import settings
-
-from verify_student.models import SoftwareSecurePhotoVerification  # pylint: disable=F0401
+from django.core.urlresolvers import reverse
+from opaque_keys.edx.keys import CourseKey
 from course_modes.models import CourseMode
-from student_account.helpers import auth_pipeline_urls  # pylint: disable=unused-import,import-error
+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()
+    }
 
 
 def set_logged_in_cookie(request, response):

common/djangoapps/student/tests/test_login.py

@@ -14,13 +14,18 @@ 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 login_oauth_token
-
+from student.views import (
+    _parse_course_id_from_string,
+    _get_course_enrollment_domain,
+    login_oauth_token,
+)
 from xmodule.modulestore.tests.factories import CourseFactory
-from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, TEST_DATA_MOCK_MODULESTORE
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 
 
 class LoginTest(TestCase):
@@ -319,6 +324,24 @@ 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):
     """
@@ -365,6 +388,15 @@ 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,
@@ -384,7 +416,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 in or Register | {platform_name}"
+        self.assertContains(noshib_response, ("Log into your {platform_name} Account | {platform_name}"
                                               .format(platform_name=settings.PLATFORM_NAME)))
         self.assertEqual(noshib_response.status_code, 200)
 

common/djangoapps/student/views.py

@@ -75,10 +75,6 @@ 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
@@ -352,14 +348,18 @@ 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`.
     """
-    external_auth_response = external_auth_login(request)
-    if external_auth_response is not None:
-        return external_auth_response
+    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'))
     if request.user.is_authenticated():
         return redirect(reverse('dashboard'))
 
@@ -383,17 +383,15 @@ def signin_user(request):
 
 @ensure_csrf_cookie
 def register_user(request, extra_context=None):
-    """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`.
+    """
+    This view will display the non-modal registration form
     """
     if request.user.is_authenticated():
         return redirect(reverse('dashboard'))
-
-    external_auth_response = external_auth_register(request)
-    if external_auth_response is not None:
-        return external_auth_response
+    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)
 
     course_id = request.GET.get('course_id')
 
@@ -901,21 +899,56 @@ def change_enrollment(request, check_access=True):
         return HttpResponseBadRequest(_("Enrollment action is invalid"))
 
 
+# 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
+
+
 @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`.
     """
-    external_auth_response = external_auth_login(request)
-    if external_auth_response is not None:
-        return external_auth_response
-
+    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())
+
     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'
 
-# 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.
+# 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.
 AUTH_ENTRY_LOGIN_2 = 'account_login'
 AUTH_ENTRY_REGISTER_2 = 'account_register'
 
@@ -129,13 +129,12 @@ AUTH_ENTRY_API = 'api'
 # to load that depend on this module.
 AUTH_DISPATCH_URLS = {
     AUTH_ENTRY_DASHBOARD: '/dashboard',
-    AUTH_ENTRY_LOGIN: '/account/login',
-    AUTH_ENTRY_REGISTER: '/account/register',
+    AUTH_ENTRY_LOGIN: '/login',
+    AUTH_ENTRY_REGISTER: '/register',
 
-    # 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.
+    # TODO (ECOM-369): Replace the dispatch URLs
+    # for `AUTH_ENTRY_LOGIN` and `AUTH_ENTRY_REGISTER`
+    # with these values, but DO NOT DELETE THESE KEYS.
     AUTH_ENTRY_LOGIN_2: '/account/login/',
     AUTH_ENTRY_REGISTER_2: '/account/register/',
 
@@ -151,10 +150,11 @@ _AUTH_ENTRY_CHOICES = frozenset([
     AUTH_ENTRY_PROFILE,
     AUTH_ENTRY_REGISTER,
 
-    # 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.
+    # 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
     AUTH_ENTRY_LOGIN_2,
     AUTH_ENTRY_REGISTER_2,
 
@@ -437,16 +437,31 @@ 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 in [AUTH_ENTRY_LOGIN, AUTH_ENTRY_LOGIN_2],
+        'is_login': auth_entry == AUTH_ENTRY_LOGIN,
         # Whether the auth pipeline entered from /register.
-        'is_register': auth_entry in [AUTH_ENTRY_REGISTER, AUTH_ENTRY_REGISTER_2],
+        'is_register': auth_entry == AUTH_ENTRY_REGISTER,
         # 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,
@@ -482,23 +497,36 @@ 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 or is_login_2) and (user_unset or user_inactive)
-    dispatch_to_register = (is_register or is_register_2) and user_unset
+    dispatch_to_login = is_login and (user_unset or user_inactive)
     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')
 
-    if dispatch_to_register:
+    # 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:
         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,6 +198,13 @@ 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.
@@ -227,14 +234,15 @@ 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)
-        # 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)
+        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)
 
     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.
@@ -254,22 +262,27 @@ 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 /account/login."""
+        """Asserts a response would redirect to /login."""
         self.assertEqual(302, response.status_code)
-        self.assertEqual('/account/login', response.get('Location'))
+        self.assertEqual('/' + pipeline.AUTH_ENTRY_LOGIN, response.get('Location'))
 
     def assert_redirect_to_register_looks_correct(self, response):
-        """Asserts a response would redirect to /account/register."""
+        """Asserts a response would redirect to /register."""
         self.assertEqual(302, response.status_code)
-        self.assertEqual('/account/register', response.get('Location'))
+        self.assertEqual('/' + pipeline.AUTH_ENTRY_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)
-        # 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)
+        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])
 
     def assert_social_auth_does_not_exist_for_user(self, user, strategy):
         """Asserts a user does not have an auth with the expected provider."""
@@ -391,10 +404,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('/account/login', auth_entry=pipeline.AUTH_ENTRY_LOGIN)
+        self.assert_exception_redirect_looks_correct('/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('/account/register', auth_entry=pipeline.AUTH_ENTRY_REGISTER)
+        self.assert_exception_redirect_looks_correct('/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,6 +11,8 @@ 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
@@ -23,6 +25,48 @@ 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('register_user')
+        path=reverse('student.views.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('register_user')
+            path=reverse('student.views.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
-)
 
-
-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()
-    }
+# 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

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", "login"),
-                "registerUrl": self._third_party_login_url("facebook", "register")
+                "loginUrl": self._third_party_login_url("facebook", "account_login"),
+                "registerUrl": self._third_party_login_url("facebook", "account_register")
             },
             {
                 "name": "Google",
                 "iconClass": "icon-google-plus",
-                "loginUrl": self._third_party_login_url("google-oauth2", "login"),
-                "registerUrl": self._third_party_login_url("google-oauth2", "register")
+                "loginUrl": self._third_party_login_url("google-oauth2", "account_login"),
+                "registerUrl": self._third_party_login_url("google-oauth2", "account_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", "login",
+                    "facebook", "account_login",
                     course_id=unicode(course.id),
                     redirect_url=course_modes_choose_url
                 ),
                 "registerUrl": self._third_party_login_url(
-                    "facebook", "register",
+                    "facebook", "account_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", "login",
+                    "google-oauth2", "account_login",
                     course_id=unicode(course.id),
                     redirect_url=course_modes_choose_url
                 ),
                 "registerUrl": self._third_party_login_url(
-                    "google-oauth2", "register",
+                    "google-oauth2", "account_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", "login",
+                    "facebook", "account_login",
                     course_id=unicode(course.id),
                     redirect_url=shoppingcart_url
                 ),
                 "registerUrl": self._third_party_login_url(
-                    "facebook", "register",
+                    "facebook", "account_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", "login",
+                    "google-oauth2", "account_login",
                     course_id=unicode(course.id),
                     redirect_url=shoppingcart_url
                 ),
                 "registerUrl": self._third_party_login_url(
-                    "google-oauth2", "register",
+                    "google-oauth2", "account_register",
                     course_id=unicode(course.id),
                     redirect_url=shoppingcart_url
                 )
@@ -546,27 +546,6 @@ 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,14 +15,6 @@ 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
@@ -68,26 +60,13 @@ def login_and_registration_form(request, initial_mode="login"):
     the user_api.
 
     Keyword Args:
-        initial_mode (string): Either "login" or "register".
+        initial_mode (string): Either "login" or "registration".
 
     """
     # 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,
@@ -306,14 +285,12 @@ 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,
-        course_id=course_id,
-        redirect_url=redirect_to
+        third_party_auth.pipeline.AUTH_ENTRY_LOGIN_2,
+        course_id=course_id
     )
     register_urls = auth_pipeline_urls(
-        third_party_auth.pipeline.AUTH_ENTRY_REGISTER,
+        third_party_auth.pipeline.AUTH_ENTRY_REGISTER_2,
         course_id=course_id
     )
 
@@ -336,20 +313,3 @@ 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

@@ -57,9 +57,8 @@
           emailOptIn = '&email_opt_in=' + $("input[name='email_opt_in']").val();
         }
 
-        <% href_url = 'accounts_login' if settings.FEATURES.get("ENABLE_COMBINED_LOGIN_REGISTRATION") else 'register_user' %>
         ## Ugh.
-        window.top.location.href = $("a.register").attr("href") || "${reverse(href_url)}?course_id=${course.id | u}&enrollment_action=enroll" + emailOptIn;
+        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;
       } else {
         $('#register_error').html(
             (xhr.responseText ? xhr.responseText : "${_("An error occurred. Please try again later.")}")
@@ -84,8 +83,7 @@
         %elif allow_registration:
         <a class="action action-register register ${'has-option-verified' if len(course_modes) > 1 else ''}"
             %if not user.is_authenticated():
-              <% href_url = 'accounts_login' if settings.FEATURES.get("ENABLE_COMBINED_LOGIN_REGISTRATION") else 'register_user' %>
-              href="${reverse(href_url)}?course_id=${course.id | u}&enrollment_action=enroll"
+              href="${reverse('register_user')}?course_id=${course.id | u}&enrollment_action=enroll"
             %endif
         >${_("Enroll in")} <strong>${course.display_number_with_default | h}</strong>
             %if len(course_modes) > 1:

lms/urls.py

@@ -18,6 +18,8 @@ urlpatterns = ('',  # nopep8
     url(r'^dashboard$', 'student.views.dashboard', name="dashboard"),
     url(r'^login_ajax$', 'student.views.login_user', name="login"),
     url(r'^login_ajax/(?P<error>[^/]*)$', 'student.views.login_user'),
+    url(r'^login$', 'student.views.signin_user', name="signin_user"),
+    url(r'^register$', 'student.views.register_user', name="register_user"),
 
     url(r'^admin_dashboard$', 'dashboard.views.dashboard'),
 
@@ -31,6 +33,7 @@ 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',
@@ -75,21 +78,6 @@ 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')),