SERVER-25064 Make mongos upgrade w:1 to w:majority for auth writes

Also extend the auth tests to validate that both variants work.

4 files changed, 87 insertions, 45 deletions

diff --git a/src/mongo/db/write_concern_options.cpp b/src/mongo/db/write_concern_options.cpp
index 028d03d4635..dd9694dcc61 100644
--- a/src/mongo/db/write_concern_options.cpp
+++ b/src/mongo/db/write_concern_options.cpp
@@ -62,6 +62,7 @@ constexpr StringData kWElectionIdFieldName = "wElectionId"_sd;
 const int WriteConcernOptions::kNoTimeout(0);
 const int WriteConcernOptions::kNoWaiting(-1);
 
+const StringData WriteConcernOptions::kWriteConcernField = "writeConcern"_sd;
 const char WriteConcernOptions::kMajority[] = "majority";
 
 const BSONObj WriteConcernOptions::Default = BSONObj();
@@ -159,7 +160,8 @@ StatusWith<WriteConcernOptions> WriteConcernOptions::extractWCFromCommand(
     }
 
     BSONElement writeConcernElement;
-    Status wcStatus = bsonExtractTypedField(cmdObj, "writeConcern", Object, &writeConcernElement);
+    Status wcStatus =
+        bsonExtractTypedField(cmdObj, kWriteConcernField, Object, &writeConcernElement);
     if (!wcStatus.isOK()) {
         if (wcStatus == ErrorCodes::NoSuchKey) {
             // Return default write concern if no write concern is given.
diff --git a/src/mongo/db/write_concern_options.h b/src/mongo/db/write_concern_options.h
index 06ddfbc56a0..85f9e0e0390 100644
--- a/src/mongo/db/write_concern_options.h
+++ b/src/mongo/db/write_concern_options.h
@@ -47,6 +47,7 @@ public:
     static const BSONObj Unacknowledged;
     static const BSONObj Majority;
 
+    static const StringData kWriteConcernField;
     static const char kMajority[];  // = "majority"
 
     WriteConcernOptions() {
diff --git a/src/mongo/s/catalog/replset/sharding_catalog_client_impl.cpp b/src/mongo/s/catalog/replset/sharding_catalog_client_impl.cpp
index 0c9e47d976e..57d701a82a6 100644
--- a/src/mongo/s/catalog/replset/sharding_catalog_client_impl.cpp
+++ b/src/mongo/s/catalog/replset/sharding_catalog_client_impl.cpp
@@ -96,8 +96,6 @@ using str::stream;
 
 namespace {
 
-const char kWriteConcernField[] = "writeConcern";
-
 const ReadPreferenceSetting kConfigReadSelector(ReadPreference::Nearest, TagSet{});
 const ReadPreferenceSetting kConfigPrimaryPreferredSelector(ReadPreference::PrimaryPreferred,
                                                             TagSet{});
@@ -767,12 +765,13 @@ Status ShardingCatalogClientImpl::dropCollection(OperationContext* txn, const Na
             return {ErrorCodes::ShardNotFound,
                     str::stream() << "shard " << shardEntry.getName() << " not found"};
         }
-        auto dropResult = shard->runCommand(
-            txn,
-            ReadPreferenceSetting{ReadPreference::PrimaryOnly},
-            ns.db().toString(),
-            BSON("drop" << ns.coll() << "writeConcern" << txn->getWriteConcern().toBSON()),
-            Shard::RetryPolicy::kIdempotent);
+        auto dropResult =
+            shard->runCommand(txn,
+                              ReadPreferenceSetting{ReadPreference::PrimaryOnly},
+                              ns.db().toString(),
+                              BSON("drop" << ns.coll() << WriteConcernOptions::kWriteConcernField
+                                          << txn->getWriteConcern().toBSON()),
+                              Shard::RetryPolicy::kIdempotent);
 
         if (!dropResult.isOK()) {
             return Status(dropResult.getStatus().code(),
@@ -1134,24 +1133,26 @@ bool ShardingCatalogClientImpl::runUserManagementWriteCommand(OperationContext*
         // convert w:1 or no write concern to w:majority before sending.
         WriteConcernOptions writeConcern;
         writeConcern.reset();
-        const char* writeConcernFieldName = "writeConcern";
-        BSONElement writeConcernElement = cmdObj[writeConcernFieldName];
+
+        BSONElement writeConcernElement = cmdObj[WriteConcernOptions::kWriteConcernField];
         bool initialCmdHadWriteConcern = !writeConcernElement.eoo();
         if (initialCmdHadWriteConcern) {
             Status status = writeConcern.parse(writeConcernElement.Obj());
             if (!status.isOK()) {
                 return Command::appendCommandStatus(*result, status);
             }
-            if (!writeConcern.validForConfigServers()) {
+
+            if (!(writeConcern.wNumNodes == 1 ||
+                  writeConcern.wMode == WriteConcernOptions::kMajority)) {
                 return Command::appendCommandStatus(
                     *result,
-                    Status(ErrorCodes::InvalidOptions,
-                           str::stream()
-                               << "Invalid replication write concern.  Writes to config server "
-                                  "replica sets must use w:'majority', got: "
-                               << writeConcern.toBSON()));
+                    {ErrorCodes::InvalidOptions,
+                     str::stream() << "Invalid replication write concern. User management write "
+                                      "commands may only use w:1 or w:'majority', got: "
+                                   << writeConcern.toBSON()});
             }
         }
+
         writeConcern.wMode = WriteConcernOptions::kMajority;
         writeConcern.wNumNodes = 0;
 
@@ -1162,13 +1163,13 @@ bool ShardingCatalogClientImpl::runUserManagementWriteCommand(OperationContext*
             BSONObjIterator cmdObjIter(cmdObj);
             while (cmdObjIter.more()) {
                 BSONElement e = cmdObjIter.next();
-                if (str::equals(e.fieldName(), writeConcernFieldName)) {
+                if (WriteConcernOptions::kWriteConcernField == e.fieldName()) {
                     continue;
                 }
                 modifiedCmd.append(e);
             }
         }
-        modifiedCmd.append(writeConcernFieldName, writeConcern.toBSON());
+        modifiedCmd.append(WriteConcernOptions::kWriteConcernField, writeConcern.toBSON());
         cmdToRun = modifiedCmd.obj();
     }
 
@@ -1230,9 +1231,9 @@ Status ShardingCatalogClientImpl::applyChunkOpsDeprecated(OperationContext* txn,
                                                           const BSONArray& preCondition,
                                                           const std::string& nss,
                                                           const ChunkVersion& lastChunkVersion) {
-    BSONObj cmd =
-        BSON("applyOps" << updateOps << "preCondition" << preCondition << kWriteConcernField
-                        << ShardingCatalogClient::kMajorityWriteConcern.toBSON());
+    BSONObj cmd = BSON("applyOps" << updateOps << "preCondition" << preCondition
+                                  << WriteConcernOptions::kWriteConcernField
+                                  << ShardingCatalogClient::kMajorityWriteConcern.toBSON());
 
     auto response = Grid::get(txn)->shardRegistry()->getConfigShard()->runCommand(
         txn,
@@ -1511,9 +1512,9 @@ Status ShardingCatalogClientImpl::_checkDbDoesNotExist(OperationContext* txn,
 Status ShardingCatalogClientImpl::_createCappedConfigCollection(OperationContext* txn,
                                                                 StringData collName,
                                                                 int cappedSize) {
-    BSONObj createCmd =
-        BSON("create" << collName << "capped" << true << "size" << cappedSize << "writeConcern"
-                      << ShardingCatalogClient::kMajorityWriteConcern.toBSON());
+    BSONObj createCmd = BSON("create" << collName << "capped" << true << "size" << cappedSize
+                                      << WriteConcernOptions::kWriteConcernField
+                                      << ShardingCatalogClient::kMajorityWriteConcern.toBSON());
 
     auto result = Grid::get(txn)->shardRegistry()->getConfigShard()->runCommand(
         txn,
diff --git a/src/mongo/s/catalog/replset/sharding_catalog_test.cpp b/src/mongo/s/catalog/replset/sharding_catalog_test.cpp
index 5ecf1899c60..e4d06494d1b 100644
--- a/src/mongo/s/catalog/replset/sharding_catalog_test.cpp
+++ b/src/mongo/s/catalog/replset/sharding_catalog_test.cpp
@@ -679,27 +679,9 @@ TEST_F(ShardingCatalogClientTest, RunUserManagementWriteCommandSuccess) {
     future.timed_get(kFutureTimeout);
 }
 
-TEST_F(ShardingCatalogClientTest, RunUserManagementWriteCommandWithInvalidWriteConcernSingleNode) {
-    // Tests that if you send a non-majority write concern it will not be accepted
-    BSONObjBuilder responseBuilder;
-    bool ok =
-        catalogClient()->runUserManagementWriteCommand(operationContext(),
-                                                       "dropUser",
-                                                       "test",
-                                                       BSON("dropUser"
-                                                            << "test"
-                                                            << "writeConcern"
-                                                            << BSON("w" << 1 << "wtimeout" << 30)),
-                                                       &responseBuilder);
-    ASSERT_FALSE(ok);
-
-    Status commandStatus = getStatusFromCommandResult(responseBuilder.obj());
-    ASSERT_EQUALS(ErrorCodes::InvalidOptions, commandStatus);
-    ASSERT_STRING_CONTAINS(commandStatus.reason(), "Invalid replication write concern");
-}
+TEST_F(ShardingCatalogClientTest, RunUserManagementWriteCommandInvalidWriteConcern) {
+    configTargeter()->setFindHostReturnValue(HostAndPort("TestHost1"));
 
-TEST_F(ShardingCatalogClientTest, RunUserManagementWriteCommandInvalidWriteConcernTwoNodes) {
-    // Tests that if you send a non-majority write concern it will not be accepted
     BSONObjBuilder responseBuilder;
     bool ok = catalogClient()->runUserManagementWriteCommand(operationContext(),
                                                              "dropUser",
@@ -716,6 +698,62 @@ TEST_F(ShardingCatalogClientTest, RunUserManagementWriteCommandInvalidWriteConce
     ASSERT_STRING_CONTAINS(commandStatus.reason(), "Invalid replication write concern");
 }
 
+TEST_F(ShardingCatalogClientTest, RunUserManagementWriteCommandRewriteWriteConcern) {
+    // Tests that if you send a w:1 write concern it gets replaced with w:majority
+    configTargeter()->setFindHostReturnValue(HostAndPort("TestHost1"));
+
+    distLock()->expectLock(
+        [](StringData name,
+           StringData whyMessage,
+           Milliseconds waitFor,
+           Milliseconds lockTryInterval) {
+            ASSERT_EQUALS("authorizationData", name);
+            ASSERT_EQUALS("dropUser", whyMessage);
+        },
+        Status::OK());
+
+    auto future = launchAsync([this] {
+        BSONObjBuilder responseBuilder;
+        bool ok = catalogClient()->runUserManagementWriteCommand(operationContext(),
+                                                                 "dropUser",
+                                                                 "test",
+                                                                 BSON("dropUser"
+                                                                      << "test"
+                                                                      << "writeConcern"
+                                                                      << BSON("w" << 1 << "wtimeout"
+                                                                                  << 30)),
+                                                                 &responseBuilder);
+        ASSERT_FALSE(ok);
+
+        Status commandStatus = getStatusFromCommandResult(responseBuilder.obj());
+        ASSERT_EQUALS(ErrorCodes::UserNotFound, commandStatus);
+    });
+
+    onCommand([](const RemoteCommandRequest& request) {
+        ASSERT_EQUALS("test", request.dbname);
+        ASSERT_EQUALS(BSON("dropUser"
+                           << "test"
+                           << "writeConcern"
+                           << BSON("w"
+                                   << "majority"
+                                   << "wtimeout"
+                                   << 30)
+                           << "maxTimeMS"
+                           << 30000),
+                      request.cmdObj);
+
+        ASSERT_EQUALS(BSON(rpc::kReplSetMetadataFieldName << 1), request.metadata);
+
+        BSONObjBuilder responseBuilder;
+        Command::appendCommandStatus(responseBuilder,
+                                     Status(ErrorCodes::UserNotFound, "User test@test not found"));
+        return responseBuilder.obj();
+    });
+
+    // Now wait for the runUserManagementWriteCommand call to return
+    future.timed_get(kFutureTimeout);
+}
+
 TEST_F(ShardingCatalogClientTest, RunUserManagementWriteCommandNotMaster) {
     configTargeter()->setFindHostReturnValue(HostAndPort("TestHost1"));