{{ message }}
Repository navigation
fix(account): validate OAuth2 state on the redirect callback - #14012
Merged
Merged
Conversation
The state handed to an OAuth2 provider now carries a per-flow nonce that is also kept in a short-lived, host-only cookie. The redirect callback checks the two against each other before exchanging the code, as RFC 6749 §10.12 asks, and answers USER_OAUTH2_STATE_INVALID when they do not match. The cookie is expired once the callback has used it. The session and the token flow share the callback, so both set it. The E2E walkers forward the cookie the way a browser would, and a new test covers a callback completed from a browser that did not start the flow. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Meldiron
marked this pull request as ready for review
September 30, 2026 10:12
Contributor
![hansi-codes[bot]](https://avatars.githubusercontent.com/in/5046327?s=60&v=4){kind=small}
✨ Benchmark resultsComparing
Per-scenario breakdown & investigation detailsMetrics below reflect the current branch (after). Δ P95 compares against the base.
Top API waits (after)
|
Contributor
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
A second flow started in the same browser, in another tab or through the token route, replaced the first one's nonce and the first callback then failed. The cookie now keeps the newest few nonces and the callback consumes only the one it matched. The E2E walkers send the cookie to the Appwrite hops only, the new test no longer pins the intermediate routes, and a client with a cookie jar shows the callback host receiving the cookie by itself. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…backs The two failure-redirect tests start a real flow and then drive the callback with a provider error from a fresh client, so the nonce cookie has to ride along like it would in a browser. The custom failure URL test now also checks that the error it lands with is the provider's. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Rewriting the cookie to drop the nonce a callback matched raced with any other callback finishing in the same browser: whichever response landed last put the other's nonce back. A browser cookie cannot make a nonce single-use, and the provider already refuses a replayed authorization code (RFC 6749 §4.1.2), so the callback now only reads the cookie and lets it expire on its own. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The OAuth2 walkers and the new tests named the nonce cookie and copied it by hand. A browser sends back whatever cookies it was given, so the tests now forward every cookie the start response set and no longer know its name. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
abnegate
added a commit
that referenced
this pull request
Sep 30, 2026
Brings in main's OAuth2 state and nonce validation (#14012), which cloud needs, along with the realtime origin alignment (#14020), subscriber target ownership (#14019), GitLab primary email (#14010), impersonation MFA (#14017), write-only webhook passwords (#14016), chunked-upload resume ownership (#14014), preview authorize validation (#14013), nightly forcing (#14015) and scheduled-function enqueue checks (#14011). The merge had no textual conflicts, and main left composer.json and composer.lock unchanged. So the lock keeps the branch's database and async pins. Main's assertResumeAllowed() in Storage Buckets/Files/Create checks Input(Database::PERMISSION_UPDATE, ...). utopia-php/database 8 has no PERMISSION_* constants, so every chunked resume would fail with an undefined-constant error. It now passes PermissionType::Update, as the rest of that action already does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 6, 2026
NancyWei123
pushed a commit
to NancyWei123/appwrite
that referenced
this pull request
Oct 6, 2026
The token flow hands userId and secret to the app, whose server often starts the flow (the SSR pattern in the docs). The nonce cookie then lands on that server instead of the browser, so every server-started sign-in failed with user_oauth2_state_invalid since appwrite#14012. The session flow keeps the check: Appwrite sets that session itself, and the flow is browser-only.
2 tasks done
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
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.
What does this PR do?
The
statehanded to an OAuth2 provider now carries a per-flow nonce that is also kept in a short-lived, host-onlya_oauth2_<projectId>cookie (HttpOnly,SameSite=Lax,Secureon HTTPS, one hour). The redirect callback checks the two against each other before exchanging the code, as RFC 6749 §10.12 recommends, and answers with the newUSER_OAUTH2_STATE_INVALID(401) when they do not match.createOAuth2SessionandcreateOAuth2Tokenshare the callback, so both set it.A browser can have several flows open at once (two tabs, the session and the token flow), so the cookie keeps the newest five nonces. The callback does not rewrite the cookie: two callbacks finishing together would race on it, and replaying an authorization code is already refused by the provider (RFC 6749 §4.1.2). The cookie expires on its own.
Flows already in flight during a rollout carry a state without a nonce and fail once with a clear "start the sign-in again" error.
Test Plan
AccountCustomClientTestandAvatarsCustomClientTestforward the cookie on the Appwrite hops the way a browser would; the provider hop never sees it.testCreateOAuth2SessionRejectsCallbackFromAnotherBrowser: a callback URL from a flow one browser started is completed by a different, signed-in browser → failure redirect withuser_oauth2_state_invalid, no session cookie set, the existing session intact and no identity added; a hand-written state without a nonce is rejected the same way; the browser that started the flow then completes the same URL and lands on its own account; and a client with a cookie jar following the redirects completes a flow with no hand-carried header.testCreateOAuth2SessionKeepsConcurrentFlows: two flows started in one browser both complete, in either order.composer lint,composer refactor:checkand PHPStan on the changed files pass locally; the E2E suites run in CI.Related PRs and Issues
server-cebump.Checklist
🤖 Generated with Claude Code