diff options
-rw-r--r-- | src/mongo/db/auth/authorization_manager.h | 7 | ||||
-rw-r--r-- | src/mongo/db/auth/authorization_manager_impl.cpp | 33 | ||||
-rw-r--r-- | src/mongo/db/auth/authorization_manager_impl.h | 6 | ||||
-rw-r--r-- | src/mongo/db/auth/authorization_manager_test.cpp | 122 | ||||
-rw-r--r-- | src/mongo/db/auth/authz_manager_external_state_mock.cpp | 5 | ||||
-rw-r--r-- | src/mongo/db/auth/user.cpp | 27 | ||||
-rw-r--r-- | src/mongo/db/auth/user.h | 2 | ||||
-rw-r--r-- | src/mongo/embedded/embedded_auth_manager.cpp | 4 | ||||
-rw-r--r-- | src/mongo/util/invalidating_lru_cache.h | 26 | ||||
-rw-r--r-- | src/mongo/util/read_through_cache.h | 22 |
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 { |