From 885083f257cc36d0af5b2a472c9ec0731abc47b7 Mon Sep 17 00:00:00 2001 From: Konstantin Tieber <46342664+konstantintieber@users.noreply.github.com> Date: Thu, 13 Aug 2026 13:36:06 +0000 Subject: [PATCH] feat(core): Enable the UI to call the public API (no-changelog) (#35692) --- packages/@n8n/constants/src/browser.ts | 17 +++ packages/cli/src/config/index.ts | 1 - .../src/controllers/api-keys.controller.ts | 18 +-- .../__tests__/http-error-classifier.test.ts | 24 +++- .../cli/src/errors/http-error-classifier.ts | 18 ++- .../mcp-api-key.service.integration.test.ts | 15 ++- ...ce.public-api-disabled.integration.test.ts | 2 +- packages/cli/src/public-api/index.ts | 16 ++- .../public-api-error-response.test.ts | 13 ++- packages/cli/src/public-api/v1/openapi.yml | 6 + .../v1/public-api-error-response.ts | 9 +- .../risk-reporters/instance-risk-reporter.ts | 4 +- packages/cli/src/server.ts | 32 +++--- .../api-key-auth.strategy.integration.test.ts | 16 +++ .../__tests__/frontend.service.test.ts | 5 +- ...n-cookie-auth.strategy.integration.test.ts | 103 ++++++++++++++++++ .../cli/src/services/api-key-auth.strategy.ts | 6 + packages/cli/src/services/frontend.service.ts | 4 +- .../services/session-cookie-auth.strategy.ts | 68 ++++++++++++ .../integration/public-api/insights.test.ts | 42 +++++++ .../instance-risk-reporter.test.ts | 2 +- packages/cli/test/integration/shared/types.ts | 1 + .../integration/shared/utils/test-server.ts | 19 ++++ .../@n8n/rest-api-client/src/utils.ts | 11 +- .../@n8n/stores/src/settings.store.test.ts | 11 ++ .../@n8n/stores/src/settings.store.ts | 3 + .../frontend/@n8n/stores/src/useRootStore.ts | 12 ++ 27 files changed, 426 insertions(+), 52 deletions(-) create mode 100644 packages/cli/src/services/__tests__/session-cookie-auth.strategy.integration.test.ts create mode 100644 packages/cli/src/services/session-cookie-auth.strategy.ts diff --git a/packages/@n8n/constants/src/browser.ts b/packages/@n8n/constants/src/browser.ts index 0fae7238d33..ffeb4172818 100644 --- a/packages/@n8n/constants/src/browser.ts +++ b/packages/@n8n/constants/src/browser.ts @@ -1 +1,18 @@ +/** `localStorage` key the per-browser identifier is persisted under. */ export const BROWSER_ID_STORAGE_KEY = 'n8n-browserId'; + +/** + * Returns a stable per-browser identifier, generating and persisting one to + * `localStorage` on first use. Sent as the `browser-id` header on authenticated + * requests so the backend can bind session cookies to the browser that + * requested them (see `AuthService.validateBrowserId`), preventing a stolen + * cookie from being replayed from a different client. + */ +export function getBrowserId(): string { + let browserId = localStorage.getItem(BROWSER_ID_STORAGE_KEY); + if (!browserId) { + browserId = crypto.randomUUID(); + localStorage.setItem(BROWSER_ID_STORAGE_KEY, browserId); + } + return browserId; +} diff --git a/packages/cli/src/config/index.ts b/packages/cli/src/config/index.ts index 353ffa8b50d..94a7ef08e1a 100644 --- a/packages/cli/src/config/index.ts +++ b/packages/cli/src/config/index.ts @@ -19,7 +19,6 @@ if (inE2ETests) { process.env.N8N_AI_ENABLED = 'true'; } else if (inTest) { globalConfig.logging.level = 'silent'; - globalConfig.publicApi.disabled = true; process.env.SKIP_STATISTICS_EVENTS = 'true'; globalConfig.auth.cookie.secure = false; process.env.N8N_SKIP_AUTH_ON_OAUTH_CALLBACK = 'false'; diff --git a/packages/cli/src/controllers/api-keys.controller.ts b/packages/cli/src/controllers/api-keys.controller.ts index d3611a4cfc9..f788cf96fd2 100644 --- a/packages/cli/src/controllers/api-keys.controller.ts +++ b/packages/cli/src/controllers/api-keys.controller.ts @@ -20,11 +20,11 @@ import type { RequestHandler } from 'express'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { EventService } from '@/events/event.service'; -import { isApiEnabled } from '@/public-api'; +import { isApiKeyAuthEnabled } from '@/public-api'; import { PublicApiKeyService } from '@/services/public-api-key.service'; -export const isApiEnabledMiddleware: RequestHandler = (_, res, next) => { - if (isApiEnabled()) { +export const isApiKeyAuthEnabledMiddleware: RequestHandler = (_, res, next) => { + if (isApiKeyAuthEnabled()) { next(); } else { res.status(404).end(); @@ -39,7 +39,7 @@ export class ApiKeysController { ) {} @GlobalScope('apiKey:create') - @Post('/', { middlewares: [isApiEnabledMiddleware] }) + @Post('/', { middlewares: [isApiKeyAuthEnabledMiddleware] }) async createApiKey( req: AuthenticatedRequest, _res: Response, @@ -64,7 +64,7 @@ export class ApiKeysController { // Every authenticated user may list their own keys. The service only // includes other users' keys for `apiKey:manage` callers; `ownership=mine` // narrows back to own. - @Get('/', { middlewares: [isApiEnabledMiddleware] }) + @Get('/', { middlewares: [isApiKeyAuthEnabledMiddleware] }) async getApiKeys(req: AuthenticatedRequest, _res: Response, @Query query: ListApiKeysQueryDto) { return await this.publicApiKeyService.getRedactedApiKeys(req.user, { take: query.take, @@ -78,7 +78,7 @@ export class ApiKeysController { // No role scope required: own keys are always revocable. The service // restricts deleting other users' keys to `apiKey:manage` holders. - @Delete('/:id', { middlewares: [isApiEnabledMiddleware] }) + @Delete('/:id', { middlewares: [isApiKeyAuthEnabledMiddleware] }) async deleteApiKey(req: AuthenticatedRequest, _res: Response, @Param('id') apiKeyId: string) { const { isOwn } = await this.publicApiKeyService.deleteApiKey(req.user, apiKeyId); @@ -93,7 +93,7 @@ export class ApiKeysController { // Owner-only — `apiKey:manage` doesn't extend to editing someone else's key. @GlobalScope('apiKey:update') - @Patch('/:id', { middlewares: [isApiEnabledMiddleware] }) + @Patch('/:id', { middlewares: [isApiKeyAuthEnabledMiddleware] }) async updateApiKey( req: AuthenticatedRequest, _res: Response, @@ -111,7 +111,7 @@ export class ApiKeysController { // Owner-only — re-issues the secret in place, keeping label, scopes and expiry. @GlobalScope('apiKey:update') - @Post('/:id/rotate', { middlewares: [isApiEnabledMiddleware] }) + @Post('/:id/rotate', { middlewares: [isApiKeyAuthEnabledMiddleware] }) async rotateApiKey(req: AuthenticatedRequest, _res: Response, @Param('id') apiKeyId: string) { const rotatedApiKey = await this.publicApiKeyService.rotateApiKey(req.user, apiKeyId); @@ -127,7 +127,7 @@ export class ApiKeysController { // No role scope required: returns the scopes the caller's role can assign // to a key — empty-ish for roles without apiKey grants. - @Get('/scopes', { middlewares: [isApiEnabledMiddleware] }) + @Get('/scopes', { middlewares: [isApiKeyAuthEnabledMiddleware] }) async getApiKeyScopes(req: AuthenticatedRequest, _res: Response) { const scopes = getApiKeyScopesForRole(req.user); return scopes; diff --git a/packages/cli/src/errors/__tests__/http-error-classifier.test.ts b/packages/cli/src/errors/__tests__/http-error-classifier.test.ts index b584a575d0b..69c8bb0c91c 100644 --- a/packages/cli/src/errors/__tests__/http-error-classifier.test.ts +++ b/packages/cli/src/errors/__tests__/http-error-classifier.test.ts @@ -1,4 +1,4 @@ -import { BadRequest } from 'express-openapi-validator/dist/framework/types'; +import { BadRequest, Unauthorized } from 'express-openapi-validator/dist/framework/types'; import { OperationalError, UnexpectedError, UserError } from 'n8n-workflow'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; @@ -52,6 +52,28 @@ describe('classifyHttpError', () => { }); }); + describe('Unauthorized', () => { + it('keeps the express-openapi-validator message when no session cookie was sent', () => { + const err = new Unauthorized({ path: '/x', message: "'X-N8N-API-KEY' header required" }); + const d = classifyHttpError(err); + expect(d).toEqual({ + kind: HttpErrorKind.httpError, + status: 401, + message: "'X-N8N-API-KEY' header required", + }); + }); + + it('replaces the api key hint with a generic message when a session cookie was sent', () => { + const err = new Unauthorized({ path: '/x', message: "'X-N8N-API-KEY' header required" }); + const d = classifyHttpError(err, { hasSessionCookie: true }); + expect(d).toEqual({ + kind: HttpErrorKind.httpError, + status: 401, + message: 'Unauthorized', + }); + }); + }); + it('tags plain Error as serverError', () => { const d = classifyHttpError(new Error('plain')); expect(d).toEqual({ diff --git a/packages/cli/src/errors/http-error-classifier.ts b/packages/cli/src/errors/http-error-classifier.ts index 9fbe3d9eb8c..d80dff32da2 100644 --- a/packages/cli/src/errors/http-error-classifier.ts +++ b/packages/cli/src/errors/http-error-classifier.ts @@ -1,4 +1,4 @@ -import { HttpError } from 'express-openapi-validator/dist/framework/types'; +import { HttpError, Unauthorized } from 'express-openapi-validator/dist/framework/types'; import { UnexpectedError, UserError } from 'n8n-workflow'; import { ResponseError } from '@/errors/response-errors/abstract/response.error'; @@ -55,7 +55,15 @@ export function isResponseError(error: Error): error is ResponseError { return false; } -export function classifyHttpError(error: Error): HttpErrorDescriptor { +export type HttpErrorClassifierContext = { + /** Whether the request carried a session cookie, regardless of whether it was valid. */ + hasSessionCookie?: boolean; +}; + +export function classifyHttpError( + error: Error, + context?: HttpErrorClassifierContext, +): HttpErrorDescriptor { if (isResponseError(error)) { const descriptor: HttpErrorDescriptor & { kind: HttpErrorKind.responseError } = { kind: HttpErrorKind.responseError, @@ -80,6 +88,12 @@ export function classifyHttpError(error: Error): HttpErrorDescriptor { return { kind: HttpErrorKind.unexpectedError, message: error.message }; } + // Replacing express-openapi-validator's 'api key header required' error message + // for requests that contained a session cookie. + if (error instanceof Unauthorized && context?.hasSessionCookie) { + return { kind: HttpErrorKind.httpError, status: 401, message: 'Unauthorized' }; + } + if (error instanceof HttpError) { return { kind: HttpErrorKind.httpError, diff --git a/packages/cli/src/modules/mcp/__tests__/mcp-api-key.service.integration.test.ts b/packages/cli/src/modules/mcp/__tests__/mcp-api-key.service.integration.test.ts index a768ffa38e1..5bb79652aea 100644 --- a/packages/cli/src/modules/mcp/__tests__/mcp-api-key.service.integration.test.ts +++ b/packages/cli/src/modules/mcp/__tests__/mcp-api-key.service.integration.test.ts @@ -4,11 +4,13 @@ import { ApiKey, ApiKeyRepository, UserRepository } from '@n8n/db'; import { Container } from '@n8n/di'; import { randomString } from 'n8n-workflow'; +import { AuthService } from '@/auth/auth.service'; import { ScopedJwtStrategy } from '@/modules/token-exchange/services/scoped-jwt.strategy'; import { TOKEN_EXCHANGE_ISSUER } from '@/modules/token-exchange/token-exchange.types'; import { ApiKeyAuthStrategy } from '@/services/api-key-auth.strategy'; import { AuthStrategyRegistry } from '@/services/auth-strategy.registry'; import { JwtService } from '@/services/jwt.service'; +import { SessionCookieAuthStrategy } from '@/services/session-cookie-auth.strategy'; import { createMember, createOwner, createOwnerWithApiKey } from '@test-integration/db/users'; import { McpServerApiKeyService } from '../mcp-api-key.service'; @@ -40,10 +42,11 @@ describe('McpServerApiKeyService.verifyApiKey (integration)', () => { beforeAll(async () => { await testDb.init(); - // Mirror the registration order set up by public API + token-exchange module init. + // Mirror the registration order set up by server.ts + token-exchange module init. const registry = Container.get(AuthStrategyRegistry); jwtService = Container.get(JwtService); registry.register(Container.get(ApiKeyAuthStrategy)); + registry.register(Container.get(SessionCookieAuthStrategy)); registry.register(Container.get(ScopedJwtStrategy)); service = Container.get(McpServerApiKeyService); @@ -140,6 +143,16 @@ describe('McpServerApiKeyService.verifyApiKey (integration)', () => { name: 'scoped JWT whose subject is not in the database', token: () => makeScopedJwt(jwtService, '00000000-0000-0000-0000-000000000000'), }, + { + // SessionCookieAuthStrategy is registered into the same registry above, + // so this proves its buildTokenGrant genuinely abstains rather than the + // audience check merely never encountering the strategy. + name: 'a valid browser session cookie (must not satisfy the mcp-server-api audience)', + token: async () => { + const owner = await createOwner(); + return Container.get(AuthService).issueJWT(owner, false); + }, + }, ])('rejects $name', async ({ token }) => { const result = await service.verifyApiKey(await token()); diff --git a/packages/cli/src/modules/mcp/__tests__/mcp-api-key.service.public-api-disabled.integration.test.ts b/packages/cli/src/modules/mcp/__tests__/mcp-api-key.service.public-api-disabled.integration.test.ts index ae7345460c0..e12679242ae 100644 --- a/packages/cli/src/modules/mcp/__tests__/mcp-api-key.service.public-api-disabled.integration.test.ts +++ b/packages/cli/src/modules/mcp/__tests__/mcp-api-key.service.public-api-disabled.integration.test.ts @@ -8,7 +8,7 @@ import { McpServerApiKeyService } from '../mcp-api-key.service'; // Regression guard for IAM-574: ApiKeyAuthStrategy must be registered regardless // of whether the public API is enabled, otherwise MCP API keys stop working on // instances that disable the public REST API. Omitting 'publicApi' from -// endpointGroups mirrors a production instance where isApiEnabled() returns +// endpointGroups mirrors a production instance where isApiKeyAuthEnabled() returns // false — the MCP endpoint group is still set up so we can exercise the real // strategy-registration wiring. // Loading the MCP module plus DB init can exceed the default 30s hook timeout diff --git a/packages/cli/src/public-api/index.ts b/packages/cli/src/public-api/index.ts index edeb9025cfb..35569de2b40 100644 --- a/packages/cli/src/public-api/index.ts +++ b/packages/cli/src/public-api/index.ts @@ -13,6 +13,7 @@ import validator from 'validator'; import { PublicApiControllerRegistry } from './public-api-controller.registry'; import { sendPublicApiErrorResponse } from './v1/public-api-error-response'; +import { AUTH_COOKIE_NAME } from '@/constants'; import { EventService } from '@/events/event.service'; import { License } from '@/license'; import { createN8nPackageMulterOptions } from '@/modules/n8n-packages/utils/import-package-upload'; @@ -321,6 +322,7 @@ function createLazyValidatorMiddleware( handlers: { ApiKeyAuth: authenticate, BearerAuth: authenticate, + CookieAuth: authenticate, }, }, }), @@ -379,11 +381,13 @@ function createApiRouter( const publicApiErrorHandler: ErrorRequestHandler = ( error: Error, - _req: express.Request, + req: express.Request, res: express.Response, _next: express.NextFunction, ) => { - sendPublicApiErrorResponse(res, error); + sendPublicApiErrorResponse(res, error, { + hasSessionCookie: Boolean(req.cookies?.[AUTH_COOKIE_NAME]), + }); }; apiController.use(publicApiErrorHandler); @@ -410,6 +414,12 @@ export const loadPublicApiVersions = async ( }; }; -export function isApiEnabled(): boolean { +/** + * Whether API-key (token) based authentication is accepted on the public API. + * Public API routes are always registered regardless of this flag — the UI + * authenticates against them with the user's session cookie instead, which + * must keep working even when token-based access is disabled. + */ +export function isApiKeyAuthEnabled(): boolean { return !Container.get(GlobalConfig).publicApi.disabled && !Container.get(License).isAPIDisabled(); } diff --git a/packages/cli/src/public-api/v1/__tests__/public-api-error-response.test.ts b/packages/cli/src/public-api/v1/__tests__/public-api-error-response.test.ts index b2c5554a82b..5bd30ffb371 100644 --- a/packages/cli/src/public-api/v1/__tests__/public-api-error-response.test.ts +++ b/packages/cli/src/public-api/v1/__tests__/public-api-error-response.test.ts @@ -1,5 +1,5 @@ import type { Response } from 'express'; -import { BadRequest } from 'express-openapi-validator/dist/framework/types'; +import { BadRequest, Unauthorized } from 'express-openapi-validator/dist/framework/types'; import { UnexpectedError, UserError, OperationalError } from 'n8n-workflow'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; @@ -96,6 +96,17 @@ describe('sendPublicApiErrorResponse', () => { expect(res._payload.body).toEqual({ message: 'schema failed' }); }); + it('forwards classifier context so session-cookie Unauthorized is serialized as a generic 401', () => { + const res = createMockRes(); + const err = new Unauthorized({ + path: '/api/v1/insights/summary', + message: "'X-N8N-API-KEY' header required", + }); + sendPublicApiErrorResponse(res, err, { hasSessionCookie: true }); + expect(res._payload.statusCode).toBe(401); + expect(res._payload.body).toEqual({ message: 'Unauthorized' }); + }); + it('maps unknown errors to 500 with a generic message', () => { const res = createMockRes(); sendPublicApiErrorResponse(res, new Error('plain')); diff --git a/packages/cli/src/public-api/v1/openapi.yml b/packages/cli/src/public-api/v1/openapi.yml index 2c9f9743ca2..943e9c7f3bc 100644 --- a/packages/cli/src/public-api/v1/openapi.yml +++ b/packages/cli/src/public-api/v1/openapi.yml @@ -221,7 +221,13 @@ components: type: http scheme: bearer bearerFormat: JWT + CookieAuth: + type: apiKey + in: cookie + # Must match AUTH_COOKIE_NAME in packages/cli/src/constants.ts + name: n8n-auth security: - ApiKeyAuth: [] - BearerAuth: [] + - CookieAuth: [] diff --git a/packages/cli/src/public-api/v1/public-api-error-response.ts b/packages/cli/src/public-api/v1/public-api-error-response.ts index 4d0fda96b09..77c430ccf67 100644 --- a/packages/cli/src/public-api/v1/public-api-error-response.ts +++ b/packages/cli/src/public-api/v1/public-api-error-response.ts @@ -1,5 +1,6 @@ import type { Response } from 'express'; +import type { HttpErrorClassifierContext } from '@/errors/http-error-classifier'; import { classifyHttpError } from '@/errors/http-error-classifier'; import { serializePublicApiError } from '@/errors/http-error-serializers'; @@ -7,8 +8,12 @@ import { serializePublicApiError } from '@/errors/http-error-serializers'; * Maps errors from the public API stack to HTTP responses. Used by the * Express error middleware in `index.ts`. */ -export function sendPublicApiErrorResponse(res: Response, error: Error): void { - const descriptor = classifyHttpError(error); +export function sendPublicApiErrorResponse( + res: Response, + error: Error, + context?: HttpErrorClassifierContext, +): void { + const descriptor = classifyHttpError(error, context); const { status, body } = serializePublicApiError(descriptor); res.status(status).json(body); } diff --git a/packages/cli/src/security-audit/risk-reporters/instance-risk-reporter.ts b/packages/cli/src/security-audit/risk-reporters/instance-risk-reporter.ts index 2c73a0e1acb..02792f394cf 100644 --- a/packages/cli/src/security-audit/risk-reporters/instance-risk-reporter.ts +++ b/packages/cli/src/security-audit/risk-reporters/instance-risk-reporter.ts @@ -9,7 +9,7 @@ import type { IWorkflowBase } from 'n8n-workflow'; import { N8N_VERSION } from '@/constants'; import { CommunityPackagesConfig } from '@/modules/community-packages/community-packages.config'; -import { isApiEnabled } from '@/public-api'; +import { isApiKeyAuthEnabled } from '@/public-api'; import { ENV_VARS_DOCS_URL, INSTANCE_REPORT, @@ -103,7 +103,7 @@ export class InstanceRiskReporter implements RiskReporter { communityPackagesEnabled: Container.get(CommunityPackagesConfig).enabled, versionNotificationsEnabled: this.globalConfig.versionNotifications.enabled, templatesEnabled: this.globalConfig.templates.enabled, - publicApiEnabled: isApiEnabled(), + publicApiEnabled: isApiKeyAuthEnabled(), }; const { exclude, include } = this.globalConfig.nodes; diff --git a/packages/cli/src/server.ts b/packages/cli/src/server.ts index 59d1824d4a3..48f907d6fde 100644 --- a/packages/cli/src/server.ts +++ b/packages/cli/src/server.ts @@ -25,7 +25,7 @@ import type { ICredentialsOverwrite } from '@/interfaces'; import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; import { handleMfaDisable, isMfaFeatureEnabled } from '@/mfa/helpers'; import { PostHogClient } from '@/posthog'; -import { isApiEnabled, loadPublicApiVersions } from '@/public-api'; +import { loadPublicApiVersions } from '@/public-api'; import { Push } from '@/push'; import * as ResponseHelper from '@/response-helper'; import type { FrontendService } from '@/services/frontend.service'; @@ -77,6 +77,7 @@ import { BrowserUseServer } from './modules/instance-ai/browser/browser-use-serv import { PubSubRegistry } from './scaling/pubsub/pubsub.registry'; import { ApiKeyAuthStrategy } from './services/api-key-auth.strategy'; import { AuthStrategyRegistry } from './services/auth-strategy.registry'; +import { SessionCookieAuthStrategy } from './services/session-cookie-auth.strategy'; @Service() export class Server extends AbstractServer { @@ -184,22 +185,17 @@ export class Server extends AbstractServer { // Register auth strategies in priority order. The registry evaluates them // sequentially — the first strategy that returns a non-null result wins. // API key auth is registered first so existing behavior is preserved. + // Session cookie auth is registered last: an explicit but wrong API + // key/bearer token fails fast rather than silently falling back to an + // ambient browser session cookie. // Additional strategies (e.g. scoped JWT from the token-exchange module) // can be appended later during their own module initialization. const registry = Container.get(AuthStrategyRegistry); registry.register(Container.get(ApiKeyAuthStrategy)); + registry.register(Container.get(SessionCookieAuthStrategy)); - // ---------------------------------------- - // Public API - // ---------------------------------------- - - if (isApiEnabled()) { - const { apiRouters, apiLatestVersion } = await loadPublicApiVersions(publicApiEndpoint); - this.app.use(...apiRouters); - if (frontendService) { - (await frontendService.getSettings()).publicApi.latestVersion = apiLatestVersion; - } - } + // Parse cookies for easier access + this.app.use(cookieParser()); // Extract BrowserId from headers this.app.use((req: APIRequest, _, next) => { @@ -207,8 +203,15 @@ export class Server extends AbstractServer { next(); }); - // Parse cookies for easier access - this.app.use(cookieParser()); + // ---------------------------------------- + // Public API + // ---------------------------------------- + + const { apiRouters, apiLatestVersion } = await loadPublicApiVersions(publicApiEndpoint); + this.app.use(...apiRouters); + if (frontendService) { + (await frontendService.getSettings()).publicApi.latestVersion = apiLatestVersion; + } const { restEndpoint, app } = this; @@ -437,7 +440,6 @@ export class Server extends AbstractServer { 'e2e', this.restEndpoint, this.endpointPresetCredentials, - isApiEnabled() ? '' : publicApiEndpoint, ...this.globalConfig.endpoints.additionalNonUIRoutes.split(':'), ].filter((u) => !!u); const nonUIRoutesRegex = new RegExp(`^/(${nonUIRoutes.join('|')})/?.*$`); diff --git a/packages/cli/src/services/__tests__/api-key-auth.strategy.integration.test.ts b/packages/cli/src/services/__tests__/api-key-auth.strategy.integration.test.ts index 1de65f0ee26..8c2beff2261 100644 --- a/packages/cli/src/services/__tests__/api-key-auth.strategy.integration.test.ts +++ b/packages/cli/src/services/__tests__/api-key-auth.strategy.integration.test.ts @@ -1,5 +1,6 @@ import { Logger } from '@n8n/backend-common'; import { testDb } from '@n8n/backend-test-utils'; +import { GlobalConfig } from '@n8n/config'; import type { AuthenticatedRequest, User } from '@n8n/db'; import { ApiKey, ApiKeyRepository, UserRepository } from '@n8n/db'; import { Container } from '@n8n/di'; @@ -215,5 +216,20 @@ describe('ApiKeyAuthStrategy', () => { expect(req.user.id).toBe(owner.id); expect(req.tokenGrant?.subject.id).toBe(owner.id); }); + + it('abstains (returns null) when N8N_PUBLIC_API_DISABLED is set, even with a valid API key', async () => { + const globalConfig = Container.get(GlobalConfig); + globalConfig.publicApi.disabled = true; + + try { + const owner = await createOwnerWithApiKey(); + const [{ apiKey }] = owner.apiKeys; + const req = mockReqWith(apiKey); + + expect(await strategy.authenticate(req)).toBeNull(); + } finally { + globalConfig.publicApi.disabled = false; + } + }); }); }); diff --git a/packages/cli/src/services/__tests__/frontend.service.test.ts b/packages/cli/src/services/__tests__/frontend.service.test.ts index a2a8caf61e7..d945de89171 100644 --- a/packages/cli/src/services/__tests__/frontend.service.test.ts +++ b/packages/cli/src/services/__tests__/frontend.service.test.ts @@ -9,7 +9,7 @@ import type { ICredentialType, INodeTypeDescription } from 'n8n-workflow'; import type { CredentialTypes } from '@/credential-types'; import type { CredentialsOverwrites } from '@/credentials-overwrites'; -import type { License } from '@/license'; +import { License } from '@/license'; import type { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; import type { MfaService } from '@/mfa/mfa.service'; import { CommunityPackagesConfig } from '@/modules/community-packages/community-packages.config'; @@ -200,6 +200,9 @@ describe('FrontendService', () => { enabled: false, }), ); + // isApiKeyAuthEnabled() reads License via the container directly, so the + // constructor-injected mock above must also be registered here. + Container.set(License, license); return { service: new FrontendService( diff --git a/packages/cli/src/services/__tests__/session-cookie-auth.strategy.integration.test.ts b/packages/cli/src/services/__tests__/session-cookie-auth.strategy.integration.test.ts new file mode 100644 index 00000000000..1bef088ef3e --- /dev/null +++ b/packages/cli/src/services/__tests__/session-cookie-auth.strategy.integration.test.ts @@ -0,0 +1,103 @@ +import { LicenseState } from '@n8n/backend-common'; +import { testDb } from '@n8n/backend-test-utils'; +import type { AuthenticatedRequest } from '@n8n/db'; +import { InvalidAuthTokenRepository, UserRepository } from '@n8n/db'; +import { Container } from '@n8n/di'; +import { mock } from 'vitest-mock-extended'; + +import { AuthService } from '@/auth/auth.service'; +import { AUTH_COOKIE_NAME } from '@/constants'; +import { createOwner } from '@test-integration/db/users'; + +import { SessionCookieAuthStrategy } from '../session-cookie-auth.strategy'; + +// MFA enforcement gates inside AuthService.authenticateUserBasedOnToken call into the +// license state; stub it so any feature check returns "not licensed". +const licenseMock = mock(); +licenseMock.isLicensed.mockReturnValue(false); +Container.set(LicenseState, licenseMock); + +const mockReqWith = (token: string): AuthenticatedRequest => { + const req = mock({ path: '/test', method: 'GET' }); + // Override the deep-mock proxy so the cookie lookup returns the exact value. + req.cookies = { [AUTH_COOKIE_NAME]: token }; + return req; +}; + +const mockReqWithoutCookie = (): AuthenticatedRequest => { + const req = mock({ path: '/test', method: 'GET' }); + // Override the deep-mock proxy so the cookie lookup returns undefined. + req.cookies = {}; + return req; +}; + +let authService: AuthService; +let authenticator: SessionCookieAuthStrategy; + +describe('SessionCookieAuthStrategy', () => { + beforeAll(async () => { + await testDb.init(); + authService = Container.get(AuthService); + authenticator = new SessionCookieAuthStrategy(authService); + }); + + beforeEach(async () => { + await testDb.truncate(['User']); + }); + + afterAll(async () => { + await testDb.terminate(); + }); + + it('returns null when no session cookie is present', async () => { + expect(await authenticator.authenticate(mockReqWithoutCookie())).toBeNull(); + }); + + it('returns true and populates req.user + req.tokenGrant on success', async () => { + const owner = await createOwner(); + const token = authService.issueJWT(owner, false); + const req = mockReqWith(token); + + expect(await authenticator.authenticate(req)).toBe(true); + expect(req.user.id).toBe(owner.id); + expect(req.tokenGrant?.subject.id).toBe(owner.id); + expect(req.tokenGrant?.scopes.length).toBeGreaterThan(0); + expect(req.tokenGrant?.apiKeyScopes?.length).toBeGreaterThan(0); + }); + + it('returns false for an invalid session cookie', async () => { + expect(await authenticator.authenticate(mockReqWith('not-a-jwt'))).toBe(false); + }); + + it('returns false for a revoked token', async () => { + const owner = await createOwner(); + const token = authService.issueJWT(owner, false); + await Container.get(InvalidAuthTokenRepository).insert({ + token, + expiresAt: new Date(Date.now() + 60_000), + }); + + expect(await authenticator.authenticate(mockReqWith(token))).toBe(false); + }); + + it('returns false for a disabled user', async () => { + const owner = await createOwner(); + const token = authService.issueJWT(owner, false); + await Container.get(UserRepository).update({ id: owner.id }, { disabled: true }); + + expect(await authenticator.authenticate(mockReqWith(token))).toBe(false); + }); + + it('buildTokenGrant always abstains — session cookies must never satisfy audience-scoped bearer-token checks', async () => { + expect(await authenticator.buildTokenGrant('anything')).toBeNull(); + expect(await authenticator.buildTokenGrant('')).toBeNull(); + + // Even a genuinely valid session JWT, passed as a raw bearer token with an + // audience mirroring a real scoped-token check, must still be refused. + const owner = await createOwner(); + const validSessionJwt = authService.issueJWT(owner, false); + expect( + await authenticator.buildTokenGrant(validSessionJwt, { audience: 'mcp-server-api' }), + ).toBeNull(); + }); +}); diff --git a/packages/cli/src/services/api-key-auth.strategy.ts b/packages/cli/src/services/api-key-auth.strategy.ts index 3647b81d457..1c71f70c976 100644 --- a/packages/cli/src/services/api-key-auth.strategy.ts +++ b/packages/cli/src/services/api-key-auth.strategy.ts @@ -5,6 +5,8 @@ import { ApiKeyRepository } from '@n8n/db'; import { Service } from '@n8n/di'; import { TokenExpiredError } from 'jsonwebtoken'; +import { isApiKeyAuthEnabled } from '@/public-api'; + import type { AuthStrategy, AuthStrategyOptions } from './auth-strategy.types'; import { JwtService } from './jwt.service'; import { API_KEY_AUDIENCE, API_KEY_ISSUER, PREFIX_LEGACY_API_KEY } from './public-api-key.service'; @@ -82,6 +84,10 @@ export class ApiKeyAuthStrategy implements AuthStrategy { } async authenticate(req: AuthenticatedRequest): Promise { + // Abstain when API-key auth is disabled so other strategies (e.g. session + // cookie) can still authenticate the request. + if (!isApiKeyAuthEnabled()) return null; + const providedApiKey = req.headers[API_KEY_HEADER]; if (typeof providedApiKey !== 'string' || !providedApiKey) return null; diff --git a/packages/cli/src/services/frontend.service.ts b/packages/cli/src/services/frontend.service.ts index 04c3490fd2b..bcc4281395e 100644 --- a/packages/cli/src/services/frontend.service.ts +++ b/packages/cli/src/services/frontend.service.ts @@ -22,7 +22,7 @@ import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; import { MfaService } from '@/mfa/mfa.service'; import { CommunityPackagesConfig } from '@/modules/community-packages/community-packages.config'; import type { CommunityPackagesService } from '@/modules/community-packages/community-packages.service'; -import { isApiEnabled } from '@/public-api'; +import { isApiKeyAuthEnabled } from '@/public-api'; import { PushConfig } from '@/push/push.config'; import { OwnershipService } from '@/services/ownership.service'; import { getSamlLoginLabel, getCurrentAuthenticationMethod } from '@/sso.ee/sso-helpers'; @@ -300,7 +300,7 @@ export class FrontendService { maxSize: this.globalConfig.dataTable.maxSize, }, publicApi: { - enabled: isApiEnabled(), + enabled: isApiKeyAuthEnabled(), latestVersion: 1, path: this.globalConfig.publicApi.path, swaggerUi: { diff --git a/packages/cli/src/services/session-cookie-auth.strategy.ts b/packages/cli/src/services/session-cookie-auth.strategy.ts new file mode 100644 index 00000000000..66d4db01dc3 --- /dev/null +++ b/packages/cli/src/services/session-cookie-auth.strategy.ts @@ -0,0 +1,68 @@ +import type { AuthenticatedRequest, TokenGrant, User } from '@n8n/db'; +import { Service } from '@n8n/di'; +import { getApiKeyScopesForRole } from '@n8n/permissions'; +import { JsonWebTokenError, TokenExpiredError } from 'jsonwebtoken'; + +import { AuthService } from '@/auth/auth.service'; +import { AUTH_COOKIE_NAME } from '@/constants'; +import { AuthError } from '@/errors/response-errors/auth.error'; + +import type { AuthStrategy, AuthStrategyOptions } from './auth-strategy.types'; + +/** + * Lets the public API accept the browser's `n8n-auth` session cookie as an + * alternative to an API key. + */ +@Service() +export class SessionCookieAuthStrategy implements AuthStrategy { + constructor(private readonly authService: AuthService) {} + + /** + * Session cookies authenticate a request via req.cookies, never via an arbitrary + * bearer token string, so this strategy has no legitimate way to answer "is this + * token valid for audience X". It must always abstain here, ignoring `token`/`options` + * entirely — otherwise a browser session JWT could satisfy an audience-scoped + * bearer-token check it was never meant to (e.g. McpServerApiKeyService.verifyApiKey). + */ + async buildTokenGrant(_token: string, _options?: AuthStrategyOptions): Promise { + return null; + } + + async authenticate(req: AuthenticatedRequest): Promise { + const token = req.cookies[AUTH_COOKIE_NAME]; + if (!token) return null; + + try { + const user = await this.authService.authenticateUserBasedOnToken( + token, + req.method, + this.authService.getEndpoint(req), + this.authService.getBrowserId(req), + ); + + const tokenGrant = this.toGrant(user); + req.user = user; + req.tokenGrant = tokenGrant; + return true; + } catch (error) { + if (this.isAuthFailure(error)) return false; + throw error; + } + } + + private toGrant(user: User): TokenGrant { + return { + scopes: user.role.scopes.map((s) => s.slug), + subject: user, + apiKeyScopes: getApiKeyScopesForRole(user), + }; + } + + private isAuthFailure(error: unknown): boolean { + return ( + error instanceof AuthError || + error instanceof TokenExpiredError || + error instanceof JsonWebTokenError + ); + } +} diff --git a/packages/cli/test/integration/public-api/insights.test.ts b/packages/cli/test/integration/public-api/insights.test.ts index 6b2bd35e447..b981e6b9456 100644 --- a/packages/cli/test/integration/public-api/insights.test.ts +++ b/packages/cli/test/integration/public-api/insights.test.ts @@ -3,6 +3,7 @@ import { createTeamProject, createWorkflow, testDb } from '@n8n/backend-test-uti import { type User } from '@n8n/db'; import { DateTime } from 'luxon'; +import { AUTH_COOKIE_NAME } from '@/constants'; import { createCompactedInsightsEvent } from '@/modules/insights/database/entities/__tests__/db-utils'; import { createOwnerWithApiKey } from '../shared/db/users'; @@ -81,6 +82,47 @@ describe('GET /insights/summary', () => { await testServer.publicApiAgentWithoutApiKey().get('/insights/summary').expect(401); }); + test('returns data via session cookie, without an API key', async () => { + const project = await createTeamProject(); + const workflow = await createWorkflow({}, project); + + await createSummaryMetrics(workflow, { + success: 3, + failure: 1, + runtimeMs: 400, + timeSavedMin: 20, + }); + + const response = await testServer + .publicApiAgentWithCookie(scopedOwner) + .get('/insights/summary') + .query({ + startDate: DateTime.utc().minus({ days: 2 }).toISO(), + endDate: DateTime.utc().plus({ days: 1 }).toISO(), + }) + .expect(200); + + expect(response.body.total.value).toBe(4); + expect(response.body.failed.value).toBe(1); + }); + + test('returns 401 with an invalid session cookie', async () => { + const agent = testServer.publicApiAgentWithoutApiKey(); + agent.jar.setCookie(`${AUTH_COOKIE_NAME}=invalid`); + + const response = await agent.get('/insights/summary').expect(401); + + expect(response.body).toEqual({ message: 'Unauthorized' }); + }); + + test('returns 401 with a hint to use an API key when no credentials are sent at all', async () => { + const agent = testServer.publicApiAgentWithoutApiKey(); + + const response = await agent.get('/insights/summary').expect(401); + + expect(response.body).toEqual({ message: "'X-N8N-API-KEY' header required" }); + }); + test('returns 403 without insights:read scope', async () => { await authUnscopedAgent.get('/insights/summary').expect(403); }); diff --git a/packages/cli/test/integration/security-audit/instance-risk-reporter.test.ts b/packages/cli/test/integration/security-audit/instance-risk-reporter.test.ts index b2aed9e6699..7530ecd87e2 100644 --- a/packages/cli/test/integration/security-audit/instance-risk-reporter.test.ts +++ b/packages/cli/test/integration/security-audit/instance-risk-reporter.test.ts @@ -255,7 +255,7 @@ test('should report security settings', async () => { communityPackagesEnabled: true, versionNotificationsEnabled: true, templatesEnabled: true, - publicApiEnabled: false, + publicApiEnabled: true, }, nodes: { nodesExclude: 'n8n-nodes-base.executeCommand, n8n-nodes-base.localFileTrigger', diff --git a/packages/cli/test/integration/shared/types.ts b/packages/cli/test/integration/shared/types.ts index ae19c10d123..3a02a1e0310 100644 --- a/packages/cli/test/integration/shared/types.ts +++ b/packages/cli/test/integration/shared/types.ts @@ -91,6 +91,7 @@ export interface TestServer { publicApiAgentFor: (user: User) => TestAgent; publicApiAgentWithApiKey: (apiKey: string) => TestAgent; publicApiAgentWithoutApiKey: () => TestAgent; + publicApiAgentWithCookie: (user: User) => TestAgent; authlessAgent: TestAgent; restlessAgent: TestAgent; license: LicenseMocker; diff --git a/packages/cli/test/integration/shared/utils/test-server.ts b/packages/cli/test/integration/shared/utils/test-server.ts index 085817e7df1..5c6fb69689c 100644 --- a/packages/cli/test/integration/shared/utils/test-server.ts +++ b/packages/cli/test/integration/shared/utils/test-server.ts @@ -19,6 +19,7 @@ import { PostHogClient } from '@/posthog'; import { Push } from '@/push'; import { ApiKeyAuthStrategy } from '@/services/api-key-auth.strategy'; import { AuthStrategyRegistry } from '@/services/auth-strategy.registry'; +import { SessionCookieAuthStrategy } from '@/services/session-cookie-auth.strategy'; import { Telemetry } from '@/telemetry'; import { resolveBackendHealthEndpointPath } from '@/utils/health-endpoint.util'; import { LicenseMocker } from '@test-integration/license'; @@ -93,6 +94,19 @@ const publicApiAgent = ( return agent; }; +const publicApiAgentWithCookie = (app: express.Application, user: User, version = 1) => { + const agent = request.agent(app); + void agent.use(prefix(`${PUBLIC_API_REST_PATH_SEGMENT}/v${version}`)); + // browser-id is needed for session cookie auth + void agent.use(async (req: superagent.SuperAgentRequest) => { + req.set('browser-id', browserId); + return await req; + }); + const token = Container.get(AuthService).issueJWT(user, user.mfaEnabled, browserId); + agent.jar.setCookie(`${AUTH_COOKIE_NAME}=${token}`); + return agent; +}; + export const setupTestServer = ({ endpointGroups, enabledFeatures, @@ -124,6 +138,7 @@ export const setupTestServer = ({ publicApiAgentFor: (user) => publicApiAgent(app, { user }), publicApiAgentWithApiKey: (apiKey) => publicApiAgent(app, { apiKey }), publicApiAgentWithoutApiKey: () => publicApiAgent(app, {}), + publicApiAgentWithCookie: (user) => publicApiAgentWithCookie(app, user), license: new LicenseMocker(), }; @@ -153,10 +168,14 @@ export const setupTestServer = ({ // Register auth strategies in priority order. The registry evaluates them // sequentially — the first strategy that returns a non-null result wins. // API key auth is registered first so existing behavior is preserved. + // Session cookie auth is registered last: an explicit but wrong API + // key/bearer token fails fast rather than silently falling back to an + // ambient browser session cookie. // Additional strategies (e.g. scoped JWT from the token-exchange module) // can be appended later during their own module initialization. const registry = Container.get(AuthStrategyRegistry); registry.register(Container.get(ApiKeyAuthStrategy)); + registry.register(Container.get(SessionCookieAuthStrategy)); const enablePublicAPI = endpointGroups?.includes('publicApi'); if (enablePublicAPI) { diff --git a/packages/frontend/@n8n/rest-api-client/src/utils.ts b/packages/frontend/@n8n/rest-api-client/src/utils.ts index 3362d753262..343a1f48058 100644 --- a/packages/frontend/@n8n/rest-api-client/src/utils.ts +++ b/packages/frontend/@n8n/rest-api-client/src/utils.ts @@ -1,4 +1,4 @@ -import { BROWSER_ID_STORAGE_KEY } from '@n8n/constants'; +import { getBrowserId } from '@n8n/constants'; import { assert } from '@n8n/utils/assert'; import type { AxiosRequestConfig, Method, RawAxiosRequestHeaders } from 'axios'; import axios from 'axios'; @@ -7,15 +7,6 @@ import type { GenericValue, IDataObject } from 'n8n-workflow'; import type { IRestApiContext } from './types'; -const getBrowserId = () => { - let browserId = localStorage.getItem(BROWSER_ID_STORAGE_KEY); - if (!browserId) { - browserId = crypto.randomUUID(); - localStorage.setItem(BROWSER_ID_STORAGE_KEY, browserId); - } - return browserId; -}; - export const NO_NETWORK_ERROR_CODE = 999; export const STREAM_SEPARATOR = '⧉⇋⇋➽⌑⧉§§\n'; diff --git a/packages/frontend/@n8n/stores/src/settings.store.test.ts b/packages/frontend/@n8n/stores/src/settings.store.test.ts index 15922616581..770becffadc 100644 --- a/packages/frontend/@n8n/stores/src/settings.store.test.ts +++ b/packages/frontend/@n8n/stores/src/settings.store.test.ts @@ -23,6 +23,7 @@ const mockRootStore = { setExecutionTimeout: vi.fn(), setMaxExecutionTimeout: vi.fn(), setInstanceId: vi.fn(), + setPublicApiPath: vi.fn(), setOauthCallbackUrls: vi.fn(), setJwksUri: vi.fn(), setN8nMetadata: vi.fn(), @@ -73,6 +74,12 @@ const mockSettings = mock({ telemetry: { enabled: false, }, + publicApi: { + enabled: true, + latestVersion: 1, + path: 'api', + swaggerUi: { enabled: true }, + }, }); const publicSettingsResponse = { @@ -337,6 +344,7 @@ describe('settings.store', () => { expect(mockRootStore.setMaxExecutionTimeout).not.toHaveBeenCalled(); expect(mockRootStore.setN8nMetadata).not.toHaveBeenCalled(); expect(mockRootStore.setBinaryDataMode).not.toHaveBeenCalled(); + expect(mockRootStore.setPublicApiPath).not.toHaveBeenCalled(); // side effects expect(sessionStarted).not.toHaveBeenCalled(); @@ -391,6 +399,9 @@ describe('settings.store', () => { expect(mockRootStore.setN8nMetadata).toHaveBeenCalled(); expect(mockRootStore.setDefaultLocale).toHaveBeenCalled(); expect(mockRootStore.setBinaryDataMode).toHaveBeenCalled(); + expect(mockRootStore.setPublicApiPath).toHaveBeenCalledWith( + `${mockSettings.publicApi.path}/v${mockSettings.publicApi.latestVersion}`, + ); // side effects expect(sessionStarted).toHaveBeenCalled(); diff --git a/packages/frontend/@n8n/stores/src/settings.store.ts b/packages/frontend/@n8n/stores/src/settings.store.ts index 1b60f972b40..42b8b23cf1d 100644 --- a/packages/frontend/@n8n/stores/src/settings.store.ts +++ b/packages/frontend/@n8n/stores/src/settings.store.ts @@ -367,6 +367,9 @@ export const useSettingsStore = defineStore(STORES.SETTINGS, () => { rootStore.setTimezone(fetchedSettings.timezone); rootStore.setExecutionTimeout(fetchedSettings.executionTimeout); rootStore.setMaxExecutionTimeout(fetchedSettings.maxExecutionTimeout); + rootStore.setPublicApiPath( + `${fetchedSettings.publicApi.path}/v${fetchedSettings.publicApi.latestVersion}`, + ); rootStore.setInstanceId(fetchedSettings.instanceId); rootStore.setOauthCallbackUrls(fetchedSettings.oauthCallbackUrls); rootStore.setN8nMetadata(fetchedSettings.n8nMetadata ?? {}); diff --git a/packages/frontend/@n8n/stores/src/useRootStore.ts b/packages/frontend/@n8n/stores/src/useRootStore.ts index b2bed813024..f69102928f0 100644 --- a/packages/frontend/@n8n/stores/src/useRootStore.ts +++ b/packages/frontend/@n8n/stores/src/useRootStore.ts @@ -29,6 +29,7 @@ export type RootStoreState = { [key: string]: string | number | undefined; }; pushRef: string; + publicApiPath: string; urlBaseWebhook: string; urlBaseEditor: string; urlBaseWebhookTest: string; @@ -57,6 +58,7 @@ export const useRootStore = defineStore(STORES.ROOT, () => { jwksUri: '', n8nMetadata: {}, pushRef: randomString(10).toLowerCase(), + publicApiPath: 'api/v1', urlBaseWebhook: 'http://localhost:5678/', urlBaseEditor: 'http://localhost:5678', urlBaseWebhookTest: 'http://localhost:5678/', @@ -127,6 +129,10 @@ export const useRootStore = defineStore(STORES.ROOT, () => { pushRef: state.value.pushRef, })); + const publicApiContext = computed(() => ({ + baseUrl: `${state.value.baseUrl}${state.value.publicApiPath}`, + })); + // #endregion // --------------------------------------------------------------------------- @@ -225,6 +231,10 @@ export const useRootStore = defineStore(STORES.ROOT, () => { state.value.pushRef = value; }; + const setPublicApiPath = (value: string) => { + state.value.publicApiPath = value; + }; + // #endregion return { @@ -239,6 +249,7 @@ export const useRootStore = defineStore(STORES.ROOT, () => { webhookWaitingUrl, restUrl, restApiContext, + publicApiContext, urlBaseEditor, urlBaseWebhook, versionCli, @@ -273,5 +284,6 @@ export const useRootStore = defineStore(STORES.ROOT, () => { setDefaultLocale, setBinaryDataMode, setPushRef, + setPublicApiPath, }; });