Added /orgs/org/users/user/keys(/key) endpoints and changed default perms on org scoped key GETs. #769

tylercloke · 2016-03-07T22:27:55Z

Merge chef/chef-zero#205 and re-run Travis once two 👍 acquired.

Now, all org-scoped key GETs will allow access to any requestor within the same org (and the superuser).
The endpoints affected are

+ /orgs/org/clients/client/keys
+ /orgs/org/clients/client/keys/key
+ /orgs/org/users/user/keys
+ /orgs/org/users/user/keys/key

of which the latter two are new endpoints.

The new permissions where implemented by overriding default permissions for those GETs and checking if the requestor of actor_keys_access_group instead (which is a global group that exists per org that contains the users and clients groups, similar to the existing read_access_group).

The existing user key endpoints

+ /users/user/keys
+ /users/user/keys/key

were not modified and contain the same permissions as beforehand.

A large part of the diff is just tests being reorganized.

Passing Build

stevendanna · 2016-03-08T13:37:22Z

src/oc_erchef/apps/oc_chef_wm/src/oc_chef_wm_routes.erl

+    {object_name, ParentName} = lists:keyfind(object_name, 1, Args),
+    {name, Name} = lists:keyfind(name, 1, Args),
+    {BaseURI, Org} = extract_from_req(Req),
+    Template = template_for_type(client_key),


Should this be template_for_type(org_user_key)?

stevendanna · 2016-03-09T00:03:29Z

I'm 👍 on the existing changes; however, per our chat discussion this needs a mover migration for existing orgs/users.

marcparadise · 2016-03-15T21:00:46Z

oc-chef-pedant/spec/api/keys/client_keys_spec.rb

-                status: 200,
-                body_exact: [
+              list_client_keys(org_name, org_client_name, current_requestor).should look_like({
+                :status => 200,


Why : to =>?

marcparadise · 2016-03-15T21:14:37Z

src/chef-mover/src/mover_actor_keys_access_group_callback.erl

+
+-record(org, {name, id, authz_id}).
+-record(group, {name, id, authz_id}).
+-define(GLOBAL_PLACEHOLDER_ORG_ID, <<"00000000000000000000000000000000">>).


I am pretty sure we have this available in an hrl we can pull in from somewhere?

marcparadise · 2016-03-15T21:39:42Z

On the whole 👍 with minor comments.

The only outstanding question (currently under discussion) is whether this would be better as an org-owned group instead of a global group, since the constraints that led us to creating global_read (global_admins) don't necessarily apply here. I don't think it would be a major change to the PR to switch it over, but we're in process of talking this through.

marcparadise · 2016-03-15T22:01:02Z

After further discussion, we’ve decided to make the group a normal org group, with permissions to prevent deletion. This allows membership to be edited as needed in situations where the customer has concerns about exposing public key and expiration data to clients.

When passing a custom org, it was posting to the default org.

Added tests around desired behavior related to allowing clients and users to access the public keys of other clients and users within the same org.

…erms on org scoped key GETs. Now, all org scoped key GETs will allow access to any requestor within the same org (and the superuser). The endpoints affected are + /orgs/org/clients/client/keys + /orgs/org/clients/client/keys/key + /orgs/org/users/user/keys + /orgs/org/users/user/keys/key of which the latter two are new endpoints. The new permissions where implemented by overriding default permissions for those GETs and checking if the requestor is a memeber of the public_key_read_access group instead. This is a new group that, by default, contains the users and clients group. Org admins can edit who is in this group, which changes who is allowed to view the org scoped user public keys. The existing user key endpoints + /users/user/keys + /users/user/keys/key were not modified and contain the same permissions as beforehand. Changelog-Entry: Added /orgs/org/users/user/keys(/key) endpoint and changed default perms on org scoped key GETs. The following endpoints' GET methods can now be accessed by any requestor that is a member of the same organization: + /organizations/:org/clients/:client/keys + /organizations/:org/clients/:client/keys/:key + /organizations/:org/users/:user/keys + /organizations/:org/users/:user/keys/:key The above org-scoped user keys endpoints are new and access to them can be controlled by an admin by editing memebership of the public_key_read_access group.

markan · 2016-03-16T22:57:29Z

src/oc_erchef/apps/oc_chef_authz/src/oc_chef_authz_org_creator.erl

@@ -87,6 +88,7 @@
           {add_acl,
            [mk_tl(container, ?CONTAINERS), mk_tl(group, [admins, clients, users]), {organization}],
            ?ALL_PERMS, [{group, admins}]},
+           {add_acl, [{group, public_key_read_access}], [read, update], [{group, admins}]},


These changes look good to me. I prefer read, update over adding read, grant. There probably would be no harm in adding others to the read list, but I'm fine with where it is.

Sweet, I think read, update is the right thing too.

markan · 2016-03-17T00:02:18Z

👍 with changes to make it org owned.

Added /orgs/org/users/user/keys(/key) endpoints and changed default perms on org scoped key GETs.

chef-supermarket added the Signed CLA label Mar 7, 2016

sdelano added the in progress label Mar 7, 2016

tylercloke force-pushed the SPOOL-71/read-on-org-scoped-keys branch from 3d741dc to fad01e9 Compare March 8, 2016 02:55

chef-supermarket added the Signed CLA label Mar 8, 2016

tylercloke changed the title ~~WIP: Read on keys~~ Added /orgs/org/users/user/keys(/key) endpoints and changed default perms on org scoped key GETs. Mar 8, 2016

tylercloke force-pushed the SPOOL-71/read-on-org-scoped-keys branch from fad01e9 to b50073e Compare March 8, 2016 03:03

chef-supermarket added the Signed CLA label Mar 8, 2016

stevendanna reviewed Mar 8, 2016
View reviewed changes

tylercloke force-pushed the SPOOL-71/read-on-org-scoped-keys branch from d7bfb0f to 93cde1b Compare March 8, 2016 22:42

chef-supermarket added the Signed CLA label Mar 8, 2016

tylercloke force-pushed the SPOOL-71/read-on-org-scoped-keys branch from 2ecd8f1 to 4063d60 Compare March 9, 2016 03:02

chef-supermarket added the Signed CLA label Mar 9, 2016

tylercloke force-pushed the SPOOL-71/read-on-org-scoped-keys branch from 4063d60 to 1d07f08 Compare March 9, 2016 19:16

chef-supermarket added the Signed CLA label Mar 9, 2016

tylercloke force-pushed the SPOOL-71/read-on-org-scoped-keys branch from 1d07f08 to 409707c Compare March 9, 2016 21:34

chef-supermarket added the Signed CLA label Mar 9, 2016

tylercloke force-pushed the SPOOL-71/read-on-org-scoped-keys branch from 4528a6e to d656799 Compare March 9, 2016 23:34

chef-supermarket added the Signed CLA label Mar 9, 2016

tylercloke force-pushed the SPOOL-71/read-on-org-scoped-keys branch from d656799 to bccb903 Compare March 10, 2016 00:04

chef-supermarket added the Signed CLA label Mar 10, 2016

tylercloke force-pushed the SPOOL-71/read-on-org-scoped-keys branch from bccb903 to 0acd73c Compare March 10, 2016 00:29

chef-supermarket added the Signed CLA label Mar 10, 2016

tylercloke force-pushed the SPOOL-71/read-on-org-scoped-keys branch from 0acd73c to ad6b51e Compare March 10, 2016 22:35

chef-supermarket added the Signed CLA label Mar 10, 2016

tylercloke mentioned this pull request Mar 11, 2016

Implemented GET /orgs/ORG/users/USER/keys(/key) endpoint recently added to server. chef/chef-zero#205

Merged

marcparadise reviewed Mar 15, 2016
View reviewed changes

tylercloke added 2 commits March 16, 2016 14:35

Fixed a bug in Pedant::Platform client_create().

ee3a008

When passing a custom org, it was posting to the default org.

Refactored and modified client and user list pedant tests.

8408adb

Added tests around desired behavior related to allowing clients and users to access the public keys of other clients and users within the same org.

tylercloke force-pushed the SPOOL-71/read-on-org-scoped-keys branch from 896b814 to 78f29b1 Compare March 16, 2016 21:35

chef-supermarket added the Signed CLA label Mar 16, 2016

tylercloke force-pushed the SPOOL-71/read-on-org-scoped-keys branch from 78f29b1 to e32cda0 Compare March 16, 2016 22:18

chef-supermarket added the Signed CLA label Mar 16, 2016

tylercloke added 2 commits March 16, 2016 15:25

Fix bug in keys routing and add org-scoped keys route.

5ab02f8

tylercloke force-pushed the SPOOL-71/read-on-org-scoped-keys branch from e32cda0 to 5ab02f8 Compare March 16, 2016 22:27

chef-supermarket added the Signed CLA label Mar 16, 2016

markan reviewed Mar 16, 2016
View reviewed changes

tylercloke added a commit that referenced this pull request Mar 17, 2016

Merge pull request #769 from chef/SPOOL-71/read-on-org-scoped-keys

5364e0f

Added /orgs/org/users/user/keys(/key) endpoints and changed default perms on org scoped key GETs.

tylercloke merged commit 5364e0f into master Mar 17, 2016

tylercloke deleted the SPOOL-71/read-on-org-scoped-keys branch March 17, 2016 00:32

sdelano removed the in progress label Mar 17, 2016

tylercloke mentioned this pull request Mar 17, 2016

Chef Server 12: View Public Keys of all Users, including clients #649

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added /orgs/org/users/user/keys(/key) endpoints and changed default perms on org scoped key GETs. #769

Added /orgs/org/users/user/keys(/key) endpoints and changed default perms on org scoped key GETs. #769

tylercloke commented Mar 7, 2016

stevendanna Mar 8, 2016

stevendanna commented Mar 9, 2016

marcparadise Mar 15, 2016

marcparadise Mar 15, 2016

marcparadise commented Mar 15, 2016

marcparadise commented Mar 15, 2016

markan Mar 16, 2016

tylercloke Mar 16, 2016

markan commented Mar 17, 2016

Added /orgs/org/users/user/keys(/key) endpoints and changed default perms on org scoped key GETs. #769

Added /orgs/org/users/user/keys(/key) endpoints and changed default perms on org scoped key GETs. #769

Conversation

tylercloke commented Mar 7, 2016

stevendanna Mar 8, 2016

Choose a reason for hiding this comment

stevendanna commented Mar 9, 2016

marcparadise Mar 15, 2016

Choose a reason for hiding this comment

marcparadise Mar 15, 2016

Choose a reason for hiding this comment

marcparadise commented Mar 15, 2016

marcparadise commented Mar 15, 2016

markan Mar 16, 2016

Choose a reason for hiding this comment

tylercloke Mar 16, 2016

Choose a reason for hiding this comment

markan commented Mar 17, 2016