diff options
author | Kyle Suarez <kyle.suarez@mongodb.com> | 2016-12-06 17:15:36 -0500 |
---|---|---|
committer | Kyle Suarez <kyle.suarez@mongodb.com> | 2016-12-06 17:18:36 -0500 |
commit | c891410987af3998f8caf2d6745a070b3ca428d8 (patch) | |
tree | 7328d6b05b02b1439703690a1b7bb61228084485 | |
parent | 90a3e41da4a7d9f0c9fae08e2babdb33600dc916 (diff) | |
download | mongo-c891410987af3998f8caf2d6745a070b3ca428d8.tar.gz |
SERVER-27194 must specify both viewOn and pipeline if modifying view when auth enabled
(cherry picked from commit 7ce8f181b87685fbcf6cc93749334c85ce36554e)
-rw-r--r-- | jstests/auth/views_authz.js | 11 | ||||
-rw-r--r-- | src/mongo/db/auth/authorization_session.cpp | 67 | ||||
-rw-r--r-- | src/mongo/db/auth/authorization_session.h | 7 |
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); |