fix: skip validation of where query paths from access result (#9349)

### What?

Previously, `payload.findByID` with `overrideAccess: false` and this
collection config
```ts
{
  slug: 'fields-and-top-access',
  access: {
    read: () => ({
      secret: {
        equals: '12345',
      },
    }),
  },
  fields: [
    {
      type: 'text',
      name: 'secret',
      access: { read: () => false },
    },
  ],
},
```

Led to the `The following path cannot be queried: secret` error because
`where` input to `validateQueryPaths` also includes the result from
access control, which shouldn't be.

This works when using `payload.find`.

The same applies to find with drafts / joins `where`. We need to
validate only user `where` input, not access control that we defined in
our config.

Also, this exact logic seems be used in `find` without drafts - we don't
use `fullWhere` here but `where`, that's why this error isn't being
thrown with `find` but only `findByID`.

d9c6288cb2/packages/payload/src/collections/operations/find.ts (L134)

d9c6288cb2/packages/payload/src/collections/operations/find.ts (L166-L171)

Fixes https://github.com/payloadcms/payload/issues/9210
This commit is contained in:
Sasha
2024-11-26 19:02:45 +02:00
committed by GitHub
parent 44c0cdb75f
commit a9f511d540
8 changed files with 155 additions and 19 deletions

View File

@@ -148,7 +148,7 @@ export const findOperation = async <
overrideAccess,
req,
versionFields: buildVersionCollectionFields(payload.config, collection.config, true),
where: fullWhere,
where: appendVersionToQueryKey(where),
})
result = await payload.db.queryDrafts<DataFromCollectionSlug<TSlug>>({

View File

@@ -96,7 +96,9 @@ export const findByIDOperation = async <
return null
}
const where = combineQueries({ id: { equals: id } }, accessResult)
const where = { id: { equals: id } }
const fullWhere = combineQueries(where, accessResult)
const sanitizedJoins = await sanitizeJoinQuery({
collectionConfig,
@@ -113,16 +115,18 @@ export const findByIDOperation = async <
transactionID: req.transactionID,
} as PayloadRequest,
select,
where,
where: fullWhere,
}
await validateQueryPaths({
collectionConfig,
overrideAccess,
req,
where,
})
// execute only if there's a custom ID and potentially overwriten access on id
if (req.payload.collections[collectionConfig.slug].customIDType) {
await validateQueryPaths({
collectionConfig,
overrideAccess,
req,
where,
})
}
// /////////////////////////////////////
// Find by ID
// /////////////////////////////////////

View File

@@ -142,7 +142,7 @@ export const updateOperation = async <
overrideAccess,
req,
versionFields: buildVersionCollectionFields(payload.config, collection.config, true),
where: versionsWhere,
where: appendVersionToQueryKey(where),
})
const query = await payload.db.queryDrafts<DataFromCollectionSlug<TSlug>>({

View File

@@ -3,6 +3,7 @@ import type { JoinQuery, PayloadRequest } from '../types/index.js'
import executeAccess from '../auth/executeAccess.js'
import { QueryError } from '../errors/QueryError.js'
import { deepCopyObjectSimple } from '../utilities/deepCopyObject.js'
import { combineQueries } from './combineQueries.js'
import { validateQueryPaths } from './queryValidation/validateQueryPaths.js'
@@ -66,19 +67,20 @@ export const sanitizeJoinQuery = async ({
joinQuery.where = combineQueries(joinQuery.where, field.where)
}
if (typeof accessResult === 'object') {
joinQuery.where = combineQueries(joinQuery.where, accessResult)
}
promises.push(
validateQueryPaths({
collectionConfig: joinCollectionConfig,
errors,
overrideAccess,
req,
where: joinQuery.where,
// incoming where input, but we shouldn't validate generated from the access control.
where: deepCopyObjectSimple(joinQuery.where),
}),
)
if (typeof accessResult === 'object') {
joinQuery.where = combineQueries(joinQuery.where, accessResult)
}
}
}

View File

@@ -40,9 +40,9 @@ export interface Config {
user: User & {
collection: 'users';
};
jobs?: {
jobs: {
tasks: unknown;
workflows?: unknown;
workflows: unknown;
};
}
export interface UserAuthOperations {

View File

@@ -509,6 +509,29 @@ export default buildConfigWithDefaults({
},
],
},
{
slug: 'fields-and-top-access',
access: {
readVersions: () => ({
'version.secret': {
equals: 'will-success-access-read',
},
}),
read: () => ({
secret: {
equals: 'will-success-access-read',
},
}),
},
versions: { drafts: true },
fields: [
{
type: 'text',
name: 'secret',
access: { read: () => false },
},
],
},
Disabled,
RichText,
Regression1,

View File

@@ -1,3 +1,4 @@
import type { NextRESTClient } from 'helpers/NextRESTClient.js'
import type {
CollectionSlug,
DataFromCollectionSlug,
@@ -28,6 +29,7 @@ import {
} from './shared.js'
let payload: Payload
let restClient: NextRESTClient
const filename = fileURLToPath(import.meta.url)
const dirname = path.dirname(filename)
describe('Access Control', () => {
@@ -35,7 +37,7 @@ describe('Access Control', () => {
let restricted: FullyRestricted
beforeAll(async () => {
;({ payload } = await initPayloadInt(dirname))
;({ payload, restClient } = await initPayloadInt(dirname))
})
beforeEach(async () => {
@@ -512,6 +514,7 @@ describe('Access Control', () => {
hidden: false,
},
})
const { docs } = await payload.findVersions({
collection: restrictedVersionsSlug,
overrideAccess: false,
@@ -522,6 +525,85 @@ describe('Access Control', () => {
expect(docs).toHaveLength(1)
})
it('should ignore false access on query constraint added by top collection level access control', async () => {
await payload.create({
collection: 'fields-and-top-access',
data: { secret: 'will-fail-access-read' },
})
const { id: hitID } = await payload.create({
collection: 'fields-and-top-access',
data: { secret: 'will-success-access-read' },
})
await payload.create({
collection: 'fields-and-top-access',
data: { secret: 'will-fail-access-read' },
})
// assert find, only will-success should be in the result
const resFind = await payload.find({
overrideAccess: false,
collection: 'fields-and-top-access',
})
expect(resFind.docs[0].id).toBe(hitID)
expect(resFind.docs).toHaveLength(1)
// assert find draft: true
const resFindDraft = await payload.find({
draft: true,
overrideAccess: false,
collection: 'fields-and-top-access',
})
expect(resFindDraft.docs).toHaveLength(1)
expect(resFind.docs[0].id).toBe(hitID)
// assert findByID
const res = await payload.findByID({
id: hitID,
collection: 'fields-and-top-access',
overrideAccess: false,
})
expect(res).toBeTruthy()
})
it('should ignore false access in versions on query constraint added by top collection level access control', async () => {
// clean up
await payload.delete({ collection: 'fields-and-top-access', where: {} })
await payload.create({
collection: 'fields-and-top-access',
data: { secret: 'will-fail-access-read' },
})
const { id: hitID } = await payload.create({
collection: 'fields-and-top-access',
data: { secret: 'will-success-access-read' },
})
await payload.create({
collection: 'fields-and-top-access',
data: { secret: 'will-fail-access-read' },
})
// Assert findVersions only will-success should be in the result
const resFind = await payload.findVersions({
overrideAccess: false,
collection: 'fields-and-top-access',
})
expect(resFind.docs).toHaveLength(1)
const version = resFind.docs[0]
expect(version.parent).toBe(hitID)
// Assert findVersionByID
const res = await payload.findVersionByID({
id: version.id,
collection: 'fields-and-top-access',
overrideAccess: false,
})
expect(res).toBeTruthy()
})
})
})

View File

@@ -28,6 +28,7 @@ export interface Config {
'hidden-fields': HiddenField;
'hidden-access': HiddenAccess;
'hidden-access-count': HiddenAccessCount;
'fields-and-top-access': FieldsAndTopAccess;
disabled: Disabled;
'rich-text': RichText;
regression1: Regression1;
@@ -54,6 +55,7 @@ export interface Config {
'hidden-fields': HiddenFieldsSelect<false> | HiddenFieldsSelect<true>;
'hidden-access': HiddenAccessSelect<false> | HiddenAccessSelect<true>;
'hidden-access-count': HiddenAccessCountSelect<false> | HiddenAccessCountSelect<true>;
'fields-and-top-access': FieldsAndTopAccessSelect<false> | FieldsAndTopAccessSelect<true>;
disabled: DisabledSelect<false> | DisabledSelect<true>;
'rich-text': RichTextSelect<false> | RichTextSelect<true>;
regression1: Regression1Select<false> | Regression1Select<true>;
@@ -334,6 +336,16 @@ export interface HiddenAccessCount {
updatedAt: string;
createdAt: string;
}
/**
* This interface was referenced by `Config`'s JSON-Schema
* via the `definition` "fields-and-top-access".
*/
export interface FieldsAndTopAccess {
id: string;
secret?: string | null;
updatedAt: string;
createdAt: string;
}
/**
* This interface was referenced by `Config`'s JSON-Schema
* via the `definition` "disabled".
@@ -670,6 +682,10 @@ export interface PayloadLockedDocument {
relationTo: 'hidden-access-count';
value: string | HiddenAccessCount;
} | null)
| ({
relationTo: 'fields-and-top-access';
value: string | FieldsAndTopAccess;
} | null)
| ({
relationTo: 'disabled';
value: string | Disabled;
@@ -930,6 +946,15 @@ export interface HiddenAccessCountSelect<T extends boolean = true> {
updatedAt?: T;
createdAt?: T;
}
/**
* This interface was referenced by `Config`'s JSON-Schema
* via the `definition` "fields-and-top-access_select".
*/
export interface FieldsAndTopAccessSelect<T extends boolean = true> {
secret?: T;
updatedAt?: T;
createdAt?: T;
}
/**
* This interface was referenced by `Config`'s JSON-Schema
* via the `definition` "disabled_select".