From 6b4ac89b842e401b2edd519650178ee072746bcb Mon Sep 17 00:00:00 2001 From: Matt Broadstone Date: Tue, 17 Apr 2018 19:53:39 -0400 Subject: [PATCH] fix(auth): prevent stalling on authentication when connected ReplicaSet was storing the authentication call in the disconnectHandler when it didn't need to, leading to auth stalling. From now on, will only store handlers if ReplSet is not actually connected. NODE-1402 --- lib/topologies/replset.js | 2 +- test/tests/unit/replset/auth_tests.js | 67 +++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 test/tests/unit/replset/auth_tests.js diff --git a/lib/topologies/replset.js b/lib/topologies/replset.js index 6333cc7bd..0189bfe05 100644 --- a/lib/topologies/replset.js +++ b/lib/topologies/replset.js @@ -1413,7 +1413,7 @@ ReplSet.prototype.auth = function(mechanism, db) { // Topology is not connected, save the call in the provided store to be // Executed at some point when the handler deems it's reconnected - if (self.s.disconnectHandler != null) { + if (!this.isConnected() && self.s.disconnectHandler != null) { if (!self.s.replicaSetState.hasPrimary() && !self.s.options.secondaryOnlyConnectionAllowed) { return self.s.disconnectHandler.add('auth', db, allArgs, {}, callback); } else if ( diff --git a/test/tests/unit/replset/auth_tests.js b/test/tests/unit/replset/auth_tests.js new file mode 100644 index 000000000..4a1a8d23c --- /dev/null +++ b/test/tests/unit/replset/auth_tests.js @@ -0,0 +1,67 @@ +'use strict'; + +const ReplSet = require('../../../../lib/topologies/replset'); +const mock = require('mongodb-mock-server'); +const ReplSetFixture = require('../common').ReplSetFixture; +const ReadPreference = require('../../../../lib/topologies/read_preference'); + +describe('Auth (ReplSet)', function() { + let test; + before(() => (test = new ReplSetFixture())); + afterEach(() => mock.cleanup()); + beforeEach(() => test.setup()); + + // mock ops store from node-mongodb-native + const mockDisconnectHandler = { + add: () => {}, + execute: () => {}, + flush: () => {} + }; + + it('should not stall on authentication when you are connected', function(done) { + let finish = err => { + finish = () => {}; + done(err); + }; + + test.primaryServer.setMessageHandler(request => { + const doc = request.document; + if (doc.ismaster) { + setTimeout(() => request.reply(test.primaryStates[0])); + } else if (doc.saslStart) { + finish(); + } + }); + + test.firstSecondaryServer.setMessageHandler(request => { + const doc = request.document; + if (doc.ismaster) { + setTimeout(() => request.reply(test.firstSecondaryStates[0]), 2000); + } else if (doc.saslStart) { + finish(); + } + }); + + const replSet = new ReplSet( + [test.primaryServer.address(), test.firstSecondaryServer.address()], + { + setName: 'rs', + haInterval: 10000, + connectionTimeout: 3000, + disconnectHandler: mockDisconnectHandler, + secondaryOnlyConnectionAllowed: true, + size: 1 + } + ); + + replSet.once('error', finish); + replSet.once('connect', () => replSet.auth('default', 'db', 'user', 'pencil', () => {})); + replSet.connect({ + readPreference: new ReadPreference('primary'), + checkServerIdentity: true, + rejectUnauthorized: true + }); + + setTimeout(() => finish('replicaset stalled when attempting to authenticate'), 5000); + }); +});