SERVER-38984 Validate unique User ID on UserCache hit

8 files changed, 70 insertions, 5 deletions

diff --git a/src/mongo/db/auth/authorization_manager.cpp b/src/mongo/db/auth/authorization_manager.cpp
index 6bbb5f486dc..9d9575c1a08 100644
--- a/src/mongo/db/auth/authorization_manager.cpp
+++ b/src/mongo/db/auth/authorization_manager.cpp
@@ -68,6 +68,7 @@ mongo::AuthInfo mongo::internalSecurity;
 
 namespace mongo {
 
+constexpr StringData AuthorizationManager::USERID_FIELD_NAME;
 constexpr StringData AuthorizationManager::USER_NAME_FIELD_NAME;
 constexpr StringData AuthorizationManager::USER_DB_FIELD_NAME;
 constexpr StringData AuthorizationManager::ROLE_NAME_FIELD_NAME;
diff --git a/src/mongo/db/auth/authorization_manager.h b/src/mongo/db/auth/authorization_manager.h
index f9cd1248d4b..1ec9f421a4c 100644
--- a/src/mongo/db/auth/authorization_manager.h
+++ b/src/mongo/db/auth/authorization_manager.h
@@ -99,6 +99,7 @@ public:
 
     static MONGO_DECLARE_SHIM(()->std::unique_ptr<AuthorizationManager>) create;
 
+    static constexpr StringData USERID_FIELD_NAME = "userId"_sd;
     static constexpr StringData USER_NAME_FIELD_NAME = "user"_sd;
     static constexpr StringData USER_DB_FIELD_NAME = "db"_sd;
     static constexpr StringData ROLE_NAME_FIELD_NAME = "role"_sd;
@@ -267,6 +268,13 @@ public:
                                                const UserName& userName) = 0;
 
     /**
+     * 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;
+
+    /**
      * Marks the given user as invalid and removes it from the user cache.
      */
     virtual void invalidateUserByName(OperationContext* opCtx, const UserName& user) = 0;
diff --git a/src/mongo/db/auth/authorization_manager_impl.cpp b/src/mongo/db/auth/authorization_manager_impl.cpp
index 93ce163d6c6..5230d3b03b4 100644
--- a/src/mongo/db/auth/authorization_manager_impl.cpp
+++ b/src/mongo/db/auth/authorization_manager_impl.cpp
@@ -456,6 +456,8 @@ Status AuthorizationManagerImpl::_initializeUserFromPrivilegeDocument(User* user
                                                 << "\"");
     }
 
+    user->setID(parser.extractUserIDFromUserDocument(privDoc));
+
     Status status = parser.initializeUserCredentialsFromUserDocument(user, privDoc);
     if (!status.isOK()) {
         return status;
@@ -576,6 +578,23 @@ StatusWith<UserHandle> AuthorizationManagerImpl::acquireUser(OperationContext* o
     return user;
 }
 
+StatusWith<UserHandle> AuthorizationManagerImpl::acquireUserForSessionRefresh(
+    OperationContext* opCtx, const UserName& userName, const User::UserId& uid) {
+    auto swUserHandle = acquireUser(opCtx, userName);
+    if (!swUserHandle.isOK()) {
+        return swUserHandle.getStatus();
+    }
+
+    auto ret = std::move(swUserHandle.getValue());
+    if (uid != ret->getID()) {
+        return {ErrorCodes::UserNotFound,
+                str::stream() << "User id from privilege document '" << userName.toString()
+                              << "' does not match user id in session."};
+    }
+
+    return ret;
+}
+
 StatusWith<UserHandle> AuthorizationManagerImpl::_acquireUserSlowPath(CacheGuard& guard,
                                                                       OperationContext* opCtx,
                                                                       const UserName& userName) {
diff --git a/src/mongo/db/auth/authorization_manager_impl.h b/src/mongo/db/auth/authorization_manager_impl.h
index 80f052338ff..66f088b69b6 100644
--- a/src/mongo/db/auth/authorization_manager_impl.h
+++ b/src/mongo/db/auth/authorization_manager_impl.h
@@ -118,6 +118,9 @@ 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;
 
     void invalidateUserByName(OperationContext* opCtx, const UserName& user) override;
 
diff --git a/src/mongo/db/auth/authorization_session_impl.cpp b/src/mongo/db/auth/authorization_session_impl.cpp
index 4bd6f2dd62c..65e7558a7f5 100644
--- a/src/mongo/db/auth/authorization_session_impl.cpp
+++ b/src/mongo/db/auth/authorization_session_impl.cpp
@@ -729,7 +729,7 @@ void AuthorizationSessionImpl::_refreshUserInfoAsNeeded(OperationContext* opCtx)
             UserName name = user->getName();
             UserHandle updatedUser;
 
-            auto swUser = authMan.acquireUser(opCtx, name);
+            auto swUser = authMan.acquireUserForSessionRefresh(opCtx, name, user->getID());
             auto& status = swUser.getStatus();
             switch (status.code()) {
                 case ErrorCodes::OK: {
diff --git a/src/mongo/db/auth/user.h b/src/mongo/db/auth/user.h
index 715b849fe9b..5b3edfb767f 100644
--- a/src/mongo/db/auth/user.h
+++ b/src/mongo/db/auth/user.h
@@ -104,6 +104,15 @@ public:
 
     explicit User(const UserName& name);
 
+    using UserId = std::vector<std::uint8_t>;
+    const UserId& getID() const {
+        return _id;
+    }
+
+    void setID(UserId id) {
+        _id = std::move(id);
+    }
+
     /**
      * Returns the user name for this user.
      */
@@ -230,6 +239,10 @@ protected:
     void _invalidate();
 
 private:
+    // Unique ID (often UUID) for this user.
+    // May be empty for legacy users.
+    UserId _id;
+
     UserName _name;
 
     // Digest of the full username
diff --git a/src/mongo/db/auth/user_document_parser.cpp b/src/mongo/db/auth/user_document_parser.cpp
index 0cbed973483..552898b8552 100644
--- a/src/mongo/db/auth/user_document_parser.cpp
+++ b/src/mongo/db/auth/user_document_parser.cpp
@@ -127,10 +127,18 @@ Status _checkV2RolesArray(const BSONElement& rolesElement) {
 }
 
 Status V2UserDocumentParser::checkValidUserDocument(const BSONObj& doc) const {
-    BSONElement userElement = doc[AuthorizationManager::USER_NAME_FIELD_NAME];
-    BSONElement userDBElement = doc[AuthorizationManager::USER_DB_FIELD_NAME];
-    BSONElement credentialsElement = doc[CREDENTIALS_FIELD_NAME];
-    BSONElement rolesElement = doc[ROLES_FIELD_NAME];
+    auto userIdElement = doc[AuthorizationManager::USERID_FIELD_NAME];
+    auto userElement = doc[AuthorizationManager::USER_NAME_FIELD_NAME];
+    auto userDBElement = doc[AuthorizationManager::USER_DB_FIELD_NAME];
+    auto credentialsElement = doc[CREDENTIALS_FIELD_NAME];
+    auto rolesElement = doc[ROLES_FIELD_NAME];
+
+    // Validate the "userId" element.
+    if (!userIdElement.eoo()) {
+        if (!userIdElement.isBinData(BinDataType::newUUID)) {
+            return _badValue("User document needs 'userId' field to be a UUID");
+        }
+    }
 
     // Validate the "user" element.
     if (userElement.type() != String)
@@ -213,6 +221,18 @@ Status V2UserDocumentParser::checkValidUserDocument(const BSONObj& doc) const {
     return Status::OK();
 }
 
+User::UserId V2UserDocumentParser::extractUserIDFromUserDocument(const BSONObj& doc) const {
+    auto userId = doc[AuthorizationManager::USERID_FIELD_NAME];
+    if (userId.isBinData(BinDataType::newUUID)) {
+        auto id = userId.uuid();
+        User::UserId ret;
+        std::copy(id.begin(), id.end(), std::back_inserter(ret));
+        return ret;
+    }
+
+    return User::UserId();
+}
+
 std::string V2UserDocumentParser::extractUserNameFromUserDocument(const BSONObj& doc) const {
     return doc[AuthorizationManager::USER_NAME_FIELD_NAME].str();
 }
diff --git a/src/mongo/db/auth/user_document_parser.h b/src/mongo/db/auth/user_document_parser.h
index a1349104224..7493ce7d324 100644
--- a/src/mongo/db/auth/user_document_parser.h
+++ b/src/mongo/db/auth/user_document_parser.h
@@ -54,6 +54,7 @@ public:
     static Status parseRoleVector(const BSONArray& rolesArray, std::vector<RoleName>* result);
 
     std::string extractUserNameFromUserDocument(const BSONObj& doc) const;
+    User::UserId extractUserIDFromUserDocument(const BSONObj& doc) const;
 
     Status initializeUserCredentialsFromUserDocument(User* user, const BSONObj& privDoc) const;