chore(core): SSO logins are considered MFA logins (#18347)

packages/cli/src/sso.ee/oidc/routes/__tests__/oidc.controller.ee.test.ts

@@ -0,0 +1,146 @@
+import type { User } from '@n8n/db';
+import { type Request, type Response } from 'express';
+import { mock } from 'jest-mock-extended';
+
+import type { AuthService } from '@/auth/auth.service';
+import type { UrlService } from '@/services/url.service';
+
+import type { OidcService } from '../../oidc.service.ee';
+import { OidcController } from '../oidc.controller.ee';
+
+const authService = mock<AuthService>();
+const oidcService = mock<OidcService>();
+const urlService = mock<UrlService>();
+const controller = new OidcController(oidcService, authService, urlService);
+
+const user = mock<User>({
+	id: '456',
+	email: 'oidc-user@example.com',
+	firstName: 'OIDC',
+	lastName: 'User',
+	password: 'password',
+	authIdentities: [],
+	role: 'global:member',
+});
+
+describe('OidcController', () => {
+	beforeEach(() => {
+		jest.clearAllMocks();
+
+		// Mock URL service
+		urlService.getInstanceBaseUrl.mockReturnValue('http://localhost:5678');
+	});
+
+	describe('callbackHandler', () => {
+		test('Should issue cookie with MFA flag set to true on successful OIDC login', async () => {
+			const req = mock<Request>({
+				originalUrl: '/sso/oidc/callback?code=auth_code&state=state_value',
+			});
+			const res = mock<Response>();
+
+			const expectedCallbackUrl = new URL(
+				'http://localhost:5678/sso/oidc/callback?code=auth_code&state=state_value',
+			);
+
+			// Mock successful OIDC login
+			oidcService.loginUser.mockResolvedValueOnce(user);
+
+			await controller.callbackHandler(req, res);
+
+			// Verify that loginUser was called with the correct callback URL
+			expect(oidcService.loginUser).toHaveBeenCalledWith(expectedCallbackUrl);
+
+			// Verify that issueCookie was called with MFA flag set to true
+			expect(authService.issueCookie).toHaveBeenCalledWith(res, user, true);
+
+			// Verify redirect to home page
+			expect(res.redirect).toHaveBeenCalledWith('/');
+		});
+
+		test('Should handle callback URL with different query parameters', async () => {
+			const req = mock<Request>({
+				originalUrl:
+					'/sso/oidc/callback?code=different_code&state=different_state&session_state=session123',
+			});
+			const res = mock<Response>();
+
+			const expectedCallbackUrl = new URL(
+				'http://localhost:5678/sso/oidc/callback?code=different_code&state=different_state&session_state=session123',
+			);
+
+			oidcService.loginUser.mockResolvedValueOnce(user);
+
+			await controller.callbackHandler(req, res);
+
+			expect(oidcService.loginUser).toHaveBeenCalledWith(expectedCallbackUrl);
+			expect(authService.issueCookie).toHaveBeenCalledWith(res, user, true);
+			expect(res.redirect).toHaveBeenCalledWith('/');
+		});
+
+		test('Should handle callback URL with no query parameters', async () => {
+			const req = mock<Request>({
+				originalUrl: '/sso/oidc/callback',
+			});
+			const res = mock<Response>();
+
+			const expectedCallbackUrl = new URL('http://localhost:5678/sso/oidc/callback');
+
+			oidcService.loginUser.mockResolvedValueOnce(user);
+
+			await controller.callbackHandler(req, res);
+
+			expect(oidcService.loginUser).toHaveBeenCalledWith(expectedCallbackUrl);
+			expect(authService.issueCookie).toHaveBeenCalledWith(res, user, true);
+			expect(res.redirect).toHaveBeenCalledWith('/');
+		});
+
+		test('Should propagate errors from OIDC service', async () => {
+			const req = mock<Request>({
+				originalUrl: '/sso/oidc/callback?code=auth_code&state=state_value',
+			});
+			const res = mock<Response>();
+
+			const loginError = new Error('OIDC login failed');
+			oidcService.loginUser.mockRejectedValueOnce(loginError);
+
+			await expect(controller.callbackHandler(req, res)).rejects.toThrow('OIDC login failed');
+
+			// Verify that issueCookie was not called when login fails
+			expect(authService.issueCookie).not.toHaveBeenCalled();
+			expect(res.redirect).not.toHaveBeenCalled();
+		});
+	});
+
+	describe('redirectToAuthProvider', () => {
+		test('Should redirect to generated authorization URL', async () => {
+			const req = mock<Request>();
+			const res = mock<Response>();
+
+			const mockAuthUrl = new URL(
+				'https://provider.com/auth?client_id=123&redirect_uri=http://localhost:5678/callback',
+			);
+			oidcService.generateLoginUrl.mockResolvedValueOnce(mockAuthUrl);
+
+			await controller.redirectToAuthProvider(req, res);
+
+			expect(oidcService.generateLoginUrl).toHaveBeenCalled();
+			expect(res.redirect).toHaveBeenCalledWith(
+				'https://provider.com/auth?client_id=123&redirect_uri=http://localhost:5678/callback',
+			);
+		});
+
+		test('Should propagate errors from OIDC service during URL generation', async () => {
+			const req = mock<Request>();
+			const res = mock<Response>();
+
+			const urlGenerationError = new Error('Failed to generate authorization URL');
+			oidcService.generateLoginUrl.mockRejectedValueOnce(urlGenerationError);
+
+			await expect(controller.redirectToAuthProvider(req, res)).rejects.toThrow(
+				'Failed to generate authorization URL',
+			);
+
+			expect(res.redirect).not.toHaveBeenCalled();
+		});
+	});
+});

packages/cli/src/sso.ee/oidc/routes/oidc.controller.ee.ts

@@ -57,7 +57,7 @@ export class OidcController {
 
 		const user = await this.oidcService.loginUser(callbackUrl);
 
-		this.authService.issueCookie(res, user, false);
+		this.authService.issueCookie(res, user, true);
 
 		res.redirect('/');
 	}

packages/cli/src/sso.ee/saml/routes/__tests__/saml.controller.ee.test.ts

@@ -2,15 +2,29 @@ import type { User } from '@n8n/db';
 import { type Response } from 'express';
 import { mock } from 'jest-mock-extended';
 
+import type { AuthService } from '@/auth/auth.service';
+import type { EventService } from '@/events/event.service';
 import type { AuthlessRequest } from '@/requests';
+import type { UrlService } from '@/services/url.service';
 
 import type { SamlService } from '../../saml.service.ee';
 import { getServiceProviderConfigTestReturnUrl } from '../../service-provider.ee';
 import type { SamlUserAttributes } from '../../types';
 import { SamlController } from '../saml.controller.ee';
 
+// Mock the saml-helpers module
+jest.mock('../../saml-helpers', () => ({
+	isConnectionTestRequest: jest.fn(),
+	isSamlLicensedAndEnabled: jest.fn(),
+}));
+
+import { isConnectionTestRequest, isSamlLicensedAndEnabled } from '../../saml-helpers';
+
+const authService = mock<AuthService>();
 const samlService = mock<SamlService>();
-const controller = new SamlController(mock(), samlService, mock(), mock());
+const urlService = mock<UrlService>();
+const eventService = mock<EventService>();
+const controller = new SamlController(authService, samlService, urlService, eventService);
 
 const user = mock<User>({
 	id: '123',
@@ -29,9 +43,17 @@ const attributes: SamlUserAttributes = {
 describe('Test views', () => {
 	const RelayState = getServiceProviderConfigTestReturnUrl();
 
+	beforeEach(() => {
+		// Mock the helper functions for test connection flow
+		(isConnectionTestRequest as jest.Mock).mockReturnValue(true);
+	});
+
 	test('Should render success with template', async () => {
 		const req = mock<AuthlessRequest>();
-		const res = mock<Response>();
+		const res = mock<Response>({
+			status: jest.fn().mockReturnThis(),
+			json: jest.fn().mockReturnThis(),
+		});
 
 		samlService.handleSamlLogin.mockResolvedValueOnce({
 			authenticatedUser: user,
@@ -46,7 +68,10 @@ describe('Test views', () => {
 
 	test('Should render failure with template', async () => {
 		const req = mock<AuthlessRequest>();
-		const res = mock<Response>();
+		const res = mock<Response>({
+			status: jest.fn().mockReturnThis(),
+			json: jest.fn().mockReturnThis(),
+		});
 
 		samlService.handleSamlLogin.mockResolvedValueOnce({
 			authenticatedUser: undefined,
@@ -61,7 +86,10 @@ describe('Test views', () => {
 
 	test('Should render error with template', async () => {
 		const req = mock<AuthlessRequest>();
-		const res = mock<Response>();
+		const res = mock<Response>({
+			status: jest.fn().mockReturnThis(),
+			json: jest.fn().mockReturnThis(),
+		});
 
 		samlService.handleSamlLogin.mockRejectedValueOnce(new Error('Test Error'));
 
@@ -70,3 +98,70 @@ describe('Test views', () => {
 		expect(res.render).toBeCalledWith('saml-connection-test-failed', { message: 'Test Error' });
 	});
 });
+
+describe('SAML Login Flow', () => {
+	beforeEach(() => {
+		jest.clearAllMocks();
+		// Mock the helper functions for actual login flow (not test connections)
+		(isConnectionTestRequest as jest.Mock).mockReturnValue(false);
+		(isSamlLicensedAndEnabled as jest.Mock).mockReturnValue(true);
+
+		// Mock URL service
+		urlService.getInstanceBaseUrl.mockReturnValue('http://localhost:5678');
+	});
+
+	test('Should issue cookie with MFA flag set to true on successful SAML login', async () => {
+		const req = mock<AuthlessRequest>({ browserId: 'test-browser-id' });
+		const res = mock<Response>();
+
+		samlService.handleSamlLogin.mockResolvedValueOnce({
+			authenticatedUser: user,
+			attributes,
+			onboardingRequired: false,
+		});
+
+		await controller.acsPost(req, res, { RelayState: '/' });
+
+		// Verify that issueCookie was called with MFA flag set to true
+		expect(authService.issueCookie).toHaveBeenCalledWith(res, user, true, 'test-browser-id');
+		expect(eventService.emit).toHaveBeenCalledWith('user-logged-in', {
+			user,
+			authenticationMethod: 'saml',
+		});
+		expect(res.redirect).toHaveBeenCalledWith('http://localhost:5678/');
+	});
+
+	test('Should issue cookie with MFA flag set to true when onboarding is required', async () => {
+		const req = mock<AuthlessRequest>({ browserId: 'test-browser-id' });
+		const res = mock<Response>();
+
+		samlService.handleSamlLogin.mockResolvedValueOnce({
+			authenticatedUser: user,
+			attributes,
+			onboardingRequired: true,
+		});
+
+		await controller.acsPost(req, res, { RelayState: '/' });
+
+		// Verify that issueCookie was called with MFA flag set to true
+		expect(authService.issueCookie).toHaveBeenCalledWith(res, user, true, 'test-browser-id');
+		expect(res.redirect).toHaveBeenCalledWith('http://localhost:5678/saml/onboarding');
+	});
+
+	test('Should respect custom RelayState redirect URL', async () => {
+		const req = mock<AuthlessRequest>({ browserId: 'test-browser-id' });
+		const res = mock<Response>();
+		const customRelayState = '/custom/redirect';
+
+		samlService.handleSamlLogin.mockResolvedValueOnce({
+			authenticatedUser: user,
+			attributes,
+			onboardingRequired: false,
+		});
+
+		await controller.acsPost(req, res, { RelayState: customRelayState });
+
+		expect(authService.issueCookie).toHaveBeenCalledWith(res, user, true, 'test-browser-id');
+		expect(res.redirect).toHaveBeenCalledWith('http://localhost:5678/custom/redirect');
+	});
+});

packages/cli/src/sso.ee/saml/routes/saml.controller.ee.ts

@@ -127,7 +127,7 @@ export class SamlController {
 
 				// Only sign in user if SAML is enabled, otherwise treat as test connection
 				if (isSamlLicensedAndEnabled()) {
-					this.authService.issueCookie(res, loginResult.authenticatedUser, false, req.browserId);
+					this.authService.issueCookie(res, loginResult.authenticatedUser, true, req.browserId);
 					if (loginResult.onboardingRequired) {
 						return res.redirect(this.urlService.getInstanceBaseUrl() + '/saml/onboarding');
 					} else {

packages/frontend/@n8n/i18n/src/locales/en.json

@@ -3127,15 +3127,15 @@
 	"settings.personal.mfa.toast.disabledMfa.error.message": "Error disabling two-factor authentication",
 	"settings.personal.mfa.toast.canEnableMfa.title": "MFA pre-requisite failed",
 	"settings.personal.mfa.enforced": "The settings on this instance <strong>require you to set up 2FA</strong>. Please enable it to continue working in this instance.",
-	"settings.personal.mfa.enforce.message": "Enforces 2FA for all users on this instance.",
-	"settings.personal.mfa.enforce.unlicensed_tooltip": "You can enforce 2FA for all users on this instance when you upgrade your plan. {action}",
+	"settings.personal.mfa.enforce.message": "Enforces 2FA for all users on this instance authenticating with email and password logins.",
+	"settings.personal.mfa.enforce.unlicensed_tooltip": "You can enforce 2FA for all users on this instance authenticating with email and password logins when you upgrade your plan. {action}",
 	"settings.personal.mfa.enforce.unlicensed_tooltip.link": "View plans",
 	"settings.personal.mfa.enforce.title": "Enforce two-factor authentication",
 	"settings.personal.mfa.enforce.error": "Cannot enforce 2FA for all users",
 	"settings.personal.mfa.enforce.enabled.title": "2FA Enforced",
-	"settings.personal.mfa.enforce.enabled.message": "Two-factor authentication is now required for all users on this instance.",
+	"settings.personal.mfa.enforce.enabled.message": "Two-factor authentication is now required for all users on this instance authenticating with email and password logins.",
 	"settings.personal.mfa.enforce.disabled.title": "2FA No Longer Enforced",
-	"settings.personal.mfa.enforce.disabled.message": "Two-factor authentication is no longer mandatory for users on this instance.",
+	"settings.personal.mfa.enforce.disabled.message": "Two-factor authentication is no longer mandatory for users on this instance authenticating with email and password logins.",
 	"settings.mfa.toast.noRecoveryCodeLeft.title": "No 2FA recovery codes remaining",
 	"settings.mfa.toast.noRecoveryCodeLeft.message": "You have used all of your recovery codes. Disable then re-enable two-factor authentication to generate new codes. <a href='/settings/personal' target='_blank' >Open settings</a>",
 	"sso.login.divider": "or",