feat: account activation now supports a next query param. ENT-4433
This change causes the activation link that’s emailed to a newly-registered user to utilize a next query parameter. The impetus for this change is an edX Enterprise use-case: we'd like newly registered Enterprise Customer admins and learners to be directed to the Enterprise Learner Portal (or Admin Portal) upon account activation. This is likely a broad enough use case to be valuable in other endeavors.
This commit is contained in:
committed by
Alex Dusenbery
parent
63a9327a9d
commit
4b247013ff
@@ -366,7 +366,7 @@ def _get_redirect_to(request_host, request_headers, request_params, request_is_h
|
||||
redirect_to = None
|
||||
elif mime_type:
|
||||
log.warning(
|
||||
"Redirect to url path with specified filed type '%(mime_type)s' not allowed: '%(redirect_to)s'",
|
||||
"Redirect to url path with specified file type '%(mime_type)s' not allowed: '%(redirect_to)s'",
|
||||
{"redirect_to": redirect_to, "mime_type": mime_type}
|
||||
)
|
||||
redirect_to = None
|
||||
|
||||
@@ -10,6 +10,7 @@ from django.conf import settings
|
||||
from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user
|
||||
from django.test import TestCase, override_settings
|
||||
from django.urls import reverse
|
||||
from django.utils.http import urlencode
|
||||
from edx_toggles.toggles.testutils import override_waffle_flag
|
||||
|
||||
from common.djangoapps.student.models import Registration
|
||||
@@ -163,6 +164,60 @@ class TestActivateAccount(TestCase):
|
||||
self.assertRedirects(response, login_page_url)
|
||||
self.assertContains(response, 'Your account could not be activated')
|
||||
|
||||
@override_settings(LOGIN_REDIRECT_WHITELIST=['localhost:1991'])
|
||||
@override_settings(FEATURES=FEATURES_WITH_AUTHN_MFE_ENABLED)
|
||||
@override_waffle_flag(REDIRECT_TO_AUTHN_MICROFRONTEND, active=True)
|
||||
def test_account_activation_with_valid_next_url(self):
|
||||
"""
|
||||
Verify that an activation link with a valid next URL will redirect
|
||||
the activated user to that next URL, even if the AuthN MFE is active
|
||||
and redirects to it are enabled.
|
||||
"""
|
||||
self._assert_user_active_state(expected_active_state=False)
|
||||
|
||||
redirect_url = 'http://localhost:1991/pied-piper/learn'
|
||||
base_activation_url = reverse('activate', args=[self.registration.activation_key])
|
||||
activation_url = '{base}?{params}'.format(
|
||||
base=base_activation_url,
|
||||
params=urlencode({'next': redirect_url}),
|
||||
)
|
||||
|
||||
# HTTP_ACCEPT is needed so the safe redirect checks pass.
|
||||
response = self.client.get(activation_url, follow=True, HTTP_ACCEPT='*/*')
|
||||
|
||||
# There's not actually a server running at localhost:1991 for testing,
|
||||
# so we should expect to land on `redirect_url` but with a status code of 404.
|
||||
self.assertRedirects(response, redirect_url, target_status_code=404)
|
||||
self._assert_user_active_state(expected_active_state=True)
|
||||
|
||||
@override_settings(LOGIN_REDIRECT_WHITELIST=['localhost:9876'])
|
||||
@override_settings(FEATURES=FEATURES_WITH_AUTHN_MFE_ENABLED)
|
||||
@override_waffle_flag(REDIRECT_TO_AUTHN_MICROFRONTEND, active=False)
|
||||
def test_account_activation_invalid_next_url_redirects_dashboard(self):
|
||||
"""
|
||||
Verify that an activation link with an invalid next URL (i.e. it's for a domain
|
||||
not in the allowed list of redirect destinations) will redirect
|
||||
the activated, but unauthenticated, user to a login URL
|
||||
that points to 'dashboard' as the next URL.
|
||||
"""
|
||||
self._assert_user_active_state(expected_active_state=False)
|
||||
|
||||
redirect_url = 'http://localhost:1991/pied-piper/learn'
|
||||
base_activation_url = reverse('activate', args=[self.registration.activation_key])
|
||||
activation_url = '{base}?{params}'.format(
|
||||
base=base_activation_url,
|
||||
params=urlencode({'next': redirect_url}),
|
||||
)
|
||||
|
||||
response = self.client.get(activation_url, follow=True, HTTP_ACCEPT='*/*')
|
||||
|
||||
expected_destination = "{login_url}?next={redirect_url}".format(
|
||||
login_url=reverse('signin_user'),
|
||||
redirect_url=reverse('dashboard'),
|
||||
)
|
||||
self.assertRedirects(response, expected_destination)
|
||||
self._assert_user_active_state(expected_active_state=True)
|
||||
|
||||
@override_settings(FEATURES=FEATURES_WITH_AUTHN_MFE_ENABLED)
|
||||
@override_waffle_flag(REDIRECT_TO_AUTHN_MICROFRONTEND, active=True)
|
||||
def test_unauthenticated_user_redirects_to_mfe(self):
|
||||
|
||||
@@ -48,9 +48,9 @@ class TestLoginHelper(TestCase):
|
||||
"Redirect to non html content 'image/*' detected from 'test/agent' after login page: '" + static_url +
|
||||
"dummy.png" + "'"),
|
||||
(logging.WARNING, "WARNING", "test.png", "text/html", None,
|
||||
"Redirect to url path with specified filed type 'image/png' not allowed: 'test.png'"),
|
||||
"Redirect to url path with specified file type 'image/png' not allowed: 'test.png'"),
|
||||
(logging.WARNING, "WARNING", static_url + "dummy.png", "text/html", None,
|
||||
"Redirect to url path with specified filed type 'image/png' not allowed: '" + static_url + "dummy.png" + "'"),
|
||||
"Redirect to url path with specified file type 'image/png' not allowed: '" + static_url + "dummy.png" + "'"),
|
||||
)
|
||||
@ddt.unpack
|
||||
def test_next_failures(self, log_level, log_name, unsafe_url, http_accept, user_agent, expected_log):
|
||||
|
||||
@@ -26,7 +26,7 @@ class SendActivationEmailTestCase(TestCase):
|
||||
registration = Registration()
|
||||
registration.register(self.student)
|
||||
|
||||
self.msg = compose_activation_email("http://www.example.com", self.student, registration)
|
||||
self.msg = compose_activation_email(self.student, registration)
|
||||
|
||||
def test_ComposeEmail(self):
|
||||
"""
|
||||
|
||||
@@ -5,6 +5,7 @@ Student Views
|
||||
|
||||
import datetime
|
||||
import logging
|
||||
import urllib.parse
|
||||
import uuid
|
||||
from collections import namedtuple
|
||||
|
||||
@@ -40,6 +41,7 @@ from common.djangoapps.course_modes.models import CourseMode
|
||||
from lms.djangoapps.courseware.courses import get_courses, sort_by_announcement, sort_by_start_date
|
||||
from common.djangoapps.edxmako.shortcuts import marketing_link, render_to_response, render_to_string # lint-amnesty, pylint: disable=unused-import
|
||||
from common.djangoapps.entitlements.models import CourseEntitlement
|
||||
from common.djangoapps.student.helpers import get_next_url_for_login_page, get_redirect_url_with_host
|
||||
from openedx.core.djangoapps.ace_common.template_context import get_base_template_context
|
||||
from openedx.core.djangoapps.catalog.utils import get_programs_with_type
|
||||
from openedx.core.djangoapps.embargo import api as embargo_api
|
||||
@@ -162,7 +164,7 @@ def index(request, extra_context=None, user=AnonymousUser()):
|
||||
return render_to_response('index.html', context)
|
||||
|
||||
|
||||
def compose_activation_email(root_url, user, user_registration=None, route_enabled=False, profile_name=''):
|
||||
def compose_activation_email(user, user_registration=None, route_enabled=False, profile_name='', redirect_url=None):
|
||||
"""
|
||||
Construct all the required params for the activation email
|
||||
through celery task
|
||||
@@ -172,10 +174,7 @@ def compose_activation_email(root_url, user, user_registration=None, route_enabl
|
||||
|
||||
message_context = generate_activation_email_context(user, user_registration)
|
||||
message_context.update({
|
||||
'confirm_activation_link': '{root_url}/activate/{activation_key}'.format(
|
||||
root_url=root_url,
|
||||
activation_key=message_context['key']
|
||||
),
|
||||
'confirm_activation_link': _get_activation_confirmation_link(message_context['key'], redirect_url),
|
||||
'route_enabled': route_enabled,
|
||||
'routed_user': user.username,
|
||||
'routed_user_email': user.email,
|
||||
@@ -196,7 +195,26 @@ def compose_activation_email(root_url, user, user_registration=None, route_enabl
|
||||
return msg
|
||||
|
||||
|
||||
def compose_and_send_activation_email(user, profile, user_registration=None):
|
||||
def _get_activation_confirmation_link(activation_key, redirect_url=None):
|
||||
"""
|
||||
Helper function to build an activation confirmation URL given an activation_key.
|
||||
The confirmation URL will include a "?next={redirect_url}" query if redirect_url
|
||||
is not null.
|
||||
"""
|
||||
root_url = configuration_helpers.get_value('LMS_ROOT_URL', settings.LMS_ROOT_URL)
|
||||
confirmation_link = '{root_url}/activate/{activation_key}'.format(
|
||||
root_url=root_url,
|
||||
activation_key=activation_key,
|
||||
)
|
||||
if not redirect_url:
|
||||
return confirmation_link
|
||||
|
||||
scheme, netloc, path, params, _, fragment = urllib.parse.urlparse(confirmation_link)
|
||||
query = urllib.parse.urlencode({'next': redirect_url})
|
||||
return urllib.parse.urlunparse((scheme, netloc, path, params, query, fragment))
|
||||
|
||||
|
||||
def compose_and_send_activation_email(user, profile, user_registration=None, redirect_url=None):
|
||||
"""
|
||||
Construct all the required params and send the activation email
|
||||
through celery task
|
||||
@@ -205,11 +223,11 @@ def compose_and_send_activation_email(user, profile, user_registration=None):
|
||||
user: current logged-in user
|
||||
profile: profile object of the current logged-in user
|
||||
user_registration: registration of the current logged-in user
|
||||
redirect_url: The URL to redirect to after successful activation
|
||||
"""
|
||||
route_enabled = settings.FEATURES.get('REROUTE_ACTIVATION_EMAIL')
|
||||
|
||||
root_url = configuration_helpers.get_value('LMS_ROOT_URL', settings.LMS_ROOT_URL)
|
||||
msg = compose_activation_email(root_url, user, user_registration, route_enabled, profile.name)
|
||||
msg = compose_activation_email(user, user_registration, route_enabled, profile.name, redirect_url)
|
||||
|
||||
send_activation_email.delay(str(msg))
|
||||
|
||||
@@ -554,6 +572,17 @@ def activate_account(request, key):
|
||||
extra_tags='account-activation aa-icon',
|
||||
)
|
||||
|
||||
# If a (safe) `next` parameter is provided in the request
|
||||
# and it's not the same as the dashboard, redirect there.
|
||||
# The `get_next_url_for_login_page()` function will only return a safe redirect URL.
|
||||
# If the provided `next` URL is not safe, that function will fill `redirect_to`
|
||||
# with a value of `reverse('dashboard')`.
|
||||
if request.GET.get('next'):
|
||||
redirect_to, root_url = get_next_url_for_login_page(request, include_host=True)
|
||||
if redirect_to != reverse('dashboard'):
|
||||
redirect_url = get_redirect_url_with_host(root_url, redirect_to)
|
||||
return redirect(redirect_url)
|
||||
|
||||
if should_redirect_to_authn_microfrontend() and not request.user.is_authenticated:
|
||||
url_path = f'/login?account_activation_status={activation_message_type}'
|
||||
return redirect(settings.AUTHN_MICROFRONTEND_URL + url_path)
|
||||
|
||||
@@ -20,11 +20,12 @@
|
||||
initialize: function(attributes, options) {
|
||||
this.ajaxType = options.method;
|
||||
this.urlRoot = options.url;
|
||||
this.nextUrl = options.nextUrl;
|
||||
},
|
||||
|
||||
sync: function(method, model) {
|
||||
var headers = {'X-CSRFToken': $.cookie('csrftoken')},
|
||||
data = {},
|
||||
data = {next: model.nextUrl},
|
||||
courseId = $.url('?course_id');
|
||||
|
||||
// If there is a course ID in the query string param,
|
||||
|
||||
@@ -199,7 +199,8 @@
|
||||
register: function(data) {
|
||||
var model = new RegisterModel({}, {
|
||||
method: data.method,
|
||||
url: data.submit_url
|
||||
url: data.submit_url,
|
||||
nextUrl: this.nextUrl
|
||||
});
|
||||
|
||||
this.subview.register = new RegisterView({
|
||||
|
||||
@@ -235,7 +235,9 @@ def create_account_with_params(request, params):
|
||||
if skip_email:
|
||||
registration.activate()
|
||||
else:
|
||||
compose_and_send_activation_email(user, profile, registration)
|
||||
redirect_to, root_url = get_next_url_for_login_page(request, include_host=True)
|
||||
redirect_url = get_redirect_url_with_host(root_url, redirect_to)
|
||||
compose_and_send_activation_email(user, profile, registration, redirect_url)
|
||||
|
||||
if settings.FEATURES.get('ENABLE_DISCUSSION_EMAIL_DIGEST'):
|
||||
try:
|
||||
|
||||
Reference in New Issue
Block a user