diff --git a/packages/cli/src/controllers/__tests__/api-keys.controller.test.ts b/packages/cli/src/controllers/__tests__/api-keys.controller.test.ts index 81025fb2ca..3f34fc1d2c 100644 --- a/packages/cli/src/controllers/__tests__/api-keys.controller.test.ts +++ b/packages/cli/src/controllers/__tests__/api-keys.controller.test.ts @@ -1,66 +1,88 @@ import { mock } from 'jest-mock-extended'; -import { randomString } from 'n8n-workflow'; import { Container } from 'typedi'; import type { ApiKey } from '@/databases/entities/api-key'; import type { User } from '@/databases/entities/user'; -import { ApiKeyRepository } from '@/databases/repositories/api-key.repository'; +import { EventService } from '@/events/event.service'; import type { ApiKeysRequest, AuthenticatedRequest } from '@/requests'; -import { API_KEY_PREFIX } from '@/services/public-api-key.service'; +import { PublicApiKeyService } from '@/services/public-api-key.service'; import { mockInstance } from '@test/mocking'; import { ApiKeysController } from '../api-keys.controller'; describe('ApiKeysController', () => { - const apiKeysRepository = mockInstance(ApiKeyRepository); + const publicApiKeyService = mockInstance(PublicApiKeyService); + const eventService = mockInstance(EventService); const controller = Container.get(ApiKeysController); let req: AuthenticatedRequest; beforeAll(() => { - req = mock({ user: mock({ id: '123' }) }); + req = { user: { id: '123' } } as AuthenticatedRequest; }); describe('createAPIKey', () => { it('should create and save an API key', async () => { + // Arrange + const apiKeyData = { id: '123', userId: '123', label: 'My API Key', - apiKey: `${API_KEY_PREFIX}${randomString(42)}`, + apiKey: 'apiKey********', createdAt: new Date(), } as ApiKey; - apiKeysRepository.upsert.mockImplementation(); + const req = mock({ user: mock({ id: '123' }) }); - apiKeysRepository.findOneByOrFail.mockResolvedValue(apiKeyData); + publicApiKeyService.createPublicApiKeyForUser.mockResolvedValue(apiKeyData); + + // Act const newApiKey = await controller.createAPIKey(req); - expect(apiKeysRepository.upsert).toHaveBeenCalled(); + // Assert + + expect(publicApiKeyService.createPublicApiKeyForUser).toHaveBeenCalled(); expect(apiKeyData).toEqual(newApiKey); + expect(eventService.emit).toHaveBeenCalledWith( + 'public-api-key-created', + expect.objectContaining({ user: req.user, publicApi: false }), + ); }); }); describe('getAPIKeys', () => { it('should return the users api keys redacted', async () => { + // Arrange + const apiKeyData = { id: '123', userId: '123', label: 'My API Key', - apiKey: `${API_KEY_PREFIX}${randomString(42)}`, + apiKey: 'apiKey***', createdAt: new Date(), + updatedAt: new Date(), } as ApiKey; - apiKeysRepository.findBy.mockResolvedValue([apiKeyData]); + publicApiKeyService.getRedactedApiKeysForUser.mockResolvedValue([apiKeyData]); + + // Act const apiKeys = await controller.getAPIKeys(req); - expect(apiKeys[0].apiKey).not.toEqual(apiKeyData.apiKey); - expect(apiKeysRepository.findBy).toHaveBeenCalledWith({ userId: req.user.id }); + + // Assert + + expect(apiKeys).toEqual([apiKeyData]); + expect(publicApiKeyService.getRedactedApiKeysForUser).toHaveBeenCalledWith( + expect.objectContaining({ id: req.user.id }), + ); }); }); describe('deleteAPIKey', () => { it('should delete the API key', async () => { + // Arrange + const user = mock({ id: '123', password: 'password', @@ -68,12 +90,22 @@ describe('ApiKeysController', () => { role: 'global:member', mfaEnabled: false, }); + const req = mock({ user, params: { id: user.id } }); + + // Act + await controller.deleteAPIKey(req); - expect(apiKeysRepository.delete).toHaveBeenCalledWith({ - userId: req.user.id, - id: req.params.id, - }); + + publicApiKeyService.deleteApiKeyForUser.mockResolvedValue(); + + // Assert + + expect(publicApiKeyService.deleteApiKeyForUser).toHaveBeenCalledWith(user, user.id); + expect(eventService.emit).toHaveBeenCalledWith( + 'public-api-key-deleted', + expect.objectContaining({ user, publicApi: false }), + ); }); }); }); diff --git a/packages/cli/src/public-api/index.ts b/packages/cli/src/public-api/index.ts index 1264f57496..92b3602828 100644 --- a/packages/cli/src/public-api/index.ts +++ b/packages/cli/src/public-api/index.ts @@ -3,16 +3,13 @@ import type { Router } from 'express'; import express from 'express'; import type { HttpError } from 'express-openapi-validator/dist/framework/types'; import fs from 'fs/promises'; -import type { OpenAPIV3 } from 'openapi-types'; import path from 'path'; import type { JsonObject } from 'swagger-ui-express'; import { Container } from 'typedi'; import validator from 'validator'; import YAML from 'yamljs'; -import { EventService } from '@/events/event.service'; import { License } from '@/license'; -import type { AuthenticatedRequest } from '@/requests'; import { PublicApiKeyService } from '@/services/public-api-key.service'; import { UrlService } from '@/services/url.service'; @@ -85,28 +82,7 @@ async function createApiRouter( }, validateSecurity: { handlers: { - ApiKeyAuth: async ( - req: AuthenticatedRequest, - _scopes: unknown, - schema: OpenAPIV3.ApiKeySecurityScheme, - ): Promise => { - const providedApiKey = req.headers[schema.name.toLowerCase()] as string; - - const user = await Container.get(PublicApiKeyService).getUserForApiKey(providedApiKey); - - if (!user) return false; - - Container.get(EventService).emit('public-api-invoked', { - userId: user.id, - path: req.path, - method: req.method, - apiVersion: version, - }); - - req.user = user; - - return true; - }, + ApiKeyAuth: Container.get(PublicApiKeyService).getAuthMiddleware(version), }, }, }), 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 new file mode 100644 index 0000000000..7c60b62983 --- /dev/null +++ b/packages/cli/src/services/__tests__/public-api-key.service.test.ts @@ -0,0 +1,147 @@ +import { mock } from 'jest-mock-extended'; +import type { InstanceSettings } from 'n8n-core'; +import type { OpenAPIV3 } from 'openapi-types'; + +import { ApiKeyRepository } from '@/databases/repositories/api-key.repository'; +import { UserRepository } from '@/databases/repositories/user.repository'; +import { getConnection } from '@/db'; +import type { EventService } from '@/events/event.service'; +import type { AuthenticatedRequest } from '@/requests'; +import { createOwnerWithApiKey } from '@test-integration/db/users'; +import * as testDb from '@test-integration/test-db'; + +import { JwtService } from '../jwt.service'; +import { PublicApiKeyService } from '../public-api-key.service'; + +const mockReqWith = (apiKey: string, path: string, method: string) => { + return mock({ + path, + method, + headers: { + 'x-n8n-api-key': apiKey, + }, + }); +}; + +const instanceSettings = mock({ encryptionKey: 'test-key' }); + +const eventService = mock(); + +const securitySchema = mock({ + name: 'X-N8N-API-KEY', +}); + +const jwtService = new JwtService(instanceSettings); + +let userRepository: UserRepository; +let apiKeyRepository: ApiKeyRepository; + +describe('PublicApiKeyService', () => { + beforeEach(async () => { + await testDb.truncate(['User']); + jest.clearAllMocks(); + }); + + beforeAll(async () => { + await testDb.init(); + userRepository = new UserRepository(getConnection()); + apiKeyRepository = new ApiKeyRepository(getConnection()); + }); + + afterAll(async () => { + await testDb.terminate(); + }); + + describe('getAuthMiddleware', () => { + it('should return false if api key is invalid', async () => { + //Arrange + + const apiKey = 'invalid'; + const path = '/test'; + const method = 'GET'; + const apiVersion = 'v1'; + + const publicApiKeyService = new PublicApiKeyService( + apiKeyRepository, + userRepository, + jwtService, + eventService, + ); + + const middleware = publicApiKeyService.getAuthMiddleware(apiVersion); + + //Act + + const response = await middleware(mockReqWith(apiKey, path, method), {}, securitySchema); + + //Assert + + expect(response).toBe(false); + }); + + it('should return false if valid api key is not in database', async () => { + //Arrange + + const apiKey = jwtService.sign({ sub: '123' }); + const path = '/test'; + const method = 'GET'; + const apiVersion = 'v1'; + + const publicApiKeyService = new PublicApiKeyService( + apiKeyRepository, + userRepository, + jwtService, + eventService, + ); + + const middleware = publicApiKeyService.getAuthMiddleware(apiVersion); + + //Act + + const response = await middleware(mockReqWith(apiKey, path, method), {}, securitySchema); + + //Assert + + expect(response).toBe(false); + }); + + it('should return true if valid api key exist in the database', async () => { + //Arrange + + const path = '/test'; + const method = 'GET'; + const apiVersion = 'v1'; + + const publicApiKeyService = new PublicApiKeyService( + apiKeyRepository, + userRepository, + jwtService, + eventService, + ); + + const owner = await createOwnerWithApiKey(); + + const [{ apiKey }] = owner.apiKeys; + + const middleware = publicApiKeyService.getAuthMiddleware(apiVersion); + + //Act + + const response = await middleware(mockReqWith(apiKey, 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', + }), + ); + }); + }); +}); diff --git a/packages/cli/src/services/public-api-key.service.ts b/packages/cli/src/services/public-api-key.service.ts index e689f3c019..bca3cd0d62 100644 --- a/packages/cli/src/services/public-api-key.service.ts +++ b/packages/cli/src/services/public-api-key.service.ts @@ -1,16 +1,28 @@ -import { randomBytes } from 'node:crypto'; -import Container, { Service } from 'typedi'; +import type { OpenAPIV3 } from 'openapi-types'; +import { Service } from 'typedi'; import { ApiKey } from '@/databases/entities/api-key'; import type { User } from '@/databases/entities/user'; import { ApiKeyRepository } from '@/databases/repositories/api-key.repository'; import { UserRepository } from '@/databases/repositories/user.repository'; +import { EventService } from '@/events/event.service'; +import type { AuthenticatedRequest } from '@/requests'; -export const API_KEY_PREFIX = 'n8n_api_'; +import { JwtService } from './jwt.service'; + +const API_KEY_AUDIENCE = 'public-api'; +const API_KEY_ISSUER = 'n8n'; +const REDACT_API_KEY_REVEAL_COUNT = 15; +const REDACT_API_KEY_MAX_LENGTH = 80; @Service() export class PublicApiKeyService { - constructor(private readonly apiKeyRepository: ApiKeyRepository) {} + constructor( + private readonly apiKeyRepository: ApiKeyRepository, + private readonly userRepository: UserRepository, + private readonly jwtService: JwtService, + private readonly eventService: EventService, + ) {} /** * Creates a new public API key for the specified user. @@ -18,7 +30,7 @@ export class PublicApiKeyService { * @returns A promise that resolves to the newly created API key. */ async createPublicApiKeyForUser(user: User) { - const apiKey = this.createApiKeyString(); + const apiKey = this.generateApiKey(user); await this.apiKeyRepository.upsert( this.apiKeyRepository.create({ userId: user.id, @@ -48,8 +60,8 @@ export class PublicApiKeyService { await this.apiKeyRepository.delete({ userId: user.id, id: apiKeyId }); } - async getUserForApiKey(apiKey: string) { - return await Container.get(UserRepository) + private async getUserForApiKey(apiKey: string) { + return await this.userRepository .createQueryBuilder('user') .innerJoin(ApiKey, 'apiKey', 'apiKey.userId = user.id') .where('apiKey.apiKey = :apiKey', { apiKey }) @@ -68,13 +80,39 @@ export class PublicApiKeyService { * ``` */ redactApiKey(apiKey: string) { - const keepLength = 5; - return ( - API_KEY_PREFIX + - apiKey.slice(API_KEY_PREFIX.length, API_KEY_PREFIX.length + keepLength) + - '*'.repeat(apiKey.length - API_KEY_PREFIX.length - keepLength) - ); + const visiblePart = apiKey.slice(0, REDACT_API_KEY_REVEAL_COUNT); + const redactedPart = '*'.repeat(apiKey.length - REDACT_API_KEY_REVEAL_COUNT); + + const completeRedactedApiKey = visiblePart + redactedPart; + + return completeRedactedApiKey.slice(0, REDACT_API_KEY_MAX_LENGTH); } - createApiKeyString = () => `${API_KEY_PREFIX}${randomBytes(40).toString('hex')}`; + getAuthMiddleware(version: string) { + return async ( + req: AuthenticatedRequest, + _scopes: unknown, + schema: OpenAPIV3.ApiKeySecurityScheme, + ): Promise => { + const providedApiKey = req.headers[schema.name.toLowerCase()] as string; + + const user = await this.getUserForApiKey(providedApiKey); + + if (!user) return false; + + this.eventService.emit('public-api-invoked', { + userId: user.id, + path: req.path, + method: req.method, + apiVersion: version, + }); + + req.user = user; + + return true; + }; + } + + private generateApiKey = (user: User) => + this.jwtService.sign({ sub: user.id, iss: API_KEY_ISSUER, aud: API_KEY_AUDIENCE }); } diff --git a/packages/cli/test/integration/shared/db/users.ts b/packages/cli/test/integration/shared/db/users.ts index 62f9f39a05..64c4d8ad85 100644 --- a/packages/cli/test/integration/shared/db/users.ts +++ b/packages/cli/test/integration/shared/db/users.ts @@ -1,17 +1,16 @@ import { hash } from 'bcryptjs'; -import { randomString } from 'n8n-workflow'; import Container from 'typedi'; import { AuthIdentity } from '@/databases/entities/auth-identity'; import { type GlobalRole, type User } from '@/databases/entities/user'; -import { ApiKeyRepository } from '@/databases/repositories/api-key.repository'; import { AuthIdentityRepository } from '@/databases/repositories/auth-identity.repository'; import { AuthUserRepository } from '@/databases/repositories/auth-user.repository'; import { UserRepository } from '@/databases/repositories/user.repository'; import { MfaService } from '@/mfa/mfa.service'; import { TOTPService } from '@/mfa/totp.service'; +import { PublicApiKeyService } from '@/services/public-api-key.service'; -import { randomApiKey, randomEmail, randomName, randomValidPassword } from '../random'; +import { randomEmail, randomName, randomValidPassword } from '../random'; // pre-computed bcrypt hash for the string 'password', using `await hash('password', 10)` const passwordHash = '$2a$10$njedH7S6V5898mj6p0Jr..IGY9Ms.qNwR7RbSzzX9yubJocKfvGGK'; @@ -81,17 +80,8 @@ export async function createUserWithMfaEnabled( }; } -const createApiKeyEntity = (user: User) => { - const apiKey = randomApiKey(); - return Container.get(ApiKeyRepository).create({ - userId: user.id, - label: randomString(10), - apiKey, - }); -}; - export const addApiKey = async (user: User) => { - return await Container.get(ApiKeyRepository).save(createApiKeyEntity(user)); + return await Container.get(PublicApiKeyService).createPublicApiKeyForUser(user); }; export async function createOwnerWithApiKey() {