From 354f9405479051adaa120ebcde001859f88461da Mon Sep 17 00:00:00 2001 From: Michael Auerswald Date: Mon, 27 Feb 2023 13:39:24 +0100 Subject: [PATCH 01/15] consolidate SSO settings --- packages/cli/src/Interfaces.ts | 12 ++++++--- packages/cli/src/Ldap/constants.ts | 4 +-- packages/cli/src/Server.ts | 23 ++++++++++++---- packages/cli/src/config/schema.ts | 26 ++++++++++--------- packages/cli/src/sso/saml/constants.ts | 6 +++++ .../saml/middleware/samlEnabledMiddleware.ts | 4 +-- packages/cli/src/sso/saml/samlHelpers.ts | 11 +++++--- packages/editor-ui/src/Interface.ts | 16 +++++++++--- packages/editor-ui/src/stores/settings.ts | 16 +++++++++--- 9 files changed, 85 insertions(+), 33 deletions(-) diff --git a/packages/cli/src/Interfaces.ts b/packages/cli/src/Interfaces.ts index d1ea87ffac06e..24f7f1f7550fc 100644 --- a/packages/cli/src/Interfaces.ts +++ b/packages/cli/src/Interfaces.ts @@ -488,9 +488,15 @@ export interface IN8nUISettings { personalizationSurveyEnabled: boolean; defaultLocale: string; userManagement: IUserManagementSettings; - ldap: { - loginLabel: string; - loginEnabled: boolean; + sso: { + saml: { + loginLabel: string; + loginEnabled: boolean; + }; + ldap: { + loginLabel: string; + loginEnabled: boolean; + }; }; publicApi: IPublicApiSettings; workflowTagsDisabled: boolean; diff --git a/packages/cli/src/Ldap/constants.ts b/packages/cli/src/Ldap/constants.ts index 630f4b6b77a45..3b7b369b80abf 100644 --- a/packages/cli/src/Ldap/constants.ts +++ b/packages/cli/src/Ldap/constants.ts @@ -4,9 +4,9 @@ export const LDAP_FEATURE_NAME = 'features.ldap'; export const LDAP_ENABLED = 'enterprise.features.ldap'; -export const LDAP_LOGIN_LABEL = 'ldap.loginLabel'; +export const LDAP_LOGIN_LABEL = 'sso.ldap.loginLabel'; -export const LDAP_LOGIN_ENABLED = 'ldap.loginEnabled'; +export const LDAP_LOGIN_ENABLED = 'sso.ldap.loginEnabled'; export const BINARY_AD_ATTRIBUTES = ['objectGUID', 'objectSid']; diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index 13cb901ac64d1..af3a5b5e56482 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -145,7 +145,7 @@ import { eventBus } from './eventbus'; import { Container } from 'typedi'; import { InternalHooks } from './InternalHooks'; import { getStatusUsingPreviousExecutionStatusMethod } from './executions/executionHelpers'; -import { isSamlLicensed } from './sso/saml/samlHelpers'; +import { getSamlLoginLabel, isSamlLoginEnabled, isSamlLicensed } from './sso/saml/samlHelpers'; import { samlControllerPublic } from './sso/saml/routes/saml.controller.public.ee'; import { SamlService } from './sso/saml/saml.service.ee'; import { samlControllerProtected } from './sso/saml/routes/saml.controller.protected.ee'; @@ -258,9 +258,15 @@ class Server extends AbstractServer { config.getEnv('userManagement.skipInstanceOwnerSetup') === false, smtpSetup: isEmailSetUp(), }, - ldap: { - loginEnabled: false, - loginLabel: '', + sso: { + saml: { + loginEnabled: false, + loginLabel: '', + }, + ldap: { + loginEnabled: false, + loginLabel: '', + }, }, publicApi: { enabled: !config.getEnv('publicApi.disabled'), @@ -325,12 +331,19 @@ class Server extends AbstractServer { }); if (isLdapEnabled()) { - Object.assign(this.frontendSettings.ldap, { + Object.assign(this.frontendSettings.sso.ldap, { loginLabel: getLdapLoginLabel(), loginEnabled: isLdapLoginEnabled(), }); } + if (isSamlLicensed()) { + Object.assign(this.frontendSettings.sso.saml, { + loginLabel: getSamlLoginLabel(), + loginEnabled: isSamlLoginEnabled(), + }); + } + if (config.get('nodes.packagesMissing').length > 0) { this.frontendSettings.missingPackages = true; } diff --git a/packages/cli/src/config/schema.ts b/packages/cli/src/config/schema.ts index 78e728c33a624..0ec5c9a623d43 100644 --- a/packages/cli/src/config/schema.ts +++ b/packages/cli/src/config/schema.ts @@ -1023,23 +1023,25 @@ export const schema = { doc: 'Whether to automatically redirect users from login dialog to initialize SSO flow.', }, saml: { - enabled: { + loginEnabled: { format: Boolean, default: false, doc: 'Whether to enable SAML SSO.', }, + loginLabel: { + format: String, + default: '', + }, }, - }, - - // TODO: move into sso settings - ldap: { - loginEnabled: { - format: Boolean, - default: false, - }, - loginLabel: { - format: String, - default: '', + ldap: { + loginEnabled: { + format: Boolean, + default: false, + }, + loginLabel: { + format: String, + default: '', + }, }, }, diff --git a/packages/cli/src/sso/saml/constants.ts b/packages/cli/src/sso/saml/constants.ts index 16565fa712c05..715c20b932c98 100644 --- a/packages/cli/src/sso/saml/constants.ts +++ b/packages/cli/src/sso/saml/constants.ts @@ -23,3 +23,9 @@ export class SamlUrls { } export const SAML_PREFERENCES_DB_KEY = 'features.saml'; + +export const SAML_ENTERPRISE_FEATURE_ENABLED = 'enterprise.features.saml'; + +export const SAML_LOGIN_LABEL = 'sso.saml.loginLabel'; + +export const SAML_LOGIN_ENABLED = 'sso.saml.loginEnabled'; diff --git a/packages/cli/src/sso/saml/middleware/samlEnabledMiddleware.ts b/packages/cli/src/sso/saml/middleware/samlEnabledMiddleware.ts index bcd1005e1fde9..bf15030d836ef 100644 --- a/packages/cli/src/sso/saml/middleware/samlEnabledMiddleware.ts +++ b/packages/cli/src/sso/saml/middleware/samlEnabledMiddleware.ts @@ -1,7 +1,7 @@ import type { RequestHandler } from 'express'; import type { AuthenticatedRequest } from '../../../requests'; import { isSamlCurrentAuthenticationMethod } from '../../ssoHelpers'; -import { isSamlEnabled, isSamlLicensed } from '../samlHelpers'; +import { isSamlLoginEnabled, isSamlLicensed } from '../samlHelpers'; export const samlLicensedOwnerMiddleware: RequestHandler = ( req: AuthenticatedRequest, @@ -16,7 +16,7 @@ export const samlLicensedOwnerMiddleware: RequestHandler = ( }; export const samlLicensedAndEnabledMiddleware: RequestHandler = (req, res, next) => { - if (isSamlEnabled() && isSamlLicensed() && isSamlCurrentAuthenticationMethod()) { + if (isSamlLoginEnabled() && isSamlLicensed() && isSamlCurrentAuthenticationMethod()) { next(); } else { res.status(401).json({ status: 'error', message: 'Unauthorized' }); diff --git a/packages/cli/src/sso/saml/samlHelpers.ts b/packages/cli/src/sso/saml/samlHelpers.ts index 20733e1fe5089..b012bc7f5ad77 100644 --- a/packages/cli/src/sso/saml/samlHelpers.ts +++ b/packages/cli/src/sso/saml/samlHelpers.ts @@ -9,18 +9,23 @@ 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_ENTERPRISE_FEATURE_ENABLED, SAML_LOGIN_ENABLED, SAML_LOGIN_LABEL } from './constants'; /** * Check whether the SAML feature is licensed and enabled in the instance */ -export function isSamlEnabled(): boolean { - return config.getEnv('sso.saml.enabled'); +export function isSamlLoginEnabled(): boolean { + return config.getEnv(SAML_LOGIN_ENABLED); +} + +export function getSamlLoginLabel(): string { + return config.getEnv(SAML_LOGIN_LABEL); } export function isSamlLicensed(): boolean { const license = getLicense(); return ( isUserManagementEnabled() && - (license.isSamlEnabled() || config.getEnv('enterprise.features.saml')) + (license.isSamlEnabled() || config.getEnv(SAML_ENTERPRISE_FEATURE_ENABLED)) ); } diff --git a/packages/editor-ui/src/Interface.ts b/packages/editor-ui/src/Interface.ts index 58a38d3868491..0e7a92aab7b86 100644 --- a/packages/editor-ui/src/Interface.ts +++ b/packages/editor-ui/src/Interface.ts @@ -762,9 +762,15 @@ export interface IN8nUISettings { enabled: boolean; }; }; - ldap: { - loginLabel: string; - loginEnabled: boolean; + sso: { + saml: { + loginLabel: string; + loginEnabled: boolean; + }; + ldap: { + loginLabel: string; + loginEnabled: boolean; + }; }; onboardingCallPromptEnabled: boolean; allowedModules: { @@ -1192,6 +1198,10 @@ export interface ISettingsState { loginLabel: string; loginEnabled: boolean; }; + saml: { + loginLabel: string; + loginEnabled: boolean; + }; onboardingCallPromptEnabled: boolean; saveDataErrorExecution: string; saveDataSuccessExecution: string; diff --git a/packages/editor-ui/src/stores/settings.ts b/packages/editor-ui/src/stores/settings.ts index 9fca2f63f9bcf..6a943132f90ab 100644 --- a/packages/editor-ui/src/stores/settings.ts +++ b/packages/editor-ui/src/stores/settings.ts @@ -24,7 +24,7 @@ import { WorkflowCallerPolicyDefaultOption, ILdapConfig, } from '@/Interface'; -import { ITelemetrySettings } from 'n8n-workflow'; +import { IDataObject, ITelemetrySettings } from 'n8n-workflow'; import { defineStore } from 'pinia'; import Vue from 'vue'; import { useRootStore } from './n8nRootStore'; @@ -54,6 +54,10 @@ export const useSettingsStore = defineStore(STORES.SETTINGS, { loginLabel: '', loginEnabled: false, }, + saml: { + loginLabel: '', + loginEnabled: false, + }, onboardingCallPromptEnabled: false, saveDataErrorExecution: 'all', saveDataSuccessExecution: 'all', @@ -87,6 +91,12 @@ export const useSettingsStore = defineStore(STORES.SETTINGS, { ldapLoginLabel(): string { return this.ldap.loginLabel; }, + isSamlLoginEnabled(): boolean { + return this.saml.loginEnabled; + }, + samlLoginLabel(): string { + return this.saml.loginLabel; + }, showSetupPage(): boolean { return this.userManagement.showSetupOnFirstLoad === true; }, @@ -168,8 +178,8 @@ export const useSettingsStore = defineStore(STORES.SETTINGS, { this.userManagement.smtpSetup = settings.userManagement.smtpSetup; this.api = settings.publicApi; this.onboardingCallPromptEnabled = settings.onboardingCallPromptEnabled; - this.ldap.loginEnabled = settings.ldap.loginEnabled; - this.ldap.loginLabel = settings.ldap.loginLabel; + this.ldap.loginEnabled = settings.sso.ldap.loginEnabled; + this.ldap.loginLabel = settings.sso.ldap.loginLabel; }, async getSettings(): Promise { const rootStore = useRootStore(); From 900e51197eb007826a88ef0c1c7f042f797a9b75 Mon Sep 17 00:00:00 2001 From: Michael Auerswald Date: Mon, 27 Feb 2023 14:21:30 +0100 Subject: [PATCH 02/15] update saml settings --- packages/cli/src/sso/saml/saml.service.ee.ts | 27 ++++++++++--------- packages/cli/src/sso/saml/samlHelpers.ts | 8 ++++++ .../cli/src/sso/saml/types/samlPreferences.ts | 3 ++- packages/editor-ui/src/stores/settings.ts | 2 ++ 4 files changed, 26 insertions(+), 14 deletions(-) diff --git a/packages/cli/src/sso/saml/saml.service.ee.ts b/packages/cli/src/sso/saml/saml.service.ee.ts index b5a2fe637b94a..72dc3bf2c2c12 100644 --- a/packages/cli/src/sso/saml/saml.service.ee.ts +++ b/packages/cli/src/sso/saml/saml.service.ee.ts @@ -14,6 +14,10 @@ import { IdentityProvider } from 'samlify'; import { createUserFromSamlAttributes, getMappedSamlAttributesFromFlowResult, + getSamlLoginLabel, + isSamlLoginEnabled, + setSamlLoginEnabled, + setSamlLoginLabel, updateUserFromSamlAttributes, } from './samlHelpers'; @@ -142,16 +146,20 @@ export class SamlService { return undefined; } - async getSamlPreferences(): Promise { + getSamlPreferences(): SamlPreferences { return { mapping: this.attributeMapping, metadata: this.metadata, + loginEnabled: isSamlLoginEnabled(), + loginLabel: getSamlLoginLabel(), }; } async setSamlPreferences(prefs: SamlPreferences): Promise { this.attributeMapping = prefs.mapping; this.metadata = prefs.metadata; + setSamlLoginEnabled(prefs.loginEnabled); + setSamlLoginLabel(prefs.loginLabel); this.getIdentityProviderInstance(true); await this.saveSamlPreferences(); } @@ -163,10 +171,9 @@ export class SamlService { if (samlPreferences) { const prefs = jsonParse(samlPreferences.value); if (prefs) { - this.attributeMapping = prefs.mapping; - this.metadata = prefs.metadata; + await this.setSamlPreferences(prefs); + return prefs; } - return prefs; } return; } @@ -175,20 +182,14 @@ export class SamlService { const samlPreferences = await Db.collections.Settings.findOne({ where: { key: SAML_PREFERENCES_DB_KEY }, }); + const settingsValue = JSON.stringify(this.getSamlPreferences()); if (samlPreferences) { - samlPreferences.value = JSON.stringify({ - mapping: this.attributeMapping, - metadata: this.metadata, - }); - samlPreferences.loadOnStartup = true; + samlPreferences.value = settingsValue; await Db.collections.Settings.save(samlPreferences); } else { await Db.collections.Settings.save({ key: SAML_PREFERENCES_DB_KEY, - value: JSON.stringify({ - mapping: this.attributeMapping, - metadata: this.metadata, - }), + value: settingsValue, loadOnStartup: true, }); } diff --git a/packages/cli/src/sso/saml/samlHelpers.ts b/packages/cli/src/sso/saml/samlHelpers.ts index b012bc7f5ad77..3eb045cfae77f 100644 --- a/packages/cli/src/sso/saml/samlHelpers.ts +++ b/packages/cli/src/sso/saml/samlHelpers.ts @@ -21,6 +21,14 @@ export function getSamlLoginLabel(): string { return config.getEnv(SAML_LOGIN_LABEL); } +export function setSamlLoginEnabled(enabled: boolean): void { + config.set(SAML_LOGIN_ENABLED, enabled); +} + +export function setSamlLoginLabel(label: string): void { + config.set(SAML_LOGIN_LABEL, label); +} + export function isSamlLicensed(): boolean { const license = getLicense(); return ( diff --git a/packages/cli/src/sso/saml/types/samlPreferences.ts b/packages/cli/src/sso/saml/types/samlPreferences.ts index d57f10b9ae3c6..4edc58fc95902 100644 --- a/packages/cli/src/sso/saml/types/samlPreferences.ts +++ b/packages/cli/src/sso/saml/types/samlPreferences.ts @@ -3,5 +3,6 @@ import type { SamlAttributeMapping } from './samlAttributeMapping'; export interface SamlPreferences { mapping: SamlAttributeMapping; metadata: string; - //TODO:SAML: add fields for separate SAML settins to generate metadata from + loginEnabled: boolean; + loginLabel: string; } diff --git a/packages/editor-ui/src/stores/settings.ts b/packages/editor-ui/src/stores/settings.ts index 6a943132f90ab..07e374a636013 100644 --- a/packages/editor-ui/src/stores/settings.ts +++ b/packages/editor-ui/src/stores/settings.ts @@ -180,6 +180,8 @@ export const useSettingsStore = defineStore(STORES.SETTINGS, { this.onboardingCallPromptEnabled = settings.onboardingCallPromptEnabled; this.ldap.loginEnabled = settings.sso.ldap.loginEnabled; this.ldap.loginLabel = settings.sso.ldap.loginLabel; + this.saml.loginEnabled = settings.sso.saml.loginEnabled; + this.saml.loginLabel = settings.sso.saml.loginLabel; }, async getSettings(): Promise { const rootStore = useRootStore(); From fa81066ea50c317b3c3402abc18598bfa4ad95c1 Mon Sep 17 00:00:00 2001 From: Michael Auerswald Date: Mon, 27 Feb 2023 14:38:24 +0100 Subject: [PATCH 03/15] fix type error --- .../routes/saml.controller.protected.ee.ts | 18 ++++++++++-------- packages/cli/src/sso/saml/samlHelpers.ts | 8 +++++++- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts b/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts index 9879cbe82c5b3..5a4376f692e8a 100644 --- a/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts +++ b/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts @@ -6,8 +6,9 @@ import { import { SamlService } from '../saml.service.ee'; import { SamlUrls } from '../constants'; import type { SamlConfiguration } from '../types/requests'; -import { AuthError } from '../../../ResponseHelper'; +import { AuthError, BadRequestError } from '@/ResponseHelper'; import { issueCookie } from '../../../auth/jwt'; +import { isSamlPreferences } from '../samlHelpers'; export const samlControllerProtected = express.Router(); @@ -18,8 +19,8 @@ export const samlControllerProtected = express.Router(); samlControllerProtected.get( SamlUrls.config, samlLicensedOwnerMiddleware, - async (req: SamlConfiguration.Read, res: express.Response) => { - const prefs = await SamlService.getInstance().getSamlPreferences(); + (req: SamlConfiguration.Read, res: express.Response) => { + const prefs = SamlService.getInstance().getSamlPreferences(); return res.send(prefs); }, ); @@ -32,11 +33,12 @@ samlControllerProtected.post( SamlUrls.config, samlLicensedOwnerMiddleware, async (req: SamlConfiguration.Update, res: express.Response) => { - const result = await SamlService.getInstance().setSamlPreferences({ - metadata: req.body.metadata, - mapping: req.body.mapping, - }); - return res.send(result); + if (isSamlPreferences(req.body)) { + const result = await SamlService.getInstance().setSamlPreferences(req.body); + return res.send(result); + } else { + throw new BadRequestError('Body is not a SamlPreferences object'); + } }, ); diff --git a/packages/cli/src/sso/saml/samlHelpers.ts b/packages/cli/src/sso/saml/samlHelpers.ts index 3eb045cfae77f..c6c0c6c30fb88 100644 --- a/packages/cli/src/sso/saml/samlHelpers.ts +++ b/packages/cli/src/sso/saml/samlHelpers.ts @@ -39,7 +39,13 @@ export function isSamlLicensed(): boolean { export const isSamlPreferences = (candidate: unknown): candidate is SamlPreferences => { const o = candidate as SamlPreferences; - return typeof o === 'object' && typeof o.metadata === 'string' && typeof o.mapping === 'object'; + return ( + typeof o === 'object' && + typeof o.metadata === 'string' && + typeof o.mapping === 'object' && + o.mapping !== null && + o.loginEnabled !== undefined + ); }; export function generatePassword(): string { From a66fe6a192f917e457187ef72748f6283040c0d3 Mon Sep 17 00:00:00 2001 From: Michael Auerswald Date: Mon, 27 Feb 2023 18:13:48 +0100 Subject: [PATCH 04/15] limit user changes when saml is enabled --- packages/cli/src/controllers/me.controller.ts | 25 +++++++++++++++++++ .../saml/middleware/samlEnabledMiddleware.ts | 5 ++-- packages/cli/src/sso/saml/samlHelpers.ts | 5 ++++ packages/cli/src/sso/ssoHelpers.ts | 6 +++++ 4 files changed, 38 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/controllers/me.controller.ts b/packages/cli/src/controllers/me.controller.ts index 5c830a6803baf..288009825a69a 100644 --- a/packages/cli/src/controllers/me.controller.ts +++ b/packages/cli/src/controllers/me.controller.ts @@ -22,6 +22,7 @@ import type { IInternalHooksClass, } from '@/Interfaces'; import { randomBytes } from 'crypto'; +import { isSamlLicensedAndEnabled } from '../sso/saml/samlHelpers'; @RestController('/me') export class MeController { @@ -77,6 +78,20 @@ export class MeController { await validateEntity(payload); + // If SAML is enabled, we don't allow the user to change their email address + if (isSamlLicensedAndEnabled()) { + if (email !== currentEmail) { + this.logger.debug( + 'Request to update user failed because SAML user may not change their email', + { + userId, + payload, + }, + ); + throw new BadRequestError('SAML user may not change their email'); + } + } + await this.userRepository.update(userId, payload); const user = await this.userRepository.findOneOrFail({ where: { id: userId }, @@ -105,6 +120,16 @@ export class MeController { async updatePassword(req: MeRequest.Password, res: Response) { const { currentPassword, newPassword } = req.body; + // If SAML is enabled, we don't allow the user to change their email address + if (isSamlLicensedAndEnabled()) { + this.logger.debug('Attempted to change password for user, while SAML is enabled', { + userId: req.user?.id, + }); + throw new BadRequestError( + 'With SAML enabled, the users must use the SAML provider to change their password', + ); + } + if (typeof currentPassword !== 'string' || typeof newPassword !== 'string') { throw new BadRequestError('Invalid payload.'); } diff --git a/packages/cli/src/sso/saml/middleware/samlEnabledMiddleware.ts b/packages/cli/src/sso/saml/middleware/samlEnabledMiddleware.ts index bf15030d836ef..257d694e48785 100644 --- a/packages/cli/src/sso/saml/middleware/samlEnabledMiddleware.ts +++ b/packages/cli/src/sso/saml/middleware/samlEnabledMiddleware.ts @@ -1,7 +1,6 @@ import type { RequestHandler } from 'express'; import type { AuthenticatedRequest } from '../../../requests'; -import { isSamlCurrentAuthenticationMethod } from '../../ssoHelpers'; -import { isSamlLoginEnabled, isSamlLicensed } from '../samlHelpers'; +import { isSamlLicensed, isSamlLicensedAndEnabled } from '../samlHelpers'; export const samlLicensedOwnerMiddleware: RequestHandler = ( req: AuthenticatedRequest, @@ -16,7 +15,7 @@ export const samlLicensedOwnerMiddleware: RequestHandler = ( }; export const samlLicensedAndEnabledMiddleware: RequestHandler = (req, res, next) => { - if (isSamlLoginEnabled() && isSamlLicensed() && isSamlCurrentAuthenticationMethod()) { + if (isSamlLicensedAndEnabled()) { next(); } else { res.status(401).json({ status: 'error', message: 'Unauthorized' }); diff --git a/packages/cli/src/sso/saml/samlHelpers.ts b/packages/cli/src/sso/saml/samlHelpers.ts index c6c0c6c30fb88..fb9e0a3b7f4d4 100644 --- a/packages/cli/src/sso/saml/samlHelpers.ts +++ b/packages/cli/src/sso/saml/samlHelpers.ts @@ -10,6 +10,7 @@ import type { SamlUserAttributes } from './types/samlUserAttributes'; import type { FlowResult } from 'samlify/types/src/flow'; import type { SamlAttributeMapping } from './types/samlAttributeMapping'; import { SAML_ENTERPRISE_FEATURE_ENABLED, SAML_LOGIN_ENABLED, SAML_LOGIN_LABEL } from './constants'; +import { isSamlCurrentAuthenticationMethod } from '../ssoHelpers'; /** * Check whether the SAML feature is licensed and enabled in the instance */ @@ -37,6 +38,10 @@ export function isSamlLicensed(): boolean { ); } +export function isSamlLicensedAndEnabled(): boolean { + return isSamlLoginEnabled() && isSamlLicensed() && isSamlCurrentAuthenticationMethod(); +} + export const isSamlPreferences = (candidate: unknown): candidate is SamlPreferences => { const o = candidate as SamlPreferences; return ( diff --git a/packages/cli/src/sso/ssoHelpers.ts b/packages/cli/src/sso/ssoHelpers.ts index f00ddc0fe5b8c..9c8b03e450de9 100644 --- a/packages/cli/src/sso/ssoHelpers.ts +++ b/packages/cli/src/sso/ssoHelpers.ts @@ -11,3 +11,9 @@ export function isSsoJustInTimeProvisioningEnabled(): boolean { export function doRedirectUsersFromLoginToSsoFlow(): boolean { return config.getEnv('sso.redirectLoginToSso'); } + +export function setCurrentAuthenticationMethod( + authenticationMethod: 'email' | 'ldap' | 'saml', +): void { + config.set('userManagement.authenticationMethod', authenticationMethod); +} From 00649bf3ca11f93bafc8b3b645d0074babf52e1d Mon Sep 17 00:00:00 2001 From: Michael Auerswald Date: Tue, 28 Feb 2023 12:19:57 +0100 Subject: [PATCH 05/15] add test --- packages/cli/src/controllers/me.controller.ts | 2 +- .../test/integration/saml/saml.api.test.ts | 78 +++++++++++++++++++ 2 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 packages/cli/test/integration/saml/saml.api.test.ts diff --git a/packages/cli/src/controllers/me.controller.ts b/packages/cli/src/controllers/me.controller.ts index 288009825a69a..e56c820155a5d 100644 --- a/packages/cli/src/controllers/me.controller.ts +++ b/packages/cli/src/controllers/me.controller.ts @@ -126,7 +126,7 @@ export class MeController { userId: req.user?.id, }); throw new BadRequestError( - 'With SAML enabled, the users must use the SAML provider to change their password', + 'With SAML enabled, users need to use their SAML provider to change passwords', ); } diff --git a/packages/cli/test/integration/saml/saml.api.test.ts b/packages/cli/test/integration/saml/saml.api.test.ts new file mode 100644 index 0000000000000..02176989f160d --- /dev/null +++ b/packages/cli/test/integration/saml/saml.api.test.ts @@ -0,0 +1,78 @@ +import express from 'express'; + +import config from '@/config'; +import type { Role } from '@db/entities/Role'; +import { randomEmail, randomName, randomValidPassword } from '../shared/random'; +import * as testDb from '../shared/testDb'; +import type { AuthAgent } from '../shared/types'; +import * as utils from '../shared/utils'; +import { setSamlLoginEnabled } from '../../../src/sso/saml/samlHelpers'; +import { setCurrentAuthenticationMethod } from '../../../src/sso/ssoHelpers'; + +let app: express.Application; +let globalOwnerRole: Role; +let globalMemberRole: Role; +let authAgent: AuthAgent; + +function enableSaml(enable: boolean) { + setSamlLoginEnabled(enable); + setCurrentAuthenticationMethod(enable ? 'saml' : 'email'); + config.set('enterprise.features.saml', enable); +} + +beforeAll(async () => { + app = await utils.initTestServer({ endpointGroups: ['me'] }); + + globalOwnerRole = await testDb.getGlobalOwnerRole(); + globalMemberRole = await testDb.getGlobalMemberRole(); + + authAgent = utils.createAuthAgent(app); +}); + +beforeEach(async () => { + await testDb.truncate(['User']); +}); + +afterAll(async () => { + await testDb.terminate(); +}); + +describe('Owner shell', () => { + test('PATCH /me should succeed with valid inputs', async () => { + const ownerShell = await testDb.createUserShell(globalOwnerRole); + const authOwnerShellAgent = authAgent(ownerShell); + const response = await authOwnerShellAgent.patch('/me').send({ + email: randomEmail(), + firstName: randomName(), + lastName: randomName(), + password: randomValidPassword(), + }); + expect(response.statusCode).toBe(200); + }); + + test('PATCH /me should throw BadRequestError if email is changed when SAML is enabled', async () => { + enableSaml(true); + const ownerShell = await testDb.createUserShell(globalOwnerRole); + const authOwnerShellAgent = authAgent(ownerShell); + const response = await authOwnerShellAgent.patch('/me').send({ + email: randomEmail(), + firstName: randomName(), + lastName: randomName(), + }); + expect(response.statusCode).toBe(400); + expect(response.body.message).toContain('SAML'); + enableSaml(false); + }); + + test('PATCH /password should throw BadRequestError if password is changed when SAML is enabled', async () => { + enableSaml(true); + const ownerShell = await testDb.createUserShell(globalOwnerRole); + const authOwnerShellAgent = authAgent(ownerShell); + const response = await authOwnerShellAgent.patch('/me/password').send({ + password: randomValidPassword(), + }); + expect(response.statusCode).toBe(400); + expect(response.body.message).toContain('SAML'); + enableSaml(false); + }); +}); From 14a60641abed781aede496608fdaacfc70b7e0cb Mon Sep 17 00:00:00 2001 From: Michael Auerswald Date: Tue, 28 Feb 2023 18:46:59 +0100 Subject: [PATCH 06/15] add toggle endpoint and fetch metadata --- packages/cli/src/sso/saml/constants.ts | 2 + .../routes/saml.controller.protected.ee.ts | 29 +++++++-- packages/cli/src/sso/saml/saml.service.ee.ts | 60 +++++++++++++++---- packages/cli/src/sso/saml/types/requests.ts | 1 + .../cli/src/sso/saml/types/samlPreferences.ts | 28 +++++++-- 5 files changed, 97 insertions(+), 23 deletions(-) diff --git a/packages/cli/src/sso/saml/constants.ts b/packages/cli/src/sso/saml/constants.ts index 715c20b932c98..700444e029e65 100644 --- a/packages/cli/src/sso/saml/constants.ts +++ b/packages/cli/src/sso/saml/constants.ts @@ -15,6 +15,8 @@ export class SamlUrls { static readonly config = '/config'; + static readonly configToggleEnabled = '/config/toggle'; + static readonly restConfig = this.samlRESTRoot + this.config; static readonly defaultRedirect = '/'; diff --git a/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts b/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts index 5a4376f692e8a..2e8b2bab55feb 100644 --- a/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts +++ b/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts @@ -8,7 +8,7 @@ import { SamlUrls } from '../constants'; import type { SamlConfiguration } from '../types/requests'; import { AuthError, BadRequestError } from '@/ResponseHelper'; import { issueCookie } from '../../../auth/jwt'; -import { isSamlPreferences } from '../samlHelpers'; +import { validate } from 'class-validator'; export const samlControllerProtected = express.Router(); @@ -27,17 +27,38 @@ samlControllerProtected.get( /** * POST /sso/saml/config - * Return SAML config + * Set SAML config */ samlControllerProtected.post( SamlUrls.config, samlLicensedOwnerMiddleware, async (req: SamlConfiguration.Update, res: express.Response) => { - if (isSamlPreferences(req.body)) { + const validationResult = await validate(req.body); + if (validationResult.length === 0) { const result = await SamlService.getInstance().setSamlPreferences(req.body); return res.send(result); } else { - throw new BadRequestError('Body is not a SamlPreferences object'); + throw new BadRequestError( + 'Body is not a valid SamlPreferences object: ' + + validationResult.map((e) => e.toString()).join(','), + ); + } + }, +); + +/** + * POST /sso/saml/config/toggle + * Set SAML config + */ +samlControllerProtected.post( + SamlUrls.configToggleEnabled, + samlLicensedOwnerMiddleware, + async (req: SamlConfiguration.Toggle, res: express.Response) => { + if (req.body.enabled !== undefined) { + await SamlService.getInstance().setSamlPreferences({ loginEnabled: req.body.enabled }); + res.sendStatus(200); + } else { + throw new BadRequestError('Body should contain a boolean "enabled" property'); } }, ); diff --git a/packages/cli/src/sso/saml/saml.service.ee.ts b/packages/cli/src/sso/saml/saml.service.ee.ts index 72dc3bf2c2c12..3852f9aaba954 100644 --- a/packages/cli/src/sso/saml/saml.service.ee.ts +++ b/packages/cli/src/sso/saml/saml.service.ee.ts @@ -2,7 +2,7 @@ import type express from 'express'; import * as Db from '@/Db'; import type { User } from '@/databases/entities/User'; import { jsonParse, LoggerProxy } from 'n8n-workflow'; -import { AuthError } from '@/ResponseHelper'; +import { AuthError, BadRequestError } from '@/ResponseHelper'; import { getServiceProviderInstance } from './serviceProvider.ee'; import type { SamlUserAttributes } from './types/samlUserAttributes'; import type { SamlAttributeMapping } from './types/samlAttributeMapping'; @@ -20,6 +20,8 @@ import { setSamlLoginLabel, updateUserFromSamlAttributes, } from './samlHelpers'; +import type { Settings } from '../../databases/entities/Settings'; +import axios from 'axios'; export class SamlService { private static instance: SamlService; @@ -44,6 +46,8 @@ export class SamlService { private _metadata = ''; + private metadataUrl = ''; + public get metadata(): string { return this._metadata; } @@ -53,7 +57,7 @@ export class SamlService { } constructor() { - this.loadSamlPreferences() + this.loadFromDbAndApplySamlPreferences() .then(() => { LoggerProxy.debug('Initializing SAML service'); }) @@ -70,7 +74,7 @@ export class SamlService { } async init(): Promise { - await this.loadSamlPreferences(); + await this.loadFromDbAndApplySamlPreferences(); } getIdentityProviderInstance(forceRecreate = false): IdentityProviderInstance { @@ -150,21 +154,30 @@ export class SamlService { return { mapping: this.attributeMapping, metadata: this.metadata, + metadataUrl: this.metadataUrl, loginEnabled: isSamlLoginEnabled(), loginLabel: getSamlLoginLabel(), }; } - async setSamlPreferences(prefs: SamlPreferences): Promise { - this.attributeMapping = prefs.mapping; - this.metadata = prefs.metadata; - setSamlLoginEnabled(prefs.loginEnabled); - setSamlLoginLabel(prefs.loginLabel); + async setSamlPreferences(prefs: SamlPreferences): Promise { + this.metadata = prefs.metadata ?? this.metadata; + this.attributeMapping = prefs.mapping ?? this.attributeMapping; + if (prefs.metadataUrl) { + this.metadataUrl = prefs.metadataUrl; + const fetchedMetadata = await this.fetchMetadataFromUrl(); + if (fetchedMetadata) { + this.metadata = fetchedMetadata; + } + } + setSamlLoginEnabled(prefs.loginEnabled ?? isSamlLoginEnabled()); + setSamlLoginLabel(prefs.loginLabel ?? getSamlLoginLabel()); this.getIdentityProviderInstance(true); - await this.saveSamlPreferences(); + const result = await this.saveSamlPreferencesToDb(); + return result; } - async loadSamlPreferences(): Promise { + async loadFromDbAndApplySamlPreferences(): Promise { const samlPreferences = await Db.collections.Settings.findOne({ where: { key: SAML_PREFERENCES_DB_KEY }, }); @@ -178,21 +191,42 @@ export class SamlService { return; } - async saveSamlPreferences(): Promise { + async saveSamlPreferencesToDb(): Promise { const samlPreferences = await Db.collections.Settings.findOne({ where: { key: SAML_PREFERENCES_DB_KEY }, }); const settingsValue = JSON.stringify(this.getSamlPreferences()); + let result: Settings; if (samlPreferences) { samlPreferences.value = settingsValue; - await Db.collections.Settings.save(samlPreferences); + result = await Db.collections.Settings.save(samlPreferences); } else { - await Db.collections.Settings.save({ + result = await Db.collections.Settings.save({ key: SAML_PREFERENCES_DB_KEY, value: settingsValue, loadOnStartup: true, }); } + if (result) return jsonParse(result.value); + return; + } + + async fetchMetadataFromUrl(): Promise { + try { + const prevRejectStatus = process.env.NODE_TLS_REJECT_UNAUTHORIZED; + process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0'; + const response = await axios.get(this.metadataUrl); + process.env.NODE_TLS_REJECT_UNAUTHORIZED = prevRejectStatus; + if (response.status === 200 && response.data) { + const xml = (await response.data) as string; + // TODO: SAML: validate XML + // throw new BadRequestError('Received XML is not valid SAML metadata.'); + return xml; + } + } catch (error) { + throw new BadRequestError('SAML Metadata URL is invalid or response is .'); + } + return; } async getAttributesFromLoginResponse( diff --git a/packages/cli/src/sso/saml/types/requests.ts b/packages/cli/src/sso/saml/types/requests.ts index c9beab0c21bdc..0b52153eacdc0 100644 --- a/packages/cli/src/sso/saml/types/requests.ts +++ b/packages/cli/src/sso/saml/types/requests.ts @@ -4,4 +4,5 @@ import type { SamlPreferences } from './samlPreferences'; export declare namespace SamlConfiguration { type Read = AuthenticatedRequest<{}, {}, {}, {}>; type Update = AuthenticatedRequest<{}, {}, SamlPreferences, {}>; + type Toggle = AuthenticatedRequest<{}, {}, { enabled: boolean }, {}>; } diff --git a/packages/cli/src/sso/saml/types/samlPreferences.ts b/packages/cli/src/sso/saml/types/samlPreferences.ts index 4edc58fc95902..97183bf0758d9 100644 --- a/packages/cli/src/sso/saml/types/samlPreferences.ts +++ b/packages/cli/src/sso/saml/types/samlPreferences.ts @@ -1,8 +1,24 @@ -import type { SamlAttributeMapping } from './samlAttributeMapping'; +import { IsBoolean, IsObject, IsOptional, IsString } from 'class-validator'; +import { SamlAttributeMapping } from './samlAttributeMapping'; -export interface SamlPreferences { - mapping: SamlAttributeMapping; - metadata: string; - loginEnabled: boolean; - loginLabel: string; +export class SamlPreferences { + @IsObject() + @IsOptional() + mapping?: SamlAttributeMapping; + + @IsString() + @IsOptional() + metadata?: string; + + @IsString() + @IsOptional() + metadataUrl?: string; + + @IsBoolean() + @IsOptional() + loginEnabled?: boolean; + + @IsString() + @IsOptional() + loginLabel?: string; } From d46971e17cd3f73e0a8f5c4a5b64f7307ced0c05 Mon Sep 17 00:00:00 2001 From: Michael Auerswald Date: Tue, 28 Feb 2023 18:51:36 +0100 Subject: [PATCH 07/15] rename enabled param --- .../cli/src/sso/saml/routes/saml.controller.protected.ee.ts | 6 +++--- packages/cli/src/sso/saml/types/requests.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts b/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts index 2e8b2bab55feb..2f64dd3225928 100644 --- a/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts +++ b/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts @@ -54,11 +54,11 @@ samlControllerProtected.post( SamlUrls.configToggleEnabled, samlLicensedOwnerMiddleware, async (req: SamlConfiguration.Toggle, res: express.Response) => { - if (req.body.enabled !== undefined) { - await SamlService.getInstance().setSamlPreferences({ loginEnabled: req.body.enabled }); + if (req.body.loginEnabled !== undefined) { + await SamlService.getInstance().setSamlPreferences({ loginEnabled: req.body.loginEnabled }); res.sendStatus(200); } else { - throw new BadRequestError('Body should contain a boolean "enabled" property'); + throw new BadRequestError('Body should contain a boolean "loginEnabled" property'); } }, ); diff --git a/packages/cli/src/sso/saml/types/requests.ts b/packages/cli/src/sso/saml/types/requests.ts index 0b52153eacdc0..a095df7870147 100644 --- a/packages/cli/src/sso/saml/types/requests.ts +++ b/packages/cli/src/sso/saml/types/requests.ts @@ -4,5 +4,5 @@ import type { SamlPreferences } from './samlPreferences'; export declare namespace SamlConfiguration { type Read = AuthenticatedRequest<{}, {}, {}, {}>; type Update = AuthenticatedRequest<{}, {}, SamlPreferences, {}>; - type Toggle = AuthenticatedRequest<{}, {}, { enabled: boolean }, {}>; + type Toggle = AuthenticatedRequest<{}, {}, { loginEnabled: boolean }, {}>; } From 8c80908ee3ec33a7dcc1445a4c819c14a9cfcaf8 Mon Sep 17 00:00:00 2001 From: Michael Auerswald Date: Wed, 1 Mar 2023 14:38:14 +0100 Subject: [PATCH 08/15] add handling of POST saml login request --- .../routes/saml.controller.protected.ee.ts | 14 ++++-- packages/cli/src/sso/saml/saml.service.ee.ts | 43 +++++++++++++++++-- packages/cli/src/sso/saml/types/index.ts | 1 + .../cli/src/sso/saml/types/samlPreferences.ts | 5 +++ .../cli/src/sso/saml/views/initSsoPost.ts | 15 +++++++ .../cli/src/sso/saml/views/initSsoRedirect.ts | 12 ++++++ 6 files changed, 82 insertions(+), 8 deletions(-) create mode 100644 packages/cli/src/sso/saml/types/index.ts create mode 100644 packages/cli/src/sso/saml/views/initSsoPost.ts create mode 100644 packages/cli/src/sso/saml/views/initSsoRedirect.ts diff --git a/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts b/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts index 2f64dd3225928..0353b26fe3490 100644 --- a/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts +++ b/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts @@ -9,6 +9,9 @@ import type { SamlConfiguration } from '../types/requests'; import { AuthError, BadRequestError } from '@/ResponseHelper'; import { issueCookie } from '../../../auth/jwt'; import { validate } from 'class-validator'; +import type { PostBindingContext } from 'samlify/types/src/entity'; +import { getInitSSOFormView } from '../views/initSsoPost'; +import { getInitSSOPostView } from '../views/initSsoRedirect'; export const samlControllerProtected = express.Router(); @@ -117,10 +120,13 @@ samlControllerProtected.get( SamlUrls.initSSO, samlLicensedAndEnabledMiddleware, async (req: express.Request, res: express.Response) => { - const url = SamlService.getInstance().getRedirectLoginRequestUrl(); - if (url) { - // TODO:SAML: redirect to the URL on the client side - return res.status(301).send(url); + const result = SamlService.getInstance().getLoginRequestUrl(); + if (result?.binding === 'redirect') { + // forced client side redirect + return res.send(getInitSSOPostView(result.context)); + // return res.status(301).send(result.context.context); + } else if (result?.binding === 'post') { + return res.send(getInitSSOFormView(result.context as PostBindingContext)); } else { throw new AuthError('SAML redirect failed, please check your SAML configuration.'); } diff --git a/packages/cli/src/sso/saml/saml.service.ee.ts b/packages/cli/src/sso/saml/saml.service.ee.ts index 3852f9aaba954..09a0738022487 100644 --- a/packages/cli/src/sso/saml/saml.service.ee.ts +++ b/packages/cli/src/sso/saml/saml.service.ee.ts @@ -22,6 +22,8 @@ import { } from './samlHelpers'; import type { Settings } from '../../databases/entities/Settings'; import axios from 'axios'; +import type { SamlLoginBinding } from './types'; +import type { BindingContext, PostBindingContext } from 'samlify/types/src/entity'; export class SamlService { private static instance: SamlService; @@ -48,6 +50,8 @@ export class SamlService { private metadataUrl = ''; + private loginBinding: SamlLoginBinding = 'post'; + public get metadata(): string { return this._metadata; } @@ -87,19 +91,48 @@ export class SamlService { return this.identityProviderInstance; } - getRedirectLoginRequestUrl(): string { + getLoginRequestUrl(binding?: SamlLoginBinding): { + binding: SamlLoginBinding; + context: BindingContext | PostBindingContext; + } { + if (binding === undefined) binding = this.loginBinding; + if (binding === 'post') { + return { + binding, + context: this.getPostLoginRequestUrl(), + }; + } else { + return { + binding, + context: this.getRedirectLoginRequestUrl(), + }; + } + } + + private getRedirectLoginRequestUrl(): BindingContext { const loginRequest = getServiceProviderInstance().createLoginRequest( this.getIdentityProviderInstance(), 'redirect', ); //TODO:SAML: debug logging LoggerProxy.debug(loginRequest.context); - return loginRequest.context; + return loginRequest; + } + + private getPostLoginRequestUrl(): PostBindingContext { + const loginRequest = getServiceProviderInstance().createLoginRequest( + this.getIdentityProviderInstance(), + 'post', + ) as PostBindingContext; + //TODO:SAML: debug logging + + LoggerProxy.debug(loginRequest.context); + return loginRequest; } async handleSamlLogin( req: express.Request, - binding: 'post' | 'redirect', + binding: SamlLoginBinding, ): Promise< | { authenticatedUser: User | undefined; @@ -155,12 +188,14 @@ export class SamlService { mapping: this.attributeMapping, metadata: this.metadata, metadataUrl: this.metadataUrl, + loginBinding: this.loginBinding, loginEnabled: isSamlLoginEnabled(), loginLabel: getSamlLoginLabel(), }; } async setSamlPreferences(prefs: SamlPreferences): Promise { + this.loginBinding = prefs.loginBinding ?? this.loginBinding; this.metadata = prefs.metadata ?? this.metadata; this.attributeMapping = prefs.mapping ?? this.attributeMapping; if (prefs.metadataUrl) { @@ -231,7 +266,7 @@ export class SamlService { async getAttributesFromLoginResponse( req: express.Request, - binding: 'post' | 'redirect', + binding: SamlLoginBinding, ): Promise { let parsedSamlResponse; try { diff --git a/packages/cli/src/sso/saml/types/index.ts b/packages/cli/src/sso/saml/types/index.ts new file mode 100644 index 0000000000000..560f7003f821a --- /dev/null +++ b/packages/cli/src/sso/saml/types/index.ts @@ -0,0 +1 @@ +export type SamlLoginBinding = 'post' | 'redirect'; diff --git a/packages/cli/src/sso/saml/types/samlPreferences.ts b/packages/cli/src/sso/saml/types/samlPreferences.ts index 97183bf0758d9..e43b588b819fb 100644 --- a/packages/cli/src/sso/saml/types/samlPreferences.ts +++ b/packages/cli/src/sso/saml/types/samlPreferences.ts @@ -1,4 +1,5 @@ import { IsBoolean, IsObject, IsOptional, IsString } from 'class-validator'; +import { SamlLoginBinding } from '.'; import { SamlAttributeMapping } from './samlAttributeMapping'; export class SamlPreferences { @@ -14,6 +15,10 @@ export class SamlPreferences { @IsOptional() metadataUrl?: string; + @IsString() + @IsOptional() + loginBinding?: SamlLoginBinding = 'redirect'; + @IsBoolean() @IsOptional() loginEnabled?: boolean; diff --git a/packages/cli/src/sso/saml/views/initSsoPost.ts b/packages/cli/src/sso/saml/views/initSsoPost.ts new file mode 100644 index 0000000000000..4df6719efb13b --- /dev/null +++ b/packages/cli/src/sso/saml/views/initSsoPost.ts @@ -0,0 +1,15 @@ +import type { PostBindingContext } from 'samlify/types/src/entity'; + +export function getInitSSOFormView(context: PostBindingContext): string { + return ` +
+ + ${context.relayState ? '' : ''} +
+`; +} diff --git a/packages/cli/src/sso/saml/views/initSsoRedirect.ts b/packages/cli/src/sso/saml/views/initSsoRedirect.ts new file mode 100644 index 0000000000000..56db9ce0838d7 --- /dev/null +++ b/packages/cli/src/sso/saml/views/initSsoRedirect.ts @@ -0,0 +1,12 @@ +import type { BindingContext } from 'samlify/types/src/entity'; + +export function getInitSSOPostView(context: BindingContext): string { + return ` + + `; +} From 3ca37a7065fa932fa2c94ab9fff93e389569a5e3 Mon Sep 17 00:00:00 2001 From: Michael Auerswald Date: Wed, 1 Mar 2023 15:06:04 +0100 Subject: [PATCH 09/15] add config test endpoint --- packages/cli/src/sso/saml/constants.ts | 2 + .../routes/saml.controller.protected.ee.ts | 8 ++++ packages/cli/src/sso/saml/saml.service.ee.ts | 37 +++++++++++++++++++ 3 files changed, 47 insertions(+) diff --git a/packages/cli/src/sso/saml/constants.ts b/packages/cli/src/sso/saml/constants.ts index 700444e029e65..6f92690f5f5d1 100644 --- a/packages/cli/src/sso/saml/constants.ts +++ b/packages/cli/src/sso/saml/constants.ts @@ -15,6 +15,8 @@ export class SamlUrls { static readonly config = '/config'; + static readonly configTest = '/config/test'; + static readonly configToggleEnabled = '/config/toggle'; static readonly restConfig = this.samlRESTRoot + this.config; diff --git a/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts b/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts index 0353b26fe3490..9a98e6926bd0f 100644 --- a/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts +++ b/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts @@ -132,3 +132,11 @@ samlControllerProtected.get( } }, ); + +samlControllerProtected.get( + SamlUrls.configTest, + async (req: express.Request, res: express.Response) => { + const testResult = await SamlService.getInstance().testSamlConnection(); + return res.send(testResult); + }, +); diff --git a/packages/cli/src/sso/saml/saml.service.ee.ts b/packages/cli/src/sso/saml/saml.service.ee.ts index 09a0738022487..d4fd3b40ba99a 100644 --- a/packages/cli/src/sso/saml/saml.service.ee.ts +++ b/packages/cli/src/sso/saml/saml.service.ee.ts @@ -295,4 +295,41 @@ export class SamlService { } return attributes; } + + async testSamlConnection(): Promise { + try { + const requestContext = this.getLoginRequestUrl(); + if (!requestContext) return false; + if (requestContext.binding === 'redirect') { + const fetchResult = await axios.get(requestContext.context.context); + if (fetchResult.status !== 200) { + LoggerProxy.debug('SAML: Error while testing SAML connection.'); + return false; + } + } else if (requestContext.binding === 'post') { + const context = requestContext.context as PostBindingContext; + const endpoint = context.entityEndpoint; + const params = new URLSearchParams(); + params.append(context.type, context.context); + if (context.relayState) { + params.append('RelayState', context.relayState); + } + const fetchResult = await axios.post(endpoint, params, { + headers: { + // eslint-disable-next-line @typescript-eslint/naming-convention + 'Content-type': 'application/x-www-form-urlencoded', + }, + }); + if (fetchResult.status !== 200) { + LoggerProxy.debug('SAML: Error while testing SAML connection.'); + return false; + } + } + return true; + } catch (error) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-argument + LoggerProxy.debug('SAML: Error while testing SAML connection: ', error); + } + return false; + } } From d28925b9b83305232af58f05cbbbb04fe510b28a Mon Sep 17 00:00:00 2001 From: Michael Auerswald Date: Thu, 2 Mar 2023 16:37:06 +0100 Subject: [PATCH 10/15] adds saml XML validation --- packages/cli/package.json | 3 +- packages/cli/src/Server.ts | 6 +- packages/cli/src/sso/saml/saml.service.ee.ts | 54 ++- packages/cli/src/sso/saml/samlValidator.ts | 92 +++++ .../schema/saml-schema-assertion-2.0.xsd.ts | 283 +++++++++++++++ .../schema/saml-schema-metadata-2.0.xsd.ts | 336 ++++++++++++++++++ .../schema/saml-schema-protocol-2.0.xsd.ts | 302 ++++++++++++++++ .../src/sso/saml/schema/xenc-schema.xsd.ts | 145 ++++++++ packages/cli/src/sso/saml/schema/xml.xsd.ts | 117 ++++++ .../saml/schema/xmldsig-core-schema.xsd.ts | 318 +++++++++++++++++ .../cli/src/sso/saml/serviceProvider.ee.ts | 10 +- .../cli/src/sso/saml/types/samlPreferences.ts | 4 + packages/cli/src/sso/ssoHelpers.ts | 5 +- packages/cli/tsconfig.json | 2 +- pnpm-lock.yaml | 12 +- 15 files changed, 1655 insertions(+), 34 deletions(-) create mode 100644 packages/cli/src/sso/saml/samlValidator.ts create mode 100644 packages/cli/src/sso/saml/schema/saml-schema-assertion-2.0.xsd.ts create mode 100644 packages/cli/src/sso/saml/schema/saml-schema-metadata-2.0.xsd.ts create mode 100644 packages/cli/src/sso/saml/schema/saml-schema-protocol-2.0.xsd.ts create mode 100644 packages/cli/src/sso/saml/schema/xenc-schema.xsd.ts create mode 100644 packages/cli/src/sso/saml/schema/xml.xsd.ts create mode 100644 packages/cli/src/sso/saml/schema/xmldsig-core-schema.xsd.ts diff --git a/packages/cli/package.json b/packages/cli/package.json index a61b8979a16a8..2b64dae5ddc84 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -129,8 +129,8 @@ "bull": "^4.10.2", "callsites": "^3.1.0", "change-case": "^4.1.1", - "class-validator": "^0.14.0", "class-transformer": "^0.5.1", + "class-validator": "^0.14.0", "client-oauth2": "^4.2.5", "compression": "^1.7.4", "connect-history-api-fallback": "^1.6.0", @@ -205,6 +205,7 @@ "validator": "13.7.0", "winston": "^3.3.3", "ws": "^8.12.0", + "xmllint-wasm": "^3.0.1", "yamljs": "^0.3.0" } } diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index af3a5b5e56482..28351cf0eb9f5 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -516,7 +516,11 @@ class Server extends AbstractServer { // ---------------------------------------- // initialize SamlService - await SamlService.getInstance().init(); + try { + await SamlService.getInstance().init(); + } catch (error) { + LoggerProxy.error(`SAML initialization failed: ${error.message}`); + } // public SAML endpoints this.app.use(`/${this.restEndpoint}/sso/saml`, samlControllerPublic); diff --git a/packages/cli/src/sso/saml/saml.service.ee.ts b/packages/cli/src/sso/saml/saml.service.ee.ts index d4fd3b40ba99a..4485a0f6c83d4 100644 --- a/packages/cli/src/sso/saml/saml.service.ee.ts +++ b/packages/cli/src/sso/saml/saml.service.ee.ts @@ -10,7 +10,7 @@ import { isSsoJustInTimeProvisioningEnabled } from '../ssoHelpers'; import type { SamlPreferences } from './types/samlPreferences'; import { SAML_PREFERENCES_DB_KEY } from './constants'; import type { IdentityProviderInstance } from 'samlify'; -import { IdentityProvider } from 'samlify'; +import { IdentityProvider, setSchemaValidator } from 'samlify'; import { createUserFromSamlAttributes, getMappedSamlAttributesFromFlowResult, @@ -24,6 +24,7 @@ import type { Settings } from '../../databases/entities/Settings'; import axios from 'axios'; import type { SamlLoginBinding } from './types'; import type { BindingContext, PostBindingContext } from 'samlify/types/src/entity'; +import { validateMetadata, validateResponse } from './samlValidator'; export class SamlService { private static instance: SamlService; @@ -46,19 +47,13 @@ export class SamlService { this._attributeMapping = mapping; } - private _metadata = ''; + private metadata = ''; private metadataUrl = ''; - private loginBinding: SamlLoginBinding = 'post'; - - public get metadata(): string { - return this._metadata; - } + private ignoreSSL = false; - public set metadata(metadata: string) { - this._metadata = metadata; - } + private loginBinding: SamlLoginBinding = 'post'; constructor() { this.loadFromDbAndApplySamlPreferences() @@ -66,19 +61,32 @@ export class SamlService { LoggerProxy.debug('Initializing SAML service'); }) .catch(() => { - LoggerProxy.error('Error initializing SAML service'); + LoggerProxy.warn('Error initializing SAML service'); }); } static getInstance(): SamlService { if (!SamlService.instance) { SamlService.instance = new SamlService(); + SamlService.instance.init().catch((error) => { + // eslint-disable-next-line @typescript-eslint/no-unsafe-argument + LoggerProxy.error(error); + }); } return SamlService.instance; } async init(): Promise { await this.loadFromDbAndApplySamlPreferences(); + setSchemaValidator({ + validate: async (response: string) => { + const valid = await validateResponse(response); + if (!valid) { + return Promise.reject(new Error('Invalid SAML response')); + } + return Promise.resolve(); + }, + }); } getIdentityProviderInstance(forceRecreate = false): IdentityProviderInstance { @@ -125,7 +133,6 @@ export class SamlService { 'post', ) as PostBindingContext; //TODO:SAML: debug logging - LoggerProxy.debug(loginRequest.context); return loginRequest; } @@ -188,6 +195,7 @@ export class SamlService { mapping: this.attributeMapping, metadata: this.metadata, metadataUrl: this.metadataUrl, + ignoreSSL: this.ignoreSSL, loginBinding: this.loginBinding, loginEnabled: isSamlLoginEnabled(), loginLabel: getSamlLoginLabel(), @@ -198,12 +206,19 @@ export class SamlService { this.loginBinding = prefs.loginBinding ?? this.loginBinding; this.metadata = prefs.metadata ?? this.metadata; this.attributeMapping = prefs.mapping ?? this.attributeMapping; + this.ignoreSSL = prefs.ignoreSSL ?? this.ignoreSSL; if (prefs.metadataUrl) { this.metadataUrl = prefs.metadataUrl; const fetchedMetadata = await this.fetchMetadataFromUrl(); if (fetchedMetadata) { this.metadata = fetchedMetadata; } + } else if (prefs.metadata) { + const validationResult = await validateMetadata(prefs.metadata); + if (!validationResult) { + throw new Error('Invalid SAML metadata'); + } + this.metadata = prefs.metadata; } setSamlLoginEnabled(prefs.loginEnabled ?? isSamlLoginEnabled()); setSamlLoginLabel(prefs.loginLabel ?? getSamlLoginLabel()); @@ -249,17 +264,22 @@ export class SamlService { async fetchMetadataFromUrl(): Promise { try { const prevRejectStatus = process.env.NODE_TLS_REJECT_UNAUTHORIZED; - process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0'; + if (this.ignoreSSL) process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0'; const response = await axios.get(this.metadataUrl); - process.env.NODE_TLS_REJECT_UNAUTHORIZED = prevRejectStatus; + if (this.ignoreSSL) process.env.NODE_TLS_REJECT_UNAUTHORIZED = prevRejectStatus; if (response.status === 200 && response.data) { const xml = (await response.data) as string; - // TODO: SAML: validate XML - // throw new BadRequestError('Received XML is not valid SAML metadata.'); + const validationResult = await validateMetadata(xml); + if (!validationResult) { + throw new BadRequestError( + `Data received from ${this.metadataUrl} is not valid SAML metadata.`, + ); + } return xml; } } catch (error) { - throw new BadRequestError('SAML Metadata URL is invalid or response is .'); + // eslint-disable-next-line @typescript-eslint/restrict-template-expressions + throw new BadRequestError(`Error fetching SAML Metadata from ${this.metadataUrl}: ${error}`); } return; } diff --git a/packages/cli/src/sso/saml/samlValidator.ts b/packages/cli/src/sso/saml/samlValidator.ts new file mode 100644 index 0000000000000..180d3affa2734 --- /dev/null +++ b/packages/cli/src/sso/saml/samlValidator.ts @@ -0,0 +1,92 @@ +import { LoggerProxy } from 'n8n-workflow'; +import type { XMLFileInfo } from 'xmllint-wasm'; +import { validateXML } from 'xmllint-wasm'; +import { xsdSamlSchemaAssertion20 } from './schema/saml-schema-assertion-2.0.xsd'; +import { xsdSamlSchemaMetadata20 } from './schema/saml-schema-metadata-2.0.xsd'; +import { xsdSamlSchemaProtocol20 } from './schema/saml-schema-protocol-2.0.xsd'; +import { xsdXenc } from './schema/xenc-schema.xsd'; +import { xsdXml } from './schema/xml.xsd'; +import { xsdXmldsigCore } from './schema/xmldsig-core-schema.xsd'; + +const xml: XMLFileInfo = { + fileName: 'xml.xsd', + contents: xsdXml, +}; + +const xmldsigCore: XMLFileInfo = { + fileName: 'xmldsig-core-schema.xsd', + contents: xsdXmldsigCore, +}; + +const xmlXenc: XMLFileInfo = { + fileName: 'xenc-schema.xsd', + contents: xsdXenc, +}; + +const xmlMetadata: XMLFileInfo = { + fileName: 'saml-schema-metadata-2.0.xsd', + contents: xsdSamlSchemaMetadata20, +}; + +const xmlAssertion: XMLFileInfo = { + fileName: 'saml-schema-assertion-2.0.xsd', + contents: xsdSamlSchemaAssertion20, +}; + +const xmlProtocol: XMLFileInfo = { + fileName: 'saml-schema-protocol-2.0.xsd', + contents: xsdSamlSchemaProtocol20, +}; + +export async function validateMetadata(metadata: string): Promise { + try { + const validationResult = await validateXML({ + xml: [ + { + fileName: 'metadata.xml', + contents: metadata, + }, + ], + extension: 'schema', + schema: [xmlMetadata], + preload: [xmlProtocol, xmlAssertion, xmldsigCore, xmlXenc, xml], + }); + if (validationResult.valid) { + console.log(validationResult); + return true; + } else { + LoggerProxy.warn('SAML Validate Metadata: Invalid metadata'); + LoggerProxy.warn(validationResult.errors.join('\n')); + } + } catch (error) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-argument + LoggerProxy.warn(error); + } + return false; +} + +export async function validateResponse(response: string): Promise { + try { + const validationResult = await validateXML({ + xml: [ + { + fileName: 'response.xml', + contents: response, + }, + ], + extension: 'schema', + schema: [xmlMetadata], + preload: [xmlMetadata, xmlAssertion, xmldsigCore, xmlXenc, xml], + }); + if (validationResult.valid) { + return true; + } else { + LoggerProxy.warn('SAML Validate Response: Failed'); + LoggerProxy.warn(validationResult.errors.join('\n')); + } + } catch (error) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-argument + LoggerProxy.warn(error); + } + return false; +} diff --git a/packages/cli/src/sso/saml/schema/saml-schema-assertion-2.0.xsd.ts b/packages/cli/src/sso/saml/schema/saml-schema-assertion-2.0.xsd.ts new file mode 100644 index 0000000000000..ff8d93c98988f --- /dev/null +++ b/packages/cli/src/sso/saml/schema/saml-schema-assertion-2.0.xsd.ts @@ -0,0 +1,283 @@ +export const xsdSamlSchemaAssertion20 = ` + + + + + + Document identifier: saml-schema-assertion-2.0 + Location: http://docs.oasis-open.org/security/saml/v2.0/ + Revision history: + V1.0 (November, 2002): + Initial Standard Schema. + V1.1 (September, 2003): + Updates within the same V1.0 namespace. + V2.0 (March, 2005): + New assertion schema for SAML V2.0 namespace. + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +`; diff --git a/packages/cli/src/sso/saml/schema/saml-schema-metadata-2.0.xsd.ts b/packages/cli/src/sso/saml/schema/saml-schema-metadata-2.0.xsd.ts new file mode 100644 index 0000000000000..664168ef9398c --- /dev/null +++ b/packages/cli/src/sso/saml/schema/saml-schema-metadata-2.0.xsd.ts @@ -0,0 +1,336 @@ +export const xsdSamlSchemaMetadata20 = ` + + + + + + + + Document identifier: saml-schema-metadata-2.0 + Location: http://docs.oasis-open.org/security/saml/v2.0/ + Revision history: + V2.0 (March, 2005): + Schema for SAML metadata, first published in SAML 2.0. + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +`; diff --git a/packages/cli/src/sso/saml/schema/saml-schema-protocol-2.0.xsd.ts b/packages/cli/src/sso/saml/schema/saml-schema-protocol-2.0.xsd.ts new file mode 100644 index 0000000000000..18e27b006b18b --- /dev/null +++ b/packages/cli/src/sso/saml/schema/saml-schema-protocol-2.0.xsd.ts @@ -0,0 +1,302 @@ +export const xsdSamlSchemaProtocol20 = ` + + + + + + Document identifier: saml-schema-protocol-2.0 + Location: http://docs.oasis-open.org/security/saml/v2.0/ + Revision history: + V1.0 (November, 2002): + Initial Standard Schema. + V1.1 (September, 2003): + Updates within the same V1.0 namespace. + V2.0 (March, 2005): + New protocol schema based in a SAML V2.0 namespace. + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +`; diff --git a/packages/cli/src/sso/saml/schema/xenc-schema.xsd.ts b/packages/cli/src/sso/saml/schema/xenc-schema.xsd.ts new file mode 100644 index 0000000000000..de9d3ca34e58b --- /dev/null +++ b/packages/cli/src/sso/saml/schema/xenc-schema.xsd.ts @@ -0,0 +1,145 @@ +export const xsdXenc = ` + + + + + ]> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +`; diff --git a/packages/cli/src/sso/saml/schema/xml.xsd.ts b/packages/cli/src/sso/saml/schema/xml.xsd.ts new file mode 100644 index 0000000000000..4487356ea5570 --- /dev/null +++ b/packages/cli/src/sso/saml/schema/xml.xsd.ts @@ -0,0 +1,117 @@ +export const xsdXml = ` + + + + + + See http://www.w3.org/XML/1998/namespace.html and + http://www.w3.org/TR/REC-xml for information about this namespace. + + This schema document describes the XML namespace, in a form + suitable for import by other schema documents. + + Note that local names in this namespace are intended to be defined + only by the World Wide Web Consortium or its subgroups. The + following names are currently defined in this namespace and should + not be used with conflicting semantics by any Working Group, + specification, or document instance: + + base (as an attribute name): denotes an attribute whose value + provides a URI to be used as the base for interpreting any + relative URIs in the scope of the element on which it + appears; its value is inherited. This name is reserved + by virtue of its definition in the XML Base specification. + + lang (as an attribute name): denotes an attribute whose value + is a language code for the natural language of the content of + any element; its value is inherited. This name is reserved + by virtue of its definition in the XML specification. + + space (as an attribute name): denotes an attribute whose + value is a keyword indicating what whitespace processing + discipline is intended for the content of the element; its + value is inherited. This name is reserved by virtue of its + definition in the XML specification. + + Father (in any context at all): denotes Jon Bosak, the chair of + the original XML Working Group. This name is reserved by + the following decision of the W3C XML Plenary and + XML Coordination groups: + + In appreciation for his vision, leadership and dedication + the W3C XML Plenary on this 10th day of February, 2000 + reserves for Jon Bosak in perpetuity the XML name + xml:Father + + + + + This schema defines attributes and an attribute group + suitable for use by + schemas wishing to allow xml:base, xml:lang or xml:space attributes + on elements they define. + + To enable this, such a schema must import this schema + for the XML namespace, e.g. as follows: + <schema . . .> + . . . + <import namespace="http://www.w3.org/XML/1998/namespace" + schemaLocation="xml.xsd"/> + + Subsequently, qualified reference to any of the attributes + or the group defined below will have the desired effect, e.g. + + <type . . .> + . . . + <attributeGroup ref="xml:specialAttrs"/> + + will define a type which will schema-validate an instance + element with any of those attributes + + + + In keeping with the XML Schema WG's standard versioning + policy, this schema document will persist at + http://www.w3.org/2001/03/xml.xsd. + At the date of issue it can also be found at + http://www.w3.org/2001/xml.xsd. + The schema document at that URI may however change in the future, + in order to remain compatible with the latest version of XML Schema + itself. In other words, if the XML Schema namespace changes, the version + of this document at + http://www.w3.org/2001/xml.xsd will change + accordingly; the version at + http://www.w3.org/2001/03/xml.xsd will not change. + + + + + + In due course, we should install the relevant ISO 2- and 3-letter + codes as the enumerated possible values . . . + + + + + + + + + + + + + + + See http://www.w3.org/TR/xmlbase/ for + information about this attribute. + + + + + + + + + +`; diff --git a/packages/cli/src/sso/saml/schema/xmldsig-core-schema.xsd.ts b/packages/cli/src/sso/saml/schema/xmldsig-core-schema.xsd.ts new file mode 100644 index 0000000000000..9cd615b616ffb --- /dev/null +++ b/packages/cli/src/sso/saml/schema/xmldsig-core-schema.xsd.ts @@ -0,0 +1,318 @@ +export const xsdXmldsigCore = ` + + + + + ]> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +`; diff --git a/packages/cli/src/sso/saml/serviceProvider.ee.ts b/packages/cli/src/sso/saml/serviceProvider.ee.ts index b99bc71a185ad..020ab83ed938b 100644 --- a/packages/cli/src/sso/saml/serviceProvider.ee.ts +++ b/packages/cli/src/sso/saml/serviceProvider.ee.ts @@ -1,18 +1,10 @@ import { getInstanceBaseUrl } from '@/UserManagement/UserManagementHelper'; import type { ServiceProviderInstance } from 'samlify'; -import { ServiceProvider, setSchemaValidator } from 'samlify'; +import { ServiceProvider } from 'samlify'; import { SamlUrls } from './constants'; let serviceProviderInstance: ServiceProviderInstance | undefined; -setSchemaValidator({ - // eslint-disable-next-line @typescript-eslint/no-unused-vars - validate: async (response: string) => { - // TODO:SAML: implment validation - return Promise.resolve('skipped'); - }, -}); - const metadata = ` =10.5.0'} + dev: false + /xpath/0.0.32: resolution: {integrity: sha512-rxMJhSIoiO8vXcWvSifKqhvV96GjiD5wYb8/QHdoRyQvraTpp4IEv944nhGausZZ3u7dhQXteZuZbaqfpB7uYw==} engines: {node: '>=0.6.0'} From 13e85f85fc0ebe4ed1656e12969d3b19c20152f4 Mon Sep 17 00:00:00 2001 From: Michael Auerswald Date: Thu, 2 Mar 2023 17:01:18 +0100 Subject: [PATCH 11/15] add comment --- .../cli/src/sso/saml/routes/saml.controller.protected.ee.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts b/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts index 9a98e6926bd0f..419086c9068cc 100644 --- a/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts +++ b/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts @@ -133,6 +133,10 @@ samlControllerProtected.get( }, ); +/** + * GET /sso/saml/config/test + * Test SAML config + */ samlControllerProtected.get( SamlUrls.configTest, async (req: express.Request, res: express.Response) => { From 88fba2b792f38ebdf42ddbaf807ea668ab0ed716 Mon Sep 17 00:00:00 2001 From: Michael Auerswald Date: Thu, 2 Mar 2023 17:01:54 +0100 Subject: [PATCH 12/15] protect test endpoint --- packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts b/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts index 419086c9068cc..065beebc6bcef 100644 --- a/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts +++ b/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts @@ -139,6 +139,7 @@ samlControllerProtected.get( */ samlControllerProtected.get( SamlUrls.configTest, + samlLicensedOwnerMiddleware, async (req: express.Request, res: express.Response) => { const testResult = await SamlService.getInstance().testSamlConnection(); return res.send(testResult); From a4612d9551675e304cd5d4ab4bd10a70c0fd844c Mon Sep 17 00:00:00 2001 From: Michael Auerswald Date: Fri, 3 Mar 2023 11:04:57 +0100 Subject: [PATCH 13/15] improve ignoreSSL and some cleanup --- packages/cli/src/Server.ts | 14 +++++---- .../routes/saml.controller.protected.ee.ts | 10 +++--- packages/cli/src/sso/saml/saml.service.ee.ts | 31 +++++++------------ packages/cli/src/sso/saml/samlValidator.ts | 3 +- 4 files changed, 27 insertions(+), 31 deletions(-) diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index 28351cf0eb9f5..4c73c7c3565a6 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -515,14 +515,16 @@ class Server extends AbstractServer { // SAML // ---------------------------------------- - // initialize SamlService - try { - await SamlService.getInstance().init(); - } catch (error) { - LoggerProxy.error(`SAML initialization failed: ${error.message}`); + // initialize SamlService if it is licensed, even if not enabled, to + // set up the initial environment + if (isSamlLicensed()) { + try { + await SamlService.getInstance().init(); + } catch (error) { + LoggerProxy.error(`SAML initialization failed: ${error.message}`); + } } - // public SAML endpoints this.app.use(`/${this.restEndpoint}/sso/saml`, samlControllerPublic); this.app.use(`/${this.restEndpoint}/sso/saml`, samlControllerProtected); diff --git a/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts b/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts index 065beebc6bcef..47e625817cb3d 100644 --- a/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts +++ b/packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts @@ -57,12 +57,11 @@ samlControllerProtected.post( SamlUrls.configToggleEnabled, samlLicensedOwnerMiddleware, async (req: SamlConfiguration.Toggle, res: express.Response) => { - if (req.body.loginEnabled !== undefined) { - await SamlService.getInstance().setSamlPreferences({ loginEnabled: req.body.loginEnabled }); - res.sendStatus(200); - } else { + if (req.body.loginEnabled === undefined) { throw new BadRequestError('Body should contain a boolean "loginEnabled" property'); } + await SamlService.getInstance().setSamlPreferences({ loginEnabled: req.body.loginEnabled }); + res.sendStatus(200); }, ); @@ -122,8 +121,9 @@ samlControllerProtected.get( async (req: express.Request, res: express.Response) => { const result = SamlService.getInstance().getLoginRequestUrl(); if (result?.binding === 'redirect') { - // forced client side redirect + // forced client side redirect through the use of a javascript redirect return res.send(getInitSSOPostView(result.context)); + // TODO:SAML: If we want the frontend to handle the redirect, we will send the redirect URL instead: // return res.status(301).send(result.context.context); } else if (result?.binding === 'post') { return res.send(getInitSSOFormView(result.context as PostBindingContext)); diff --git a/packages/cli/src/sso/saml/saml.service.ee.ts b/packages/cli/src/sso/saml/saml.service.ee.ts index 4485a0f6c83d4..b204c1686b73b 100644 --- a/packages/cli/src/sso/saml/saml.service.ee.ts +++ b/packages/cli/src/sso/saml/saml.service.ee.ts @@ -22,6 +22,7 @@ import { } from './samlHelpers'; 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'; @@ -55,23 +56,9 @@ export class SamlService { private loginBinding: SamlLoginBinding = 'post'; - constructor() { - this.loadFromDbAndApplySamlPreferences() - .then(() => { - LoggerProxy.debug('Initializing SAML service'); - }) - .catch(() => { - LoggerProxy.warn('Error initializing SAML service'); - }); - } - static getInstance(): SamlService { if (!SamlService.instance) { SamlService.instance = new SamlService(); - SamlService.instance.init().catch((error) => { - // eslint-disable-next-line @typescript-eslint/no-unsafe-argument - LoggerProxy.error(error); - }); } return SamlService.instance; } @@ -263,10 +250,11 @@ export class SamlService { async fetchMetadataFromUrl(): Promise { try { - const prevRejectStatus = process.env.NODE_TLS_REJECT_UNAUTHORIZED; - if (this.ignoreSSL) process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0'; - const response = await axios.get(this.metadataUrl); - if (this.ignoreSSL) process.env.NODE_TLS_REJECT_UNAUTHORIZED = prevRejectStatus; + // TODO:SAML: this will not work once axios is upgraded to > 1.2.0 (see checkServerIdentity) + const agent = new https.Agent({ + rejectUnauthorized: !this.ignoreSSL, + }); + const response = await axios.get(this.metadataUrl, { httpsAgent: agent }); if (response.status === 200 && response.data) { const xml = (await response.data) as string; const validationResult = await validateMetadata(xml); @@ -318,10 +306,14 @@ export class SamlService { async testSamlConnection(): Promise { try { + // TODO:SAML: this will not work once axios is upgraded to > 1.2.0 (see checkServerIdentity) + const agent = new https.Agent({ + rejectUnauthorized: !this.ignoreSSL, + }); const requestContext = this.getLoginRequestUrl(); if (!requestContext) return false; if (requestContext.binding === 'redirect') { - const fetchResult = await axios.get(requestContext.context.context); + const fetchResult = await axios.get(requestContext.context.context, { httpsAgent: agent }); if (fetchResult.status !== 200) { LoggerProxy.debug('SAML: Error while testing SAML connection.'); return false; @@ -339,6 +331,7 @@ export class SamlService { // eslint-disable-next-line @typescript-eslint/naming-convention 'Content-type': 'application/x-www-form-urlencoded', }, + httpsAgent: agent, }); if (fetchResult.status !== 200) { LoggerProxy.debug('SAML: Error while testing SAML connection.'); diff --git a/packages/cli/src/sso/saml/samlValidator.ts b/packages/cli/src/sso/saml/samlValidator.ts index 180d3affa2734..bf24e9782cdde 100644 --- a/packages/cli/src/sso/saml/samlValidator.ts +++ b/packages/cli/src/sso/saml/samlValidator.ts @@ -52,7 +52,7 @@ export async function validateMetadata(metadata: string): Promise { preload: [xmlProtocol, xmlAssertion, xmldsigCore, xmlXenc, xml], }); if (validationResult.valid) { - console.log(validationResult); + LoggerProxy.debug('SAML Metadata is valid'); return true; } else { LoggerProxy.warn('SAML Validate Metadata: Invalid metadata'); @@ -79,6 +79,7 @@ export async function validateResponse(response: string): Promise { preload: [xmlMetadata, xmlAssertion, xmldsigCore, xmlXenc, xml], }); if (validationResult.valid) { + LoggerProxy.debug('SAML Response is valid'); return true; } else { LoggerProxy.warn('SAML Validate Response: Failed'); From 322a2ab5016311fd75b813b00259a0e03e631229 Mon Sep 17 00:00:00 2001 From: Michael Auerswald Date: Fri, 3 Mar 2023 15:28:48 +0100 Subject: [PATCH 14/15] fix wrong schema used --- packages/cli/src/sso/saml/samlValidator.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/sso/saml/samlValidator.ts b/packages/cli/src/sso/saml/samlValidator.ts index bf24e9782cdde..aeae6b794566f 100644 --- a/packages/cli/src/sso/saml/samlValidator.ts +++ b/packages/cli/src/sso/saml/samlValidator.ts @@ -75,9 +75,10 @@ export async function validateResponse(response: string): Promise { }, ], extension: 'schema', - schema: [xmlMetadata], + schema: [xmlProtocol], preload: [xmlMetadata, xmlAssertion, xmldsigCore, xmlXenc, xml], }); + console.log(validationResult, response); if (validationResult.valid) { LoggerProxy.debug('SAML Response is valid'); return true; From cdf7b7f434eeb760376a605c01f02ae027eed7cb Mon Sep 17 00:00:00 2001 From: Michael Auerswald Date: Fri, 3 Mar 2023 15:32:09 +0100 Subject: [PATCH 15/15] remove console.log --- packages/cli/src/sso/saml/samlValidator.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/cli/src/sso/saml/samlValidator.ts b/packages/cli/src/sso/saml/samlValidator.ts index aeae6b794566f..2eaa3505ae851 100644 --- a/packages/cli/src/sso/saml/samlValidator.ts +++ b/packages/cli/src/sso/saml/samlValidator.ts @@ -78,7 +78,6 @@ export async function validateResponse(response: string): Promise { schema: [xmlProtocol], preload: [xmlMetadata, xmlAssertion, xmldsigCore, xmlXenc, xml], }); - console.log(validationResult, response); if (validationResult.valid) { LoggerProxy.debug('SAML Response is valid'); return true;