From cf1a6fa18cc96ea2b1be8307edce8f00b28b6163 Mon Sep 17 00:00:00 2001 From: Andreas Fitzek Date: Tue, 26 May 2026 17:09:07 +0200 Subject: [PATCH] fix(core): Allow dynamic credential OAuth callbacks without skip-auth env var (#31103) --- .../oauth/oauth1-credential.controller.ts | 8 +-- .../oauth/oauth2-credential.controller.ts | 4 +- .../controllers/oauth/oauth1.api.test.ts | 62 +++++++++++++++++++ .../controllers/oauth/oauth2.api.test.ts | 51 ++++++++++++++- 4 files changed, 115 insertions(+), 10 deletions(-) diff --git a/packages/cli/src/controllers/oauth/oauth1-credential.controller.ts b/packages/cli/src/controllers/oauth/oauth1-credential.controller.ts index 3fb9cd92329..b8c8d5bacca 100644 --- a/packages/cli/src/controllers/oauth/oauth1-credential.controller.ts +++ b/packages/cli/src/controllers/oauth/oauth1-credential.controller.ts @@ -6,11 +6,7 @@ import { ensureError, jsonStringify } from 'n8n-workflow'; import { OAuthRequest } from '@/requests'; -import { - OauthService, - skipAuthOnOAuthCallback, - type OAuth1CredentialData, -} from '@/oauth/oauth.service'; +import { OauthService, type OAuth1CredentialData } from '@/oauth/oauth.service'; @RestController('/oauth1-credential') export class OAuth1CredentialController { @@ -35,7 +31,7 @@ export class OAuth1CredentialController { } /** Verify and store app code. Generate access tokens and store for respective credential */ - @Get('/callback', { usesTemplates: true, skipAuth: skipAuthOnOAuthCallback }) + @Get('/callback', { usesTemplates: true, allowUnauthenticated: true }) async handleCallback(req: OAuthRequest.OAuth1Credential.Callback, res: Response) { try { const { oauth_verifier, oauth_token, state: encodedState } = req.query; diff --git a/packages/cli/src/controllers/oauth/oauth2-credential.controller.ts b/packages/cli/src/controllers/oauth/oauth2-credential.controller.ts index 8911dc04337..51ceecf3b24 100644 --- a/packages/cli/src/controllers/oauth/oauth2-credential.controller.ts +++ b/packages/cli/src/controllers/oauth/oauth2-credential.controller.ts @@ -11,7 +11,7 @@ import { ensureError, jsonParse, jsonStringify } from 'n8n-workflow'; import { ExternalHooks } from '@/external-hooks'; import { OAuthJweServiceProxy } from '@/oauth/oauth-jwe-service.proxy'; -import { OauthService, OauthVersion, skipAuthOnOAuthCallback } from '@/oauth/oauth.service'; +import { OauthService, OauthVersion } from '@/oauth/oauth.service'; import { OAuthRequest } from '@/requests'; @RestController('/oauth2-credential') @@ -33,7 +33,7 @@ export class OAuth2CredentialController { } /** Verify and store app code. Generate access tokens and store for respective credential */ - @Get('/callback', { usesTemplates: true, skipAuth: skipAuthOnOAuthCallback }) + @Get('/callback', { usesTemplates: true, allowUnauthenticated: true }) async handleCallback(req: OAuthRequest.OAuth2Credential.Callback, res: Response) { try { const { code, state: encodedState } = req.query; diff --git a/packages/cli/test/integration/controllers/oauth/oauth1.api.test.ts b/packages/cli/test/integration/controllers/oauth/oauth1.api.test.ts index 2366acfbafc..6741c110299 100644 --- a/packages/cli/test/integration/controllers/oauth/oauth1.api.test.ts +++ b/packages/cli/test/integration/controllers/oauth/oauth1.api.test.ts @@ -55,6 +55,68 @@ describe('OAuth1 API', () => { nock.cleanAll(); }); + describe('callback route accessibility', () => { + // The callback route must be reachable without + // an n8n session (so external/dynamic-credential OAuth flows complete) while the handler + // still enforces session-bound validation for static credentials. + it('should reach the handler when called without authentication', async () => { + const renderSpy = jest.spyOn(Response, 'render').mockImplementation(function (this: any) { + this.end(); + return this; + }); + + await testServer.authlessAgent + .get('/oauth1-credential/callback') + .query({ + oauth_token: 'request_token', + oauth_verifier: 'verifier', + state: 'invalid_state', + }) + .expect(200); + + expect(renderSpy).toHaveBeenCalledWith( + 'oauth-error-callback', + expect.objectContaining({ + error: expect.objectContaining({ message: expect.any(String) }), + }), + ); + }); + + it('should reject an unauthenticated callback for a static credential', async () => { + const oauthService = Container.get(OauthService); + const csrfSpy = jest.spyOn(oauthService, 'createCsrfState').mockClear(); + const renderSpy = jest.spyOn(Response, 'render').mockImplementation(function (this: any) { + this.end(); + return this; + }); + + nock('https://test.domain') + .post('/oauth1/request_token') + .reply(200, 'oauth_token=request_token&oauth_token_secret=request_secret'); + + await testServer + .authAgentFor(owner) + .get('/oauth1-credential/auth') + .query({ id: credential.id }) + .expect(200); + + const [, state] = await csrfSpy.mock.results[0].value; + + await testServer.authlessAgent + .get('/oauth1-credential/callback') + .query({ + oauth_token: 'request_token', + oauth_verifier: 'verifier', + state, + }) + .expect(200); + + expect(renderSpy).toHaveBeenCalledWith('oauth-error-callback', { + error: { message: 'Unauthorized' }, + }); + }); + }); + describe('OAuth reconnect authorization', () => { const expectNoCsrfStateOnCredential = async (credentialId: string) => { const stored = await getCredentialById(credentialId); diff --git a/packages/cli/test/integration/controllers/oauth/oauth2.api.test.ts b/packages/cli/test/integration/controllers/oauth/oauth2.api.test.ts index 083b07dd9ee..8ce33670492 100644 --- a/packages/cli/test/integration/controllers/oauth/oauth2.api.test.ts +++ b/packages/cli/test/integration/controllers/oauth/oauth2.api.test.ts @@ -160,6 +160,52 @@ describe('OAuth2 API', () => { }); }); + describe('callback route accessibility', () => { + // The callback route must be reachable without + // an n8n session (so external/dynamic-credential OAuth flows complete) while the handler + // still enforces session-bound validation for static credentials. + it('should reach the handler when called without authentication', async () => { + const renderSpy = jest.spyOn(Response, 'render').mockImplementation(function (this: any) { + this.end(); + return this; + }); + + await testServer.authlessAgent + .get('/oauth2-credential/callback') + .query({ code: 'auth_code', state: 'invalid_state' }) + .expect(200); + + expect(renderSpy).toHaveBeenCalledWith( + 'oauth-error-callback', + expect.objectContaining({ + error: expect.objectContaining({ message: expect.any(String) }), + }), + ); + }); + + it('should reject an unauthenticated callback for a static credential', async () => { + const oauthService = Container.get(OauthService); + const csrfSpy = jest.spyOn(oauthService, 'createCsrfState').mockClear(); + const renderSpy = jest.spyOn(Response, 'render').mockImplementation(function (this: any) { + this.end(); + return this; + }); + + await ownerAgent.get('/oauth2-credential/auth').query({ id: credential.id }).expect(200); + + const [, state] = await csrfSpy.mock.results[0].value; + + await testServer.authlessAgent + .get('/oauth2-credential/callback') + .query({ code: 'auth_code', state }) + .expect(200); + + expect(renderSpy).toHaveBeenCalledWith('oauth-error-callback', { + error: { message: 'Unauthorized' }, + }); + }); + }); + it('should handle a valid callback without auth', async () => { const oauthService = Container.get(OauthService); const csrfSpy = jest.spyOn(oauthService, 'createCsrfState').mockClear(); @@ -262,9 +308,10 @@ describe('OAuth2 API', () => { await shareCredentialWithUsers(credential, [sharee]); const oauthService = Container.get(OauthService); - const renderSpy = (Response.render = jest.fn(function () { + const renderSpy = jest.spyOn(Response, 'render').mockImplementation(function (this: any) { this.end(); - })); + return this; + }); // Build a callback state whose decrypted userId equals the requesting member, // so the userId equality check inside decodeCsrfState passes and the credential