From 0a05dc292bc00a850cd00a6f09e22f10cbe32608 Mon Sep 17 00:00:00 2001 From: leoaulasneo98 Date: Tue, 11 Mar 2025 12:27:51 -0400 Subject: [PATCH] Saml redirect mfe (#36197) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: Redirect non-enterprise SAML to authn MFE The original request was that enterprise users with tpa hint and SAML should not be redirected to MFE. The current condition also excludes regular non-enterprise users with SAML authentication from the MFE. * test: Add test for enterprise SAML authentication MFE redirection logic This test validates the conditional redirection to the authentication microfrontend (MFE) for enterprise and SAML authentication scenarios. The test covers different combinations of: - Enterprise customer presence - Third-party authentication provider - SAML provider status - Redirection setting Ensures that enterprise customers with SAML providers are not redirected to the authentication MFE, while other scenarios follow the standard redirection rules. * fix: change spaced between line codes in test_logistration.py --------- Co-authored-by: Andrés González --- .../djangoapps/user_authn/views/login_form.py | 13 +--- .../views/tests/test_logistration.py | 69 +++++++++++++++++++ 2 files changed, 72 insertions(+), 10 deletions(-) diff --git a/openedx/core/djangoapps/user_authn/views/login_form.py b/openedx/core/djangoapps/user_authn/views/login_form.py index bb78a9df1a..3729c40868 100644 --- a/openedx/core/djangoapps/user_authn/views/login_form.py +++ b/openedx/core/djangoapps/user_authn/views/login_form.py @@ -187,12 +187,7 @@ def login_and_registration_form(request, initial_mode="login"): 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. + # except if user is an enterprise user with tpa_hint_provider coming from a SAML IDP. saml_provider = False running_pipeline = pipeline.get(request) if running_pipeline: @@ -202,10 +197,8 @@ def login_and_registration_form(request, initial_mode="login"): 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: + if should_redirect_to_authn_microfrontend() and not \ + (enterprise_customer and tpa_hint_provider and 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 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 3220cd5139..c187e7e6c0 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_logistration.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_logistration.py @@ -648,6 +648,75 @@ class LoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMixin, ModuleSto assert response['Content-Language'] == 'es-es' + @ddt.data( + (None, None, None, True), + ({'name': 'Test Enterprise', 'uuid': 'test-uuid'}, None, None, True), + ({'name': 'Test Enterprise', 'uuid': 'test-uuid'}, 'test-provider', None, True), + ({'name': 'Test Enterprise', 'uuid': 'test-uuid'}, 'test-provider', True, False) + ) + @ddt.unpack + @override_settings(FEATURES=FEATURES_WITH_AUTHN_MFE_ENABLED) + def test_enterprise_saml_redirection(self, enterprise_customer_data, provider_id, is_saml, should_redirect): + """ + Test that authentication MFE redirection respects the enterprise + SAML provider conditions. + In particular, verify that if we have an enterprise customer with a SAML-based tpa_hint_provider, + we do NOT redirect to the MFE, but handle the request in LMS. All other combinations should + redirect to the MFE when it's enabled. + """ + if provider_id and is_saml: + self.enable_saml() + self._configure_testshib_provider('TestShib', provider_id) + + with ( + mock.patch( + 'openedx.core.djangoapps.user_authn.views.login_form.enterprise_customer_for_request' + ) as mock_ec, + mock.patch( + 'openedx.core.djangoapps.user_authn.views.login_form.should_redirect_to_authn_microfrontend' + ) as mock_should_redirect, + mock.patch( + 'openedx.core.djangoapps.user_authn.views.login_form.third_party_auth.utils.is_saml_provider' + ) as mock_is_saml + ): + mock_ec.return_value = enterprise_customer_data + mock_should_redirect.return_value = should_redirect + mock_is_saml.return_value = (True, None) if is_saml else (False, None) + + params = {} + if provider_id: + params['tpa_hint'] = provider_id + + if provider_id and is_saml: + pipeline_target = 'openedx.core.djangoapps.user_authn.views.login_form.third_party_auth.pipeline' + with mock.patch(pipeline_target + '.get') as mock_pipeline: + pipeline_data = { + 'backend': 'tpa-saml', + 'kwargs': { + 'response': { + 'idp_name': provider_id + }, + 'details': { + 'email': 'test@example.com', + 'fullname': 'Test User', + 'username': 'testuser' + } + } + } + mock_pipeline.return_value = pipeline_data + response = self.client.get(reverse('signin_user'), params) + else: + response = self.client.get(reverse('signin_user'), params) + + if should_redirect: + self.assertRedirects( + response, + settings.AUTHN_MICROFRONTEND_URL + '/login' + + ('?' + urlencode(params) if params else ''), + fetch_redirect_response=False + ) + else: + self.assertEqual(response.status_code, 200) + @skip_unless_lms class AccountCreationTestCaseWithSiteOverrides(SiteMixin, TestCase):