This is an automated email from the ASF dual-hosted git repository. tillt pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 00e0707e32735886f61ab4f9c03d4b06a295d1b5 Author: Till Toenshoff <[email protected]> AuthorDate: Tue Nov 20 14:46:11 2018 +0100 Refactored createSubject and authorizeLogAccess to common/authorization. Moves 'createSubject' out of common/http into common/authorization. Removes duplicate 'authorizeLogAccess' out of master.cpp and slave.cpp. Introduces 'authorizeLogAccess' within common/authorization. Review: https://reviews.apache.org/r/69385/ --- src/common/authorization.cpp | 49 ++++++++++++++++++++++++++++++++++++++++++++ src/common/authorization.hpp | 18 ++++++++++++++++ src/common/http.cpp | 27 +----------------------- src/common/http.hpp | 10 --------- src/master/http.cpp | 1 + src/master/master.cpp | 25 ++++------------------ src/master/master.hpp | 3 --- src/master/quota_handler.cpp | 2 ++ src/slave/http.cpp | 1 + src/slave/slave.cpp | 29 +++++--------------------- src/slave/slave.hpp | 3 --- 11 files changed, 81 insertions(+), 87 deletions(-) diff --git a/src/common/authorization.cpp b/src/common/authorization.cpp index 5064ad2..fa0c0e8 100644 --- a/src/common/authorization.cpp +++ b/src/common/authorization.cpp @@ -17,15 +17,22 @@ #include "common/authorization.hpp" #include <algorithm> +#include <string> + +#include <mesos/mesos.hpp> #include <process/collect.hpp> #include <stout/foreach.hpp> +#include <stout/none.hpp> +using std::string; using std::vector; using process::Future; +using process::http::authentication::Principal; + namespace mesos { namespace authorization { @@ -37,5 +44,47 @@ Future<bool> collectAuthorizations(const vector<Future<bool>>& authorizations) }); } + +const Option<Subject> createSubject(const Option<Principal>& principal) +{ + if (principal.isSome()) { + Subject subject; + + if (principal->value.isSome()) { + subject.set_value(principal->value.get()); + } + + foreachpair (const string& key, const string& value, principal->claims) { + Label* claim = subject.mutable_claims()->mutable_labels()->Add(); + claim->set_key(key); + claim->set_value(value); + } + + return subject; + } + + return None(); +} + + +Future<bool> authorizeLogAccess( + const Option<Authorizer*>& authorizer, + const Option<Principal>& principal) +{ + if (authorizer.isNone()) { + return true; + } + + Request request; + request.set_action(ACCESS_MESOS_LOG); + + Option<Subject> subject = createSubject(principal); + if (subject.isSome()) { + request.mutable_subject()->CopyFrom(subject.get()); + } + + return authorizer.get()->authorized(request); +} + } // namespace authorization { } // namespace mesos { diff --git a/src/common/authorization.hpp b/src/common/authorization.hpp index 439996a..fb73f30 100644 --- a/src/common/authorization.hpp +++ b/src/common/authorization.hpp @@ -19,7 +19,14 @@ #include <vector> +#include <mesos/authentication/authenticator.hpp> + +#include <mesos/authorizer/authorizer.hpp> + #include <process/future.hpp> +#include <process/http.hpp> + +#include <stout/option.hpp> namespace mesos { namespace authorization { @@ -29,6 +36,17 @@ namespace authorization { process::Future<bool> collectAuthorizations( const std::vector<process::Future<bool>>& authorizations); +// Creates a `Subject` for authorization purposes when given an +// authenticated `Principal`. This function accepts and returns an +// `Option` to make call sites cleaner, since it is possible that +// `principal` will be `NONE`. +const Option<Subject> createSubject( + const Option<process::http::authentication::Principal>& principal); + +process::Future<bool> authorizeLogAccess( + const Option<Authorizer*>& authorizer, + const Option<process::http::authentication::Principal>& principal); + } // namespace authorization { } // namespace mesos { diff --git a/src/common/http.cpp b/src/common/http.cpp index eeeed97..2be94b2 100644 --- a/src/common/http.cpp +++ b/src/common/http.cpp @@ -48,6 +48,7 @@ #include <stout/os/permissions.hpp> +#include "common/authorization.hpp" #include "common/http.hpp" #include "messages/messages.hpp" @@ -876,32 +877,6 @@ static void json(JSON::StringWriter* writer, const Value::Text& text) writer->set(text.value()); } -namespace authorization { - -const Option<authorization::Subject> createSubject( - const Option<Principal>& principal) -{ - if (principal.isSome()) { - authorization::Subject subject; - - if (principal->value.isSome()) { - subject.set_value(principal->value.get()); - } - - foreachpair (const string& key, const string& value, principal->claims) { - Label* claim = subject.mutable_claims()->mutable_labels()->Add(); - claim->set_key(key); - claim->set_value(value); - } - - return subject; - } - - return None(); -} - -} // namespace authorization { - const AuthorizationCallbacks createAuthorizationCallbacks( Authorizer* authorizer) { diff --git a/src/common/http.hpp b/src/common/http.hpp index 6ca54a6..ac9ed5e 100644 --- a/src/common/http.hpp +++ b/src/common/http.hpp @@ -173,16 +173,6 @@ void json( void json(JSON::ObjectWriter* writer, const Task& task); void json(JSON::ObjectWriter* writer, const TaskStatus& status); -namespace authorization { - -// Creates a subject for authorization purposes when given an authenticated -// principal. This function accepts and returns an `Option` to make call sites -// cleaner, since it is possible that `principal` will be `NONE`. -const Option<authorization::Subject> createSubject( - const Option<process::http::authentication::Principal>& principal); - -} // namespace authorization { - const process::http::authorization::AuthorizationCallbacks createAuthorizationCallbacks(Authorizer* authorizer); diff --git a/src/master/http.cpp b/src/master/http.cpp index 75ab6ea..68ee2a6 100644 --- a/src/master/http.cpp +++ b/src/master/http.cpp @@ -68,6 +68,7 @@ #include <stout/utils.hpp> #include <stout/uuid.hpp> +#include "common/authorization.hpp" #include "common/http.hpp" #include "common/protobuf_utils.hpp" #include "common/resources_utils.hpp" diff --git a/src/master/master.cpp b/src/master/master.cpp index 0b43dc7..b4b02d8 100644 --- a/src/master/master.cpp +++ b/src/master/master.cpp @@ -1074,10 +1074,11 @@ void Master::initialize() provide("app", path::join(flags.webui_dir, "app")); provide("assets", path::join(flags.webui_dir, "assets")); - const PID<Master> masterPid = self(); + // TODO(tillt): Use generalized lambda capture once we adopt C++14. + Option<Authorizer*> _authorizer = authorizer; - auto authorize = [masterPid](const Option<Principal>& principal) { - return dispatch(masterPid, &Master::authorizeLogAccess, principal); + auto authorize = [_authorizer](const Option<Principal>& principal) { + return authorization::authorizeLogAccess(_authorizer, principal); }; // Expose the log file for the webui. Fall back to 'log_dir' if @@ -1413,24 +1414,6 @@ void Master::_exited(Framework* framework) } -Future<bool> Master::authorizeLogAccess(const Option<Principal>& principal) -{ - if (authorizer.isNone()) { - return true; - } - - authorization::Request request; - request.set_action(authorization::ACCESS_MESOS_LOG); - - Option<authorization::Subject> subject = createSubject(principal); - if (subject.isSome()) { - request.mutable_subject()->CopyFrom(subject.get()); - } - - return authorizer.get()->authorized(request); -} - - void Master::consume(MessageEvent&& event) { // There are three cases about the message's UPID with respect to diff --git a/src/master/master.hpp b/src/master/master.hpp index baa6668..3b3c1a4 100644 --- a/src/master/master.hpp +++ b/src/master/master.hpp @@ -1179,9 +1179,6 @@ private: const hashset<SlaveID>& toRemoveGone, const process::Future<bool>& registrarResult); - process::Future<bool> authorizeLogAccess( - const Option<process::http::authentication::Principal>& principal); - std::vector<std::string> filterRoles( const process::Owned<ObjectApprovers>& approvers) const; diff --git a/src/master/quota_handler.cpp b/src/master/quota_handler.cpp index a4975fd..8d417a9 100644 --- a/src/master/quota_handler.cpp +++ b/src/master/quota_handler.cpp @@ -37,6 +37,8 @@ #include <stout/strings.hpp> #include <stout/utils.hpp> +#include "common/authorization.hpp" + #include "logging/logging.hpp" #include "master/quota.hpp" diff --git a/src/slave/http.cpp b/src/slave/http.cpp index 816aed1..ba38926 100644 --- a/src/slave/http.cpp +++ b/src/slave/http.cpp @@ -57,6 +57,7 @@ #include <stout/strings.hpp> #include <stout/unreachable.hpp> +#include "common/authorization.hpp" #include "common/build.hpp" #include "common/http.hpp" #include "common/recordio.hpp" diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp index 74f6fb9..858b786 100644 --- a/src/slave/slave.cpp +++ b/src/slave/slave.cpp @@ -82,6 +82,7 @@ #include "authentication/cram_md5/authenticatee.hpp" +#include "common/authorization.hpp" #include "common/build.hpp" #include "common/protobuf_utils.hpp" #include "common/resources_utils.hpp" @@ -836,13 +837,11 @@ void Slave::initialize() }); }); - const PID<Slave> slavePid = self(); + // TODO(tillt): Use generalized lambda capture once we adopt C++14. + Option<Authorizer*> _authorizer = authorizer; - auto authorize = [slavePid](const Option<Principal>& principal) { - return dispatch( - slavePid, - &Slave::authorizeLogAccess, - principal); + auto authorize = [_authorizer](const Option<Principal>& principal) { + return authorization::authorizeLogAccess(_authorizer, principal); }; // Expose the log file for the webui. Fall back to 'log_dir' if @@ -8478,24 +8477,6 @@ Future<bool> Slave::authorizeTask( } -Future<bool> Slave::authorizeLogAccess(const Option<Principal>& principal) -{ - if (authorizer.isNone()) { - return true; - } - - authorization::Request request; - request.set_action(authorization::ACCESS_MESOS_LOG); - - Option<authorization::Subject> subject = createSubject(principal); - if (subject.isSome()) { - request.mutable_subject()->CopyFrom(subject.get()); - } - - return authorizer.get()->authorized(request); -} - - Future<bool> Slave::authorizeSandboxAccess( const Option<Principal>& principal, const FrameworkID& frameworkId, diff --git a/src/slave/slave.hpp b/src/slave/slave.hpp index 0bd3401..edf7269 100644 --- a/src/slave/slave.hpp +++ b/src/slave/slave.hpp @@ -608,9 +608,6 @@ private: const TaskInfo& task, const FrameworkInfo& frameworkInfo); - process::Future<bool> authorizeLogAccess( - const Option<process::http::authentication::Principal>& principal); - process::Future<bool> authorizeSandboxAccess( const Option<process::http::authentication::Principal>& principal, const FrameworkID& frameworkId,
