Skip to content

Commit

Permalink
BugFix: Fail gracefully when SAML Response is invalid. Fixes #238
Browse files Browse the repository at this point in the history
Before, given junk input, validatePostResponse would fail with:

   TypeError: Cannot read property 'documentElement' of null

Now we'll fail with:

    SAMLResponse is not valid base64-encoded XML

To make this work, we primarily just needed to as a simple additional error check,
but to throw the error properly, we needed to move a bit of logic into
a nearby promise, but keeping some variable definitions in the outer
scope where they continue to be expected.
  • Loading branch information
markstos committed Oct 9, 2017
1 parent a84a722 commit 8354683
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 6 deletions.
20 changes: 14 additions & 6 deletions lib/passport-saml/saml.js
Original file line number Diff line number Diff line change
Expand Up @@ -504,15 +504,23 @@ SAML.prototype.validateSignature = function (fullXml, currentNode, cert) {

SAML.prototype.validatePostResponse = function (container, callback) {
var self = this;
var xml = new Buffer(container.SAMLResponse, 'base64').toString('utf8');
var doc = new xmldom.DOMParser().parseFromString(xml);

var inResponseTo = xpath(doc, "/*[local-name()='Response']/@InResponseTo");
if(inResponseTo){
inResponseTo = inResponseTo.length ? inResponseTo[0].nodeValue : null;
}
var xml, doc, inResponseTo;

Q.fcall(function(){
xml = new Buffer(container.SAMLResponse, 'base64').toString('utf8');
doc = new xmldom.DOMParser({
}).parseFromString(xml);

if (!doc.hasOwnProperty('documentElement'))
throw new Error('SAMLResponse is not valid base64-encoded XML');

inResponseTo = xpath(doc, "/*[local-name()='Response']/@InResponseTo");

if(inResponseTo){
inResponseTo = inResponseTo.length ? inResponseTo[0].nodeValue : null;
}

if(self.options.validateInResponseTo){
if (inResponseTo) {
return Q.ninvoke(self.cacheProvider, 'get', inResponseTo)
Expand Down
9 changes: 9 additions & 0 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 8354683

Please sign in to comment.