From 966b97a54e7a4b25316716c20e1f524b64cf8a22 Mon Sep 17 00:00:00 2001
From: Mark Stosberg <mark@rideamigos.com>
Date: Tue, 14 Aug 2018 10:34:40 -0400
Subject: [PATCH] internal: provide unique failure messages for invalid
 signatures. Fixes #146

Before, there were 6 different cases in which "Invalid Signature" could
be returned, making it hard to debug which case had been triggered.

Now we return a unique message in each case. For improvement backwards
compatibility, all cases still contain the string "Invalid signature",
so code that was checking for that regex will be fine.

If code was checking for the exact string "Invalid Signature", it will
need to be updated.
---
 lib/passport-saml/saml.js | 10 +++++-----
 test/tests.js             | 10 +++++-----
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/lib/passport-saml/saml.js b/lib/passport-saml/saml.js
index 1b7e4a80..e6d69d0a 100644
--- a/lib/passport-saml/saml.js
+++ b/lib/passport-saml/saml.js
@@ -583,7 +583,7 @@ SAML.prototype.validatePostResponse = function (container, callback) {
     if (assertions.length + encryptedAssertions.length > 1) {
       // There's no reason I know of that we want to handle multiple assertions, and it seems like a
       //   potential risk vector for signature scope issues, so treat this as an invalid signature
-      throw new Error('Invalid signature');
+      throw new Error('Invalid signature: multiple assertions');
     }
 
     if (assertions.length == 1) {
@@ -612,7 +612,7 @@ SAML.prototype.validatePostResponse = function (container, callback) {
           if (self.options.cert &&
               !validSignature &&
               !self.validateSignature(decryptedXml, decryptedAssertions[0], certs))
-            throw new Error('Invalid signature');
+            throw new Error('Invalid signature from encrypted assertion');
 
           self.processValidlySignedAssertion(decryptedAssertions[0].toString(), inResponseTo, callback);
         });
@@ -640,7 +640,7 @@ SAML.prototype.validatePostResponse = function (container, callback) {
                 var nestedStatusCode = statusCode[0].StatusCode;
                 if (nestedStatusCode && nestedStatusCode[0].$.Value === "urn:oasis:names:tc:SAML:2.0:status:NoPassive") {
                   if (self.options.cert && !validSignature) {
-                    throw new Error('Invalid signature');
+                    throw new Error('Invalid signature: NoPassive');
                   }
                   return callback(null, null, false);
                 }
@@ -672,7 +672,7 @@ SAML.prototype.validatePostResponse = function (container, callback) {
           }
         } else {
           if (self.options.cert && !validSignature) {
-            throw new Error('Invalid signature');
+            throw new Error('Invalid signature: No response found');
           }
           var logoutResponse = doc.LogoutResponse;
           if (logoutResponse){
@@ -916,7 +916,7 @@ SAML.prototype.validatePostRequest = function (container, callback) {
     .then(function(certs) {
       // Check if this document has a valid top-level signature
       if (self.options.cert && !self.validateSignature(xml, dom.documentElement, certs)) {
-        return callback(new Error('Invalid signature'));
+        return callback(new Error('Invalid signature on documentElement'));
       }
 
       processValidlySignedPostRequest(self, doc, callback);
diff --git a/test/tests.js b/test/tests.js
index a4717d1b..ce715087 100644
--- a/test/tests.js
+++ b/test/tests.js
@@ -839,7 +839,7 @@ describe( 'passport-saml /', function() {
           var samlObj = new SAML( samlConfig );
           samlObj.validatePostResponse( container, function( err, profile, logout ) {
             should.exist( err );
-            err.message.should.match( 'Invalid signature' );
+            err.message.should.match(/Invalid signature/);
             done();
           });
         });
@@ -854,7 +854,7 @@ describe( 'passport-saml /', function() {
           var samlObj = new SAML( samlConfig );
           samlObj.validatePostResponse( container, function( err, profile, logout ) {
             should.exist( err );
-            err.message.should.match( 'Invalid signature' );
+            err.message.should.match(/Invalid signature/);
             done();
           });
         });
@@ -869,7 +869,7 @@ describe( 'passport-saml /', function() {
           var samlObj = new SAML( samlConfig );
           samlObj.validatePostResponse( container, function( err, profile, logout ) {
             should.exist( err );
-            err.message.should.match( 'Invalid signature' );
+            err.message.should.match(/Invalid signature/);
             done();
           });
         });
@@ -887,7 +887,7 @@ describe( 'passport-saml /', function() {
           var samlObj = new SAML( samlConfig );
           samlObj.validatePostResponse( container, function( err, profile, logout ) {
             should.exist( err );
-            err.message.should.match( 'Invalid signature' );
+            err.message.should.match(/Invalid signature/);
             done();
           });
         });
@@ -1594,7 +1594,7 @@ describe( 'passport-saml /', function() {
       };
       samlObj.validatePostRequest(body, function(err) {
         should.exist(err);
-        err.should.eql(new Error('Invalid signature'));
+        err.should.eql(new Error('Invalid signature on documentElement'));
         done();
       });
     });