Make logistration generally available if feature flag is active
Makes logistration available at /login and /register as well as /accounts/login/ and /accounts/register/. In addition: - Adds support for redirect URLs in third party auth for combined login/registration page - Adds support for external auth on the combined login/registration page - Removes old login and registration acceptance tests - Adds deprecation warnings to old login and register views - Moves third party auth util to student_account - Adds exception for microsites (theming)
This commit is contained in:
@@ -113,10 +113,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'
|
||||
|
||||
@@ -134,9 +134,10 @@ AUTH_DISPATCH_URLS = {
|
||||
AUTH_ENTRY_LOGIN: '/login',
|
||||
AUTH_ENTRY_REGISTER: '/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/',
|
||||
|
||||
@@ -152,11 +153,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,
|
||||
|
||||
@@ -447,31 +447,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,
|
||||
@@ -507,44 +492,31 @@ 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))
|
||||
or
|
||||
((is_register or is_register_2) and 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 the user has a linked account, but has not yet activated
|
||||
# we should send them to the login page. The login page
|
||||
# will tell them that they need to activate their account.
|
||||
if dispatch_to_login:
|
||||
return redirect(_create_redirect_url(AUTH_DISPATCH_URLS[AUTH_ENTRY_LOGIN], strategy))
|
||||
|
||||
# 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(_create_redirect_url(AUTH_DISPATCH_URLS[AUTH_ENTRY_LOGIN_2], strategy))
|
||||
|
||||
if is_register and user_unset:
|
||||
if dispatch_to_register:
|
||||
return redirect(_create_redirect_url(AUTH_DISPATCH_URLS[AUTH_ENTRY_REGISTER], strategy))
|
||||
|
||||
# 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(_create_redirect_url(AUTH_DISPATCH_URLS[AUTH_ENTRY_REGISTER_2], strategy))
|
||||
|
||||
# If the user has a linked account, but has not yet activated
|
||||
# we should send them to the login page. The login page
|
||||
# will tell them that they need to activate their account.
|
||||
if is_register and user_inactive:
|
||||
return redirect(_create_redirect_url(AUTH_DISPATCH_URLS[AUTH_ENTRY_LOGIN], strategy))
|
||||
|
||||
if is_register_2 and user_inactive:
|
||||
return redirect(_create_redirect_url(AUTH_DISPATCH_URLS[AUTH_ENTRY_LOGIN_2], strategy))
|
||||
|
||||
|
||||
def _create_redirect_url(url, strategy):
|
||||
""" Given a URL and a Strategy, construct the appropriate redirect URL.
|
||||
|
||||
@@ -202,13 +202,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.
|
||||
@@ -238,15 +231,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.
|
||||
@@ -268,25 +260,20 @@ class IntegrationTest(testutil.TestCase, test.TestCase):
|
||||
def assert_redirect_to_login_looks_correct(self, response):
|
||||
"""Asserts a response would redirect to /login."""
|
||||
self.assertEqual(302, response.status_code)
|
||||
self.assertEqual('/' + pipeline.AUTH_ENTRY_LOGIN, response.get('Location'))
|
||||
self.assertEqual('/login', response.get('Location'))
|
||||
|
||||
def assert_redirect_to_register_looks_correct(self, response):
|
||||
"""Asserts a response would redirect to /register."""
|
||||
self.assertEqual(302, response.status_code)
|
||||
self.assertEqual('/' + pipeline.AUTH_ENTRY_REGISTER, response.get('Location'))
|
||||
self.assertEqual('/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."""
|
||||
|
||||
Reference in New Issue
Block a user