Checking whether group has role assignment on domain without specifying a role ID result in HTTP 200

[Guang Yee]

It should've been either 400 or 404. Steps to reproduce.

1. install a vanilla devstack
2. use "openstack group list" to find a group ID. Any group will do. i.e.

openstack group list
+----------------------------------+-----------+
| ID | Name |
+----------------------------------+-----------+
| 64e5dcd8dea0429ca22f97bcac4629bc | admins |
| 9ff3c6f47a034223ad19bb6d0dd52bb6 | nonadmins |
+----------------------------------+-----------+

3. get a token. i.e. "openstack token issue"
4. call the check group assignment on domain API using curl without specifying the role ID and you can see an HTTP 200 is returned. i.e.

curl -v --head -H 'X-Auth-Token: gAAAAABYtwwzxv9T3fxnHY3Js2ln2lTvoi1fukAYe0NSXgoV9S1qI808zQSYJyKb1AtTBy3MNUJFONBb7rpsIAu12zfRlZulfOgl7vvD_EM1DkMogpIRQvotJN1aYKMq8XqcgZ-NikolKCpUfas30GMQPFOoPpJdz0qjfIcniX0ihzVRTDqVcb0' http://localhost/identity/v3/domains/default/groups/64e5dcd8dea0429ca22f97bcac4629bc/roles/
* Trying 127.0.0.1...
* Connected to localhost (127.0.0.1) port 80 (#0)
> HEAD /identity/v3/domains/default/groups/64e5dcd8dea0429ca22f97bcac4629bc/roles/ HTTP/1.1
> Host: localhost
> User-Agent: curl/7.47.0
> Accept: */*
> X-Auth-Token: gAAAAABYtwwzxv9T3fxnHY3Js2ln2lTvoi1fukAYe0NSXgoV9S1qI808zQSYJyKb1AtTBy3MNUJFONBb7rpsIAu12zfRlZulfOgl7vvD_EM1DkMogpIRQvotJN1aYKMq8XqcgZ-NikolKCpUfas30GMQPFOoPpJdz0qjfIcniX0ihzVRTDqVcb0
>
< HTTP/1.1 200 OK
HTTP/1.1 200 OK
< Date: Wed, 01 Mar 2017 18:06:01 GMT
Date: Wed, 01 Mar 2017 18:06:01 GMT
< Server: Apache/2.4.18 (Ubuntu)
Server: Apache/2.4.18 (Ubuntu)
< Vary: X-Auth-Token
Vary: X-Auth-Token
< x-openstack-request-id: req-9ea5a135-4128-4967-8552-d1c6a7b63c97
x-openstack-request-id: req-9ea5a135-4128-4967-8552-d1c6a7b63c97
< Content-Length: 158
Content-Length: 158
< Content-Type: application/json
Content-Type: application/json

<
* Connection #0 to host localhost left intact

[Kristi Nikolla (knikolla)]

Wouldn't that make the request list the roles for that group in that domain?

/v3/domains/{domain_id}/groups/{group_id}/roles
https://developer.openstack.org/api-ref/identity/v3/index.html?expanded=list-role-assignments-for-group-on-domain-detail#list-role-assignments-for-group-on-domain

[Guang Yee (guang-yee)]

Notice the difference between a HEAD and GET request. HEAD is for checking assignment while GET is for listing assignments. In this case, its a HEAD request.

[Kristi Nikolla (knikolla)]

https://github.com/openstack/keystone/blob/master/keystone/assignment/routers.py#L190

Is registering the prior api call I linked to for both GET and HEAD. The same is done for project/domains and user/group roles.

[Lance Bragstad (lbragstad)]

The impact that sticks out to me is that people maintaining their own clients might be calling HEAD list_grants [0] instead of HEAD check_grant [1] without them knowing it if they don't validate the role id properly when building the url.

I'd like to run this by someone from the API WG to see if we'd be able to do anything about it. I don't think we'd be able to correct the behavior at this point.

Regardless, we should document this thoroughly so that folks writing their own clients against the API know the importance of validating the role_id and the impact it has between the two APIs.

[0] https://github.com/openstack/keystone/blob/a103486efeefca821ac722cbad6fc31b2e3f133b/keystone/assignment/routers.py#L190
[1] https://github.com/openstack/keystone/blob/a103486efeefca821ac722cbad6fc31b2e3f133b/keystone/assignment/routers.py#L169

[Guang Yee (guang-yee)]

Lance, I think this is really an API issue. API is a contract. If client sends invalid request, it is the server's responsibility to response with the appropriate error code and message so client can take corrective actions.

[Lance Bragstad (lbragstad)]

I agree that the server should provide information for corrective actions, but part of the problem is that both APIs are very similar. If a client expects to call check_grant but doesn't ensure the role id is actually a part of the URL, how is keystone suppose to know the difference? To keystone, it doesn't see the request as invalid, because it's a different API call.

Keystone should also return the same response for GET as we do for HEAD requests [0]. So, while using HEAD to list_grants doesn't really make much sense, I'm not sure how we can fix it without breaking backwards compatibility.

[0] https://tools.ietf.org/html/rfc7231#section-4.3.2

[Guang Yee (guang-yee)]

I think we either need to revise the API contract or fix the code.

https://github.com/openstack/keystone/blob/master/api-ref/source/v3/roles.inc#L407

I guessing we just need to change this to head_action instead of get_head_action?

https://github.com/openstack/keystone/blob/a103486efeefca821ac722cbad6fc31b2e3f133b/keystone/assignment/routers.py#L169

[Lance Bragstad (lbragstad)]

I'm curious why we would want to break up the GET and HEAD actions of check_grant. To me that seems to be working fine. Despite breaking the routes up and carrying different logic in HEAD versus GET, apache might not honor that [0]. So even if we did change it, apache still might route the requests to GET and just truncate the body (depending on how keystone is being run).

[0] http://eavesdrop.openstack.org/irclogs/%23openstack-keystone/%23openstack-keystone.2017-03-02.log.html#t2017-03-02T21:05:34

[Lance Bragstad (lbragstad)]

The problem here is that keystone supports GET and HEAD on /v3/domains/{domain_id}/groups/{group_id}/roles/{role_id} and /v3/domains/{domain_id}/groups/{group_id}/roles/. We don't know if the client wanted to make a /v3/domains/{domain_id}/groups/{group_id}/roles/{role_id} request versus a /v3/domains/{domain_id}/groups/{group_id}/roles/ request.

[Morgan Fainberg (mdrnstm)]

This isn't a bug. IF the {role_id} at the end of the call is not passed, we use the list action of:

/v3/domains/{domain_id}/groups/{group_id}/roles/ (regardless of head or get action)

If a role_id is passed, you're calling a different API. This is not a great design, but this is working as intended.