SERVER-83488 Move role acquisition for X509 and OIDC into UserRequest objects (#26917)

GitOrigin-RevId: bddf66106c00720df0d787f218cf94e6d4fffcd4

14 files changed, 212 insertions, 145 deletions

diff --git a/src/mongo/db/auth/BUILD.bazel b/src/mongo/db/auth/BUILD.bazel
index 2e8f31cfb11..04d01be35d9 100644
--- a/src/mongo/db/auth/BUILD.bazel
+++ b/src/mongo/db/auth/BUILD.bazel
@@ -542,6 +542,7 @@ mongo_cc_library(
         "user_request_x509.cpp",
     ],
     hdrs = [
+        "user_name.h",
         "user_request_x509.h",
     ],
     deps = [
diff --git a/src/mongo/db/auth/authorization_manager_impl.cpp b/src/mongo/db/auth/authorization_manager_impl.cpp
index b65d67bf810..93bff0d69c3 100644
--- a/src/mongo/db/auth/authorization_manager_impl.cpp
+++ b/src/mongo/db/auth/authorization_manager_impl.cpp
@@ -247,33 +247,6 @@ void handleWaitForUserCacheInvalidation(OperationContext* opCtx, const UserHandl
     }
 }
 
-// TODO SERVER-83488 - move user request role resolution into the UserRequest object.
-std::unique_ptr<UserRequest> getX509UserRequest(OperationContext* opCtx, const UserName& username) {
-#ifdef MONGO_CONFIG_SSL
-    std::shared_ptr<transport::Session> session;
-    if (opCtx && opCtx->getClient()) {
-        session = opCtx->getClient()->session();
-    }
-
-    if (!allowRolesFromX509Certificates || !session) {
-        return std::make_unique<UserRequestGeneral>(username, boost::none);
-    }
-
-    auto& sslPeerInfo = SSLPeerInfo::forSession(session);
-    auto&& peerRoles = sslPeerInfo.roles();
-    if (peerRoles.empty() || (sslPeerInfo.subjectName().toString() != username.getUser())) {
-        return std::make_unique<UserRequestGeneral>(username, boost::none);
-    }
-
-    std::set<RoleName> requestRoles;
-    std::copy(
-        peerRoles.begin(), peerRoles.end(), std::inserter(requestRoles, requestRoles.begin()));
-
-    return std::make_unique<UserRequestX509>(username, std::move(requestRoles), sslPeerInfo);
-#endif
-    return std::make_unique<UserRequestGeneral>(username, boost::none);
-}
-
 }  // namespace
 
 int authorizationManagerCacheSize;
@@ -449,12 +422,6 @@ StatusWith<UserHandle> AuthorizationManagerImpl::acquireUser(
     OperationContext* opCtx, std::unique_ptr<UserRequest> request) try {
     const UserName userName = request->getUserName();
 
-    // X.509 will give us our roles for initial acquire, but we have to lose them during
-    // reacquire (for now) so reparse those roles into the request if not already present.
-    if (request->getType() == UserRequest::UserRequestType::X509) {
-        request = getX509UserRequest(opCtx, userName);
-    }
-
     auto systemUser = internalSecurity.getUser();
     if (userName == (*systemUser)->getName()) {
         uassert(ErrorCodes::OperationFailed,
@@ -511,16 +478,14 @@ StatusWith<UserHandle> AuthorizationManagerImpl::reacquireUser(OperationContext*
         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."
-    // TODO SERVER-72678 avoid this edge case hack when rearchitecting user acquisition. This is
-    // necessary now to preserve the mechanismData from the original UserRequest while eliminating
-    // the roles. If the roles aren't reset to none, it will cause LDAP acquisition to be bypassed
-    // in favor of reusing the ones from before.
-    std::unique_ptr<UserRequest> requestWithoutRoles = user->getUserRequest()->clone();
-    requestWithoutRoles->setRoles(boost::none);
+    // Since we throw in the constructor if we have an error in acquiring roles for OIDC and X509,
+    // we want to catch these exceptions and return them as proper statuses to be handled later.
+    auto swRequestWithoutRoles = user->getUserRequest()->cloneForReacquire();
+    if (!swRequestWithoutRoles.isOK()) {
+        return swRequestWithoutRoles.getStatus();
+    }
 
-    auto swUserHandle = acquireUser(opCtx, std::move(requestWithoutRoles));
+    auto swUserHandle = acquireUser(opCtx, std::move(swRequestWithoutRoles.getValue()));
     if (!swUserHandle.isOK()) {
         return swUserHandle.getStatus();
     }
@@ -594,29 +559,42 @@ Status AuthorizationManagerImpl::refreshExternalUsers(OperationContext* opCtx) {
     // insertOrAssign if they differ.
     bool isRefreshed{false};
     for (const auto& cachedUser : cachedUsers) {
-        // TODO SERVER-83488: look into whether constructing a new UserRequest makes the most
-        // sense, or re-gather the roles upfront in this pathway.
-        auto storedUserStatus =
-            _externalState->getUserObject(opCtx,
-                                          UserRequestGeneral(cachedUser->getName(), boost::none),
-                                          CurOp::get(opCtx)->getUserAcquisitionStats());
-        if (!storedUserStatus.isOK()) {
-            // If the user simply is not found, then just invalidate the cached user and continue.
-            if (storedUserStatus.getStatus().code() == ErrorCodes::UserNotFound) {
+        auto handleError = [this](const Status& status, const UserHandle& cachedUser) -> Status {
+            if (status.code() == ErrorCodes::UserNotFound) {
                 _userCache.invalidateKey(
                     cachedUser->getUserRequest()->generateUserRequestCacheKey());
-                continue;
+                return Status::OK();
             } else {
-                return storedUserStatus.getStatus();
+                return status;
+            }
+        };
+
+        auto swUserReq = cachedUser->getUserRequest()->cloneForReacquire();
+        if (!swUserReq.isOK()) {
+            auto status = handleError(swUserReq.getStatus(), cachedUser);
+            if (!status.isOK()) {
+                return status;
+            }
+            continue;
+        }
+
+        auto& userReq = swUserReq.getValue();
+
+        auto storedUserStatus = _externalState->getUserObject(
+            opCtx, *userReq.get(), CurOp::get(opCtx)->getUserAcquisitionStats());
+
+        if (!storedUserStatus.isOK()) {
+            auto status = handleError(storedUserStatus.getStatus(), cachedUser);
+            if (!status.isOK()) {
+                return status;
             }
+            continue;
         }
 
         if (cachedUser->hasDifferentRoles(storedUserStatus.getValue())) {
-            _userCache.insertOrAssign(
-                std::make_unique<UserRequestGeneral>(cachedUser->getName(), boost::none)
-                    ->generateUserRequestCacheKey(),
-                std::move(storedUserStatus.getValue()),
-                Date_t::now());
+            _userCache.insertOrAssign(userReq->generateUserRequestCacheKey(),
+                                      std::move(storedUserStatus.getValue()),
+                                      Date_t::now());
             isRefreshed = true;
         }
     }
diff --git a/src/mongo/db/auth/authorization_session_test.cpp b/src/mongo/db/auth/authorization_session_test.cpp
index 7f9dd287d91..fc5fd74cde4 100644
--- a/src/mongo/db/auth/authorization_session_test.cpp
+++ b/src/mongo/db/auth/authorization_session_test.cpp
@@ -144,10 +144,10 @@ const transport::TransportLayerMock transportLayer;
 
 TEST_F(AuthorizationSessionTest, MultiAuthSameUserAllowed) {
     ASSERT_OK(createUser(kUser1Test, {}));
-    ASSERT_OK(
-        authzSession->addAndAuthorizeUser(_opCtx.get(), kUser1TestRequest->clone(), boost::none));
-    ASSERT_OK(
-        authzSession->addAndAuthorizeUser(_opCtx.get(), kUser1TestRequest->clone(), boost::none));
+    ASSERT_OK(authzSession->addAndAuthorizeUser(
+        _opCtx.get(), std::move(kUser1TestRequest->clone().getValue()), boost::none));
+    ASSERT_OK(authzSession->addAndAuthorizeUser(
+        _opCtx.get(), std::move(kUser1TestRequest->clone().getValue()), boost::none));
     authzSession->logoutAllDatabases(_client.get(), "Test finished");
 }
 
@@ -155,10 +155,10 @@ TEST_F(AuthorizationSessionTest, MultiAuthSameDBDisallowed) {
     ASSERT_OK(createUser(kUser1Test, {}));
     ASSERT_OK(createUser(kUser2Test, {}));
 
-    ASSERT_OK(
-        authzSession->addAndAuthorizeUser(_opCtx.get(), kUser1TestRequest->clone(), boost::none));
-    ASSERT_NOT_OK(
-        authzSession->addAndAuthorizeUser(_opCtx.get(), kUser2TestRequest->clone(), boost::none));
+    ASSERT_OK(authzSession->addAndAuthorizeUser(
+        _opCtx.get(), std::move(kUser1TestRequest->clone().getValue()), boost::none));
+    ASSERT_NOT_OK(authzSession->addAndAuthorizeUser(
+        _opCtx.get(), std::move(kUser2TestRequest->clone().getValue()), boost::none));
     authzSession->logoutAllDatabases(_client.get(), "Test finished");
 }
 
@@ -166,10 +166,10 @@ TEST_F(AuthorizationSessionTest, MultiAuthMultiDBDisallowed) {
     ASSERT_OK(createUser(kUser1Test, {}));
     ASSERT_OK(createUser(kUser2Test, {}));
 
-    ASSERT_OK(
-        authzSession->addAndAuthorizeUser(_opCtx.get(), kUser1TestRequest->clone(), boost::none));
-    ASSERT_NOT_OK(
-        authzSession->addAndAuthorizeUser(_opCtx.get(), kUser2TestRequest->clone(), boost::none));
+    ASSERT_OK(authzSession->addAndAuthorizeUser(
+        _opCtx.get(), std::move(kUser1TestRequest->clone().getValue()), boost::none));
+    ASSERT_NOT_OK(authzSession->addAndAuthorizeUser(
+        _opCtx.get(), std::move(kUser2TestRequest->clone().getValue()), boost::none));
     authzSession->logoutAllDatabases(_client.get(), "Test finished");
 }
 
@@ -198,12 +198,13 @@ TEST_F(AuthorizationSessionTest, AddUserAndCheckAuthorization) {
     // Check that you can't authorize a user that doesn't exist.
     ASSERT_EQUALS(
         ErrorCodes::UserNotFound,
-        authzSession->addAndAuthorizeUser(_opCtx.get(), kSpencerTestRequest->clone(), boost::none));
+        authzSession->addAndAuthorizeUser(
+            _opCtx.get(), std::move(kSpencerTestRequest->clone().getValue()), boost::none));
 
     // Add a user with readWrite and dbAdmin on the test DB
     ASSERT_OK(createUser({"spencer", "test"}, {{"readWrite", "test"}, {"dbAdmin", "test"}}));
-    ASSERT_OK(
-        authzSession->addAndAuthorizeUser(_opCtx.get(), kSpencerTestRequest->clone(), boost::none));
+    ASSERT_OK(authzSession->addAndAuthorizeUser(
+        _opCtx.get(), std::move(kSpencerTestRequest->clone().getValue()), boost::none));
 
     ASSERT_TRUE(
         authzSession->isAuthorizedForActionsOnResource(testFooCollResource, ActionType::insert));
@@ -215,8 +216,8 @@ TEST_F(AuthorizationSessionTest, AddUserAndCheckAuthorization) {
 
     // Add an admin user with readWriteAnyDatabase
     ASSERT_OK(createUser({"admin", "admin"}, {{"readWriteAnyDatabase", "admin"}}));
-    ASSERT_OK(
-        authzSession->addAndAuthorizeUser(_opCtx.get(), kAdminAdminRequest->clone(), boost::none));
+    ASSERT_OK(authzSession->addAndAuthorizeUser(
+        _opCtx.get(), std::move(kAdminAdminRequest->clone().getValue()), boost::none));
 
     ASSERT_TRUE(authzSession->isAuthorizedForActionsOnResource(
         ResourcePattern::forExactNamespace(
@@ -272,8 +273,8 @@ TEST_F(AuthorizationSessionTest, DuplicateRolesOK) {
     // Add a user with doubled-up readWrite and single dbAdmin on the test DB
     ASSERT_OK(createUser(kSpencerTest,
                          {{"readWrite", "test"}, {"dbAdmin", "test"}, {"readWrite", "test"}}));
-    ASSERT_OK(
-        authzSession->addAndAuthorizeUser(_opCtx.get(), kSpencerTestRequest->clone(), boost::none));
+    ASSERT_OK(authzSession->addAndAuthorizeUser(
+        _opCtx.get(), std::move(kSpencerTestRequest->clone().getValue()), boost::none));
 
     ASSERT_TRUE(
         authzSession->isAuthorizedForActionsOnResource(testFooCollResource, ActionType::insert));
@@ -305,8 +306,8 @@ TEST_F(AuthorizationSessionTest, SystemCollectionsAccessControl) {
                          {{"readWriteAnyDatabase", "admin"}, {"dbAdminAnyDatabase", "admin"}}));
     ASSERT_OK(createUser(kUserAdminAnyTest, {{"userAdminAnyDatabase", "admin"}}));
 
-    ASSERT_OK(
-        authzSession->addAndAuthorizeUser(_opCtx.get(), kRWAnyTestRequest->clone(), boost::none));
+    ASSERT_OK(authzSession->addAndAuthorizeUser(
+        _opCtx.get(), std::move(kRWAnyTestRequest->clone().getValue()), boost::none));
 
     ASSERT_FALSE(
         authzSession->isAuthorizedForActionsOnResource(testUsersCollResource, ActionType::insert));
@@ -323,7 +324,7 @@ TEST_F(AuthorizationSessionTest, SystemCollectionsAccessControl) {
     authzSession->logoutDatabase(_client.get(), kTestDB, "Kill the test!"_sd);
 
     ASSERT_OK(authzSession->addAndAuthorizeUser(
-        _opCtx.get(), kUserAdminAnyTestRequest->clone(), boost::none));
+        _opCtx.get(), std::move(kUserAdminAnyTestRequest->clone().getValue()), boost::none));
     ASSERT_FALSE(
         authzSession->isAuthorizedForActionsOnResource(testUsersCollResource, ActionType::insert));
     ASSERT_TRUE(
@@ -338,8 +339,8 @@ TEST_F(AuthorizationSessionTest, SystemCollectionsAccessControl) {
         authzSession->isAuthorizedForActionsOnResource(otherProfileCollResource, ActionType::find));
     authzSession->logoutDatabase(_client.get(), kTestDB, "Kill the test!"_sd);
 
-    ASSERT_OK(
-        authzSession->addAndAuthorizeUser(_opCtx.get(), kRWTestRequest->clone(), boost::none));
+    ASSERT_OK(authzSession->addAndAuthorizeUser(
+        _opCtx.get(), std::move(kRWTestRequest->clone().getValue()), boost::none));
 
     ASSERT_FALSE(
         authzSession->isAuthorizedForActionsOnResource(testUsersCollResource, ActionType::insert));
@@ -356,7 +357,7 @@ TEST_F(AuthorizationSessionTest, SystemCollectionsAccessControl) {
     authzSession->logoutDatabase(_client.get(), kTestDB, "Kill the test!"_sd);
 
     ASSERT_OK(authzSession->addAndAuthorizeUser(
-        _opCtx.get(), kUserAdminTestRequest->clone(), boost::none));
+        _opCtx.get(), std::move(kUserAdminTestRequest->clone().getValue()), boost::none));
     ASSERT_FALSE(
         authzSession->isAuthorizedForActionsOnResource(testUsersCollResource, ActionType::insert));
     ASSERT_FALSE(
@@ -377,8 +378,8 @@ void AuthorizationSessionTest::testInvalidateUser() {
         transport::MockSession::create(&transportLayer);
     const auto& sslPeerInfo = SSLPeerInfo::forSession(session);
 
-    std::unique_ptr<UserRequest> userRequest =
-        std::make_unique<UserRequestX509>(kSpencerTest, boost::none, sslPeerInfo);
+    std::unique_ptr<UserRequest> userRequest = std::move(
+        UserRequestX509::makeUserRequestX509(kSpencerTest, boost::none, sslPeerInfo).getValue());
 
     // Add a readWrite user
     ASSERT_OK(createUser(kSpencerTest, {{"readWrite", "test"}}));
@@ -431,8 +432,8 @@ TEST_F(AuthorizationSessionTest, InvalidateUserByName) {
 TEST_F(AuthorizationSessionTest, UseOldUserInfoInFaceOfConnectivityProblems) {
     // Add a readWrite user
     ASSERT_OK(createUser({"spencer", "test"}, {{"readWrite", "test"}}));
-    ASSERT_OK(
-        authzSession->addAndAuthorizeUser(_opCtx.get(), kSpencerTestRequest->clone(), boost::none));
+    ASSERT_OK(authzSession->addAndAuthorizeUser(
+        _opCtx.get(), std::move(kSpencerTestRequest->clone().getValue()), boost::none));
 
     ASSERT_TRUE(
         authzSession->isAuthorizedForActionsOnResource(testFooCollResource, ActionType::find));
@@ -504,7 +505,7 @@ TEST_F(AuthorizationSessionTest, AcquireUserObtainsAndValidatesAuthenticationRes
         auto client = getServiceContext()->getService()->makeClient("testClient", mock_session);
         auto opCtx = client->makeOperationContext();
         ASSERT_OK(authzSession->addAndAuthorizeUser(
-            opCtx.get(), kSpencerTestRequest->clone(), boost::none));
+            opCtx.get(), std::move(kSpencerTestRequest->clone().getValue()), boost::none));
         authzSession->logoutDatabase(client.get(), kTestDB, "Kill the test!"_sd);
     };
 
@@ -517,12 +518,12 @@ TEST_F(AuthorizationSessionTest, AcquireUserObtainsAndValidatesAuthenticationRes
         auto client = getServiceContext()->getService()->makeClient("testClient", mock_session);
         auto opCtx = client->makeOperationContext();
         ASSERT_NOT_OK(authzSession->addAndAuthorizeUser(
-            opCtx.get(), kSpencerTestRequest->clone(), boost::none));
+            opCtx.get(), std::move(kSpencerTestRequest->clone().getValue()), boost::none));
     };
 
     // The empty RestrictionEnvironment will cause addAndAuthorizeUser to fail.
-    ASSERT_NOT_OK(
-        authzSession->addAndAuthorizeUser(_opCtx.get(), kSpencerTestRequest->clone(), boost::none));
+    ASSERT_NOT_OK(authzSession->addAndAuthorizeUser(
+        _opCtx.get(), std::move(kSpencerTestRequest->clone().getValue()), boost::none));
 
     // A clientSource from the 192.168.0.0/24 block will succeed in connecting to a server
     // listening on 192.168.0.2.
@@ -1284,8 +1285,8 @@ TEST_F(AuthorizationSessionTestWithoutAuth,
 
 TEST_F(AuthorizationSessionTest, AuthorizedSessionIsNotCoauthorizedNobody) {
     ASSERT_OK(createUser(kSpencerTest, {}));
-    ASSERT_OK(
-        authzSession->addAndAuthorizeUser(_opCtx.get(), kSpencerTestRequest->clone(), boost::none));
+    ASSERT_OK(authzSession->addAndAuthorizeUser(
+        _opCtx.get(), std::move(kSpencerTestRequest->clone().getValue()), boost::none));
     ASSERT_FALSE(authzSession->isCoauthorizedWith(boost::none));
     authzSession->logoutDatabase(_client.get(), kTestDB, "Kill the test!"_sd);
 }
@@ -1294,8 +1295,8 @@ TEST_F(AuthorizationSessionTestWithoutAuth,
        AuthorizedSessionIsCoauthorizedNobodyWhenAuthIsDisabled) {
     ASSERT_FALSE(authzManager->isAuthEnabled());
     ASSERT_OK(createUser(kSpencerTest, {}));
-    ASSERT_OK(
-        authzSession->addAndAuthorizeUser(_opCtx.get(), kSpencerTestRequest->clone(), boost::none));
+    ASSERT_OK(authzSession->addAndAuthorizeUser(
+        _opCtx.get(), std::move(kSpencerTestRequest->clone().getValue()), boost::none));
     ASSERT_TRUE(authzSession->isCoauthorizedWith(kSpencerTest));
     authzSession->logoutDatabase(_client.get(), kTestDB, "Kill the test!"_sd);
 }
@@ -1432,8 +1433,8 @@ TEST_F(AuthorizationSessionTest, MayBypassWriteBlockingModeIsSetCorrectly) {
                                                                        << "db"
                                                                        << "test"))),
                                                BSONObj()));
-    ASSERT_OK(
-        authzSession->addAndAuthorizeUser(_opCtx.get(), kSpencerTestRequest->clone(), boost::none));
+    ASSERT_OK(authzSession->addAndAuthorizeUser(
+        _opCtx.get(), std::move(kSpencerTestRequest->clone().getValue()), boost::none));
     ASSERT_FALSE(authzSession->mayBypassWriteBlockingMode());
 
     // Add a user with restore role on admin db and ensure we can bypass
@@ -1450,8 +1451,8 @@ TEST_F(AuthorizationSessionTest, MayBypassWriteBlockingModeIsSetCorrectly) {
                                                BSONObj()));
     authzSession->logoutDatabase(_client.get(), kTestDB, "End of test"_sd);
 
-    ASSERT_OK(
-        authzSession->addAndAuthorizeUser(_opCtx.get(), kGMarksAdminRequest->clone(), boost::none));
+    ASSERT_OK(authzSession->addAndAuthorizeUser(
+        _opCtx.get(), std::move(kGMarksAdminRequest->clone().getValue()), boost::none));
     ASSERT_TRUE(authzSession->mayBypassWriteBlockingMode());
 
     // Remove that user by logging out of the admin db and ensure we can't bypass anymore
@@ -1473,8 +1474,8 @@ TEST_F(AuthorizationSessionTest, MayBypassWriteBlockingModeIsSetCorrectly) {
                                                BSONObj()));
     authzSession->logoutDatabase(_client.get(), kAdminDB, ""_sd);
 
-    ASSERT_OK(
-        authzSession->addAndAuthorizeUser(_opCtx.get(), kAdminAdminRequest->clone(), boost::none));
+    ASSERT_OK(authzSession->addAndAuthorizeUser(
+        _opCtx.get(), std::move(kAdminAdminRequest->clone().getValue()), boost::none));
     ASSERT_TRUE(authzSession->mayBypassWriteBlockingMode());
 
     // Remove non-privileged user by logging out of test db and ensure we can still bypass
@@ -1491,14 +1492,14 @@ TEST_F(AuthorizationSessionTest, InvalidExpirationTime) {
     Date_t expirationTime = clockSource()->now() - Hours(1);
     ASSERT_OK(createUser({"spencer", "test"}, {{"readWrite", "test"}, {"dbAdmin", "test"}}));
     ASSERT_NOT_OK(authzSession->addAndAuthorizeUser(
-        _opCtx.get(), kSpencerTestRequest->clone(), expirationTime));
+        _opCtx.get(), std::move(kSpencerTestRequest->clone().getValue()), expirationTime));
 }
 
 TEST_F(AuthorizationSessionTest, NoExpirationTime) {
     // Create and authorize valid user with no expiration.
     ASSERT_OK(createUser({"spencer", "test"}, {{"readWrite", "test"}, {"dbAdmin", "test"}}));
-    ASSERT_OK(
-        authzSession->addAndAuthorizeUser(_opCtx.get(), kSpencerTestRequest->clone(), boost::none));
+    ASSERT_OK(authzSession->addAndAuthorizeUser(
+        _opCtx.get(), std::move(kSpencerTestRequest->clone().getValue()), boost::none));
     assertActive(testFooCollResource, ActionType::insert);
 
     // Assert that moving the clock forward has no impact on a session without expiration time.
@@ -1538,7 +1539,7 @@ TEST_F(AuthorizationSessionTest, TenantSeparation) {
     // User with tenant ID #1 with basic read/write privileges on "test" should be able to write to
     // tenant ID #1's test collection, and no others.
     ASSERT_OK(authzSession->addAndAuthorizeUser(
-        _opCtx.get(), kTenant1UserTestRequest->clone(), boost::none));
+        _opCtx.get(), std::move(kTenant1UserTestRequest->clone().getValue()), boost::none));
     assertActive(testTenant1FooCollResource, ActionType::insert);
     assertNotAuthorized(testFooCollResource, ActionType::insert);
     assertNotAuthorized(testTenant2FooCollResource, ActionType::insert);
@@ -1553,7 +1554,7 @@ TEST_F(AuthorizationSessionTest, TenantSeparation) {
     // User with tenant ID #2 with readWriteAny should be able to write to any of tenant ID #2's
     // normal collections, and no others.
     ASSERT_OK(authzSession->addAndAuthorizeUser(
-        _opCtx.get(), kTenant2UserTestRequest->clone(), boost::none));
+        _opCtx.get(), std::move(kTenant2UserTestRequest->clone().getValue()), boost::none));
     assertActive(testTenant2FooCollResource, ActionType::insert);
     assertNotAuthorized(testFooCollResource, ActionType::insert);
     assertNotAuthorized(testTenant1FooCollResource, ActionType::insert);
@@ -1567,8 +1568,8 @@ TEST_F(AuthorizationSessionTest, TenantSeparation) {
 
     // User with no tenant ID with basic read/write privileges on "test" should be able to write to
     // the no-tenant test collection, and no others.
-    ASSERT_OK(
-        authzSession->addAndAuthorizeUser(_opCtx.get(), kUser1TestRequest->clone(), boost::none));
+    ASSERT_OK(authzSession->addAndAuthorizeUser(
+        _opCtx.get(), std::move(kUser1TestRequest->clone().getValue()), boost::none));
     assertActive(testFooCollResource, ActionType::insert);
     assertNotAuthorized(testTenant1FooCollResource, ActionType::insert);
     assertNotAuthorized(testTenant2FooCollResource, ActionType::insert);
@@ -1581,8 +1582,8 @@ TEST_F(AuthorizationSessionTest, TenantSeparation) {
     // User with no tenant ID with root should be able to write to any tenant's normal
     // collections, because boost::none acts as "any tenant" for privileges which don't specify a
     // namespace/DB, and root has the useTenant privilege.
-    ASSERT_OK(
-        authzSession->addAndAuthorizeUser(_opCtx.get(), kUser2TestRequest->clone(), boost::none));
+    ASSERT_OK(authzSession->addAndAuthorizeUser(
+        _opCtx.get(), std::move(kUser2TestRequest->clone().getValue()), boost::none));
     assertActive(testFooCollResource, ActionType::insert);
     assertActive(testTenant1FooCollResource, ActionType::insert);
     assertActive(testTenant2FooCollResource, ActionType::insert);
@@ -1610,7 +1611,7 @@ TEST_F(AuthorizationSessionTest, TenantSeparation) {
     // User with tenant ID 2 with __system privileges should be able to write to any of tenant 2's
     // collections, including system collections.
     ASSERT_OK(authzSession->addAndAuthorizeUser(
-        _opCtx.get(), tenant2SystemUserRequest->clone(), boost::none));
+        _opCtx.get(), std::move(tenant2SystemUserRequest->clone().getValue()), boost::none));
     assertActive(testTenant2FooCollResource, ActionType::insert);
     assertNotAuthorized(testFooCollResource, ActionType::insert);
     assertNotAuthorized(testTenant1FooCollResource, ActionType::insert);
@@ -1645,7 +1646,7 @@ TEST_F(AuthorizationSessionTest, ExpiredSessionWithReauth) {
     ASSERT_OK(createUser({"spencer", "test"}, {{"readWrite", "test"}, {"dbAdmin", "test"}}));
     ASSERT_OK(createUser({"admin", "admin"}, {{"readWriteAnyDatabase", "admin"}}));
     ASSERT_OK(authzSession->addAndAuthorizeUser(
-        _opCtx.get(), kSpencerTestRequest->clone(), expirationTime));
+        _opCtx.get(), std::move(kSpencerTestRequest->clone().getValue()), expirationTime));
 
     // Assert that advancing the clock by 30 minutes does not trigger expiration.
     auto clock = clockSource();
@@ -1664,7 +1665,7 @@ TEST_F(AuthorizationSessionTest, ExpiredSessionWithReauth) {
     // Authorize the same user again to simulate re-login.
     expirationTime += Hours(2);
     ASSERT_OK(authzSession->addAndAuthorizeUser(
-        _opCtx.get(), kSpencerTestRequest->clone(), expirationTime));
+        _opCtx.get(), std::move(kSpencerTestRequest->clone().getValue()), expirationTime));
     assertActive(testFooCollResource, ActionType::insert);
 
     // Expire the user again, this time by setting clock to the exact expiration time boundary.
@@ -1673,8 +1674,8 @@ TEST_F(AuthorizationSessionTest, ExpiredSessionWithReauth) {
     assertExpired(testFooCollResource, ActionType::insert);
 
     // Assert that a different user cannot log in on the expired connection.
-    ASSERT_NOT_OK(
-        authzSession->addAndAuthorizeUser(_opCtx.get(), kAdminAdminRequest->clone(), boost::none));
+    ASSERT_NOT_OK(authzSession->addAndAuthorizeUser(
+        _opCtx.get(), std::move(kAdminAdminRequest->clone().getValue()), boost::none));
     assertExpired(testFooCollResource, ActionType::insert);
 
     // Check that explicit logout from an expired connection works as expected.
@@ -1713,7 +1714,7 @@ TEST_F(AuthorizationSessionTest, ExpirationWithSecurityTokenNOK) {
                                          const Date_t& expect) {
             auth::ValidatedTenancyScope::set(_opCtx.get(), validatedTenancyScope);
             ASSERT_OK(authzSession->addAndAuthorizeUser(
-                _opCtx.get(), kTenant1UserTestRequest->clone(), expire));
+                _opCtx.get(), std::move(kTenant1UserTestRequest->clone().getValue()), expire));
             ASSERT_EQ(authzSession->getExpiration(), expect);
 
             // Reset for next test.
@@ -1738,7 +1739,7 @@ TEST_F(AuthorizationSessionTest, ExpirationWithSecurityTokenNOK) {
         // Perform authentication checks.
         auth::ValidatedTenancyScope::set(_opCtx.get(), validatedTenancyScope);
         ASSERT_OK(authzSession->addAndAuthorizeUser(
-            _opCtx.get(), kTenant1UserTestRequest->clone(), boost::none));
+            _opCtx.get(), std::move(kTenant1UserTestRequest->clone().getValue()), boost::none));
 
         // Assert that the session is authenticated and authorized as expected.
         assertSecurityToken(testTenant1FooCollResource, ActionType::insert);
@@ -1749,7 +1750,7 @@ TEST_F(AuthorizationSessionTest, ExpirationWithSecurityTokenNOK) {
 
         // Assert that another user can't be authorized while the security token is auth'd.
         ASSERT_NOT_OK(authzSession->addAndAuthorizeUser(
-            _opCtx.get(), kUser1TestRequest->clone(), boost::none));
+            _opCtx.get(), std::move(kUser1TestRequest->clone().getValue()), boost::none));
 
         // Check that starting a new request without the security token decoration results in token
         // user logout.
@@ -1763,7 +1764,7 @@ TEST_F(AuthorizationSessionTest, ExpirationWithSecurityTokenNOK) {
             boost::none, "anydb"_sd, "somecollection"_sd);
         const auto kSomeCollRsrc = ResourcePattern::forExactNamespace(kSomeCollNss);
         ASSERT_OK(authzSession->addAndAuthorizeUser(
-            _opCtx.get(), kUser1TestRequest->clone(), Date_t() + Hours{1}));
+            _opCtx.get(), std::move(kUser1TestRequest->clone().getValue()), Date_t() + Hours{1}));
         assertActive(kSomeCollRsrc, ActionType::insert);
 
         // Check that logout proceeds normally.
@@ -1784,7 +1785,7 @@ TEST_F(AuthorizationSessionTest, ExpirationWithSecurityTokenNOK) {
         auth::ValidatedTenancyScope::set(_opCtx.get(), validatedTenancyScope);
 
         ASSERT_OK(authzSession->addAndAuthorizeUser(
-            _opCtx.get(), kTenant2UserTestRequest->clone(), boost::none));
+            _opCtx.get(), std::move(kTenant2UserTestRequest->clone().getValue()), boost::none));
 
         // Ensure that even though it has the readWriteAny role, this user only has privileges on
         // collections with matching tenant ID.
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 b7ae6b2d6d1..406c7859e82 100644
--- a/src/mongo/db/auth/authz_manager_external_state_local.cpp
+++ b/src/mongo/db/auth/authz_manager_external_state_local.cpp
@@ -332,7 +332,12 @@ StatusWith<User> AuthzManagerExternalStateLocal::getUserObject(
     const UserRequest& userReq,
     const SharedUserAcquisitionStats& userAcquisitionStats) try {
     std::vector<RoleName> directRoles;
-    User user(userReq.clone());
+
+    auto swReq = userReq.clone();
+    if (!swReq.isOK()) {
+        return swReq.getStatus();
+    }
+    User user(std::move(swReq.getValue()));
 
     const UserRequest* request = user.getUserRequest();
     const UserName& userName = request->getUserName();
diff --git a/src/mongo/db/auth/authz_manager_external_state_s.cpp b/src/mongo/db/auth/authz_manager_external_state_s.cpp
index 2a12af8cc6e..db395bdfc89 100644
--- a/src/mongo/db/auth/authz_manager_external_state_s.cpp
+++ b/src/mongo/db/auth/authz_manager_external_state_s.cpp
@@ -127,7 +127,12 @@ StatusWith<User> AuthzManagerExternalStateMongos::getUserObject(
         return status;
     }
 
-    User user(userReq.clone());
+    auto swReq = userReq.clone();
+    if (!swReq.isOK()) {
+        return swReq.getStatus();
+    }
+
+    User user(std::move(swReq.getValue()));
     V2UserDocumentParser dp;
     dp.setTenantId(getActiveTenant(opCtx));
     status = dp.initializeUserFromUserDocument(userDoc, &user);
diff --git a/src/mongo/db/auth/sasl_commands.cpp b/src/mongo/db/auth/sasl_commands.cpp
index 2baa28bacd0..0e30d9eadb4 100644
--- a/src/mongo/db/auth/sasl_commands.cpp
+++ b/src/mongo/db/auth/sasl_commands.cpp
@@ -194,7 +194,7 @@ SaslReply doSaslStep(OperationContext* opCtx,
     }
 
     if (mechanism.isSuccess()) {
-        auto request = mechanism.makeUserRequest();
+        auto request = uassertStatusOK(mechanism.makeUserRequest());
         auto expirationTime = mechanism.getExpirationTime();
         uassertStatusOK(AuthorizationSession::get(opCtx->getClient())
                             ->addAndAuthorizeUser(opCtx, std::move(request), expirationTime));
diff --git a/src/mongo/db/auth/sasl_mechanism_registry.h b/src/mongo/db/auth/sasl_mechanism_registry.h
index 624a58c0a80..f073c1d0cd9 100644
--- a/src/mongo/db/auth/sasl_mechanism_registry.h
+++ b/src/mongo/db/auth/sasl_mechanism_registry.h
@@ -247,9 +247,9 @@ public:
     /**
      * Create a UserRequest to send to AuthorizationSession.
      */
-    virtual std::unique_ptr<UserRequest> makeUserRequest() const {
-        return std::make_unique<UserRequestGeneral>(
-            UserName(getPrincipalName(), getAuthenticationDatabase()), boost::none);
+    virtual StatusWith<std::unique_ptr<UserRequest>> makeUserRequest() const {
+        return std::unique_ptr<UserRequest>(std::make_unique<UserRequestGeneral>(
+            UserName(getPrincipalName(), getAuthenticationDatabase()), boost::none));
     }
 
 protected:
diff --git a/src/mongo/db/auth/sasl_plain_server_conversation.cpp b/src/mongo/db/auth/sasl_plain_server_conversation.cpp
index 1f931a4aa31..25adcdd3412 100644
--- a/src/mongo/db/auth/sasl_plain_server_conversation.cpp
+++ b/src/mongo/db/auth/sasl_plain_server_conversation.cpp
@@ -140,7 +140,7 @@ StatusWith<std::tuple<bool, std::string>> SASLPlainServerMechanism::stepImpl(
     }
 
     // The authentication database is also the source database for the user.
-    auto swUser = [&]() {
+    auto swUser = [&]() -> StatusWith<UserHandle> {
         if (gEnableDetailedConnectionHealthMetricLogLines.load()) {
             ScopedCallbackTimer timer([&](Microseconds elapsed) {
                 LOGV2(6788606,
@@ -150,7 +150,12 @@ StatusWith<std::tuple<bool, std::string>> SASLPlainServerMechanism::stepImpl(
             });
         }
 
-        return authManager->acquireUser(opCtx, makeUserRequest());
+        auto swRequest = makeUserRequest();
+        if (!swRequest.isOK()) {
+            return swRequest.getStatus();
+        }
+
+        return authManager->acquireUser(opCtx, std::move(swRequest.getValue()));
     }();
 
     if (!swUser.isOK()) {
diff --git a/src/mongo/db/auth/sasl_x509_server_conversation.cpp b/src/mongo/db/auth/sasl_x509_server_conversation.cpp
index 90eb0cbe96c..26e9df87a0e 100644
--- a/src/mongo/db/auth/sasl_x509_server_conversation.cpp
+++ b/src/mongo/db/auth/sasl_x509_server_conversation.cpp
@@ -99,12 +99,12 @@ std::string getUserName(Client* client, StringData inputData, const SSLPeerInfo&
 
 }  // namespace
 
-std::unique_ptr<UserRequest> SaslX509ServerMechanism::makeUserRequest() const {
+StatusWith<std::unique_ptr<UserRequest>> SaslX509ServerMechanism::makeUserRequest() const {
     std::unique_ptr<UserRequest> request = std::make_unique<UserRequestGeneral>(
         UserName(getPrincipalName(), getAuthenticationDatabase()), boost::none);
 
     if (!haveClient()) {
-        return request;
+        return std::move(request);
     }
 
     // TODO: SERVER-72648 - pass opCtx to this function
@@ -122,21 +122,21 @@ std::unique_ptr<UserRequest> SaslX509ServerMechanism::makeUserRequest() const {
     }
 
     if (!allowRolesFromX509Certificates || !session) {
-        return request;
+        return std::move(request);
     }
 
     const auto& sslPeerInfo = SSLPeerInfo::forSession(session);
     auto&& peerRoles = sslPeerInfo.roles();
     if (peerRoles.empty() ||
         (sslPeerInfo.subjectName().toString() != request->getUserName().getUser())) {
-        return request;
+        return std::move(request);
     }
 
     std::set<RoleName> requestRoles;
     std::copy(
         peerRoles.begin(), peerRoles.end(), std::inserter(requestRoles, requestRoles.begin()));
 
-    return std::make_unique<UserRequestX509>(
+    return UserRequestX509::makeUserRequestX509(
         UserName(getPrincipalName(), getAuthenticationDatabase()),
         std::move(requestRoles),
         sslPeerInfo);
diff --git a/src/mongo/db/auth/sasl_x509_server_conversation.h b/src/mongo/db/auth/sasl_x509_server_conversation.h
index 49f7e260e79..408f1569218 100644
--- a/src/mongo/db/auth/sasl_x509_server_conversation.h
+++ b/src/mongo/db/auth/sasl_x509_server_conversation.h
@@ -54,7 +54,7 @@ public:
 
     bool isClusterMember(Client* client) const override;
 
-    std::unique_ptr<UserRequest> makeUserRequest() const override;
+    StatusWith<std::unique_ptr<UserRequest>> makeUserRequest() const override;
 
 private:
     static constexpr unsigned int kMaxStep = 1;
diff --git a/src/mongo/db/auth/user.h b/src/mongo/db/auth/user.h
index b1a7b7d1032..52d48f3847e 100644
--- a/src/mongo/db/auth/user.h
+++ b/src/mongo/db/auth/user.h
@@ -126,7 +126,13 @@ public:
     virtual const boost::optional<std::set<RoleName>>& getRoles() const = 0;
     virtual UserRequestType getType() const = 0;
     virtual void setRoles(boost::optional<std::set<RoleName>> roles) = 0;
-    virtual std::unique_ptr<UserRequest> clone() const = 0;
+    virtual StatusWith<std::unique_ptr<UserRequest>> clone() const = 0;
+
+    /**
+     * Version of clone that clones the UserRequest by erasing the roles from
+     * the document and re-fetching the roles for OIDC / X509.
+     */
+    virtual StatusWith<std::unique_ptr<UserRequest>> cloneForReacquire() const = 0;
     virtual UserRequestCacheKey generateUserRequestCacheKey() const = 0;
 
     static std::vector<std::string> getUserNameAndRolesVector(
@@ -158,8 +164,14 @@ public:
         this->roles = std::move(roles);
     }
 
-    std::unique_ptr<UserRequest> clone() const override {
-        return std::make_unique<UserRequestGeneral>(getUserName(), getRoles());
+    StatusWith<std::unique_ptr<UserRequest>> clone() const override {
+        return std::unique_ptr<UserRequest>(
+            std::make_unique<UserRequestGeneral>(getUserName(), getRoles()));
+    }
+
+    StatusWith<std::unique_ptr<UserRequest>> cloneForReacquire() const override {
+        return std::unique_ptr<UserRequest>(
+            std::make_unique<UserRequestGeneral>(getUserName(), boost::none));
     }
 
     UserRequestCacheKey generateUserRequestCacheKey() const override;
diff --git a/src/mongo/db/auth/user_request_x509.cpp b/src/mongo/db/auth/user_request_x509.cpp
index b1ee435ba5b..ef19943435b 100644
--- a/src/mongo/db/auth/user_request_x509.cpp
+++ b/src/mongo/db/auth/user_request_x509.cpp
@@ -35,12 +35,41 @@ namespace mongo {
 
 #ifdef MONGO_CONFIG_SSL
 
+StatusWith<std::unique_ptr<UserRequest>> UserRequestX509::makeUserRequestX509(
+    UserName name,
+    boost::optional<std::set<RoleName>> roles,
+    const SSLPeerInfo& peerInfo,
+    bool forReacquire) {
+    auto request =
+        std::make_unique<UserRequestX509>(std::move(name), std::move(roles), std::move(peerInfo));
+
+    if (!forReacquire) {
+        return std::unique_ptr<UserRequest>(std::move(request));
+    }
+
+    request->_tryAcquireRoles();
+    return std::unique_ptr<UserRequest>(std::move(request));
+}
+
 UserRequest::UserRequestCacheKey UserRequestX509::generateUserRequestCacheKey() const {
     auto hashElements = getUserNameAndRolesVector(getUserName(), getRoles());
     getPeerInfo().appendPeerInfoToVector(hashElements);
     return UserRequestCacheKey(getUserName(), hashElements);
 }
 
+void UserRequestX509::_tryAcquireRoles() {
+    auto&& peerRoles = getPeerInfo().roles();
+    if (peerRoles.empty()) {
+        return;
+    }
+
+    std::set<RoleName> requestRoles;
+    std::copy(
+        peerRoles.begin(), peerRoles.end(), std::inserter(requestRoles, requestRoles.begin()));
+
+    setRoles(std::move(requestRoles));
+}
+
 #endif  // MONGO_CONFIG_SSL
 
 }  // namespace mongo
diff --git a/src/mongo/db/auth/user_request_x509.h b/src/mongo/db/auth/user_request_x509.h
index dd4b696c2e3..b77d1dd06c6 100644
--- a/src/mongo/db/auth/user_request_x509.h
+++ b/src/mongo/db/auth/user_request_x509.h
@@ -29,11 +29,12 @@
 
 #pragma once
 
-#include "mongo/db/auth/user.h"
 
 #include <boost/optional.hpp>
 #include <boost/optional/optional.hpp>
 
+#include "mongo/db/auth/user.h"
+#include "mongo/db/auth/user_name.h"
 #include "mongo/util/net/ssl_peer_info.h"
 
 namespace mongo {
@@ -43,25 +44,55 @@ namespace mongo {
 /**
  * This is the version of UserRequest that is used by X509. It provides
  * a way to store X509 metadata for retrieving roles.
+ *
+ * When constructing the UserRequestX509, you must use the static function
+ * makeUserRequestX509. It will automatically populate the roles from the
+ * SSLPeerInfo struct if they exist.
  */
 class UserRequestX509 : public UserRequestGeneral {
 public:
-    UserRequestX509(UserName name,
-                    boost::optional<std::set<RoleName>> roles,
-                    const SSLPeerInfo& peerInfo)
-        : UserRequestGeneral(std::move(name), std::move(roles)), _peerInfo(peerInfo) {}
+    // We define this function as a friend so that makeUserRequestX509
+    // can use it.
+    friend std::unique_ptr<UserRequestX509> std::make_unique<UserRequestX509>(
+        mongo::UserName&& name,
+        boost::optional<std::set<mongo::RoleName>>&& roles,
+        const mongo::SSLPeerInfo&& peerInfo);
+
+    /**
+     * Makes a new UserRequestX509. Toggling for re-acquire to true enables
+     * a re-fetch of the roles from the certificate.
+     */
+    static StatusWith<std::unique_ptr<UserRequest>> makeUserRequestX509(
+        UserName name,
+        boost::optional<std::set<RoleName>> roles,
+        const SSLPeerInfo& peerInfo,
+        bool forReacquire = true);
+
     UserRequestType getType() const final {
         return UserRequestType::X509;
     }
     const SSLPeerInfo& getPeerInfo() const {
         return _peerInfo;
     }
-    std::unique_ptr<UserRequest> clone() const final {
-        return std::make_unique<UserRequestX509>(getUserName(), getRoles(), getPeerInfo());
+    StatusWith<std::unique_ptr<UserRequest>> clone() const final {
+        return makeUserRequestX509(getUserName(), getRoles(), getPeerInfo(), false);
     }
+
+    StatusWith<std::unique_ptr<UserRequest>> cloneForReacquire() const final {
+        return makeUserRequestX509(getUserName(), getRoles(), getPeerInfo());
+    }
+
     UserRequestCacheKey generateUserRequestCacheKey() const final;
 
+protected:
+    UserRequestX509(UserName name,
+                    boost::optional<std::set<RoleName>> roles,
+                    const SSLPeerInfo& peerInfo)
+        : UserRequestGeneral(std::move(name), std::move(roles)), _peerInfo(peerInfo) {}
+
 private:
+    void _tryAcquireRoles();
+
     const SSLPeerInfo& _peerInfo;
 };
 
diff --git a/src/mongo/db/commands/authentication_commands.cpp b/src/mongo/db/commands/authentication_commands.cpp
index 7264617c988..f3903628ff1 100644
--- a/src/mongo/db/commands/authentication_commands.cpp
+++ b/src/mongo/db/commands/authentication_commands.cpp
@@ -174,7 +174,7 @@ std::unique_ptr<UserRequest> getX509UserRequest(OperationContext* opCtx, const U
     auto roles = std::set<RoleName>();
     std::copy(peerRoles.begin(), peerRoles.end(), std::inserter(roles, roles.begin()));
 
-    return std::make_unique<UserRequestX509>(username, roles, sslPeerInfo);
+    return uassertStatusOK(UserRequestX509::makeUserRequestX509(username, roles, sslPeerInfo));
 }
 
 constexpr auto kX509AuthenticationDisabledMessage = "x.509 authentication is disabled."_sd;