Verify: feedback/fixes from Bryan addressed

Bug #843049 reported by klmitch
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
OpenStack Identity (keystone)
Invalid
Wishlist
Dolph Mathews

Bug Description

I'm glad to see section 2 begin by defining concepts. I'd suggest adding all these concepts to the openstack glossary at http://wiki.openstack.org/Glossary. The github site for keystone defines these concepts in the README, but also includes group, so I'd add that to section 2. I see groups and tenant groups are defined in the appendix as an extension. Why an extension? If an operator doesn't want to use them, can't they just ignore them?

The tenant concept is interesting. This may have been discussed elsewhere on this list, but I missed that conversation. I gather it's a somewhat flexible notion. I've been immersed in ITIL lately, and it's language can help express this concept. Tell me if this is consistent with your idea of tenant: A tenant is a configuration within the service that holds configuration items for a specific "customer" of the service, where customer is a party with an independent right to assign users, groups, and roles to consume the capabilities of the service.

In example 3.1 the POST content type says it's a JSON request, but the POST'ed payload shows as XML. I'm a little concerned that the password appears to be sent in clear text.

In 3.3 we discuss paginated collections, and navigation via "next" and "previous" links, but you need to differentiate the limit and marker URI scheme for these cases. "next" links go to the page whose records are AFTER the marker, and "previous" links go to the page whose records are BEFORE the mark, so you can't use the same URI constructs for both. I recommend ?after=lastID and ?before=firstID. There's also no reason to actually specify the URI structure in the spec . "next" and "previous" are enough.

In section 4.3 I wonder if there should be a notion of the authentication mechanism included, along the lines of RFC 2617's basic and digest authentication schemes. The RFC has a good treatment of various security issues in it's section 4. See http://tools.ietf.org/html/rfc2617 . I expect we'd be measured against this bar.

A general comment for section 5 is that for write operations the payload format should be discussed. Eg: when we PUT against /user/userId/enabled, exactly what does the payload look like.
In section 5.2.2, the first row shows using PUT against /users to create a user, but I expect you want POST here.

In 5.2.4 you PUT against /tenants to create a user, but I expect you want POST here, too. In 5.4 you discuss this as a POST.

In 5.2.6, we should describe what a roleRef is exactly.

I'm not sure why we cover tenants in both 5.2.4 and 5.4. Similarly, we cover tokens in both 5.2.3 and 5.3.

In 5.5, we cover Base URLs. This should be covered in the concepts too, then.

In 6.1.2, we PUT to /groups to create a group, but I think we want POST here.

One thing that I would suggest adding that seems to come up a lot is a mechanism for tracking changes that will affect authorization logic or drive provisioning/deprovisioning activity. I suggest adding atom feeds of change events such as: user adds and deletes, revoked tokens, adds and deletes of users to roles, groups, and tenant groups. These atom feeds will allow authorization logic to be based on cached information that can be kept current within a desired latency by efficient polling of the atom feeds.

Tags: bug

Related branches

Dolph Mathews (dolph)
Changed in keystone:
assignee: nobody → Dolph Mathews (dolph)
Changed in keystone:
importance: High → Wishlist
Dolph Mathews (dolph)
Changed in keystone:
status: Confirmed → Invalid
Revision history for this message
Adil Ishaq (iradvisor) wrote :

Rename and Fix Identity if possible.

information type: Public → Public Security
Revision history for this message
Jeremy Stanley (fungi) wrote :

When marking a bug as a suspected security vulnerability, please leave a detailed comment explaining your reasons. In this case you switched a 12-year-old bug report (which was marked invalid 11 years ago) to indicate you suspect it represents a security vulnerability, without any clear justification for doing so. I'm switching it back to a normal public bug report now.

information type: Public Security → Public
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.