SERVER-15192 Make AuthzManager logOp listener rollback-safe

7 files changed, 105 insertions, 25 deletions

diff --git a/src/mongo/db/auth/authorization_manager.cpp b/src/mongo/db/auth/authorization_manager.cpp
index 9e546d7da3d..f8b8ca2b3b2 100644
--- a/src/mongo/db/auth/authorization_manager.cpp
+++ b/src/mongo/db/auth/authorization_manager.cpp
@@ -993,13 +993,14 @@ namespace {
     }
 
     void AuthorizationManager::logOp(
+            OperationContext* txn,
             const char* op,
             const char* ns,
             const BSONObj& o,
             BSONObj* o2,
             bool* b) {
 
-        _externalState->logOp(op, ns, o, o2, b);
+        _externalState->logOp(txn, op, ns, o, o2, b);
         if (appliesToAuthzData(op, ns, o)) {
             _invalidateRelevantCacheData(op, ns, o, o2);
         }
diff --git a/src/mongo/db/auth/authorization_manager.h b/src/mongo/db/auth/authorization_manager.h
index 1e2d193b65f..c3371bb11c7 100644
--- a/src/mongo/db/auth/authorization_manager.h
+++ b/src/mongo/db/auth/authorization_manager.h
@@ -427,7 +427,8 @@ namespace mongo {
          * Hook called by replication code to let the AuthorizationManager observe changes
          * to relevant collections.
          */
-        void logOp(const char* opstr,
+        void logOp(OperationContext* txn,
+                   const char* opstr,
                    const char* ns,
                    const BSONObj& obj,
                    BSONObj* patt,
diff --git a/src/mongo/db/auth/authz_manager_external_state.h b/src/mongo/db/auth/authz_manager_external_state.h
index 107724f9c59..4c54cabc1aa 100644
--- a/src/mongo/db/auth/authz_manager_external_state.h
+++ b/src/mongo/db/auth/authz_manager_external_state.h
@@ -229,6 +229,7 @@ namespace mongo {
         virtual void releaseAuthzUpdateLock() = 0;
 
         virtual void logOp(
+                OperationContext* txn,
                 const char* op,
                 const char* ns,
                 const BSONObj& o,
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 507db983a53..989573e316a 100644
--- a/src/mongo/db/auth/authz_manager_external_state_local.cpp
+++ b/src/mongo/db/auth/authz_manager_external_state_local.cpp
@@ -36,12 +36,14 @@
 #include "mongo/bson/util/bson_extract.h"
 #include "mongo/db/auth/authorization_manager.h"
 #include "mongo/db/auth/user_document_parser.h"
+#include "mongo/db/operation_context.h"
 #include "mongo/util/log.h"
 #include "mongo/util/mongoutils/str.h"
 
 namespace mongo {
 
     using std::vector;
+    using boost::optional;
 
     AuthzManagerExternalStateLocal::AuthzManagerExternalStateLocal() :
         _roleGraphState(roleGraphStateInitial) {}
@@ -365,47 +367,97 @@ namespace {
         return status;
     }
 
-    void AuthzManagerExternalStateLocal::logOp(
-            const char* op,
-            const char* ns,
-            const BSONObj& o,
-            BSONObj* o2,
-            bool* b) {
+    class AuthzManagerExternalStateLocal::AuthzManagerLogOpHandler : public RecoveryUnit::Change {
+    public:
+        AuthzManagerLogOpHandler(AuthzManagerExternalStateLocal* externalState,
+                                 const std::string& op,
+                                 const std::string& ns,
+                                 const BSONObj& o,
+                                 const optional<BSONObj>& o2,
+                                 const optional<bool>& b):
+            _externalState(externalState),
+            _op(op),
+            _ns(ns),
+            _o(o.getOwned()),
+            _o2(o2 ? o2->getOwned() : o2), // since o2 is an optional<BSONObj>
+            _b(b) {
 
-        if (ns == AuthorizationManager::rolesCollectionNamespace.ns() ||
-            ns == AuthorizationManager::adminCommandNamespace.ns()) {
+        }
 
-            boost::lock_guard<boost::mutex> lk(_roleGraphMutex);
-            Status status = _roleGraph.handleLogOp(op, NamespaceString(ns), o, o2);
+        virtual void commit() {
+            boost::lock_guard<boost::mutex> lk(_externalState->_roleGraphMutex);
+            Status status = _externalState->_roleGraph.handleLogOp(_op.c_str(),
+                                                                   NamespaceString(_ns.c_str()),
+                                                                   _o,
+                                                                   _o2 ? _o2.get_ptr() : NULL);
 
             if (status == ErrorCodes::OplogOperationUnsupported) {
-                _roleGraph = RoleGraph();
-                _roleGraphState = roleGraphStateInitial;
+                _externalState->_roleGraph = RoleGraph();
+                _externalState->_roleGraphState = _externalState->roleGraphStateInitial;
                 BSONObjBuilder oplogEntryBuilder;
-                oplogEntryBuilder << "op" << op << "ns" << ns << "o" << o;
-                if (o2)
-                    oplogEntryBuilder << "o2" << *o2;
-                if (b)
-                    oplogEntryBuilder << "b" << *b;
+                oplogEntryBuilder << "op" << _op << "ns" << _ns << "o" << _o;
+                if (_o2)
+                    oplogEntryBuilder << "o2" << _o2;
+                if (_b)
+                    oplogEntryBuilder << "b" << _b;
                 error() << "Unsupported modification to roles collection in oplog; "
                     "restart this process to reenable user-defined roles; " << status.reason() <<
                     "; Oplog entry: " << oplogEntryBuilder.done();
             }
             else if (!status.isOK()) {
                 warning() << "Skipping bad update to roles collection in oplog. " << status <<
-                    " Oplog entry: " << op;
+                    " Oplog entry: " << _op;
             }
-            status = _roleGraph.recomputePrivilegeData();
+            status = _externalState->_roleGraph.recomputePrivilegeData();
             if (status == ErrorCodes::GraphContainsCycle) {
-                _roleGraphState = roleGraphStateHasCycle;
+                _externalState->_roleGraphState = _externalState->roleGraphStateHasCycle;
                 error() << "Inconsistent role graph during authorization manager initialization.  "
                     "Only direct privileges available. " << status.reason() <<
-                    " after applying oplog entry " << op;
+                    " after applying oplog entry " << _op;
             }
             else {
                 fassert(17183, status);
-                _roleGraphState = roleGraphStateConsistent;
+                _externalState->_roleGraphState = _externalState->roleGraphStateConsistent;
             }
+
+        }
+
+        virtual void rollback() { }
+
+    private:
+        AuthzManagerExternalStateLocal* _externalState;
+        const std::string _op;
+        const std::string _ns;
+        const BSONObj _o;
+        const optional<BSONObj> _o2;
+        const optional<bool> _b;
+    };
+
+    void AuthzManagerExternalStateLocal::logOp(
+            OperationContext* txn,
+            const char* op,
+            const char* ns,
+            const BSONObj& o,
+            BSONObj* o2,
+            bool* b) {
+
+        if (ns == AuthorizationManager::rolesCollectionNamespace.ns() ||
+            ns == AuthorizationManager::adminCommandNamespace.ns()) {
+
+            std::string opStr(op);
+            std::string nsStr(ns);
+            optional<BSONObj> o2Copy;
+            if (o2)
+                o2Copy = *o2;
+            optional<bool> bCopy;
+            if (b)
+                bCopy = *b;
+            txn->recoveryUnit()->registerChange(new AuthzManagerLogOpHandler(this,
+                                                                             opStr,
+                                                                             nsStr,
+                                                                             o,
+                                                                             o2Copy,
+                                                                             bCopy));
         }
     }
 
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 1fd95a6cadc..87648b911c0 100644
--- a/src/mongo/db/auth/authz_manager_external_state_local.h
+++ b/src/mongo/db/auth/authz_manager_external_state_local.h
@@ -65,6 +65,7 @@ namespace mongo {
                                                 std::vector<BSONObj>* result);
 
         virtual void logOp(
+                OperationContext* txn,
                 const char* op,
                 const char* ns,
                 const BSONObj& o,
@@ -82,6 +83,11 @@ namespace mongo {
         };
 
         /**
+         * RecoveryUnit::Change subclass used to commit work for AuthzManager logOp listener.
+         */
+        class AuthzManagerLogOpHandler;
+
+        /**
          * Initializes the role graph from the contents of the admin.system.roles collection.
          */
         Status _initializeRoleGraph(OperationContext* txn);
diff --git a/src/mongo/db/auth/authz_manager_external_state_mock.cpp b/src/mongo/db/auth/authz_manager_external_state_mock.cpp
index bcf2c7847c8..c031cbc52cc 100644
--- a/src/mongo/db/auth/authz_manager_external_state_mock.cpp
+++ b/src/mongo/db/auth/authz_manager_external_state_mock.cpp
@@ -152,14 +152,17 @@ namespace {
             toInsert = document.copy();
         }
         _documents[collectionName].push_back(toInsert);
+
         if (_authzManager) {
             _authzManager->logOp(
+                    txn,
                     "i",
                     collectionName.ns().c_str(),
                     toInsert,
                     NULL,
                     NULL);
         }
+
         return Status::OK();
     }
 
@@ -190,14 +193,17 @@ namespace {
             BSONObj newObj = document.getObject().copy();
             *iter = newObj;
             BSONObj idQuery = driver.makeOplogEntryQuery(newObj, false);
+
             if (_authzManager) {
                 _authzManager->logOp(
+                        txn,
                         "u",
                         collectionName.ns().c_str(),
                         logObj,
                         &idQuery,
                         NULL);
             }
+
             return Status::OK();
         }
         else if (status == ErrorCodes::NoMatchingDocument && upsert) {
@@ -243,14 +249,17 @@ namespace {
             BSONObj idQuery = (*iter)["_id"].wrap();
             _documents[collectionName].erase(iter);
             ++n;
+
             if (_authzManager) {
                 _authzManager->logOp(
+                        txn,
                         "d",
                         collectionName.ns().c_str(),
                         idQuery,
                         NULL,
                         NULL);
             }
+
         }
         *numRemoved = n;
         return Status::OK();
diff --git a/src/mongo/db/repl/oplog.cpp b/src/mongo/db/repl/oplog.cpp
index e4d64cdb1f9..f44be889852 100644
--- a/src/mongo/db/repl/oplog.cpp
+++ b/src/mongo/db/repl/oplog.cpp
@@ -379,6 +379,10 @@ namespace {
         if ( getGlobalReplicationCoordinator()->isReplEnabled() ) {
             _logOp(txn, opstr, ns, 0, obj, patt, b, fromMigrate);
         }
+        //
+        // rollback-safe logOp listeners
+        //
+        getGlobalAuthorizationManager()->logOp(txn, opstr, ns, obj, patt, b);
 
         try {
             // TODO SERVER-15192 remove this once all listeners are rollback-safe.
@@ -392,7 +396,6 @@ namespace {
             txn->recoveryUnit()->registerChange(new RollbackPreventer());
             logOpForSharding(txn, opstr, ns, obj, patt, fromMigrate);
             logOpForDbHash(ns);
-            getGlobalAuthorizationManager()->logOp(opstr, ns, obj, patt, b);
 
             if ( strstr( ns, ".system.js" ) ) {
                 Scope::storedFuncMod(); // this is terrible
@@ -783,12 +786,19 @@ namespace {
         else {
             throw MsgAssertionException( 14825 , ErrorMsg("error in applyOperation : unknown opType ", *opType) );
         }
+
+        // AuthorizationManager's logOp method registers a RecoveryUnit::Change
+        // and to do so we need to have begun a UnitOfWork
+        WriteUnitOfWork wuow(txn);
         getGlobalAuthorizationManager()->logOp(
+                txn,
                 opType,
                 ns,
                 o,
                 fieldO2.isABSONObj() ? &o2 : NULL,
                 !fieldB.eoo() ? &valueB : NULL );
+        wuow.commit();
+
         return failedUpdate;
     }