From 9779cf7f7dfc4267ff0d0814b5291ef6e3a288a6 Mon Sep 17 00:00:00 2001 From: Jacob Fletcher Date: Wed, 14 May 2025 15:25:32 -0400 Subject: [PATCH] feat: prevent query preset lockout (#12322) Prevents an accidental lockout of query preset documents. An "accidental lockout" occurs when the user sets access control on a preset and excludes themselves. This can happen in a variety of scenarios, including: - You select `specificUsers` without specifying yourself - You select `specificRoles` without specifying a role that you are a part of - Etc. #### How it works To make this happen, we use a custom validation function that executes access against the user's proposed changes. If those changes happen to remove access for them, we throw a validation error and prevent that change from ever taking place. This means that only a user with proper access can remove another user from the preset. You cannot remove yourself. To do this, we create a temporary record in the database that we can query against. We use transactions to ensure that the temporary record is not persisted once our work is completed. Since not all Payload projects have transactions enabled, we flag these temporary records with the `isTemp` field. Once created, we query the temp document to determine its permissions. If any of the operations throw an error, this means the user can no longer act on them, and we throw a validation error. #### Alternative Approach A previous approach that was explored was to add an `owner` field to the presets collection. This way, the "owner" of the preset would be able to completely bypass all access control, effectively eliminating the possibility of a lockout event. But this doesn't work for other users who may have update access. E.g. they could still accidentally remove themselves from the read or update operation, preventing them from accessing that preset after submitting the form. We need a solution that works for all users, not just the owner. --- .vscode/launch.json | 7 + packages/payload/src/admin/RichText.ts | 1 - .../collections/operations/local/create.ts | 1 + packages/payload/src/query-presets/config.ts | 10 + .../payload/src/query-presets/constraints.ts | 2 + .../src/query-presets/preventLockout.ts | 92 ++++++++ .../src/utilities/getEntityPolicies.ts | 3 + .../elements/ListControls/useQueryPresets.tsx | 3 + .../QueryPresets/fields/WhereField/index.tsx | 2 +- test/query-presets/collections/Pages/index.ts | 3 - test/query-presets/collections/Posts/index.ts | 3 - test/query-presets/config.ts | 8 +- test/query-presets/e2e.spec.ts | 78 ++++--- test/query-presets/fields/roles.ts | 8 +- test/query-presets/int.spec.ts | 212 +++++++++++++----- test/query-presets/payload-types.ts | 65 +++--- test/query-presets/seed.ts | 51 ++--- tsconfig.base.json | 2 +- 18 files changed, 390 insertions(+), 161 deletions(-) create mode 100644 packages/payload/src/query-presets/preventLockout.ts diff --git a/.vscode/launch.json b/.vscode/launch.json index 2bf9d41eb1..c8b6d1051c 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -63,6 +63,13 @@ "request": "launch", "type": "node-terminal" }, + { + "command": "pnpm tsx --no-deprecation test/dev.ts query-presets", + "cwd": "${workspaceFolder}", + "name": "Run Dev Query Presets", + "request": "launch", + "type": "node-terminal" + }, { "command": "pnpm tsx --no-deprecation test/dev.ts login-with-username", "cwd": "${workspaceFolder}", diff --git a/packages/payload/src/admin/RichText.ts b/packages/payload/src/admin/RichText.ts index 375cff4b90..5c9e89d72a 100644 --- a/packages/payload/src/admin/RichText.ts +++ b/packages/payload/src/admin/RichText.ts @@ -108,7 +108,6 @@ export type BeforeChangeRichTextHookArgs< previousSiblingDoc?: TData /** The previous value of the field, before changes */ previousValue?: TValue - /** * The original siblingData with locales (not modified by any hooks). */ diff --git a/packages/payload/src/collections/operations/local/create.ts b/packages/payload/src/collections/operations/local/create.ts index ba7cb94953..a9c558dcc4 100644 --- a/packages/payload/src/collections/operations/local/create.ts +++ b/packages/payload/src/collections/operations/local/create.ts @@ -149,6 +149,7 @@ export default async function createLocal< } const req = await createLocalReq(options, payload) + req.file = file ?? (await getFileByPath(filePath)) return createOperation({ diff --git a/packages/payload/src/query-presets/config.ts b/packages/payload/src/query-presets/config.ts index ba2d3ee6ff..9fd822909e 100644 --- a/packages/payload/src/query-presets/config.ts +++ b/packages/payload/src/query-presets/config.ts @@ -113,6 +113,16 @@ export const getQueryPresetsConfig = (config: Config): CollectionConfig => ({ : [], required: true, }, + { + name: 'isTemp', + type: 'checkbox', + admin: { + description: + "This is a tempoary field used to determine if updating the preset would remove the user's access to it. When `true`, this record will be deleted after running the preset's `validate` function.", + disabled: true, + hidden: true, + }, + }, ], hooks: { beforeValidate: [ diff --git a/packages/payload/src/query-presets/constraints.ts b/packages/payload/src/query-presets/constraints.ts index 33603d2f1d..921749ec12 100644 --- a/packages/payload/src/query-presets/constraints.ts +++ b/packages/payload/src/query-presets/constraints.ts @@ -5,6 +5,7 @@ import type { Field } from '../fields/config/types.js' import { fieldAffectsData } from '../fields/config/types.js' import { toWords } from '../utilities/formatLabels.js' +import { preventLockout } from './preventLockout.js' import { operations, type QueryPresetConstraint } from './types.js' export const getConstraints = (config: Config): Field => ({ @@ -101,4 +102,5 @@ export const getConstraints = (config: Config): Field => ({ label: () => toWords(operation), })), label: 'Sharing settings', + validate: preventLockout, }) diff --git a/packages/payload/src/query-presets/preventLockout.ts b/packages/payload/src/query-presets/preventLockout.ts new file mode 100644 index 0000000000..4f285e1d08 --- /dev/null +++ b/packages/payload/src/query-presets/preventLockout.ts @@ -0,0 +1,92 @@ +import type { Validate } from '../fields/config/types.js' + +import { APIError } from '../errors/APIError.js' +import { createLocalReq } from '../utilities/createLocalReq.js' +import { initTransaction } from '../utilities/initTransaction.js' +import { killTransaction } from '../utilities/killTransaction.js' +import { queryPresetsCollectionSlug } from './config.js' + +/** + * Prevents "accidental lockouts" where a user makes an update that removes their own access to the preset. + * This is effectively an access control function proxied through a `validate` function. + * How it works: + * 1. Creates a temporary record with the incoming data + * 2. Attempts to read and update that record with the incoming user + * 3. If either of those fail, throws an error to the user + * 4. Once finished, prevents the temp record from persisting to the database + */ +export const preventLockout: Validate = async ( + value, + { data, overrideAccess, req: incomingReq }, +) => { + // Use context to ensure an infinite loop doesn't occur + if (!incomingReq.context._preventLockout && !overrideAccess) { + const req = await createLocalReq( + { + context: { + _preventLockout: true, + }, + req: { + user: incomingReq.user, + }, + }, + incomingReq.payload, + ) + + // Might be `null` if no transactions are enabled + const transaction = await initTransaction(req) + + // create a temp record to validate the constraints, using the req + const tempPreset = await req.payload.create({ + collection: queryPresetsCollectionSlug, + data: { + ...data, + isTemp: true, + }, + req, + }) + + let canUpdate = false + let canRead = false + + try { + await req.payload.findByID({ + id: tempPreset.id, + collection: queryPresetsCollectionSlug, + overrideAccess: false, + req, + user: req.user, + }) + + canRead = true + + await req.payload.update({ + id: tempPreset.id, + collection: queryPresetsCollectionSlug, + data: tempPreset, + overrideAccess: false, + req, + user: req.user, + }) + + canUpdate = true + } catch (_err) { + if (!canRead || !canUpdate) { + throw new APIError('Cannot remove yourself from this preset.', 403, {}, true) + } + } finally { + if (transaction) { + await killTransaction(req) + } else { + // delete the temp record + await req.payload.delete({ + id: tempPreset.id, + collection: queryPresetsCollectionSlug, + req, + }) + } + } + } + + return true as unknown as true +} diff --git a/packages/payload/src/utilities/getEntityPolicies.ts b/packages/payload/src/utilities/getEntityPolicies.ts index 104a6ead84..0ec0538b6e 100644 --- a/packages/payload/src/utilities/getEntityPolicies.ts +++ b/packages/payload/src/utilities/getEntityPolicies.ts @@ -78,6 +78,7 @@ export async function getEntityPolicies(args: T): Promise(args: T): Promise(args: T): Promise ({ 'payload-query-presets': { + isTemp: { + not_equals: true, + }, relatedCollection: { equals: collectionSlug, }, diff --git a/packages/ui/src/elements/QueryPresets/fields/WhereField/index.tsx b/packages/ui/src/elements/QueryPresets/fields/WhereField/index.tsx index a44fd868bb..9220c0dccf 100644 --- a/packages/ui/src/elements/QueryPresets/fields/WhereField/index.tsx +++ b/packages/ui/src/elements/QueryPresets/fields/WhereField/index.tsx @@ -104,7 +104,7 @@ export const QueryPresetsWhereField: JSONFieldClientComponent = ({ {value ? transformWhereToNaturalLanguage( value as Where, - getTranslation(collectionConfig.labels.plural, i18n), + getTranslation(collectionConfig?.labels?.plural, i18n), ) : 'No where query'} diff --git a/test/query-presets/collections/Pages/index.ts b/test/query-presets/collections/Pages/index.ts index 7bd8a3659a..f470d7c64e 100644 --- a/test/query-presets/collections/Pages/index.ts +++ b/test/query-presets/collections/Pages/index.ts @@ -15,7 +15,4 @@ export const Pages: CollectionConfig = { type: 'text', }, ], - versions: { - drafts: true, - }, } diff --git a/test/query-presets/collections/Posts/index.ts b/test/query-presets/collections/Posts/index.ts index 84aec17300..2cfd03122d 100644 --- a/test/query-presets/collections/Posts/index.ts +++ b/test/query-presets/collections/Posts/index.ts @@ -15,7 +15,4 @@ export const Posts: CollectionConfig = { type: 'text', }, ], - versions: { - drafts: true, - }, } diff --git a/test/query-presets/config.ts b/test/query-presets/config.ts index ebf5dd1119..294549b952 100644 --- a/test/query-presets/config.ts +++ b/test/query-presets/config.ts @@ -23,10 +23,8 @@ export default buildConfigWithDefaults({ // plural: 'Reports', // }, access: { - read: ({ req: { user } }) => - user ? user && !user?.roles?.some((role) => role === 'anonymous') : false, - update: ({ req: { user } }) => - user ? user && !user?.roles?.some((role) => role === 'anonymous') : false, + read: ({ req: { user } }) => Boolean(user?.roles?.length && !user?.roles?.includes('user')), + update: ({ req: { user } }) => Boolean(user?.roles?.length && !user?.roles?.includes('user')), }, constraints: { read: [ @@ -60,7 +58,7 @@ export default buildConfigWithDefaults({ ], }, }, - collections: [Pages, Users, Posts], + collections: [Pages, Posts, Users], onInit: async (payload) => { if (process.env.SEED_IN_CONFIG_ONINIT !== 'false') { await seed(payload) diff --git a/test/query-presets/e2e.spec.ts b/test/query-presets/e2e.spec.ts index 6f2cc43805..866ddee039 100644 --- a/test/query-presets/e2e.spec.ts +++ b/test/query-presets/e2e.spec.ts @@ -8,7 +8,7 @@ import * as path from 'path' import { fileURLToPath } from 'url' import type { PayloadTestSDK } from '../helpers/sdk/index.js' -import type { Config } from './payload-types.js' +import type { Config, PayloadQueryPreset } from './payload-types.js' import { ensureCompilationIsDone, @@ -39,6 +39,13 @@ let serverURL: string let everyoneID: string | undefined let context: BrowserContext let user: any +let ownerUser: any + +let seededData: { + everyone: PayloadQueryPreset + onlyMe: PayloadQueryPreset + specificUsers: PayloadQueryPreset +} describe('Query Presets', () => { beforeAll(async ({ browser }, testInfo) => { @@ -60,6 +67,19 @@ describe('Query Presets', () => { }) ?.then((res) => res.user) // TODO: this type is wrong + ownerUser = await payload + .find({ + collection: 'users', + where: { + name: { + equals: 'Owner', + }, + }, + limit: 1, + depth: 0, + }) + ?.then((res) => res.docs[0]) + initPageConsoleErrorCatch(page) await ensureCompilationIsDone({ page, serverURL }) @@ -83,7 +103,7 @@ describe('Query Presets', () => { }, }) - const [, everyone] = await Promise.all([ + const [, everyone, onlyMe, specificUsers] = await Promise.all([ payload.delete({ collection: 'payload-preferences', where: { @@ -106,18 +126,24 @@ describe('Query Presets', () => { }), payload.create({ collection: 'payload-query-presets', - data: seedData.everyone, + data: seedData.everyone({ ownerUserID: ownerUser?.id || '' }), }), payload.create({ collection: 'payload-query-presets', - data: seedData.onlyMe, + data: seedData.onlyMe({ ownerUserID: ownerUser?.id || '' }), }), payload.create({ collection: 'payload-query-presets', - data: seedData.specificUsers({ userID: user?.id || '' }), + data: seedData.specificUsers({ ownerUserID: ownerUser?.id || '', adminUserID: user.id }), }), ]) + seededData = { + everyone, + onlyMe, + specificUsers, + } + everyoneID = everyone.id } catch (error) { console.error('Error in beforeEach:', error) @@ -126,12 +152,12 @@ describe('Query Presets', () => { test('should select preset and apply filters', async () => { await page.goto(pagesUrl.list) - await selectPreset({ page, presetTitle: seedData.everyone.title }) + await selectPreset({ page, presetTitle: seededData.everyone.title }) await assertURLParams({ page, - columns: seedData.everyone.columns, - where: seedData.everyone.where, + columns: seededData.everyone.columns, + where: seededData.everyone.where, presetID: everyoneID, }) @@ -140,14 +166,14 @@ describe('Query Presets', () => { test('should clear selected preset and reset filters', async () => { await page.goto(pagesUrl.list) - await selectPreset({ page, presetTitle: seedData.everyone.title }) + await selectPreset({ page, presetTitle: seededData.everyone.title }) await clearSelectedPreset({ page }) expect(true).toBe(true) }) test('should delete a preset, clear selection, and reset changes', async () => { await page.goto(pagesUrl.list) - await selectPreset({ page, presetTitle: seedData.everyone.title }) + await selectPreset({ page, presetTitle: seededData.everyone.title }) await openListMenu({ page }) await clickListMenuItem({ page, menuItemLabel: 'Delete' }) @@ -172,21 +198,21 @@ describe('Query Presets', () => { await expect( modal.locator('tbody tr td button', { - hasText: exactText(seedData.everyone.title), + hasText: exactText(seededData.everyone.title), }), ).toBeHidden() }) test('should save last used preset to preferences and load on initial render', async () => { await page.goto(pagesUrl.list) - await selectPreset({ page, presetTitle: seedData.everyone.title }) + await selectPreset({ page, presetTitle: seededData.everyone.title }) await page.reload() await assertURLParams({ page, - columns: seedData.everyone.columns, - where: seedData.everyone.where, + columns: seededData.everyone.columns, + where: seededData.everyone.where, // presetID: everyoneID, }) @@ -209,7 +235,7 @@ describe('Query Presets', () => { }), ).toBeHidden() - await selectPreset({ page, presetTitle: seedData.everyone.title }) + await selectPreset({ page, presetTitle: seededData.everyone.title }) await openListMenu({ page }) @@ -249,7 +275,7 @@ describe('Query Presets', () => { }), ).toBeHidden() - await selectPreset({ page, presetTitle: seedData.onlyMe.title }) + await selectPreset({ page, presetTitle: seededData.onlyMe.title }) await toggleColumn(page, { columnLabel: 'ID' }) @@ -271,7 +297,7 @@ describe('Query Presets', () => { test('should conditionally render "update for everyone" label based on if preset is shared', async () => { await page.goto(pagesUrl.list) - await selectPreset({ page, presetTitle: seedData.onlyMe.title }) + await selectPreset({ page, presetTitle: seededData.onlyMe.title }) await toggleColumn(page, { columnLabel: 'ID' }) @@ -284,7 +310,7 @@ describe('Query Presets', () => { }), ).toBeVisible() - await selectPreset({ page, presetTitle: seedData.everyone.title }) + await selectPreset({ page, presetTitle: seededData.everyone.title }) await toggleColumn(page, { columnLabel: 'ID' }) @@ -300,7 +326,7 @@ describe('Query Presets', () => { test('should reset active changes', async () => { await page.goto(pagesUrl.list) - await selectPreset({ page, presetTitle: seedData.everyone.title }) + await selectPreset({ page, presetTitle: seededData.everyone.title }) const { columnContainer } = await toggleColumn(page, { columnLabel: 'ID' }) @@ -318,7 +344,7 @@ describe('Query Presets', () => { test('should only enter modified state when changes are made to an active preset', async () => { await page.goto(pagesUrl.list) await expect(page.locator('.list-controls__modified')).toBeHidden() - await selectPreset({ page, presetTitle: seedData.everyone.title }) + await selectPreset({ page, presetTitle: seededData.everyone.title }) await expect(page.locator('.list-controls__modified')).toBeHidden() await toggleColumn(page, { columnLabel: 'ID' }) await expect(page.locator('.list-controls__modified')).toBeVisible() @@ -337,14 +363,14 @@ describe('Query Presets', () => { await page.goto(pagesUrl.list) - await selectPreset({ page, presetTitle: seedData.everyone.title }) + await selectPreset({ page, presetTitle: seededData.everyone.title }) await clickListMenuItem({ page, menuItemLabel: 'Edit' }) const drawer = page.locator('[id^=doc-drawer_payload-query-presets_0_]') const titleValue = drawer.locator('input[name="title"]') - await expect(titleValue).toHaveValue(seedData.everyone.title) + await expect(titleValue).toHaveValue(seededData.everyone.title) - const newTitle = `${seedData.everyone.title} (Updated)` + const newTitle = `${seededData.everyone.title} (Updated)` await drawer.locator('input[name="title"]').fill(newTitle) await saveDocAndAssert(page) @@ -391,9 +417,9 @@ describe('Query Presets', () => { }) test('only shows query presets related to the underlying collection', async () => { - // no results on `users` collection - const postsUrl = new AdminUrlUtil(serverURL, 'posts') - await page.goto(postsUrl.list) + // no results on `posts` collection + const postsURL = new AdminUrlUtil(serverURL, 'posts') + await page.goto(postsURL.list) const drawer = await openQueryPresetDrawer({ page }) await expect(drawer.locator('.table table > tbody > tr')).toHaveCount(0) await expect(drawer.locator('.collection-list__no-results')).toBeVisible() diff --git a/test/query-presets/fields/roles.ts b/test/query-presets/fields/roles.ts index ad5ff3a66c..9304cb2afc 100644 --- a/test/query-presets/fields/roles.ts +++ b/test/query-presets/fields/roles.ts @@ -9,13 +9,13 @@ export const roles: Field = { label: 'Admin', value: 'admin', }, + { + label: 'Editor', + value: 'editor', + }, { label: 'User', value: 'user', }, - { - label: 'Anonymous', - value: 'anonymous', - }, ], } diff --git a/test/query-presets/int.spec.ts b/test/query-presets/int.spec.ts index 42bd771ac1..6fdc915e52 100644 --- a/test/query-presets/int.spec.ts +++ b/test/query-presets/int.spec.ts @@ -1,4 +1,3 @@ -import type { NextRESTClient } from 'helpers/NextRESTClient.js' import type { Payload, User } from 'payload' import path from 'path' @@ -10,10 +9,9 @@ import { initPayloadInt } from '../helpers/initPayloadInt.js' const queryPresetsCollectionSlug = 'payload-query-presets' let payload: Payload -let restClient: NextRESTClient -let user: User -let user2: User -let anonymousUser: User +let adminUser: User +let editorUser: User +let publicUser: User const filename = fileURLToPath(import.meta.url) const dirname = path.dirname(filename) @@ -21,9 +19,9 @@ const dirname = path.dirname(filename) describe('Query Presets', () => { beforeAll(async () => { // @ts-expect-error: initPayloadInt does not have a proper type definition - ;({ payload, restClient } = await initPayloadInt(dirname)) + ;({ payload } = await initPayloadInt(dirname)) - user = await payload + adminUser = await payload .login({ collection: 'users', data: { @@ -33,7 +31,7 @@ describe('Query Presets', () => { }) ?.then((result) => result.user) - user2 = await payload + editorUser = await payload .login({ collection: 'users', data: { @@ -43,11 +41,11 @@ describe('Query Presets', () => { }) ?.then((result) => result.user) - anonymousUser = await payload + publicUser = await payload .login({ collection: 'users', data: { - email: 'anonymous@email.com', + email: 'public@email.com', password: regularUser.password, }, }) @@ -155,7 +153,8 @@ describe('Query Presets', () => { it('should respect access when set to "specificUsers"', async () => { const presetForSpecificUsers = await payload.create({ collection: queryPresetsCollectionSlug, - user, + user: adminUser, + overrideAccess: false, data: { title: 'Specific Users', where: { @@ -166,11 +165,11 @@ describe('Query Presets', () => { access: { read: { constraint: 'specificUsers', - users: [user.id], + users: [adminUser.id], }, update: { constraint: 'specificUsers', - users: [user.id], + users: [adminUser.id], }, }, relatedCollection: 'pages', @@ -180,7 +179,7 @@ describe('Query Presets', () => { const foundPresetWithUser1 = await payload.findByID({ collection: queryPresetsCollectionSlug, depth: 0, - user, + user: adminUser, overrideAccess: false, id: presetForSpecificUsers.id, }) @@ -188,53 +187,53 @@ describe('Query Presets', () => { expect(foundPresetWithUser1.id).toBe(presetForSpecificUsers.id) try { - const foundPresetWithUser2 = await payload.findByID({ + const foundPresetWithEditorUser = await payload.findByID({ collection: queryPresetsCollectionSlug, depth: 0, - user: user2, + user: editorUser, overrideAccess: false, id: presetForSpecificUsers.id, }) - expect(foundPresetWithUser2).toBeFalsy() + expect(foundPresetWithEditorUser).toBeFalsy() } catch (error: unknown) { expect((error as Error).message).toBe('Not Found') } - const presetUpdatedByUser1 = await payload.update({ + const presetUpdatedByAdminUser = await payload.update({ collection: queryPresetsCollectionSlug, id: presetForSpecificUsers.id, - user, + user: adminUser, overrideAccess: false, data: { title: 'Specific Users (Updated)', }, }) - expect(presetUpdatedByUser1.title).toBe('Specific Users (Updated)') + expect(presetUpdatedByAdminUser.title).toBe('Specific Users (Updated)') try { - const presetUpdatedByUser2 = await payload.update({ + const presetUpdatedByEditorUser = await payload.update({ collection: queryPresetsCollectionSlug, id: presetForSpecificUsers.id, - user: user2, + user: editorUser, overrideAccess: false, data: { title: 'Specific Users (Updated)', }, }) - expect(presetUpdatedByUser2).toBeFalsy() + expect(presetUpdatedByEditorUser).toBeFalsy() } catch (error: unknown) { expect((error as Error).message).toBe('You are not allowed to perform this action.') } }) it('should respect access when set to "onlyMe"', async () => { - // create a new doc so that the creating user is the owner const presetForOnlyMe = await payload.create({ collection: queryPresetsCollectionSlug, - user, + overrideAccess: false, + user: adminUser, data: { title: 'Only Me', where: { @@ -257,7 +256,7 @@ describe('Query Presets', () => { const foundPresetWithUser1 = await payload.findByID({ collection: queryPresetsCollectionSlug, depth: 0, - user, + user: adminUser, overrideAccess: false, id: presetForOnlyMe.id, }) @@ -265,15 +264,15 @@ describe('Query Presets', () => { expect(foundPresetWithUser1.id).toBe(presetForOnlyMe.id) try { - const foundPresetWithUser2 = await payload.findByID({ + const foundPresetWithEditorUser = await payload.findByID({ collection: queryPresetsCollectionSlug, depth: 0, - user: user2, + user: editorUser, overrideAccess: false, id: presetForOnlyMe.id, }) - expect(foundPresetWithUser2).toBeFalsy() + expect(foundPresetWithEditorUser).toBeFalsy() } catch (error: unknown) { expect((error as Error).message).toBe('Not Found') } @@ -281,7 +280,7 @@ describe('Query Presets', () => { const presetUpdatedByUser1 = await payload.update({ collection: queryPresetsCollectionSlug, id: presetForOnlyMe.id, - user, + user: adminUser, overrideAccess: false, data: { title: 'Only Me (Updated)', @@ -291,17 +290,17 @@ describe('Query Presets', () => { expect(presetUpdatedByUser1.title).toBe('Only Me (Updated)') try { - const presetUpdatedByUser2 = await payload.update({ + const presetUpdatedByEditorUser = await payload.update({ collection: queryPresetsCollectionSlug, id: presetForOnlyMe.id, - user: user2, + user: editorUser, overrideAccess: false, data: { title: 'Only Me (Updated)', }, }) - expect(presetUpdatedByUser2).toBeFalsy() + expect(presetUpdatedByEditorUser).toBeFalsy() } catch (error: unknown) { expect((error as Error).message).toBe('You are not allowed to perform this action.') } @@ -310,7 +309,8 @@ describe('Query Presets', () => { it('should respect access when set to "everyone"', async () => { const presetForEveryone = await payload.create({ collection: queryPresetsCollectionSlug, - user, + overrideAccess: false, + user: adminUser, data: { title: 'Everyone', where: { @@ -336,27 +336,27 @@ describe('Query Presets', () => { const foundPresetWithUser1 = await payload.findByID({ collection: queryPresetsCollectionSlug, depth: 0, - user, + user: adminUser, overrideAccess: false, id: presetForEveryone.id, }) expect(foundPresetWithUser1.id).toBe(presetForEveryone.id) - const foundPresetWithUser2 = await payload.findByID({ + const foundPresetWithEditorUser = await payload.findByID({ collection: queryPresetsCollectionSlug, depth: 0, - user: user2, + user: editorUser, overrideAccess: false, id: presetForEveryone.id, }) - expect(foundPresetWithUser2.id).toBe(presetForEveryone.id) + expect(foundPresetWithEditorUser.id).toBe(presetForEveryone.id) const presetUpdatedByUser1 = await payload.update({ collection: queryPresetsCollectionSlug, id: presetForEveryone.id, - user, + user: adminUser, overrideAccess: false, data: { title: 'Everyone (Update 1)', @@ -365,17 +365,105 @@ describe('Query Presets', () => { expect(presetUpdatedByUser1.title).toBe('Everyone (Update 1)') - const presetUpdatedByUser2 = await payload.update({ + const presetUpdatedByEditorUser = await payload.update({ collection: queryPresetsCollectionSlug, id: presetForEveryone.id, - user: user2, + user: editorUser, overrideAccess: false, data: { title: 'Everyone (Update 2)', }, }) - expect(presetUpdatedByUser2.title).toBe('Everyone (Update 2)') + expect(presetUpdatedByEditorUser.title).toBe('Everyone (Update 2)') + }) + + it('should prevent accidental lockout', async () => { + // attempt to create a preset without access to read or update + try { + 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: [], + }, + }, + }, + }) + + expect(presetWithoutAccess).toBeFalsy() + } catch (error: unknown) { + expect((error as Error).message).toBe('Cannot remove yourself from this preset.') + } + + const presetWithUser1 = await payload.create({ + collection: queryPresetsCollectionSlug, + user: adminUser, + overrideAccess: false, + data: { + title: 'Prevent Lockout', + relatedCollection: 'pages', + access: { + read: { + constraint: 'specificUsers', + users: [adminUser.id], + }, + update: { + constraint: 'specificUsers', + users: [adminUser.id], + }, + delete: { + constraint: 'specificUsers', + users: [adminUser.id], + }, + }, + }, + }) + + // attempt to update the preset to lock the user out of access + try { + const presetUpdatedByUser1 = await payload.update({ + collection: queryPresetsCollectionSlug, + id: presetWithUser1.id, + user: adminUser, + overrideAccess: false, + data: { + title: 'Prevent Lockout (Updated)', + access: { + read: { + constraint: 'specificUsers', + users: [], + }, + update: { + constraint: 'specificUsers', + users: [], + }, + delete: { + constraint: 'specificUsers', + users: [], + }, + }, + }, + }) + + expect(presetUpdatedByUser1).toBeFalsy() + } catch (error: unknown) { + expect((error as Error).message).toBe('Cannot remove yourself from this preset.') + } }) }) @@ -383,7 +471,8 @@ describe('Query Presets', () => { it('should respect top-level access control overrides', async () => { const preset = await payload.create({ collection: queryPresetsCollectionSlug, - user, + user: adminUser, + overrideAccess: false, data: { title: 'Top-Level Access Control Override', relatedCollection: 'pages', @@ -404,7 +493,7 @@ describe('Query Presets', () => { const foundPresetWithUser1 = await payload.findByID({ collection: queryPresetsCollectionSlug, depth: 0, - user, + user: adminUser, overrideAccess: false, id: preset.id, }) @@ -412,15 +501,15 @@ describe('Query Presets', () => { expect(foundPresetWithUser1.id).toBe(preset.id) try { - const foundPresetWithAnonymousUser = await payload.findByID({ + const foundPresetWithPublicUser = await payload.findByID({ collection: queryPresetsCollectionSlug, depth: 0, - user: anonymousUser, + user: publicUser, overrideAccess: false, id: preset.id, }) - expect(foundPresetWithAnonymousUser).toBeFalsy() + expect(foundPresetWithPublicUser).toBeFalsy() } catch (error: unknown) { expect((error as Error).message).toBe('You are not allowed to perform this action.') } @@ -429,7 +518,8 @@ describe('Query Presets', () => { it('should respect access when set to "specificRoles"', async () => { const presetForSpecificRoles = await payload.create({ collection: queryPresetsCollectionSlug, - user, + user: adminUser, + overrideAccess: false, data: { title: 'Specific Roles', where: { @@ -454,7 +544,7 @@ describe('Query Presets', () => { const foundPresetWithUser1 = await payload.findByID({ collection: queryPresetsCollectionSlug, depth: 0, - user, + user: adminUser, overrideAccess: false, id: presetForSpecificRoles.id, }) @@ -462,15 +552,15 @@ describe('Query Presets', () => { expect(foundPresetWithUser1.id).toBe(presetForSpecificRoles.id) try { - const foundPresetWithUser2 = await payload.findByID({ + const foundPresetWithEditorUser = await payload.findByID({ collection: queryPresetsCollectionSlug, depth: 0, - user: user2, + user: editorUser, overrideAccess: false, id: presetForSpecificRoles.id, }) - expect(foundPresetWithUser2).toBeFalsy() + expect(foundPresetWithEditorUser).toBeFalsy() } catch (error: unknown) { expect((error as Error).message).toBe('Not Found') } @@ -478,7 +568,7 @@ describe('Query Presets', () => { const presetUpdatedByUser1 = await payload.update({ collection: queryPresetsCollectionSlug, id: presetForSpecificRoles.id, - user, + user: adminUser, overrideAccess: false, data: { title: 'Specific Roles (Updated)', @@ -488,17 +578,17 @@ describe('Query Presets', () => { expect(presetUpdatedByUser1.title).toBe('Specific Roles (Updated)') try { - const presetUpdatedByUser2 = await payload.update({ + const presetUpdatedByEditorUser = await payload.update({ collection: queryPresetsCollectionSlug, id: presetForSpecificRoles.id, - user: user2, + user: editorUser, overrideAccess: false, data: { title: 'Specific Roles (Updated)', }, }) - expect(presetUpdatedByUser2).toBeFalsy() + expect(presetUpdatedByEditorUser).toBeFalsy() } catch (error: unknown) { expect((error as Error).message).toBe('You are not allowed to perform this action.') } @@ -508,7 +598,7 @@ describe('Query Presets', () => { // create a preset with the read constraint set to "noone" const presetForNoone = await payload.create({ collection: queryPresetsCollectionSlug, - user, + user: adminUser, data: { relatedCollection: 'pages', title: 'Noone', @@ -529,7 +619,7 @@ describe('Query Presets', () => { const foundPresetWithUser1 = await payload.findByID({ collection: queryPresetsCollectionSlug, depth: 0, - user, + user: adminUser, overrideAccess: false, id: presetForNoone.id, }) @@ -545,7 +635,8 @@ describe('Query Presets', () => { try { const result = await payload.create({ collection: 'payload-query-presets', - user, + user: adminUser, + overrideAccess: false, data: { title: 'Disabled Query Presets', relatedCollection: 'pages', @@ -563,7 +654,8 @@ describe('Query Presets', () => { it('transforms "where" query objects into the "and" / "or" format', async () => { const result = await payload.create({ collection: queryPresetsCollectionSlug, - user, + user: adminUser, + overrideAccess: false, data: { title: 'Where Object Formatting', where: { diff --git a/test/query-presets/payload-types.ts b/test/query-presets/payload-types.ts index 3918740441..61c9398cac 100644 --- a/test/query-presets/payload-types.ts +++ b/test/query-presets/payload-types.ts @@ -68,8 +68,8 @@ export interface Config { blocks: {}; collections: { pages: Page; - users: User; posts: Post; + users: User; 'payload-locked-documents': PayloadLockedDocument; 'payload-preferences': PayloadPreference; 'payload-migrations': PayloadMigration; @@ -78,8 +78,8 @@ export interface Config { collectionsJoins: {}; collectionsSelect: { pages: PagesSelect | PagesSelect; - users: UsersSelect | UsersSelect; posts: PostsSelect | PostsSelect; + users: UsersSelect | UsersSelect; 'payload-locked-documents': PayloadLockedDocumentsSelect | PayloadLockedDocumentsSelect; 'payload-preferences': PayloadPreferencesSelect | PayloadPreferencesSelect; 'payload-migrations': PayloadMigrationsSelect | PayloadMigrationsSelect; @@ -126,7 +126,16 @@ export interface Page { text?: string | null; updatedAt: string; createdAt: string; - _status?: ('draft' | 'published') | null; +} +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "posts". + */ +export interface Post { + id: string; + text?: string | null; + updatedAt: string; + createdAt: string; } /** * This interface was referenced by `Config`'s JSON-Schema @@ -135,7 +144,7 @@ export interface Page { export interface User { id: string; name?: string | null; - roles?: ('admin' | 'user' | 'anonymous')[] | null; + roles?: ('admin' | 'editor' | 'user')[] | null; updatedAt: string; createdAt: string; email: string; @@ -147,17 +156,6 @@ export interface User { lockUntil?: string | null; password?: string | null; } -/** - * This interface was referenced by `Config`'s JSON-Schema - * via the `definition` "posts". - */ -export interface Post { - id: string; - text?: string | null; - updatedAt: string; - createdAt: string; - _status?: ('draft' | 'published') | null; -} /** * This interface was referenced by `Config`'s JSON-Schema * via the `definition` "payload-locked-documents". @@ -169,13 +167,13 @@ export interface PayloadLockedDocument { relationTo: 'pages'; value: string | Page; } | null) - | ({ - relationTo: 'users'; - value: string | User; - } | null) | ({ relationTo: 'posts'; value: string | Post; + } | null) + | ({ + relationTo: 'users'; + value: string | User; } | null); globalSlug?: string | null; user: { @@ -231,12 +229,12 @@ export interface PayloadQueryPreset { read?: { constraint?: ('everyone' | 'onlyMe' | 'specificUsers' | 'specificRoles' | 'noone') | null; users?: (string | User)[] | null; - roles?: ('admin' | 'user' | 'anonymous')[] | null; + roles?: ('admin' | 'editor' | 'user')[] | null; }; update?: { constraint?: ('everyone' | 'onlyMe' | 'specificUsers' | 'specificRoles') | null; users?: (string | User)[] | null; - roles?: ('admin' | 'user' | 'anonymous')[] | null; + roles?: ('admin' | 'editor' | 'user')[] | null; }; delete?: { constraint?: ('everyone' | 'onlyMe' | 'specificUsers') | null; @@ -262,6 +260,10 @@ export interface PayloadQueryPreset { | boolean | null; relatedCollection: 'pages' | 'posts'; + /** + * This is a tempoary field used to determine if updating the preset would remove the user's access to it. When `true`, this record will be deleted after running the preset's `validate` function. + */ + isTemp?: boolean | null; updatedAt: string; createdAt: string; } @@ -273,7 +275,15 @@ export interface PagesSelect { text?: T; updatedAt?: T; createdAt?: T; - _status?: T; +} +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "posts_select". + */ +export interface PostsSelect { + text?: T; + updatedAt?: T; + createdAt?: T; } /** * This interface was referenced by `Config`'s JSON-Schema @@ -292,16 +302,6 @@ export interface UsersSelect { loginAttempts?: T; lockUntil?: T; } -/** - * This interface was referenced by `Config`'s JSON-Schema - * via the `definition` "posts_select". - */ -export interface PostsSelect { - text?: T; - updatedAt?: T; - createdAt?: T; - _status?: T; -} /** * This interface was referenced by `Config`'s JSON-Schema * via the `definition` "payload-locked-documents_select". @@ -368,6 +368,7 @@ export interface PayloadQueryPresetsSelect { where?: T; columns?: T; relatedCollection?: T; + isTemp?: T; updatedAt?: T; createdAt?: T; } diff --git a/test/query-presets/seed.ts b/test/query-presets/seed.ts index 029c283919..1027fa22e3 100644 --- a/test/query-presets/seed.ts +++ b/test/query-presets/seed.ts @@ -10,11 +10,11 @@ type SeededQueryPreset = { } & Omit export const seedData: { - everyone: SeededQueryPreset - onlyMe: SeededQueryPreset - specificUsers: (args: { userID: string }) => SeededQueryPreset + everyone: () => SeededQueryPreset + onlyMe: () => SeededQueryPreset + specificUsers: (args: { adminUserID: string }) => SeededQueryPreset } = { - onlyMe: { + onlyMe: () => ({ relatedCollection: pagesSlug, isShared: false, title: 'Only Me', @@ -40,8 +40,8 @@ export const seedData: { equals: 'example page', }, }, - }, - everyone: { + }), + everyone: () => ({ relatedCollection: pagesSlug, isShared: true, title: 'Everyone', @@ -67,8 +67,8 @@ export const seedData: { equals: 'example page', }, }, - }, - specificUsers: ({ userID }: { userID: string }) => ({ + }), + specificUsers: ({ adminUserID }: { adminUserID: string }) => ({ title: 'Specific Users', isShared: true, where: { @@ -79,15 +79,15 @@ export const seedData: { access: { read: { constraint: 'specificUsers', - users: [userID], + users: [adminUserID], }, update: { constraint: 'specificUsers', - users: [userID], + users: [adminUserID], }, delete: { constraint: 'specificUsers', - users: [userID], + users: [adminUserID], }, }, columns: [ @@ -101,7 +101,7 @@ export const seedData: { } export const seed = async (_payload: Payload) => { - const [devUser] = await executePromises( + const [adminUser] = await executePromises( [ () => _payload.create({ @@ -119,18 +119,18 @@ export const seed = async (_payload: Payload) => { data: { email: regularCredentials.email, password: regularCredentials.password, - name: 'User', - roles: ['user'], + name: 'Editor', + roles: ['editor'], }, }), () => _payload.create({ collection: usersSlug, data: { - email: 'anonymous@email.com', + email: 'public@email.com', password: regularCredentials.password, - name: 'User', - roles: ['anonymous'], + name: 'Public User', + roles: ['user'], }, }), ], @@ -149,29 +149,30 @@ export const seed = async (_payload: Payload) => { () => _payload.create({ collection: 'payload-query-presets', - user: devUser, + user: adminUser, overrideAccess: false, - data: seedData.specificUsers({ userID: devUser?.id || '' }), + data: seedData.specificUsers({ + adminUserID: adminUser?.id || '', + }), }), () => _payload.create({ collection: 'payload-query-presets', - user: devUser, + user: adminUser, overrideAccess: false, - data: seedData.everyone, + data: seedData.everyone(), }), () => _payload.create({ collection: 'payload-query-presets', - user: devUser, + user: adminUser, overrideAccess: false, - data: seedData.onlyMe, + data: seedData.onlyMe(), }), () => _payload.create({ collection: 'payload-query-presets', - user: devUser, - overrideAccess: false, + user: adminUser, data: { relatedCollection: 'pages', title: 'Noone', diff --git a/tsconfig.base.json b/tsconfig.base.json index 12877b1be6..b7c0ef5d5c 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -31,7 +31,7 @@ } ], "paths": { - "@payload-config": ["./test/_community/config.ts"], + "@payload-config": ["./test/query-presets/config.ts"], "@payloadcms/admin-bar": ["./packages/admin-bar/src"], "@payloadcms/live-preview": ["./packages/live-preview/src"], "@payloadcms/live-preview-react": ["./packages/live-preview-react/src/index.ts"],