fix(ui): properly sets hasSavePermission on nested documents (#6394)

This commit is contained in:
Jacob Fletcher
2024-05-17 13:41:38 -04:00
committed by GitHub
parent 89b6055d61
commit 18009349c0
13 changed files with 337 additions and 187 deletions

View File

@@ -9,6 +9,8 @@ import { RenderCustomComponent } from '@payloadcms/ui/elements/RenderCustomCompo
import { DocumentInfoProvider } from '@payloadcms/ui/providers/DocumentInfo'
import { EditDepthProvider } from '@payloadcms/ui/providers/EditDepth'
import { FormQueryParamsProvider } from '@payloadcms/ui/providers/FormQueryParams'
import { hasSavePermission as getHasSavePermission } from '@payloadcms/ui/utilities/hasSavePermission'
import { isEditing as getIsEditing } from '@payloadcms/ui/utilities/isEditing'
import { notFound, redirect } from 'next/navigation.js'
import { docAccessOperation } from 'payload/operations'
import React from 'react'
@@ -52,7 +54,7 @@ export const Document: React.FC<AdminViewProps> = async ({
const collectionSlug = collectionConfig?.slug || undefined
const globalSlug = globalConfig?.slug || undefined
const isEditing = Boolean(globalSlug || (collectionSlug && !!id))
const isEditing = getIsEditing({ id, collectionSlug, globalSlug })
let ViewOverride: EditViewComponent
let CustomView: EditViewComponent
@@ -83,9 +85,7 @@ export const Document: React.FC<AdminViewProps> = async ({
action = `${serverURL}${apiRoute}/${collectionSlug}${isEditing ? `/${id}` : ''}`
hasSavePermission =
(isEditing && permissions?.collections?.[collectionSlug]?.update?.permission) ||
(!isEditing && permissions?.collections?.[collectionSlug]?.create?.permission)
hasSavePermission = getHasSavePermission({ collectionSlug, docPermissions, isEditing })
apiURL = `${serverURL}${apiRoute}/${collectionSlug}/${id}?locale=${locale.code}${
collectionConfig.versions?.drafts ? '&draft=true' : ''
@@ -118,7 +118,8 @@ export const Document: React.FC<AdminViewProps> = async ({
}
docPermissions = permissions?.globals?.[globalSlug]
hasSavePermission = isEditing && docPermissions?.update?.permission
hasSavePermission = getHasSavePermission({ docPermissions, globalSlug, isEditing })
action = `${serverURL}${apiRoute}/globals/${globalSlug}`
apiURL = `${serverURL}${apiRoute}/${globalSlug}?locale=${locale.code}${

View File

@@ -223,7 +223,7 @@ export const DefaultEditView: React.FC = () => {
disableActions={disableActions}
hasSavePermission={hasSavePermission}
id={id}
isEditing={Boolean(id)}
isEditing={isEditing}
permissions={docPermissions}
slug={collectionConfig?.slug || globalConfig?.slug}
/>

View File

@@ -64,6 +64,7 @@ const PreviewView: React.FC<Props> = ({
hasSavePermission,
initialData,
initialState,
isEditing,
onSave: onSaveFromProps,
} = useDocumentInfo()
@@ -161,7 +162,7 @@ const PreviewView: React.FC<Props> = ({
disableActions={disableActions}
hasSavePermission={hasSavePermission}
id={id}
isEditing={Boolean(id)}
isEditing={isEditing}
permissions={docPermissions}
slug={collectionConfig?.slug || globalConfig?.slug}
/>

View File

@@ -13,6 +13,7 @@ export const FormSubmit = forwardRef<HTMLButtonElement, Props>((props, ref) => {
const { type = 'submit', buttonId: id, children, disabled: disabledFromProps } = props
const processing = useFormProcessing()
const { disabled } = useForm()
const canSave = !(disabledFromProps || processing || disabled)
return (

View File

@@ -12,6 +12,8 @@ import type { DocumentInfoContext, DocumentInfoProps } from './types.js'
import { LoadingOverlay } from '../../elements/Loading/index.js'
import { formatDocTitle } from '../../utilities/formatDocTitle.js'
import { getFormState } from '../../utilities/getFormState.js'
import { hasSavePermission as getHasSavePermission } from '../../utilities/hasSavePermission.js'
import { isEditing as getIsEditing } from '../../utilities/isEditing.js'
import { reduceFieldsToValues } from '../../utilities/reduceFieldsToValues.js'
import { useAuth } from '../Auth/index.js'
import { useConfig } from '../Config/index.js'
@@ -63,25 +65,23 @@ export const DocumentInfoProvider: React.FC<
const baseURL = `${serverURL}${api}`
let slug: string
let pluralType: 'collections' | 'globals'
let preferencesKey: string
if (globalSlug) {
slug = globalSlug
pluralType = 'globals'
preferencesKey = `global-${slug}`
}
if (collectionSlug) {
slug = collectionSlug
pluralType = 'collections'
if (id) {
preferencesKey = `collection-${slug}-${id}`
}
}
const operation = collectionSlug && !id ? 'create' : 'update'
const isEditing = getIsEditing({ id, collectionSlug, globalSlug })
const operation = isEditing ? 'update' : 'create'
const shouldFetchVersions = Boolean(versionsConfig && docPermissions?.readVersions?.permission)
const getVersions = useCallback(async () => {
@@ -212,15 +212,16 @@ export const DocumentInfoProvider: React.FC<
}, [i18n, globalSlug, collectionSlug, id, baseURL, locale, versionsConfig, shouldFetchVersions])
const getDocPermissions = React.useCallback(async () => {
let docAccessURL: string
const params = {
locale: locale || undefined,
}
if (pluralType === 'globals') {
docAccessURL = `/globals/${slug}/access`
} else if (pluralType === 'collections' && id) {
docAccessURL = `/${slug}/access/${id}`
}
if (isEditing) {
const docAccessURL = collectionSlug
? `/${collectionSlug}/access/${id}`
: globalSlug
? `/globals/${globalSlug}/access`
: null
if (docAccessURL) {
const res = await fetch(`${serverURL}${api}${docAccessURL}?${qs.stringify(params)}`, {
@@ -229,17 +230,44 @@ export const DocumentInfoProvider: React.FC<
'Accept-Language': i18n.language,
},
})
const json = await res.json()
const json: DocumentPermissions = await res.json()
setDocPermissions(json)
setHasSavePermission(json?.update?.permission)
} else {
// fallback to permissions from the entity type
// (i.e. create has no id)
setDocPermissions(permissions?.[pluralType]?.[slug])
setHasSavePermission(permissions?.[pluralType]?.[slug]?.update?.permission)
setHasSavePermission(
getHasSavePermission({ collectionSlug, docPermissions: json, globalSlug, isEditing }),
)
}
}, [serverURL, api, pluralType, slug, id, permissions, i18n.language, locale])
} else {
// when creating new documents, there is no permissions saved for this document yet
// use the generic entity permissions instead
const newDocPermissions = collectionSlug
? permissions?.collections?.[collectionSlug]
: permissions?.globals?.[globalSlug]
setDocPermissions(newDocPermissions)
setHasSavePermission(
getHasSavePermission({
collectionSlug,
docPermissions: newDocPermissions,
globalSlug,
isEditing,
}),
)
}
}, [
serverURL,
api,
id,
permissions,
i18n.language,
locale,
collectionSlug,
globalSlug,
isEditing,
])
const getDocPreferences = useCallback(() => {
return getPreference<DocumentPreferences>(preferencesKey)

View File

@@ -0,0 +1,29 @@
import type { CollectionPermission, DocumentPermissions, GlobalPermission } from 'payload/auth'
export const hasSavePermission = (args: {
/*
* Pass either `collectionSlug` or `globalSlug`
*/
collectionSlug?: string
docPermissions: DocumentPermissions
/*
* Pass either `collectionSlug` or `globalSlug`
*/
globalSlug?: string
isEditing: boolean
}) => {
const { collectionSlug, docPermissions, globalSlug, isEditing } = args
if (collectionSlug) {
return Boolean(
(isEditing && docPermissions?.update?.permission) ||
(!isEditing && (docPermissions as CollectionPermission)?.create?.permission),
)
}
if (globalSlug) {
return Boolean((docPermissions as GlobalPermission)?.update?.permission)
}
return false
}

View File

@@ -0,0 +1,9 @@
export const isEditing = ({
id,
collectionSlug,
globalSlug,
}: {
collectionSlug?: string
globalSlug?: string
id?: number | string
}): boolean => Boolean(globalSlug || (collectionSlug && !!id))

View File

@@ -4,8 +4,10 @@ import { buildConfigWithDefaults } from '../buildConfigWithDefaults.js'
import { devUser } from '../credentials.js'
import { TestButton } from './TestButton.js'
import {
createNotUpdateSlug,
docLevelAccessSlug,
firstArrayText,
fullyRestrictedSlug,
hiddenAccessCountSlug,
hiddenAccessSlug,
hiddenFieldsSlug,
@@ -15,7 +17,6 @@ import {
readOnlyGlobalSlug,
readOnlySlug,
relyOnRequestHeadersSlug,
restrictedSlug,
restrictedVersionsSlug,
secondArrayText,
siblingDataSlug,
@@ -203,10 +204,16 @@ export default buildConfigWithDefaults({
relationTo: userRestrictedSlug,
hasMany: true,
},
{
name: 'createNotUpdateDocs',
type: 'relationship',
relationTo: 'create-not-update',
hasMany: true,
},
],
},
{
slug: restrictedSlug,
slug: fullyRestrictedSlug,
fields: [
{
name: 'name',
@@ -257,6 +264,24 @@ export default buildConfigWithDefaults({
delete: () => false,
},
},
{
slug: createNotUpdateSlug,
admin: {
useAsTitle: 'name',
},
fields: [
{
name: 'name',
type: 'text',
},
],
access: {
create: () => true,
read: () => true,
update: () => false,
delete: () => false,
},
},
{
slug: restrictedVersionsSlug,
versions: true,

View File

@@ -28,15 +28,16 @@ import {
} from '../helpers.js'
import { AdminUrlUtil } from '../helpers/adminUrlUtil.js'
import { initPayloadE2ENoConfig } from '../helpers/initPayloadE2ENoConfig.js'
import { POLL_TOPASS_TIMEOUT, TEST_TIMEOUT, TEST_TIMEOUT_LONG } from '../playwright.config.js'
import { POLL_TOPASS_TIMEOUT, TEST_TIMEOUT_LONG } from '../playwright.config.js'
import {
createNotUpdateSlug,
docLevelAccessSlug,
fullyRestrictedSlug,
noAdminAccessEmail,
nonAdminUserEmail,
nonAdminUserSlug,
readOnlyGlobalSlug,
readOnlySlug,
restrictedSlug,
restrictedVersionsSlug,
slug,
unrestrictedSlug,
@@ -46,7 +47,6 @@ const dirname = path.dirname(filename)
/**
* TODO: Access Control
* prevent user from logging in (canAccessAdmin)
*
* FSK: 'should properly prevent / allow public users from reading a restricted field'
*
@@ -59,6 +59,7 @@ describe('access control', () => {
let page: Page
let url: AdminUrlUtil
let restrictedUrl: AdminUrlUtil
let unrestrictedURL: AdminUrlUtil
let readOnlyCollectionUrl: AdminUrlUtil
let readOnlyGlobalUrl: AdminUrlUtil
let restrictedVersionsUrl: AdminUrlUtil
@@ -71,7 +72,8 @@ describe('access control', () => {
;({ payload, serverURL } = await initPayloadE2ENoConfig<Config>({ dirname }))
url = new AdminUrlUtil(serverURL, slug)
restrictedUrl = new AdminUrlUtil(serverURL, restrictedSlug)
restrictedUrl = new AdminUrlUtil(serverURL, fullyRestrictedSlug)
unrestrictedURL = new AdminUrlUtil(serverURL, unrestrictedSlug)
readOnlyCollectionUrl = new AdminUrlUtil(serverURL, readOnlySlug)
readOnlyGlobalUrl = new AdminUrlUtil(serverURL, readOnlySlug)
restrictedVersionsUrl = new AdminUrlUtil(serverURL, restrictedVersionsSlug)
@@ -93,6 +95,7 @@ describe('access control', () => {
logoutURL = `${serverURL}${adminRoute}${logoutRoute}`
})
describe('fields', () => {
test('field without read access should not show', async () => {
const { id } = await createDoc({ restrictedField: 'restricted' })
@@ -125,12 +128,18 @@ describe('access control', () => {
await expect(page.locator('#field-restrictedCollapsibleText')).toHaveCount(0)
})
describe('restricted collection', () => {
test('should not show field without permission', async () => {
await page.goto(url.account)
await expect(page.locator('#field-roles')).toBeHidden()
})
})
describe('collection - fully restricted', () => {
let existingDoc: ReadOnlyCollection
beforeAll(async () => {
existingDoc = await payload.create({
collection: restrictedSlug,
collection: fullyRestrictedSlug,
data: {
name: 'name',
},
@@ -139,7 +148,7 @@ describe('access control', () => {
test('should not show in card list', async () => {
await page.goto(url.admin)
await expect(page.locator(`#card-${restrictedSlug}`)).toHaveCount(0)
await expect(page.locator(`#card-${fullyRestrictedSlug}`)).toHaveCount(0)
})
test('should not show in nav', async () => {
@@ -169,14 +178,7 @@ describe('access control', () => {
})
})
describe('restricted fields', () => {
test('should not show field without permission', async () => {
await page.goto(url.account)
await expect(page.locator('#field-roles')).toBeHidden()
})
})
describe('read-only collection', () => {
describe('collection - read-only', () => {
let existingDoc: ReadOnlyCollection
beforeAll(async () => {
@@ -232,7 +234,84 @@ describe('access control', () => {
})
})
describe('readVersions', () => {
describe('collection - create but not edit', () => {
test('should not show edit button', async () => {
const createNotUpdateURL = new AdminUrlUtil(serverURL, createNotUpdateSlug)
await page.goto(createNotUpdateURL.create)
await page.waitForURL(createNotUpdateURL.create)
await expect(page.locator('#field-name')).toBeVisible()
await page.locator('#field-name').fill('name')
await expect(page.locator('#field-name')).toHaveValue('name')
await expect(page.locator('#action-save')).toBeVisible()
await page.locator('#action-save').click()
await expect(page.locator('.Toastify')).toContainText('successfully')
await expect(page.locator('#action-save')).toBeHidden()
await expect(page.locator('#field-name')).toBeDisabled()
})
test('should maintain access control in document drawer', async () => {
const unrestrictedDoc = await payload.create({
collection: unrestrictedSlug,
data: {
name: 'unrestricted-123',
},
})
await page.goto(unrestrictedURL.edit(unrestrictedDoc.id.toString()))
const addDocButton = page.locator(
'#createNotUpdateDocs-add-new button.relationship-add-new__add-button.doc-drawer__toggler',
)
await expect(addDocButton).toBeVisible()
await addDocButton.click()
const documentDrawer = page.locator('[id^=doc-drawer_create-not-update_1_]')
await expect(documentDrawer).toBeVisible()
await expect(documentDrawer.locator('#action-save')).toBeVisible()
await documentDrawer.locator('#field-name').fill('name')
await expect(documentDrawer.locator('#field-name')).toHaveValue('name')
await documentDrawer.locator('#action-save').click()
await expect(page.locator('.Toastify')).toContainText('successfully')
await expect(documentDrawer.locator('#action-save')).toBeHidden()
await expect(documentDrawer.locator('#field-name')).toBeDisabled()
})
})
describe('collection - dynamic update access', () => {
test('maintain access control in document drawer', async () => {
const unrestrictedDoc = await payload.create({
collection: unrestrictedSlug,
data: {
name: 'unrestricted-123',
},
})
await page.goto(unrestrictedURL.edit(unrestrictedDoc.id.toString()))
const addDocButton = page.locator(
'#userRestrictedDocs-add-new button.relationship-add-new__add-button.doc-drawer__toggler',
)
await addDocButton.click()
const documentDrawer = page.locator('[id^=doc-drawer_user-restricted_1_]')
await expect(documentDrawer).toBeVisible()
await documentDrawer.locator('#field-name').fill('anonymous@email.com')
await documentDrawer.locator('#action-save').click()
await expect(page.locator('.Toastify')).toContainText('successfully')
await expect(documentDrawer.locator('#field-name')).toBeDisabled()
await documentDrawer.locator('button.doc-drawer__header-close').click()
await expect(documentDrawer).toBeHidden()
await addDocButton.click()
const documentDrawer2 = page.locator('[id^=doc-drawer_user-restricted_1_]')
await expect(documentDrawer2).toBeVisible()
await documentDrawer2.locator('#field-name').fill('dev@payloadcms.com')
await documentDrawer2.locator('#action-save').click()
await expect(page.locator('.Toastify')).toContainText('successfully')
await expect(documentDrawer2.locator('#field-name')).toBeEnabled()
})
})
describe('collection - restricted versions', () => {
let existingDoc: RestrictedVersion
beforeAll(async () => {
@@ -323,44 +402,7 @@ describe('access control', () => {
})
})
test('maintain access control in document drawer', async () => {
const unrestrictedDoc = await payload.create({
collection: unrestrictedSlug,
data: {
name: 'unrestricted-123',
},
})
// navigate to the `unrestricted` document and open the drawers to test access
const unrestrictedURL = new AdminUrlUtil(serverURL, unrestrictedSlug)
await page.goto(unrestrictedURL.edit(unrestrictedDoc.id.toString()))
const addDocButton = page.locator(
'#userRestrictedDocs-add-new button.relationship-add-new__add-button.doc-drawer__toggler',
)
await addDocButton.click()
const documentDrawer = page.locator('[id^=doc-drawer_user-restricted_1_]')
await expect(documentDrawer).toBeVisible()
await documentDrawer.locator('#field-name').fill('anonymous@email.com')
await documentDrawer.locator('#action-save').click()
await expect(page.locator('.Toastify')).toContainText('successfully')
// ensure user is not allowed to edit this document
await expect(documentDrawer.locator('#field-name')).toBeDisabled()
await documentDrawer.locator('button.doc-drawer__header-close').click()
await expect(documentDrawer).toBeHidden()
await addDocButton.click()
const documentDrawer2 = page.locator('[id^=doc-drawer_user-restricted_1_]')
await expect(documentDrawer2).toBeVisible()
await documentDrawer2.locator('#field-name').fill('dev@payloadcms.com')
await documentDrawer2.locator('#action-save').click()
await expect(page.locator('.Toastify')).toContainText('successfully')
// ensure user is allowed to edit this document
await expect(documentDrawer2.locator('#field-name')).toBeEnabled()
})
describe('admin access', () => {
test('should block admin access to admin user', async () => {
const adminURL = `${serverURL}/admin`
await page.goto(adminURL)
@@ -430,6 +472,7 @@ describe('access control', () => {
await expect(page.locator('.next-error-h1')).toBeVisible()
})
})
})
// eslint-disable-next-line @typescript-eslint/require-await
async function createDoc(data: any): Promise<TypeWithID & Record<string, unknown>> {

View File

@@ -8,11 +8,11 @@ import { initPayloadInt } from '../helpers/initPayloadInt.js'
import configPromise, { requestHeaders } from './config.js'
import {
firstArrayText,
fullyRestrictedSlug,
hiddenAccessCountSlug,
hiddenAccessSlug,
hiddenFieldsSlug,
relyOnRequestHeadersSlug,
restrictedSlug,
restrictedVersionsSlug,
secondArrayText,
siblingDataSlug,
@@ -36,7 +36,7 @@ describe('Access Control', () => {
})
restricted = await payload.create({
collection: restrictedSlug,
collection: fullyRestrictedSlug,
data: { name: 'restricted' },
})
})
@@ -324,7 +324,7 @@ describe('Access Control', () => {
it('should allow overrideAccess: false', async () => {
const req = async () =>
await payload.update({
collection: restrictedSlug,
collection: fullyRestrictedSlug,
id: restricted.id,
data: { name: updatedName },
overrideAccess: false, // this should respect access control
@@ -335,7 +335,7 @@ describe('Access Control', () => {
it('should allow overrideAccess: true', async () => {
const doc = await payload.update({
collection: restrictedSlug,
collection: fullyRestrictedSlug,
id: restricted.id,
data: { name: updatedName },
overrideAccess: true, // this should override access control
@@ -346,7 +346,7 @@ describe('Access Control', () => {
it('should allow overrideAccess by default', async () => {
const doc = await payload.update({
collection: restrictedSlug,
collection: fullyRestrictedSlug,
id: restricted.id,
data: { name: updatedName },
})
@@ -357,7 +357,7 @@ describe('Access Control', () => {
it('should allow overrideAccess: false - update many', async () => {
const req = async () =>
await payload.update({
collection: restrictedSlug,
collection: fullyRestrictedSlug,
where: {
id: { equals: restricted.id },
},
@@ -370,7 +370,7 @@ describe('Access Control', () => {
it('should allow overrideAccess: true - update many', async () => {
const doc = await payload.update({
collection: restrictedSlug,
collection: fullyRestrictedSlug,
where: {
id: { equals: restricted.id },
},
@@ -383,7 +383,7 @@ describe('Access Control', () => {
it('should allow overrideAccess by default - update many', async () => {
const doc = await payload.update({
collection: restrictedSlug,
collection: fullyRestrictedSlug,
where: {
id: { equals: restricted.id },
},

View File

@@ -12,9 +12,10 @@ export interface Config {
'non-admin-user': NonAdminUser;
posts: Post;
unrestricted: Unrestricted;
restricted: Restricted;
'fully-restricted': FullyRestricted;
'read-only-collection': ReadOnlyCollection;
'user-restricted': UserRestricted;
'create-not-update': CreateNotUpdate;
'restricted-versions': RestrictedVersion;
'sibling-data': SiblingDatum;
'rely-on-request-headers': RelyOnRequestHeader;
@@ -97,6 +98,7 @@ export interface Unrestricted {
id: string;
name?: string | null;
userRestrictedDocs?: (string | UserRestricted)[] | null;
createNotUpdateDocs?: (string | CreateNotUpdate)[] | null;
updatedAt: string;
createdAt: string;
}
@@ -112,9 +114,19 @@ export interface UserRestricted {
}
/**
* This interface was referenced by `Config`'s JSON-Schema
* via the `definition` "restricted".
* via the `definition` "create-not-update".
*/
export interface Restricted {
export interface CreateNotUpdate {
id: string;
name?: string | null;
updatedAt: string;
createdAt: string;
}
/**
* This interface was referenced by `Config`'s JSON-Schema
* via the `definition` "fully-restricted".
*/
export interface FullyRestricted {
id: string;
name?: string | null;
updatedAt: string;

View File

@@ -7,7 +7,8 @@ export const readOnlySlug = 'read-only-collection'
export const readOnlyGlobalSlug = 'read-only-global'
export const userRestrictedSlug = 'user-restricted'
export const restrictedSlug = 'restricted'
export const fullyRestrictedSlug = 'fully-restricted'
export const createNotUpdateSlug = 'create-not-update'
export const restrictedVersionsSlug = 'restricted-versions'
export const siblingDataSlug = 'sibling-data'
export const relyOnRequestHeadersSlug = 'rely-on-request-headers'

View File

@@ -37,7 +37,7 @@
],
"paths": {
"@payload-config": [
"./test/_community/config.ts"
"./test/access-control/config.ts"
],
"@payloadcms/live-preview": [
"./packages/live-preview/src"