SERVER-54964 Require valid client for all audit events

8 files changed, 200 insertions, 82 deletions

diff --git a/src/mongo/db/audit.cpp b/src/mongo/db/audit.cpp
index 7d376722003..08b811dc3f2 100644
--- a/src/mongo/db/audit.cpp
+++ b/src/mongo/db/audit.cpp
@@ -36,37 +36,51 @@ namespace audit {
 
 ImpersonatedClientAttrs::ImpersonatedClientAttrs(Client* client) {}
 
-void logAuthentication(Client*, const AuthenticateEvent&) {}
+void logAuthentication(Client* client, const AuthenticateEvent&) {
+    invariant(client);
+}
 
 void logCommandAuthzCheck(Client* client,
                           const OpMsgRequest& cmdObj,
                           const CommandInterface& command,
-                          ErrorCodes::Error result) {}
+                          ErrorCodes::Error result) {
+    invariant(client);
+}
 
 void logDeleteAuthzCheck(Client* client,
                          const NamespaceString& ns,
                          const BSONObj& pattern,
-                         ErrorCodes::Error result) {}
+                         ErrorCodes::Error result) {
+    invariant(client);
+}
 
 void logGetMoreAuthzCheck(Client* client,
                           const NamespaceString& ns,
                           long long cursorId,
-                          ErrorCodes::Error result) {}
+                          ErrorCodes::Error result) {
+    invariant(client);
+}
 
 void logInsertAuthzCheck(Client* client,
                          const NamespaceString& ns,
                          const BSONObj& insertedObj,
-                         ErrorCodes::Error result) {}
+                         ErrorCodes::Error result) {
+    invariant(client);
+}
 
 void logKillCursorsAuthzCheck(Client* client,
                               const NamespaceString& ns,
                               long long cursorId,
-                              ErrorCodes::Error result) {}
+                              ErrorCodes::Error result) {
+    invariant(client);
+}
 
 void logQueryAuthzCheck(Client* client,
                         const NamespaceString& ns,
                         const BSONObj& query,
-                        ErrorCodes::Error result) {}
+                        ErrorCodes::Error result) {
+    invariant(client);
+}
 
 void logUpdateAuthzCheck(Client* client,
                          const NamespaceString& ns,
@@ -74,129 +88,203 @@ void logUpdateAuthzCheck(Client* client,
                          const write_ops::UpdateModification& update,
                          bool isUpsert,
                          bool isMulti,
-                         ErrorCodes::Error result) {}
+                         ErrorCodes::Error result) {
+    invariant(client);
+}
 
 void logCreateUser(Client* client,
                    const UserName& username,
                    bool password,
                    const BSONObj* customData,
                    const std::vector<RoleName>& roles,
-                   const boost::optional<BSONArray>& restrictions) {}
+                   const boost::optional<BSONArray>& restrictions) {
+    invariant(client);
+}
 
-void logDropUser(Client* client, const UserName& username) {}
+void logDropUser(Client* client, const UserName& username) {
+    invariant(client);
+}
 
-void logDropAllUsersFromDatabase(Client* client, StringData dbname) {}
+void logDropAllUsersFromDatabase(Client* client, StringData dbname) {
+    invariant(client);
+}
 
 void logUpdateUser(Client* client,
                    const UserName& username,
                    bool password,
                    const BSONObj* customData,
                    const std::vector<RoleName>* roles,
-                   const boost::optional<BSONArray>& restrictions) {}
+                   const boost::optional<BSONArray>& restrictions) {
+    invariant(client);
+}
 
 void logGrantRolesToUser(Client* client,
                          const UserName& username,
-                         const std::vector<RoleName>& roles) {}
+                         const std::vector<RoleName>& roles) {
+    invariant(client);
+}
 
 void logRevokeRolesFromUser(Client* client,
                             const UserName& username,
-                            const std::vector<RoleName>& roles) {}
+                            const std::vector<RoleName>& roles) {
+    invariant(client);
+}
 
 void logCreateRole(Client* client,
                    const RoleName& role,
                    const std::vector<RoleName>& roles,
                    const PrivilegeVector& privileges,
-                   const boost::optional<BSONArray>& restrictions) {}
+                   const boost::optional<BSONArray>& restrictions) {
+    invariant(client);
+}
 
 void logUpdateRole(Client* client,
                    const RoleName& role,
                    const std::vector<RoleName>* roles,
                    const PrivilegeVector* privileges,
-                   const boost::optional<BSONArray>& restrictions) {}
+                   const boost::optional<BSONArray>& restrictions) {
+    invariant(client);
+}
 
-void logDropRole(Client* client, const RoleName& role) {}
+void logDropRole(Client* client, const RoleName& role) {
+    invariant(client);
+}
 
-void logDropAllRolesFromDatabase(Client* client, StringData dbname) {}
+void logDropAllRolesFromDatabase(Client* client, StringData dbname) {
+    invariant(client);
+}
 
 void logGrantRolesToRole(Client* client, const RoleName& role, const std::vector<RoleName>& roles) {
 }
 
 void logRevokeRolesFromRole(Client* client,
                             const RoleName& role,
-                            const std::vector<RoleName>& roles) {}
+                            const std::vector<RoleName>& roles) {
+    invariant(client);
+}
 
 void logGrantPrivilegesToRole(Client* client,
                               const RoleName& role,
-                              const PrivilegeVector& privileges) {}
+                              const PrivilegeVector& privileges) {
+    invariant(client);
+}
 
 void logRevokePrivilegesFromRole(Client* client,
                                  const RoleName& role,
-                                 const PrivilegeVector& privileges) {}
+                                 const PrivilegeVector& privileges) {
+    invariant(client);
+}
 
-void logReplSetReconfig(Client* client, const BSONObj* oldConfig, const BSONObj* newConfig) {}
+void logReplSetReconfig(Client* client, const BSONObj* oldConfig, const BSONObj* newConfig) {
+    invariant(client);
+}
 
-void logApplicationMessage(Client* client, StringData msg) {}
+void logApplicationMessage(Client* client, StringData msg) {
+    invariant(client);
+}
 
-void logStartupOptions(Client* client, const BSONObj& startupOptions) {}
+void logStartupOptions(Client* client, const BSONObj& startupOptions) {
+    invariant(client);
+}
 
-void logShutdown(Client* client) {}
+void logShutdown(Client* client) {
+    invariant(client);
+}
 
 void logLogout(Client* client,
                StringData reason,
                const BSONArray& initialUsers,
-               const BSONArray& updatedUsers) {}
+               const BSONArray& updatedUsers) {
+    invariant(client);
+}
 
 void logCreateIndex(Client* client,
                     const BSONObj* indexSpec,
                     StringData indexname,
                     const NamespaceString& nsname,
                     StringData indexBuildState,
-                    ErrorCodes::Error result) {}
+                    ErrorCodes::Error result) {
+    invariant(client);
+}
 
-void logCreateCollection(Client* client, const NamespaceString& nsname) {}
+void logCreateCollection(Client* client, const NamespaceString& nsname) {
+    invariant(client);
+}
 
 void logCreateView(Client* client,
                    const NamespaceString& nsname,
                    StringData viewOn,
                    BSONArray pipeline,
-                   ErrorCodes::Error code) {}
+                   ErrorCodes::Error code) {
+    invariant(client);
+}
 
-void logImportCollection(Client* client, const NamespaceString& nsname) {}
+void logImportCollection(Client* client, const NamespaceString& nsname) {
+    invariant(client);
+}
 
-void logCreateDatabase(Client* client, StringData dbname) {}
+void logCreateDatabase(Client* client, StringData dbname) {
+    invariant(client);
+}
 
 
-void logDropIndex(Client* client, StringData indexname, const NamespaceString& nsname) {}
+void logDropIndex(Client* client, StringData indexname, const NamespaceString& nsname) {
+    invariant(client);
+}
 
-void logDropCollection(Client* client, const NamespaceString& nsname) {}
+void logDropCollection(Client* client, const NamespaceString& nsname) {
+    invariant(client);
+}
 
 void logDropView(Client* client,
                  const NamespaceString& nsname,
                  StringData viewOn,
                  const std::vector<BSONObj>& pipeline,
-                 ErrorCodes::Error code) {}
+                 ErrorCodes::Error code) {
+    invariant(client);
+}
 
-void logDropDatabase(Client* client, StringData dbname) {}
+void logDropDatabase(Client* client, StringData dbname) {
+    invariant(client);
+}
 
 void logRenameCollection(Client* client,
                          const NamespaceString& source,
-                         const NamespaceString& target) {}
+                         const NamespaceString& target) {
+    invariant(client);
+}
 
-void logEnableSharding(Client* client, StringData dbname) {}
+void logEnableSharding(Client* client, StringData dbname) {
+    invariant(client);
+}
 
-void logAddShard(Client* client, StringData name, const std::string& servers, long long maxSize) {}
+void logAddShard(Client* client, StringData name, const std::string& servers, long long maxSize) {
+    invariant(client);
+}
 
-void logRemoveShard(Client* client, StringData shardname) {}
+void logRemoveShard(Client* client, StringData shardname) {
+    invariant(client);
+}
 
-void logShardCollection(Client* client, StringData ns, const BSONObj& keyPattern, bool unique) {}
+void logShardCollection(Client* client, StringData ns, const BSONObj& keyPattern, bool unique) {
+    invariant(client);
+}
 
-void logRefineCollectionShardKey(Client* client, StringData ns, const BSONObj& keyPattern) {}
+void logRefineCollectionShardKey(Client* client, StringData ns, const BSONObj& keyPattern) {
+    invariant(client);
+}
 
-void logInsertOperation(Client* client, const NamespaceString& nss, const BSONObj& doc) {}
+void logInsertOperation(Client* client, const NamespaceString& nss, const BSONObj& doc) {
+    invariant(client);
+}
 
-void logUpdateOperation(Client* client, const NamespaceString& nss, const BSONObj& doc) {}
+void logUpdateOperation(Client* client, const NamespaceString& nss, const BSONObj& doc) {
+    invariant(client);
+}
 
-void logRemoveOperation(Client* client, const NamespaceString& nss, const BSONObj& doc) {}
+void logRemoveOperation(Client* client, const NamespaceString& nss, const BSONObj& doc) {
+    invariant(client);
+}
 
 #endif
 
diff --git a/src/mongo/db/auth/auth_decorations.cpp b/src/mongo/db/auth/auth_decorations.cpp
index a3b5059e380..9dc6ccf868e 100644
--- a/src/mongo/db/auth/auth_decorations.cpp
+++ b/src/mongo/db/auth/auth_decorations.cpp
@@ -64,7 +64,13 @@ public:
         }
     }
 
-    void onDestroyClient(Client* client) override {}
+    void onDestroyClient(Client* client) override {
+        // Logout before the client is destroyed.
+        auto& authzSession = getAuthorizationSession(client);
+        if (authzSession) {
+            authzSession->logoutAllDatabases(client, "Client has disconnected");
+        }
+    }
 
     void onCreateOperationContext(OperationContext* opCtx) override {}
     void onDestroyOperationContext(OperationContext* opCtx) override {}
diff --git a/src/mongo/db/auth/authorization_session.h b/src/mongo/db/auth/authorization_session.h
index a62f70b29a1..08892418c59 100644
--- a/src/mongo/db/auth/authorization_session.h
+++ b/src/mongo/db/auth/authorization_session.h
@@ -163,10 +163,14 @@ public:
     // Gets an iterator over the roles of all authenticated users stored in this manager.
     virtual RoleNameIterator getAuthenticatedRoleNames() = 0;
 
+    // Removes any authenticated principals and revokes any privileges that were granted via those
+    // principals. This function modifies state. Synchronizes with the Client lock.
+    virtual void logoutAllDatabases(Client* client, StringData reason) = 0;
+
     // Removes any authenticated principals whose authorization credentials came from the given
     // database, and revokes any privileges that were granted via that principal. This function
     // modifies state. Synchronizes with the Client lock.
-    virtual void logoutDatabase(OperationContext* opCtx, StringData dbname) = 0;
+    virtual void logoutDatabase(Client* client, StringData dbname, StringData reason) = 0;
 
     // Adds the internalSecurity user to the set of authenticated users.
     // Used to grant internal threads full access. Takes in the Client
diff --git a/src/mongo/db/auth/authorization_session_impl.cpp b/src/mongo/db/auth/authorization_session_impl.cpp
index 13175b74549..a6f09b04ff7 100644
--- a/src/mongo/db/auth/authorization_session_impl.cpp
+++ b/src/mongo/db/auth/authorization_session_impl.cpp
@@ -77,14 +77,8 @@ AuthorizationSessionImpl::AuthorizationSessionImpl(
     : _externalState(std::move(externalState)), _impersonationFlag(false) {}
 
 AuthorizationSessionImpl::~AuthorizationSessionImpl() {
-    // Emit logout audit event. Since the AuthorizationSessionImpl is being destroyed, there will
-    // be no users remaining after this event.
-    if (_authenticatedUsers.count() > 0) {
-        audit::logLogout(Client::getCurrent(),
-                         "Implicit logout due to client connection closure",
-                         _authenticatedUsers.toBSON(),
-                         BSONArray());
-    }
+    invariant(_authenticatedUsers.count() == 0,
+              "All authenticated users should be logged out by the Client destruction hook");
 }
 
 AuthorizationManager& AuthorizationSessionImpl::getAuthorizationManager() {
@@ -149,17 +143,31 @@ User* AuthorizationSessionImpl::getSingleUser() {
     return lookupUser(userName);
 }
 
-void AuthorizationSessionImpl::logoutDatabase(OperationContext* opCtx, StringData dbname) {
-    stdx::lock_guard<Client> lk(*opCtx->getClient());
+void AuthorizationSessionImpl::logoutAllDatabases(Client* client, StringData reason) {
+    stdx::lock_guard<Client> lk(*client);
+
+    auto users = std::exchange(_authenticatedUsers, {});
+    if (users.count() == 0) {
+        return;
+    }
+
+    audit::logLogout(client, reason, users.toBSON(), BSONArray());
+
+    clearImpersonatedUserData();
+    _buildAuthenticatedRolesVector();
+}
+
+
+void AuthorizationSessionImpl::logoutDatabase(Client* client,
+                                              StringData dbname,
+                                              StringData reason) {
+    stdx::lock_guard<Client> lk(*client);
 
     // Emit logout audit event and then remove all users logged into dbname.
     UserSet updatedUsers(_authenticatedUsers);
     updatedUsers.removeByDBName(dbname);
     if (updatedUsers.count() != _authenticatedUsers.count()) {
-        audit::logLogout(opCtx->getClient(),
-                         str::stream() << "Explicit logout from db '" << dbname << "'",
-                         _authenticatedUsers.toBSON(),
-                         updatedUsers.toBSON());
+        audit::logLogout(client, reason, _authenticatedUsers.toBSON(), updatedUsers.toBSON());
     }
     std::swap(_authenticatedUsers, updatedUsers);
 
diff --git a/src/mongo/db/auth/authorization_session_impl.h b/src/mongo/db/auth/authorization_session_impl.h
index 598c05d5239..fa1848680e9 100644
--- a/src/mongo/db/auth/authorization_session_impl.h
+++ b/src/mongo/db/auth/authorization_session_impl.h
@@ -88,7 +88,8 @@ public:
 
     RoleNameIterator getAuthenticatedRoleNames() override;
 
-    void logoutDatabase(OperationContext* opCtx, StringData dbname) override;
+    void logoutAllDatabases(Client* client, StringData reason) override;
+    void logoutDatabase(Client* client, StringData dbname, StringData reason) override;
 
     void grantInternalAuthorization(Client* client) override;
 
diff --git a/src/mongo/db/auth/authorization_session_test.cpp b/src/mongo/db/auth/authorization_session_test.cpp
index c983ba68313..50f348d991f 100644
--- a/src/mongo/db/auth/authorization_session_test.cpp
+++ b/src/mongo/db/auth/authorization_session_test.cpp
@@ -83,22 +83,12 @@ private:
 
 class AuthorizationSessionTest : public ScopedGlobalServiceContextForTest, public unittest::Test {
 public:
-    FailureCapableAuthzManagerExternalStateMock* managerState;
-    transport::TransportLayerMock transportLayer;
-    transport::SessionHandle session;
-    ServiceContext::UniqueClient client;
-    ServiceContext::UniqueOperationContext _opCtx;
-    AuthzSessionExternalStateMock* sessionState;
-    AuthorizationManager* authzManager;
-    std::unique_ptr<AuthorizationSessionForTest> authzSession;
-    BSONObj credentials;
-
     void setUp() {
-        session = transportLayer.createSession();
-        client = getServiceContext()->makeClient("testClient", session);
+        _session = transportLayer.createSession();
+        _client = getServiceContext()->makeClient("testClient", _session);
         RestrictionEnvironment::set(
-            session, std::make_unique<RestrictionEnvironment>(SockAddr(), SockAddr()));
-        _opCtx = client->makeOperationContext();
+            _session, std::make_unique<RestrictionEnvironment>(SockAddr(), SockAddr()));
+        _opCtx = _client->makeOperationContext();
         auto localManagerState = std::make_unique<FailureCapableAuthzManagerExternalStateMock>();
         managerState = localManagerState.get();
         managerState->setAuthzVersion(AuthorizationManager::schemaVersion26Final);
@@ -120,6 +110,21 @@ public:
                                << scram::Secrets<SHA256Block>::generateCredentials(
                                       "a", saslGlobalParams.scramSHA256IterationCount.load()));
     }
+
+    void tearDown() override {
+        authzSession->logoutAllDatabases(_client.get(), "Ending AuthorizationSessionTest");
+    }
+
+protected:
+    FailureCapableAuthzManagerExternalStateMock* managerState;
+    transport::TransportLayerMock transportLayer;
+    transport::SessionHandle _session;
+    ServiceContext::UniqueClient _client;
+    ServiceContext::UniqueOperationContext _opCtx;
+    AuthzSessionExternalStateMock* sessionState;
+    AuthorizationManager* authzManager;
+    std::unique_ptr<AuthorizationSessionForTest> authzSession;
+    BSONObj credentials;
 };
 
 const NamespaceString testFooNss("test.foo");
@@ -218,7 +223,7 @@ TEST_F(AuthorizationSessionTest, AddUserAndCheckAuthorization) {
     ASSERT_TRUE(
         authzSession->isAuthorizedForActionsOnResource(testFooCollResource, ActionType::insert));
 
-    authzSession->logoutDatabase(_opCtx.get(), "test");
+    authzSession->logoutDatabase(_client.get(), "test", "Kill the test!");
     ASSERT_TRUE(
         authzSession->isAuthorizedForActionsOnResource(otherFooCollResource, ActionType::insert));
     ASSERT_TRUE(
@@ -226,7 +231,7 @@ TEST_F(AuthorizationSessionTest, AddUserAndCheckAuthorization) {
     ASSERT_FALSE(
         authzSession->isAuthorizedForActionsOnResource(testFooCollResource, ActionType::collMod));
 
-    authzSession->logoutDatabase(_opCtx.get(), "admin");
+    authzSession->logoutDatabase(_client.get(), "admin", "Fire the admin!");
     ASSERT_FALSE(
         authzSession->isAuthorizedForActionsOnResource(otherFooCollResource, ActionType::insert));
     ASSERT_FALSE(
@@ -542,7 +547,7 @@ TEST_F(AuthorizationSessionTest, AcquireUserObtainsAndValidatesAuthenticationRes
 
     auto assertWorks = [this](StringData clientSource, StringData serverAddress) {
         RestrictionEnvironment::set(
-            session,
+            _session,
             std::make_unique<RestrictionEnvironment>(SockAddr(clientSource, 5555, AF_UNSPEC),
                                                      SockAddr(serverAddress, 27017, AF_UNSPEC)));
         ASSERT_OK(authzSession->addAndAuthorizeUser(_opCtx.get(), UserName("spencer", "test")));
@@ -550,7 +555,7 @@ TEST_F(AuthorizationSessionTest, AcquireUserObtainsAndValidatesAuthenticationRes
 
     auto assertFails = [this](StringData clientSource, StringData serverAddress) {
         RestrictionEnvironment::set(
-            session,
+            _session,
             std::make_unique<RestrictionEnvironment>(SockAddr(clientSource, 5555, AF_UNSPEC),
                                                      SockAddr(serverAddress, 27017, AF_UNSPEC)));
         ASSERT_NOT_OK(authzSession->addAndAuthorizeUser(_opCtx.get(), UserName("spencer", "test")));
diff --git a/src/mongo/db/commands/authentication_commands.cpp b/src/mongo/db/commands/authentication_commands.cpp
index 0f5b5b7a546..00c455021b1 100644
--- a/src/mongo/db/commands/authentication_commands.cpp
+++ b/src/mongo/db/commands/authentication_commands.cpp
@@ -171,14 +171,16 @@ public:
             auto dbname = request().getDbName();
             auto* as = AuthorizationSession::get(opCtx->getClient());
 
-            as->logoutDatabase(opCtx, dbname);
+            as->logoutDatabase(opCtx->getClient(), dbname, "Logging out on user request");
             if (getTestCommandsEnabled() && (dbname == kAdminDB)) {
                 // Allows logging out as the internal user against the admin database, however
                 // this actually logs out of the local database as well. This is to
                 // support the auth passthrough test framework on mongos (since you can't use the
                 // local database on a mongos, so you can't logout as the internal user
                 // without this).
-                as->logoutDatabase(opCtx, kLocalDB);
+                as->logoutDatabase(opCtx->getClient(),
+                                   kLocalDB,
+                                   "Logging out from local database for test purposes");
             }
         }
     };
diff --git a/src/mongo/embedded/embedded_auth_session.cpp b/src/mongo/embedded/embedded_auth_session.cpp
index 17cdb5a3610..596866407ff 100644
--- a/src/mongo/embedded/embedded_auth_session.cpp
+++ b/src/mongo/embedded/embedded_auth_session.cpp
@@ -106,7 +106,11 @@ public:
         // Always okay to do something, on embedded.
     }
 
-    void logoutDatabase(OperationContext* opCtx, const StringData) override {
+    void logoutAllDatabases(Client*, StringData) override {
+        // Since we didn't actively authorize, we do not actively deauthorize.
+    }
+
+    void logoutDatabase(Client*, StringData, StringData) override {
         UASSERT_NOT_IMPLEMENTED;
     }