Return-Path: <commits-return-10051-archive-asf-public=cust-asf.ponee.io@mesos.apache.org>
X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io
Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io
Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183])
	by cust-asf2.ponee.io (Postfix) with ESMTP id 2E8EA2007D1
	for <archive-asf-public-internal@cust-asf2.ponee.io>; Thu, 12 May 2016 20:16:35 +0200 (CEST)
Received: by cust-asf.ponee.io (Postfix)
	id 2D2B31602BF; Thu, 12 May 2016 18:16:35 +0000 (UTC)
Delivered-To: archive-asf-public@cust-asf.ponee.io
Received: from mail.apache.org (hermes.apache.org [140.211.11.3])
	by cust-asf.ponee.io (Postfix) with SMTP id 546E5160A15
	for <archive-asf-public@cust-asf.ponee.io>; Thu, 12 May 2016 20:16:34 +0200 (CEST)
Received: (qmail 16532 invoked by uid 500); 12 May 2016 18:16:33 -0000
Mailing-List: contact commits-help@mesos.apache.org; run by ezmlm
Precedence: bulk
List-Help: <mailto:commits-help@mesos.apache.org>
List-Unsubscribe: <mailto:commits-unsubscribe@mesos.apache.org>
List-Post: <mailto:commits@mesos.apache.org>
List-Id: <commits.mesos.apache.org>
Reply-To: dev@mesos.apache.org
Delivered-To: mailing list commits@mesos.apache.org
Received: (qmail 16453 invoked by uid 99); 12 May 2016 18:16:33 -0000
Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23)
    by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 12 May 2016 18:16:33 +0000
Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33)
	id 3A4BFDFDE3; Thu, 12 May 2016 18:16:33 +0000 (UTC)
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: alexr@apache.org
To: commits@mesos.apache.org
Date: Thu, 12 May 2016 18:16:34 -0000
Message-Id: <8d00cd8ba4304c61950ab180507e0737@git.apache.org>
In-Reply-To: <afb0086a59a5427da98e20e48b42d82e@git.apache.org>
References: <afb0086a59a5427da98e20e48b42d82e@git.apache.org>
X-Mailer: ASF-Git Admin Mailer
Subject: [2/4] mesos git commit: Added authorization of the '/flags' master
 endpoint.
archived-at: Thu, 12 May 2016 18:16:35 -0000

Added authorization of the '/flags' master endpoint.

Access to the '/flags' master endpoint is authorized using
the 'GET_ENDPOINT_WITH_PATH' action.

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


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

Branch: refs/heads/master
Commit: 8f12d09c0811ebd12c9b5f6b6f9a1ec5afa8b484
Parents: 8854990
Author: Jan Schlicht <jan@mesosphere.io>
Authored: Thu May 12 19:21:31 2016 +0200
Committer: Alexander Rukletsov <alexr@apache.org>
Committed: Thu May 12 19:48:41 2016 +0200

----------------------------------------------------------------------
 docs/configuration.md                    |   6 +
 src/master/http.cpp                      |  73 ++++++++-
 src/master/master.hpp                    |  15 ++
 src/tests/master_authorization_tests.cpp | 204 ++++++++++++++++++++++++++
 4 files changed, 297 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/8f12d09c/docs/configuration.md
----------------------------------------------------------------------
diff --git a/docs/configuration.md b/docs/configuration.md
index 34271c7..458b7bd 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -458,6 +458,12 @@ Example:
       "principals": { "values": ["a"] },
       "quota_principals": { "values": ["a"] }
     }
+  ],
+  "get_endpoints": [
+    {
+      "principals": { "values": ["a"] },
+      "paths": { "values": ["/flags"] }
+    }
   ]
 }</code></pre>
   </td>

http://git-wip-us.apache.org/repos/asf/mesos/blob/8f12d09c/src/master/http.cpp
----------------------------------------------------------------------
diff --git a/src/master/http.cpp b/src/master/http.cpp
index 56fb876..8c1141e 100644
--- a/src/master/http.cpp
+++ b/src/master/http.cpp
@@ -78,6 +78,7 @@ using process::AUTHENTICATION;
 using process::AUTHORIZATION;
 using process::Clock;
 using process::DESCRIPTION;
+using process::Failure;
 using process::Future;
 using process::HELP;
 using process::TLDR;
@@ -857,7 +858,43 @@ string Master::Http::FLAGS_HELP()
 
 Future<Response> Master::Http::flags(
     const Request& request,
-    const Option<string>& /*principal*/) const
+    const Option<string>& principal) const
+{
+  // TODO(nfnt): Remove check for enabled
+  // authorization as part of MESOS-5346.
+  if (request.method != "GET" && master->authorizer.isSome()) {
+    return MethodNotAllowed({"GET"}, request.method);
+  }
+
+  // Paths are of the form "/master/endpoint". We're only interested
+  // in the part after "/master" and tokenize the path accordingly.
+  //
+  // TODO(nfnt): Refactor this into a helper function in `Master::Http` so
+  // that other endpoints can reuse it. In the long run, absolute paths for
+  // endpoins should be supported, see MESOS-5369.
+  const vector<string> pathComponents =
+    strings::tokenize(request.url.path, "/", 2);
+
+  if (pathComponents.size() != 2u ||
+      pathComponents[0] != master->self().id) {
+    return Failure("Unexpected path '" + request.url.path + "'");
+  }
+  const string endpoint = "/" + pathComponents[1];
+
+  return authorizeEndpoint(principal, endpoint, request.method)
+    .then(defer(
+          master->self(),
+          [this, request](bool authorized) -> Future<Response> {
+            if (!authorized) {
+              return Forbidden();
+            }
+
+            return _flags(request);
+          }));
+}
+
+
+Future<Response> Master::Http::_flags(const Request& request) const
 {
   JSON::Object object;
 
@@ -2811,6 +2848,40 @@ Future<Response> Master::Http::_operation(
     });
 }
 
+
+Future<bool> Master::Http::authorizeEndpoint(
+    const Option<string>& principal,
+    const string& endpoint,
+    const string& method) const
+{
+  if (master->authorizer.isNone()) {
+    return true;
+  }
+
+  authorization::Request request;
+
+  // TODO(nfnt): Add an additional method when POST requests
+  // need to be authorized separately from GET requests.
+  if (method == "GET") {
+    request.set_action(authorization::GET_ENDPOINT_WITH_PATH);
+  } else {
+    return Failure("Unexpected request method '" + method + "'");
+  }
+
+  if (principal.isSome()) {
+    request.mutable_subject()->set_value(principal.get());
+  }
+
+  request.mutable_object()->set_value(endpoint);
+
+  LOG(INFO) << "Authorizing principal '"
+            << (principal.isSome() ? principal.get() : "ANY")
+            << "' to " << method
+            << " the '" << endpoint << "' endpoint";
+
+  return master->authorizer.get()->authorized(request);
+}
+
 } // namespace master {
 } // namespace internal {
 } // namespace mesos {

http://git-wip-us.apache.org/repos/asf/mesos/blob/8f12d09c/src/master/master.hpp
----------------------------------------------------------------------
diff --git a/src/master/master.hpp b/src/master/master.hpp
index 3e55114..bd0fa98 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -1202,6 +1202,9 @@ private:
 
   private:
     // Continuations.
+    process::Future<process::http::Response> _flags(
+        const process::http::Request& request) const;
+
     process::Future<process::http::Response> _teardown(
         const FrameworkID& id) const;
 
@@ -1229,6 +1232,18 @@ private:
         Resources required,
         const Offer::Operation& operation) const;
 
+    // Authorizes access to an HTTP endpoint. The `method` parameter
+    // determines which ACL action will be used in the authorization.
+    // It is expected that the caller has validated that `method` is
+    // supported by this function. Currently "GET" is supported.
+    //
+    // TODO(nfnt): Prefer types instead of strings
+    // for `endpoint` and `method`, see MESOS-5300.
+    process::Future<bool> authorizeEndpoint(
+        const Option<std::string>& principal,
+        const std::string& endpoint,
+        const std::string& method) const;
+
     Master* master;
 
     // NOTE: The quota specific pieces of the Operator API are factored

http://git-wip-us.apache.org/repos/asf/mesos/blob/8f12d09c/src/tests/master_authorization_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_authorization_tests.cpp b/src/tests/master_authorization_tests.cpp
index 804b39a..5c221f0 100644
--- a/src/tests/master_authorization_tests.cpp
+++ b/src/tests/master_authorization_tests.cpp
@@ -14,6 +14,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include <string>
 #include <vector>
 
 #include <gmock/gmock.h>
@@ -21,16 +22,23 @@
 #include <mesos/executor.hpp>
 #include <mesos/scheduler.hpp>
 
+#include <mesos/authorizer/authorizer.hpp>
+
 #include <mesos/master/allocator.hpp>
 
+#include <mesos/module/authorizer.hpp>
+
 #include <process/clock.hpp>
 #include <process/future.hpp>
+#include <process/http.hpp>
 #include <process/pid.hpp>
 #include <process/protobuf.hpp>
 
 #include <stout/gtest.hpp>
 #include <stout/try.hpp>
 
+#include "authorizer/local/authorizer.hpp"
+
 #include "master/master.hpp"
 
 #include "master/allocator/mesos/allocator.hpp"
@@ -43,8 +51,11 @@
 
 #include "tests/containerizer.hpp"
 #include "tests/mesos.hpp"
+#include "tests/module.hpp"
 #include "tests/utils.hpp"
 
+namespace http = process::http;
+
 using mesos::internal::master::Master;
 
 using mesos::internal::master::allocator::MesosAllocatorProcess;
@@ -60,6 +71,11 @@ using process::Owned;
 using process::PID;
 using process::Promise;
 
+using process::http::Forbidden;
+using process::http::OK;
+using process::http::Response;
+
+using std::string;
 using std::vector;
 
 using testing::_;
@@ -1009,6 +1025,194 @@ TEST_F(MasterAuthorizationTest, FrameworkRemovedBeforeReregistration)
   Clock::settle();
 }
 
+
+template <typename T>
+class MasterAuthorizerTest : public MesosTest {};
+
+
+typedef ::testing::Types<
+  LocalAuthorizer,
+  tests::Module<Authorizer, TestLocalAuthorizer>>
+  AuthorizerTypes;
+
+
+TYPED_TEST_CASE(MasterAuthorizerTest, AuthorizerTypes);
+
+
+// This test verifies that only authorized principals
+// can access the '/flags' endpoint.
+TYPED_TEST(MasterAuthorizerTest, AuthorizeFlagsEndpoint)
+{
+  const string endpoint = "flags";
+
+  // Setup ACLs so that only the default principal
+  // can access the '/flags' endpoint.
+  ACLs acls;
+  acls.set_permissive(false);
+
+  mesos::ACL::GetEndpoint* acl = acls.add_get_endpoints();
+  acl->mutable_principals()->add_values(DEFAULT_CREDENTIAL.principal());
+  acl->mutable_paths()->add_values("/" + endpoint);
+
+  // Create an `Authorizer` with the ACLs.
+  Try<Authorizer*> create = TypeParam::create(parameterize(acls));
+  ASSERT_SOME(create);
+  Owned<Authorizer> authorizer(create.get());
+
+  Try<Owned<cluster::Master>> master = this->StartMaster(authorizer.get());
+  ASSERT_SOME(master);
+
+  Future<Response> response = http::get(
+      master.get()->pid,
+      endpoint,
+      None(),
+      createBasicAuthHeaders(DEFAULT_CREDENTIAL));
+
+  AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response)
+    << response.get().body;
+
+  response = http::get(
+      master.get()->pid,
+      endpoint,
+      None(),
+      createBasicAuthHeaders(DEFAULT_CREDENTIAL_2));
+
+  AWAIT_EXPECT_RESPONSE_STATUS_EQ(Forbidden().status, response)
+    << response.get().body;
+}
+
+
+// This test verifies that access to the '/flags' endpoint can be authorized
+// without authentication if an authorization rule exists that applies to
+// anyone. The authorizer will map the absence of a principal to "ANY".
+TYPED_TEST(MasterAuthorizerTest, AuthorizeFlagsEndpointWithoutPrincipal)
+{
+  const string endpoint = "flags";
+
+  // Setup ACLs so that any principal can access the '/flags' endpoint.
+  ACLs acls;
+  acls.set_permissive(false);
+
+  mesos::ACL::GetEndpoint* acl = acls.add_get_endpoints();
+  acl->mutable_principals()->set_type(mesos::ACL::Entity::ANY);
+  acl->mutable_paths()->add_values("/" + endpoint);
+
+  master::Flags masterFlags = this->CreateMasterFlags();
+  masterFlags.authenticate_http = false;
+
+  // Create an `Authorizer` with the ACLs.
+  Try<Authorizer*> create = TypeParam::create(parameterize(acls));
+  ASSERT_SOME(create);
+  Owned<Authorizer> authorizer(create.get());
+
+  Try<Owned<cluster::Master>> master =
+    this->StartMaster(authorizer.get(), masterFlags);
+  ASSERT_SOME(master);
+
+  Future<Response> response = http::get(master.get()->pid, endpoint);
+
+  AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response)
+    << response.get().body;
+}
+
+
+// Parameterized fixture for master-specific authorization tests. The
+// path of the tested endpoint is passed as the only parameter.
+class MasterEndpointTest:
+    public MesosTest,
+    public ::testing::WithParamInterface<string> {};
+
+
+// The tests are parameterized by the endpoint being queried.
+INSTANTIATE_TEST_CASE_P(
+    Endpoint,
+    MasterEndpointTest,
+    ::testing::Values(string("flags")));
+
+
+// Tests that a master endpoint handler forms
+// correct queries against the authorizer.
+TEST_P(MasterEndpointTest, AuthorizedRequest)
+{
+  const string endpoint = GetParam();
+
+  MockAuthorizer mockAuthorizer;
+
+  Try<Owned<cluster::Master>> master = StartMaster(&mockAuthorizer);
+  ASSERT_SOME(master);
+
+  Future<authorization::Request> request;
+  EXPECT_CALL(mockAuthorizer, authorized(_))
+    .WillOnce(DoAll(FutureArg<0>(&request),
+                    Return(true)));
+
+  Future<Response> response = http::get(
+      master.get()->pid,
+      endpoint,
+      None(),
+      createBasicAuthHeaders(DEFAULT_CREDENTIAL));
+
+  AWAIT_READY(request);
+
+  const string principal = DEFAULT_CREDENTIAL.principal();
+  EXPECT_EQ(principal, request.get().subject().value());
+
+  // TODO(nfnt): Once master endpoint handlers use more than just
+  // `GET_ENDPOINT_WITH_PATH` we should factor out the request method
+  // and expected authorization action and parameterize
+  // `MasterEndpointTest` on that.
+  EXPECT_EQ(authorization::GET_ENDPOINT_WITH_PATH, request.get().action());
+
+  EXPECT_EQ("/" + endpoint, request.get().object().value());
+
+  AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response)
+    << response.get().body;
+}
+
+
+// Tests that unauthorized requests for a master endpoint are properly rejected.
+TEST_P(MasterEndpointTest, UnauthorizedRequest)
+{
+  const string endpoint = GetParam();
+
+  MockAuthorizer mockAuthorizer;
+
+  Try<Owned<cluster::Master>> master = StartMaster(&mockAuthorizer);
+  ASSERT_SOME(master);
+
+  EXPECT_CALL(mockAuthorizer, authorized(_))
+    .WillOnce(Return(false));
+
+  Future<Response> response = http::get(
+      master.get()->pid,
+      endpoint,
+      None(),
+      createBasicAuthHeaders(DEFAULT_CREDENTIAL));
+
+  AWAIT_EXPECT_RESPONSE_STATUS_EQ(Forbidden().status, response)
+    << response.get().body;
+}
+
+
+// Tests that requests for a master endpoint
+// always succeed if the authorizer is absent.
+TEST_P(MasterEndpointTest, NoAuthorizer)
+{
+  const string endpoint = GetParam();
+
+  Try<Owned<cluster::Master>> master = StartMaster(CreateMasterFlags());
+  ASSERT_SOME(master);
+
+  Future<Response> response = http::get(
+      master.get()->pid,
+      endpoint,
+      None(),
+      createBasicAuthHeaders(DEFAULT_CREDENTIAL));
+
+  AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response)
+    << response.get().body;
+}
+
 } // namespace tests {
 } // namespace internal {
 } // namespace mesos {