Merge pull request #1231 from agrare/add_edit_support_configuration_s…

…cript_payloads

Add support for edit configuration_script_payloads

(cherry picked from commit 9a77ef2)

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

config/api.yml

@@ -23,6 +23,11 @@
     - :get
     - :post
     - :delete
+    :gppp: &gppp
+    - :get
+    - :put
+    - :post
+    - :patch
     :gpppd: &gpppd
     - :get
     - :put
@@ -986,18 +991,24 @@
     :options:
     - :collection
     - :subcollection
-    :verbs: *g
+    :verbs: *gppp
     :klass: ConfigurationScriptPayload
     :subcollections:
     - :authentications
     :collection_actions:
       :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

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)