Skip to content

Commit

Permalink
internal: provide unique failure messages for invalid signatures. Fixes
Browse files Browse the repository at this point in the history
#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.
  • Loading branch information
markstos committed Aug 14, 2018
1 parent 2de3528 commit 966b97a
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 10 deletions.
10 changes: 5 additions & 5 deletions lib/passport-saml/saml.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
});
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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){
Expand Down Expand Up @@ -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);
Expand Down
10 changes: 5 additions & 5 deletions test/tests.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 966b97a

Please sign in to comment.