Skip to content

Commit dd494be

Browse files
JarrodMFleschJarrodMFlesch
authored
fix: adds missing transactions to login and logout operations (#15134)
Fixes #14610 Adds transactions to login and logout operations. Note: the github diff viewer is very misleading. If you look at the diff locally all that was changed was initializing a transaction and killing it if an error is thrown in the login/logout operations.
1 parent ea15f00 commit dd494be

9 files changed

Lines changed: 391 additions & 233 deletions

File tree

‎packages/payload/src/auth/operations/login.ts‎

Lines changed: 171 additions & 154 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,15 @@ import {
1515
ValidationError,
1616
} from '../../errors/index.js'
1717
import { afterRead } from '../../fields/hooks/afterRead/index.js'
18-
import { Forbidden } from '../../index.js'
18+
import { commitTransaction, Forbidden, initTransaction } from '../../index.js'
1919
import { appendNonTrashedFilter } from '../../utilities/appendNonTrashedFilter.js'
2020
import { killTransaction } from '../../utilities/killTransaction.js'
2121
import { sanitizeInternalFields } from '../../utilities/sanitizeInternalFields.js'
2222
import { getFieldsToSign } from '../getFieldsToSign.js'
2323
import { getLoginOptions } from '../getLoginOptions.js'
2424
import { isUserLocked } from '../isUserLocked.js'
2525
import { jwtSign } from '../jwt.js'
26-
import { addSessionToUser } from '../sessions.js'
26+
import { addSessionToUser, revokeSession } from '../sessions.js'
2727
import { authenticateLocalStrategy } from '../strategies/local/authenticate.js'
2828
import { incrementLoginAttempts } from '../strategies/local/incrementLoginAttempts.js'
2929
import { resetLoginAttempts } from '../strategies/local/resetLoginAttempts.js'
@@ -77,176 +77,179 @@ export const loginOperation = async <TSlug extends CollectionSlug>(
7777
throw new Forbidden(args.req.t)
7878
}
7979

80-
try {
81-
// /////////////////////////////////////
82-
// beforeOperation - Collection
83-
// /////////////////////////////////////
84-
85-
args = await buildBeforeOperation({
86-
args,
87-
collection: args.collection.config,
88-
operation: 'login',
80+
// /////////////////////////////////////
81+
// beforeOperation - Collection
82+
// /////////////////////////////////////
83+
84+
args = await buildBeforeOperation({
85+
args,
86+
collection: args.collection.config,
87+
operation: 'login',
88+
})
89+
90+
const {
91+
collection: { config: collectionConfig },
92+
data,
93+
depth,
94+
overrideAccess,
95+
req,
96+
req: {
97+
fallbackLocale,
98+
locale,
99+
payload,
100+
payload: { secret },
101+
},
102+
showHiddenFields,
103+
} = args
104+
105+
// /////////////////////////////////////
106+
// Login
107+
// /////////////////////////////////////
108+
109+
const { email: unsanitizedEmail, password } = data
110+
const loginWithUsername = collectionConfig.auth.loginWithUsername
111+
112+
const sanitizedEmail =
113+
typeof unsanitizedEmail === 'string' ? unsanitizedEmail.toLowerCase().trim() : null
114+
const sanitizedUsername =
115+
'username' in data && typeof data?.username === 'string'
116+
? data.username.toLowerCase().trim()
117+
: null
118+
119+
const { canLoginWithEmail, canLoginWithUsername } = getLoginOptions(loginWithUsername)
120+
121+
// cannot login with email, did not provide username
122+
if (!canLoginWithEmail && !sanitizedUsername) {
123+
throw new ValidationError({
124+
collection: collectionConfig.slug,
125+
errors: [{ message: req.i18n.t('validation:required'), path: 'username' }],
89126
})
127+
}
90128

91-
const {
92-
collection: { config: collectionConfig },
93-
data,
94-
depth,
95-
overrideAccess,
96-
req,
97-
req: {
98-
fallbackLocale,
99-
locale,
100-
payload,
101-
payload: { secret },
102-
},
103-
showHiddenFields,
104-
} = args
105-
106-
// /////////////////////////////////////
107-
// Login
108-
// /////////////////////////////////////
109-
110-
const { email: unsanitizedEmail, password } = data
111-
const loginWithUsername = collectionConfig.auth.loginWithUsername
112-
113-
const sanitizedEmail =
114-
typeof unsanitizedEmail === 'string' ? unsanitizedEmail.toLowerCase().trim() : null
115-
const sanitizedUsername =
116-
'username' in data && typeof data?.username === 'string'
117-
? data.username.toLowerCase().trim()
118-
: null
129+
// cannot login with username, did not provide email
130+
if (!canLoginWithUsername && !sanitizedEmail) {
131+
throw new ValidationError({
132+
collection: collectionConfig.slug,
133+
errors: [{ message: req.i18n.t('validation:required'), path: 'email' }],
134+
})
135+
}
119136

120-
const { canLoginWithEmail, canLoginWithUsername } = getLoginOptions(loginWithUsername)
137+
// can login with either email or username, did not provide either
138+
if (!sanitizedUsername && !sanitizedEmail) {
139+
throw new ValidationError({
140+
collection: collectionConfig.slug,
141+
errors: [
142+
{ message: req.i18n.t('validation:required'), path: 'email' },
143+
{ message: req.i18n.t('validation:required'), path: 'username' },
144+
],
145+
})
146+
}
121147

122-
// cannot login with email, did not provide username
123-
if (!canLoginWithEmail && !sanitizedUsername) {
124-
throw new ValidationError({
125-
collection: collectionConfig.slug,
126-
errors: [{ message: req.i18n.t('validation:required'), path: 'username' }],
127-
})
128-
}
148+
// did not provide password for login
149+
if (typeof password !== 'string' || password.trim() === '') {
150+
throw new ValidationError({
151+
collection: collectionConfig.slug,
152+
errors: [{ message: req.i18n.t('validation:required'), path: 'password' }],
153+
})
154+
}
129155

130-
// cannot login with username, did not provide email
131-
if (!canLoginWithUsername && !sanitizedEmail) {
132-
throw new ValidationError({
133-
collection: collectionConfig.slug,
134-
errors: [{ message: req.i18n.t('validation:required'), path: 'email' }],
135-
})
136-
}
156+
let whereConstraint: Where = {}
157+
const emailConstraint: Where = {
158+
email: {
159+
equals: sanitizedEmail,
160+
},
161+
}
162+
const usernameConstraint: Where = {
163+
username: {
164+
equals: sanitizedUsername,
165+
},
166+
}
137167

138-
// can login with either email or username, did not provide either
139-
if (!sanitizedUsername && !sanitizedEmail) {
140-
throw new ValidationError({
141-
collection: collectionConfig.slug,
142-
errors: [
143-
{ message: req.i18n.t('validation:required'), path: 'email' },
144-
{ message: req.i18n.t('validation:required'), path: 'username' },
168+
if (canLoginWithEmail && canLoginWithUsername && (sanitizedUsername || sanitizedEmail)) {
169+
if (sanitizedUsername) {
170+
whereConstraint = {
171+
or: [
172+
usernameConstraint,
173+
{
174+
email: {
175+
equals: sanitizedUsername,
176+
},
177+
},
145178
],
146-
})
179+
}
180+
} else {
181+
whereConstraint = {
182+
or: [
183+
emailConstraint,
184+
{
185+
username: {
186+
equals: sanitizedEmail,
187+
},
188+
},
189+
],
190+
}
147191
}
192+
} else if (canLoginWithEmail && sanitizedEmail) {
193+
whereConstraint = emailConstraint
194+
} else if (canLoginWithUsername && sanitizedUsername) {
195+
whereConstraint = usernameConstraint
196+
}
148197

149-
// did not provide password for login
150-
if (typeof password !== 'string' || password.trim() === '') {
151-
throw new ValidationError({
152-
collection: collectionConfig.slug,
153-
errors: [{ message: req.i18n.t('validation:required'), path: 'password' }],
154-
})
155-
}
198+
// Exclude trashed users
199+
whereConstraint = appendNonTrashedFilter({
200+
enableTrash: collectionConfig.trash,
201+
trash: false,
202+
where: whereConstraint,
203+
})
156204

157-
let whereConstraint: Where = {}
158-
const emailConstraint: Where = {
159-
email: {
160-
equals: sanitizedEmail,
161-
},
162-
}
163-
const usernameConstraint: Where = {
164-
username: {
165-
equals: sanitizedUsername,
166-
},
167-
}
205+
let user = (await payload.db.findOne<TypedUser>({
206+
collection: collectionConfig.slug,
207+
req,
208+
where: whereConstraint,
209+
})) as TypedUser
168210

169-
if (canLoginWithEmail && canLoginWithUsername && (sanitizedUsername || sanitizedEmail)) {
170-
if (sanitizedUsername) {
171-
whereConstraint = {
172-
or: [
173-
usernameConstraint,
174-
{
175-
email: {
176-
equals: sanitizedUsername,
177-
},
178-
},
179-
],
180-
}
181-
} else {
182-
whereConstraint = {
183-
or: [
184-
emailConstraint,
185-
{
186-
username: {
187-
equals: sanitizedEmail,
188-
},
189-
},
190-
],
191-
}
192-
}
193-
} else if (canLoginWithEmail && sanitizedEmail) {
194-
whereConstraint = emailConstraint
195-
} else if (canLoginWithUsername && sanitizedUsername) {
196-
whereConstraint = usernameConstraint
197-
}
211+
checkLoginPermission({
212+
loggingInWithUsername: Boolean(canLoginWithUsername && sanitizedUsername),
213+
req,
214+
user,
215+
})
198216

199-
// Exclude trashed users
200-
whereConstraint = appendNonTrashedFilter({
201-
enableTrash: collectionConfig.trash,
202-
trash: false,
203-
where: whereConstraint,
204-
})
217+
user.collection = collectionConfig.slug
218+
user._strategy = 'local-jwt'
205219

206-
let user = (await payload.db.findOne<TypedUser>({
207-
collection: collectionConfig.slug,
208-
req,
209-
where: whereConstraint,
210-
})) as TypedUser
220+
const authResult = await authenticateLocalStrategy({ doc: user, password })
221+
user = sanitizeInternalFields(user)
211222

212-
checkLoginPermission({
213-
loggingInWithUsername: Boolean(canLoginWithUsername && sanitizedUsername),
214-
req,
215-
user,
216-
})
223+
const maxLoginAttemptsEnabled = args.collection.config.auth.maxLoginAttempts > 0
217224

218-
user.collection = collectionConfig.slug
219-
user._strategy = 'local-jwt'
220-
221-
const authResult = await authenticateLocalStrategy({ doc: user, password })
222-
user = sanitizeInternalFields(user)
223-
224-
const maxLoginAttemptsEnabled = args.collection.config.auth.maxLoginAttempts > 0
225-
226-
if (!authResult) {
227-
if (maxLoginAttemptsEnabled) {
228-
await incrementLoginAttempts({
229-
collection: collectionConfig,
230-
payload: req.payload,
231-
req,
232-
user,
233-
})
234-
235-
// Re-check login permissions and max attempts after incrementing attempts, in case parallel updates occurred
236-
checkLoginPermission({
237-
loggingInWithUsername: Boolean(canLoginWithUsername && sanitizedUsername),
238-
req,
239-
user,
240-
})
241-
}
225+
if (!authResult) {
226+
if (maxLoginAttemptsEnabled) {
227+
await incrementLoginAttempts({
228+
collection: collectionConfig,
229+
payload: req.payload,
230+
user,
231+
})
242232

243-
throw new AuthenticationError(req.t)
233+
// Re-check login permissions and max attempts after incrementing attempts, in case parallel updates occurred
234+
checkLoginPermission({
235+
loggingInWithUsername: Boolean(canLoginWithUsername && sanitizedUsername),
236+
req,
237+
user,
238+
})
244239
}
245240

246-
if (collectionConfig.auth.verify && user._verified === false) {
247-
throw new UnverifiedEmail({ t: req.t })
248-
}
241+
throw new AuthenticationError(req.t)
242+
}
243+
244+
if (collectionConfig.auth.verify && user._verified === false) {
245+
throw new UnverifiedEmail({ t: req.t })
246+
}
249247

248+
// Authentication successful - start transaction for remaining operations
249+
const shouldCommit = await initTransaction(args.req)
250+
let sid: string | undefined
251+
252+
try {
250253
/*
251254
* Correct password accepted - re‑check that the account didn't
252255
* get locked by parallel bad attempts in the meantime.
@@ -277,12 +280,13 @@ export const loginOperation = async <TSlug extends CollectionSlug>(
277280
user,
278281
}
279282

280-
const { sid } = await addSessionToUser({
283+
const session = await addSessionToUser({
281284
collectionConfig,
282285
payload,
283286
req,
284287
user,
285288
})
289+
sid = session.sid
286290

287291
if (sid) {
288292
fieldsToSignArgs.sid = sid
@@ -392,12 +396,25 @@ export const loginOperation = async <TSlug extends CollectionSlug>(
392396
result,
393397
})
394398

399+
if (shouldCommit) {
400+
await commitTransaction(req)
401+
}
402+
395403
// /////////////////////////////////////
396404
// Return results
397405
// /////////////////////////////////////
398406

399407
return result
400408
} catch (error: unknown) {
409+
if (sid) {
410+
await revokeSession({
411+
collectionConfig,
412+
payload,
413+
req,
414+
sid,
415+
user,
416+
})
417+
}
401418
await killTransaction(args.req)
402419
throw error
403420
}

0 commit comments

Comments
 (0)