From 4355f8fc089fee7e4f0833410e1f41686a7708bd Mon Sep 17 00:00:00 2001 From: Chris Barth Date: Fri, 22 Jul 2022 11:05:15 -0400 Subject: [PATCH] Assert booleans when constructing options object (#85) --- src/saml.ts | 16 +++++++-- src/types.ts | 2 +- src/utility.ts | 9 +++++ test/samlRequest.spec.ts | 73 ++++++++++++++++++++++++++++++++-------- 4 files changed, 82 insertions(+), 18 deletions(-) diff --git a/src/saml.ts b/src/saml.ts index 110b9d59..4b03e7b5 100644 --- a/src/saml.ts +++ b/src/saml.ts @@ -29,7 +29,7 @@ import { XMLValue, } from "./types"; import { AuthenticateOptions, AuthorizeOptions } from "./passport-saml-types"; -import { assertRequired } from "./utility"; +import { assertBooleanIfPresent, assertRequired } from "./utility"; import { buildXml2JsObject, buildXmlBuilderObject, @@ -122,6 +122,16 @@ class SAML { assertRequired(ctorOptions.issuer, "issuer is required"); assertRequired(ctorOptions.cert, "cert is required"); + // Prevent a JS user from passing in "false", which is truthy, and doing the wrong thing + assertBooleanIfPresent(ctorOptions.passive); + assertBooleanIfPresent(ctorOptions.disableRequestedAuthnContext); + assertBooleanIfPresent(ctorOptions.forceAuthn); + assertBooleanIfPresent(ctorOptions.skipRequestCompression); + assertBooleanIfPresent(ctorOptions.disableRequestAcsUrl); + assertBooleanIfPresent(ctorOptions.allowCreate); + assertBooleanIfPresent(ctorOptions.wantAssertionsSigned); + assertBooleanIfPresent(ctorOptions.signMetadata); + const options: SamlOptions = { ...ctorOptions, passive: ctorOptions.passive ?? false, @@ -160,7 +170,7 @@ class SAML { signatureAlgorithm: ctorOptions.signatureAlgorithm ?? "sha1", // sha1, sha256, or sha512 authnRequestBinding: ctorOptions.authnRequestBinding ?? "HTTP-Redirect", generateUniqueId: ctorOptions.generateUniqueId ?? generateUniqueId, - + signMetadata: ctorOptions.signMetadata ?? false, racComparison: ctorOptions.racComparison ?? "exact", }; @@ -249,7 +259,7 @@ class SAML { if (isPassive) request["samlp:AuthnRequest"]["@IsPassive"] = true; - if (this.options.forceAuthn) { + if (this.options.forceAuthn === true) { request["samlp:AuthnRequest"]["@ForceAuthn"] = true; } diff --git a/src/types.ts b/src/types.ts index 99ad64c9..2195c8fb 100644 --- a/src/types.ts +++ b/src/types.ts @@ -137,7 +137,7 @@ export interface SamlOptions extends Partial, MandatorySamlO wantAssertionsSigned: boolean; maxAssertionAgeMs: number; generateUniqueId: () => string; - signMetadata?: boolean; + signMetadata: boolean; // InResponseTo Validation validateInResponseTo: ValidateInResponseTo; diff --git a/src/utility.ts b/src/utility.ts index 24e4fbf0..706a9c9c 100644 --- a/src/utility.ts +++ b/src/utility.ts @@ -7,6 +7,15 @@ export function assertRequired(value: T | null | undefined, error?: string): } } +export function assertBooleanIfPresent( + value: T | null | undefined, + error?: string +): asserts value { + if (value != null && typeof value != "boolean") { + throw new TypeError(error ?? "value is set but not boolean"); + } +} + export function signXmlResponse(samlMessage: string, options: SamlSigningOptions): string { const responseXpath = '//*[local-name(.)="Response" and namespace-uri(.)="urn:oasis:names:tc:SAML:2.0:protocol"]'; diff --git a/test/samlRequest.spec.ts b/test/samlRequest.spec.ts index 3cf4ea49..1c657216 100644 --- a/test/samlRequest.spec.ts +++ b/test/samlRequest.spec.ts @@ -95,9 +95,7 @@ describe("SAML request", function () { const encodedSamlRequest = samlRequestMatchValues?.[1]; let buffer = Buffer.from(encodedSamlRequest, "base64"); - if (!config.skipRequestCompression) { - buffer = zlib.inflateRawSync(buffer); - } + buffer = zlib.inflateRawSync(buffer); return parseStringPromise(buffer.toString()); }) @@ -160,9 +158,7 @@ describe("SAML request", function () { const encodedSamlRequest = samlRequestMatchValues?.[1]; let buffer = Buffer.from(encodedSamlRequest, "base64"); - if (!config.skipRequestCompression) { - buffer = zlib.inflateRawSync(buffer); - } + buffer = zlib.inflateRawSync(buffer); return parseStringPromise(buffer.toString()); }) @@ -229,9 +225,7 @@ describe("SAML request", function () { const encodedSamlRequest = samlRequestMatchValues?.[1]; let buffer = Buffer.from(encodedSamlRequest, "base64"); - if (!config.skipRequestCompression) { - buffer = zlib.inflateRawSync(buffer); - } + buffer = zlib.inflateRawSync(buffer); return parseStringPromise(buffer.toString()); }) @@ -247,7 +241,7 @@ describe("SAML request", function () { entryPoint: "https://wwwexampleIdp.com/saml", cert: FAKE_CERT, issuer: "onelogin_saml", - forceAuthn: false, + forceAuthn: true, passive: true, }; @@ -259,6 +253,7 @@ describe("SAML request", function () { ProtocolBinding: "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST", AssertionConsumerServiceURL: "http://localhost/saml/consume", Destination: "https://wwwexampleIdp.com/saml", + ForceAuthn: "true", IsPassive: "true", }, "saml:Issuer": [ @@ -288,7 +283,7 @@ describe("SAML request", function () { }; const oSAML = new SAML(config); - oSAML + return oSAML .getAuthorizeFormAsync("http://localhost/saml/consume") .then((formBody) => { expect(formBody).to.match(/[^]*/); @@ -297,9 +292,59 @@ describe("SAML request", function () { const encodedSamlRequest = samlRequestMatchValues?.[1]; let buffer = Buffer.from(encodedSamlRequest, "base64"); - if (!config.skipRequestCompression) { - buffer = zlib.inflateRawSync(buffer); - } + buffer = zlib.inflateRawSync(buffer); + + return parseStringPromise(buffer.toString()); + }) + .then((doc) => { + delete doc["samlp:AuthnRequest"]["$"]["ID"]; + delete doc["samlp:AuthnRequest"]["$"]["IssueInstant"]; + expect(doc).to.deep.equal(result); + }); + }); + + it("Config with disableRequestedAuthnContext, skipRequestCompression, disableRequestAcsUrl", async function () { + const config: SamlConfig = { + entryPoint: "https://wwwexampleIdp.com/saml", + cert: FAKE_CERT, + issuer: "onelogin_saml", + disableRequestedAuthnContext: true, + skipRequestCompression: true, + disableRequestAcsUrl: true, + }; + + const result = { + "samlp:AuthnRequest": { + $: { + "xmlns:samlp": "urn:oasis:names:tc:SAML:2.0:protocol", + Version: "2.0", + ProtocolBinding: "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST", + Destination: "https://wwwexampleIdp.com/saml", + }, + "saml:Issuer": [ + { _: "onelogin_saml", $: { "xmlns:saml": "urn:oasis:names:tc:SAML:2.0:assertion" } }, + ], + "samlp:NameIDPolicy": [ + { + $: { + "xmlns:samlp": "urn:oasis:names:tc:SAML:2.0:protocol", + Format: "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress", + AllowCreate: "true", + }, + }, + ], + }, + }; + + const oSAML = new SAML(config); + return oSAML + .getAuthorizeFormAsync("http://localhost/saml/consume") + .then((formBody) => { + expect(formBody).to.match(/[^]*/); + const samlRequestMatchValues = formBody.match(/