Skip to content

Commit dfb0021

Browse files
authored
fix: client config context inheritance (#13790)
Redirecting from login to any non-auth collection crashes the page with the following error: ``` Cannot read properties of null (reading 'fields') ``` TL;DR: the page-level config context was threading stale methods from the root config provider. #### Background The client config is now gated behind authentication as of #13714, so it changes based on authentication status. If the root layout is mounted before authentication, it puts the unauthenticated client config into state for the entire app to consume. On login, the root layout does not re-render, so the page itself needs to generate a fresh client config and sync it up. This leads to race conditions, however, where if the login page included a `?redirect=` param, the redirect would take place _before_ the page-level client config could sync to the layout, and ultimately crash the page. This was addressed in #13786. While this fixed redirects to the "users" collection, this collection is _already_ included in the client config (soon to be omitted by #13785). So if you redirect to any other collection, the above error occurs. #### Problem The page-level config context is only overriding the `config` property, keeping stale methods from the root config provider. This means calling `getEntityConfig` during this moment in the time would reference the stale config, although `config` itself would be fresh. #### Solution Wrap the page with an entirely new context provider. Do not thread inherited methods from the root provider, this way all new methods get instantiated using the fresh config. --- - To see the specific tasks where the Asana app for GitHub is being used, see below: - https://app.asana.com/0/0/1211332845301596
1 parent 4278e72 commit dfb0021

File tree

11 files changed

+211
-158
lines changed

11 files changed

+211
-158
lines changed

packages/next/src/views/Root/index.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import type {
1010
SanitizedGlobalConfig,
1111
} from 'payload'
1212

13-
import { RootPageConfigProvider } from '@payloadcms/ui'
13+
import { PageConfigProvider } from '@payloadcms/ui'
1414
import { RenderServerComponent } from '@payloadcms/ui/elements/RenderServerComponent'
1515
import { getClientConfig } from '@payloadcms/ui/utilities/getClientConfig'
1616
import { notFound, redirect } from 'next/navigation.js'
@@ -300,7 +300,7 @@ export const RootPage = async ({
300300
})
301301

302302
return (
303-
<RootPageConfigProvider config={clientConfig}>
303+
<PageConfigProvider config={clientConfig}>
304304
{!templateType && <React.Fragment>{RenderedView}</React.Fragment>}
305305
{templateType === 'minimal' && (
306306
<MinimalTemplate className={templateClassName}>{RenderedView}</MinimalTemplate>
@@ -331,6 +331,6 @@ export const RootPage = async ({
331331
{RenderedView}
332332
</DefaultTemplate>
333333
)}
334-
</RootPageConfigProvider>
334+
</PageConfigProvider>
335335
)
336336
}

packages/payload/src/config/client.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ export type UnauthenticatedClientConfig = {
6363
globals: []
6464
routes: ClientConfig['routes']
6565
serverURL: ClientConfig['serverURL']
66-
unauthenticated: ClientConfig['unauthenticated']
66+
unauthenticated: true
6767
}
6868

6969
export const serverOnlyAdminConfigProperties: readonly Partial<ServerOnlyRootAdminProperties>[] = []

packages/ui/src/exports/client/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ export {
303303
RouteTransitionProvider,
304304
useRouteTransition,
305305
} from '../../providers/RouteTransition/index.js'
306-
export { ConfigProvider, RootPageConfigProvider, useConfig } from '../../providers/Config/index.js'
306+
export { ConfigProvider, PageConfigProvider, useConfig } from '../../providers/Config/index.js'
307307
export { DocumentEventsProvider, useDocumentEvents } from '../../providers/DocumentEvents/index.js'
308308
export { DocumentInfoProvider, useDocumentInfo } from '../../providers/DocumentInfo/index.js'
309309
export { useDocumentTitle } from '../../providers/DocumentTitle/index.js'

packages/ui/src/providers/Config/index.tsx

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -100,41 +100,42 @@ export const ConfigProvider: React.FC<{
100100
export const useConfig = (): ClientConfigContext => use(RootConfigContext)
101101

102102
/**
103-
* This provider shadows the ConfigProvider on the _page_ level, allowing us to
103+
* This provider shadows the `ConfigProvider` on the _page_ level, allowing us to
104104
* update the config when needed, e.g. after authentication.
105-
* The layout ConfigProvider is not updated on page navigation / authentication,
105+
* The layout `ConfigProvider` is not updated on page navigation / authentication,
106106
* as the layout does not re-render in those cases.
107107
*
108108
* If the config here has the same reference as the config from the layout, we
109109
* simply reuse the context from the layout to avoid unnecessary re-renders.
110+
*
111+
* @experimental This component is experimental and may change or be removed in future releases. Use at your own discretion.
110112
*/
111-
export const RootPageConfigProvider: React.FC<{
113+
export const PageConfigProvider: React.FC<{
112114
readonly children: React.ReactNode
113115
readonly config: ClientConfig
114116
}> = ({ children, config: configFromProps }) => {
115-
const rootLayoutConfig = useConfig()
116-
const { config, getEntityConfig, setConfig } = rootLayoutConfig
117+
const { config: rootConfig, setConfig: setRootConfig } = useConfig()
117118

118119
/**
119-
* This useEffect is required in order for the _page_ to be able to refresh the client config,
120-
* which may have been cached on the _layout_ level, where the ConfigProvider is managed.
120+
* This `useEffect` is required in order for the _page_ to be able to refresh the client config,
121+
* which may have been cached on the _layout_ level, where the `ConfigProvider` is managed.
121122
* Since the layout does not re-render on page navigation / authentication, we need to manually
122123
* update the config, as the user may have been authenticated in the process, which affects the client config.
123124
*/
124125
useEffect(() => {
125-
setConfig(configFromProps)
126-
}, [configFromProps, setConfig])
127-
128-
if (config !== configFromProps && config.unauthenticated !== configFromProps.unauthenticated) {
129-
// Between the unauthenticated config becoming authenticated (or the other way around) and the useEffect
130-
// running, the config will be stale. In order to avoid having the wrong config in the context in that
131-
// brief moment, we shadow the context here on the _page_ level and provide the updated config immediately.
132-
return (
133-
<RootConfigContext value={{ config: configFromProps, getEntityConfig, setConfig }}>
134-
{children}
135-
</RootConfigContext>
136-
)
126+
setRootConfig(configFromProps)
127+
}, [configFromProps, setRootConfig])
128+
129+
// If this component receives a different config than what is in context from the layout, it is stale.
130+
// While stale, we instantiate a new context provider that provides the new config until the root context is updated.
131+
// Unfortunately, referential equality alone does not work bc the reference is lost during server/client serialization,
132+
// so we need to also compare the `unauthenticated` property.
133+
if (
134+
rootConfig !== configFromProps &&
135+
rootConfig.unauthenticated !== configFromProps.unauthenticated
136+
) {
137+
return <ConfigProvider config={configFromProps}>{children}</ConfigProvider>
137138
}
138139

139-
return <RootConfigContext value={rootLayoutConfig}>{children}</RootConfigContext>
140+
return children
140141
}

test/access-control/e2e.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@ import {
1717
ensureCompilationIsDone,
1818
exactText,
1919
initPageConsoleErrorCatch,
20-
login,
2120
saveDocAndAssert,
2221
} from '../helpers.js'
2322
import { AdminUrlUtil } from '../helpers/adminUrlUtil.js'
23+
import { login } from '../helpers/e2e/auth/login.js'
2424
import { initPayloadE2ENoConfig } from '../helpers/initPayloadE2ENoConfig.js'
2525
import { POLL_TOPASS_TIMEOUT, TEST_TIMEOUT_LONG } from '../playwright.config.js'
2626
import {

test/admin-root/e2e.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@ import { fileURLToPath } from 'url'
88
import {
99
ensureCompilationIsDone,
1010
initPageConsoleErrorCatch,
11-
login,
1211
saveDocAndAssert,
1312
// throttleTest,
1413
} from '../helpers.js'
1514
import { AdminUrlUtil } from '../helpers/adminUrlUtil.js'
15+
import { login } from '../helpers/e2e/auth/login.js'
1616
import { initPayloadE2ENoConfig } from '../helpers/initPayloadE2ENoConfig.js'
1717
import { TEST_TIMEOUT_LONG } from '../playwright.config.js'
1818

test/auth/e2e.spec.ts

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import type { BrowserContext, Page } from '@playwright/test'
22

33
import { expect, test } from '@playwright/test'
44
import { devUser } from 'credentials.js'
5+
import { login } from 'helpers/e2e/auth/login.js'
6+
import { logout } from 'helpers/e2e/auth/logout.js'
57
import { openNav } from 'helpers/e2e/toggleNav.js'
68
import path from 'path'
79
import { fileURLToPath } from 'url'
@@ -15,7 +17,6 @@ import {
1517
exactText,
1618
getRoutes,
1719
initPageConsoleErrorCatch,
18-
login,
1920
saveDocAndAssert,
2021
} from '../helpers.js'
2122
import { AdminUrlUtil } from '../helpers/adminUrlUtil.js'
@@ -388,21 +389,24 @@ describe('Auth', () => {
388389
await saveDocAndAssert(page)
389390
})
390391

391-
test('ensure login page with redirect to users document redirects properly after login, without client error', async () => {
392-
await page.goto(url.admin)
393-
394-
await page.goto(`${serverURL}/admin/logout`)
395-
396-
await expect(page.locator('.login')).toBeVisible()
397-
392+
test('ensure `?redirect=` param is injected into the URL and handled properly after login', async () => {
398393
const users = await payload.find({
399394
collection: slug,
400395
limit: 1,
401396
})
397+
402398
const userDocumentRoute = `${serverURL}/admin/collections/users/${users?.docs?.[0]?.id}`
403399

400+
await logout(page, serverURL)
401+
402+
// This will send the user back to the login page with a `?redirect=` param
404403
await page.goto(userDocumentRoute)
405404

405+
await expect
406+
.poll(() => page.url(), { timeout: POLL_TOPASS_TIMEOUT })
407+
.toContain('/admin/login?redirect=')
408+
409+
// Important: do not use the login helper here, as this may clear the redirect param
406410
await expect(page.locator('#field-email')).toBeVisible()
407411
await expect(page.locator('#field-password')).toBeVisible()
408412

@@ -414,8 +418,37 @@ describe('Auth', () => {
414418
.toBe(userDocumentRoute)
415419

416420
// Previously, this would crash the page with a "Cannot read properties of undefined (reading 'match')" error
417-
418421
await expect(page.locator('#field-roles')).toBeVisible()
422+
423+
// Now do this again, only with a page that is not in the user's collection
424+
const notInUserCollection = await payload.create({
425+
collection: 'relationsCollection',
426+
data: {},
427+
})
428+
429+
await logout(page, serverURL)
430+
431+
await page.goto(
432+
`${serverURL}/admin/collections/relationsCollection/${notInUserCollection.id}`,
433+
)
434+
435+
await expect
436+
.poll(() => page.url(), { timeout: POLL_TOPASS_TIMEOUT })
437+
.toContain('/admin/login?redirect=')
438+
439+
// Important: do not use the login helper here, as this may clear the redirect param
440+
await expect(page.locator('#field-email')).toBeVisible()
441+
await expect(page.locator('#field-password')).toBeVisible()
442+
443+
await page.locator('.form-submit > button').click()
444+
445+
// Expect to be redirected to the correct page
446+
await expect
447+
.poll(() => page.url(), { timeout: POLL_TOPASS_TIMEOUT })
448+
.toBe(`${serverURL}/admin/collections/relationsCollection/${notInUserCollection.id}`)
449+
450+
// Previously, this would crash the page with a "Cannot read properties of null (reading 'fields')" error
451+
await expect(page.locator('#field-rel')).toBeVisible()
419452
})
420453
})
421454
})

test/helpers.ts

Lines changed: 1 addition & 116 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,7 @@ import { devUser } from './credentials.js'
1818
import { openNav } from './helpers/e2e/toggleNav.js'
1919
import { POLL_TOPASS_TIMEOUT } from './playwright.config.js'
2020

21-
type AdminRoutes = NonNullable<Config['admin']>['routes']
22-
23-
type LoginArgs = {
24-
customAdminRoutes?: AdminRoutes
25-
customRoutes?: Config['routes']
26-
data?: {
27-
email: string
28-
password: string
29-
}
30-
page: Page
31-
serverURL: string
32-
}
21+
export type AdminRoutes = NonNullable<Config['admin']>['routes']
3322

3423
const random = (min: number, max: number) => Math.floor(Math.random() * (max - min + 1)) + min
3524

@@ -177,110 +166,6 @@ export async function throttleTest({
177166
return client
178167
}
179168

180-
/**
181-
* Logs a user in by navigating via click-ops instead of using page.goto()
182-
*/
183-
export async function loginClientSide(args: LoginArgs): Promise<void> {
184-
const { customAdminRoutes, customRoutes, data = devUser, page, serverURL } = args
185-
const {
186-
routes: { admin: incomingAdminRoute } = {},
187-
admin: { routes: { login: incomingLoginRoute, createFirstUser } = {} },
188-
} = getRoutes({ customAdminRoutes, customRoutes })
189-
190-
const adminRoute = formatAdminURL({ serverURL, adminRoute: incomingAdminRoute, path: '' })
191-
const loginRoute = formatAdminURL({
192-
serverURL,
193-
adminRoute: incomingAdminRoute,
194-
path: incomingLoginRoute,
195-
})
196-
const createFirstUserRoute = formatAdminURL({
197-
serverURL,
198-
adminRoute: incomingAdminRoute,
199-
path: createFirstUser,
200-
})
201-
202-
if ((await page.locator('#nav-toggler').count()) > 0) {
203-
// a user is already logged in - log them out
204-
await openNav(page)
205-
await expect(page.locator('.nav__controls [aria-label="Log out"]')).toBeVisible()
206-
await page.locator('.nav__controls [aria-label="Log out"]').click()
207-
208-
if (await page.locator('dialog#leave-without-saving').isVisible()) {
209-
await page.locator('dialog#leave-without-saving #confirm-action').click()
210-
}
211-
212-
await page.waitForURL(loginRoute)
213-
}
214-
215-
await wait(500)
216-
await page.fill('#field-email', data.email)
217-
await page.fill('#field-password', data.password)
218-
await wait(500)
219-
await page.click('[type=submit]')
220-
221-
await expect(page.locator('.step-nav__home')).toBeVisible()
222-
if ((await page.locator('a.step-nav__home').count()) > 0) {
223-
await page.locator('a.step-nav__home').click()
224-
}
225-
226-
await page.waitForURL(adminRoute)
227-
228-
await expect(() => expect(page.url()).not.toContain(loginRoute)).toPass({
229-
timeout: POLL_TOPASS_TIMEOUT,
230-
})
231-
await expect(() => expect(page.url()).not.toContain(createFirstUserRoute)).toPass({
232-
timeout: POLL_TOPASS_TIMEOUT,
233-
})
234-
}
235-
236-
export async function login(args: LoginArgs): Promise<void> {
237-
const { customAdminRoutes, customRoutes, data = devUser, page, serverURL } = args
238-
239-
const {
240-
admin: {
241-
routes: { createFirstUser, login: incomingLoginRoute, logout: incomingLogoutRoute } = {},
242-
},
243-
routes: { admin: incomingAdminRoute } = {},
244-
} = getRoutes({ customAdminRoutes, customRoutes })
245-
246-
const logoutRoute = formatAdminURL({
247-
serverURL,
248-
adminRoute: incomingAdminRoute,
249-
path: incomingLogoutRoute,
250-
})
251-
252-
await page.goto(logoutRoute)
253-
await wait(500)
254-
255-
const adminRoute = formatAdminURL({ serverURL, adminRoute: incomingAdminRoute, path: '' })
256-
const loginRoute = formatAdminURL({
257-
serverURL,
258-
adminRoute: incomingAdminRoute,
259-
path: incomingLoginRoute,
260-
})
261-
const createFirstUserRoute = formatAdminURL({
262-
serverURL,
263-
adminRoute: incomingAdminRoute,
264-
path: createFirstUser,
265-
})
266-
267-
await page.goto(loginRoute)
268-
await wait(500)
269-
await page.fill('#field-email', data.email)
270-
await page.fill('#field-password', data.password)
271-
await wait(500)
272-
await page.click('[type=submit]')
273-
await page.waitForURL(adminRoute)
274-
275-
await expect(() => expect(page.url()).not.toContain(loginRoute)).toPass({
276-
timeout: POLL_TOPASS_TIMEOUT,
277-
})
278-
279-
await expect(() => expect(page.url()).not.toContain(createFirstUserRoute)).toPass({
280-
timeout: POLL_TOPASS_TIMEOUT,
281-
})
282-
}
283-
284169
export async function saveDocHotkeyAndAssert(page: Page): Promise<void> {
285170
const ua = page.evaluate(() => navigator.userAgent)
286171
const isMac = (await ua).includes('Mac OS X')

0 commit comments

Comments
 (0)