Revert "SERVER-59148: Periodically refresh LDAP users in authorization user cache"

This reverts commit fc05532015895c8907437ea0c06fe83ab6c6f1dc.

10 files changed, 2 insertions, 252 deletions

diff --git a/src/mongo/db/auth/authorization_manager.h b/src/mongo/db/auth/authorization_manager.h
index 1459d72078c..0c92156d70f 100644
--- a/src/mongo/db/auth/authorization_manager.h
+++ b/src/mongo/db/auth/authorization_manager.h
@@ -320,13 +320,6 @@ public:
     virtual void invalidateUsersFromDB(OperationContext* opCtx, StringData dbname) = 0;
 
     /**
-     * Retrieves all users whose source is "$external" and checks if the corresponding user in the
-     * backing store has a different set of roles now. If so, it updates the cache entry with the
-     * new UserHandle.
-     */
-    virtual Status refreshExternalUsers(OperationContext* opCtx) = 0;
-
-    /**
      * Initializes the authorization manager.  Depending on what version the authorization
      * system is at, this may involve building up the user cache and/or the roles graph.
      * Call this function at startup and after resynchronizing a secondary.
diff --git a/src/mongo/db/auth/authorization_manager_impl.cpp b/src/mongo/db/auth/authorization_manager_impl.cpp
index 2804a04de40..5ddc068c1d2 100644
--- a/src/mongo/db/auth/authorization_manager_impl.cpp
+++ b/src/mongo/db/auth/authorization_manager_impl.cpp
@@ -667,39 +667,6 @@ void AuthorizationManagerImpl::invalidateUserCache(OperationContext* opCtx) {
     _userCache.invalidateAll();
 }
 
-Status AuthorizationManagerImpl::refreshExternalUsers(OperationContext* opCtx) {
-    LOGV2_DEBUG(5914801, 2, "Refreshing all users from the $external database");
-    // First, get a snapshot of the UserHandles in the cache.
-    std::vector<UserHandle> cachedUsers = _userCache.getValueHandlesIfKey(
-        [&](const UserRequest& userRequest) { return userRequest.name.getDB() == "$external"_sd; });
-
-    // Then, retrieve the corresponding Users from the backing store for users in the $external
-    // database. Compare each of these user objects with the cached user object and call
-    // insertOrAssign if they differ.
-    bool isRefreshed{false};
-    for (const auto& cachedUser : cachedUsers) {
-        UserRequest request(cachedUser->getName(), boost::none);
-        auto storedUserStatus = _externalState->getUserObject(opCtx, request);
-        if (!storedUserStatus.isOK()) {
-            return storedUserStatus.getStatus();
-        }
-
-        if (cachedUser->hasDifferentRoles(storedUserStatus.getValue())) {
-            _userCache.insertOrAssign(
-                request, std::move(storedUserStatus.getValue()), Date_t::now());
-            isRefreshed = true;
-        }
-    }
-
-    // If any entries were refreshed, then the cache generation must be bumped for mongos to refresh
-    // its cache.
-    if (isRefreshed) {
-        _updateCacheGeneration();
-    }
-
-    return Status::OK();
-}
-
 Status AuthorizationManagerImpl::initialize(OperationContext* opCtx) {
     Status status = _externalState->initialize(opCtx);
     if (!status.isOK())
diff --git a/src/mongo/db/auth/authorization_manager_impl.h b/src/mongo/db/auth/authorization_manager_impl.h
index a569a6f9723..6b6d059d790 100644
--- a/src/mongo/db/auth/authorization_manager_impl.h
+++ b/src/mongo/db/auth/authorization_manager_impl.h
@@ -107,12 +107,6 @@ public:
 
     void invalidateUsersFromDB(OperationContext* opCtx, StringData dbname) override;
 
-    /**
-     * Verify role information for users in the $external database and insert updated information
-     * into the cache if necessary. Currently, this is only used to refresh LDAP users.
-     */
-    Status refreshExternalUsers(OperationContext* opCtx) override;
-
     Status initialize(OperationContext* opCtx) override;
 
     /**
diff --git a/src/mongo/db/auth/authorization_manager_test.cpp b/src/mongo/db/auth/authorization_manager_test.cpp
index f5e5598af4b..001433e82f9 100644
--- a/src/mongo/db/auth/authorization_manager_test.cpp
+++ b/src/mongo/db/auth/authorization_manager_test.cpp
@@ -258,127 +258,5 @@ TEST_F(AuthorizationManagerTest, testAcquireV2UserWithUnrecognizedActions) {
     ASSERT(actions.empty());
 }
 
-TEST_F(AuthorizationManagerTest, testRefreshExternalV2User) {
-    constexpr auto kUserFieldName = "user"_sd;
-    constexpr auto kDbFieldName = "db"_sd;
-    constexpr auto kRoleFieldName = "role"_sd;
-
-    // Insert one user on db test and two users on db $external.
-    BSONObj externalCredentials = BSON("external" << true);
-    std::vector<BSONObj> userDocs{BSON("_id"
-                                       << "admin.v2read"
-                                       << "user"
-                                       << "v2read"
-                                       << "db"
-                                       << "test"
-                                       << "credentials" << credentials << "roles"
-                                       << BSON_ARRAY(BSON("role"
-                                                          << "read"
-                                                          << "db"
-                                                          << "test"))),
-                                  BSON("_id"
-                                       << "admin.v2externalOne"
-                                       << "user"
-                                       << "v2externalOne"
-                                       << "db"
-                                       << "$external"
-                                       << "credentials" << externalCredentials << "roles"
-                                       << BSON_ARRAY(BSON("role"
-                                                          << "read"
-                                                          << "db"
-                                                          << "test"))),
-                                  BSON("_id"
-                                       << "admin.v2externalTwo"
-                                       << "user"
-                                       << "v2externalTwo"
-                                       << "db"
-                                       << "$external"
-                                       << "credentials" << externalCredentials << "roles"
-                                       << BSON_ARRAY(BSON("role"
-                                                          << "read"
-                                                          << "db"
-                                                          << "test")))};
-
-    std::vector<BSONObj> initialRoles{BSON("role"
-                                           << "read"
-                                           << "db"
-                                           << "test")};
-    std::vector<BSONObj> updatedRoles{BSON("role"
-                                           << "readWrite"
-                                           << "db"
-                                           << "test")};
-
-    for (const auto& userDoc : userDocs) {
-        ASSERT_OK(externalState->insertPrivilegeDocument(opCtx.get(), userDoc, BSONObj()));
-    }
-
-    // Acquire these users to force the AuthorizationManager to load these users into the user
-    // cache.
-    for (const auto& userDoc : userDocs) {
-        auto swUser = authzManager->acquireUser(
-            opCtx.get(),
-            UserName(userDoc.getStringField(kUserFieldName), userDoc.getStringField(kDbFieldName)));
-        ASSERT_OK(swUser.getStatus());
-        auto user = std::move(swUser.getValue());
-        ASSERT_EQUALS(
-            UserName(userDoc.getStringField(kUserFieldName), userDoc.getStringField(kDbFieldName)),
-            user->getName());
-        ASSERT(user.isValid());
-
-        RoleNameIterator cachedUserRoles = user->getRoles();
-        for (const auto& userDocRole : initialRoles) {
-            ASSERT_EQUALS(cachedUserRoles.next(),
-                          RoleName(userDocRole.getStringField(kRoleFieldName),
-                                   userDocRole.getStringField(kDbFieldName)));
-        }
-        ASSERT_FALSE(cachedUserRoles.more());
-    }
-
-    // Update each of the users added into the external state so that they gain the readWrite role.
-    for (const auto& userDoc : userDocs) {
-        BSONObj updateQuery = BSON("user" << userDoc.getStringField(kUserFieldName));
-        ASSERT_OK(
-            externalState->updateOne(opCtx.get(),
-                                     AuthorizationManager::usersCollectionNamespace,
-                                     updateQuery,
-                                     BSON("$set" << BSON("roles" << BSON_ARRAY(updatedRoles[0]))),
-                                     true,
-                                     BSONObj()));
-    }
-
-    // Refresh all external entries in the authorization manager's cache.
-    ASSERT_OK(authzManager->refreshExternalUsers(opCtx.get()));
-
-    // Retrieve all users from the cache and verify that only the external ones contain the newly
-    // added role.
-    for (const auto& userDoc : userDocs) {
-        auto swUser = authzManager->acquireUser(
-            opCtx.get(),
-            UserName(userDoc.getStringField(kUserFieldName), userDoc.getStringField(kDbFieldName)));
-        ASSERT_OK(swUser.getStatus());
-        auto user = std::move(swUser.getValue());
-        ASSERT_EQUALS(
-            UserName(userDoc.getStringField(kUserFieldName), userDoc.getStringField(kDbFieldName)),
-            user->getName());
-        ASSERT(user.isValid());
-
-        RoleNameIterator cachedUserRolesIt = user->getRoles();
-        if (userDoc.getStringField(kDbFieldName) == "$external"_sd) {
-            for (const auto& userDocRole : updatedRoles) {
-                ASSERT_EQUALS(cachedUserRolesIt.next(),
-                              RoleName(userDocRole.getStringField(kRoleFieldName),
-                                       userDocRole.getStringField(kDbFieldName)));
-            }
-        } else {
-            for (const auto& userDocRole : initialRoles) {
-                ASSERT_EQUALS(cachedUserRolesIt.next(),
-                              RoleName(userDocRole.getStringField(kRoleFieldName),
-                                       userDocRole.getStringField(kDbFieldName)));
-            }
-        }
-        ASSERT_FALSE(cachedUserRolesIt.more());
-    }
-}
-
 }  // namespace
 }  // namespace mongo
diff --git a/src/mongo/db/auth/authz_manager_external_state_mock.cpp b/src/mongo/db/auth/authz_manager_external_state_mock.cpp
index 00dd9b421c7..cb684ed4f84 100644
--- a/src/mongo/db/auth/authz_manager_external_state_mock.cpp
+++ b/src/mongo/db/auth/authz_manager_external_state_mock.cpp
@@ -226,11 +226,10 @@ Status AuthzManagerExternalStateMock::updateOne(OperationContext* opCtx,
             return status;
         BSONObj newObj = document.getObject().copy();
         *iter = newObj;
-        BSONElement idQuery = newObj["_id"_sd];
-        BSONObj idQueryObj = idQuery.isABSONObj() ? idQuery.Obj() : BSON("_id" << idQuery);
+        BSONObj idQuery = newObj["_id"_sd].Obj();
 
         if (_authzManager) {
-            _authzManager->logOp(opCtx, "u", collectionName, logObj, &idQueryObj);
+            _authzManager->logOp(opCtx, "u", collectionName, logObj, &idQuery);
         }
 
         return Status::OK();
diff --git a/src/mongo/db/auth/user.cpp b/src/mongo/db/auth/user.cpp
index 36e2c26ff93..6ed98aae6f3 100644
--- a/src/mongo/db/auth/user.cpp
+++ b/src/mongo/db/auth/user.cpp
@@ -117,8 +117,6 @@ void User::setIndirectRoles(RoleNameIterator indirectRoles) {
     while (indirectRoles.more()) {
         _indirectRoles.push_back(indirectRoles.next());
     }
-    // Keep indirectRoles sorted for more efficient comparison against other users.
-    std::sort(_indirectRoles.begin(), _indirectRoles.end());
 }
 
 void User::setPrivileges(const PrivilegeVector& privileges) {
@@ -183,29 +181,4 @@ Status User::validateRestrictions(OperationContext* opCtx) const {
     return Status::OK();
 }
 
-bool User::hasDifferentRoles(const User& otherUser) const {
-    // If the number of direct or indirect roles in the users' are not the same, they have
-    // different roles.
-    if (_roles.size() != otherUser._roles.size() ||
-        _indirectRoles.size() != otherUser._indirectRoles.size()) {
-        return true;
-    }
-
-    // At this point, it is known that the users have the same number of direct roles. The
-    // direct roles sets are equivalent if all of the roles in the first user's directRoles are
-    // also in the other user's directRoles.
-    for (const auto& role : _roles) {
-        if (otherUser._roles.find(role) == otherUser._roles.end()) {
-            return true;
-        }
-    }
-
-    // Indirect roles should always be sorted.
-    dassert(std::is_sorted(_indirectRoles.begin(), _indirectRoles.end()));
-    dassert(std::is_sorted(otherUser._indirectRoles.begin(), otherUser._indirectRoles.end()));
-
-    return !std::equal(
-        _indirectRoles.begin(), _indirectRoles.end(), otherUser._indirectRoles.begin());
-}
-
 }  // namespace mongo
diff --git a/src/mongo/db/auth/user.h b/src/mongo/db/auth/user.h
index 53e5e2a7574..4a6b204ca76 100644
--- a/src/mongo/db/auth/user.h
+++ b/src/mongo/db/auth/user.h
@@ -245,8 +245,6 @@ public:
      */
     Status validateRestrictions(OperationContext* opCtx) const;
 
-    bool hasDifferentRoles(const User& otherUser) const;
-
 private:
     // Unique ID (often UUID) for this user. May be empty for legacy users.
     UserId _id;
diff --git a/src/mongo/embedded/embedded_auth_manager.cpp b/src/mongo/embedded/embedded_auth_manager.cpp
index fd89bb01d34..beb23608acd 100644
--- a/src/mongo/embedded/embedded_auth_manager.cpp
+++ b/src/mongo/embedded/embedded_auth_manager.cpp
@@ -124,10 +124,6 @@ public:
         UASSERT_NOT_IMPLEMENTED;
     }
 
-    Status refreshExternalUsers(OperationContext* opCtx) override {
-        UASSERT_NOT_IMPLEMENTED;
-    }
-
     Status initialize(OperationContext* opCtx) override {
         UASSERT_NOT_IMPLEMENTED;
     }
diff --git a/src/mongo/util/invalidating_lru_cache.h b/src/mongo/util/invalidating_lru_cache.h
index d07eb12e0ec..f2307dc007f 100644
--- a/src/mongo/util/invalidating_lru_cache.h
+++ b/src/mongo/util/invalidating_lru_cache.h
@@ -535,32 +535,6 @@ public:
         }
     }
 
-    /**
-     * Returns a vector of ValueHandles for all of the entries that satisfy matchPredicate.
-     */
-    template <typename Pred>
-    std::vector<ValueHandle> getEntriesIf(Pred matchPredicate) {
-        std::vector<ValueHandle> entries;
-        entries.reserve(_cache.size() + _evictedCheckedOutValues.size());
-        {
-            stdx::lock_guard lg(_mutex);
-            for (const auto& entry : _cache) {
-                if (matchPredicate(entry.first, &entry.second->value)) {
-                    entries.push_back(ValueHandle(entry.second));
-                }
-            }
-
-            for (const auto& entry : _evictedCheckedOutValues) {
-                if (auto storedValue = entry.second.lock()) {
-                    if (matchPredicate(entry.first, &storedValue->value)) {
-                        entries.push_back(ValueHandle(std::move(storedValue)));
-                    }
-                }
-            }
-        }
-        return entries;
-    }
-
     struct CachedItemInfo {
         Key key;            // The key of the item in the cache
         long int useCount;  // The number of callers of 'get', which still have the item checked-out
diff --git a/src/mongo/util/read_through_cache.h b/src/mongo/util/read_through_cache.h
index 06ce434114e..2fe230bdd41 100644
--- a/src/mongo/util/read_through_cache.h
+++ b/src/mongo/util/read_through_cache.h
@@ -430,28 +430,6 @@ public:
     }
 
     /**
-     * Returns a vector of ValueHandles for all of the keys that satisfy matchPredicate.
-     */
-    template <typename Pred>
-    std::vector<ValueHandle> getValueHandlesIfKey(const Pred& matchPredicate) {
-        stdx::unique_lock ul(_mutex);
-        auto invalidatingCacheValues = _cache.getEntriesIf(
-            [&](const Key& key, const StoredValue*) { return matchPredicate(key); });
-        ul.unlock();
-
-        std::vector<ValueHandle> valueHandles;
-        valueHandles.reserve(invalidatingCacheValues.size());
-        std::transform(invalidatingCacheValues.begin(),
-                       invalidatingCacheValues.end(),
-                       std::back_inserter(valueHandles),
-                       [](auto& invalidatingCacheValue) {
-                           return ValueHandle(std::move(invalidatingCacheValue));
-                       });
-
-        return valueHandles;
-    }
-
-    /**
      * Returns statistics information about the cache for reporting purposes.
      */
     std::vector<typename Cache::CachedItemInfo> getCacheInfo() const {