fix(core): Allow dynamic credential OAuth callbacks without skip-auth env var (#31103)

This commit is contained in:
Andreas Fitzek 2026-05-26 17:09:07 +02:00 committed by GitHub
parent 979a53baa4
commit cf1a6fa18c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 115 additions and 10 deletions

View File

@ -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;

View File

@ -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;

View File

@ -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);

View File

@ -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