Skip to content

Commit e0f026c

Browse files
AhtishamShahidUsamaSadiq
authored andcommitted
Revert "Revert "Saml redirect mfe (#36197)" (#36550)" (#36554)
This reverts commit 447cd79.
1 parent ccaa986 commit e0f026c

File tree

2 files changed

+72
-10
lines changed

2 files changed

+72
-10
lines changed

openedx/core/djangoapps/user_authn/views/login_form.py

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -187,12 +187,7 @@ def login_and_registration_form(request, initial_mode="login"):
187187
log.exception("Unknown tpa_hint provider: %s", ex)
188188

189189
# Redirect to authn MFE if it is enabled
190-
# AND
191-
# user is not an enterprise user
192-
# AND
193-
# tpa_hint_provider is not available
194-
# AND
195-
# user is not coming from a SAML IDP.
190+
# except if user is an enterprise user with tpa_hint_provider coming from a SAML IDP.
196191
saml_provider = False
197192
running_pipeline = pipeline.get(request)
198193
if running_pipeline:
@@ -202,10 +197,8 @@ def login_and_registration_form(request, initial_mode="login"):
202197

203198
enterprise_customer = enterprise_customer_for_request(request)
204199

205-
if should_redirect_to_authn_microfrontend() and \
206-
not enterprise_customer and \
207-
not tpa_hint_provider and \
208-
not saml_provider:
200+
if should_redirect_to_authn_microfrontend() and not \
201+
(enterprise_customer and tpa_hint_provider and saml_provider):
209202

210203
# This is to handle a case where a logged-in cookie is not present but the user is authenticated.
211204
# Note: If we don't handle this learner is redirected to authn MFE and then back to dashboard

openedx/core/djangoapps/user_authn/views/tests/test_logistration.py

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -648,6 +648,75 @@ def test_browser_language_dialent(self):
648648

649649
assert response['Content-Language'] == 'es-es'
650650

651+
@ddt.data(
652+
(None, None, None, True),
653+
({'name': 'Test Enterprise', 'uuid': 'test-uuid'}, None, None, True),
654+
({'name': 'Test Enterprise', 'uuid': 'test-uuid'}, 'test-provider', None, True),
655+
({'name': 'Test Enterprise', 'uuid': 'test-uuid'}, 'test-provider', True, False)
656+
)
657+
@ddt.unpack
658+
@override_settings(FEATURES=FEATURES_WITH_AUTHN_MFE_ENABLED)
659+
def test_enterprise_saml_redirection(self, enterprise_customer_data, provider_id, is_saml, should_redirect):
660+
"""
661+
Test that authentication MFE redirection respects the enterprise + SAML provider conditions.
662+
In particular, verify that if we have an enterprise customer with a SAML-based tpa_hint_provider,
663+
we do NOT redirect to the MFE, but handle the request in LMS. All other combinations should
664+
redirect to the MFE when it's enabled.
665+
"""
666+
if provider_id and is_saml:
667+
self.enable_saml()
668+
self._configure_testshib_provider('TestShib', provider_id)
669+
670+
with (
671+
mock.patch(
672+
'openedx.core.djangoapps.user_authn.views.login_form.enterprise_customer_for_request'
673+
) as mock_ec,
674+
mock.patch(
675+
'openedx.core.djangoapps.user_authn.views.login_form.should_redirect_to_authn_microfrontend'
676+
) as mock_should_redirect,
677+
mock.patch(
678+
'openedx.core.djangoapps.user_authn.views.login_form.third_party_auth.utils.is_saml_provider'
679+
) as mock_is_saml
680+
):
681+
mock_ec.return_value = enterprise_customer_data
682+
mock_should_redirect.return_value = should_redirect
683+
mock_is_saml.return_value = (True, None) if is_saml else (False, None)
684+
685+
params = {}
686+
if provider_id:
687+
params['tpa_hint'] = provider_id
688+
689+
if provider_id and is_saml:
690+
pipeline_target = 'openedx.core.djangoapps.user_authn.views.login_form.third_party_auth.pipeline'
691+
with mock.patch(pipeline_target + '.get') as mock_pipeline:
692+
pipeline_data = {
693+
'backend': 'tpa-saml',
694+
'kwargs': {
695+
'response': {
696+
'idp_name': provider_id
697+
},
698+
'details': {
699+
'email': 'test@example.com',
700+
'fullname': 'Test User',
701+
'username': 'testuser'
702+
}
703+
}
704+
}
705+
mock_pipeline.return_value = pipeline_data
706+
response = self.client.get(reverse('signin_user'), params)
707+
else:
708+
response = self.client.get(reverse('signin_user'), params)
709+
710+
if should_redirect:
711+
self.assertRedirects(
712+
response,
713+
settings.AUTHN_MICROFRONTEND_URL + '/login' +
714+
('?' + urlencode(params) if params else ''),
715+
fetch_redirect_response=False
716+
)
717+
else:
718+
self.assertEqual(response.status_code, 200)
719+
651720

652721
@skip_unless_lms
653722
class AccountCreationTestCaseWithSiteOverrides(SiteMixin, TestCase):

0 commit comments

Comments
 (0)