Implemented authorization for agent `GET_RESOURCE_PROVIDER` calls. Review: https://reviews.apache.org/r/68104/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/58cd5445 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/58cd5445 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/58cd5445 Branch: refs/heads/master Commit: 58cd544596fa499d632da060e36d1ab6987b2dbd Parents: d9a6c55 Author: Benjamin Bannier <benjamin.bann...@mesosphere.io> Authored: Wed Aug 1 11:31:01 2018 -0700 Committer: Chun-Hung Hsiao <chhs...@mesosphere.io> Committed: Wed Aug 1 11:31:01 2018 -0700 ---------------------------------------------------------------------- src/slave/http.cpp | 42 ++++++++++++++++++++++++++---------------- src/tests/api_tests.cpp | 19 ++++++++++++++++++- 2 files changed, 44 insertions(+), 17 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/58cd5445/src/slave/http.cpp ---------------------------------------------------------------------- diff --git a/src/slave/http.cpp b/src/slave/http.cpp index ab5864d..1b6d266 100644 --- a/src/slave/http.cpp +++ b/src/slave/http.cpp @@ -85,6 +85,7 @@ using mesos::authorization::VIEW_FLAGS; using mesos::authorization::VIEW_FRAMEWORK; using mesos::authorization::VIEW_TASK; using mesos::authorization::VIEW_EXECUTOR; +using mesos::authorization::VIEW_RESOURCE_PROVIDER; using mesos::authorization::VIEW_ROLE; using mesos::authorization::SET_LOG_LEVEL; using mesos::authorization::ATTACH_CONTAINER_INPUT; @@ -1813,27 +1814,36 @@ Future<Response> Http::getResourceProviders( LOG(INFO) << "Processing GET_RESOURCE_PROVIDERS call"; - // TODO(nfnt): Authorize this call (MESOS-8314). - - agent::Response response; - response.set_type(mesos::agent::Response::GET_RESOURCE_PROVIDERS); + return ObjectApprovers::create( + slave->authorizer, principal, {VIEW_RESOURCE_PROVIDER}) + .then(defer( + slave->self(), + [this, acceptType]( + const Owned<ObjectApprovers>& approvers) -> Response { + agent::Response response; + response.set_type(mesos::agent::Response::GET_RESOURCE_PROVIDERS); - agent::Response::GetResourceProviders* resourceProviders = - response.mutable_get_resource_providers(); + agent::Response::GetResourceProviders* resourceProviders = + response.mutable_get_resource_providers(); - foreachvalue (ResourceProvider* resourceProvider, - slave->resourceProviders) { - agent::Response::GetResourceProviders::ResourceProvider* provider = - resourceProviders->add_resource_providers(); + foreachvalue ( + ResourceProvider* resourceProvider, slave->resourceProviders) { + if (!approvers->approved<VIEW_RESOURCE_PROVIDER>()) { + continue; + } + agent::Response::GetResourceProviders::ResourceProvider* provider = + resourceProviders->add_resource_providers(); - provider->mutable_resource_provider_info() - ->CopyFrom(resourceProvider->info); + provider->mutable_resource_provider_info()->CopyFrom( + resourceProvider->info); - provider->mutable_total_resources()->CopyFrom( - resourceProvider->totalResources); - } + provider->mutable_total_resources()->CopyFrom( + resourceProvider->totalResources); + } - return OK(serialize(acceptType, evolve(response)), stringify(acceptType)); + return OK( + serialize(acceptType, evolve(response)), stringify(acceptType)); + })); } http://git-wip-us.apache.org/repos/asf/mesos/blob/58cd5445/src/tests/api_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/api_tests.cpp b/src/tests/api_tests.cpp index 182622a..5cf35e6 100644 --- a/src/tests/api_tests.cpp +++ b/src/tests/api_tests.cpp @@ -7000,6 +7000,15 @@ TEST_P(AgentAPITest, GetResourceProviders) FUTURE_PROTOBUF(UpdateSlaveMessage(), _, _); slave::Flags slaveFlags = CreateSlaveFlags(); + slaveFlags.authenticate_http_readwrite = true; + + { + // `DEFAULT_CREDENTIAL_2` is not allowed to view any resource providers. + mesos::ACL::ViewResourceProvider* acl = + slaveFlags.acls->add_view_resource_providers(); + acl->mutable_principals()->add_values(DEFAULT_CREDENTIAL_2.principal()); + acl->mutable_resource_providers()->set_type(mesos::ACL::Entity::NONE); + } Try<Owned<cluster::Slave>> slave = StartSlave(&detector, slaveFlags); ASSERT_SOME(slave); @@ -7047,7 +7056,6 @@ TEST_P(AgentAPITest, GetResourceProviders) AWAIT_READY(v1Response); ASSERT_TRUE(v1Response->IsInitialized()); ASSERT_EQ(v1::agent::Response::GET_RESOURCE_PROVIDERS, v1Response->type()); - EXPECT_EQ(1, v1Response->get_resource_providers().resource_providers_size()); const mesos::v1::ResourceProviderInfo& responseInfo = @@ -7067,6 +7075,15 @@ TEST_P(AgentAPITest, GetResourceProviders) .total_resources(); EXPECT_EQ(v1::Resources(resource), responseResources); + + // `DEFAULT_CREDENTIAL_2` is not allow to view any resource provider + // information. + v1Response = + post(slave.get()->pid, v1Call, contentType, DEFAULT_CREDENTIAL_2); + AWAIT_READY(v1Response); + ASSERT_TRUE(v1Response->IsInitialized()); + ASSERT_EQ(v1::agent::Response::GET_RESOURCE_PROVIDERS, v1Response->type()); + EXPECT_EQ(0, v1Response->get_resource_providers().resource_providers_size()); }