Add optional setting to set a ceiling on how old a SAML response is a…

…llowed to be (#577)

Co-authored-by: Mark Stosberg <mark@rideamigos.com>

README.md

@@ -128,6 +128,7 @@ type Profile = {
 - `identifierFormat`: optional name identifier format to request from identity provider (default: `urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress`)
 - `wantAssertionsSigned`: if truthy, add `WantAssertionsSigned="true"` to the metadata, to specify that the IdP should always sign the assertions.
 - `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`.
+- `maxAssertionAgeMs`: Amount of time after which the framework should consider an assertion expired. If the limit imposed by this variable is stricter than the limit imposed by `NotOnOrAfter`, this limit will be used when determining if an assertion is expired.
 - `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

src/node-saml/saml.ts

@@ -140,6 +140,7 @@ class SAML {
       skipRequestCompression: ctorOptions.skipRequestCompression ?? false,
       disableRequestAcsUrl: ctorOptions.disableRequestAcsUrl ?? false,
       acceptedClockSkewMs: ctorOptions.acceptedClockSkewMs ?? 0,
+      maxAssertionAgeMs: ctorOptions.maxAssertionAgeMs ?? 0,
       path: ctorOptions.path ?? "/saml/consume",
       host: ctorOptions.host ?? "localhost",
       issuer: ctorOptions.issuer ?? "onelogin_saml",
@@ -1075,11 +1076,17 @@ class SAML {
           if (confirmData && confirmData.$) {
             const subjectNotBefore = confirmData.$.NotBefore;
             const subjectNotOnOrAfter = confirmData.$.NotOnOrAfter;
+            const maxTimeLimitMs = this.processMaxAgeAssertionTime(
+              this.options.maxAssertionAgeMs,
+              subjectNotOnOrAfter,
+              assertion.$.IssueInstant
+            );
 
             const subjErr = this.checkTimestampsValidityError(
               nowMs,
               subjectNotBefore,
-              subjectNotOnOrAfter
+              subjectNotOnOrAfter,
+              maxTimeLimitMs
             );
             if (subjErr) {
               throw subjErr;
@@ -1126,10 +1133,16 @@ class SAML {
       throw new Error(msg);
     }
     if (conditions && conditions.$) {
+      const maxTimeLimitMs = this.processMaxAgeAssertionTime(
+        this.options.maxAssertionAgeMs,
+        conditions.$.NotOnOrAfter,
+        assertion.$.IssueInstant
+      );
       const conErr = this.checkTimestampsValidityError(
         nowMs,
         conditions.$.NotBefore,
-        conditions.$.NotOnOrAfter
+        conditions.$.NotOnOrAfter,
+        maxTimeLimitMs
       );
       if (conErr) throw conErr;
     }
@@ -1190,18 +1203,27 @@ class SAML {
     return { profile, loggedOut: false };
   }
 
-  private checkTimestampsValidityError(nowMs: number, notBefore: string, notOnOrAfter: string) {
+  private checkTimestampsValidityError(
+    nowMs: number,
+    notBefore: string,
+    notOnOrAfter: string,
+    maxTimeLimitMs?: number
+  ) {
     if (this.options.acceptedClockSkewMs == -1) return null;
 
     if (notBefore) {
-      const notBeforeMs = Date.parse(notBefore);
+      const notBeforeMs = this.dateStringToTimestamp(notBefore, "NotBefore");
       if (nowMs + this.options.acceptedClockSkewMs < notBeforeMs)
         return new Error("SAML assertion not yet valid");
     }
     if (notOnOrAfter) {
-      const notOnOrAfterMs = Date.parse(notOnOrAfter);
+      const notOnOrAfterMs = this.dateStringToTimestamp(notOnOrAfter, "NotOnOrAfter");
       if (nowMs - this.options.acceptedClockSkewMs >= notOnOrAfterMs)
-        return new Error("SAML assertion expired");
+        return new Error("SAML assertion expired: clocks skewed too much");
+    }
+    if (maxTimeLimitMs) {
+      if (nowMs - this.options.acceptedClockSkewMs >= maxTimeLimitMs)
+        return new Error("SAML assertion expired: assertion too old");
     }
 
     return null;
@@ -1404,6 +1426,49 @@ class SAML {
 
     throw new Error("Invalid key");
   }
+
+  /**
+   * Process max age assertion and use it if it is more restrictive than the NotOnOrAfter age
+   * assertion received in the SAMLResponse.
+   *
+   * @param maxAssertionAgeMs Max time after IssueInstant that we will accept assertion, in Ms.
+   * @param notOnOrAfter Expiration provided in response.
+   * @param issueInstant Time when response was issued.
+   * @returns {*} The expiration time to be used, in Ms.
+   */
+  private processMaxAgeAssertionTime(
+    maxAssertionAgeMs: number,
+    notOnOrAfter: string,
+    issueInstant: string
+  ): number {
+    const notOnOrAfterMs = this.dateStringToTimestamp(notOnOrAfter, "NotOnOrAfter");
+    const issueInstantMs = this.dateStringToTimestamp(issueInstant, "IssueInstant");
+
+    if (maxAssertionAgeMs === 0) {
+      return notOnOrAfterMs;
+    }
+
+    const maxAssertionTimeMs = issueInstantMs + maxAssertionAgeMs;
+    return maxAssertionTimeMs < notOnOrAfterMs ? maxAssertionTimeMs : notOnOrAfterMs;
+  }
+
+  /**
+   * Convert a date string to a timestamp (in milliseconds).
+   *
+   * @param dateString A string representation of a date
+   * @param label Descriptive name of the date being passed in, e.g. "NotOnOrAfter"
+   * @throws Will throw an error if parsing `dateString` returns `NaN`
+   * @returns {number} The timestamp (in milliseconds) representation of the given date
+   */
+  private dateStringToTimestamp(dateString: string, label: string): number {
+    const dateMs = Date.parse(dateString);
+
+    if (isNaN(dateMs)) {
+      throw new Error(`Error parsing ${label}: '${dateString}' is not a valid date`);
+    }
+
+    return dateMs;
+  }
 }
 
 export { SAML };

src/node-saml/types.ts

@@ -109,6 +109,7 @@ export interface SamlOptions extends Partial<SamlSigningOptions>, MandatorySamlO
   audience?: string;
   scoping?: SamlScopingConfig;
   wantAssertionsSigned?: boolean;
+  maxAssertionAgeMs: number;
 
   // InResponseTo Validation
   validateInResponseTo: boolean;

test/node-saml/test-signatures.spec.ts

@@ -25,7 +25,7 @@ describe("Signatures", function () {
 
       //== Run the test in `func`
       await assert.rejects(samlObj.validatePostResponseAsync(samlResponseBody), {
-        message: shouldErrorWith || "SAML assertion expired",
+        message: shouldErrorWith || "SAML assertion expired: clocks skewed too much",
       });
       //== Assert times `validateSignature` was called
       validateSignatureSpy.callCount.should.eql(amountOfSignatureChecks);
@@ -210,15 +210,15 @@ describe("Signatures", function () {
   describe("Signatures on saml:Response - 1 saml:Assertion + 1 saml:Advice containing 2 saml:Assertion", () => {
     //== VALID
     it(
-      "R1A2Ad - signed root+asrt+advi => error",
+      "R1A2Ad - signed root+asrt+advi => valid",
       testOneResponse("/valid/response.root-signed.assertion-signed.2advice-signed.xml", false, 1)
     );
     it(
-      "R1A2Ad - signed root+asrt => error",
+      "R1A2Ad - signed root+asrt => valid",
       testOneResponse("/valid/response.root-signed.assertion-signed.2advice-unsigned.xml", false, 1)
     );
     it(
-      "R1A2Ad - signed root => error",
+      "R1A2Ad - signed root => valid",
       testOneResponse(
         "/valid/response.root-signed.assertion-unsigned.2advice-unsigned.xml",
         false,