diff --git a/app/controllers/api/providers_controller.rb b/app/controllers/api/providers_controller.rb index 4ae7935846..01ab94ea03 100644 --- a/app/controllers/api/providers_controller.rb +++ b/app/controllers/api/providers_controller.rb @@ -59,16 +59,6 @@ def delete_resource(type, id = nil, _data = nil) end end - def custom_attributes_edit_resource(object, type, id, data = nil) - formatted_data = format_provider_custom_attributes(data) - super(object, type, id, formatted_data) - end - - def custom_attributes_add_resource(object, type, id, data = nil) - formatted_data = format_provider_custom_attributes(data) - super(object, type, id, formatted_data) - end - def import_vm_resource(type, id = nil, data = {}) raise BadRequestError, "Must specify an id for import of VM to a #{type} resource" unless id @@ -99,19 +89,6 @@ def options private - def format_provider_custom_attributes(attribute) - if CustomAttribute::ALLOWED_API_VALUE_TYPES.include? attribute["field_type"] - attribute["value"] = attribute.delete("field_type").safe_constantize.parse(attribute["value"]) - end - attribute["section"] ||= "metadata" unless @req.action == "edit" - if attribute["section"].present? && !CustomAttribute::ALLOWED_API_SECTIONS.include?(attribute["section"]) - raise "Invalid attribute section specified: #{attribute["section"]}" - end - attribute - rescue => err - raise BadRequestError, "Invalid provider custom attributes specified - #{err}" - end - def provider_ident(provider) "Provider id:#{provider.id} name:'#{provider.name}'" end diff --git a/app/controllers/api/subcollections/custom_attributes.rb b/app/controllers/api/subcollections/custom_attributes.rb index 795df9e0ac..7054b747b2 100644 --- a/app/controllers/api/subcollections/custom_attributes.rb +++ b/app/controllers/api/subcollections/custom_attributes.rb @@ -6,16 +6,17 @@ def custom_attributes_query_resource(object) end def custom_attributes_add_resource(object, _type, _id, data = nil) - if object.respond_to?(:custom_attributes) - add_custom_attribute(object, data) - else - raise BadRequestError, "#{object.class.name} does not support management of custom attributes" - end + raise BadRequestError, "#{object.class.name} does not support management of custom attributes" unless object.respond_to?(:custom_attributes) + add_custom_attribute(object, data) + rescue => err + raise BadRequestError, "Could not add custom attributes - #{err}" end def custom_attributes_edit_resource(object, _type, id = nil, data = nil) ca = find_custom_attribute(object, id, data) edit_custom_attribute(object, ca, data) + rescue => err + raise BadRequestError, "Could not edit custom attributes - #{err}" end def custom_attributes_delete_resource(object, _type, id = nil, data = nil) @@ -44,6 +45,7 @@ def add_custom_attribute(object, data) def edit_custom_attribute(object, ca, data) return if ca.blank? + data = format_custom_attributes(data) update_custom_attributes(ca, data) update_custom_field(object, ca) ca @@ -57,6 +59,7 @@ def delete_custom_attribute(object, ca) end def update_custom_attributes(ca, data) + data = format_custom_attributes(data) ca.update_attributes(data.slice("name", "value", "section")) end @@ -79,12 +82,21 @@ def find_custom_attribute_by_data(object, data) end def new_custom_attribute(data) - name = data["name"].to_s.strip - raise BadRequestError, "Must specify a name for a custom attribute to be added" if name.blank? - CustomAttribute.new(:name => name, - :value => data["value"], - :source => data["source"].blank? ? "EVM" : data["source"], - :section => data["section"]) + data["section"] ||= "metadata" + data["source"] ||= "EVM" + raise "Must specify a name for a custom attribute to be added" if data["name"].blank? + data = format_custom_attributes(data) + CustomAttribute.new(data) + end + + def format_custom_attributes(attribute) + if CustomAttribute::ALLOWED_API_VALUE_TYPES.include?(attribute["field_type"]) + attribute["value"] = attribute.delete("field_type").safe_constantize.parse(attribute["value"]) + end + if attribute["section"].present? && !CustomAttribute::ALLOWED_API_SECTIONS.include?(attribute["section"]) + raise "Invalid attribute section specified: #{attribute["section"]}" + end + attribute end end end diff --git a/spec/requests/custom_attributes_spec.rb b/spec/requests/custom_attributes_spec.rb index 340ce67873..41994dc322 100644 --- a/spec/requests/custom_attributes_spec.rb +++ b/spec/requests/custom_attributes_spec.rb @@ -40,4 +40,37 @@ expect(response).to have_http_status(:ok) expect(response.parsed_body['href']).to include(api_provider_custom_attribute_url(nil, provider, custom_attribute)) end + + it 'returns a bad_request for invalid values of section' do + vm = FactoryGirl.create(:vm_vmware) + api_basic_authorize subcollection_action_identifier(:vms, :custom_attributes, :add, :post) + + post(api_vm_custom_attributes_url(nil, vm), :params => { :action => :add, :resources => [{:section => "bad_section", :name => "test01", :value => "val01"}] }) + + expected = { + 'error' => a_hash_including( + 'kind' => 'bad_request', + 'message' => a_string_including('Invalid attribute section specified') + ) + } + expect(response.parsed_body).to include(expected) + expect(response).to have_http_status(:bad_request) + end + + it 'does not allow editing of custom attributes with incorrect values' do + vm = FactoryGirl.create(:vm_vmware) + custom_attribute = FactoryGirl.create(:custom_attribute, :resource => vm, :name => 'foo', :value => 'bar') + api_basic_authorize subcollection_action_identifier(:vms, :custom_attributes, :edit, :post) + + post(api_vm_custom_attribute_url(nil, vm, custom_attribute), :params => { :action => :edit, :section => "bad_section", :name => "foo", :value => "bar" }) + + expected = { + 'error' => a_hash_including( + 'kind' => 'bad_request', + 'message' => a_string_including('Invalid attribute section specified') + ) + } + expect(response.parsed_body).to include(expected) + expect(response).to have_http_status(:bad_request) + end end diff --git a/spec/requests/providers_spec.rb b/spec/requests/providers_spec.rb index 7a83957c81..a8c3c2d5eb 100644 --- a/spec/requests/providers_spec.rb +++ b/spec/requests/providers_spec.rb @@ -276,8 +276,7 @@ def have_endpoint_attributes(expected_hash) post(provider_ca_url, :params => gen_request(:add, [{"name" => "name3", "value" => "value3", "section" => "bad_section"}])) - expect_bad_request("Invalid provider custom attributes specified - " \ - "Invalid attribute section specified: bad_section") + expect_bad_request("Could not add custom attributes - Invalid attribute section specified: bad_section") end it "add custom attributes to a provider" do