From 22b1858ee837b5f59633bb38a4808995c89f7e4a Mon Sep 17 00:00:00 2001 From: Jacob Fletcher Date: Tue, 20 May 2025 17:15:18 -0400 Subject: [PATCH] fix: auto inject req.user into query preset constraints (#12461) In #12322 we prevented against accidental query preset lockout by throwing a validation error when the user is going to change the preset in a way that removes their own access to it. This, however, puts the responsibility on the user to make the corrections and is an unnecessary step. For example, the API currently forbids leaving yourself out of the `users` array when specifying the `specificUsers` constraint, but when you encounter this error, have to update the field manually and try again. To improve the experience, we now automatically inject the requesting user onto the `users` array when this constraint is selected. This will guarantee they have access and prevent an accidental lockout while also avoiding the API error feedback loop. --- .../payload/src/query-presets/constraints.ts | 10 ++- .../src/query-presets/preventLockout.ts | 2 +- test/query-presets/int.spec.ts | 86 +++++++++++++------ 3 files changed, 65 insertions(+), 33 deletions(-) diff --git a/packages/payload/src/query-presets/constraints.ts b/packages/payload/src/query-presets/constraints.ts index 921749ec1..f13d9ef9e 100644 --- a/packages/payload/src/query-presets/constraints.ts +++ b/packages/payload/src/query-presets/constraints.ts @@ -65,10 +65,12 @@ export const getConstraints = (config: Config): Field => ({ hooks: { beforeChange: [ ({ data, req }) => { - if (data?.access?.[operation]?.constraint === 'onlyMe') { - if (req.user) { - return [req.user.id] - } + if (data?.access?.[operation]?.constraint === 'onlyMe' && req.user) { + return [req.user.id] + } + + if (data?.access?.[operation]?.constraint === 'specificUsers' && req.user) { + return [...(data?.access?.[operation]?.users || []), req.user.id] } return data?.access?.[operation]?.users diff --git a/packages/payload/src/query-presets/preventLockout.ts b/packages/payload/src/query-presets/preventLockout.ts index 4f285e1d0..bf9e6e85d 100644 --- a/packages/payload/src/query-presets/preventLockout.ts +++ b/packages/payload/src/query-presets/preventLockout.ts @@ -72,7 +72,7 @@ export const preventLockout: Validate = async ( canUpdate = true } catch (_err) { if (!canRead || !canUpdate) { - throw new APIError('Cannot remove yourself from this preset.', 403, {}, true) + throw new APIError('This action will lock you out of this preset.', 403, {}, true) } } finally { if (transaction) { diff --git a/test/query-presets/int.spec.ts b/test/query-presets/int.spec.ts index 6fdc915e5..d72bbbf53 100644 --- a/test/query-presets/int.spec.ts +++ b/test/query-presets/int.spec.ts @@ -379,27 +379,25 @@ describe('Query Presets', () => { }) it('should prevent accidental lockout', async () => { - // attempt to create a preset without access to read or update try { + // create a preset using "specificRoles" + // this will ensure the user on the request is _NOT_ automatically added to the `users` list + // and will throw a validation error instead const presetWithoutAccess = await payload.create({ collection: queryPresetsCollectionSlug, - user: adminUser, + user: editorUser, overrideAccess: false, data: { title: 'Prevent Lockout', relatedCollection: 'pages', access: { read: { - constraint: 'specificUsers', - users: [], + constraint: 'specificRoles', + roles: ['admin'], }, update: { - constraint: 'specificUsers', - users: [], - }, - delete: { - constraint: 'specificUsers', - users: [], + constraint: 'specificRoles', + roles: ['admin'], }, }, }, @@ -407,9 +405,49 @@ describe('Query Presets', () => { expect(presetWithoutAccess).toBeFalsy() } catch (error: unknown) { - expect((error as Error).message).toBe('Cannot remove yourself from this preset.') + expect((error as Error).message).toBe('This action will lock you out of this preset.') } + // create a preset using "specificUsers" + // this will ensure the user on the request _IS_ automatically added to the `users` list + // this will avoid a validation error + const presetWithoutAccess = await payload.create({ + collection: queryPresetsCollectionSlug, + user: adminUser, + overrideAccess: false, + data: { + title: 'Prevent Lockout', + relatedCollection: 'pages', + access: { + read: { + constraint: 'specificUsers', + users: [], + }, + update: { + constraint: 'specificUsers', + users: [], + }, + delete: { + constraint: 'specificUsers', + users: [], + }, + }, + }, + }) + + // the user on the request is automatically added to the `users` array + expect( + presetWithoutAccess.access?.read?.users?.find( + (user) => (typeof user === 'string' ? user : user.id) === adminUser.id, + ), + ).toBeTruthy() + + expect( + presetWithoutAccess.access?.update?.users?.find( + (user) => (typeof user === 'string' ? user : user.id) === adminUser.id, + ), + ).toBeTruthy() + const presetWithUser1 = await payload.create({ collection: queryPresetsCollectionSlug, user: adminUser, @@ -419,16 +457,12 @@ describe('Query Presets', () => { relatedCollection: 'pages', access: { read: { - constraint: 'specificUsers', - users: [adminUser.id], + constraint: 'specificRoles', + roles: ['admin'], }, update: { - constraint: 'specificUsers', - users: [adminUser.id], - }, - delete: { - constraint: 'specificUsers', - users: [adminUser.id], + constraint: 'specificRoles', + roles: ['admin'], }, }, }, @@ -445,16 +479,12 @@ describe('Query Presets', () => { title: 'Prevent Lockout (Updated)', access: { read: { - constraint: 'specificUsers', - users: [], + constraint: 'specificRoles', + roles: ['user'], }, update: { - constraint: 'specificUsers', - users: [], - }, - delete: { - constraint: 'specificUsers', - users: [], + constraint: 'specificRoles', + roles: ['user'], }, }, }, @@ -462,7 +492,7 @@ describe('Query Presets', () => { expect(presetUpdatedByUser1).toBeFalsy() } catch (error: unknown) { - expect((error as Error).message).toBe('Cannot remove yourself from this preset.') + expect((error as Error).message).toBe('This action will lock you out of this preset.') } }) })