Fix multi saml strategy race conditions

node-saml#425
stavros-wb · Feb 26, 2020 · 840aca7 · 840aca7
1 parent fb1bda0
commit 840aca7
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 12 deletions.
diff --git 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
@@ -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);