From 233d39b231486c3d34f2fc93a8d815469d52255a Mon Sep 17 00:00:00 2001
From: Varun Ravichandran <varun.ravichandran@mongodb.com>
Date: Wed, 22 Feb 2023 22:36:12 +0000
Subject: SERVER-74292: Return errors for command requests with invalid
 impersonated user metadata fields

(cherry picked from commit 70b4adbb66c6396afda5b3fc73a9d61f6015e9c7)
---
 jstests/auth/impersonation-deny.js                 | 45 ++++++++++++++++++----
 .../rpc/metadata/impersonated_user_metadata.cpp    | 14 ++++++-
 2 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/jstests/auth/impersonation-deny.js b/jstests/auth/impersonation-deny.js
index e3a962942ca..4494737299c 100644
--- a/jstests/auth/impersonation-deny.js
+++ b/jstests/auth/impersonation-deny.js
@@ -8,9 +8,8 @@ function testMongod(mongod, systemuserpwd = undefined) {
     const admin = mongod.getDB('admin');
     admin.createUser({user: 'admin', pwd: 'admin', roles: ['root']});
 
-    function assertUnauthorized(cmd, msg) {
-        const errmsg =
-            assert.commandFailedWithCode(admin.runCommand(cmd), ErrorCodes.Unauthorized).errmsg;
+    function assertError(cmd, msg, code) {
+        const errmsg = assert.commandFailedWithCode(admin.runCommand(cmd), code).errmsg;
         assert(errmsg.includes(msg), "Error message is missing '" + msg + "': " + errmsg);
     }
 
@@ -18,7 +17,8 @@ function testMongod(mongod, systemuserpwd = undefined) {
 
     // Localhost authbypass is disabled, and we haven't logged in,
     // so normal auth-required commands should fail.
-    assertUnauthorized({usersInfo: 1}, 'Command usersInfo requires authentication');
+    assertError(
+        {usersInfo: 1}, 'Command usersInfo requires authentication', ErrorCodes.Unauthorized);
 
     // Hello command requires no auth, so it works fine.
     assert.commandWorked(admin.runCommand({hello: 1}));
@@ -33,7 +33,8 @@ function testMongod(mongod, systemuserpwd = undefined) {
             "$impersonatedRoles": [{role: 'root', db: 'admin'}],
         }
     };
-    assertUnauthorized(kImpersonatedHello, 'Unauthorized use of impersonation metadata');
+    assertError(
+        kImpersonatedHello, 'Unauthorized use of impersonation metadata', ErrorCodes.Unauthorized);
 
     // TODO SERVER-72448: Remove
     const kImpersonatedHelloLegacy = {
@@ -43,11 +44,41 @@ function testMongod(mongod, systemuserpwd = undefined) {
             "$impersonatedRoles": [{role: 'root', db: 'admin'}],
         }
     };
-    assertUnauthorized(kImpersonatedHelloLegacy, 'Unauthorized use of impersonation metadata');
+    assertError(kImpersonatedHelloLegacy,
+                'Unauthorized use of impersonation metadata',
+                ErrorCodes.Unauthorized);
+
+    // TODO SERVER-72448: Remove, checks that both legacy and new impersonation metadata fields
+    // cannot be set simultaneously.
+    const kImpersonatedHelloBoth = {
+        hello: 1,
+        "$audit": {
+            "$impersonatedUser": {user: 'admin', db: 'admin'},
+            "$impersonatedUsers": [{user: 'admin', db: 'admin'}],
+            "$impersonatedRoles": [{role: 'root', db: 'admin'}],
+        }
+    };
+    assertError(kImpersonatedHelloBoth,
+                'Cannot specify both $impersonatedUser and $impersonatedUsers',
+                ErrorCodes.BadValue);
+
+    // TODO SERVER-72448: Remove, checks that the legacy impersonation metadata field can only
+    // contain at most 1 field if specified.
+    const kImpersonatedHelloLegacyMultiple = {
+        hello: 1,
+        "$audit": {
+            "$impersonatedUsers": [{user: 'admin', db: 'admin'}, {user: 'test', db: 'pwd'}],
+            "$impersonatedRoles": [{role: 'root', db: 'admin'}],
+        }
+    };
+    assertError(kImpersonatedHelloLegacyMultiple,
+                'Can only impersonate up to one user per connection',
+                ErrorCodes.BadValue);
 
     // Try as admin (root role), should still fail.
     admin.auth('admin', 'admin');
-    assertUnauthorized(kImpersonatedHello, 'Unauthorized use of impersonation metadata');
+    assertError(
+        kImpersonatedHello, 'Unauthorized use of impersonation metadata', ErrorCodes.Unauthorized);
     admin.logout();
 
     if (systemuserpwd !== undefined) {
diff --git a/src/mongo/rpc/metadata/impersonated_user_metadata.cpp b/src/mongo/rpc/metadata/impersonated_user_metadata.cpp
index 9ff13f97c95..944f98088e0 100644
--- a/src/mongo/rpc/metadata/impersonated_user_metadata.cpp
+++ b/src/mongo/rpc/metadata/impersonated_user_metadata.cpp
@@ -59,10 +59,22 @@ void readImpersonatedUserMetadata(const BSONElement& elem, OperationContext* opC
         auto data = ImpersonatedUserMetadata::parse(errCtx, elem.embeddedObject());
 
         // TODO SERVER-72448: Remove the getUsers() pathway
-        const bool userExists = data.getUser() || (data.getUsers() && !data.getUsers()->empty());
+        // In the meantime, we only accept $impersonatedUser OR $impersonatedUsers with exactly 1
+        // user.
+        auto newImpersonatedUser = data.getUser();
+        auto legacyImpersonatedUser = data.getUsers();
+        uassert(ErrorCodes::BadValue,
+                "Cannot specify both $impersonatedUser and $impersonatedUsers",
+                !newImpersonatedUser || !legacyImpersonatedUser);
+        uassert(ErrorCodes::BadValue,
+                "Can only impersonate up to one user per connection",
+                !legacyImpersonatedUser || legacyImpersonatedUser->empty() ||
+                    legacyImpersonatedUser->size() == 1);
 
         // Set the impersonation data only if there are actually impersonated
         // users/roles.
+        const bool userExists =
+            newImpersonatedUser || (legacyImpersonatedUser && !legacyImpersonatedUser->empty());
         if (userExists || !data.getRoles().empty()) {
             newData = std::move(data);
         }
-- 
cgit v1.2.1