From 840aca74c88651e3ca1aee0456c3b29823b24a22 Mon Sep 17 00:00:00 2001 From: Stavros Soleas Date: Wed, 26 Feb 2020 13:25:54 +0200 Subject: [PATCH] Fix multi saml strategy race conditions https://github.com/bergie/passport-saml/issues/425 --- multiSamlStrategy.js | 31 +++++++++++++++++++++++++------ test/multiSamlStrategy.js | 20 ++++++++++++++------ 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/multiSamlStrategy.js b/multiSamlStrategy.js index 00f4ea75f..764b3dafe 100644 --- a/multiSamlStrategy.js +++ b/multiSamlStrategy.js @@ -31,8 +31,12 @@ 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 +48,12 @@ 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 +69,19 @@ 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 72939ded9..95419eee7 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);