[2/2] mesos git commit: Added authentication to the '/registry' endpoint.

[me...@apache.org]

Added authentication to the '/registry' endpoint.

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


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

Branch: refs/heads/master
Commit: 5d2cec1f3f94ecb0bd7faf44de1b9d2649cb42d5
Parents: ad072a1
Author: Jan Schlicht <ja...@mesosphere.io>
Authored: Fri Apr 1 01:19:02 2016 -0700
Committer: Adam B <ad...@mesosphere.io>
Committed: Fri Apr 1 02:55:54 2016 -0700

----------------------------------------------------------------------
 src/local/local.cpp           |  3 +-
 src/master/main.cpp           |  3 +-
 src/master/registrar.cpp      | 46 ++++++++++++++++++++++-----
 src/master/registrar.hpp      |  9 +++++-
 src/tests/cluster.cpp         |  3 +-
 src/tests/registrar_tests.cpp | 64 ++++++++++++++++++++++++++++++++++++++
 6 files changed, 117 insertions(+), 11 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/5d2cec1f/src/local/local.cpp
----------------------------------------------------------------------
diff --git a/src/local/local.cpp b/src/local/local.cpp
index e777ea2..06f6118 100644
--- a/src/local/local.cpp
+++ b/src/local/local.cpp
@@ -210,7 +210,8 @@ PID<Master> launch(const Flags& flags, Allocator* _allocator)
     CHECK_NOTNULL(storage);
 
     state = new state::protobuf::State(storage);
-    registrar = new Registrar(flags, state);
+    registrar =
+      new Registrar(flags, state, master::DEFAULT_HTTP_AUTHENTICATION_REALM);
     repairer = new Repairer();
 
     contender = new StandaloneMasterContender();

http://git-wip-us.apache.org/repos/asf/mesos/blob/5d2cec1f/src/master/main.cpp
----------------------------------------------------------------------
diff --git a/src/master/main.cpp b/src/master/main.cpp
index 58561cf..9dcdbda 100644
--- a/src/master/main.cpp
+++ b/src/master/main.cpp
@@ -326,7 +326,8 @@ int main(int argc, char** argv)
   CHECK_NOTNULL(storage);
 
   state::protobuf::State* state = new state::protobuf::State(storage);
-  Registrar* registrar = new Registrar(flags, state);
+  Registrar* registrar =
+    new Registrar(flags, state, DEFAULT_HTTP_AUTHENTICATION_REALM);
   Repairer* repairer = new Repairer();
 
   Files files(DEFAULT_HTTP_AUTHENTICATION_REALM);

http://git-wip-us.apache.org/repos/asf/mesos/blob/5d2cec1f/src/master/registrar.cpp
----------------------------------------------------------------------
diff --git a/src/master/registrar.cpp b/src/master/registrar.cpp
index def40b9..9f298a1 100644
--- a/src/master/registrar.cpp
+++ b/src/master/registrar.cpp
@@ -57,6 +57,7 @@ using process::Failure;
 using process::Future;
 using process::HELP;
 using process::Owned;
+using process::PID;
 using process::Process;
 using process::Promise;
 using process::TLDR;
@@ -80,12 +81,16 @@ using process::http::Request;
 class RegistrarProcess : public Process<RegistrarProcess>
 {
 public:
-  RegistrarProcess(const Flags& _flags, State* _state)
+  RegistrarProcess(
+      const Flags& _flags,
+      State* _state,
+      const Option<string>& _authenticationRealm)
     : ProcessBase(process::ID::generate("registrar")),
       metrics(*this),
       updating(false),
       flags(_flags),
-      state(_state) {}
+      state(_state),
+      authenticationRealm(_authenticationRealm) {}
 
   virtual ~RegistrarProcess() {}
 
@@ -96,13 +101,26 @@ public:
 protected:
   virtual void initialize()
   {
-    route("/registry", registryHelp(), &RegistrarProcess::registry);
+    if (authenticationRealm.isSome()) {
+      route(
+          "/registry",
+          authenticationRealm.get(),
+          registryHelp(),
+          &RegistrarProcess::registry);
+    } else {
+      route(
+          "/registry",
+          registryHelp(),
+          lambda::bind(&RegistrarProcess::registry, this, lambda::_1, None()));
+    }
   }
 
 private:
   // HTTP handlers.
   // /registrar(N)/registry
-  Future<Response> registry(const Request& request);
+  Future<Response> registry(
+      const Request& request,
+      const Option<string>& /* principal */);
   static string registryHelp();
 
   // The 'Recover' operation adds the latest MasterInfo.
@@ -208,6 +226,10 @@ private:
   // When an error is encountered from abort(), we'll fail all
   // subsequent operations.
   Option<Error> error;
+
+  // The authentication realm, if any, into which this process'
+  // endpoints will be installed.
+  Option<string> authenticationRealm;
 };
 
 
@@ -235,7 +257,9 @@ void fail(deque<Owned<Operation>>* operations, const string& message)
 }
 
 
-Future<Response> RegistrarProcess::registry(const Request& request)
+Future<Response> RegistrarProcess::registry(
+    const Request& request,
+    const Option<string>& /* principal */)
 {
   JSON::Object result;
 
@@ -509,9 +533,12 @@ void RegistrarProcess::abort(const string& message)
 }
 
 
-Registrar::Registrar(const Flags& flags, State* state)
+Registrar::Registrar(
+    const Flags& flags,
+    State* state,
+    const Option<string>& authenticationRealm)
 {
-  process = new RegistrarProcess(flags, state);
+  process = new RegistrarProcess(flags, state, authenticationRealm);
   spawn(process);
 }
 
@@ -535,6 +562,11 @@ Future<bool> Registrar::apply(Owned<Operation> operation)
   return dispatch(process, &RegistrarProcess::apply, operation);
 }
 
+PID<RegistrarProcess> Registrar::pid() const
+{
+  return process->self();
+}
+
 } // namespace master {
 } // namespace internal {
 } // namespace mesos {

http://git-wip-us.apache.org/repos/asf/mesos/blob/5d2cec1f/src/master/registrar.hpp
----------------------------------------------------------------------
diff --git a/src/master/registrar.hpp b/src/master/registrar.hpp
index f087569..b28e306 100644
--- a/src/master/registrar.hpp
+++ b/src/master/registrar.hpp
@@ -23,6 +23,7 @@
 
 #include <process/future.hpp>
 #include <process/owned.hpp>
+#include <process/pid.hpp>
 
 #include "master/flags.hpp"
 #include "master/registry.hpp"
@@ -102,7 +103,9 @@ class Registrar
 public:
   // If flags.registry_strict is true, all operations will be
   // permitted.
-  Registrar(const Flags& flags, state::protobuf::State* state);
+  Registrar(const Flags& flags,
+            state::protobuf::State* state,
+            const Option<std::string>& authenticationRealm = None());
   ~Registrar();
 
   // Recovers the Registry, persisting the new Master information.
@@ -122,6 +125,10 @@ public:
   //     or recovery failed.
   process::Future<bool> apply(process::Owned<Operation> operation);
 
+  // Gets the pid of the underlying process.
+  // Used in tests.
+  process::PID<RegistrarProcess> pid() const;
+
 private:
   RegistrarProcess* process;
 };

http://git-wip-us.apache.org/repos/asf/mesos/blob/5d2cec1f/src/tests/cluster.cpp
----------------------------------------------------------------------
diff --git a/src/tests/cluster.cpp b/src/tests/cluster.cpp
index 2da0bd7..14d0d34 100644
--- a/src/tests/cluster.cpp
+++ b/src/tests/cluster.cpp
@@ -213,7 +213,8 @@ Try<process::Owned<Master>> Master::start(
 
   // Instantiate some other master dependencies.
   master->state.reset(new state::protobuf::State(master->storage.get()));
-  master->registrar.reset(new master::Registrar(flags, master->state.get()));
+  master->registrar.reset(new master::Registrar(
+      flags, master->state.get(), master::DEFAULT_HTTP_AUTHENTICATION_REALM));
   master->repairer.reset(new master::Repairer());
 
   if (slaveRemovalLimiter.isNone() && flags.slave_removal_rate_limit.isSome()) {

http://git-wip-us.apache.org/repos/asf/mesos/blob/5d2cec1f/src/tests/registrar_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/registrar_tests.cpp b/src/tests/registrar_tests.cpp
index 39caf9b..f8e141b 100644
--- a/src/tests/registrar_tests.cpp
+++ b/src/tests/registrar_tests.cpp
@@ -24,6 +24,8 @@
 #include <mesos/attributes.hpp>
 #include <mesos/type_utils.hpp>
 
+#include <mesos/authentication/http/basic_authenticator_factory.hpp>
+
 #include <process/clock.hpp>
 #include <process/gmock.hpp>
 #include <process/gtest.hpp>
@@ -56,6 +58,8 @@
 #include "state/protobuf.hpp"
 #include "state/storage.hpp"
 
+#include "tests/mesos.hpp"
+
 using namespace mesos::internal::master;
 
 using namespace process;
@@ -71,6 +75,11 @@ using std::string;
 using std::vector;
 
 using process::Clock;
+using process::Owned;
+
+using process::http::OK;
+using process::http::Response;
+using process::http::Unauthorized;
 
 using google::protobuf::RepeatedPtrField;
 
@@ -93,6 +102,8 @@ namespace tests {
 namespace quota = mesos::internal::master::quota;
 namespace weights = mesos::internal::master::weights;
 
+namespace authentication = process::http::authentication;
+
 using namespace mesos::maintenance;
 using namespace mesos::quota;
 
@@ -100,6 +111,8 @@ using namespace mesos::internal::master::maintenance;
 using namespace mesos::internal::master::quota;
 using namespace mesos::internal::master::weights;
 
+using mesos::http::authentication::BasicAuthenticatorFactory;
+
 using state::Entry;
 using state::LogStorage;
 using state::Storage;
@@ -1093,6 +1106,57 @@ TEST_P(RegistrarTest, Abort)
 }
 
 
+// Tests that requests to the '/registry' endpoint are authenticated when HTTP
+// authentication is enabled.
+TEST_P(RegistrarTest, Authentication)
+{
+  const string AUTHENTICATION_REALM = "realm";
+
+  Credentials credentials;
+  credentials.add_credentials()->CopyFrom(DEFAULT_CREDENTIAL);
+
+  Try<authentication::Authenticator*> authenticator =
+    BasicAuthenticatorFactory::create(AUTHENTICATION_REALM, credentials);
+
+  ASSERT_SOME(authenticator);
+
+  AWAIT_READY(authentication::setAuthenticator(
+      AUTHENTICATION_REALM,
+      Owned<authentication::Authenticator>(authenticator.get())));
+
+  Registrar registrar(flags, state, AUTHENTICATION_REALM);
+
+  // Requests without credentials should be rejected.
+  Future<Response> response = process::http::get(registrar.pid(), "registry");
+  AWAIT_EXPECT_RESPONSE_STATUS_EQ(Unauthorized({}).status, response)
+    << response.get().body;
+
+  Credential badCredential;
+  badCredential.set_principal("bad-principal");
+  badCredential.set_secret("bad-secret");
+
+  // Requests with bad credentials should be rejected.
+  response = process::http::get(
+      registrar.pid(),
+      "registry",
+      None(),
+      createBasicAuthHeaders(badCredential));
+  AWAIT_EXPECT_RESPONSE_STATUS_EQ(Unauthorized({}).status, response)
+    << response.get().body;
+
+  // Requests with good credentials should be permitted.
+  response = process::http::get(
+      registrar.pid(),
+      "registry",
+      None(),
+      createBasicAuthHeaders(DEFAULT_CREDENTIAL));
+  AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response)
+    << response.get().body;
+
+  AWAIT_READY(authentication::unsetAuthenticator(AUTHENTICATION_REALM));
+}
+
+
 class Registrar_BENCHMARK_Test
   : public RegistrarTestBase,
     public WithParamInterface<size_t> {};