fix: unlock access not being correctly denied when using a where query (#14585)

Fixes #14459

We were previously not throwing an error from the endpoint if validation
failed for unlock and the UI was using permissions from the wrong hook,
I've added a jsdoc to explain the difference between permissions form
useAuth and useDocumentInfo

Author: paulpopus

packages/payload/src/auth/executeAccess.ts

@@ -15,20 +15,20 @@ export const executeAccess = async (
   access: Access,
 ): Promise<AccessResult> => {
   if (access) {
-    const result = await access({
+    const resolvedConstraint = await access({
       id,
       data,
       isReadingStaticFile,
       req,
     })
 
-    if (!result) {
+    if (!resolvedConstraint) {
       if (!disableErrors) {
         throw new Forbidden(req.t)
       }
     }
 
-    return result
+    return resolvedConstraint
   }
 
   if (req.user) {

packages/payload/src/auth/operations/unlock.ts

@@ -8,7 +8,7 @@ import type { CollectionSlug } from '../../index.js'
 import type { PayloadRequest, Where } from '../../types/index.js'
 
 import { APIError } from '../../errors/index.js'
-import { Forbidden } from '../../index.js'
+import { combineQueries, Forbidden } from '../../index.js'
 import { appendNonTrashedFilter } from '../../utilities/appendNonTrashedFilter.js'
 import { commitTransaction } from '../../utilities/commitTransaction.js'
 import { initTransaction } from '../../utilities/initTransaction.js'
@@ -58,33 +58,36 @@ export const unlockOperation = async <TSlug extends CollectionSlug>(
 
   try {
     const shouldCommit = await initTransaction(req)
+    let whereConstraint: Where = {}
 
     // /////////////////////////////////////
     // Access
     // /////////////////////////////////////
 
     if (!overrideAccess) {
-      await executeAccess({ req }, collectionConfig.access.unlock)
+      const accessResult = await executeAccess({ req }, collectionConfig.access.unlock)
+
+      if (accessResult && typeof accessResult === 'object') {
+        whereConstraint = accessResult
+      }
     }
 
     // /////////////////////////////////////
     // Unlock
     // /////////////////////////////////////
 
-    let whereConstraint: Where = {}
-
     if (canLoginWithEmail && sanitizedEmail) {
-      whereConstraint = {
+      whereConstraint = combineQueries(whereConstraint, {
         email: {
           equals: sanitizedEmail,
         },
-      }
+      })
     } else if (canLoginWithUsername && sanitizedUsername) {
-      whereConstraint = {
+      whereConstraint = combineQueries(whereConstraint, {
         username: {
           equals: sanitizedUsername,
         },
-      }
+      })
     }
 
     // Exclude trashed users unless `trash: true`
@@ -113,13 +116,14 @@ export const unlockOperation = async <TSlug extends CollectionSlug>(
       result = true
     } else {
       result = null
+      throw new Forbidden(req.t)
     }
 
     if (shouldCommit) {
       await commitTransaction(req)
     }
 
-    return result!
+    return result
   } catch (error: unknown) {
     await killTransaction(req)
     throw error

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

@@ -25,6 +25,40 @@ export type UserWithToken<T = ClientUser> = {
 export type AuthContext<T = ClientUser> = {
   fetchFullUser: () => Promise<null | TypedUser>
   logOut: () => Promise<boolean>
+  /**
+   * These are the permissions for the current user from a global scope.
+   *
+   * When checking for permissions on document specific level, use the `useDocumentInfo` hook instead.
+   *
+   * @example
+   *
+   * ```tsx
+   * import { useAuth } from 'payload/ui'
+   *
+   * const MyComponent: React.FC = () => {
+   *   const { permissions } = useAuth()
+   *
+   *   if (permissions?.collections?.myCollection?.create) {
+   *     // user can create documents in 'myCollection'
+   *   }
+   *
+   *   return null
+   * }
+   * ```
+   *
+   * with useDocumentInfo:
+   *
+   * ```tsx
+   * import { useDocumentInfo } from 'payload/ui'
+   *
+   * const MyComponent: React.FC = () => {
+   *  const { docPermissions } = useDocumentInfo()
+   *  if (docPermissions?.create) {
+   *   // user can create this document
+   *  }
+   *  return null
+   * } ```
+   */
   permissions?: SanitizedPermissions
   refreshCookie: (forceRefresh?: boolean) => void
   refreshCookieAsync: () => Promise<ClientUser>

packages/ui/src/views/Edit/Auth/index.tsx

@@ -14,7 +14,6 @@ import { CheckboxField } from '../../../fields/Checkbox/index.js'
 import { ConfirmPasswordField } from '../../../fields/ConfirmPassword/index.js'
 import { PasswordField } from '../../../fields/Password/index.js'
 import { useFormFields, useFormModified } from '../../../forms/Form/context.js'
-import { useAuth } from '../../../providers/Auth/index.js'
 import { useConfig } from '../../../providers/Config/index.js'
 import { useDocumentInfo } from '../../../providers/DocumentInfo/index.js'
 import { useTranslation } from '../../../providers/Translation/index.js'
@@ -39,7 +38,6 @@ export const Auth: React.FC<Props> = (props) => {
     verify,
   } = props
 
-  const { permissions } = useAuth()
   const [changingPassword, setChangingPassword] = useState(requirePassword)
   const enableAPIKey = useFormFields(([fields]) => (fields && fields?.enableAPIKey) || null)
   const dispatchFields = useFormFields((reducer) => reducer[1])
@@ -131,14 +129,12 @@ export const Auth: React.FC<Props> = (props) => {
   const canReadApiKey = apiKeyPermissions === true || apiKeyPermissions?.read
 
   const hasPermissionToUnlock: boolean = useMemo(() => {
-    const collection = permissions?.collections?.[collectionSlug]
-
-    if (collection) {
-      return Boolean('unlock' in collection ? collection.unlock : undefined)
+    if (docPermissions) {
+      return Boolean('unlock' in docPermissions ? docPermissions.unlock : undefined)
     }
 
     return false
-  }, [permissions, collectionSlug])
+  }, [docPermissions])
 
   const handleChangePassword = useCallback(
     (changingPassword: boolean) => {

test/access-control/config.ts

@@ -2,7 +2,7 @@ import { fileURLToPath } from 'node:url'
 import path from 'path'
 const filename = fileURLToPath(import.meta.url)
 const dirname = path.dirname(filename)
-import type { FieldAccess } from 'payload'
+import type { FieldAccess, Where } from 'payload'
 
 import { buildEditorState, type DefaultNodeTypes } from '@payloadcms/richtext-lexical'
 
@@ -112,6 +112,18 @@ export default buildConfigWithDefaults(
               setTimeout(resolve, 50, true) // set to 'true' or 'false' here to simulate the response
             })
           },
+          unlock: ({ req }) => {
+            if (req.user && req.user.collection === 'users') {
+              // admin users can only unlock themselves
+              return {
+                id: {
+                  equals: req.user.id,
+                },
+              }
+            }
+
+            return false
+          },
         },
         auth: true,
         fields: [

test/access-control/e2e.spec.ts

@@ -61,6 +61,7 @@ let payload: PayloadTestSDK<Config>
 describe('Access Control', () => {
   let page: Page
   let url: AdminUrlUtil
+  let usersUrl: AdminUrlUtil
   let restrictedUrl: AdminUrlUtil
   let unrestrictedURL: AdminUrlUtil
   let readOnlyCollectionUrl: AdminUrlUtil
@@ -81,6 +82,7 @@ describe('Access Control', () => {
     ;({ payload, serverURL } = await initPayloadE2ENoConfig<Config>({ dirname }))
 
     url = new AdminUrlUtil(serverURL, slug)
+    usersUrl = new AdminUrlUtil(serverURL, 'users')
     restrictedUrl = new AdminUrlUtil(serverURL, fullyRestrictedSlug)
     richTextUrl = new AdminUrlUtil(serverURL, 'rich-text')
     unrestrictedURL = new AdminUrlUtil(serverURL, unrestrictedSlug)
@@ -614,6 +616,29 @@ describe('Access Control', () => {
       await openDocControls(page)
       await expect(page.locator('#action-delete')).toBeVisible()
     })
+
+    test('can only unlock self when admin', async () => {
+      await page.goto(usersUrl.list)
+
+      const adminUserRow = page.locator('.table tr').filter({ hasText: devUser.email })
+      const nonAdminUserRow = page.locator('.table tr').filter({ hasText: nonAdminEmail })
+
+      // Ensure admin user cannot unlock other users
+      await adminUserRow.locator('.cell-id a').click()
+      await page.waitForURL(`**/collections/users/**`)
+
+      const unlockButton = page.locator('#force-unlock')
+      await expect(unlockButton).toBeVisible()
+      await unlockButton.click()
+      await expect(page.locator('.payload-toast-container')).toContainText('Successfully unlocked')
+
+      await page.goto(usersUrl.list)
+
+      // Ensure non-admin user cannot see unlock button
+      await nonAdminUserRow.locator('.cell-id a').click()
+      await page.waitForURL(`**/collections/users/**`)
+      await expect(page.locator('#force-unlock')).toBeHidden()
+    })
   })
 
   describe('admin access', () => {