SERVER-32640 migrate cursor kill audit into authorization check

8 files changed, 13 insertions, 31 deletions

diff --git a/src/mongo/db/commands/killcursors_cmd.cpp b/src/mongo/db/commands/killcursors_cmd.cpp
index 675b776352e..b7ddbffb75c 100644
--- a/src/mongo/db/commands/killcursors_cmd.cpp
+++ b/src/mongo/db/commands/killcursors_cmd.cpp
@@ -54,7 +54,7 @@ struct KillCursorsCmd {
         }
 
         auto cursorManager = CursorManager::get(opCtx);
-        return cursorManager->killCursor(opCtx, id, true /* shouldAudit */);
+        return cursorManager->killCursor(opCtx, id);
     }
 };
 KillCursorsCmdBase<KillCursorsCmd> cmdKillCursors;
diff --git a/src/mongo/db/commands/killcursors_common.h b/src/mongo/db/commands/killcursors_common.h
index da9f5cf6e5f..900038fad59 100644
--- a/src/mongo/db/commands/killcursors_common.h
+++ b/src/mongo/db/commands/killcursors_common.h
@@ -92,13 +92,13 @@ public:
             const auto& nss = killCursorsRequest.getNamespace();
             for (CursorId id : killCursorsRequest.getCursorIds()) {
                 auto status = Impl::doCheckAuth(opCtx, nss, id);
+                audit::logKillCursorsAuthzCheck(opCtx->getClient(), nss, id, status.code());
                 if (!status.isOK()) {
                     if (status.code() == ErrorCodes::CursorNotFound) {
                         // Not found isn't an authorization issue.
                         // run() will raise it as a return value.
                         continue;
                     }
-                    audit::logKillCursorsAuthzCheck(opCtx->getClient(), nss, id, status.code());
                     uassertStatusOK(status);  // throws
                 }
             }
@@ -120,9 +120,6 @@ public:
                 } else {
                     cursorsAlive.push_back(id);
                 }
-
-                audit::logKillCursorsAuthzCheck(
-                    opCtx->getClient(), killCursorsRequest.getNamespace(), id, status.code());
             }
 
             KillCursorsReply reply;
diff --git a/src/mongo/db/commands/killoperations_cmd.cpp b/src/mongo/db/commands/killoperations_cmd.cpp
index 0d5128f11f0..05a2d703342 100644
--- a/src/mongo/db/commands/killoperations_cmd.cpp
+++ b/src/mongo/db/commands/killoperations_cmd.cpp
@@ -40,7 +40,7 @@ public:
         auto cursorManager = CursorManager::get(opCtx);
         for (auto& cursorId : cursorManager->getCursorsForOpKeys(opKeys)) {
             LOGV2(4664802, "Attempting to kill cursor", "cursorId"_attr = cursorId);
-            auto status = cursorManager->killCursor(opCtx, cursorId, true /* shouldAudit */);
+            auto status = cursorManager->killCursor(opCtx, cursorId);
 
             if (!status.isOK()) {
                 LOGV2(
diff --git a/src/mongo/db/cursor_manager.cpp b/src/mongo/db/cursor_manager.cpp
index b5b72bfe2c2..d5dc80b445a 100644
--- a/src/mongo/db/cursor_manager.cpp
+++ b/src/mongo/db/cursor_manager.cpp
@@ -132,7 +132,7 @@ void CursorManager::set(ServiceContext* svcCtx, std::unique_ptr<CursorManager> n
 std::pair<Status, int> CursorManager::killCursorsWithMatchingSessions(
     OperationContext* opCtx, const SessionKiller::Matcher& matcher) {
     auto eraser = [&](CursorManager& mgr, CursorId id) {
-        uassertStatusOK(mgr.killCursor(opCtx, id, true));
+        uassertStatusOK(mgr.killCursor(opCtx, id));
         LOGV2(20528,
               "killing cursor: {id} as part of killing session(s)",
               "Killing cursor as part of killing session(s)",
@@ -446,13 +446,10 @@ void CursorManager::deregisterAndDestroyCursor(
     cursor->dispose(opCtx);
 }
 
-Status CursorManager::killCursor(OperationContext* opCtx, CursorId id, bool shouldAudit) {
+Status CursorManager::killCursor(OperationContext* opCtx, CursorId id) {
     auto lockedPartition = _cursorMap->lockOnePartition(id);
     auto it = lockedPartition->find(id);
     if (it == lockedPartition->end()) {
-        if (shouldAudit) {
-            audit::logKillCursorsAuthzCheck(opCtx->getClient(), {}, id, ErrorCodes::CursorNotFound);
-        }
         return {ErrorCodes::CursorNotFound, str::stream() << "Cursor id not found: " << id};
     }
     auto cursor = it->second;
@@ -466,18 +463,10 @@ Status CursorManager::killCursor(OperationContext* opCtx, CursorId id, bool shou
             cursor->_operationUsingCursor->getServiceContext()->killOperation(
                 lk, cursor->_operationUsingCursor, ErrorCodes::CursorKilled);
         }
-
-        if (shouldAudit) {
-            audit::logKillCursorsAuthzCheck(opCtx->getClient(), cursor->nss(), id, ErrorCodes::OK);
-        }
         return Status::OK();
     }
     std::unique_ptr<ClientCursor, ClientCursor::Deleter> ownedCursor(cursor);
 
-    if (shouldAudit) {
-        audit::logKillCursorsAuthzCheck(opCtx->getClient(), cursor->nss(), id, ErrorCodes::OK);
-    }
-
     deregisterAndDestroyCursor(std::move(lockedPartition), opCtx, std::move(ownedCursor));
     return Status::OK();
 }
diff --git a/src/mongo/db/cursor_manager.h b/src/mongo/db/cursor_manager.h
index c44729169bd..6746ba7dd49 100644
--- a/src/mongo/db/cursor_manager.h
+++ b/src/mongo/db/cursor_manager.h
@@ -140,7 +140,7 @@ public:
      *
      * If 'shouldAudit' is true, will perform audit logging.
      */
-    Status killCursor(OperationContext* opCtx, CursorId id, bool shouldAudit);
+    Status killCursor(OperationContext* opCtx, CursorId id);
 
     /**
      * Returns an OK status if we're authorized to erase the cursor. Otherwise, returns
diff --git a/src/mongo/db/run_op_kill_cursors.cpp b/src/mongo/db/run_op_kill_cursors.cpp
index 794a3a819e9..66a76301e43 100644
--- a/src/mongo/db/run_op_kill_cursors.cpp
+++ b/src/mongo/db/run_op_kill_cursors.cpp
@@ -78,7 +78,7 @@ bool killCursorIfAuthorized(OperationContext* opCtx, CursorId id) {
     // Release the pin so that the cursor can be killed.
     pin.getValue().release();
 
-    Status killStatus = cursorManager->killCursor(opCtx, id, true /* shouldAudit */);
+    Status killStatus = cursorManager->killCursor(opCtx, id);
     massert(28697,
             killStatus.reason(),
             killStatus.code() == ErrorCodes::OK || killStatus.code() == ErrorCodes::CursorNotFound);
diff --git a/src/mongo/db/service_entry_point_common.cpp b/src/mongo/db/service_entry_point_common.cpp
index 96bc89faeda..a4aef92897f 100644
--- a/src/mongo/db/service_entry_point_common.cpp
+++ b/src/mongo/db/service_entry_point_common.cpp
@@ -2074,9 +2074,7 @@ DbResponse receivedGetMore(OperationContext* opCtx,
             //
             // If killing the cursor fails, ignore the error and don't try again. The cursor
             // should be reaped by the client cursor timeout thread.
-            CursorManager::get(opCtx)
-                ->killCursor(opCtx, cursorid, false /* shouldAudit */)
-                .ignore();
+            CursorManager::get(opCtx)->killCursor(opCtx, cursorid).ignore();
         }
 
         BSONObjBuilder err;
diff --git a/src/mongo/dbtests/cursor_manager_test.cpp b/src/mongo/dbtests/cursor_manager_test.cpp
index 0796db802e2..cad5cfe1cfa 100644
--- a/src/mongo/dbtests/cursor_manager_test.cpp
+++ b/src/mongo/dbtests/cursor_manager_test.cpp
@@ -144,7 +144,6 @@ class CursorManagerTestCustomOpCtx : public CursorManagerTest {
  */
 TEST_F(CursorManagerTest, ShouldBeAbleToKillPinnedCursor) {
     CursorManager* cursorManager = useCursorManager();
-    const bool shouldAudit = false;
     OperationContext* const pinningOpCtx = _opCtx.get();
 
     auto cursorPin = cursorManager->registerCursor(
@@ -159,7 +158,7 @@ TEST_F(CursorManagerTest, ShouldBeAbleToKillPinnedCursor) {
          PrivilegeVector()});
 
     auto cursorId = cursorPin.getCursor()->cursorid();
-    ASSERT_OK(cursorManager->killCursor(_opCtx.get(), cursorId, shouldAudit));
+    ASSERT_OK(cursorManager->killCursor(_opCtx.get(), cursorId));
 
     // The original operation should have been interrupted since the cursor was pinned.
     ASSERT_EQ(pinningOpCtx->checkForInterruptNoAssert(), ErrorCodes::CursorKilled);
@@ -170,7 +169,6 @@ TEST_F(CursorManagerTest, ShouldBeAbleToKillPinnedCursor) {
  */
 TEST_F(CursorManagerTest, ShouldBeAbleToKillPinnedCursorMultiClient) {
     CursorManager* cursorManager = useCursorManager();
-    const bool shouldAudit = false;
     OperationContext* const pinningOpCtx = _opCtx.get();
 
     // Pin the cursor from one client.
@@ -199,7 +197,7 @@ TEST_F(CursorManagerTest, ShouldBeAbleToKillPinnedCursorMultiClient) {
 
     auto killCursorOpCtx = killCursorClient->makeOperationContext();
     invariant(killCursorOpCtx);
-    ASSERT_OK(cursorManager->killCursor(killCursorOpCtx.get(), cursorId, shouldAudit));
+    ASSERT_OK(cursorManager->killCursor(killCursorOpCtx.get(), cursorId));
 
     // The original operation should have been interrupted since the cursor was pinned.
     ASSERT_EQ(pinningOpCtx->checkForInterruptNoAssert(), ErrorCodes::CursorKilled);
@@ -511,7 +509,7 @@ TEST_F(CursorManagerTestCustomOpCtx, OneCursorWithASession) {
 
     // Remove the cursor from the manager.
     pinned.release();
-    ASSERT_OK(useCursorManager()->killCursor(opCtx.get(), cursorId, false));
+    ASSERT_OK(useCursorManager()->killCursor(opCtx.get(), cursorId));
 
     // There should be no more cursor entries by session id.
     LogicalSessionIdSet sessions;
@@ -547,7 +545,7 @@ TEST_F(CursorManagerTestCustomOpCtx, MultipleCursorsWithSameSession) {
 
     // Remove one cursor from the manager.
     pinned.release();
-    ASSERT_OK(useCursorManager()->killCursor(opCtx.get(), cursorId1, false));
+    ASSERT_OK(useCursorManager()->killCursor(opCtx.get(), cursorId1));
 
     // Should still be able to retrieve the session.
     lsids.clear();
@@ -649,7 +647,7 @@ TEST_F(CursorManagerTestCustomOpCtx, OneCursorWithAnOperationKey) {
 
     // Remove the cursor from the manager and verify that we can't retrieve it.
     pinned.release();
-    ASSERT_OK(useCursorManager()->killCursor(opCtx.get(), cursorId, false));
+    ASSERT_OK(useCursorManager()->killCursor(opCtx.get(), cursorId));
     ASSERT(useCursorManager()->getCursorsForOpKeys({opKey}).empty());
 }