feat(editor): SSO setup (n8n-io#5736)

* feat(editor): SSO settings page * feat(editor): SSO settings page * feat(editor): SSO settings page * feat(editor): SSO settings page * feat(editor): SSO settings page * feat(editor): SSO settings page * Merge remote-tracking branch 'origin/master' into pay-170-sso-set-up-page # Conflicts: # packages/cli/src/sso/saml/routes/saml.controller.ee.ts * feat(editor): Prevent SSO settings page route * feat(editor): some UI improvements * fix(editor): SSO settings saml config optional chaining * fix return values saml controller * fix(editor): drop dompurify * fix(editor): save xml as is * return authenticationMethod with settings * fix(editor): add missing prop to server * chore(editor): code formatting * fix ldap/saml enable toggle endpoint * fix missing import * prevent faulty ldap setting from breaking startup * remove sso fake-door from users page * fix(editor): update SSO settings route permissions + unit testing * fix(editor): update vite config for test * fix(editor): add paddings to SSO settings page buttons, add translation * fix(editor): fix saml unit test * fix(core): Improve saml test connection function (n8n-io#5899) improve-saml-test-connection return --------- Co-authored-by: Michael Auerswald <[email protected]> Co-authored-by: Romain Minaud <[email protected]>
fl-g6 · Apr 24, 2023 · 5d80757 · 5d80757
1 parent 5d1cf5b
commit 5d80757
Show file tree

Hide file tree

Showing 23 changed files with 1,066 additions and 560 deletions.
diff --git a/packages/cli/src/Interfaces.ts b/packages/cli/src/Interfaces.ts
@@ -24,6 +24,7 @@ import type {
 	IExecutionsSummary,
 	FeatureFlags,
 	WorkflowSettings,
+	AuthenticationMethod,
 } from 'n8n-workflow';
 
 import type { ActiveWorkflowRunner } from '@/ActiveWorkflowRunner';
@@ -552,6 +553,7 @@ export interface IUserManagementSettings {
 	enabled: boolean;
 	showSetupOnFirstLoad?: boolean;
 	smtpSetup: boolean;
+	authenticationMethod: AuthenticationMethod;
 }
 export interface IActiveDirectorySettings {
 	enabled: boolean;

diff --git a/packages/cli/src/Ldap/helpers.ts b/packages/cli/src/Ldap/helpers.ts
@@ -26,10 +26,12 @@ import { jsonParse, LoggerProxy as Logger } from 'n8n-workflow';
 import { License } from '@/License';
 import { InternalHooks } from '@/InternalHooks';
 import {
+	getCurrentAuthenticationMethod,
 	isEmailCurrentAuthenticationMethod,
 	isLdapCurrentAuthenticationMethod,
 	setCurrentAuthenticationMethod,
 } from '@/sso/ssoHelpers';
+import { InternalServerError } from '../ResponseHelper';
 
 /**
  *  Check whether the LDAP feature is disabled in the instance
@@ -54,25 +56,21 @@ export const setLdapLoginLabel = (value: string): void => {
 /**
  * Set the LDAP login enabled to the configuration object
  */
-export const setLdapLoginEnabled = async (value: boolean): Promise<void> => {
-	if (config.get(LDAP_LOGIN_ENABLED) === value) {
-		return;
-	}
-	// only one auth method can be active at a time, with email being the default
-	if (value && isEmailCurrentAuthenticationMethod()) {
-		// enable ldap login and disable email login, but only if email is the current auth method
-		config.set(LDAP_LOGIN_ENABLED, true);
-		await setCurrentAuthenticationMethod('ldap');
-	} else if (!value && isLdapCurrentAuthenticationMethod()) {
-		// disable ldap login, but only if ldap is the current auth method
-		config.set(LDAP_LOGIN_ENABLED, false);
-		await setCurrentAuthenticationMethod('email');
+export async function setLdapLoginEnabled(enabled: boolean): Promise<void> {
+	if (isEmailCurrentAuthenticationMethod() || isLdapCurrentAuthenticationMethod()) {
+		if (enabled) {
+			config.set(LDAP_LOGIN_ENABLED, true);
+			await setCurrentAuthenticationMethod('ldap');
+		} else if (!enabled) {
+			config.set(LDAP_LOGIN_ENABLED, false);
+			await setCurrentAuthenticationMethod('email');
+		}
 	} else {
-		Logger.warn(
-			'Cannot switch LDAP login enabled state when an authentication method other than email is active',
+		throw new InternalServerError(
+			`Cannot switch LDAP login enabled state when an authentication method other than email or ldap is active (current: ${getCurrentAuthenticationMethod()})`,
 		);
 	}
-};
+}
 
 /**
  * Retrieve the LDAP login label from the configuration object
@@ -217,7 +215,15 @@ export const handleLdapInit = async (): Promise<void> => {
 
 	const ldapConfig = await getLdapConfig();
 
-	await setGlobalLdapConfigVariables(ldapConfig);
+	try {
+		await setGlobalLdapConfigVariables(ldapConfig);
+	} catch (error) {
+		Logger.error(
+			`Cannot set LDAP login enabled state when an authentication method other than email or ldap is active (current: ${getCurrentAuthenticationMethod()})`,
+			// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
+			error,
+		);
+	}
 
 	// init LDAP manager with the current
 	// configuration

diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts
@@ -157,6 +157,7 @@ import { getSamlLoginLabel, isSamlLoginEnabled, isSamlLicensed } from './sso/sam
 import { SamlController } from './sso/saml/routes/saml.controller.ee';
 import { SamlService } from './sso/saml/saml.service.ee';
 import { LdapManager } from './Ldap/LdapManager.ee';
+import { getCurrentAuthenticationMethod } from './sso/ssoHelpers';
 
 const exec = promisify(callbackExec);
 
@@ -269,6 +270,7 @@ class Server extends AbstractServer {
 					config.getEnv('userManagement.isInstanceOwnerSetUp') === false &&
 					config.getEnv('userManagement.skipInstanceOwnerSetup') === false,
 				smtpSetup: isEmailSetUp(),
+				authenticationMethod: getCurrentAuthenticationMethod(),
 			},
 			sso: {
 				saml: {
@@ -328,6 +330,7 @@ class Server extends AbstractServer {
 		// refresh user management status
 		Object.assign(this.frontendSettings.userManagement, {
 			enabled: isUserManagementEnabled(),
+			authenticationMethod: getCurrentAuthenticationMethod(),
 			showSetupOnFirstLoad:
 				config.getEnv('userManagement.disabled') === false &&
 				config.getEnv('userManagement.isInstanceOwnerSetUp') === false &&

diff --git a/packages/cli/src/sso/saml/constants.ts b/packages/cli/src/sso/saml/constants.ts
@@ -3,8 +3,6 @@ export class SamlUrls {
 
 	static readonly initSSO = '/initsso';
 
-	static readonly restInitSSO = this.samlRESTRoot + this.initSSO;
-
 	static readonly acs = '/acs';
 
 	static readonly restAcs = this.samlRESTRoot + this.acs;
@@ -17,9 +15,9 @@ export class SamlUrls {
 
 	static readonly configTest = '/config/test';
 
-	static readonly configToggleEnabled = '/config/toggle';
+	static readonly configTestReturn = '/config/test/return';
 
-	static readonly restConfig = this.samlRESTRoot + this.config;
+	static readonly configToggleEnabled = '/config/toggle';
 
 	static readonly defaultRedirect = '/';
 

diff --git a/packages/cli/src/sso/saml/routes/saml.controller.ee.ts b/packages/cli/src/sso/saml/routes/saml.controller.ee.ts
@@ -16,7 +16,11 @@ import type { PostBindingContext } from 'samlify/types/src/entity';
 import { isSamlLicensedAndEnabled } from '../samlHelpers';
 import type { SamlLoginBinding } from '../types';
 import { AuthenticatedRequest } from '@/requests';
-import { getServiceProviderEntityId, getServiceProviderReturnUrl } from '../serviceProvider.ee';
+import {
+	getServiceProviderConfigTestReturnUrl,
+	getServiceProviderEntityId,
+	getServiceProviderReturnUrl,
+} from '../serviceProvider.ee';
 
 @RestController('/sso/saml')
 export class SamlController {
@@ -34,25 +38,25 @@ export class SamlController {
 	 * Return SAML config
 	 */
 	@Get(SamlUrls.config, { middlewares: [samlLicensedOwnerMiddleware] })
-	async configGet(req: AuthenticatedRequest, res: express.Response) {
+	async configGet() {
 		const prefs = this.samlService.samlPreferences;
-		return res.send({
+		return {
 			...prefs,
 			entityID: getServiceProviderEntityId(),
 			returnUrl: getServiceProviderReturnUrl(),
-		});
+		};
 	}
 
 	/**
 	 * POST /sso/saml/config
 	 * Set SAML config
 	 */
 	@Post(SamlUrls.config, { middlewares: [samlLicensedOwnerMiddleware] })
-	async configPost(req: SamlConfiguration.Update, res: express.Response) {
+	async configPost(req: SamlConfiguration.Update) {
 		const validationResult = await validate(req.body);
 		if (validationResult.length === 0) {
 			const result = await this.samlService.setSamlPreferences(req.body);
-			return res.send(result);
+			return result;
 		} else {
 			throw new BadRequestError(
 				'Body is not a valid SamlPreferences object: ' +
@@ -100,6 +104,10 @@ export class SamlController {
 	private async acsHandler(req: express.Request, res: express.Response, binding: SamlLoginBinding) {
 		const loginResult = await this.samlService.handleSamlLogin(req, binding);
 		if (loginResult) {
+			// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
+			if (req.body.RelayState && req.body.RelayState === getServiceProviderConfigTestReturnUrl()) {
+				return res.status(202).send(loginResult.attributes);
+			}
 			if (loginResult.authenticatedUser) {
 				// Only sign in user if SAML is enabled, otherwise treat as test connection
 				if (isSamlLicensedAndEnabled()) {
@@ -134,13 +142,13 @@ export class SamlController {
 	 */
 	@Get(SamlUrls.configTest, { middlewares: [samlLicensedOwnerMiddleware] })
 	async configTestGet(req: AuthenticatedRequest, res: express.Response) {
-		return this.handleInitSSO(res);
+		return this.handleInitSSO(res, getServiceProviderConfigTestReturnUrl());
 	}
 
-	private async handleInitSSO(res: express.Response) {
-		const result = this.samlService.getLoginRequestUrl();
+	private async handleInitSSO(res: express.Response, relayState?: string) {
+		const result = this.samlService.getLoginRequestUrl(relayState);
 		if (result?.binding === 'redirect') {
-			return res.send(result.context.context);
+			return result.context.context;
 		} else if (result?.binding === 'post') {
 			return res.send(getInitSSOFormView(result.context as PostBindingContext));
 		} else {

diff --git a/packages/cli/src/sso/saml/saml.service.ee.ts b/packages/cli/src/sso/saml/saml.service.ee.ts
@@ -20,12 +20,13 @@ import {
 	setSamlLoginLabel,
 	updateUserFromSamlAttributes,
 } from './samlHelpers';
-import type { Settings } from '../../databases/entities/Settings';
+import type { Settings } from '@/databases/entities/Settings';
 import axios from 'axios';
 import https from 'https';
 import type { SamlLoginBinding } from './types';
 import type { BindingContext, PostBindingContext } from 'samlify/types/src/entity';
 import { validateMetadata, validateResponse } from './samlValidator';
+import { getInstanceBaseUrl } from '@/UserManagement/UserManagementHelper';
 
 @Service()
 export class SamlService {
@@ -48,6 +49,7 @@ export class SamlService {
 		loginLabel: 'SAML',
 		wantAssertionsSigned: true,
 		wantMessageSigned: true,
+		relayState: getInstanceBaseUrl(),
 		signatureConfig: {
 			prefix: 'ds',
 			location: {
@@ -92,36 +94,40 @@ export class SamlService {
 		return getServiceProviderInstance(this._samlPreferences);
 	}
 
-	getLoginRequestUrl(binding?: SamlLoginBinding): {
+	getLoginRequestUrl(
+		relayState?: string,
+		binding?: SamlLoginBinding,
+	): {
 		binding: SamlLoginBinding;
 		context: BindingContext | PostBindingContext;
 	} {
 		if (binding === undefined) binding = this._samlPreferences.loginBinding ?? 'redirect';
 		if (binding === 'post') {
 			return {
 				binding,
-				context: this.getPostLoginRequestUrl(),
+				context: this.getPostLoginRequestUrl(relayState),
 			};
 		} else {
 			return {
 				binding,
-				context: this.getRedirectLoginRequestUrl(),
+				context: this.getRedirectLoginRequestUrl(relayState),
 			};
 		}
 	}
 
-	private getRedirectLoginRequestUrl(): BindingContext {
-		const loginRequest = this.getServiceProviderInstance().createLoginRequest(
-			this.getIdentityProviderInstance(),
-			'redirect',
-		);
+	private getRedirectLoginRequestUrl(relayState?: string): BindingContext {
+		const sp = this.getServiceProviderInstance();
+		sp.entitySetting.relayState = relayState ?? getInstanceBaseUrl();
+		const loginRequest = sp.createLoginRequest(this.getIdentityProviderInstance(), 'redirect');
 		//TODO:SAML: debug logging
 		LoggerProxy.debug(loginRequest.context);
 		return loginRequest;
 	}
 
-	private getPostLoginRequestUrl(): PostBindingContext {
-		const loginRequest = this.getServiceProviderInstance().createLoginRequest(
+	private getPostLoginRequestUrl(relayState?: string): PostBindingContext {
+		const sp = this.getServiceProviderInstance();
+		sp.entitySetting.relayState = relayState ?? getInstanceBaseUrl();
+		const loginRequest = sp.createLoginRequest(
 			this.getIdentityProviderInstance(),
 			'post',
 		) as PostBindingContext;

diff --git a/packages/cli/src/sso/saml/samlHelpers.ts b/packages/cli/src/sso/saml/samlHelpers.ts
@@ -4,19 +4,19 @@ import * as Db from '@/Db';
 import { AuthIdentity } from '@db/entities/AuthIdentity';
 import { User } from '@db/entities/User';
 import { License } from '@/License';
-import { AuthError } from '@/ResponseHelper';
+import { AuthError, InternalServerError } from '@/ResponseHelper';
 import { hashPassword, isUserManagementEnabled } from '@/UserManagement/UserManagementHelper';
 import type { SamlPreferences } from './types/samlPreferences';
 import type { SamlUserAttributes } from './types/samlUserAttributes';
 import type { FlowResult } from 'samlify/types/src/flow';
 import type { SamlAttributeMapping } from './types/samlAttributeMapping';
 import { SAML_LOGIN_ENABLED, SAML_LOGIN_LABEL } from './constants';
 import {
+	getCurrentAuthenticationMethod,
 	isEmailCurrentAuthenticationMethod,
 	isSamlCurrentAuthenticationMethod,
 	setCurrentAuthenticationMethod,
 } from '../ssoHelpers';
-import { LoggerProxy } from 'n8n-workflow';
 /**
  *  Check whether the SAML feature is licensed and enabled in the instance
  */
@@ -30,18 +30,17 @@ export function getSamlLoginLabel(): string {
 
 // can only toggle between email and saml, not directly to e.g. ldap
 export async function setSamlLoginEnabled(enabled: boolean): Promise<void> {
-	if (config.get(SAML_LOGIN_ENABLED) === enabled) {
-		return;
-	}
-	if (enabled && isEmailCurrentAuthenticationMethod()) {
-		config.set(SAML_LOGIN_ENABLED, true);
-		await setCurrentAuthenticationMethod('saml');
-	} else if (!enabled && isSamlCurrentAuthenticationMethod()) {
-		config.set(SAML_LOGIN_ENABLED, false);
-		await setCurrentAuthenticationMethod('email');
+	if (isEmailCurrentAuthenticationMethod() || isSamlCurrentAuthenticationMethod()) {
+		if (enabled) {
+			config.set(SAML_LOGIN_ENABLED, true);
+			await setCurrentAuthenticationMethod('saml');
+		} else if (!enabled) {
+			config.set(SAML_LOGIN_ENABLED, false);
+			await setCurrentAuthenticationMethod('email');
+		}
 	} else {
-		LoggerProxy.warn(
-			'Cannot switch SAML login enabled state when an authentication method other than email is active',
+		throw new InternalServerError(
+			`Cannot switch SAML login enabled state when an authentication method other than email or saml is active (current: ${getCurrentAuthenticationMethod()})`,
 		);
 	}
 }

diff --git a/packages/cli/src/sso/saml/serviceProvider.ee.ts b/packages/cli/src/sso/saml/serviceProvider.ee.ts
@@ -15,6 +15,10 @@ export function getServiceProviderReturnUrl(): string {
 	return getInstanceBaseUrl() + SamlUrls.restAcs;
 }
 
+export function getServiceProviderConfigTestReturnUrl(): string {
+	return getInstanceBaseUrl() + SamlUrls.configTestReturn;
+}
+
 // TODO:SAML: make these configurable for the end user
 export function getServiceProviderInstance(prefs: SamlPreferences): ServiceProviderInstance {
 	if (serviceProviderInstance === undefined) {
@@ -24,6 +28,7 @@ export function getServiceProviderInstance(prefs: SamlPreferences): ServiceProvi
 			wantAssertionsSigned: prefs.wantAssertionsSigned,
 			wantMessageSigned: prefs.wantMessageSigned,
 			signatureConfig: prefs.signatureConfig,
+			relayState: prefs.relayState,
 			nameIDFormat: ['urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress'],
 			assertionConsumerService: [
 				{

diff --git a/packages/cli/src/sso/saml/types/samlPreferences.ts b/packages/cli/src/sso/saml/types/samlPreferences.ts
@@ -57,4 +57,8 @@ export class SamlPreferences {
 			action: 'after',
 		},
 	};
+
+	@IsString()
+	@IsOptional()
+	relayState?: string = '';
 }
diff --git a/packages/cli/test/integration/saml/saml.api.test.ts b/packages/cli/test/integration/saml/saml.api.test.ts
@@ -127,7 +127,7 @@ describe('Instance owner', () => {
 				.send({
 					loginEnabled: true,
 				})
-				.expect(200);
+				.expect(500);
 
 			expect(getCurrentAuthenticationMethod()).toBe('ldap');
 		});