SERVER-55172 Add and use a fail point to wait for user cache invalidation

7 files changed, 126 insertions, 59 deletions

diff --git a/src/mongo/db/auth/SConscript b/src/mongo/db/auth/SConscript
index a068a11714b..63b8b4fa121 100644
--- a/src/mongo/db/auth/SConscript
+++ b/src/mongo/db/auth/SConscript
@@ -87,7 +87,6 @@ env.Library(
 env.Library(
     target='auth_impl_internal_local',
     source=[
-        'auth_types.idl',
         'authz_manager_external_state_local.cpp',
     ],
     LIBDEPS=[
@@ -97,6 +96,7 @@ env.Library(
         '$BUILD_DIR/mongo/db/catalog/collection_catalog',
         '$BUILD_DIR/mongo/db/concurrency/lock_manager',
         '$BUILD_DIR/mongo/db/db_raii',
+        'auth_types',
     ]
 )
 
@@ -165,6 +165,7 @@ env.Library(
         '$BUILD_DIR/mongo/util/net/ssl_types',
         'address_restriction',
         'auth',
+        'auth_types',
         'auth_umc',
         'authorization_manager_global',
         'authprivilege',
@@ -230,6 +231,19 @@ env.Library(
 )
 
 env.Library(
+    target='auth_types',
+    source=[
+        'auth_types.idl',
+    ],
+    LIBDEPS=[
+        '$BUILD_DIR/mongo/base',
+        '$BUILD_DIR/mongo/idl/idl_parser',
+        'auth',
+        'authprivilege',
+    ],
+)
+
+env.Library(
     target='auth_umc',
     source=[
         'impersonation_session.cpp',
diff --git a/src/mongo/db/auth/auth_types.idl b/src/mongo/db/auth/auth_types.idl
index a653d0152ab..3b55541a4da 100644
--- a/src/mongo/db/auth/auth_types.idl
+++ b/src/mongo/db/auth/auth_types.idl
@@ -72,5 +72,9 @@ structs:
     fields:
       resolveRolesDelayMS:
         type: int
-        default: 0
         validator: { gte: 0 }
+  waitForUserCacheInvalidationFailPoint:
+    description: Data for waitForUserCacheInvalidation fail point
+    fields:
+      userName:
+        type: UserName
diff --git a/src/mongo/db/auth/authorization_manager.h b/src/mongo/db/auth/authorization_manager.h
index 1dcd8208b0f..0c92156d70f 100644
--- a/src/mongo/db/auth/authorization_manager.h
+++ b/src/mongo/db/auth/authorization_manager.h
@@ -306,9 +306,8 @@ public:
     /**
      * Validate the ID associated with a known user while refreshing session cache.
      */
-    virtual StatusWith<UserHandle> acquireUserForSessionRefresh(OperationContext* opCtx,
-                                                                const UserName& userName,
-                                                                const User::UserId& uid) = 0;
+    virtual StatusWith<UserHandle> reacquireUser(OperationContext* opCtx,
+                                                 const UserHandle& user) = 0;
 
     /**
      * Marks the given user as invalid and removes it from the user cache.
diff --git a/src/mongo/db/auth/authorization_manager_impl.cpp b/src/mongo/db/auth/authorization_manager_impl.cpp
index 729df9178a7..807c17ef80b 100644
--- a/src/mongo/db/auth/authorization_manager_impl.cpp
+++ b/src/mongo/db/auth/authorization_manager_impl.cpp
@@ -44,6 +44,7 @@
 #include "mongo/config.h"
 #include "mongo/crypto/mechanism_scram.h"
 #include "mongo/db/auth/address_restriction.h"
+#include "mongo/db/auth/auth_types_gen.h"
 #include "mongo/db/auth/authorization_manager_global_parameters_gen.h"
 #include "mongo/db/auth/authorization_manager_impl_parameters_gen.h"
 #include "mongo/db/auth/authorization_session_impl.h"
@@ -283,6 +284,40 @@ std::unique_ptr<AuthorizationManager> authorizationManagerCreateImpl(
 auto authorizationManagerCreateRegistration =
     MONGO_WEAK_FUNCTION_REGISTRATION(AuthorizationManager::create, authorizationManagerCreateImpl);
 
+MONGO_FAIL_POINT_DEFINE(waitForUserCacheInvalidation);
+void handleWaitForUserCacheInvalidation(OperationContext* opCtx, const UserHandle& user) {
+    auto fp = waitForUserCacheInvalidation.scopedIf([&](const auto& bsonData) {
+        IDLParserErrorContext ctx("waitForUserCacheInvalidation");
+        auto data = WaitForUserCacheInvalidationFailPoint::parse(ctx, bsonData);
+
+        const auto& blockedUserName = data.getUserName();
+        return blockedUserName == user->getName();
+    });
+
+    if (!fp.isActive()) {
+        return;
+    }
+
+    // Since we do not have notifications from both the user cache and the fail point itself,
+    // loop until our condition is satisfied. To avoid this loop, we would need a way to union
+    // notifications from both. This may be possible with a CancellationToken and a condition
+    // variable or with some novel Notifiable/Waitable-like type that synthesizes multiple
+    // notifications.
+    constexpr auto kCheckPeriod = Milliseconds{1};
+    auto m = MONGO_MAKE_LATCH();
+    auto cv = stdx::condition_variable{};
+    auto pred = [&] { return !fp.isStillEnabled() || !user.isValid(); };
+    auto waitOneCycle = [&] {
+        auto lk = stdx::unique_lock(m);
+        return !opCtx->waitForConditionOrInterruptFor(cv, lk, kCheckPeriod, pred);
+    };
+
+    while (waitOneCycle()) {
+        // Not yet finished.
+    }
+}
+
+
 }  // namespace
 
 int authorizationManagerCacheSize;
@@ -465,15 +500,23 @@ StatusWith<UserHandle> AuthorizationManagerImpl::acquireUser(OperationContext* o
     return ex.toStatus();
 }
 
-StatusWith<UserHandle> AuthorizationManagerImpl::acquireUserForSessionRefresh(
-    OperationContext* opCtx, const UserName& userName, const User::UserId& uid) {
+StatusWith<UserHandle> AuthorizationManagerImpl::reacquireUser(OperationContext* opCtx,
+                                                               const UserHandle& user) {
+    const UserName& userName = user->getName();
+    handleWaitForUserCacheInvalidation(opCtx, user);
+    if (user.isValid()) {
+        return user;
+    }
+
+    // Make a good faith effort to acquire an up-to-date user object, since the one
+    // we've cached is marked "out-of-date."
     auto swUserHandle = acquireUser(opCtx, userName);
     if (!swUserHandle.isOK()) {
         return swUserHandle.getStatus();
     }
 
     auto ret = std::move(swUserHandle.getValue());
-    if (uid != ret->getID()) {
+    if (user->getID() != ret->getID()) {
         return {ErrorCodes::UserNotFound,
                 str::stream() << "User id from privilege document '" << userName.toString()
                               << "' does not match user id in session."};
diff --git a/src/mongo/db/auth/authorization_manager_impl.h b/src/mongo/db/auth/authorization_manager_impl.h
index c197f100dbd..6b6d059d790 100644
--- a/src/mongo/db/auth/authorization_manager_impl.h
+++ b/src/mongo/db/auth/authorization_manager_impl.h
@@ -98,9 +98,7 @@ public:
                                     std::vector<BSONObj>* result) override;
 
     StatusWith<UserHandle> acquireUser(OperationContext* opCtx, const UserName& userName) override;
-    StatusWith<UserHandle> acquireUserForSessionRefresh(OperationContext* opCtx,
-                                                        const UserName& userName,
-                                                        const User::UserId& uid) override;
+    StatusWith<UserHandle> reacquireUser(OperationContext* opCtx, const UserHandle& user) override;
 
     /**
      * Invalidate a user, and repin it if necessary.
diff --git a/src/mongo/db/auth/authorization_session_impl.cpp b/src/mongo/db/auth/authorization_session_impl.cpp
index 0686c974bcb..be0346d38cb 100644
--- a/src/mongo/db/auth/authorization_session_impl.cpp
+++ b/src/mongo/db/auth/authorization_session_impl.cpp
@@ -502,59 +502,33 @@ bool AuthorizationSessionImpl::isAuthenticated() {
 void AuthorizationSessionImpl::_refreshUserInfoAsNeeded(OperationContext* opCtx) {
     AuthorizationManager& authMan = getAuthorizationManager();
     UserSet::iterator it = _authenticatedUsers.begin();
+    auto removeUser = [&](const auto& it) {
+        // Take out a lock on the client here to ensure that no one reads while
+        // _authenticatedUsers is being modified.
+        stdx::lock_guard<Client> lk(*opCtx->getClient());
 
-    while (it != _authenticatedUsers.end()) {
-        auto& user = *it;
-        if (!user.isValid()) {
-            // Make a good faith effort to acquire an up-to-date user object, since the one
-            // we've cached is marked "out-of-date."
-            UserName name = user->getName();
-            UserHandle updatedUser;
-
-            auto swUser = authMan.acquireUserForSessionRefresh(opCtx, name, user->getID());
-            auto& status = swUser.getStatus();
-
-            // Take out a lock on the client here to ensure that no one reads while
-            // _authenticatedUsers is being modified.
-            stdx::lock_guard<Client> lk(*opCtx->getClient());
+        // The user is invalid, so make sure that we erase it from _authenticateUsers.
+        _authenticatedUsers.removeAt(it);
+    };
 
-            // The user is invalid, so make sure that we erase it from _authenticateUsers at the
-            // end of this block.
-            auto removeGuard = makeGuard([&] { _authenticatedUsers.removeAt(it++); });
+    auto replaceUser = [&](const auto& it, UserHandle updatedUser) {
+        // Take out a lock on the client here to ensure that no one reads while
+        // _authenticatedUsers is being modified.
+        stdx::lock_guard<Client> lk(*opCtx->getClient());
+        _authenticatedUsers.replaceAt(it, std::move(updatedUser));
+    };
 
+    while (it != _authenticatedUsers.end()) {
+        // Anchor the UserHandle on the stack so we can refer to it throughout this iteration.
+        const auto currentUser = *it;
+        const auto& name = currentUser->getName();
+        auto swUser = authMan.reacquireUser(opCtx, currentUser);
+        if (!swUser.isOK()) {
+            auto& status = swUser.getStatus();
             switch (status.code()) {
-                case ErrorCodes::OK: {
-                    updatedUser = std::move(swUser.getValue());
-                    try {
-                        auto restrictionStatus = updatedUser->validateRestrictions(opCtx);
-                        if (!restrictionStatus.isOK()) {
-                            LOGV2(20242,
-                                  "Removed user with unmet authentication restrictions from "
-                                  "session cache of user information. Restriction failed",
-                                  "user"_attr = name,
-                                  "reason"_attr = restrictionStatus.reason());
-                            // If we remove from the UserSet, we cannot increment the iterator.
-                            continue;
-                        }
-                    } catch (...) {
-                        LOGV2(20243,
-                              "Evaluating authentication restrictions for user resulted in an "
-                              "unknown exception. Removing user from the session cache",
-                              "user"_attr = name);
-                        continue;
-                    }
-
-                    // Success! Replace the old User object with the updated one.
-                    removeGuard.dismiss();
-                    _authenticatedUsers.replaceAt(it, std::move(updatedUser));
-                    LOGV2_DEBUG(20244,
-                                1,
-                                "Updated session cache of user information for user",
-                                "user"_attr = name);
-                    break;
-                }
                 case ErrorCodes::UserNotFound: {
                     // User does not exist anymore; remove it from _authenticatedUsers.
+                    removeUser(it++);
                     LOGV2(20245,
                           "Removed deleted user from session cache of user information",
                           "user"_attr = name);
@@ -562,6 +536,7 @@ void AuthorizationSessionImpl::_refreshUserInfoAsNeeded(OperationContext* opCtx)
                 }
                 case ErrorCodes::UnsupportedFormat: {
                     // An auth subsystem has explicitly indicated a failure.
+                    removeUser(it++);
                     LOGV2(20246,
                           "Removed user from session cache of user information because of "
                           "refresh failure",
@@ -579,10 +554,38 @@ void AuthorizationSessionImpl::_refreshUserInfoAsNeeded(OperationContext* opCtx)
                                   "to use old information",
                                   "user"_attr = name,
                                   "error"_attr = redact(status));
-                    removeGuard.dismiss();
                     break;
             }
+        } else if (!currentUser.isValid()) {
+            // Our user handle has changed, update the our list of users.
+            auto updatedUser = std::move(swUser.getValue());
+            try {
+                uassertStatusOK(updatedUser->validateRestrictions(opCtx));
+            } catch (const DBException& ex) {
+                removeUser(it++);
+
+                LOGV2(20242,
+                      "Removed user with unmet authentication restrictions from "
+                      "session cache of user information. Restriction failed",
+                      "user"_attr = name,
+                      "reason"_attr = ex.reason());
+                continue;  // No need to advance "it" in this case.
+            } catch (...) {
+                removeUser(it++);
+
+                LOGV2(20243,
+                      "Evaluating authentication restrictions for user resulted in an "
+                      "unknown exception. Removing user from the session cache",
+                      "user"_attr = name);
+                continue;  // No need to advance "it" in this case.
+            }
+
+            replaceUser(it, std::move(updatedUser));
+
+            LOGV2_DEBUG(
+                20244, 1, "Updated session cache of user information for user", "user"_attr = name);
         }
+
         ++it;
     }
     _buildAuthenticatedRolesVector();
diff --git a/src/mongo/db/auth/authz_manager_external_state_local.cpp b/src/mongo/db/auth/authz_manager_external_state_local.cpp
index 953e3ea1ec4..f5067d6aa33 100644
--- a/src/mongo/db/auth/authz_manager_external_state_local.cpp
+++ b/src/mongo/db/auth/authz_manager_external_state_local.cpp
@@ -318,6 +318,12 @@ StatusWith<User> AuthzManagerExternalStateLocal::getUserObject(OperationContext*
     user.addPrivileges(data.privileges.get());
     user.setIndirectRestrictions(data.restrictions.get());
 
+    LOGV2_DEBUG(5517200,
+                3,
+                "Acquired new user object",
+                "userName"_attr = userName,
+                "directRoles"_attr = directRoles);
+
     return std::move(user);
 } catch (const AssertionException& ex) {
     return ex.toStatus();