From 0882eddcbf660092d6a11688ffd0301b614aa9cb Mon Sep 17 00:00:00 2001 From: Devasia Joseph Date: Wed, 28 Oct 2026 09:47:20 +0530 Subject: [PATCH 1/2] fix: preserve next redirect across OAuth session cleanup EdXOAuth2.start() logs out an already-authenticated user before starting a new OAuth flow. Django's logout() flushes the session, which also drops the sanitized `next` value social-auth stored one step earlier, so the user lands on LOGIN_REDIRECT_URL instead of the page they requested. Read `next` before the logout and write it back afterwards. The cleanup itself is unchanged. The existing test did not catch this because social-core's TestStrategy keeps its session in a private dict rather than request.session, so the flush never reached it. The new test points the strategy at request.session, matching DjangoStrategy. Coding assistant: Claude Code --- CHANGELOG.rst | 5 +++++ src/auth_backends/backends.py | 4 ++++ src/auth_backends/tests/test_backends.py | 20 ++++++++++++++++++++ 3 files changed, 29 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 7f4cb08..41f953b 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -12,6 +12,11 @@ Change Log Unreleased ---------- +Fixed +~~~~~ + +* Session cleanup in ``EdXOAuth2.start()`` no longer discards the ``next`` redirect that social-auth stores before the OAuth flow begins. Users who were already logged in were sent to ``LOGIN_REDIRECT_URL`` instead of the page they asked for. + [4.6.2] - 2025-10-16 -------------------- diff --git a/src/auth_backends/backends.py b/src/auth_backends/backends.py index c2d7675..bd69fc5 100644 --- a/src/auth_backends/backends.py +++ b/src/auth_backends/backends.py @@ -98,7 +98,11 @@ def start(self): existing_username ) + # logout() flushes the session, which also holds the post-login redirect social-auth just stored. + next_url = self.strategy.session_get('next') logout(request) + if next_url: + self.strategy.session_set('next', next_url) return super().start() diff --git a/src/auth_backends/tests/test_backends.py b/src/auth_backends/tests/test_backends.py index e73237f..faa6e7e 100644 --- a/src/auth_backends/tests/test_backends.py +++ b/src/auth_backends/tests/test_backends.py @@ -159,6 +159,26 @@ def test_start_with_session_cleanup(self, user_authenticated, mock_logger, mock_ else: mock_logger.info.assert_not_called() + @pytest.mark.django_db + @patch('auth_backends.backends.set_custom_attribute') + @patch('auth_backends.backends.logger') + def test_start_with_session_cleanup_preserves_next(self, _mock_logger, _mock_set_attr): + """The post-login redirect stored by social-auth must survive the session cleanup.""" + request = RequestFactory().get('/auth/login/edx-oauth2/?next=/credentials/abc/') + request.user = User.objects.create_user(username='existing_user', email='existing@example.com') + middleware = SessionMiddleware(lambda req: None) + middleware.process_request(request) + request.session.save() + self.backend.strategy.request = request + # In production the strategy's session IS request.session; the fake test strategy keeps its own dict. + self.backend.strategy._session = request.session # pylint: disable=protected-access + self.backend.strategy.session_set('next', '/credentials/abc/') + + self.do_start() + + self.assertTrue(request.user.is_anonymous) + self.assertEqual(self.backend.strategy.session_get('next'), '/credentials/abc/') + def test_partial_pipeline(self): self.do_partial_pipeline() From 9a97775973aca0bb0c433d2a4d23c27eae60b8d7 Mon Sep 17 00:00:00 2001 From: Devasia Joseph Date: Wed, 28 Oct 2026 19:44:22 +0530 Subject: [PATCH 2/2] fix: also preserve FIELDS_STORED_IN_SESSION across OAuth session cleanup social-auth's do_auth() stores every field named in FIELDS_STORED_IN_SESSION right before start(), at the same point it stores the sanitized next. The forced logout() flushed those too. Carry them across alongside next. No current EdXOAuth2 consumer sets FIELDS_STORED_IN_SESSION, so this is hardening for deployments that add fields later. Closes #471 Coding assistant: Claude Code --- CHANGELOG.rst | 1 + src/auth_backends/backends.py | 11 +++++++---- src/auth_backends/tests/test_backends.py | 3 +++ 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 41f953b..28c36b3 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -16,6 +16,7 @@ Fixed ~~~~~ * Session cleanup in ``EdXOAuth2.start()`` no longer discards the ``next`` redirect that social-auth stores before the OAuth flow begins. Users who were already logged in were sent to ``LOGIN_REDIRECT_URL`` instead of the page they asked for. +* The same cleanup also keeps any fields listed in ``FIELDS_STORED_IN_SESSION``, which social-auth stores at the same point. [4.6.2] - 2025-10-16 diff --git a/src/auth_backends/backends.py b/src/auth_backends/backends.py index bd69fc5..01c1430 100644 --- a/src/auth_backends/backends.py +++ b/src/auth_backends/backends.py @@ -98,11 +98,14 @@ def start(self): existing_username ) - # logout() flushes the session, which also holds the post-login redirect social-auth just stored. - next_url = self.strategy.session_get('next') + # logout() flushes the session, which also holds what social-auth just stored for this request: + # the sanitized redirect and any FIELDS_STORED_IN_SESSION. Carry only those across. + stored_names = ['next', *self.setting('FIELDS_STORED_IN_SESSION', [])] + stored_values = {name: self.strategy.session_get(name) for name in stored_names} logout(request) - if next_url: - self.strategy.session_set('next', next_url) + for name, value in stored_values.items(): + if value is not None: + self.strategy.session_set(name, value) return super().start() diff --git a/src/auth_backends/tests/test_backends.py b/src/auth_backends/tests/test_backends.py index faa6e7e..508bf36 100644 --- a/src/auth_backends/tests/test_backends.py +++ b/src/auth_backends/tests/test_backends.py @@ -172,12 +172,15 @@ def test_start_with_session_cleanup_preserves_next(self, _mock_logger, _mock_set self.backend.strategy.request = request # In production the strategy's session IS request.session; the fake test strategy keeps its own dict. self.backend.strategy._session = request.session # pylint: disable=protected-access + self.set_social_auth_setting('FIELDS_STORED_IN_SESSION', ['auth_entry']) self.backend.strategy.session_set('next', '/credentials/abc/') + self.backend.strategy.session_set('auth_entry', 'login') self.do_start() self.assertTrue(request.user.is_anonymous) self.assertEqual(self.backend.strategy.session_get('next'), '/credentials/abc/') + self.assertEqual(self.backend.strategy.session_get('auth_entry'), 'login') def test_partial_pipeline(self): self.do_partial_pipeline()