SERVER-27194 must specify both viewOn and pipeline if modifying view when auth enabled

(cherry picked from commit 7ce8f181b87685fbcf6cc93749334c85ce36554e)

3 files changed, 52 insertions, 33 deletions

diff --git a/jstests/auth/views_authz.js b/jstests/auth/views_authz.js
index 605b166be41..6e797be6c8e 100644
--- a/jstests/auth/views_authz.js
+++ b/jstests/auth/views_authz.js
@@ -103,6 +103,17 @@
             ErrorCodes.Unauthorized,
             "modified a view to read an unreadable collection via $graphLookup in a $facet");
 
+        // When auth is enabled, users must specify both "viewOn" and "pipeline" when running
+        // collMod on a view; specifying only one or the other is not allowed. Without both the
+        // "viewOn" and "pipeline" specified, authorization checks cannot determine if the users
+        // have the necessary privileges.
+        assert.commandFailedWithCode(viewsDB.runCommand({collMod: "view", pipeline: []}),
+                                     ErrorCodes.InvalidOptions,
+                                     "modified a view without having to specify 'viewOn'");
+        assert.commandFailedWithCode(viewsDB.runCommand({collMod: "view", viewOn: "other"}),
+                                     ErrorCodes.InvalidOptions,
+                                     "modified a view without having to specify 'pipeline'");
+
         // Performing a find on a readable view returns a cursor that allows us to perform a getMore
         // even if the underlying collection is unreadable.
         // TODO(SERVER-24771): getMore does not work yet for sharded clusters
diff --git a/src/mongo/db/auth/authorization_session.cpp b/src/mongo/db/auth/authorization_session.cpp
index 6acd07ac4e6..ea70a82907c 100644
--- a/src/mongo/db/auth/authorization_session.cpp
+++ b/src/mongo/db/auth/authorization_session.cpp
@@ -60,6 +60,25 @@ using std::vector;
 
 namespace {
 const std::string ADMIN_DBNAME = "admin";
+
+// Checks if this connection has the privileges necessary to create or modify the view 'viewNs'
+// to be a view on 'viewOnNs' with pipeline 'viewPipeline'. Call this function after verifying
+// that the user has the 'createCollection' or 'collMod' action, respectively.
+Status checkAuthForCreateOrModifyView(AuthorizationSession* authzSession,
+                                      const NamespaceString& viewNs,
+                                      const NamespaceString& viewOnNs,
+                                      const BSONObj& viewPipeline) {
+    // It's safe to allow a user to create or modify a view if they can't read it anyway.
+    if (!authzSession->isAuthorizedForActionsOnNamespace(viewNs, ActionType::find)) {
+        return Status::OK();
+    }
+
+    // This check ignores some invalid pipeline specifications. For example, if a user specifies a
+    // view definition with an invalid specification like {$lookup: "blah"}, the authorization check
+    // will succeed but the pipeline will fail to parse later in Command::run().
+    return authzSession->checkAuthForAggregate(
+        viewOnNs, BSON("aggregate" << viewOnNs.coll() << "pipeline" << viewPipeline));
+}
 }  // namespace
 
 AuthorizationSession::AuthorizationSession(std::unique_ptr<AuthzSessionExternalState> externalState)
@@ -445,7 +464,13 @@ Status AuthorizationSession::checkAuthForCreate(const NamespaceString& ns, const
         if (!hasCreateCollectionAction) {
             return Status(ErrorCodes::Unauthorized, "unauthorized");
         }
-        return checkAuthForCreateOrModifyView(ns, cmdObj);
+
+        // Parse the viewOn namespace and the pipeline. If no pipeline was specified, use the empty
+        // pipeline.
+        NamespaceString viewOnNs(ns.db(), cmdObj["viewOn"].checkAndGetStringData());
+        auto pipeline =
+            cmdObj.hasField("pipeline") ? BSONArray(cmdObj["pipeline"].Obj()) : BSONArray();
+        return checkAuthForCreateOrModifyView(this, ns, viewOnNs, pipeline);
     }
 
     // To create a regular collection, ActionType::createCollection or ActionType::insert are
@@ -462,34 +487,24 @@ Status AuthorizationSession::checkAuthForCollMod(const NamespaceString& ns, cons
         return Status(ErrorCodes::Unauthorized, "unauthorized");
     }
 
-    // Check for additional required privileges if attempting to modify a view.
-    if (cmdObj["viewOn"] || cmdObj["pipeline"]) {
-        return checkAuthForCreateOrModifyView(ns, cmdObj);
+    // Check for additional required privileges if attempting to modify a view. When auth is
+    // enabled, users must specify both "viewOn" and "pipeline" together. This prevents a user from
+    // exposing more information in the original underlying namespace by only changing "pipeline",
+    // or looking up more information via the original pipeline by only changing "viewOn".
+    const bool hasViewOn = cmdObj.hasField("viewOn");
+    const bool hasPipeline = cmdObj.hasField("pipeline");
+    if (hasViewOn != hasPipeline) {
+        return Status(
+            ErrorCodes::InvalidOptions,
+            "Must specify both 'viewOn' and 'pipeline' when modifying a view and auth is enabled");
     }
-
-    return Status::OK();
-}
-
-Status AuthorizationSession::checkAuthForCreateOrModifyView(const NamespaceString& ns,
-                                                            const BSONObj& cmdObj) {
-    // It's safe to allow a user to create or modify a view if they can't read it anyway.
-    if (!isAuthorizedForActionsOnNamespace(ns, ActionType::find)) {
-        return Status::OK();
+    if (hasViewOn) {
+        NamespaceString viewOnNs(ns.db(), cmdObj["viewOn"].checkAndGetStringData());
+        auto viewPipeline = BSONArray(cmdObj["pipeline"].Obj());
+        return checkAuthForCreateOrModifyView(this, ns, viewOnNs, viewPipeline);
     }
 
-    // The user can read the view they're trying to create/modify, so we must ensure that they also
-    // have the find action on all namespaces in "viewOn" and "pipeline". If "pipeline" is not
-    // specified, default to the empty pipeline.
-    auto viewPipeline =
-        cmdObj.hasField("pipeline") ? BSONArray(cmdObj["pipeline"].Obj()) : BSONArray();
-
-
-    // This check ignores some invalid pipeline specifications. For example, if a user specifies a
-    // view definition with an invalid specification like {$lookup: "blah"}, the authorization check
-    // will succeed but the pipeline will fail to parse later in Command::run().
-    NamespaceString viewOnNss(ns.db(), cmdObj["viewOn"].checkAndGetStringData());
-    return checkAuthForAggregate(
-        viewOnNss, BSON("aggregate" << viewOnNss.coll() << "pipeline" << viewPipeline));
+    return Status::OK();
 }
 
 Status AuthorizationSession::checkAuthorizedToGrantPrivilege(const Privilege& privilege) {
diff --git a/src/mongo/db/auth/authorization_session.h b/src/mongo/db/auth/authorization_session.h
index 42e7ecfe060..2c5631705a7 100644
--- a/src/mongo/db/auth/authorization_session.h
+++ b/src/mongo/db/auth/authorization_session.h
@@ -191,13 +191,6 @@ public:
     // supplied in 'cmdObj'.
     Status checkAuthForCollMod(const NamespaceString& ns, const BSONObj& cmdObj);
 
-    // Checks if this connection has the privileges necessary to create or modify the view 'ns'.
-    // Call this function after verifying that the user has the 'createCollection' or 'collMod'
-    // action, respectively.
-    //
-    // 'cmdObj' must have a String field named 'viewOn'.
-    Status checkAuthForCreateOrModifyView(const NamespaceString& ns, const BSONObj& cmdObj);
-
     // Checks if this connection has the privileges necessary to grant the given privilege
     // to a role.
     Status checkAuthorizedToGrantPrivilege(const Privilege& privilege);