SERVER-56550 Require consistent API params in getMore and txns

Transaction-continuing commands must use the same API parameters as the
transaction's first command (it is no longer optional), and similarly
getMore must use the same as the cursor-creating command.

13 files changed, 88 insertions, 97 deletions

diff --git a/src/mongo/db/commands/getmore_cmd.cpp b/src/mongo/db/commands/getmore_cmd.cpp
index 9624761a5e9..1772d5b3a25 100644
--- a/src/mongo/db/commands/getmore_cmd.cpp
+++ b/src/mongo/db/commands/getmore_cmd.cpp
@@ -225,18 +225,11 @@ void setUpOperationContextStateForGetMore(OperationContext* opCtx,
     opCtx->setWriteConcern(cursor.getWriteConcernOptions());
 
     auto apiParamsFromClient = APIParameters::get(opCtx);
-    // TODO (SERVER-56550): Do this check even if !apiParamsFromClient.getParamsPassed().
-    if (apiParamsFromClient.getParamsPassed()) {
-        uassert(
-            ErrorCodes::APIMismatchError,
-            "API param conflict: getMore used params {}, the cursor-creating command used {}"_format(
-                apiParamsFromClient.toBSON().toString(),
-                cursor.getAPIParameters().toBSON().toString()),
-            apiParamsFromClient == cursor.getAPIParameters());
-    }
-
-    // TODO (SERVER-56550): Remove.
-    APIParameters::get(opCtx) = cursor.getAPIParameters();
+    uassert(
+        ErrorCodes::APIMismatchError,
+        "API parameter mismatch: getMore used params {}, the cursor-creating command used {}"_format(
+            apiParamsFromClient.toBSON().toString(), cursor.getAPIParameters().toBSON().toString()),
+        apiParamsFromClient == cursor.getAPIParameters());
 
     setUpOperationDeadline(opCtx, cursor, cmd, disableAwaitDataFailpointActive);
 
diff --git a/src/mongo/db/commands/user_management_commands.cpp b/src/mongo/db/commands/user_management_commands.cpp
index d8c7582ad16..9c42dec4997 100644
--- a/src/mongo/db/commands/user_management_commands.cpp
+++ b/src/mongo/db/commands/user_management_commands.cpp
@@ -885,10 +885,8 @@ private:
             _sessionInfo.serialize(cmdBuilder);
         }
 
-        if (_state == TransactionState::kInit || !_isReplSet) {
-            // Set a default apiVersion for all UMC commands
-            cmdBuilder->append("apiVersion", kOne);
-        }
+        // Set a default apiVersion for all UMC commands
+        cmdBuilder->append("apiVersion", kOne);
 
         auto svcCtx = _client->getServiceContext();
         auto sep = svcCtx->getServiceEntryPoint();
diff --git a/src/mongo/db/initialize_api_parameters.cpp b/src/mongo/db/initialize_api_parameters.cpp
index 3962494fae6..812a452ea31 100644
--- a/src/mongo/db/initialize_api_parameters.cpp
+++ b/src/mongo/db/initialize_api_parameters.cpp
@@ -107,9 +107,7 @@ void enforceRequireAPIVersion(OperationContext* opCtx, Command* command) {
     auto isInternalClient =
         !client->session() || (client->session()->getTags() & transport::Session::kInternalClient);
 
-    // TODO (SERVER-56550): Don't excuse getMore or transaction-continuing commands.
-    if (gRequireApiVersion.load() && !opCtx->getClient()->isInDirectClient() && !isInternalClient &&
-        command->getName() != "getMore" && !opCtx->isContinuingMultiDocumentTransaction()) {
+    if (gRequireApiVersion.load() && !opCtx->getClient()->isInDirectClient() && !isInternalClient) {
         uassert(
             498870,
             "The apiVersion parameter is required, please configure your MongoClient's API version",
diff --git a/src/mongo/db/s/transaction_coordinator.cpp b/src/mongo/db/s/transaction_coordinator.cpp
index a551447964a..35d7f5f00f8 100644
--- a/src/mongo/db/s/transaction_coordinator.cpp
+++ b/src/mongo/db/s/transaction_coordinator.cpp
@@ -177,7 +177,7 @@ TransactionCoordinator::TransactionCoordinator(OperationContext* operationContex
                 std::move(opTime));
         })
         .thenRunOn(Grid::get(_serviceContext)->getExecutorPool()->getFixedExecutor())
-        .then([this] {
+        .then([this, apiParams] {
             {
                 stdx::lock_guard<Latch> lg(_mutex);
                 _participantsDurable = true;
@@ -204,8 +204,12 @@ TransactionCoordinator::TransactionCoordinator(OperationContext* operationContex
                     return Future<void>::makeReady();
             }
 
-            return txn::sendPrepare(
-                       _serviceContext, *_sendPrepareScheduler, _lsid, _txnNumber, *_participants)
+            return txn::sendPrepare(_serviceContext,
+                                    *_sendPrepareScheduler,
+                                    _lsid,
+                                    _txnNumber,
+                                    apiParams,
+                                    *_participants)
                 .then([this](PrepareVoteConsensus consensus) mutable {
                     {
                         stdx::lock_guard<Latch> lg(_mutex);
diff --git a/src/mongo/db/s/transaction_coordinator_test.cpp b/src/mongo/db/s/transaction_coordinator_test.cpp
index 42a4da83b4c..5b060c8d968 100644
--- a/src/mongo/db/s/transaction_coordinator_test.cpp
+++ b/src/mongo/db/s/transaction_coordinator_test.cpp
@@ -409,7 +409,8 @@ TEST_F(TransactionCoordinatorDriverTest,
 TEST_F(TransactionCoordinatorDriverTest,
        SendPrepareReturnsAbortDecisionWhenFirstParticipantVotesAbortAndSecondVotesCommit) {
     txn::AsyncWorkScheduler aws(getServiceContext());
-    auto future = txn::sendPrepare(getServiceContext(), aws, _lsid, _txnNumber, kTwoShardIdList);
+    auto future = txn::sendPrepare(
+        getServiceContext(), aws, _lsid, _txnNumber, APIParameters(), kTwoShardIdList);
 
     onCommands({[&](const executor::RemoteCommandRequest& request) { return kNoSuchTransaction; },
                 [&](const executor::RemoteCommandRequest& request) { return kPrepareOk; }});
@@ -424,7 +425,8 @@ TEST_F(TransactionCoordinatorDriverTest,
 TEST_F(TransactionCoordinatorDriverTest,
        SendPrepareReturnsAbortDecisionWhenFirstParticipantVotesCommitAndSecondVotesAbort) {
     txn::AsyncWorkScheduler aws(getServiceContext());
-    auto future = txn::sendPrepare(getServiceContext(), aws, _lsid, _txnNumber, kTwoShardIdList);
+    auto future = txn::sendPrepare(
+        getServiceContext(), aws, _lsid, _txnNumber, APIParameters(), kTwoShardIdList);
 
     assertPrepareSentAndRespondWithSuccess();
     assertPrepareSentAndRespondWithNoSuchTransaction();
@@ -438,7 +440,8 @@ TEST_F(TransactionCoordinatorDriverTest,
 TEST_F(TransactionCoordinatorDriverTest,
        SendPrepareReturnsAbortDecisionWhenBothParticipantsVoteAbort) {
     txn::AsyncWorkScheduler aws(getServiceContext());
-    auto future = txn::sendPrepare(getServiceContext(), aws, _lsid, _txnNumber, kTwoShardIdList);
+    auto future = txn::sendPrepare(
+        getServiceContext(), aws, _lsid, _txnNumber, APIParameters(), kTwoShardIdList);
 
     onCommands({[&](const executor::RemoteCommandRequest& request) { return kNoSuchTransaction; },
                 [&](const executor::RemoteCommandRequest& request) { return kNoSuchTransaction; }});
@@ -455,7 +458,8 @@ TEST_F(TransactionCoordinatorDriverTest,
     const auto maxPrepareTimestamp = Timestamp(2, 1);
 
     txn::AsyncWorkScheduler aws(getServiceContext());
-    auto future = txn::sendPrepare(getServiceContext(), aws, _lsid, _txnNumber, kTwoShardIdList);
+    auto future = txn::sendPrepare(
+        getServiceContext(), aws, _lsid, _txnNumber, APIParameters(), kTwoShardIdList);
 
     assertPrepareSentAndRespondWithSuccess(firstPrepareTimestamp);
     assertPrepareSentAndRespondWithSuccess(maxPrepareTimestamp);
@@ -472,7 +476,8 @@ TEST_F(TransactionCoordinatorDriverTest,
     const auto maxPrepareTimestamp = Timestamp(2, 1);
 
     txn::AsyncWorkScheduler aws(getServiceContext());
-    auto future = txn::sendPrepare(getServiceContext(), aws, _lsid, _txnNumber, kTwoShardIdList);
+    auto future = txn::sendPrepare(
+        getServiceContext(), aws, _lsid, _txnNumber, APIParameters(), kTwoShardIdList);
 
     assertPrepareSentAndRespondWithSuccess(maxPrepareTimestamp);
     assertPrepareSentAndRespondWithSuccess(firstPrepareTimestamp);
@@ -489,7 +494,8 @@ TEST_F(TransactionCoordinatorDriverTest,
     const auto maxPrepareTimestamp = Timestamp(2, 1);
 
     txn::AsyncWorkScheduler aws(getServiceContext());
-    auto future = txn::sendPrepare(getServiceContext(), aws, _lsid, _txnNumber, kTwoShardIdList);
+    auto future = txn::sendPrepare(
+        getServiceContext(), aws, _lsid, _txnNumber, APIParameters(), kTwoShardIdList);
 
     assertPrepareSentAndRespondWithSuccess(firstPrepareTimestamp);
     assertPrepareSentAndRespondWithSuccess(maxPrepareTimestamp);
@@ -505,7 +511,8 @@ TEST_F(TransactionCoordinatorDriverTest,
     const auto timestamp = Timestamp(1, 1);
 
     txn::AsyncWorkScheduler aws(getServiceContext());
-    auto future = txn::sendPrepare(getServiceContext(), aws, _lsid, _txnNumber, kTwoShardIdList);
+    auto future = txn::sendPrepare(
+        getServiceContext(), aws, _lsid, _txnNumber, APIParameters(), kTwoShardIdList);
 
     assertPrepareSentAndRespondWithSuccess(timestamp);
     assertCommandSentAndRespondWith(
@@ -521,7 +528,8 @@ TEST_F(TransactionCoordinatorDriverTest,
 TEST_F(TransactionCoordinatorDriverTest,
        SendPrepareReturnsErrorWhenOneShardReturnsReadConcernMajorityNotEnabled) {
     txn::AsyncWorkScheduler aws(getServiceContext());
-    auto future = txn::sendPrepare(getServiceContext(), aws, _lsid, _txnNumber, kTwoShardIdList);
+    auto future = txn::sendPrepare(
+        getServiceContext(), aws, _lsid, _txnNumber, APIParameters(), kTwoShardIdList);
 
     assertPrepareSentAndRespondWithSuccess(Timestamp(100, 1));
     assertCommandSentAndRespondWith(
diff --git a/src/mongo/db/s/transaction_coordinator_util.cpp b/src/mongo/db/s/transaction_coordinator_util.cpp
index f8e7bbb42c9..0bc0d019c6c 100644
--- a/src/mongo/db/s/transaction_coordinator_util.cpp
+++ b/src/mongo/db/s/transaction_coordinator_util.cpp
@@ -233,12 +233,15 @@ Future<PrepareVoteConsensus> sendPrepare(ServiceContext* service,
                                          txn::AsyncWorkScheduler& scheduler,
                                          const LogicalSessionId& lsid,
                                          TxnNumber txnNumber,
+                                         const APIParameters& apiParams,
                                          const txn::ParticipantsList& participants) {
     PrepareTransaction prepareTransaction;
     prepareTransaction.setDbName(NamespaceString::kAdminDb);
-    auto prepareObj = prepareTransaction.toBSON(
-        BSON("lsid" << lsid.toBSON() << "txnNumber" << txnNumber << "autocommit" << false
-                    << WriteConcernOptions::kWriteConcernField << WriteConcernOptions::Majority));
+    BSONObjBuilder bob(BSON("lsid" << lsid.toBSON() << "txnNumber" << txnNumber << "autocommit"
+                                   << false << WriteConcernOptions::kWriteConcernField
+                                   << WriteConcernOptions::Majority));
+    apiParams.appendInfo(&bob);
+    auto prepareObj = prepareTransaction.toBSON(bob.obj());
 
     std::vector<Future<PrepareResponse>> responses;
 
diff --git a/src/mongo/db/s/transaction_coordinator_util.h b/src/mongo/db/s/transaction_coordinator_util.h
index 5563e5a29e0..36a2b2fcf3d 100644
--- a/src/mongo/db/s/transaction_coordinator_util.h
+++ b/src/mongo/db/s/transaction_coordinator_util.h
@@ -98,6 +98,7 @@ Future<PrepareVoteConsensus> sendPrepare(ServiceContext* service,
                                          txn::AsyncWorkScheduler& scheduler,
                                          const LogicalSessionId& lsid,
                                          TxnNumber txnNumber,
+                                         const APIParameters& apiParams,
                                          const txn::ParticipantsList& participants);
 
 /**
diff --git a/src/mongo/db/s/txn_two_phase_commit_cmds.cpp b/src/mongo/db/s/txn_two_phase_commit_cmds.cpp
index e8105484ca2..41d47497a77 100644
--- a/src/mongo/db/s/txn_two_phase_commit_cmds.cpp
+++ b/src/mongo/db/s/txn_two_phase_commit_cmds.cpp
@@ -51,6 +51,10 @@ MONGO_FAIL_POINT_DEFINE(participantReturnNetworkErrorForPrepareAfterExecutingPre
 
 class PrepareTransactionCmd : public TypedCommand<PrepareTransactionCmd> {
 public:
+    bool acceptsAnyApiVersionParameters() const override {
+        return true;
+    }
+
     class PrepareTimestamp {
     public:
         PrepareTimestamp(Timestamp timestamp) : _timestamp(std::move(timestamp)) {}
diff --git a/src/mongo/db/service_entry_point_common.cpp b/src/mongo/db/service_entry_point_common.cpp
index e60c89a3b65..d539a17b0d3 100644
--- a/src/mongo/db/service_entry_point_common.cpp
+++ b/src/mongo/db/service_entry_point_common.cpp
@@ -901,18 +901,15 @@ void CheckoutSessionAndInvokeCommand::_checkOutSession() {
     _sessionTxnState = std::make_unique<MongoDOperationContextSession>(opCtx);
     _txnParticipant.emplace(TransactionParticipant::get(opCtx));
 
-    // TODO (SERVER-56550): Do this check even if !apiParamsFromClient.getParamsPassed().
     auto apiParamsFromClient = APIParameters::get(opCtx);
-    if (apiParamsFromClient.getParamsPassed()) {
-        auto apiParamsFromTxn = _txnParticipant->getAPIParameters(opCtx);
-        uassert(
-            ErrorCodes::APIMismatchError,
-            "API param conflict: {} used params {}, the transaction's first command used {}"_format(
-                invocation->definition()->getName(),
-                apiParamsFromClient.toBSON().toString(),
-                apiParamsFromTxn.toBSON().toString()),
-            apiParamsFromTxn == apiParamsFromClient);
-    }
+    auto apiParamsFromTxn = _txnParticipant->getAPIParameters(opCtx);
+    uassert(
+        ErrorCodes::APIMismatchError,
+        "API parameter mismatch: {} used params {}, the transaction's first command used {}"_format(
+            invocation->definition()->getName(),
+            apiParamsFromClient.toBSON().toString(),
+            apiParamsFromTxn.toBSON().toString()),
+        apiParamsFromTxn == apiParamsFromClient);
 
     if (!opCtx->getClient()->isInDirectClient()) {
         bool beganOrContinuedTxn{false};
@@ -1000,9 +997,6 @@ void CheckoutSessionAndInvokeCommand::_checkOutSession() {
             }
         }
     }
-
-    // Use the API parameters that were stored when the transaction was initiated.
-    APIParameters::get(opCtx) = _txnParticipant->getAPIParameters(opCtx);
 }
 
 void CheckoutSessionAndInvokeCommand::_tapError(Status status) {
diff --git a/src/mongo/s/commands/strategy.cpp b/src/mongo/s/commands/strategy.cpp
index 4429987d9f3..e16e8c7340a 100644
--- a/src/mongo/s/commands/strategy.cpp
+++ b/src/mongo/s/commands/strategy.cpp
@@ -600,6 +600,8 @@ Status ParseAndRunCommand::RunInvocation::_setup() {
         ClientMetadata::setFromMetadata(opCtx->getClient(), metaElem);
     }
 
+    enforceRequireAPIVersion(opCtx, command);
+
     auto& apiParams = APIParameters::get(opCtx);
     auto& apiVersionMetrics = APIVersionMetrics::get(opCtx->getServiceContext());
     if (auto clientMetadata = ClientMetadata::get(opCtx->getClient())) {
@@ -814,9 +816,6 @@ Status ParseAndRunCommand::RunInvocation::_setup() {
     // the execution needs to adjust its behavior based on this.
     opCtx->setIsStartingMultiDocumentTransaction(startTransaction);
 
-    // Once API params and txn state are set on opCtx, enforce the "requireApiVersion" setting.
-    enforceRequireAPIVersion(opCtx, command);
-
     command->incrementCommandsExecuted();
 
     if (command->shouldAffectCommandCounter()) {
diff --git a/src/mongo/s/query/cluster_find.cpp b/src/mongo/s/query/cluster_find.cpp
index 4f9f6ddd446..9f2665d95ac 100644
--- a/src/mongo/s/query/cluster_find.cpp
+++ b/src/mongo/s/query/cluster_find.cpp
@@ -455,18 +455,11 @@ Status setUpOperationContextStateForGetMore(OperationContext* opCtx,
     }
 
     auto apiParamsFromClient = APIParameters::get(opCtx);
-    // TODO (SERVER-56550): Do this check even if !apiParamsFromClient.getParamsPassed().
-    if (apiParamsFromClient.getParamsPassed()) {
-        uassert(
-            ErrorCodes::APIMismatchError,
-            "API param conflict: getMore used params {}, the cursor-creating command used {}"_format(
-                apiParamsFromClient.toBSON().toString(),
-                cursor->getAPIParameters().toBSON().toString()),
+    uassert(ErrorCodes::APIMismatchError,
+            "API parameter mismatch: getMore used params {}, the cursor-creating command "
+            "used {}"_format(apiParamsFromClient.toBSON().toString(),
+                             cursor->getAPIParameters().toBSON().toString()),
             apiParamsFromClient == cursor->getAPIParameters());
-    }
-
-    // TODO (SERVER-56550): Remove.
-    APIParameters::get(opCtx) = cursor->getAPIParameters();
 
     // If the originating command had a 'comment' field, we extract it and set it on opCtx. Note
     // that if the 'getMore' command itself has a 'comment' field, we give precedence to it.
diff --git a/src/mongo/s/transaction_router.cpp b/src/mongo/s/transaction_router.cpp
index 57f5e636a13..160bce00191 100644
--- a/src/mongo/s/transaction_router.cpp
+++ b/src/mongo/s/transaction_router.cpp
@@ -31,6 +31,8 @@
 
 #include "mongo/platform/basic.h"
 
+#include <fmt/format.h>
+
 #include "mongo/s/transaction_router.h"
 
 #include "mongo/client/read_preference.h"
@@ -60,6 +62,8 @@
 namespace mongo {
 namespace {
 
+using namespace fmt::literals;
+
 // TODO SERVER-39704: Remove this fail point once the router can safely retry within a transaction
 // on stale version and snapshot errors.
 MONGO_FAIL_POINT_DEFINE(enableStaleVersionAndSnapshotRetriesWithinTransactions);
@@ -900,13 +904,13 @@ void TransactionRouter::Router::beginOrContinueTxn(OperationContext* opCtx,
     } else if (txnNumber == o().txnNumber) {
         // This is the same transaction as the one in progress.
         auto apiParamsFromClient = APIParameters::get(opCtx);
-        // TODO (SERVER-56550): Do this check even if !apiParamsFromClient.getParamsPassed().
-        if (apiParamsFromClient.getParamsPassed() &&
-            (action == TransactionActions::kContinue || action == TransactionActions::kCommit)) {
-            uassert(ErrorCodes::APIMismatchError,
-                    "A transaction-continuing command must use the same API parameters as "
-                    "the first command in the transaction",
-                    apiParamsFromClient == o().apiParameters);
+        if (action == TransactionActions::kContinue || action == TransactionActions::kCommit) {
+            uassert(
+                ErrorCodes::APIMismatchError,
+                "API parameter mismatch: transaction-continuing command used {}, the transaction's"
+                " first command used {}"_format(apiParamsFromClient.toBSON().toString(),
+                                                o().apiParameters.toBSON().toString()),
+                apiParamsFromClient == o().apiParameters);
         }
 
         switch (action) {
diff --git a/src/mongo/s/transaction_router_test.cpp b/src/mongo/s/transaction_router_test.cpp
index b1edf1164d8..4e286306a4d 100644
--- a/src/mongo/s/transaction_router_test.cpp
+++ b/src/mongo/s/transaction_router_test.cpp
@@ -719,7 +719,6 @@ TEST_F(TransactionRouterTestWithDefaultSession, AttachTxnValidatesReadConcernIfA
     }
 }
 
-// TODO (SERVER-56550): Test that API parameters are required in txn-continuing commands.
 TEST_F(TransactionRouterTestWithDefaultSession, SameAPIParametersAfterFirstStatement) {
     APIParameters apiParameters = APIParameters();
     apiParameters.setAPIVersion("1");
@@ -758,6 +757,27 @@ TEST_F(TransactionRouterTestWithDefaultSession, DifferentAPIParametersAfterFirst
         ErrorCodes::APIMismatchError);
 }
 
+TEST_F(TransactionRouterTestWithDefaultSession, NoAPIParametersAfterFirstStatement) {
+    APIParameters apiParameters = APIParameters();
+    apiParameters.setAPIVersion("1");
+    APIParameters::get(operationContext()) = apiParameters;
+    TxnNumber txnNum{3};
+
+    auto txnRouter = TransactionRouter::get(operationContext());
+    txnRouter.beginOrContinueTxn(
+        operationContext(), txnNum, TransactionRouter::TransactionActions::kStart);
+    txnRouter.setDefaultAtClusterTime(operationContext());
+
+    // Can't continue without params. (Must reset readConcern from "snapshot".)
+    APIParameters::get(operationContext()) = APIParameters();
+    repl::ReadConcernArgs::get(operationContext()) = repl::ReadConcernArgs();
+    ASSERT_THROWS_CODE(
+        txnRouter.beginOrContinueTxn(
+            operationContext(), txnNum, TransactionRouter::TransactionActions::kContinue),
+        DBException,
+        ErrorCodes::APIMismatchError);
+}
+
 TEST_F(TransactionRouterTestWithDefaultSession, CannotSpecifyReadConcernAfterFirstStatement) {
     TxnNumber txnNum{3};
 
@@ -2300,34 +2320,6 @@ TEST_F(TransactionRouterTestWithDefaultSession, ContinueOnlyOnStaleVersionOnFirs
     ASSERT_FALSE(txnRouter.canContinueOnStaleShardOrDbError("update", kStaleConfigStatus));
 }
 
-TEST_F(TransactionRouterTestWithDefaultSession,
-       ContinuingTransactionPlacesItsAPIParametersOnOpCtx) {
-    APIParameters apiParams = APIParameters();
-    apiParams.setAPIVersion("2");
-    apiParams.setAPIStrict(true);
-    apiParams.setAPIDeprecationErrors(true);
-
-    APIParameters::get(operationContext()) = apiParams;
-    repl::ReadConcernArgs::get(operationContext()) = repl::ReadConcernArgs();
-
-    TxnNumber txnNum{3};
-
-    auto txnRouter = TransactionRouter::get(operationContext());
-
-    txnRouter.beginOrContinueTxn(
-        operationContext(), txnNum, TransactionRouter::TransactionActions::kStart);
-    txnRouter.setDefaultAtClusterTime(operationContext());
-
-    APIParameters::get(operationContext()) = APIParameters();
-    txnRouter.beginOrContinueTxn(
-        operationContext(), txnNum, TransactionRouter::TransactionActions::kContinue);
-
-    auto storedAPIParams = APIParameters::get(operationContext());
-    ASSERT_EQ("2", *storedAPIParams.getAPIVersion());
-    ASSERT_TRUE(*storedAPIParams.getAPIStrict());
-    ASSERT_TRUE(*storedAPIParams.getAPIDeprecationErrors());
-}
-
 TEST_F(TransactionRouterTestWithDefaultSession, ContinuingTransactionPlacesItsReadConcernOnOpCtx) {
     TxnNumber txnNum{3};