From d250c1461db9e9b90e589a15cc9e99e4826f8eb3 Mon Sep 17 00:00:00 2001
From: Gilad Wolff <gilad@cloudera.com>
Date: Thu, 5 Dec 2019 08:36:54 -0800
Subject: [PATCH] improve debug output (#279)

This commit improves the debug output by adding the SAML response ID to
the debug message and by exposing the validation errors from xml-crypto
in case signature validation fails.

Here are examples of the improve output:

passport-saml checkSignature failed for pfx3b63c7be-fe86-62fd-8cb5-16ab6273efaa. Validation errors: invalid signature: for uri #pfx3b63c7be-fe86-62fd-8cb5-16ab6273efaa calculated digest is 0AbCoTZl3NxNBiPUyucHk/7gay8= but the xml to validate supplies digest DCnPTQYBb1hKspbe6fg1U3q8xn4= +-2017d

passport-saml validatePostResponse for id "_6a377272c8662561acf1056274ef3f81" resulted in an error: Error: SAML provider returned Responder error: InvalidNameIDPolicy +1ms

Testing done:
ran 'DEBUG=passport-saml npm run-script test' successfully,
saw the expected output.
---
 lib/passport-saml/saml.js | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/lib/passport-saml/saml.js b/lib/passport-saml/saml.js
index 4adcbc7e..e4356f47 100644
--- a/lib/passport-saml/saml.js
+++ b/lib/passport-saml/saml.js
@@ -303,7 +303,7 @@ SAML.prototype.generateLogoutResponse = function (req, logoutRequest) {
 };
 
 SAML.prototype.requestToUrl = function (request, response, operation, additionalParameters, callback) {
-  
+
   const requestToUrlHelper = (err, buffer) => {
     if (err) {
       return callback(err);
@@ -537,7 +537,7 @@ SAML.prototype.validateSignature = function (fullXml, currentNode, certs) {
   if (signatures.length != 1) {
     return false;
   }
-  
+
   const signature = signatures[0];
   return certs.some(certToCheck => {
     return this.validateSignatureForCert(signature, certToCheck, fullXml, currentNode);
@@ -570,11 +570,19 @@ SAML.prototype.validateSignatureForCert = function (signature, cert, fullXml, cu
   if (totalReferencedNodes.length > 1) {
     return false;
   }
-  return sig.checkSignature(fullXml);
+
+  if (!sig.checkSignature(fullXml)) {
+    debug('checkSignature failed for refId %s. Validation errors: %s',
+          refId,
+          sig.validationErrors);
+    return false;
+  }
+  return true;
 };
 
 SAML.prototype.validatePostResponse = function (container, callback) {
   var xml, doc, inResponseTo;
+  var responseId = 'unknown';
 
   Q.fcall(() => {
     xml = Buffer.from(container.SAMLResponse, 'base64').toString('utf8');
@@ -584,6 +592,11 @@ SAML.prototype.validatePostResponse = function (container, callback) {
     if (!Object.prototype.hasOwnProperty.call(doc, 'documentElement'))
       throw new Error('SAMLResponse is not valid base64-encoded XML');
 
+    var id = xpath(doc, "/*[local-name()='Response']/@ID");
+    if (id && id.length) {
+      responseId = id[0].nodeValue;
+    }
+
     inResponseTo = xpath(doc, "/*[local-name()='Response']/@InResponseTo");
 
     if (inResponseTo) {
@@ -708,7 +721,7 @@ SAML.prototype.validatePostResponse = function (container, callback) {
       });
   })
   .fail(err => {
-    debug('validatePostResponse resulted in an error: %s', err);
+    debug('validatePostResponse for id %s resulted in an error: %s', responseId, err);
     if (this.options.validateInResponseTo) {
       Q.ninvoke(this.cacheProvider, 'remove', inResponseTo)
       .then(function() {
@@ -904,7 +917,7 @@ SAML.prototype.processValidlySignedAssertion = function(xml, samlResponseXml, in
     if (inResponseTo) {
       profile.inResponseTo = inResponseTo;
     }
-    
+
     var authnStatement = assertion.AuthnStatement;
     if (authnStatement) {
       if (authnStatement[0].$ && authnStatement[0].$.SessionIndex) {