From 64f6011ba4bc39445a2d471489024dab6dbdab50 Mon Sep 17 00:00:00 2001 From: Jacob Fletcher Date: Tue, 20 May 2025 13:55:02 -0400 Subject: [PATCH] prevents non-admins from changing admin role --- docs/query-presets/overview.mdx | 35 +++++++++++++----- test/query-presets/config.ts | 34 ++++++++--------- test/query-presets/int.spec.ts | 65 +++++++++++++-------------------- 3 files changed, 66 insertions(+), 68 deletions(-) diff --git a/docs/query-presets/overview.mdx b/docs/query-presets/overview.mdx index 0bf9224269..1604e5ddd4 100644 --- a/docs/query-presets/overview.mdx +++ b/docs/query-presets/overview.mdx @@ -192,19 +192,34 @@ const config = buildConfig({ hooks: { beforeValidate: [ // this is a custom `beforeValidate` hook that runs before the preset is validated - // it ensures that if the user is trying to change a constraint to "everyone", they must be an admin + // it ensures that only admins can add or remove the "admin" role from a preset ({ data, req, originalDoc }) => { - const isSharingWithEveryone = - (data?.access?.read?.constraint === 'everyone' && - (!originalDoc || - originalDoc?.access?.read?.constraint !== 'everyone')) || - (data?.access?.update?.constraint === 'everyone' && - (!originalDoc || - originalDoc?.access?.update?.constraint !== 'everyone')) + const adminRoleChanged = (current, original) => { + const currentHasAdmin = current?.roles?.includes('admin') ?? false + const originalHasAdmin = original?.roles?.includes('admin') ?? false + return currentHasAdmin !== originalHasAdmin + } - if (isSharingWithEveryone && !req.user?.roles?.includes('admin')) { + const readChanged = + data?.access?.read?.constraint === 'specificRoles' && + adminRoleChanged( + data?.access?.read, + originalDoc?.access?.read || {}, + ) + + const updateChanged = + data?.access?.update?.constraint === 'specificRoles' && + adminRoleChanged( + data?.access?.update, + originalDoc?.access?.update || {}, + ) + + if ( + (readChanged || updateChanged) && + !req.user?.roles?.includes('admin') + ) { throw new APIError( - 'You must be an admin to share this preset with everyone.', + 'You must be an admin to add or remove the admin role from a preset', 403, {}, true, diff --git a/test/query-presets/config.ts b/test/query-presets/config.ts index 23437488c3..be13be7511 100644 --- a/test/query-presets/config.ts +++ b/test/query-presets/config.ts @@ -30,17 +30,25 @@ export default buildConfigWithDefaults({ hooks: { beforeValidate: [ // this is a custom `beforeValidate` hook that runs before the preset is validated - // it ensures that if the user is trying to change a constraint to "onlyAdmins", they must be an admin themselves + // it ensures that only admins can add or remove the "admin" role from a preset ({ data, req, originalDoc }) => { - const isSharingWithAdmins = - (data?.access?.read?.constraint === 'onlyAdmins' && - (!originalDoc || originalDoc?.access?.read?.constraint !== 'onlyAdmins')) || - (data?.access?.update?.constraint === 'onlyAdmins' && - (!originalDoc || originalDoc?.access?.update?.constraint !== 'onlyAdmins')) + const adminRoleChanged = (current, original) => { + const currentHasAdmin = current?.roles?.includes('admin') ?? false + const originalHasAdmin = original?.roles?.includes('admin') ?? false + return currentHasAdmin !== originalHasAdmin + } - if (isSharingWithAdmins && !req.user?.roles?.includes('admin')) { + const readRoleChanged = + data?.access?.read?.constraint === 'specificRoles' && + adminRoleChanged(data?.access?.read, originalDoc?.access?.read || {}) + + const updateRoleChanged = + data?.access?.update?.constraint === 'specificRoles' && + adminRoleChanged(data?.access?.update, originalDoc?.access?.update || {}) + + if ((readRoleChanged || updateRoleChanged) && !req.user?.roles?.includes('admin')) { throw new APIError( - 'You must be an admin to share this preset with admins.', + 'You must be an admin to add or remove the "admin" role from a preset.', 403, {}, true, @@ -68,11 +76,6 @@ export default buildConfigWithDefaults({ value: 'noone', access: () => false, }, - { - label: 'Only Admins', - value: 'onlyAdmins', - access: ({ req }) => Boolean(req.user?.roles?.includes('admin')), - }, ], update: [ { @@ -85,11 +88,6 @@ export default buildConfigWithDefaults({ }, }), }, - { - label: 'Only Admins', - value: 'onlyAdmins', - access: ({ req }) => Boolean(req.user?.roles?.includes('admin')), - }, ], }, }, diff --git a/test/query-presets/int.spec.ts b/test/query-presets/int.spec.ts index d1090b950f..2adaf2a816 100644 --- a/test/query-presets/int.spec.ts +++ b/test/query-presets/int.spec.ts @@ -515,16 +515,16 @@ describe('Query Presets', () => { } }) - it('should run custom hooks that can control access', async () => { + it('should only allow admins to share presets with other admins (custom hook)', async () => { // create a preset with the read constraint set to "admins" // ensure that _ONLY_ other admins can do this try { - const presetForAdmins = await payload.create({ + const presetForEditorsAndAdmins = await payload.create({ collection: queryPresetsCollectionSlug, user: editorUser, overrideAccess: false, data: { - title: 'Admins', + title: 'Editors and Admins', where: { text: { equals: 'example page', @@ -532,29 +532,31 @@ describe('Query Presets', () => { }, access: { read: { - constraint: 'onlyAdmins', + constraint: 'specificRoles', + roles: ['admin', 'editor'], }, update: { - constraint: 'onlyAdmins', + constraint: 'specificRoles', + roles: ['admin', 'editor'], }, }, relatedCollection: 'pages', }, }) - expect(presetForAdmins).toBeFalsy() + expect(presetForEditorsAndAdmins).toBeFalsy() } catch (error: unknown) { expect((error as Error).message).toBe( - 'You must be an admin to share this preset with admins.', + 'You must be an admin to add or remove the "admin" role from a preset.', ) } - const presetForEveryone = await payload.create({ + const presetForEditors = await payload.create({ collection: queryPresetsCollectionSlug, - user: adminUser, + user: editorUser, overrideAccess: false, data: { - title: 'Everyone', + title: 'Editors', where: { text: { equals: 'example page', @@ -562,34 +564,37 @@ describe('Query Presets', () => { }, access: { read: { - constraint: 'everyone', + constraint: 'specificRoles', + roles: ['editor'], }, update: { - constraint: 'everyone', + constraint: 'specificRoles', + roles: ['editor'], }, }, relatedCollection: 'pages', }, }) - expect(presetForEveryone).toBeTruthy() - - // attempt to update the preset to only admins + expect(presetForEditors).toBeTruthy() + // attempt to update the preset to include admins try { const presetUpdatedByEditorUser = await payload.update({ collection: queryPresetsCollectionSlug, - id: presetForEveryone.id, + id: presetForEditors.id, user: editorUser, overrideAccess: false, data: { - title: 'Everyone (Now Admins)', + title: 'Editors (Now Admins)', access: { read: { - constraint: 'onlyAdmins', + constraint: 'specificRoles', + roles: ['admin', 'editor'], }, update: { - constraint: 'onlyAdmins', + constraint: 'specificRoles', + roles: ['admin', 'editor'], }, }, }, @@ -598,29 +603,9 @@ describe('Query Presets', () => { expect(presetUpdatedByEditorUser).toBeFalsy() } catch (error: unknown) { expect((error as Error).message).toBe( - 'You must be an admin to share this preset with admins.', + 'You must be an admin to add or remove the "admin" role from a preset.', ) } - - const presetUpdatedByAdminUser = await payload.update({ - collection: queryPresetsCollectionSlug, - id: presetForEveryone.id, - user: adminUser, - overrideAccess: false, - data: { - title: 'Everyone (Now Admins)', - access: { - read: { - constraint: 'onlyAdmins', - }, - update: { - constraint: 'onlyAdmins', - }, - }, - }, - }) - - expect(presetUpdatedByAdminUser.title).toBe('Everyone (Now Admins)') }) it('should respect access when set to "specificRoles"', async () => {