Fix: browser OAuth flow never sets session cookies (redirect format)#271
Open
carlosplanchon wants to merge 1 commit into
Open
Fix: browser OAuth flow never sets session cookies (redirect format)#271carlosplanchon wants to merge 1 commit into
carlosplanchon wants to merge 1 commit into
Conversation
The Google OAuth callback set the session cookies on the injected response parameter and then returned a RedirectResponse. FastAPI only merges the injected response's headers into the final response on the serialization path; when an endpoint returns a Response directly they are dropped. Net effect: with response_format=redirect (the default, i.e. the real browser flow) the OAuth dance completed and the session was created server-side, but the browser never received session_id/csrf_token and the user landed logged out. The json flow worked, which is why the test suite (json-only for the success case) never caught it. Build the RedirectResponse first, set the cookies on it, and return it. Adds a regression test asserting the 302 carries both Set-Cookie headers (fails without the fix). Found live while integrating a self-hosted OIDC provider; reproduced with the stock Google flow. Signed-off-by: Carlos Andrés Planchón Prestes <carlosandresplanchonprestes@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The OAuth callback sets session cookies on the injected response parameter but returns a RedirectResponse directly; FastAPI drops the injected response's headers in that path, so the browser flow (response_format=redirect, the default) completes the dance but the user lands logged out. Only the json flow worked, which is why tests never caught it. Fix: set the cookies on the RedirectResponse itself. Includes a regression test that fails without the fix. Found live while integrating a self-hosted OIDC provider (Zitadel); reproduces with the stock Google flow.