Skip to content

Commit

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

* commit '4d97ffcb7918526cfc6101207fe04d14a0abd23e':
  README: link to related sections and clarify decryptionCert docs
  Update deps to latest
  Updated README to include sha512 as a listed option
  Remove unused ejs package from devDeps
  Use latest version of xml-encryption
  Fixes node-saml#170: Clarify that the certificate are looking for is:
  Add the ability to sign with SHA-512
  Fix tests on Node.js:stable
  Add test for an encrypted Okta response
  Send EncryptedAssertion node when trying to decrypt the assertion
  Updagrade to xml-encryption 0.9
  Fix tests with latest version of shouldjs
  v0.15.0
  • Loading branch information
@cjbarth
cjbarth committed Sep 10, 2018
2 parents 0f112e2 + 4d97ffc commit 8994f95
Show file tree
Hide file tree
Showing 5 changed files with 117 additions and 103 deletions.
10 changes: 5 additions & 5 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ Config parameter details:
* `host`: host for callback; will be combined with path and protocol to construct callback url if `callbackUrl` is not specified (default: `localhost`)
* `entryPoint`: identity provider entrypoint
* `issuer`: issuer string to supply to identity provider
* `cert`: see 'security and signatures'
* `privateCert`: see 'security and signatures'
* `cert`: see [Security and signatures](#security-and-signatures)
* `privateCert`: see [Security and signatures](#security-and-signatures)
* `decryptionPvk`: optional private key that will be used to attempt to decrypt any encrypted assertions that are received
* `signatureAlgorithm`: optionally set the signature algorithm for signing requests, valid values are 'sha1' (default) or 'sha256'
* `signatureAlgorithm`: optionally set the signature algorithm for signing requests, valid values are 'sha1' (default), 'sha256', or 'sha512'
* Additional SAML behaviors
* `additionalParams`: dictionary of additional query params to add to all requests
* `additionalAuthorizeParams`: dictionary of additional query params to add to 'authorize' requests
Expand Down Expand Up @@ -104,7 +104,7 @@ app.get('/login',

As a convenience, the strategy object exposes a `generateServiceProviderMetadata` method which will generate a service provider metadata document suitable for supplying to an identity provider. This method will only work on strategies which are configured with a `callbackUrl` (since the relative path for the callback is not sufficient information to generate a complete metadata document).

The `decryptionCert` argument should be a certificate matching the `decryptionPvk` and is required if the strategy is configured with a `decryptionPvk`.
The `decryptionCert` argument should be a public certificate matching the `decryptionPvk` and is required if the strategy is configured with a `decryptionPvk`.


## Security and signatures
Expand All @@ -117,7 +117,7 @@ Authentication requests sent by Passport-SAML can be signed using RSA-SHA1. To s
privateCert: fs.readFileSync('./cert.pem', 'utf-8')
```

It is a good idea to validate the incoming SAML Responses. For this, you can provide the Identity Provider's certificate using the `cert` confguration key:
It is a good idea to validate the incoming SAML Responses. For this, you can provide the Identity Provider's public signing certificate using the `cert` configuration key:

```javascript
cert: 'MIICizCCAfQCCQCY8tKaMc0BMjANBgkqh ... W=='
Expand Down
37 changes: 19 additions & 18 deletions lib/passport-saml/saml.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,13 @@ SAML.prototype.initialize = function (options) {
options.cacheProvider = new InMemoryCacheProvider(
{keyExpirationPeriodMs: options.requestIdExpirationPeriodMs });
}

if (!options.logoutUrl) {
// Default to Entry Point
options.logoutUrl = options.entryPoint || '';
}

// sha1 or sha256
// sha1, sha256, or sha512
if (!options.signatureAlgorithm) {
options.signatureAlgorithm = 'sha1';
}
Expand Down Expand Up @@ -114,6 +114,10 @@ SAML.prototype.signRequest = function (samlMessage) {
samlMessage.SigAlg = 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha256';
signer = crypto.createSign('RSA-SHA256');
break;
case 'sha512':
samlMessage.SigAlg = 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha512';
signer = crypto.createSign('RSA-SHA512');
break;
default:
samlMessage.SigAlg = 'http://www.w3.org/2000/09/xmldsig#rsa-sha1';
signer = crypto.createSign('RSA-SHA1');
Expand Down Expand Up @@ -157,7 +161,7 @@ SAML.prototype.generateAuthorizeRequest = function (req, isPassive, callback) {
'@IssueInstant': instant,
'@ProtocolBinding': 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST',
'@AssertionConsumerServiceURL': self.getCallbackUrl(req),
'@Destination': self.options.entryPoint,
'@Destination': self.options.entryPoint,
'saml:Issuer' : {
'@xmlns:saml' : 'urn:oasis:names:tc:SAML:2.0:assertion',
'#text': self.options.issuer
Expand Down Expand Up @@ -371,7 +375,7 @@ SAML.prototype.getAuthorizeForm = function (req, callback) {
.replace(/"/g, '"')
.replace(/</g, '&lt;')
.replace(/>/g, '&gt;')
// Add other replacements here for HTML only
// 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);
Expand Down Expand Up @@ -552,29 +556,26 @@ SAML.prototype.validatePostResponse = function (container, callback) {
if (!self.options.decryptionPvk)
throw new Error('No decryption key for encrypted SAML response');

var encryptedDatas = xpath( encryptedAssertions[0], "./*[local-name()='EncryptedData']");
if (encryptedDatas.length != 1)
throw new Error('Invalid signature');
var encryptedDataXml = encryptedDatas[0].toString();
var encryptedAssertionXml = encryptedAssertions[0].toString();

var xmlencOptions = { key: self.options.decryptionPvk };
return Q.ninvoke(xmlenc, 'decrypt', encryptedDataXml, xmlencOptions)
return Q.ninvoke(xmlenc, 'decrypt', encryptedAssertionXml, xmlencOptions)
.then(function(decryptedXml) {
var decryptedDoc = new xmldom.DOMParser().parseFromString(decryptedXml);
var decryptedAssertions = xpath(decryptedDoc, "/*[local-name()='Assertion']");
if (decryptedAssertions.length != 1)
throw new Error('Invalid EncryptedAssertion content');

if (self.options.cert &&
!validSignature &&
if (self.options.cert &&
!validSignature &&
!self.validateSignature(decryptedXml, decryptedAssertions[0], self.options.cert))
throw new Error('Invalid signature');

self.processValidlySignedAssertion(decryptedAssertions[0].toString(), inResponseTo, callback);
});
}

// If there's no assertion, fall back on xml2js response parsing for the status &
// If there's no assertion, fall back on xml2js response parsing for the status &
// LogoutResponse code.

var parserConfig = {
Expand Down Expand Up @@ -708,7 +709,7 @@ SAML.prototype.processValidlySignedAssertion = function(xml, inResponseTo, callb
}
}
}

// 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 Expand Up @@ -916,13 +917,13 @@ SAML.prototype.generateServiceProviderMetadata = function( decryptionCert ) {
}
}
},
'#list' : [
'EncryptionMethod' : [
// this should be the set that the xmlenc library supports
{ 'EncryptionMethod': { '@Algorithm': 'http://www.w3.org/2001/04/xmlenc#aes256-cbc' } },
{ 'EncryptionMethod': { '@Algorithm': 'http://www.w3.org/2001/04/xmlenc#aes128-cbc' } },
{ 'EncryptionMethod': { '@Algorithm': 'http://www.w3.org/2001/04/xmlenc#tripledes-cbc' } },
{ '@Algorithm': 'http://www.w3.org/2001/04/xmlenc#aes256-cbc' },
{ '@Algorithm': 'http://www.w3.org/2001/04/xmlenc#aes128-cbc' },
{ '@Algorithm': 'http://www.w3.org/2001/04/xmlenc#tripledes-cbc' }
]
};
}
}

if (this.options.logoutCallbackUrl) {
Expand Down
19 changes: 9 additions & 10 deletions package.json
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "passport-saml",
"version": "0.14.0",
"version": "0.16.1",
"licenses": [
{
"type": "MIT",
Expand Down Expand Up @@ -31,23 +31,22 @@
"main": "./lib/passport-saml",
"dependencies": {
"passport-strategy": "*",
"q": "1.1.x",
"q": "^1.5.0",
"xml-crypto": "^0.9.0",
"xml-encryption": "~0.10",
"xml2js": "0.4.x",
"xml-crypto": "0.8.x",
"xmldom": "0.1.x",
"xmlbuilder": "2.5.x",
"xml-encryption": "~0.7"
"xmlbuilder": "^8.2.2",
"xmldom": "0.1.x"
},
"devDependencies": {
"body-parser": "1.9.x",
"ejs": "1.0.x",
"body-parser": "^1.17.1",
"express": "4.x",
"jshint": "*",
"mocha": "*",
"passport": "0.3.x",
"request": "*",
"should": "*",
"sinon": "^1.10.2",
"passport": "0.3.x"
"sinon": "^2.1.0"
},
"engines": {
"node": ">= 0.8.0"
Expand Down
2 changes: 1 addition & 1 deletion test/samlTests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ describe('SAML.js', function() {
// NOTE: This test only tests existence of the assertion, not the correctness
it('calls callback with saml request object', function(done) {
saml.getAuthorizeUrl(req, function(err, target) {
url.parse(target, true).query.should.have.property('SAMLRequest');
should(url.parse(target, true).query).have.property('SAMLRequest');
done();
});
});
Expand Down
Loading

0 comments on commit 8994f95

Please sign in to comment.