Repository: mesos Updated Branches: refs/heads/master ad2a80ef6 -> b9998d147
Changed the ObjectApprover interface for optional subject and objects. Review 47505 changed the Request.object and subject to become optional fields.This patch adapts the ObjectApprover interface to deal with this behavior. Review: https://reviews.apache.org/r/48101/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/b9998d14 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/b9998d14 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/b9998d14 Branch: refs/heads/master Commit: b9998d1473d50fe8fe4001c69c44dbfba16d7fea Parents: ad2a80e Author: Joerg Schad <jo...@mesosphere.io> Authored: Tue May 31 20:57:18 2016 -0600 Committer: Vinod Kone <vinodk...@gmail.com> Committed: Tue May 31 20:57:18 2016 -0600 ---------------------------------------------------------------------- include/mesos/authorizer/authorizer.hpp | 5 +- src/authorizer/local/authorizer.cpp | 208 ++++++++++++++------------- src/authorizer/local/authorizer.hpp | 2 +- src/master/http.cpp | 2 +- src/tests/mesos.cpp | 2 +- src/tests/mesos.hpp | 2 +- 6 files changed, 114 insertions(+), 107 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/b9998d14/include/mesos/authorizer/authorizer.hpp ---------------------------------------------------------------------- diff --git a/include/mesos/authorizer/authorizer.hpp b/include/mesos/authorizer/authorizer.hpp index ea49681..452a92b 100644 --- a/include/mesos/authorizer/authorizer.hpp +++ b/include/mesos/authorizer/authorizer.hpp @@ -72,7 +72,8 @@ public: * NOTE: As this function can be used synchronously by actors * it is essential that it does not block! */ - virtual Try<bool> approved(const Object& object) const noexcept = 0; + virtual Try<bool> approved( + const Option<Object>& object) const noexcept = 0; virtual ~ObjectApprover() = default; }; @@ -153,7 +154,7 @@ public: * @return An `ObjectApprover` for the given `subject` and `action`. */ virtual process::Future<process::Owned<ObjectApprover>> getObjectApprover( - const authorization::Subject& subject, + const Option<authorization::Subject>& subject, const authorization::Action& action) = 0; protected: http://git-wip-us.apache.org/repos/asf/mesos/blob/b9998d14/src/authorizer/local/authorizer.cpp ---------------------------------------------------------------------- diff --git a/src/authorizer/local/authorizer.cpp b/src/authorizer/local/authorizer.cpp index 5f73f03..e88027d 100644 --- a/src/authorizer/local/authorizer.cpp +++ b/src/authorizer/local/authorizer.cpp @@ -168,7 +168,7 @@ class LocalAuthorizerObjectApprover : public ObjectApprover public: LocalAuthorizerObjectApprover( const vector<GenericACL>& acls, - const authorization::Subject& subject, + const Option<authorization::Subject>& subject, const authorization::Action& action, const bool permissive) : acls_(acls), @@ -177,11 +177,11 @@ public: permissive_(permissive) {} virtual Try<bool> approved( - const ObjectApprover::Object& object) const noexcept override { + const Option<ObjectApprover::Object>& object) const noexcept override { // Construct subject. ACL::Entity aclSubject; - if (subject_.has_value()) { - aclSubject.add_values(subject_.value()); + if (subject_.isSome()) { + aclSubject.add_values(subject_->value()); aclSubject.set_type(mesos::ACL::Entity::SOME); } else { aclSubject.set_type(mesos::ACL::Entity::ANY); @@ -189,112 +189,114 @@ public: // Construct object. ACL::Entity aclObject; - switch (action_) { - // All actions using `object.value` for authorization. - // Missing `object.value` implies 'ANY'. - case authorization::REGISTER_FRAMEWORK_WITH_ROLE: - case authorization::TEARDOWN_FRAMEWORK_WITH_PRINCIPAL: - case authorization::RUN_TASK_WITH_USER: - case authorization::RESERVE_RESOURCES_WITH_ROLE: - case authorization::UNRESERVE_RESOURCES_WITH_PRINCIPAL: - case authorization::CREATE_VOLUME_WITH_ROLE: - case authorization::DESTROY_VOLUME_WITH_PRINCIPAL: - case authorization::GET_QUOTA_WITH_ROLE: - case authorization::UPDATE_QUOTA_WITH_ROLE: - case authorization::SET_QUOTA_WITH_ROLE: - case authorization::DESTROY_QUOTA_WITH_PRINCIPAL: - case authorization::GET_WEIGHT_WITH_ROLE: - case authorization::UPDATE_WEIGHT_WITH_ROLE: - case authorization::GET_ENDPOINT_WITH_PATH: { - // Construct object. - if (object.value != NULL) { - aclObject.add_values(*(object.value)); + + if (object.isNone()) { + aclObject.set_type(mesos::ACL::Entity::ANY); + } else { + switch (action_) { + // All actions using `object.value` for authorization. + case authorization::REGISTER_FRAMEWORK_WITH_ROLE: + case authorization::TEARDOWN_FRAMEWORK_WITH_PRINCIPAL: + case authorization::RUN_TASK_WITH_USER: + case authorization::RESERVE_RESOURCES_WITH_ROLE: + case authorization::UNRESERVE_RESOURCES_WITH_PRINCIPAL: + case authorization::CREATE_VOLUME_WITH_ROLE: + case authorization::DESTROY_VOLUME_WITH_PRINCIPAL: + case authorization::GET_QUOTA_WITH_ROLE: + case authorization::UPDATE_QUOTA_WITH_ROLE: + case authorization::SET_QUOTA_WITH_ROLE: + case authorization::DESTROY_QUOTA_WITH_PRINCIPAL: + case authorization::GET_WEIGHT_WITH_ROLE: + case authorization::UPDATE_WEIGHT_WITH_ROLE: + case authorization::GET_ENDPOINT_WITH_PATH: { + // Check object has the required types set. + CHECK_NOTNULL(object->value); + + aclObject.add_values(*(object->value)); aclObject.set_type(mesos::ACL::Entity::SOME); - } else { - aclObject.set_type(mesos::ACL::Entity::ANY); - } - break; - } - case authorization::ACCESS_MESOS_LOG: { - aclObject.set_type(mesos::ACL::Entity::ANY); + break; + } + case authorization::ACCESS_MESOS_LOG: { + aclObject.set_type(mesos::ACL::Entity::ANY); - break; - } - case authorization::ACCESS_SANDBOX: { - // Check object has the required types set. - CHECK_NOTNULL(object.executor_info); - CHECK_NOTNULL(object.framework_info); + break; + } + case authorization::ACCESS_SANDBOX: { + // Check object has the required types set. + CHECK_NOTNULL(object->executor_info); + CHECK_NOTNULL(object->framework_info); + + if (object->executor_info->command().has_user()) { + aclObject.add_values(object->executor_info->command().user()); + aclObject.set_type(mesos::ACL::Entity::SOME); + } else { + aclObject.add_values(object->framework_info->user()); + aclObject.set_type(mesos::ACL::Entity::SOME); + } - if (object.executor_info->command().has_user()) { - aclObject.add_values(object.executor_info->command().user()); - aclObject.set_type(mesos::ACL::Entity::SOME); - } else { - aclObject.add_values(object.framework_info->user()); - aclObject.set_type(mesos::ACL::Entity::SOME); + break; } + case authorization::VIEW_FRAMEWORK: { + // Check object has the required types set. + CHECK_NOTNULL(object->framework_info); - break; - } - case authorization::VIEW_FRAMEWORK: { - // Check object has the required types set. - CHECK_NOTNULL(object.framework_info); + aclObject.add_values(object->framework_info->user()); + aclObject.set_type(mesos::ACL::Entity::SOME); - aclObject.add_values(object.framework_info->user()); - aclObject.set_type(mesos::ACL::Entity::SOME); + break; + } + case authorization::VIEW_TASK: { + CHECK(object->task != NULL || object->task_info != NULL); + CHECK_NOTNULL(object->framework_info); + + // First we consider either whether `Task` or `TaskInfo` + // have `user` set. As fallback we use `FrameworkInfo.user`. + Option<string> taskUser = None(); + if (object->task != NULL && object->task->has_user()) { + taskUser = object->task->user(); + } else if (object->task_info != NULL) { + // Within TaskInfo the user can be either set in `command` + // or `executor.command`. + if (object->task_info->has_command() && + object->task_info->command().has_user()) { + taskUser = object->task_info->command().user(); + } else if (object->task_info->has_executor() && + object->task_info->executor().command().has_user()) { + taskUser = object->task_info->executor().command().user(); + } + } - break; - } - case authorization::VIEW_TASK: { - CHECK(object.task != NULL || object.task_info != NULL); - CHECK_NOTNULL(object.framework_info); - - // First we consider either whether `Task` or `TaskInfo` - // have `user` set. As fallback we use `FrameworkInfo.user`. - Option<string> taskUser = None(); - if (object.task != NULL && object.task->has_user()) { - taskUser = object.task->user(); - } else if (object.task_info != NULL) { - // Within TaskInfo the user can be either set in `command` - // or `executor.command`. - if (object.task_info->has_command() && - object.task_info->command().has_user()) { - taskUser = object.task_info->command().user(); - } else if (object.task_info->has_executor() && - object.task_info->executor().command().has_user()) { - taskUser = object.task_info->executor().command().user(); + // In case there is no `user` set on task level we fallback + // to the `FrameworkInfo.user`. + if (taskUser.isNone()) { + taskUser = object->framework_info->user(); } - } + aclObject.add_values(taskUser.get()); + aclObject.set_type(mesos::ACL::Entity::SOME); - // In case there is no `user` set on task level we fallback - // to the `FrameworkInfo.user`. - if (taskUser.isNone()) { - taskUser = object.framework_info->user(); + break; } - aclObject.add_values(taskUser.get()); - aclObject.set_type(mesos::ACL::Entity::SOME); - - break; - } - case authorization::VIEW_EXECUTOR: { - CHECK_NOTNULL(object.executor_info); - CHECK_NOTNULL(object.framework_info); + case authorization::VIEW_EXECUTOR: { + CHECK_NOTNULL(object->executor_info); + CHECK_NOTNULL(object->framework_info); + + if (object->executor_info->command().has_user()) { + aclObject.add_values(object->executor_info->command().user()); + aclObject.set_type(mesos::ACL::Entity::SOME); + } else { + aclObject.add_values(object->framework_info->user()); + aclObject.set_type(mesos::ACL::Entity::SOME); + } - if (object.executor_info->command().has_user()) { - aclObject.add_values(object.executor_info->command().user()); - aclObject.set_type(mesos::ACL::Entity::SOME); - } else { - aclObject.add_values(object.framework_info->user()); - aclObject.set_type(mesos::ACL::Entity::SOME); + break; } - - break; + case authorization::UNKNOWN: + LOG(WARNING) << "Authorization for action '" << action_ + << "' is not defined and therefore not authorized"; + return false; + break; } - case authorization::UNKNOWN: - LOG(WARNING) << "Authorization for action '" << action_ - << "' is not defined and therefore not authorized"; - return false; - break; } return approved(acls_, aclSubject, aclObject); @@ -317,7 +319,7 @@ private: } const vector<GenericACL> acls_; - const authorization::Subject subject_; + const Option<authorization::Subject> subject_; const authorization::Action action_; const bool permissive_; }; @@ -368,7 +370,11 @@ public: { return getObjectApprover(request.subject(), request.action()) .then([=](const Owned<ObjectApprover>& objectApprover) -> Future<bool> { - ObjectApprover::Object object(request.object()); + Option<ObjectApprover::Object> object = None(); + if (request.has_object()) { + object = ObjectApprover::Object(request.object()); + } + Try<bool> result = objectApprover->approved(object); if (result.isError()) { return Failure(result.error()); @@ -378,7 +384,7 @@ public: } Future<Owned<ObjectApprover>> getObjectApprover( - const authorization::Subject& subject, + const Option<authorization::Subject>& subject, const authorization::Action& action) { // Implementation of the ObjectApprover interface denying all objects. @@ -386,7 +392,7 @@ public: { public: virtual Try<bool> approved( - const ObjectApprover::Object& object) const noexcept override + const Option<ObjectApprover::Object>& object) const noexcept override { return false; } @@ -794,7 +800,7 @@ process::Future<bool> LocalAuthorizer::authorized( Future<Owned<ObjectApprover>> LocalAuthorizer::getObjectApprover( - const authorization::Subject& subject, + const Option<authorization::Subject>& subject, const authorization::Action& action) { return dispatch( http://git-wip-us.apache.org/repos/asf/mesos/blob/b9998d14/src/authorizer/local/authorizer.hpp ---------------------------------------------------------------------- diff --git a/src/authorizer/local/authorizer.hpp b/src/authorizer/local/authorizer.hpp index 9242889..794443a 100644 --- a/src/authorizer/local/authorizer.hpp +++ b/src/authorizer/local/authorizer.hpp @@ -66,7 +66,7 @@ public: const authorization::Request& request); virtual process::Future<process::Owned<ObjectApprover>> getObjectApprover( - const authorization::Subject& subject, + const Option<authorization::Subject>& subject, const authorization::Action& action); private: http://git-wip-us.apache.org/repos/asf/mesos/blob/b9998d14/src/master/http.cpp ---------------------------------------------------------------------- diff --git a/src/master/http.cpp b/src/master/http.cpp index 1245326..824c6e5 100644 --- a/src/master/http.cpp +++ b/src/master/http.cpp @@ -170,7 +170,7 @@ class AcceptingObjectApprover : public ObjectApprover { public: virtual Try<bool> approved( - const ObjectApprover::Object& object) const noexcept override + const Option<ObjectApprover::Object>& object) const noexcept override { return true; } http://git-wip-us.apache.org/repos/asf/mesos/blob/b9998d14/src/tests/mesos.cpp ---------------------------------------------------------------------- diff --git a/src/tests/mesos.cpp b/src/tests/mesos.cpp index 6c64bd5..51d223a 100644 --- a/src/tests/mesos.cpp +++ b/src/tests/mesos.cpp @@ -675,7 +675,7 @@ MockAuthorizer::MockAuthorizer() { public: virtual Try<bool> approved( - const ObjectApprover::Object& object) const noexcept override + const Option<ObjectApprover::Object>& object) const noexcept override { return true; } http://git-wip-us.apache.org/repos/asf/mesos/blob/b9998d14/src/tests/mesos.hpp ---------------------------------------------------------------------- diff --git a/src/tests/mesos.hpp b/src/tests/mesos.hpp index 5d7b5d1..259c24d 100644 --- a/src/tests/mesos.hpp +++ b/src/tests/mesos.hpp @@ -1716,7 +1716,7 @@ public: MOCK_METHOD2( getObjectApprover, process::Future<process::Owned<ObjectApprover>>( - const authorization::Subject& subject, + const Option<authorization::Subject>& subject, const authorization::Action& action)); };
