From 35e42c9810a7f01880c9e153782ea40001741e12 Mon Sep 17 00:00:00 2001 From: Steven Giron Date: Fri, 19 Jun 2026 11:05:34 -0500 Subject: [PATCH 1/7] feat: update tpa config --- .../djangoapps/third_party_auth/middleware.py | 47 ++++++++++++ .../third_party_auth/tests/test_middleware.py | 68 ++++++++++++++++- .../core/djangoapps/user_api/legacy_urls.py | 48 ++++++++++-- .../user_api/tests/test_legacy_urls.py | 73 +++++++++++++++++++ 4 files changed, 228 insertions(+), 8 deletions(-) create mode 100644 openedx/core/djangoapps/user_api/tests/test_legacy_urls.py 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..ef61eebf815c 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,62 @@ 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): + 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): + 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): + 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..6be70a9f1fdb --- /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): + 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): + 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): + 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): + """ + The error code and backend name should be popped from the session + so a stale error doesn't leak into a later, unrelated request. + """ + 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 From 2140ed0a8b5242fb40778f6bd4cbb9438a9c87b8 Mon Sep 17 00:00:00 2001 From: Steven Giron Date: Fri, 19 Jun 2026 11:26:03 -0500 Subject: [PATCH 2/7] fix: silent TPA error messages in MFE account --- .../djangoapps/third_party_auth/tests/test_middleware.py | 7 +++++++ .../core/djangoapps/user_api/tests/test_legacy_urls.py | 8 ++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/common/djangoapps/third_party_auth/tests/test_middleware.py b/common/djangoapps/third_party_auth/tests/test_middleware.py index ef61eebf815c..bbe97a930b44 100644 --- a/common/djangoapps/third_party_auth/tests/test_middleware.py +++ b/common/djangoapps/third_party_auth/tests/test_middleware.py @@ -51,6 +51,7 @@ def test_http_exception_redirection(self): assert response.status_code == 302 assert target_url.endswith(login_url) + @ddt.ddt class TPAErrorSessionTestCase(TestCase): """ @@ -60,12 +61,16 @@ class TPAErrorSessionTestCase(TestCase): """ 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 @@ -85,12 +90,14 @@ class FakeBackend: ) @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 diff --git a/openedx/core/djangoapps/user_api/tests/test_legacy_urls.py b/openedx/core/djangoapps/user_api/tests/test_legacy_urls.py index 6be70a9f1fdb..d5a78d6dfbda 100644 --- a/openedx/core/djangoapps/user_api/tests/test_legacy_urls.py +++ b/openedx/core/djangoapps/user_api/tests/test_legacy_urls.py @@ -19,6 +19,7 @@ class AccountSettingsRedirectViewTests(TestCase): """ 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: @@ -28,6 +29,7 @@ def _build_request(self, error_code=None, backend_name=None): 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) @@ -36,6 +38,7 @@ def test_redirects_without_params_when_no_error_in_session(self): 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) @@ -61,10 +64,7 @@ def test_redirects_with_other_error_code_without_duplicate_provider_param(self): assert 'duplicate_provider' not in response.url def test_session_error_keys_are_consumed(self): - """ - The error code and backend name should be popped from the session - so a stale error doesn't leak into a later, unrelated request. - """ + """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) From 252f9460cbf35d9fc8ba1c17f0228e02f49940de Mon Sep 17 00:00:00 2001 From: Steven Giron Date: Thu, 25 Jun 2026 09:19:52 -0500 Subject: [PATCH 3/7] feat: pass provider name as dynamic error_code param in account redirect --- openedx/core/djangoapps/user_api/legacy_urls.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/openedx/core/djangoapps/user_api/legacy_urls.py b/openedx/core/djangoapps/user_api/legacy_urls.py index 57e76bb98a03..ddad608185fb 100644 --- a/openedx/core/djangoapps/user_api/legacy_urls.py +++ b/openedx/core/djangoapps/user_api/legacy_urls.py @@ -48,9 +48,10 @@ def account_settings_redirect_view(request): params = {'tpa_error_code': error_code} - if error_code == 'duplicate_provider' and backend_name: + if 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 + provider_name = enabled_providers[0].name if enabled_providers else backend_name + params[error_code] = provider_name return redirect(f'{account_mfe_url}/?{urlencode(params)}') From 181e2d5232f812615d417d0c74fd14a792ddf952 Mon Sep 17 00:00:00 2001 From: Steven Giron Date: Wed, 22 Jul 2026 10:34:15 -0500 Subject: [PATCH 4/7] feat: surface TPA errors to Account MFE via endpoint instead of query param --- .../djangoapps/third_party_auth/middleware.py | 47 --------- .../third_party_auth/tests/test_middleware.py | 95 ++++++++++--------- .../core/djangoapps/user_api/legacy_urls.py | 37 ++------ .../user_api/tests/test_legacy_urls.py | 68 ++++--------- .../djangoapps/user_api/tests/test_views.py | 77 +++++++++++++++ openedx/core/djangoapps/user_api/urls.py | 5 + openedx/core/djangoapps/user_api/views.py | 29 ++++++ 7 files changed, 188 insertions(+), 170 deletions(-) diff --git a/common/djangoapps/third_party_auth/middleware.py b/common/djangoapps/third_party_auth/middleware.py index a726f7196b1b..ecac46050c17 100644 --- a/common/djangoapps/third_party_auth/middleware.py +++ b/common/djangoapps/third_party_auth/middleware.py @@ -8,35 +8,12 @@ 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.""" @@ -55,32 +32,8 @@ 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 bbe97a930b44..73f2349166a4 100644 --- a/common/djangoapps/third_party_auth/tests/test_middleware.py +++ b/common/djangoapps/third_party_auth/tests/test_middleware.py @@ -14,11 +14,7 @@ from common.djangoapps.student.helpers import get_next_url_for_login_page 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.middleware import ExceptionMiddleware from common.djangoapps.third_party_auth.tests.testutil import TestCase from openedx.core.djangolib.testing.utils import skip_unless_lms @@ -53,17 +49,20 @@ def test_http_exception_redirection(self): @ddt.ddt -class TPAErrorSessionTestCase(TestCase): +class ExceptionMiddlewareAccountSettingsDispatchTestCase(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. + Tests that ExceptionMiddleware.get_redirect_uri() dispatches to the URL + registered in AUTH_DISPATCH_URLS for the current auth_entry, and that it + no longer needs to duplicate the error message in a custom session key: + SocialAuthExceptionMiddleware.process_exception (the parent class) + already leaves it in the Django messages framework for any + SocialAuthBaseException, tagged 'social-auth '. The + Account MFE reads that message via + openedx.core.djangoapps.user_api.views.ThirdPartyAuthErrorMessageView. """ - 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. - """ + def _build_request(self, 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 @@ -74,45 +73,53 @@ class FakeBackend: 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'), + social_exceptions.AuthAlreadyAssociated, + social_exceptions.AuthCanceled, + social_exceptions.AuthFailed, + social_exceptions.AuthTokenError, + social_exceptions.AuthStateMissing, + social_exceptions.AuthStateForbidden, + social_exceptions.AuthTokenRevoked, + social_exceptions.AuthUnreachableProvider, + social_exceptions.InvalidEmail, ) - @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')) + def test_dispatches_to_account_settings_url_for_any_tpa_exception(self, exception_class): + """The redirect target is /account/settings for the account_settings flow, regardless of exception type.""" + request = self._build_request() - assert request.session.get(TPA_ERROR_CODE_SESSION_KEY) == expected_code - assert request.session.get(TPA_ERROR_BACKEND_SESSION_KEY) == 'tpa-saml' + redirect_uri = ExceptionMiddleware(get_response=lambda r: None).get_redirect_uri( + request, exception_class('tpa-saml') + ) + + assert redirect_uri == pipeline.AUTH_DISPATCH_URLS[pipeline.AUTH_ENTRY_ACCOUNT_SETTINGS] - 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')) + def test_dispatches_elsewhere_outside_account_settings_entry(self): + """AUTH_DISPATCH_URLS is keyed by auth_entry, not by exception type.""" + request = self._build_request(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 + redirect_uri = ExceptionMiddleware(get_response=lambda r: None).get_redirect_uri( + request, social_exceptions.AuthAlreadyAssociated('tpa-saml') + ) - def test_error_outside_account_settings_entry_does_not_touch_session(self): + assert redirect_uri == pipeline.AUTH_DISPATCH_URLS[pipeline.AUTH_ENTRY_LOGIN] + + @skip_unless_lms + def test_process_exception_leaves_social_auth_tagged_message_for_mfe(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. + End to end: process_exception (the parent implementation) must still + queue a Django message tagged with 'social-auth' for the Account MFE + to read, since we no longer save it ourselves. """ - request = self._build_request( - social_exceptions.AuthAlreadyAssociated('tpa-saml'), - auth_entry=pipeline.AUTH_ENTRY_LOGIN, - ) + request = self._build_request() + MessageMiddleware(get_response=lambda request: None).process_request(request) + exception = social_exceptions.AuthAlreadyAssociated('tpa-saml') + + ExceptionMiddleware(get_response=lambda r: None).process_exception(request, exception) - assert TPA_ERROR_CODE_SESSION_KEY not in request.session - assert TPA_ERROR_BACKEND_SESSION_KEY not in request.session + queued_messages = list(request._messages) # pylint: disable=protected-access + assert len(queued_messages) == 1 + assert queued_messages[0].extra_tags.split() == ['social-auth', 'tpa-saml'] + assert str(queued_messages[0]) == str(exception) diff --git a/openedx/core/djangoapps/user_api/legacy_urls.py b/openedx/core/djangoapps/user_api/legacy_urls.py index ddad608185fb..ba8e979bd678 100644 --- a/openedx/core/djangoapps/user_api/legacy_urls.py +++ b/openedx/core/djangoapps/user_api/legacy_urls.py @@ -1,18 +1,11 @@ """ 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 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 @@ -27,33 +20,19 @@ 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. + Unlike a plain RedirectView, this re-evaluates ACCOUNT_MICROFRONTEND_URL + per request so site-aware configuration is honored. - Only 'duplicate_provider' is currently rendered by the MFE - (AuthAlreadyAssociated). Other error codes are sent via 'tpa_error_code' - for forward-compatibility. + Any third-party-auth error message from the pipeline is *not* forwarded + here as a query param: SocialAuthExceptionMiddleware already leaves it in + the Django messages framework, and the Account MFE reads it directly via + ThirdPartyAuthErrorMessageView (openedx/core/djangoapps/user_api/views.py). """ 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 backend_name: - enabled_providers = list(provider.Registry.get_enabled_by_backend_name(backend_name)) - provider_name = enabled_providers[0].name if enabled_providers else backend_name - params[error_code] = provider_name - - return redirect(f'{account_mfe_url}/?{urlencode(params)}') + ) + return redirect(account_mfe_url) urlpatterns = [ # This redirect is needed for backward compatibility with the old URL structure for the authentication diff --git a/openedx/core/djangoapps/user_api/tests/test_legacy_urls.py b/openedx/core/djangoapps/user_api/tests/test_legacy_urls.py index d5a78d6dfbda..e4b3ca703495 100644 --- a/openedx/core/djangoapps/user_api/tests/test_legacy_urls.py +++ b/openedx/core/djangoapps/user_api/tests/test_legacy_urls.py @@ -1,12 +1,10 @@ """ Tests for account_settings_redirect_view in legacy_urls.py. """ +from unittest import mock + 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 @@ -14,60 +12,30 @@ 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. + RedirectView. + + Any third-party-auth error message is intentionally *not* forwarded here: + it's read by the Account MFE from ThirdPartyAuthErrorMessageView instead + (see openedx/core/djangoapps/user_api/views.py), so this view is a plain, + site-aware redirect. """ - def _build_request(self, error_code=None, backend_name=None): - """Build a fake request with optional TPA error session data.""" + def test_redirects_to_account_mfe(self): + """Redirect to the configured Account MFE URL, with no query params.""" 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') + @override_settings(ACCOUNT_MICROFRONTEND_URL='https://fallback.example.com') + def test_uses_site_configuration_value_over_django_settings(self): + """The Account MFE URL is re-evaluated per request, honoring site configuration.""" + request = RequestFactory().get('/account/settings') - account_settings_redirect_view(request) + with_site_override = 'openedx.core.djangoapps.site_configuration.helpers.get_value' + with mock.patch(with_site_override, return_value='https://site-specific.example.com'): + response = 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 + assert response.url == 'https://site-specific.example.com' diff --git a/openedx/core/djangoapps/user_api/tests/test_views.py b/openedx/core/djangoapps/user_api/tests/test_views.py index 143002b30df6..8c3b3a11dfbb 100644 --- a/openedx/core/djangoapps/user_api/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/tests/test_views.py @@ -2,6 +2,10 @@ import ddt import pytest +from django.contrib import messages as django_messages +from django.contrib.messages.storage.session import SessionStorage +from django.http import HttpResponse +from django.test import RequestFactory from django.test.utils import override_settings from django.urls import reverse from opaque_keys.edx.keys import CourseKey @@ -99,6 +103,79 @@ def test_get_list_empty(self): assert result['results'] == [] +@skip_unless_lms +class ThirdPartyAuthErrorMessageViewTests(ApiTestCase): + """ + Tests for the endpoint the Account MFE polls once on mount to read the + pending third-party-auth error message left in the session by + common.djangoapps.third_party_auth.middleware.ExceptionMiddleware (via + SocialAuthExceptionMiddleware.process_exception). + """ + + URL = '/api/user/v1/accounts/third_party_auth_error/' + TEST_PASSWORD = 'Password1234' + + def setUp(self): + super().setUp() + self.user = UserFactory.create(password=self.TEST_PASSWORD) + self.client.login(username=self.user.username, password=self.TEST_PASSWORD) + + def _queue_session_message(self, text, extra_tags=''): + """Simulate SocialAuthExceptionMiddleware queuing a Django message in the current session.""" + session = self.client.session + request = RequestFactory().get('/') + request.session = session + storage = SessionStorage(request) + storage.add(django_messages.constants.ERROR, text, extra_tags=extra_tags) + storage.update(HttpResponse()) + session.save() + + def test_requires_authentication(self): + """ + Anonymous requests are rejected. DRF returns 403 (not 401) here + because SessionAuthentication.authenticate_header() returns None, so + no WWW-Authenticate challenge is issued. + """ + self.client.logout() + + response = self.client.get(self.URL) + + assert response.status_code == 403 + + def test_returns_null_when_no_pending_message(self): + """No 'social-auth'-tagged message in the session means nothing to show.""" + response = self.client.get(self.URL) + + assert response.status_code == 200 + assert response.json() == {'user_message': None} + + def test_returns_pending_social_auth_message(self): + """A message tagged 'social-auth ...' (as SocialAuthExceptionMiddleware tags it) is surfaced.""" + self._queue_session_message('This account is already in use.', extra_tags='social-auth tpa-saml') + + response = self.client.get(self.URL) + + assert response.status_code == 200 + assert response.json() == {'user_message': 'This account is already in use.'} + + def test_message_is_consumed_on_read(self): + """Matches Django's messages flash semantics: read once, gone on the next read.""" + self._queue_session_message('This account is already in use.', extra_tags='social-auth tpa-saml') + + self.client.get(self.URL) + second_response = self.client.get(self.URL) + + assert second_response.json() == {'user_message': None} + + def test_ignores_messages_without_social_auth_tag(self): + """Unrelated Django messages (e.g. from other flows) are not leaked through this endpoint.""" + self._queue_session_message('Unrelated message', extra_tags='some-other-tag') + + response = self.client.get(self.URL) + + assert response.json() == {'user_message': None} + + class UserApiTestCase(UserAPITestCase): """ Generalized test case class for specific implementations below diff --git a/openedx/core/djangoapps/user_api/urls.py b/openedx/core/djangoapps/user_api/urls.py index 5928a1d4f422..04861ef27b71 100644 --- a/openedx/core/djangoapps/user_api/urls.py +++ b/openedx/core/djangoapps/user_api/urls.py @@ -115,6 +115,11 @@ ProfileImageView.as_view(), name='accounts_profile_image_api' ), + path( + 'v1/accounts/third_party_auth_error/', + user_api_views.ThirdPartyAuthErrorMessageView.as_view(), + name='third_party_auth_error_message' + ), re_path( fr'^v1/accounts/{settings.USERNAME_PATTERN}/deactivate/$', AccountDeactivationView.as_view(), diff --git a/openedx/core/djangoapps/user_api/views.py b/openedx/core/djangoapps/user_api/views.py index 2504c38db9bd..7995d6a600b8 100644 --- a/openedx/core/djangoapps/user_api/views.py +++ b/openedx/core/djangoapps/user_api/views.py @@ -1,5 +1,6 @@ """HTTP end-points for the User API. """ +from django.contrib import messages as django_messages from django.contrib.auth.models import User # pylint: disable=imported-auth-user from django.http import HttpResponse from django.utils.decorators import method_decorator @@ -12,6 +13,7 @@ from rest_framework import generics, status, viewsets from rest_framework.exceptions import ParseError from rest_framework.permissions import IsAuthenticated +from rest_framework.response import Response from rest_framework.views import APIView from openedx.core.djangoapps.django_comment_common.models import Role @@ -161,3 +163,30 @@ class CountryTimeZoneListView(generics.ListAPIView): def get_queryset(self): country_code = self.request.GET.get("country_code", None) return get_country_time_zones(country_code) + + +class ThirdPartyAuthErrorMessageView(APIView): + """ + Surfaces the pending third-party-auth error message, if any, that + social_django's SocialAuthExceptionMiddleware (see + common.djangoapps.third_party_auth.middleware.ExceptionMiddleware) left + in the request's Django messages. + + The Account MFE is a separate single-page app and can't render Django's + session-based messages framework directly, so it calls this endpoint + once on mount instead of receiving the error via redirect query params. + Messages are consumed on read, matching Django's messages flash + semantics: a second call right after the first returns + ``user_message: null``. + """ + + authentication_classes = (SessionAuthenticationAllowInactiveUser,) + permission_classes = (IsAuthenticated,) + + def get(self, request): + user_message = None + for message in django_messages.get_messages(request): + if "social-auth" in (message.extra_tags or "").split(): + user_message = str(message) + break + return Response({"user_message": user_message}) From 8cafd3eb28fe158e6b6fdf658eb9e0b73cf2fb54 Mon Sep 17 00:00:00 2001 From: Steven Giron Date: Wed, 5 Aug 2026 08:20:43 -0500 Subject: [PATCH 5/7] fix: preserve unrelated Django messages in TPA error endpoint Reading django.contrib.messages marks the whole session storage as consumed, not just the message we want. Without re-queuing the rest, any unrelated message queued in the same session (from some other, unrelated flow) was being silently dropped instead of shown wherever it was actually meant to appear. Now only the first social-auth message is consumed; everything else is re-queued. Also add a drf_yasg schema to the endpoint, matching the convention already used elsewhere in this app, and expand test coverage: message preservation, multiple queued social-auth messages, unrecognized exceptions, and the full set of TPA exception types for both the redirect-dispatch and message-queuing paths. --- .../third_party_auth/tests/test_middleware.py | 32 ++++++++++++-- .../user_api/tests/test_legacy_urls.py | 7 +++ .../djangoapps/user_api/tests/test_views.py | 38 ++++++++++++++++ openedx/core/djangoapps/user_api/views.py | 43 ++++++++++++++++++- 4 files changed, 115 insertions(+), 5 deletions(-) diff --git a/common/djangoapps/third_party_auth/tests/test_middleware.py b/common/djangoapps/third_party_auth/tests/test_middleware.py index 73f2349166a4..255a81be9021 100644 --- a/common/djangoapps/third_party_auth/tests/test_middleware.py +++ b/common/djangoapps/third_party_auth/tests/test_middleware.py @@ -107,15 +107,26 @@ def test_dispatches_elsewhere_outside_account_settings_entry(self): assert redirect_uri == pipeline.AUTH_DISPATCH_URLS[pipeline.AUTH_ENTRY_LOGIN] @skip_unless_lms - def test_process_exception_leaves_social_auth_tagged_message_for_mfe(self): + @ddt.data( + social_exceptions.AuthAlreadyAssociated, + social_exceptions.AuthCanceled, + social_exceptions.AuthFailed, + social_exceptions.AuthTokenError, + social_exceptions.AuthStateMissing, + social_exceptions.AuthStateForbidden, + social_exceptions.AuthTokenRevoked, + social_exceptions.AuthUnreachableProvider, + ) + def test_process_exception_leaves_social_auth_tagged_message_for_mfe(self, exception_class): """ End to end: process_exception (the parent implementation) must still queue a Django message tagged with 'social-auth' for the Account MFE - to read, since we no longer save it ourselves. + to read, since we no longer save it ourselves. Covers every + recognized third-party-auth exception, not just AuthAlreadyAssociated. """ request = self._build_request() MessageMiddleware(get_response=lambda request: None).process_request(request) - exception = social_exceptions.AuthAlreadyAssociated('tpa-saml') + exception = exception_class('tpa-saml') ExceptionMiddleware(get_response=lambda r: None).process_exception(request, exception) @@ -123,3 +134,18 @@ def test_process_exception_leaves_social_auth_tagged_message_for_mfe(self): assert len(queued_messages) == 1 assert queued_messages[0].extra_tags.split() == ['social-auth', 'tpa-saml'] assert str(queued_messages[0]) == str(exception) + + @skip_unless_lms + def test_process_exception_does_not_queue_message_for_unrecognized_exception(self): + """ + Non-SocialAuthBaseException errors are untouched by + SocialAuthExceptionMiddleware.process_exception -- confirms we're not + accidentally tagging unrelated exceptions as TPA errors. + """ + request = self._build_request() + MessageMiddleware(get_response=lambda request: None).process_request(request) + + result = ExceptionMiddleware(get_response=lambda r: None).process_exception(request, ValueError('boom')) + + assert result is None + assert list(request._messages) == [] # pylint: disable=protected-access diff --git a/openedx/core/djangoapps/user_api/tests/test_legacy_urls.py b/openedx/core/djangoapps/user_api/tests/test_legacy_urls.py index e4b3ca703495..28aabe256122 100644 --- a/openedx/core/djangoapps/user_api/tests/test_legacy_urls.py +++ b/openedx/core/djangoapps/user_api/tests/test_legacy_urls.py @@ -4,6 +4,7 @@ from unittest import mock from django.test import RequestFactory, TestCase, override_settings +from django.urls import resolve from openedx.core.djangoapps.user_api.legacy_urls import account_settings_redirect_view @@ -39,3 +40,9 @@ def test_uses_site_configuration_value_over_django_settings(self): response = account_settings_redirect_view(request) assert response.url == 'https://site-specific.example.com' + + def test_account_and_account_settings_urls_route_here(self): + """The legacy /account and /account/settings paths (with or without a trailing slash) reach this view.""" + for path in ('/account', '/account/', '/account/settings', '/account/settings/'): + match = resolve(path) + assert match.func == account_settings_redirect_view diff --git a/openedx/core/djangoapps/user_api/tests/test_views.py b/openedx/core/djangoapps/user_api/tests/test_views.py index 8c3b3a11dfbb..4b06ae0c5ef0 100644 --- a/openedx/core/djangoapps/user_api/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/tests/test_views.py @@ -175,6 +175,44 @@ def test_ignores_messages_without_social_auth_tag(self): assert response.json() == {'user_message': None} + def test_preserves_unrelated_message_for_a_later_read(self): + """ + Reading the storage at all marks the whole thing consumed, so an + unrelated message queued in the same session must be explicitly + re-queued -- otherwise it would be silently dropped here instead of + being shown wherever it was actually meant to be displayed. + """ + self._queue_session_message('Unrelated message', extra_tags='some-other-tag') + + response = self.client.get(self.URL) + assert response.json() == {'user_message': None} + + # Read it back through a fresh storage bound to the client's session, + # the same way _queue_session_message wrote it. + session = self.client.session + request = RequestFactory().get('/') + request.session = session + remaining = list(SessionStorage(request)) + assert [str(m) for m in remaining] == ['Unrelated message'] + assert remaining[0].extra_tags == 'some-other-tag' + + def test_only_first_social_auth_message_is_returned_rest_are_preserved(self): + """If somehow two social-auth messages are queued, only the first is returned this call.""" + self._queue_session_message('First error', extra_tags='social-auth tpa-saml') + session = self.client.session + request = RequestFactory().get('/') + request.session = session + storage = SessionStorage(request) + storage.add(django_messages.constants.ERROR, 'Second error', extra_tags='social-auth tpa-saml') + storage.update(HttpResponse()) + session.save() + + first_response = self.client.get(self.URL) + second_response = self.client.get(self.URL) + + assert first_response.json() == {'user_message': 'First error'} + assert second_response.json() == {'user_message': 'Second error'} + class UserApiTestCase(UserAPITestCase): """ diff --git a/openedx/core/djangoapps/user_api/views.py b/openedx/core/djangoapps/user_api/views.py index 7995d6a600b8..343441fc0d67 100644 --- a/openedx/core/djangoapps/user_api/views.py +++ b/openedx/core/djangoapps/user_api/views.py @@ -6,6 +6,8 @@ from django.utils.decorators import method_decorator from django.views.decorators.csrf import ensure_csrf_cookie from django_filters.rest_framework import DjangoFilterBackend +from drf_yasg import openapi +from drf_yasg.utils import swagger_auto_schema from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser from opaque_keys import InvalidKeyError from opaque_keys.edx import locator @@ -165,6 +167,21 @@ def get_queryset(self): return get_country_time_zones(country_code) +third_party_auth_error_message_schema = openapi.Schema( + type=openapi.TYPE_OBJECT, + properties={ + "user_message": openapi.Schema( + type=openapi.TYPE_STRING, + x_nullable=True, + description=( + "Human-readable, translated message describing the pending " + "third-party-auth error, or null if there is none pending." + ), + ), + }, +) + + class ThirdPartyAuthErrorMessageView(APIView): """ Surfaces the pending third-party-auth error message, if any, that @@ -183,10 +200,32 @@ class ThirdPartyAuthErrorMessageView(APIView): authentication_classes = (SessionAuthenticationAllowInactiveUser,) permission_classes = (IsAuthenticated,) + @swagger_auto_schema( + responses={ + status.HTTP_200_OK: third_party_auth_error_message_schema, + status.HTTP_401_UNAUTHORIZED: "", + status.HTTP_403_FORBIDDEN: "", + }, + ) def get(self, request): + """ + GET /api/user/v1/accounts/third_party_auth_error/ + + Returns the pending third-party-auth error message for the current + user, consuming it (it will not be returned again on the next call). + """ user_message = None + other_messages = [] + # Iterating the storage at all marks it fully read, so any messages + # we don't claim here must be explicitly re-queued below -- otherwise + # they'd be silently dropped for whichever view next tries to render + # Django messages (e.g. an unrelated notice queued in the same + # session). for message in django_messages.get_messages(request): - if "social-auth" in (message.extra_tags or "").split(): + if user_message is None and "social-auth" in (message.extra_tags or "").split(): user_message = str(message) - break + else: + other_messages.append(message) + for message in other_messages: + django_messages.add_message(request, message.level, message.message, extra_tags=message.extra_tags) return Response({"user_message": user_message}) From 843bc8566efcc9559f88190cdeb0fd0315209939 Mon Sep 17 00:00:00 2001 From: Steven Giron Date: Wed, 5 Aug 2026 08:45:33 -0500 Subject: [PATCH 6/7] fix: address pylint failures in new tests - use-implicit-booleaness-not-comparison: "list(...) == []" -> "not list(...)" - comparison-with-callable: match.func == view_function needs an explicit disable, same pattern already used elsewhere in this codebase (e.g. common/djangoapps/util/date_utils.py). --- common/djangoapps/third_party_auth/tests/test_middleware.py | 2 +- openedx/core/djangoapps/user_api/tests/test_legacy_urls.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/common/djangoapps/third_party_auth/tests/test_middleware.py b/common/djangoapps/third_party_auth/tests/test_middleware.py index 255a81be9021..b10efd40ea6c 100644 --- a/common/djangoapps/third_party_auth/tests/test_middleware.py +++ b/common/djangoapps/third_party_auth/tests/test_middleware.py @@ -148,4 +148,4 @@ def test_process_exception_does_not_queue_message_for_unrecognized_exception(sel result = ExceptionMiddleware(get_response=lambda r: None).process_exception(request, ValueError('boom')) assert result is None - assert list(request._messages) == [] # pylint: disable=protected-access + assert not list(request._messages) # pylint: disable=protected-access diff --git a/openedx/core/djangoapps/user_api/tests/test_legacy_urls.py b/openedx/core/djangoapps/user_api/tests/test_legacy_urls.py index 28aabe256122..f358b4d5c950 100644 --- a/openedx/core/djangoapps/user_api/tests/test_legacy_urls.py +++ b/openedx/core/djangoapps/user_api/tests/test_legacy_urls.py @@ -45,4 +45,4 @@ def test_account_and_account_settings_urls_route_here(self): """The legacy /account and /account/settings paths (with or without a trailing slash) reach this view.""" for path in ('/account', '/account/', '/account/settings', '/account/settings/'): match = resolve(path) - assert match.func == account_settings_redirect_view + assert match.func == account_settings_redirect_view # pylint: disable=comparison-with-callable From 11b14f971ab716ddc4f4b655144b274ef99f49f5 Mon Sep 17 00:00:00 2001 From: Steven Giron Date: Wed, 5 Aug 2026 09:15:43 -0500 Subject: [PATCH 7/7] fix: limit test account path only to lms --- .../core/djangoapps/user_api/tests/test_legacy_urls.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/user_api/tests/test_legacy_urls.py b/openedx/core/djangoapps/user_api/tests/test_legacy_urls.py index f358b4d5c950..47706670d40f 100644 --- a/openedx/core/djangoapps/user_api/tests/test_legacy_urls.py +++ b/openedx/core/djangoapps/user_api/tests/test_legacy_urls.py @@ -7,6 +7,7 @@ from django.urls import resolve from openedx.core.djangoapps.user_api.legacy_urls import account_settings_redirect_view +from openedx.core.djangolib.testing.utils import skip_unless_lms @override_settings(ACCOUNT_MICROFRONTEND_URL='https://account.example.com') @@ -41,8 +42,13 @@ def test_uses_site_configuration_value_over_django_settings(self): assert response.url == 'https://site-specific.example.com' + @skip_unless_lms def test_account_and_account_settings_urls_route_here(self): - """The legacy /account and /account/settings paths (with or without a trailing slash) reach this view.""" + """ + The legacy /account and /account/settings paths (with or without a + trailing slash) reach this view. LMS-only: legacy_urls.py is wired + into lms/urls.py, not cms/urls.py -- CMS has no account settings page. + """ for path in ('/account', '/account/', '/account/settings', '/account/settings/'): match = resolve(path) assert match.func == account_settings_redirect_view # pylint: disable=comparison-with-callable