Skip to content

Commit

Permalink
Merge commit '094c660a0560625ccd29dbbffff4b7168844b4ef' into CB/query…
Browse files Browse the repository at this point in the history
…-string-signing

* commit '094c660a0560625ccd29dbbffff4b7168844b4ef':
  Fix scope issues in PR node-saml#131.
  Refactor HTTP-Post tests to be a branch in existing authnRequest tests, rather than largely duplicated separate test.
  Remove a comment left behind in the wrong place (and content now covered in documentation)
  Minor documentation clarification.
  Changing HTTP-Post AuthnRequest binding option to be part of SAML object options, named authnRequestBinding, and adding to documentation.
  Add a basic test for HTTP-Post authn binding support (PR node-saml#129)
  Fix Subject dereference bug
  Update package.json
  Update package.json
  Added missing semicolon
  Adds HTTP-POST binding support for the SAML <AuthnRequest>
  Add test for new nameid attributes
  Do not sign custom query string parameters
  Only add to logout xml if present in authn response
  Add NameQualifier and SPNameQualifier to nameID
  • Loading branch information
cjbarth committed Sep 10, 2018
2 parents fce6d23 + 094c660 commit 0f112e2
Show file tree
Hide file tree
Showing 4 changed files with 200 additions and 27 deletions.
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ Config parameter details:
* `authnContext`: if truthy, name identifier format to request auth context (default: `urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport`)
* `forceAuthn`: if set to true, the initial SAML request from the service provider specifies that the IdP should force re-authentication of the user, even if they possess a valid session.
* `skipRequestCompression`: if set to true, the SAML request from the service provider won't be compressed.
* `authnRequestBinding`: if set to `HTTP-POST`, will request authentication from IDP via HTTP POST binding, otherwise defaults to HTTP Redirect
* InResponseTo Validation
* `validateInResponseTo`: if truthy, then InResponseTo will be validated from incoming SAML responses
* `requestIdExpirationPeriodMs`: Defines the expiration time when a Request ID generated for a SAML request will not be valid if seen in a SAML response in the `InResponseTo` field. Default is 8 hours.
Expand Down Expand Up @@ -108,7 +109,7 @@ The `decryptionCert` argument should be a certificate matching the `decryptionPv

## Security and signatures

Passport-SAML uses the HTTP Redirect Binding for its `AuthnRequest`s, and expects to receive the messages back via the HTTP POST binding.
Passport-SAML uses the HTTP Redirect Binding for its `AuthnRequest`s (unless overridden with the `authnRequestBinding` parameter), and expects to receive the messages back via the HTTP POST binding.

Authentication requests sent by Passport-SAML can be signed using RSA-SHA1. To sign them you need to provide a private key in the PEM format via the `privateCert` configuration key. For example:

Expand Down
121 changes: 103 additions & 18 deletions lib/passport-saml/saml.js
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,14 @@ SAML.prototype.generateLogoutRequest = function (req) {
}
};

if (typeof(req.user.nameQualifier) !== 'undefined') {
request['samlp:LogoutRequest']['saml:NameID']['@NameQualifier'] = req.user.nameQualifier;
}

if (typeof(req.user.spNameQualifier) !== 'undefined') {
request['samlp:LogoutRequest']['saml:NameID']['@SPNameQualifier'] = req.user.spNameQualifier;
}

if (req.user.sessionIndex) {
request['samlp:LogoutRequest']['saml2p:SessionIndex'] = {
'@xmlns:saml2p': 'urn:oasis:names:tc:SAML:2.0:protocol',
Expand Down Expand Up @@ -349,6 +357,80 @@ SAML.prototype.getAuthorizeUrl = function (req, callback) {
});
};

SAML.prototype.getAuthorizeForm = function (req, callback) {
var self = this;

// The quoteattr() function is used in a context, where the result will not be evaluated by javascript
// but must be interpreted by an XML or HTML parser, and it must absolutely avoid breaking the syntax
// of an element attribute.
var quoteattr = function(s, preserveCR) {
preserveCR = preserveCR ? '&#13;' : '\n';
return ('' + s) // Forces the conversion to string.
.replace(/&/g, '&amp;') // This MUST be the 1st replacement.
.replace(/'/g, '&apos;') // The 4 other predefined entities, required.
.replace(/"/g, '&quot;')
.replace(/</g, '&lt;')
.replace(/>/g, '&gt;')
// Add other replacements here for HTML only
// Or for XML, only if the named entities are defined in its DTD.
.replace(/\r\n/g, preserveCR) // Must be before the next replacement.
.replace(/[\r\n]/g, preserveCR);
};

var getAuthorizeFormHelper = function(err, buffer) {
if (err) {
return callback(err);
}

var operation = 'authorize';
var additionalParameters = self.getAdditionalParams(req, operation);
var samlMessage = {
SAMLRequest: buffer.toString('base64')
};

Object.keys(additionalParameters).forEach(function(k) {
samlMessage[k] = additionalParameters[k] || '';
});

var formInputs = Object.keys(samlMessage).map(function(k) {
return '<input type="hidden" name="' + k + '" value="' + quoteattr(samlMessage[k]) + '" />';
}).join('\r\n');

callback(null, [
'<!DOCTYPE html>',
'<html>',
'<head>',
'<meta charset="utf-8">',
'<meta http-equiv="x-ua-compatible" content="ie=edge">',
'</head>',
'<body onload="document.forms[0].submit()">',
'<noscript>',
'<p><strong>Note:</strong> Since your browser does not support JavaScript, you must press the button below once to proceed.</p>',
'</noscript>',
'<form method="post" action="' + encodeURI(self.options.entryPoint) + '">',
formInputs,
'<input type="submit" value="Submit" />',
'</form>',
'<script>document.forms[0].style.display="none";</script>', // Hide the form if JavaScript is enabled
'</body>',
'</html>'
].join('\r\n'));
};

self.generateAuthorizeRequest(req, self.options.passive, function(err, request) {
if (err) {
return callback(err);
}

if (self.options.skipRequestCompression) {
getAuthorizeFormHelper(null, new Buffer(request, 'utf8'));
} else {
zlib.deflateRaw(request, getAuthorizeFormHelper);
}
});

};

SAML.prototype.getLogoutUrl = function(req, callback) {
var request = this.generateLogoutRequest(req);
var operation = 'logout';
Expand Down Expand Up @@ -591,39 +673,42 @@ SAML.prototype.processValidlySignedAssertion = function(xml, inResponseTo, callb
}

var subject = assertion.Subject;
var subjectConfirmation, confirmData;
if (subject) {
var nameID = subject[0].NameID;
if (nameID) {
profile.nameID = nameID[0]._ || nameID[0];

if (nameID[0].$ && nameID[0].$.Format) {
profile.nameIDFormat = nameID[0].$.Format;
profile.nameQualifier = nameID[0].$.NameQualifier;
profile.spNameQualifier = nameID[0].$.SPNameQualifier;
}
}
}

var subjectConfirmation = subject[0].SubjectConfirmation ?
subject[0].SubjectConfirmation[0] : null;
var confirmData = subjectConfirmation && subjectConfirmation.SubjectConfirmationData ?
subjectConfirmation.SubjectConfirmationData[0] : null;
if (subject[0].SubjectConfirmation && subject[0].SubjectConfirmation.length > 1) {
msg = 'Unable to process multiple SubjectConfirmations in SAML assertion';
throw new Error(msg);
}
subjectConfirmation = subject[0].SubjectConfirmation ?
subject[0].SubjectConfirmation[0] : null;
confirmData = subjectConfirmation && subjectConfirmation.SubjectConfirmationData ?
subjectConfirmation.SubjectConfirmationData[0] : null;
if (subject[0].SubjectConfirmation && subject[0].SubjectConfirmation.length > 1) {
msg = 'Unable to process multiple SubjectConfirmations in SAML assertion';
throw new Error(msg);
}

if (subjectConfirmation) {
if (confirmData && confirmData.$) {
var subjectNotBefore = confirmData.$.NotBefore;
var subjectNotOnOrAfter = confirmData.$.NotOnOrAfter;
if (subjectConfirmation) {
if (confirmData && confirmData.$) {
var subjectNotBefore = confirmData.$.NotBefore;
var subjectNotOnOrAfter = confirmData.$.NotOnOrAfter;

var subjErr = self.checkTimestampsValidityError(
nowMs, subjectNotBefore, subjectNotOnOrAfter);
if (subjErr) {
throw subjErr;
var subjErr = self.checkTimestampsValidityError(
nowMs, subjectNotBefore, subjectNotOnOrAfter);
if (subjErr) {
throw subjErr;
}
}
}
}

// Test to see that if we have a SubjectConfirmation InResponseTo that it matches
// the 'InResponseTo' attribute set in the Response
if (self.options.validateInResponseTo) {
Expand Down
28 changes: 23 additions & 5 deletions lib/passport-saml/strategy.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ function Strategy (options, verify) {
this._verify = verify;
this._saml = new saml.SAML(options);
this._passReqToCallback = !!options.passReqToCallback;
this._authnRequestBinding = options.authnRequestBinding || 'HTTP-Redirect';
}

util.inherits(Strategy, passport.Strategy);
Expand Down Expand Up @@ -74,14 +75,31 @@ Strategy.prototype.authenticate = function (req, options) {
} else if (req.body && req.body.SAMLRequest) {
this._saml.validatePostRequest(req.body, validateCallback);
} else {
var operation = {
'login-request': 'getAuthorizeUrl',
'logout-request': 'getLogoutUrl'
var requestHandler = {
'login-request': function() {
if (self._authnRequestBinding === 'HTTP-POST') {
this._saml.getAuthorizeForm(req, function(err, data) {
if (err) {
self.error(err);
} else {
var res = req.res;
res.send(data);
}
});
} else { // Defaults to HTTP-Redirect
this._saml.getAuthorizeUrl(req, redirectIfSuccess);
}
}.bind(self),
'logout-request': function() {
this._saml.getLogoutUrl(req, redirectIfSuccess);
}.bind(self)
}[options.samlFallback];
if (!operation) {

if (typeof requestHandler !== 'function') {
return self.fail();
}
this._saml[operation](req, redirectIfSuccess);

requestHandler();
}
};

Expand Down
75 changes: 72 additions & 3 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 0f112e2

Please sign in to comment.