From cd280b117ab9f08e7f381f55c36228f4a940d0aa Mon Sep 17 00:00:00 2001 From: Keenan Brock Date: Wed, 16 Aug 2023 13:21:59 -0400 Subject: [PATCH] Merge pull request #1231 from agrare/add_edit_support_configuration_script_payloads Add support for edit configuration_script_payloads (cherry picked from commit 9a77ef21ecb166e529a77a9b51f6e299ac56d034) --- ...onfiguration_script_payloads_controller.rb | 38 +++++ config/api.yml | 13 +- .../configuration_script_payloads_spec.rb | 141 +++++++++++++++++- 3 files changed, 190 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/configuration_script_payloads_controller.rb b/app/controllers/api/configuration_script_payloads_controller.rb index 7c93c56796..98caf498ed 100644 --- a/app/controllers/api/configuration_script_payloads_controller.rb +++ b/app/controllers/api/configuration_script_payloads_controller.rb @@ -1,5 +1,43 @@ module Api class ConfigurationScriptPayloadsController < BaseController include Subcollections::Authentications + + def edit_resource(type, id, data) + resource = resource_search(id, type) + + allowed_params = %w[description credentials] + allowed_params += %w[name payload payload_type] if resource.configuration_script_source.nil? + + unpermitted_params = data.keys.map(&:to_s) - allowed_params + raise BadRequestError, _("Invalid parameters: %{params}" % {:params => unpermitted_params.join(", ")}) if unpermitted_params.any? + + # If a credentials payload is provided, map any requested authentication + # records to the configuration_script_payload via the + # authentications_configuration_script_payloads join table. + unless data["credentials"].nil? + # Credentials can be a static string or a payload with an external + # Authentication record referenced by credential_ref and credential_field. + credential_refs = data["credentials"].values.select { |val| val.kind_of?(Hash) }.pluck("credential_ref") + # Lookup the Authentication record by ems_ref in the parent manager's + # list of authentications. + credentials = resource.manager&.authentications&.where(:ems_ref => credential_refs) || [] + # Filter the collection based on the current user's RBAC roles. + credentials, _ = collection_filterer(credentials, "authentications", ::Authentication) + # If any requested authentications were unable to be found, either due + # to a bad credential_ref or due to RBAC then raise a 400 BadRequestError. + missing_credential_refs = credential_refs - credentials.pluck(:ems_ref) + if missing_credential_refs.any? + raise BadRequestError, + _("Could not find credentials %{missing_credential_refs}") % + {:missing_credential_refs => missing_credential_refs} + end + # Reset the authentications collection with the current set of credentials. + # This will also remove any credential references not in the new payload. + resource.authentications = credentials + end + + resource.update!(data.except(*ID_ATTRS)) + resource + end end end diff --git a/config/api.yml b/config/api.yml index 5b78ff167f..4847154ac8 100644 --- a/config/api.yml +++ b/config/api.yml @@ -23,6 +23,11 @@ - :get - :post - :delete + :gppp: &gppp + - :get + - :put + - :post + - :patch :gpppd: &gpppd - :get - :put @@ -986,7 +991,7 @@ :options: - :collection - :subcollection - :verbs: *g + :verbs: *gppp :klass: ConfigurationScriptPayload :subcollections: - :authentications @@ -994,10 +999,16 @@ :get: - :name: read :identifier: embedded_configuration_script_payload_view + :post: + - :name: edit + :identifier: embedded_configuration_script_payload_edit :resource_actions: :get: - :name: read :identifier: embedded_configuration_script_payload_view + :post: + - :name: edit + :identifier: embedded_configuration_script_payload_edit :subcollection_actions: :get: - :name: read diff --git a/spec/requests/configuration_script_payloads_spec.rb b/spec/requests/configuration_script_payloads_spec.rb index 2c7aa34f5e..3871b0f157 100644 --- a/spec/requests/configuration_script_payloads_spec.rb +++ b/spec/requests/configuration_script_payloads_spec.rb @@ -49,8 +49,147 @@ end end + describe 'POST /api/configuration_script_payloads' do + let(:manager) { FactoryBot.create(:ext_management_system) } + let(:script_payload) { FactoryBot.create(:configuration_script_payload, :manager => manager) } + + context "edit" do + it 'forbids edit of a configuration_script_payload without an appropriate role' do + api_basic_authorize + + post(api_configuration_script_payloads_url, :params => {:action => 'edit', :name => 'foo'}) + expect(response).to have_http_status(:forbidden) + end + + it 'can edit a configuration_script_payload' do + api_basic_authorize collection_action_identifier(:configuration_script_payloads, :edit, :post) + + post(api_configuration_script_payloads_url, :params => {:action => 'edit', :resources => [{:id => script_payload.id, :name => 'foo', :credentials => {"my-cred" => "credential123"}}]}) + + expect(response).to have_http_status(:ok) + expect(response.parsed_body).to include('results' => [a_hash_including('name' => 'foo')]) + expect(script_payload.reload.name).to eq('foo') + expect(script_payload.credentials).to include("my-cred" => "credential123") + end + + it "fails if the credential can't be found" do + api_basic_authorize collection_action_identifier(:configuration_script_payloads, :edit, :post) + + post(api_configuration_script_payloads_url, :params => {:action => 'edit', :resources => [{:id => script_payload.id, :name => 'foo', :credentials => {"my-cred" => {"credential_ref" => "my-credential", "credential_field" => "userid"}}}]}) + expect(response).to have_http_status(:bad_request) + end + + context "with an authentication reference in credentials" do + let!(:authentication) { FactoryBot.create(:authentication, :ems_ref => "my-credential", :resource => manager) } + + context "owned by another tenant" do + let(:tenant_1) { FactoryBot.create(:tenant) } + let(:tenant_2) { FactoryBot.create(:tenant) } + let(:group_1) { FactoryBot.create(:miq_group, :tenant => tenant_1, :miq_user_role => @role) } + let(:group_2) { FactoryBot.create(:miq_group, :tenant => tenant_2) } + let(:user_2) { FactoryBot.create(:user, :miq_groups => [group_2]) } + let!(:authentication) { FactoryBot.create(:authentication, :ems_ref => "my-credential", :resource => manager, :evm_owner => user_2, :miq_group => group_2) } + + before do + @user.miq_groups << group_1 + @user.update!(:current_group => group_1) + end + + it "fails if the credential is owned by another tenant" do + api_basic_authorize(collection_action_identifier(:configuration_script_payloads, :edit, :post)) + post(api_configuration_script_payloads_url, :params => {:action => 'edit', :resources => [{:id => script_payload.id, :name => 'foo', :credentials => {"my-cred" => {"credential_ref" => "my-credential", "credential_field" => "userid"}}}]}) + expect(response).to have_http_status(:bad_request) + end + end + + it "adds the authentication to the configuration_script_payload.authentications" do + api_basic_authorize collection_action_identifier(:configuration_script_payloads, :edit, :post) + + post(api_configuration_script_payloads_url, :params => {:action => 'edit', :resources => [{:id => script_payload.id, :name => 'foo', :credentials => {"my-cred" => {"credential_ref" => "my-credential", "credential_field" => "userid"}}}]}) + expect(script_payload.reload.authentications).to include(authentication) + end + + context "with an existing associated authentication record" do + before { script_payload.authentications << authentication } + + it "doesn't duplicate records" do + api_basic_authorize collection_action_identifier(:configuration_script_payloads, :edit, :post) + + post(api_configuration_script_payloads_url, :params => {:action => 'edit', :resources => [{:id => script_payload.id, :name => 'foo', :credentials => {"my-cred" => {"credential_ref" => "my-credential", "credential_field" => "userid"}}}]}) + expect(script_payload.reload.authentications.count).to eq(1) + end + + it "removes associated authentications" do + api_basic_authorize collection_action_identifier(:configuration_script_payloads, :edit, :post) + + post(api_configuration_script_payloads_url, :params => {:action => 'edit', :resources => [{:id => script_payload.id, :name => 'foo', :credentials => {}}]}) + expect(script_payload.reload.authentications.count).to be_zero + end + end + end + + context "with a configuration_script_source" do + let(:script_source) { FactoryBot.create(:configuration_script_source) } + let(:script_payload) { FactoryBot.create(:configuration_script_payload, :configuration_script_source => script_source) } + + it "cannot modify the name, payload, payload_type" do + api_basic_authorize collection_action_identifier(:configuration_script_payloads, :edit, :post) + + post(api_configuration_script_payloads_url, :params => {:action => 'edit', :resources => [{:id => script_payload.id, :name => "foo", :payload => "---\n", :payload_type => "yaml"}]}) + + expect(response).to have_http_status(:bad_request) + expect(response.parsed_body["error"]).to include("message" => "Invalid parameters: name, payload, payload_type") + end + end + end + end + + describe 'PUT /api/configuration_script_payloads/:id' do + let(:script_payload) { FactoryBot.create(:configuration_script_payload) } + + it 'forbids put on a configuration_script_payload without an appropriate role' do + api_basic_authorize + + put(api_configuration_script_payload_url(nil, script_payload), :params => {:name => 'foo'}) + + expect(response).to have_http_status(:forbidden) + end + + it 'can update a configuration_script_payload' do + api_basic_authorize action_identifier(:configuration_script_payloads, :edit) + + put(api_configuration_script_payload_url(nil, script_payload), :params => {:name => 'foo'}) + + expect(response).to have_http_status(:ok) + expect(response.parsed_body).to include('name' => 'foo') + expect(script_payload.reload.name).to eq('foo') + end + end + + describe 'PATCH /api/configuration_script_payloads/:id' do + let(:script_payload) { FactoryBot.create(:configuration_script_payload) } + + it 'forbids put on a configuration_script_payload without an appropriate role' do + api_basic_authorize + + patch(api_configuration_script_payload_url(nil, script_payload), :params => {:name => 'foo'}) + + expect(response).to have_http_status(:forbidden) + end + + it 'can update a configuration_script_payload' do + api_basic_authorize action_identifier(:configuration_script_payloads, :edit) + + patch(api_configuration_script_payload_url(nil, script_payload), :params => {:name => 'foo'}) + + expect(response).to have_http_status(:ok) + expect(response.parsed_body).to include('name' => 'foo') + expect(script_payload.reload.name).to eq('foo') + end + end + describe 'GET /api/configuration_script_payloads/:id/authentications' do - it 'returns the configuration script sources authentications' do + it 'returns the configuration script payloads authentications' do authentication = FactoryBot.create(:authentication) playbook = FactoryBot.create(:configuration_script_payload, :authentications => [authentication]) api_basic_authorize subcollection_action_identifier(:configuration_script_payloads, :authentications, :read, :get)