From a706ad773ddd6dd3ce511127f7d96c66d3439875 Mon Sep 17 00:00:00 2001 From: Troy Sankey Date: Mon, 10 Aug 2026 14:57:04 -0700 Subject: [PATCH] feat: replace enterprise logistration logic with openedx-filters hooks Replace the enterprise-specific logic in the user_authn logistration views with six generic filters, removing all enterprise_support imports and enterprise-specific logic: - LogistrationViewContextGenerated - AuthnMFEContextGenerated - LogistrationViewRenderCompleted - LoginFormGenerated - RegistrationFormGenerated - LoginAltRedirectURLRequested ENT-11568 --- common/djangoapps/third_party_auth/models.py | 10 +- .../djangoapps/third_party_auth/pipeline.py | 14 +- mypy.ini | 12 + .../user_authn/api/tests/data_mock.py | 51 ++ .../user_authn/api/tests/test_serializers.py | 29 ++ .../core/djangoapps/user_authn/serializers.py | 12 + .../core/djangoapps/user_authn/views/login.py | 66 +-- .../djangoapps/user_authn/views/login_form.py | 96 ++-- .../user_authn/views/registration_form.py | 190 ++++---- .../user_authn/views/tests/test_filters.py | 450 ++++++++++++++++++ .../user_authn/views/tests/test_login.py | 109 ----- .../views/tests/test_logistration.py | 258 +--------- .../user_authn/views/tests/test_utils.py | 52 +- .../core/djangoapps/user_authn/views/utils.py | 46 +- requirements/constraints.txt | 2 +- requirements/edx/base.txt | 4 +- requirements/edx/development.txt | 4 +- requirements/edx/doc.txt | 4 +- requirements/edx/testing.txt | 4 +- 19 files changed, 855 insertions(+), 558 deletions(-) diff --git a/common/djangoapps/third_party_auth/models.py b/common/djangoapps/third_party_auth/models.py index e172d3c7d6e5..309ebd78c984 100644 --- a/common/djangoapps/third_party_auth/models.py +++ b/common/djangoapps/third_party_auth/models.py @@ -15,6 +15,7 @@ from django.db import models from django.utils import timezone from django.utils.translation import gettext_lazy as _ +from openedx_filters.authentication.types import RunningPipelineKwargs from organizations.models import Organization from social_core.backends.base import BaseAuth from social_core.backends.oauth import OAuthAuth @@ -317,7 +318,7 @@ def get_social_auth_uid(self, remote_id): return remote_id @classmethod - def get_register_form_data(cls, pipeline_kwargs): + def get_register_form_data(cls, pipeline_kwargs: RunningPipelineKwargs): """Gets dict of data to display on the register form. register_user uses this to populate @@ -325,8 +326,11 @@ def get_register_form_data(cls, pipeline_kwargs): provider, preventing duplicate data entry. Args: - pipeline_kwargs: dict of string -> object. Keyword arguments - accumulated by the pipeline thus far. + pipeline_kwargs (RunningPipelineKwargs): Keyword arguments accumulated by the + pipeline thus far. This method is reachable from pipeline steps of the + registration form filter, which may live in other repositories, so the + argument's declared shape is the shared cross-repository contract in + ``openedx_filters.authentication.types``. Returns: Dict of string -> string. Keys are names of form fields; values are diff --git a/common/djangoapps/third_party_auth/pipeline.py b/common/djangoapps/third_party_auth/pipeline.py index 757075a61221..f81379ecffa1 100644 --- a/common/djangoapps/third_party_auth/pipeline.py +++ b/common/djangoapps/third_party_auth/pipeline.py @@ -78,6 +78,7 @@ def B(*args, **kwargs): from django.shortcuts import redirect from django.urls import reverse from edx_django_utils.monitoring import set_custom_attribute +from openedx_filters.authentication.types import RunningPipeline from social_core.exceptions import AuthException from social_core.pipeline import partial from social_core.utils import module_member, slugify @@ -212,8 +213,15 @@ def get_unlink_form_name(self): return self.provider.provider_id + '_unlink_form' -def get(request): - """Gets the running pipeline's data from the passed request.""" +def get(request) -> RunningPipeline | None: + """Gets the running pipeline's data from the passed request. + + The returned mapping is the shared cross-repository contract for third-party auth + pipeline state: it is handed to pipeline steps of the login and registration form + filters, which may live in other repositories. See + ``openedx_filters.authentication.types.RunningPipeline`` for the declared shape, and + keep this function's dict literal in agreement with it. + """ strategy = social_django.utils.load_strategy(request) token = strategy.session_get('partial_pipeline_token') @@ -222,7 +230,7 @@ def get(request): token = strategy.session_get('partial_pipeline_token') partial_object = strategy.partial_load(token) - pipeline_data = None + pipeline_data: RunningPipeline | None = None if partial_object: pipeline_data = {'kwargs': partial_object.kwargs, 'backend': partial_object.backend} return pipeline_data diff --git a/mypy.ini b/mypy.ini index 6e36cde13e6c..3318d9c96d93 100644 --- a/mypy.ini +++ b/mypy.ini @@ -6,6 +6,16 @@ plugins = mypy_django_plugin.main, mypy_drf_plugin.main files = + # Third-party auth pipeline state is a cross-repository contract (see + # openedx_filters.authentication.types.RunningPipeline). pipeline.py must be a check + # root rather than merely imported: follow_imports = silent suppresses errors inside + # unlisted modules, so the dict literal that produces that contract would otherwise + # go unchecked. + common/djangoapps/third_party_auth/pipeline.py, + common/djangoapps/third_party_auth/provider.py, + openedx/core/djangoapps/user_authn/views/utils.py, + openedx/core/djangoapps/user_authn/views/login_form.py, + openedx/core/djangoapps/user_authn/views/registration_form.py, cms/lib/xblock, cms/djangoapps/contentstore/rest_api/v2/views, cms/djangoapps/contentstore/xblock_storage_handlers, @@ -64,6 +74,8 @@ ignore_missing_imports = True ignore_missing_imports = True [mypy-search.*] ignore_missing_imports = True +[mypy-six.*] +ignore_missing_imports = True [mypy-rules.*] ignore_missing_imports = True [mypy-web_fragments.*] diff --git a/openedx/core/djangoapps/user_authn/api/tests/data_mock.py b/openedx/core/djangoapps/user_authn/api/tests/data_mock.py index ffb9ad11b203..9f138e781a0a 100644 --- a/openedx/core/djangoapps/user_authn/api/tests/data_mock.py +++ b/openedx/core/djangoapps/user_authn/api/tests/data_mock.py @@ -112,3 +112,54 @@ 'extended_profile': [] } } + +# Entries a plugin may contribute through the AuthnMFEContextGenerated filter. The +# serializer declares no fields for these, so they are merged into the response as-is, +# at whatever depth the contributor chose. +EXTRA_CONTEXT_DATA = { + 'brandingName': 'Acme Corp', + 'brandingLogoUrl': 'https://example.com/acme-logo.png', + 'brandingStrings': { + 'welcome': 'Welcome, Acme learners!', + }, +} + +MFE_CONTEXT_WITH_EXTRA_CONTEXT_DATA = { + 'context_data': { + 'currentProvider': None, + 'platformName': 'edX', + 'providers': [], + 'secondaryProviders': [], + 'finishAuthUrl': None, + 'errorMessage': None, + 'registerFormSubmitButtonText': 'Create Account', + 'autoSubmitRegForm': False, + 'syncLearnerProfileData': False, + 'countryCode': '', + 'welcomePageRedirectUrl': '', + 'pipeline_user_details': {}, + 'extra_context': EXTRA_CONTEXT_DATA, + }, +} + +SERIALIZED_MFE_CONTEXT_WITH_EXTRA_CONTEXT_DATA = { + 'contextData': { + 'currentProvider': None, + 'platformName': 'edX', + 'providers': [], + 'secondaryProviders': [], + 'finishAuthUrl': None, + 'errorMessage': None, + 'registerFormSubmitButtonText': 'Create Account', + 'autoSubmitRegForm': False, + 'syncLearnerProfileData': False, + 'countryCode': '', + 'welcomePageRedirectUrl': '', + 'pipelineUserDetails': {}, + **EXTRA_CONTEXT_DATA, + }, + 'registrationFields': {}, + 'optionalFields': { + 'extended_profile': [], + }, +} diff --git a/openedx/core/djangoapps/user_authn/api/tests/test_serializers.py b/openedx/core/djangoapps/user_authn/api/tests/test_serializers.py index 3b22508958aa..34c1b3cd5bf5 100644 --- a/openedx/core/djangoapps/user_authn/api/tests/test_serializers.py +++ b/openedx/core/djangoapps/user_authn/api/tests/test_serializers.py @@ -5,8 +5,10 @@ from django.test import TestCase from openedx.core.djangoapps.user_authn.api.tests.data_mock import ( + MFE_CONTEXT_WITH_EXTRA_CONTEXT_DATA, MFE_CONTEXT_WITH_TPA_DATA, MFE_CONTEXT_WITHOUT_TPA_DATA, + SERIALIZED_MFE_CONTEXT_WITH_EXTRA_CONTEXT_DATA, SERIALIZED_MFE_CONTEXT_WITH_TPA_DATA, SERIALIZED_MFE_CONTEXT_WITHOUT_TPA_DATA, ) @@ -44,3 +46,30 @@ def test_mfe_context_serializer_default_response(self): serialized_data, SERIALIZED_MFE_CONTEXT_WITHOUT_TPA_DATA ) + + def test_mfe_context_serializer_with_extra_context(self): + """ + Test that entries the serializer declares no fields for are merged into contextData + from extra_context, preserving their nesting. + """ + output_data = MFEContextSerializer( + MFE_CONTEXT_WITH_EXTRA_CONTEXT_DATA + ).data + + assert output_data == SERIALIZED_MFE_CONTEXT_WITH_EXTRA_CONTEXT_DATA + + def test_mfe_context_serializer_ignores_undeclared_context_keys(self): + """ + Test that context entries outside extra_context are still dropped, so the response + shape stays under the serializer's control. + """ + context = { + 'context_data': { + **MFE_CONTEXT_WITHOUT_TPA_DATA['context_data'], + 'skipRegistrationOptionalCheckboxes': True, + }, + } + + serialized_data = MFEContextSerializer(context).data + + assert 'skipRegistrationOptionalCheckboxes' not in serialized_data['contextData'] diff --git a/openedx/core/djangoapps/user_authn/serializers.py b/openedx/core/djangoapps/user_authn/serializers.py index c088b7eda7db..5ba9a2a52df3 100644 --- a/openedx/core/djangoapps/user_authn/serializers.py +++ b/openedx/core/djangoapps/user_authn/serializers.py @@ -61,6 +61,18 @@ def get_pipelineUserDetails(self, obj): return PipelineUserDetailsSerializer(obj.get('pipeline_user_details')).data return {} + def to_representation(self, instance): + """ + Serialize the declared fields, then merge in the context's ``extra_context`` entries. + + ``extra_context`` holds entries contributed by plugins, which this serializer cannot + declare fields for. They are merged as-is: the contributor owns their shape, and no + coercion is applied. + """ + representation = super().to_representation(instance) + representation.update(instance.get('extra_context') or {}) + return representation + class MFEContextSerializer(serializers.Serializer): """ diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index 6a012d8d212d..d09dbc1b0cf5 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -7,8 +7,6 @@ import hashlib import json import logging -import re -import urllib from django.conf import settings from django.contrib.auth import authenticate, get_user_model @@ -29,6 +27,7 @@ from eventtracking import tracker from openedx_events.learning.data import UserData, UserPersonalData from openedx_events.learning.signals import SESSION_LOGIN_COMPLETED +from openedx_filters.authentication.filters import LoginAltRedirectURLRequested from openedx_filters.learning.filters import StudentLoginRequested from rest_framework import status from rest_framework.views import APIView @@ -54,12 +53,12 @@ is_require_third_party_auth_enabled, should_redirect_to_authn_microfrontend, ) +from openedx.core.djangoapps.user_authn.utils import is_safe_login_or_logout_redirect from openedx.core.djangoapps.user_authn.views.login_form import get_login_session_form from openedx.core.djangoapps.user_authn.views.password_reset import send_password_reset_email_for_user -from openedx.core.djangoapps.user_authn.views.utils import API_V1, ENTERPRISE_ENROLLMENT_URL_REGEX, UUID4_REGEX +from openedx.core.djangoapps.user_authn.views.utils import API_V1 from openedx.core.djangoapps.util.user_messages import PageLevelMessages from openedx.core.djangolib.markup import HTML, Text -from openedx.features.enterprise_support.api import activate_learner_enterprise, get_enterprise_learner_data_from_api log = logging.getLogger("edx.student") AUDIT_LOG = logging.getLogger("audit") @@ -477,33 +476,45 @@ def finish_auth(request): ) -def enterprise_selection_page(request, user, next_url): +def _get_alt_redirect_url(request, redirect_url, user): """ - Updates redirect url to enterprise selection page if user is associated - with multiple enterprises otherwise return the next url. + Ask the configured pipeline steps for an alternative post-login redirect URL. - param: - next_url(string): The URL to redirect to after multiple enterprise selection or in case - the selection page is bypassed e.g when dealing with direct enrolment urls. - """ - redirect_url = next_url + The pipeline is arbitrary configured code, so its answer is held to the same + open-redirect protections as a caller-supplied ``?next=`` parameter: an unsafe URL is + discarded and the caller's own destination is used instead. - response = get_enterprise_learner_data_from_api(user) - if response and len(response) > 1: - redirect_url = reverse("enterprise_select_active") + "/?success_url=" + urllib.parse.quote(next_url) + Arguments: + request (HttpRequest) + redirect_url (str): the destination the caller intends to send the user to. + user (User): the authenticated user. - # Check to see if next url has an enterprise in it. In this case if user is associated with - # that enterprise, activate that enterprise and bypass the selection page. - if re.match(ENTERPRISE_ENROLLMENT_URL_REGEX, urllib.parse.unquote(next_url)): - enterprise_in_url = re.search(UUID4_REGEX, next_url).group(0) - for enterprise in response: - if enterprise_in_url == str(enterprise["enterprise_customer"]["uuid"]): - is_activated_successfully = activate_learner_enterprise(request, user, enterprise_in_url) - if is_activated_successfully: - redirect_url = next_url - break + Returns: str + the alternative redirect url if safe, else the given redirect_url. + """ + # .. filter_implemented_name: LoginAltRedirectURLRequested + # .. filter_type: org.openedx.authentication.login.alt_redirect_url.requested.v1 + alt_redirect_url, __ = LoginAltRedirectURLRequested.run_filter( + redirect_url=redirect_url, + user=user, + ) + + if alt_redirect_url == redirect_url: + return redirect_url + + if not alt_redirect_url or not is_safe_login_or_logout_redirect( + redirect_to=alt_redirect_url, + request_host=request.get_host(), + dot_client_id=request.POST.get("client_id"), + require_https=request.is_secure(), + ): + log.warning( + "Unsafe alternative redirect URL detected after login: '%(alt_redirect_url)s'", + {"alt_redirect_url": alt_redirect_url}, + ) + return redirect_url - return redirect_url + return alt_redirect_url @ensure_csrf_cookie @@ -648,7 +659,8 @@ def login_user(request, api_version="v1"): # pylint: disable=too-many-statement elif should_redirect_to_authn_microfrontend(): next_url, root_url = get_next_url_for_login_page(request, include_host=True) redirect_url = get_redirect_url_with_host( - root_url, enterprise_selection_page(request, possibly_authenticated_user, finish_auth_url or next_url) + root_url, + _get_alt_redirect_url(request, finish_auth_url or next_url, possibly_authenticated_user), ) if ( diff --git a/openedx/core/djangoapps/user_authn/views/login_form.py b/openedx/core/djangoapps/user_authn/views/login_form.py index 624259c9c3cf..112703b8adf2 100644 --- a/openedx/core/djangoapps/user_authn/views/login_form.py +++ b/openedx/core/djangoapps/user_authn/views/login_form.py @@ -13,6 +13,11 @@ from django.views.decorators.csrf import ensure_csrf_cookie from django.views.decorators.http import require_http_methods from django_ratelimit.decorators import ratelimit +from openedx_filters.authentication.filters import ( + LoginFormGenerated, + LogistrationViewContextGenerated, + LogistrationViewRenderCompleted, +) from common.djangoapps import third_party_auth from common.djangoapps.edxmako.shortcuts import render_to_response @@ -31,49 +36,14 @@ ) from openedx.core.djangoapps.user_authn.views.password_reset import get_password_reset_form from openedx.core.djangoapps.user_authn.views.registration_form import RegistrationFormFactory -from openedx.core.djangoapps.user_authn.views.utils import third_party_auth_context -from openedx.features.enterprise_support.api import enterprise_customer_for_request, enterprise_enabled -from openedx.features.enterprise_support.utils import ( - get_enterprise_slug_login_url, - handle_enterprise_cookies_for_logistration, - update_logistration_context_for_enterprise, +from openedx.core.djangoapps.user_authn.views.utils import ( + get_running_third_party_auth_state, + third_party_auth_context, ) log = logging.getLogger(__name__) -def _apply_third_party_auth_overrides(request, form_desc): - """Modify the login form if the user has authenticated with a third-party provider. - If a user has successfully authenticated with a third-party provider, - and an email is associated with it then we fill in the email field with readonly property. - Arguments: - request (HttpRequest): The request for the registration form, used - to determine if the user has successfully authenticated - with a third-party provider. - form_desc (FormDescription): The registration form description - """ - if third_party_auth.is_enabled(): - running_pipeline = third_party_auth.pipeline.get(request) - if running_pipeline: - current_provider = third_party_auth.provider.Registry.get_from_pipeline(running_pipeline) - if current_provider and enterprise_customer_for_request(request): - pipeline_kwargs = running_pipeline.get('kwargs') - - # Details about the user sent back from the provider. - details = pipeline_kwargs.get('details') - email = details.get('email', '') - - # override the email field. - form_desc.override_field_properties( - "email", - default=email, - restrictions={"readonly": "readonly"} if email else { - "min_length": accounts.EMAIL_MIN_LENGTH, - "max_length": accounts.EMAIL_MAX_LENGTH, - } - ) - - def get_login_session_form(request): """Return a description of the login form. @@ -89,7 +59,17 @@ def get_login_session_form(request): """ form_desc = FormDescription("post", reverse("user_api_login_session", kwargs={'api_version': 'v1'})) - _apply_third_party_auth_overrides(request, form_desc) + running_pipeline, current_provider = get_running_third_party_auth_state(request) + + # Field property overrides applied by pipeline steps take effect when the fields are + # added below, so the overrides must be applied before the fields are added. + # .. filter_implemented_name: LoginFormGenerated + # .. filter_type: org.openedx.authentication.login.form.generated.v1 + form_desc, running_pipeline, current_provider = LoginFormGenerated.run_filter( + form_desc=form_desc, + running_pipeline=running_pipeline, + current_provider=current_provider, + ) # Translators: This label appears above a field on the login form # meant to hold the user's email address. @@ -186,13 +166,6 @@ def login_and_registration_form(request, initial_mode="login"): except (KeyError, ValueError, IndexError) as ex: log.exception("Unknown tpa_hint provider: %s", ex) - # Redirect to authn MFE if it is enabled - # AND - # user is not an enterprise user - # AND - # tpa_hint_provider is not available - # AND - # user is not coming from a SAML IDP. saml_provider = False running_pipeline = pipeline.get(request) if running_pipeline: @@ -200,20 +173,15 @@ def login_and_registration_form(request, initial_mode="login"): running_pipeline.get('backend'), running_pipeline.get('kwargs') ) - enterprise_customer = enterprise_customer_for_request(request) - - if should_redirect_to_authn_microfrontend() and \ - not enterprise_customer and \ - not tpa_hint_provider and \ - not saml_provider: - - # This is to handle a case where a logged-in cookie is not present but the user is authenticated. - # Note: If we don't handle this learner is redirected to authn MFE and then back to dashboard - # instead of the desired redirect URL (e.g. finish_auth) resulting in learners not enrolling - # into the courses. - if request.user.is_authenticated and redirect_to: - return redirect(redirect_to) + has_external_provider = bool(tpa_hint_provider or saml_provider) + # Redirect to the authn MFE when it is enabled, UNLESS auth is provided externally. + # + # NOTE: SAML/TPA users authenticating externally must remain on the legacy login/registration page because the + # new AuthN MFE does not yet fully support provider branding, hinted-login dialog, etc. + # + # TODO: Remove the `has_external_provider` check as soon as the AuthN MFE is fully-featured. + if should_redirect_to_authn_microfrontend() and not has_external_provider: query_params = request.GET.urlencode() url_path = '/{}{}'.format( initial_mode, @@ -260,8 +228,6 @@ def login_and_registration_form(request, initial_mode="login"): 'ALLOW_PUBLIC_ACCOUNT_CREATION', settings.ALLOW_PUBLIC_ACCOUNT_CREATION), 'register_links_allowed': settings.SHOW_REGISTRATION_LINKS, 'is_account_recovery_feature_enabled': is_secondary_email_feature_enabled(), - 'enterprise_slug_login_url': get_enterprise_slug_login_url(), - 'is_enterprise_enable': enterprise_enabled(), 'is_require_third_party_auth_enabled': is_require_third_party_auth_enabled(), 'enable_coppa_compliance': settings.ENABLE_COPPA_COMPLIANCE, 'edx_user_info_cookie_name': settings.EDXMKTG_USER_INFO_COOKIE_NAME, @@ -277,11 +243,15 @@ def login_and_registration_form(request, initial_mode="login"): ), } - update_logistration_context_for_enterprise(request, context, enterprise_customer) + # .. filter_implemented_name: LogistrationViewContextGenerated + # .. filter_type: org.openedx.authentication.logistration_view.context.generated.v1 + context = LogistrationViewContextGenerated.run_filter(context=context) response = render_to_response('student_account/login_and_register.html', context) - handle_enterprise_cookies_for_logistration(request, response, context) + # .. filter_implemented_name: LogistrationViewRenderCompleted + # .. filter_type: org.openedx.authentication.logistration_view.render.completed.v1 + response, __ = LogistrationViewRenderCompleted.run_filter(response=response, context=context) return response diff --git a/openedx/core/djangoapps/user_authn/views/registration_form.py b/openedx/core/djangoapps/user_authn/views/registration_form.py index 1d658fca76a9..d6c14d333430 100644 --- a/openedx/core/djangoapps/user_authn/views/registration_form.py +++ b/openedx/core/djangoapps/user_authn/views/registration_form.py @@ -10,14 +10,15 @@ from django import forms from django.conf import settings from django.contrib.auth.models import User # pylint: disable=imported-auth-user -from django.core.exceptions import ImproperlyConfigured -from django.core.validators import RegexValidator, ValidationError, slug_re +from django.core.exceptions import ImproperlyConfigured, ValidationError +from django.core.validators import RegexValidator, slug_re from django.db.models import Model from django.forms import widgets from django.urls import reverse from django.utils.translation import gettext as _ from django_countries import countries from eventtracking import tracker +from openedx_filters.authentication.filters import RegistrationFormGenerated from common.djangoapps import third_party_auth from common.djangoapps.edxmako.shortcuts import marketing_link @@ -34,9 +35,11 @@ from openedx.core.djangoapps.user_api.helpers import FormDescription from openedx.core.djangoapps.user_authn.utils import check_pwned_password from openedx.core.djangoapps.user_authn.utils import is_registration_api_v1 as is_api_v1 -from openedx.core.djangoapps.user_authn.views.utils import remove_disabled_country_from_list +from openedx.core.djangoapps.user_authn.views.utils import ( + get_running_third_party_auth_state, + remove_disabled_country_from_list, +) from openedx.core.djangolib.markup import HTML, Text -from openedx.features.enterprise_support.api import enterprise_customer_for_request log = logging.getLogger(__name__) @@ -581,7 +584,21 @@ def get_registration_form(self, request): """ self.request = request form_desc = FormDescription("post", self._get_registration_submit_url(request)) - self._apply_third_party_auth_overrides(request, form_desc) + + # Field property overrides applied below take effect when the fields are added in + # the loop, so they must be applied before the field loop. The platform's own + # third-party-auth overrides are applied first; the filter runs afterwards so its + # pipeline steps can further adjust the form description. + running_pipeline, current_provider = get_running_third_party_auth_state(request) + form_desc = self._apply_third_party_auth_overrides(form_desc, running_pipeline, current_provider) + + # .. filter_implemented_name: RegistrationFormGenerated + # .. filter_type: org.openedx.authentication.registration.form.generated.v1 + form_desc, running_pipeline, current_provider = RegistrationFormGenerated.run_filter( + form_desc=form_desc, + running_pipeline=running_pipeline, + current_provider=current_provider, + ) # Custom form fields can be added via the form set in settings.PROFILE_EXTENSION_FORM # (or deprecated settings.REGISTRATION_EXTENSION_FORM) @@ -1289,7 +1306,7 @@ def _add_terms_of_service_field(self, form_desc, required=True): }, ) - def _apply_third_party_auth_overrides(self, request, form_desc): + def _apply_third_party_auth_overrides(self, form_desc, running_pipeline, current_provider): """Modify the registration form if the user has authenticated with a third-party provider. If a user has successfully authenticated with a third-party provider, but does not yet have an account with EdX, we want to fill in @@ -1299,96 +1316,87 @@ def _apply_third_party_auth_overrides(self, request, form_desc): (random) password on the assumption that they will be using third-party auth to log in. Arguments: - request (HttpRequest): The request for the registration form, used - to determine if the user has successfully authenticated - with a third-party provider. form_desc (FormDescription): The registration form description + running_pipeline (dict): The third party auth pipeline running for the request, + or None if there is none. + current_provider (ProviderConfig): The provider of the running pipeline, or None + if there is no running pipeline or its provider could not be determined. + Returns: + FormDescription: the (possibly modified) registration form description """ - # pylint: disable=too-many-nested-blocks - if third_party_auth.is_enabled(): - running_pipeline = third_party_auth.pipeline.get(request) - if running_pipeline: - current_provider = third_party_auth.provider.Registry.get_from_pipeline(running_pipeline) - - if current_provider: - # Override username / email / full name - field_overrides = current_provider.get_register_form_data( - running_pipeline.get('kwargs') - ) + if current_provider: + # Override username / email / full name + field_overrides = current_provider.get_register_form_data( + running_pipeline.get('kwargs') + ) - # When the TPA Provider is configured to skip the registration form and we are in an - # enterprise context, we need to hide all fields except for terms of service and - # ensure that the user explicitly checks that field. - # pylint: disable=consider-using-ternary - hide_registration_fields_except_tos = ( - ( - current_provider.skip_registration_form and enterprise_customer_for_request(request) - ) or current_provider.sync_learner_profile_data - ) + hide_registration_fields_except_tos = current_provider.sync_learner_profile_data + + for field_name in self.DEFAULT_FIELDS + self.EXTRA_FIELDS: + if field_name not in field_overrides: + continue + + # Special handling for marketing_emails_opt_in: + # If SAML provider config has skip_registration_optional_checkboxes=True, + # don't let the provider's get_register_form_data override the default + skip_override = False + if field_name == 'marketing_emails_opt_in': + saml_config = self._get_saml_provider_config() + if saml_config and saml_config.skip_registration_optional_checkboxes: + log.debug( + "Skipping provider override for marketing_emails_opt_in " + "due to SAML config for provider: %s", + saml_config.slug + ) + skip_override = True - for field_name in self.DEFAULT_FIELDS + self.EXTRA_FIELDS: - if field_name not in field_overrides: - continue - - # Special handling for marketing_emails_opt_in: - # If SAML provider config has skip_registration_optional_checkboxes=True, - # don't let the provider's get_register_form_data override the default - skip_override = False - if field_name == 'marketing_emails_opt_in': - saml_config = self._get_saml_provider_config() - if saml_config and saml_config.skip_registration_optional_checkboxes: - log.debug( - "Skipping provider override for marketing_emails_opt_in " - "due to SAML config for provider: %s", - saml_config.slug - ) - skip_override = True + if not skip_override: + form_desc.override_field_properties( + field_name, default=field_overrides[field_name] + ) - if not skip_override: - form_desc.override_field_properties( - field_name, default=field_overrides[field_name] - ) + if ( + field_name not in ['terms_of_service', 'honor_code'] and + field_overrides[field_name] and + hide_registration_fields_except_tos + ): + field_default = field_overrides[field_name] + form_desc.override_field_properties( + field_name, + field_type="hidden", + default=field_default, + label="", + instructions="", + ) - if ( - field_name not in ['terms_of_service', 'honor_code'] and - field_overrides[field_name] and - hide_registration_fields_except_tos - ): - field_default = field_overrides[field_name] - form_desc.override_field_properties( - field_name, - field_type="hidden", - default=field_default, - label="", - instructions="", - ) + # Hide the confirm_email field + form_desc.override_field_properties( + "confirm_email", + default="", + field_type="hidden", + required=False, + label="", + instructions="", + restrictions={} + ) - # Hide the confirm_email field - form_desc.override_field_properties( - "confirm_email", - default="", - field_type="hidden", - required=False, - label="", - instructions="", - restrictions={} - ) + # Hide the password field + form_desc.override_field_properties( + "password", + default="", + field_type="hidden", + required=False, + label="", + instructions="", + restrictions={} + ) + # used to identify that request is running third party social auth + form_desc.add_field( + "social_auth_provider", + field_type="hidden", + label="", + default=current_provider.name if current_provider.name else "Third Party", + required=False, + ) - # Hide the password field - form_desc.override_field_properties( - "password", - default="", - field_type="hidden", - required=False, - label="", - instructions="", - restrictions={} - ) - # used to identify that request is running third party social auth - form_desc.add_field( - "social_auth_provider", - field_type="hidden", - label="", - default=current_provider.name if current_provider.name else "Third Party", - required=False, - ) + return form_desc diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_filters.py b/openedx/core/djangoapps/user_authn/views/tests/test_filters.py index 41f994501ef7..41fe122bfdc8 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_filters.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_filters.py @@ -1,6 +1,8 @@ """ Test that various filters are fired for the vies in the user_authn app. """ +from unittest.mock import Mock, patch + from django.contrib.auth import get_user_model from django.test import override_settings from django.urls import reverse @@ -272,3 +274,451 @@ def test_login_without_filter_configuration(self): response = self.client.post(self.url, data) self.assertEqual(status.HTTP_200_OK, response.status_code) # noqa: PT009 + + +class TestFormDescriptionPipelineStep(PipelineStep): + """ + Utility class used when getting steps for pipeline. + """ + + def run_filter(self, form_desc, running_pipeline, current_provider): # pylint: disable=arguments-differ + """Pipeline step that overrides the default value of the email field.""" + form_desc.override_field_properties("email", default="filtered@example.com") + return { + "form_desc": form_desc, + "running_pipeline": running_pipeline, + "current_provider": current_provider, + } + + +class TestLogistrationContextPipelineStep(PipelineStep): + """ + Utility class used when getting steps for pipeline. + """ + + def run_filter(self, context): # pylint: disable=arguments-differ + """Pipeline step that modifies the logistration page context.""" + context["data"]["platform_name"] = "Filtered Platform Name" + return { + "context": context, + } + + +class TestLogistrationResponsePipelineStep(PipelineStep): + """ + Utility class used when getting steps for pipeline. + """ + + def run_filter(self, response, context): # pylint: disable=arguments-differ + """Pipeline step that sets a cookie on the logistration response.""" + response.set_cookie("logistration-filter", "applied") + return { + "response": response, + "context": context, + } + + +class TestPostLoginRedirectPipelineStep(PipelineStep): + """ + Utility class used when getting steps for pipeline. + """ + + def run_filter(self, redirect_url, user): # pylint: disable=arguments-differ + """Pipeline step that overrides the post-login redirect URL.""" + return { + "redirect_url": "/custom/post/login", + "user": user, + } + + +class TestUnsafePostLoginRedirectPipelineStep(PipelineStep): + """ + Utility class used when getting steps for pipeline. + """ + + def run_filter(self, redirect_url, user): # pylint: disable=arguments-differ + """Pipeline step that overrides the post-login redirect URL with an off-site one.""" + return { + "redirect_url": "http://evil.example.com/phish", + "user": user, + } + + +class TestAuthnMFEContextPipelineStep(PipelineStep): + """ + Utility class used when getting steps for pipeline. + """ + + def run_filter(self, context, extra_context): # pylint: disable=arguments-differ + """ + Pipeline step that modifies a declared context entry and contributes an undeclared one. + """ + context["platformName"] = "Filtered Platform Name" + extra_context["brandingStrings"] = {"welcome": "Filtered Welcome"} + return { + "context": context, + "extra_context": extra_context, + } + + +@skip_unless_lms +class LoginFormFiltersTest(UserAPITestCase): + """ + Tests for the Open edX Filters associated with the login form description. + + This class guarantees that the following filter is triggered while the login form + description is built, on every request (not only during third-party auth): + + - LoginFormGenerated + """ + + def setUp(self): # pylint: disable=arguments-differ + super().setUp() + self.url = reverse("user_api_login_session", kwargs={"api_version": "v1"}) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.authentication.login.form.generated.v1": { + "pipeline": [ + "openedx.core.djangoapps.user_authn.views.tests.test_filters.TestFormDescriptionPipelineStep", + ], + "fail_silently": False, + }, + }, + ) + def test_login_form_filter_executed(self): + """ + Test whether the login form filter is triggered while the form is built, without + any third-party-auth pipeline running. + + Expected result: + - LoginFormGenerated is triggered and executes TestFormDescriptionPipelineStep. + - The email field default is overridden in the serialized form description. + """ + response = self.client.get(self.url) + + self.assertContains(response, "filtered@example.com") + + @override_settings(OPEN_EDX_FILTERS_CONFIG={}) + def test_login_form_without_filter_configuration(self): + """ + Test usual login form description, without filter's intervention. + + Expected result: + - LoginFormGenerated does not have any effect on the form description. + """ + response = self.client.get(self.url) + + assert response.status_code == status.HTTP_200_OK + self.assertNotContains(response, "filtered@example.com") + + +@skip_unless_lms +class RegistrationFormFiltersTest(UserAPITestCase): + """ + Tests for the Open edX Filters associated with the registration form description. + + This class guarantees that the following filter is triggered while the registration + form description is built, on every request (not only during third-party auth): + + - RegistrationFormGenerated + """ + + def setUp(self): # pylint: disable=arguments-differ + super().setUp() + self.url = reverse("user_api_registration") + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.authentication.registration.form.generated.v1": { + "pipeline": [ + "openedx.core.djangoapps.user_authn.views.tests.test_filters.TestFormDescriptionPipelineStep", + ], + "fail_silently": False, + }, + }, + ) + def test_registration_form_filter_executed(self): + """ + Test whether the registration form filter is triggered while the form is built, + without any third-party-auth pipeline running. + + Expected result: + - RegistrationFormGenerated is triggered and executes TestFormDescriptionPipelineStep. + - The email field default is overridden in the serialized form description. + """ + response = self.client.get(self.url) + + self.assertContains(response, "filtered@example.com") + + @override_settings(OPEN_EDX_FILTERS_CONFIG={}) + def test_registration_form_without_filter_configuration(self): + """ + Test usual registration form description, without filter's intervention. + + Expected result: + - RegistrationFormGenerated does not have any effect on the form description. + """ + response = self.client.get(self.url) + + assert response.status_code == status.HTTP_200_OK + self.assertNotContains(response, "filtered@example.com") + + +@skip_unless_lms +class LogistrationPageFiltersTest(UserAPITestCase): + """ + Tests for the Open edX Filters associated with the legacy logistration page. + + This class guarantees that the following filters are triggered while the combined + login/registration page is rendered: + + - LogistrationViewContextGenerated + - LogistrationViewRenderCompleted + """ + + def setUp(self): # pylint: disable=arguments-differ + super().setUp() + self.url = reverse("signin_user") + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.authentication.logistration_view.context.generated.v1": { + "pipeline": [ + "openedx.core.djangoapps.user_authn.views.tests.test_filters.TestLogistrationContextPipelineStep", + ], + "fail_silently": False, + }, + }, + ) + def test_logistration_context_filter_executed(self): + """ + Test whether the logistration context filter is triggered before the page is rendered. + + Expected result: + - LogistrationViewContextGenerated is triggered and executes TestLogistrationContextPipelineStep. + - The platform name overridden by the pipeline step is rendered into the page. + """ + response = self.client.get(self.url, HTTP_ACCEPT="text/html") + + self.assertContains(response, "Filtered Platform Name") + + @override_settings(OPEN_EDX_FILTERS_CONFIG={}) + def test_logistration_context_without_filter_configuration(self): + """ + Test usual logistration page rendering, without filter's intervention. + + Expected result: + - LogistrationViewContextGenerated does not have any effect on the context. + """ + response = self.client.get(self.url, HTTP_ACCEPT="text/html") + + assert response.status_code == status.HTTP_200_OK + self.assertNotContains(response, "Filtered Platform Name") + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.authentication.logistration_view.render.completed.v1": { + "pipeline": [ + "openedx.core.djangoapps.user_authn.views.tests.test_filters.TestLogistrationResponsePipelineStep", + ], + "fail_silently": False, + }, + }, + ) + def test_logistration_response_filter_executed(self): + """ + Test whether the logistration response filter is triggered after the page is rendered. + + Expected result: + - LogistrationViewRenderCompleted is triggered and executes TestLogistrationResponsePipelineStep. + - The cookie set by the pipeline step is present on the response. + """ + response = self.client.get(self.url, HTTP_ACCEPT="text/html") + + assert response.status_code == status.HTTP_200_OK + assert response.cookies["logistration-filter"].value == "applied" + + @override_settings(OPEN_EDX_FILTERS_CONFIG={}) + def test_logistration_response_without_filter_configuration(self): + """ + Test usual logistration page rendering, without filter's intervention. + + Expected result: + - LogistrationViewRenderCompleted does not have any effect on the response. + """ + response = self.client.get(self.url, HTTP_ACCEPT="text/html") + + assert response.status_code == status.HTTP_200_OK + assert "logistration-filter" not in response.cookies + + +@skip_unless_lms +class PostLoginRedirectFiltersTest(UserAPITestCase): + """ + Tests for the Open edX Filters associated with the post-login redirect URL. + + This class guarantees that the following filters are triggered after a successful login: + + - LoginAltRedirectURLRequested + """ + + def setUp(self): # pylint: disable=arguments-differ + super().setUp() + self.user = UserFactory.create( + username="test", + email="test@example.com", + password="password", + ) + self.user_profile = UserProfileFactory.create(user=self.user, name="Test Example") + self.url = reverse("login_api") + + @patch( + "openedx.core.djangoapps.user_authn.views.login.should_redirect_to_authn_microfrontend", + Mock(return_value=True), + ) + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.authentication.login.alt_redirect_url.requested.v1": { + "pipeline": [ + "openedx.core.djangoapps.user_authn.views.tests.test_filters.TestPostLoginRedirectPipelineStep", + ], + "fail_silently": False, + }, + }, + ) + def test_post_login_redirect_filter_executed(self): + """ + Test whether the post-login redirect filter is triggered after a successful login. + + Expected result: + - LoginAltRedirectURLRequested is triggered and executes TestPostLoginRedirectPipelineStep. + - The redirect URL returned in the response comes from the pipeline step. + """ + data = { + "email": "test@example.com", + "password": "password", + } + + response = self.client.post(self.url, data) + + assert response.status_code == status.HTTP_200_OK + assert response.json()["redirect_url"].endswith("/custom/post/login") + + @patch( + "openedx.core.djangoapps.user_authn.views.login.should_redirect_to_authn_microfrontend", + Mock(return_value=True), + ) + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.authentication.login.alt_redirect_url.requested.v1": { + "pipeline": [ + "openedx.core.djangoapps.user_authn.views.tests.test_filters." + "TestUnsafePostLoginRedirectPipelineStep", + ], + "fail_silently": False, + }, + }, + ) + def test_post_login_redirect_filter_returning_unsafe_url(self): + """ + Test that an off-site redirect URL returned by the filter is discarded. + + Expected result: + - LoginAltRedirectURLRequested is triggered and executes + TestUnsafePostLoginRedirectPipelineStep. + - The off-site URL is rejected and the user is redirected to the default next URL. + """ + data = { + "email": "test@example.com", + "password": "password", + } + + response = self.client.post(self.url, data) + + assert response.status_code == status.HTTP_200_OK + assert "evil.example.com" not in response.json()["redirect_url"] + assert response.json()["redirect_url"].endswith("/dashboard") + + @patch( + "openedx.core.djangoapps.user_authn.views.login.should_redirect_to_authn_microfrontend", + Mock(return_value=True), + ) + @override_settings(OPEN_EDX_FILTERS_CONFIG={}) + def test_post_login_redirect_without_filter_configuration(self): + """ + Test usual post-login redirect, without filter's intervention. + + Expected result: + - LoginAltRedirectURLRequested does not have any effect on the redirect URL. + - The user is redirected to the default next URL. + """ + data = { + "email": "test@example.com", + "password": "password", + } + + response = self.client.post(self.url, data) + + assert response.status_code == status.HTTP_200_OK + assert response.json()["redirect_url"].endswith("/dashboard") + + +@skip_unless_lms +class AuthnMFEContextFiltersTest(UserAPITestCase): + """ + Tests for the Open edX Filters associated with the authentication MFE context. + + This class guarantees that the following filter is triggered while the context served + to the authentication MFE is built: + + - AuthnMFEContextGenerated + """ + + def setUp(self): # pylint: disable=arguments-differ + super().setUp() + self.url = reverse("mfe_context") + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.authentication.mfe.context.generated.v1": { + "pipeline": [ + "openedx.core.djangoapps.user_authn.views.tests.test_filters.TestAuthnMFEContextPipelineStep", + ], + "fail_silently": False, + }, + }, + ) + def test_authn_mfe_context_filter_executed(self): + """ + Test whether the authentication MFE context filter is triggered while the context is + built, and that both of its arguments reach the response. + + Expected result: + - AuthnMFEContextGenerated is triggered and executes TestAuthnMFEContextPipelineStep. + - The declared platformName entry is overridden by the pipeline step. + - The undeclared brandingStrings entry contributed through extra_context is merged + into contextData with its nesting preserved. + """ + response = self.client.get(self.url) + + assert response.status_code == status.HTTP_200_OK + context_data = response.json()["contextData"] + assert context_data["platformName"] == "Filtered Platform Name" + assert context_data["brandingStrings"] == {"welcome": "Filtered Welcome"} + + @override_settings(OPEN_EDX_FILTERS_CONFIG={}) + def test_authn_mfe_context_without_filter_configuration(self): + """ + Test usual authentication MFE context, without filter's intervention. + + Expected result: + - AuthnMFEContextGenerated does not have any effect on the context. + - No entry is added to contextData beyond the serializer's declared fields. + """ + response = self.client.get(self.url) + + assert response.status_code == status.HTTP_200_OK + context_data = response.json()["contextData"] + assert context_data["platformName"] != "Filtered Platform Name" + assert "brandingStrings" not in context_data diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_login.py b/openedx/core/djangoapps/user_authn/views/tests/test_login.py index 433381814a1e..4acb4ee48fd5 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_login.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_login.py @@ -7,7 +7,6 @@ import hashlib import json import unicodedata -import urllib.parse from unittest.mock import Mock, patch import ddt @@ -45,7 +44,6 @@ ) from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms from openedx.core.lib.api.test_utils import ApiTestCase -from openedx.features.enterprise_support.tests.factories import EnterpriseCustomerUserFactory @ddt.ddt @@ -205,113 +203,6 @@ def test_login_success_with_redirect(self, next_url, course_id, expected_redirec self._assert_response(response, success=True) self._assert_redirect_url(response, expected_redirect) - @ddt.data(('/dashboard', False), ('/enterprise/select/active/?success_url=/dashboard', True)) - @ddt.unpack - @override_settings(ENABLE_AUTHN_MICROFRONTEND=True, ENABLE_ENTERPRISE_INTEGRATION=True) - @override_settings(LOGIN_REDIRECT_WHITELIST=['openedx.service']) - @patch('openedx.features.enterprise_support.api.EnterpriseApiClient') - @patch('openedx.core.djangoapps.user_authn.views.login.reverse') - @skip_unless_lms - def test_login_success_for_multiple_enterprises( - self, expected_redirect, user_has_multiple_enterprises, reverse_mock, mock_api_client_class - ): - """ - Test that if multiple enterprise feature is enabled, user is redirected - to correct page - """ - api_response = {'results': []} - enterprise = EnterpriseCustomerUserFactory(user_id=self.user.id).enterprise_customer - api_response['results'].append( - { - "enterprise_customer": { - "uuid": enterprise.uuid, - "name": enterprise.name, - "active": enterprise.active, - } - } - ) - - if user_has_multiple_enterprises: - enterprise = EnterpriseCustomerUserFactory(user_id=self.user.id).enterprise_customer - api_response['results'].append( - { - "enterprise_customer": { - "uuid": enterprise.uuid, - "name": enterprise.name, - "active": enterprise.active, - } - } - ) - - mock_client = mock_api_client_class.return_value - mock_client.fetch_enterprise_learner_data.return_value = api_response - reverse_mock.return_value = '/enterprise/select/active' - - response, _ = self._login_response( - self.user.email, - self.password, - HTTP_ACCEPT='*/*', - ) - self._assert_response(response, success=True) - self._assert_redirect_url(response, settings.LMS_ROOT_URL + expected_redirect) - - @ddt.data(('', True), ('/enterprise/select/active/?success_url=', False)) - @ddt.unpack - @override_settings(ENABLE_AUTHN_MICROFRONTEND=True, ENABLE_ENTERPRISE_INTEGRATION=True) - @patch('openedx.features.enterprise_support.api.EnterpriseApiClient') - @patch('openedx.core.djangoapps.user_authn.views.login.activate_learner_enterprise') - @patch('openedx.core.djangoapps.user_authn.views.login.reverse') - @skip_unless_lms - def test_enterprise_in_url( - self, expected_redirect, is_activated, reverse_mock, mock_activate_learner_enterprise, mock_api_client_class - ): - """ - If user has multiple enterprises and the enterprise is present in url, - activate that url - """ - api_response = {} - enterprise_1 = EnterpriseCustomerUserFactory(user_id=self.user.id).enterprise_customer - enterprise_2 = EnterpriseCustomerUserFactory(user_id=self.user.id).enterprise_customer - api_response['results'] = [ - { - "enterprise_customer": { - "uuid": enterprise_1.uuid, - "name": enterprise_1.name, - "active": enterprise_1.active, - } - }, - { - "enterprise_customer": { - "uuid": enterprise_2.uuid, - "name": enterprise_2.name, - "active": enterprise_2.active, - } - } - ] - - next_url = '/enterprise/{}/course/{}/enroll/?catalog=catalog_uuid&utm_medium=enterprise'.format( - enterprise_1.uuid, - 'course-v1:testX+test101+2T2020' - ) - - mock_client = mock_api_client_class.return_value - mock_client.fetch_enterprise_learner_data.return_value = api_response - mock_activate_learner_enterprise.return_value = is_activated - reverse_mock.return_value = '/enterprise/select/active' - - response, _ = self._login_response( - self.user.email, - self.password, - extra_post_params={'next': next_url}, - HTTP_ACCEPT='*/*', - ) - - if not is_activated: - next_url = urllib.parse.quote(next_url) - - self._assert_response(response, success=True) - self._assert_redirect_url(response, settings.LMS_ROOT_URL + expected_redirect + next_url) - @patch.dict("django.conf.settings.FEATURES", {'SQUELCH_PII_IN_LOGS': True}) def test_login_success_no_pii(self): response, mock_audit_log = self._login_response( diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_logistration.py b/openedx/core/djangoapps/user_authn/views/tests/test_logistration.py index 3bd716b3780e..bb42b71f749c 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_logistration.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_logistration.py @@ -1,34 +1,22 @@ """ Tests for Logistration views. """ - -from http.cookies import SimpleCookie -from unittest import mock from urllib.parse import urlencode import ddt from django.conf import settings -from django.contrib import messages -from django.contrib.auth.models import AnonymousUser -from django.contrib.messages.middleware import MessageMiddleware -from django.contrib.sessions.middleware import SessionMiddleware from django.core.files.uploadedfile import SimpleUploadedFile from django.test import TestCase -from django.test.client import RequestFactory from django.test.utils import override_settings from django.urls import reverse -from django.utils.translation import gettext as _ from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.third_party_auth.tests.testutil import ThirdPartyAuthTestMixin, simulate_running_pipeline from common.djangoapps.util.testing import UrlResetMixin -from lms.djangoapps.branding.api import get_privacy_url from openedx.core.djangoapps.site_configuration.tests.mixins import SiteMixin from openedx.core.djangoapps.theming.tests.test_util import with_comprehensive_theme_context from openedx.core.djangoapps.user_authn.cookies import JWT_COOKIE_NAMES from openedx.core.djangoapps.user_authn.tests.utils import setup_login_oauth_client -from openedx.core.djangoapps.user_authn.views.login_form import login_and_registration_form from openedx.core.djangolib.js_utils import dump_js_escaped_json -from openedx.core.djangolib.markup import HTML, Text from openedx.core.djangolib.testing.utils import skip_unless_lms from xmodule.modulestore.tests.django_utils import ( ModuleStoreTestCase, # pylint: disable=wrong-import-order @@ -190,29 +178,17 @@ def test_login_and_registration_form_signin_not_preserves_params(self, theme, ur @ddt.data("signin_user", "register_user") def test_third_party_auth_disabled(self, url_name): response = self.client.get(reverse(url_name)) - self._assert_third_party_auth_data(response, None, None, [], None) + self._assert_third_party_auth_data(response, None, None, []) - @mock.patch('openedx.core.djangoapps.user_authn.views.login_form.enterprise_customer_for_request') @ddt.data( - ("signin_user", None, None, None, False), - ("register_user", None, None, None, False), - ("signin_user", "google-oauth2", "Google", None, False), - ("register_user", "google-oauth2", "Google", None, False), - ("signin_user", "facebook", "Facebook", None, False), - ("register_user", "facebook", "Facebook", None, False), - ("signin_user", "dummy", "Dummy", None, False), - ("register_user", "dummy", "Dummy", None, False), - ( - "signin_user", - "google-oauth2", - "Google", - { - 'name': 'FakeName', - 'logo': 'https://host.com/logo.jpg', - 'welcome_msg': 'No message' - }, - True - ) + ("signin_user", None, None), + ("register_user", None, None), + ("signin_user", "google-oauth2", "Google"), + ("register_user", "google-oauth2", "Google"), + ("signin_user", "facebook", "Facebook"), + ("register_user", "facebook", "Facebook"), + ("signin_user", "dummy", "Dummy"), + ("register_user", "dummy", "Dummy"), ) @ddt.unpack def test_third_party_auth( @@ -220,9 +196,6 @@ def test_third_party_auth( url_name, current_backend, current_provider, - expected_enterprise_customer_mock_attrs, - add_user_details, - enterprise_customer_mock, ): params = [ ('course_id', 'course-v1:Org+Course+Run'), @@ -232,26 +205,10 @@ def test_third_party_auth( ('next', '/custom/final/destination'), ] - if expected_enterprise_customer_mock_attrs: - expected_ec = { - 'name': expected_enterprise_customer_mock_attrs['name'], - 'branding_configuration': { - 'logo': 'https://host.com/logo.jpg', - 'welcome_message': expected_enterprise_customer_mock_attrs['welcome_msg'] - } - } - else: - expected_ec = None - - email = None - if add_user_details: - email = 'test@test.com' - enterprise_customer_mock.return_value = expected_ec - # Simulate a running pipeline if current_backend is not None: pipeline_target = "openedx.core.djangoapps.user_authn.views.login_form.third_party_auth.pipeline" - with simulate_running_pipeline(pipeline_target, current_backend, email=email): + with simulate_running_pipeline(pipeline_target, current_backend): response = self.client.get(reverse(url_name), params, HTTP_ACCEPT="text/html") # Do NOT simulate a running pipeline @@ -296,91 +253,6 @@ def test_third_party_auth( current_backend, current_provider, expected_providers, - expected_ec, - add_user_details - ) - - def _configure_testshib_provider(self, provider_name, idp_slug): - """ - Enable and configure the TestShib SAML IdP as a third_party_auth provider. - """ - kwargs = {} - kwargs.setdefault('name', provider_name) - kwargs.setdefault('enabled', True) - kwargs.setdefault('visible', True) - kwargs.setdefault('slug', idp_slug) - kwargs.setdefault('entity_id', 'https://idp.testshib.org/idp/shibboleth') - kwargs.setdefault('metadata_source', 'https://mock.testshib.org/metadata/testshib-providers.xml') - kwargs.setdefault('icon_class', 'fa-university') - kwargs.setdefault('attr_email', 'dummy-email-attr') - kwargs.setdefault('max_session_length', None) - kwargs.setdefault('skip_registration_form', False) - self.configure_saml_provider(**kwargs) - - @mock.patch('django.conf.settings.MESSAGE_STORAGE', 'django.contrib.messages.storage.cookie.CookieStorage') - @mock.patch('openedx.core.djangoapps.user_authn.views.login_form.enterprise_customer_for_request') - @ddt.data( - ( - 'signin_user', - 'tpa-saml', - 'TestShib', - ) - ) - @ddt.unpack - def test_saml_auth_with_error( - self, - url_name, - current_backend, - current_provider, - enterprise_customer_mock, - ): - params = [] - request = RequestFactory().get(reverse(url_name), params, HTTP_ACCEPT='text/html') - SessionMiddleware(get_response=lambda request: None).process_request(request) - request.user = AnonymousUser() - - self.enable_saml() - dummy_idp = 'testshib' - self._configure_testshib_provider(current_provider, dummy_idp) - enterprise_customer_data = { - 'uuid': '72416e52-8c77-4860-9584-15e5b06220fb', - 'name': 'Dummy Enterprise', - 'identity_provider': dummy_idp, - } - enterprise_customer_mock.return_value = enterprise_customer_data - dummy_error_message = 'Authentication failed: SAML login failed ' \ - '["invalid_response"] [SAML Response must contain 1 assertion]' - - # Add error message for error in auth pipeline - MessageMiddleware(get_response=lambda request: None).process_request(request) - messages.error(request, dummy_error_message, extra_tags='social-auth') - - # Simulate a running pipeline - pipeline_response = { - 'response': { - 'idp_name': dummy_idp - } - } - pipeline_target = 'openedx.core.djangoapps.user_authn.views.login_form.third_party_auth.pipeline' - with simulate_running_pipeline(pipeline_target, current_backend, **pipeline_response): - with mock.patch('common.djangoapps.edxmako.request_context.get_current_request', return_value=request): - response = login_and_registration_form(request) - - expected_error_message = Text(_( - 'We are sorry, you are not authorized to access {platform_name} via this channel. ' - 'Please contact your learning administrator or manager in order to access {platform_name}.' - '{line_break}{line_break}' - 'Error Details:{line_break}{error_message}') - ).format( - platform_name=settings.PLATFORM_NAME, - error_message=dummy_error_message, - line_break=HTML('
') - ) - self._assert_saml_auth_data_with_error( - response, - current_backend, - current_provider, - expected_error_message ) def test_hinted_login(self): @@ -462,79 +334,6 @@ def test_settings_tpa_hinted_login_dialog_disabled(self, url_name, auth_entry): target_status_code=302 ) - @mock.patch('openedx.core.djangoapps.user_authn.views.login_form.enterprise_customer_for_request') - @ddt.data( - ('signin_user', False, None, None, False), - ('register_user', False, None, None, False), - ('signin_user', True, 'Fake EC', 'http://logo.com/logo.jpg', False), - ('register_user', True, 'Fake EC', 'http://logo.com/logo.jpg', False), - ('signin_user', True, 'Fake EC', 'http://logo.com/logo.jpg', True), - ('register_user', True, 'Fake EC', 'http://logo.com/logo.jpg', True), - ('signin_user', True, 'Fake EC', None, False), - ('register_user', True, 'Fake EC', None, False), - ) - @ddt.unpack - def test_enterprise_register(self, url_name, ec_present, ec_name, logo_url, is_proxy, mock_get_ec): - """ - Verify that when an EnterpriseCustomer is received on the login and register views, - the appropriate sidebar is rendered. - """ - if ec_present: - mock_get_ec.return_value = { - 'name': ec_name, - 'branding_configuration': {'logo': logo_url} - } - else: - mock_get_ec.return_value = None - - params = [] - if is_proxy: - params.append(("proxy_login", "True")) - - response = self.client.get(reverse(url_name), params, HTTP_ACCEPT="text/html") - - enterprise_sidebar_div_id = 'enterprise-content-container' - - if not ec_present: - self.assertNotContains(response, text=enterprise_sidebar_div_id) - else: - self.assertContains(response, text=enterprise_sidebar_div_id) - if is_proxy: - welcome_message = settings.ENTERPRISE_PROXY_LOGIN_WELCOME_TEMPLATE - else: - welcome_message = settings.ENTERPRISE_SPECIFIC_BRANDED_WELCOME_TEMPLATE - expected_message = Text(welcome_message).format( - start_bold=HTML(''), - end_bold=HTML(''), - line_break=HTML('
'), - enterprise_name=ec_name, - platform_name=settings.PLATFORM_NAME, - privacy_policy_link_start=HTML("").format( - pp_url=get_privacy_url() - ), - privacy_policy_link_end=HTML(""), - ) - self.assertContains(response, expected_message) - if logo_url: - self.assertContains(response, logo_url) - - def test_enterprise_cookie_delete(self): - """ - Test that enterprise cookies are deleted in login/registration views. - - Cookies must be deleted in login/registration views so that *default* login/registration branding - is displayed to subsequent requests from non-enterprise customers. - """ - cookies = SimpleCookie() - cookies[settings.ENTERPRISE_CUSTOMER_COOKIE_NAME] = 'test-enterprise-customer' - response = self.client.get(reverse('signin_user'), HTTP_ACCEPT="text/html", cookies=cookies) - - assert settings.ENTERPRISE_CUSTOMER_COOKIE_NAME in response.cookies - enterprise_cookie = response.cookies[settings.ENTERPRISE_CUSTOMER_COOKIE_NAME] - - assert enterprise_cookie['domain'] == settings.BASE_COOKIE_DOMAIN - assert enterprise_cookie.value == '' - def test_login_registration_xframe_protected(self): resp = self.client.get( reverse("register_user"), @@ -553,8 +352,7 @@ def test_login_registration_xframe_protected(self): assert resp['X-Frame-Options'] == 'ALLOW' - def _assert_third_party_auth_data(self, response, current_backend, current_provider, providers, expected_ec, - add_user_details=False): + def _assert_third_party_auth_data(self, response, current_backend, current_provider, providers): """Verify that third party auth info is rendered correctly in a DOM data attribute. """ finish_auth_url = None if current_backend: @@ -568,41 +366,9 @@ def _assert_third_party_auth_data(self, response, current_backend, current_provi "errorMessage": None, "registerFormSubmitButtonText": "Create Account", "syncLearnerProfileData": False, - "pipeline_user_details": {"email": "test@test.com"} if add_user_details else {}, + "pipeline_user_details": {}, "skipRegistrationOptionalCheckboxes": False } - if expected_ec is not None: - # If we set an EnterpriseCustomer, third-party auth providers ought to be hidden. - auth_info['providers'] = [] - auth_info = dump_js_escaped_json(auth_info) - - expected_data = '"third_party_auth": {auth_info}'.format( # noqa: UP032 - auth_info=auth_info - ) - self.assertContains(response, expected_data) - - def _assert_saml_auth_data_with_error( - self, response, current_backend, current_provider, expected_error_message - ): - """ - Verify that third party auth info is rendered correctly in a DOM data attribute. - """ - finish_auth_url = None - if current_backend: - finish_auth_url = reverse('social:complete', kwargs={'backend': current_backend}) + '?' - - auth_info = { - 'currentProvider': current_provider, - 'platformName': settings.PLATFORM_NAME, - 'providers': [], - 'secondaryProviders': [], - 'finishAuthUrl': finish_auth_url, - 'errorMessage': expected_error_message, - 'registerFormSubmitButtonText': 'Create Account', - 'syncLearnerProfileData': False, - 'pipeline_user_details': {'response': {'idp_name': 'testshib'}}, - 'skipRegistrationOptionalCheckboxes': False - } auth_info = dump_js_escaped_json(auth_info) expected_data = '"third_party_auth": {auth_info}'.format( # noqa: UP032 diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_utils.py b/openedx/core/djangoapps/user_authn/views/tests/test_utils.py index e9fb7f71a81c..a5cc3a7255b6 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_utils.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_utils.py @@ -7,14 +7,18 @@ import ddt from django.db.models import Model -from django.test import TestCase +from django.test import RequestFactory, TestCase from django.test.utils import override_settings from openedx.core.djangoapps.user_authn.views.registration_form import ( get_extended_profile_model, get_registration_extension_form, ) -from openedx.core.djangoapps.user_authn.views.utils import _get_username_prefix, get_auto_generated_username +from openedx.core.djangoapps.user_authn.views.utils import ( + _get_username_prefix, + get_auto_generated_username, + get_running_third_party_auth_state, +) @ddt.ddt @@ -323,3 +327,47 @@ def test_get_registration_extension_form_malformed_path(self, mock_logger: Mock) error_calls = mock_logger.error.call_args_list self.assertGreater(len(error_calls), 0, "Expected an error to be logged") # noqa: PT009 + + +@ddt.ddt +class TestGetRunningThirdPartyAuthState(TestCase): + """ + Test case for the get_running_third_party_auth_state function. + """ + + @patch('openedx.core.djangoapps.user_authn.views.utils.third_party_auth') + def test_third_party_auth_disabled(self, mock_third_party_auth): + """ + No pipeline or provider is returned when third party auth is disabled. + """ + mock_third_party_auth.is_enabled.return_value = False + + assert get_running_third_party_auth_state(RequestFactory().get('/')) == (None, None) + + @patch('openedx.core.djangoapps.user_authn.views.utils.pipeline') + @patch('openedx.core.djangoapps.user_authn.views.utils.third_party_auth') + def test_no_running_pipeline(self, mock_third_party_auth, mock_pipeline): + """ + No pipeline or provider is returned when no pipeline is running for the request. + """ + mock_third_party_auth.is_enabled.return_value = True + mock_pipeline.get.return_value = None + + assert get_running_third_party_auth_state(RequestFactory().get('/')) == (None, None) + mock_third_party_auth.provider.Registry.get_from_pipeline.assert_not_called() + + @patch('openedx.core.djangoapps.user_authn.views.utils.pipeline') + @patch('openedx.core.djangoapps.user_authn.views.utils.third_party_auth') + def test_running_pipeline_and_provider_returned(self, mock_third_party_auth, mock_pipeline): + """ + The running pipeline and the provider it is using are both returned. + """ + running_pipeline = {'kwargs': {'details': {'email': 'learner@example.com'}}} + mock_third_party_auth.is_enabled.return_value = True + mock_pipeline.get.return_value = running_pipeline + current_provider = mock_third_party_auth.provider.Registry.get_from_pipeline.return_value + + assert get_running_third_party_auth_state(RequestFactory().get('/')) == ( + running_pipeline, current_provider, + ) + mock_third_party_auth.provider.Registry.get_from_pipeline.assert_called_once_with(running_pipeline) diff --git a/openedx/core/djangoapps/user_authn/views/utils.py b/openedx/core/djangoapps/user_authn/views/utils.py index 4d852114e2a6..0bc654deb3f5 100644 --- a/openedx/core/djangoapps/user_authn/views/utils.py +++ b/openedx/core/djangoapps/user_authn/views/utils.py @@ -10,21 +10,22 @@ from django.conf import settings from django.contrib import messages +from django.http import HttpRequest from django.utils.translation import gettext as _ from ipware.ip import get_client_ip +from openedx_filters.authentication.filters import AuthnMFEContextGenerated +from openedx_filters.authentication.types import RunningPipeline from text_unidecode import unidecode from common.djangoapps import third_party_auth from common.djangoapps.third_party_auth import pipeline -from common.djangoapps.third_party_auth.models import clean_username +from common.djangoapps.third_party_auth.models import ProviderConfig, clean_username from openedx.core.djangoapps.embargo.models import GlobalRestrictedCountry from openedx.core.djangoapps.geoinfo.api import country_code_from_ip from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers log = logging.getLogger(__name__) API_V1 = 'v1' -UUID4_REGEX = '[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}' -ENTERPRISE_ENROLLMENT_URL_REGEX = fr'/enterprise/{UUID4_REGEX}/course/{settings.COURSE_KEY_REGEX}/enroll' def third_party_auth_context(request, redirect_to, tpa_hint=None): @@ -118,17 +119,52 @@ def third_party_auth_context(request, redirect_to, tpa_hint=None): return context -def get_mfe_context(request, redirect_to, tpa_hint=None): +def get_running_third_party_auth_state( + request: HttpRequest, +) -> tuple[RunningPipeline | None, ProviderConfig | None]: """ - Returns Authn MFE context. + Return the third-party auth pipeline running for the request, and its provider. + + Arguments: + request (HttpRequest): The request to inspect for a running pipeline. + + Returns: + tuple[RunningPipeline, ProviderConfig]: the running pipeline and its provider. + + Both are None when third party auth is disabled or when no pipeline is running for + the request. When a pipeline *is* running but its provider could not be determined, + the pipeline is returned alongside a None provider. The invariant is therefore + one-directional: a non-None provider implies a non-None pipeline, but not the + reverse. """ + if not third_party_auth.is_enabled(): + return None, None + + running_pipeline = pipeline.get(request) + if not running_pipeline: + return None, None + return running_pipeline, third_party_auth.provider.Registry.get_from_pipeline(running_pipeline) + + +def get_mfe_context(request, redirect_to, tpa_hint=None): + """Return Authn MFE context including country code and any plugin-provided data.""" ip_address = get_client_ip(request)[0] country_code = country_code_from_ip(ip_address) context = third_party_auth_context(request, redirect_to, tpa_hint) + context.update({ 'countryCode': country_code, }) + + # .. filter_implemented_name: AuthnMFEContextGenerated + # .. filter_type: org.openedx.authentication.mfe.context.generated.v1 + context, extra_context = AuthnMFEContextGenerated.run_filter(context=context, extra_context={}) + + # Entries the pipeline contributed that ContextDataSerializer does not declare fields for. + # The serializer merges them into the response alongside the declared fields. + context['extra_context'] = extra_context + return context diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 7a365e18aa74..dda3d8e33996 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -44,7 +44,7 @@ django-stubs<6 # The team that owns this package will manually bump this package rather than having it pulled in automatically. # This is to allow them to better control its deployment and to do it in a process that works better # for them. -edx-enterprise==8.7.2 +edx-enterprise==8.8.0 # Date: 2023-07-26 # Our legacy Sass code is incompatible with anything except this ancient libsass version. diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index ec833fe134c4..a02051a1943b 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -484,7 +484,7 @@ edx-drf-extensions==10.6.0 # enterprise-integrated-channels # openedx-authz # openedx-core -edx-enterprise==8.7.2 +edx-enterprise==8.8.0 # via # -c requirements/constraints.txt # -r requirements/edx/kernel.in @@ -862,7 +862,7 @@ openedx-events==11.2.0 # openedx-authz # openedx-core # ora2 -openedx-filters==3.8.0 +openedx-filters==3.9.0 # via # -r requirements/edx/kernel.in # edx-enterprise diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index bf0ff461d660..12d2fa586a07 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -757,7 +757,7 @@ edx-drf-extensions==10.6.0 # enterprise-integrated-channels # openedx-authz # openedx-core -edx-enterprise==8.7.2 +edx-enterprise==8.8.0 # via # -c requirements/constraints.txt # -r requirements/edx/doc.txt @@ -1415,7 +1415,7 @@ openedx-events==11.2.0 # openedx-authz # openedx-core # ora2 -openedx-filters==3.8.0 +openedx-filters==3.9.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index eb09a0572ad9..9bd0bfdd7a16 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -574,7 +574,7 @@ edx-drf-extensions==10.6.0 # enterprise-integrated-channels # openedx-authz # openedx-core -edx-enterprise==8.7.2 +edx-enterprise==8.8.0 # via # -c requirements/constraints.txt # -r requirements/edx/base.txt @@ -1043,7 +1043,7 @@ openedx-events==11.2.0 # openedx-authz # openedx-core # ora2 -openedx-filters==3.8.0 +openedx-filters==3.9.0 # via # -r requirements/edx/base.txt # edx-enterprise diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 96e2df3e966e..ef101a0fdbfb 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -590,7 +590,7 @@ edx-drf-extensions==10.6.0 # enterprise-integrated-channels # openedx-authz # openedx-core -edx-enterprise==8.7.2 +edx-enterprise==8.8.0 # via # -c requirements/constraints.txt # -r requirements/edx/base.txt @@ -1082,7 +1082,7 @@ openedx-events==11.2.0 # openedx-authz # openedx-core # ora2 -openedx-filters==3.8.0 +openedx-filters==3.9.0 # via # -r requirements/edx/base.txt # edx-enterprise