fix(ui): ensure document unlocks when logging out from edit view of a locked document (#13142)

### What?

Refactors the `LeaveWithoutSaving` modal to be generic and delegates
document unlock logic back to the `DefaultEditView` component via a
callback.

### Why?

Previously, `unlockDocument` was triggered in a cleanup `useEffect` in
the edit view. When logging out from the edit view, the unlock request
would often fail due to the session ending — leaving the document in a
locked state.

### How?

- Introduced `onConfirm` and `onPrevent` props for `LeaveWithoutSaving`.
- Moved all document lock/unlock logic into `DefaultEditView`’s
`handleLeaveConfirm`.
- Captures the next navigation target via `onPrevent` and evaluates
whether to unlock based on:
  - Locking being enabled.
  - Current user owning the lock.
- Navigation not targeting internal admin views (`/preview`, `/api`,
`/versions`).

---------

Co-authored-by: Jarrod Flesch <jarrodmflesch@gmail.com>
This commit is contained in:
Patrik
2025-07-24 12:18:49 -04:00
committed by GitHub
parent a83ed5ebb5
commit 7e81d30808
13 changed files with 473 additions and 301 deletions

View File

@@ -85,7 +85,14 @@ export const CreateFirstUserClient: React.FC<{
return (
<Form
action={`${serverURL}${apiRoute}/${userSlug}/first-register`}
initialState={initialState}
initialState={{
...initialState,
'confirm-password': {
...initialState['confirm-password'],
valid: initialState['confirm-password']['valid'] || false,
value: initialState['confirm-password']['value'] || '',
},
}}
method="POST"
onChange={[onChange]}
onSuccess={handleFirstRegister}

View File

@@ -12,7 +12,12 @@ import { usePreventLeave } from './usePreventLeave.js'
const modalSlug = 'leave-without-saving'
export const LeaveWithoutSaving: React.FC = () => {
type LeaveWithoutSavingProps = {
onConfirm?: () => Promise<void> | void
onPrevent?: (nextHref: null | string) => void
}
export const LeaveWithoutSaving: React.FC<LeaveWithoutSavingProps> = ({ onConfirm, onPrevent }) => {
const { closeModal, openModal } = useModal()
const modified = useFormModified()
const { isValid } = useForm()
@@ -22,23 +27,34 @@ export const LeaveWithoutSaving: React.FC = () => {
const prevent = Boolean((modified || !isValid) && user)
const onPrevent = useCallback(() => {
const handlePrevent = useCallback(() => {
const activeHref = (document.activeElement as HTMLAnchorElement)?.href || null
if (onPrevent) {
onPrevent(activeHref)
}
openModal(modalSlug)
}, [openModal])
}, [openModal, onPrevent])
const handleAccept = useCallback(() => {
closeModal(modalSlug)
}, [closeModal])
usePreventLeave({ hasAccepted, onAccept: handleAccept, onPrevent, prevent })
usePreventLeave({ hasAccepted, onAccept: handleAccept, onPrevent: handlePrevent, prevent })
const onCancel: OnCancel = useCallback(() => {
closeModal(modalSlug)
}, [closeModal])
const onConfirm = useCallback(() => {
const handleConfirm = useCallback(async () => {
if (onConfirm) {
try {
await onConfirm()
} catch (err) {
console.error('Error in LeaveWithoutSaving onConfirm:', err)
}
}
setHasAccepted(true)
}, [])
}, [onConfirm])
return (
<ConfirmationModal
@@ -48,7 +64,7 @@ export const LeaveWithoutSaving: React.FC = () => {
heading={t('general:leaveWithoutSaving')}
modalSlug={modalSlug}
onCancel={onCancel}
onConfirm={onConfirm}
onConfirm={handleConfirm}
/>
)
}

View File

@@ -36,6 +36,7 @@ export const RenderTitle: React.FC<RenderTitleProps> = (props) => {
className={[className, baseClass, idAsTitle && `${baseClass}--has-id`]
.filter(Boolean)
.join(' ')}
data-doc-id={id}
title={title}
>
{isInitializing ? (

View File

@@ -113,6 +113,16 @@ const DocumentInfo: React.FC<
'idle',
)
const documentLockState = useRef<{
hasShownLockedModal: boolean
isLocked: boolean
user: ClientUser | number | string
} | null>({
hasShownLockedModal: false,
isLocked: false,
user: null,
})
const updateUploadStatus = useCallback(
(status: 'failed' | 'idle' | 'uploading') => {
setUploadStatus(status)
@@ -344,6 +354,7 @@ const DocumentInfo: React.FC<
docConfig,
docPermissions,
documentIsLocked,
documentLockState,
getDocPermissions,
getDocPreferences,
hasPublishedDoc,

View File

@@ -49,6 +49,11 @@ export type DocumentInfoContext = {
currentEditor?: ClientUser | null | number | string
docConfig?: ClientCollectionConfig | ClientGlobalConfig
documentIsLocked?: boolean
documentLockState: React.RefObject<{
hasShownLockedModal: boolean
isLocked: boolean
user: ClientUser | number | string
} | null>
getDocPermissions: (data?: Data) => Promise<void>
getDocPreferences: () => Promise<DocumentPreferences>
incrementVersionCount: () => void

View File

@@ -70,6 +70,7 @@ export function DefaultEditView({
disableLeaveWithoutSaving,
docPermissions,
documentIsLocked,
documentLockState,
getDocPermissions,
getDocPreferences,
globalSlug,
@@ -164,16 +165,6 @@ export function DefaultEditView({
const isLockExpired = Date.now() > lockExpiryTime
const documentLockStateRef = useRef<{
hasShownLockedModal: boolean
isLocked: boolean
user: ClientUser | number | string
} | null>({
hasShownLockedModal: false,
isLocked: false,
user: null,
})
const schemaPathSegments = useMemo(() => [entitySlug], [entitySlug])
const [validateBeforeSubmit, setValidateBeforeSubmit] = useState(() => {
@@ -184,13 +175,15 @@ export function DefaultEditView({
return false
})
const nextHrefRef = React.useRef<null | string>(null)
const handleDocumentLocking = useCallback(
(lockedState: LockedState) => {
setDocumentIsLocked(true)
const previousOwnerID =
typeof documentLockStateRef.current?.user === 'object'
? documentLockStateRef.current?.user?.id
: documentLockStateRef.current?.user
typeof documentLockState.current?.user === 'object'
? documentLockState.current?.user?.id
: documentLockState.current?.user
if (lockedState) {
const lockedUserID =
@@ -198,14 +191,14 @@ export function DefaultEditView({
? lockedState.user
: lockedState.user.id
if (!documentLockStateRef.current || lockedUserID !== previousOwnerID) {
if (!documentLockState.current || lockedUserID !== previousOwnerID) {
if (previousOwnerID === user.id && lockedUserID !== user.id) {
setShowTakeOverModal(true)
documentLockStateRef.current.hasShownLockedModal = true
documentLockState.current.hasShownLockedModal = true
}
documentLockStateRef.current = {
hasShownLockedModal: documentLockStateRef.current?.hasShownLockedModal || false,
documentLockState.current = {
hasShownLockedModal: documentLockState.current?.hasShownLockedModal || false,
isLocked: true,
user: lockedState.user as ClientUser,
}
@@ -213,9 +206,52 @@ export function DefaultEditView({
}
}
},
[setCurrentEditor, setDocumentIsLocked, user?.id],
[documentLockState, setCurrentEditor, setDocumentIsLocked, user?.id],
)
const handlePrevent = useCallback((nextHref: null | string) => {
nextHrefRef.current = nextHref
}, [])
const handleLeaveConfirm = useCallback(async () => {
const lockUser = documentLockState.current?.user
const isLockOwnedByCurrentUser =
typeof lockUser === 'object' ? lockUser?.id === user?.id : lockUser === user?.id
if (isLockingEnabled && documentIsLocked && (id || globalSlug)) {
// Check where user is trying to go
const nextPath = nextHrefRef.current ? new URL(nextHrefRef.current).pathname : ''
const isInternalView = ['/preview', '/api', '/versions'].some((path) =>
nextPath.includes(path),
)
// Only retain the lock if the user is still viewing the document
if (!isInternalView) {
if (isLockOwnedByCurrentUser) {
try {
await unlockDocument(id, collectionSlug ?? globalSlug)
setDocumentIsLocked(false)
setCurrentEditor(null)
} catch (err) {
console.error('Failed to unlock before leave', err)
}
}
}
}
}, [
collectionSlug,
documentIsLocked,
documentLockState,
globalSlug,
id,
isLockingEnabled,
setCurrentEditor,
setDocumentIsLocked,
unlockDocument,
user?.id,
])
const onSave = useCallback(
async (json): Promise<FormState> => {
const controller = handleAbortRef(abortOnSaveRef)
@@ -342,7 +378,7 @@ export function DefaultEditView({
const docPreferences = await getDocPreferences()
const { lockedState, state } = await getFormState({
const result = await getFormState({
id,
collectionSlug,
docPermissions,
@@ -360,6 +396,12 @@ export function DefaultEditView({
updateLastEdited,
})
if (!result) {
return
}
const { lockedState, state } = result
if (isLockingEnabled) {
handleDocumentLocking(lockedState)
}
@@ -386,38 +428,9 @@ export function DefaultEditView({
// Clean up when the component unmounts or when the document is unlocked
useEffect(() => {
return () => {
if (isLockingEnabled && documentIsLocked && (id || globalSlug)) {
// Only retain the lock if the user is still viewing the document
const shouldUnlockDocument = !['preview', 'api', 'versions'].some((path) =>
window.location.pathname.includes(path),
)
if (shouldUnlockDocument) {
// Check if this user is still the current editor
if (
typeof documentLockStateRef.current?.user === 'object'
? documentLockStateRef.current?.user?.id === user?.id
: documentLockStateRef.current?.user === user?.id
) {
void unlockDocument(id, collectionSlug ?? globalSlug)
setDocumentIsLocked(false)
setCurrentEditor(null)
}
}
}
setShowTakeOverModal(false)
}
}, [
collectionSlug,
globalSlug,
id,
unlockDocument,
user,
setCurrentEditor,
isLockingEnabled,
documentIsLocked,
setDocumentIsLocked,
])
}, [])
useEffect(() => {
const abortOnChange = abortOnChangeRef.current
@@ -437,7 +450,7 @@ export function DefaultEditView({
: currentEditor !== user?.id) &&
!isReadOnlyForIncomingUser &&
!showTakeOverModal &&
!documentLockStateRef.current?.hasShownLockedModal &&
!documentLockState.current?.hasShownLockedModal &&
!isLockExpired
const isFolderCollection = config.folders && collectionSlug === config.folders?.slug
@@ -487,7 +500,7 @@ export function DefaultEditView({
false,
updateDocumentEditor,
setCurrentEditor,
documentLockStateRef,
documentLockState,
isLockingEnabled,
)
}
@@ -505,7 +518,9 @@ export function DefaultEditView({
}}
/>
)}
{!isReadOnlyForIncomingUser && preventLeaveWithoutSaving && <LeaveWithoutSaving />}
{!isReadOnlyForIncomingUser && preventLeaveWithoutSaving && (
<LeaveWithoutSaving onConfirm={handleLeaveConfirm} onPrevent={handlePrevent} />
)}
{!isInDrawer && (
<SetDocumentStepNav
collectionSlug={collectionConfig?.slug}
@@ -552,7 +567,7 @@ export function DefaultEditView({
true,
updateDocumentEditor,
setCurrentEditor,
documentLockStateRef,
documentLockState,
isLockingEnabled,
setIsReadOnlyForIncomingUser,
)

View File

@@ -2,10 +2,9 @@ import { fileURLToPath } from 'node:url'
import path from 'path'
const filename = fileURLToPath(import.meta.url)
const dirname = path.dirname(filename)
import { v4 as uuid } from 'uuid'
import { buildConfigWithDefaults } from '../buildConfigWithDefaults.js'
import { devUser } from '../credentials.js'
import { seed } from './seed.js'
import {
apiKeysSlug,
namedSaveToJWTValue,
@@ -263,33 +262,7 @@ export default buildConfigWithDefaults({
],
},
],
onInit: async (payload) => {
await payload.create({
collection: 'users',
data: {
custom: 'Hello, world!',
email: devUser.email,
password: devUser.password,
roles: ['admin'],
},
})
await payload.create({
collection: apiKeysSlug,
data: {
apiKey: uuid(),
enableAPIKey: true,
},
})
await payload.create({
collection: apiKeysSlug,
data: {
apiKey: uuid(),
enableAPIKey: true,
},
})
},
onInit: seed,
typescript: {
outputFile: path.resolve(dirname, 'payload-types.ts'),
},

View File

@@ -1,8 +1,8 @@
import type { BrowserContext, Page } from '@playwright/test'
import type { SanitizedConfig } from 'payload'
import { expect, test } from '@playwright/test'
import { devUser } from 'credentials.js'
import { openNav } from 'helpers/e2e/toggleNav.js'
import path from 'path'
import { fileURLToPath } from 'url'
import { v4 as uuid } from 'uuid'
@@ -15,6 +15,7 @@ import {
exactText,
getRoutes,
initPageConsoleErrorCatch,
login,
saveDocAndAssert,
} from '../helpers.js'
import { AdminUrlUtil } from '../helpers/adminUrlUtil.js'
@@ -28,59 +29,12 @@ const dirname = path.dirname(filename)
let payload: PayloadTestSDK<Config>
const { beforeAll, describe } = test
const { beforeAll, afterAll, describe } = test
const headers = {
'Content-Type': 'application/json',
}
const createFirstUser = async ({
page,
serverURL,
}: {
customAdminRoutes?: SanitizedConfig['admin']['routes']
customRoutes?: SanitizedConfig['routes']
page: Page
serverURL: string
}) => {
const {
admin: {
routes: { createFirstUser: createFirstUserRoute },
},
routes: { admin: adminRoute },
} = getRoutes({})
// wait for create first user route
await page.goto(serverURL + `${adminRoute}${createFirstUserRoute}`)
// forget to fill out confirm password
await page.locator('#field-email').fill(devUser.email)
await page.locator('#field-password').fill(devUser.password)
await page.locator('.form-submit > button').click()
await expect(page.locator('.field-type.confirm-password .field-error')).toHaveText(
'This field is required.',
)
// make them match, but does not pass password validation
await page.locator('#field-email').fill(devUser.email)
await page.locator('#field-password').fill('12')
await page.locator('#field-confirm-password').fill('12')
await page.locator('.form-submit > button').click()
await expect(page.locator('.field-type.password .field-error')).toHaveText(
'This value must be longer than the minimum length of 3 characters.',
)
await page.locator('#field-email').fill(devUser.email)
await page.locator('#field-password').fill(devUser.password)
await page.locator('#field-confirm-password').fill(devUser.password)
await page.locator('#field-custom').fill('Hello, world!')
await page.locator('.form-submit > button').click()
await expect
.poll(() => page.url(), { timeout: POLL_TOPASS_TIMEOUT })
.not.toContain('create-first-user')
}
describe('Auth', () => {
let page: Page
let context: BrowserContext
@@ -97,35 +51,92 @@ describe('Auth', () => {
context = await browser.newContext()
page = await context.newPage()
initPageConsoleErrorCatch(page)
await ensureCompilationIsDone({ page, serverURL, noAutoLogin: true })
// Undo onInit seeding, as we need to test this without having a user created, or testing create-first-user
})
describe('create first user', () => {
beforeAll(async () => {
await reInitializeDB({
serverURL,
snapshotKey: 'auth',
snapshotKey: 'create-first-user',
deleteOnly: true,
})
await payload.create({
collection: apiKeysSlug,
data: {
apiKey: uuid(),
enableAPIKey: true,
await ensureCompilationIsDone({ page, serverURL, noAutoLogin: true })
await payload.delete({
collection: slug,
where: {
email: {
exists: true,
},
},
})
await payload.create({
collection: apiKeysSlug,
data: {
apiKey: uuid(),
enableAPIKey: true,
},
})
await createFirstUser({ page, serverURL })
async function waitForVisibleAuthFields() {
await expect(page.locator('#field-email')).toBeVisible()
await expect(page.locator('#field-password')).toBeVisible()
await expect(page.locator('#field-confirm-password')).toBeVisible()
}
await ensureCompilationIsDone({ page, serverURL })
test('should create first user and redirect to admin', async () => {
const {
admin: {
routes: { createFirstUser: createFirstUserRoute },
},
routes: { admin: adminRoute },
} = getRoutes({})
// wait for create first user route
await page.goto(serverURL + `${adminRoute}${createFirstUserRoute}`)
await expect(page.locator('.create-first-user')).toBeVisible()
await waitForVisibleAuthFields()
// forget to fill out confirm password
await page.locator('#field-email').fill(devUser.email)
await page.locator('#field-password').fill(devUser.password)
await page.locator('.form-submit > button').click()
await expect(page.locator('.field-type.confirm-password .field-error')).toHaveText(
'This field is required.',
)
// make them match, but does not pass password validation
await page.locator('#field-email').fill(devUser.email)
await page.locator('#field-password').fill('12')
await page.locator('#field-confirm-password').fill('12')
await page.locator('.form-submit > button').click()
await expect(page.locator('.field-type.password .field-error')).toHaveText(
'This value must be longer than the minimum length of 3 characters.',
)
// should fill out all fields correctly
await page.locator('#field-email').fill(devUser.email)
await page.locator('#field-password').fill(devUser.password)
await page.locator('#field-confirm-password').fill(devUser.password)
await page.locator('#field-custom').fill('Hello, world!')
await page.locator('.form-submit > button').click()
await expect
.poll(() => page.url(), { timeout: POLL_TOPASS_TIMEOUT })
.not.toContain('create-first-user')
})
})
describe('non create first user', () => {
beforeAll(async () => {
await reInitializeDB({
serverURL,
snapshotKey: 'auth',
deleteOnly: false,
})
await ensureCompilationIsDone({ page, serverURL, noAutoLogin: true })
await login({ page, serverURL })
})
describe('passwords', () => {
@@ -133,6 +144,15 @@ describe('Auth', () => {
url = new AdminUrlUtil(serverURL, slug)
})
afterAll(async () => {
// reset password to original password
await page.goto(url.account)
await page.locator('#change-password').click()
await page.locator('#field-password').fill(devUser.password)
await page.locator('#field-confirm-password').fill(devUser.password)
await saveDocAndAssert(page, '#action-save')
})
test('should allow change password', async () => {
await page.goto(url.account)
const emailBeforeSave = await page.locator('#field-email').inputValue()
@@ -196,6 +216,58 @@ describe('Auth', () => {
await expect(page.locator('#users-api-result')).toHaveText('Goodbye, world!')
await expect(page.locator('#use-auth-result')).toHaveText('Goodbye, world!')
})
// Need to test unlocking documents on logout here as this test suite does not auto login users
test('should unlock document on logout after editing without saving', async () => {
await page.goto(url.list)
await page.locator('.table .row-1 .cell-custom a').click()
const textInput = page.locator('#field-namedSaveToJWT')
await expect(textInput).toBeVisible()
const docID = (await page.locator('.render-title').getAttribute('data-doc-id')) as string
const lockDocRequest = page.waitForResponse(
(response) =>
response.request().method() === 'POST' && response.request().url() === url.edit(docID),
)
await textInput.fill('some text')
await lockDocRequest
const lockedDocs = await payload.find({
collection: 'payload-locked-documents',
limit: 1,
pagination: false,
})
await expect.poll(() => lockedDocs.docs.length).toBe(1)
await openNav(page)
await page.locator('.nav .nav__controls a[href="/admin/logout"]').click()
// Locate the modal container
const modalContainer = page.locator('.payload__modal-container')
await expect(modalContainer).toBeVisible()
// Click the "Leave anyway" button
await page
.locator('#leave-without-saving .confirmation-modal__controls .btn--style-primary')
.click()
await expect(page.locator('.login')).toBeVisible()
const unlockedDocs = await payload.find({
collection: 'payload-locked-documents',
limit: 1,
pagination: false,
})
await expect.poll(() => unlockedDocs.docs.length).toBe(0)
// added so tests after this do not need to re-login
await login({ page, serverURL })
})
})
describe('api-keys', () => {
@@ -264,3 +336,4 @@ describe('Auth', () => {
})
})
})
})

View File

@@ -248,11 +248,13 @@ export interface User {
hash?: string | null;
loginAttempts?: number | null;
lockUntil?: string | null;
sessions: {
sessions?:
| {
id: string;
createdAt?: string | null;
expiresAt: string;
}[];
}[]
| null;
password?: string | null;
}
/**
@@ -270,11 +272,13 @@ export interface PartialDisableLocalStrategy {
hash?: string | null;
loginAttempts?: number | null;
lockUntil?: string | null;
sessions: {
sessions?:
| {
id: string;
createdAt?: string | null;
expiresAt: string;
}[];
}[]
| null;
password?: string | null;
}
/**
@@ -316,11 +320,13 @@ export interface PublicUser {
_verificationToken?: string | null;
loginAttempts?: number | null;
lockUntil?: string | null;
sessions: {
sessions?:
| {
id: string;
createdAt?: string | null;
expiresAt: string;
}[];
}[]
| null;
password?: string | null;
}
/**

34
test/auth/seed.ts Normal file
View File

@@ -0,0 +1,34 @@
import type { Config } from 'payload'
import { v4 as uuid } from 'uuid'
import { devUser } from '../credentials.js'
import { apiKeysSlug } from './shared.js'
export const seed: Config['onInit'] = async (payload) => {
await payload.create({
collection: 'users',
data: {
custom: 'Hello, world!',
email: devUser.email,
password: devUser.password,
roles: ['admin'],
},
})
await payload.create({
collection: apiKeysSlug,
data: {
apiKey: uuid(),
enableAPIKey: true,
},
})
await payload.create({
collection: apiKeysSlug,
data: {
apiKey: uuid(),
enableAPIKey: true,
},
})
}

View File

@@ -98,10 +98,26 @@ export async function ensureCompilationIsDone({
await page.goto(adminURL)
await page.waitForURL(
readyURL ??
(noAutoLogin ? `${adminURL + (adminURL.endsWith('/') ? '' : '/')}login` : adminURL),
if (readyURL) {
await page.waitForURL(readyURL)
} else {
await expect
.poll(
() => {
if (noAutoLogin) {
const baseAdminURL = adminURL + (adminURL.endsWith('/') ? '' : '/')
return (
page.url() === `${baseAdminURL}create-first-user` ||
page.url() === `${baseAdminURL}login`
)
} else {
return page.url() === adminURL
}
},
{ timeout: POLL_TOPASS_TIMEOUT },
)
.toBe(true)
}
console.log('Successfully compiled')
return

View File

@@ -15,7 +15,7 @@ const handler: PayloadHandler = async (req) => {
}
const query: {
deleteOnly?: boolean
deleteOnly?: string
snapshotKey?: string
uploadsDir?: string | string[]
} = qs.parse(req.url.split('?')[1] ?? '', {
@@ -31,7 +31,8 @@ const handler: PayloadHandler = async (req) => {
snapshotKey: String(query.snapshotKey),
// uploadsDir can be string or stringlist
uploadsDir: query.uploadsDir as string | string[],
deleteOnly: query.deleteOnly,
// query value will be a string of 'true' or 'false'
deleteOnly: query.deleteOnly === 'true',
})
return Response.json(

View File

@@ -174,6 +174,13 @@ export interface User {
hash?: string | null;
loginAttempts?: number | null;
lockUntil?: string | null;
sessions?:
| {
id: string;
createdAt?: string | null;
expiresAt: string;
}[]
| null;
password?: string | null;
}
/**
@@ -288,6 +295,13 @@ export interface UsersSelect<T extends boolean = true> {
hash?: T;
loginAttempts?: T;
lockUntil?: T;
sessions?:
| T
| {
id?: T;
createdAt?: T;
expiresAt?: T;
};
}
/**
* This interface was referenced by `Config`'s JSON-Schema