Auth before cloning collection during rollback SERVER-4115

4 files changed, 94 insertions, 24 deletions

diff --git a/db/cloner.cpp b/db/cloner.cpp
index 4b68a473ffa..26c2f74b6df 100644
--- a/db/cloner.cpp
+++ b/db/cloner.cpp
@@ -68,7 +68,7 @@ namespace mongo {
         /** copy the entire database */
         bool go(const char *masterHost, string& errmsg, const string& fromdb, bool logForRepl, bool slaveOk, bool useReplAuth, bool snapshot, bool mayYield, bool mayBeInterrupted, int *errCode = 0);
 
-        bool copyCollection( const string& from , const string& ns , const BSONObj& query , string& errmsg , bool mayYield, bool mayBeInterrupted, bool copyIndexes = true, bool logForRepl = true );
+        bool copyCollection( const string& ns , const BSONObj& query , string& errmsg , bool mayYield, bool mayBeInterrupted, bool copyIndexes = true, bool logForRepl = true );
     };
 
     /* for index info object:
@@ -244,18 +244,19 @@ namespace mongo {
         }
     }
 
-    bool copyCollectionFromRemote(const string& host, const string& ns, const BSONObj& query, string& errmsg, bool logForRepl, bool mayYield, bool mayBeInterrupted) {
+    bool copyCollectionFromRemote(const string& host, const string& ns, string& errmsg) {
         Cloner c;
-        return c.copyCollection(host, ns, query, errmsg, mayYield, mayBeInterrupted, /*copyIndexes*/ true, logForRepl);
-    }
 
-    bool Cloner::copyCollection( const string& from , const string& ns , const BSONObj& query , string& errmsg , bool mayYield, bool mayBeInterrupted, bool copyIndexes, bool logForRepl ) {
-        auto_ptr<DBClientConnection> myconn;
-        myconn.reset( new DBClientConnection() );
-        if ( ! myconn->connect( from , errmsg ) )
-            return false;
+        DBClientConnection *conn = new DBClientConnection();
+        // cloner owns conn in auto_ptr
+        c.setConnection(conn);
+        uassert(15908, errmsg, conn->connect(host, errmsg) && replAuthenticate(conn));
+
+        return c.copyCollection(ns, BSONObj(), errmsg, true, false, /*copyIndexes*/ true, false);
+    }
 
-        conn.reset( myconn.release() );
+    bool Cloner::copyCollection( const string& ns, const BSONObj& query, string& errmsg,
+                                 bool mayYield, bool mayBeInterrupted, bool copyIndexes, bool logForRepl ) {
 
         writelock lk(ns); // TODO: make this lower down
         Client::Context ctx(ns);
@@ -521,7 +522,14 @@ namespace mongo {
                   << " query: " << query << " " << ( copyIndexes ? "" : ", not copying indexes" ) << endl;
 
             Cloner c;
-            return c.copyCollection( fromhost , collection , query, errmsg , true, false, copyIndexes );
+            auto_ptr<DBClientConnection> myconn;
+            myconn.reset( new DBClientConnection() );
+            if ( ! myconn->connect( fromhost , errmsg ) )
+                return false;
+
+            c.setConnection( myconn.release() );
+
+            return c.copyCollection( collection , query, errmsg , true, false, copyIndexes );
         }
     } cmdclonecollection;
 
diff --git a/db/cloner.h b/db/cloner.h
index 94264f85e58..130fea0fac1 100644
--- a/db/cloner.h
+++ b/db/cloner.h
@@ -34,6 +34,6 @@ namespace mongo {
                    bool slaveOk, bool useReplAuth, bool snapshot, bool mayYield,
                    bool mayBeInterrupted, int *errCode = 0);
 
-    bool copyCollectionFromRemote(const string& host, const string& ns, const BSONObj& query, string& errmsg, bool logForRepl, bool mayYield, bool mayBeInterrupted);
+    bool copyCollectionFromRemote(const string& host, const string& ns, string& errmsg);
 
 } // namespace mongo
diff --git a/db/repl/rs_rollback.cpp b/db/repl/rs_rollback.cpp
index f012e6577a8..97a910e8309 100644
--- a/db/repl/rs_rollback.cpp
+++ b/db/repl/rs_rollback.cpp
@@ -388,24 +388,18 @@ namespace mongo {
             for( set<string>::iterator i = h.collectionsToResync.begin(); i != h.collectionsToResync.end(); i++ ) {
                 string ns = *i;
                 sethbmsg(str::stream() << "rollback 4.1 coll resync " << ns);
-                Client::Context c(*i);
-                try {
+
+                Client::Context c(ns);
+                {
                     bob res;
                     string errmsg;
                     dropCollection(ns, errmsg, res);
                     {
                         dbtemprelease r;
-                        bool ok = copyCollectionFromRemote(them->getServerAddress(), ns, bo(), errmsg, false, true, false);
-                        if( !ok ) {
-                            log() << "replSet rollback error resyncing collection " << ns << ' ' << errmsg << rsLog;
-                            throw "rollback error resyncing rollection [1]";
-                        }
+                        bool ok = copyCollectionFromRemote(them->getServerAddress(), ns, errmsg);
+                        uassert(15909, str::stream() << "replSet rollback error resyncing collection " << ns << ' ' << errmsg, ok);
                     }
                 }
-                catch(...) {
-                    log() << "replset rollback error resyncing collection " << ns << rsLog;
-                    throw "rollback error resyncing rollection [2]";
-                }
             }
 
             /* we did more reading from primary, so check it again for a rollback (which would mess us up), and
@@ -423,7 +417,7 @@ namespace mongo {
                         setMinValid(newMinValid);
                     }
                 }
-                catch(...) {
+                catch (DBException& e) {
                     err = "can't get/set minvalid";
                 }
                 if( h.rbid != getRBID(r.conn()) ) {
diff --git a/jstests/replsets/auth3.js b/jstests/replsets/auth3.js
new file mode 100644
index 00000000000..ff1e6ccd97d
--- /dev/null
+++ b/jstests/replsets/auth3.js
@@ -0,0 +1,68 @@
+var path = "jstests/libs/";
+
+var rs = new ReplSetTest({"nodes" : {node0 : {}, node1 : {}, arbiter : {}}, keyFile : path+"key1"});
+rs.startSet();
+rs.initiate();
+
+master = rs.getMaster();
+print("adding user");
+master.getDB("admin").addUser("foo", "bar");
+
+var checkValidState = function(i) {
+    assert.soon(function() {
+        var result = rs.nodes[i].getDB("admin").runCommand({isMaster : 1});
+        printjson(result);
+        return result.secondary || result.ismaster;
+    });
+};
+
+var safeInsert = function() {
+    master = rs.getMaster();
+    master.getDB("admin").auth("foo", "bar");
+    master.getDB("foo").bar.insert({x:1});
+    var insertWorked = master.getDB("foo").runCommand({getlasterror:1});
+    printjson(insertWorked);
+    assert.eq(insertWorked.ok, 1);
+}
+
+print("authing");
+for (var i=0; i<2; i++) {
+    checkValidState(i);
+
+    // if this is run before initial sync finishes, we won't be logged in
+    rs.nodes[i].getDB("admin").auth("foo", "bar");
+}
+
+print("make common point");
+
+safeInsert();
+rs.awaitReplication();
+
+print("write stuff to 0&2")
+rs.stop(1);
+
+master = rs.getMaster();
+master.getDB("foo").bar.drop();
+print("last op: "+tojson(master.getDB("local").oplog.rs.find().sort({$natural:-1}).limit(1).next()));
+
+print("write stuff to 1&2")
+rs.stop(0);
+rs.restart(1);
+
+safeInsert();
+print("last op: "+tojson(master.getDB("local").oplog.rs.find().sort({$natural:-1}).limit(1).next()));
+
+rs.restart(0);
+
+print("doing rollback!");
+assert.soon(function() {
+    var result = rs.nodes[0].getDB("admin").runCommand({isMaster : 1});
+    printjson(result);
+    return !result.secondary && !result.ismaster;
+});
+
+print("make sure 0 successfully rolls back");
+m = rs.nodes[0];
+
+checkValidState(0);
+checkValidState(1);