can't delete volumes when non admin

Bug #1274053 reported by Yves-Gwenael Bourhis
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
OpenStack Dashboard (Horizon)
Fix Released
High
David Lyle

Bug Description

Environment:
===========
devstack, master branch on 29/01/2014 (older versions work).

Steps to reproduce:
================
- Log in as demo (or any NON-admin user) and into the demo tenant/project
- Create a volume.

The created volume can not be deleted. Only admins can delete the volumes a user created.
The only way for a user to delete the volume(s) he created is via the "cinder delete" command in CLI (as user, no need to be admin) so it is not a permissions issue since the user can perform the volume deletion via CLI.

Revision history for this message
Yves-Gwenael Bourhis (yves-gwenael-bourhis) wrote :

This bug was introduced by https://github.com/openstack/horizon/commit/985bd7390d0df59019732c1bd52c5642deee4465
changing in settings.py:

POLICY_CHECK_FUNCTION = policy.check
to:
POLICY_CHECK_FUNCTION = None

Fixes the issue, BUT this should be overrideable in local_settings.py
However, the settings.py file loads local_settings.py before setting this constant, so it is not overrideable.

Revision history for this message
Julie Pichon (jpichon) wrote :

You can also change the cinder rule for deletion to read as "[]" instead of the default rule, which performs a admin_or_owner check.

https://github.com/openstack/horizon/blob/master/openstack_dashboard/conf/cinder_policy.json#L10

I'm not marking the bug as invalid because I wonder if we may want to change that rule back - it is a surprising change. I think it might not be matching the cinder policy either - volume:delete is not defined at https://github.com/openstack/cinder/blob/master/etc/cinder/policy.json which seems to mean [] implicitly.

Changed in horizon:
status: New → Confirmed
Revision history for this message
Yves-Gwenael Bourhis (yves-gwenael-bourhis) wrote :

Thanks Julie,

admin_or_owner check should allow the owner since the owner has the permission to delete his own volumes (on a devstack as on our infra), so this is where something goes wrong and needs investigation.

Revision history for this message
Julie Pichon (jpichon) wrote :

Do you mean that even the user who created the volume cannot delete it? On devstack once I change the (openstack_dashboard/conf/cinder_policy.json) rule to "[]" like the others and reload the page, the Delete button does appear and works fine.

Note that we're not syncing yet with Cinder's policy file so we have to keep our own copy of the policy in /opt/stack/horizon/ (or possibly /etc/openstack_openstack/ in a real install). Therefore even if you can delete using the command line, it's possible that the policy file Horizon knows about does not correctly reflect the policy you actually have in place.

Revision history for this message
Yves-Gwenael Bourhis (yves-gwenael-bourhis) wrote :

Hi Julie,

With:
    "volume:delete": []
It works fine.

What I meant is that with:
    "default": [["rule:admin_or_owner"]],
    "volume:delete": [["rule:default"]]
We would expect the user to be able to delete if he is the owner ("admin_or_owner" sounds like this), but even when being the owner it doesn't work.

Let me know if anything was unclear.

Revision history for this message
Julie Pichon (jpichon) wrote :

Hah-ah! I finally get it, sorry for the confusion. I now see the problem, and can reproduce it: if I create a volume as user demo, I cannot see the delete button even though it is my volume.

The rule:default actually defines owner as member of the project (my misunderstanding as well), not creator.

The problem seem to be coming from the code that initialises the target if no details are set, at https://github.com/openstack/horizon/blob/985bd7390d/openstack_dashboard/policy.py#L111 . When I reproduce the problem on devstack, project_id is None therefore the target project doesn't match. Because the project_id attribute exists (it just happens to be set to None), we don't set it to the user's project_id on line 112.

My initial thought would be to add an additional check for "None" since as far as I know, it's not possible for a resource in OpenStack to not be associated with a project.

Changed in horizon:
status: Confirmed → Triaged
importance: Undecided → High
milestone: none → icehouse-3
David Lyle (david-lyle)
Changed in horizon:
assignee: nobody → David Lyle (david-lyle)
Changed in horizon:
status: Triaged → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to horizon (master)

Reviewed: https://review.openstack.org/70827
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=ae82f1d83ccedb04da53f550dfe45615b19004fa
Submitter: Jenkins
Branch: master

commit ae82f1d83ccedb04da53f550dfe45615b19004fa
Author: David Lyle <email address hidden>
Date: Mon Feb 3 13:09:04 2014 -0700

    Policy check needs to check for None in target

    For objects that don't have a project_id attached to them, the
    property can be set to None, which does not get overridden in
    the policy engine where it being not set currently does. This
    adds a check for user_id and project_id being None and setting them
    if they are.

    Change-Id: I7aeb6d3830a19a7191de9944f8de90ee12dbf127
    Closes-bug: #1274053

Changed in horizon:
status: In Progress → Fix Committed
Thierry Carrez (ttx)
Changed in horizon:
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in horizon:
milestone: icehouse-3 → 2014.1
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.