Require cert for every strategy (#548)

A lot of tests are fixed. There is one test that wasn't hermetically sealed.

Author: cjbarth

.prettierignore

@@ -2,3 +2,5 @@
 node_modules
 lib
 package-lock.json
+.eslintcache
+.prettierignore

README.md

@@ -30,7 +30,8 @@ passport.use(new SamlStrategy(
   {
     path: '/login/callback',
     entryPoint: 'https://openidp.feide.no/simplesaml/saml2/idp/SSOService.php',
-    issuer: 'passport-saml'
+    issuer: 'passport-saml',
+    cert: 'fake cert', // cert must be provided
   },
   function(profile, done) {
     findByEmail(profile.email, function(err, user) {
@@ -53,7 +54,7 @@ const { MultiSamlStrategy } = require('passport-saml');
 
 passport.use(new MultiSamlStrategy(
   {
-    passReqToCallback: true, //makes req available in callback
+    passReqToCallback: true, // makes req available in callback
     getSamlOptions: function(request, done) {
       findProvider(request, function(err, provider) {
         if (err) {
@@ -124,18 +125,18 @@ type Profile = {
 - **Additional SAML behaviors**
 - `additionalParams`: dictionary of additional query params to add to all requests; if an object with this key is passed to `authenticate`, the dictionary of additional query params will be appended to those present on the returned URL, overriding any specified by initialization options' additional parameters (`additionalParams`, `additionalAuthorizeParams`, and `additionalLogoutParams`)
 - `additionalAuthorizeParams`: dictionary of additional query params to add to 'authorize' requests
-- `identifierFormat`: if truthy, name identifier format to request from identity provider (default: `urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress`)
+- `identifierFormat`: optional name identifier format to request from identity provider (default: `urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress`)
 - `acceptedClockSkewMs`: Time in milliseconds of skew that is acceptable between client and server when checking `OnBefore` and `NotOnOrAfter` assertion condition validity timestamps. Setting to `-1` will disable checking these conditions entirely. Default is `0`.
 - `attributeConsumingServiceIndex`: optional `AttributeConsumingServiceIndex` attribute to add to AuthnRequest to instruct the IDP which attribute set to attach to the response ([link](http://blog.aniljohn.com/2014/01/data-minimization-front-channel-saml-attribute-requests.html))
 - `disableRequestedAuthnContext`: if truthy, do not request a specific authentication context. This is [known to help when authenticating against Active Directory](https://github.com/node-saml/passport-saml/issues/226) (AD FS) servers.
 - `authnContext`: if truthy, name identifier format to request auth context (default: `urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport`); array of values is also supported
-- `RACComparison`: Requested Authentication Context comparison type. Possible values are 'exact','minimum','maximum','better'. Default is 'exact'.
+- `racComparison`: Requested Authentication Context comparison type. Possible values are 'exact','minimum','maximum','better'. Default is 'exact'.
 
 - `forceAuthn`: if set to true, the initial SAML request from the service provider specifies that the IdP should force re-authentication of the user, even if they possess a valid session.
 - `providerName`: optional human-readable name of the requester for use by the presenter's user agent or the identity provider
 - `skipRequestCompression`: if set to true, the SAML request from the service provider won't be compressed.
 - `authnRequestBinding`: if set to `HTTP-POST`, will request authentication from IDP via HTTP POST binding, otherwise defaults to HTTP Redirect
-- `disableRequestACSUrl`: if truthy, SAML AuthnRequest from the service provider will not include the optional AssertionConsumerServiceURL. Default is falsy so it is automatically included.
+- `disableRequestAcsUrl`: if truthy, SAML AuthnRequest from the service provider will not include the optional AssertionConsumerServiceURL. Default is falsy so it is automatically included.
 - `scoping`: An optional configuration which implements the functionality [explained in the SAML spec paragraph "3.4.1.2 Element <Scoping>"](https://docs.oasis-open.org/security/saml/v2.0/saml-core-2.0-os.pdf). The config object is structured as following:
 
 ```javascript

docs/adfs/README.md

@@ -55,7 +55,7 @@ passport.use(
       identifierFormat: null,
       // this is configured under the Advanced tab in AD FS relying party
       signatureAlgorithm: "sha256",
-      RACComparison: "exact", // default to exact RequestedAuthnContext Comparison Type
+      racComparison: "exact", // default to exact RequestedAuthnContext Comparison Type
     },
     function (profile, done) {
       return done(null, {

package.json

@@ -45,6 +45,7 @@
     "prettier-format": "prettier --config .prettierrc.json --write .",
     "prettier-watch": "onchange -k -p 100 \".\" -- prettier --config .prettierrc.json --write {{file}}",
     "test": "npm run prettier-check && npm run lint && npm run tsc && mocha",
+    "test-watch": "mocha --watch",
     "tsc": "tsc",
     "tsc-watch": "tsc --watch",
     "watch": "concurrently --kill-others \"npm:*-watch\""

src/passport-saml/inmemory-cache-provider.ts

@@ -29,15 +29,10 @@ export class CacheProvider {
   constructor(options: Partial<CacheProviderOptions>) {
     this.cacheKeys = {};
 
-    if (!options) {
-      options = {};
-    }
-
-    if (!options.keyExpirationPeriodMs) {
-      options.keyExpirationPeriodMs = 28800000; // 8 hours
-    }
-
-    this.options = options as CacheProviderOptions;
+    this.options = {
+      ...options,
+      keyExpirationPeriodMs: options?.keyExpirationPeriodMs ?? 28800000, // 8 hours,
+    };
 
     // Expire old cache keys
     const expirationTimer = setInterval(() => {

src/passport-saml/multiSamlStrategy.ts

@@ -8,35 +8,35 @@ import {
   AuthorizeOptions,
   MultiSamlConfig,
   RequestWithUser,
+  SamlConfig,
   VerifyWithoutRequest,
   VerifyWithRequest,
 } from "./types";
 
 class MultiSamlStrategy extends SamlStrategy {
-  _options: MultiSamlConfig;
+  static readonly newSamlProviderOnConstruct = false;
+
+  _options: SamlConfig & MultiSamlConfig;
 
   constructor(options: MultiSamlConfig, verify: VerifyWithRequest);
   constructor(options: MultiSamlConfig, verify: VerifyWithoutRequest);
   constructor(options: MultiSamlConfig, verify: never) {
-    if (!options || typeof options.getSamlOptions != "function") {
+    if (!options || typeof options.getSamlOptions !== "function") {
       throw new Error("Please provide a getSamlOptions function");
     }
 
-    if (!options.requestIdExpirationPeriodMs) {
-      options.requestIdExpirationPeriodMs = 28800000; // 8 hours
-    }
-
-    if (!options.cacheProvider) {
-      options.cacheProvider = new InMemoryCacheProvider({
-        keyExpirationPeriodMs: options.requestIdExpirationPeriodMs,
-      });
-    }
+    // Force the type on this since we've disabled `newOnConstruct`
+    // so the `SAML` constructor will not be called at this time
+    // and there are defaults for all `strategy`-required options.
+    const samlConfig = {
+      ...options,
+    } as SamlConfig & MultiSamlConfig;
 
-    super(options, verify);
-    this._options = options;
+    super(samlConfig, verify);
+    this._options = samlConfig;
   }
 
-  authenticate(req: RequestWithUser, options: AuthenticateOptions) {
+  authenticate(req: RequestWithUser, options: AuthenticateOptions): void {
     this._options.getSamlOptions(req, (err, samlOptions) => {
       if (err) {
         return this.error(err);
@@ -90,6 +90,11 @@ class MultiSamlStrategy extends SamlStrategy {
       );
     });
   }
+
+  // This is reduntant, but helps with testing
+  error(err: Error): void {
+    super.error(err);
+  }
 }
 
 export = MultiSamlStrategy;

src/passport-saml/saml-post-signing.ts

@@ -1,6 +1,6 @@
 import { SignedXml } from "xml-crypto";
 import * as algorithms from "./algorithms";
-import { SamlOptions, SamlSigningOptions } from "./types";
+import { SamlSigningOptions } from "./types";
 
 const authnRequestXPath =
   '/*[local-name(.)="AuthnRequest" and namespace-uri(.)="urn:oasis:names:tc:SAML:2.0:protocol"]';
@@ -11,7 +11,11 @@ const defaultTransforms = [
   "http://www.w3.org/2001/10/xml-exc-c14n#",
 ];
 
-export function signSamlPost(samlMessage: string, xpath: string, options: SamlSigningOptions) {
+export function signSamlPost(
+  samlMessage: string,
+  xpath: string,
+  options: SamlSigningOptions
+): string {
   if (!samlMessage) throw new Error("samlMessage is required");
   if (!xpath) throw new Error("xpath is required");
   if (!options) {