Skip to content

Commit a1a4b00

Browse files
committed
feat: update tpa config
1 parent 3d0f9c0 commit a1a4b00

8 files changed

Lines changed: 232 additions & 12 deletions

File tree

common/djangoapps/third_party_auth/middleware.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,35 @@
88
from django.utils.deprecation import MiddlewareMixin
99
from django.utils.translation import gettext as _
1010
from requests import HTTPError
11+
from social_core import exceptions as social_exceptions
1112
from social_django.middleware import SocialAuthExceptionMiddleware
1213

1314
from common.djangoapps.student.helpers import get_next_url_for_login_page
1415

1516
from . import pipeline
1617

18+
# Maps each social_core exception class to a stable, language-independent
19+
# error code. This is used by account_settings_redirect_view
20+
# (openedx/core/djangoapps/user_api/legacy_urls.py) to forward third-party-auth
21+
# errors to the Account MFE as a query param, since a plain RedirectView does
22+
# not forward Django messages. The Account MFE currently only renders
23+
# 'duplicate_provider' (see frontend-app-account, AccountSettingsPage.jsx);
24+
# the rest are included for forward-compatibility.
25+
TPA_ERROR_CODES = (
26+
(social_exceptions.AuthAlreadyAssociated, 'duplicate_provider'),
27+
(social_exceptions.AuthCanceled, 'auth_canceled'),
28+
(social_exceptions.AuthFailed, 'auth_failed'),
29+
(social_exceptions.AuthTokenError, 'token_error'),
30+
(social_exceptions.AuthStateMissing, 'state_missing'),
31+
(social_exceptions.AuthStateForbidden, 'state_forbidden'),
32+
(social_exceptions.AuthTokenRevoked, 'token_revoked'),
33+
(social_exceptions.AuthUnreachableProvider, 'unreachable_provider'),
34+
)
35+
36+
# Session keys used to pass the error code from the middleware to
37+
# account_settings_redirect_view.
38+
TPA_ERROR_CODE_SESSION_KEY = 'tpa_error_code'
39+
TPA_ERROR_BACKEND_SESSION_KEY = 'tpa_error_backend'
1740

1841
class ExceptionMiddleware(SocialAuthExceptionMiddleware, MiddlewareMixin):
1942
"""Custom middleware that handles conditional redirection."""
@@ -32,8 +55,32 @@ def get_redirect_uri(self, request, exception):
3255
if auth_entry and auth_entry in pipeline.AUTH_DISPATCH_URLS:
3356
redirect_uri = pipeline.AUTH_DISPATCH_URLS[auth_entry]
3457

58+
# For the account_settings flow, /account/settings is a plain
59+
# RedirectView that does not forward Django messages to the Account
60+
# MFE, so the error would otherwise be silently dropped. Save a
61+
# stable error code (and backend name) in the session here, while we
62+
# still have the real exception instance, so
63+
# account_settings_redirect_view can read it and forward it to the
64+
# MFE as a query param.
65+
if auth_entry == pipeline.AUTH_ENTRY_ACCOUNT_SETTINGS:
66+
self._save_tpa_error_in_session(request, exception)
67+
3568
return redirect_uri
3669

70+
@staticmethod
71+
def _save_tpa_error_in_session(request, exception):
72+
"""
73+
Stores a stable error code for `exception` in the session, along with
74+
the backend name, if `exception` is a recognized third-party-auth
75+
error. No-ops otherwise.
76+
"""
77+
for exc_class, code in TPA_ERROR_CODES:
78+
if isinstance(exception, exc_class):
79+
request.session[TPA_ERROR_CODE_SESSION_KEY] = code
80+
backend = getattr(request, 'backend', None)
81+
request.session[TPA_ERROR_BACKEND_SESSION_KEY] = getattr(backend, 'name', None)
82+
break
83+
3784
def process_exception(self, request, exception):
3885
"""Handles specific exception raised by Python Social Auth eg HTTPError."""
3986

common/djangoapps/third_party_auth/tests/test_middleware.py

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,20 @@
55

66
from unittest import mock
77

8+
import ddt
89
from django.contrib.messages.middleware import MessageMiddleware
910
from django.http import HttpResponse
1011
from django.test.client import RequestFactory
1112
from requests.exceptions import HTTPError
13+
from social_core import exceptions as social_exceptions
1214

1315
from common.djangoapps.student.helpers import get_next_url_for_login_page
14-
from common.djangoapps.third_party_auth.middleware import ExceptionMiddleware
16+
from common.djangoapps.third_party_auth import pipeline
17+
from common.djangoapps.third_party_auth.middleware import (
18+
TPA_ERROR_BACKEND_SESSION_KEY,
19+
TPA_ERROR_CODE_SESSION_KEY,
20+
ExceptionMiddleware,
21+
)
1522
from common.djangoapps.third_party_auth.tests.testutil import TestCase
1623
from openedx.core.djangolib.testing.utils import skip_unless_lms
1724

@@ -43,3 +50,62 @@ def test_http_exception_redirection(self):
4350

4451
assert response.status_code == 302
4552
assert target_url.endswith(login_url)
53+
54+
@ddt.ddt
55+
class TPAErrorSessionTestCase(TestCase):
56+
"""
57+
Tests that ExceptionMiddleware.get_redirect_uri() correctly saves a
58+
stable error code in the session for the account_settings flow, so that
59+
account_settings_redirect_view can later forward it to the Account MFE.
60+
"""
61+
62+
def _build_request(self, exception, auth_entry=pipeline.AUTH_ENTRY_ACCOUNT_SETTINGS):
63+
request = RequestFactory().get('/auth/login/tpa-saml/')
64+
request.session = {}
65+
request.session[pipeline.AUTH_ENTRY_KEY] = auth_entry
66+
67+
class FakeBackend:
68+
name = 'tpa-saml'
69+
request.backend = FakeBackend()
70+
request.social_strategy = mock.MagicMock()
71+
request.social_strategy.setting.return_value = None
72+
73+
ExceptionMiddleware(get_response=lambda r: None).get_redirect_uri(request, exception)
74+
return request
75+
76+
@ddt.data(
77+
(social_exceptions.AuthAlreadyAssociated, 'duplicate_provider'),
78+
(social_exceptions.AuthCanceled, 'auth_canceled'),
79+
(social_exceptions.AuthFailed, 'auth_failed'),
80+
(social_exceptions.AuthTokenError, 'token_error'),
81+
(social_exceptions.AuthStateMissing, 'state_missing'),
82+
(social_exceptions.AuthStateForbidden, 'state_forbidden'),
83+
(social_exceptions.AuthTokenRevoked, 'token_revoked'),
84+
(social_exceptions.AuthUnreachableProvider, 'unreachable_provider'),
85+
)
86+
@ddt.unpack
87+
def test_recognized_exception_saves_error_code_in_session(self, exception_class, expected_code):
88+
request = self._build_request(exception_class('tpa-saml'))
89+
90+
assert request.session.get(TPA_ERROR_CODE_SESSION_KEY) == expected_code
91+
assert request.session.get(TPA_ERROR_BACKEND_SESSION_KEY) == 'tpa-saml'
92+
93+
def test_unrecognized_exception_does_not_touch_session(self):
94+
request = self._build_request(social_exceptions.InvalidEmail('tpa-saml'))
95+
96+
assert TPA_ERROR_CODE_SESSION_KEY not in request.session
97+
assert TPA_ERROR_BACKEND_SESSION_KEY not in request.session
98+
99+
def test_error_outside_account_settings_entry_does_not_touch_session(self):
100+
"""
101+
The session should only be populated for the account_settings flow;
102+
AUTH_DISPATCH_URLS already handles /login and /register correctly
103+
without needing this extra context.
104+
"""
105+
request = self._build_request(
106+
social_exceptions.AuthAlreadyAssociated('tpa-saml'),
107+
auth_entry=pipeline.AUTH_ENTRY_LOGIN,
108+
)
109+
110+
assert TPA_ERROR_CODE_SESSION_KEY not in request.session
111+
assert TPA_ERROR_BACKEND_SESSION_KEY not in request.session

openedx/core/djangoapps/user_api/legacy_urls.py

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,18 @@
11
"""
22
Defines the URL routes for this app.
33
"""
4+
from urllib.parse import urlencode
5+
46
from django.conf import settings
7+
from django.shortcuts import redirect
58
from django.urls import include, path, re_path
6-
from django.views.generic import RedirectView
79
from rest_framework import routers
810

11+
from common.djangoapps.third_party_auth import provider
12+
from common.djangoapps.third_party_auth.middleware import (
13+
TPA_ERROR_BACKEND_SESSION_KEY,
14+
TPA_ERROR_CODE_SESSION_KEY,
15+
)
916
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
1017

1118
from . import views as user_api_views
@@ -15,16 +22,43 @@
1522
USER_API_ROUTER.register(r'users', user_api_views.UserViewSet)
1623
USER_API_ROUTER.register(r'user_prefs', user_api_views.UserPreferenceViewSet)
1724

25+
def account_settings_redirect_view(request):
26+
"""
27+
Backward-compatible redirect for /account and /account/settings to the
28+
Account MFE.
29+
30+
Unlike a plain RedirectView, this reads third-party-auth error info from
31+
the session (set by ExceptionMiddleware.get_redirect_uri) and forwards it
32+
to the MFE as query params, instead of dropping it.
33+
34+
Only 'duplicate_provider' is currently rendered by the MFE
35+
(AuthAlreadyAssociated). Other error codes are sent via 'tpa_error_code'
36+
for forward-compatibility.
37+
"""
38+
account_mfe_url = configuration_helpers.get_value(
39+
'ACCOUNT_MICROFRONTEND_URL',
40+
settings.ACCOUNT_MICROFRONTEND_URL,
41+
).rstrip('/')
42+
43+
error_code = request.session.pop(TPA_ERROR_CODE_SESSION_KEY, None)
44+
backend_name = request.session.pop(TPA_ERROR_BACKEND_SESSION_KEY, None)
45+
46+
if not error_code:
47+
return redirect(account_mfe_url)
48+
49+
params = {'tpa_error_code': error_code}
50+
51+
if error_code == 'duplicate_provider' and backend_name:
52+
enabled_providers = list(provider.Registry.get_enabled_by_backend_name(backend_name))
53+
params['duplicate_provider'] = enabled_providers[0].name if enabled_providers else backend_name
54+
55+
return redirect(f'{account_mfe_url}/?{urlencode(params)}')
56+
1857
urlpatterns = [
1958
# This redirect is needed for backward compatibility with the old URL structure for the authentication
2059
# workflows using third-party authentication providers until the authentication workflows fully support
2160
# the URL structure with MFEs.
22-
re_path(r'^account(?:/settings)?/?$', RedirectView.as_view(
23-
url=configuration_helpers.get_value(
24-
'ACCOUNT_MICROFRONTEND_URL',
25-
settings.ACCOUNT_MICROFRONTEND_URL,
26-
)),
27-
),
61+
re_path(r'^account(?:/settings)?/?$', account_settings_redirect_view),
2862
path('user_api/v1/', include(USER_API_ROUTER.urls)),
2963
re_path(
3064
fr'^user_api/v1/preferences/(?P<pref_key>{UserPreference.KEY_REGEX})/users/$',
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
"""
2+
Tests for account_settings_redirect_view in legacy_urls.py.
3+
"""
4+
from django.test import RequestFactory, TestCase, override_settings
5+
6+
from common.djangoapps.third_party_auth.middleware import (
7+
TPA_ERROR_BACKEND_SESSION_KEY,
8+
TPA_ERROR_CODE_SESSION_KEY,
9+
)
10+
from openedx.core.djangoapps.user_api.legacy_urls import account_settings_redirect_view
11+
12+
13+
@override_settings(ACCOUNT_MICROFRONTEND_URL='https://account.example.com')
14+
class AccountSettingsRedirectViewTests(TestCase):
15+
"""
16+
Tests for the view that replaces the legacy /account/settings
17+
RedirectView, so that third-party-auth errors saved in the session by
18+
ExceptionMiddleware are forwarded to the Account MFE as query params.
19+
"""
20+
21+
def _build_request(self, error_code=None, backend_name=None):
22+
request = RequestFactory().get('/account/settings')
23+
request.session = {}
24+
if error_code:
25+
request.session[TPA_ERROR_CODE_SESSION_KEY] = error_code
26+
if backend_name:
27+
request.session[TPA_ERROR_BACKEND_SESSION_KEY] = backend_name
28+
return request
29+
30+
def test_redirects_without_params_when_no_error_in_session(self):
31+
request = self._build_request()
32+
33+
response = account_settings_redirect_view(request)
34+
35+
assert response.status_code == 302
36+
assert response.url == 'https://account.example.com'
37+
38+
def test_redirects_with_duplicate_provider_param(self):
39+
request = self._build_request(error_code='duplicate_provider', backend_name='tpa-saml')
40+
41+
response = account_settings_redirect_view(request)
42+
43+
assert response.status_code == 302
44+
assert response.url.startswith('https://account.example.com/?')
45+
assert 'tpa_error_code=duplicate_provider' in response.url
46+
assert 'duplicate_provider=' in response.url
47+
48+
def test_redirects_with_other_error_code_without_duplicate_provider_param(self):
49+
"""
50+
For error codes other than 'duplicate_provider', only tpa_error_code
51+
should be sent -- 'duplicate_provider' is specific to the
52+
AuthAlreadyAssociated case, which is the only one the Account MFE
53+
currently knows how to render.
54+
"""
55+
request = self._build_request(error_code='auth_canceled', backend_name='tpa-saml')
56+
57+
response = account_settings_redirect_view(request)
58+
59+
assert response.status_code == 302
60+
assert 'tpa_error_code=auth_canceled' in response.url
61+
assert 'duplicate_provider' not in response.url
62+
63+
def test_session_error_keys_are_consumed(self):
64+
"""
65+
The error code and backend name should be popped from the session
66+
so a stale error doesn't leak into a later, unrelated request.
67+
"""
68+
request = self._build_request(error_code='duplicate_provider', backend_name='tpa-saml')
69+
70+
account_settings_redirect_view(request)
71+
72+
assert TPA_ERROR_CODE_SESSION_KEY not in request.session
73+
assert TPA_ERROR_BACKEND_SESSION_KEY not in request.session

requirements/edx/base.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ django==5.2.15
252252
# xss-utils
253253
django-appconf==1.2.0
254254
# via django-statici18n
255-
django-autocomplete-light==4.0.1
255+
django-autocomplete-light==4.0.3
256256
# via -r requirements/edx/kernel.in
257257
django-cache-memoize==0.2.1
258258
# via edx-enterprise

requirements/edx/development.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,7 @@ django-appconf==1.2.0
423423
# -r requirements/edx/doc.txt
424424
# -r requirements/edx/testing.txt
425425
# django-statici18n
426-
django-autocomplete-light==4.0.1
426+
django-autocomplete-light==4.0.3
427427
# via
428428
# -r requirements/edx/doc.txt
429429
# -r requirements/edx/testing.txt

requirements/edx/doc.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ django-appconf==1.2.0
318318
# via
319319
# -r requirements/edx/base.txt
320320
# django-statici18n
321-
django-autocomplete-light==4.0.1
321+
django-autocomplete-light==4.0.3
322322
# via -r requirements/edx/base.txt
323323
django-cache-memoize==0.2.1
324324
# via

requirements/edx/testing.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,7 @@ django-appconf==1.2.0
339339
# via
340340
# -r requirements/edx/base.txt
341341
# django-statici18n
342-
django-autocomplete-light==4.0.1
342+
django-autocomplete-light==4.0.3
343343
# via -r requirements/edx/base.txt
344344
django-cache-memoize==0.2.1
345345
# via

0 commit comments

Comments
 (0)