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/11/16 10:00:09 UTC

[3/3] mesos git commit: Enabled multiple field based authorization in the authorizer interface.

Enabled multiple field based authorization in the authorizer interface.

Updates the authorizer interfaces and well as the local authorizer,
such that all actions which were limited to use a _role_ or a
_principal_ as an object, are able to use whole protobuf messages
as objects. This change enables more sofisticated authorization
mechanisms.

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


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

Branch: refs/heads/master
Commit: bc0e6d7b0b367e5ff67dd5f395e1e06938b02399
Parents: 40c2e5f
Author: Alexander Rojas <al...@mesosphere.io>
Authored: Tue Nov 15 19:04:25 2016 -0800
Committer: Adam B <ad...@mesosphere.io>
Committed: Wed Nov 16 01:55:03 2016 -0800

----------------------------------------------------------------------
 include/mesos/authorizer/authorizer.hpp   |   6 +-
 include/mesos/authorizer/authorizer.proto |  54 ++++++++++++
 src/authorizer/local/authorizer.cpp       | 115 +++++++++++++++++++++----
 3 files changed, 157 insertions(+), 18 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/bc0e6d7b/include/mesos/authorizer/authorizer.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/authorizer/authorizer.hpp b/include/mesos/authorizer/authorizer.hpp
index cb365c7..7217600 100644
--- a/include/mesos/authorizer/authorizer.hpp
+++ b/include/mesos/authorizer/authorizer.hpp
@@ -61,7 +61,9 @@ public:
         task_info(object.has_task_info() ? &object.task_info() : nullptr),
         executor_info(
             object.has_executor_info() ? &object.executor_info() : nullptr),
-        quota_info(object.has_quota_info() ? &object.quota_info() : nullptr) {}
+        quota_info(object.has_quota_info() ? &object.quota_info() : nullptr),
+        weight_info(object.has_weight_info() ? &object.weight_info() : nullptr),
+        resource(object.has_resource() ? &object.resource() : nullptr) {}
 
     const std::string* value;
     const FrameworkInfo* framework_info;
@@ -69,6 +71,8 @@ public:
     const TaskInfo* task_info;
     const ExecutorInfo* executor_info;
     const quota::QuotaInfo* quota_info;
+    const WeightInfo* weight_info;
+    const Resource* resource;
   };
 
   /**

http://git-wip-us.apache.org/repos/asf/mesos/blob/bc0e6d7b/include/mesos/authorizer/authorizer.proto
----------------------------------------------------------------------
diff --git a/include/mesos/authorizer/authorizer.proto b/include/mesos/authorizer/authorizer.proto
index b6a9f14..0696a62 100644
--- a/include/mesos/authorizer/authorizer.proto
+++ b/include/mesos/authorizer/authorizer.proto
@@ -46,11 +46,17 @@ message Object {
   optional TaskInfo task_info = 4;
   optional ExecutorInfo executor_info = 5;
   optional quota.QuotaInfo quota_info = 6;
+  optional WeightInfo weight_info = 7;
+  optional Resource resource = 8;
 }
 
 
 // List of authorizable actions supported in Mesos.
+// NOTE: Values in this enum should be kept in
+// numerical order to prevent accidental aliasing.
 enum Action {
+  option allow_alias = true;
+
   // This must be the first enum value in this list, to
   // ensure that if 'type' is not set, the default value
   // is UNKNOWN. This enables enum values to be added
@@ -58,19 +64,67 @@ enum Action {
   UNKNOWN = 0;
 
   // Actions named *_WITH_foo may set a foo in `Object.value`.
+
+  // `REGISTER_FRAMEWORK` will have an object with `FrameworkInfo` set.
+  // The `_WITH_ROLE` alias is deprecated and will be removed after
+  // Mesos 1.2's deprecation cycle ends. The `value` field will continue
+  // to be set until that time.
+  REGISTER_FRAMEWORK = 1;
   REGISTER_FRAMEWORK_WITH_ROLE = 1;
 
   // `RUN_TASK` will have an object with `FrameworkInfo` and `TaskInfo` set.
   RUN_TASK = 2;
 
+  // `TEARDOWN_FRAMEWORK` will have an object with `FrameworkInfo` set.
+  // The `_WITH_PRINCIPAL` alias is deprecated and will be removed after
+  // Mesos 1.2's deprecation cycle ends. The `value` field will continue
+  // to be set until that time.
+  TEARDOWN_FRAMEWORK = 3;
   TEARDOWN_FRAMEWORK_WITH_PRINCIPAL = 3;
+
+  // `RESERVE_RESOURCES` will have an object with `Resource` set.
+  // The `_WITH_ROLE` alias is deprecated and will be removed after
+  // Mesos 1.2's deprecation cycle ends. The `value` field will continue
+  // to be set until that time.
+  RESERVE_RESOURCES = 4;
   RESERVE_RESOURCES_WITH_ROLE = 4;
+
+  // `UNRESERVE_RESOURCES` will have an object with `Resource` set.
+  // The `_WITH_PRINCIPAL` alias is deprecated and will be removed after
+  // Mesos 1.2's deprecation cycle ends. The `value` field will continue
+  // to be set until that time.
+  UNRESERVE_RESOURCES = 5;
   UNRESERVE_RESOURCES_WITH_PRINCIPAL = 5;
+
+  // `CREATE_VOLUME` will have an object with `Resource` set.
+  // The `_WITH_ROLE` alias is deprecated and will be removed after
+  // Mesos 1.2's deprecation cycle ends. The `value` field will continue
+  // to be set until that time.
+  CREATE_VOLUME = 6;
   CREATE_VOLUME_WITH_ROLE = 6;
+
+  // `DESTROY_VOLUME` will have an object with `Resource` set.
+  // The `_WITH_PRINCIPAL` alias is deprecated and will be removed after
+  // Mesos 1.2's deprecation cycle ends. The `value` field will continue
+  // to be set until that time.
+  DESTROY_VOLUME = 7;
   DESTROY_VOLUME_WITH_PRINCIPAL = 7;
+
   GET_ENDPOINT_WITH_PATH = 8;
   VIEW_ROLE = 9;
+
+  // `UPDATE_WEIGHT` will have an object with `WeightInfo` set.
+  // The `_WITH_ROLE` alias is deprecated and will be removed after
+  // Mesos 1.2's deprecation cycle ends. The `value` field will continue
+  // to be set until that time.
+  UPDATE_WEIGHT = 10;
   UPDATE_WEIGHT_WITH_ROLE = 10;
+
+  // `GET_QUOTA` will have an object with `QuotaInfo` set.
+  // The `_WITH_ROLE` alias is deprecated and will be removed after
+  // Mesos 1.2's deprecation cycle ends. The `value` field will continue
+  // to be set until that time.
+  GET_QUOTA = 11;
   GET_QUOTA_WITH_ROLE = 11;
 
   // `UPDATE_QUOTA` will have an object with a `QuotaInfo` set.

http://git-wip-us.apache.org/repos/asf/mesos/blob/bc0e6d7b/src/authorizer/local/authorizer.cpp
----------------------------------------------------------------------
diff --git a/src/authorizer/local/authorizer.cpp b/src/authorizer/local/authorizer.cpp
index f1dff65..77e05dd 100644
--- a/src/authorizer/local/authorizer.cpp
+++ b/src/authorizer/local/authorizer.cpp
@@ -221,15 +221,7 @@ public:
     } else {
       switch (action_) {
         // All actions using `object.value` for authorization.
-        case authorization::REGISTER_FRAMEWORK_WITH_ROLE:
-        case authorization::TEARDOWN_FRAMEWORK_WITH_PRINCIPAL:
-        case authorization::RESERVE_RESOURCES_WITH_ROLE:
-        case authorization::UNRESERVE_RESOURCES_WITH_PRINCIPAL:
-        case authorization::CREATE_VOLUME_WITH_ROLE:
-        case authorization::DESTROY_VOLUME_WITH_PRINCIPAL:
-        case authorization::GET_QUOTA_WITH_ROLE:
         case authorization::VIEW_ROLE:
-        case authorization::UPDATE_WEIGHT_WITH_ROLE:
         case authorization::GET_ENDPOINT_WITH_PATH: {
           // Check object has the required types set.
           CHECK_NOTNULL(object->value);
@@ -239,6 +231,92 @@ public:
 
           break;
         }
+        case authorization::REGISTER_FRAMEWORK: {
+          aclObject.set_type(mesos::ACL::Entity::SOME);
+          if (object->framework_info) {
+            aclObject.add_values(object->framework_info->role());
+          } else if (object->value) {
+            aclObject.add_values(*(object->value));
+          } else {
+            aclObject.set_type(mesos::ACL::Entity::ANY);
+          }
+
+          break;
+        }
+        case authorization::TEARDOWN_FRAMEWORK: {
+          aclObject.set_type(mesos::ACL::Entity::SOME);
+          if (object->framework_info) {
+            aclObject.add_values(object->framework_info->principal());
+          } else if (object->value) {
+            aclObject.add_values(*(object->value));
+          } else {
+            aclObject.set_type(mesos::ACL::Entity::ANY);
+          }
+
+          break;
+        }
+        case authorization::CREATE_VOLUME:
+        case authorization::RESERVE_RESOURCES: {
+          aclObject.set_type(mesos::ACL::Entity::SOME);
+          if (object->resource) {
+            aclObject.add_values(object->resource->role());
+          } else if (object->value) {
+            aclObject.add_values(*(object->value));
+          } else {
+            aclObject.set_type(mesos::ACL::Entity::ANY);
+          }
+
+          break;
+        }
+        case authorization::DESTROY_VOLUME: {
+          aclObject.set_type(mesos::ACL::Entity::SOME);
+          if (object->resource) {
+            aclObject.add_values(
+                object->resource->disk().persistence().principal());
+          } else if (object->value) {
+            aclObject.add_values(*(object->value));
+          } else {
+            aclObject.set_type(mesos::ACL::Entity::ANY);
+          }
+
+          break;
+        }
+        case authorization::UNRESERVE_RESOURCES: {
+          aclObject.set_type(mesos::ACL::Entity::SOME);
+          if (object->resource) {
+            aclObject.add_values(object->resource->reservation().principal());
+          } else if (object->value) {
+            aclObject.add_values(*(object->value));
+          } else {
+            aclObject.set_type(mesos::ACL::Entity::ANY);
+          }
+
+          break;
+        }
+        case authorization::GET_QUOTA: {
+          aclObject.set_type(mesos::ACL::Entity::SOME);
+          if (object->quota_info) {
+            aclObject.add_values(object->quota_info->role());
+          } else if (object->value) {
+            aclObject.add_values(*(object->value));
+          } else {
+            aclObject.set_type(mesos::ACL::Entity::ANY);
+          }
+
+          break;
+        }
+        case authorization::UPDATE_WEIGHT: {
+          aclObject.set_type(mesos::ACL::Entity::SOME);
+          if (object->weight_info) {
+            aclObject.add_values(object->weight_info->role());
+          } else if (object->value) {
+            aclObject.add_values(*(object->value));
+          } else {
+            aclObject.set_type(mesos::ACL::Entity::ANY);
+          }
+
+          break;
+        }
         case authorization::RUN_TASK: {
           aclObject.set_type(mesos::ACL::Entity::SOME);
           if (object->task_info && object->task_info->has_command() &&
@@ -253,6 +331,7 @@ public:
           } else {
             aclObject.set_type(mesos::ACL::Entity::ANY);
           }
+
           break;
         }
         case authorization::ACCESS_MESOS_LOG: {
@@ -497,7 +576,7 @@ private:
     vector<GenericACL> acls_;
 
     switch (action) {
-      case authorization::REGISTER_FRAMEWORK_WITH_ROLE:
+      case authorization::REGISTER_FRAMEWORK:
         foreach (
             const ACL::RegisterFramework& acl, acls.register_frameworks()) {
           GenericACL acl_;
@@ -509,7 +588,7 @@ private:
 
         return acls_;
         break;
-      case authorization::TEARDOWN_FRAMEWORK_WITH_PRINCIPAL:
+      case authorization::TEARDOWN_FRAMEWORK:
         foreach (
             const ACL::TeardownFramework& acl, acls.teardown_frameworks()) {
           GenericACL acl_;
@@ -532,7 +611,7 @@ private:
 
         return acls_;
         break;
-      case authorization::RESERVE_RESOURCES_WITH_ROLE:
+      case authorization::RESERVE_RESOURCES:
         foreach (const ACL::ReserveResources& acl, acls.reserve_resources()) {
           GenericACL acl_;
           acl_.subjects = acl.principals();
@@ -543,7 +622,7 @@ private:
 
         return acls_;
         break;
-      case authorization::UNRESERVE_RESOURCES_WITH_PRINCIPAL:
+      case authorization::UNRESERVE_RESOURCES:
         foreach (
             const ACL::UnreserveResources& acl, acls.unreserve_resources()) {
           GenericACL acl_;
@@ -555,7 +634,7 @@ private:
 
         return acls_;
         break;
-      case authorization::CREATE_VOLUME_WITH_ROLE:
+      case authorization::CREATE_VOLUME:
         foreach (const ACL::CreateVolume& acl, acls.create_volumes()) {
           GenericACL acl_;
           acl_.subjects = acl.principals();
@@ -566,7 +645,7 @@ private:
 
         return acls_;
         break;
-      case authorization::DESTROY_VOLUME_WITH_PRINCIPAL:
+      case authorization::DESTROY_VOLUME:
         foreach (const ACL::DestroyVolume& acl, acls.destroy_volumes()) {
           GenericACL acl_;
           acl_.subjects = acl.principals();
@@ -577,7 +656,7 @@ private:
 
         return acls_;
         break;
-      case authorization::GET_QUOTA_WITH_ROLE:
+      case authorization::GET_QUOTA:
         foreach (const ACL::GetQuota& acl, acls.get_quotas()) {
           GenericACL acl_;
           acl_.subjects = acl.principals();
@@ -629,7 +708,7 @@ private:
 
         return acls_;
         break;
-      case authorization::UPDATE_WEIGHT_WITH_ROLE:
+      case authorization::UPDATE_WEIGHT:
         foreach (const ACL::UpdateWeight& acl, acls.update_weights()) {
           GenericACL acl_;
           acl_.subjects = acl.principals();
@@ -845,7 +924,9 @@ process::Future<bool> LocalAuthorizer::authorized(
       request.object().has_task() ||
       request.object().has_task_info() ||
       request.object().has_executor_info() ||
-      request.object().has_quota_info())));
+      request.object().has_quota_info() ||
+      request.object().has_weight_info() ||
+      request.object().has_resource())));
 
   typedef Future<bool> (LocalAuthorizerProcess::*F)(
       const authorization::Request&);


Re: [3/3] mesos git commit: Enabled multiple field based authorization in the authorizer interface.

Posted by Alex Rukletsov <al...@mesosphere.com>.
Fixed in
https://github.com/apache/mesos/commit/d2ab4b49d3cc0b86bacc5ec3400b46cfa70c3a7b

On Fri, Nov 18, 2016 at 4:48 AM, Benjamin Bannier <
benjamin.bannier@mesosphere.io> wrote:

> Hi,
>
> This introduces a possibly uninitialized member `weight_info` which
> Coverity immediately detected. I filed MESOS-6604 for that. Could you
> please take that on @Alexander?
>
>
> Cheers,
>
> Benjamin
>
> > On Nov 16, 2016, at 6:00 PM, me@apache.org wrote:
> >
> > Enabled multiple field based authorization in the authorizer interface.
> >
> > Updates the authorizer interfaces and well as the local authorizer,
> > such that all actions which were limited to use a _role_ or a
> > _principal_ as an object, are able to use whole protobuf messages
> > as objects. This change enables more sofisticated authorization
> > mechanisms.
> >
> > Review: https://reviews.apache.org/r/52600/
> >
> >
> > Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
> > Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/bc0e6d7b
> > Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/bc0e6d7b
> > Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/bc0e6d7b
> >
> > Branch: refs/heads/master
> > Commit: bc0e6d7b0b367e5ff67dd5f395e1e06938b02399
> > Parents: 40c2e5f
> > Author: Alexander Rojas <al...@mesosphere.io>
> > Authored: Tue Nov 15 19:04:25 2016 -0800
> > Committer: Adam B <ad...@mesosphere.io>
> > Committed: Wed Nov 16 01:55:03 2016 -0800
> >
> > ----------------------------------------------------------------------
> > include/mesos/authorizer/authorizer.hpp   |   6 +-
> > include/mesos/authorizer/authorizer.proto |  54 ++++++++++++
> > src/authorizer/local/authorizer.cpp       | 115
> +++++++++++++++++++++----
> > 3 files changed, 157 insertions(+), 18 deletions(-)
> > ----------------------------------------------------------------------
> >
> >
> > http://git-wip-us.apache.org/repos/asf/mesos/blob/bc0e6d7b/
> include/mesos/authorizer/authorizer.hpp
> > ----------------------------------------------------------------------
> > diff --git a/include/mesos/authorizer/authorizer.hpp
> b/include/mesos/authorizer/authorizer.hpp
> > index cb365c7..7217600 100644
> > --- a/include/mesos/authorizer/authorizer.hpp
> > +++ b/include/mesos/authorizer/authorizer.hpp
> > @@ -61,7 +61,9 @@ public:
> >         task_info(object.has_task_info() ? &object.task_info() :
> nullptr),
> >         executor_info(
> >             object.has_executor_info() ? &object.executor_info() :
> nullptr),
> > -        quota_info(object.has_quota_info() ? &object.quota_info() :
> nullptr) {}
> > +        quota_info(object.has_quota_info() ? &object.quota_info() :
> nullptr),
> > +        weight_info(object.has_weight_info() ? &object.weight_info() :
> nullptr),
> > +        resource(object.has_resource() ? &object.resource() : nullptr)
> {}
> >
> >     const std::string* value;
> >     const FrameworkInfo* framework_info;
> > @@ -69,6 +71,8 @@ public:
> >     const TaskInfo* task_info;
> >     const ExecutorInfo* executor_info;
> >     const quota::QuotaInfo* quota_info;
> > +    const WeightInfo* weight_info;
> > +    const Resource* resource;
> >   };
> >
> >   /**
> >
> > http://git-wip-us.apache.org/repos/asf/mesos/blob/bc0e6d7b/
> include/mesos/authorizer/authorizer.proto
> > ----------------------------------------------------------------------
> > diff --git a/include/mesos/authorizer/authorizer.proto
> b/include/mesos/authorizer/authorizer.proto
> > index b6a9f14..0696a62 100644
> > --- a/include/mesos/authorizer/authorizer.proto
> > +++ b/include/mesos/authorizer/authorizer.proto
> > @@ -46,11 +46,17 @@ message Object {
> >   optional TaskInfo task_info = 4;
> >   optional ExecutorInfo executor_info = 5;
> >   optional quota.QuotaInfo quota_info = 6;
> > +  optional WeightInfo weight_info = 7;
> > +  optional Resource resource = 8;
> > }
> >
> >
> > // List of authorizable actions supported in Mesos.
> > +// NOTE: Values in this enum should be kept in
> > +// numerical order to prevent accidental aliasing.
> > enum Action {
> > +  option allow_alias = true;
> > +
> >   // This must be the first enum value in this list, to
> >   // ensure that if 'type' is not set, the default value
> >   // is UNKNOWN. This enables enum values to be added
> > @@ -58,19 +64,67 @@ enum Action {
> >   UNKNOWN = 0;
> >
> >   // Actions named *_WITH_foo may set a foo in `Object.value`.
> > +
> > +  // `REGISTER_FRAMEWORK` will have an object with `FrameworkInfo` set.
> > +  // The `_WITH_ROLE` alias is deprecated and will be removed after
> > +  // Mesos 1.2's deprecation cycle ends. The `value` field will continue
> > +  // to be set until that time.
> > +  REGISTER_FRAMEWORK = 1;
> >   REGISTER_FRAMEWORK_WITH_ROLE = 1;
> >
> >   // `RUN_TASK` will have an object with `FrameworkInfo` and `TaskInfo`
> set.
> >   RUN_TASK = 2;
> >
> > +  // `TEARDOWN_FRAMEWORK` will have an object with `FrameworkInfo` set.
> > +  // The `_WITH_PRINCIPAL` alias is deprecated and will be removed after
> > +  // Mesos 1.2's deprecation cycle ends. The `value` field will continue
> > +  // to be set until that time.
> > +  TEARDOWN_FRAMEWORK = 3;
> >   TEARDOWN_FRAMEWORK_WITH_PRINCIPAL = 3;
> > +
> > +  // `RESERVE_RESOURCES` will have an object with `Resource` set.
> > +  // The `_WITH_ROLE` alias is deprecated and will be removed after
> > +  // Mesos 1.2's deprecation cycle ends. The `value` field will continue
> > +  // to be set until that time.
> > +  RESERVE_RESOURCES = 4;
> >   RESERVE_RESOURCES_WITH_ROLE = 4;
> > +
> > +  // `UNRESERVE_RESOURCES` will have an object with `Resource` set.
> > +  // The `_WITH_PRINCIPAL` alias is deprecated and will be removed after
> > +  // Mesos 1.2's deprecation cycle ends. The `value` field will continue
> > +  // to be set until that time.
> > +  UNRESERVE_RESOURCES = 5;
> >   UNRESERVE_RESOURCES_WITH_PRINCIPAL = 5;
> > +
> > +  // `CREATE_VOLUME` will have an object with `Resource` set.
> > +  // The `_WITH_ROLE` alias is deprecated and will be removed after
> > +  // Mesos 1.2's deprecation cycle ends. The `value` field will continue
> > +  // to be set until that time.
> > +  CREATE_VOLUME = 6;
> >   CREATE_VOLUME_WITH_ROLE = 6;
> > +
> > +  // `DESTROY_VOLUME` will have an object with `Resource` set.
> > +  // The `_WITH_PRINCIPAL` alias is deprecated and will be removed after
> > +  // Mesos 1.2's deprecation cycle ends. The `value` field will continue
> > +  // to be set until that time.
> > +  DESTROY_VOLUME = 7;
> >   DESTROY_VOLUME_WITH_PRINCIPAL = 7;
> > +
> >   GET_ENDPOINT_WITH_PATH = 8;
> >   VIEW_ROLE = 9;
> > +
> > +  // `UPDATE_WEIGHT` will have an object with `WeightInfo` set.
> > +  // The `_WITH_ROLE` alias is deprecated and will be removed after
> > +  // Mesos 1.2's deprecation cycle ends. The `value` field will continue
> > +  // to be set until that time.
> > +  UPDATE_WEIGHT = 10;
> >   UPDATE_WEIGHT_WITH_ROLE = 10;
> > +
> > +  // `GET_QUOTA` will have an object with `QuotaInfo` set.
> > +  // The `_WITH_ROLE` alias is deprecated and will be removed after
> > +  // Mesos 1.2's deprecation cycle ends. The `value` field will continue
> > +  // to be set until that time.
> > +  GET_QUOTA = 11;
> >   GET_QUOTA_WITH_ROLE = 11;
> >
> >   // `UPDATE_QUOTA` will have an object with a `QuotaInfo` set.
> >
> > http://git-wip-us.apache.org/repos/asf/mesos/blob/bc0e6d7b/
> src/authorizer/local/authorizer.cpp
> > ----------------------------------------------------------------------
> > diff --git a/src/authorizer/local/authorizer.cpp b/src/authorizer/local/
> authorizer.cpp
> > index f1dff65..77e05dd 100644
> > --- a/src/authorizer/local/authorizer.cpp
> > +++ b/src/authorizer/local/authorizer.cpp
> > @@ -221,15 +221,7 @@ public:
> >     } else {
> >       switch (action_) {
> >         // All actions using `object.value` for authorization.
> > -        case authorization::REGISTER_FRAMEWORK_WITH_ROLE:
> > -        case authorization::TEARDOWN_FRAMEWORK_WITH_PRINCIPAL:
> > -        case authorization::RESERVE_RESOURCES_WITH_ROLE:
> > -        case authorization::UNRESERVE_RESOURCES_WITH_PRINCIPAL:
> > -        case authorization::CREATE_VOLUME_WITH_ROLE:
> > -        case authorization::DESTROY_VOLUME_WITH_PRINCIPAL:
> > -        case authorization::GET_QUOTA_WITH_ROLE:
> >         case authorization::VIEW_ROLE:
> > -        case authorization::UPDATE_WEIGHT_WITH_ROLE:
> >         case authorization::GET_ENDPOINT_WITH_PATH: {
> >           // Check object has the required types set.
> >           CHECK_NOTNULL(object->value);
> > @@ -239,6 +231,92 @@ public:
> >
> >           break;
> >         }
> > +        case authorization::REGISTER_FRAMEWORK: {
> > +          aclObject.set_type(mesos::ACL::Entity::SOME);
> > +          if (object->framework_info) {
> > +            aclObject.add_values(object->framework_info->role());
> > +          } else if (object->value) {
> > +            aclObject.add_values(*(object->value));
> > +          } else {
> > +            aclObject.set_type(mesos::ACL::Entity::ANY);
> > +          }
> > +
> > +          break;
> > +        }
> > +        case authorization::TEARDOWN_FRAMEWORK: {
> > +          aclObject.set_type(mesos::ACL::Entity::SOME);
> > +          if (object->framework_info) {
> > +            aclObject.add_values(object->framework_info->principal());
> > +          } else if (object->value) {
> > +            aclObject.add_values(*(object->value));
> > +          } else {
> > +            aclObject.set_type(mesos::ACL::Entity::ANY);
> > +          }
> > +
> > +          break;
> > +        }
> > +        case authorization::CREATE_VOLUME:
> > +        case authorization::RESERVE_RESOURCES: {
> > +          aclObject.set_type(mesos::ACL::Entity::SOME);
> > +          if (object->resource) {
> > +            aclObject.add_values(object->resource->role());
> > +          } else if (object->value) {
> > +            aclObject.add_values(*(object->value));
> > +          } else {
> > +            aclObject.set_type(mesos::ACL::Entity::ANY);
> > +          }
> > +
> > +          break;
> > +        }
> > +        case authorization::DESTROY_VOLUME: {
> > +          aclObject.set_type(mesos::ACL::Entity::SOME);
> > +          if (object->resource) {
> > +            aclObject.add_values(
> > +                object->resource->disk().persistence().principal());
> > +          } else if (object->value) {
> > +            aclObject.add_values(*(object->value));
> > +          } else {
> > +            aclObject.set_type(mesos::ACL::Entity::ANY);
> > +          }
> > +
> > +          break;
> > +        }
> > +        case authorization::UNRESERVE_RESOURCES: {
> > +          aclObject.set_type(mesos::ACL::Entity::SOME);
> > +          if (object->resource) {
> > +            aclObject.add_values(object->resource->reservation().
> principal());
> > +          } else if (object->value) {
> > +            aclObject.add_values(*(object->value));
> > +          } else {
> > +            aclObject.set_type(mesos::ACL::Entity::ANY);
> > +          }
> > +
> > +          break;
> > +        }
> > +        case authorization::GET_QUOTA: {
> > +          aclObject.set_type(mesos::ACL::Entity::SOME);
> > +          if (object->quota_info) {
> > +            aclObject.add_values(object->quota_info->role());
> > +          } else if (object->value) {
> > +            aclObject.add_values(*(object->value));
> > +          } else {
> > +            aclObject.set_type(mesos::ACL::Entity::ANY);
> > +          }
> > +
> > +          break;
> > +        }
> > +        case authorization::UPDATE_WEIGHT: {
> > +          aclObject.set_type(mesos::ACL::Entity::SOME);
> > +          if (object->weight_info) {
> > +            aclObject.add_values(object->weight_info->role());
> > +          } else if (object->value) {
> > +            aclObject.add_values(*(object->value));
> > +          } else {
> > +            aclObject.set_type(mesos::ACL::Entity::ANY);
> > +          }
> > +
> > +          break;
> > +        }
> >         case authorization::RUN_TASK: {
> >           aclObject.set_type(mesos::ACL::Entity::SOME);
> >           if (object->task_info && object->task_info->has_command() &&
> > @@ -253,6 +331,7 @@ public:
> >           } else {
> >             aclObject.set_type(mesos::ACL::Entity::ANY);
> >           }
> > +
> >           break;
> >         }
> >         case authorization::ACCESS_MESOS_LOG: {
> > @@ -497,7 +576,7 @@ private:
> >     vector<GenericACL> acls_;
> >
> >     switch (action) {
> > -      case authorization::REGISTER_FRAMEWORK_WITH_ROLE:
> > +      case authorization::REGISTER_FRAMEWORK:
> >         foreach (
> >             const ACL::RegisterFramework& acl,
> acls.register_frameworks()) {
> >           GenericACL acl_;
> > @@ -509,7 +588,7 @@ private:
> >
> >         return acls_;
> >         break;
> > -      case authorization::TEARDOWN_FRAMEWORK_WITH_PRINCIPAL:
> > +      case authorization::TEARDOWN_FRAMEWORK:
> >         foreach (
> >             const ACL::TeardownFramework& acl,
> acls.teardown_frameworks()) {
> >           GenericACL acl_;
> > @@ -532,7 +611,7 @@ private:
> >
> >         return acls_;
> >         break;
> > -      case authorization::RESERVE_RESOURCES_WITH_ROLE:
> > +      case authorization::RESERVE_RESOURCES:
> >         foreach (const ACL::ReserveResources& acl,
> acls.reserve_resources()) {
> >           GenericACL acl_;
> >           acl_.subjects = acl.principals();
> > @@ -543,7 +622,7 @@ private:
> >
> >         return acls_;
> >         break;
> > -      case authorization::UNRESERVE_RESOURCES_WITH_PRINCIPAL:
> > +      case authorization::UNRESERVE_RESOURCES:
> >         foreach (
> >             const ACL::UnreserveResources& acl,
> acls.unreserve_resources()) {
> >           GenericACL acl_;
> > @@ -555,7 +634,7 @@ private:
> >
> >         return acls_;
> >         break;
> > -      case authorization::CREATE_VOLUME_WITH_ROLE:
> > +      case authorization::CREATE_VOLUME:
> >         foreach (const ACL::CreateVolume& acl, acls.create_volumes()) {
> >           GenericACL acl_;
> >           acl_.subjects = acl.principals();
> > @@ -566,7 +645,7 @@ private:
> >
> >         return acls_;
> >         break;
> > -      case authorization::DESTROY_VOLUME_WITH_PRINCIPAL:
> > +      case authorization::DESTROY_VOLUME:
> >         foreach (const ACL::DestroyVolume& acl, acls.destroy_volumes()) {
> >           GenericACL acl_;
> >           acl_.subjects = acl.principals();
> > @@ -577,7 +656,7 @@ private:
> >
> >         return acls_;
> >         break;
> > -      case authorization::GET_QUOTA_WITH_ROLE:
> > +      case authorization::GET_QUOTA:
> >         foreach (const ACL::GetQuota& acl, acls.get_quotas()) {
> >           GenericACL acl_;
> >           acl_.subjects = acl.principals();
> > @@ -629,7 +708,7 @@ private:
> >
> >         return acls_;
> >         break;
> > -      case authorization::UPDATE_WEIGHT_WITH_ROLE:
> > +      case authorization::UPDATE_WEIGHT:
> >         foreach (const ACL::UpdateWeight& acl, acls.update_weights()) {
> >           GenericACL acl_;
> >           acl_.subjects = acl.principals();
> > @@ -845,7 +924,9 @@ process::Future<bool> LocalAuthorizer::authorized(
> >       request.object().has_task() ||
> >       request.object().has_task_info() ||
> >       request.object().has_executor_info() ||
> > -      request.object().has_quota_info())));
> > +      request.object().has_quota_info() ||
> > +      request.object().has_weight_info() ||
> > +      request.object().has_resource())));
> >
> >   typedef Future<bool> (LocalAuthorizerProcess::*F)(
> >       const authorization::Request&);
> >
>
>

Re: [3/3] mesos git commit: Enabled multiple field based authorization in the authorizer interface.

Posted by Benjamin Bannier <be...@mesosphere.io>.
Hi,

This introduces a possibly uninitialized member `weight_info` which Coverity immediately detected. I filed MESOS-6604 for that. Could you please take that on @Alexander?


Cheers,

Benjamin

> On Nov 16, 2016, at 6:00 PM, me@apache.org wrote:
> 
> Enabled multiple field based authorization in the authorizer interface.
> 
> Updates the authorizer interfaces and well as the local authorizer,
> such that all actions which were limited to use a _role_ or a
> _principal_ as an object, are able to use whole protobuf messages
> as objects. This change enables more sofisticated authorization
> mechanisms.
> 
> Review: https://reviews.apache.org/r/52600/
> 
> 
> Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
> Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/bc0e6d7b
> Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/bc0e6d7b
> Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/bc0e6d7b
> 
> Branch: refs/heads/master
> Commit: bc0e6d7b0b367e5ff67dd5f395e1e06938b02399
> Parents: 40c2e5f
> Author: Alexander Rojas <al...@mesosphere.io>
> Authored: Tue Nov 15 19:04:25 2016 -0800
> Committer: Adam B <ad...@mesosphere.io>
> Committed: Wed Nov 16 01:55:03 2016 -0800
> 
> ----------------------------------------------------------------------
> include/mesos/authorizer/authorizer.hpp   |   6 +-
> include/mesos/authorizer/authorizer.proto |  54 ++++++++++++
> src/authorizer/local/authorizer.cpp       | 115 +++++++++++++++++++++----
> 3 files changed, 157 insertions(+), 18 deletions(-)
> ----------------------------------------------------------------------
> 
> 
> http://git-wip-us.apache.org/repos/asf/mesos/blob/bc0e6d7b/include/mesos/authorizer/authorizer.hpp
> ----------------------------------------------------------------------
> diff --git a/include/mesos/authorizer/authorizer.hpp b/include/mesos/authorizer/authorizer.hpp
> index cb365c7..7217600 100644
> --- a/include/mesos/authorizer/authorizer.hpp
> +++ b/include/mesos/authorizer/authorizer.hpp
> @@ -61,7 +61,9 @@ public:
>         task_info(object.has_task_info() ? &object.task_info() : nullptr),
>         executor_info(
>             object.has_executor_info() ? &object.executor_info() : nullptr),
> -        quota_info(object.has_quota_info() ? &object.quota_info() : nullptr) {}
> +        quota_info(object.has_quota_info() ? &object.quota_info() : nullptr),
> +        weight_info(object.has_weight_info() ? &object.weight_info() : nullptr),
> +        resource(object.has_resource() ? &object.resource() : nullptr) {}
> 
>     const std::string* value;
>     const FrameworkInfo* framework_info;
> @@ -69,6 +71,8 @@ public:
>     const TaskInfo* task_info;
>     const ExecutorInfo* executor_info;
>     const quota::QuotaInfo* quota_info;
> +    const WeightInfo* weight_info;
> +    const Resource* resource;
>   };
> 
>   /**
> 
> http://git-wip-us.apache.org/repos/asf/mesos/blob/bc0e6d7b/include/mesos/authorizer/authorizer.proto
> ----------------------------------------------------------------------
> diff --git a/include/mesos/authorizer/authorizer.proto b/include/mesos/authorizer/authorizer.proto
> index b6a9f14..0696a62 100644
> --- a/include/mesos/authorizer/authorizer.proto
> +++ b/include/mesos/authorizer/authorizer.proto
> @@ -46,11 +46,17 @@ message Object {
>   optional TaskInfo task_info = 4;
>   optional ExecutorInfo executor_info = 5;
>   optional quota.QuotaInfo quota_info = 6;
> +  optional WeightInfo weight_info = 7;
> +  optional Resource resource = 8;
> }
> 
> 
> // List of authorizable actions supported in Mesos.
> +// NOTE: Values in this enum should be kept in
> +// numerical order to prevent accidental aliasing.
> enum Action {
> +  option allow_alias = true;
> +
>   // This must be the first enum value in this list, to
>   // ensure that if 'type' is not set, the default value
>   // is UNKNOWN. This enables enum values to be added
> @@ -58,19 +64,67 @@ enum Action {
>   UNKNOWN = 0;
> 
>   // Actions named *_WITH_foo may set a foo in `Object.value`.
> +
> +  // `REGISTER_FRAMEWORK` will have an object with `FrameworkInfo` set.
> +  // The `_WITH_ROLE` alias is deprecated and will be removed after
> +  // Mesos 1.2's deprecation cycle ends. The `value` field will continue
> +  // to be set until that time.
> +  REGISTER_FRAMEWORK = 1;
>   REGISTER_FRAMEWORK_WITH_ROLE = 1;
> 
>   // `RUN_TASK` will have an object with `FrameworkInfo` and `TaskInfo` set.
>   RUN_TASK = 2;
> 
> +  // `TEARDOWN_FRAMEWORK` will have an object with `FrameworkInfo` set.
> +  // The `_WITH_PRINCIPAL` alias is deprecated and will be removed after
> +  // Mesos 1.2's deprecation cycle ends. The `value` field will continue
> +  // to be set until that time.
> +  TEARDOWN_FRAMEWORK = 3;
>   TEARDOWN_FRAMEWORK_WITH_PRINCIPAL = 3;
> +
> +  // `RESERVE_RESOURCES` will have an object with `Resource` set.
> +  // The `_WITH_ROLE` alias is deprecated and will be removed after
> +  // Mesos 1.2's deprecation cycle ends. The `value` field will continue
> +  // to be set until that time.
> +  RESERVE_RESOURCES = 4;
>   RESERVE_RESOURCES_WITH_ROLE = 4;
> +
> +  // `UNRESERVE_RESOURCES` will have an object with `Resource` set.
> +  // The `_WITH_PRINCIPAL` alias is deprecated and will be removed after
> +  // Mesos 1.2's deprecation cycle ends. The `value` field will continue
> +  // to be set until that time.
> +  UNRESERVE_RESOURCES = 5;
>   UNRESERVE_RESOURCES_WITH_PRINCIPAL = 5;
> +
> +  // `CREATE_VOLUME` will have an object with `Resource` set.
> +  // The `_WITH_ROLE` alias is deprecated and will be removed after
> +  // Mesos 1.2's deprecation cycle ends. The `value` field will continue
> +  // to be set until that time.
> +  CREATE_VOLUME = 6;
>   CREATE_VOLUME_WITH_ROLE = 6;
> +
> +  // `DESTROY_VOLUME` will have an object with `Resource` set.
> +  // The `_WITH_PRINCIPAL` alias is deprecated and will be removed after
> +  // Mesos 1.2's deprecation cycle ends. The `value` field will continue
> +  // to be set until that time.
> +  DESTROY_VOLUME = 7;
>   DESTROY_VOLUME_WITH_PRINCIPAL = 7;
> +
>   GET_ENDPOINT_WITH_PATH = 8;
>   VIEW_ROLE = 9;
> +
> +  // `UPDATE_WEIGHT` will have an object with `WeightInfo` set.
> +  // The `_WITH_ROLE` alias is deprecated and will be removed after
> +  // Mesos 1.2's deprecation cycle ends. The `value` field will continue
> +  // to be set until that time.
> +  UPDATE_WEIGHT = 10;
>   UPDATE_WEIGHT_WITH_ROLE = 10;
> +
> +  // `GET_QUOTA` will have an object with `QuotaInfo` set.
> +  // The `_WITH_ROLE` alias is deprecated and will be removed after
> +  // Mesos 1.2's deprecation cycle ends. The `value` field will continue
> +  // to be set until that time.
> +  GET_QUOTA = 11;
>   GET_QUOTA_WITH_ROLE = 11;
> 
>   // `UPDATE_QUOTA` will have an object with a `QuotaInfo` set.
> 
> http://git-wip-us.apache.org/repos/asf/mesos/blob/bc0e6d7b/src/authorizer/local/authorizer.cpp
> ----------------------------------------------------------------------
> diff --git a/src/authorizer/local/authorizer.cpp b/src/authorizer/local/authorizer.cpp
> index f1dff65..77e05dd 100644
> --- a/src/authorizer/local/authorizer.cpp
> +++ b/src/authorizer/local/authorizer.cpp
> @@ -221,15 +221,7 @@ public:
>     } else {
>       switch (action_) {
>         // All actions using `object.value` for authorization.
> -        case authorization::REGISTER_FRAMEWORK_WITH_ROLE:
> -        case authorization::TEARDOWN_FRAMEWORK_WITH_PRINCIPAL:
> -        case authorization::RESERVE_RESOURCES_WITH_ROLE:
> -        case authorization::UNRESERVE_RESOURCES_WITH_PRINCIPAL:
> -        case authorization::CREATE_VOLUME_WITH_ROLE:
> -        case authorization::DESTROY_VOLUME_WITH_PRINCIPAL:
> -        case authorization::GET_QUOTA_WITH_ROLE:
>         case authorization::VIEW_ROLE:
> -        case authorization::UPDATE_WEIGHT_WITH_ROLE:
>         case authorization::GET_ENDPOINT_WITH_PATH: {
>           // Check object has the required types set.
>           CHECK_NOTNULL(object->value);
> @@ -239,6 +231,92 @@ public:
> 
>           break;
>         }
> +        case authorization::REGISTER_FRAMEWORK: {
> +          aclObject.set_type(mesos::ACL::Entity::SOME);
> +          if (object->framework_info) {
> +            aclObject.add_values(object->framework_info->role());
> +          } else if (object->value) {
> +            aclObject.add_values(*(object->value));
> +          } else {
> +            aclObject.set_type(mesos::ACL::Entity::ANY);
> +          }
> +
> +          break;
> +        }
> +        case authorization::TEARDOWN_FRAMEWORK: {
> +          aclObject.set_type(mesos::ACL::Entity::SOME);
> +          if (object->framework_info) {
> +            aclObject.add_values(object->framework_info->principal());
> +          } else if (object->value) {
> +            aclObject.add_values(*(object->value));
> +          } else {
> +            aclObject.set_type(mesos::ACL::Entity::ANY);
> +          }
> +
> +          break;
> +        }
> +        case authorization::CREATE_VOLUME:
> +        case authorization::RESERVE_RESOURCES: {
> +          aclObject.set_type(mesos::ACL::Entity::SOME);
> +          if (object->resource) {
> +            aclObject.add_values(object->resource->role());
> +          } else if (object->value) {
> +            aclObject.add_values(*(object->value));
> +          } else {
> +            aclObject.set_type(mesos::ACL::Entity::ANY);
> +          }
> +
> +          break;
> +        }
> +        case authorization::DESTROY_VOLUME: {
> +          aclObject.set_type(mesos::ACL::Entity::SOME);
> +          if (object->resource) {
> +            aclObject.add_values(
> +                object->resource->disk().persistence().principal());
> +          } else if (object->value) {
> +            aclObject.add_values(*(object->value));
> +          } else {
> +            aclObject.set_type(mesos::ACL::Entity::ANY);
> +          }
> +
> +          break;
> +        }
> +        case authorization::UNRESERVE_RESOURCES: {
> +          aclObject.set_type(mesos::ACL::Entity::SOME);
> +          if (object->resource) {
> +            aclObject.add_values(object->resource->reservation().principal());
> +          } else if (object->value) {
> +            aclObject.add_values(*(object->value));
> +          } else {
> +            aclObject.set_type(mesos::ACL::Entity::ANY);
> +          }
> +
> +          break;
> +        }
> +        case authorization::GET_QUOTA: {
> +          aclObject.set_type(mesos::ACL::Entity::SOME);
> +          if (object->quota_info) {
> +            aclObject.add_values(object->quota_info->role());
> +          } else if (object->value) {
> +            aclObject.add_values(*(object->value));
> +          } else {
> +            aclObject.set_type(mesos::ACL::Entity::ANY);
> +          }
> +
> +          break;
> +        }
> +        case authorization::UPDATE_WEIGHT: {
> +          aclObject.set_type(mesos::ACL::Entity::SOME);
> +          if (object->weight_info) {
> +            aclObject.add_values(object->weight_info->role());
> +          } else if (object->value) {
> +            aclObject.add_values(*(object->value));
> +          } else {
> +            aclObject.set_type(mesos::ACL::Entity::ANY);
> +          }
> +
> +          break;
> +        }
>         case authorization::RUN_TASK: {
>           aclObject.set_type(mesos::ACL::Entity::SOME);
>           if (object->task_info && object->task_info->has_command() &&
> @@ -253,6 +331,7 @@ public:
>           } else {
>             aclObject.set_type(mesos::ACL::Entity::ANY);
>           }
> +
>           break;
>         }
>         case authorization::ACCESS_MESOS_LOG: {
> @@ -497,7 +576,7 @@ private:
>     vector<GenericACL> acls_;
> 
>     switch (action) {
> -      case authorization::REGISTER_FRAMEWORK_WITH_ROLE:
> +      case authorization::REGISTER_FRAMEWORK:
>         foreach (
>             const ACL::RegisterFramework& acl, acls.register_frameworks()) {
>           GenericACL acl_;
> @@ -509,7 +588,7 @@ private:
> 
>         return acls_;
>         break;
> -      case authorization::TEARDOWN_FRAMEWORK_WITH_PRINCIPAL:
> +      case authorization::TEARDOWN_FRAMEWORK:
>         foreach (
>             const ACL::TeardownFramework& acl, acls.teardown_frameworks()) {
>           GenericACL acl_;
> @@ -532,7 +611,7 @@ private:
> 
>         return acls_;
>         break;
> -      case authorization::RESERVE_RESOURCES_WITH_ROLE:
> +      case authorization::RESERVE_RESOURCES:
>         foreach (const ACL::ReserveResources& acl, acls.reserve_resources()) {
>           GenericACL acl_;
>           acl_.subjects = acl.principals();
> @@ -543,7 +622,7 @@ private:
> 
>         return acls_;
>         break;
> -      case authorization::UNRESERVE_RESOURCES_WITH_PRINCIPAL:
> +      case authorization::UNRESERVE_RESOURCES:
>         foreach (
>             const ACL::UnreserveResources& acl, acls.unreserve_resources()) {
>           GenericACL acl_;
> @@ -555,7 +634,7 @@ private:
> 
>         return acls_;
>         break;
> -      case authorization::CREATE_VOLUME_WITH_ROLE:
> +      case authorization::CREATE_VOLUME:
>         foreach (const ACL::CreateVolume& acl, acls.create_volumes()) {
>           GenericACL acl_;
>           acl_.subjects = acl.principals();
> @@ -566,7 +645,7 @@ private:
> 
>         return acls_;
>         break;
> -      case authorization::DESTROY_VOLUME_WITH_PRINCIPAL:
> +      case authorization::DESTROY_VOLUME:
>         foreach (const ACL::DestroyVolume& acl, acls.destroy_volumes()) {
>           GenericACL acl_;
>           acl_.subjects = acl.principals();
> @@ -577,7 +656,7 @@ private:
> 
>         return acls_;
>         break;
> -      case authorization::GET_QUOTA_WITH_ROLE:
> +      case authorization::GET_QUOTA:
>         foreach (const ACL::GetQuota& acl, acls.get_quotas()) {
>           GenericACL acl_;
>           acl_.subjects = acl.principals();
> @@ -629,7 +708,7 @@ private:
> 
>         return acls_;
>         break;
> -      case authorization::UPDATE_WEIGHT_WITH_ROLE:
> +      case authorization::UPDATE_WEIGHT:
>         foreach (const ACL::UpdateWeight& acl, acls.update_weights()) {
>           GenericACL acl_;
>           acl_.subjects = acl.principals();
> @@ -845,7 +924,9 @@ process::Future<bool> LocalAuthorizer::authorized(
>       request.object().has_task() ||
>       request.object().has_task_info() ||
>       request.object().has_executor_info() ||
> -      request.object().has_quota_info())));
> +      request.object().has_quota_info() ||
> +      request.object().has_weight_info() ||
> +      request.object().has_resource())));
> 
>   typedef Future<bool> (LocalAuthorizerProcess::*F)(
>       const authorization::Request&);
>