From 874279c530bcc8cc720514b481893c59992feb26 Mon Sep 17 00:00:00 2001 From: Jacob Fletcher Date: Mon, 29 Jul 2024 12:57:57 -0400 Subject: [PATCH] fix(next): infinite loop when logging into root admin (#7412) --- .../src/utilities/initPage/handleAdminPage.ts | 10 ++--- .../utilities/initPage/handleAuthRedirect.ts | 4 +- .../next/src/utilities/initPage/shared.ts | 34 ++++++++++++++-- test/access-control/e2e.spec.ts | 4 +- test/admin-root/app/favicon.ico | Bin 0 -> 15406 bytes test/admin-root/config.ts | 7 ++++ test/admin-root/e2e.spec.ts | 4 +- test/admin/e2e/1/e2e.spec.ts | 6 +-- test/admin/e2e/2/e2e.spec.ts | 6 +-- test/auth/e2e.spec.ts | 4 +- test/helpers.ts | 37 ++++++++++++------ tsconfig.json | 2 +- 12 files changed, 82 insertions(+), 36 deletions(-) create mode 100644 test/admin-root/app/favicon.ico diff --git a/packages/next/src/utilities/initPage/handleAdminPage.ts b/packages/next/src/utilities/initPage/handleAdminPage.ts index bbc544a54..24868a322 100644 --- a/packages/next/src/utilities/initPage/handleAdminPage.ts +++ b/packages/next/src/utilities/initPage/handleAdminPage.ts @@ -7,7 +7,7 @@ import type { import { notFound } from 'next/navigation.js' -import { isAdminAuthRoute, isAdminRoute } from './shared.js' +import { getRouteWithoutAdmin, isAdminAuthRoute, isAdminRoute } from './shared.js' export const handleAdminPage = ({ adminRoute, @@ -20,9 +20,9 @@ export const handleAdminPage = ({ permissions: Permissions route: string }) => { - if (isAdminRoute(route, adminRoute)) { - const baseAdminRoute = adminRoute && adminRoute !== '/' ? route.replace(adminRoute, '') : route - const routeSegments = baseAdminRoute.split('/').filter(Boolean) + if (isAdminRoute({ adminRoute, config, route })) { + const routeWithoutAdmin = getRouteWithoutAdmin({ adminRoute, route }) + const routeSegments = routeWithoutAdmin.split('/').filter(Boolean) const [entityType, entitySlug, createOrID] = routeSegments const collectionSlug = entityType === 'collections' ? entitySlug : undefined const globalSlug = entityType === 'globals' ? entitySlug : undefined @@ -47,7 +47,7 @@ export const handleAdminPage = ({ } } - if (!permissions.canAccessAdmin && !isAdminAuthRoute(config, route, adminRoute)) { + if (!permissions.canAccessAdmin && !isAdminAuthRoute({ adminRoute, config, route })) { notFound() } diff --git a/packages/next/src/utilities/initPage/handleAuthRedirect.ts b/packages/next/src/utilities/initPage/handleAuthRedirect.ts index d6b66b9dc..5840ffda1 100644 --- a/packages/next/src/utilities/initPage/handleAuthRedirect.ts +++ b/packages/next/src/utilities/initPage/handleAuthRedirect.ts @@ -22,7 +22,7 @@ export const handleAuthRedirect = ({ routes: { admin: adminRoute }, } = config - if (!isAdminAuthRoute(config, route, adminRoute)) { + if (!isAdminAuthRoute({ adminRoute, config, route })) { if (searchParams && 'redirect' in searchParams) delete searchParams.redirect const redirectRoute = encodeURIComponent( @@ -36,7 +36,7 @@ export const handleAuthRedirect = ({ const customLoginRoute = typeof redirectUnauthenticatedUser === 'string' ? redirectUnauthenticatedUser : undefined - const loginRoute = isAdminRoute(route, adminRoute) + const loginRoute = isAdminRoute({ adminRoute, config, route }) ? adminLoginRoute : customLoginRoute || loginRouteFromConfig diff --git a/packages/next/src/utilities/initPage/shared.ts b/packages/next/src/utilities/initPage/shared.ts index ab5857723..986f573cd 100644 --- a/packages/next/src/utilities/initPage/shared.ts +++ b/packages/next/src/utilities/initPage/shared.ts @@ -11,16 +11,42 @@ const authRouteKeys: (keyof SanitizedConfig['admin']['routes'])[] = [ 'reset', ] -export const isAdminRoute = (route: string, adminRoute: string) => { - return route.startsWith(adminRoute) +export const isAdminRoute = ({ + adminRoute, + config, + route, +}: { + adminRoute: string + config: SanitizedConfig + route: string +}): boolean => { + return route.startsWith(adminRoute) && !isAdminAuthRoute({ adminRoute, config, route }) } -export const isAdminAuthRoute = (config: SanitizedConfig, route: string, adminRoute: string) => { +export const isAdminAuthRoute = ({ + adminRoute, + config, + route, +}: { + adminRoute: string + config: SanitizedConfig + route: string +}): boolean => { const authRoutes = config.admin?.routes ? Object.entries(config.admin.routes) .filter(([key]) => authRouteKeys.includes(key as keyof SanitizedConfig['admin']['routes'])) .map(([_, value]) => value) : [] - return authRoutes.some((r) => route.replace(adminRoute, '').startsWith(r)) + return authRoutes.some((r) => getRouteWithoutAdmin({ adminRoute, route }).startsWith(r)) +} + +export const getRouteWithoutAdmin = ({ + adminRoute, + route, +}: { + adminRoute: string + route: string +}): string => { + return adminRoute && adminRoute !== '/' ? route.replace(adminRoute, '') : route } diff --git a/test/access-control/e2e.spec.ts b/test/access-control/e2e.spec.ts index 9a2d35628..d8df8a8a0 100644 --- a/test/access-control/e2e.spec.ts +++ b/test/access-control/e2e.spec.ts @@ -18,7 +18,7 @@ import { closeNav, ensureCompilationIsDone, exactText, - getAdminRoutes, + getRoutes, initPageConsoleErrorCatch, login, openDocControls, @@ -99,7 +99,7 @@ describe('access control', () => { routes: { logout: logoutRoute }, }, routes: { admin: adminRoute }, - } = getAdminRoutes({}) + } = getRoutes({}) logoutURL = `${serverURL}${adminRoute}${logoutRoute}` }) diff --git a/test/admin-root/app/favicon.ico b/test/admin-root/app/favicon.ico new file mode 100644 index 0000000000000000000000000000000000000000..5a6dadf06e8b75752010608f3476bc250a0653bf GIT binary patch literal 15406 zcmeHOX>1i$6dsrV;t&6tS5!n0StJ;ZC~nc9xPV4uh!Qo532HQI+|pp8iHgQZLX^-} z3PmV81=*>xwE|^d3zW6c7TPzs<3Uui0EF>Y{!dd z;VVMr;fj!g6>_Xf0evl6DRlXoQ6R2*4#JhUTkqs_m3My+ROEfXz`<@ z&8vU3{yo-TYulAqe_`=B*56d4D1+N_`i8jjSx6kK0WdW7jo6MaA+qUhi2n5fgnz#? z^S6**tnq6wqzkOS{ZC}$TbcWJ&ml_&dgl?~YwiVp{)nu`Gq$rFgg@>BzIrH_nhap- z$-Y_6&G>;?V91>i-#-(GHWhi_Oo-r^kPM>r?*l*Yp7eZF-461eAHaxSwzKZIj~}i` z-@gZE83>CW1gUNkNNZjOY4s$W)6avr;u#PYKH%=3vIeZVn8me_HOTsUDuo;xNza6^ zj>Ag&DP4i--1U4#*;ic^p|&jea?2>M-$aXOTC__=*G;F?e7otFQs!>j{pojP(>owH z{Rq*&KkQvk^sMuo_sm}Bu@UR&(KSY-6~a}6dVRmxvNa90hd$kS=qg|Fn7ZPiFD`r1 zS9)&w+2v0_r1h_tl)l!{QZ|3#8av+Gf)+3wW1+oFiCYAU8oV=ocBDU*Wc21}BauoN>=(bPMX8j2G1*3~Y z-zWPvFIf}`OJAOWtaE5z@ozr^mr@QE-*o;?=NI_Dw3D9%U9{X3tiAb3@G%#jJ@BT^ zIk!$0D$5#BhVj0dH<{=rv6VG;&MBKrm@Vw#in3YZIiWiI38hmV7LX(z-F&F(oz zloCh2w4&QSNyd9@hPIgySnLY*S>SD28SQZ^=4N975=X0jJGbU5Yp;HC(+oTVn|A2z zKGD1w__=olU;b(ED=n)rmUwsI@yF<_-yO5H4}yAeE5<)Z1|C11KUuui4j%8dt4CNo z*K5uNsV%bcP4v48F7!Ag7>^NZ9}2wu$$xCb+Eeg@p@W2R=OD@23k&&w4DoC#er$33 z<-Ga_p-pYw=82zt>p{Sm2?P0m<3V_5t`So%2T!%*C7 zB3{QMf2FnagqqQyb>|3qVYauKdIEIe4Ctt%4Cy?Of1GlJNaq^z6?Tnoo?7xT5*CdG zt$m;8-fPF7&Ar;XwNKVL**DScpELh|GX9=whh~GH#-z{o>dG=Uzr`hwVT^QDHgHqk zG2)#pS1gmB_WOBk`)4?x;wkvdco*A?DGxBRehS9yCMR>-FvpGZHAP&{x#lbS!Ap8u zNCRozYb9T!IrE77g4Vt#MgG)~I65yF_-VdVUT2!z2#Q&G6(e#9bDHwY>&;n?z4$u$ znvtk))`8Me=b6VKEEt1(vE;=9h$|=b`52I%qj)DC+CgjE?P=ezd~2)d&tru(&`rhaNl(o_0kT+Us_wi%_Bc|2+N0YrAOBxPJBV_FDSumBiMrIP7y}Mh<(32P#+vRo=}8^2q!i3~ zGSvu>z9+jUrQmbR=3({^*+4zDgV}(zkF$z4l5!G#NYz7pnRASXuH%T)nKzh%xqt?e zMYMjJFZgL}DBqoOj{?oo+T5KemsB1fXy$|>15YaE(_^YQ;B2`FN-&sZHKv8 z>E?3L+cED}s(S%*FUr}TtDf7$@?rGOW9VaVW`1_<>S1i#4=V1}>g7hD^FN())N_~{ zb7n1*1D*Zp|47KcNB`4Mbez5Hao1?|-*`?TW7nT!kc6FLz=iPNw|yxW0%P#~EBC?D zOK~fY1MRC>D96C&ourI0lXo(I!^%NnykIUwkk^AX-`Sk$&YvOt*UB{s { page = await context.newPage() initPageConsoleErrorCatch(page) + await login({ page, serverURL, customRoutes: { admin: adminRoute } }) + await ensureCompilationIsDone({ customRoutes: { admin: adminRoute, diff --git a/test/admin/e2e/1/e2e.spec.ts b/test/admin/e2e/1/e2e.spec.ts index 3d2fb825c..0a15bb7f7 100644 --- a/test/admin/e2e/1/e2e.spec.ts +++ b/test/admin/e2e/1/e2e.spec.ts @@ -10,7 +10,7 @@ import { checkPageTitle, ensureCompilationIsDone, exactText, - getAdminRoutes, + getRoutes, initPageConsoleErrorCatch, openDocControls, openNav, @@ -76,7 +76,7 @@ describe('admin1', () => { let customFieldsURL: AdminUrlUtil let disableDuplicateURL: AdminUrlUtil let serverURL: string - let adminRoutes: ReturnType + let adminRoutes: ReturnType let loginURL: string beforeAll(async ({ browser }, testInfo) => { @@ -106,7 +106,7 @@ describe('admin1', () => { await ensureCompilationIsDone({ customAdminRoutes, page, serverURL }) - adminRoutes = getAdminRoutes({ customAdminRoutes }) + adminRoutes = getRoutes({ customAdminRoutes }) loginURL = `${serverURL}${adminRoutes.routes.admin}${adminRoutes.admin.routes.login}` }) diff --git a/test/admin/e2e/2/e2e.spec.ts b/test/admin/e2e/2/e2e.spec.ts index 4e8a061e5..3f930e52f 100644 --- a/test/admin/e2e/2/e2e.spec.ts +++ b/test/admin/e2e/2/e2e.spec.ts @@ -10,7 +10,7 @@ import type { Config, Geo, Post } from '../../payload-types.js' import { ensureCompilationIsDone, exactText, - getAdminRoutes, + getRoutes, initPageConsoleErrorCatch, openDocDrawer, openNav, @@ -44,7 +44,7 @@ describe('admin2', () => { let postsUrl: AdminUrlUtil let serverURL: string - let adminRoutes: ReturnType + let adminRoutes: ReturnType beforeAll(async ({ browser }, testInfo) => { const prebuild = Boolean(process.env.CI) @@ -69,7 +69,7 @@ describe('admin2', () => { await ensureCompilationIsDone({ customAdminRoutes, page, serverURL }) - adminRoutes = getAdminRoutes({ customAdminRoutes }) + adminRoutes = getRoutes({ customAdminRoutes }) }) beforeEach(async () => { await reInitializeDB({ diff --git a/test/auth/e2e.spec.ts b/test/auth/e2e.spec.ts index 4f46b329b..0ef4bf838 100644 --- a/test/auth/e2e.spec.ts +++ b/test/auth/e2e.spec.ts @@ -13,7 +13,7 @@ import type { Config } from './payload-types.js' import { ensureCompilationIsDone, - getAdminRoutes, + getRoutes, initPageConsoleErrorCatch, saveDocAndAssert, } from '../helpers.js' @@ -49,7 +49,7 @@ const createFirstUser = async ({ routes: { createFirstUser: createFirstUserRoute }, }, routes: { admin: adminRoute }, - } = getAdminRoutes({ + } = getRoutes({ customAdminRoutes, customRoutes, }) diff --git a/test/helpers.ts b/test/helpers.ts index d5fe4f967..28a13b6f0 100644 --- a/test/helpers.ts +++ b/test/helpers.ts @@ -1,6 +1,7 @@ import type { BrowserContext, ChromiumBrowserContext, Locator, Page } from '@playwright/test' import type { Config } from 'payload' +import { formatAdminURL } from '@payloadcms/ui/shared' import { expect } from '@playwright/test' import { defaults } from 'payload' import { wait } from 'payload/shared' @@ -65,7 +66,7 @@ export async function ensureCompilationIsDone({ }): Promise { const { routes: { admin: adminRoute }, - } = getAdminRoutes({ customAdminRoutes, customRoutes }) + } = getRoutes({ customAdminRoutes, customRoutes }) const adminURL = `${serverURL}${adminRoute}` @@ -114,7 +115,7 @@ export async function firstRegister(args: FirstRegisterArgs): Promise { const { routes: { admin: adminRoute }, - } = getAdminRoutes({ customAdminRoutes, customRoutes }) + } = getRoutes({ customAdminRoutes, customRoutes }) await page.goto(`${serverURL}${adminRoute}`) await page.fill('#field-email', devUser.email) @@ -130,27 +131,37 @@ export async function login(args: LoginArgs): Promise { const { admin: { - routes: { createFirstUser: createFirstUserRoute, login: loginRoute }, + routes: { createFirstUser, login: incomingLoginRoute }, }, - routes: { admin: adminRoute }, - } = getAdminRoutes({ customAdminRoutes, customRoutes }) + routes: { admin: incomingAdminRoute }, + } = getRoutes({ customAdminRoutes, customRoutes }) - await page.goto(`${serverURL}${adminRoute}${loginRoute}`) - await page.waitForURL(`${serverURL}${adminRoute}${loginRoute}`) + const adminRoute = formatAdminURL({ serverURL, adminRoute: incomingAdminRoute, path: '' }) + const loginRoute = formatAdminURL({ + serverURL, + adminRoute: incomingAdminRoute, + path: incomingLoginRoute, + }) + const createFirstUserRoute = formatAdminURL({ + serverURL, + adminRoute: incomingAdminRoute, + path: createFirstUser, + }) + + await page.goto(loginRoute) + await page.waitForURL(loginRoute) 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}${adminRoute}`) + await page.waitForURL(adminRoute) - await expect(() => expect(page.url()).not.toContain(`${adminRoute}${loginRoute}`)).toPass({ + await expect(() => expect(page.url()).not.toContain(loginRoute)).toPass({ timeout: POLL_TOPASS_TIMEOUT, }) - await expect(() => - expect(page.url()).not.toContain(`${adminRoute}${createFirstUserRoute}`), - ).toPass({ + await expect(() => expect(page.url()).not.toContain(createFirstUserRoute)).toPass({ timeout: POLL_TOPASS_TIMEOUT, }) } @@ -328,7 +339,7 @@ export function describeIfInCIOrHasLocalstack(): jest.Describe { type AdminRoutes = Config['admin']['routes'] -export function getAdminRoutes({ +export function getRoutes({ customAdminRoutes, customRoutes, }: { diff --git a/tsconfig.json b/tsconfig.json index cdabe07c2..f14df6abb 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -37,7 +37,7 @@ ], "paths": { "@payload-config": [ - "./test/admin/config.ts" + "./test/_community/config.ts" ], "@payloadcms/live-preview": [ "./packages/live-preview/src"