Fixes #39447 - Guard dangerous host bulk actions with scope hash

app/controllers/api/v2/hosts_bulk_actions_controller.rb

@@ -4,11 +4,17 @@ class HostsBulkActionsController < V2::BaseController
       include Api::Version2
       include Api::V2::BulkHostsExtension
 
+      before_action :validate_scope_hash!,
+        :only => [:bulk_destroy, :build, :reassign_hostgroup, :change_owner,
+                  :disassociate, :change_power_state, :assign_organization,
+                  :assign_location]
       before_action :find_deletable_hosts, :only => [:bulk_destroy]
       before_action :find_editable_hosts, :only => [:build, :reassign_hostgroup, :change_owner, :disassociate, :change_power_state, :manage_notifications]
       before_action :validate_power_action, :only => [:change_power_state]
 
       def_param_group :bulk_host_ids do
+        param :scope_hash, String, :required => false, :desc => N_("Fingerprint of the visible host scope when the bulk action was initiated")
+        param :skip_scope_hash, :bool, :required => false, :desc => N_("Skip validation of the visible host scope fingerprint for direct API usage")
         param :included, Hash, :desc => N_("Hosts to include in the action"), :required => true, :action_aware => true do
           param :search, String, :required => false, :desc => N_("Search string describing which hosts to perform the action on")
           param :ids, Array, :required => false, :desc => N_("List of host ids to perform the action on")
@@ -23,7 +29,12 @@ class HostsBulkActionsController < V2::BaseController
       api :DELETE, "/hosts/bulk/", N_("Delete multiple hosts")
       param_group :bulk_host_ids
       def bulk_destroy
-        process_response @hosts.destroy_all
+        deleted_hosts_count = @hosts.count
+        destroy_result = @hosts.destroy_all
+
+        process_response(destroy_result, {
+          :message => n_("%s host deleted", "%s hosts deleted", deleted_hosts_count) % deleted_hosts_count,
+        })
       end
 
       api :PUT, "/hosts/bulk/build", N_("Build")
@@ -209,6 +220,48 @@ def action_permission
 
       private
 
+      def validate_scope_hash!
+        return if skip_scope_hash?
+
+        if scope_hash_param.blank?
+          Rails.logger.debug do
+            "Bulk scope hash missing for #{self.class}##{action_name} " \
+              "(user_id=#{User.current&.id}, organization_id=#{Organization.current&.id}, " \
+              "location_id=#{Location.current&.id})"
+          end
+
+          render_error(:custom_error, :status => :conflict, :locals => {
+            :message => _('The host scope changed or is missing integrity data. Refresh and retry the bulk action.'),
+          })
+          return
+        end
+
+        current_scope_hash = ::Hosts::BulkScopeHash.for_current_viewer
+        return if ActiveSupport::SecurityUtils.secure_compare(scope_hash_param, current_scope_hash)
+
+        Rails.logger.debug do
+          "Bulk scope hash mismatch for #{self.class}##{action_name} " \
+            "(user_id=#{User.current&.id}, organization_id=#{Organization.current&.id}, " \
+            "location_id=#{Location.current&.id}, received_scope_hash=#{scope_hash_param.inspect}, " \
+            "current_scope_hash=#{current_scope_hash.inspect})"
+        end
+
+        render_error(:custom_error, :status => :conflict, :locals => {
+          :message => _('The host scope changed or the organization/location context was modified in another tab. Refresh and retry the bulk action.'),
+        })
+      end
+
+      def scope_hash_param
+        params[:scope_hash].presence || params.dig(:hosts_bulk_action, :scope_hash).presence
+      end
+
+      def skip_scope_hash?
+        Foreman::Cast.to_bool(
+          params[:skip_scope_hash].presence ||
+          params.dig(:hosts_bulk_action, :skip_scope_hash).presence
+        )
+      end
+
       def find_deletable_hosts
         find_bulk_hosts(:destroy_hosts, included: params)
       end

app/controllers/api/v2/hosts_controller.rb

@@ -12,7 +12,7 @@ class HostsController < V2::BaseController
       include HostsControllerExtension
 
       before_action :find_optional_nested_object, :except => [:facts]
-      before_action :find_resource, :except => [:index, :create, :facts]
+      before_action :find_resource, :except => [:index, :create, :facts, :bulk_scope_hash]
       check_permissions_for %w{power boot}
       before_action :process_parameter_attributes, :only => %w{update}
 
@@ -26,6 +26,7 @@ class HostsController < V2::BaseController
       end
 
       api :GET, "/hosts/", N_("List all hosts")
+      api :GET, "/hosts/bulk_scope_hash", N_("Get hash of the current visible host scope")
       api :GET, "/hostgroups/:hostgroup_id/hosts", N_("List all hosts for a host group")
       api :GET, "/locations/:location_id/hosts", N_("List hosts per location")
       api :GET, "/organizations/:organization_id/hosts", N_("List hosts per organization")
@@ -60,6 +61,10 @@ def index
         end
       end
 
+      def bulk_scope_hash
+        render :json => { :scope_hash => ::Hosts::BulkScopeHash.for_current_viewer }
+      end
+
       api :GET, "/hosts/:id/", N_("Show a host")
       param :id, :identifier_dottable, :required => true
       param :show_hidden_parameters, :bool, :desc => N_("Display hidden parameter values")
@@ -377,6 +382,8 @@ def interface_attributes(params)
 
       def action_permission
         case params[:action]
+          when 'bulk_scope_hash'
+            :view
           when 'power_status'
             :power
           when 'power'

app/services/hosts/bulk_scope_hash.rb

@@ -0,0 +1,34 @@
+require 'digest'
+
+module Hosts
+  class BulkScopeHash
+    def self.for_current_viewer
+      new(
+        user: User.current,
+        organization: Organization.current,
+        location: Location.current
+      ).to_s
+    end
+
+    def initialize(user:, organization:, location:)
+      @user = user
+      @organization = organization
+      @location = location
+    end
+
+    def to_s
+      digest = Digest::SHA256.new
+      digest << "user:#{@user&.id}|org:#{@organization&.id}|loc:#{@location&.id}|"
+
+      hosts_scope.pluck(:id).each { |id| digest << "#{id}," }
+
+      digest.hexdigest
+    end
+
+    private
+
+    def hosts_scope
+      ::Host::Managed.authorized(:view_hosts).reselect(:id).reorder(:id)
+    end
+  end
+end

config/initializers/f_foreman_permissions.rb

@@ -234,7 +234,7 @@
                                                :templates, :overview, :nics, :get_power_state, :preview_host_collection, :welcome, :statuses],
                                     :dashboard => [:OutOfSync, :errors, :active],
                                     :unattended => [:host_template, :hostgroup_template],
-                                     :"api/v2/hosts" => [:index, :show, :get_status, :vm_compute_attributes, :template, :templates, :enc, :inherited_parameters],
+                                     :"api/v2/hosts" => [:index, :show, :get_status, :vm_compute_attributes, :template, :templates, :enc, :inherited_parameters, :bulk_scope_hash],
                                      :"api/v2/interfaces" => [:index, :show],
                                      :"api/v2/host_statuses" => [:index],
                                      :locations =>  [:mismatches],

config/routes/api/v2.rb

@@ -4,6 +4,7 @@
     # new v2 routes that point to v2
     scope "(:apiv)", :module => :v2, :defaults => {:apiv => 'v2'}, :apiv => /v2/, :constraints => ApiConstraints.new(:version => 2, :default => true) do
       match 'hosts/bulk', :to => 'hosts_bulk_actions#bulk_destroy', :via => [:delete]
+      get 'hosts/bulk_scope_hash', :to => 'hosts#bulk_scope_hash'
       put 'hosts/bulk/assign_organization', :to => 'hosts_bulk_actions#assign_organization'
       put 'hosts/bulk/assign_location', :to => 'hosts_bulk_actions#assign_location'
       match 'hosts/bulk/build', :to => 'hosts_bulk_actions#build', :via => [:put]

test/controllers/api/v2/hosts_bulk_actions_controller_test.rb

@@ -12,6 +12,7 @@ def setup
       @usergroup = FactoryBot.create(:usergroup)
       @host_ids = [@host1.id, @host2.id, @host3.id]
     end
+    ::Hosts::BulkScopeHash.stubs(:for_current_viewer).returns(valid_scope_hash)
   end
 
   def valid_bulk_params(host_ids = @host_ids)
@@ -24,13 +25,22 @@ def valid_bulk_params(host_ids = @host_ids)
       :excluded => {
         :ids => [],
       },
+      :scope_hash => valid_scope_hash,
     }
   end
 
   def valid_power_params(host_ids = @host_ids, action = 'start')
     valid_bulk_params(host_ids).merge(:power => action)
   end
 
+  def valid_bulk_destroy_params(host_ids = @host_ids)
+    {
+      :organization_id => @organization.id,
+      :ids => host_ids,
+      :scope_hash => valid_scope_hash,
+    }
+  end
+
   test "should change owner with user id" do
     put :change_owner, params: valid_bulk_params.merge(:owner_id => @user.id_and_type)
 
@@ -115,6 +125,7 @@ def valid_power_params(host_ids = @host_ids, action = 'start')
     put :change_owner, params: {
       :organization_id => @organization.id,
       :location_id => @location.id,
+      :scope_hash => valid_scope_hash,
       :included => {
         :search => 'name ~ *',
       },
@@ -258,8 +269,184 @@ def valid_power_params(host_ids = @host_ids, action = 'start')
     end
   end
 
+  context "bulk_destroy" do
+    test "successfully deletes all selected hosts" do
+      assert_difference('Host.count', -@host_ids.size) do
+        delete :bulk_destroy, params: valid_bulk_destroy_params
+      end
+
+      assert_response :success
+    end
+  end
+
+  context "build" do
+    test "successfully builds all selected hosts" do
+      bulk_manager = mock('BulkHostsManager')
+      BulkHostsManager.expects(:new).with(hosts: anything).returns(bulk_manager)
+      bulk_manager.expects(:build).with(reboot: false).returns([])
+
+      put :build, params: valid_bulk_params
+
+      assert_response :success
+      body = ActiveSupport::JSON.decode(@response.body)
+      assert_match(/Built 3 hosts/, body['message'])
+    end
+  end
+
+  context "disassociate" do
+    test "successfully disassociates selected hosts" do
+      bulk_manager = mock('BulkHostsManager')
+      BulkHostsManager.expects(:new).with(hosts: anything).returns(bulk_manager)
+      bulk_manager.expects(:disassociate)
+
+      put :disassociate, params: valid_bulk_params
+
+      assert_response :success
+      body = ActiveSupport::JSON.decode(@response.body)
+      assert_match(/Updated hosts: Disassociated from compute resource/, body['message'])
+    end
+  end
+
+  context "scope_hash validation" do
+    test "allows bulk destroy without scope hash when skip_scope_hash is true" do
+      assert_difference('Host.count', -@host_ids.size) do
+        delete :bulk_destroy,
+          params: valid_bulk_destroy_params.except(:scope_hash).merge(:skip_scope_hash => true)
+      end
+
+      assert_response :success
+    end
+
+    test "rejects bulk destroy when scope hash is missing" do
+      assert_scope_hash_conflict(
+        :delete,
+        :bulk_destroy,
+        valid_bulk_destroy_params.except(:scope_hash)
+      )
+    end
+
+    test "rejects bulk destroy when scope hash mismatches" do
+      assert_scope_hash_conflict(
+        :delete,
+        :bulk_destroy,
+        valid_bulk_destroy_params.merge(:scope_hash => 'stale-scope-hash')
+      )
+    end
+
+    test "rejects build when scope hash is missing" do
+      assert_scope_hash_conflict(:put, :build, valid_bulk_params.except(:scope_hash))
+    end
+
+    test "allows build without scope hash when skip_scope_hash is true" do
+      bulk_manager = mock('BulkHostsManager')
+      BulkHostsManager.expects(:new).with(hosts: anything).returns(bulk_manager)
+      bulk_manager.expects(:build).with(reboot: false).returns([])
+
+      put :build,
+        params: valid_bulk_params.except(:scope_hash).merge(:skip_scope_hash => true)
+
+      assert_response :success
+    end
+
+    test "rejects build when scope hash mismatches" do
+      assert_scope_hash_conflict(:put, :build, valid_bulk_params.merge(:scope_hash => 'stale-scope-hash'))
+    end
+
+    test "rejects hostgroup reassignment when scope hash is missing" do
+      assert_scope_hash_conflict(
+        :put,
+        :reassign_hostgroup,
+        valid_bulk_params.except(:scope_hash).merge(:hostgroup_id => 1)
+      )
+    end
+
+    test "rejects hostgroup reassignment when scope hash mismatches" do
+      assert_scope_hash_conflict(
+        :put,
+        :reassign_hostgroup,
+        valid_bulk_params.merge(:hostgroup_id => 1, :scope_hash => 'stale-scope-hash')
+      )
+    end
+
+    test "rejects owner change when scope hash is missing" do
+      assert_scope_hash_conflict(
+        :put,
+        :change_owner,
+        valid_bulk_params.except(:scope_hash).merge(:owner_id => @user.id_and_type)
+      )
+    end
+
+    test "rejects owner change when scope hash mismatches" do
+      assert_scope_hash_conflict(
+        :put,
+        :change_owner,
+        valid_bulk_params.merge(:owner_id => @user.id_and_type, :scope_hash => 'stale-scope-hash')
+      )
+    end
+
+    test "rejects disassociate when scope hash is missing" do
+      assert_scope_hash_conflict(:put, :disassociate, valid_bulk_params.except(:scope_hash))
+    end
+
+    test "rejects disassociate when scope hash mismatches" do
+      assert_scope_hash_conflict(:put, :disassociate, valid_bulk_params.merge(:scope_hash => 'stale-scope-hash'))
+    end
+
+    test "rejects organization assignment when scope hash is missing" do
+      assert_scope_hash_conflict(
+        :put,
+        :assign_organization,
+        valid_bulk_params.except(:scope_hash).merge(:id => @organization.id, :mismatch_setting => true)
+      )
+    end
+
+    test "rejects organization assignment when scope hash mismatches" do
+      assert_scope_hash_conflict(
+        :put,
+        :assign_organization,
+        valid_bulk_params.merge(:id => @organization.id, :mismatch_setting => true, :scope_hash => 'stale-scope-hash')
+      )
+    end
+
+    test "rejects location assignment when scope hash is missing" do
+      assert_scope_hash_conflict(
+        :put,
+        :assign_location,
+        valid_bulk_params.except(:scope_hash).merge(:id => @location.id, :mismatch_setting => true)
+      )
+    end
+
+    test "rejects location assignment when scope hash mismatches" do
+      assert_scope_hash_conflict(
+        :put,
+        :assign_location,
+        valid_bulk_params.merge(:id => @location.id, :mismatch_setting => true, :scope_hash => 'stale-scope-hash')
+      )
+    end
+
+    test "rejects power change when scope hash is missing" do
+      assert_scope_hash_conflict(:put, :change_power_state, valid_power_params.except(:scope_hash))
+    end
+
+    test "rejects power change when scope hash mismatches" do
+      assert_scope_hash_conflict(:put, :change_power_state, valid_power_params.merge(:scope_hash => 'stale-scope-hash'))
+    end
+  end
+
   private
 
+  def valid_scope_hash
+    'valid-scope-hash'
+  end
+
+  def assert_scope_hash_conflict(http_method, action, params)
+    send(http_method, action, params: params)
+
+    assert_response :conflict
+    body = ActiveSupport::JSON.decode(@response.body)
+    assert_match(/Refresh and retry the bulk action/, body['error']['message'])
+  end
+
   def set_session_user
     { :user => users(:admin).id }
   end