feat: Make underlying oauth abstractions more generic (#22741)

2026-09-24 23:22:38 +08:00 · 2025-12-05 15:16:10 +00:00
parent decbc162aa
commit 5926ebf094
8 changed files with 1233 additions and 922 deletions
@@ -1,47 +0,0 @@
-import { shouldSkipAuthOnOAuthCallback } from '../abstract-oauth.controller';
-
-describe('shouldSkipAuthOnOAuthCallback', () => {
-	const originalEnv = process.env.N8N_SKIP_AUTH_ON_OAUTH_CALLBACK;
-
-	afterEach(() => {
-		// Restore original environment variable after each test
-		if (originalEnv === undefined) {
-			delete process.env.N8N_SKIP_AUTH_ON_OAUTH_CALLBACK;
-		} else {
-			process.env.N8N_SKIP_AUTH_ON_OAUTH_CALLBACK = originalEnv;
-		}
-	});
-
-	describe('when N8N_SKIP_AUTH_ON_OAUTH_CALLBACK is not set', () => {
-		beforeEach(() => {
-			delete process.env.N8N_SKIP_AUTH_ON_OAUTH_CALLBACK;
-		});
-
-		it('should return true', () => {
-			expect(shouldSkipAuthOnOAuthCallback()).toBe(false);
-		});
-	});
-
-	describe('with various environment variable values', () => {
-		const testCases = [
-			{ value: 'true', expected: true },
-			{ value: 'TRUE', expected: true },
-			{ value: 'True', expected: true },
-			{ value: 'false', expected: false },
-			{ value: 'FALSE', expected: false },
-			{ value: 'False', expected: false },
-			{ value: '', expected: false },
-			{ value: '1', expected: false },
-			{ value: 'yes', expected: false },
-			{ value: 'on', expected: false },
-			{ value: 'enabled', expected: false },
-			{ value: '   ', expected: false },
-			{ value: ' true ', expected: false },
-		] as const;
-
-		test.each(testCases)('"%s" value should return %s', ({ value, expected }) => {
-			process.env.N8N_SKIP_AUTH_ON_OAUTH_CALLBACK = value;
-			expect(shouldSkipAuthOnOAuthCallback()).toBe(expected);
-		});
-	});
-});
@@ -1,57 +1,22 @@
 import { Logger } from '@n8n/backend-common';
 import { mockInstance } from '@n8n/backend-test-utils';
-import { Time } from '@n8n/constants';
-import type { CredentialsEntity, User } from '@n8n/db';
-import { CredentialsRepository, GLOBAL_OWNER_ROLE } from '@n8n/db';
+import { type CredentialsEntity, type User } from '@n8n/db';
 import { Container } from '@n8n/di';
-import Csrf from 'csrf';
+import { mock } from 'jest-mock-extended';
+import axios from 'axios';
 import type { Response } from 'express';
-import { captor, mock } from 'jest-mock-extended';
-import { Cipher, type InstanceSettings, ExternalSecretsProxy } from 'n8n-core';
-import type { IWorkflowExecuteAdditionalData } from 'n8n-workflow';
-import nock from 'nock';
-
 import { OAuth1CredentialController } from '@/controllers/oauth/oauth1-credential.controller';
-import { CredentialsFinderService } from '@/credentials/credentials-finder.service';
-import { CredentialsHelper } from '@/credentials-helper';
-import { VariablesService } from '@/environments.ee/variables/variables.service.ee';
-import { BadRequestError } from '@/errors/response-errors/bad-request.error';
-import { NotFoundError } from '@/errors/response-errors/not-found.error';
-import { ExternalHooks } from '@/external-hooks';
 import type { OAuthRequest } from '@/requests';
-import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data';
+import { OauthService } from '@/oauth/oauth.service';
+import { ExternalHooks } from '@/external-hooks';

-jest.mock('@/workflow-execute-additional-data');
+jest.mock('axios');

 describe('OAuth1CredentialController', () => {
+	const oauthService = mockInstance(OauthService);
+
 	mockInstance(Logger);
 	mockInstance(ExternalHooks);
-	mockInstance(ExternalSecretsProxy);
-	mockInstance(VariablesService, {
-		getAllCached: async () => [],
-	});
-	const additionalData = mock<IWorkflowExecuteAdditionalData>();
-	(WorkflowExecuteAdditionalData.getBase as jest.Mock).mockReturnValue(additionalData);
-
-	const cipher = new Cipher(mock<InstanceSettings>({ encryptionKey: 'password' }));
-	Container.set(Cipher, cipher);
-	const credentialsHelper = mockInstance(CredentialsHelper);
-	const credentialsRepository = mockInstance(CredentialsRepository);
-	const credentialsFinderService = mockInstance(CredentialsFinderService);
-
-	const csrfSecret = 'csrf-secret';
-	const user = mock<User>({
-		id: '123',
-		password: 'password',
-		authIdentities: [],
-		role: GLOBAL_OWNER_ROLE,
-	});
-	const credential = mock<CredentialsEntity>({
-		id: '1',
-		name: 'Test Credential',
-		type: 'oAuth1Api',
-		data: cipher.encrypt({}),
-	});

 	const controller = Container.get(OAuth1CredentialController);

@@ -64,64 +29,26 @@ describe('OAuth1CredentialController', () => {
 	});

 	describe('getAuthUri', () => {
-		it('should throw a BadRequestError when credentialId is missing in the query', async () => {
-			const req = mock<OAuthRequest.OAuth1Credential.Auth>({ query: { id: '' } });
-			await expect(controller.getAuthUri(req)).rejects.toThrowError(
-				new BadRequestError('Required credential ID is missing'),
-			);
-		});
-
-		it('should throw a NotFoundError when no matching credential is found for the user', async () => {
-			credentialsFinderService.findCredentialForUser.mockResolvedValueOnce(null);
-
-			const req = mock<OAuthRequest.OAuth1Credential.Auth>({ user, query: { id: '1' } });
-			await expect(controller.getAuthUri(req)).rejects.toThrowError(
-				new NotFoundError('Credential not found'),
-			);
-		});
-
 		it('should return a valid auth URI', async () => {
-			jest.spyOn(Csrf.prototype, 'secretSync').mockReturnValueOnce(csrfSecret);
-			jest.spyOn(Csrf.prototype, 'create').mockReturnValueOnce('token');
-			credentialsFinderService.findCredentialForUser.mockResolvedValueOnce(credential);
-			credentialsHelper.getDecrypted.mockResolvedValueOnce({});
-			credentialsHelper.applyDefaultsAndOverwrites.mockResolvedValueOnce({
+			const mockResolvedCredential = mock<CredentialsEntity>({ id: '1' });
+			oauthService.getCredential.mockResolvedValueOnce(mockResolvedCredential);
+			oauthService.createCsrfState.mockReturnValueOnce(['csrf-secret', 'state']);
+			oauthService.getOAuthCredentials.mockResolvedValueOnce({
 				requestTokenUrl: 'https://example.domain/oauth/request_token',
 				authUrl: 'https://example.domain/oauth/authorize',
 				accessTokenUrl: 'https://example.domain/oauth/access_token',
-				signatureMethod: 'HMAC-SHA1',
+				signatureMethod: 'HMAC-SHA1' as const,
+			});
+			jest.mocked(axios).request.mockResolvedValueOnce({ data: { oauth_token: 'random-token' } });
+			const req = mock<OAuthRequest.OAuth1Credential.Auth>({
+				user: mock<User>({ id: '123' }),
+				query: { id: '1' },
 			});
-			nock('https://example.domain')
-				.post('/oauth/request_token', {
-					oauth_callback:
-						'http://localhost:5678/rest/oauth1-credential/callback?state=eyJ0b2tlbiI6InRva2VuIiwiY2lkIjoiMSIsImNyZWF0ZWRBdCI6MTcwNjc1MDYyNTY3OCwidXNlcklkIjoiMTIzIn0=',
-				})
-				.once()
-				.reply(200, { oauth_token: 'random-token' });
-
-			const req = mock<OAuthRequest.OAuth1Credential.Auth>({ user, query: { id: '1' } });
 			const authUri = await controller.getAuthUri(req);
 			expect(authUri).toEqual('https://example.domain/oauth/authorize?oauth_token=random-token');
-			const dataCaptor = captor();
-			expect(credentialsRepository.update).toHaveBeenCalledWith(
-				'1',
-				expect.objectContaining({
-					data: dataCaptor,
-					id: '1',
-					name: 'Test Credential',
-					type: 'oAuth1Api',
-				}),
-			);
-			expect(cipher.decrypt(dataCaptor.value)).toEqual(
-				JSON.stringify({ csrfSecret: 'csrf-secret' }),
-			);
-			expect(credentialsHelper.getDecrypted).toHaveBeenCalledWith(
-				additionalData,
-				credential,
-				credential.type,
-				'internal',
-				undefined,
-				false,
+			expect(oauthService.encryptAndSaveData).toHaveBeenCalledWith(
+				mockResolvedCredential,
+				expect.objectContaining({ csrfSecret: 'csrf-secret' }),
 			);
 		});
 	});
@@ -149,113 +76,40 @@ describe('OAuth1CredentialController', () => {
 			invalidReq.query = { state: 'test' } as OAuthRequest.OAuth1Credential.Callback['query'];
 			await controller.handleCallback(invalidReq, res);

-			expect(res.render).toHaveBeenCalledWith('oauth-error-callback', {
-				error: {
-					message: 'Insufficient parameters for OAuth1 callback.',
-					reason: 'Received following query parameters: {"state":"test"}',
-				},
-			});
-			expect(credentialsRepository.findOneBy).not.toHaveBeenCalled();
-		});
-
-		it('should render the error page when `state` query param is invalid', async () => {
-			const invalidReq = mock<OAuthRequest.OAuth1Credential.Callback>({
-				query: {
-					oauth_verifier: 'verifier',
-					oauth_token: 'token',
-					state: 'test',
-				},
-			});
-			await controller.handleCallback(invalidReq, res);
-
-			expect(res.render).toHaveBeenCalledWith('oauth-error-callback', {
-				error: {
-					message: 'Invalid state format',
-				},
-			});
-			expect(credentialsRepository.findOneBy).not.toHaveBeenCalled();
-		});
-
-		it('should render the error page when credential is not found in DB', async () => {
-			credentialsRepository.findOneBy.mockResolvedValueOnce(null);
-
-			await controller.handleCallback(req, res);
-
-			expect(res.render).toHaveBeenCalledWith('oauth-error-callback', {
-				error: {
-					message: 'OAuth callback failed because of insufficient permissions',
-				},
-			});
-			expect(credentialsRepository.findOneBy).toHaveBeenCalledTimes(1);
-			expect(credentialsRepository.findOneBy).toHaveBeenCalledWith({ id: '1' });
-		});
-
-		it('should render the error page when state differs from the stored state in the credential', async () => {
-			credentialsRepository.findOneBy.mockResolvedValue(credential);
-			credentialsHelper.getDecrypted.mockResolvedValue({ csrfSecret: 'invalid' });
-
-			await controller.handleCallback(req, res);
-
-			expect(res.render).toHaveBeenCalledWith('oauth-error-callback', {
-				error: {
-					message: 'The OAuth callback state is invalid!',
-				},
-			});
-		});
-
-		it('should render the error page when state is older than 5 minutes', async () => {
-			credentialsRepository.findOneBy.mockResolvedValue(credential);
-			credentialsHelper.getDecrypted.mockResolvedValue({ csrfSecret });
-			jest.spyOn(Csrf.prototype, 'verify').mockReturnValueOnce(true);
-
-			jest.advanceTimersByTime(10 * Time.minutes.toMilliseconds);
-
-			await controller.handleCallback(req, res);
-
-			expect(res.render).toHaveBeenCalledWith('oauth-error-callback', {
-				error: {
-					message: 'The OAuth callback state is invalid!',
-				},
-			});
+			expect(oauthService.renderCallbackError).toHaveBeenCalledWith(
+				res,
+				'Insufficient parameters for OAuth1 callback.',
+				'Received following query parameters: {"state":"test"}',
+			);
 		});

 		it('should exchange the code for a valid token, and save it to DB', async () => {
-			credentialsRepository.findOneBy.mockResolvedValue(credential);
-			credentialsHelper.getDecrypted.mockResolvedValue({ csrfSecret });
-			credentialsHelper.applyDefaultsAndOverwrites.mockResolvedValueOnce({
+			const mockResolvedCredential = mock<CredentialsEntity>({ id: '1' });
+			oauthService.getCredential.mockResolvedValueOnce(mockResolvedCredential);
+			// @ts-ignore
+			oauthService.getDecryptedData.mockResolvedValue({ csrfSecret: 'invalid' });
+			oauthService.getOAuthCredentials.mockResolvedValueOnce({
 				requestTokenUrl: 'https://example.domain/oauth/request_token',
 				accessTokenUrl: 'https://example.domain/oauth/access_token',
 				signatureMethod: 'HMAC-SHA1',
 			});
-			jest.spyOn(Csrf.prototype, 'verify').mockReturnValueOnce(true);
-			nock('https://example.domain')
-				.post('/oauth/access_token', 'oauth_token=token&oauth_verifier=verifier')
-				.once()
-				.reply(200, 'access_token=new_token');
+			oauthService.resolveCredential.mockResolvedValueOnce([
+				mockResolvedCredential,
+				{ csrfSecret: 'invalid' },
+				{ accessTokenUrl: 'https://example.domain/oauth/access_token' },
+			]);
+			jest.mocked(axios).post.mockResolvedValueOnce({ data: { access_token: 'new_token' } });

 			await controller.handleCallback(req, res);
-			const dataCaptor = captor();
-			expect(credentialsRepository.update).toHaveBeenCalledWith(
-				'1',
+			// @ts-ignore
+			expect(oauthService.encryptAndSaveData).toHaveBeenCalledWith(
+				mockResolvedCredential,
 				expect.objectContaining({
-					data: dataCaptor,
-					id: '1',
-					name: 'Test Credential',
-					type: 'oAuth1Api',
+					oauthTokenData: { access_token: 'new_token' },
 				}),
-			);
-			expect(cipher.decrypt(dataCaptor.value)).toEqual(
-				JSON.stringify({ oauthTokenData: { access_token: 'new_token' } }),
+				['csrfSecret'],
 			);
 			expect(res.render).toHaveBeenCalledWith('oauth-callback');
-			expect(credentialsHelper.getDecrypted).toHaveBeenCalledWith(
-				additionalData,
-				credential,
-				credential.type,
-				'internal',
-				undefined,
-				true,
-			);
 		});
 	});
 });
@@ -9,7 +9,9 @@ import clientOAuth1 from 'oauth-1.0a';

 import { OAuthRequest } from '@/requests';

-import { AbstractOAuthController, skipAuthOnOAuthCallback } from './abstract-oauth.controller';
+import { OauthService, OauthVersion, skipAuthOnOAuthCallback } from '@/oauth/oauth.service';
+import { Logger } from '@n8n/backend-common';
+import { ExternalHooks } from '@/external-hooks';

 interface OAuth1CredentialData {
 	signatureMethod: 'HMAC-SHA256' | 'HMAC-SHA512' | 'HMAC-SHA1';
@@ -27,24 +29,24 @@ const algorithmMap = {
 } as const;

@RestController('/oauth1-credential')
-export class OAuth1CredentialController extends AbstractOAuthController {
-	override oauthVersion = 1;
+export class OAuth1CredentialController {
+	constructor(
+		private readonly oauthService: OauthService,
+		private readonly externalHooks: ExternalHooks,
+		private readonly logger: Logger,
+	) {}

 	/** Get Authorization url */
 	@Get('/auth')
 	async getAuthUri(req: OAuthRequest.OAuth1Credential.Auth): Promise<string> {
-		const credential = await this.getCredential(req);
-		const additionalData = await this.getAdditionalData();
-		const decryptedDataOriginal = await this.getDecryptedDataForAuthUri(credential, additionalData);
-		const oauthCredentials = await this.applyDefaultsAndOverwrites<OAuth1CredentialData>(
-			credential,
-			decryptedDataOriginal,
-			additionalData,
-		);
-		const [csrfSecret, state] = this.createCsrfState(
-			credential.id,
-			skipAuthOnOAuthCallback ? undefined : req.user.id,
-		);
+		const credential = await this.oauthService.getCredential(req);
+		const oauthCredentials =
+			await this.oauthService.getOAuthCredentials<OAuth1CredentialData>(credential);
+
+		const [csrfSecret, state] = this.oauthService.createCsrfState({
+			cid: credential.id,
+			userId: skipAuthOnOAuthCallback ? undefined : req.user.id,
+		});

 		const signatureMethod = oauthCredentials.signatureMethod;

@@ -62,7 +64,7 @@ export class OAuth1CredentialController extends AbstractOAuthController {
 		};

 		const oauthRequestData = {
-			oauth_callback: `${this.baseUrl}/callback?state=${state}`,
+			oauth_callback: `${this.oauthService.getBaseUrl(OauthVersion.V1)}/callback?state=${state}`,
 		};

 		await this.externalHooks.run('oauth1.authenticate', [oAuthOptions, oauthRequestData]);
@@ -91,7 +93,7 @@ export class OAuth1CredentialController extends AbstractOAuthController {

 		const returnUri = `${oauthCredentials.authUrl}?oauth_token=${responseJson.oauth_token}`;

-		await this.encryptAndSaveData(credential, { csrfSecret });
+		await this.oauthService.encryptAndSaveData(credential, { csrfSecret });

 		this.logger.debug('OAuth1 authorization successful for new credential', {
 			userId: req.user.id,
@@ -108,7 +110,7 @@ export class OAuth1CredentialController extends AbstractOAuthController {
 			const { oauth_verifier, oauth_token, state: encodedState } = req.query;

 			if (!oauth_verifier || !oauth_token || !encodedState) {
-				return this.renderCallbackError(
+				return this.oauthService.renderCallbackError(
 					res,
 					'Insufficient parameters for OAuth1 callback.',
 					`Received following query parameters: ${JSON.stringify(req.query)}`,
@@ -116,7 +118,7 @@ export class OAuth1CredentialController extends AbstractOAuthController {
 			}

 			const [credential, _, oauthCredentials] =
-				await this.resolveCredential<OAuth1CredentialData>(req);
+				await this.oauthService.resolveCredential<OAuth1CredentialData>(req);

 			// Form URL encoded body https://datatracker.ietf.org/doc/html/rfc5849#section-3.5.2
 			const oauthToken = await axios.post<string>(
@@ -131,15 +133,18 @@ export class OAuth1CredentialController extends AbstractOAuthController {

 			const oauthTokenData = Object.fromEntries(paramParser.entries());

-			await this.encryptAndSaveData(credential, { oauthTokenData }, ['csrfSecret']);
+			console.log('oauthTokenData', oauthTokenData);
+
+			await this.oauthService.encryptAndSaveData(credential, { oauthTokenData }, ['csrfSecret']);

 			this.logger.debug('OAuth1 callback successful for new credential', {
 				credentialId: credential.id,
 			});
 			return res.render('oauth-callback');
 		} catch (e) {
+			console.log('error', e);
 			const error = ensureError(e);
-			return this.renderCallbackError(
+			return this.oauthService.renderCallbackError(
 				res,
 				error.message,
 				'body' in error ? jsonStringify(error.body) : undefined,
@@ -20,44 +20,31 @@ import {
 import pkceChallenge from 'pkce-challenge';
 import * as qs from 'querystring';

-import { AbstractOAuthController, skipAuthOnOAuthCallback } from './abstract-oauth.controller';
 import {
 	oAuthAuthorizationServerMetadataSchema,
 	dynamicClientRegistrationResponseSchema,
 } from './oauth2-dynamic-client-registration.schema';

-import { GENERIC_OAUTH2_CREDENTIALS_WITH_EDITABLE_SCOPE as GENERIC_OAUTH2_CREDENTIALS_WITH_EDITABLE_SCOPE } from '@/constants';
 import { BadRequestError } from '@/errors/response-errors/bad-request.error';
 import { OAuthRequest } from '@/requests';
+import { OauthService, OauthVersion, skipAuthOnOAuthCallback } from '@/oauth/oauth.service';
+import { Logger } from '@n8n/backend-common';
+import { ExternalHooks } from '@/external-hooks';

@RestController('/oauth2-credential')
-export class OAuth2CredentialController extends AbstractOAuthController {
-	override oauthVersion = 2;
+export class OAuth2CredentialController {
+	constructor(
+		private readonly oauthService: OauthService,
+		private readonly logger: Logger,
+		private readonly externalHooks: ExternalHooks,
+	) {}

 	/** Get Authorization url */
 	@Get('/auth')
 	async getAuthUri(req: OAuthRequest.OAuth2Credential.Auth): Promise<string> {
-		const credential = await this.getCredential(req);
-		const additionalData = await this.getAdditionalData();
-		const decryptedDataOriginal = await this.getDecryptedDataForAuthUri(credential, additionalData);
-
-		// At some point in the past we saved hidden scopes to credentials (but shouldn't)
-		// Delete scope before applying defaults to make sure new scopes are present on reconnect
-		// Generic Oauth2 API is an exception because it needs to save the scope
-
-		if (
-			decryptedDataOriginal?.scope &&
-			credential.type.includes('OAuth2') &&
-			!GENERIC_OAUTH2_CREDENTIALS_WITH_EDITABLE_SCOPE.includes(credential.type)
-		) {
-			delete decryptedDataOriginal.scope;
-		}
-
-		const oauthCredentials = await this.applyDefaultsAndOverwrites<OAuth2CredentialData>(
-			credential,
-			decryptedDataOriginal,
-			additionalData,
-		);
+		const credential = await this.oauthService.getCredential(req);
+		const oauthCredentials: OAuth2CredentialData =
+			await this.oauthService.getOAuthCredentials<OAuth2CredentialData>(credential);

 		const toUpdate: ICredentialDataDecryptedObject = {};

@@ -102,7 +89,7 @@ export class OAuth2CredentialController extends AbstractOAuthController {
 				authentication,
 			);
 			const registerPayload = {
-				redirect_uris: [`${this.baseUrl}/callback`],
+				redirect_uris: [`${this.oauthService.getBaseUrl(OauthVersion.V2)}/callback`],
 				token_endpoint_auth_method,
 				grant_types,
 				response_types: ['code'],
@@ -135,10 +122,10 @@ export class OAuth2CredentialController extends AbstractOAuthController {
 		}

 		// Generate a CSRF prevention token and send it as an OAuth2 state string
-		const [csrfSecret, state] = this.createCsrfState(
-			credential.id,
-			skipAuthOnOAuthCallback ? undefined : req.user.id,
-		);
+		const [csrfSecret, state] = this.oauthService.createCsrfState({
+			cid: credential.id,
+			userId: skipAuthOnOAuthCallback ? undefined : req.user.id,
+		});

 		const oAuthOptions = {
 			...this.convertCredentialToOptions(oauthCredentials),
@@ -162,7 +149,7 @@ export class OAuth2CredentialController extends AbstractOAuthController {
 			toUpdate.codeVerifier = code_verifier;
 		}

-		await this.encryptAndSaveData(credential, toUpdate);
+		await this.oauthService.encryptAndSaveData(credential, toUpdate);

 		const oAuthObj = new ClientOAuth2(oAuthOptions);
 		const returnUri = oAuthObj.code.getUri();
@@ -181,7 +168,7 @@ export class OAuth2CredentialController extends AbstractOAuthController {
 		try {
 			const { code, state: encodedState } = req.query;
 			if (!code || !encodedState) {
-				return this.renderCallbackError(
+				return this.oauthService.renderCallbackError(
 					res,
 					'Insufficient parameters for OAuth2 callback.',
 					`Received following query parameters: ${JSON.stringify(req.query)}`,
@@ -189,7 +176,7 @@ export class OAuth2CredentialController extends AbstractOAuthController {
 			}

 			const [credential, decryptedDataOriginal, oauthCredentials] =
-				await this.resolveCredential<OAuth2CredentialData>(req);
+				await this.oauthService.resolveCredential<OAuth2CredentialData>(req);

 			let options: Partial<ClientOAuth2Options> = {};

@@ -233,7 +220,7 @@ export class OAuth2CredentialController extends AbstractOAuthController {
 				...oauthToken.data,
 			};

-			await this.encryptAndSaveData(credential, { oauthTokenData }, ['csrfSecret']);
+			await this.oauthService.encryptAndSaveData(credential, { oauthTokenData }, ['csrfSecret']);

 			this.logger.debug('OAuth2 callback successful for credential', {
 				credentialId: credential.id,
@@ -242,7 +229,7 @@ export class OAuth2CredentialController extends AbstractOAuthController {
 			return res.render('oauth-callback');
 		} catch (e) {
 			const error = ensureError(e);
-			return this.renderCallbackError(
+			return this.oauthService.renderCallbackError(
 				res,
 				error.message,
 				'body' in error ? jsonStringify(error.body) : undefined,
@@ -257,7 +244,7 @@ export class OAuth2CredentialController extends AbstractOAuthController {
 			accessTokenUri: credential.accessTokenUrl ?? '',
 			authorizationUri: credential.authUrl ?? '',
 			authentication: credential.authentication ?? 'header',
-			redirectUri: `${this.baseUrl}/callback`,
+			redirectUri: `${this.oauthService.getBaseUrl(OauthVersion.V2)}/callback`,
 			scopes: split(credential.scope ?? 'openid', ','),
 			scopesSeparator: credential.scope?.includes(',') ? ',' : ' ',
 			ignoreSSLIssues: credential.ignoreSSLIssues ?? false,
@@ -0,0 +1,693 @@
+import { Logger } from '@n8n/backend-common';
+import { GlobalConfig } from '@n8n/config';
+import { Time } from '@n8n/constants';
+import { mockInstance } from '@n8n/backend-test-utils';
+import type { AuthenticatedRequest, CredentialsEntity, ICredentialsDb, User } from '@n8n/db';
+import { CredentialsRepository } from '@n8n/db';
+import { Container } from '@n8n/di';
+import { mock } from 'jest-mock-extended';
+import type { Response } from 'express';
+import type { IWorkflowExecuteAdditionalData } from 'n8n-workflow';
+import { UnexpectedError } from 'n8n-workflow';
+
+import { OauthService, OauthVersion, shouldSkipAuthOnOAuthCallback } from '@/oauth/oauth.service';
+import { CredentialsFinderService } from '@/credentials/credentials-finder.service';
+import { CredentialsHelper } from '@/credentials-helper';
+import { AuthError } from '@/errors/response-errors/auth.error';
+import { BadRequestError } from '@/errors/response-errors/bad-request.error';
+import { NotFoundError } from '@/errors/response-errors/not-found.error';
+import type { OAuthRequest } from '@/requests';
+import { UrlService } from '@/services/url.service';
+import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data';
+
+jest.mock('@/workflow-execute-additional-data');
+
+describe('OauthService', () => {
+	const logger = mockInstance(Logger);
+	const credentialsHelper = mockInstance(CredentialsHelper);
+	const credentialsRepository = mockInstance(CredentialsRepository);
+	const credentialsFinderService = mockInstance(CredentialsFinderService);
+	const urlService = mockInstance(UrlService);
+	const globalConfig = mockInstance(GlobalConfig);
+
+	let service: OauthService;
+
+	const timestamp = 1706750625678;
+	jest.useFakeTimers({ advanceTimers: true });
+
+	beforeEach(() => {
+		jest.setSystemTime(new Date(timestamp));
+		jest.clearAllMocks();
+
+		globalConfig.endpoints = { rest: 'rest' } as any;
+		urlService.getInstanceBaseUrl.mockReturnValue('http://localhost:5678');
+		jest
+			.mocked(WorkflowExecuteAdditionalData.getBase)
+			.mockResolvedValue(mock<IWorkflowExecuteAdditionalData>());
+
+		service = new OauthService(
+			logger,
+			credentialsHelper,
+			credentialsRepository,
+			credentialsFinderService,
+			urlService,
+			globalConfig,
+		);
+	});
+
+	describe('shouldSkipAuthOnOAuthCallback', () => {
+		it('should return false when env var is not set', () => {
+			delete process.env.N8N_SKIP_AUTH_ON_OAUTH_CALLBACK;
+			expect(shouldSkipAuthOnOAuthCallback()).toBe(false);
+		});
+
+		it('should return false when env var is "false"', () => {
+			process.env.N8N_SKIP_AUTH_ON_OAUTH_CALLBACK = 'false';
+			expect(shouldSkipAuthOnOAuthCallback()).toBe(false);
+		});
+
+		it('should return true when env var is "true"', () => {
+			process.env.N8N_SKIP_AUTH_ON_OAUTH_CALLBACK = 'true';
+			expect(shouldSkipAuthOnOAuthCallback()).toBe(true);
+		});
+
+		it('should return true when env var is "TRUE" (case insensitive)', () => {
+			process.env.N8N_SKIP_AUTH_ON_OAUTH_CALLBACK = 'TRUE';
+			expect(shouldSkipAuthOnOAuthCallback()).toBe(true);
+		});
+	});
+
+	describe('getBaseUrl', () => {
+		it('should return correct URL for OAuth1', () => {
+			const url = service.getBaseUrl(OauthVersion.V1);
+			expect(url).toBe('http://localhost:5678/rest/oauth1-credential');
+			expect(urlService.getInstanceBaseUrl).toHaveBeenCalled();
+		});
+
+		it('should return correct URL for OAuth2', () => {
+			const url = service.getBaseUrl(OauthVersion.V2);
+			expect(url).toBe('http://localhost:5678/rest/oauth2-credential');
+		});
+	});
+
+	describe('getCredential', () => {
+		it('should throw BadRequestError when credential ID is missing', async () => {
+			const req = {
+				query: {},
+				user: mock<User>({ id: '123' }),
+			} as unknown as OAuthRequest.OAuth2Credential.Auth;
+
+			Object.defineProperty(req.query, 'id', {
+				value: undefined,
+				writable: true,
+				enumerable: true,
+			});
+
+			const promise = service.getCredential(req);
+			await expect(promise).rejects.toThrow(BadRequestError);
+			await expect(promise).rejects.toThrow('Required credential ID is missing');
+		});
+
+		it('should throw NotFoundError when credential is not found', async () => {
+			const req = mock<OAuthRequest.OAuth2Credential.Auth>({
+				query: { id: 'credential-id' },
+				user: mock<User>({ id: '123' }),
+			});
+
+			credentialsFinderService.findCredentialForUser.mockResolvedValue(null);
+
+			await expect(service.getCredential(req)).rejects.toThrow(NotFoundError);
+			expect(logger.error).toHaveBeenCalledWith(
+				'OAuth credential authorization failed because the current user does not have the correct permissions',
+				{ userId: '123' },
+			);
+		});
+
+		it('should return credential when found', async () => {
+			const mockCredential = mock<CredentialsEntity>({ id: 'credential-id' });
+			const req = mock<OAuthRequest.OAuth2Credential.Auth>({
+				query: { id: 'credential-id' },
+				user: mock<User>({ id: '123' }),
+			});
+
+			credentialsFinderService.findCredentialForUser.mockResolvedValue(mockCredential);
+
+			const result = await service.getCredential(req);
+
+			expect(result).toBe(mockCredential);
+			expect(credentialsFinderService.findCredentialForUser).toHaveBeenCalledWith(
+				'credential-id',
+				req.user,
+				['credential:read'],
+			);
+		});
+	});
+
+	describe('getAdditionalData', () => {
+		it('should return workflow execute additional data', async () => {
+			const mockAdditionalData = mock<IWorkflowExecuteAdditionalData>();
+			jest.mocked(WorkflowExecuteAdditionalData.getBase).mockResolvedValue(mockAdditionalData);
+
+			const result = await (service as any).getAdditionalData();
+
+			expect(result).toBe(mockAdditionalData);
+			expect(WorkflowExecuteAdditionalData.getBase).toHaveBeenCalled();
+		});
+	});
+
+	describe('getDecryptedDataForAuthUri', () => {
+		it('should call getDecryptedData with raw=false', async () => {
+			const credential = mock<ICredentialsDb>({ id: '1', type: 'test' });
+			const additionalData = mock<IWorkflowExecuteAdditionalData>();
+			const mockDecryptedData = { clientId: 'test' };
+
+			credentialsHelper.getDecrypted.mockResolvedValue(mockDecryptedData);
+
+			const result = await (service as any).getDecryptedDataForAuthUri(credential, additionalData);
+
+			expect(result).toBe(mockDecryptedData);
+			expect(credentialsHelper.getDecrypted).toHaveBeenCalledWith(
+				additionalData,
+				credential,
+				credential.type,
+				'internal',
+				undefined,
+				false,
+			);
+		});
+	});
+
+	describe('getDecryptedDataForCallback', () => {
+		it('should call getDecryptedData with raw=true', async () => {
+			const credential = mock<ICredentialsDb>({ id: '1', type: 'test' });
+			const additionalData = mock<IWorkflowExecuteAdditionalData>();
+			const mockDecryptedData = { csrfSecret: 'secret' };
+
+			credentialsHelper.getDecrypted.mockResolvedValue(mockDecryptedData);
+
+			const result = await (service as any).getDecryptedDataForCallback(credential, additionalData);
+
+			expect(result).toBe(mockDecryptedData);
+			expect(credentialsHelper.getDecrypted).toHaveBeenCalledWith(
+				additionalData,
+				credential,
+				credential.type,
+				'internal',
+				undefined,
+				true,
+			);
+		});
+	});
+
+	describe('applyDefaultsAndOverwrites', () => {
+		it('should apply defaults and overwrites', async () => {
+			const credential = mock<ICredentialsDb>({ id: '1', type: 'test' });
+			const decryptedData = { clientId: 'test' };
+			const additionalData = mock<IWorkflowExecuteAdditionalData>();
+			const mockResult = { clientId: 'test', clientSecret: 'secret' };
+
+			credentialsHelper.applyDefaultsAndOverwrites.mockResolvedValue(mockResult);
+
+			const result = await (service as any).applyDefaultsAndOverwrites(
+				credential,
+				decryptedData,
+				additionalData,
+			);
+
+			expect(result).toBe(mockResult);
+			expect(credentialsHelper.applyDefaultsAndOverwrites).toHaveBeenCalledWith(
+				additionalData,
+				decryptedData,
+				credential,
+				credential.type,
+				'internal',
+				undefined,
+				undefined,
+			);
+		});
+	});
+
+	describe('encryptAndSaveData', () => {
+		it('should encrypt and save data to repository', async () => {
+			const { Cipher } = await import('n8n-core');
+			const cipher = Container.get(Cipher);
+			const encryptedData = cipher.encrypt({ existing: 'data' });
+
+			const credential = mock<ICredentialsDb>({
+				id: '1',
+				type: 'test',
+				data: encryptedData,
+			});
+			const toUpdate = { clientId: 'new-id' };
+			const toDelete = ['oldField'];
+
+			await service.encryptAndSaveData(credential, toUpdate, toDelete);
+
+			expect(credentialsRepository.update).toHaveBeenCalledWith('1', {
+				id: '1',
+				name: expect.anything(),
+				type: 'test',
+				data: expect.any(String),
+				updatedAt: expect.any(Date),
+			});
+		});
+
+		it('should use empty array for toDelete when not provided', async () => {
+			const { Cipher } = await import('n8n-core');
+			const cipher = Container.get(Cipher);
+			const encryptedData = cipher.encrypt({ existing: 'data' });
+
+			const credential = mock<ICredentialsDb>({
+				id: '1',
+				type: 'test',
+				data: encryptedData,
+			});
+			const toUpdate = { clientId: 'new-id' };
+
+			await service.encryptAndSaveData(credential, toUpdate);
+
+			expect(credentialsRepository.update).toHaveBeenCalledWith('1', {
+				id: '1',
+				name: expect.anything(),
+				type: 'test',
+				data: expect.any(String),
+				updatedAt: expect.any(Date),
+			});
+		});
+	});
+
+	describe('getCredentialWithoutUser', () => {
+		it('should return credential from repository', async () => {
+			const mockCredential = mock<ICredentialsDb>({ id: '1' });
+			credentialsRepository.findOneBy.mockResolvedValue(mockCredential as any);
+
+			const result = await (service as any).getCredentialWithoutUser('1');
+
+			expect(result).toBe(mockCredential);
+			expect(credentialsRepository.findOneBy).toHaveBeenCalledWith({ id: '1' });
+		});
+
+		it('should return null when credential not found', async () => {
+			credentialsRepository.findOneBy.mockResolvedValue(null);
+
+			const result = await (service as any).getCredentialWithoutUser('1');
+
+			expect(result).toBeNull();
+		});
+	});
+
+	describe('createCsrfState', () => {
+		it('should create CSRF state with correct structure', () => {
+			const data = { cid: 'credential-id', userId: 'user-id' };
+			jest.setSystemTime(new Date(timestamp));
+
+			const [csrfSecret, encodedState] = service.createCsrfState(data);
+
+			expect(typeof csrfSecret).toBe('string');
+			expect(csrfSecret.length).toBeGreaterThan(0);
+
+			const decoded = JSON.parse(Buffer.from(encodedState, 'base64').toString());
+			expect(decoded.cid).toBe('credential-id');
+			expect(decoded.userId).toBe('user-id');
+			expect(decoded.token).toBeDefined();
+			expect(decoded.createdAt).toBe(timestamp);
+		});
+
+		it('should include additional data in state', () => {
+			const data = { cid: 'credential-id', customField: 'custom-value' };
+			jest.setSystemTime(new Date(timestamp));
+
+			const [, encodedState] = service.createCsrfState(data);
+
+			const decoded = JSON.parse(Buffer.from(encodedState, 'base64').toString());
+			expect(decoded.customField).toBe('custom-value');
+		});
+	});
+
+	describe('decodeCsrfState', () => {
+		it('should decode valid CSRF state', () => {
+			const state = {
+				token: 'token',
+				cid: 'credential-id',
+				userId: 'user-id',
+				createdAt: timestamp,
+			};
+			const encodedState = Buffer.from(JSON.stringify(state)).toString('base64');
+			const req = mock<AuthenticatedRequest>({
+				user: mock<User>({ id: 'user-id' }),
+			});
+
+			const result = (service as any).decodeCsrfState(encodedState, req);
+
+			expect(result).toEqual(state);
+		});
+
+		it('should throw error when state format is invalid', () => {
+			const invalidState = 'not-base64-json';
+			const req = mock<AuthenticatedRequest>({
+				user: mock<User>({ id: 'user-id' }),
+			});
+
+			expect(() => (service as any).decodeCsrfState(invalidState, req)).toThrow(
+				'Invalid state format',
+			);
+		});
+
+		it('should throw UnexpectedError when cid is missing', () => {
+			const state = {
+				token: 'token',
+				createdAt: timestamp,
+			};
+			const encodedState = Buffer.from(JSON.stringify(state)).toString('base64');
+			const req = mock<AuthenticatedRequest>({
+				user: mock<User>({ id: 'user-id' }),
+			});
+
+			expect(() => (service as any).decodeCsrfState(encodedState, req)).toThrow(UnexpectedError);
+		});
+
+		it('should throw UnexpectedError when token is missing', () => {
+			const state = {
+				cid: 'credential-id',
+				createdAt: timestamp,
+			};
+			const encodedState = Buffer.from(JSON.stringify(state)).toString('base64');
+			const req = mock<AuthenticatedRequest>({
+				user: mock<User>({ id: 'user-id' }),
+			});
+
+			expect(() => (service as any).decodeCsrfState(encodedState, req)).toThrow(UnexpectedError);
+		});
+
+		it('should throw AuthError when userId does not match', () => {
+			const state = {
+				token: 'token',
+				cid: 'credential-id',
+				userId: 'different-user-id',
+				createdAt: timestamp,
+			};
+			const encodedState = Buffer.from(JSON.stringify(state)).toString('base64');
+			const req = mock<AuthenticatedRequest>({
+				user: mock<User>({ id: 'user-id' }),
+			});
+
+			expect(() => (service as any).decodeCsrfState(encodedState, req)).toThrow(AuthError);
+			expect(() => (service as any).decodeCsrfState(encodedState, req)).toThrow('Unauthorized');
+		});
+
+		it('should throw AuthError when req.user is undefined', () => {
+			const state = {
+				token: 'token',
+				cid: 'credential-id',
+				userId: 'user-id',
+				createdAt: timestamp,
+			};
+			const encodedState = Buffer.from(JSON.stringify(state)).toString('base64');
+			const req = mock<AuthenticatedRequest>({
+				user: undefined,
+			});
+
+			expect(() => (service as any).decodeCsrfState(encodedState, req)).toThrow(AuthError);
+		});
+	});
+
+	describe('verifyCsrfState', () => {
+		it('should return true for valid CSRF state', () => {
+			const csrfSecret = 'csrf-secret';
+			const token = new (require('csrf'))();
+			const stateToken = token.create(csrfSecret);
+
+			const state = {
+				token: stateToken,
+				cid: 'credential-id',
+				createdAt: Date.now(),
+			};
+			const decrypted = { csrfSecret };
+
+			const result = (service as any).verifyCsrfState(decrypted, state);
+
+			expect(result).toBe(true);
+		});
+
+		it('should return false when CSRF state is expired', () => {
+			const csrfSecret = 'csrf-secret';
+			const token = new (require('csrf'))();
+			const stateToken = token.create(csrfSecret);
+
+			const expiredTime = Date.now() - 6 * Time.minutes.toMilliseconds;
+			const state = {
+				token: stateToken,
+				cid: 'credential-id',
+				createdAt: expiredTime,
+			};
+			const decrypted = { csrfSecret };
+
+			const result = (service as any).verifyCsrfState(decrypted, state);
+
+			expect(result).toBe(false);
+		});
+
+		it('should return false when csrfSecret is missing', () => {
+			const token = new (require('csrf'))();
+			const csrfSecret = 'csrf-secret';
+			const stateToken = token.create(csrfSecret);
+
+			const state = {
+				token: stateToken,
+				cid: 'credential-id',
+				createdAt: Date.now(),
+			};
+			const decrypted = {};
+
+			const result = (service as any).verifyCsrfState(decrypted, state);
+
+			expect(result).toBe(false);
+		});
+
+		it('should return false when token verification fails', () => {
+			const state = {
+				token: 'invalid-token',
+				cid: 'credential-id',
+				createdAt: Date.now(),
+			};
+			const decrypted = { csrfSecret: 'csrf-secret' };
+
+			const result = (service as any).verifyCsrfState(decrypted, state);
+
+			expect(result).toBe(false);
+		});
+	});
+
+	describe('resolveCredential', () => {
+		it('should resolve credential successfully', async () => {
+			const state = {
+				token: 'token',
+				cid: 'credential-id',
+				userId: 'user-id',
+				createdAt: timestamp,
+			};
+
+			const mockCredential = mock<CredentialsEntity>({ id: 'credential-id' });
+			const mockDecryptedData = { csrfSecret: 'csrf-secret' };
+			const mockOAuthCredentials = { clientId: 'client-id' };
+			const mockAdditionalData = mock<IWorkflowExecuteAdditionalData>();
+
+			const token = new (require('csrf'))();
+			const stateToken = token.create('csrf-secret');
+			state.token = stateToken;
+
+			const req = mock<OAuthRequest.OAuth2Credential.Callback>({
+				query: { state: Buffer.from(JSON.stringify(state)).toString('base64') },
+				user: mock<User>({ id: 'user-id' }),
+			});
+
+			credentialsRepository.findOneBy.mockResolvedValue(mockCredential);
+			jest.mocked(WorkflowExecuteAdditionalData.getBase).mockResolvedValue(mockAdditionalData);
+			credentialsHelper.getDecrypted.mockResolvedValue(mockDecryptedData);
+			credentialsHelper.applyDefaultsAndOverwrites.mockResolvedValue(mockOAuthCredentials);
+
+			jest.spyOn(service as any, 'verifyCsrfState').mockReturnValue(true);
+
+			const result = await service.resolveCredential(req);
+
+			expect(result).toEqual([mockCredential, mockDecryptedData, mockOAuthCredentials]);
+		});
+
+		it('should throw UnexpectedError when credential is not found', async () => {
+			const state = {
+				token: 'token',
+				cid: 'credential-id',
+				userId: 'user-id',
+				createdAt: timestamp,
+			};
+			const encodedState = Buffer.from(JSON.stringify(state)).toString('base64');
+
+			const req = mock<OAuthRequest.OAuth2Credential.Callback>({
+				query: { state: encodedState },
+				user: mock<User>({ id: 'user-id' }),
+			});
+
+			credentialsRepository.findOneBy.mockResolvedValue(null);
+
+			await expect(service.resolveCredential(req)).rejects.toThrow(UnexpectedError);
+			await expect(service.resolveCredential(req)).rejects.toThrow(
+				'OAuth callback failed because of insufficient permissions',
+			);
+		});
+
+		it('should throw UnexpectedError when CSRF state is invalid', async () => {
+			const state = {
+				token: 'token',
+				cid: 'credential-id',
+				userId: 'user-id',
+				createdAt: timestamp,
+			};
+			const encodedState = Buffer.from(JSON.stringify(state)).toString('base64');
+
+			const mockCredential = mock<CredentialsEntity>({ id: 'credential-id' });
+			const mockDecryptedData = { csrfSecret: 'csrf-secret' };
+			const mockOAuthCredentials = { clientId: 'client-id' };
+			const mockAdditionalData = mock<IWorkflowExecuteAdditionalData>();
+
+			const req = mock<OAuthRequest.OAuth2Credential.Callback>({
+				query: { state: encodedState },
+				user: mock<User>({ id: 'user-id' }),
+			});
+
+			credentialsRepository.findOneBy.mockResolvedValue(mockCredential);
+			jest.mocked(WorkflowExecuteAdditionalData.getBase).mockResolvedValue(mockAdditionalData);
+			credentialsHelper.getDecrypted.mockResolvedValue(mockDecryptedData);
+			credentialsHelper.applyDefaultsAndOverwrites.mockResolvedValue(mockOAuthCredentials);
+
+			jest.spyOn(service as any, 'verifyCsrfState').mockReturnValue(false);
+
+			await expect(service.resolveCredential(req)).rejects.toThrow(UnexpectedError);
+			await expect(service.resolveCredential(req)).rejects.toThrow(
+				'The OAuth callback state is invalid!',
+			);
+		});
+	});
+
+	describe('renderCallbackError', () => {
+		it('should render error page with message', () => {
+			const res = mock<Response>();
+			const message = 'Test error message';
+
+			service.renderCallbackError(res, message);
+
+			expect(res.render).toHaveBeenCalledWith('oauth-error-callback', {
+				error: { message },
+			});
+		});
+
+		it('should render error page with message and reason', () => {
+			const res = mock<Response>();
+			const message = 'Test error message';
+			const reason = 'Test reason';
+
+			service.renderCallbackError(res, message, reason);
+
+			expect(res.render).toHaveBeenCalledWith('oauth-error-callback', {
+				error: { message, reason },
+			});
+		});
+	});
+
+	describe('getOAuthCredentials', () => {
+		it('should return OAuth credentials', async () => {
+			const credential = mock<CredentialsEntity>({
+				id: '1',
+				type: 'testOAuth2Api',
+			});
+			const mockDecryptedData = { clientId: 'client-id' };
+			const mockOAuthCredentials = { clientId: 'client-id', clientSecret: 'secret' };
+			const mockAdditionalData = mock<IWorkflowExecuteAdditionalData>();
+
+			jest.mocked(WorkflowExecuteAdditionalData.getBase).mockResolvedValue(mockAdditionalData);
+			credentialsHelper.getDecrypted.mockResolvedValue(mockDecryptedData);
+			credentialsHelper.applyDefaultsAndOverwrites.mockResolvedValue(mockOAuthCredentials);
+
+			const result = await service.getOAuthCredentials(credential);
+
+			expect(result).toBe(mockOAuthCredentials);
+		});
+
+		it('should delete scope for non-generic OAuth2 credentials', async () => {
+			const credential = mock<CredentialsEntity>({
+				id: '1',
+				type: 'testOAuth2Api',
+			});
+			const mockDecryptedData = { clientId: 'client-id', scope: 'old-scope' };
+			const mockOAuthCredentials = { clientId: 'client-id' };
+			const mockAdditionalData = mock<IWorkflowExecuteAdditionalData>();
+
+			jest.mocked(WorkflowExecuteAdditionalData.getBase).mockResolvedValue(mockAdditionalData);
+			credentialsHelper.getDecrypted.mockResolvedValue(mockDecryptedData);
+			credentialsHelper.applyDefaultsAndOverwrites.mockResolvedValue(mockOAuthCredentials);
+
+			await service.getOAuthCredentials(credential);
+
+			expect(credentialsHelper.applyDefaultsAndOverwrites).toHaveBeenCalledWith(
+				mockAdditionalData,
+				{ clientId: 'client-id' },
+				credential,
+				credential.type,
+				'internal',
+				undefined,
+				undefined,
+			);
+		});
+
+		it('should not delete scope for generic OAuth2 credentials with editable scope', async () => {
+			const credential = mock<CredentialsEntity>({
+				id: '1',
+				type: 'oAuth2Api',
+			});
+			const mockDecryptedData = { clientId: 'client-id', scope: 'old-scope' };
+			const mockOAuthCredentials = { clientId: 'client-id', scope: 'old-scope' };
+			const mockAdditionalData = mock<IWorkflowExecuteAdditionalData>();
+
+			jest.mocked(WorkflowExecuteAdditionalData.getBase).mockResolvedValue(mockAdditionalData);
+			credentialsHelper.getDecrypted.mockResolvedValue(mockDecryptedData);
+			credentialsHelper.applyDefaultsAndOverwrites.mockResolvedValue(mockOAuthCredentials);
+
+			await service.getOAuthCredentials(credential);
+
+			expect(credentialsHelper.applyDefaultsAndOverwrites).toHaveBeenCalledWith(
+				mockAdditionalData,
+				{ clientId: 'client-id', scope: 'old-scope' },
+				credential,
+				credential.type,
+				'internal',
+				undefined,
+				undefined,
+			);
+		});
+
+		it('should not delete scope for non-OAuth2 credentials', async () => {
+			const credential = mock<CredentialsEntity>({
+				id: '1',
+				type: 'testApi',
+			});
+			const mockDecryptedData = { clientId: 'client-id', scope: 'old-scope' };
+			const mockOAuthCredentials = { clientId: 'client-id', scope: 'old-scope' };
+			const mockAdditionalData = mock<IWorkflowExecuteAdditionalData>();
+
+			jest.mocked(WorkflowExecuteAdditionalData.getBase).mockResolvedValue(mockAdditionalData);
+			credentialsHelper.getDecrypted.mockResolvedValue(mockDecryptedData);
+			credentialsHelper.applyDefaultsAndOverwrites.mockResolvedValue(mockOAuthCredentials);
+
+			await service.getOAuthCredentials(credential);
+
+			expect(credentialsHelper.applyDefaultsAndOverwrites).toHaveBeenCalledWith(
+				mockAdditionalData,
+				{ clientId: 'client-id', scope: 'old-scope' },
+				credential,
+				credential.type,
+				'internal',
+				undefined,
+				undefined,
+			);
+		});
+	});
+});
@@ -10,28 +10,33 @@ import { Credentials } from 'n8n-core';
 import type { ICredentialDataDecryptedObject, IWorkflowExecuteAdditionalData } from 'n8n-workflow';
 import { jsonParse, UnexpectedError } from 'n8n-workflow';

-import { RESPONSE_ERROR_MESSAGES } from '@/constants';
+import {
+	GENERIC_OAUTH2_CREDENTIALS_WITH_EDITABLE_SCOPE,
+	RESPONSE_ERROR_MESSAGES,
+} from '@/constants';
 import { CredentialsFinderService } from '@/credentials/credentials-finder.service';
 import { CredentialsHelper } from '@/credentials-helper';
 import { AuthError } from '@/errors/response-errors/auth.error';
 import { BadRequestError } from '@/errors/response-errors/bad-request.error';
 import { NotFoundError } from '@/errors/response-errors/not-found.error';
-import { ExternalHooks } from '@/external-hooks';
 import type { OAuthRequest } from '@/requests';
 import { UrlService } from '@/services/url.service';
 import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data';

-type CsrfStateParam = {
-	/** Id of the oAuth credential in the DB */
-	cid: string;
+type CsrfStateRequired = {
 	/** Random CSRF token, used to verify the signature of the CSRF state */
 	token: string;
 	/** Creation timestamp of the CSRF state. Used for expiration.  */
 	createdAt: number;
-	/** User who initiated OAuth flow, included to prevent cross-user credential hijacking. Optional only if `skipAuthOnOAuthCallback` is enabled. */
-	userId?: string;
 };

+type CreateCsrfStateData = {
+	cid: string;
+	[key: string]: unknown;
+};
+
+type CsrfState = CsrfStateRequired & CreateCsrfStateData;
+
 const MAX_CSRF_AGE = 5 * Time.minutes.toMilliseconds;

 export function shouldSkipAuthOnOAuthCallback() {
@@ -41,13 +46,15 @@ export function shouldSkipAuthOnOAuthCallback() {

 export const skipAuthOnOAuthCallback = shouldSkipAuthOnOAuthCallback();

-@Service()
-export abstract class AbstractOAuthController {
-	abstract oauthVersion: number;
+export const enum OauthVersion {
+	V1 = 1,
+	V2 = 2,
+}

+@Service()
+export class OauthService {
 	constructor(
 		protected readonly logger: Logger,
-		protected readonly externalHooks: ExternalHooks,
 		private readonly credentialsHelper: CredentialsHelper,
 		private readonly credentialsRepository: CredentialsRepository,
 		private readonly credentialsFinderService: CredentialsFinderService,
@@ -55,14 +62,12 @@ export abstract class AbstractOAuthController {
 		private readonly globalConfig: GlobalConfig,
 	) {}

-	get baseUrl() {
+	getBaseUrl(oauthVersion: OauthVersion) {
 		const restUrl = `${this.urlService.getInstanceBaseUrl()}/${this.globalConfig.endpoints.rest}`;
-		return `${restUrl}/oauth${this.oauthVersion}-credential`;
+		return `${restUrl}/oauth${oauthVersion}-credential`;
 	}

-	protected async getCredential(
-		req: OAuthRequest.OAuth2Credential.Auth,
-	): Promise<CredentialsEntity> {
+	async getCredential(req: OAuthRequest.OAuth2Credential.Auth): Promise<CredentialsEntity> {
 		const { id: credentialId } = req.query;

 		if (!credentialId) {
@@ -77,7 +82,7 @@ export abstract class AbstractOAuthController {

 		if (!credential) {
 			this.logger.error(
-				`OAuth${this.oauthVersion} credential authorization failed because the current user does not have the correct permissions`,
+				'OAuth credential authorization failed because the current user does not have the correct permissions',
 				{ userId: req.user.id },
 			);
 			throw new NotFoundError(RESPONSE_ERROR_MESSAGES.NO_CREDENTIAL);
@@ -141,7 +146,7 @@ export abstract class AbstractOAuthController {
 		)) as unknown as T;
 	}

-	protected async encryptAndSaveData(
+	async encryptAndSaveData(
 		credential: ICredentialsDb,
 		toUpdate: ICredentialDataDecryptedObject,
 		toDelete: string[] = [],
@@ -159,21 +164,20 @@ export abstract class AbstractOAuthController {
 		return await this.credentialsRepository.findOneBy({ id: credentialId });
 	}

-	createCsrfState(credentialsId: string, userId?: string): [string, string] {
+	createCsrfState(data: CreateCsrfStateData): [string, string] {
 		const token = new Csrf();
 		const csrfSecret = token.secretSync();
-		const state: CsrfStateParam = {
+		const state: CsrfState = {
 			token: token.create(csrfSecret),
-			cid: credentialsId,
 			createdAt: Date.now(),
-			userId,
+			...data,
 		};
 		return [csrfSecret, Buffer.from(JSON.stringify(state)).toString('base64')];
 	}

-	protected decodeCsrfState(encodedState: string, req: AuthenticatedRequest): CsrfStateParam {
+	protected decodeCsrfState(encodedState: string, req: AuthenticatedRequest): CsrfState {
 		const errorMessage = 'Invalid state format';
-		const decoded = jsonParse<CsrfStateParam>(Buffer.from(encodedState, 'base64').toString(), {
+		const decoded = jsonParse<CsrfState>(Buffer.from(encodedState, 'base64').toString(), {
 			errorMessage,
 		});

@@ -190,7 +194,7 @@ export abstract class AbstractOAuthController {

 	protected verifyCsrfState(
 		decrypted: ICredentialDataDecryptedObject & { csrfSecret?: string },
-		state: CsrfStateParam,
+		state: CsrfState,
 	) {
 		const token = new Csrf();

@@ -201,7 +205,7 @@ export abstract class AbstractOAuthController {
 		);
 	}

-	protected async resolveCredential<T>(
+	async resolveCredential<T>(
 		req: OAuthRequest.OAuth1Credential.Callback | OAuthRequest.OAuth2Credential.Callback,
 	): Promise<[ICredentialsDb, ICredentialDataDecryptedObject, T]> {
 		const { state: encodedState } = req.query;
@@ -230,7 +234,31 @@ export abstract class AbstractOAuthController {
 		return [credential, decryptedDataOriginal, oauthCredentials];
 	}

-	protected renderCallbackError(res: Response, message: string, reason?: string) {
+	renderCallbackError(res: Response, message: string, reason?: string) {
 		res.render('oauth-error-callback', { error: { message, reason } });
 	}
+
+	async getOAuthCredentials<T>(credential: CredentialsEntity): Promise<T> {
+		const additionalData = await this.getAdditionalData();
+		const decryptedDataOriginal = await this.getDecryptedDataForAuthUri(credential, additionalData);
+
+		// At some point in the past we saved hidden scopes to credentials (but shouldn't)
+		// Delete scope before applying defaults to make sure new scopes are present on reconnect
+		// Generic Oauth2 API is an exception because it needs to save the scope
+		if (
+			decryptedDataOriginal?.scope &&
+			credential.type.includes('OAuth2') &&
+			!GENERIC_OAUTH2_CREDENTIALS_WITH_EDITABLE_SCOPE.includes(credential.type)
+		) {
+			delete decryptedDataOriginal.scope;
+		}
+
+		const oauthCredentials = await this.applyDefaultsAndOverwrites<T>(
+			credential,
+			decryptedDataOriginal,
+			additionalData,
+		);
+
+		return oauthCredentials;
+	}
 }
@@ -5,8 +5,8 @@ import { response as Response } from 'express';
 import nock from 'nock';
 import { parse as parseQs } from 'querystring';

-import { OAuth2CredentialController } from '@/controllers/oauth/oauth2-credential.controller';
 import { CredentialsHelper } from '@/credentials-helper';
+import { OauthService } from '@/oauth/oauth.service';
 import { saveCredential } from '@test-integration/db/credentials';
 import { createMember, createOwner } from '@test-integration/db/users';
 import type { SuperAgentTest } from '@test-integration/types';
@@ -52,8 +52,8 @@ describe('OAuth2 API', () => {
 	});

 	it('should return a valid auth URL when the auth flow is initiated', async () => {
-		const controller = Container.get(OAuth2CredentialController);
-		const csrfSpy = jest.spyOn(controller, 'createCsrfState').mockClear();
+		const oauthService = Container.get(OauthService);
+		const csrfSpy = jest.spyOn(oauthService, 'createCsrfState').mockClear();

 		const response = await ownerAgent
 			.get('/oauth2-credential/auth')
@@ -76,8 +76,8 @@ describe('OAuth2 API', () => {
 	});

 	it('should fail on auth when callback is called as another user', async () => {
-		const controller = Container.get(OAuth2CredentialController);
-		const csrfSpy = jest.spyOn(controller, 'createCsrfState').mockClear();
+		const oauthService = Container.get(OauthService);
+		const csrfSpy = jest.spyOn(oauthService, 'createCsrfState').mockClear();
 		const renderSpy = (Response.render = jest.fn(function () {
 			this.end();
 		}));
@@ -98,8 +98,8 @@ describe('OAuth2 API', () => {
 	});

 	it('should handle a valid callback without auth', async () => {
-		const controller = Container.get(OAuth2CredentialController);
-		const csrfSpy = jest.spyOn(controller, 'createCsrfState').mockClear();
+		const oauthService = Container.get(OauthService);
+		const csrfSpy = jest.spyOn(oauthService, 'createCsrfState').mockClear();
 		const renderSpy = (Response.render = jest.fn(function () {
 			this.end();
 		}));