From 6cd5b253f18444d51657701bbe8ed6c333aba153 Mon Sep 17 00:00:00 2001 From: Jacob Fletcher Date: Wed, 17 Apr 2024 10:31:39 -0400 Subject: [PATCH] fix(next): admin access control (#5887) --- packages/next/src/utilities/initPage.ts | 16 +++++++++++-- test/access-control/config.ts | 23 ++++++++++++++---- test/access-control/e2e.spec.ts | 31 +++++++++++++++++++++++-- test/access-control/shared.ts | 2 ++ test/helpers.ts | 22 ++++++++++++++---- test/helpers/sdk/index.ts | 12 ++++++++++ test/helpers/sdk/types.ts | 13 ++++++++++- tsconfig.json | 4 ++-- 8 files changed, 108 insertions(+), 15 deletions(-) diff --git a/packages/next/src/utilities/initPage.ts b/packages/next/src/utilities/initPage.ts index efcededc7..561675a28 100644 --- a/packages/next/src/utilities/initPage.ts +++ b/packages/next/src/utilities/initPage.ts @@ -25,6 +25,8 @@ type Args = { searchParams: { [key: string]: string | string[] | undefined } } +const authRoutes = ['/login', '/logout', '/create-first-user', '/forgot', '/reset', '/verify'] + export const initPage = async ({ config: configPromise, redirectUnauthenticatedUser = false, @@ -79,13 +81,19 @@ export const initPage = async ({ .filter(Boolean), } - const routeSegments = route.replace(payload.config.routes.admin, '').split('/').filter(Boolean) + const { + routes: { admin: adminRoute }, + } = payload.config + + const routeSegments = route.replace(adminRoute, '').split('/').filter(Boolean) const [entityType, entitySlug, createOrID] = routeSegments const collectionSlug = entityType === 'collections' ? entitySlug : undefined const globalSlug = entityType === 'globals' ? entitySlug : undefined const docID = collectionSlug && createOrID !== 'create' ? createOrID : undefined - if (redirectUnauthenticatedUser && !user && route !== '/login') { + const isAuthRoute = authRoutes.some((r) => r === route.replace(adminRoute, '')) + + if (redirectUnauthenticatedUser && !user && !isAuthRoute) { if (searchParams && 'redirect' in searchParams) delete searchParams.redirect const stringifiedSearchParams = Object.keys(searchParams ?? {}).length @@ -95,6 +103,10 @@ export const initPage = async ({ redirect(`${routes.admin}/login?redirect=${route + stringifiedSearchParams}`) } + if (!permissions.canAccessAdmin && !isAuthRoute) { + notFound() + } + let collectionConfig: SanitizedCollectionConfig let globalConfig: SanitizedGlobalConfig diff --git a/test/access-control/config.ts b/test/access-control/config.ts index 450de34e1..4b38c242e 100644 --- a/test/access-control/config.ts +++ b/test/access-control/config.ts @@ -8,6 +8,7 @@ import { firstArrayText, hiddenAccessSlug, hiddenFieldsSlug, + noAdminAccessEmail, readOnlySlug, relyOnRequestHeadersSlug, restrictedSlug, @@ -41,6 +42,7 @@ const UseRequestHeadersAccess: FieldAccess = ({ req: { headers } }) => { export default buildConfigWithDefaults({ admin: { user: 'users', + autoLogin: false, }, globals: [ { @@ -76,12 +78,17 @@ export default buildConfigWithDefaults({ slug: 'users', auth: true, access: { - // admin: () => true, - admin: async () => - new Promise((resolve) => { + // admin: () => true, + admin: async ({ req }) => { + if (req.user?.email === noAdminAccessEmail) { + return false + } + + return new Promise((resolve) => { // Simulate a request to an external service to determine access, i.e. another instance of Payload setTimeout(resolve, 50, true) // set to 'true' or 'false' here to simulate the response - }), + }) + }, }, fields: [ { @@ -431,6 +438,14 @@ export default buildConfigWithDefaults({ }, }) + await payload.create({ + collection: 'users', + data: { + email: noAdminAccessEmail, + password: 'test', + }, + }) + await payload.create({ collection: slug, data: { diff --git a/test/access-control/e2e.spec.ts b/test/access-control/e2e.spec.ts index e14431dcd..b588e4a84 100644 --- a/test/access-control/e2e.spec.ts +++ b/test/access-control/e2e.spec.ts @@ -1,8 +1,10 @@ import type { Page } from '@playwright/test' -import type { Payload, TypeWithID } from 'payload/types' +import type { TypeWithID } from 'payload/types' import { expect, test } from '@playwright/test' +import { devUser } from 'credentials.js' import path from 'path' +import { wait } from 'payload/utilities' import { fileURLToPath } from 'url' import type { PayloadTestSDK } from '../helpers/sdk/index.js' @@ -13,6 +15,7 @@ import { ensureAutoLoginAndCompilationIsDone, exactText, initPageConsoleErrorCatch, + login, openDocControls, openNav, saveDocAndAssert, @@ -22,6 +25,7 @@ import { initPayloadE2ENoConfig } from '../helpers/initPayloadE2ENoConfig.js' import { POLL_TOPASS_TIMEOUT } from '../playwright.config.js' import { docLevelAccessSlug, + noAdminAccessEmail, readOnlySlug, restrictedSlug, restrictedVersionsSlug, @@ -61,7 +65,8 @@ describe('access control', () => { const context = await browser.newContext() page = await context.newPage() initPageConsoleErrorCatch(page) - await ensureAutoLoginAndCompilationIsDone({ page, serverURL }) + + await login({ page, serverURL }) }) test('field without read access should not show', async () => { @@ -328,6 +333,28 @@ describe('access control', () => { // ensure user is allowed to edit this document await expect(documentDrawer2.locator('#field-name')).toBeEnabled() }) + + test('should completely block admin access', async () => { + const adminURL = `${serverURL}/admin` + await page.goto(adminURL) + await page.waitForURL(adminURL) + + await expect(page.locator('.dashboard')).toBeVisible() + + await page.goto(`${serverURL}/admin/logout`) + await page.waitForURL(`${serverURL}/admin/logout`) + + await login({ + page, + serverURL, + data: { + email: noAdminAccessEmail, + password: 'test', + }, + }) + + await expect(page.locator('.next-error-h1')).toBeVisible() + }) }) // eslint-disable-next-line @typescript-eslint/require-await diff --git a/test/access-control/shared.ts b/test/access-control/shared.ts index ef1ed5eaa..0b98730f1 100644 --- a/test/access-control/shared.ts +++ b/test/access-control/shared.ts @@ -14,3 +14,5 @@ export const docLevelAccessSlug = 'doc-level-access' export const hiddenFieldsSlug = 'hidden-fields' export const hiddenAccessSlug = 'hidden-access' + +export const noAdminAccessEmail = 'no-admin-access@payloadcms.com' diff --git a/test/helpers.ts b/test/helpers.ts index 7497a65d3..9da25609c 100644 --- a/test/helpers.ts +++ b/test/helpers.ts @@ -13,6 +13,10 @@ type FirstRegisterArgs = { } type LoginArgs = { + data?: { + email: string + password: string + } page: Page serverURL: string } @@ -91,14 +95,24 @@ export async function firstRegister(args: FirstRegisterArgs): Promise { } export async function login(args: LoginArgs): Promise { - const { page, serverURL } = args + const { page, serverURL, data = devUser } = args - await page.goto(`${serverURL}/admin`) - await page.fill('#field-email', devUser.email) - await page.fill('#field-password', devUser.password) + await page.goto(`${serverURL}/admin/login`) + await page.waitForURL(`${serverURL}/admin/login`) + await wait(500) + await page.fill('#field-email', data.email) + await page.fill('#field-password', data.password) await wait(500) await page.click('[type=submit]') await page.waitForURL(`${serverURL}/admin`) + + await expect(() => expect(page.url()).not.toContain(`/admin/login`)).toPass({ + timeout: POLL_TOPASS_TIMEOUT, + }) + + await expect(() => expect(page.url()).not.toContain(`/admin/create-first-user`)).toPass({ + timeout: POLL_TOPASS_TIMEOUT, + }) } export async function saveDocHotkeyAndAssert(page: Page): Promise { diff --git a/test/helpers/sdk/index.ts b/test/helpers/sdk/index.ts index c89c13e88..5998f13a2 100644 --- a/test/helpers/sdk/index.ts +++ b/test/helpers/sdk/index.ts @@ -7,6 +7,7 @@ import type { FetchOptions, FindArgs, GeneratedTypes, + LoginArgs, UpdateArgs, UpdateGlobalArgs, } from './types.js' @@ -70,6 +71,17 @@ export class PayloadTestSDK({ + jwt, + ...args + }: LoginArgs) => { + return this.fetch({ + operation: 'login', + args, + jwt, + }) + } + sendEmail = async ({ jwt, ...args }: { jwt?: string } & SendMailOptions): Promise => { return this.fetch({ operation: 'sendEmail', diff --git a/test/helpers/sdk/types.ts b/test/helpers/sdk/types.ts index f107a5a62..4dbaaf7f4 100644 --- a/test/helpers/sdk/types.ts +++ b/test/helpers/sdk/types.ts @@ -25,7 +25,7 @@ export type GeneratedTypes = { export type FetchOptions = { args?: Record jwt?: string - operation: 'create' | 'delete' | 'find' | 'sendEmail' | 'update' | 'updateGlobal' + operation: 'create' | 'delete' | 'find' | 'login' | 'sendEmail' | 'update' | 'updateGlobal' reduceJSON?: (json: any) => R } @@ -122,6 +122,17 @@ export type FindArgs< where?: Where } & BaseArgs +export type LoginArgs< + TGeneratedTypes extends GeneratedTypes, + TSlug extends keyof TGeneratedTypes['collections'], +> = { + collection: TSlug + data: { + email: string + password: string + } +} & BaseArgs + export type DeleteArgs< TGeneratedTypes extends GeneratedTypes, TSlug extends keyof TGeneratedTypes['collections'], diff --git a/tsconfig.json b/tsconfig.json index d55001935..046160a5d 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -37,7 +37,7 @@ ], "paths": { "@payload-config": [ - "./test/_community/config.ts" + "./test/access-control/config.ts" ], "@payloadcms/live-preview": [ "./packages/live-preview/src" @@ -161,4 +161,4 @@ ".next/types/**/*.ts", "scripts/**/*.ts" ] -} +} \ No newline at end of file