Integrate third party auth into the combined login/registration page.
Change third party auth login failure code to a 401, to detect authentication success with no linked account. If already authenticated, redirect immediately to the dashboard. Use "Location" header correctly for 302 redirects from student views. Add utility functions for simulating a running third-party auth pipeline. Add a utility function for checking whether third party auth is enabled. Respect default values sent by the server
This commit is contained in:
@@ -0,0 +1,15 @@
|
||||
"""Third party authentication. """
|
||||
|
||||
from microsite_configuration import microsite
|
||||
|
||||
|
||||
def is_enabled():
|
||||
"""Check whether third party authentication has been enabled. """
|
||||
|
||||
# We do this imports internally to avoid initializing settings prematurely
|
||||
from django.conf import settings
|
||||
|
||||
return microsite.get_value(
|
||||
"ENABLE_THIRD_PARTY_AUTH",
|
||||
settings.FEATURES.get("ENABLE_THIRD_PARTY_AUTH")
|
||||
)
|
||||
|
||||
@@ -82,12 +82,27 @@ AUTH_ENTRY_DASHBOARD = 'dashboard'
|
||||
AUTH_ENTRY_LOGIN = 'login'
|
||||
AUTH_ENTRY_PROFILE = 'profile'
|
||||
AUTH_ENTRY_REGISTER = 'register'
|
||||
|
||||
# TODO (ECOM-369): Repace `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'
|
||||
|
||||
_AUTH_ENTRY_CHOICES = frozenset([
|
||||
AUTH_ENTRY_DASHBOARD,
|
||||
AUTH_ENTRY_LOGIN,
|
||||
AUTH_ENTRY_PROFILE,
|
||||
AUTH_ENTRY_REGISTER
|
||||
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.
|
||||
AUTH_ENTRY_LOGIN_2,
|
||||
AUTH_ENTRY_REGISTER_2,
|
||||
])
|
||||
|
||||
_DEFAULT_RANDOM_PASSWORD_LENGTH = 12
|
||||
_PASSWORD_CHARSET = string.letters + string.digits
|
||||
|
||||
@@ -346,11 +361,28 @@ def parse_query_params(strategy, response, *args, **kwargs):
|
||||
'is_register': auth_entry == AUTH_ENTRY_REGISTER,
|
||||
# Whether the auth pipeline entered from /profile.
|
||||
'is_profile': auth_entry == AUTH_ENTRY_PROFILE,
|
||||
|
||||
# TODO (ECOM-369): Delete these once the A/B test
|
||||
# for the combined login/registration form completes.
|
||||
'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).
|
||||
@partial.partial
|
||||
def redirect_to_supplementary_form(strategy, details, response, uid, is_dashboard=None, is_login=None, is_profile=None, is_register=None, user=None, *args, **kwargs):
|
||||
def redirect_to_supplementary_form(
|
||||
strategy, details, response, uid,
|
||||
is_dashboard=None, is_login=None, is_profile=None, is_register=None,
|
||||
is_login_2=None, is_register_2=None,
|
||||
user=None, *args, **kwargs
|
||||
):
|
||||
"""Dispatches user to views outside the pipeline if necessary."""
|
||||
|
||||
# We're deliberately verbose here to make it clear what the intended
|
||||
@@ -364,20 +396,33 @@ def redirect_to_supplementary_form(strategy, details, response, uid, is_dashboar
|
||||
# It is important that we always execute the entire pipeline. Even if
|
||||
# behavior appears correct without executing a step, it means important
|
||||
# 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)
|
||||
|
||||
# TODO (ECOM-369): Consolidate this with `dispatch_to_login`
|
||||
# once the A/B test completes.
|
||||
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('/login', name='signin_user')
|
||||
|
||||
# TODO (ECOM-369): Consolidate this with `dispatch_to_login`
|
||||
# once the A/B test completes.
|
||||
if dispatch_to_login_2:
|
||||
return redirect(reverse(AUTH_ENTRY_LOGIN_2))
|
||||
|
||||
if is_register and user_unset:
|
||||
return redirect('/register', name='register_user')
|
||||
|
||||
# TODO (ECOM-369): Consolidate this with `is_register`
|
||||
# once the A/B test completes.
|
||||
if is_register_2 and user_unset:
|
||||
return redirect(reverse(AUTH_ENTRY_REGISTER_2))
|
||||
|
||||
@partial.partial
|
||||
def login_analytics(*args, **kwargs):
|
||||
""" Sends login info to Segment.io """
|
||||
@@ -387,6 +432,12 @@ def login_analytics(*args, **kwargs):
|
||||
'is_login': 'edx.bi.user.account.authenticated',
|
||||
'is_dashboard': 'edx.bi.user.account.linked',
|
||||
'is_profile': 'edx.bi.user.account.linked',
|
||||
|
||||
# Backwards compatibility: during an A/B test for the combined
|
||||
# login/registration form, we introduced a new login end-point.
|
||||
# Since users may continue to have this in their sessions after
|
||||
# the test concludes, we need to continue accepting this action.
|
||||
'is_login_2': 'edx.bi.user.account.authenticated',
|
||||
}
|
||||
|
||||
# Note: we assume only one of the `action` kwargs (is_dashboard, is_login) to be
|
||||
@@ -408,7 +459,7 @@ def login_analytics(*args, **kwargs):
|
||||
},
|
||||
context={
|
||||
'Google Analytics': {
|
||||
'clientId': tracking_context.get('client_id')
|
||||
'clientId': tracking_context.get('client_id')
|
||||
}
|
||||
}
|
||||
)
|
||||
|
||||
@@ -4,7 +4,9 @@ Utilities for writing third_party_auth tests.
|
||||
Used by Django and non-Django tests; must not have Django deps.
|
||||
"""
|
||||
|
||||
from contextlib import contextmanager
|
||||
import unittest
|
||||
import mock
|
||||
|
||||
from third_party_auth import provider
|
||||
|
||||
@@ -37,3 +39,81 @@ class TestCase(unittest.TestCase):
|
||||
provider.Registry._reset()
|
||||
provider.Registry.configure_once(self._original_providers)
|
||||
super(TestCase, self).tearDown()
|
||||
|
||||
|
||||
@contextmanager
|
||||
def simulate_running_pipeline(pipeline_target, backend, email=None, fullname=None, username=None):
|
||||
"""Simulate that a pipeline is currently running.
|
||||
|
||||
You can use this context manager to test packages that rely on third party auth.
|
||||
|
||||
This uses `mock.patch` to override some calls in `third_party_auth.pipeline`,
|
||||
so you will need to provide the "target" module *as it is imported*
|
||||
in the software under test. For example, if `foo/bar.py` does this:
|
||||
|
||||
>>> from third_party_auth import pipeline
|
||||
|
||||
then you will need to do something like this:
|
||||
|
||||
>>> with simulate_running_pipeline("foo.bar.pipeline", "google-oauth2"):
|
||||
>>> bar.do_something_with_the_pipeline()
|
||||
|
||||
If, on the other hand, `foo/bar.py` had done this:
|
||||
|
||||
>>> import third_party_auth
|
||||
|
||||
then you would use the target "foo.bar.third_party_auth.pipeline" instead.
|
||||
|
||||
Arguments:
|
||||
|
||||
pipeline_target (string): The path to `third_party_auth.pipeline` as it is imported
|
||||
in the software under test.
|
||||
|
||||
backend (string): The name of the backend currently running, for example "google-oauth2".
|
||||
Note that this is NOT the same as the name of the *provider*. See the Python
|
||||
social auth documentation for the names of the backends.
|
||||
|
||||
Keyword Arguments:
|
||||
email (string): If provided, simulate that the current provider has
|
||||
included the user's email address (useful for filling in the registration form).
|
||||
|
||||
fullname (string): If provided, simulate that the current provider has
|
||||
included the user's full name (useful for filling in the registration form).
|
||||
|
||||
username (string): If provided, simulate that the pipeline has provided
|
||||
this suggested username. This is something that the `third_party_auth`
|
||||
app generates itself and should be available by the time the user
|
||||
is authenticating with a third-party provider.
|
||||
|
||||
Returns:
|
||||
None
|
||||
|
||||
"""
|
||||
pipeline_data = {
|
||||
"backend": backend,
|
||||
"kwargs": {
|
||||
"details": {}
|
||||
}
|
||||
}
|
||||
if email is not None:
|
||||
pipeline_data["kwargs"]["details"]["email"] = email
|
||||
if fullname is not None:
|
||||
pipeline_data["kwargs"]["details"]["fullname"] = fullname
|
||||
if username is not None:
|
||||
pipeline_data["kwargs"]["username"] = username
|
||||
|
||||
pipeline_get = mock.patch("{pipeline}.get".format(pipeline=pipeline_target), spec=True)
|
||||
pipeline_running = mock.patch("{pipeline}.running".format(pipeline=pipeline_target), spec=True)
|
||||
|
||||
mock_get = pipeline_get.start()
|
||||
mock_running = pipeline_running.start()
|
||||
|
||||
mock_get.return_value = pipeline_data
|
||||
mock_running.return_value = True
|
||||
|
||||
try:
|
||||
yield
|
||||
|
||||
finally:
|
||||
pipeline_get.stop()
|
||||
pipeline_running.stop()
|
||||
|
||||
Reference in New Issue
Block a user