Skip to content

Commit 55ce8e6

Browse files
authored
fix: locked documents with read access for users (#8950)
### What? When read access is restricted on the `users` collection - restricted users would not have access to other users complete user data object only their IDs when accessing `user.value`. ### Why? This is problematic when determining the lock status of a document from a restricted users perspective as `user.id` would not exist - the user data would not be an object in this case but instead a `string` or `number` value for user ID ### How? This PR properly handles both cases now and checks if the incoming user data is an object or just a `string` / `number`.
1 parent b417c1f commit 55ce8e6

File tree

56 files changed

+189
-64
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

56 files changed

+189
-64
lines changed

packages/next/src/elements/DocumentLocked/index.tsx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import type { ClientUser } from 'payload'
33

44
import { Button, Modal, useModal, useTranslation } from '@payloadcms/ui'
5+
import { isClientUserObject } from '@payloadcms/ui/shared'
56
import React, { useEffect } from 'react'
67

78
import './index.scss'
@@ -30,7 +31,7 @@ export const DocumentLocked: React.FC<{
3031
onReadOnly: () => void
3132
onTakeOver: () => void
3233
updatedAt?: null | number
33-
user?: ClientUser
34+
user?: ClientUser | number | string
3435
}> = ({ handleGoBack, isActive, onReadOnly, onTakeOver, updatedAt, user }) => {
3536
const { closeModal, openModal } = useModal()
3637
const { t } = useTranslation()
@@ -49,7 +50,10 @@ export const DocumentLocked: React.FC<{
4950
<div className={`${baseClass}__content`}>
5051
<h1>{t('general:documentLocked')}</h1>
5152
<p>
52-
<strong>{user?.email ?? user?.id}</strong> {t('general:currentlyEditing')}
53+
<strong>
54+
{isClientUserObject(user) ? (user.email ?? user.id) : `${t('general:user')}: ${user}`}
55+
</strong>{' '}
56+
{t('general:currentlyEditing')}
5357
</p>
5458
<p>
5559
{t('general:editedSince')} <strong>{formatDate(updatedAt)}</strong>

packages/next/src/views/Dashboard/Default/index.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ const baseClass = 'dashboard'
1717

1818
export type DashboardProps = {
1919
globalData: Array<{
20-
data: { _isLocked: boolean; _lastEditedAt: string; _userEditing: ClientUser | null }
20+
data: { _isLocked: boolean; _lastEditedAt: string; _userEditing: ClientUser | number | string }
2121
lockDuration?: number
2222
slug: string
2323
}>

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

Lines changed: 33 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,10 @@ export const Dashboard: React.FC<AdminViewProps> = async ({
3131
payload,
3232
user,
3333
},
34+
req,
3435
visibleEntities,
3536
} = initPageResult
3637

37-
const lockDurationDefault = 300 // Default 5 minutes in seconds
38-
3938
const CustomDashboardComponent = config.admin.components?.views?.Dashboard
4039

4140
const collections = config.collections.filter(
@@ -50,39 +49,44 @@ export const Dashboard: React.FC<AdminViewProps> = async ({
5049
visibleEntities.globals.includes(global.slug),
5150
)
5251

53-
const globalConfigs = config.globals.map((global) => ({
54-
slug: global.slug,
55-
lockDuration:
56-
global.lockDocuments === false
57-
? null // Set lockDuration to null if locking is disabled
58-
: typeof global.lockDocuments === 'object'
52+
// Query locked global documents only if there are globals in the config
53+
let globalData = []
54+
55+
if (config.globals.length > 0) {
56+
const lockedDocuments = await payload.find({
57+
collection: 'payload-locked-documents',
58+
depth: 1,
59+
overrideAccess: false,
60+
pagination: false,
61+
req,
62+
where: {
63+
globalSlug: {
64+
exists: true,
65+
},
66+
},
67+
})
68+
69+
// Map over globals to include `lockDuration` and lock data for each global slug
70+
globalData = config.globals.map((global) => {
71+
const lockDurationDefault = 300
72+
const lockDuration =
73+
typeof global.lockDocuments === 'object'
5974
? global.lockDocuments.duration
60-
: lockDurationDefault,
61-
}))
62-
63-
// Filter the slugs based on permissions and visibility
64-
const filteredGlobalConfigs = globalConfigs.filter(
65-
({ slug, lockDuration }) =>
66-
lockDuration !== null && // Ensure lockDuration is valid
67-
permissions?.globals?.[slug]?.read?.permission &&
68-
visibleEntities.globals.includes(slug),
69-
)
75+
: lockDurationDefault
7076

71-
const globalData = await Promise.all(
72-
filteredGlobalConfigs.map(async ({ slug, lockDuration }) => {
73-
const data = await payload.findGlobal({
74-
slug,
75-
depth: 0,
76-
includeLockStatus: true,
77-
})
77+
const lockedDoc = lockedDocuments.docs.find((doc) => doc.globalSlug === global.slug)
7878

7979
return {
80-
slug,
81-
data,
80+
slug: global.slug,
81+
data: {
82+
_isLocked: !!lockedDoc,
83+
_lastEditedAt: lockedDoc?.updatedAt ?? null,
84+
_userEditing: lockedDoc?.user?.value ?? null,
85+
},
8286
lockDuration,
8387
}
84-
}),
85-
)
88+
})
89+
}
8690

8791
const navGroups = groupNavItems(
8892
[

packages/next/src/views/Edit/Default/index.tsx

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ export const DefaultEditView: React.FC = () => {
144144
const documentLockStateRef = useRef<{
145145
hasShownLockedModal: boolean
146146
isLocked: boolean
147-
user: ClientUser
147+
user: ClientUser | number | string
148148
} | null>({
149149
hasShownLockedModal: false,
150150
isLocked: false,
@@ -268,7 +268,10 @@ export const DefaultEditView: React.FC = () => {
268268
setDocumentIsLocked(true)
269269

270270
if (isLockingEnabled) {
271-
const previousOwnerId = documentLockStateRef.current?.user?.id
271+
const previousOwnerId =
272+
typeof documentLockStateRef.current?.user === 'object'
273+
? documentLockStateRef.current?.user?.id
274+
: documentLockStateRef.current?.user
272275

273276
if (lockedState) {
274277
if (!documentLockStateRef.current || lockedState.user.id !== previousOwnerId) {
@@ -328,7 +331,11 @@ export const DefaultEditView: React.FC = () => {
328331
// Unlock the document only if we're actually navigating away from the document
329332
if (documentId && documentIsLocked && !isStayingWithinDocument) {
330333
// Check if this user is still the current editor
331-
if (documentLockStateRef.current?.user?.id === user?.id) {
334+
if (
335+
typeof documentLockStateRef.current?.user === 'object'
336+
? documentLockStateRef.current?.user?.id === user?.id
337+
: documentLockStateRef.current?.user === user?.id
338+
) {
332339
void unlockDocument(id, collectionSlug ?? globalSlug)
333340
setDocumentIsLocked(false)
334341
setCurrentEditor(null)
@@ -352,7 +359,9 @@ export const DefaultEditView: React.FC = () => {
352359
const shouldShowDocumentLockedModal =
353360
documentIsLocked &&
354361
currentEditor &&
355-
currentEditor.id !== user?.id &&
362+
(typeof currentEditor === 'object'
363+
? currentEditor.id !== user?.id
364+
: currentEditor !== user?.id) &&
356365
!isReadOnlyForIncomingUser &&
357366
!showTakeOverModal &&
358367
!documentLockStateRef.current?.hasShownLockedModal &&

packages/next/src/views/LivePreview/index.client.tsx

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ const PreviewView: React.FC<Props> = ({
129129
const documentLockStateRef = useRef<{
130130
hasShownLockedModal: boolean
131131
isLocked: boolean
132-
user: ClientUser
132+
user: ClientUser | number | string
133133
} | null>({
134134
hasShownLockedModal: false,
135135
isLocked: false,
@@ -208,7 +208,10 @@ const PreviewView: React.FC<Props> = ({
208208
setDocumentIsLocked(true)
209209

210210
if (isLockingEnabled) {
211-
const previousOwnerId = documentLockStateRef.current?.user?.id
211+
const previousOwnerId =
212+
typeof documentLockStateRef.current?.user === 'object'
213+
? documentLockStateRef.current?.user?.id
214+
: documentLockStateRef.current?.user
212215

213216
if (lockedState) {
214217
if (!documentLockStateRef.current || lockedState.user.id !== previousOwnerId) {
@@ -267,7 +270,11 @@ const PreviewView: React.FC<Props> = ({
267270
// Unlock the document only if we're actually navigating away from the document
268271
if (documentId && documentIsLocked && !isStayingWithinDocument) {
269272
// Check if this user is still the current editor
270-
if (documentLockStateRef.current?.user?.id === user?.id) {
273+
if (
274+
typeof documentLockStateRef.current?.user === 'object'
275+
? documentLockStateRef.current?.user?.id === user?.id
276+
: documentLockStateRef.current?.user === user?.id
277+
) {
271278
void unlockDocument(id, collectionSlug ?? globalSlug)
272279
setDocumentIsLocked(false)
273280
setCurrentEditor(null)
@@ -291,7 +298,9 @@ const PreviewView: React.FC<Props> = ({
291298
const shouldShowDocumentLockedModal =
292299
documentIsLocked &&
293300
currentEditor &&
294-
currentEditor.id !== user.id &&
301+
(typeof currentEditor === 'object'
302+
? currentEditor.id !== user?.id
303+
: currentEditor !== user?.id) &&
295304
!isReadOnlyForIncomingUser &&
296305
!showTakeOverModal &&
297306
// eslint-disable-next-line react-compiler/react-compiler

packages/payload/src/collections/operations/find.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,7 @@ export const findOperation = async <
187187
collection: 'payload-locked-documents',
188188
depth: 1,
189189
limit: sanitizedLimit,
190+
overrideAccess: false,
190191
pagination: false,
191192
req,
192193
where: {

packages/payload/src/collections/operations/findByID.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ export const findByIDOperation = async <
139139
collection: 'payload-locked-documents',
140140
depth: 1,
141141
limit: 1,
142+
overrideAccess: false,
142143
pagination: false,
143144
req,
144145
where: {

packages/payload/src/globals/operations/findOne.ts

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,21 +65,37 @@ export const findOneOperation = async <T extends Record<string, unknown>>(
6565
// /////////////////////////////////////
6666
// Include Lock Status if required
6767
// /////////////////////////////////////
68-
6968
if (includeLockStatus && slug) {
7069
let lockStatus = null
7170

7271
try {
72+
const lockDocumentsProp = globalConfig?.lockDocuments
73+
74+
const lockDurationDefault = 300 // Default 5 minutes in seconds
75+
const lockDuration =
76+
typeof lockDocumentsProp === 'object' ? lockDocumentsProp.duration : lockDurationDefault
77+
const lockDurationInMilliseconds = lockDuration * 1000
78+
7379
const lockedDocument = await req.payload.find({
7480
collection: 'payload-locked-documents',
7581
depth: 1,
7682
limit: 1,
83+
overrideAccess: false,
7784
pagination: false,
7885
req,
7986
where: {
80-
globalSlug: {
81-
equals: slug,
82-
},
87+
and: [
88+
{
89+
globalSlug: {
90+
equals: slug,
91+
},
92+
},
93+
{
94+
updatedAt: {
95+
greater_than: new Date(new Date().getTime() - lockDurationInMilliseconds),
96+
},
97+
},
98+
],
8399
},
84100
})
85101

@@ -91,7 +107,6 @@ export const findOneOperation = async <T extends Record<string, unknown>>(
91107
}
92108

93109
doc._isLocked = !!lockStatus
94-
doc._lastEditedAt = lockStatus?.updatedAt ?? null
95110
doc._userEditing = lockStatus?.user?.value ?? null
96111
}
97112

packages/translations/src/clientKeys.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ export const clientTranslationKeys = createClientTranslationKeys([
127127
'general:addFilter',
128128
'general:adminTheme',
129129
'general:and',
130+
'general:anotherUser',
130131
'general:anotherUserTakenOver',
131132
'general:applyChanges',
132133
'general:ascending',

packages/translations/src/languages/ar.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,7 @@ export const arTranslations: DefaultTranslationsObject = {
177177
addFilter: 'أضف فلتر',
178178
adminTheme: 'شكل واجهة المستخدم',
179179
and: 'و',
180+
anotherUser: 'مستخدم آخر',
180181
anotherUserTakenOver: 'قام مستخدم آخر بالاستيلاء على تحرير هذا المستند.',
181182
applyChanges: 'طبق التغييرات',
182183
ascending: 'تصاعدي',

0 commit comments

Comments
 (0)