Open Stack

Hey Kevin,

On Wed, 2012-10-10 at 10:06 -0500, Kevin L. Mitchell wrote:
> On Wed, 2012-10-10 at 14:45 +0100, Mark McLoughlin wrote:
> > > I think the policy implementation is clearer when every complex policy
> > > decision is "denormalized" into one action and one target with a
> > > simple true-false response. If you want some roles to be able to (for
> > > example) modify a specific field, then there should be an action for
> > > modifying that particular field. Otherwise I think the policy will
> > > become overly complex and hard to follow.
> > 
> > That seems like a good point. I admit that I didn't pay enough attention
> > to how this would actually be used.
> > 
> > In the docs, Kevin says:
> > 
> >   Consider a function that not only wants to check whether a user is
> >   allowed to modify an object, but wants to see which set of fields the
> >   user is allowed to modify.  For this, we can use the new "case"
> >   expression::
> > 
> >       case {
> >         "fulladmin" = role:admin;
> >         "projectadmin" = project_id:%(project_id)s and role:projectadmin
> >       }
> > 
> > If we take a specific example of admin_actions:pause, this would allow
> > the rule to not just say "the action is allowed" but also "the action is
> > allowed because the user is a full admin" or "the user is a project
> > admin".
> > 
> > The question then would be, what does nova do with that info? And if
> > nova might make a decision that something is allowed for "fulladmin" but
> > not "projectadmin", then why aren't they not two separate rules in the
> > policy? By hard-coding the "full vs project admin" decision in Nova, it
> > means it's not configurable in the policy.
> > 
> > Kevin - maybe you could elaborate more on the specific use case?
> 
> The specific case I had in mind was a hypothetical case where an API
> update method allows all fields to be updated for admin users but only
> some fields to be updated for project admins.  For instance, maybe a
> full admin can change which project the object belongs to as well as
> some limit, while a project admin should only be able to alter the
> limit.  I believe this was one element of the (now reverted) user quotas
> patch, which ended up having quota keys
> "compute_extension:quotas:update_admin" and
> "compute_extension:quotas:update_project_admin".

So, we're talking about this:

 https://github.com/openstack/nova/commit/391f345d

      "admin_or_owner":  [["role:admin"], ["project_id:%(project_id)s"]],
 +    "admin_or_projectadmin": [["role:projectadmin"], ["role:admin"]],
 
 -    "compute_extension:quotas:update": [["rule:admin_api"]],
 +    "compute_extension:quotas:update_for_project": [["rule:admin_api"]],
 +    "compute_extension:quotas:update_for_user": [["rule:admin_or_projectadmin"]],

And then:

 +        if 'user_id' in params:
 +            # Project admins are able to modify per-user quotas.
 +            authorize_action(context, 'update_for_user')
 ...
 +        else:
 +            # Only admins are able to modify per-project quotas.
 +            authorize_action(context, 'update_for_project')

(There's an obvious issue in the above that we're making the
"projectadmin" role apply to all projects, not just ones the
projectadmin is a member of)

Your idea would be to make it more like:

 +        if 'user_id' in params:
 +            # Project admins are able to modify per-user quotas.
 +            authorize_action(context, 'update', require='projectadmin')
 ...
 +        else:
 +            # Only admins are able to modify per-project quotas.
 +            authorize_action(context, 'update', require='fulladmin')

Which is just as configurable in the policy, but won't result in an
explosion of rules where we want to make similar distinctions. It is
confusing, though, since you'd need to document it in the policy with
e.g

 +    # Valid rule results are False, 'fulladmin' or 'projectadmin'
 +    "compute_extension:quotas:update": [["rule:admin_or_projectadmin"]],

But I think the specific example we're talking about here is pretty
dumb. Having a 'projectadmin' role which applies to all projects you're
a member doesn't seem very useful. We really need users to have
per-project roles, and that concept would need to be added in keystone.

> In my opinion, there is some merit to the desire to keep policies
> simple, i.e., true-false decision making; but I fear we could end up
> with an explosion of policy rules like those the user quotas patch
> added, which I believe will make the policy files harder to understand
> as we enhance nova.   I (obviously :) feel that having the "case"
> enhancement strikes a good balance between the two extremes here.

I'm leaning towards dropping this feature for now until we have a really
compelling use case for it. Now that I understand the context a bit
better, I think I'd be against adding this "global projectadmin role"
idea at all.

The danger with leaving it in even if there's no good use case for it is
that folks might find bad/confusing use cases for it :)

Cheers,
Mark.