fix: prevent silent errors during SSO (account settings)

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."""
 

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

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<pref_key>{UserPreference.KEY_REGEX})/users/$',

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