Skip to content

Instantly share code, notes, and snippets.

@marcparadise
Last active August 29, 2015 14:14
Show Gist options
  • Save marcparadise/ee3904f9de7955491c4a to your computer and use it in GitHub Desktop.
Save marcparadise/ee3904f9de7955491c4a to your computer and use it in GitHub Desktop.
Keys - list keys use cases
## Approach
* initial pass will handle GET for client and user keys (list keys cases)
* client and user will be handled in the same module, since the actual code is operating on keys and not clients and users. The only functional difference in handling will be isolating the correct entity being operated on for ACL checks.
## Deviations from RFC (will be submitting a PR against the RFC to keep it accurate) :
* The RFC specifies we'll just return a list of URIs in response to GET. This isn't consistent with other APIs, and it is also harder for Manage to use - they would have to parse the URI to come up with the object name. Instead, return will be in the form `[ { "name" : "KEYNAME", "uri" : "URI" } ] `.
* From a usability perspective, it may also be worth adding an 'expired' boolean flag to the return from this API, so that clients have a quick way to see and flag expired keys.
* In other sections, the RFC refers to 'key_id' as the identifier returned. To reduce confusion and keep it consistent with other APIs, we'll use consistently use 'name' instead of 'key_id' across endpoints. There is no chance of confusion with the entity name (client or user) because those are explicit in the URI and won't be provided in any request or response body.
## Things we'll touch:
### Tests:
* expand existing tests in chef-pedant[1] to add specific GET endpoint coverage for named clients and user
* modify existing key auth tests to use the API whenever we are checking a key list - though we may be limited in what we can do here until we add support of GET for named keys.
### oc_erchef:
#### chef_db
* `pgsql_statements.config` to add `fetch_keys_for_object_id`.
#### chef_objects
* `chef_key.erl` - technically this isn't a chef object, but it will behave exactly like one, so it makes sense to follow the same model we do for other objects. Initially this will support fetching list of key_names for a given object id.
* eunit tests for chef_key
#### oc_chef_wm
* `dispatch.conf` - map routes for `/organizations/ORG/clients/C/keys` and `/users/U/keys` to `oc_chef_wm_keys`.
* `oc_chef_wm.hrl`: new record `key_state{`actor_id`, `actor_authz_id`}
* `oc_chef_wm_routes.erl`: `org_route` for generated URIs, closest existing model is behavior for cookbook_version.
* `oc_chef_wm_keys.erl` - handle GET of keys for both named user and client. The only user/client-specific code will be resolving which entity is being used at the start of the request so that we have the correct object id to use as key into `keys` and we can authorize for the correct authzid. Existing `chef_db:fetch/2` with `#chef_client{name = X}` || `#chef_user{username = X}` will do this.
* itests for GET behavior
### Follow-ups
* for put/post/delete: if we capture this in oc_chef_action:log_action is any further change to analytics back-end needed to properly handle the record?
* assumptions: keys should not be searchable via solr
### Possible Cleanups
* I'd like to move the keys testing module to oc-chef-pedant (actually I'd like to merge everything soon, but it may be reasonable to move keys tests for this project) -- needing to update two repositories because of a tag name every time we want to ship a tweaked test is just annoying...
* oc_chef_sql.erl is no longer needed, and we can move the sql statement file name out to config, and merge our two sql statement config files - having two is confusing when there is no longer a reason to do so.
@marcparadise
Copy link
Author

GET /organizations/ORG/users/USER currently returns user data for someone with correct permissions (org admin), including public key. Should we support /keys here as well?

@marcparadise
Copy link
Author

Will need to update this for LB routing - we'll need to identify the keys endpoint as such in routing rules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment