Repository: mesos
Updated Branches:
  refs/heads/master 71e5099c5 -> 6224c8c7b


Added authorization to agent's '/containers' endpoint.

Used `GET_ENDPOINT_WITH_PATH` coarse-grained authz on agent's
'/containers' endpoint to enable authorization on this endpoint.
Updated docs and tests as well.

Review: https://reviews.apache.org/r/47530/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/6224c8c7
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/6224c8c7
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/6224c8c7

Branch: refs/heads/master
Commit: 6224c8c7b8c0877701ed1d6c9d2de952d560ffdb
Parents: 71e5099
Author: Abhishek Dasgupta <a10gu...@linux.vnet.ibm.com>
Authored: Thu May 19 09:53:14 2016 +0200
Committer: Alexander Rukletsov <al...@apache.org>
Committed: Thu May 19 11:00:19 2016 +0200

----------------------------------------------------------------------
 docs/endpoints/slave/containers.md      |  6 +++-
 src/slave/http.cpp                      | 41 ++++++++++++++++++++++++++--
 src/slave/slave.hpp                     |  7 ++++-
 src/tests/slave_authorization_tests.cpp |  5 +++-
 4 files changed, 54 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/6224c8c7/docs/endpoints/slave/containers.md
----------------------------------------------------------------------
diff --git a/docs/endpoints/slave/containers.md 
b/docs/endpoints/slave/containers.md
index 959f40b..f8bcb38 100644
--- a/docs/endpoints/slave/containers.md
+++ b/docs/endpoints/slave/containers.md
@@ -50,4 +50,8 @@ Example (**Note**: this is not exhaustive):
 
 ### AUTHENTICATION ###
 This endpoint requires authentication iff HTTP authentication is
-enabled.
\ No newline at end of file
+enabled.
+
+### AUTHORIZATION ###
+The request principal should be authorized to query this endpoint.
+See the authorization documentation for details.
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/mesos/blob/6224c8c7/src/slave/http.cpp
----------------------------------------------------------------------
diff --git a/src/slave/http.cpp b/src/slave/http.cpp
index fb48ec6..dd1f509 100644
--- a/src/slave/http.cpp
+++ b/src/slave/http.cpp
@@ -744,13 +744,50 @@ string Slave::Http::CONTAINERS_HELP()
           "    }",
           "}]",
           "```"),
-      AUTHENTICATION(true));
+      AUTHENTICATION(true),
+      AUTHORIZATION(
+          "The request principal should be authorized to query this endpoint.",
+          "See the authorization documentation for details."));
 }
 
 
 Future<Response> Slave::Http::containers(
     const Request& request,
-    const Option<string>& /* principal */) const
+    const Option<string>& principal) const
+{
+  // TODO(a10gupta): Remove check for enabled
+  // authorization as part of MESOS-5346.
+  if (request.method != "GET" && slave->authorizer.isSome()) {
+    return MethodNotAllowed({"GET"}, request.method);
+  }
+
+  Try<string> endpoint = extractEndpoint(request.url);
+  if (endpoint.isError()) {
+    return Failure("Failed to extract endpoint: " + endpoint.error());
+  }
+
+  const PID<Slave> pid = slave->self();
+
+  // NOTE: We should not leak the slave instance because the its
+  // lifetime is not guaranteed. See MESOS-5293 for details.
+  const Slave* localSlave = slave;
+
+  return authorizeEndpoint(principal, endpoint.get(), request.method)
+    .then(defer(
+        pid,
+        [pid, localSlave, request](bool authorized) -> Future<Response> {
+          if (!authorized) {
+            return Forbidden();
+          }
+
+          return _containers(request, localSlave);
+        }));
+}
+
+
+Future<Response> Slave::Http::_containers(
+    const Request& request,
+    const Slave* slave)
 {
   Owned<list<JSON::Object>> metadata(new list<JSON::Object>());
   list<Future<ContainerStatus>> statusFutures;

http://git-wip-us.apache.org/repos/asf/mesos/blob/6224c8c7/src/slave/slave.hpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.hpp b/src/slave/slave.hpp
index 209f071..0de6a57 100644
--- a/src/slave/slave.hpp
+++ b/src/slave/slave.hpp
@@ -458,7 +458,7 @@ private:
     // /slave/containers
     process::Future<process::http::Response> containers(
         const process::http::Request& request,
-        const Option<std::string>& /* principal */) const;
+        const Option<std::string>& principal) const;
 
     static std::string EXECUTOR_HELP();
     static std::string FLAGS_HELP();
@@ -479,6 +479,11 @@ private:
         const ResourceUsage& usage,
         const process::http::Request& request);
 
+    // Continuation for `/containers` endpoint
+    static process::Future<process::http::Response> _containers(
+        const process::http::Request& request,
+        const Slave* slave);
+
     // Helper routines for endpoint authorization.
     Try<std::string> extractEndpoint(const process::http::URL& url) const;
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/6224c8c7/src/tests/slave_authorization_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/slave_authorization_tests.cpp 
b/src/tests/slave_authorization_tests.cpp
index 843cf1c..98fde49 100644
--- a/src/tests/slave_authorization_tests.cpp
+++ b/src/tests/slave_authorization_tests.cpp
@@ -178,7 +178,10 @@ INSTANTIATE_TEST_CASE_P(
     Endpoint,
     SlaveEndpointTest,
     ::testing::Values(
-        "monitor/statistics", "monitor/statistics.json", "flags"));
+        "monitor/statistics",
+        "monitor/statistics.json",
+        "flags",
+        "containers"));
 
 
 // Tests that an agent endpoint handler forms

Reply via email to