fix(core): Handle MCP OAuth2 token expiry and PKCE negotiation (#34494)

Co-authored-by: Tran Anh Kiet <22877233+trananhkiet@users.noreply.github.com>
2026-10-30 19:35:36 +08:00 · 2026-07-23 08:21:56 +00:00
parent 91ac921d45
commit c7b90d0051
12 changed files with 751 additions and 31 deletions
@@ -9,6 +9,7 @@ export interface ClientOAuth2TokenData extends Record<string, string | undefined
 	access_token: string;
 	refresh_token: string;
 	expires_in?: string;
+	n8n_expires_at?: string;
 	scope?: string | undefined;
 }

@@ -31,11 +31,15 @@ export interface OAuth2CredentialData {
 	authQueryParameters?: string;
 	additionalBodyProperties?: string;
 	grantType: OAuth2GrantType;
+	/** Whether authorization-code flows should use PKCE in addition to client authentication. */
+	usePkce?: boolean;
 	ignoreSSLIssues?: boolean;
 	tokenExpiredStatusCode?: number;
 	oauthTokenData?: {
 		access_token: string;
 		refresh_token?: string;
+		expires_in?: string;
+		n8n_expires_at?: string;
 		resource?: string;
 	};
 	useDynamicClientRegistration?: boolean;
@@ -31,6 +31,10 @@ vi.mock('@n8n/ai-utilities', async () => {
 const MockedClient = Client as MockedClass<typeof Client>;

 describe('utils', () => {
+	afterEach(() => {
+		vi.restoreAllMocks();
+	});
+
 	describe('tryRefreshOAuth2Token', () => {
 		it('should refresh an OAuth2 token without headers', async () => {
 			const ctx = mockDeep<IExecuteFunctions>();
@@ -150,6 +154,104 @@ describe('utils', () => {
 			expect(result.headers).toBeUndefined();
 		});

+		it('should ignore a provider expiry field when the internal expiry is unknown', async () => {
+			const ctx = mockDeep<IExecuteFunctions>();
+			const credentials = {
+				oauthTokenData: {
+					access_token: 'access-token',
+					refresh_token: 'refresh-token',
+					expires_at: '1700000060',
+				},
+			};
+			ctx.getCredentials.mockResolvedValue(credentials);
+
+			const result = await getAuthHeaders(ctx, 'mcpOAuth2Api');
+
+			expect(result).toEqual({
+				headers: { Authorization: 'Bearer access-token' },
+				credentials,
+			});
+			expect(ctx.helpers.refreshOAuth2Token).not.toHaveBeenCalled();
+		});
+
+		it('should not proactively refresh legacy credentials without an absolute expiry', async () => {
+			const ctx = mockDeep<IExecuteFunctions>();
+			const credentials = {
+				oauthTokenData: {
+					access_token: 'access-token',
+					refresh_token: 'refresh-token',
+					expires_in: '3600',
+				},
+			};
+			ctx.getCredentials.mockResolvedValue(credentials);
+
+			const result = await getAuthHeaders(ctx, 'mcpOAuth2Api');
+
+			expect(result.headers).toEqual({ Authorization: 'Bearer access-token' });
+			expect(ctx.helpers.refreshOAuth2Token).not.toHaveBeenCalled();
+		});
+
+		it('should refresh mcpOAuth2Api credentials before the access token expires', async () => {
+			const now = 1_700_000_000_000;
+			vi.spyOn(Date, 'now').mockReturnValue(now);
+			const ctx = mockDeep<IExecuteFunctions>();
+			const credentials = {
+				oauthTokenData: {
+					access_token: 'access-token',
+					refresh_token: 'refresh-token',
+					n8n_expires_at: String(now + 60_000),
+				},
+			};
+			ctx.getCredentials.mockResolvedValue(credentials);
+			ctx.helpers.refreshOAuth2Token.mockResolvedValue({
+				access_token: 'new-access-token',
+			});
+
+			const result = await getAuthHeaders(ctx, 'mcpOAuth2Api');
+
+			expect(result.headers).toEqual({ Authorization: 'Bearer new-access-token' });
+			expect(ctx.helpers.refreshOAuth2Token).toHaveBeenCalledWith('mcpOAuth2Api');
+		});
+
+		it('should not refresh mcpOAuth2Api credentials when the access token is still valid', async () => {
+			const now = 1_700_000_000_000;
+			vi.spyOn(Date, 'now').mockReturnValue(now);
+			const ctx = mockDeep<IExecuteFunctions>();
+			const credentials = {
+				oauthTokenData: {
+					access_token: 'access-token',
+					refresh_token: 'refresh-token',
+					n8n_expires_at: String(now + 10 * 60_000),
+				},
+			};
+			ctx.getCredentials.mockResolvedValue(credentials);
+
+			const result = await getAuthHeaders(ctx, 'mcpOAuth2Api');
+
+			expect(result.headers).toEqual({ Authorization: 'Bearer access-token' });
+			expect(ctx.helpers.refreshOAuth2Token).not.toHaveBeenCalled();
+		});
+
+		it('should not immediately refresh a short-lived token', async () => {
+			const now = 1_700_000_000_000;
+			vi.spyOn(Date, 'now').mockReturnValue(now);
+			const ctx = mockDeep<IExecuteFunctions>();
+			const credentials = {
+				oauthTokenData: {
+					access_token: 'access-token',
+					refresh_token: 'refresh-token',
+					expires_in: '60',
+					n8n_expires_at: String(now + 60_000),
+				},
+			};
+			ctx.getCredentials.mockResolvedValue(credentials);
+
+			const result = await getAuthHeaders(ctx, 'mcpOAuth2Api');
+
+			expect(result.headers).toEqual({ Authorization: 'Bearer access-token' });
+			expect(ctx.helpers.refreshOAuth2Token).not.toHaveBeenCalled();
+		});
+
 		it('should return the headers and credentials for headerAuth', async () => {
 			const ctx = mockDeep<IExecuteFunctions>();
 			const credentials = {
@@ -73,6 +73,13 @@ type OnUnauthorizedHandler = (
 	headers?: Record<string, string>,
 ) => Promise<Record<string, string> | null>;

+const OAUTH2_REFRESH_BUFFER_MS = 2 * 60 * 1000;
+const OAUTH2_REFRESH_BUFFER_RATIO = 0.1;
+
+type McpOAuth2Credentials = ICredentialDataDecryptedObject & {
+	oauthTokenData?: ClientOAuth2TokenData;
+};
+
 type ConnectMcpClientError =
 	| { type: 'invalid_url'; error: Error }
 	| { type: 'connection'; error: Error }
@@ -338,8 +345,26 @@ function createAuthFetch(
 	};
 }

+function shouldRefreshOAuth2Token(credentials: McpOAuth2Credentials): boolean {
+	const tokenData = credentials.oauthTokenData;
+	if (!tokenData?.refresh_token) return false;
+
+	const expiresAt = Number(tokenData.n8n_expires_at);
+	if (!Number.isFinite(expiresAt)) {
+		return false;
+	}
+
+	const expiresInMs = Number(tokenData.expires_in) * 1000;
+	const refreshBufferMs =
+		Number.isFinite(expiresInMs) && expiresInMs > 0
+			? Math.min(OAUTH2_REFRESH_BUFFER_MS, expiresInMs * OAUTH2_REFRESH_BUFFER_RATIO)
+			: OAUTH2_REFRESH_BUFFER_MS;
+
+	return Date.now() + refreshBufferMs >= expiresAt;
+}
+
 export async function getAuthHeaders(
-	ctx: Pick<IExecuteFunctions, 'getCredentials'>,
+	ctx: IExecuteFunctions | ISupplyDataFunctions | ILoadOptionsFunctions,
 	authentication: McpAuthenticationOption,
 ): Promise<{
 	headers?: Record<string, string>;
@@ -347,11 +372,18 @@ export async function getAuthHeaders(
 }> {
 	if (isMcpOAuth2Authentication(authentication)) {
 		const credentials = await ctx
-			.getCredentials<{ oauthTokenData?: { access_token?: string } }>(authentication)
+			.getCredentials<McpOAuth2Credentials>(authentication)
 			.catch(() => null);

 		if (!credentials) return {};

+		if (shouldRefreshOAuth2Token(credentials)) {
+			const refreshedHeaders = await tryRefreshOAuth2Token(ctx, authentication);
+			if (refreshedHeaders) {
+				return { headers: refreshedHeaders, credentials };
+			}
+		}
+
 		if (!credentials.oauthTokenData?.access_token) {
 			return { credentials };
 		}
@@ -229,6 +229,64 @@ describe('CredentialsHelper', () => {
 				),
 			).rejects.toThrow('save workflow to view');
 		});
+
+		test('preserves PKCE flag negotiated by dynamic client registration', async () => {
+			const credentialType: ICredentialType = {
+				name: 'mcpOAuth2Api',
+				displayName: 'MCP OAuth2 API',
+				properties: [
+					{
+						displayName: 'Use Dynamic Client Registration',
+						name: 'useDynamicClientRegistration',
+						type: 'boolean',
+						default: true,
+					},
+				],
+			};
+			mockNodesAndCredentials.getCredential.calledWith(credentialType.name).mockReturnValue({
+				type: credentialType,
+				sourcePath: '',
+			});
+			const credentialsOverwrites = mock<CredentialsOverwrites>();
+			credentialsOverwrites.applyOverwrite.mockImplementation((_type, data) => data);
+			const helper = new CredentialsHelper(
+				new CredentialTypes(mockNodesAndCredentials),
+				credentialsOverwrites,
+				credentialsRepository,
+				dynamicCredentialProxy,
+				secretsProviderRepository,
+				licenseState,
+				externalSecretsConfig,
+				mock<AiGatewayService>(),
+			);
+
+			const result = await helper.applyDefaultsAndOverwrites(
+				mock<IWorkflowExecuteAdditionalData>({ variables: {} }),
+				{
+					useDynamicClientRegistration: true,
+					clientId: 'registered-client-id',
+					clientSecret: 'registered-secret',
+					authUrl: 'https://auth.example.com/authorize',
+					accessTokenUrl: 'https://auth.example.com/token',
+					grantType: 'authorizationCode',
+					authentication: 'body',
+					usePkce: true,
+				},
+				credentialType.name,
+				'internal',
+			);
+
+			expect(result).toMatchObject({
+				useDynamicClientRegistration: true,
+				clientId: 'registered-client-id',
+				clientSecret: 'registered-secret',
+				authUrl: 'https://auth.example.com/authorize',
+				accessTokenUrl: 'https://auth.example.com/token',
+				grantType: 'authorizationCode',
+				authentication: 'body',
+				usePkce: true,
+			});
+		});
 	});

 	describe('authenticate', () => {
@@ -813,6 +813,197 @@ describe('OAuth2CredentialController', () => {
 			expect(oauthService.encryptAndSaveData).toHaveBeenCalled();
 		});

+		it('should send code_verifier for authorization code flow when PKCE is enabled', async () => {
+			const now = 1_700_000_000_000;
+			vi.spyOn(Date, 'now').mockReturnValue(now);
+			const { ClientOAuth2 } = await import('@n8n/client-oauth2');
+			const mockGetToken = vi.fn().mockResolvedValue({
+				data: {
+					access_token: 'new_token',
+					expires_at: 'provider-expiry',
+					expires_in: 3600,
+				},
+			});
+			vi.mocked(ClientOAuth2).mockImplementation(function () {
+				return { code: { getToken: mockGetToken } } as any;
+			});
+
+			const mockResolvedCredential = mock<CredentialsEntity>({ id: '1' });
+			const mockState = {
+				token: 'token',
+				cid: '1',
+				userId: '123',
+				origin: 'static-credential' as const,
+				createdAt: timestamp,
+				data: 'encrypted-data',
+			};
+			oauthService.resolveCredential.mockResolvedValueOnce([
+				mockResolvedCredential,
+				{ csrfSecret: 'csrf-secret', codeVerifier: 'code_verifier' },
+				{
+					clientId: 'client_id',
+					clientSecret: 'client_secret',
+					authUrl: 'https://example.domain/oauth2/auth',
+					accessTokenUrl: 'https://example.domain/oauth2/token',
+					scope: 'openid',
+					grantType: 'authorizationCode',
+					authentication: 'header',
+					usePkce: true,
+				},
+				mockState,
+				{ csrfSecret: 'csrf-secret', codeVerifier: 'code_verifier' },
+			]);
+			oauthService.getBaseUrl.mockReturnValue('http://localhost:5678/rest/oauth2-credential');
+			externalHooks.run.mockResolvedValue(undefined);
+
+			const req = mock<OAuthRequest.OAuth2Credential.Callback>({
+				query: {
+					code: 'auth_code',
+					state: validState,
+				},
+				originalUrl: '/oauth2-credential/callback?code=auth_code&state=state',
+			});
+
+			await controller.handleCallback(req, res);
+
+			expect(mockGetToken).toHaveBeenCalledWith(
+				expect.stringContaining('code=auth_code'),
+				expect.objectContaining({
+					body: { code_verifier: 'code_verifier' },
+				}),
+			);
+			expect(oauthService.encryptAndSaveData).toHaveBeenCalledWith(
+				mockResolvedCredential,
+				expect.objectContaining({
+					oauthTokenData: expect.objectContaining({
+						expires_at: 'provider-expiry',
+						n8n_expires_at: String(now + 3_600_000),
+					}),
+				}),
+			);
+		});
+
+		it('should send code_verifier with client authentication fields for body-authenticated authorization code flow', async () => {
+			const { ClientOAuth2 } = await import('@n8n/client-oauth2');
+			const mockGetToken = vi.fn().mockResolvedValue({
+				data: { access_token: 'new_token' },
+			});
+			vi.mocked(ClientOAuth2).mockImplementation(function () {
+				return { code: { getToken: mockGetToken } } as any;
+			});
+
+			const mockResolvedCredential = mock<CredentialsEntity>({ id: '1' });
+			oauthService.resolveCredential.mockResolvedValueOnce([
+				mockResolvedCredential,
+				{ csrfSecret: 'csrf-secret', codeVerifier: 'code_verifier' },
+				{
+					clientId: 'client_id',
+					clientSecret: 'client_secret',
+					authUrl: 'https://example.domain/oauth2/auth',
+					accessTokenUrl: 'https://example.domain/oauth2/token',
+					grantType: 'authorizationCode',
+					authentication: 'body',
+					usePkce: true,
+				},
+				{
+					token: 'token',
+					cid: '1',
+					userId: '123',
+					origin: 'static-credential',
+					createdAt: timestamp,
+					data: 'encrypted-data',
+				},
+				{ csrfSecret: 'csrf-secret', codeVerifier: 'code_verifier' },
+			]);
+			oauthService.getBaseUrl.mockReturnValue('http://localhost:5678/rest/oauth2-credential');
+			externalHooks.run.mockResolvedValue(undefined);
+
+			const req = mock<OAuthRequest.OAuth2Credential.Callback>({
+				query: { code: 'auth_code', state: validState },
+				originalUrl: '/oauth2-credential/callback?code=auth_code&state=state',
+			});
+
+			await controller.handleCallback(req, res);
+
+			expect(mockGetToken).toHaveBeenCalledWith(
+				expect.stringContaining('code=auth_code'),
+				expect.objectContaining({
+					body: {
+						code_verifier: 'code_verifier',
+						client_id: 'client_id',
+						client_secret: 'client_secret',
+					},
+				}),
+			);
+			expect(oauthService.encryptAndSaveData).toHaveBeenCalledWith(
+				mockResolvedCredential,
+				expect.objectContaining({
+					oauthTokenData: expect.objectContaining({ access_token: 'new_token' }),
+				}),
+			);
+		});
+
+		it('should preserve the existing absolute expiry when the token response omits expires_in', async () => {
+			const now = 1_700_000_000_000;
+			const existingExpiresAt = String(now + 60_000);
+			vi.spyOn(Date, 'now').mockReturnValue(now);
+			const { ClientOAuth2 } = await import('@n8n/client-oauth2');
+			vi.mocked(ClientOAuth2).mockImplementation(function () {
+				return {
+					code: { getToken: vi.fn().mockResolvedValue({ data: { access_token: 'new_token' } }) },
+				} as any;
+			});
+
+			const mockResolvedCredential = mock<CredentialsEntity>({ id: '1' });
+			oauthService.resolveCredential.mockResolvedValueOnce([
+				mockResolvedCredential,
+				{
+					oauthTokenData: {
+						access_token: 'old_token',
+						expires_in: 3600,
+						n8n_expires_at: existingExpiresAt,
+					},
+				},
+				{
+					clientId: 'client_id',
+					clientSecret: 'client_secret',
+					authUrl: 'https://example.domain/oauth2/auth',
+					accessTokenUrl: 'https://example.domain/oauth2/token',
+					grantType: 'authorizationCode',
+					authentication: 'header',
+				},
+				{
+					token: 'token',
+					cid: '1',
+					userId: '123',
+					origin: 'static-credential',
+					createdAt: timestamp,
+					data: 'encrypted-data',
+				},
+				{ csrfSecret: 'csrf-secret' },
+			]);
+			oauthService.getBaseUrl.mockReturnValue('http://localhost:5678/rest/oauth2-credential');
+			externalHooks.run.mockResolvedValue(undefined);
+
+			const req = mock<OAuthRequest.OAuth2Credential.Callback>({
+				query: { code: 'auth_code', state: validState },
+				originalUrl: '/oauth2-credential/callback?code=auth_code&state=state',
+			});
+
+			await controller.handleCallback(req, res);
+
+			expect(oauthService.encryptAndSaveData).toHaveBeenCalledWith(
+				mockResolvedCredential,
+				expect.objectContaining({
+					oauthTokenData: expect.objectContaining({
+						access_token: 'new_token',
+						expires_in: 3600,
+						n8n_expires_at: existingExpiresAt,
+					}),
+				}),
+			);
+		});
+
 		it('should handle body authentication method', async () => {
 			const { ClientOAuth2 } = await import('@n8n/client-oauth2');
 			const mockGetToken = vi.fn().mockResolvedValue({
@@ -58,7 +58,7 @@ export class OAuth2CredentialController {

 			const oAuthOptions = this.convertCredentialToOptions(oauthCredentials);

-			const isPkce = oauthCredentials.grantType === 'pkce';
+			const isPkce = oauthCredentials.grantType === 'pkce' || oauthCredentials.usePkce === true;
 			const isBodyAuth = oauthCredentials.authentication === 'body';

 			const body: Record<string, string> = { ...(oAuthOptions.body ?? {}) };
@@ -111,6 +111,11 @@ export class OAuth2CredentialController {
 				...tokenResponse,
 			} as ICredentialDataDecryptedObject;

+			const expiresInSeconds = Number(tokenResponse.expires_in);
+			if (Number.isFinite(expiresInSeconds) && expiresInSeconds > 0) {
+				oauthTokenData.n8n_expires_at = String(Date.now() + expiresInSeconds * 1000);
+			}
+
 			if (typeof state.resource === 'string') {
 				oauthTokenData.resource = state.resource;
 			}
@@ -587,8 +587,7 @@ export class CredentialsHelper extends ICredentialsHelper {
 			decryptedData.allowedDomains = decryptedDataOriginal.allowedDomains;
 		}

-		// When using dynamic client registration, fields
-		// for client ID, secret, auth URL, access token URL, grant type and authentication
+		// When using dynamic client registration, OAuth fields negotiated at runtime
 		// are not shown in the UI, so we need to copy them from the original data.
 		if (decryptedData.useDynamicClientRegistration) {
 			decryptedData.clientId = decryptedDataOriginal.clientId;
@@ -597,6 +596,7 @@ export class CredentialsHelper extends ICredentialsHelper {
 			decryptedData.accessTokenUrl = decryptedDataOriginal.accessTokenUrl;
 			decryptedData.grantType = decryptedDataOriginal.grantType;
 			decryptedData.authentication = decryptedDataOriginal.authentication;
+			decryptedData.usePkce = decryptedDataOriginal.usePkce;
 		}

 		const additionalKeys = getAdditionalKeys(additionalData, mode, null, {
@@ -2254,11 +2254,12 @@ describe('OauthService', () => {
 			expect(callArgs[1]).toHaveProperty('authUrl', 'https://example.domain/oauth2/auth');
 			expect(callArgs[1]).toHaveProperty('accessTokenUrl', 'https://example.domain/oauth2/token');
 			expect(callArgs[1]).toHaveProperty('clientId', 'registered_client_id');
-			expect(callArgs[1]).toHaveProperty('clientSecret', 'registered_client_secret');
+			expect(callArgs[1]).not.toHaveProperty('clientSecret');
 			expect(callArgs[1]).toHaveProperty('scope', 'openid profile');
 			expect(callArgs[1]).toHaveProperty('grantType', 'pkce');
 			expect(callArgs[1]).not.toHaveProperty('csrfSecret');
 			expect(callArgs[1]).not.toHaveProperty('codeVerifier');
+			expect(callArgs[2]).toEqual(expect.arrayContaining(['authentication', 'clientSecret']));
 			expect(cacheService.set).toHaveBeenCalledWith(
 				expect.stringMatching(/^oauth:flow:/),
 				expect.objectContaining({
@@ -3729,6 +3730,9 @@ describe('OauthService', () => {
 								const url = new URL(options.authorizationUri ?? '');
 								if (options.resource) url.searchParams.set('resource', options.resource);
 								if (options.state) url.searchParams.set('state', options.state);
+								for (const [key, value] of Object.entries(options.query ?? {})) {
+									if (typeof value === 'string') url.searchParams.set(key, value);
+								}
 								return url.toString();
 							},
 						}),
@@ -3889,6 +3893,7 @@ describe('OauthService', () => {
 					oauthCredentials,
 					'https://auth.example.com/issuer',
 					toUpdate,
+					[],
 				);

 				expect(httpClientMock.get).toHaveBeenNthCalledWith(
@@ -3944,6 +3949,7 @@ describe('OauthService', () => {
 						oauthCredentials,
 						'https://auth.example.com',
 						toUpdate,
+						[],
 					);

 					expect(oauthCredentials.grantType).toBe('clientCredentials');
@@ -3966,6 +3972,35 @@ describe('OauthService', () => {
 				},
 			);

+			it('should clear a stale PKCE flag when selecting client credentials', async () => {
+				const oauthCredentials = makeDcrCredentials({ usePkce: true });
+				const toUpdate = {};
+
+				vi.mocked(httpClientMock.get).mockResolvedValueOnce({
+					data: makeMetadata({
+						grant_types_supported: ['client_credentials'],
+						token_endpoint_auth_methods_supported: ['client_secret_basic'],
+					}),
+				});
+				vi.mocked(httpClientMock.post).mockResolvedValueOnce({
+					data: { client_id: 'registered-client-id', client_secret: 'registered-secret' },
+				});
+
+				await (service as any).performDynamicClientRegistration(
+					oauthCredentials,
+					'https://auth.example.com',
+					toUpdate,
+					[],
+				);
+
+				expect(oauthCredentials).toMatchObject({
+					grantType: 'clientCredentials',
+					usePkce: false,
+				});
+				expect(toUpdate).toEqual(expect.objectContaining({ usePkce: false }));
+				expect((service as any).shouldUsePkce(oauthCredentials)).toBe(false);
+			});
+
 			it('should default client credentials flow to header authentication when the server omits token_endpoint_auth_methods_supported', async () => {
 				const oauthCredentials = makeDcrCredentials();
 				const toUpdate = {};
@@ -3981,6 +4016,7 @@ describe('OauthService', () => {
 					oauthCredentials,
 					'https://auth.example.com',
 					toUpdate,
+					[],
 				);

 				expect(oauthCredentials.grantType).toBe('clientCredentials');
@@ -4017,6 +4053,7 @@ describe('OauthService', () => {
 					oauthCredentials,
 					'https://auth.example.com',
 					toUpdate,
+					[],
 				);

 				expect(oauthCredentials.grantType).toBe('authorizationCode');
@@ -4043,6 +4080,7 @@ describe('OauthService', () => {
 						makeDcrCredentials(),
 						'https://auth.example.com',
 						{},
+						[],
 					),
 				).rejects.toThrow('No supported grant type and authentication method found');
 				expect(httpClientMock.post).not.toHaveBeenCalled();
@@ -4073,14 +4111,17 @@ describe('OauthService', () => {
 							oauthCredentials,
 							'https://auth.example.com',
 							toUpdate,
+							[],
 						);

 						expect(oauthCredentials.grantType).toBe('authorizationCode');
 						expect(oauthCredentials.authentication).toBe(authentication);
+						expect(oauthCredentials.usePkce).toBe(true);
 						expect(toUpdate).toEqual(
 							expect.objectContaining({
 								grantType: 'authorizationCode',
 								authentication,
+								usePkce: true,
 								clientId: 'registered-client-id',
 								clientSecret: 'registered-secret',
 							}),
@@ -4095,6 +4136,49 @@ describe('OauthService', () => {
 					},
 				);

+				it('should honor the advertised order when both client authentication methods are supported', async () => {
+					const oauthCredentials = makeDcrCredentials();
+					const toUpdate = {};
+
+					vi.mocked(httpClientMock.get).mockResolvedValueOnce({
+						data: makeMetadata({
+							grant_types_supported: ['authorization_code'],
+							token_endpoint_auth_methods_supported: ['client_secret_post', 'client_secret_basic'],
+							code_challenge_methods_supported: ['S256'],
+						}),
+					});
+					vi.mocked(httpClientMock.post).mockResolvedValueOnce({
+						data: { client_id: 'registered-client-id', client_secret: 'registered-secret' },
+					});
+
+					await (service as any).performDynamicClientRegistration(
+						oauthCredentials,
+						'https://auth.example.com',
+						toUpdate,
+						[],
+					);
+
+					expect(oauthCredentials.grantType).toBe('authorizationCode');
+					expect(oauthCredentials.authentication).toBe('body');
+					expect(oauthCredentials.usePkce).toBe(true);
+					expect(toUpdate).toEqual(
+						expect.objectContaining({
+							grantType: 'authorizationCode',
+							authentication: 'body',
+							usePkce: true,
+							clientId: 'registered-client-id',
+							clientSecret: 'registered-secret',
+						}),
+					);
+					expect(httpClientMock.post).toHaveBeenCalledWith(
+						'https://auth.example.com/oauth2/register',
+						expect.objectContaining({
+							grant_types: ['authorization_code', 'refresh_token'],
+							token_endpoint_auth_method: 'client_secret_post',
+						}),
+					);
+				});
+
 				it('should register PKCE when the server supports both S256 and the none auth method', async () => {
 					const oauthCredentials = makeDcrCredentials();
 					const toUpdate = {};
@@ -4107,13 +4191,14 @@ describe('OauthService', () => {
 						}),
 					});
 					vi.mocked(httpClientMock.post).mockResolvedValueOnce({
-						data: { client_id: 'registered-client-id' },
+						data: { client_id: 'registered-client-id', client_secret: 'ignored-secret' },
 					});

 					await (service as any).performDynamicClientRegistration(
 						oauthCredentials,
 						'https://auth.example.com',
 						toUpdate,
+						[],
 					);

 					expect(oauthCredentials.grantType).toBe('pkce');
@@ -4126,6 +4211,38 @@ describe('OauthService', () => {
 					);
 				});

+				it('should clear stale client authentication when DCR selects public-client PKCE', async () => {
+					const oauthCredentials = makeDcrCredentials({
+						clientSecret: 'stale-secret',
+						authentication: 'header',
+					});
+					const toUpdate = {};
+					const toDelete: string[] = [];
+
+					vi.mocked(httpClientMock.get).mockResolvedValueOnce({
+						data: makeMetadata({
+							grant_types_supported: ['authorization_code'],
+							token_endpoint_auth_methods_supported: ['none'],
+							code_challenge_methods_supported: ['S256'],
+						}),
+					});
+					vi.mocked(httpClientMock.post).mockResolvedValueOnce({
+						data: { client_id: 'registered-client-id' },
+					});
+
+					await (service as any).performDynamicClientRegistration(
+						oauthCredentials,
+						'https://auth.example.com',
+						toUpdate,
+						toDelete,
+					);
+
+					expect(oauthCredentials.grantType).toBe('pkce');
+					expect(oauthCredentials.clientSecret).toBeUndefined();
+					expect(oauthCredentials.authentication).toBeUndefined();
+					expect(toDelete).toEqual(expect.arrayContaining(['clientSecret', 'authentication']));
+				});
+
 				it('should register PKCE when the server supports S256 and omits token_endpoint_auth_methods_supported', async () => {
 					const oauthCredentials = makeDcrCredentials();
 					const toUpdate = {};
@@ -4144,6 +4261,7 @@ describe('OauthService', () => {
 						oauthCredentials,
 						'https://auth.example.com',
 						toUpdate,
+						[],
 					);

 					expect(oauthCredentials.grantType).toBe('pkce');
@@ -4174,6 +4292,7 @@ describe('OauthService', () => {
 						oauthCredentials,
 						'https://auth.example.com',
 						toUpdate,
+						[],
 					);

 					expect(oauthCredentials.grantType).toBe('pkce');
@@ -4312,6 +4431,7 @@ describe('OauthService', () => {
 					oauthCredentials,
 					'https://as.example.com',
 					{},
+					[],
 				);

 				expect(requestMock).toHaveBeenCalledWith(
@@ -4470,6 +4590,113 @@ describe('OauthService', () => {
 				);
 			});

+			it('should include PKCE parameters for confidential authorization-code clients that advertise S256', async () => {
+				const pkceChallenge = await import('pkce-challenge');
+				vi.mocked(pkceChallenge.default).mockResolvedValue({
+					code_verifier: 'code_verifier',
+					code_challenge: 'code_challenge',
+				});
+				vi.spyOn(service, 'getOAuthCredentials').mockResolvedValue(makeDcrCredentials());
+				vi.mocked(httpClientMock.get).mockResolvedValueOnce({
+					data: {
+						authorization_servers: ['https://auth.example.com'],
+						resource: 'https://mcp.example.com/mcp',
+					},
+				});
+				vi.mocked(httpClientMock.get).mockResolvedValueOnce({
+					data: {
+						authorization_endpoint: 'https://auth.example.com/oauth2/auth',
+						token_endpoint: 'https://auth.example.com/oauth2/token',
+						registration_endpoint: 'https://auth.example.com/oauth2/register',
+						grant_types_supported: ['authorization_code', 'refresh_token'],
+						token_endpoint_auth_methods_supported: ['client_secret_basic'],
+						code_challenge_methods_supported: ['S256'],
+					},
+				});
+				vi.mocked(httpClientMock.post).mockResolvedValueOnce({
+					data: { client_id: 'registered-client-id', client_secret: 'registered-secret' },
+				});
+
+				const authUri = await service.generateAOauth2AuthUri(credential, {
+					cid: credential.id,
+					origin: 'static-credential',
+					userId: 'user-id',
+				});
+
+				const url = new URL(authUri);
+				expect(url.searchParams.get('code_challenge')).toBe('code_challenge');
+				expect(url.searchParams.get('code_challenge_method')).toBe('S256');
+				expect(httpClientMock.post).toHaveBeenCalledWith(
+					'https://auth.example.com/oauth2/register',
+					expect.objectContaining({
+						token_endpoint_auth_method: 'client_secret_basic',
+					}),
+				);
+				expect(service.encryptAndSaveData).toHaveBeenCalledWith(
+					expect.anything(),
+					expect.objectContaining({
+						grantType: 'authorizationCode',
+						authentication: 'header',
+						usePkce: true,
+					}),
+					[],
+				);
+				expect(cacheService.set).toHaveBeenLastCalledWith(
+					expect.any(String),
+					expect.objectContaining({ codeVerifier: 'code_verifier' }),
+					expect.any(Number),
+				);
+			});
+
+			it('should persist stale client authentication cleanup when DCR selects public-client PKCE', async () => {
+				const pkceChallenge = await import('pkce-challenge');
+				vi.mocked(pkceChallenge.default).mockResolvedValue({
+					code_verifier: 'code_verifier',
+					code_challenge: 'code_challenge',
+				});
+				vi.spyOn(service, 'getOAuthCredentials').mockResolvedValue(
+					makeDcrCredentials({
+						clientSecret: 'stale-secret',
+						authentication: 'header',
+					}),
+				);
+				vi.mocked(httpClientMock.get).mockResolvedValueOnce({
+					data: {
+						authorization_servers: ['https://auth.example.com'],
+						resource: 'https://mcp.example.com/mcp',
+					},
+				});
+				vi.mocked(httpClientMock.get).mockResolvedValueOnce({
+					data: {
+						authorization_endpoint: 'https://auth.example.com/oauth2/auth',
+						token_endpoint: 'https://auth.example.com/oauth2/token',
+						registration_endpoint: 'https://auth.example.com/oauth2/register',
+						grant_types_supported: ['authorization_code'],
+						token_endpoint_auth_methods_supported: ['none'],
+						code_challenge_methods_supported: ['S256'],
+					},
+				});
+				vi.mocked(httpClientMock.post).mockResolvedValueOnce({
+					data: { client_id: 'registered-client-id' },
+				});
+
+				await service.generateAOauth2AuthUri(credential, {
+					cid: credential.id,
+					origin: 'static-credential',
+					userId: 'user-id',
+				});
+
+				expect(service.encryptAndSaveData).toHaveBeenCalledWith(
+					expect.anything(),
+					expect.objectContaining({
+						grantType: 'pkce',
+						usePkce: true,
+						clientId: 'registered-client-id',
+					}),
+					expect.arrayContaining(['authentication', 'clientSecret']),
+				);
+			});
+
 			it('should omit resource when discovery and credential input do not provide one', async () => {
 				vi.spyOn(service, 'getOAuthCredentials').mockResolvedValue(makeDcrCredentials());
 				vi.mocked(httpClientMock.get).mockResolvedValueOnce({
@@ -905,6 +905,7 @@ export class OauthService {
 			await this.getOAuthCredentials<OAuth2CredentialData>(credential);

 		const toUpdate: ICredentialDataDecryptedObject = {};
+		const toDelete: string[] = [];

 		let authorizationServerUrl = oauthCredentials.serverUrl;
 		let discoveredScopes: string[] | undefined;
@@ -929,6 +930,7 @@ export class OauthService {
 				oauthCredentials,
 				authorizationServerUrl!,
 				toUpdate,
+				toDelete,
 				discoveredScopes,
 			);
 		}
@@ -951,7 +953,7 @@ export class OauthService {
 		await this.externalHooks.run('oauth2.authenticate', [oAuthOptions]);

 		const flowState: OauthFlowState = { csrfSecret, stateData: csrfData };
-		if (oauthCredentials.grantType === 'pkce') {
+		if (this.shouldUsePkce(oauthCredentials)) {
 			const { code_verifier, code_challenge } = await pkceChallenge();
 			oAuthOptions.query = {
 				...oAuthOptions.query,
@@ -965,8 +967,8 @@ export class OauthService {

 		// Only persist DCR-driven updates to the credential. CSRF/PKCE state lives in the cache
 		// to avoid cross-user races on shared credentials.
-		if (Object.keys(toUpdate).length > 0) {
-			await this.encryptAndSaveData(credential, toUpdate);
+		if (Object.keys(toUpdate).length > 0 || toDelete.length > 0) {
+			await this.encryptAndSaveData(credential, toUpdate, toDelete);
 		}

 		const oAuthObj = new ClientOAuth2(oAuthOptions);
@@ -984,6 +986,7 @@ export class OauthService {
 		oauthCredentials: OAuth2CredentialData,
 		authorizationServerUrl: string,
 		toUpdate: ICredentialDataDecryptedObject,
+		toDelete: string[],
 		discoveredResourceScopes?: string[],
 	): Promise<void> {
 		// Step 2: Discover Authorization Server Metadata (RFC 8414 / OpenID Connect)
@@ -1062,16 +1065,21 @@ export class OauthService {
 			toUpdate.scope = scope;
 		}

-		const { grantType, authentication } = this.selectGrantTypeAndAuthenticationMethod(
+		const { grantType, authentication, usePkce } = this.selectGrantTypeAndAuthenticationMethod(
 			metadataValidation.data.grant_types_supported ?? ['authorization_code', 'implicit'],
 			metadataValidation.data.token_endpoint_auth_methods_supported ?? [],
 			metadataValidation.data.code_challenge_methods_supported ?? [],
 		);
 		oauthCredentials.grantType = grantType;
 		toUpdate.grantType = grantType;
+		oauthCredentials.usePkce = usePkce;
+		toUpdate.usePkce = usePkce;
 		if (authentication) {
 			oauthCredentials.authentication = authentication;
 			toUpdate.authentication = authentication;
+		} else {
+			delete oauthCredentials.authentication;
+			toDelete.push('authentication');
 		}

 		const { grant_types, token_endpoint_auth_method } = this.mapGrantTypeAndAuthenticationMethod(
@@ -1110,9 +1118,12 @@ export class OauthService {
 		const { client_id, client_secret } = registrationValidation.data;
 		oauthCredentials.clientId = client_id;
 		toUpdate.clientId = client_id;
-		if (client_secret) {
+		if (authentication && client_secret) {
 			oauthCredentials.clientSecret = client_secret;
 			toUpdate.clientSecret = client_secret;
+		} else {
+			delete oauthCredentials.clientSecret;
+			toDelete.push('clientSecret');
 		}
 	}

@@ -1299,6 +1310,13 @@ export class OauthService {
 		return options;
 	}

+	private shouldUsePkce(credential: OAuth2CredentialData): boolean {
+		return (
+			credential.grantType === 'pkce' ||
+			(credential.grantType === 'authorizationCode' && credential.usePkce === true)
+		);
+	}
+
 	/**
 	 * Fetches a `.well-known` discovery document and returns its parsed JSON body.
 	 * Only a 200 is accepted (RFC 8414 / RFC 9728 / OpenID Connect discovery endpoints respond with 200).
@@ -1391,7 +1409,11 @@ export class OauthService {
 		grantTypes: string[],
 		tokenEndpointAuthMethods: string[],
 		codeChallengeMethods: string[],
-	): { grantType: OAuth2GrantType; authentication?: OAuth2AuthenticationMethod } {
+	): {
+		grantType: OAuth2GrantType;
+		authentication?: OAuth2AuthenticationMethod;
+		usePkce: boolean;
+	} {
 		const supportsPkce = codeChallengeMethods.includes('S256');

 		if (grantTypes.includes('authorization_code')) {
@@ -1401,46 +1423,55 @@ export class OauthService {
 				supportsPkce &&
 				(tokenEndpointAuthMethods.length === 0 || tokenEndpointAuthMethods.includes('none'))
 			) {
-				return { grantType: 'pkce' };
+				return { grantType: 'pkce', usePkce: true };
 			}

-			if (tokenEndpointAuthMethods.includes('client_secret_basic')) {
-				return { grantType: 'authorizationCode', authentication: 'header' };
-			}
-
-			if (tokenEndpointAuthMethods.includes('client_secret_post')) {
-				return { grantType: 'authorizationCode', authentication: 'body' };
+			const authentication = this.selectClientSecretAuthenticationMethod(tokenEndpointAuthMethods);
+			if (authentication) {
+				return {
+					grantType: 'authorizationCode',
+					authentication,
+					usePkce: supportsPkce,
+				};
 			}

 			// S256 advertised alongside only unrecognized methods: fall back to public-client PKCE.
 			if (supportsPkce) {
-				return { grantType: 'pkce' };
+				return { grantType: 'pkce', usePkce: true };
 			}

 			// Server omitted token_endpoint_auth_methods_supported: default to client_secret_basic (RFC 8414).
 			if (tokenEndpointAuthMethods.length === 0) {
-				return { grantType: 'authorizationCode', authentication: 'header' };
+				return { grantType: 'authorizationCode', authentication: 'header', usePkce: false };
 			}
 		}

 		if (grantTypes.includes('client_credentials')) {
-			if (tokenEndpointAuthMethods.includes('client_secret_basic')) {
-				return { grantType: 'clientCredentials', authentication: 'header' };
-			}
-
-			if (tokenEndpointAuthMethods.includes('client_secret_post')) {
-				return { grantType: 'clientCredentials', authentication: 'body' };
+			const authentication = this.selectClientSecretAuthenticationMethod(tokenEndpointAuthMethods);
+			if (authentication) {
+				return { grantType: 'clientCredentials', authentication, usePkce: false };
 			}

 			// Server omitted token_endpoint_auth_methods_supported: default to client_secret_basic (RFC 8414).
 			if (tokenEndpointAuthMethods.length === 0) {
-				return { grantType: 'clientCredentials', authentication: 'header' };
+				return { grantType: 'clientCredentials', authentication: 'header', usePkce: false };
 			}
 		}

 		throw new BadRequestError('No supported grant type and authentication method found');
 	}

+	private selectClientSecretAuthenticationMethod(
+		tokenEndpointAuthMethods: string[],
+	): OAuth2AuthenticationMethod | undefined {
+		for (const authMethod of tokenEndpointAuthMethods) {
+			if (authMethod === 'client_secret_basic') return 'header';
+			if (authMethod === 'client_secret_post') return 'body';
+		}
+
+		return undefined;
+	}
+
 	private mapGrantTypeAndAuthenticationMethod(
 		grantType: OAuth2GrantType,
 		authentication?: OAuth2AuthenticationMethod,
@@ -58,6 +58,10 @@ describe('refreshOAuth2Token', () => {
 		} as unknown as ICredentialDataDecryptedObject);
 	});

+	afterEach(() => {
+		vi.restoreAllMocks();
+	});
+
 	test('should refresh the OAuth2 token with pkce grant type', async () => {
 		mockThis.getCredentials.mockResolvedValue({
 			...mockCredentialData,
@@ -101,6 +105,54 @@ describe('refreshOAuth2Token', () => {
 		);
 	});

+	test('should store an absolute expiry when refreshed token has expires_in', async () => {
+		const now = 1_700_000_000_000;
+		const dateNow = vi.spyOn(Date, 'now').mockReturnValue(now);
+		mockThis.getCredentials.mockResolvedValue({
+			...mockCredentialData,
+			clientSecret: undefined,
+			grantType: 'pkce',
+		});
+		nock(baseUrl)
+			.post('/token', {
+				client_id: 'test-client-id',
+				grant_type: 'refresh_token',
+				refresh_token: 'old-refresh-token',
+			})
+			.reply(200, {
+				access_token: 'new-token',
+				refresh_token: 'new-refresh-token',
+				expires_in: '3600',
+			});
+
+		const result = await refreshOAuth2Token.call(
+			mockThis,
+			'test-credentials-type',
+			mockNode,
+			mockAdditionalData,
+		);
+
+		expect(result).toEqual({
+			access_token: 'new-token',
+			refresh_token: 'new-refresh-token',
+			expires_in: '3600',
+			n8n_expires_at: String(now + 3_600_000),
+		});
+		expect(
+			mockAdditionalData.credentialsHelper.updateCredentialsOauthTokenData,
+		).toHaveBeenCalledWith(
+			mockNode.credentials!['test-credentials-type'],
+			'test-credentials-type',
+			expect.objectContaining({
+				oauthTokenData: expect.objectContaining({
+					n8n_expires_at: String(now + 3_600_000),
+				}),
+			}),
+			mockAdditionalData,
+		);
+		dateNow.mockRestore();
+	});
+
 	test('should refresh the OAuth2 token with client credentials grant type', async () => {
 		mockThis.getCredentials.mockResolvedValue({
 			...mockCredentialData,
@@ -176,6 +228,7 @@ describe('refreshOAuth2Token', () => {
 		expect(result).toEqual({
 			access_token: 'new-token',
 			refresh_token: 'new-refresh-token',
+			resource: 'https://mcp.example.com/',
 		});
 		expect(
 			mockAdditionalData.credentialsHelper.updateCredentialsOauthTokenData,
@@ -84,6 +84,18 @@ function buildSigningToken(
 	);
 }

+function addExpiresAt(tokenData: ClientOAuth2TokenData): ClientOAuth2TokenData {
+	const expiresInSeconds = Number(tokenData.expires_in);
+	if (!Number.isFinite(expiresInSeconds) || expiresInSeconds <= 0) {
+		return tokenData;
+	}
+
+	return {
+		...tokenData,
+		n8n_expires_at: String(Date.now() + expiresInSeconds * 1000),
+	};
+}
+
 interface RefreshOAuth2TokenContext {
 	credentials: OAuth2CredentialData;
 	token: ClientOAuth2Token;
@@ -228,7 +240,7 @@ async function refreshOrFetchToken(ctx: RefreshOAuth2TokenContext): Promise<Clie
 		// Merge old and new token data so fields that the authorization server
 		// does not echo back on refresh (e.g. `resource`) are preserved from the
 		// original token response.
-		const newOAuthTokenData = { ...token.data, ...newToken.data };
+		const newOAuthTokenData = addExpiresAt({ ...token.data, ...newToken.data });

 		// If the server doesn't echo the resource back, restore it from the
 		// previous token data to ensure it's not lost on refresh.
@@ -253,7 +265,11 @@ async function refreshOrFetchToken(ctx: RefreshOAuth2TokenContext): Promise<Clie
 			credentials as unknown as ICredentialDataDecryptedObject,
 			credentialsType,
 		);
-		let signingToken = newToken;
+		let signingToken = buildSigningToken(
+			token.client,
+			credentials.oauthTokenData as ClientOAuth2TokenData,
+			oAuth2Options,
+		);
 		if (preAuthData) {
 			Object.assign(credentials, preAuthData);
 			signingToken = buildSigningToken(