diff --git a/.travis.yml b/.travis.yml index 308371c953..c2539a0944 100644 --- a/.travis.yml +++ b/.travis.yml @@ -75,7 +75,7 @@ matrix: before_script: cd oc-chef-pedant script: bundle exec rake chef_zero_spec env: - - "GEMFILE_MOD=\"gem 'rake'; gem 'chef-zero', github: 'chef/chef-zero'\"" + - "GEMFILE_MOD=\"gem 'rake'; gem 'chef-zero', github: 'chef/chef-zero', branch: 'master' \"" # Remove things only used by erlang install: after_failure: @@ -93,7 +93,7 @@ matrix: before_script: cd oc-chef-pedant script: bundle exec rake chef_zero_spec env: - - "GEMFILE_MOD=\"gem 'rake'; gem 'chef-zero', github: 'chef/chef-zero'\"" + - "GEMFILE_MOD=\"gem 'rake'; gem 'chef-zero', github: 'chef/chef-zero', branch: 'master' \"" - CHEF_FS=1 # Remove things only used by erlang install: diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index f911e31e69..255c1d7336 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -10,21 +10,29 @@ For prior releases, see [PRIOR\_RELEASE\_NOTES.md](PRIOR_RELEASE_NOTES.md). ## 12.8.1 ### Chef Server -* The object ACL API now requires that any users being added to an - object's ACL exist in the same organization as the object itself. - Existing users that are not organization members and have already been added - to an ACL will not be affected, and can still be seen in the GET + +#### API Changes + +* The object ACL update API (`/organizations/ORG/OBJECT_TYPE/OBJECT_NAME/_acl/PERM`) + now requires that any users being added to an object's ACL exist in the same + organization as the object itself. Existing users that are not organization members + and have already been added to an ACL will not be affected, and can still be seen in the GET response for this API. * When a client and a user of the same name exist in the organization, the HTTP response code `422` is returned and the error text will indicate which client/user was ambiguous. * The object ACL API now accepts `clients` and `users` fields which will allow you to disambiguate in this situation by separating - clients from users in the request. When using this option, the `actors` field must be set to `[]` in the request. - The GET API response for object ACL has not been changed. + clients from users in the request. When using this option, the `actors` field + must be set to `[]` in the request. * 400 responses in object ACL updates will now include the specific missing or incorrect entities where appropriate. - +* The GET API response for object ACL has not been changed by default, + but if the query argument `?detail=granular` is provided, the json + body will contain `actors` which will be an empty array; and + `clients:[...]`, `users:[...]`. This body is compatible + with the PUT API changes, and can be used in that request without + modification. ## 12.8.0 (2016-07-06) diff --git a/oc-chef-pedant/spec/api/account/account_acl_spec.rb b/oc-chef-pedant/spec/api/account/account_acl_spec.rb index a5d1762b28..916c205642 100644 --- a/oc-chef-pedant/spec/api/account/account_acl_spec.rb +++ b/oc-chef-pedant/spec/api/account/account_acl_spec.rb @@ -817,6 +817,8 @@ def rand_id # the defaults of defaults, which are overridden below for the different # types: let(:actors) { ["pivotal", platform.admin_user.name].uniq } + let(:users) { ["pivotal", platform.admin_user.name].uniq } + let(:clients) { [] } let(:groups) { ["admins"] } let(:read_groups) { groups } let(:update_groups) { groups } @@ -831,6 +833,14 @@ def rand_id "grant" => {"actors" => actors, "groups" => grant_groups} }} + let(:granular_acl_body) {{ + "create" => {"actors" => [], "users" => users, "clients" => clients, "groups" => groups}, + "read" => {"actors" => [], "users" => users, "clients" => clients, "groups" => read_groups}, + "update" => {"actors" => [], "users" => users, "clients" => clients, "groups" => update_groups}, + "delete" => {"actors" => [], "users" => users, "clients" => clients, "groups" => delete_groups}, + "grant" => {"actors" => [], "users" => users, "clients" => clients, "groups" => grant_groups}, + }} + # Mainly this is for the different creation bodies, but also for the # different default ACLs for each type, etc. We love consistency! case type @@ -841,6 +851,9 @@ def rand_id # actors list ["pivotal", new_object, setup_user.name].uniq } + let(:clients) { [ new_object ] } + let(:users) { ["pivotal", setup_user.name].uniq } + let(:read_groups) { ["users", "admins"] } let(:delete_groups) { ["users", "admins"] } when "groups" @@ -854,6 +867,7 @@ def rand_id "containerpath" => "/" }} let(:actors) { [platform.admin_user.name] } + let(:users) { [platform.admin_user.name] } let(:groups) { [] } let(:grant_groups) { [] } when "data" @@ -968,9 +982,16 @@ def rand_id it "can get object ACL" do get(request_url, platform.admin_user).should look_like({ :status => 200, - :body => acl_body + :body_exact => acl_body }) end + it "can get a granular object ACL" do + get("#{request_url}?detail=granular", platform.admin_user).should look_like({ + :status => 200, + :body_exact => granular_acl_body + }) + + end end context "default normal user" do diff --git a/omnibus/config/projects/chef-server.rb b/omnibus/config/projects/chef-server.rb index e6afa0cabd..8e91cb4c88 100644 --- a/omnibus/config/projects/chef-server.rb +++ b/omnibus/config/projects/chef-server.rb @@ -24,7 +24,7 @@ replace "private-chef" conflict "private-chef" install_dir "/opt/opscode" -build_version "12.8.0" +build_version "12.8.1" build_iteration 1 override :rabbitmq, version: "3.3.4" diff --git a/src/oc_erchef/apps/oc_chef_authz/src/oc_chef_authz_acl.erl b/src/oc_erchef/apps/oc_chef_authz/src/oc_chef_authz_acl.erl index 0806c1b554..fefe9ac704 100644 --- a/src/oc_erchef/apps/oc_chef_authz/src/oc_chef_authz_acl.erl +++ b/src/oc_erchef/apps/oc_chef_authz/src/oc_chef_authz_acl.erl @@ -30,6 +30,7 @@ fetch_id/4, fetch_cookbook_id/3, fetch/2, + fetch/3, has_grant_on/3, validate_actors_clients_users/2, update_part/5, @@ -206,12 +207,15 @@ fetch_cookbook_id(DbContext, Name, OrgId) -> -spec fetch(chef_type(), id()) -> list() | {error, term()}. fetch(Type, AuthzId) -> + fetch(Type, AuthzId, undefined). + +fetch(Type, AuthzId, Granular) -> Path = acl_path(Type, AuthzId), SuperuserId = envy:get(oc_chef_authz, authz_superuser_id, binary), Result = oc_chef_authz_http:request(Path, get, ?DEFAULT_HEADERS, [], SuperuserId), case Result of {ok, Record} -> - ids_to_names(Record); + convert_all_ids_to_names(Record, Granular); {error, forbidden} -> forbidden; Other -> @@ -329,27 +333,37 @@ convert_actor_ids_to_names(AuthzIds) -> oc_chef_group:find_clients_names(AuthzIds, fun chef_sql:select_rows/1), {UserNames, DefunctActorAuthzIds} = oc_chef_group:find_users_names(RemainingAuthzIds, fun chef_sql:select_rows/1), - {ClientNames ++ UserNames, DefunctActorAuthzIds}. + {ClientNames, UserNames, DefunctActorAuthzIds}. + + +convert_all_ids_to_names(Record, Granular) -> + convert_ids_to_names_in_part([<<"create">>, <<"read">>, <<"update">>, + <<"delete">>, <<"grant">>], + Record, Granular). --spec process_part(binary(), ejson_term()) -> ejson_term(). -process_part(Part, Record) -> +-spec convert_ids_to_names_in_part(list(binary()), ejson_term(), granular|undefined) -> ejson_term(). +convert_ids_to_names_in_part([], Record, _Granular) -> + Record; +convert_ids_to_names_in_part([Part | Rest], Record, Granular) -> Members = ej:get({Part}, Record), ActorIds = ej:get({<<"actors">>}, Members), GroupIds = ej:get({<<"groups">>}, Members), - {ActorNames, DefunctActorAuthzIds} = convert_actor_ids_to_names(ActorIds), + {ClientNames, UserNames, DefunctActorAuthzIds} = convert_actor_ids_to_names(ActorIds), {GroupNames, DefunctGroupAuthzIds} = convert_group_ids_to_names(GroupIds), - % We do this for groups, probably good to do it here too oc_chef_authz_cleanup:add_authz_ids(DefunctActorAuthzIds, DefunctGroupAuthzIds), - Members1 = ej:set({<<"actors">>}, Members, ActorNames), + Members1 = part_with_actors(Members, ClientNames, UserNames, Granular), Members2 = ej:set({<<"groups">>}, Members1, GroupNames), - ej:set({Part}, Record, Members2). - -ids_to_names(Record) -> - Record1 = process_part(<<"create">>, Record), - Record2 = process_part(<<"read">>, Record1), - Record3 = process_part(<<"update">>, Record2), - Record4 = process_part(<<"delete">>, Record3), - process_part(<<"grant">>, Record4). + NewRecord = ej:set({Part}, Record, Members2), + convert_ids_to_names_in_part(Rest, NewRecord, Granular). + + +part_with_actors(PartRecord, Clients, Users, granular) -> + PartRecord0 = ej:set({<<"users">>}, PartRecord, Users), + PartRecord1 = ej:set({<<"clients">>}, PartRecord0, Clients), + ej:set({<<"actors">>}, PartRecord1, []); +part_with_actors(PartRecord, Clients, Users, _) -> + ej:set({<<"actors">>}, PartRecord, Clients ++ Users). + % Path helper functions diff --git a/src/oc_erchef/apps/oc_chef_authz/src/oc_chef_group.erl b/src/oc_erchef/apps/oc_chef_authz/src/oc_chef_group.erl index ddbba6ebcc..565af426b7 100644 --- a/src/oc_erchef/apps/oc_chef_authz/src/oc_chef_group.erl +++ b/src/oc_erchef/apps/oc_chef_authz/src/oc_chef_group.erl @@ -6,7 +6,6 @@ -module(oc_chef_group). -include("oc_chef_types.hrl"). --include_lib("mixer/include/mixer.hrl"). -behaviour(chef_object). @@ -380,6 +379,8 @@ find_authz_id_in_names(QueryName, Args, CallbackFun) -> Other end. +query_and_diff_authz_ids(_QueryName, [], _) -> + {[], []}; query_and_diff_authz_ids(QueryName, AuthzIds, CallbackFun) -> case CallbackFun({QueryName, [AuthzIds]}) of not_found -> diff --git a/src/oc_erchef/apps/oc_chef_authz/test/oc_chef_authz_acl_tests.erl b/src/oc_erchef/apps/oc_chef_authz/test/oc_chef_authz_acl_tests.erl index 25654e3215..c2767abaaa 100644 --- a/src/oc_erchef/apps/oc_chef_authz/test/oc_chef_authz_acl_tests.erl +++ b/src/oc_erchef/apps/oc_chef_authz/test/oc_chef_authz_acl_tests.erl @@ -1,4 +1,4 @@ -%% -*- erlang-indent-level: 4;indent-tabs-mode: nil; fill-column: 92-*- +%% -*- erlang-indent-level: 4; indent-tabs-mode: nil; fill-column: 92-*- %% ex: ts=4 sw=4 et %% %% @author Marc A. Paradise @@ -109,8 +109,21 @@ validate_actors_clients_users_test_() -> } ]. +part_with_actors_test_() -> + Subject = fun oc_chef_authz_acl:part_with_actors/4, + [ + {"when 'granular' is set, response includes clients, users, empty actors", + ?_assertEqual({[{<<"users">>, [<<"u1">>]}, + {<<"clients">>, [<<"c1">>]}, + {<<"actors">>, []}]}, + Subject({[]}, [<<"c1">>], [<<"u1">>], granular))}, + {"when 'granular' is not set, response is only 'actors'", + ?_assertEqual({[{<<"actors">>, [<<"c1">>, <<"u1">>]}]}, + Subject({[]}, [<<"c1">>], [<<"u1">>], undefined))} + ]. + %% -%% Helpers for inputs and outputs. +%% Helpers for test inputs and outputs. %% valid_user_data_response(_, Names) -> diff --git a/src/oc_erchef/apps/oc_chef_wm/src/chef_wm_util.erl b/src/oc_erchef/apps/oc_chef_wm/src/chef_wm_util.erl index 214453a86a..336bad22e0 100644 --- a/src/oc_erchef/apps/oc_chef_wm/src/chef_wm_util.erl +++ b/src/oc_erchef/apps/oc_chef_wm/src/chef_wm_util.erl @@ -163,7 +163,9 @@ not_found_message(policy_group, Name) -> %% "Cannot load data bag item not_really_there for data bag sack" --spec error_message_envelope(binary() | ejson_term()) -> ejson_term(). +-spec error_message_envelope(binary() | list() | ejson_term()) -> ejson_term(). +error_message_envelope(Message) when is_list(Message) -> + error_message_envelope(iolist_to_binary(Message)); error_message_envelope(Message) when is_binary(Message) orelse is_tuple(Message) -> %% Tuple guard is really intended for grabbing EJson-encoded json objects, but we don't diff --git a/src/oc_erchef/apps/oc_chef_wm/src/oc_chef_wm_acl.erl b/src/oc_erchef/apps/oc_chef_wm/src/oc_chef_wm_acl.erl index 7760876cc6..9a84c174b3 100644 --- a/src/oc_erchef/apps/oc_chef_wm/src/oc_chef_wm_acl.erl +++ b/src/oc_erchef/apps/oc_chef_wm/src/oc_chef_wm_acl.erl @@ -80,10 +80,14 @@ validate_request('GET', Req, #base_state{chef_db_context = DbContext, auth_info(Req, State) -> check_acl_auth(Req, State). -to_json(Req, #base_state{resource_state = AclState} = State) -> - case fetch(AclState) of +to_json(Req, #base_state{resource_state = #acl_state{type = Type, authz_id = AuthzId}} = State) -> + Granular = case wrq:get_qs_value("detail", Req) of + "granular" -> granular; + _ -> undefined + end, + case oc_chef_authz_acl:fetch(Type, AuthzId, Granular) of forbidden -> - {{halt, 403}, Req, State#base_state{log_msg = acl_not_found}}; + {{halt, 403}, Req, State#base_state{log_msg = requestor_access_failed}}; Ejson -> Json = chef_json:encode(Ejson), {Json, Req, State} @@ -129,8 +133,5 @@ check_acl_auth(Req, #base_state{requestor_id = RequestorId, end end. -fetch(#acl_state{type = Type, authz_id = AuthzId}) -> - oc_chef_authz_acl:fetch(Type,AuthzId). - malformed_request_message(Any, _Req, _State) -> error({unexpected_malformed_request_message, Any}). diff --git a/src/oc_erchef/apps/oc_chef_wm/src/oc_chef_wm_acl_permission.erl b/src/oc_erchef/apps/oc_chef_wm/src/oc_chef_wm_acl_permission.erl index 4c3fe9271b..d96a54ff01 100644 --- a/src/oc_erchef/apps/oc_chef_wm/src/oc_chef_wm_acl_permission.erl +++ b/src/oc_erchef/apps/oc_chef_wm/src/oc_chef_wm_acl_permission.erl @@ -112,19 +112,24 @@ from_json(Req, #base_state{organization_guid = OrgId, forbidden -> {{halt, 400}, Req, State}; {ambiguous_actor, Actors} -> - Text = iolist_to_binary([<<"The actor(s) ">>, chef_wm_malformed:bin_str_join(Actors, ", "), <<" exist as both ">>, - <<"clients and users within this organization.">>]), - Req1 = wrq:set_resp_body(chef_json:encode({[{<<"error">>, [Text]}]}), Req), + Message = [<<"The following actor(s) exist as both clients and users within this organization: ">>, + chef_wm_malformed:bin_str_join(Actors, ", "), + <<". To perform this update, supply separate 'clients' and 'users' ">>, + <<"fields in your request, and use an empty array for the value of 'actors'.">>], + Body = chef_wm_util:error_message_envelope(Message), + Req1 = wrq:set_resp_body(chef_json:encode(Body), Req), {{halt, 422}, Req1, State#base_state{log_msg = {ambiguous_actor, Actors}}}; {invalid, Type, Names} -> - Text = iolist_to_binary([<<"The ">>, atom_to_list(Type), <<"(s) ">>, chef_wm_malformed:bin_str_join(Names, ", "), - <<" do not exist in this organization.">>]), - Req1 = wrq:set_resp_body(chef_json:encode({[{<<"error">>, [Text]}]}), Req), + Body = chef_wm_util:error_message_envelope([<<"The ">>, atom_to_list(Type), + <<"(s) ">>, chef_wm_malformed:bin_str_join(Names, ", "), + <<" do not exist in this organization.">>]), + Req1 = wrq:set_resp_body(chef_json:encode(Body), Req), {{halt, 400}, Req1, State#base_state{log_msg = {invalid_object_in_ace, Names}}}; {bad_actor, Actors} -> - Text = iolist_to_binary([<<"The actor(s) ">>, chef_wm_malformed:bin_str_join(Actors, ", "), " do not >>", - <<"exist in this organization as clients or users.">>]), - Req1 = wrq:set_resp_body(chef_json:encode({[{<<"error">>, [Text]}]}), Req), + Body = chef_wm_util:error_message_envelope([<<"The actor(s) ">>, + chef_wm_malformed:bin_str_join(Actors, ", "), + <<" do not exist in this organization as clients or users.">>]), + Req1 = wrq:set_resp_body(chef_json:encode(Body), Req), {{halt, 400}, Req1, State#base_state{log_msg = {bad_actor, Actors}}}; _Other -> % So we return 200 instead of 204, for backwards compatibility: diff --git a/src/oc_erchef/include/oc_chef_wm.hrl b/src/oc_erchef/include/oc_chef_wm.hrl index f0e3d4c4e1..7648676edd 100644 --- a/src/oc_erchef/include/oc_chef_wm.hrl +++ b/src/oc_erchef/include/oc_chef_wm.hrl @@ -363,7 +363,9 @@ -record(acl_state, { type, authz_id, - acl_data + acl_data, + granular :: 'granular' | undefined + }). -record(association_state, { diff --git a/src/oc_erchef/rebar.config b/src/oc_erchef/rebar.config index 4ed8e373a2..78849ccbe9 100644 --- a/src/oc_erchef/rebar.config +++ b/src/oc_erchef/rebar.config @@ -63,7 +63,7 @@ {d, 'CHEF_DB_DARKLAUNCH', xdarklaunch_req}, {d, 'CHEF_WM_DARKLAUNCH', xdarklaunch_req}, {parse_transform, lager_transform}, - % warnings_as_errors, + warnings_as_errors, debug_info, {platform_define, "^[0-9]+", namespaced_types}, {i, "include"},