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.

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

Add a set_current_group method for users #176

Merged
merged 1 commit into from
Nov 13, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 15 additions & 7 deletions app/controllers/api/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ module Api
class UsersController < BaseController
INVALID_USER_ATTRS = %w(id href current_group_id settings).freeze # Cannot update other people's settings
INVALID_SELF_USER_ATTRS = %w(id href current_group_id).freeze
EDITABLE_ATTRS = %w(password email settings group current_group).freeze
EDITABLE_ATTRS = %w(password email settings group).freeze

include Subcollections::Tags

Expand Down Expand Up @@ -37,7 +37,6 @@ def create_resource(_type, _id, data)
def edit_resource(type, id, data)
id == User.current_user.id ? validate_self_user_data(data) : validate_user_data(data)
parse_set_group(data)
parse_set_current_group(data)
parse_set_settings(data, resource_search(id, type, collection_class(type)))
super
end
Expand All @@ -48,6 +47,20 @@ def delete_resource(type, id = nil, data = nil)
super
end

def set_current_group_resource(_type, id, data)
User.current_user.tap do |user|
raise "Can only edit authenticated user's current group" unless user.id == id
group_id = parse_group(data["current_group"])
raise "Must specify a current_group" unless group_id
new_group = user.miq_groups.where(:id => group_id).first
raise "User must belong to group" unless new_group
# Cannot use update_attributes! due to the allowed ability to switch between groups that may have different RBAC visibility on a user's miq_groups
user.update_attribute(:current_group, new_group)
end
rescue => err
raise BadRequestError, "Cannot set current_group - #{err}"
end

private

def update_target_is_api_user?
Expand All @@ -66,11 +79,6 @@ def parse_set_group(data)
data["miq_groups"] = groups if groups
end

def parse_set_current_group(data)
current_group = parse_fetch_group(data.delete("current_group"))
data["current_group"] = current_group if current_group
end

def parse_set_settings(data, user = nil)
settings = data.delete("settings")
if settings.present?
Expand Down
1 change: 1 addition & 0 deletions config/api.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2779,6 +2779,7 @@
:identifier: rbac_user_edit
- :name: delete
:identifier: rbac_user_delete
- :name: set_current_group
:delete:
- :name: delete
:identifier: rbac_user_delete
Expand Down
76 changes: 66 additions & 10 deletions spec/requests/users_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -253,16 +253,6 @@
expect(user1.reload.name).to eq("updated name")
end

it "can set a user's current group" do
api_basic_authorize collection_action_identifier(:users, :edit)
user1.miq_groups << group2

post(api_user_url(nil, user1), :params => gen_request(:edit, "current_group" => { "href" => api_group_url(nil, group2) }))

expect(response).to have_http_status(:ok)
expect(response.parsed_body["current_group_id"]).to eq(group2.id.to_s)
end

it "supports single user edit of other attributes including group change" do
api_basic_authorize collection_action_identifier(:users, :edit)

Expand Down Expand Up @@ -425,4 +415,70 @@
expect(response).to have_http_status(:ok)
end
end

describe "set_current_group" do
it "can set the current group from the user's miq_groups" do
api_basic_authorize
@user.miq_groups << group2

post(api_user_url(nil, @user), :params => {
:action => "set_current_group",
:current_group => { :href => api_group_url(nil, group2) }
})

expected = {
"current_group_id" => group2.id.to_s
}
expect(response).to have_http_status(:ok)
expect(response.parsed_body).to include(expected)
end

it "requires that the user belong to the current group" do
api_basic_authorize

post(api_user_url(nil, @user), :params => {
:action => "set_current_group",
:current_group => { :href => api_group_url(nil, group2) }
})

expected = {
"error" => a_hash_including(
"kind" => "bad_request",
"message" => "Cannot set current_group - User must belong to group"
)
}
expect(response).to have_http_status(:bad_request)
expect(response.parsed_body).to include(expected)
end

it "requires the current group to be specified" do
api_basic_authorize

post(api_user_url(nil, @user), :params => {:action => "set_current_group"})

expected = {
"error" => a_hash_including(
"kind" => "bad_request",
"message" => "Cannot set current_group - Must specify a current_group"
)
}
expect(response).to have_http_status(:bad_request)
expect(response.parsed_body).to include(expected)
end

it "only allows editing of the validated user's groups" do
api_basic_authorize

post(api_user_url(nil, user1), :params => {:action => "set_current_group"})

expected = {
"error" => a_hash_including(
"kind" => "bad_request",
"message" => "Cannot set current_group - Can only edit authenticated user's current group"
)
}
expect(response.parsed_body).to include(expected)
expect(response).to have_http_status(:bad_request)
end
end
end