Added a test for `MODIFY_RESOURE_PROVIDER_CONFIG` authorization. This patch adds a unit test for the new authorization for adding, updating and removing resource provider config files. It also renames the ACL entity field to fit in the context better.
Review: https://reviews.apache.org/r/64479/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/24d17b29 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/24d17b29 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/24d17b29 Branch: refs/heads/master Commit: 24d17b297a1743fb01414ebb2bd95e8d4bb2b0e9 Parents: 2c606ae Author: Chun-Hung Hsiao <[email protected]> Authored: Mon Dec 11 15:09:42 2017 -0800 Committer: Jie Yu <[email protected]> Committed: Mon Dec 11 15:09:42 2017 -0800 ---------------------------------------------------------------------- include/mesos/authorizer/acls.proto | 6 ++-- src/authorizer/local/authorizer.cpp | 13 +++++-- src/tests/authorization_tests.cpp | 60 ++++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/24d17b29/include/mesos/authorizer/acls.proto ---------------------------------------------------------------------- diff --git a/include/mesos/authorizer/acls.proto b/include/mesos/authorizer/acls.proto index aca9aa8..40a1425 100644 --- a/include/mesos/authorizer/acls.proto +++ b/include/mesos/authorizer/acls.proto @@ -478,8 +478,8 @@ message ACL { // Use Entity type ANY or NONE to allow or deny access. // // TODO(chhsiao): Consider allowing granular permission to act upon - // SOME particular operating system users (e.g., linux users). - required Entity users = 2; + // SOME resource provider types and names. + required Entity resource_providers = 2; } } @@ -556,5 +556,5 @@ message ACLs { repeated ACL.KillStandaloneContainer kill_standalone_container = 42; repeated ACL.WaitStandaloneContainer wait_standalone_container = 43; repeated ACL.RemoveStandaloneContainer remove_standalone_container = 44; - repeated ACL.ModifyResourceProviderConfig modify_resource_provider_config = 45; + repeated ACL.ModifyResourceProviderConfig modify_resource_provider_configs = 45; } http://git-wip-us.apache.org/repos/asf/mesos/blob/24d17b29/src/authorizer/local/authorizer.cpp ---------------------------------------------------------------------- diff --git a/src/authorizer/local/authorizer.cpp b/src/authorizer/local/authorizer.cpp index 809c2e4..09f2618 100644 --- a/src/authorizer/local/authorizer.cpp +++ b/src/authorizer/local/authorizer.cpp @@ -1480,10 +1480,10 @@ private: return acls_; case authorization::MODIFY_RESOURCE_PROVIDER_CONFIG: foreach (const ACL::ModifyResourceProviderConfig& acl, - acls.modify_resource_provider_config()) { + acls.modify_resource_provider_configs()) { GenericACL acl_; acl_.subjects = acl.principals(); - acl_.objects = acl.users(); + acl_.objects = acl.resource_providers(); acls_.push_back(acl_); } @@ -1652,6 +1652,15 @@ Option<Error> LocalAuthorizer::validate(const ACLs& acls) } } + foreach (const ACL::ModifyResourceProviderConfig& acl, + acls.modify_resource_provider_configs()) { + if (acl.resource_providers().type() == ACL::Entity::SOME) { + return Error( + "acls.modify_resource_provider_config type must be either NONE or " + "ANY"); + } + } + // TODO(alexr): Consider validating not only protobuf, but also the original // JSON in order to spot misspelled names. A misspelled action may affect // authorization result and hence lead to a security issue (e.g. when there http://git-wip-us.apache.org/repos/asf/mesos/blob/24d17b29/src/tests/authorization_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/authorization_tests.cpp b/src/tests/authorization_tests.cpp index f2e86e2..35eecc8 100644 --- a/src/tests/authorization_tests.cpp +++ b/src/tests/authorization_tests.cpp @@ -5375,6 +5375,66 @@ TYPED_TEST(AuthorizationTest, GetMaintenanceStatus) } } + +// This tests the authorization of requests to ModifyResourceProviderConfig. +TYPED_TEST(AuthorizationTest, ModifyResourceProviderConfig) +{ + ACLs acls; + + { + // "foo" principal can modify resource provider configs on agents. + mesos::ACL::ModifyResourceProviderConfig* acl = + acls.add_modify_resource_provider_configs(); + acl->mutable_principals()->add_values("foo"); + acl->mutable_resource_providers()->set_type(mesos::ACL::Entity::ANY); + } + + { + // Nobody else can modify resource provider configs. + mesos::ACL::ModifyResourceProviderConfig* acl = + acls.add_modify_resource_provider_configs(); + acl->mutable_principals()->set_type(mesos::ACL::Entity::ANY); + acl->mutable_resource_providers()->set_type(mesos::ACL::Entity::NONE); + } + + Try<Authorizer*> create = TypeParam::create(parameterize(acls)); + ASSERT_SOME(create); + Owned<Authorizer> authorizer(create.get()); + + { + // "foo" is allowed to modify resource provider configs. The request + // should succeed. + authorization::Request request; + request.set_action(authorization::MODIFY_RESOURCE_PROVIDER_CONFIG); + request.mutable_subject()->set_value("foo"); + + AWAIT_EXPECT_TRUE(authorizer->authorized(request)); + } + + { + // "bar" is not allowed to modify resource provider configs. The + // request should fail. + authorization::Request request; + request.set_action(authorization::MODIFY_RESOURCE_PROVIDER_CONFIG); + request.mutable_subject()->set_value("bar"); + + AWAIT_EXPECT_FALSE(authorizer->authorized(request)); + } + + { + // Test that no authorizer is created with invalid ACLs. + ACLs invalid; + + mesos::ACL::ModifyResourceProviderConfig* acl = + invalid.add_modify_resource_provider_configs(); + acl->mutable_principals()->add_values("foo"); + acl->mutable_resource_providers()->add_values("yoda"); + + Try<Authorizer*> create = TypeParam::create(parameterize(invalid)); + EXPECT_ERROR(create); + } +} + } // namespace tests { } // namespace internal { } // namespace mesos {
