SERVER-13850 Make sure cache entry is up to date before using it to update a user

(cherry picked from commit 06033e18fb1fe66d00f130227317d9ae531bb6f5)

3 files changed, 33 insertions, 8 deletions

diff --git a/jstests/auth/mongos_cache_invalidation.js b/jstests/auth/mongos_cache_invalidation.js
index 36ca253b3a7..a392975d645 100644
--- a/jstests/auth/mongos_cache_invalidation.js
+++ b/jstests/auth/mongos_cache_invalidation.js
@@ -33,15 +33,17 @@ st.s0.getDB('admin').createRole({role: 'myRole',
                                  roles: [],
                                  privileges: [{resource: {cluster: true},
                                                actions: ['invalidateUserCache']}]});
-
+st.s0.getDB('test').createUser({user: 'spencer',
+                                pwd: 'pwd',
+                                roles: ['read',
+                                        {role: 'myRole', db: 'admin'},
+                                        {role: 'userAdminAnyDatabase', db: 'admin'}]});
+st.s0.getDB('admin').logout();
 
 var db1 = st.s0.getDB('test');
-db1.createUser({user: 'spencer', pwd: 'pwd', roles: ['read', {role: 'myRole', db: 'admin'}]});
 db1.auth('spencer', 'pwd');
-
 var db2 = st.s1.getDB('test');
 db2.auth('spencer', 'pwd');
-
 var db3 = st.s2.getDB('test');
 db3.auth('spencer', 'pwd');
 
@@ -139,6 +141,32 @@ db3.auth('spencer', 'pwd');
      assert.writeOK(db3.foo.update({}, { $inc: { a: 1 }}));
  })();
 
+(function testConcurrentUserModification() {
+     jsTestLog("Testing having 2 mongoses modify the same user at the same time"); // SERVER-13850
+
+     assert.writeOK(db1.foo.update({}, { $inc: { a: 1 }}));
+     assert.writeOK(db3.foo.update({}, { $inc: { a: 1}}));
+
+     db1.getSiblingDB('test').revokeRolesFromUser("spencer", ['readWrite']);
+
+     // At this point db3 still thinks "spencer" has readWrite.  Use it to add a different role
+     // and make sure it doesn't add back readWrite
+     hasAuthzError(db1.foo.update({}, { $inc: { a: 1 }}));
+     assert.writeOK(db3.foo.update({}, { $inc: { a: 1}}));
+
+     db3.getSiblingDB('test').grantRolesToUser("spencer", ['dbAdmin']);
+
+     hasAuthzError(db1.foo.update({}, { $inc: { a: 1 }}));
+     // modifying "spencer" should force db3 to update its cache entry for "spencer"
+     hasAuthzError(db3.foo.update({}, { $inc: { a: 1 }}));
+
+     // Make sure nothing changes from invalidating the cache
+     db1.adminCommand('invalidateUserCache');
+     db3.adminCommand('invalidateUserCache');
+     hasAuthzError(db1.foo.update({}, { $inc: { a: 1 }}));
+     hasAuthzError(db3.foo.update({}, { $inc: { a: 1 }}));
+ })();
+
 (function testDroppingUser() {
      jsTestLog("Testing propagation of dropping users");
 
diff --git a/src/mongo/db/auth/user_cache_invalidator_job.cpp b/src/mongo/db/auth/user_cache_invalidator_job.cpp
index 64201da2be3..5eeada66ea5 100644
--- a/src/mongo/db/auth/user_cache_invalidator_job.cpp
+++ b/src/mongo/db/auth/user_cache_invalidator_job.cpp
@@ -53,10 +53,6 @@ namespace {
 } // namespace
 
     void UserCacheInvalidator::run() {
-        if (!_authzManager->isAuthEnabled()) {
-            return; // Nothing to do
-        }
-
         Client::initThread("UserCacheInvalidatorThread");
         while (true) {
             sleepsecs(userCacheInvalidationIntervalSecs);
diff --git a/src/mongo/db/commands/user_management_commands.cpp b/src/mongo/db/commands/user_management_commands.cpp
index 7ef8dd05d44..510488d5c17 100644
--- a/src/mongo/db/commands/user_management_commands.cpp
+++ b/src/mongo/db/commands/user_management_commands.cpp
@@ -123,6 +123,7 @@ namespace mongo {
                                       const UserName& userName,
                                       unordered_set<RoleName>* roles) {
         User* user;
+        authzManager->invalidateUserByName(userName); // Need to make sure cache entry is up to date
         Status status = authzManager->acquireUser(userName, &user);
         if (!status.isOK()) {
             return status;