Open Stack

I have an initial draft of application credential capabilities available for
review[1]. The spec[2] was not straightforward to implement. There were a few 
parts that I found unintuitive, user-unfriendly, and overcomplicated. The
current proposed implementation differs from the spec in some ways that I want
to discuss with the team. Given that non-client library freeze is next week, 
and the changes we need in keystonemiddleware also require changes in 
keystoneclient, I'm not sure there is enough time to properly flesh this out
and allow for thorough code review, but if we miss the deadline we can be ready
to get this in in the beginning of next cycle (with apologies to everyone waiting
on this feature - I just want to make sure we get it right with minimal regrets).

* Naming

As always, naming is hard. In the spec, we've called the property that is
attached to the app cred "capabilities" (for the sake of this email I'm going
to call it user-created-rules), and we've called the operator-configured list
of available endpoints "permissible path templates" (for the sake of this email
I'm going to call it operator-created-rules). I find both confusing and 
awkward.

"Permissible path templates" is not a great name because the rule is actually
about more than just the path, it's about the request as a whole, including the 
method. I'd like to avoid saying "template" too because that evokes a picture
of something like a Jinja or ERB template, which is not what this is, and 
because I'd like to avoid the whole string substitution thing - more on that
below. In the implementation, I've renamed the operator-created-rules to
"access rules". I stole this from Istio after Adam pointed out they have a
really similar concept[3]. I really like this name because I think it
well-describes the thing we're building without already being overloaded.

So far, I've kept the user-created-rules as "capabilities" but I'm not a fan of
it because it's an overloaded word and not very descriptive, although in my
opinion it is still more descriptive than "whitelist" which is what we were
originally going to call it. It might make sense to relate this property
somehow to the operator-created-rules - perhaps by calling it
access_rules_list, or granted_access_rules. Or we could call *this* thing the 
access rules, and call the other thing allowed_access_rules or
permitted_access_rules.

* Substitutions

The way the spec lays out variable components of the URL paths for both
user-created-rules and operator-created-rules is unnecessarily complex and in
some cases faulty. The only way I can explain how complicated it is is to try 
to give an example:

Let's say we want to allow a user to create an application credential that
allows the holder to issue a GET request on the identity API that looks like
/v3/projects/ef7284b4-3a75-4570-8ea8-b30214f18538/tags/foobar. The spec says
that the string '/v3/projects/{project_id}/tags/{tag}' is what should be
provided verbatim in the "path" attribute of a "capability", then there should
be a "substitutions" attribute that sets {"tag": "foobar"}, then the project_id
should be taken from the token scope at app cred usage time. When the 
capability is validated against the operator-created-rules at app cred creation
time, it needs to check that the path string matches exactly, that the keys of
the "substitutions" dict matches the "user template keys" list, and that keys
required by the "context template keys" are provided by the token context.

Taking the project ID, domain ID, or user ID from the token scope is not going
to work because some of these APIs may actually be system-scoped APIs - it's
just not a hard and fast rule that a project/domain/user ID in the URL maps to
the same user and scope of the token used to create it. Once we do away with
that, it stops making sense to have a separate attribute for the user-provided
substitutions when they could just include that in the URL path to begin with.
So the proposed implementation simply allows the wildcards * and ** in both the 
operator-created-rules and user-created-rules, no python-formatting variable
substitutions.

* UUIDs

The spec says each operator-created-rule should have its own UUID, and the end
user needs to look up and provide this UUID when they create the app cred rule.
This has the benefit of having a fast lookup because we've put the onus on the
user to look up the rule themselves, but I think it is very user-unfriendly. In
the proposed implementation, I've done away with UUIDs on the
operator-created-rules, and instead a match is looked up based on the service
type, request path and method, and the "allow_chained" attribute (more on that
next). Depending on how big we think this list of APIs will get, this will have
some impact on performance on creation time (not at token validation time).

UUIDs make sense for singleton resources that are created in a database. I
imagine this starting as an operator-managed configuration file, and maybe some
day in the future a catalog of this sort could be published by the services so
that the operator doesn't have to maintain it themselves. To that end, I've
implemented the operator-created-rules driver with a JSON file backend. But
with this style of implementation, including UUIDs for every rule is awkward -
it only makes sense if we're generating the resources within keystone and
storing them in a database.

* allow_chained

The allow_chained attribute looks and feels awkward and adds complexity to the code
 Is there really a case when either the user or operator would not want to allow a
service to make a request on behalf of a user who is making a more general request?
Also, can we find a better name for it?

Those are all the major question marks for me. There are some other minor
differentiations from the spec and I will propose an update that makes it
consistent with reality after we have these other questions sorted out.

Colleen

[1] https://review.openstack.org/#/q/topic:bp/whitelist-extension-for-app-creds
[2] http://specs.openstack.org/openstack/keystone-specs/specs/keystone/stein/capabilities-app-creds.html
[3] https://istio.io/docs/reference/config/authorization/istio.rbac.v1alpha1/#AccessRule