SERVER-38556 Handle transaction events in RoleGraph update

9 files changed, 254 insertions, 104 deletions

diff --git a/src/mongo/db/auth/SConscript b/src/mongo/db/auth/SConscript
index e66d6b4e703..f26a2c3d5b2 100644
--- a/src/mongo/db/auth/SConscript
+++ b/src/mongo/db/auth/SConscript
@@ -142,6 +142,7 @@ env.Library(
     target='role_graph_update',
     source=[
         'role_graph_update.cpp',
+        env.Idlc("auth_options.idl")[0],
     ],
     LIBDEPS=[
         'auth',
diff --git a/src/mongo/db/auth/auth_options.idl b/src/mongo/db/auth/auth_options.idl
new file mode 100644
index 00000000000..488e311ee75
--- /dev/null
+++ b/src/mongo/db/auth/auth_options.idl
@@ -0,0 +1,44 @@
+# Copyright (C) 2018-present MongoDB, Inc.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the Server Side Public License, version 1,
+# as published by MongoDB, Inc.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# Server Side Public License for more details.
+#
+# You should have received a copy of the Server Side Public License
+# along with this program. If not, see
+# <http://www.mongodb.com/licensing/server-side-public-license>.
+#
+# As a special exception, the copyright holders give permission to link the
+# code of portions of this program with the OpenSSL library under certain
+# conditions as described in each individual source file and distribute
+# linked combinations including the program with the OpenSSL library. You
+# must comply with the Server Side Public License in all respects for
+# all of the code used other than as permitted herein. If you modify file(s)
+# with this exception, you may extend this exception to your version of the
+# file(s), but you are not obligated to do so. If you do not wish to do so,
+# delete this exception statement from your version. If you delete this
+# exception statement from all source files in the program, then also delete
+# it in the license file.
+#
+
+global:
+  cpp_namespace: "mongo"
+
+imports:
+  - "mongo/idl/basic_types.idl"
+
+server_parameters:
+  roleGraphInvalidationIsFatal:
+    # Role graph invalidation is non-fatal by default. Making this fatal
+    # during testing makes invalidation events visible.
+    description: "Make role graph invalidation terminate the server"
+    default: false
+    set_at: startup
+    cpp_vartype: bool
+    cpp_varname: roleGraphInvalidationIsFatal
+    test_only: true
diff --git a/src/mongo/db/auth/authorization_manager_impl.cpp b/src/mongo/db/auth/authorization_manager_impl.cpp
index bf5d941adf6..adf16c787d8 100644
--- a/src/mongo/db/auth/authorization_manager_impl.cpp
+++ b/src/mongo/db/auth/authorization_manager_impl.cpp
@@ -212,8 +212,6 @@ private:
     AuthorizationManager* _authzManager = nullptr;
 } authorizationManagerPinnedUsers;
 
-const auto inUserManagementCommandsFlag = OperationContext::declareDecoration<bool>();
-
 }  // namespace
 
 int authorizationManagerCacheSize;
@@ -660,21 +658,12 @@ StatusWith<UserHandle> AuthorizationManagerImpl::_acquireUserSlowPath(CacheGuard
     }
 }
 
-void AuthorizationManagerImpl::_recachePinnedUsers(CacheGuard& guard, OperationContext* opCtx) {
-    const auto findPinnedUser = [&](const auto& userName) {
-        return std::find_if(_pinnedUsers.begin(), _pinnedUsers.end(), [&](const auto& userHandle) {
-            return (userHandle->getName() == userName);
-        });
-    };
-
+std::vector<UserHandle> AuthorizationManagerImpl::_fetchPinnedUsers(CacheGuard& guard,
+                                                                    OperationContext* opCtx) {
     // Get the list of users to pin
     const auto usersToPin = authorizationManagerPinnedUsers.getUserNames();
     if (usersToPin.empty()) {
-        // If there are pinned users, clear them all out so they fall out of the cache
-        if (!_pinnedUsers.empty()) {
-            _pinnedUsers.clear();
-        }
-        return;
+        return {};
     }
 
     // Remove any users that shouldn't be pinned anymore or that are invalid.
@@ -691,6 +680,13 @@ void AuthorizationManagerImpl::_recachePinnedUsers(CacheGuard& guard, OperationC
                          });
                  });
 
+    const auto findPinnedUser = [&](const auto& userName) {
+        return std::find_if(
+            newPinnedUsers.begin(), newPinnedUsers.end(), [&](const auto& userHandle) {
+                return (userHandle->getName() == userName);
+            });
+    };
+
 
     while (guard.otherUpdateInFetchPhase()) {
         guard.wait();
@@ -702,7 +698,9 @@ void AuthorizationManagerImpl::_recachePinnedUsers(CacheGuard& guard, OperationC
 
     bool cacheUpdated = false;
     for (const auto& userName : usersToPin) {
-        if (findPinnedUser(userName) != _pinnedUsers.end()) {
+        auto existingPin = findPinnedUser(userName);
+        if (existingPin != newPinnedUsers.end()) {
+            newPinnedUsers.push_back(*existingPin);
             continue;
         }
 
@@ -723,7 +721,7 @@ void AuthorizationManagerImpl::_recachePinnedUsers(CacheGuard& guard, OperationC
     if (cacheUpdated)
         _updateCacheGeneration_inlock(guard);
 
-    _pinnedUsers = std::move(newPinnedUsers);
+    return newPinnedUsers;
 }
 
 Status AuthorizationManagerImpl::_fetchUserV2(OperationContext* opCtx,
@@ -748,11 +746,26 @@ Status AuthorizationManagerImpl::_fetchUserV2(OperationContext* opCtx,
 
 void AuthorizationManagerImpl::invalidateUserByName(OperationContext* opCtx,
                                                     const UserName& userName) {
+    setPinnedUsers(invalidateUserByNameNoPin(opCtx, userName));
+}
+
+std::vector<UserHandle> AuthorizationManagerImpl::invalidateUserByNameNoPin(
+    OperationContext* opCtx, const UserName& userName) {
     CacheGuard guard(opCtx, this);
     _updateCacheGeneration_inlock(guard);
     _userCache.invalidate(userName);
 
-    _recachePinnedUsers(guard, opCtx);
+    return _fetchPinnedUsers(guard, opCtx);
+}
+
+void AuthorizationManagerImpl::setPinnedUsers_inlock(const CacheGuard& guard,
+                                                     std::vector<UserHandle> refreshedUsers) {
+    _pinnedUsers = std::move(refreshedUsers);
+}
+
+void AuthorizationManagerImpl::setPinnedUsers(std::vector<UserHandle> refreshedUsers) {
+    stdx::lock_guard<stdx::mutex> lock(_cacheWriteMutex);
+    _pinnedUsers = std::move(refreshedUsers);
 }
 
 void AuthorizationManagerImpl::invalidateUsersFromDB(OperationContext* opCtx, StringData dbname) {
@@ -761,16 +774,23 @@ void AuthorizationManagerImpl::invalidateUsersFromDB(OperationContext* opCtx, St
     _userCache.invalidateIf(
         [&](const UserName& user, const User*) { return user.getDB() == dbname; });
 
-    _recachePinnedUsers(guard, opCtx);
+
+    setPinnedUsers_inlock(guard, _fetchPinnedUsers(guard, opCtx));
 }
 
 void AuthorizationManagerImpl::invalidateUserCache(OperationContext* opCtx) {
+    setPinnedUsers(invalidateUserCacheNoPin(opCtx));
+}
+
+std::vector<UserHandle> AuthorizationManagerImpl::invalidateUserCacheNoPin(
+    OperationContext* opCtx) {
     CacheGuard guard(opCtx, this);
     _invalidateUserCache_inlock(guard);
 
-    _recachePinnedUsers(guard, opCtx);
+    return _fetchPinnedUsers(guard, opCtx);
 }
 
+
 void AuthorizationManagerImpl::_invalidateUserCache_inlock(const CacheGuard& guard) {
     _updateCacheGeneration_inlock(guard);
     _userCache.invalidateIf([](const UserName& a, const User*) { return true; });
@@ -840,81 +860,19 @@ bool appliesToAuthzData(const char* op, const NamespaceString& nss, const BSONOb
     }
 }
 
-// Updates to users in the oplog are done by matching on the _id, which will always have the
-// form "<dbname>.<username>".  This function extracts the UserName from that string.
-StatusWith<UserName> extractUserNameFromIdString(StringData idstr) {
-    size_t splitPoint = idstr.find('.');
-    if (splitPoint == string::npos) {
-        return StatusWith<UserName>(ErrorCodes::FailedToParse,
-                                    mongoutils::str::stream()
-                                        << "_id entries for user documents must be of "
-                                           "the form <dbname>.<username>.  Found: "
-                                        << idstr);
-    }
-    return StatusWith<UserName>(
-        UserName(idstr.substr(splitPoint + 1), idstr.substr(0, splitPoint)));
-}
-
 }  // namespace
 
 void AuthorizationManagerImpl::_updateCacheGeneration_inlock(const CacheGuard&) {
     _fetchGeneration = OID::gen();
 }
 
-void AuthorizationManagerImpl::_invalidateRelevantCacheData(OperationContext* opCtx,
-                                                            const char* op,
-                                                            const NamespaceString& ns,
-                                                            const BSONObj& o,
-                                                            const BSONObj* o2) {
-    // When we're doing a user management command we lock the admin DB for the duration
-    // of the command and invalidate the cache at the end of the command, so we don't need
-    // to invalidate it based on calls to logOp().
-    if (inUserManagementCommandsFlag(opCtx)) {
-        LOG(1) << "Skipping cache invalidation in opObserver because of active user command";
-        return;
-    }
-
-    if (ns == AuthorizationManager::rolesCollectionNamespace ||
-        ns == AuthorizationManager::versionCollectionNamespace) {
-        invalidateUserCache(opCtx);
-        return;
-    }
-
-    if (*op == 'i' || *op == 'd' || *op == 'u') {
-        // If you got into this function isAuthzNamespace() must have returned true, and we've
-        // already checked that it's not the roles or version collection.
-        invariant(ns == AuthorizationManager::usersCollectionNamespace);
-
-        StatusWith<UserName> userName = (*op == 'u')
-            ? extractUserNameFromIdString((*o2)["_id"].str())
-            : extractUserNameFromIdString(o["_id"].str());
-
-        if (!userName.isOK()) {
-            warning() << "Invalidating user cache based on user being updated failed, will "
-                         "invalidate the entire cache instead: "
-                      << userName.getStatus();
-            invalidateUserCache(opCtx);
-            return;
-        }
-        invalidateUserByName(opCtx, userName.getValue());
-    } else {
-        invalidateUserCache(opCtx);
-    }
-}
-
 void AuthorizationManagerImpl::logOp(OperationContext* opCtx,
                                      const char* op,
                                      const NamespaceString& nss,
                                      const BSONObj& o,
                                      const BSONObj* o2) {
     if (appliesToAuthzData(op, nss, o)) {
-        // AuthzManagerExternalState's logOp method registers a RecoveryUnit::Change
-        // and to do so we need to have begun a UnitOfWork
-        WriteUnitOfWork wuow(opCtx);
-        _externalState->logOp(opCtx, op, nss, o, o2);
-        wuow.commit();
-
-        _invalidateRelevantCacheData(opCtx, op, nss, o, o2);
+        _externalState->logOp(opCtx, this, op, nss, o, o2);
     }
 }
 
@@ -932,7 +890,7 @@ std::vector<AuthorizationManager::CachedUserInfo> AuthorizationManagerImpl::getU
 }
 
 void AuthorizationManagerImpl::setInUserManagementCommand(OperationContext* opCtx, bool val) {
-    inUserManagementCommandsFlag(opCtx) = val;
+    _externalState->setInUserManagementCommand(opCtx, val);
 }
 
 }  // namespace mongo
diff --git a/src/mongo/db/auth/authorization_manager_impl.h b/src/mongo/db/auth/authorization_manager_impl.h
index 8b1d1dd06a1..a0b82fecade 100644
--- a/src/mongo/db/auth/authorization_manager_impl.h
+++ b/src/mongo/db/auth/authorization_manager_impl.h
@@ -121,14 +121,38 @@ public:
                                                         const UserName& userName,
                                                         const User::UserId& uid) override;
 
+    /**
+     * Invalidate a user, and repin it if necessary.
+     */
     void invalidateUserByName(OperationContext* opCtx, const UserName& user) override;
+    /**
+     * Invalidate a user, and return a full set of pinned users to be later attached to
+     * the AuthorizationManager.
+     */
+    std::vector<UserHandle> invalidateUserByNameNoPin(OperationContext* opCtx,
+                                                      const UserName& user);
 
     void invalidateUsersFromDB(OperationContext* opCtx, StringData dbname) override;
 
     Status initialize(OperationContext* opCtx) override;
 
+    /**
+     * Invalidate the user cache, and repin all pinned users.
+     */
     void invalidateUserCache(OperationContext* opCtx) override;
 
+    /**
+     * Invalidate the user cache, without repinning users. Instead return the set
+     * of users which should be later pinned.
+     */
+    std::vector<UserHandle> invalidateUserCacheNoPin(OperationContext* opCtx);
+
+    /**
+     * Attach a set of previously acquired Users to the AuthorizationManager
+     * as pinned users.
+     */
+    void setPinnedUsers(std::vector<UserHandle>);
+
     Status _initializeUserFromPrivilegeDocument(User* user, const BSONObj& privDoc) override;
 
     void logOp(OperationContext* opCtx,
@@ -149,6 +173,12 @@ private:
     friend class AuthorizationManagerImpl::CacheGuard;
 
     /**
+     * Attach a set of previously acquired Users to the AuthorizationManager
+     * as pinned users, under the protection of an existing CacheGuard.
+     */
+    void setPinnedUsers_inlock(const CacheGuard&, std::vector<UserHandle>);
+
+    /**
      * Invalidates all User objects in the cache and removes them from the cache.
      * Should only be called when already holding _cacheMutex.
      */
@@ -171,7 +201,7 @@ private:
     void _updateCacheGeneration_inlock(const CacheGuard&);
 
 
-    void _recachePinnedUsers(CacheGuard& guard, OperationContext* opCtx);
+    std::vector<UserHandle> _fetchPinnedUsers(CacheGuard& guard, OperationContext* opCtx);
 
     StatusWith<UserHandle> _acquireUserSlowPath(CacheGuard& guard,
                                                 OperationContext* opCtx,
diff --git a/src/mongo/db/auth/authorization_manager_test.cpp b/src/mongo/db/auth/authorization_manager_test.cpp
index 2c5573c01e1..7c8c1520e83 100644
--- a/src/mongo/db/auth/authorization_manager_test.cpp
+++ b/src/mongo/db/auth/authorization_manager_test.cpp
@@ -430,7 +430,7 @@ TEST_F(AuthorizationManagerLogOpTest, testCreateAnyCollectionAddsNoRecoveryUnits
     ASSERT_EQ(size_t(0), registeredChanges);
 }
 
-TEST_F(AuthorizationManagerLogOpTest, testRawInsertToRolesCollectionAddsRecoveryUnits) {
+TEST_F(AuthorizationManagerLogOpTest, testRawInsertAddsRecoveryUnits) {
     authzManager->logOp(opCtx.get(),
                         "i",
                         {"admin", "system.profile"},
@@ -445,7 +445,7 @@ TEST_F(AuthorizationManagerLogOpTest, testRawInsertToRolesCollectionAddsRecovery
                         BSON("_id"
                              << "admin.user"),
                         nullptr);
-    ASSERT_EQ(size_t(0), registeredChanges);
+    ASSERT_EQ(size_t(1), registeredChanges);
 
     authzManager->logOp(opCtx.get(),
                         "i",
@@ -453,7 +453,7 @@ TEST_F(AuthorizationManagerLogOpTest, testRawInsertToRolesCollectionAddsRecovery
                         BSON("_id"
                              << "admin.user"),
                         nullptr);
-    ASSERT_EQ(size_t(1), registeredChanges);
+    ASSERT_EQ(size_t(2), registeredChanges);
 }
 
 }  // namespace
diff --git a/src/mongo/db/auth/authz_manager_external_state.h b/src/mongo/db/auth/authz_manager_external_state.h
index 361ff2a3f1f..f7733da4e70 100644
--- a/src/mongo/db/auth/authz_manager_external_state.h
+++ b/src/mongo/db/auth/authz_manager_external_state.h
@@ -36,6 +36,7 @@
 #include "mongo/base/shim.h"
 #include "mongo/base/status.h"
 #include "mongo/db/auth/authorization_manager.h"
+#include "mongo/db/auth/authorization_manager_impl.h"
 #include "mongo/db/auth/privilege_format.h"
 #include "mongo/db/auth/role_name.h"
 #include "mongo/db/auth/user.h"
@@ -161,6 +162,7 @@ public:
     virtual bool hasAnyPrivilegeDocuments(OperationContext* opCtx) = 0;
 
     virtual void logOp(OperationContext* opCtx,
+                       AuthorizationManagerImpl* authManager,
                        const char* op,
                        const NamespaceString& ns,
                        const BSONObj& o,
@@ -195,6 +197,8 @@ public:
         return false;
     }
 
+    virtual void setInUserManagementCommand(OperationContext* opCtx, bool val) {}
+
 protected:
     AuthzManagerExternalState();  // This class should never be instantiated directly.
 
diff --git a/src/mongo/db/auth/authz_manager_external_state_local.cpp b/src/mongo/db/auth/authz_manager_external_state_local.cpp
index d04e941d039..48bb3c2c5b0 100644
--- a/src/mongo/db/auth/authz_manager_external_state_local.cpp
+++ b/src/mongo/db/auth/authz_manager_external_state_local.cpp
@@ -38,6 +38,7 @@
 #include "mongo/bson/mutable/document.h"
 #include "mongo/bson/mutable/element.h"
 #include "mongo/bson/util/bson_extract.h"
+#include "mongo/db/auth/auth_options_gen.h"
 #include "mongo/db/auth/privilege_parser.h"
 #include "mongo/db/auth/user_document_parser.h"
 #include "mongo/db/operation_context.h"
@@ -48,6 +49,11 @@
 
 namespace mongo {
 
+namespace {
+
+const auto inUserManagementCommandsFlag = OperationContext::declareDecoration<bool>();
+}
+
 using std::vector;
 
 Status AuthzManagerExternalStateLocal::initialize(OperationContext* opCtx) {
@@ -529,35 +535,69 @@ public:
     // None of the parameters below (except opCtx and externalState) need to live longer than the
     // instantiations of this class
     AuthzManagerLogOpHandler(OperationContext* opCtx,
+                             AuthorizationManagerImpl* authzManager,
                              AuthzManagerExternalStateLocal* externalState,
                              const char* op,
                              const NamespaceString& nss,
                              const BSONObj& o,
                              const BSONObj* o2)
         : _opCtx(opCtx),
+          _authzManager(authzManager),
           _externalState(externalState),
           _op(op),
           _nss(nss),
           _o(o.getOwned()),
+          _o2(o2 ? boost::optional<BSONObj>(o2->getOwned()) : boost::none),
+          _refreshedPinnedUsers(_invalidateRelevantCacheData()) {}
+
+    void commit(boost::optional<Timestamp>) final {
+        if (_nss == AuthorizationManager::rolesCollectionNamespace ||
+            _nss == AuthorizationManager::adminCommandNamespace) {
+            _refreshRoleGraph();
+        }
+
+        if (_refreshedPinnedUsers) {
+            _authzManager->setPinnedUsers(std::move(*_refreshedPinnedUsers));
+        }
+    }
+
+    void rollback() final {}
+
+private:
+    // Updates to users in the oplog are done by matching on the _id, which will always have the
+    // form "<dbname>.<username>".  This function extracts the UserName from that string.
+    static StatusWith<UserName> extractUserNameFromIdString(StringData idstr) {
+        size_t splitPoint = idstr.find('.');
+        if (splitPoint == std::string::npos) {
+            return StatusWith<UserName>(ErrorCodes::FailedToParse,
+                                        mongoutils::str::stream()
+                                            << "_id entries for user documents must be of "
+                                               "the form <dbname>.<username>.  Found: "
+                                            << idstr);
+        }
+        return StatusWith<UserName>(
+            UserName(idstr.substr(splitPoint + 1), idstr.substr(0, splitPoint)));
+    }
 
-          _isO2Set(o2 ? true : false),
-          _o2(_isO2Set ? o2->getOwned() : BSONObj()) {}
 
-    virtual void commit(boost::optional<Timestamp>) {
+    void _refreshRoleGraph() {
         stdx::lock_guard<stdx::mutex> lk(_externalState->_roleGraphMutex);
         Status status = _externalState->_roleGraph.handleLogOp(
-            _opCtx, _op.c_str(), _nss, _o, _isO2Set ? &_o2 : NULL);
+            _opCtx, _op.c_str(), _nss, _o, _o2 ? &*_o2 : NULL);
 
         if (status == ErrorCodes::OplogOperationUnsupported) {
             _externalState->_roleGraph = RoleGraph();
             _externalState->_roleGraphState = _externalState->roleGraphStateInitial;
             BSONObjBuilder oplogEntryBuilder;
             oplogEntryBuilder << "op" << _op << "ns" << _nss.ns() << "o" << _o;
-            if (_isO2Set)
-                oplogEntryBuilder << "o2" << _o2;
+            if (_o2) {
+                oplogEntryBuilder << "o2" << *_o2;
+            }
             error() << "Unsupported modification to roles collection in oplog; "
                        "restart this process to reenable user-defined roles; "
                     << redact(status) << "; Oplog entry: " << redact(oplogEntryBuilder.done());
+            // If a setParameter is enabled, this condition is fatal.
+            fassert(51152, !roleGraphInvalidationIsFatal);
         } else if (!status.isOK()) {
             warning() << "Skipping bad update to roles collection in oplog. " << redact(status)
                       << " Oplog entry: " << redact(_op);
@@ -574,29 +614,77 @@ public:
         }
     }
 
-    virtual void rollback() {}
+    boost::optional<std::vector<UserHandle>> _invalidateRelevantCacheData() {
+        // When we're doing a user management command we lock the admin DB for the duration
+        // of the command and invalidate the cache at the end of the command, so we don't need
+        // to invalidate it based on calls to logOp().
+        if (inUserManagementCommandsFlag(_opCtx)) {
+            LOG(1) << "Skipping cache invalidation in opObserver because of active user command";
+            return boost::none;
+        }
+
+        if (_nss == AuthorizationManager::rolesCollectionNamespace ||
+            _nss == AuthorizationManager::versionCollectionNamespace) {
+            return _authzManager->invalidateUserCacheNoPin(_opCtx);
+        }
+
+        if (_op == "i" || _op == "d" || _op == "u") {
+            // If you got into this function isAuthzNamespace() must have returned true, and we've
+            // already checked that it's not the roles or version collection.
+            invariant(_nss == AuthorizationManager::usersCollectionNamespace);
+
+            StatusWith<UserName> userName = (_op == "u")
+                ? extractUserNameFromIdString((*_o2)["_id"].str())
+                : extractUserNameFromIdString(_o["_id"].str());
+
+            if (!userName.isOK()) {
+                warning() << "Invalidating user cache based on user being updated failed, will "
+                             "invalidate the entire cache instead: "
+                          << userName.getStatus();
+                return _authzManager->invalidateUserCacheNoPin(_opCtx);
+            }
+            return _authzManager->invalidateUserByNameNoPin(_opCtx, userName.getValue());
+        } else {
+            return _authzManager->invalidateUserCacheNoPin(_opCtx);
+        }
+    }
+
 
-private:
     OperationContext* _opCtx;
+    AuthorizationManagerImpl* _authzManager;
     AuthzManagerExternalStateLocal* _externalState;
     const std::string _op;
     const NamespaceString _nss;
     const BSONObj _o;
+    const boost::optional<BSONObj> _o2;
 
-    const bool _isO2Set;
-    const BSONObj _o2;
+    boost::optional<std::vector<UserHandle>> _refreshedPinnedUsers;
 };
 
 void AuthzManagerExternalStateLocal::logOp(OperationContext* opCtx,
+                                           AuthorizationManagerImpl* authzManager,
                                            const char* op,
                                            const NamespaceString& nss,
                                            const BSONObj& o,
                                            const BSONObj* o2) {
     if (nss == AuthorizationManager::rolesCollectionNamespace ||
+        nss == AuthorizationManager::versionCollectionNamespace ||
+        nss == AuthorizationManager::usersCollectionNamespace ||
         nss == AuthorizationManager::adminCommandNamespace) {
-        opCtx->recoveryUnit()->registerChange(
-            new AuthzManagerLogOpHandler(opCtx, this, op, nss, o, o2));
+
+        auto change = new AuthzManagerLogOpHandler(opCtx, authzManager, this, op, nss, o, o2);
+        // AuthzManagerExternalState's logOp method registers a RecoveryUnit::Change
+        // and to do so we need to have begun a UnitOfWork
+        WriteUnitOfWork wuow(opCtx);
+
+        opCtx->recoveryUnit()->registerChange(change);
+
+        wuow.commit();
     }
 }
 
+void AuthzManagerExternalStateLocal::setInUserManagementCommand(OperationContext* opCtx, bool val) {
+    inUserManagementCommandsFlag(opCtx) = val;
+}
+
 }  // namespace mongo
diff --git a/src/mongo/db/auth/authz_manager_external_state_local.h b/src/mongo/db/auth/authz_manager_external_state_local.h
index 492f182cbbd..b3dd05cefb3 100644
--- a/src/mongo/db/auth/authz_manager_external_state_local.h
+++ b/src/mongo/db/auth/authz_manager_external_state_local.h
@@ -103,11 +103,15 @@ public:
                          const BSONObj& projection,
                          const stdx::function<void(const BSONObj&)>& resultProcessor) = 0;
 
-    virtual void logOp(OperationContext* opCtx,
-                       const char* op,
-                       const NamespaceString& ns,
-                       const BSONObj& o,
-                       const BSONObj* o2);
+    void logOp(OperationContext* opCtx,
+               AuthorizationManagerImpl* authManager,
+               const char* op,
+               const NamespaceString& ns,
+               const BSONObj& o,
+               const BSONObj* o2) final;
+
+
+    void setInUserManagementCommand(OperationContext* opCtx, bool val) final;
 
     /**
      * Takes a user document, and processes it with the RoleGraph, in order to recursively
diff --git a/src/mongo/db/auth/role_graph_update.cpp b/src/mongo/db/auth/role_graph_update.cpp
index 5a2584eaba3..4335bd58697 100644
--- a/src/mongo/db/auth/role_graph_update.cpp
+++ b/src/mongo/db/auth/role_graph_update.cpp
@@ -32,6 +32,7 @@
 #include "mongo/base/status.h"
 #include "mongo/bson/mutable/document.h"
 #include "mongo/bson/mutable/element.h"
+#include "mongo/bson/unordered_fields_bsonobj_comparator.h"
 #include "mongo/bson/util/bson_extract.h"
 #include "mongo/db/auth/address_restriction.h"
 #include "mongo/db/auth/authorization_manager.h"
@@ -297,6 +298,10 @@ Status handleOplogCommand(RoleGraph* roleGraph, const BSONObj& cmdObj) {
         return Status::OK();
     }
 
+    if (cmdName == "commitTransaction" || cmdName == "abortTransaction") {
+        return Status::OK();
+    }
+
     if (cmdName == "dropIndexes" || cmdName == "deleteIndexes") {
         return Status::OK();
     }
@@ -305,6 +310,22 @@ Status handleOplogCommand(RoleGraph* roleGraph, const BSONObj& cmdObj) {
         // We don't care about these if they're not on the roles collection.
         return Status::OK();
     }
+    if (cmdName == "createIndexes" &&
+        cmdObj.firstElement().str() == rolesCollectionNamespace.coll()) {
+        UnorderedFieldsBSONObjComparator instance;
+        if (instance.evaluate(cmdObj == (BSON("createIndexes"
+                                              << "system.roles"
+                                              << "v"
+                                              << 2
+                                              << "name"
+                                              << "role_1_db_1"
+                                              << "key"
+                                              << BSON("role" << 1 << "db" << 1)
+                                              << "unique"
+                                              << true)))) {
+            return Status::OK();
+        }
+    }
 
     if ((cmdName == "collMod") && (cmdObj.nFields() == 1)) {
         // We also don't care about empty modifications even if they are on roles collection