From d46d2efc689cb7b1b4fb2df9f656d5dc3d7a5fc9 Mon Sep 17 00:00:00 2001 From: Varun Ravichandran Date: Thu, 16 Sep 2021 21:04:36 +0000 Subject: SERVER-59148: Periodically refresh LDAP users in authorization user cache --- src/mongo/db/auth/authorization_manager.h | 7 ++ src/mongo/db/auth/authorization_manager_impl.cpp | 39 ++++++ src/mongo/db/auth/authorization_manager_impl.h | 6 + src/mongo/db/auth/authorization_manager_test.cpp | 134 +++++++++++++++++++++ src/mongo/db/auth/authorization_session_impl.cpp | 14 +++ .../db/auth/authz_manager_external_state_mock.cpp | 5 +- src/mongo/db/auth/user.cpp | 27 +++++ src/mongo/db/auth/user.h | 6 + src/mongo/db/auth/user_cache_invalidator_job.cpp | 18 +++ src/mongo/embedded/embedded_auth_manager.cpp | 4 + src/mongo/util/invalidating_lru_cache.h | 26 ++++ src/mongo/util/read_through_cache.h | 23 ++++ 12 files changed, 307 insertions(+), 2 deletions(-) diff --git a/src/mongo/db/auth/authorization_manager.h b/src/mongo/db/auth/authorization_manager.h index d344fe43b53..ad08c94de06 100644 --- a/src/mongo/db/auth/authorization_manager.h +++ b/src/mongo/db/auth/authorization_manager.h @@ -329,6 +329,13 @@ 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. diff --git a/src/mongo/db/auth/authorization_manager_impl.cpp b/src/mongo/db/auth/authorization_manager_impl.cpp index 0b29680b2af..bbe2f433592 100644 --- a/src/mongo/db/auth/authorization_manager_impl.cpp +++ b/src/mongo/db/auth/authorization_manager_impl.cpp @@ -699,6 +699,45 @@ 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 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()) { + // If the user simply is not found, then just invalidate the cached user and continue. + if (storedUserStatus.getStatus().code() == ErrorCodes::UserNotFound) { + _userCache.invalidate(request); + continue; + } else { + 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 2be78cfa629..a3fbf85af00 100644 --- a/src/mongo/db/auth/authorization_manager_impl.h +++ b/src/mongo/db/auth/authorization_manager_impl.h @@ -107,6 +107,12 @@ 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 001433e82f9..f91f3f0a329 100644 --- a/src/mongo/db/auth/authorization_manager_test.cpp +++ b/src/mongo/db/auth/authorization_manager_test.cpp @@ -258,5 +258,139 @@ 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 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 initialRoles{BSON("role" + << "read" + << "db" + << "test")}; + std::vector 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. Store the users into a vector so that they are checked out. + std::vector checkedOutUsers; + checkedOutUsers.reserve(userDocs.size()); + 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()); + checkedOutUsers.push_back(std::move(user)); + } + + // 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())); + + // Assert that all checked-out $external users are now marked invalid. + for (const auto& checkedOutUser : checkedOutUsers) { + if (checkedOutUser->getName().getDB() == "$external"_sd) { + ASSERT(!checkedOutUser.isValid()); + } else { + ASSERT(checkedOutUser.isValid()); + } + } + + // 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/authorization_session_impl.cpp b/src/mongo/db/auth/authorization_session_impl.cpp index 72fdbf82002..d3eda30eefc 100644 --- a/src/mongo/db/auth/authorization_session_impl.cpp +++ b/src/mongo/db/auth/authorization_session_impl.cpp @@ -582,6 +582,20 @@ void AuthorizationSessionImpl::_refreshUserInfoAsNeeded(OperationContext* opCtx) auto swUser = authMan.reacquireUser(opCtx, currentUser); if (!swUser.isOK()) { auto& status = swUser.getStatus(); + // If an external user is no longer in the cache and cannot be acquired from the cache's + // backing external service, it should be removed from _authenticatedUsers. This + // guarantees that no operations can be performed until the external authorization + // provider comes back up. + if (name.getDB() == "$external"_sd) { + removeUser(it++); + LOGV2(5914804, + "Removed external user from session cache of user information because of " + "error status", + "user"_attr = name, + "status"_attr = status); + continue; // No need to advance "it" in this case. + } + switch (status.code()) { case ErrorCodes::UserNotFound: { // User does not exist anymore; remove it from _authenticatedUsers. 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 cb684ed4f84..00dd9b421c7 100644 --- a/src/mongo/db/auth/authz_manager_external_state_mock.cpp +++ b/src/mongo/db/auth/authz_manager_external_state_mock.cpp @@ -226,10 +226,11 @@ Status AuthzManagerExternalStateMock::updateOne(OperationContext* opCtx, return status; BSONObj newObj = document.getObject().copy(); *iter = newObj; - BSONObj idQuery = newObj["_id"_sd].Obj(); + BSONElement idQuery = newObj["_id"_sd]; + BSONObj idQueryObj = idQuery.isABSONObj() ? idQuery.Obj() : BSON("_id" << idQuery); if (_authzManager) { - _authzManager->logOp(opCtx, "u", collectionName, logObj, &idQuery); + _authzManager->logOp(opCtx, "u", collectionName, logObj, &idQueryObj); } return Status::OK(); diff --git a/src/mongo/db/auth/user.cpp b/src/mongo/db/auth/user.cpp index 6b6cf35d259..840496d0106 100644 --- a/src/mongo/db/auth/user.cpp +++ b/src/mongo/db/auth/user.cpp @@ -130,6 +130,8 @@ 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) { @@ -253,4 +255,29 @@ void User::reportForUsersInfo(BSONObjBuilder* builder, } } +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 4a1045ad4d7..8414aca0aa8 100644 --- a/src/mongo/db/auth/user.h +++ b/src/mongo/db/auth/user.h @@ -302,6 +302,12 @@ public: bool showPrivileges, bool showAuthenticationRestrictions) const; + /** + * Returns true if the User object has at least one different direct or indirect role from the + * otherUser. + */ + 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/db/auth/user_cache_invalidator_job.cpp b/src/mongo/db/auth/user_cache_invalidator_job.cpp index 5cb272ea96b..d9b3fc8d3d7 100644 --- a/src/mongo/db/auth/user_cache_invalidator_job.cpp +++ b/src/mongo/db/auth/user_cache_invalidator_job.cpp @@ -184,6 +184,24 @@ void UserCacheInvalidator::run() { LOGV2_WARNING(20268, "Error invalidating user cache", "error"_attr = e.toStatus()); } _previousGeneration = swCurrentGeneration.getValue(); + } else { + // If LDAP authorization is enabled and the authz cache generation has not changed, the + // external users should be refreshed to ensure that any cached users evicted on the config + // server are appropriately refreshed here. + auto refreshStatus = _authzManager->refreshExternalUsers(opCtx.get()); + if (!refreshStatus.isOK()) { + LOGV2_WARNING(5914803, + "Error refreshing external users in user cache, so invalidating external " + "users in cache", + "error"_attr = refreshStatus); + try { + _authzManager->invalidateUsersFromDB(opCtx.get(), "$external"_sd); + } catch (const DBException& e) { + LOGV2_WARNING(5914805, + "Error invalidating $external users from user cache", + "error"_attr = e.toStatus()); + } + } } } diff --git a/src/mongo/embedded/embedded_auth_manager.cpp b/src/mongo/embedded/embedded_auth_manager.cpp index beb23608acd..fd89bb01d34 100644 --- a/src/mongo/embedded/embedded_auth_manager.cpp +++ b/src/mongo/embedded/embedded_auth_manager.cpp @@ -124,6 +124,10 @@ 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 f2307dc007f..d07eb12e0ec 100644 --- a/src/mongo/util/invalidating_lru_cache.h +++ b/src/mongo/util/invalidating_lru_cache.h @@ -535,6 +535,32 @@ public: } } + /** + * Returns a vector of ValueHandles for all of the entries that satisfy matchPredicate. + */ + template + std::vector getEntriesIf(Pred matchPredicate) { + std::vector 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 2fe230bdd41..65c5a39d96c 100644 --- a/src/mongo/util/read_through_cache.h +++ b/src/mongo/util/read_through_cache.h @@ -429,6 +429,29 @@ public: invalidateKeyIf([](const Key&) { return true; }); } + /** + * Returns a vector of ValueHandles for all of the keys that satisfy matchPredicate. + */ + template + std::vector getValueHandlesIfKey(const Pred& matchPredicate) { + auto invalidatingCacheValues = [&]() { + stdx::lock_guard lg(_mutex); + return _cache.getEntriesIf( + [&](const Key& key, const StoredValue*) { return matchPredicate(key); }); + }(); + + std::vector 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. */ -- cgit v1.2.1