Skip to content

Commit

Permalink
Merge pull request #356 from abellotti/catch_settings_config_errors
Browse files Browse the repository at this point in the history
Enhanced API to catch Settings validation errors
  • Loading branch information
Fryguy authored Apr 9, 2018
2 parents 5571f72 + 4f64cec commit 82d558f
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 1 deletion.
6 changes: 5 additions & 1 deletion app/controllers/api/base_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,11 @@ def settings
case @req.method
when :patch
raise ForbiddenError, "You are not authorized to edit settings." unless super_admin?
resource.add_settings_for_resource(@req.json_body)
begin
resource.add_settings_for_resource(@req.json_body)
rescue Vmdb::Settings::ConfigurationInvalid => err
raise BadRequestError, "Settings validation failed - #{err}"
end
when :delete
raise ForbiddenError, "You are not authorized to remove settings." unless super_admin?
resource.remove_settings_path_for_resource(*@req.json_body)
Expand Down
16 changes: 16 additions & 0 deletions spec/requests/servers_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,22 @@
expect(response).to have_http_status(:unauthorized)
end

it "returns a bad_request to an update if the settings validation failed" do
api_basic_authorize(:user => super_admin.userid, :password => super_admin.password)

patch(api_server_settings_url(nil, server), :params => {:authentication => {:mode => "bogus_auth_mode"}})

expected = {
'error' => a_hash_including(
'kind' => 'bad_request',
'message' => a_string_including('Settings validation failed - ')
)
}

expect(response).to have_http_status(:bad_request)
expect(response.parsed_body).to include(expected)
end

context "with an existing settings change" do
before do
server.add_settings_for_resource("api" => {"authentication_timeout" => "7331.minutes"})
Expand Down

0 comments on commit 82d558f

Please sign in to comment.