volume type apis missing context.elevated

[Gerald McBrearty]

Within cinder volume types class api -> class VolumeTypesManageController
The _create, _delete, _update methods all check to see if
the user is authorized to run this action.
The all have a
    authorize(context)
call.

And the policy file looks like this:
etc/cinder/policy.json
   "volume_extension:types_manage": "rule:admin_api",

The problem is once you get down to
volume_type_create, volume_type_update, volume_type_destroy
these methods all have a decorator
@require_admin_context

What is missing is a context.elevated when each of the api routines
calls volume_types.create, volume_types.update,
and volume_types.destroy

This allows the operator to use the policy to control access through the api

[Mitsuhiro Tanino (mitsuhiro-tanino)]

Hi,

I'm trying to understand more detail of this problem.
Currently, if a user have admin role, the user can create, delete, update volume types without problem based on the policy.json.

Why do we need context.elevated to these APIs?
  - volume_types.create, volume_types.update, and volume_types.destroy

When I changed followings,
  - Add context.elevated to these APIs
  - Change rule from "rule:admin_api" to "rule:admin_or_owner" in policy.json
all users who belongs different projects could create/delete/update volume types.
I think this situation is not desirable.

Am I missing something?

[Gerald McBrearty (gfm-r)]

Hi,
That is a really good question.

We have operators that require the separation of admin roles more that just "admin", as an example operators might want to separate out the roles so that they have an all powerful admin role but also give volume type administration to a different admin role such as a volume type admin.

The purpose of a policy file is to allow an operator control over what roles can execute what operations. In this case the API is properly checking the policy for allowing the operation but then failing if an operator changes what who can execute the
operation from "admin". The problem is the failing a different authorization check later defeats the purpose of having a policy file.

I agree that it doesn't sound wise to make this admin_or_owner. I wasn't asking in this defect to change the default from "admin" in the policy file.

[Mitsuhiro Tanino (mitsuhiro-tanino)]

Thank you for your explanation.

Do you have an example policy file which realizes your scenario above? Or do we need additional features in addition to context.elevated for the APIs?

[Gerald McBrearty (gfm-r)]

"volume_extension:types_manage": "rule:admin_or_storage_type_admin"

From a testing standpoint to verify that the code change works one could use admin_or_owner to verify that the API is works with a given rule.

[Mitsuhiro Tanino (mitsuhiro-tanino)]

Thanks.
I tried following steps and the user who have "storage_type_admin" can delete volume type as we expected.
I'll post a patch.

- Pass context.elevated instead of context during volume_types.create, volume_types.update, and volume_types.destroy
- Adds storage_type_admin role
- Adds "admin_or_storage_type_admin" rule to policy.json like this
    "admin_or_storage_type_admin": "is_admin:True or role:storage_type_admin",

- Modify rule for types_manage like this;
    "volume_extension:types_manage": "rule:admin_or_storage_type_admin",

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/274204

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/274204
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=4ccd1bd15100b7046e634323e55ad610ef52e0ab
Submitter: Jenkins
Branch: master

commit 4ccd1bd15100b7046e634323e55ad610ef52e0ab
Author: Mitsuhiro Tanino <email address hidden>
Date: Fri Jan 29 12:48:33 2016 -0500

    Permit volume type operations for policy authorized users

    Currently, following volume type operations are not
    permitted for non admin users because these db operations
    require admin context.

    * create
    * update
    * delete
    * type-access-add
    * type-access-remove

    In order to allow a cloud operator to use the policy based
    user access control for these operations, a context during
    these operations should be elevated before db operations.

    After applying this change, the cloud operator can manage
    policy for volume type operations like this.

    1. To permit volume type operations for specific user,
       add "storage_type_admin" role.

    2. Add "admin_or_storage_type_admin" rule to policy.json.
       "admin_or_storage_type_admin":
           "is_admin:True or role:storage_type_admin",

    3. Modify rule for types_manage.
       "volume_extension:types_manage":
           "rule:admin_or_storage_type_admin",

    Change-Id: I1e91ad6573f78cfa35c36209944ea1d074a17604
    Closes-Bug: #1538305

[Thierry Carrez (ttx)]

This issue was fixed in the openstack/cinder 8.0.0.0rc1 release candidate.