fix(core): Skip the preAuthentication retry while the stored token is fresh (no-changelog) (#38927)

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

packages/core/src/execution-engine/node-execution-context/utils/request-helpers/__tests__/authentication.test.ts

@@ -125,6 +125,36 @@ describe('httpRequestWithAuthentication', () => {
 		);
 	});
 
+	// An explicit preAuthenticationRetryStatusCode replaces the default, so a caller can leave 401 out.
+	test('does NOT refresh on 401 when preAuthenticationRetryStatusCode excludes it', async () => {
+		mockAdditionalData.credentialsHelper.getParentTypes.mockReturnValue([]);
+		mockThis.getCredentials.mockResolvedValue({ sessionToken: 'stale' });
+		const requestOptions: IHttpRequestOptions = { method: 'GET', url: `${baseUrl}/items` };
+		mockAdditionalData.credentialsHelper.authenticate.mockResolvedValue(requestOptions);
+
+		const error401 = Object.assign(new Error('401 - session expired'), {
+			response: { status: 401 },
+		});
+		request.mockRejectedValueOnce(error401);
+
+		await expect(
+			httpRequestWithAuthentication.call(
+				mockThis,
+				'testSessionAuth',
+				requestOptions,
+				mockWorkflow,
+				mockNode,
+				mockAdditionalData,
+				{ preAuthenticationRetryStatusCode: [403, 404] },
+			),
+		).rejects.toSatisfy(
+			(thrown: unknown) => thrown instanceof NodeApiError && thrown.cause === error401,
+		);
+
+		expect(request).toHaveBeenCalledTimes(1);
+		expect(mockAdditionalData.credentialsHelper.preAuthentication).toHaveBeenCalledTimes(1);
+	});
+
 	// By default this path's generic preAuthentication retry only fires on exactly
 	// 401 (unlike the legacy requestWithAuthentication, which retries on any error —
 	// see the "requestWithAuthentication (legacy)" suite below) — a 404/403 here
@@ -196,6 +226,84 @@ describe('httpRequestWithAuthentication', () => {
 		expect(request).toHaveBeenCalledTimes(2);
 	});
 
+	// `skipPreAuthenticationRetryWhileTokenIsFresh`: a 403/404 also means "does not exist", so
+	// gate those on the expiry the credential stored, or a batch of missing resources costs one
+	// token exchange plus one credential write each.
+	describe('skipPreAuthenticationRetryWhileTokenIsFresh', () => {
+		const options = {
+			preAuthenticationRetryStatusCode: [401, 403, 404],
+			skipPreAuthenticationRetryWhileTokenIsFresh: true,
+		};
+
+		const runWith = async (
+			credentials: ICredentialDataDecryptedObject,
+			status: number,
+			requestOptions: IHttpRequestOptions = { method: 'GET', url: `${baseUrl}/items` },
+		) => {
+			mockAdditionalData.credentialsHelper.getParentTypes.mockReturnValue([]);
+			mockThis.getCredentials.mockResolvedValue(credentials);
+			mockAdditionalData.credentialsHelper.preAuthentication
+				.mockResolvedValueOnce(undefined)
+				.mockResolvedValueOnce({ accessToken: 'fresh' });
+			mockAdditionalData.credentialsHelper.authenticate.mockResolvedValue(requestOptions);
+			request
+				.mockRejectedValueOnce(
+					Object.assign(new Error(`${status} - failed`), { response: { status } }),
+				)
+				.mockResolvedValueOnce({ ok: true });
+
+			return await httpRequestWithAuthentication.call(
+				mockThis,
+				'testServiceAccount',
+				requestOptions,
+				mockWorkflow,
+				mockNode,
+				mockAdditionalData,
+				options,
+			);
+		};
+
+		test('does not retry a 404 while the stored token is still live', async () => {
+			await expect(
+				runWith({ accessToken: 'live', n8n_expires_at: String(Date.now() + 3_600_000) }, 404),
+			).rejects.toThrow(NodeApiError);
+
+			expect(request).toHaveBeenCalledTimes(1);
+			// only the unforced call that runs before the request
+			expect(mockAdditionalData.credentialsHelper.preAuthentication).toHaveBeenCalledTimes(1);
+		});
+
+		test('retries a 404 once the stored token is at its expiry', async () => {
+			const result = await runWith(
+				{ accessToken: 'stale', n8n_expires_at: String(Date.now() - 1_000) },
+				404,
+			);
+
+			expect(result).toEqual({ ok: true });
+			expect(request).toHaveBeenCalledTimes(2);
+		});
+
+		test.each([
+			['unknown', {}],
+			['unparsable', { n8n_expires_at: 'tomorrow' }],
+		])('retries a 404 when the stored expiry is %s', async (_label, stored) => {
+			const result = await runWith({ accessToken: 'stale', ...stored }, 404);
+
+			expect(result).toEqual({ ok: true });
+			expect(request).toHaveBeenCalledTimes(2);
+		});
+
+		test('still retries a 401 while the stored token reads as live, since the server rejected it', async () => {
+			const result = await runWith(
+				{ accessToken: 'revoked', n8n_expires_at: String(Date.now() + 3_600_000) },
+				401,
+			);
+
+			expect(result).toEqual({ ok: true });
+			expect(request).toHaveBeenCalledTimes(2);
+		});
+	});
+
 	test('refreshes but does NOT resend a drained form-data body on 401; the original error surfaces', async () => {
 		mockAdditionalData.credentialsHelper.getParentTypes.mockReturnValue([]);
 		mockThis.getCredentials.mockResolvedValue({ sessionToken: 'stale' });

packages/core/src/execution-engine/node-execution-context/utils/request-helpers/authentication.ts

@@ -18,7 +18,35 @@ import { ExecutionBaseError, NodeApiError, NodeOperationError } from 'n8n-workfl
 import { callEvalMockHandler, normalizeLegacyRequest } from '@/execution-engine/eval-mock-helpers';
 
 import { proxyRequestToAxios } from './legacy-request-adapter';
-import { hasSingleUseBody, isTokenExpiredStatusCode, requestOAuth1, requestOAuth2 } from './oauth';
+import {
+	hasSingleUseBody,
+	isTokenExpiryInFuture,
+	isTokenExpiredStatusCode,
+	requestOAuth1,
+	requestOAuth2,
+} from './oauth';
+
+/**
+ * Whether a failed request earns the generic `preAuthentication` refresh-and-resend. Only the
+ * statuses in `preAuthenticationRetryStatusCode` (default 401) do; an explicit list replaces the
+ * default. A 401 means the server rejected the token, so it skips the expiry gate. Any other
+ * configured status can be ambiguous (a gateway that answers 404 both for an expired token and for
+ * a resource that does not exist), so `skipPreAuthenticationRetryWhileTokenIsFresh` lets a caller
+ * gate those on the expiry the credential stored in `n8n_expires_at`, instead of paying a token
+ * exchange and a credential write per missing resource. An absent or unparsable expiry still
+ * retries.
+ */
+function shouldRetryAfterPreAuthentication(
+	status: unknown,
+	credentials: ICredentialDataDecryptedObject,
+	options?: IAdditionalCredentialOptions,
+): boolean {
+	if (!isTokenExpiredStatusCode(status, options?.preAuthenticationRetryStatusCode ?? 401)) {
+		return false;
+	}
+	if (status === 401 || options?.skipPreAuthenticationRetryWhileTokenIsFresh !== true) return true;
+	return !isTokenExpiryInFuture(credentials.n8n_expires_at);
+}
 
 export async function httpRequestWithAuthentication(
 	this: IAllExecuteFunctions,
@@ -110,16 +138,16 @@ export async function httpRequestWithAuthentication(
 		// if there is a pre authorization method defined and
 		// the method failed due to unauthorized request
 		if (
-			// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
-			isTokenExpiredStatusCode(
-				error.response?.status,
-				additionalCredentialOptions?.preAuthenticationRetryStatusCode ?? 401,
-			) &&
 			additionalData.credentialsHelper.preAuthentication !== undefined &&
 			// OAuth 401s are already retried inside requestOAuth1/2 and leave
 			// credentialsDecrypted unset; with nothing refreshed, resending the same
 			// request (possibly with a consumed single-use body) could only fail again
-			credentialsDecrypted !== undefined
+			credentialsDecrypted !== undefined &&
+			shouldRetryAfterPreAuthentication(
+				error.response?.status,
+				credentialsDecrypted,
+				additionalCredentialOptions,
+			)
 		) {
 			try {
 				// try to refresh the credentials

packages/core/src/execution-engine/node-execution-context/utils/request-helpers/oauth.ts

@@ -340,9 +340,9 @@ const TOKEN_EXPIRY_BUFFER_MS = 60_000;
  * Whether the stored token still has time left on it. An absent or unparsable expiry counts as
  * expired, so callers fall back to refreshing when the expiry is unknown.
  */
-function isStoredTokenUnexpired(credentials: OAuth2CredentialData): boolean {
-	const expiresAt = Number(credentials.oauthTokenData?.n8n_expires_at);
-	return Number.isFinite(expiresAt) && Date.now() + TOKEN_EXPIRY_BUFFER_MS < expiresAt;
+export function isTokenExpiryInFuture(expiresAt: unknown): boolean {
+	const timestamp = Number(expiresAt);
+	return Number.isFinite(timestamp) && Date.now() + TOKEN_EXPIRY_BUFFER_MS < timestamp;
 }
 
 function isSingleUseValue(value: unknown): boolean {
@@ -503,7 +503,7 @@ export async function requestOAuth2(
 		if (shouldSkipTokenRefresh) return false;
 		if (!isTokenExpiredStatusCode(status, tokenExpiredStatusCode)) return false;
 		if (status === 401 || oAuth2Options?.skipRefreshWhileTokenIsFresh !== true) return true;
-		return !isStoredTokenUnexpired(credentials);
+		return !isTokenExpiryInFuture(credentials.oauthTokenData?.n8n_expires_at);
 	};
 
 	const refreshCtx: RefreshOAuth2TokenContext = {

packages/nodes-base/credentials/AtlassianServiceAccountApi.credentials.ts

@@ -18,7 +18,20 @@ import {
 
 const TOKEN_URL = 'https://auth.atlassian.com/oauth/token';
 
-export async function getAccessToken(credentials: ICredentialDataDecryptedObject): Promise<string> {
+/**
+ * Epoch-millisecond expiry for the minted token, stored alongside it so core can tell a live
+ * token from one that has to be renewed when the gateway answers an ambiguous 403/404 (ENT-408).
+ * An omitted or unusable `expires_in` yields '', which core reads as "unknown" and retries on.
+ */
+function expiresAtFrom(response: object): string {
+	const expiresIn = Number((response as { expires_in?: unknown }).expires_in);
+	if (!Number.isFinite(expiresIn) || expiresIn <= 0) return '';
+	return String(Date.now() + expiresIn * 1000);
+}
+
+export async function getAccessToken(
+	credentials: ICredentialDataDecryptedObject,
+): Promise<{ accessToken: string; n8n_expires_at: string }> {
 	const stringOrEmpty = (value: unknown) => (typeof value === 'string' ? value.trim() : '');
 	const clientId = stringOrEmpty(credentials.clientId);
 	const clientSecret = stringOrEmpty(credentials.clientSecret);
@@ -61,7 +74,7 @@ export async function getAccessToken(credentials: ICredentialDataDecryptedObject
 		throw new OperationalError('Atlassian authentication did not return an access token');
 	}
 
-	return response.access_token;
+	return { accessToken: response.access_token, n8n_expires_at: expiresAtFrom(response) };
 }
 
 export class AtlassianServiceAccountApi implements ICredentialType {
@@ -83,6 +96,14 @@ export class AtlassianServiceAccountApi implements ICredentialType {
 			},
 			default: '',
 		},
+		{
+			// Written by `preAuthentication`, read by core's refresh-and-resend gate. Declared so it
+			// survives `applyDefaultsAndOverwrites`, which drops undeclared credential fields.
+			displayName: 'Access Token Expires At',
+			name: 'n8n_expires_at',
+			type: 'hidden',
+			default: '',
+		},
 		{
 			displayName:
 				"The service account's scopes are chosen when its OAuth 2.0 credential is created in Atlassian administration and can't be changed afterwards. Product permissions still apply on top — grant the service account access to projects and spaces like any other user.",
@@ -114,8 +135,7 @@ export class AtlassianServiceAccountApi implements ICredentialType {
 	];
 
 	async preAuthentication(this: IHttpRequestHelper, credentials: ICredentialDataDecryptedObject) {
-		const accessToken = await getAccessToken(credentials);
-		return { accessToken };
+		return await getAccessToken(credentials);
 	}
 
 	async authenticate(

packages/nodes-base/credentials/test/AtlassianServiceAccountApi.credentials.test.ts

@@ -75,7 +75,9 @@ describe('AtlassianServiceAccountApi Credential', () => {
 		it('exchanges client credentials for an access token without a scope parameter', async () => {
 			const result = await callPreAuthentication(baseCredentials);
 
-			expect(result).toEqual({ accessToken: 'abc' });
+			expect(result.accessToken).toBe('abc');
+			// Stored so core can skip the refresh-and-resend while the token is still live
+			expect(Number(result.n8n_expires_at)).toBeGreaterThan(Date.now() + 3_500_000);
 			expect(requestMock).toHaveBeenCalledTimes(1);
 			const options = requestMock.mock.calls[0][0] as IHttpRequestOptions;
 			expect(options.url).toBe('https://auth.atlassian.com/oauth/token');
@@ -91,6 +93,22 @@ describe('AtlassianServiceAccountApi Credential', () => {
 			expect(requestsMock).toHaveBeenCalledWith({ useDefaultSsrfPolicy: 'unsafe' });
 		});
 
+		it.each([
+			['omitted', {}],
+			['zero', { expires_in: 0 }],
+			['not a number', { expires_in: 'soon' }],
+		])(
+			'leaves the expiry empty when expires_in is %s, so core keeps retrying',
+			async (_label, extra) => {
+				requestMock.mockResolvedValue({ access_token: 'abc', ...extra });
+
+				expect(await getAccessToken(baseCredentials)).toEqual({
+					accessToken: 'abc',
+					n8n_expires_at: '',
+				});
+			},
+		);
+
 		it('trims whitespace from the client ID and secret', async () => {
 			await getAccessToken({ ...baseCredentials, clientId: '  client-id  ' });
 

packages/nodes-base/nodes/Confluence/test/transport/index.test.ts

@@ -666,7 +666,10 @@ describe('credential routing (authentication selector)', () => {
 		expect(mockHttpRequestWithAuthentication).toHaveBeenLastCalledWith(
 			'atlassianServiceAccountApi',
 			expect.anything(),
-			{ preAuthenticationRetryStatusCode: [401, 403, 404] },
+			{
+				preAuthenticationRetryStatusCode: [401, 403, 404],
+				skipPreAuthenticationRetryWhileTokenIsFresh: true,
+			},
 		);
 	});
 

packages/nodes-base/nodes/Confluence/transport/index.ts

@@ -18,13 +18,16 @@ export const SERVICE_ACCOUNT_CREDENTIAL_NAME = 'atlassianServiceAccountApi';
 
 // The gateway answers 403/404 for an expired token, not the 401 n8n's credential-refresh
 // helpers look for (ENT-408). 401 stays, since a revoked token still gets one, and
-// `skipRefreshWhileTokenIsFresh` keeps a genuinely missing page from paying for a refresh.
-// The two credentials take different refresh paths, hence the two option names.
+// the `skip...WhileTokenIsFresh` options keep a genuinely missing page from paying for a refresh.
+// The two credentials take different refresh paths, hence the two sets of option names.
 const ADDITIONAL_CREDENTIAL_OPTIONS: Record<string, IAdditionalCredentialOptions> = {
 	[CONFLUENCE_CREDENTIAL_NAME]: {
 		oauth2: { tokenExpiredStatusCode: [401, 403, 404], skipRefreshWhileTokenIsFresh: true },
 	},
-	[SERVICE_ACCOUNT_CREDENTIAL_NAME]: { preAuthenticationRetryStatusCode: [401, 403, 404] },
+	[SERVICE_ACCOUNT_CREDENTIAL_NAME]: {
+		preAuthenticationRetryStatusCode: [401, 403, 404],
+		skipPreAuthenticationRetryWhileTokenIsFresh: true,
+	},
 };
 
 /**

packages/workflow/src/interfaces.ts

@@ -56,6 +56,14 @@ export interface IAdditionalCredentialOptions {
 	 * override when a gateway signals an expired token with a different status.
 	 */
 	preAuthenticationRetryStatusCode?: number | number[];
+	/**
+	 * Whether a `preAuthenticationRetryStatusCode` other than 401 only forces the
+	 * refresh-and-resend when the token the credential stored in `n8n_expires_at` is at or past
+	 * its expiry. The counterpart of `IOAuth2Options.skipRefreshWhileTokenIsFresh`, for
+	 * credentials that mint their own token in `preAuthentication`. A listed 401 and an unknown
+	 * expiry still retry.
+	 */
+	skipPreAuthenticationRetryWhileTokenIsFresh?: boolean;
 }
 
 export type IAllExecuteFunctions =