Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

400 Bad Request trying to grant permissions to client when user has same name #111

Closed
jkeiser opened this issue Feb 25, 2015 · 7 comments
Closed

400 Bad Request trying to grant permissions to client when user has same name #111

jkeiser opened this issue Feb 25, 2015 · 7 comments

Comments

@jkeiser
Copy link
Contributor

jkeiser commented Feb 25, 2015

When a user on the server (but not in an org) has the same name as a client in the org, you cannot add that client to the permissions of any object. It gives you back a 400 instead. I would expect it to add the permissions (assuming the user isn't also in the org and there is no ambiguity).

This affects chef-provisioning and any client (including the webui) that tries to add permissions to objects.

Detail

When I have:

  • An organization "foo" with node "bar"
  • A user "mario" on the Hosted or Enterprise Chef Server who is not in organization "foo"
  • A client "mario" in organization "foo"

And I do a PUT /organizations/foo/nodes/bar with:

{
  "read": {
    "actors": [
      "mario",
      "pivotal"
    ]
  }
}

I expect the request to succeed and mario to have read access to bar.

Instead, I get this:

[2015-02-24T11:38:58-08:00] DEBUG: ---- HTTP Status and Header Data: ----
[2015-02-24T11:38:58-08:00] DEBUG: HTTP 1.1 400 Bad Request
[2015-02-24T11:38:58-08:00] DEBUG: server: ngx_openresty
[2015-02-24T11:38:58-08:00] DEBUG: date: Tue, 24 Feb 2015 19:38:57 GMT
[2015-02-24T11:38:58-08:00] DEBUG: content-type: text/html
[2015-02-24T11:38:58-08:00] DEBUG: content-length: 156
[2015-02-24T11:38:58-08:00] DEBUG: connection: close
[2015-02-24T11:38:58-08:00] DEBUG: x-ops-api-info: flavor=hec;version=12.0.0;oc_erchef=0.29.2
[2015-02-24T11:38:58-08:00] DEBUG: ---- End HTTP Status/Header Data ----
[2015-02-24T11:38:58-08:00] DEBUG: ---- HTTP Response Body ----
[2015-02-24T11:38:58-08:00] DEBUG: <html><head><title>400 Bad Request</title></head><body><h1>Bad Request</h1>Bad Request<p><hr><address>mochiweb+webmachine web server</address></body></html>
[2015-02-24T11:38:58-08:00] DEBUG: ---- End HTTP Response Body -----
@jkeiser jkeiser mentioned this issue Feb 25, 2015
Open
@marcparadise
Copy link
Member 

                                                                                  If you pull in the latest oc_erchef with multi-key (1.2.1 or later iirc) the long-standing set of "same name" bugs should be resolved.  Though I've not seen this particular one yet (400 isn't what I'd expect). I'll take a look tomorrow ‎to see if it's via unrelated path.  We're still working out the schedule for when this gets deployed to Hosted. It's in CS12 v12.0.3 and later, once I can confirm it fixed. Otherwise we'll add it to the list. 

@stevendanna
Copy link
Contributor

@marcparadise I'm pretty sure this one is not covered by the keys work. The actor in question here isn't the one making the request so the disambiguation strategy of the key work (figuring out the actor by looking at what key auth'd) doesn't apply.

@sdelano
Copy link
Contributor

sdelano commented May 13, 2015

@jkeiser - Did you try to PUT to the /nodes/foo endpoint with data that was intended for /nodes/foo/_acl? From the debug output provided, it's hard to tell. It would also be beneficial to get the debug output that includes the headers and any other information sent to the client, just in case this 400 was due to other such missing information.

@sdelano
Copy link
Contributor

sdelano commented May 13, 2015

Okay, I did some more digging here today, and regardless of the URLs put above, the premise is still true - you get a 400 Bad Request back from the API when you try to add a client with the same name as another user on the Chef Server that's not in the current org. You do not, however, get a 400 when you try to add a client that does not have a matching username.

Testing this out locally, I get the following log message from oc_erchef:

2015-05-13T21:40:54Z [email protected] method=PUT; path=/organizations/stephen-org/nodes/bar/_acl/update; status=400; req_id=g3IAA2QAEGVyY2hlZkAxMjcuMC4wLjEDAAIlYwAAAAAAAAAA; org_name
=stephen-org; msg=bad_actor; couchdb_groups=false; couchdb_organizations=false; couchdb_containers=false; couchdb_acls=false; 503_mode=false; couchdb_associations=false; couchdb_association_requests=false; req_time=23; rdbms_time=5; rdbms_count=4; user=stephen; req_api_version=0;

@sdelano
Copy link
Contributor

sdelano commented May 14, 2015

And I've located the code that has an obvious bug in it here:

convert_actor_names_to_ids(Names, OrgId) ->
    ClientIds = oc_chef_group:find_client_authz_ids(Names, OrgId,
                                                    fun chef_sql:select_rows/1),
    UserIds = oc_chef_group:find_user_authz_ids(Names, fun chef_sql:select_rows/1),
    ClientIds ++ UserIds.

names_to_ids(Ace, OrgId) ->
    ActorNames = ej:get({<<"actors">>}, Ace),
    GroupNames = ej:get({<<"groups">>}, Ace),
    ActorIds = convert_actor_names_to_ids(ActorNames, OrgId),
    % Check to make sure everything got converted; if something is missing,
    % there was an invalid actor or group name in the request body
    case length(ActorNames) == length(ActorIds) of
        false ->
            throw(bad_actor);
        _ ->
            %% the rest doesn't matter

If the length of authz ids for actors is different from the length of plaintext names in the list, then we return bad_actor (which is what we're seeing in the log). convert_actor_names_to_ids simply fetches the authz ids for all the clients and users that matches those names and combines them together, resulting in (in the failure case) an authz id for both the user and the client. This will never work.

@cmoscardi
Copy link

+1, just ran into this.

marcparadise pushed a commit that referenced this issue Aug 18, 2016
[SPOOL-197] [#111] clients can be added to ACL even if user exist
b0da5fb 
This addresses three long-standing issues:

1. when a user exists in the Chef Server, it was not possible to add
   a client to an object's ACL if that client had the same name. The
   logic that retrieved actors based on names obeyed org constraints for
   clients, but looked at the global users list without consideration
   for whether or not the users were members of the organization.  This
   has been corrected, so that the presence of a user anywhere in the
   system can no longer block a same-named client from being added to an
   object's ACL.
2. when an actor being added does not exist or is not in the
   organization, the request would fail with a 400 'missing/invalid actor'
   message. It would not give any indication of which actor(s) caused a
   problem. This has been corrected, and the error message now includes the
   list of actor(s) that could not be added.
3. when an actor being added exists as both an org client and a user,
   the same "400 missing/invalid actor" message would be sent.
   Occurrences of this will be reduced now that we restrict the search
   to users in the org, but can still occur if an org-user shares a name
   with a client.   We have changed this to reply with "422 (Unprocessable Entity)".
   The error message explains that the actor name(s) are ambiguous and
   provides the list of names.

   Further updates will provide a means to

The next round of updates will expand the acl API to accept and provide
`clients` and `users` attributes.  Using those attributes instead of
`actors` when updating ACLs will give a workaround for the scenario described above (3)
and will be the preferred method for updating ACLs via the API.
marcparadise pushed a commit that referenced this issue Aug 18, 2016
[SPOOL-197] [#111] clients can be added to ACL even if user exist
a2a7803 
This addresses three long-standing issues:

1. when a user exists in the Chef Server, it was not possible to add
   a client to an object's ACL if that client had the same name. The
   logic that retrieved actors based on names obeyed org constraints for
   clients, but looked at the global users list without consideration
   for whether or not the users were members of the organization.  This
   has been corrected, so that the presence of a user anywhere in the
   system can no longer block a same-named client from being added to an
   object's ACL.
2. when an actor being added does not exist or is not in the
   organization, the request would fail with a 400 'missing/invalid actor'
   message. It would not give any indication of which actor(s) caused a
   problem. This has been corrected, and the error message now includes the
   list of actor(s) that could not be added.
3. when an actor being added exists as both an org client and a user,
   the same "400 missing/invalid actor" message would be sent.
   Occurrences of this will be reduced now that we restrict the search
   to users in the org, but can still occur if an org-user shares a name
   with a client.   We have changed this to reply with "422 (Unprocessable Entity)".
   The error message explains that the actor name(s) are ambiguous and
   provides the list of names.

   Further updates will provide a means to

The next round of updates will expand the acl API to accept and provide
`clients` and `users` attributes.  Using those attributes instead of
`actors` when updating ACLs will give a workaround for the scenario described above (3)
and will be the preferred method for updating ACLs via the API.
marcparadise pushed a commit that referenced this issue Aug 18, 2016
[SPOOL-197] [#111] clients can be added to ACL even if user exist
dc5526b 
This addresses three long-standing issues:

1. when a user exists in the Chef Server, it was not possible to add
   a client to an object's ACL if that client had the same name. The
   logic that retrieved actors based on names obeyed org constraints for
   clients, but looked at the global users list without consideration
   for whether or not the users were members of the organization.  This
   has been corrected, so that the presence of a user anywhere in the
   system can no longer block a same-named client from being added to an
   object's ACL.
2. when an actor being added does not exist or is not in the
   organization, the request would fail with a 400 'missing/invalid actor'
   message. It would not give any indication of which actor(s) caused a
   problem. This has been corrected, and the error message now includes the
   list of actor(s) that could not be added.
3. when an actor being added exists as both an org client and a user,
   the same "400 missing/invalid actor" message would be sent.
   Occurrences of this will be reduced now that we restrict the search
   to users in the org, but can still occur if an org-user shares a name
   with a client.   We have changed this to reply with "422 (Unprocessable Entity)".
   The error message explains that the actor name(s) are ambiguous and
   provides the list of names.

   Further updates will provide a means to

The next round of updates will expand the acl API to accept and provide
`clients` and `users` attributes.  Using those attributes instead of
`actors` when updating ACLs will give a workaround for the scenario described above (3)
and will be the preferred method for updating ACLs via the API.
marcparadise pushed a commit that referenced this issue Aug 18, 2016
[SPOOL-197] [#111] clients can be added to ACL even if user exist
13da1a5 
This addresses three long-standing issues:

1. when a user exists in the Chef Server, it was not possible to add
   a client to an object's ACL if that client had the same name. The
   logic that retrieved actors based on names obeyed org constraints for
   clients, but looked at the global users list without consideration
   for whether or not the users were members of the organization.  This
   has been corrected, so that the presence of a user anywhere in the
   system can no longer block a same-named client from being added to an
   object's ACL.
2. when an actor being added does not exist or is not in the
   organization, the request would fail with a 400 'missing/invalid actor'
   message. It would not give any indication of which actor(s) caused a
   problem. This has been corrected, and the error message now includes the
   list of actor(s) that could not be added.
3. when an actor being added exists as both an org client and a user,
   the same "400 missing/invalid actor" message would be sent.
   Occurrences of this will be reduced now that we restrict the search
   to users in the org, but can still occur if an org-user shares a name
   with a client.   We have changed this to reply with "422 (Unprocessable Entity)".
   The error message explains that the actor name(s) are ambiguous and
   provides the list of names.

   Further updates will provide a means to

The next round of updates will expand the acl API to accept and provide
`clients` and `users` attributes.  Using those attributes instead of
`actors` when updating ACLs will give a workaround for the scenario described above (3)
and will be the preferred method for updating ACLs via the API.
marcparadise pushed a commit that referenced this issue Aug 18, 2016
[SPOOL-197] [#111] clients can be added to ACL even if user exist
977b18c 
This addresses three long-standing issues:

1. when a user exists in the Chef Server, it was not possible to add
   a client to an object's ACL if that client had the same name. The
   logic that retrieved actors based on names obeyed org constraints for
   clients, but looked at the global users list without consideration
   for whether or not the users were members of the organization.  This
   has been corrected, so that the presence of a user anywhere in the
   system can no longer block a same-named client from being added to an
   object's ACL.
2. when an actor being added does not exist or is not in the
   organization, the request would fail with a 400 'missing/invalid actor'
   message. It would not give any indication of which actor(s) caused a
   problem. This has been corrected, and the error message now includes the
   list of actor(s) that could not be added.
3. when an actor being added exists as both an org client and a user,
   the same "400 missing/invalid actor" message would be sent.
   Occurrences of this will be reduced now that we restrict the search
   to users in the org, but can still occur if an org-user shares a name
   with a client.   We have changed this to reply with "422 (Unprocessable Entity)".
   The error message explains that the actor name(s) are ambiguous and
   provides the list of names.

   Further updates will provide a means to

The next round of updates will expand the acl API to accept and provide
`clients` and `users` attributes.  Using those attributes instead of
`actors` when updating ACLs will give a workaround for the scenario described above (3)
and will be the preferred method for updating ACLs via the API.
marcparadise pushed a commit that referenced this issue Aug 18, 2016
[SPOOL-197] [#111] clients can be added to ACL even if user exist
d0032bd 
This addresses three long-standing issues:

1. when a user exists in the Chef Server, it was not possible to add
   a client to an object's ACL if that client had the same name. The
   logic that retrieved actors based on names obeyed org constraints for
   clients, but looked at the global users list without consideration
   for whether or not the users were members of the organization.  This
   has been corrected, so that the presence of a user anywhere in the
   system can no longer block a same-named client from being added to an
   object's ACL.
2. when an actor being added does not exist or is not in the
   organization, the request would fail with a 400 'missing/invalid actor'
   message. It would not give any indication of which actor(s) caused a
   problem. This has been corrected, and the error message now includes the
   list of actor(s) that could not be added.
3. when an actor being added exists as both an org client and a user,
   the same "400 missing/invalid actor" message would be sent.
   Occurrences of this will be reduced now that we restrict the search
   to users in the org, but can still occur if an org-user shares a name
   with a client.   We have changed this to reply with "422 (Unprocessable Entity)".
   The error message explains that the actor name(s) are ambiguous and
   provides the list of names.

   Further updates will provide a means to

The next round of updates will expand the acl API to accept and provide
`clients` and `users` attributes.  Using those attributes instead of
`actors` when updating ACLs will give a workaround for the scenario described above (3)
and will be the preferred method for updating ACLs via the API.
@marcparadise marcparadise mentioned this issue Aug 18, 2016
Merged
1 task
marcparadise pushed a commit that referenced this issue Aug 18, 2016
[SPOOL-197] [#111] clients can be added to ACL even if user exist
633c555 
This addresses three long-standing issues:

1. when a user exists in the Chef Server, it was not possible to add
   a client to an object's ACL if that client had the same name. The
   logic that retrieved actors based on names obeyed org constraints for
   clients, but looked at the global users list without consideration
   for whether or not the users were members of the organization.  This
   has been corrected, so that the presence of a user anywhere in the
   system can no longer block a same-named client from being added to an
   object's ACL.
2. when an actor being added does not exist or is not in the
   organization, the request would fail with a 400 'missing/invalid actor'
   message. It would not give any indication of which actor(s) caused a
   problem. This has been corrected, and the error message now includes the
   list of actor(s) that could not be added.
3. when an actor being added exists as both an org client and a user,
   the same "400 missing/invalid actor" message would be sent.
   Occurrences of this will be reduced now that we restrict the search
   to users in the org, but can still occur if an org-user shares a name
   with a client.   We have changed this to reply with "422 (Unprocessable Entity)".
   The error message explains that the actor name(s) are ambiguous and
   provides the list of names.

   Further updates will provide a means to

The next round of updates will expand the acl API to accept and provide
`clients` and `users` attributes.  Using those attributes instead of
`actors` when updating ACLs will give a workaround for the scenario described above (3)
and will be the preferred method for updating ACLs via the API.
marcparadise pushed a commit that referenced this issue Aug 19, 2016
[SPOOL-197] - [#111] - Improve test coverage, error message
2436495 
* Fixed formatting in error message
* Added eunit tests for oc_chef_authz_acl
* Updates for code review comments.

Signed-off-by: Marc Paradise <[email protected]>
marcparadise pushed a commit that referenced this issue Aug 22, 2016
[SPOOL-197] [#111] clients can be added to ACL even if user exist
d7457ad 
This addresses three long-standing issues:

1. when a user exists in the Chef Server, it was not possible to add
   a client to an object's ACL if that client had the same name. The
   logic that retrieved actors based on names obeyed org constraints for
   clients, but looked at the global users list without consideration
   for whether or not the users were members of the organization.  This
   has been corrected, so that the presence of a user anywhere in the
   system can no longer block a same-named client from being added to an
   object's ACL.
2. when an actor being added does not exist or is not in the
   organization, the request would fail with a 400 'missing/invalid actor'
   message. It would not give any indication of which actor(s) caused a
   problem. This has been corrected, and the error message now includes the
   list of actor(s) that could not be added.
3. when an actor being added exists as both an org client and a user,
   the same "400 missing/invalid actor" message would be sent.
   Occurrences of this will be reduced now that we restrict the search
   to users in the org, but can still occur if an org-user shares a name
   with a client.   We have changed this to reply with "422 (Unprocessable Entity)".
   The error message explains that the actor name(s) are ambiguous and
   provides the list of names.

The next round of updates will expand the acl API to accept and provide
`clients` and `users` attributes.  Using those attributes instead of
`actors` when updating ACLs will give a workaround for the scenario described above (3)
and will be the preferred method for updating ACLs via the API.
marcparadise pushed a commit that referenced this issue Aug 22, 2016
[SPOOL-197] - [#111] - Improve test coverage, error message
1f1d20b 
* Fixed formatting in error message
* Added eunit tests for oc_chef_authz_acl
* Updates for code review comments.

Signed-off-by: Marc Paradise <[email protected]>
marcparadise added a commit that referenced this issue Aug 22, 2016
@marcparadise
Merge pull request #905 from chef/SPOOL-197
5298c20 
[SPOOL-197] [#111] clients can be added to ACL even if user exist
@marcparadise
Copy link
Member

Resolved with changes from:

I'll close this issue once the last of those lands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@sdelano @jkeiser @stevendanna @cmoscardi @marcparadise