diff --git a/packages/@n8n/api-types/src/api-keys.ts b/packages/@n8n/api-types/src/api-keys.ts index e3dd185f83f..935b281db20 100644 --- a/packages/@n8n/api-types/src/api-keys.ts +++ b/packages/@n8n/api-types/src/api-keys.ts @@ -3,6 +3,13 @@ import type { ApiKeyScope } from '@n8n/permissions'; /** Unix timestamp. Seconds since epoch */ export type UnixTimestamp = number | null; +export type ApiKeyOwner = { + id: string; + firstName: string | null; + lastName: string | null; + email: string; +}; + export type ApiKey = { id: string; label: string; @@ -14,6 +21,8 @@ export type ApiKey = { scopes: ApiKeyScope[]; /** ISO timestamp of the last time the key authenticated a request, or null if never used. */ lastUsedAt: string | null; + /** The user who owns this key. Populated on list endpoints; absent on create. */ + owner?: ApiKeyOwner; }; export type ApiKeyWithRawValue = ApiKey & { rawApiKey: string }; diff --git a/packages/@n8n/permissions/src/__tests__/__snapshots__/scope-information.test.ts.snap b/packages/@n8n/permissions/src/__tests__/__snapshots__/scope-information.test.ts.snap index 2e2befd525f..531115fff18 100644 --- a/packages/@n8n/permissions/src/__tests__/__snapshots__/scope-information.test.ts.snap +++ b/packages/@n8n/permissions/src/__tests__/__snapshots__/scope-information.test.ts.snap @@ -188,6 +188,10 @@ exports[`Scope Information > ensure scopes are defined correctly 1`] = ` "breakingChanges:list", "breakingChanges:*", "apiKey:manage", + "apiKey:list", + "apiKey:create", + "apiKey:delete", + "apiKey:update", "apiKey:*", "encryptionKey:manage", "encryptionKey:*", diff --git a/packages/@n8n/permissions/src/constants.ee.ts b/packages/@n8n/permissions/src/constants.ee.ts index ad8abb95197..12680bc9172 100644 --- a/packages/@n8n/permissions/src/constants.ee.ts +++ b/packages/@n8n/permissions/src/constants.ee.ts @@ -68,7 +68,7 @@ export const RESOURCES = { chatHub: ['manage', 'message'] as const, chatHubAgent: [...DEFAULT_OPERATIONS] as const, breakingChanges: ['list'] as const, - apiKey: ['manage'] as const, + apiKey: ['manage', 'list', 'create', 'delete', 'update'] as const, encryptionKey: ['manage'] as const, credentialResolver: [...DEFAULT_OPERATIONS] as const, instanceAi: ['message', 'manage', 'gateway'] as const, diff --git a/packages/@n8n/permissions/src/roles/scopes/global-scopes.ee.ts b/packages/@n8n/permissions/src/roles/scopes/global-scopes.ee.ts index 142444efaef..5d8abca11d4 100644 --- a/packages/@n8n/permissions/src/roles/scopes/global-scopes.ee.ts +++ b/packages/@n8n/permissions/src/roles/scopes/global-scopes.ee.ts @@ -137,6 +137,10 @@ export const GLOBAL_OWNER_SCOPES: Scope[] = [ 'breakingChanges:list', 'execution:reveal', 'apiKey:manage', + 'apiKey:list', + 'apiKey:create', + 'apiKey:delete', + 'apiKey:update', 'encryptionKey:manage', 'credentialResolver:create', 'credentialResolver:read', @@ -180,7 +184,10 @@ export const GLOBAL_MEMBER_SCOPES: Scope[] = [ 'chatHubAgent:update', 'chatHubAgent:delete', 'chatHubAgent:list', - 'apiKey:manage', + 'apiKey:list', + 'apiKey:create', + 'apiKey:delete', + 'apiKey:update', 'credentialResolver:list', 'instanceAi:message', 'instanceAi:gateway', 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 81c278f98c4..e5d89dea450 100644 --- a/packages/cli/src/controllers/__tests__/api-keys.controller.test.ts +++ b/packages/cli/src/controllers/__tests__/api-keys.controller.test.ts @@ -14,11 +14,6 @@ describe('ApiKeysController', () => { const controller = Container.get(ApiKeysController); - let req: AuthenticatedRequest; - beforeAll(() => { - req = { user: { id: '123' } } as AuthenticatedRequest; - }); - describe('createAPIKey', () => { it('should create and save an API key', async () => { // Arrange @@ -92,28 +87,16 @@ describe('ApiKeysController', () => { }); describe('getAPIKeys', () => { - it('forwards pagination params to the service and returns its envelope', async () => { - const apiKeyData = { - id: '123', - userId: '123', - label: 'My API Key', - apiKey: 'apiKey***', - createdAt: new Date(), - updatedAt: new Date(), - } as ApiKey; + it('delegates to the service with the authenticated user and pagination params', async () => { + publicApiKeyService.getRedactedApiKeys.mockResolvedValue({ items: [], count: 0 }); + const req = mock({ user: mock({ id: '123' }) }); - publicApiKeyService.getRedactedApiKeysForUser.mockResolvedValue({ - items: [{ ...apiKeyData, expiresAt: null }], - count: 1, + await controller.getApiKeys(req, mock(), { take: 10, skip: 5 } as never); + + expect(publicApiKeyService.getRedactedApiKeys).toHaveBeenCalledWith(req.user, { + take: 10, + skip: 5, }); - - const result = await controller.getApiKeys(req, mock(), { take: 10, skip: 5 } as never); - - expect(result).toEqual({ items: [{ ...apiKeyData, expiresAt: null }], count: 1 }); - expect(publicApiKeyService.getRedactedApiKeysForUser).toHaveBeenCalledWith( - expect.objectContaining({ id: req.user.id }), - { take: 10, skip: 5 }, - ); }); }); @@ -131,15 +114,15 @@ describe('ApiKeysController', () => { const req = mock({ user, params: { id: user.id } }); + publicApiKeyService.deleteApiKey.mockResolvedValue(); + // Act await controller.deleteApiKey(req, mock(), user.id); - publicApiKeyService.deleteApiKeyForUser.mockResolvedValue(); - // Assert - expect(publicApiKeyService.deleteApiKeyForUser).toHaveBeenCalledWith(user, user.id); + expect(publicApiKeyService.deleteApiKey).toHaveBeenCalledWith(user, user.id); expect(eventService.emit).toHaveBeenCalledWith( 'public-api-key-deleted', expect.objectContaining({ user, publicApi: false }), diff --git a/packages/cli/src/controllers/api-keys.controller.ts b/packages/cli/src/controllers/api-keys.controller.ts index f1a93716802..2b87f84755a 100644 --- a/packages/cli/src/controllers/api-keys.controller.ts +++ b/packages/cli/src/controllers/api-keys.controller.ts @@ -37,7 +37,7 @@ export class ApiKeysController { /** * Create an API Key */ - @GlobalScope('apiKey:manage') + @GlobalScope('apiKey:create') @Post('/', { middlewares: [isApiEnabledMiddleware] }) async createApiKey( req: AuthenticatedRequest, @@ -61,24 +61,27 @@ export class ApiKeysController { } /** - * Get API keys + * Get API keys. The service returns every key on the instance for callers + * with `apiKey:manage` (owners and admins) and the caller's own keys for + * everyone else. */ - @GlobalScope('apiKey:manage') + @GlobalScope('apiKey:list') @Get('/', { middlewares: [isApiEnabledMiddleware] }) async getApiKeys(req: AuthenticatedRequest, _res: Response, @Query query: PaginationDto) { - return await this.publicApiKeyService.getRedactedApiKeysForUser(req.user, { + return await this.publicApiKeyService.getRedactedApiKeys(req.user, { take: query.take, skip: query.skip, }); } /** - * Delete an API Key + * Delete an API Key. Callers can always delete their own keys; admins + * (holders of `apiKey:manage`) can also revoke other users' keys. */ - @GlobalScope('apiKey:manage') + @GlobalScope('apiKey:delete') @Delete('/:id', { middlewares: [isApiEnabledMiddleware] }) async deleteApiKey(req: AuthenticatedRequest, _res: Response, @Param('id') apiKeyId: string) { - await this.publicApiKeyService.deleteApiKeyForUser(req.user, apiKeyId); + await this.publicApiKeyService.deleteApiKey(req.user, apiKeyId); this.eventService.emit('public-api-key-deleted', { user: req.user, publicApi: false }); @@ -86,9 +89,10 @@ export class ApiKeysController { } /** - * Patch an API Key + * Patch an API Key. Owner-only — admins cannot edit another user's + * label or scopes. */ - @GlobalScope('apiKey:manage') + @GlobalScope('apiKey:update') @Patch('/:id', { middlewares: [isApiEnabledMiddleware] }) async updateApiKey( req: AuthenticatedRequest, @@ -105,7 +109,7 @@ export class ApiKeysController { return { success: true }; } - @GlobalScope('apiKey:manage') + @GlobalScope('apiKey:list') @Get('/scopes', { middlewares: [isApiEnabledMiddleware] }) async getApiKeyScopes(req: AuthenticatedRequest, _res: Response) { const scopes = getApiKeyScopesForRole(req.user); diff --git a/packages/cli/src/services/public-api-key.service.ts b/packages/cli/src/services/public-api-key.service.ts index be04d44b979..26a6e2f499b 100644 --- a/packages/cli/src/services/public-api-key.service.ts +++ b/packages/cli/src/services/public-api-key.service.ts @@ -3,10 +3,13 @@ import type { User } from '@n8n/db'; import { ApiKey, ApiKeyRepository, withTransaction } from '@n8n/db'; import { Service } from '@n8n/di'; import type { ApiKeyScope, AuthPrincipal } from '@n8n/permissions'; -import { getApiKeyScopesForRole, getOwnerOnlyApiKeyScopes } from '@n8n/permissions'; +import { getApiKeyScopesForRole, getOwnerOnlyApiKeyScopes, hasGlobalScope } from '@n8n/permissions'; // eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import import type { EntityManager } from '@n8n/typeorm'; import { randomUUID } from 'crypto'; + +import { NotFoundError } from '@/errors/response-errors/not-found.error'; + import { JwtService } from './jwt.service'; export const API_KEY_AUDIENCE = 'public-api'; @@ -45,28 +48,40 @@ export class PublicApiKeyService { } /** - * Retrieves a page of redacted API keys for a given user, ordered by - * `createdAt` descending. `count` is the total across all pages. + * Retrieves a page of redacted API keys with owner info attached, ordered + * by `createdAt` descending. Returns every key on the instance for callers + * with `apiKey:manage` (owners and admins); otherwise scopes to the + * caller's own keys. `count` is the total across all pages. */ - async getRedactedApiKeysForUser(user: User, options: { take?: number; skip?: number } = {}) { + async getRedactedApiKeys(caller: User, options: { take?: number; skip?: number } = {}) { + const canSeeAll = hasGlobalScope(caller, 'apiKey:manage'); const [apiKeys, count] = await this.apiKeyRepository.findAndCount({ - where: { userId: user.id, audience: API_KEY_AUDIENCE }, + where: { audience: API_KEY_AUDIENCE, ...(canSeeAll ? {} : { userId: caller.id }) }, + relations: { user: true }, order: { createdAt: 'DESC' }, take: options.take, skip: options.skip, }); return { - items: apiKeys.map((apiKeyRecord) => ({ - ...apiKeyRecord, - apiKey: this.redactApiKey(apiKeyRecord.apiKey), - expiresAt: this.getApiKeyExpiration(apiKeyRecord.apiKey), - })), + items: apiKeys.map((apiKeyRecord) => this.toRedactedApiKey(apiKeyRecord)), count, }; } - async deleteApiKeyForUser(user: User, apiKeyId: string) { - await this.apiKeyRepository.delete({ userId: user.id, id: apiKeyId }); + /** + * Deletes an API key. The caller must either own the key or hold the + * `apiKey:manage` global scope (granted to owners and admins). When + * neither condition matches we return 404 rather than 403 so the caller + * cannot probe for the existence of another user's key. + */ + async deleteApiKey(caller: User, apiKeyId: string) { + const canDeleteAny = hasGlobalScope(caller, 'apiKey:manage'); + const result = await this.apiKeyRepository.delete({ + id: apiKeyId, + audience: API_KEY_AUDIENCE, + ...(canDeleteAny ? {} : { userId: caller.id }), + }); + if (!result.affected) throw new NotFoundError('API key not found'); } async deleteAllApiKeysForUser(user: User, tx?: EntityManager) { @@ -89,6 +104,21 @@ export class PublicApiKeyService { await this.apiKeyRepository.update({ id: apiKeyId, userId: user.id }, { label, scopes }); } + private toRedactedApiKey(apiKeyRecord: ApiKey) { + const { user, ...rest } = apiKeyRecord; + return { + ...rest, + apiKey: this.redactApiKey(apiKeyRecord.apiKey), + expiresAt: this.getApiKeyExpiration(apiKeyRecord.apiKey), + owner: { + id: user.id, + firstName: user.firstName ?? null, + lastName: user.lastName ?? null, + email: user.email, + }, + }; + } + /** * Redacts an API key by replacing a portion of it with asterisks. * diff --git a/packages/cli/test/integration/api-keys.api.test.ts b/packages/cli/test/integration/api-keys.api.test.ts index 37309d8aa2f..0197257c688 100644 --- a/packages/cli/test/integration/api-keys.api.test.ts +++ b/packages/cli/test/integration/api-keys.api.test.ts @@ -11,7 +11,13 @@ import { } from '@n8n/permissions'; import { PublicApiKeyService } from '@/services/public-api-key.service'; -import { createOwnerWithApiKey, createUser, createUserShell } from './shared/db/users'; +import { + createAdmin, + createMemberWithApiKey, + createOwnerWithApiKey, + createUser, + createUserShell, +} from './shared/db/users'; import type { SuperAgentTest } from './shared/types'; import * as utils from './shared/utils/'; @@ -266,6 +272,13 @@ describe('Owner shell', () => { expect(retrieveAllApiKeysResponse.statusCode).toBe(200); + const expectedOwner = { + id: ownerShell.id, + firstName: ownerShell.firstName ?? null, + lastName: ownerShell.lastName ?? null, + email: ownerShell.email, + }; + expect(retrieveAllApiKeysResponse.body.data.count).toBe(2); expect(retrieveAllApiKeysResponse.body.data.items[0]).toEqual({ id: apiKeyWithExpiration.body.data.id, @@ -278,6 +291,7 @@ describe('Owner shell', () => { scopes: ['workflow:create'], audience: 'public-api', lastUsedAt: null, + owner: expectedOwner, }); expect(retrieveAllApiKeysResponse.body.data.items[1]).toEqual({ @@ -291,6 +305,7 @@ describe('Owner shell', () => { scopes: ['workflow:create'], audience: 'public-api', lastUsedAt: null, + owner: expectedOwner, }); }); @@ -463,6 +478,13 @@ describe('Member', () => { expect(retrieveAllApiKeysResponse.statusCode).toBe(200); + const expectedOwner = { + id: member.id, + firstName: member.firstName ?? null, + lastName: member.lastName ?? null, + email: member.email, + }; + expect(retrieveAllApiKeysResponse.body.data.count).toBe(2); expect(retrieveAllApiKeysResponse.body.data.items[0]).toEqual({ id: apiKeyWithExpiration.body.data.id, @@ -475,6 +497,7 @@ describe('Member', () => { scopes: ['workflow:create'], audience: 'public-api', lastUsedAt: null, + owner: expectedOwner, }); expect(retrieveAllApiKeysResponse.body.data.items[1]).toEqual({ @@ -488,6 +511,7 @@ describe('Member', () => { scopes: ['workflow:create'], audience: 'public-api', lastUsedAt: null, + owner: expectedOwner, }); }); @@ -559,3 +583,55 @@ describe('Pagination', () => { expect(new Set(pagedIds)).toEqual(new Set(createdIds)); }); }); + +describe('Cross-user behavior (admin scope)', () => { + test("GET /api-keys returns every user's keys for an owner", async () => { + const ownerWithKey = await createOwnerWithApiKey(); + const memberWithKey = await createMemberWithApiKey(); + + const response = await testServer.authAgentFor(ownerWithKey).get('/api-keys').expect(200); + + const ids = (response.body.data.items as Array<{ id: string }>).map((k) => k.id); + expect(ids).toEqual( + expect.arrayContaining([ownerWithKey.apiKeys[0].id, memberWithKey.apiKeys[0].id]), + ); + expect(ids).toHaveLength(2); + }); + + test('GET /api-keys returns only the caller’s keys for a member', async () => { + const memberWithKey = await createMemberWithApiKey(); + await createOwnerWithApiKey(); + + const response = await testServer.authAgentFor(memberWithKey).get('/api-keys').expect(200); + + const ids = (response.body.data.items as Array<{ id: string }>).map((k) => k.id); + expect(ids).toEqual([memberWithKey.apiKeys[0].id]); + }); + + test('DELETE /api-keys/:id 404s when a member targets another user’s key', async () => { + const ownerWithKey = await createOwnerWithApiKey(); + const member = await createUser({ role: GLOBAL_MEMBER_ROLE }); + + await testServer + .authAgentFor(member) + .delete(`/api-keys/${ownerWithKey.apiKeys[0].id}`) + .expect(404); + + // Owner's key still exists. + const ownerKeys = await Container.get(ApiKeyRepository).findBy({ userId: ownerWithKey.id }); + expect(ownerKeys).toHaveLength(1); + }); + + test('DELETE /api-keys/:id lets an admin revoke another user’s key', async () => { + const admin = await createAdmin(); + const memberWithKey = await createMemberWithApiKey(); + + await testServer + .authAgentFor(admin) + .delete(`/api-keys/${memberWithKey.apiKeys[0].id}`) + .expect(200); + + const memberKeys = await Container.get(ApiKeyRepository).findBy({ userId: memberWithKey.id }); + expect(memberKeys).toHaveLength(0); + }); +}); diff --git a/packages/frontend/editor-ui/src/app/router.ts b/packages/frontend/editor-ui/src/app/router.ts index 00f11559cfa..ca8e79856f9 100644 --- a/packages/frontend/editor-ui/src/app/router.ts +++ b/packages/frontend/editor-ui/src/app/router.ts @@ -866,7 +866,7 @@ export const routes: RouteRecordRaw[] = [ middleware: ['authenticated', 'rbac'], middlewareOptions: { rbac: { - scope: ['apiKey:manage'], + scope: ['apiKey:list'], }, }, telemetry: {