diff --git a/packages/payload/src/auth/operations/login.ts b/packages/payload/src/auth/operations/login.ts index 8a05935a67e..09eaefe1d2e 100644 --- a/packages/payload/src/auth/operations/login.ts +++ b/packages/payload/src/auth/operations/login.ts @@ -203,6 +203,14 @@ export const loginOperation = async ( where: whereConstraint, }) + // Start transaction before the user lookup so that the entire + // login flow (read user → authenticate → add session → sign JWT) + // is atomic. This prevents a race condition where concurrent logins + // read the same session state and one overwrites the other. + const shouldCommit = await initTransaction(args.req) + + try { + let user = (await payload.db.findOne({ collection: collectionConfig.slug, req, @@ -246,11 +254,7 @@ export const loginOperation = async ( throw new UnverifiedEmail({ t: req.t }) } - // Authentication successful - start transaction for remaining operations - const shouldCommit = await initTransaction(args.req) let sid: string | undefined - - try { /* * Correct password accepted - re‑check that the account didn't * get locked by parallel bad attempts in the meantime. diff --git a/packages/payload/src/auth/sessions.ts b/packages/payload/src/auth/sessions.ts index c5e2d0bd2d7..fbba2563646 100644 --- a/packages/payload/src/auth/sessions.ts +++ b/packages/payload/src/auth/sessions.ts @@ -18,7 +18,15 @@ export const removeExpiredSessions = (sessions: UserSession[]) => { } /** - * Adds a session to the user and removes expired sessions + * Adds a session to the user and removes expired sessions. + * + * To avoid a race condition where concurrent logins read the same stale + * `sessions` array and one overwrites the other (last-write-wins), this + * function now performs a fresh read of the user's current sessions from the + * database before appending the new session. Because this runs inside the + * login transaction, the read-modify-write sequence is atomic with respect + * to other concurrent login transactions. + * * @returns The session ID (sid) if sessions are used */ export const addSessionToUser = async ({ @@ -34,32 +42,44 @@ export const addSessionToUser = async ({ }): Promise<{ sid?: string }> => { let sid: string | undefined if (collectionConfig.auth.useSessions) { - // Add session to user sid = uuid() const now = new Date() const tokenExpInMs = collectionConfig.auth.tokenExpiration * 1000 const expiresAt = new Date(now.getTime() + tokenExpInMs) - const session = { id: sid, createdAt: now, expiresAt } + const newSession: UserSession = { id: sid, createdAt: now, expiresAt } - if (!user.sessions?.length) { - user.sessions = [session] - } else { - user.sessions = removeExpiredSessions(user.sessions) - user.sessions.push(session) - } + // Read fresh session data from DB to avoid stale-read race conditions + // during concurrent logins. The surrounding transaction in loginOperation + // ensures this read-modify-write is serialized per user. + const freshUser = await payload.db.findOne({ + collection: collectionConfig.slug, + req, + select: { + sessions: true, + }, + where: { id: { equals: user.id } }, + }) - // Prevent updatedAt from being updated when only adding a session - user.updatedAt = null + const existingSessions = freshUser?.sessions ?? [] + const activeSessions = removeExpiredSessions(existingSessions) + activeSessions.push(newSession) + // Update only the sessions field to minimize write conflicts await payload.db.updateOne({ id: user.id, collection: collectionConfig.slug, - data: user, + data: { + sessions: activeSessions, + // Prevent updatedAt from being updated when only adding a session + updatedAt: null, + } as Partial, req, returning: false, }) + // Keep the in-memory user object consistent + user.sessions = activeSessions user.collection = collectionConfig.slug user._strategy = 'local-jwt' } @@ -83,13 +103,28 @@ export const revokeSession = async ({ user: null | (TypeWithID & UntypedUser) }): Promise => { if (collectionConfig.auth.useSessions && user && user.sessions?.length) { - user.sessions = user.sessions.filter((session) => session.id !== sid) - await payload.db.updateOne({ - id: user.id, + // Read fresh sessions from DB to avoid revoking based on stale data + const freshUser = await payload.db.findOne({ collection: collectionConfig.slug, - data: user, req, - returning: false, + select: { + sessions: true, + }, + where: { id: { equals: user.id } }, }) + + if (freshUser?.sessions?.length) { + const updatedSessions = freshUser.sessions.filter((session) => session.id !== sid) + await payload.db.updateOne({ + id: user.id, + collection: collectionConfig.slug, + data: { + sessions: updatedSessions, + } as Partial, + req, + returning: false, + }) + user.sessions = updatedSessions + } } } diff --git a/test/auth/int.spec.ts b/test/auth/int.spec.ts index 99203a50fa8..1528fe9cb47 100644 --- a/test/auth/int.spec.ts +++ b/test/auth/int.spec.ts @@ -989,6 +989,129 @@ describe('Auth', () => { }) }) + describe('Concurrent Login Sessions', () => { + let concurrentUserEmail: string + const concurrentPassword = 'concurrent-test-pass' + + beforeEach(async () => { + concurrentUserEmail = `concurrent-${uuid()}@test.com` + await payload.create({ + collection: slug, + data: { + email: concurrentUserEmail, + password: concurrentPassword, + }, + }) + }) + + it('should preserve all sessions when multiple logins occur concurrently', async () => { + const concurrentLogins = 5 + + // Fire multiple login requests at the same time + const loginPromises = Array.from({ length: concurrentLogins }, () => + restClient + .POST(`/${slug}/login`, { + body: JSON.stringify({ + email: concurrentUserEmail, + password: concurrentPassword, + }), + headers: { + 'Content-Type': 'application/json', + }, + }) + .then((res) => res.json()), + ) + + const results = await Promise.allSettled(loginPromises) + + // All logins should succeed + const successful = results.filter( + (r) => r.status === 'fulfilled' && r.value.token, + ) + expect(successful.length).toBe(concurrentLogins) + + // Each token should be valid — /me should return a user + const mePromises = successful.map((r) => { + const loginResult = (r as PromiseFulfilledResult<{ token: string }>).value + return restClient + .GET(`/${slug}/me`, { + headers: { + Authorization: `JWT ${loginResult.token}`, + }, + }) + .then((res) => res.json()) + }) + + const meResults = await Promise.all(mePromises) + + // Every token should return a valid user (not null) + const validUsers = meResults.filter( + (me) => me.user && me.user.email === concurrentUserEmail, + ) + + expect(validUsers.length).toBe(concurrentLogins) + }) + + it('should not lose sessions due to concurrent login race conditions', async () => { + const concurrentLogins = 3 + + const loginPromises = Array.from({ length: concurrentLogins }, () => + restClient + .POST(`/${slug}/login`, { + body: JSON.stringify({ + email: concurrentUserEmail, + password: concurrentPassword, + }), + headers: { + 'Content-Type': 'application/json', + }, + }) + .then((res) => res.json()), + ) + + const results = await Promise.allSettled(loginPromises) + + const tokens = results + .filter((r) => r.status === 'fulfilled' && r.value.token) + .map((r) => (r as PromiseFulfilledResult<{ token: string }>).value.token) + + expect(tokens.length).toBe(concurrentLogins) + + // Verify the user has all sessions stored in the database + const userResult = await payload.find({ + collection: slug, + limit: 1, + showHiddenFields: true, + where: { + email: { + equals: concurrentUserEmail, + }, + }, + }) + + const userDoc = userResult.docs[0]! + + // If sessions are enabled (default), all sessions should be present + if (userDoc.sessions) { + expect(userDoc.sessions.length).toBeGreaterThanOrEqual(concurrentLogins) + } + + // Each token should independently work with /me + for (const tkn of tokens) { + const meResponse = await restClient.GET(`/${slug}/me`, { + headers: { + Authorization: `JWT ${tkn}`, + }, + }) + + const meData = await meResponse.json() + expect(meData.user).not.toBeNull() + expect(meData.user?.email).toBe(concurrentUserEmail) + } + }) + }) + + it('should allow forgot-password by email', async () => { // TODO: Spy on payload sendEmail function const response = await restClient.POST(`/${slug}/forgot-password`, {