fix(auth): resolve concurrent login session race condition#16052
Open
brick-pixel wants to merge 1 commit intopayloadcms:mainfrom
Open
fix(auth): resolve concurrent login session race condition#16052brick-pixel wants to merge 1 commit intopayloadcms:mainfrom
brick-pixel wants to merge 1 commit intopayloadcms:mainfrom
Conversation
Concurrent login requests could cause session loss due to a read-modify-write race in addSessionToUser. When multiple logins executed simultaneously, each read the same stale sessions array from the in-memory user object. The last write won, silently discarding sessions created by earlier concurrent requests. Tokens from lost sessions then failed /me checks with no user returned. Changes: - Move transaction start before user lookup in loginOperation so the entire login flow is atomic - Read fresh session data from database in addSessionToUser instead of relying on the potentially stale in-memory user object - Update only the sessions field to minimize write surface - Apply the same fresh-read pattern to revokeSession - Add integration tests for concurrent login session preservation
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?
Fix a race condition where concurrent login requests cause session loss, resulting in valid tokens returning no user from
/me.Why?
When multiple login requests execute simultaneously for the same user,
addSessionToUserreads the sessions array from the in-memory user object. Each concurrent request sees the same stale snapshot. The last write wins, silently discarding sessions created by earlier requests. Tokens from lost sessions then fail/mechecks — the server returns 200 but withuser: null.This is reproducible with a simple concurrent login test (see linked issue for repro repo).
How?
1. Moved transaction boundary earlier in
loginOperation(login.ts)The transaction now wraps the entire login flow including user lookup, so the read-modify-write on sessions is serialized per user.
2. Fresh DB read in
addSessionToUser(sessions.ts)Instead of relying on the stale in-memory
user.sessions, the function now reads the current sessions directly from the database within the transaction before appending the new session. Only thesessionsfield is written back to minimize the update surface.3. Same pattern applied to
revokeSession(sessions.ts)Revoke also reads fresh session data before filtering, preventing stale-data bugs on concurrent logout/error paths.
4. Integration tests (test/auth/int.spec.ts)
Two new test cases verify that concurrent logins preserve all sessions and that every resulting token independently works with
/me.Diff summary
packages/payload/src/auth/operations/login.tsinitTransactionbefore user lookuppackages/payload/src/auth/sessions.tstest/auth/int.spec.ts+183 / -21 lines across 3 files
Fixes #16027