Skip to content

Commit

Permalink
feat(core): Add SAML XML validation (#5600)
Browse files Browse the repository at this point in the history
* consolidate SSO settings

* update saml settings

* fix type error

* limit user changes when saml is enabled

* add test

* add toggle endpoint and fetch metadata

* rename enabled param

* add handling of POST saml login request

* add config test endpoint

* adds saml XML validation

* add comment

* protect test endpoint

* improve ignoreSSL and some cleanup

* fix wrong schema used

* remove console.log
  • Loading branch information
flipswitchingmonkey committed Mar 6, 2023
1 parent ddfa16c commit ca66ec8
Show file tree
Hide file tree
Showing 16 changed files with 1,672 additions and 51 deletions.
3 changes: 2 additions & 1 deletion packages/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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"
}
}
12 changes: 9 additions & 3 deletions packages/cli/src/Server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -515,10 +515,16 @@ class Server extends AbstractServer {
// SAML
// ----------------------------------------

// initialize SamlService
await SamlService.getInstance().init();
// 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);

Expand Down
15 changes: 10 additions & 5 deletions packages/cli/src/sso/saml/routes/saml.controller.protected.ee.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
},
);

Expand Down Expand Up @@ -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));
Expand All @@ -133,8 +133,13 @@ samlControllerProtected.get(
},
);

/**
* GET /sso/saml/config/test
* Test SAML config
*/
samlControllerProtected.get(
SamlUrls.configTest,
samlLicensedOwnerMiddleware,
async (req: express.Request, res: express.Response) => {
const testResult = await SamlService.getInstance().testSamlConnection();
return res.send(testResult);
Expand Down
71 changes: 42 additions & 29 deletions packages/cli/src/sso/saml/saml.service.ee.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -22,8 +22,10 @@ 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';

export class SamlService {
private static instance: SamlService;
Expand All @@ -46,29 +48,13 @@ export class SamlService {
this._attributeMapping = mapping;
}

private _metadata = '';
private metadata = '';

private metadataUrl = '';

private loginBinding: SamlLoginBinding = 'post';

public get metadata(): string {
return this._metadata;
}

public set metadata(metadata: string) {
this._metadata = metadata;
}
private ignoreSSL = false;

constructor() {
this.loadFromDbAndApplySamlPreferences()
.then(() => {
LoggerProxy.debug('Initializing SAML service');
})
.catch(() => {
LoggerProxy.error('Error initializing SAML service');
});
}
private loginBinding: SamlLoginBinding = 'post';

static getInstance(): SamlService {
if (!SamlService.instance) {
Expand All @@ -79,6 +65,15 @@ export class SamlService {

async init(): Promise<void> {
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 {
Expand Down Expand Up @@ -125,7 +120,6 @@ export class SamlService {
'post',
) as PostBindingContext;
//TODO:SAML: debug logging

LoggerProxy.debug(loginRequest.context);
return loginRequest;
}
Expand Down Expand Up @@ -188,6 +182,7 @@ export class SamlService {
mapping: this.attributeMapping,
metadata: this.metadata,
metadataUrl: this.metadataUrl,
ignoreSSL: this.ignoreSSL,
loginBinding: this.loginBinding,
loginEnabled: isSamlLoginEnabled(),
loginLabel: getSamlLoginLabel(),
Expand All @@ -198,12 +193,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());
Expand Down Expand Up @@ -248,18 +250,24 @@ export class SamlService {

async fetchMetadataFromUrl(): Promise<string | undefined> {
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;
// 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;
// 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;
}
Expand Down Expand Up @@ -298,10 +306,14 @@ export class SamlService {

async testSamlConnection(): Promise<boolean> {
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;
Expand All @@ -319,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.');
Expand Down
93 changes: 93 additions & 0 deletions packages/cli/src/sso/saml/samlValidator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
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<boolean> {
try {
const validationResult = await validateXML({
xml: [
{
fileName: 'metadata.xml',
contents: metadata,
},
],
extension: 'schema',
schema: [xmlMetadata],
preload: [xmlProtocol, xmlAssertion, xmldsigCore, xmlXenc, xml],
});
if (validationResult.valid) {
LoggerProxy.debug('SAML Metadata is valid');
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<boolean> {
try {
const validationResult = await validateXML({
xml: [
{
fileName: 'response.xml',
contents: response,
},
],
extension: 'schema',
schema: [xmlProtocol],
preload: [xmlMetadata, xmlAssertion, xmldsigCore, xmlXenc, xml],
});
if (validationResult.valid) {
LoggerProxy.debug('SAML Response is 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;
}
Loading

0 comments on commit ca66ec8

Please sign in to comment.