You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by me...@apache.org on 2016/07/06 18:36:40 UTC

mesos git commit: Added authz to /files/debug endpoint.

Repository: mesos
Updated Branches:
  refs/heads/master 29f848b73 -> 778441ddf


Added authz to /files/debug endpoint.

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


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

Branch: refs/heads/master
Commit: 778441ddf44782108cb39023325a4fd8f7d5d451
Parents: 29f848b
Author: Abhishek Dasgupta <a1...@linux.vnet.ibm.com>
Authored: Wed Jul 6 10:47:49 2016 -0700
Committer: Adam B <ad...@mesosphere.io>
Committed: Wed Jul 6 11:36:27 2016 -0700

----------------------------------------------------------------------
 docs/authorization.md     |  1 +
 docs/upgrades.md          |  1 +
 src/common/http.cpp       |  4 ++-
 src/files/files.cpp       | 51 ++++++++++++++++++++++------
 src/files/files.hpp       |  5 ++-
 src/master/main.cpp       |  4 +--
 src/slave/main.cpp        |  2 +-
 src/tests/files_tests.cpp | 77 ++++++++++++++++++++++++++++++++++--------
 8 files changed, 116 insertions(+), 29 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/778441dd/docs/authorization.md
----------------------------------------------------------------------
diff --git a/docs/authorization.md b/docs/authorization.md
index 002501c..fc027ac 100644
--- a/docs/authorization.md
+++ b/docs/authorization.md
@@ -269,6 +269,7 @@ entries, each representing an authorizable action:
 
 The `get_endpoints` action covers:
 
+* `/files/debug`
 * `/logging/toggle`
 * `/metrics/snapshot`
 * `/slave(id)/containers`

http://git-wip-us.apache.org/repos/asf/mesos/blob/778441dd/docs/upgrades.md
----------------------------------------------------------------------
diff --git a/docs/upgrades.md b/docs/upgrades.md
index 255b5bd..431e6b3 100644
--- a/docs/upgrades.md
+++ b/docs/upgrades.md
@@ -272,6 +272,7 @@ We categorize the changes as follows:
 * Mesos 1.0 introduces authorization support for several HTTP endpoints. Note that some of these endpoints are used by the web UI, and thus using the web UI in a cluster with authorization enabled will require that ACLs be set appropriately. Please refer to the [authorization documentation](authorization.md) for details.
 
 * The endpoints with coarse grained authorization enabled are:
+  - `/files/debug`
   - `/logging/toggle`
   - `/metrics/snapshot`
   - `/slave(id)/containers`

http://git-wip-us.apache.org/repos/asf/mesos/blob/778441dd/src/common/http.cpp
----------------------------------------------------------------------
diff --git a/src/common/http.cpp b/src/common/http.cpp
index 0060121..dfe44da 100644
--- a/src/common/http.cpp
+++ b/src/common/http.cpp
@@ -71,9 +71,11 @@ ostream& operator<<(ostream& stream, ContentType contentType)
 namespace internal {
 
 // Set of endpoint whose access is protected with the authorization
-// action `GET_EDNPOINTS_WITH_PATH`.
+// action `GET_ENDPOINTS_WITH_PATH`.
 hashset<string> AUTHORIZABLE_ENDPOINTS{
     "/containers",
+    "/files/debug",
+    "/files/debug.json",
     "/logging/toggle",
     "/metrics/snapshot",
     "/monitor/statistics",

http://git-wip-us.apache.org/repos/asf/mesos/blob/778441dd/src/files/files.cpp
----------------------------------------------------------------------
diff --git a/src/files/files.cpp b/src/files/files.cpp
index a5a1b86..0368f67 100644
--- a/src/files/files.cpp
+++ b/src/files/files.cpp
@@ -53,12 +53,16 @@
 
 #include <stout/os/constants.hpp>
 
+#include "common/http.hpp"
+
 #include "files/files.hpp"
 
 #include "logging/logging.hpp"
 
 using namespace process;
 
+using mesos::Authorizer;
+
 using process::AUTHENTICATION;
 using process::AUTHORIZATION;
 using process::DESCRIPTION;
@@ -71,8 +75,6 @@ using process::http::Forbidden;
 using process::http::InternalServerError;
 using process::http::NotFound;
 using process::http::OK;
-using process::http::Response;
-using process::http::Request;
 
 using std::list;
 using std::map;
@@ -82,10 +84,14 @@ using std::vector;
 namespace mesos {
 namespace internal {
 
+using process::http::Response;
+using process::http::Request;
+
 class FilesProcess : public Process<FilesProcess>
 {
 public:
-  FilesProcess(const Option<string>& _authenticationRealm);
+  FilesProcess(const Option<string>& _authenticationRealm,
+               const Option<Authorizer*>& _authorizer);
 
   // Files implementation.
   Future<Nothing> attach(
@@ -166,12 +172,19 @@ private:
   // The authentication realm, if any, into which this process'
   // endpoints will be installed.
   Option<string> authenticationRealm;
+
+  // FilesProcess needs an authorizer object to add authorization in
+  // `/files/debug` endpoint.
+  Option<Authorizer*> authorizer;
 };
 
 
-FilesProcess::FilesProcess(const Option<string>& _authenticationRealm)
+FilesProcess::FilesProcess(
+    const Option<string>& _authenticationRealm,
+    const Option<Authorizer*>& _authorizer)
   : ProcessBase("files"),
-    authenticationRealm(_authenticationRealm) {}
+    authenticationRealm(_authenticationRealm),
+    authorizer(_authorizer) {}
 
 
 void FilesProcess::initialize()
@@ -691,18 +704,35 @@ const string FilesProcess::DEBUG_HELP = HELP(
     DESCRIPTION(
         "This endpoint shows the internal virtual path map as a",
         "JSON object."),
-    AUTHENTICATION(true));
+    AUTHENTICATION(true),
+    AUTHORIZATION(
+        "The request principal should be authorized to query this endpoint.",
+        "See the authorization documentation for details."));
 
 
 Future<Response> FilesProcess::debug(
     const Request& request,
-    const Option<string>& /* principal */)
+    const Option<string>&  principal )
 {
   JSON::Object object;
   foreachpair (const string& name, const string& path, paths) {
     object.values[name] = path;
   }
-  return OK(object, request.url.query.get("jsonp"));
+
+  const Option<string>& jsonp = request.url.query.get("jsonp");
+
+  return authorizeEndpoint(
+      request.url.path,
+      request.method,
+      authorizer,
+      principal)
+    .then(defer([object, jsonp](bool authorized) -> Future<Response> {
+          if (!authorized) {
+            return Forbidden();
+          }
+
+          return OK(object, jsonp);
+        }));
 }
 
 
@@ -774,9 +804,10 @@ Result<string> FilesProcess::resolve(const string& path)
 }
 
 
-Files::Files(const Option<string>& authenticationRealm)
+Files::Files(const Option<string>& authenticationRealm,
+             const Option<Authorizer*>& authorizer)
 {
-  process = new FilesProcess(authenticationRealm);
+  process = new FilesProcess(authenticationRealm, authorizer);
   spawn(process);
 }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/778441dd/src/files/files.hpp
----------------------------------------------------------------------
diff --git a/src/files/files.hpp b/src/files/files.hpp
index b767d5b..06a91a5 100644
--- a/src/files/files.hpp
+++ b/src/files/files.hpp
@@ -29,6 +29,8 @@
 
 #include <string>
 
+#include <mesos/authorizer/authorizer.hpp>
+
 #include <process/future.hpp>
 #include <process/http.hpp>
 
@@ -59,7 +61,8 @@ class FilesProcess;
 class Files
 {
 public:
-  Files(const Option<std::string>& authenticationRealm = None());
+  Files(const Option<std::string>& authenticationRealm = None(),
+        const Option<mesos::Authorizer*>& authorizer = None());
   ~Files();
 
   // Returns the result of trying to attach the specified path

http://git-wip-us.apache.org/repos/asf/mesos/blob/778441dd/src/master/main.cpp
----------------------------------------------------------------------
diff --git a/src/master/main.cpp b/src/master/main.cpp
index 84f3b07..9775b8a 100644
--- a/src/master/main.cpp
+++ b/src/master/main.cpp
@@ -437,8 +437,6 @@ int main(int argc, char** argv)
   Registrar* registrar =
     new Registrar(flags, state, DEFAULT_HTTP_AUTHENTICATION_REALM);
 
-  Files files(DEFAULT_HTTP_AUTHENTICATION_REALM);
-
   MasterContender* contender;
   MasterDetector* detector;
 
@@ -506,6 +504,8 @@ int main(int argc, char** argv)
         createAuthorizationCallbacks(authorizer_.get()));
   }
 
+  Files files(DEFAULT_HTTP_AUTHENTICATION_REALM, authorizer_);
+
   Option<shared_ptr<RateLimiter>> slaveRemovalLimiter = None();
   if (flags.agent_removal_rate_limit.isSome()) {
     // Parse the flag value.

http://git-wip-us.apache.org/repos/asf/mesos/blob/778441dd/src/slave/main.cpp
----------------------------------------------------------------------
diff --git a/src/slave/main.cpp b/src/slave/main.cpp
index a7ed669..4624392 100644
--- a/src/slave/main.cpp
+++ b/src/slave/main.cpp
@@ -405,7 +405,7 @@ int main(int argc, char** argv)
         createAuthorizationCallbacks(authorizer_.get()));
   }
 
-  Files files(DEFAULT_HTTP_AUTHENTICATION_REALM);
+  Files files(DEFAULT_HTTP_AUTHENTICATION_REALM, authorizer_);
   GarbageCollector gc;
   StatusUpdateManager statusUpdateManager(flags);
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/778441dd/src/tests/files_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/files_tests.cpp b/src/tests/files_tests.cpp
index 31337e2..a30e452 100644
--- a/src/tests/files_tests.cpp
+++ b/src/tests/files_tests.cpp
@@ -484,25 +484,74 @@ TEST_F(FilesTest, DownloadTest)
 // Tests that the '/files/debug' endpoint works as expected.
 TEST_F(FilesTest, DebugTest)
 {
-  Files files;
-  process::UPID upid("files", process::address());
+  // Verifies that without any authorizer or authenticator, the '/files/debug'
+  // endpoint works as expected.
+  {
+    Files files;
+    process::UPID upid("files", process::address());
 
-  ASSERT_SOME(os::mkdir("real-path-1"));
-  ASSERT_SOME(os::mkdir("real-path-2"));
+    ASSERT_SOME(os::mkdir("real-path-1"));
+    ASSERT_SOME(os::mkdir("real-path-2"));
 
-  AWAIT_EXPECT_READY(files.attach("real-path-1", "virtual-path-1"));
-  AWAIT_EXPECT_READY(files.attach("real-path-2", "virtual-path-2"));
+    AWAIT_EXPECT_READY(files.attach("real-path-1", "virtual-path-1"));
+    AWAIT_EXPECT_READY(files.attach("real-path-2", "virtual-path-2"));
 
-  // Construct the expected JSON output.
-  const string cwd = os::getcwd();
-  JSON::Object expected;
-  expected.values["virtual-path-1"] = path::join(cwd, "real-path-1");
-  expected.values["virtual-path-2"] = path::join(cwd, "real-path-2");
+    // Construct the expected JSON output.
+    const string cwd = os::getcwd();
+    JSON::Object expected;
+    expected.values["virtual-path-1"] = path::join(cwd, "real-path-1");
+    expected.values["virtual-path-2"] = path::join(cwd, "real-path-2");
 
-  Future<Response> response = process::http::get(upid, "debug");
+    Future<Response> response = process::http::get(upid, "debug");
 
-  AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response);
-  AWAIT_EXPECT_RESPONSE_BODY_EQ(stringify(expected), response);
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response);
+    AWAIT_EXPECT_RESPONSE_BODY_EQ(stringify(expected), response);
+  }
+
+  // Verifies that unauthorized requests for the '/files/debug' endpoint are
+  // properly rejected.
+  {
+    MockAuthorizer mockAuthorizer;
+
+    Files files(None(), &mockAuthorizer);
+    process::UPID upid("files", process::address());
+
+    EXPECT_CALL(mockAuthorizer, authorized(_))
+      .WillOnce(Return(false));
+
+    Future<Response> response = process::http::get(upid, "debug");
+
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(Forbidden().status, response);
+  }
+
+  // Verifies that with an authorizer, the '/files/debug' endpoint works as
+  // expected.
+  {
+    MockAuthorizer mockAuthorizer;
+
+    Files files(None(), &mockAuthorizer);
+    process::UPID upid("files", process::address());
+
+    EXPECT_CALL(mockAuthorizer, authorized(_))
+      .WillOnce(Return(true));
+
+    ASSERT_SOME(os::mkdir("real-path-1"));
+    ASSERT_SOME(os::mkdir("real-path-2"));
+
+    AWAIT_EXPECT_READY(files.attach("real-path-1", "virtual-path-1"));
+    AWAIT_EXPECT_READY(files.attach("real-path-2", "virtual-path-2"));
+
+    // Construct the expected JSON output.
+    const string cwd = os::getcwd();
+    JSON::Object expected;
+    expected.values["virtual-path-1"] = path::join(cwd, "real-path-1");
+    expected.values["virtual-path-2"] = path::join(cwd, "real-path-2");
+
+    Future<Response> response = process::http::get(upid, "debug");
+
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response);
+    AWAIT_EXPECT_RESPONSE_BODY_EQ(stringify(expected), response);
+  }
 }