You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by gr...@apache.org on 2017/07/14 03:53:22 UTC
[2/2] mesos git commit: Refactored authorization acceptors into a
single class.
Refactored authorization acceptors into a single class.
Replaced different authorization-related Acceptor classes with one
AuthorizationAcceptor class.
Removed the ObjectAcceptor parent class, since no inheritance features
are provided by it.
Review: https://reviews.apache.org/r/60716/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/9e208293
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/9e208293
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/9e208293
Branch: refs/heads/master
Commit: 9e208293ba482d843e5c56a40d997ba18e764b58
Parents: 15656be
Author: Quinn Leng <qu...@gmail.com>
Authored: Thu Jul 13 17:44:03 2017 -0700
Committer: Greg Mann <gr...@gmail.com>
Committed: Thu Jul 13 20:41:31 2017 -0700
----------------------------------------------------------------------
src/common/http.cpp | 90 ++++++++----------------------------------------
src/common/http.hpp | 68 +++++++++++++-----------------------
src/master/http.cpp | 22 +++++++-----
3 files changed, 53 insertions(+), 127 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/9e208293/src/common/http.cpp
----------------------------------------------------------------------
diff --git a/src/common/http.cpp b/src/common/http.cpp
index a9c2a4a..3825a13 100644
--- a/src/common/http.cpp
+++ b/src/common/http.cpp
@@ -1143,50 +1143,25 @@ void logRequest(const process::http::Request& request)
}
-Future<Owned<AuthorizeFrameworkInfoAcceptor>>
- AuthorizeFrameworkInfoAcceptor::create(
- const Option<Principal>& principal,
- const Option<Authorizer*>& authorizer)
-{
- if (authorizer.isNone()) {
- return Owned<AuthorizeFrameworkInfoAcceptor>(
- new AuthorizeFrameworkInfoAcceptor(Owned<ObjectApprover>(
- new AcceptingObjectApprover())));
- }
-
- const Option<authorization::Subject> subject =
- authorization::createSubject(principal);
-
- return authorizer.get()->getObjectApprover(
- subject,
- authorization::VIEW_FRAMEWORK)
- .then([=](const Owned<ObjectApprover>& approver) {
- return Owned<AuthorizeFrameworkInfoAcceptor>(
- new AuthorizeFrameworkInfoAcceptor(approver));
- });
-}
-
-
-Future<Owned<AuthorizeTaskAcceptor>> AuthorizeTaskAcceptor::create(
+Future<Owned<AuthorizationAcceptor>> AuthorizationAcceptor::create(
const Option<Principal>& principal,
- const Option<Authorizer*>& authorizer)
+ const Option<Authorizer*>& authorizer,
+ const authorization::Action& action)
{
- if (authorizer.isNone()) {
- return Owned<AuthorizeTaskAcceptor>(
- new AuthorizeTaskAcceptor(Owned<ObjectApprover>(
- new AcceptingObjectApprover())));
- }
+ if (authorizer.isNone()) {
+ return Owned<AuthorizationAcceptor>(
+ new AuthorizationAcceptor(Owned<ObjectApprover>(
+ new AcceptingObjectApprover())));
+ }
- const Option<authorization::Subject> subject =
- authorization::createSubject(principal);
+ const Option<authorization::Subject> subject =
+ authorization::createSubject(principal);
- return authorizer.get()->getObjectApprover(
- subject,
- authorization::VIEW_TASK)
- .then([=](const Owned<ObjectApprover>& approver) {
- return Owned<AuthorizeTaskAcceptor>(
- new AuthorizeTaskAcceptor(approver));
- });
+ return authorizer.get()->getObjectApprover(subject, action)
+ .then([=](const Owned<ObjectApprover>& approver) {
+ return Owned<AuthorizationAcceptor>(
+ new AuthorizationAcceptor(approver));
+ });
}
@@ -1211,41 +1186,6 @@ TaskIDAcceptor::TaskIDAcceptor(const Option<std::string>& _taskId)
}
-bool AuthorizeFrameworkInfoAcceptor::accept(const FrameworkInfo& frameworkInfo)
-{
- ObjectApprover::Object object;
- object.framework_info = &frameworkInfo;
-
- Try<bool> approved = objectApprover->approved(object);
- if (approved.isError()) {
- LOG(WARNING) << "Error during FrameworkInfo authorization: "
- << approved.error();
- return false;
- }
-
- return approved.get();
-}
-
-
-bool AuthorizeTaskAcceptor::accept(
- const Task& task,
- const FrameworkInfo& frameworkInfo)
-{
- ObjectApprover::Object object;
- object.task = &task;
- object.framework_info = &frameworkInfo;
-
- Try<bool> approved = objectApprover->approved(object);
-
- if (approved.isError()) {
- LOG(WARNING) << "Error during Task authorization: " << approved.error();
- return false;
- }
-
- return approved.get();
-}
-
-
bool FrameworkIDAcceptor::accept(const FrameworkID& _frameworkId)
{
if (frameworkId.isSome()) {
http://git-wip-us.apache.org/repos/asf/mesos/blob/9e208293/src/common/http.hpp
----------------------------------------------------------------------
diff --git a/src/common/http.hpp b/src/common/http.hpp
index 93c9b2e..4822a23 100644
--- a/src/common/http.hpp
+++ b/src/common/http.hpp
@@ -161,21 +161,32 @@ public:
};
-/**
- * Determines which objects will be accepted when filtering results based on
- * authorization or other criteria.
- */
-class ObjectAcceptor
+// Determines which objects will be accepted based on authorization.
+class AuthorizationAcceptor
{
public:
- virtual ~ObjectAcceptor() = default;
-};
+ static process::Future<process::Owned<AuthorizationAcceptor>> create(
+ const Option<process::http::authentication::Principal>& principal,
+ const Option<Authorizer*>& authorizer,
+ const authorization::Action& action);
+ template <typename... Args>
+ bool accept(Args&... args)
+ {
+ Try<bool> approved =
+ objectApprover->approved(ObjectApprover::Object(args...));
+ if (approved.isError()) {
+ LOG(WARNING) << "Error during authorization: " << approved.error();
+ return false;
+ }
+
+ return approved.get();
+ }
-// Parent class for authorization-based acceptors.
-class AuthorizationAcceptor : public ObjectAcceptor
-{
protected:
+ // TODO(qleng): Currently, `Owned` is implemented with `shared_ptr` and allows
+ // copying. In the future, if `Owned` is implemented with `unique_ptr`, we
+ // will need to pass by rvalue reference here instead (see MESOS-5122).
AuthorizationAcceptor(const process::Owned<ObjectApprover>& approver)
: objectApprover(approver) {}
@@ -183,46 +194,15 @@ protected:
};
-class AuthorizeFrameworkInfoAcceptor : public AuthorizationAcceptor
-{
-public:
- static process::Future<process::Owned<AuthorizeFrameworkInfoAcceptor>> create(
- const Option<process::http::authentication::Principal>& principal,
- const Option<Authorizer*>& authorizer);
-
- bool accept(const FrameworkInfo& frameworkInfo);
-
-protected:
- AuthorizeFrameworkInfoAcceptor(const process::Owned<ObjectApprover>& approver)
- : AuthorizationAcceptor(approver) {}
-};
-
-
-class AuthorizeTaskAcceptor : public AuthorizationAcceptor
-{
-public:
- static process::Future<process::Owned<AuthorizeTaskAcceptor>> create(
- const Option<process::http::authentication::Principal>& principal,
- const Option<Authorizer*>& authorizer);
-
- bool accept(
- const Task& task,
- const FrameworkInfo& frameworkInfo);
-
-protected:
- AuthorizeTaskAcceptor(const process::Owned<ObjectApprover>& approver)
- : AuthorizationAcceptor(approver) {}
-};
-
-
/**
* Filtering results based on framework ID. When no framework ID is specified
* it will accept all inputs.
*/
-class FrameworkIDAcceptor : public ObjectAcceptor
+class FrameworkIDAcceptor
{
public:
FrameworkIDAcceptor(const Option<std::string>& _frameworkId);
+
bool accept(const FrameworkID& frameworkId);
protected:
@@ -234,7 +214,7 @@ protected:
* Filtering results based on task ID. When no task ID is specified
* it will accept all inputs.
*/
-class TaskIDAcceptor : public ObjectAcceptor
+class TaskIDAcceptor
{
public:
TaskIDAcceptor(const Option<std::string>& _taskId);
http://git-wip-us.apache.org/repos/asf/mesos/blob/9e208293/src/master/http.cpp
----------------------------------------------------------------------
diff --git a/src/master/http.cpp b/src/master/http.cpp
index 4ec275f..3ddb54b 100644
--- a/src/master/http.cpp
+++ b/src/master/http.cpp
@@ -3890,10 +3890,16 @@ Future<Response> Master::Http::tasks(
Option<string> order = request.url.query.get("order");
string _order = order.isSome() && (order.get() == "asc") ? "asc" : "des";
- Future<Owned<AuthorizeFrameworkInfoAcceptor>> authorizeFrameworkInfo =
- AuthorizeFrameworkInfoAcceptor::create(principal, master->authorizer);
- Future<Owned<AuthorizeTaskAcceptor>> authorizeTask =
- AuthorizeTaskAcceptor::create(principal, master->authorizer);
+ Future<Owned<AuthorizationAcceptor>> authorizeFrameworkInfo =
+ AuthorizationAcceptor::create(
+ principal,
+ master->authorizer,
+ authorization::VIEW_FRAMEWORK);
+ Future<Owned<AuthorizationAcceptor>> authorizeTask =
+ AuthorizationAcceptor::create(
+ principal,
+ master->authorizer,
+ authorization::VIEW_TASK);
Future<Owned<FrameworkIDAcceptor>> selectFrameworkId =
Owned<FrameworkIDAcceptor>(
new FrameworkIDAcceptor(request.url.query.get("framework_id")));
@@ -3907,12 +3913,12 @@ Future<Response> Master::Http::tasks(
selectTaskId)
.then(defer(
master->self(),
- [=](const tuple<Owned<AuthorizeFrameworkInfoAcceptor>,
- Owned<AuthorizeTaskAcceptor>,
+ [=](const tuple<Owned<AuthorizationAcceptor>,
+ Owned<AuthorizationAcceptor>,
Owned<FrameworkIDAcceptor>,
Owned<TaskIDAcceptor>>& acceptors)-> Future<Response> {
- Owned<AuthorizeFrameworkInfoAcceptor> authorizeFrameworkInfo;
- Owned<AuthorizeTaskAcceptor> authorizeTask;
+ Owned<AuthorizationAcceptor> authorizeFrameworkInfo;
+ Owned<AuthorizationAcceptor> authorizeTask;
Owned<FrameworkIDAcceptor> selectFrameworkId;
Owned<TaskIDAcceptor> selectTaskId;
tie(authorizeFrameworkInfo,