fixed checks for inresponseto and validateExpiration

lib/response.ts

@@ -30,7 +30,7 @@
 };
 
 class WrapError extends Error {
   inner?: any;
 }
 
 /**@deprecated Use parseIssuer instead */
@@ -167,7 +167,7 @@
   // Save the js object derived from xml and check status code
   const assertionObj = await xmlToJs(decryptedSignedXml, cb); // this is our assertion object
 
-  parseResponseAndVersion(assertionObj, responseObj, function onParse(err, assertion, version) {
+  parseResponseAndVersion(assertionObj, responseObj, async function onParse(err, assertion, version) {
     if (err) {
       cb(err);
       return;
@@ -188,11 +188,41 @@
       return;
     }
 
-    if (options.inResponseTo && assertion.inResponseTo && assertion.inResponseTo !== options.inResponseTo) {
+    // Fail closed: when the caller supplies an expected InResponseTo (an
+    // SP-initiated flow), the assertion must carry a matching, signed
+    // InResponseTo. A missing value is a mismatch, not a reason to skip the
+    // check, otherwise a captured assertion can be replayed into a different
+    // login by stripping InResponseTo from the unsigned <Response> wrapper.
+    if (options.inResponseTo && assertion.inResponseTo !== options.inResponseTo) {
       cb(new Error('Invalid InResponseTo.'));
       return;
     }
 
+    // Optional one-time replay protection. The library is stateless, so the
+    // caller supplies the store. The callback receives the signed assertion
+    // identifiers and must return true when the assertion has already been
+    // seen (a replay), keying entries by assertion ID until NotOnOrAfter.
+    if (typeof options.assertionReplayValidator === 'function') {
+      let alreadyUsed: boolean;
+      try {
+        alreadyUsed = await options.assertionReplayValidator({
+          assertionId: tokenHandler.getAssertionId(assertion),
+          sessionIndex: assertion.AuthnStatement?.['@']?.SessionIndex,
+          notOnOrAfter: assertion.Conditions?.['@']?.NotOnOrAfter,
+          inResponseTo: assertion.inResponseTo,
+        });
+      } catch (e) {
+        const error = new WrapError('An error occurred during assertion replay validation.');
+        error.inner = e;
+        cb(error);
+        return;
+      }
+      if (alreadyUsed) {
+        cb(new Error('Assertion has already been used (replay detected).'));
+        return;
+      }
+    }
+
     parseAttributes(assertion, tokenHandler, cb);
   });
 };
@@ -266,7 +296,18 @@
   }
 
   const tokenHandler = tokenHandlers[version];
-  assertion.inResponseTo = tokenHandler.getInResponseTo(responseObj);
+
+  // Derive InResponseTo only from signed content so it cannot be forged by
+  // tampering with the unsigned <Response> wrapper. When the whole Response is
+  // signed, `assertionObj` is the Response subtree and Response/@InResponseTo is
+  // trustworthy. In the common assertion-only-signed case, fall back to the
+  // bearer SubjectConfirmationData/@InResponseTo, which is inside the signed
+  // assertion. The outer `responseObj` wrapper is never trusted here.
+  const signedResponseInResponseTo = assertionObj.Response
+    ? tokenHandler.getInResponseTo(assertionObj)
+    : undefined;
+  assertion.inResponseTo =
+    signedResponseInResponseTo || tokenHandler.getSubjectConfirmationInResponseTo(assertion);
 
   cb(null, assertion, version, response);
 }
@@ -331,7 +372,7 @@
   audience: string;
   issuer: string;
   acsUrl: string;
   claims: Record<string, any>;
   requestId: string;
   privateKey: string;
   publicKey: string;

lib/saml20.ts

@@ -101,6 +101,8 @@ const parse = (assertion) => {
     claims: claims,
     issuer: getProp(assertion, 'Issuer'),
     sessionIndex: getProp(assertion, 'AuthnStatement.@.SessionIndex'),
+    assertionId: getAssertionId(assertion),
+    notOnOrAfter: getAttribute(assertion, 'Conditions.@.NotOnOrAfter'),
   };
 };
 
@@ -134,23 +136,110 @@ const validateAudience = (assertion, realm, strictValidation = false) => {
   }
 };
 
+const clockSkewMs = 10 * 60 * 1000; // 10 minutes clock skew.
+
+// Collect every SubjectConfirmationData element across all SubjectConfirmation
+// entries. Bearer assertions carry their expiration here rather than (or in
+// addition to) Conditions.
+const getSubjectConfirmationData = (assertion): Record<string, unknown>[] => {
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  let confirmations = getAttribute<any>(assertion, 'Subject.SubjectConfirmation');
+  if (!confirmations) {
+    return [];
+  }
+  confirmations = Array.isArray(confirmations) ? confirmations : [confirmations];
+  const data: Record<string, unknown>[] = [];
+  for (const confirmation of confirmations) {
+    let scd = getAttribute<Record<string, unknown> | Record<string, unknown>[]>(
+      confirmation,
+      'SubjectConfirmationData'
+    );
+    if (!scd) {
+      continue;
+    }
+    scd = Array.isArray(scd) ? scd : [scd];
+    data.push(...scd);
+  }
+  return data;
+};
+
 const validateExpiration = (assertion) => {
-  const dteNotBefore = getProp(assertion, 'Conditions.@.NotBefore');
-  let notBefore: any = new Date(dteNotBefore);
-  notBefore = notBefore.setMinutes(notBefore.getMinutes() - 10); // 10 minutes clock skew
+  const now = Date.now();
+
+  // A SAML assertion need not carry a Conditions window: a bearer assertion is
+  // time-boxed by SubjectConfirmationData/@NotOnOrAfter instead. Gather every
+  // NotBefore (lower) and NotOnOrAfter (upper) bound from both places and
+  // enforce each one. An assertion with no upper bound anywhere is rejected
+  // rather than treated as "never expires" — that was the original NaN defect,
+  // where new Date(undefined) made `!(now < NaN || now > NaN)` evaluate to true.
+  const lowerBounds: string[] = [];
+  const upperBounds: string[] = [];
+
+  const conditionsNotBefore = getAttribute<string | undefined>(assertion, 'Conditions.@.NotBefore');
+  const conditionsNotOnOrAfter = getAttribute<string | undefined>(assertion, 'Conditions.@.NotOnOrAfter');
+  if (conditionsNotBefore) lowerBounds.push(conditionsNotBefore);
+  if (conditionsNotOnOrAfter) upperBounds.push(conditionsNotOnOrAfter);
+
+  for (const scd of getSubjectConfirmationData(assertion)) {
+    const attrs = (scd['@'] as Record<string, string> | undefined) ?? {};
+    if (attrs.NotBefore) lowerBounds.push(attrs.NotBefore);
+    if (attrs.NotOnOrAfter) upperBounds.push(attrs.NotOnOrAfter);
+  }
 
-  const dteNotOnOrAfter = getProp(assertion, 'Conditions.@.NotOnOrAfter');
-  let notOnOrAfter: any = new Date(dteNotOnOrAfter);
-  notOnOrAfter = notOnOrAfter.setMinutes(notOnOrAfter.getMinutes() + 10); // 10 minutes clock skew
+  // Require at least one enforceable expiration.
+  if (upperBounds.length === 0) {
+    return false;
+  }
 
-  const now = new Date();
-  return !(now < notBefore || now > notOnOrAfter);
+  // Every present bound must be parseable and currently satisfied. A bound that
+  // is present but unparseable is treated as invalid.
+  for (const value of lowerBounds) {
+    const ms = new Date(value).getTime();
+    if (Number.isNaN(ms) || now < ms - clockSkewMs) {
+      return false;
+    }
+  }
+  for (const value of upperBounds) {
+    const ms = new Date(value).getTime();
+    if (Number.isNaN(ms) || now > ms + clockSkewMs) {
+      return false;
+    }
+  }
+
+  return true;
 };
 
+// InResponseTo read from the outer <Response> wrapper. Only trust this when the
+// whole Response is signed; the wrapper is unsigned in the common
+// assertion-only-signed case.
 const getInResponseTo = (xml) => {
   return getProp(xml, 'Response.@.InResponseTo');
 };
 
-const saml20 = { getInResponseTo, validateExpiration, validateAudience, parse };
+// InResponseTo carried inside the bearer SubjectConfirmationData. This element
+// lives inside the <Assertion> and is therefore covered by the assertion
+// signature even when the outer <Response> wrapper is not signed.
+const getSubjectConfirmationInResponseTo = (assertion): string | undefined => {
+  for (const scd of getSubjectConfirmationData(assertion)) {
+    const inResponseTo = (scd['@'] as Record<string, string> | undefined)?.InResponseTo;
+    if (inResponseTo) {
+      return inResponseTo;
+    }
+  }
+  return undefined;
+};
+
+const getAssertionId = (assertion): string | undefined => {
+  return getAttribute<string | undefined>(assertion, '@.ID');
+};
+
+const saml20 = {
+  getInResponseTo,
+  getSubjectConfirmationInResponseTo,
+  getAssertionId,
+  validateExpiration,
+  validateAudience,
+  parse,
+};
 
 export default saml20;

lib/typings.ts

@@ -12,7 +12,21 @@
 
 export interface SAMLProfile {
   audience: string;
   claims: Record<string, any>;
   issuer: string;
   sessionIndex: string;
+  // Signed assertion identifier and validity bound, exposed so callers can
+  // implement one-time replay protection keyed by assertionId until notOnOrAfter.
+  assertionId?: string;
+  notOnOrAfter?: string;
+}
+
+// Identifiers from the signed assertion passed to a caller-supplied replay
+// validator. The validator returns true when the assertion has already been
+// used (a replay) and must be rejected.
+export interface AssertionReplayInfo {
+  assertionId?: string;
+  sessionIndex?: string;
+  notOnOrAfter?: string;
+  inResponseTo?: string;
 }

test/assets/certificates/testIdpCert.crt

@@ -0,0 +1,19 @@
+-----BEGIN CERTIFICATE-----
+MIIDBzCCAe+gAwIBAgIUfmiCRt513V6v242KcE4fS3tXnqkwDQYJKoZIhvcNAQEL
+BQAwEzERMA8GA1UEAwwIdGVzdC1pZHAwHhcNMjYwNjIyMjM1MzI2WhcNNDYwNjE3
+MjM1MzI2WjATMREwDwYDVQQDDAh0ZXN0LWlkcDCCASIwDQYJKoZIhvcNAQEBBQAD
+ggEPADCCAQoCggEBAM6XayR93ey62b2EpioV03K2eW4xB3psPUdQQ5cxqUgjUjKy
+jSVleHcCCfHf/uqVYibxlUc5tfTRARNdCx8lB6Ob+MZLdTI0NS2tso5LBHvgywpe
+ZqdD3aBHVHczxGVAkTX18XdwR0Ri31gPcZGP+tChKInJeTPo6i/tVbWmON/kP3NJ
+mWYHrWfZHe+vKuytAE7Oqqn+LKM45FR6KlFjWuW11voshatRRWnGARIBDiQL2477
+id7d/UnSXZcqYgq34OfpNAy56v25YPFn7t4xtMa/8ZJGfsbKX24JNvlwLXFKT0v5
+y0KOCiAjVRjuMrOB1pobVwDAnjtxQyn1QAuxfCcCAwEAAaNTMFEwHQYDVR0OBBYE
+FFhkCT/OWx/bn8P1AQYq8vxSEBH5MB8GA1UdIwQYMBaAFFhkCT/OWx/bn8P1AQYq
+8vxSEBH5MA8GA1UdEwEB/wQFMAMBAf8wDQYJKoZIhvcNAQELBQADggEBAG2r5QR1
+WRfD25DrjK8zMcm4NxZCmBvrxHO+f1Z9I5pdCazXrRY66jMt+VVayYZ/eZez+tPH
+PEeFNaHBQLnpQdI7HQsAYU+f79bgctSRMbeT3Mjdx5b8mhp9I256Zs/NyTYeEp8V
+wk9sjvhrr/pBA+t6oQKJwvfK7+u3pE42eIWxPSYqNQRKrhKPX4Ej7/3EQyMNfIwo
+KF2WJYnXhe2IyccsOXayHtQkTLXOccIYkLIbgTb6auZKSMo7EvPN+op0Vhvjk49u
+VfPLI076c1mSFrVL/x0yQ8c0YFf0eAFCl4pcKrNp125gT9MW1QAMgqKV/7tx8RnW
+zEstp1SkQL9ZCa0=
+-----END CERTIFICATE-----

test/assets/certificates/testIdpKey.pem

@@ -0,0 +1,28 @@
+-----BEGIN PRIVATE KEY-----
+MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQDOl2skfd3sutm9
+hKYqFdNytnluMQd6bD1HUEOXMalII1Iyso0lZXh3Agnx3/7qlWIm8ZVHObX00QET
+XQsfJQejm/jGS3UyNDUtrbKOSwR74MsKXmanQ92gR1R3M8RlQJE19fF3cEdEYt9Y
+D3GRj/rQoSiJyXkz6Oov7VW1pjjf5D9zSZlmB61n2R3vryrsrQBOzqqp/iyjOORU
+eipRY1rltdb6LIWrUUVpxgESAQ4kC9uO+4ne3f1J0l2XKmIKt+Dn6TQMuer9uWDx
+Z+7eMbTGv/GSRn7Gyl9uCTb5cC1xSk9L+ctCjgogI1UY7jKzgdaaG1cAwJ47cUMp
+9UALsXwnAgMBAAECggEAAzkkkEywbjWaGRVdPFHb2zHSoi/8pGHU8OxlKZI6SGhY
+q3bSse8r2nt7KT1r7kAHaIEjaZmSZ6/tGt68Qi+jN1/DGWDrAq4C2GQZ4ZN1DfO7
+Zz2Cz4BFEG+cd0GlAkloGpXsPwdO7Ve3kVmoVXOQH7or9j5g+DjdkoLa7/sYbxHK
+6FNHQIXOsizZ+MhcIQBX3eraoqcpaWafED0nvwocJhtN7AVmVXulW79Fs8P09egh
++qTY456XXf/bkpMcedc4sWso3bvvA1j+iS0Rr8bHzAPqA1B3Z0iKgkmvJsJlm8fF
+vgt7nOFsm8C15SmZikvPShvkxh8A4RHAQbpNY/hK1QKBgQDiZccgF89som7My9DA
+ttN9OMclc5QC6mQ0E52PPKGIW3sSuDav03rg3hIyrpFcPa/hq8gq4nzByBL/nnL1
+bO4Fofi52Anicnn/O06M0eHvmP43DZ5mBiu4cf4x6IaQmf1U8JrUZF9lavyW04ai
+aNTTRggIZlm7fBLQMhJdFrN4ywKBgQDpmquDvt6xMK+BrXzLQyBGDnaLeE/bt5IZ
+kUzGGGxmc9VcZi7UftZOqlbuzS0qfE48Ck0miVdYpt7lL8BMtOXjoBu4MrwdQQMS
+jYNynGfpiHY8Zr4zGoPqQDg56FdObo1RB5EfljFGZnTSFPW4oHbJg5loYyBoTMsm
+3LTqJkTKlQKBgQCZjXJrP/r9sYX4/VwO+XGkAvh/XE7NU3C3KX66AeOFepaU8cCV
+rJgxIC2zllcc+vHp2/sdqxP20t6f5TYPY9xkkaEDW5YIsqAwDmeOd2QIf/ocGO6Q
+QCszJI3GB/IM7YS3MaGx4IobXV8IZVtxmCyRR3R3TgQad2LDNtLhtF3x1QKBgQDm
+rJfPGYx3dfbo27KOWLOm2iNPJ7fb5BJ98s/YEUgBh0JZ4oE9zh27QlNjrfF6sZLj
+kNyMQDSjUuxpblS6qisUMgcNRfQiAw+Qo3L4mt+1aM4waNhKSFWY3F9pNzf3OA2N
+xSYWBc6UkRmsVYwrCzEhXjT/MltPAv3cWza+vJlTXQKBgClaYJ7A7Z5USwdTwNJ5
+UZp30uxKFMnSO9DhsmMgGPBQRsS5UPlDRHCQqoyx1aJyPuaaJICZL7QcCWwA1pCD
+wO9vxti8MdAk32F5UlR5FQz6bWrsgLWWY7+eA+FAATxSoMZoxQ1aEO0eiKb86K2E
+A55k9iw1jIhb1Oc4hjbjDrZ6
+-----END PRIVATE KEY-----