fix(core): Fix evaluation of N8N_SKIP_AUTH_ON_OAUTH_CALLBACK (#16944)

packages/cli/src/config/index.ts

@@ -23,7 +23,7 @@ if (inE2ETests) {
 	globalConfig.publicApi.disabled = true;
 	process.env.SKIP_STATISTICS_EVENTS = 'true';
 	globalConfig.auth.cookie.secure = false;
-	process.env.N8N_SKIP_AUTH_ON_OAUTH_CALLBACK = 'true';
+	process.env.N8N_SKIP_AUTH_ON_OAUTH_CALLBACK = 'false';
 }
 
 // Load schema after process.env has been overwritten

packages/cli/src/controllers/oauth/__tests__/abstract-oauth.controller.test.ts

@@ -0,0 +1,47 @@
+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(true);
+		});
+	});
+
+	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);
+		});
+	});
+});

packages/cli/src/controllers/oauth/abstract-oauth.controller.ts

@@ -34,9 +34,13 @@ type CsrfStateParam = {
 
 const MAX_CSRF_AGE = 5 * Time.minutes.toMilliseconds;
 
-// TODO: Flip this flag in v2
+export function shouldSkipAuthOnOAuthCallback() {
-// https://linear.app/n8n/issue/CAT-329
+	// TODO: Flip this flag in v2 https://linear.app/n8n/issue/CAT-329
-export const skipAuthOnOAuthCallback = process.env.N8N_SKIP_AUTH_ON_OAUTH_CALLBACK !== 'true';
+	const value = process.env.N8N_SKIP_AUTH_ON_OAUTH_CALLBACK?.toLowerCase() ?? 'true';
+	return value === 'true';
+}
+
+export const skipAuthOnOAuthCallback = shouldSkipAuthOnOAuthCallback();
 
 @Service()
 export abstract class AbstractOAuthController {

packages/cli/src/controllers/oauth/oauth2-credential.controller.ts

@@ -5,7 +5,12 @@ import { Response } from 'express';
 import omit from 'lodash/omit';
 import set from 'lodash/set';
 import split from 'lodash/split';
-import { type ICredentialDataDecryptedObject, jsonParse, jsonStringify } from 'n8n-workflow';
+import {
+	ensureError,
+	type ICredentialDataDecryptedObject,
+	jsonParse,
+	jsonStringify,
+} from 'n8n-workflow';
 import pkceChallenge from 'pkce-challenge';
 import * as qs from 'querystring';
 
@@ -149,11 +154,11 @@ export class OAuth2CredentialController extends AbstractOAuthController {
 			});
 
 			return res.render('oauth-callback');
-		} catch (error) {
+		} catch (e) {
+			const error = ensureError(e);
 			return this.renderCallbackError(
 				res,
-				(error as Error).message,
+				error.message,
-				// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
 				'body' in error ? jsonStringify(error.body) : undefined,
 			);
 		}