diff --git a/common/djangoapps/third_party_auth/middleware.py b/common/djangoapps/third_party_auth/middleware.py index ecac46050c17..a726f7196b1b 100644 --- a/common/djangoapps/third_party_auth/middleware.py +++ b/common/djangoapps/third_party_auth/middleware.py @@ -8,12 +8,35 @@ from django.utils.deprecation import MiddlewareMixin from django.utils.translation import gettext as _ from requests import HTTPError +from social_core import exceptions as social_exceptions from social_django.middleware import SocialAuthExceptionMiddleware from common.djangoapps.student.helpers import get_next_url_for_login_page from . import pipeline +# Maps each social_core exception class to a stable, language-independent +# error code. This is used by account_settings_redirect_view +# (openedx/core/djangoapps/user_api/legacy_urls.py) to forward third-party-auth +# errors to the Account MFE as a query param, since a plain RedirectView does +# not forward Django messages. The Account MFE currently only renders +# 'duplicate_provider' (see frontend-app-account, AccountSettingsPage.jsx); +# the rest are included for forward-compatibility. +TPA_ERROR_CODES = ( + (social_exceptions.AuthAlreadyAssociated, 'duplicate_provider'), + (social_exceptions.AuthCanceled, 'auth_canceled'), + (social_exceptions.AuthFailed, 'auth_failed'), + (social_exceptions.AuthTokenError, 'token_error'), + (social_exceptions.AuthStateMissing, 'state_missing'), + (social_exceptions.AuthStateForbidden, 'state_forbidden'), + (social_exceptions.AuthTokenRevoked, 'token_revoked'), + (social_exceptions.AuthUnreachableProvider, 'unreachable_provider'), +) + +# Session keys used to pass the error code from the middleware to +# account_settings_redirect_view. +TPA_ERROR_CODE_SESSION_KEY = 'tpa_error_code' +TPA_ERROR_BACKEND_SESSION_KEY = 'tpa_error_backend' class ExceptionMiddleware(SocialAuthExceptionMiddleware, MiddlewareMixin): """Custom middleware that handles conditional redirection.""" @@ -32,8 +55,32 @@ def get_redirect_uri(self, request, exception): if auth_entry and auth_entry in pipeline.AUTH_DISPATCH_URLS: redirect_uri = pipeline.AUTH_DISPATCH_URLS[auth_entry] + # For the account_settings flow, /account/settings is a plain + # RedirectView that does not forward Django messages to the Account + # MFE, so the error would otherwise be silently dropped. Save a + # stable error code (and backend name) in the session here, while we + # still have the real exception instance, so + # account_settings_redirect_view can read it and forward it to the + # MFE as a query param. + if auth_entry == pipeline.AUTH_ENTRY_ACCOUNT_SETTINGS: + self._save_tpa_error_in_session(request, exception) + return redirect_uri + @staticmethod + def _save_tpa_error_in_session(request, exception): + """ + Stores a stable error code for `exception` in the session, along with + the backend name, if `exception` is a recognized third-party-auth + error. No-ops otherwise. + """ + for exc_class, code in TPA_ERROR_CODES: + if isinstance(exception, exc_class): + request.session[TPA_ERROR_CODE_SESSION_KEY] = code + backend = getattr(request, 'backend', None) + request.session[TPA_ERROR_BACKEND_SESSION_KEY] = getattr(backend, 'name', None) + break + def process_exception(self, request, exception): """Handles specific exception raised by Python Social Auth eg HTTPError.""" diff --git a/common/djangoapps/third_party_auth/tests/test_middleware.py b/common/djangoapps/third_party_auth/tests/test_middleware.py index 503907202061..bbe97a930b44 100644 --- a/common/djangoapps/third_party_auth/tests/test_middleware.py +++ b/common/djangoapps/third_party_auth/tests/test_middleware.py @@ -5,13 +5,20 @@ from unittest import mock +import ddt from django.contrib.messages.middleware import MessageMiddleware from django.http import HttpResponse from django.test.client import RequestFactory from requests.exceptions import HTTPError +from social_core import exceptions as social_exceptions from common.djangoapps.student.helpers import get_next_url_for_login_page -from common.djangoapps.third_party_auth.middleware import ExceptionMiddleware +from common.djangoapps.third_party_auth import pipeline +from common.djangoapps.third_party_auth.middleware import ( + TPA_ERROR_BACKEND_SESSION_KEY, + TPA_ERROR_CODE_SESSION_KEY, + ExceptionMiddleware, +) from common.djangoapps.third_party_auth.tests.testutil import TestCase from openedx.core.djangolib.testing.utils import skip_unless_lms @@ -43,3 +50,69 @@ def test_http_exception_redirection(self): assert response.status_code == 302 assert target_url.endswith(login_url) + + +@ddt.ddt +class TPAErrorSessionTestCase(TestCase): + """ + Tests that ExceptionMiddleware.get_redirect_uri() correctly saves a + stable error code in the session for the account_settings flow, so that + account_settings_redirect_view can later forward it to the Account MFE. + """ + + def _build_request(self, exception, auth_entry=pipeline.AUTH_ENTRY_ACCOUNT_SETTINGS): + """ + Build a fake request with session and backend for testing TPA error handling. + """ + request = RequestFactory().get('/auth/login/tpa-saml/') + request.session = {} + request.session[pipeline.AUTH_ENTRY_KEY] = auth_entry + + class FakeBackend: + name = 'tpa-saml' + + request.backend = FakeBackend() + request.social_strategy = mock.MagicMock() + request.social_strategy.setting.return_value = None + + ExceptionMiddleware(get_response=lambda r: None).get_redirect_uri(request, exception) + return request + + @ddt.data( + (social_exceptions.AuthAlreadyAssociated, 'duplicate_provider'), + (social_exceptions.AuthCanceled, 'auth_canceled'), + (social_exceptions.AuthFailed, 'auth_failed'), + (social_exceptions.AuthTokenError, 'token_error'), + (social_exceptions.AuthStateMissing, 'state_missing'), + (social_exceptions.AuthStateForbidden, 'state_forbidden'), + (social_exceptions.AuthTokenRevoked, 'token_revoked'), + (social_exceptions.AuthUnreachableProvider, 'unreachable_provider'), + ) + @ddt.unpack + def test_recognized_exception_saves_error_code_in_session(self, exception_class, expected_code): + """Verify known exceptions store correct error codes in session.""" + request = self._build_request(exception_class('tpa-saml')) + + assert request.session.get(TPA_ERROR_CODE_SESSION_KEY) == expected_code + assert request.session.get(TPA_ERROR_BACKEND_SESSION_KEY) == 'tpa-saml' + + def test_unrecognized_exception_does_not_touch_session(self): + """Verify unknown exceptions do not modify session.""" + request = self._build_request(social_exceptions.InvalidEmail('tpa-saml')) + + assert TPA_ERROR_CODE_SESSION_KEY not in request.session + assert TPA_ERROR_BACKEND_SESSION_KEY not in request.session + + def test_error_outside_account_settings_entry_does_not_touch_session(self): + """ + The session should only be populated for the account_settings flow; + AUTH_DISPATCH_URLS already handles /login and /register correctly + without needing this extra context. + """ + request = self._build_request( + social_exceptions.AuthAlreadyAssociated('tpa-saml'), + auth_entry=pipeline.AUTH_ENTRY_LOGIN, + ) + + assert TPA_ERROR_CODE_SESSION_KEY not in request.session + assert TPA_ERROR_BACKEND_SESSION_KEY not in request.session diff --git a/openedx/core/djangoapps/user_api/legacy_urls.py b/openedx/core/djangoapps/user_api/legacy_urls.py index f2ec67c9cefb..57e76bb98a03 100644 --- a/openedx/core/djangoapps/user_api/legacy_urls.py +++ b/openedx/core/djangoapps/user_api/legacy_urls.py @@ -1,11 +1,18 @@ """ Defines the URL routes for this app. """ +from urllib.parse import urlencode + from django.conf import settings +from django.shortcuts import redirect from django.urls import include, path, re_path -from django.views.generic import RedirectView from rest_framework import routers +from common.djangoapps.third_party_auth import provider +from common.djangoapps.third_party_auth.middleware import ( + TPA_ERROR_BACKEND_SESSION_KEY, + TPA_ERROR_CODE_SESSION_KEY, +) from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from . import views as user_api_views @@ -15,16 +22,43 @@ USER_API_ROUTER.register(r'users', user_api_views.UserViewSet) USER_API_ROUTER.register(r'user_prefs', user_api_views.UserPreferenceViewSet) +def account_settings_redirect_view(request): + """ + Backward-compatible redirect for /account and /account/settings to the + Account MFE. + + Unlike a plain RedirectView, this reads third-party-auth error info from + the session (set by ExceptionMiddleware.get_redirect_uri) and forwards it + to the MFE as query params, instead of dropping it. + + Only 'duplicate_provider' is currently rendered by the MFE + (AuthAlreadyAssociated). Other error codes are sent via 'tpa_error_code' + for forward-compatibility. + """ + account_mfe_url = configuration_helpers.get_value( + 'ACCOUNT_MICROFRONTEND_URL', + settings.ACCOUNT_MICROFRONTEND_URL, + ).rstrip('/') + + error_code = request.session.pop(TPA_ERROR_CODE_SESSION_KEY, None) + backend_name = request.session.pop(TPA_ERROR_BACKEND_SESSION_KEY, None) + + if not error_code: + return redirect(account_mfe_url) + + params = {'tpa_error_code': error_code} + + if error_code == 'duplicate_provider' and backend_name: + enabled_providers = list(provider.Registry.get_enabled_by_backend_name(backend_name)) + params['duplicate_provider'] = enabled_providers[0].name if enabled_providers else backend_name + + return redirect(f'{account_mfe_url}/?{urlencode(params)}') + urlpatterns = [ # This redirect is needed for backward compatibility with the old URL structure for the authentication # workflows using third-party authentication providers until the authentication workflows fully support # the URL structure with MFEs. - re_path(r'^account(?:/settings)?/?$', RedirectView.as_view( - url=configuration_helpers.get_value( - 'ACCOUNT_MICROFRONTEND_URL', - settings.ACCOUNT_MICROFRONTEND_URL, - )), - ), + re_path(r'^account(?:/settings)?/?$', account_settings_redirect_view), path('user_api/v1/', include(USER_API_ROUTER.urls)), re_path( fr'^user_api/v1/preferences/(?P{UserPreference.KEY_REGEX})/users/$', diff --git a/openedx/core/djangoapps/user_api/tests/test_legacy_urls.py b/openedx/core/djangoapps/user_api/tests/test_legacy_urls.py new file mode 100644 index 000000000000..d5a78d6dfbda --- /dev/null +++ b/openedx/core/djangoapps/user_api/tests/test_legacy_urls.py @@ -0,0 +1,73 @@ +""" +Tests for account_settings_redirect_view in legacy_urls.py. +""" +from django.test import RequestFactory, TestCase, override_settings + +from common.djangoapps.third_party_auth.middleware import ( + TPA_ERROR_BACKEND_SESSION_KEY, + TPA_ERROR_CODE_SESSION_KEY, +) +from openedx.core.djangoapps.user_api.legacy_urls import account_settings_redirect_view + + +@override_settings(ACCOUNT_MICROFRONTEND_URL='https://account.example.com') +class AccountSettingsRedirectViewTests(TestCase): + """ + Tests for the view that replaces the legacy /account/settings + RedirectView, so that third-party-auth errors saved in the session by + ExceptionMiddleware are forwarded to the Account MFE as query params. + """ + + def _build_request(self, error_code=None, backend_name=None): + """Build a fake request with optional TPA error session data.""" + request = RequestFactory().get('/account/settings') + request.session = {} + if error_code: + request.session[TPA_ERROR_CODE_SESSION_KEY] = error_code + if backend_name: + request.session[TPA_ERROR_BACKEND_SESSION_KEY] = backend_name + return request + + def test_redirects_without_params_when_no_error_in_session(self): + """Redirect to Account MFE without query params when session has no errors.""" + request = self._build_request() + + response = account_settings_redirect_view(request) + + assert response.status_code == 302 + assert response.url == 'https://account.example.com' + + def test_redirects_with_duplicate_provider_param(self): + """Redirect includes duplicate_provider-specific query param when applicable.""" + request = self._build_request(error_code='duplicate_provider', backend_name='tpa-saml') + + response = account_settings_redirect_view(request) + + assert response.status_code == 302 + assert response.url.startswith('https://account.example.com/?') + assert 'tpa_error_code=duplicate_provider' in response.url + assert 'duplicate_provider=' in response.url + + def test_redirects_with_other_error_code_without_duplicate_provider_param(self): + """ + For error codes other than 'duplicate_provider', only tpa_error_code + should be sent -- 'duplicate_provider' is specific to the + AuthAlreadyAssociated case, which is the only one the Account MFE + currently knows how to render. + """ + request = self._build_request(error_code='auth_canceled', backend_name='tpa-saml') + + response = account_settings_redirect_view(request) + + assert response.status_code == 302 + assert 'tpa_error_code=auth_canceled' in response.url + assert 'duplicate_provider' not in response.url + + def test_session_error_keys_are_consumed(self): + """Ensure TPA error session keys are cleared after redirect.""" + request = self._build_request(error_code='duplicate_provider', backend_name='tpa-saml') + + account_settings_redirect_view(request) + + assert TPA_ERROR_CODE_SESSION_KEY not in request.session + assert TPA_ERROR_BACKEND_SESSION_KEY not in request.session