From bc97d3d6f261ed31ddeb85df33862bed463a65a9 Mon Sep 17 00:00:00 2001 From: dsod Date: Sat, 20 Aug 2022 01:33:29 +0200 Subject: [PATCH] Keep original request and add properties to it, instead of copy --- src/auth/operations/local/forgotPassword.ts | 7 +- src/auth/operations/local/login.ts | 13 ++-- src/auth/operations/local/resetPassword.ts | 9 +-- src/auth/operations/local/unlock.ts | 9 +-- src/collections/operations/local/create.ts | 26 ++++---- src/collections/operations/local/find.ts | 14 ++-- src/collections/operations/local/findByID.ts | 14 ++-- .../operations/local/findVersionByID.ts | 13 ++-- test/access-control/config.ts | 21 ++++++ test/access-control/int.spec.ts | 65 ++++++++++++++++--- test/access-control/payload-types.ts | 10 +++ 11 files changed, 128 insertions(+), 73 deletions(-) diff --git a/src/auth/operations/local/forgotPassword.ts b/src/auth/operations/local/forgotPassword.ts index cff6b45907..81ea1fc46e 100644 --- a/src/auth/operations/local/forgotPassword.ts +++ b/src/auth/operations/local/forgotPassword.ts @@ -19,15 +19,12 @@ async function localForgotPassword(payload: Payload, options: Options): Promise< data, expiration, disableEmail, - req: incomingReq = {}, + req = {} as PayloadRequest, } = options; const collection = payload.collections[collectionSlug]; - const req = { - ...incomingReq, - payloadAPI: 'local', - } as PayloadRequest; + req.payloadAPI = 'local'; if (!req.payloadDataLoader) req.payloadDataLoader = getDataLoader(req); diff --git a/src/auth/operations/local/login.ts b/src/auth/operations/local/login.ts index 0587c09956..281fa3f17d 100644 --- a/src/auth/operations/local/login.ts +++ b/src/auth/operations/local/login.ts @@ -23,7 +23,7 @@ export type Options = { async function localLogin(payload: Payload, options: Options): Promise { const { collection: collectionSlug, - req: incomingReq = {}, + req = {} as PayloadRequest, res, depth, locale, @@ -35,13 +35,10 @@ async function localLogin(payload: Payload, options: const collection = payload.collections[collectionSlug]; - const req = { - ...incomingReq, - payloadAPI: 'local', - payload, - locale: undefined, - fallbackLocale: undefined, - } as PayloadRequest; + req.payloadAPI = 'local'; + req.payload = payload; + req.locale = undefined; + req.fallbackLocale = undefined; if (!req.payloadDataLoader) req.payloadDataLoader = getDataLoader(req); diff --git a/src/auth/operations/local/resetPassword.ts b/src/auth/operations/local/resetPassword.ts index e37f806247..1e04f07bf1 100644 --- a/src/auth/operations/local/resetPassword.ts +++ b/src/auth/operations/local/resetPassword.ts @@ -18,16 +18,13 @@ async function localResetPassword(payload: Payload, options: Options): Promise collection: collectionSlug, data, overrideAccess = true, - req: incomingReq = {}, + req = {} as PayloadRequest, } = options; const collection = payload.collections[collectionSlug]; - const req = { - ...incomingReq, - payload, - payloadAPI: 'local', - } as PayloadRequest; + req.payload = payload; + req.payloadAPI = 'local'; if (!req.payloadDataLoader) req.payloadDataLoader = getDataLoader(req); diff --git a/src/collections/operations/local/create.ts b/src/collections/operations/local/create.ts index 6dd74fc10c..452a4cfca2 100644 --- a/src/collections/operations/local/create.ts +++ b/src/collections/operations/local/create.ts @@ -1,9 +1,9 @@ +import { UploadedFile } from 'express-fileupload'; import { Payload } from '../../..'; import { PayloadRequest } from '../../../express/types'; import { Document } from '../../../types'; import getFileByPath from '../../../uploads/getFileByPath'; import create from '../create'; -import { File } from '../../../uploads/types'; import { getDataLoader } from '../../dataloader'; @@ -18,7 +18,7 @@ export type Options = { disableVerificationEmail?: boolean showHiddenFields?: boolean filePath?: string - file?: File + file?: UploadedFile overwriteExistingFiles?: boolean req?: PayloadRequest draft?: boolean @@ -38,23 +38,21 @@ export default async function createLocal(payload: Payload, options: Op filePath, file, overwriteExistingFiles = false, - req: incomingReq, + req = {} as PayloadRequest, draft, } = options; const collection = payload.collections[collectionSlug]; - const req = { - ...incomingReq || {}, - user, - payloadAPI: 'local', - locale: locale || incomingReq?.locale || (payload?.config?.localization ? payload?.config?.localization?.defaultLocale : null), - fallbackLocale: fallbackLocale || incomingReq?.fallbackLocale || null, - payload, - files: { - file: file ?? getFileByPath(filePath), - }, - } as PayloadRequest; + req.payloadAPI = 'local'; + req.locale = locale || req?.locale || (payload?.config?.localization ? payload?.config?.localization?.defaultLocale : null); + req.fallbackLocale = fallbackLocale || req?.fallbackLocale || null; + req.payload = payload; + req.files = { + file: (file as UploadedFile) ?? (getFileByPath(filePath) as UploadedFile), + }; + + if (typeof user !== 'undefined') req.user = user; if (!req.payloadDataLoader) req.payloadDataLoader = getDataLoader(req); diff --git a/src/collections/operations/local/find.ts b/src/collections/operations/local/find.ts index 3955d6b7c7..1fa20492c2 100644 --- a/src/collections/operations/local/find.ts +++ b/src/collections/operations/local/find.ts @@ -43,19 +43,15 @@ export default async function findLocal(payload: Pay sort, draft = false, pagination = true, - req: incomingReq, + req = {} as PayloadRequest, } = options; const collection = payload.collections[collectionSlug]; - const req = { - user: undefined, - ...incomingReq || {}, - payloadAPI: 'local', - locale: locale || incomingReq?.locale || (payload?.config?.localization ? payload?.config?.localization?.defaultLocale : null), - fallbackLocale: fallbackLocale || incomingReq?.fallbackLocale || null, - payload, - } as PayloadRequest; + req.payloadAPI = 'local'; + req.locale = locale || req?.locale || (payload?.config?.localization ? payload?.config?.localization?.defaultLocale : null); + req.fallbackLocale = fallbackLocale || req?.fallbackLocale || null; + req.payload = payload; if (!req.payloadDataLoader) req.payloadDataLoader = getDataLoader(req); diff --git a/src/collections/operations/local/findByID.ts b/src/collections/operations/local/findByID.ts index 819dcbd10d..eb16079dfe 100644 --- a/src/collections/operations/local/findByID.ts +++ b/src/collections/operations/local/findByID.ts @@ -33,20 +33,16 @@ export default async function findByIDLocal(payload: overrideAccess = true, disableErrors = false, showHiddenFields, - req: incomingReq, + req = {} as PayloadRequest, draft = false, } = options; const collection = payload.collections[collectionSlug]; - const req = { - user: undefined, - ...incomingReq || {}, - payloadAPI: 'local', - locale: locale || incomingReq?.locale || (payload?.config?.localization ? payload?.config?.localization?.defaultLocale : null), - fallbackLocale: fallbackLocale || incomingReq?.fallbackLocale || null, - payload, - } as PayloadRequest; + req.payloadAPI = 'local'; + req.locale = locale || req?.locale || (payload?.config?.localization ? payload?.config?.localization?.defaultLocale : null); + req.fallbackLocale = fallbackLocale || req?.fallbackLocale || null; + req.payload = payload; if (typeof user !== 'undefined') req.user = user; diff --git a/src/collections/operations/local/findVersionByID.ts b/src/collections/operations/local/findVersionByID.ts index 1eedd589a3..2d12eac6cb 100644 --- a/src/collections/operations/local/findVersionByID.ts +++ b/src/collections/operations/local/findVersionByID.ts @@ -28,18 +28,15 @@ export default async function findVersionByIDLocal overrideAccess = true, disableErrors = false, showHiddenFields, - req: incomingReq, + req = {} as PayloadRequest, } = options; const collection = payload.collections[collectionSlug]; - const req = { - ...incomingReq || {}, - payloadAPI: 'local', - locale: locale || incomingReq?.locale || this?.config?.localization?.defaultLocale, - fallbackLocale: fallbackLocale || incomingReq?.fallbackLocale || null, - payload, - } as PayloadRequest; + req.payloadAPI = 'local'; + req.locale = locale || req?.locale || this?.config?.localization?.defaultLocale; + req.fallbackLocale = fallbackLocale || req?.fallbackLocale || null; + req.payload = payload; if (!req.payloadDataLoader) req.payloadDataLoader = getDataLoader(req); diff --git a/test/access-control/config.ts b/test/access-control/config.ts index 55cb9f155b..3cc59faf8a 100644 --- a/test/access-control/config.ts +++ b/test/access-control/config.ts @@ -9,6 +9,7 @@ export const readOnlySlug = 'read-only-collection'; export const restrictedSlug = 'restricted'; export const restrictedVersionsSlug = 'restricted-versions'; export const siblingDataSlug = 'sibling-data'; +export const relyOnRequestHeadersSlug = 'rely-on-request-headers'; const openAccess = { create: () => true, @@ -24,6 +25,11 @@ const PublicReadabilityAccess: FieldAccess = ({ req: { user }, siblingData }) => return false; }; +export const requestHeaders = {authorization: 'Bearer testBearerToken'}; +const UseRequestHeadersAccess: FieldAccess = ({ req: { headers } }) => { + return !!headers && headers.authorization === requestHeaders.authorization; +}; + export default buildConfig({ collections: [ { @@ -115,6 +121,21 @@ export default buildConfig({ }, ], }, + { + slug: relyOnRequestHeadersSlug, + access: { + create: UseRequestHeadersAccess, + read: UseRequestHeadersAccess, + update: UseRequestHeadersAccess, + delete: UseRequestHeadersAccess, + }, + fields: [ + { + name: 'name', + type: 'text', + }, + ], + }, ], onInit: async (payload) => { await payload.create({ diff --git a/test/access-control/int.spec.ts b/test/access-control/int.spec.ts index b3a97db5ca..391d3fe3e0 100644 --- a/test/access-control/int.spec.ts +++ b/test/access-control/int.spec.ts @@ -1,9 +1,11 @@ import mongoose from 'mongoose'; import payload from '../../src'; +import type { Options as CreateOptions } from '../../src/collections/operations/local/create'; import { Forbidden } from '../../src/errors'; +import type { PayloadRequest } from '../../src/types'; import { initPayloadTest } from '../helpers/configHelpers'; -import { restrictedSlug, siblingDataSlug, slug } from './config'; -import type { Restricted, Post, SiblingDatum } from './payload-types'; +import { relyOnRequestHeadersSlug, requestHeaders, restrictedSlug, siblingDataSlug, slug } from './config'; +import type { Restricted, Post, SiblingDatum, RelyOnRequestHeader } from './payload-types'; import { firstArrayText, secondArrayText } from './shared'; describe('Access Control', () => { @@ -74,7 +76,7 @@ describe('Access Control', () => { describe('Collections', () => { describe('restricted collection', () => { it('field without read access should not show', async () => { - const { id } = await createDoc({ restrictedField: 'restricted' }); + const { id } = await createDoc({ restrictedField: 'restricted' }); const retrievedDoc = await payload.findByID({ collection: slug, id, overrideAccess: false }); @@ -82,7 +84,7 @@ describe('Access Control', () => { }); it('field without read access should not show when overrideAccess: true', async () => { - const { id, restrictedField } = await createDoc({ restrictedField: 'restricted' }); + const { id, restrictedField } = await createDoc({ restrictedField: 'restricted' }); const retrievedDoc = await payload.findByID({ collection: slug, id, overrideAccess: true }); @@ -90,13 +92,59 @@ describe('Access Control', () => { }); it('field without read access should not show when overrideAccess default', async () => { - const { id, restrictedField } = await createDoc({ restrictedField: 'restricted' }); + const { id, restrictedField } = await createDoc({ restrictedField: 'restricted' }); const retrievedDoc = await payload.findByID({ collection: slug, id }); expect(retrievedDoc.restrictedField).toEqual(restrictedField); }); }); + describe('non-enumerated request properties passed to access control', () => { + it('access control ok when passing request headers', async () => { + const req = Object.defineProperty({}, 'headers', { + value: requestHeaders, + enumerable: false, + }) as PayloadRequest; + const name = 'name'; + const overrideAccess = false; + + const { id } = await createDoc({ name }, relyOnRequestHeadersSlug, { req, overrideAccess }); + const docById = await payload.findByID({ collection: relyOnRequestHeadersSlug, id, req, overrideAccess }); + const { docs: docsByName } = await payload.find({ + collection: relyOnRequestHeadersSlug, + where: { + name: { + equals: name, + }, + }, + req, + overrideAccess, + }); + + expect(docById).not.toBeUndefined(); + expect(docsByName.length).toBeGreaterThan(0); + }); + + it('access control fails when omitting request headers', async () => { + const name = 'name'; + const overrideAccess = false; + + await expect(() => createDoc({ name }, relyOnRequestHeadersSlug, { overrideAccess })).rejects.toThrow(Forbidden); + const { id } = await createDoc({ name }, relyOnRequestHeadersSlug); + + await expect(() => payload.findByID({ collection: relyOnRequestHeadersSlug, id, overrideAccess })).rejects.toThrow(Forbidden); + + await expect(() => payload.find({ + collection: relyOnRequestHeadersSlug, + where: { + name: { + equals: name, + }, + }, + overrideAccess, + })).rejects.toThrow(Forbidden); + }); + }); }); describe('Override Access', () => { @@ -172,9 +220,10 @@ describe('Access Control', () => { }); }); -async function createDoc(data: Partial): Promise { - return payload.create({ - collection: slug, +async function createDoc(data: Partial, overrideSlug = slug, options?: Partial>): Promise { + return payload.create({ + ...options, + collection: overrideSlug, data: data ?? {}, }); } diff --git a/test/access-control/payload-types.ts b/test/access-control/payload-types.ts index 1a6091d0a2..87bc313598 100644 --- a/test/access-control/payload-types.ts +++ b/test/access-control/payload-types.ts @@ -60,6 +60,16 @@ export interface SiblingDatum { createdAt: string; updatedAt: string; } +/** + * This interface was referenced by `Config`'s JSON-Schema + * via the `definition` "rely-on-request-headers". + */ +export interface RelyOnRequestHeader { + id: string; + name?: string; + createdAt: string; + updatedAt: string; +} /** * This interface was referenced by `Config`'s JSON-Schema * via the `definition` "users".