get trust missing @controller.protected

[Steve Martinelli]

Currently, there is no @controller.protected decorator surrounding the get_trust function at the trust controller level.
Since there is an entry for it in our sample policy.json files, it probably should be protected.

[OpenStack Infra (hudson-openstack)]

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

[Ajaya Agrawal (ajayaa)]

This bug is fairly trivial to fix. But while writing test case I discovered that there is no relationship between the user and trust. So rules like user_id:%(user_id)s or user_id:%(trust.trustor_user_id)s won't work. Basically in the enforcer target is empty. So only role based policy checking will be possible. That means only admin should be allowed to call this api which does not make sense.

Any idea on how to tackle this would be appreciated.

[Lance Bragstad (lbragstad)]

This seems to be somewhat along the same lines as bug 1373599

I'm trying to recreate the failures locally, and it seems to be around issue with using unscoped token to create the trust? Still digging around...

[OpenStack Infra (hudson-openstack)]

Change abandoned by Morgan Fainberg (<email address hidden>) on branch: master
Review: https://review.openstack.org/73907
Reason: This change is being abandoned because it has a negative score and has not seen an update in > 60 days. Feel free to re-instate this patch (as the author) by using the "restore" button or any member of the core team can re-instate the patch.

[OpenStack Infra (hudson-openstack)]

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

[Lin Hua Cheng (lin-hua-cheng)]

To be able to consume the policy rule for "get_trust", the policy rule have to be updated.

Current rule is:
    "identity:get_trust": "rule:admin_or_owner",

Using the "rule:owner" is not applicable for trust, the correct policy should be checking if the user is the trustor or trustee.

The policy file should be updated as:
     "trustor": "user_id:%(target.trust.trustor_user_id)s",
     "trustee": "user_id:%(target.trust.trustee_user_id)s",
     "trustor_or_trustee": "rule:trustor or rule:trustee",
     "identity:get_trust": "rule:admin_required or rule:trustor_or_trustee",

So after updating the policy file and associating the policy check to the get_trust() method, this should reflect the same policy check what the code does.

The unit test passes, however grenade gate is failing. Grenade does not update the policy file with the latest version. This is to make sure that new code is compatible with older version of policy file. Protecting the "get_trust" method with the old policy rule won't work due to the wrong definition of trust "owner".

So.. We can't really make "get_trust" without breaking backward compatibility.

An approach suggested is:
1. Remove the get_trust from the policy file since it is unused anyway.
2. Figure out later how to add back the policy check and protect the "get_trust" method.

[OpenStack Infra (hudson-openstack)]

Change abandoned by Lin Hua Cheng (<email address hidden>) on branch: master
Review: https://review.openstack.org/172620
Reason: To consume the policy rule for get_trust, it would require updating the policy json. However, updating the policy json does not get updated automatically. So protecting the get_trust will break keystone's compatability with old policy file. This is the reason why grenade has been failing.

[Lin Hua Cheng (lin-hua-cheng)]

I am going to open a separate bug to cleanup the policy file. And keep this one open for keeping the history and adding back the policy check later.

[Steve Martinelli (stevemar)]

unassigning due to inactivity,

i'm also wondering if we should mark this as won't fix

[David Stanek (dstanek)]

I think that since this is protected in code that it would be safe to mark this as "won't fix". The only reason I think we'd want to keep this open is if someone has a vision of how this needs to work.

[David Stanek (dstanek)]

Marking as WONTFIX because we can't really add the decorator at this point.