From 0ffdcc685f4e917a02e62dbaccec7cc8ebbf695d Mon Sep 17 00:00:00 2001 From: Dan Ribbens Date: Thu, 25 Apr 2024 09:39:09 -0400 Subject: [PATCH] fix: disable api key checkbox does not remove api key (#6017) --- .../views/collections/Edit/Auth/APIKey.tsx | 21 +++-- .../views/collections/Edit/Auth/index.tsx | 2 +- .../payload/src/auth/baseFields/apiKey.ts | 12 ++- test/auth/e2e.spec.ts | 4 +- test/auth/int.spec.ts | 94 ++++++++++++++++++- 5 files changed, 116 insertions(+), 17 deletions(-) diff --git a/packages/payload/src/admin/components/views/collections/Edit/Auth/APIKey.tsx b/packages/payload/src/admin/components/views/collections/Edit/Auth/APIKey.tsx index b3ef2ab393..352114507c 100644 --- a/packages/payload/src/admin/components/views/collections/Edit/Auth/APIKey.tsx +++ b/packages/payload/src/admin/components/views/collections/Edit/Auth/APIKey.tsx @@ -7,14 +7,14 @@ import CopyToClipboard from '../../../../elements/CopyToClipboard' import GenerateConfirmation from '../../../../elements/GenerateConfirmation' import { useFormFields } from '../../../../forms/Form/context' import Label from '../../../../forms/Label' -import useField from '../../../../forms/useField' import { fieldBaseClass } from '../../../../forms/field-types/shared' +import useField from '../../../../forms/useField' const path = 'apiKey' const baseClass = 'api-key' -const APIKey: React.FC<{ readOnly?: boolean }> = ({ readOnly }) => { - const [initialAPIKey, setInitialAPIKey] = useState(null) +const APIKey: React.FC<{ enabled: boolean; readOnly?: boolean }> = ({ enabled, readOnly }) => { + const [initialAPIKey] = useState(uuidv4()) const [highlightedField, setHighlightedField] = useState(false) const { t } = useTranslation() @@ -51,14 +51,13 @@ const APIKey: React.FC<{ readOnly?: boolean }> = ({ readOnly }) => { const { setValue, value } = fieldType useEffect(() => { - setInitialAPIKey(uuidv4()) - }, []) - - useEffect(() => { - if (!apiKeyValue) { + if (!apiKeyValue && enabled) { setValue(initialAPIKey) } - }, [apiKeyValue, setValue, initialAPIKey]) + if (!enabled) { + setValue(null) + } + }, [apiKeyValue, enabled, setValue, initialAPIKey]) useEffect(() => { if (highlightedField) { @@ -68,6 +67,10 @@ const APIKey: React.FC<{ readOnly?: boolean }> = ({ readOnly }) => { } }, [highlightedField]) + if (!enabled) { + return null + } + return (
diff --git a/packages/payload/src/admin/components/views/collections/Edit/Auth/index.tsx b/packages/payload/src/admin/components/views/collections/Edit/Auth/index.tsx index 8b9e96267a..2388072945 100644 --- a/packages/payload/src/admin/components/views/collections/Edit/Auth/index.tsx +++ b/packages/payload/src/admin/components/views/collections/Edit/Auth/index.tsx @@ -145,7 +145,7 @@ const Auth: React.FC = (props) => { {useAPIKey && (
- {enableAPIKey?.value && } +
)} {verify && } diff --git a/packages/payload/src/auth/baseFields/apiKey.ts b/packages/payload/src/auth/baseFields/apiKey.ts index 7e525e9ff8..bc4faec068 100644 --- a/packages/payload/src/auth/baseFields/apiKey.ts +++ b/packages/payload/src/auth/baseFields/apiKey.ts @@ -20,7 +20,6 @@ export default [ Field: () => null, }, }, - defaultValue: false, label: labels['authentication:enableAPIKey'], }, { @@ -46,16 +45,19 @@ export default [ hidden: true, hooks: { beforeValidate: [ - async ({ data, req, value }) => { + ({ data, req, value }) => { + if (data.apiKey === false || data.apiKey === null) { + return null + } + if (data.enableAPIKey === false || data.enableAPIKey === null) { + return null + } if (data.apiKey) { return crypto .createHmac('sha1', req.payload.secret) .update(data.apiKey as string) .digest('hex') } - if (data.enableAPIKey === false) { - return null - } return value }, ], diff --git a/test/auth/e2e.spec.ts b/test/auth/e2e.spec.ts index 3395a038fd..e848d6814b 100644 --- a/test/auth/e2e.spec.ts +++ b/test/auth/e2e.spec.ts @@ -1,6 +1,7 @@ import type { Page } from '@playwright/test' import { expect, test } from '@playwright/test' +import { v4 as uuid } from 'uuid' import payload from '../../packages/payload/src' import { initPageConsoleErrorCatch, login, saveDocAndAssert } from '../helpers' @@ -82,6 +83,7 @@ describe('auth', () => { user = await payload.create({ collection: apiKeysSlug, data: { + apiKey: uuid(), enableAPIKey: true, }, }) @@ -117,7 +119,7 @@ describe('auth', () => { const response = await fetch(`${apiURL}/${apiKeysSlug}/me`, { headers: { ...headers, - Authorization: `${slug} API-Key ${user.apiKey}`, + Authorization: `${apiKeysSlug} API-Key ${user.apiKey}`, }, }).then((res) => res.json()) diff --git a/test/auth/int.spec.ts b/test/auth/int.spec.ts index 696f059b0a..134a243107 100644 --- a/test/auth/int.spec.ts +++ b/test/auth/int.spec.ts @@ -1,5 +1,6 @@ import { GraphQLClient } from 'graphql-request' import jwtDecode from 'jwt-decode' +import { v4 as uuid } from 'uuid' import type { User } from '../../packages/payload/src/auth' @@ -7,7 +8,7 @@ import payload from '../../packages/payload/src' import configPromise from '../collections-graphql/config' import { devUser } from '../credentials' import { initPayloadTest } from '../helpers/configHelpers' -import { namedSaveToJWTValue, saveToJWTKey, slug } from './shared' +import { apiKeysSlug, namedSaveToJWTValue, saveToJWTKey, slug } from './shared' require('isomorphic-fetch') @@ -680,5 +681,96 @@ describe('Auth', () => { expect(fail.status).toStrictEqual(404) }) + + it('should not remove an API key from a user when updating other fields', async () => { + const apiKey = uuid() + const user = await payload.create({ + collection: 'api-keys', + data: { + apiKey, + enableAPIKey: true, + }, + }) + + const updatedUser = await payload.update({ + id: user.id, + collection: 'api-keys', + data: { + enableAPIKey: true, + }, + }) + + const userResult = await payload.find({ + collection: 'api-keys', + where: { + id: { + equals: user.id, + }, + }, + }) + + expect(updatedUser.apiKey).toStrictEqual(user.apiKey) + expect(userResult.docs[0].apiKey).toStrictEqual(user.apiKey) + }) + + it('should disable api key after updating apiKey: null', async () => { + const apiKey = uuid() + const user = await payload.create({ + collection: apiKeysSlug, + data: { + apiKey, + enableAPIKey: true, + }, + }) + + const updatedUser = await payload.update({ + id: user.id, + collection: apiKeysSlug, + data: { + apiKey: null, + }, + }) + + // use the api key in a fetch to assert that it is disabled + const response = await fetch(`${apiUrl}/${apiKeysSlug}/me`, { + headers: { + ...headers, + Authorization: `${apiKeysSlug} API-Key ${apiKey}`, + }, + }).then((res) => res.json()) + + expect(updatedUser.apiKey).toBeNull() + expect(response.user).toBeNull() + }) + + it('should disable api key after updating with enableAPIKey:false', async () => { + const apiKey = uuid() + const user = await payload.create({ + collection: apiKeysSlug, + data: { + apiKey, + enableAPIKey: true, + }, + }) + + const updatedUser = await payload.update({ + id: user.id, + collection: apiKeysSlug, + data: { + enableAPIKey: false, + }, + }) + + // use the api key in a fetch to assert that it is disabled + const response = await fetch(`${apiUrl}/${apiKeysSlug}/me`, { + headers: { + ...headers, + Authorization: `${apiKeysSlug} API-Key ${apiKey}`, + }, + }).then((res) => res.json()) + + expect(updatedUser.apiKey).toStrictEqual(apiKey) + expect(response.user).toBeNull() + }) }) })