diff --git a/packages/cli/src/services/__tests__/public-api-key.service.test.ts b/packages/cli/src/services/__tests__/public-api-key.service.test.ts index 86db071f35..8aafb6f84b 100644 --- a/packages/cli/src/services/__tests__/public-api-key.service.test.ts +++ b/packages/cli/src/services/__tests__/public-api-key.service.test.ts @@ -1,5 +1,8 @@ +import { Container } from '@n8n/di'; import { mock } from 'jest-mock-extended'; +import { DateTime } from 'luxon'; import type { InstanceSettings } from 'n8n-core'; +import { randomString } from 'n8n-workflow'; import type { OpenAPIV3 } from 'openapi-types'; import { ApiKeyRepository } from '@/databases/repositories/api-key.repository'; @@ -143,6 +146,85 @@ describe('PublicApiKeyService', () => { }), ); }); + + it('should return false if expired JWT is used', async () => { + //Arrange + + const path = '/test'; + const method = 'GET'; + const apiVersion = 'v1'; + + const publicApiKeyService = new PublicApiKeyService( + apiKeyRepository, + userRepository, + jwtService, + eventService, + ); + + const dateInThePast = DateTime.now().minus({ days: 1 }).toUnixInteger(); + + const owner = await createOwnerWithApiKey({ + expiresAt: dateInThePast, + }); + + const [{ apiKey }] = owner.apiKeys; + + const middleware = publicApiKeyService.getAuthMiddleware(apiVersion); + + //Act + + const response = await middleware(mockReqWith(apiKey, path, method), {}, securitySchema); + + //Assert + + expect(response).toBe(false); + }); + + it('should work with non JWT (legacy) api keys', async () => { + //Arrange + + const path = '/test'; + const method = 'GET'; + const apiVersion = 'v1'; + const legacyApiKey = `n8n_api_${randomString(10)}`; + + const publicApiKeyService = new PublicApiKeyService( + apiKeyRepository, + userRepository, + jwtService, + eventService, + ); + + const owner = await createOwnerWithApiKey(); + + const [{ apiKey }] = owner.apiKeys; + + await Container.get(ApiKeyRepository).update({ apiKey }, { apiKey: legacyApiKey }); + + const middleware = publicApiKeyService.getAuthMiddleware(apiVersion); + + //Act + + const response = await middleware( + mockReqWith(legacyApiKey, path, method), + {}, + securitySchema, + ); + + //Assert + + expect(response).toBe(true); + expect(eventService.emit).toHaveBeenCalledTimes(1); + expect(eventService.emit).toHaveBeenCalledWith( + 'public-api-invoked', + expect.objectContaining({ + userId: owner.id, + path, + method, + apiVersion: 'v1', + }), + ); + }); }); describe('redactApiKey', () => { diff --git a/packages/cli/src/services/public-api-key.service.ts b/packages/cli/src/services/public-api-key.service.ts index 719f922fb2..c21129412b 100644 --- a/packages/cli/src/services/public-api-key.service.ts +++ b/packages/cli/src/services/public-api-key.service.ts @@ -17,6 +17,7 @@ const API_KEY_AUDIENCE = 'public-api'; const API_KEY_ISSUER = 'n8n'; const REDACT_API_KEY_REVEAL_COUNT = 4; const REDACT_API_KEY_MAX_LENGTH = 10; +const PREFIX_LEGACY_API_KEY = 'n8n_api_'; @Service() export class PublicApiKeyService { @@ -107,14 +108,17 @@ export class PublicApiKeyService { if (!user) return false; - try { - this.jwtService.verify(providedApiKey, { - issuer: API_KEY_ISSUER, - audience: API_KEY_AUDIENCE, - }); - } catch (e) { - if (e instanceof TokenExpiredError) return false; - throw e; + // Legacy API keys are not JWTs and do not need to be verified. + if (!providedApiKey.startsWith(PREFIX_LEGACY_API_KEY)) { + try { + this.jwtService.verify(providedApiKey, { + issuer: API_KEY_ISSUER, + audience: API_KEY_AUDIENCE, + }); + } catch (e) { + if (e instanceof TokenExpiredError) return false; + throw e; + } } this.eventService.emit('public-api-invoked', { diff --git a/packages/cli/test/integration/shared/db/users.ts b/packages/cli/test/integration/shared/db/users.ts index af0cf99820..be40ed7d23 100644 --- a/packages/cli/test/integration/shared/db/users.ts +++ b/packages/cli/test/integration/shared/db/users.ts @@ -80,23 +80,30 @@ export async function createUserWithMfaEnabled( }; } -export const addApiKey = async (user: User) => { +export const addApiKey = async ( + user: User, + { expiresAt = null }: { expiresAt?: number | null } = {}, +) => { return await Container.get(PublicApiKeyService).createPublicApiKeyForUser(user, { label: randomName(), - expiresAt: null, + expiresAt, }); }; -export async function createOwnerWithApiKey() { +export async function createOwnerWithApiKey({ + expiresAt = null, +}: { expiresAt?: number | null } = {}) { const owner = await createOwner(); - const apiKey = await addApiKey(owner); + const apiKey = await addApiKey(owner, { expiresAt }); owner.apiKeys = [apiKey]; return owner; } -export async function createMemberWithApiKey() { +export async function createMemberWithApiKey({ + expiresAt = null, +}: { expiresAt?: number | null } = {}) { const member = await createMember(); - const apiKey = await addApiKey(member); + const apiKey = await addApiKey(member, { expiresAt }); member.apiKeys = [apiKey]; return member; }