Additional support to check NotBefore and NotOnOrAfter as part of Sub…

…jectConfirmation element
node-saml · Jun 6, 2014 · a827156 · a827156
1 parent ce523d6
commit a827156
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 18 deletions.
diff --git a/README.md b/README.md
@@ -117,8 +117,11 @@ Here is a configuration that has been proven to work with ADFS:
 
 Please note that ADFS needs to have a trust established to your service in order for this to work.
 
-## Assertion Conditions - NotBefore and NotOnOrAfter
+## SAML Response Validation - NotBefore and NotOnOrAfter
 
 If the `NotBefore` or the `NotOnOrAfter` attributes are returned in the SAML response, Passport-SAML will validate them
 against the current time +/- a configurable clock skew value.  The default for the skew is 0s.  This is to account for
 differences between the clock time on the client (Node server with Passport-SAML) and the server (Identity provider).
+
+`NotBefore` and `NotOnOrAfter` can be part of either the `SubjectConfirmation` element, or within in the `Assertion/Conditions` element
+in the SAML response.
diff --git a/lib/passport-saml/saml.js b/lib/passport-saml/saml.js
@@ -305,7 +305,7 @@ SAML.prototype.validatePostResponse = function (container, callback) {
         !self.validateSignature(xml, assertions[0], self.options.cert)) {
       return callback(new Error('Invalid signature'), null, false);
     }
-    return processValidlySignedAssertion(assertions[0].toString(), self.options, callback);
+    return self.processValidlySignedAssertion(assertions[0].toString(), self.options, callback);
   }
 
   if (encryptedAssertions.length == 1) {
@@ -330,7 +330,7 @@ SAML.prototype.validatePostResponse = function (container, callback) {
       if (self.options.cert && !self.validateSignature(decryptedXml, decryptedAssertions[0], self.options.cert))
         return callback(new Error('Invalid signature'), null, false);
 
-      return processValidlySignedAssertion(decryptedAssertions[0].toString(), self.options, callback);
+      return self.processValidlySignedAssertion(decryptedAssertions[0].toString(), self.options, callback);
     });
   }
 
@@ -375,7 +375,8 @@ SAML.prototype.validatePostResponse = function (container, callback) {
   });
 };
 
-function processValidlySignedAssertion (xml, options, callback) {
+SAML.prototype.processValidlySignedAssertion = function(xml, options, callback) {
+  var self = this;
   var parserConfig = {
     explicitRoot: true,
     tagNameProcessors: [xml2js.processors.stripPrefix]
@@ -404,26 +405,33 @@ function processValidlySignedAssertion (xml, options, callback) {
           profile.nameIDFormat = nameID[0].$.Format;
         }
       }
+    } 
+
+    var nowMs = new Date().getTime();
+
+    var subjectConfirmation = subject[0].SubjectConfirmation ? subject[0].SubjectConfirmation[0]: null;
+    if(subjectConfirmation){
+      var confirmData = subjectConfirmation.SubjectConfirmationData ? subjectConfirmation.SubjectConfirmationData[0] : null;
+      if(confirmData && confirmData.$){
+        var subjectNotBefore = confirmData.$.NotBefore;
+        var subjectNotOnOrAfter = confirmData.$.NotOnOrAfter;
+
+        var subjErr = self.getTimestampsValidityError(nowMs, subjectNotBefore, subjectNotOnOrAfter);
+        if(subjErr){
+          return callback(subjErr, null, false);
+        }
+      }
     }
 
     var conditions = assertion.Conditions ? assertion.Conditions[0] : null;
     if (assertion.Conditions.length > 1) {
       var msg = 'Unable to process multiple conditions in SAML assertion';
       return callback(new Error(msg), null, false);
     }
-    if (conditions && conditions.$ && options.acceptedClockSkewMs >= 0) {
-      var nowMs = new Date().getTime();
-      if (conditions.$.NotBefore) {
-        var notBeforeMs = Date.parse(conditions.$.NotBefore);
-        if (nowMs + options.acceptedClockSkewMs < notBeforeMs) {
-          return callback(new Error('SAML assertion not yet valid'), null, false);
-        }
-      }
-      if (conditions.$.NotOnOrAfter) {
-        var notOnOrAfterMs = Date.parse(conditions.$.NotOnOrAfter);
-        if(nowMs - options.acceptedClockSkewMs >= notOnOrAfterMs) {
-          return callback(new Error('SAML assertion expired'), null, false);
-        }
+    if(conditions && conditions.$) {
+      var conErr = self.getTimestampsValidityError(nowMs, conditions.$.NotBefore, conditions.$.NotOnOrAfter);
+      if(conErr){
+        return callback(conErr, null, false);
       }
     }
 
@@ -458,7 +466,32 @@ function processValidlySignedAssertion (xml, options, callback) {
 
     callback(null, profile, false);
   });
-}
+};
+
+SAML.prototype.getTimestampsValidityError = function(nowMs, notBefore, notOnOrAfter){
+
+    var self = this;
+    if(self.options.acceptedClockSkewMs == -1){
+        return null;
+    }
+
+    if(notBefore){
+        var notBeforeMs = Date.parse(notBefore);
+
+        if(nowMs + self.options.acceptedClockSkewMs < notBeforeMs){
+            return new Error('SAML assertion not yet valid');
+        }
+    }
+    if(notOnOrAfter){
+
+        var notOnOrAfterMs = Date.parse(notOnOrAfter);
+        if(nowMs - self.options.acceptedClockSkewMs >= notOnOrAfterMs){
+            return new Error('SAML assertion expired');
+        }
+    }
+
+    return null;
+};
 
 SAML.prototype.validatePostRequest = function (container, callback) {
   var self = this;