SERVER-55801 Fix Authorization Contract state tracking during setFCV

3 files changed, 19 insertions, 1 deletions

diff --git a/src/mongo/db/auth/authorization_session_impl.cpp b/src/mongo/db/auth/authorization_session_impl.cpp
index be0346d38cb..86a125746c9 100644
--- a/src/mongo/db/auth/authorization_session_impl.cpp
+++ b/src/mongo/db/auth/authorization_session_impl.cpp
@@ -94,7 +94,7 @@ bool checkContracts() {
 
 AuthorizationSessionImpl::AuthorizationSessionImpl(
     std::unique_ptr<AuthzSessionExternalState> externalState, InstallMockForTestingOrAuthImpl)
-    : _externalState(std::move(externalState)), _impersonationFlag(false) {}
+    : _externalState(std::move(externalState)), _impersonationFlag(false), _checkContracts(false) {}
 
 AuthorizationSessionImpl::~AuthorizationSessionImpl() {
     invariant(_authenticatedUsers.count() == 0,
@@ -112,9 +112,11 @@ void AuthorizationSessionImpl::startRequest(OperationContext* opCtx) {
 
 void AuthorizationSessionImpl::startContractTracking() {
     if (!checkContracts()) {
+        _checkContracts = false;
         return;
     }
 
+    _checkContracts = true;
     _contract.clear();
 }
 
@@ -856,6 +858,11 @@ void AuthorizationSessionImpl::verifyContract(const AuthorizationContract* contr
         return;
     }
 
+    // Do not check a contract if we decided earlier not to clear the contract tracking state.
+    if (!_checkContracts) {
+        return;
+    }
+
     // Make a mutable copy so that the common auth checks can be added.
     auto tempContract = *contract;
 
diff --git a/src/mongo/db/auth/authorization_session_impl.h b/src/mongo/db/auth/authorization_session_impl.h
index 8887a06fc4b..19c90b97f74 100644
--- a/src/mongo/db/auth/authorization_session_impl.h
+++ b/src/mongo/db/auth/authorization_session_impl.h
@@ -202,5 +202,12 @@ private:
     // of authorization checks they perform. After a command completes running, MongoDB verifies the
     // set of checks performed is a subset of the checks declared in the contract.
     AuthorizationContract _contract;
+
+    // Contract checking is feature guarded. As such we may decide at the start of command to not
+    // track it but reach a different decision after the command has been run because the FCV has
+    // changed. We must record our first decision.
+    //
+    // TODO SERVER-52364 - remove this variable after the feature flag is removed.
+    bool _checkContracts;
 };
 }  // namespace mongo
diff --git a/src/mongo/db/auth/authorization_session_test.cpp b/src/mongo/db/auth/authorization_session_test.cpp
index 689f3642e9e..1c959118caa 100644
--- a/src/mongo/db/auth/authorization_session_test.cpp
+++ b/src/mongo/db/auth/authorization_session_test.cpp
@@ -159,6 +159,8 @@ const ResourcePattern thirdProfileCollResource(
     ResourcePattern::forExactNamespace(NamespaceString("third.system.profile")));
 
 TEST_F(AuthorizationSessionTest, AddUserAndCheckAuthorization) {
+    authzSession->startContractTracking();
+
     // Check that disabling auth checks works
     ASSERT_FALSE(
         authzSession->isAuthorizedForActionsOnResource(testFooCollResource, ActionType::insert));
@@ -1345,6 +1347,8 @@ TEST_F(AuthorizationSessionTest, CanUseUUIDNamespacesWithPrivilege) {
     BSONObj uuidObj = BSON("a" << UUID::gen());
     BSONObj invalidObj = BSON("a" << 12);
 
+    authzSession->startContractTracking();
+
     // Strings require no privileges
     ASSERT_TRUE(authzSession->isAuthorizedToParseNamespaceElement(stringObj.firstElement()));