From 973dd9f06dc434f6dc33b84707f0388e3dde565a Mon Sep 17 00:00:00 2001 From: Trenton Holmes <797416+stumpylog@users.noreply.github.com> Date: Sun, 26 Jul 2026 13:26:18 -0700 Subject: [PATCH] Fix: don't clobber in-flight oauth_state on repeated ui_settings fetches --- src/documents/tests/test_api_uisettings.py | 36 +++++++++++++++++++++ src/documents/views.py | 10 ++++-- src/paperless_mail/tests/test_mail_oauth.py | 20 ++++++++++++ src/paperless_mail/views.py | 14 ++++++-- 4 files changed, 74 insertions(+), 6 deletions(-) diff --git a/src/documents/tests/test_api_uisettings.py b/src/documents/tests/test_api_uisettings.py index 811b77286..a943802e2 100644 --- a/src/documents/tests/test_api_uisettings.py +++ b/src/documents/tests/test_api_uisettings.py @@ -172,3 +172,39 @@ class TestApiUiSettings(DirectoriesMixin, APITestCase): self.assertIsNotNone( response.data["settings"]["outlook_oauth_url"], ) + + @override_settings( + OAUTH_CALLBACK_BASE_URL="http://localhost:8000", + GMAIL_OAUTH_CLIENT_ID="abc123", + GMAIL_OAUTH_CLIENT_SECRET="def456", + GMAIL_OAUTH_ENABLED=True, + OUTLOOK_OAUTH_CLIENT_ID="ghi789", + OUTLOOK_OAUTH_CLIENT_SECRET="jkl012", + OUTLOOK_OAUTH_ENABLED=True, + ) + def test_settings_oauth_state_reused_across_calls(self) -> None: + """ + GIVEN: + - A mail account oauth flow has been started, storing a state in the session + WHEN: + - The ui_settings endpoint is called again before the flow completes + THEN: + - The same oauth state is reused, not replaced with a new one + """ + response = self.client.get(self.ENDPOINT, format="json") + self.assertEqual(response.status_code, status.HTTP_200_OK) + first_state = self.client.session["oauth_state"] + first_gmail_url = response.data["settings"]["gmail_oauth_url"] + first_outlook_url = response.data["settings"]["outlook_oauth_url"] + + response = self.client.get(self.ENDPOINT, format="json") + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(self.client.session["oauth_state"], first_state) + self.assertEqual( + response.data["settings"]["gmail_oauth_url"], + first_gmail_url, + ) + self.assertEqual( + response.data["settings"]["outlook_oauth_url"], + first_outlook_url, + ) diff --git a/src/documents/views.py b/src/documents/views.py index 64594a3e7..b1d4ec566 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -4002,15 +4002,19 @@ class UiSettingsView(GenericAPIView[Any]): ui_settings["auditlog_enabled"] = settings.AUDIT_LOG_ENABLED if settings.GMAIL_OAUTH_ENABLED or settings.OUTLOOK_OAUTH_ENABLED: - manager = PaperlessMailOAuth2Manager() + # Reuse an in-flight state instead of minting a new one on every + # settings fetch, or a concurrent request can invalidate a login + # that's still in progress. + manager = PaperlessMailOAuth2Manager( + state=request.session.get("oauth_state"), + ) + request.session["oauth_state"] = manager.state if settings.GMAIL_OAUTH_ENABLED: ui_settings["gmail_oauth_url"] = manager.get_gmail_authorization_url() - request.session["oauth_state"] = manager.state if settings.OUTLOOK_OAUTH_ENABLED: ui_settings["outlook_oauth_url"] = ( manager.get_outlook_authorization_url() ) - request.session["oauth_state"] = manager.state ui_settings["email_enabled"] = settings.EMAIL_ENABLED diff --git a/src/paperless_mail/tests/test_mail_oauth.py b/src/paperless_mail/tests/test_mail_oauth.py index 52b96ca83..89fceb724 100644 --- a/src/paperless_mail/tests/test_mail_oauth.py +++ b/src/paperless_mail/tests/test_mail_oauth.py @@ -138,6 +138,16 @@ class TestMailOAuth( MailAccount.objects.filter(imap_server="imap.gmail.com").exists(), ) + # State is single-use and was cleared by the callback above, so a new + # flow needs a new state stored in the session. + session = self.client.session + session.update( + { + "oauth_state": "test_state", + }, + ) + session.save() + # Test Outlook OAuth callback response = self.client.get( "/api/oauth/callback/?code=test_code&state=test_state", @@ -180,6 +190,16 @@ class TestMailOAuth( MailAccount.objects.filter(imap_server="imap.gmail.com").exists(), ) + # State is single-use and was cleared by the callback above, so a + # new flow needs a new state stored in the session. + session = self.client.session + session.update( + { + "oauth_state": "test_state", + }, + ) + session.save() + # Test Outlook OAuth callback response = self.client.get( "/api/oauth/callback/?code=test_code&state=test_state", diff --git a/src/paperless_mail/views.py b/src/paperless_mail/views.py index 2e36f1b03..4b1032181 100644 --- a/src/paperless_mail/views.py +++ b/src/paperless_mail/views.py @@ -4,6 +4,7 @@ from datetime import timedelta from http import HTTPStatus from typing import Any +from django.conf import settings from django.http import HttpResponseBadRequest from django.http import HttpResponseForbidden from django.http import HttpResponseRedirect @@ -256,9 +257,14 @@ class OauthCallbackView(GenericAPIView[Any]): ) return HttpResponseBadRequest("Invalid request, see logs for more detail") - oauth_manager = PaperlessMailOAuth2Manager( - state=request.session.get("oauth_state"), - ) + session_state = request.session.get("oauth_state") + if not session_state and not settings.DEBUG: + logger.error( + "Invalid oauth callback request: no state in session", + ) + return HttpResponseBadRequest("Invalid request, see logs for more detail") + + oauth_manager = PaperlessMailOAuth2Manager(state=session_state) state = request.query_params.get("state", "") if not oauth_manager.validate_state(state): @@ -315,3 +321,5 @@ class OauthCallbackView(GenericAPIView[Any]): return HttpResponseRedirect( f"{oauth_manager.oauth_redirect_url}?oauth_success=0", ) + finally: + request.session.pop("oauth_state", None)