403 on trusts delete causes DELETE_FAILED

[Steve Baker]

The second time the stack is delete is attempted the delete succeeds.

2014-04-17 15:35:54.174 ERROR heat.engine.parser [-] You are not authorized to perform the requested action. (HTTP 403)
2014-04-17 15:35:54.174 TRACE heat.engine.parser Traceback (most recent call last):
2014-04-17 15:35:54.174 TRACE heat.engine.parser File "/home/steveb/dev/localstack/heat/heat/engine/parser.py", line 667, in delete
2014-04-17 15:35:54.174 TRACE heat.engine.parser self.clients.keystone().delete_trust(trust_id)
2014-04-17 15:35:54.174 TRACE heat.engine.parser File "/home/steveb/dev/localstack/heat/heat/common/heat_keystoneclient.py", line 255, in delete_trust
2014-04-17 15:35:54.174 TRACE heat.engine.parser self.client.trusts.delete(trust_id)
2014-04-17 15:35:54.174 TRACE heat.engine.parser File "/home/steveb/dev/localstack/python-keystoneclient/keystoneclient/v3/contrib/trusts.py", line 87, in delete
2014-04-17 15:35:54.174 TRACE heat.engine.parser return super(TrustManager, self).delete(trust_id=base.getid(trust))
2014-04-17 15:35:54.174 TRACE heat.engine.parser File "/home/steveb/dev/localstack/python-keystoneclient/keystoneclient/base.py", line 66, in func
2014-04-17 15:35:54.174 TRACE heat.engine.parser return f(*args, **new_kwargs)
2014-04-17 15:35:54.174 TRACE heat.engine.parser File "/home/steveb/dev/localstack/python-keystoneclient/keystoneclient/base.py", line 360, in delete
2014-04-17 15:35:54.174 TRACE heat.engine.parser self.build_url(dict_args_in_out=kwargs))
2014-04-17 15:35:54.174 TRACE heat.engine.parser File "/home/steveb/dev/localstack/python-keystoneclient/keystoneclient/base.py", line 197, in _delete
2014-04-17 15:35:54.174 TRACE heat.engine.parser return self.client.delete(url)
2014-04-17 15:35:54.174 TRACE heat.engine.parser File "/home/steveb/dev/localstack/python-keystoneclient/keystoneclient/httpclient.py", line 605, in delete
2014-04-17 15:35:54.174 TRACE heat.engine.parser return self._cs_request(url, 'DELETE', **kwargs)
2014-04-17 15:35:54.174 TRACE heat.engine.parser File "/home/steveb/dev/localstack/python-keystoneclient/keystoneclient/httpclient.py", line 582, in _cs_request
2014-04-17 15:35:54.174 TRACE heat.engine.parser return self.request(url, method, **kwargs)
2014-04-17 15:35:54.174 TRACE heat.engine.parser File "/home/steveb/dev/localstack/python-keystoneclient/keystoneclient/httpclient.py", line 564, in request
2014-04-17 15:35:54.174 TRACE heat.engine.parser resp = super(HTTPClient, self).request(url, method, **kwargs)
2014-04-17 15:35:54.174 TRACE heat.engine.parser File "/home/steveb/dev/localstack/python-keystoneclient/keystoneclient/baseclient.py", line 21, in request
2014-04-17 15:35:54.174 TRACE heat.engine.parser return self.session.request(url, method, **kwargs)
2014-04-17 15:35:54.174 TRACE heat.engine.parser File "/home/steveb/dev/localstack/python-keystoneclient/keystoneclient/utils.py", line 318, in inner
2014-04-17 15:35:54.174 TRACE heat.engine.parser return func(*args, **kwargs)
2014-04-17 15:35:54.174 TRACE heat.engine.parser File "/home/steveb/dev/localstack/python-keystoneclient/keystoneclient/session.py", line 251, in request
2014-04-17 15:35:54.174 TRACE heat.engine.parser raise exceptions.from_response(resp, method, url)
2014-04-17 15:35:54.174 TRACE heat.engine.parser Forbidden: You are not authorized to perform the requested action. (HTTP 403)
2014-04-17 15:35:54.174 TRACE heat.engine.parser

[Steven Hardy (shardy)]

Any steps to reproduce? I've never seen this problem (unless you try to delete the stack as a different user the first time?) so more info on how you triggered it would be most helpful :)

[Steve Baker (steve-stevebaker)]

I think it is reproduced in the demo tenant when you create the stack with user admin, then delete it with user demo. I've lowered the Importance.

[Steven Hardy (shardy)]

> I think it is reproduced in the demo tenant when you create the stack with user admin, then delete it with user demo. I've lowered the Importance.

That's not expected to work - you have to delete the stack either as the user who created it, or an admin:

https://github.com/openstack/keystone/blob/master/keystone/trust/controllers.py#L44

So e.g you could create a stack as demo and delete it as either demo or admin, but if you create the stack as admin, keystone will only allow you to delete it as admin (or some other user with the admin role in the project.

So unless there's some other reproduce scenario not involving trying to do admin things as demo, I think this is invalid

[Steve Baker (steve-stevebaker)]

But the second time the delete is attempted it works, so it sounds like we need an authorise check before the delete is started so the user is told why they cannot delete the stack.

[Steven Hardy (shardy)]

> But the second time the delete is attempted it works

Hmm, yeah that does sound wrong, I guess we're deleting the stored trust ID by nullifying the user_creds record despite failing to delete the trust, when really it should be a persistent DELETE_FAILED state until you try the delete as admin.

I'll look into a fix for that.

[Steve Baker (steve-stevebaker)]

I think you wouldn't want to have some other user put your stack into a persistent DELETE_FAILED state. Would it be possible to authorise the user before attempting to delete?

[Steven Hardy (shardy)]

> I think you wouldn't want to have some other user put your stack into a persistent DELETE_FAILED state

Well then arguably they should have policy enforced separation between the users, as even if we hard-code something which prevents this scenario, the user could easily do anything else like abandon or update the stack (if they're allowed to even try deleting it)

> Would it be possible to authorise the user before attempting to delete?

Well yes, but only if we hard-code an assumption of stack-owner-or-admin which matches what's currently in keystone.

I'm not actually sure why that enforcement is done in the code and not the policy in the keystone trusts code, but obviously it only makes sense for us to hard-code something which is likely to stack hard-coded in keystone - if that check moves to the policy (where, arguably, it should be), then we can't enforce it as we don't have visibility of the potentially deployer modified policy rules.

Another possibility might be to try deleting the trust first and fail deletion before deleting any other resources if the stack-deleting user lacks rights to delete the trust? This would still end up in a DELETE_FAILED state, but would avoid enforcing policy in the engine code, while failing in a less interruptive way.

[Steven Hardy (shardy)]

I meant *stay* hard coded :)

[Steven Hardy (shardy)]

Ok, so I've got a use-case which changes my view on this problem somewhat:

1. User A creates a stack in project X
2. User A leaves company, user B is given same roles in project X to manage their stacks
3. User B deletes stack, sees error above, unless they're an admin.

I guess this implies we do need to do cleanup via the trust (e.g impersonating the trustor, e.g User A who originally created the stack), or allow a stack-update to create a new trust on update if the user doing the update differs from the current owner.

[Steven Hardy (shardy)]

Ok so, thinking about this a bit more:

1. We probably need a check on delete to ensure the user_id doing the delete is the stack owner (e.g the trustee of the trust we have stored)
2. If they are not, the delete should fail non-destructively, with a message saying why
3. Optionally, the user should be able to claim ownership of the stack, e.g heat stack-update --owner or something, which would provide a recovery path when the trustee/owner we have stored no longer exists in keystone.
4. In the event a user takes ownership of the stack, we update the stored trust with a new one based on their user_id as the trustee - in the event the old trustee still exists, we may need an admin cleanup path to avoid leaking the old trust (this is not an issue in the event the user no longer exists, as the trusts should be deleted with the user).

I'll post a more detailed description of this to openstack-dev for further discussion.

[Steven Hardy (shardy)]

Note this is related to https://bugs.launchpad.net/heat/+bug/1342593

[Kevin Fox (kevpn)]

I've run into this bug too. User A created stack, user B on same project could not delete stack. They are both are sysadmins on the project and should be able to manage all the stacks on the project.

[Kevin Fox (kevpn)]

This is worse for me now. a second delete, even as the trust owner does not work. It fails like:
Delete_Failed: Error deleting project: Could not find project, af09994b513145aa83b81823870947b3. (HTTP 404)

[Kevin Fox (kevpn)]

I had to work around this by poking this back into keystone's db:
insert into project(id, name, extra, description, enabled, domain_id) values('af09994b513145aa83b81823870947b3', 'kfoxtest', '{}', 'Kfox test', 1, '911bbc94745043aca7a41c62ce2a47c7');

Then I could delete the stack as the stack owner.

[Steven Hardy (shardy)]

@Kevin Fox: Can you provide any more details about the project delete failure?

AFAICS the project having been deleted should not cause that error:

https://github.com/openstack/heat/blob/master/heat/common/heat_keystoneclient.py#L490

So the backtrace showing where that error is coming from would be helpful.

[Steven Hardy (shardy)]

Ok, I've run out of time to look at the update-owner stuff for juno, so the simplest solution seems to be to use the trust to delete the trust via impersonation.

[Steven Hardy (shardy)]

@Kevin Fox: I'm suspecting you had some other issue, as I've just re-tested, and the delete works on the second attempt, as initially reported.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/120132
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=dc3c22c66bc45ad4da0dd15e5b30e28a36a36d36
Submitter: Jenkins
Branch: master

commit dc3c22c66bc45ad4da0dd15e5b30e28a36a36d36
Author: Steven Hardy <email address hidden>
Date: Tue Sep 9 12:24:42 2014 +0100

    Cleanup trust on delete with stored context if needed

    On delete, it's possible that someone other than the user who
    created the stack may do the delete, e.g if there are a group
    of heat users in a project who share responsibility for stacks.

    Currently this causes an error if the user is not an admin,
    because keystone default policy means only the trustor user (e.g
    the user who originally delegated to heat via the trust on create)
    or an admin may delete the trust.

    So we check if the context user_id matches the user_creds trustor,
    and if it doesn't we switch to the stored context to do the trust
    cleanup so we avoid the delete failure.

    I've avoided switching to the stored context unconditionally because
    of the non-trivial overhead of creating a new keystoneclient object,
    getting a new trust-scoped token etc.

    Closes-Bug: #1308834
    Change-Id: If408534ac7164e9d281dfbb6d4d39da97992f322