github-mirror/n8n
1
0
Fork 0
mirror of https://github.com/n8n-io/n8n.git synced 2026-10-17 05:29:44 +02:00
Code Issues Actions 14 Packages Projects Releases Wiki Activity Graph

fix(core): Key each OAuth authorization request to its own session (#39930)

Browse Source 
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Ricardo Espinoza 2026-10-15 11:43:45 +00:00 committed by GitHub
parent 1659fa3737
commit 3c447b42a7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
24 changed files with 823 additions and 169 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
packages/cli/src/modules/oauth-server/__tests__/chat-trigger-resource.resolver.api.test.ts
View File

@ -16,6 +16,8 @@ import request from 'supertest';
import { createMember, createOwner } from '@test-integration/db/users';
import { setupTestServer } from '@test-integration/utils';
import { pendingFlowFrom } from './oauth-flow-test-utils';
import { AuthService } from '@/auth/auth.service';
import { AUTH_COOKIE_NAME } from '@/constants';
import { OAuthTokenService } from '@/modules/oauth-server/oauth-token.service';
@ -451,13 +453,9 @@ describe('consent reuse on a second visit', () => {
.get('/oauth/authorize')
.query(authorizeQuery(resourceUrl, cc1, 'state-1'));
expect(first.statusCode).toBe(302);
expect(first.headers.location).toBe('/oauth/consent');
expect(first.headers.location).toContain('/oauth/consent');
const rawSetCookie: string | string[] = first.headers['set-cookie'] ?? [];
const setCookies = Array.isArray(rawSetCookie) ? rawSetCookie : [rawSetCookie];
const sessionCookie = setCookies
.map((cookie) => cookie.split(';')[0])
.find((cookie) => cookie.startsWith('n8n-oauth-session='));
const { flowId, cookie: sessionCookie } = pendingFlowFrom(first);
expect(sessionCookie).toBeDefined();
// `/consent/approve` lives under the `/rest` prefix, unlike the root-level
@ -468,7 +466,7 @@ describe('consent reuse on a second visit', () => {
const approve = await consentAgent
.post('/consent/approve')
.send({ approved: true, scopes: [] });
.send({ approved: true, scopes: [], flow: flowId });
expect(approve.statusCode).toBe(200);
const { codeChallenge: cc2 } = await pkce();
@ -477,7 +475,7 @@ describe('consent reuse on a second visit', () => {
.query(authorizeQuery(resourceUrl, cc2, 'state-2'));
expect(second.statusCode).toBe(302);
expect(second.headers.location).not.toBe('/oauth/consent');
expect(second.headers.location).not.toContain('/oauth/consent');
expect(second.headers.location).toContain(resourceUrl);
});
@ -515,13 +513,9 @@ describe('consent reuse on a second visit', () => {
.get('/oauth/authorize')
.query(authorizeQuery(resourceUrl, codeChallenge, 'state-1'));
expect(first.statusCode).toBe(302);
expect(first.headers.location).toBe('/oauth/consent');
expect(first.headers.location).toContain('/oauth/consent');
const rawSetCookie: string | string[] = first.headers['set-cookie'] ?? [];
const setCookies = Array.isArray(rawSetCookie) ? rawSetCookie : [rawSetCookie];
const sessionCookie = setCookies
.map((cookie) => cookie.split(';')[0])
.find((cookie) => cookie.startsWith('n8n-oauth-session='));
const { flowId, cookie: sessionCookie } = pendingFlowFrom(first);
expect(sessionCookie).toBeDefined();
// The visitor now logs in (this is the "just logged in" step from the ticket),
@ -529,7 +523,7 @@ describe('consent reuse on a second visit', () => {
const consentAgent = testServer.authAgentFor(owner);
consentAgent.jar.setCookie(sessionCookie ?? '');
const details = await consentAgent.get('/consent/details');
const details = await consentAgent.get(`/consent/details?flow=${flowId}`);
// Desired: already consented, so this should signal auto-approval instead of
// requiring another manual click.

109
packages/cli/src/modules/oauth-server/__tests__/oauth-consent.controller.api.test.ts
View File

@ -11,6 +11,7 @@ import { setupTestServer } from '@test-integration/utils';
import type { OAuthClient } from '../database/entities/oauth-client.entity';
import { OAuthClientRepository } from '../database/repositories/oauth-client.repository';
import type { OAuthSessionPayload } from '../oauth-session.service';
import { sessionCookieFor, TEST_FLOW_ID } from './oauth-flow-test-utils';
const testServer = setupTestServer({ endpointGroups: ['mcp'], modules: ['oauth-server', 'mcp'] });
@ -74,8 +75,8 @@ describe('GET /rest/consent/details', () => {
const response = await testServer
.authAgentFor(owner)
.get('/consent/details')
.set('Cookie', `n8n-oauth-session=${sessionToken}`);
.get(`/consent/details?flow=${TEST_FLOW_ID}`)
.set('Cookie', sessionCookieFor(TEST_FLOW_ID, sessionToken));
expect(response.statusCode).toBe(200);
expect(response.body.data).toEqual({
@ -120,8 +121,8 @@ describe('GET /rest/consent/details', () => {
const response = await testServer
.authAgentFor(owner)
.get('/consent/details')
.set('Cookie', `n8n-oauth-session=${sessionToken}`);
.get(`/consent/details?flow=${TEST_FLOW_ID}`)
.set('Cookie', sessionCookieFor(TEST_FLOW_ID, sessionToken));
expect(response.statusCode).toBe(200);
expect(response.body.data).toEqual({
@ -153,8 +154,8 @@ describe('GET /rest/consent/details', () => {
const response = await testServer
.authAgentFor(owner)
.get('/consent/details')
.set('Cookie', `n8n-oauth-session=${sessionToken}`);
.get(`/consent/details?flow=${TEST_FLOW_ID}`)
.set('Cookie', sessionCookieFor(TEST_FLOW_ID, sessionToken));
expect(response.statusCode).toBe(422);
expect(response.body).toEqual({
@ -168,7 +169,9 @@ describe('GET /rest/consent/details', () => {
});
test('should return 400 when session cookie is missing', async () => {
const response = await testServer.authAgentFor(owner).get('/consent/details');
const response = await testServer
.authAgentFor(owner)
.get(`/consent/details?flow=${TEST_FLOW_ID}`);
expect(response.statusCode).toBeGreaterThanOrEqual(400);
expect(response.body).toEqual({
@ -180,8 +183,8 @@ describe('GET /rest/consent/details', () => {
test('should return 400 when session token is invalid', async () => {
const response = await testServer
.authAgentFor(owner)
.get('/consent/details')
.set('Cookie', 'n8n-oauth-session=invalid-token');
.get(`/consent/details?flow=${TEST_FLOW_ID}`)
.set('Cookie', sessionCookieFor(TEST_FLOW_ID, 'invalid-token'));
expect(response.statusCode).toBeGreaterThanOrEqual(400);
expect(response.body).toEqual({
@ -202,8 +205,8 @@ describe('GET /rest/consent/details', () => {
const response = await testServer
.authAgentFor(owner)
.get('/consent/details')
.set('Cookie', `n8n-oauth-session=${sessionToken}`);
.get(`/consent/details?flow=${TEST_FLOW_ID}`)
.set('Cookie', sessionCookieFor(TEST_FLOW_ID, sessionToken));
expect(response.statusCode).toBeGreaterThanOrEqual(400);
expect(response.body).toEqual({
@ -215,18 +218,18 @@ describe('GET /rest/consent/details', () => {
test('should clear session cookie on invalid token', async () => {
const response = await testServer
.authAgentFor(owner)
.get('/consent/details')
.set('Cookie', 'n8n-oauth-session=invalid-token');
.get(`/consent/details?flow=${TEST_FLOW_ID}`)
.set('Cookie', sessionCookieFor(TEST_FLOW_ID, 'invalid-token'));
expect(response.statusCode).toBeGreaterThanOrEqual(400);
const setCookieHeader = response.headers['set-cookie'];
expect(setCookieHeader).toBeDefined();
expect(setCookieHeader[0]).toContain('n8n-oauth-session=');
expect(setCookieHeader[0]).toContain(sessionCookieFor(TEST_FLOW_ID, ''));
expect(setCookieHeader[0]).toMatch(/Max-Age=0|Expires=Thu, 01 Jan 1970/);
});
test('should require authentication', async () => {
const response = await testServer.authlessAgent.get('/consent/details');
const response = await testServer.authlessAgent.get(`/consent/details?flow=${TEST_FLOW_ID}`);
expect(response.statusCode).toBe(401);
});
@ -251,16 +254,16 @@ describe('GET /rest/consent/details', () => {
const ownerResponse = await testServer
.authAgentFor(owner)
.get('/consent/details')
.set('Cookie', `n8n-oauth-session=${sessionToken}`);
.get(`/consent/details?flow=${TEST_FLOW_ID}`)
.set('Cookie', sessionCookieFor(TEST_FLOW_ID, sessionToken));
expect(ownerResponse.statusCode).toBe(200);
expect(ownerResponse.body.data.clientName).toBe('Test Client 2');
const memberResponse = await testServer
.authAgentFor(member)
.get('/consent/details')
.set('Cookie', `n8n-oauth-session=${sessionToken}`);
.get(`/consent/details?flow=${TEST_FLOW_ID}`)
.set('Cookie', sessionCookieFor(TEST_FLOW_ID, sessionToken));
expect(memberResponse.statusCode).toBe(200);
expect(memberResponse.body.data.clientName).toBe('Test Client 2');
@ -294,8 +297,8 @@ describe('POST /rest/consent/approve', () => {
const response = await testServer
.authAgentFor(owner)
.post('/consent/approve')
.set('Cookie', `n8n-oauth-session=${sessionToken}`)
.send({ approved: true, scopes: ['workflow:read'] });
.set('Cookie', sessionCookieFor(TEST_FLOW_ID, sessionToken))
.send({ approved: true, scopes: ['workflow:read'], flow: TEST_FLOW_ID });
expect(response.statusCode).toBe(200);
expect(response.body.data).toEqual({
@ -316,8 +319,8 @@ describe('POST /rest/consent/approve', () => {
const response = await testServer
.authAgentFor(owner)
.post('/consent/approve')
.set('Cookie', `n8n-oauth-session=${sessionToken}`)
.send({ approved: true, scopes: ['workflow:read', 'workflow:read'] });
.set('Cookie', sessionCookieFor(TEST_FLOW_ID, sessionToken))
.send({ approved: true, scopes: ['workflow:read', 'workflow:read'], flow: TEST_FLOW_ID });
expect(response.statusCode).toBe(400);
});
@ -326,8 +329,8 @@ describe('POST /rest/consent/approve', () => {
const response = await testServer
.authAgentFor(owner)
.post('/consent/approve')
.set('Cookie', `n8n-oauth-session=${sessionToken}`)
.send({ approved: false });
.set('Cookie', sessionCookieFor(TEST_FLOW_ID, sessionToken))
.send({ approved: false, flow: TEST_FLOW_ID });
expect(response.statusCode).toBe(200);
expect(response.body.data).toEqual({
@ -348,14 +351,14 @@ describe('POST /rest/consent/approve', () => {
const response = await testServer
.authAgentFor(owner)
.post('/consent/approve')
.set('Cookie', `n8n-oauth-session=${sessionToken}`)
.send({ approved: true, scopes: ['workflow:read'] });
.set('Cookie', sessionCookieFor(TEST_FLOW_ID, sessionToken))
.send({ approved: true, scopes: ['workflow:read'], flow: TEST_FLOW_ID });
expect(response.statusCode).toBe(200);
const setCookieHeader = response.headers['set-cookie'];
expect(setCookieHeader).toBeDefined();
expect(setCookieHeader[0]).toContain('n8n-oauth-session=');
expect(setCookieHeader[0]).toContain(sessionCookieFor(TEST_FLOW_ID, ''));
expect(setCookieHeader[0]).toMatch(/Max-Age=0|Expires=Thu, 01 Jan 1970/);
});
@ -363,8 +366,8 @@ describe('POST /rest/consent/approve', () => {
const response = await testServer
.authAgentFor(owner)
.post('/consent/approve')
.set('Cookie', `n8n-oauth-session=${sessionToken}`)
.send({});
.set('Cookie', sessionCookieFor(TEST_FLOW_ID, sessionToken))
.send({ flow: TEST_FLOW_ID });
expect(response.statusCode).toBeGreaterThanOrEqual(400);
expect(response.body).toMatchObject({
@ -378,8 +381,8 @@ describe('POST /rest/consent/approve', () => {
const response = await testServer
.authAgentFor(owner)
.post('/consent/approve')
.set('Cookie', `n8n-oauth-session=${sessionToken}`)
.send({ approved: 'yes' });
.set('Cookie', sessionCookieFor(TEST_FLOW_ID, sessionToken))
.send({ approved: 'yes', flow: TEST_FLOW_ID });
expect(response.statusCode).toBeGreaterThanOrEqual(400);
expect(response.body).toMatchObject({
@ -394,7 +397,7 @@ describe('POST /rest/consent/approve', () => {
const response = await testServer
.authAgentFor(owner)
.post('/consent/approve')
.send({ approved: true, scopes: ['workflow:read'] });
.send({ approved: true, scopes: ['workflow:read'], flow: TEST_FLOW_ID });
expect(response.statusCode).toBeGreaterThanOrEqual(400);
expect(response.body).toEqual({
@ -407,8 +410,8 @@ describe('POST /rest/consent/approve', () => {
const response = await testServer
.authAgentFor(owner)
.post('/consent/approve')
.set('Cookie', 'n8n-oauth-session=invalid-token')
.send({ approved: true, scopes: ['workflow:read'] });
.set('Cookie', sessionCookieFor(TEST_FLOW_ID, 'invalid-token'))
.send({ approved: true, scopes: ['workflow:read'], flow: TEST_FLOW_ID });
expect(response.statusCode).toBeGreaterThanOrEqual(400);
expect(response.body.status).toBe('error');
@ -418,20 +421,20 @@ describe('POST /rest/consent/approve', () => {
const response = await testServer
.authAgentFor(owner)
.post('/consent/approve')
.set('Cookie', 'n8n-oauth-session=invalid-token')
.send({ approved: true, scopes: ['workflow:read'] });
.set('Cookie', sessionCookieFor(TEST_FLOW_ID, 'invalid-token'))
.send({ approved: true, scopes: ['workflow:read'], flow: TEST_FLOW_ID });
const setCookieHeader = response.headers['set-cookie'];
expect(setCookieHeader).toBeDefined();
expect(setCookieHeader[0]).toContain('n8n-oauth-session=');
expect(setCookieHeader[0]).toContain(sessionCookieFor(TEST_FLOW_ID, ''));
expect(setCookieHeader[0]).toMatch(/Max-Age=0|Expires=Thu, 01 Jan 1970/);
});
test('should require authentication', async () => {
const response = await testServer.authlessAgent
.post('/consent/approve')
.set('Cookie', `n8n-oauth-session=${sessionToken}`)
.send({ approved: true, scopes: ['workflow:read'] });
.set('Cookie', sessionCookieFor(TEST_FLOW_ID, sessionToken))
.send({ approved: true, scopes: ['workflow:read'], flow: TEST_FLOW_ID });
expect(response.statusCode).toBe(401);
});
@ -440,8 +443,8 @@ describe('POST /rest/consent/approve', () => {
const response = await testServer
.authAgentFor(owner)
.post('/consent/approve')
.set('Cookie', `n8n-oauth-session=${sessionToken}`)
.send({ approved: true, scopes: ['workflow:read'] });
.set('Cookie', sessionCookieFor(TEST_FLOW_ID, sessionToken))
.send({ approved: true, scopes: ['workflow:read'], flow: TEST_FLOW_ID });
expect(response.statusCode).toBe(200);
@ -463,8 +466,8 @@ describe('POST /rest/consent/approve', () => {
const response = await testServer
.authAgentFor(owner)
.post('/consent/approve')
.set('Cookie', `n8n-oauth-session=${sessionToken}`)
.send({ approved: false });
.set('Cookie', sessionCookieFor(TEST_FLOW_ID, sessionToken))
.send({ approved: false, flow: TEST_FLOW_ID });
expect(response.statusCode).toBe(200);
@ -483,8 +486,8 @@ describe('POST /rest/consent/approve', () => {
const ownerResponse = await testServer
.authAgentFor(owner)
.post('/consent/approve')
.set('Cookie', `n8n-oauth-session=${sessionToken}`)
.send({ approved: true, scopes: ['workflow:read'] });
.set('Cookie', sessionCookieFor(TEST_FLOW_ID, sessionToken))
.send({ approved: true, scopes: ['workflow:read'], flow: TEST_FLOW_ID });
expect(ownerResponse.statusCode).toBe(200);
expect(ownerResponse.body.data.redirectUrl).toContain('code=');
@ -499,8 +502,8 @@ describe('POST /rest/consent/approve', () => {
const memberResponse = await testServer
.authAgentFor(member)
.post('/consent/approve')
.set('Cookie', `n8n-oauth-session=${newSessionToken}`)
.send({ approved: false });
.set('Cookie', sessionCookieFor(TEST_FLOW_ID, newSessionToken))
.send({ approved: false, flow: TEST_FLOW_ID });
expect(memberResponse.statusCode).toBe(200);
expect(memberResponse.body.data.redirectUrl).toContain('error=access_denied');
@ -528,8 +531,8 @@ describe('Consent Flow - End-to-End', () => {
const detailsResponse = await testServer
.authAgentFor(owner)
.get('/consent/details')
.set('Cookie', `n8n-oauth-session=${sessionToken}`);
.get(`/consent/details?flow=${TEST_FLOW_ID}`)
.set('Cookie', sessionCookieFor(TEST_FLOW_ID, sessionToken));
expect(detailsResponse.statusCode).toBe(200);
expect(detailsResponse.body.data.clientName).toBe('End-to-End Test Client');
@ -537,8 +540,8 @@ describe('Consent Flow - End-to-End', () => {
const approvalResponse = await testServer
.authAgentFor(owner)
.post('/consent/approve')
.set('Cookie', `n8n-oauth-session=${sessionToken}`)
.send({ approved: true, scopes: ['workflow:read'] });
.set('Cookie', sessionCookieFor(TEST_FLOW_ID, sessionToken))
.send({ approved: true, scopes: ['workflow:read'], flow: TEST_FLOW_ID });
expect(approvalResponse.statusCode).toBe(200);
expect(approvalResponse.body.data.status).toBe('success');
@ -547,7 +550,7 @@ describe('Consent Flow - End-to-End', () => {
const setCookieHeader = approvalResponse.headers['set-cookie'];
expect(setCookieHeader).toBeDefined();
expect(setCookieHeader[0]).toContain('n8n-oauth-session=');
expect(setCookieHeader[0]).toContain(sessionCookieFor(TEST_FLOW_ID, ''));
expect(setCookieHeader[0]).toMatch(/Max-Age=0|Expires=Thu, 01 Jan 1970/);
});
});

28
packages/cli/src/modules/oauth-server/__tests__/oauth-flow-test-utils.ts Normal file
View File

@ -0,0 +1,28 @@
import { OAUTH_SESSION_COOKIE_PREFIX } from '../oauth-session.service';
type ResponseWithHeaders = { headers: Record<string, string | string[] | undefined> };
/**
* Read the pending authorization flow out of an /authorize redirect: the flow
* id it sends the browser to the consent screen with, and the session cookie it
* set for that flow.
*/
export const pendingFlowFrom = (response: ResponseWithHeaders) => {
const location = String(response.headers.location ?? '');
const flowId = new URL(location, 'http://localhost').searchParams.get('flow') ?? '';
const rawSetCookie = response.headers['set-cookie'] ?? [];
const setCookies = Array.isArray(rawSetCookie) ? rawSetCookie : [rawSetCookie];
const cookie = setCookies
.map((setCookie) => setCookie.split(';')[0])
.find((setCookie) => setCookie.startsWith(`${OAUTH_SESSION_COOKIE_PREFIX}${flowId}=`));
return { flowId, cookie };
};
/** Cookie header carrying one flow's session token. */
export const sessionCookieFor = (flowId: string, sessionToken: string) =>
`${OAUTH_SESSION_COOKIE_PREFIX}${flowId}=${sessionToken}`;
/** A well-formed flow id for tests that mint their own session instead of calling /authorize. */
export const TEST_FLOW_ID = 'ktest00-0123456789abcdef01234567';

19
packages/cli/src/modules/oauth-server/__tests__/oauth-server.service.test.ts
View File

@ -31,6 +31,8 @@ import { OAuthTokenService } from '../oauth-token.service';
const SUPPORTED_SCOPES = ['tool:listWorkflows', 'tool:getWorkflowDetails'];
const TEST_RESOURCE_URL = 'https://n8n.example.com/mcp-server/http';
/** Flow id the mocked session service hands back for each pending authorization. */
const FLOW_ID = 'ktest00-0123456789abcdef01234567';
let logger: Mocked<Logger>;
let oauthSessionService: Mocked<OAuthSessionService>;
@ -100,6 +102,7 @@ describe('OAuthServerService', () => {
vi.clearAllMocks();
getAllowedRedirectUris.mockResolvedValue([]);
isResourceAvailable.mockResolvedValue(true);
oauthSessionService.createSession.mockReturnValue(FLOW_ID);
});
describe('clientsStore', () => {
@ -675,7 +678,7 @@ describe('OAuthServerService', () => {
state: 'state-xyz',
resource: 'https://n8n.example.com/mcp-server/http',
});
expect(res.redirect).toHaveBeenCalledWith('/oauth/consent');
expect(res.redirect).toHaveBeenCalledWith(`/oauth/consent?flow=${FLOW_ID}`);
});
it('should reject with invalid_target when the named resource is unavailable', async () => {
@ -795,7 +798,7 @@ describe('OAuthServerService', () => {
await service.authorize(client, params, res);
expect(oauthSessionService.createSession).toHaveBeenCalled();
expect(res.redirect).toHaveBeenCalledWith('/oauth/consent');
expect(res.redirect).toHaveBeenCalledWith(`/oauth/consent?flow=${FLOW_ID}`);
});
});
@ -902,7 +905,7 @@ describe('OAuthServerService', () => {
res,
expect.objectContaining({ redirectUri: 'http://localhost:52680/callback' }),
);
expect(res.redirect).toHaveBeenCalledWith('/oauth/consent');
expect(res.redirect).toHaveBeenCalledWith(`/oauth/consent?flow=${FLOW_ID}`);
});
it('should reject a loopback redirect URI whose path differs from the allowlist entry', async () => {
@ -997,7 +1000,7 @@ describe('OAuthServerService', () => {
state: 'state-xyz',
resource: undefined,
});
expect(res.redirect).toHaveBeenCalledWith('/oauth/consent');
expect(res.redirect).toHaveBeenCalledWith(`/oauth/consent?flow=${FLOW_ID}`);
expect(logger.warn).not.toHaveBeenCalled();
});
@ -1027,7 +1030,7 @@ describe('OAuthServerService', () => {
await service.authorize(client, params, res);
expect(oauthSessionService.createSession).not.toHaveBeenCalled();
expect(oauthSessionService.clearSession).toHaveBeenCalledWith(res);
expect(oauthSessionService.clearSession).not.toHaveBeenCalled();
expect(res.status).toHaveBeenCalledWith(400);
expect(res.json).toHaveBeenCalledWith({
error: 'invalid_target',
@ -1067,10 +1070,10 @@ describe('OAuthServerService', () => {
state: null,
resource: 'https://n8n.example.com/mcp-server/http',
});
expect(res.redirect).toHaveBeenCalledWith('/oauth/consent');
expect(res.redirect).toHaveBeenCalledWith(`/oauth/consent?flow=${FLOW_ID}`);
});
it('should handle errors and clear session', async () => {
it('should handle errors without touching other pending flows', async () => {
const client = {
client_id: 'client-123',
client_name: 'Test Client',
@ -1105,7 +1108,7 @@ describe('OAuthServerService', () => {
error,
clientId: 'client-123',
});
expect(oauthSessionService.clearSession).toHaveBeenCalledWith(res);
expect(oauthSessionService.clearSession).not.toHaveBeenCalled();
expect(res.status).toHaveBeenCalledWith(500);
expect(res.json).toHaveBeenCalledWith({
error: 'server_error',

168
packages/cli/src/modules/oauth-server/__tests__/oauth-session.service.test.ts
View File

@ -1,18 +1,25 @@
import type { GlobalConfig } from '@n8n/config';
import type { Request, Response } from 'express';
import { JsonWebTokenError, TokenExpiredError } from 'jsonwebtoken';
import { mock } from 'vitest-mock-extended';
import { ZodError } from 'zod';
import { JwtService } from '@n8n/backend-services';
import { OAuthSessionService, type OAuthSessionPayload } from '../oauth-session.service';
import {
OAUTH_SESSION_COOKIE_PREFIX,
OAuthSessionService,
type OAuthSessionPayload,
} from '../oauth-session.service';
describe('OAuthSessionService', () => {
const globalConfig = mock<GlobalConfig>({
userManagement: { jwtSecret: 'random-secret' },
auth: { cookie: { secure: true, samesite: 'lax' } },
endpoints: { rest: 'rest' },
});
const jwtService = new JwtService(mock(), globalConfig, mock());
const oauthSessionService = new OAuthSessionService(jwtService);
const oauthSessionService = new OAuthSessionService(jwtService, globalConfig);
const sessionPayload: OAuthSessionPayload = {
clientId: 'client-id',
@ -21,6 +28,163 @@ describe('OAuthSessionService', () => {
state: 'client-state',
};
/** A response whose request carries the given pending flow cookies. */
const responseWith = (cookies: Record<string, string> = {}) =>
mock<Response>({ req: mock<Request>({ cookies }) });
describe('createSession', () => {
it('should store the session under a flow id of its own', () => {
const res = responseWith();
const flowId = oauthSessionService.createSession(res, sessionPayload);
expect(flowId).toMatch(/^[0-9a-z]{6,12}-[0-9a-f]{24}$/);
// Exact, so a `path` narrower than the default cannot be added back:
// it would hide the pending flows from /authorize, where they are
// evicted, or from the consent endpoints, where they are read.
expect(res.cookie).toHaveBeenCalledWith(
`${OAUTH_SESSION_COOKIE_PREFIX}${flowId}`,
expect.any(String),
{
httpOnly: true,
secure: true,
sameSite: 'lax',
maxAge: 10 * 60 * 1000,
},
);
});
// `strict` would drop the cookie when another site hands the browser to
// /authorize, which is how every flow starts.
it('should relax a strict sameSite setting to lax', () => {
globalConfig.auth.cookie.samesite = 'strict';
const res = responseWith();
try {
oauthSessionService.createSession(res, sessionPayload);
} finally {
globalConfig.auth.cookie.samesite = 'lax';
}
expect(res.cookie).toHaveBeenCalledWith(
expect.any(String),
expect.any(String),
expect.objectContaining({ sameSite: 'lax' }),
);
});
it('should pass a none sameSite setting through', () => {
globalConfig.auth.cookie.samesite = 'none';
const res = responseWith();
try {
oauthSessionService.createSession(res, sessionPayload);
} finally {
globalConfig.auth.cookie.samesite = 'lax';
}
expect(res.cookie).toHaveBeenCalledWith(
expect.any(String),
expect.any(String),
expect.objectContaining({ sameSite: 'none' }),
);
});
it('should give two authorization requests separate cookies', () => {
const first = responseWith();
const firstFlowId = oauthSessionService.createSession(first, sessionPayload);
// The browser now holds the first flow's cookie and starts a second flow.
const second = responseWith({
[`${OAUTH_SESSION_COOKIE_PREFIX}${firstFlowId}`]: 'first-token',
});
const secondFlowId = oauthSessionService.createSession(second, {
...sessionPayload,
codeChallenge: 'second-challenge',
});
expect(secondFlowId).not.toBe(firstFlowId);
expect(second.clearCookie).not.toHaveBeenCalled();
expect(second.cookie).toHaveBeenCalledWith(
`${OAUTH_SESSION_COOKIE_PREFIX}${secondFlowId}`,
expect.any(String),
expect.anything(),
);
});
it('should evict the oldest pending flow once the cap is reached', () => {
// Three already pending, so the new one takes the oldest one's slot. Flow ids
// carry a timestamp prefix, so sorting the names sorts them oldest first.
const pending = ['a00000', 'b00000', 'c00000'].map(
(prefix) => `${OAUTH_SESSION_COOKIE_PREFIX}${prefix}-${'0'.repeat(24)}`,
);
const res = responseWith(Object.fromEntries(pending.map((name) => [name, 'token'])));
oauthSessionService.createSession(res, sessionPayload);
expect(res.clearCookie).toHaveBeenCalledTimes(1);
expect(res.clearCookie).toHaveBeenCalledWith(pending[0], expect.anything());
});
it('should leave cookies that are not pending flows alone', () => {
const res = responseWith({ 'n8n-auth': 'auth-token' });
oauthSessionService.createSession(res, sessionPayload);
expect(res.clearCookie).not.toHaveBeenCalled();
});
});
describe('getSessionToken', () => {
it('should return the token of the named flow', () => {
const flowId = `ktest00-${'a'.repeat(24)}`;
const cookies = { [`${OAUTH_SESSION_COOKIE_PREFIX}${flowId}`]: 'session-token' };
expect(oauthSessionService.getSessionToken(cookies, flowId)).toBe('session-token');
});
it('should not return the token of a different flow', () => {
const cookies = { [`${OAUTH_SESSION_COOKIE_PREFIX}ktest00-${'a'.repeat(24)}`]: 'other' };
expect(
oauthSessionService.getSessionToken(cookies, `ktest01-${'b'.repeat(24)}`),
).toBeUndefined();
});
// Keyed by the malformed id, so only the guard can make these pass: an
// implementation that looked the name up regardless would return the token.
it.each(['', 'not-a-flow-id', '../n8n-auth', '__proto__'])(
'should reject the malformed flow id %s',
(flowId) => {
const cookies = { [`${OAUTH_SESSION_COOKIE_PREFIX}${flowId}`]: 'stale-token' };
expect(oauthSessionService.getSessionToken(cookies, flowId)).toBeUndefined();
},
);
});
describe('clearSession', () => {
it('should clear only the named flow', () => {
const res = responseWith();
const flowId = `ktest00-${'a'.repeat(24)}`;
oauthSessionService.clearSession(res, flowId);
expect(res.clearCookie).toHaveBeenCalledWith(
`${OAUTH_SESSION_COOKIE_PREFIX}${flowId}`,
expect.anything(),
);
});
it('should do nothing for a malformed flow id', () => {
const res = responseWith();
oauthSessionService.clearSession(res, 'not-a-flow-id');
expect(res.clearCookie).not.toHaveBeenCalled();
});
});
describe('verifySession', () => {
it('should return the payload of an authorization session token', () => {
const token = jwtService.sign('oauthSession', sessionPayload, { expiresIn: '10m' });

166
packages/cli/src/modules/oauth-server/__tests__/oauth.controller.api.test.ts
View File

@ -3,15 +3,21 @@ import { GlobalConfig } from '@n8n/config';
import type { User } from '@n8n/db';
import { ControllerRegistryMetadata, type Controller } from '@n8n/decorators';
import { Container } from '@n8n/di';
import { randomUUID } from 'node:crypto';
import request from 'supertest';
import { AuthService } from '@/auth/auth.service';
import { AUTH_COOKIE_NAME } from '@/constants';
import { McpSettingsService } from '@/modules/mcp/mcp.settings.service';
import { UrlService } from '@n8n/backend-services';
import { ProtectedResourceRegistry } from '@n8n/backend-services/auth';
import { createOwner } from '@test-integration/db/users';
import { setupTestServer } from '@test-integration/utils';
import { OAuthClientRepository } from '../database/repositories/oauth-client.repository';
import { OAuthServerConfig } from '../oauth-server.config';
import type { OAuthController as OAuthControllerClass } from '../oauth.controller';
import { pendingFlowFrom } from './oauth-flow-test-utils';
const testServer = setupTestServer({ modules: ['oauth-server', 'mcp'], endpointGroups: ['mcp'] });
@ -715,13 +721,11 @@ describe('Full authorization-code flow (PKCE)', () => {
state: 'flow-state',
});
expect(authorizeResponse.statusCode).toBe(302);
expect(authorizeResponse.headers.location).toBe('/oauth/consent');
const rawSetCookie: string | string[] = authorizeResponse.headers['set-cookie'] ?? [];
const setCookies = Array.isArray(rawSetCookie) ? rawSetCookie : [rawSetCookie];
const sessionCookie = setCookies
.map((cookie) => cookie.split(';')[0])
.find((cookie) => cookie.startsWith('n8n-oauth-session='));
// The redirect names this authorization request, so the consent screen can
// ask for the one it was opened for.
const { flowId, cookie: sessionCookie } = pendingFlowFrom(authorizeResponse);
expect(authorizeResponse.headers.location).toBe(`/oauth/consent?flow=${flowId}`);
expect(sessionCookie).toBeDefined();
// 3. Consent approval as an authenticated user.
@ -739,7 +743,7 @@ describe('Full authorization-code flow (PKCE)', () => {
authAgent.jar.setCookie(sessionCookie ?? '');
const consentResponse = await authAgent
.post('/consent/approve')
.send({ approved: true, scopes: grantedScopes });
.send({ approved: true, scopes: grantedScopes, flow: flowId });
expect(consentResponse.statusCode).toBe(200);
const redirectUrl = new URL(consentResponse.body.data.redirectUrl);
@ -783,6 +787,154 @@ describe('Full authorization-code flow (PKCE)', () => {
expect(refreshResponse.body.access_token).toEqual(expect.any(String));
expect(refreshResponse.body.access_token).not.toBe(tokenResponse.body.access_token);
});
// A client that starts two authorization requests from one browser (a desktop
// app that launches its bridge twice, or a user who retries) used to lose the
// first one: both requests shared a single session cookie, so the approval
// minted a code bound to the second request's PKCE challenge while the process
// waiting on the callback held the first request's verifier.
test('should keep two authorization requests from one browser independent', async () => {
const { createHash, randomBytes } = await import('node:crypto');
const pkcePair = () => {
const verifier = randomBytes(32).toString('base64url');
return { verifier, challenge: createHash('sha256').update(verifier).digest('base64url') };
};
const first = pkcePair();
const second = pkcePair();
const registerResponse = await testServer.restlessAgent.post('/mcp-oauth/register').send({
client_name: 'Concurrent Flow Client',
redirect_uris: ['https://example.com/callback'],
grant_types: ['authorization_code'],
token_endpoint_auth_method: 'none',
});
const clientId = registerResponse.body.client_id;
// One browser, so one cookie jar for both authorization requests. Its own
// agent, because the flow cookies outlive each request and the shared
// agents are reused by every other test in this file.
const browser = request.agent(testServer.app);
const authorize = async (codeChallenge: string, state: string) => {
const response = await browser.get('/mcp-oauth/authorize').query({
client_id: clientId,
redirect_uri: 'https://example.com/callback',
response_type: 'code',
code_challenge: codeChallenge,
code_challenge_method: 'S256',
state,
});
expect(response.statusCode).toBe(302);
const flow = pendingFlowFrom(response);
expect(flow.cookie).toBeDefined();
return flow;
};
// Both requests are pending at once, each with its own flow id and cookie.
const firstFlow = await authorize(first.challenge, 'state-first');
const secondFlow = await authorize(second.challenge, 'state-second');
expect(firstFlow.flowId).not.toBe(secondFlow.flowId);
// The consent screen carries both cookies and asks for one flow by id.
const consentAgent = testServer.authAgentFor(owner);
consentAgent.jar.setCookie(firstFlow.cookie ?? '');
consentAgent.jar.setCookie(secondFlow.cookie ?? '');
const details = await consentAgent.get(`/consent/details?flow=${firstFlow.flowId}`);
expect(details.body).toMatchObject({ data: { clientId } });
// Approve the first request, granting exactly what its screen offered.
const consentResponse = await consentAgent
.post('/consent/approve')
.send({ approved: true, scopes: details.body.data.scopes, flow: firstFlow.flowId });
expect(consentResponse.body).toMatchObject({ data: { status: 'success' } });
// The code carries the first request's state, and only the first request's
// verifier redeems it.
const redirectUrl = new URL(consentResponse.body.data.redirectUrl);
expect(redirectUrl.searchParams.get('state')).toBe('state-first');
const code = redirectUrl.searchParams.get('code');
const exchange = async (codeVerifier: string) =>
await testServer.restlessAgent.post('/mcp-oauth/token').type('form').send({
grant_type: 'authorization_code',
code: code!,
client_id: clientId,
code_verifier: codeVerifier,
redirect_uri: 'https://example.com/callback',
});
const wrongVerifier = await exchange(second.verifier);
expect(wrongVerifier.statusCode).toBe(400);
expect(wrongVerifier.body.error).toBe('invalid_grant');
const rightVerifier = await exchange(first.verifier);
expect(rightVerifier.body).toMatchObject({ access_token: expect.any(String) });
// The second request survived the first one's approval and is still decidable.
const secondConsent = await consentAgent
.post('/consent/approve')
.send({ approved: true, scopes: details.body.data.scopes, flow: secondFlow.flowId });
expect(secondConsent.body).toMatchObject({ data: { status: 'success' } });
expect(new URL(secondConsent.body.data.redirectUrl).searchParams.get('state')).toBe(
'state-second',
);
});
// Eviction depends on the browser offering its pending flow cookies back to
// /authorize, which is a property of how the cookies are scoped, not of the
// service. A mock request cannot show that: it is handed the cookies it is
// asked about. So this drives a real cookie jar past the cap and reads the
// result through the consent endpoint.
test('should evict the oldest pending flow once a browser passes the cap', async () => {
// Written straight to the table: the DCR endpoint is IP rate limited, and
// this test is about pending flows rather than registration.
const clientId = `pending-flow-cap-${randomUUID()}`;
await Container.get(OAuthClientRepository).save({
id: clientId,
name: 'Pending Flow Cap Client',
redirectUris: ['https://example.com/callback'],
grantTypes: ['authorization_code'],
tokenEndpointAuthMethod: 'none',
});
// One signed-in browser: no path prefix, so it reaches both the
// /mcp-oauth/authorize routes and the /rest/consent ones out of one jar,
// offering each the cookies that request's path is entitled to.
const browser = request.agent(testServer.app);
browser.jar.setCookie(
`${AUTH_COOKIE_NAME}=${Container.get(AuthService).issueJWT(owner, owner.mfaEnabled, 'test-browser-id')}`,
);
const authorize = async (state: string) => {
const response = await browser.get('/mcp-oauth/authorize').query({
client_id: clientId,
redirect_uri: 'https://example.com/callback',
response_type: 'code',
code_challenge: 'E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM',
code_challenge_method: 'S256',
state,
});
expect(response.statusCode).toBe(302);
return pendingFlowFrom(response);
};
const flows = [];
for (const state of ['first', 'second', 'third', 'fourth']) {
flows.push(await authorize(state));
}
expect(new Set(flows.map((flow) => flow.flowId)).size).toBe(4);
// The cap is three, so the fourth request pushed the first one out and the
// browser no longer holds its cookie.
const evicted = await browser.get(`/rest/consent/details?flow=${flows[0].flowId}`);
expect(evicted.statusCode).toBe(400);
// The three newest survived, each still readable under its own flow id.
for (const flow of flows.slice(1)) {
const details = await browser.get(`/rest/consent/details?flow=${flow.flowId}`);
expect(details.body).toMatchObject({ data: { clientId } });
}
});
});
describe('Neutral /oauth/* endpoint aliases', () => {

34
packages/cli/src/modules/oauth-server/__tests__/oauth.helpers.test.ts
View File

@ -218,4 +218,38 @@ describe('OAuthHelpers', () => {
expect(OAuthHelpers.setIssuerParam('/oauth/consent', issuer)).toBe('/oauth/consent');
});
});
describe('basicAuthClientId', () => {
const basicHeader = (credentials: string) =>
`Basic ${Buffer.from(credentials, 'utf8').toString('base64')}`;
it('should return the client id from the username half', () => {
expect(OAuthHelpers.basicAuthClientId(basicHeader('client-123:s3cret'))).toBe('client-123');
});
it('should not return the secret, even when the secret holds a colon', () => {
const result = OAuthHelpers.basicAuthClientId(basicHeader('client-123:s3:cret'));
expect(result).toBe('client-123');
expect(result).not.toContain('s3');
});
it('should form-decode the client id', () => {
expect(OAuthHelpers.basicAuthClientId(basicHeader('client%20123:secret'))).toBe('client 123');
});
it('should return an id that is not form-encoded as sent', () => {
expect(OAuthHelpers.basicAuthClientId(basicHeader('client%id:secret'))).toBe('client%id');
});
it.each([
undefined,
'',
'Bearer abc123',
'Basic',
'Basic ' + Buffer.from(':s').toString('base64'),
])('should return undefined for the header %s', (header) => {
expect(OAuthHelpers.basicAuthClientId(header)).toBeUndefined();
});
});
});

13
packages/cli/src/modules/oauth-server/__tests__/workflow-mcp-trigger-authorization.api.test.ts
View File

@ -20,6 +20,7 @@ import { CacheService, UrlService, JwtService } from '@n8n/backend-services';
import { OAuthClientRepository } from '../database/repositories/oauth-client.repository';
import type { OAuthSessionPayload } from '../oauth-session.service';
import { sessionCookieFor, TEST_FLOW_ID } from './oauth-flow-test-utils';
/**
* Authorization for OAuth-protected MCP Server Triggers. A workflow whose MCP
@ -194,8 +195,8 @@ describe('consent gate: workflow name and authorization code require execute acc
const response = await testServer
.authAgentFor(memberNoAccess)
.get('/consent/details')
.set('Cookie', `n8n-oauth-session=${sessionToken}`);
.get(`/consent/details?flow=${TEST_FLOW_ID}`)
.set('Cookie', sessionCookieFor(TEST_FLOW_ID, sessionToken));
expect(JSON.stringify(response.body)).not.toContain(workflowName);
});
@ -207,8 +208,8 @@ describe('consent gate: workflow name and authorization code require execute acc
const response = await testServer
.authAgentFor(owner)
.get('/consent/details')
.set('Cookie', `n8n-oauth-session=${sessionToken}`);
.get(`/consent/details?flow=${TEST_FLOW_ID}`)
.set('Cookie', sessionCookieFor(TEST_FLOW_ID, sessionToken));
expect(response.body.data.resourceName).toBe(workflowName);
});
@ -220,8 +221,8 @@ describe('consent gate: workflow name and authorization code require execute acc
const response = await testServer
.authAgentFor(memberNoAccess)
.post('/consent/approve')
.set('Cookie', `n8n-oauth-session=${sessionToken}`)
.send({ approved: true })
.set('Cookie', sessionCookieFor(TEST_FLOW_ID, sessionToken))
.send({ approved: true, flow: TEST_FLOW_ID })
.expect(403);
expect(String(response.body.data?.redirectUrl ?? '')).not.toContain('code=');

2
packages/cli/src/modules/oauth-server/dto/approve-consent-request.dto.ts
View File

@ -11,4 +11,6 @@ export class ApproveConsentRequestDto extends Z.class({
message: 'scopes must not contain duplicates',
})
.optional(),
/** Id of the pending authorization request to decide, from the /authorize redirect. */
flow: z.string().max(64),
}) {}

45
packages/cli/src/modules/oauth-server/oauth-consent.controller.ts
View File

@ -9,6 +9,9 @@ import { OAuthConsentService } from './oauth-consent.service';
import { OAuthSessionService } from './oauth-session.service';
import { ForbiddenError } from '@n8n/errors';
/** `flow` names the pending authorization request to read, set by the /authorize redirect. */
type ConsentDetailsRequest = AuthenticatedRequest<{}, {}, {}, { flow?: string }>;
@RestController('/consent')
export class OAuthConsentController {
constructor(
@ -18,20 +21,22 @@ export class OAuthConsentController {
) {}
@Get('/details', { usesTemplates: true })
async getConsentDetails(req: AuthenticatedRequest, res: Response) {
async getConsentDetails(req: ConsentDetailsRequest, res: Response) {
const flowId = typeof req.query.flow === 'string' ? req.query.flow : '';
try {
const sessionToken = this.getAndValidateSessionToken(req, res);
const sessionToken = this.getAndValidateSessionToken(req, res, flowId);
if (!sessionToken) return;
const consentDetails = await this.consentService.getConsentDetails(sessionToken, req.user);
if (!consentDetails) {
this.sendInvalidSessionError(res, true);
this.sendInvalidSessionError(res, flowId, true);
return;
}
if (!consentDetails.ok) {
this.oauthSessionService.clearSession(res);
this.oauthSessionService.clearSession(res, flowId);
if (consentDetails.reason === 'forbidden') {
this.sendErrorResponse(
res,
@ -46,7 +51,7 @@ export class OAuthConsentController {
if (consentDetails.autoApproved) {
// The session's decision is already made — consume it, same as a manual approval.
this.oauthSessionService.clearSession(res);
this.oauthSessionService.clearSession(res, flowId);
res.json({
data: {
autoApproved: true,
@ -72,7 +77,7 @@ export class OAuthConsentController {
});
} catch (error) {
this.logger.error('Failed to get consent details', { error });
this.oauthSessionService.clearSession(res);
this.oauthSessionService.clearSession(res, flowId);
this.sendErrorResponse(res, 500, 'Failed to load authorization details');
}
}
@ -84,7 +89,7 @@ export class OAuthConsentController {
@Body payload: ApproveConsentRequestDto,
) {
try {
const sessionToken = this.getAndValidateSessionToken(req, res);
const sessionToken = this.getAndValidateSessionToken(req, res, payload.flow);
if (!sessionToken) return;
const result = await this.consentService.handleConsentDecision(
@ -94,7 +99,7 @@ export class OAuthConsentController {
payload.scopes,
);
this.oauthSessionService.clearSession(res);
this.oauthSessionService.clearSession(res, payload.flow);
res.json({
data: {
@ -104,7 +109,7 @@ export class OAuthConsentController {
});
} catch (error) {
this.logger.error('Failed to process consent', { error });
this.oauthSessionService.clearSession(res);
this.oauthSessionService.clearSession(res, payload.flow);
if (error instanceof ForbiddenError) {
this.sendErrorResponse(
res,
@ -126,17 +131,27 @@ export class OAuthConsentController {
});
}
private sendInvalidSessionError(res: Response, clearCookie = false): void {
private sendInvalidSessionError(res: Response, flowId: string, clearCookie = false): void {
if (clearCookie) {
this.oauthSessionService.clearSession(res);
this.oauthSessionService.clearSession(res, flowId);
}
this.sendErrorResponse(res, 400, 'Invalid or expired authorization session');
}
private getAndValidateSessionToken(req: AuthenticatedRequest, res: Response): string | null {
const sessionToken = this.oauthSessionService.getSessionToken(req.cookies);
/**
* Resolve the pending authorization request the caller names. An unknown
* flow id reads as an expired session: the browser only reaches this screen
* through an /authorize redirect carrying the id, and the first decision
* made on a flow consumes it.
*/
private getAndValidateSessionToken(
req: AuthenticatedRequest,
res: Response,
flowId: string,
): string | null {
const sessionToken = this.oauthSessionService.getSessionToken(req.cookies, flowId);
if (!sessionToken) {
this.sendInvalidSessionError(res);
this.sendInvalidSessionError(res, flowId);
return null;
}
@ -145,7 +160,7 @@ export class OAuthConsentController {
return sessionToken;
} catch (error) {
this.logger.debug('Invalid session token', { error });
this.sendInvalidSessionError(res, true);
this.sendInvalidSessionError(res, flowId, true);
return null;
}
}

12
packages/cli/src/modules/oauth-server/oauth-server.service.ts
View File

@ -473,9 +473,8 @@ export class OAuthServerService implements OAuthServerProvider {
return;
}
this.oauthSessionService.createSession(res, sessionPayload);
res.redirect('/oauth/consent');
const flowId = this.oauthSessionService.createSession(res, sessionPayload);
res.redirect(`/oauth/consent?flow=${flowId}`);
} catch (error) {
if (error instanceof InvalidResourceIndicatorError) {
this.logger.warn('Rejecting OAuth authorization request with invalid resource', {
@ -483,7 +482,6 @@ export class OAuthServerService implements OAuthServerProvider {
resource: error.resource,
expectedResource: error.expectedResource,
});
this.oauthSessionService.clearSession(res);
res.status(400).json({
error: 'invalid_target',
error_description: 'Invalid resource indicator',
@ -492,7 +490,6 @@ export class OAuthServerService implements OAuthServerProvider {
}
this.logger.error('Error in authorize method', { error, clientId: client.client_id });
this.oauthSessionService.clearSession(res);
res.status(500).json({ error: 'server_error', error_description: 'Internal server error' });
}
}
@ -579,6 +576,11 @@ export class OAuthServerService implements OAuthServerProvider {
});
}
this.logger.debug('OAuth token exchange succeeded', {
clientId: client.client_id,
userId: authRecord.userId,
});
return {
access_token: accessToken,
token_type: 'Bearer',

106
packages/cli/src/modules/oauth-server/oauth-session.service.ts
View File

@ -1,6 +1,8 @@
import { GlobalConfig } from '@n8n/config';
import { Time } from '@n8n/constants';
import { Service } from '@n8n/di';
import { Response } from 'express';
import type { CookieOptions, Request, Response } from 'express';
import { randomBytes } from 'node:crypto';
import { z } from 'zod';
import { JwtService } from '@n8n/backend-services';
@ -17,31 +19,56 @@ const oauthSessionPayloadSchema = z.object({
export type OAuthSessionPayload = z.infer<typeof oauthSessionPayloadSchema>;
export const OAUTH_SESSION_COOKIE_NAME = 'n8n-oauth-session';
/** Each pending authorization request owns a cookie, named by its flow id. */
export const OAUTH_SESSION_COOKIE_PREFIX = 'n8n-oauth-session-';
const SESSION_EXPIRY_MS = 10 * Time.minutes.toMilliseconds; // 10 minutes
/**
* Pending flows one browser may hold at once, which is what keeps the request
* header bounded: the consent screen is sent every pending flow's cookie.
*
* Two /authorize calls that overlap both read the same cookie snapshot, so a
* burst can hold more than this for a moment. The next /authorize sees them all
* and prunes back to the cap, so an overshoot decays instead of accumulating.
*/
const MAX_PENDING_FLOWS = 3;
/** The shape `newFlowId` mints. Checked before a flow id reaches a cookie name. */
const FLOW_ID_PATTERN = /^[0-9a-z]{6,12}-[0-9a-f]{24}$/;
/**
* Manages OAuth authorization session state using JWT-based cookies
* Stores temporary session data during the authorization flow
*/
@Service()
export class OAuthSessionService {
constructor(private readonly jwtService: JwtService) {}
constructor(
private readonly jwtService: JwtService,
private readonly globalConfig: GlobalConfig,
) {}
/**
* Create OAuth session token and set it as a cookie
* Store one authorization request under a new flow id, and return that id
* for the consent screen to ask for its own session by.
*
* One cookie per /authorize call, so a second request cannot replace the one
* a consent screen is already showing: the parameters the user approves stay
* the ones they were shown.
*/
createSession(res: Response, payload: OAuthSessionPayload): void {
createSession(res: Response, payload: OAuthSessionPayload): string {
const flowId = this.newFlowId();
const sessionToken = this.jwtService.sign('oauthSession', payload, {
expiresIn: '10m',
});
res.cookie(OAUTH_SESSION_COOKIE_NAME, sessionToken, {
httpOnly: true,
secure: process.env.NODE_ENV === 'production',
sameSite: 'lax',
this.evictOldestFlows(res, MAX_PENDING_FLOWS - 1);
res.cookie(OAUTH_SESSION_COOKIE_PREFIX + flowId, sessionToken, {
...this.cookieOptions(),
maxAge: SESSION_EXPIRY_MS,
});
return flowId;
}
/**
@ -56,17 +83,62 @@ export class OAuthSessionService {
return oauthSessionPayloadSchema.parse(payload);
}
/**
* Clear OAuth session cookie
*/
clearSession(res: Response): void {
res.clearCookie(OAUTH_SESSION_COOKIE_NAME);
/** Clear one flow's cookie, leaving the browser's other pending flows. */
clearSession(res: Response, flowId: string): void {
if (!FLOW_ID_PATTERN.test(flowId)) return;
res.clearCookie(OAUTH_SESSION_COOKIE_PREFIX + flowId, this.cookieOptions());
}
getSessionToken(cookies: Record<string, string | undefined>, flowId: string): string | undefined {
if (!FLOW_ID_PATTERN.test(flowId)) return undefined;
return cookies[OAUTH_SESSION_COOKIE_PREFIX + flowId];
}
/**
* Extract session token from request cookies
* A base36 millisecond prefix plus 96 random bits. The prefix makes the
* cookie names sort by age, which is all `evictOldestFlows` needs; the
* random half is what makes a flow id unguessable.
*/
getSessionToken(cookies: Record<string, string | undefined>): string | undefined {
return cookies[OAUTH_SESSION_COOKIE_NAME];
private newFlowId(): string {
return `${Date.now().toString(36)}-${randomBytes(12).toString('hex')}`;
}
/**
* Drop the oldest pending flows, leaving room for `keep` of them. Flow ids
* minted by one instance share a prefix length, so sorting the cookie names
* sorts the flows oldest first.
*/
private evictOldestFlows(res: Response, keep: number): void {
const pending = Object.keys(this.parsedCookies(res.req))
.filter((name) => name.startsWith(OAUTH_SESSION_COOKIE_PREFIX))
.sort();
for (const name of pending.slice(0, Math.max(0, pending.length - keep))) {
res.clearCookie(name, this.cookieOptions());
}
}
/**
* Attributes come from the instance's cookie config, the same source as the
* auth cookie, with `sameSite` clamped to a `lax` minimum because `strict`
* would drop the cookie on a hand-off into /authorize from another site.
*
* The default path is deliberate. /authorize and the consent endpoints sit
* under different roots, so scoping to either one hides the pending flows
* from the other, and `evictOldestFlows` would never see a cookie to evict.
*/
private cookieOptions(): CookieOptions {
const { secure, samesite } = this.globalConfig.auth.cookie;
return {
httpOnly: true,
secure,
sameSite: samesite === 'strict' ? 'lax' : samesite,
};
}
private parsedCookies(req: Request): Record<string, unknown> {
const cookies: unknown = req.cookies;
if (cookies === null || typeof cookies !== 'object') return {};
return { ...cookies };
}
}

36
packages/cli/src/modules/oauth-server/oauth.controller.ts
View File

@ -14,6 +14,7 @@ import {
} from '@n8n/decorators';
import { Container } from '@n8n/di';
import { AuthenticationService } from '@n8n/inbound-auth';
import { isRecord } from '@n8n/utils/is-record';
import type { Response, Request, RequestHandler, Router } from 'express';
import { UrlService } from '@n8n/backend-services';
@ -77,6 +78,40 @@ const rfc9207IssuerParam: RequestHandler = (_req, res, next) => {
next();
};
/**
* The SDK's token handler serializes every rejection (unknown client, expired
* or reused code, PKCE mismatch, ...) straight into the HTTP response without
* logging, which leaves a failed token exchange with no trace in the server
* logs at all. Wrap `res.json` so the token route reports what it rejected and
* for which client.
*/
const logTokenFailures: RequestHandler = (req, res, next) => {
const originalJson = res.json.bind(res);
res.json = (body?: unknown) => {
if (res.statusCode >= 400) {
const errorBody = isRecord(body) ? body : {};
// The SDK's handler parses the form body before it responds, so the
// request body is populated by the time this runs.
const requestBody = isRecord(req.body) ? req.body : {};
// A `client_secret_basic` request has no `client_id` field; its id is the
// Basic username.
const clientId =
typeof requestBody.client_id === 'string'
? requestBody.client_id
: OAuthHelpers.basicAuthClientId(req.headers.authorization);
logger.warn('OAuth token request failed', {
statusCode: res.statusCode,
error: errorBody.error,
errorDescription: errorBody.error_description,
grantType: requestBody.grant_type,
clientId,
});
}
return originalJson(body);
};
next();
};
// Built once and mounted under both the legacy `/mcp-oauth/*` paths (existing
// DCR clients hold them in their stored discovery metadata) and the neutral
// `/oauth/*` paths that future, non-MCP protected resources will advertise.
@ -119,6 +154,7 @@ const sharedEndpointRouters = (basePath: '/mcp-oauth' | '/oauth'): StaticRouterM
path: `${basePath}/token`,
router: tokenRouter,
skipAuth: true,
middlewares: [logTokenFailures],
ipRateLimit: createIpRateLimit(
oauthServerConfig.rateLimitToken,
5 * Time.minutes.toMilliseconds,

20
packages/cli/src/modules/oauth-server/oauth.helpers.ts
View File

@ -62,4 +62,24 @@ export class OAuthHelpers {
targetUrl.searchParams.set('iss', issuer);
return targetUrl.toString();
}
/**
* Client id from a `client_secret_basic` Authorization header, which carries
* it as the form-encoded username (RFC 6749 §2.3.1). Returns the username
* only, so the secret never reaches a caller.
*/
static basicAuthClientId(header?: string): string | undefined {
const encoded = /^Basic (\S+)$/i.exec(header ?? '')?.[1];
if (!encoded) return undefined;
const [username] = Buffer.from(encoded, 'base64').toString('utf8').split(':');
if (!username) return undefined;
try {
return decodeURIComponent(username);
} catch {
// A client that did not form-encode its id; report it as sent.
return username;
}
}
}

34
packages/cli/src/webhooks/__tests__/webhook-request-sanitizer.test.ts
View File

@ -2,7 +2,7 @@ import type { Request } from 'express';
import { mock } from 'vitest-mock-extended';
import { OIDC_NONCE_COOKIE_NAME, OIDC_STATE_COOKIE_NAME } from '@/constants';
import { OAUTH_SESSION_COOKIE_NAME } from '@/modules/oauth-server/oauth-session.service';
import { OAUTH_SESSION_COOKIE_PREFIX } from '@/modules/oauth-server/oauth-session.service';
import { OIDC_ID_TOKEN_COOKIE_NAME } from '@/modules/sso-oidc/constants';
import { OAUTH_BINDING_COOKIE_NAME } from '@/oauth/oauth-browser-binding.service';
import { sanitizeWebhookRequest } from '@/webhooks/webhook-request-sanitizer';
@ -342,7 +342,10 @@ describe('webhookRequestSanitizer', () => {
describe('cookies n8n issues for its own flows', () => {
const N8N_ISSUED_COOKIES = [
OAUTH_SESSION_COOKIE_NAME,
// One authorization flow's session cookie, named by its flow id.
`${OAUTH_SESSION_COOKIE_PREFIX}ktest00-0123456789abcdef01234567`,
// The name the authorization session used before it was keyed per flow.
'n8n-oauth-session',
OAUTH_BINDING_COOKIE_NAME,
OIDC_ID_TOKEN_COOKIE_NAME,
OIDC_STATE_COOKIE_NAME,
@ -364,6 +367,33 @@ describe('webhookRequestSanitizer', () => {
expect(mockRequest.cookies).toEqual({ 'other-cookie': 'value' });
});
// A browser holds one authorization-session cookie for each pending flow, so
// matching the prefix has to take all of them, not one known name.
it('should remove every pending authorization flow cookie', () => {
const first = `${OAUTH_SESSION_COOKIE_PREFIX}ktest00-0123456789abcdef01234567`;
const second = `${OAUTH_SESSION_COOKIE_PREFIX}ktest01-89abcdef0123456789abcdef`;
mockRequest.headers = { cookie: `${first}=a; ${second}=b; other-cookie=value` };
mockRequest.cookies = { [first]: 'a', [second]: 'b', 'other-cookie': 'value' };
sanitizeWebhookRequest(mockRequest);
expect(mockRequest.headers.cookie).toBe('other-cookie=value');
expect(mockRequest.cookies).toEqual({ 'other-cookie': 'value' });
});
it('should leave a cookie that begins with the session name but not the separator', () => {
mockRequest.headers = { cookie: 'n8n-oauth-sessions=abc123; other-cookie=value' };
mockRequest.cookies = { 'n8n-oauth-sessions': 'abc123', 'other-cookie': 'value' };
sanitizeWebhookRequest(mockRequest);
expect(mockRequest.headers.cookie).toBe('n8n-oauth-sessions=abc123; other-cookie=value');
expect(mockRequest.cookies).toEqual({
'n8n-oauth-sessions': 'abc123',
'other-cookie': 'value',
});
});
});
// The chat endpoints skip sanitizing for their own node type, so the hosted page

14
packages/cli/src/webhooks/webhook-request-sanitizer.ts
View File

@ -9,7 +9,7 @@ import {
OIDC_NONCE_COOKIE_NAME,
OIDC_STATE_COOKIE_NAME,
} from '@/constants';
import { OAUTH_SESSION_COOKIE_NAME } from '@/modules/oauth-server/oauth-session.service';
import { OAUTH_SESSION_COOKIE_PREFIX } from '@/modules/oauth-server/oauth-session.service';
import { OIDC_ID_TOKEN_COOKIE_NAME } from '@/modules/sso-oidc/constants';
import { OAUTH_BINDING_COOKIE_NAME } from '@/oauth/oauth-browser-binding.service';
@ -31,7 +31,9 @@ const BROWSER_ID_COOKIE_NAME = 'n8n-browserId';
const DISALLOWED_COOKIES = new Set([
AUTH_COOKIE_NAME,
BROWSER_ID_COOKIE_NAME,
OAUTH_SESSION_COOKIE_NAME,
// The OAuth authorization session used to be one cookie with this exact
// name. A browser can still hold one for the 10 minutes after an upgrade.
'n8n-oauth-session',
OAUTH_BINDING_COOKIE_NAME,
OIDC_ID_TOKEN_COOKIE_NAME,
OIDC_STATE_COOKIE_NAME,
@ -44,9 +46,13 @@ const DISALLOWED_COOKIES = new Set([
// The form auth cookie's name appends the workflow or execution it was minted
// for (`<prefix>-…`), so it is matched by prefix rather than listed above. The
// separator is required so an unrelated cookie that merely begins with the
// prefix (e.g. `n8n-form-authentic`) passes through untouched.
// prefix (e.g. `n8n-form-authentic`) passes through untouched. The OAuth
// authorization session appends its flow id the same way, and its prefix
// already ends in the separator.
const isDisallowedCookie = (name: string) =>
DISALLOWED_COOKIES.has(name) || name.startsWith(`${FORM_AUTH_COOKIE_PREFIX}-`);
DISALLOWED_COOKIES.has(name) ||
name.startsWith(`${FORM_AUTH_COOKIE_PREFIX}-`) ||
name.startsWith(OAUTH_SESSION_COOKIE_PREFIX);
/**
* Removes a cookie with the given name from the request header

8
packages/cli/test/integration/public-api/oauth-consent-scopes.test.ts
View File

@ -16,6 +16,10 @@ import { PublicApiProtectedResource } from '@/modules/inbound-auth-core/public-a
import { TrustedSourceDiscoveryService } from '@/modules/inbound-auth-core/trusted-source-discovery.service';
import { TrustedSourceDbStore } from '@/modules/inbound-auth-core/trusted-source.store';
import { OAuthClientRepository } from '@/modules/oauth-server/database/repositories/oauth-client.repository';
import {
sessionCookieFor,
TEST_FLOW_ID,
} from '@/modules/oauth-server/__tests__/oauth-flow-test-utils';
import type { OAuthSessionPayload } from '@/modules/oauth-server/oauth-session.service';
import { createMember, createOwner } from '@test-integration/db/users';
import { setupTestServer } from '@test-integration/utils';
@ -105,8 +109,8 @@ async function consentAndExchange(user: User, scopes: string[], requestedScopes?
const approval = await testServer
.authAgentFor(user)
.post('/consent/approve')
.set('Cookie', `n8n-oauth-session=${session}`)
.send({ approved: true, scopes });
.set('Cookie', sessionCookieFor(TEST_FLOW_ID, session))
.send({ approved: true, scopes, flow: TEST_FLOW_ID });
if (approval.statusCode !== 200) return { approval, token: undefined };
const code = new URL(approval.body.data.redirectUrl as string).searchParams.get('code');

15
packages/frontend/@n8n/rest-api-client/src/api/consent.ts
View File

@ -42,14 +42,23 @@ export interface ConsentApprovalResponse {
redirectUrl: string;
}
export async function getConsentDetails(context: IRestApiContext): Promise<ConsentDetails> {
return await makeRestApiRequest(context, 'GET', '/consent/details');
/**
* `flow` is the id the /authorize redirect put in this page's URL. It names the
* pending authorization request, so the screen reads and decides the one
* request it was opened for, even when the browser holds several.
*/
export async function getConsentDetails(
context: IRestApiContext,
flow: string,
): Promise<ConsentDetails> {
return await makeRestApiRequest(context, 'GET', '/consent/details', { flow });
}
export async function approveConsent(
context: IRestApiContext,
flow: string,
approved: boolean,
scopes?: string[],
): Promise<ConsentApprovalResponse> {
return await makeRestApiRequest(context, 'POST', '/consent/approve', { approved, scopes });
return await makeRestApiRequest(context, 'POST', '/consent/approve', { flow, approved, scopes });
}

16
packages/frontend/editor-ui/src/app/stores/consent.store.test.ts
View File

@ -19,6 +19,9 @@ vi.mock('@n8n/stores/useRootStore', () => ({
})),
}));
/** Id of the pending authorization request the screen was opened for. */
const FLOW_ID = 'ktest00-0123456789abcdef01234567';
describe('useConsentStore', () => {
let consentStore: ReturnType<typeof useConsentStore>;
@ -32,9 +35,10 @@ describe('useConsentStore', () => {
const details = { clientName: 'Test Client', clientId: 'client-1', scopes: [] };
getConsentDetails.mockResolvedValue(details);
const result = await consentStore.fetchConsentDetails();
const result = await consentStore.fetchConsentDetails(FLOW_ID);
expect(result).toEqual(details);
expect(getConsentDetails).toHaveBeenCalledWith(expect.anything(), FLOW_ID);
expect(consentStore.consentDetails).toEqual(details);
expect(consentStore.isLoading).toBe(false);
});
@ -42,7 +46,7 @@ describe('useConsentStore', () => {
it('sets a resource_unavailable error code on a 422', async () => {
getConsentDetails.mockRejectedValue(new ResponseError('gone', { httpStatusCode: 422 }));
await expect(consentStore.fetchConsentDetails()).rejects.toThrow('gone');
await expect(consentStore.fetchConsentDetails(FLOW_ID)).rejects.toThrow('gone');
expect(consentStore.errorCode).toBe('resource_unavailable');
expect(consentStore.error).toBe('gone');
@ -59,11 +63,11 @@ describe('useConsentStore', () => {
resolveStale = resolve;
}),
);
const staleCall = consentStore.fetchConsentDetails();
const staleCall = consentStore.fetchConsentDetails(FLOW_ID);
const freshDetails = { clientName: 'Fresh Client', clientId: 'client-2', scopes: [] };
getConsentDetails.mockResolvedValueOnce(freshDetails);
await consentStore.fetchConsentDetails();
await consentStore.fetchConsentDetails(FLOW_ID);
expect(consentStore.consentDetails).toEqual(freshDetails);
@ -80,11 +84,11 @@ describe('useConsentStore', () => {
rejectStale = reject;
}),
);
const staleCall = consentStore.fetchConsentDetails().catch(() => {});
const staleCall = consentStore.fetchConsentDetails(FLOW_ID).catch(() => {});
const freshDetails = { clientName: 'Fresh Client', clientId: 'client-2', scopes: [] };
getConsentDetails.mockResolvedValueOnce(freshDetails);
await consentStore.fetchConsentDetails();
await consentStore.fetchConsentDetails(FLOW_ID);
rejectStale(new Error('stale failure'));
await staleCall;

13
packages/frontend/editor-ui/src/app/stores/consent.store.ts
View File

@ -20,14 +20,14 @@ export const useConsentStore = defineStore(STORES.CONSENT, () => {
// the most recently issued one.
let latestRequestId = 0;
const fetchConsentDetails = async () => {
const fetchConsentDetails = async (flow: string) => {
const requestId = ++latestRequestId;
isLoading.value = true;
error.value = null;
errorCode.value = null;
try {
const response = await consentApi.getConsentDetails(rootStore.restApiContext);
const response = await consentApi.getConsentDetails(rootStore.restApiContext, flow);
if (requestId !== latestRequestId) return consentDetails.value;
consentDetails.value = response;
return consentDetails.value;
@ -48,12 +48,17 @@ export const useConsentStore = defineStore(STORES.CONSENT, () => {
}
};
const approveConsent = async (approved: boolean, scopes?: string[]) => {
const approveConsent = async (flow: string, approved: boolean, scopes?: string[]) => {
isLoading.value = true;
error.value = null;
try {
const response = await consentApi.approveConsent(rootStore.restApiContext, approved, scopes);
const response = await consentApi.approveConsent(
rootStore.restApiContext,
flow,
approved,
scopes,
);
return response;
} catch (err) {
error.value = err instanceof Error ? err.message : 'Failed to process consent';

23
packages/frontend/editor-ui/src/app/views/OAuthConsentView.test.ts
View File

@ -9,6 +9,14 @@ import { nextTick } from 'vue';
vi.mock('@n8n/rest-api-client/api/consent');
/** The /authorize redirect puts the pending request's flow id in the page URL. */
const FLOW_ID = 'ktest00-0123456789abcdef01234567';
vi.mock('vue-router', async (importOriginal) => ({
...(await importOriginal<typeof import('vue-router')>()),
useRoute: () => ({ query: { flow: FLOW_ID } }),
}));
const renderComponent = createComponentRenderer(OAuthConsentView);
let locationHrefSpy: ReturnType<typeof vi.spyOn>;
@ -49,6 +57,15 @@ describe('OAuthConsentView', () => {
locationHrefSpy?.mockRestore();
});
// The URL is the only place the flow id comes from, so the screen has to read
// and show the request the /authorize redirect named.
it('should load the flow named in the URL', async () => {
renderComponent();
await waitAllPromises();
expect(consentStore.fetchConsentDetails).toHaveBeenCalledWith(FLOW_ID);
});
it('should show the workflow name and hide the permission list when a resource is named', async () => {
consentStore.consentDetails = {
clientName: 'Test MCP Client',
@ -209,7 +226,7 @@ describe('OAuthConsentView', () => {
await userEvent.click(denyButton);
await waitAllPromises();
expect(consentStore.approveConsent).toHaveBeenCalledWith(false);
expect(consentStore.approveConsent).toHaveBeenCalledWith(FLOW_ID, false);
expect(window.location.href).toBe(window.BASE_PATH ?? '/');
});
@ -229,7 +246,7 @@ describe('OAuthConsentView', () => {
await userEvent.click(allowButton);
await waitAllPromises();
expect(consentStore.approveConsent).toHaveBeenCalledWith(true, undefined);
expect(consentStore.approveConsent).toHaveBeenCalledWith(FLOW_ID, true, undefined);
expect(window.location.href).toBe(redirectUrl);
});
@ -424,7 +441,7 @@ describe('OAuthConsentView', () => {
await userEvent.click(getByTestId('consent-allow-button'));
await waitAllPromises();
expect(consentStore.approveConsent).toHaveBeenCalledWith(true, [
expect(consentStore.approveConsent).toHaveBeenCalledWith(FLOW_ID, true, [
'workflow:read',
'workflow:write',
'execution:read',

40
packages/frontend/editor-ui/src/app/views/OAuthConsentView.vue
View File

@ -4,6 +4,7 @@ import { useDocumentTitle } from '@/app/composables/useDocumentTitle';
import { useI18n } from '@n8n/i18n';
import type { BaseTextKey } from '@n8n/i18n';
import { onMounted, onUnmounted, computed, ref, watch } from 'vue';
import { useRoute } from 'vue-router';
import type { ConsentDetailsPicker } from '@n8n/rest-api-client/api/consent';
import {
N8nButton,
@ -23,6 +24,14 @@ import ScopesSelector from '@/app/components/scopes/ScopesSelector.vue';
const consentStore = useConsentStore();
const route = useRoute();
// Id of the authorization request this screen was opened for, put in the URL by
// the /authorize redirect. Without it the server has no session to show.
const flowId = computed(() => (typeof route.query.flow === 'string' ? route.query.flow : ''));
// The flow the details on screen belong to. Decisions name this one, not the
// current URL, so the request the user approves is the request they were shown.
const shownFlowId = ref('');
const i18n = useI18n();
const documentTitle = useDocumentTitle();
const toast = useToast();
@ -115,6 +124,7 @@ watch(
const handleAllow = async () => {
try {
const response = await consentStore.approveConsent(
shownFlowId.value,
true,
hasScopes.value ? selectedScopes.value : undefined,
);
@ -133,7 +143,7 @@ const handleAllow = async () => {
const handleDeny = async () => {
try {
await consentStore.approveConsent(false);
await consentStore.approveConsent(shownFlowId.value, false);
telemetry.track('User denied MCP consent', {
client_name: clientDetails.value?.clientName,
});
@ -149,11 +159,11 @@ const handleClose = () => {
window.location.href = window.BASE_PATH ?? '/';
};
onMounted(async () => {
documentTitle.set(i18n.baseText('oauth.consentView.title'));
const loadDetails = async (flow: string) => {
try {
const details = await consentStore.fetchConsentDetails();
if (!isActive) return;
const details = await consentStore.fetchConsentDetails(flow);
if (!isActive || flow !== flowId.value) return;
shownFlowId.value = flow;
detailsResolved.value = true;
if (details?.autoApproved && details.redirectUrl) {
autoApprovedRedirect.value = true;
@ -165,10 +175,28 @@ onMounted(async () => {
available_scopes_count: availableScopes.value.length,
});
} catch (err) {
if (!isActive) return;
if (!isActive || flow !== flowId.value) return;
shownFlowId.value = flow;
toast.showError(err, i18n.baseText('oauth.consentView.error.fetchDetails'));
}
};
onMounted(() => {
documentTitle.set(i18n.baseText('oauth.consentView.title'));
});
// A second /authorize can land on this route with a different flow while the
// component stays mounted, which would leave one request on screen and another
// under the buttons. Reload whenever the id changes.
watch(
flowId,
async (flow) => {
detailsResolved.value = false;
shownFlowId.value = '';
await loadDetails(flow);
},
{ immediate: true },
);
</script>
<template>

44
packages/quality/testing/playwright/services/mcp-oauth-api-helper.ts
View File

@ -58,6 +58,13 @@ export interface AuthorizationFlowResult {
* the authorize redirect. Both live in this request context's cookie jar.
*/
export class McpOAuthApiHelper {
/**
* Flow id of the authorization request the last `authorize` call started.
* Each request has its own session cookie, so the consent calls have to name
* the one they decide.
*/
private pendingFlowId = '';
constructor(private readonly api: ApiHelpers) {}
/** Generates a PKCE verifier and its S256 challenge (RFC 7636). */
@ -128,7 +135,8 @@ export class McpOAuthApiHelper {
/**
* Starts the authorization flow. Does not follow the redirect so tests can
* assert on it. The OAuth session cookie from the response is stored in the
* request context's cookie jar for the subsequent consent calls.
* request context's cookie jar, and the flow id from the redirect is kept
* for the subsequent consent calls.
*/
async authorize(params: {
clientId: string;
@ -138,12 +146,20 @@ export class McpOAuthApiHelper {
resource?: string;
basePath?: OAuthEndpointBasePath;
}): Promise<APIResponse> {
return await this.api.request.get(this.buildAuthorizeUrl(params), { maxRedirects: 0 });
const response = await this.api.request.get(this.buildAuthorizeUrl(params), {
maxRedirects: 0,
});
const location = response.headers().location ?? '';
this.pendingFlowId = new URL(location, 'http://localhost').searchParams.get('flow') ?? '';
return response;
}
/** Requires a signed-in user and a pending OAuth session (see authorize). */
async getConsentDetails(): Promise<APIResponse> {
return await this.api.request.get('/rest/consent/details');
/**
* Requires a signed-in user and a pending OAuth session (see authorize).
* Defaults to the flow the last `authorize` call started.
*/
async getConsentDetails(flow = this.pendingFlowId): Promise<APIResponse> {
return await this.api.request.get(`/rest/consent/details?flow=${flow}`);
}
/**
@ -151,10 +167,14 @@ export class McpOAuthApiHelper {
* Approvals must grant at least one scope; when none are given, everything
* the consent details offer is granted — mirroring the consent UI default.
*/
async approveConsent(approved: boolean, scopes?: string[]): Promise<APIResponse> {
async approveConsent(
approved: boolean,
scopes?: string[],
flow = this.pendingFlowId,
): Promise<APIResponse> {
let grantedScopes = scopes;
if (approved && !grantedScopes) {
const details = await this.getConsentDetails();
const details = await this.getConsentDetails(flow);
if (details.ok()) {
const body = (await details.json()) as { data: { scopes?: string[] } };
const available = body.data.scopes ?? [];
@ -162,7 +182,7 @@ export class McpOAuthApiHelper {
}
}
return await this.api.request.post('/rest/consent/approve', {
data: { approved, ...(grantedScopes && { scopes: grantedScopes }) },
data: { approved, flow, ...(grantedScopes && { scopes: grantedScopes }) },
});
}
@ -170,8 +190,12 @@ export class McpOAuthApiHelper {
* Approves or denies the pending consent and returns the redirect URL the
* client would be sent back to (carrying either the code or the error).
*/
async submitConsentOrFail(approved: boolean, scopes?: string[]): Promise<URL> {
const response = await this.approveConsent(approved, scopes);
async submitConsentOrFail(
approved: boolean,
scopes?: string[],
flow = this.pendingFlowId,
): Promise<URL> {
const response = await this.approveConsent(approved, scopes, flow);
if (!response.ok()) {
throw new TestError(
`Failed to submit consent: ${response.status()} ${await response.text()}`,

3
packages/quality/testing/playwright/tests/e2e/mcp/mcp-oauth.spec.ts
View File

@ -106,7 +106,8 @@ test.describe(
state,
});
expect(authorizeResponse.status()).toBe(302);
expect(authorizeResponse.headers().location).toBe('/oauth/consent');
// The redirect names the pending authorization request it just stored.
expect(authorizeResponse.headers().location).toMatch(/^\/oauth\/consent\?flow=/);
// The consent page can resolve the pending session to the client name
const detailsResponse = await api.mcpOauth.getConsentDetails();