diff --git a/README.md b/README.md index 649cf5b8..aa8ebf14 100644 --- a/README.md +++ b/README.md @@ -79,6 +79,9 @@ You can override these defaults by passing a new value through the `getSamlOptio 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. +> :warning: **There's a race condition [bug](https://github.com/bergie/passport-saml/issues/425) in versions < 1.3.3 which makes it vulnerable to DOS attacks**: Please use > 1.3.3 if you want to use this issue + + #### The profile object: The profile object referenced above contains the following: diff --git a/multiSamlStrategy.js b/multiSamlStrategy.js index 00f4ea75..d7bb53c7 100644 --- a/multiSamlStrategy.js +++ b/multiSamlStrategy.js @@ -31,8 +31,10 @@ MultiSamlStrategy.prototype.authenticate = function (req, options) { return self.error(err); } - self._saml = new saml.SAML(Object.assign({}, self._options, samlOptions)); - self.constructor.super_.prototype.authenticate.call(self, req, options); + var samlService = new saml.SAML(Object.assign({}, self._options, samlOptions)); + var strategy = Object.assign({}, self, {_saml: samlService}); + Object.setPrototypeOf(strategy, self); + self.constructor.super_.prototype.authenticate.call(strategy, req, options); }); }; @@ -44,8 +46,10 @@ MultiSamlStrategy.prototype.logout = function (req, callback) { return callback(err); } - self._saml = new saml.SAML(Object.assign({}, self._options, samlOptions)); - self.constructor.super_.prototype.logout.call(self, req, callback); + var samlService = new saml.SAML(Object.assign({}, self._options, samlOptions)); + var strategy = Object.assign({}, self, {_saml: samlService}); + Object.setPrototypeOf(strategy, self); + self.constructor.super_.prototype.logout.call(strategy, req, callback); }); }; @@ -61,8 +65,10 @@ MultiSamlStrategy.prototype.generateServiceProviderMetadata = function( req, dec return callback(err); } - self._saml = new saml.SAML(Object.assign({}, self._options, samlOptions)); - return callback(null, self.constructor.super_.prototype.generateServiceProviderMetadata.call(self, decryptionCert, signingCert )); + var samlService = new saml.SAML(Object.assign({}, self._options, samlOptions)); + var strategy = Object.assign({}, self, {_saml: samlService}); + Object.setPrototypeOf(strategy, self); + return callback(null, self.constructor.super_.prototype.generateServiceProviderMetadata.call(strategy, decryptionCert, signingCert)); }); }; diff --git a/test/multiSamlStrategy.js b/test/multiSamlStrategy.js index 72939ded..95419eee 100644 --- a/test/multiSamlStrategy.js +++ b/test/multiSamlStrategy.js @@ -68,6 +68,7 @@ describe('strategy#authenticate', function() { }); it('uses given options to setup internal saml provider', function(done) { + var superAuthenticateStub = this.superAuthenticateStub; var samlOptions = { issuer: 'http://foo.issuer', callbackUrl: 'http://foo.callback', @@ -84,7 +85,9 @@ describe('strategy#authenticate', function() { function getSamlOptions (req, fn) { try { fn(null, samlOptions); - strategy._saml.options.should.containEql(Object.assign({}, + sinon.assert.calledOnce(superAuthenticateStub) + superAuthenticateStub.calledWith(Object.assign( + {}, { cacheProvider: 'mock cache provider' }, samlOptions )); @@ -104,19 +107,19 @@ describe('strategy#authenticate', function() { describe('strategy#logout', function() { beforeEach(function() { - this.superAuthenticateStub = sinon.stub(SamlStrategy.prototype, 'logout'); + this.superLogoutMock = sinon.stub(SamlStrategy.prototype, 'logout'); }); afterEach(function() { - this.superAuthenticateStub.restore(); + this.superLogoutMock.restore(); }); it('calls super with request and auth options', function(done) { - var superAuthenticateStub = this.superAuthenticateStub; + var superLogoutMock = this.superLogoutMock; function getSamlOptions (req, fn) { try { fn(); - sinon.assert.calledOnce(superAuthenticateStub); + sinon.assert.calledOnce(superLogoutMock); done(); } catch (err2) { done(err2); @@ -148,6 +151,7 @@ describe('strategy#logout', function() { }); it('uses given options to setup internal saml provider', function(done) { + var superLogoutMock = this.superLogoutMock; var samlOptions = { issuer: 'http://foo.issuer', callbackUrl: 'http://foo.callback', @@ -164,7 +168,11 @@ describe('strategy#logout', function() { function getSamlOptions (req, fn) { try { fn(null, samlOptions); - strategy._saml.options.should.containEql(samlOptions); + sinon.assert.calledOnce(superLogoutMock) + superLogoutMock.calledWith(Object.assign( + {}, + samlOptions + )); done(); } catch (err2) { done(err2);