Skip to content

Commit

Permalink
Support InResponseTo validations in MultiSaml
Browse files Browse the repository at this point in the history
Either use cache provided by user, or a default memory
cache to store InResponse parameters. This cache is not
yet partitioned per provider, which means a malicious
provider could do replay attacks by using anothers
unconsummed `InResponse` values

node-saml#334
  • Loading branch information
stavros-wb committed Feb 8, 2019
1 parent e2154f2 commit 7045d8a
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 10 deletions.
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ passport.use(new MultiSamlStrategy(
})
);
```
The options passed when the `MultiSamlStrategy` is initialized are also passed as default values to each provider.
e.g. If you provide an `issuer` on `MultiSamlStrategy`, this will be also a default value for every provider.
You can override these defaults by passing a new value through the `getSamlOptions` function.

Using multiple providers supports `validateInResponseTo`, but all the `InResponse` values are stored on the same Cache. This means, if you're using the default `InMemoryCache`, that all providers have access to it and a provider might get its response validated against another's request. [Issue Report](!https://github.com/bergie/passport-saml/issues/334). To amend this you should provide a different cache provider per SAML provider, through the `getSamlOptions` function.

#### The profile object:

Expand Down
20 changes: 15 additions & 5 deletions multiSamlStrategy.js
Original file line number Diff line number Diff line change
@@ -1,40 +1,50 @@
var util = require('util');
var saml = require('./lib/passport-saml/saml');
var InMemoryCacheProvider = require('./lib/passport-saml/inmemory-cache-provider').CacheProvider;
var SamlStrategy = require('./lib/passport-saml/strategy');

function MultiSamlStrategy (options, verify) {
if (!options || typeof options.getSamlOptions != 'function') {
throw new Error('Please provide a getSamlOptions function');
}

if(!options.requestIdExpirationPeriodMs){
options.requestIdExpirationPeriodMs = 28800000; // 8 hours
}

if(!options.cacheProvider){
options.cacheProvider = new InMemoryCacheProvider(
{keyExpirationPeriodMs: options.requestIdExpirationPeriodMs });
}

SamlStrategy.call(this, options, verify);
this._getSamlOptions = options.getSamlOptions;
this._options = options;
}

util.inherits(MultiSamlStrategy, SamlStrategy);

MultiSamlStrategy.prototype.authenticate = function (req, options) {
var self = this;

this._getSamlOptions(req, function (err, samlOptions) {
this._options.getSamlOptions(req, function (err, samlOptions) {
if (err) {
return self.error(err);
}

self._saml = new saml.SAML(samlOptions);
self._saml = new saml.SAML(Object.assign({}, self._options, samlOptions));
self.constructor.super_.prototype.authenticate.call(self, req, options);
});
};

MultiSamlStrategy.prototype.logout = function (req, options) {
var self = this;

this._getSamlOptions(req, function (err, samlOptions) {
this._options.getSamlOptions(req, function (err, samlOptions) {
if (err) {
return self.error(err);
}

self._saml = new saml.SAML(samlOptions);
self._saml = new saml.SAML(Object.assign({}, self._options, samlOptions));
self.constructor.super_.prototype.logout.call(self, req, options);
});
};
Expand Down
15 changes: 10 additions & 5 deletions test/multiSamlStrategy.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ describe('strategy#authenticate', function() {
done();
};

var strategy = new MultiSamlStrategy({ getSamlOptions: getSamlOptions }, verify);
var strategy = new MultiSamlStrategy({
getSamlOptions: getSamlOptions
}, verify);
strategy.authenticate();
});

Expand All @@ -57,7 +59,7 @@ describe('strategy#authenticate', function() {
strategy.authenticate();
});

it('uses geted options to setup internal saml provider', function(done) {
it('uses given options to setup internal saml provider', function(done) {
var samlOptions = {
issuer: 'http://foo.issuer',
callbackUrl: 'http://foo.callback',
Expand All @@ -73,12 +75,15 @@ describe('strategy#authenticate', function() {

function getSamlOptions (req, fn) {
fn(null, samlOptions);
strategy._saml.options.should.containEql(samlOptions);
strategy._saml.options.should.containEql(Object.assign({},
cacheProvider: 'mock cache provider',
samlOptions
));
done();
}

var strategy = new MultiSamlStrategy(
{ getSamlOptions: getSamlOptions },
{ getSamlOptions: getSamlOptions, cacheProvider: 'mock cache provider'},
verify
);
strategy.authenticate();
Expand Down Expand Up @@ -122,7 +127,7 @@ describe('strategy#logout', function() {
strategy.logout();
});

it('uses geted options to setup internal saml provider', function(done) {
it('uses given options to setup internal saml provider', function(done) {
var samlOptions = {
issuer: 'http://foo.issuer',
callbackUrl: 'http://foo.callback',
Expand Down

0 comments on commit 7045d8a

Please sign in to comment.