diff --git a/lib/rbac/filterer.rb b/lib/rbac/filterer.rb index b6c24c6e1f0..2b8055039cc 100644 --- a/lib/rbac/filterer.rb +++ b/lib/rbac/filterer.rb @@ -289,7 +289,22 @@ def skip_references?(options, attrs) end def include_references(scope, klass, include_for_find, exp_includes, skip) - return scope if skip + if skip + # We want to no-op here, since the rescue means we should just execute + # the rest of the method as usual and skip the `return`. + begin + ActiveRecord::Base.connection.explain(scope.to_sql) + return scope + rescue ActiveRecord::StatementInvalid => e + unless Rails.env.production? + warn "There was an issue with the Rbac filter without references!" + warn "Consider trying to fix this edge case in Rbac::Filterer! Error Below:" + warn e.message + warn e.backtrace + end + end + end + ref_includes = Hash(include_for_find).merge(Hash(exp_includes)) unless polymorphic_include?(klass, ref_includes) scope = scope.references(include_for_find).references(exp_includes) diff --git a/spec/lib/rbac/filterer_spec.rb b/spec/lib/rbac/filterer_spec.rb index 3c0352f5b50..b07bb86a19d 100644 --- a/spec/lib/rbac/filterer_spec.rb +++ b/spec/lib/rbac/filterer_spec.rb @@ -1804,6 +1804,69 @@ def get_rbac_results_for_and_expect_objects(klass, expected_objects) expect(resulting_scope.references_values).to eq([]) end + + context "when the scope is invalid without .references" do + let(:scope) { klass.where("host.name = 'foo'") } + let(:method_args) { [scope, klass, include_for_find, exp_includes, skip] } + let(:resulting_scope) { subject.send(:include_references, *method_args) } + + let(:explain_error_match) do + Regexp.new(Regexp.escape(<<~ERR.chomp)) + PG::UndefinedTable: ERROR: missing FROM-clause entry for table "host" + LINE 1: ...nfraManager::Vm') AND "vms"."template" = 'f' AND (host.name ... + ^ + : EXPLAIN SELECT "vms".* FROM "vms" WHERE "vms"."type" + ERR + end + + before { allow(subject).to receive(:warn) } + + it "adds .references to the scope" do + expect(resulting_scope.references_values).to eq(["{:miq_server=>{}}", "{:host=>{}}"]) + end + + it "warns that there was an issue in test mode" do + # Make sure part of the warn output includes stacktrace lines from + # rbac and this file. Does a bit of line addition to avoid this + # being too brittle and breaking easily, but expect it to break if + # you update Rbac::Filterer#include_references + method_file, method_line = subject.method(:include_references).source_location + explain_stacktrace_includes = [ + "#{method_file}:#{method_line + 5}:in `include_references'", + Thread.current.backtrace[1].gsub(/:\d*:/) { |sub| ":#{sub.tr(":", "").to_i + 7}:" } + ] + + expect(subject).to receive(:warn).with("There was an issue with the Rbac filter without references!") + expect(subject).to receive(:warn).with("Consider trying to fix this edge case in Rbac::Filterer! Error Below:") + expect(subject).to receive(:warn).with(explain_error_match) + expect(subject).to receive(:warn).with(array_including(explain_stacktrace_includes)) + resulting_scope + end + + it "warns that there was an issue in development mode" do + expect(Rails).to receive(:env).and_return(ActiveSupport::StringInquirer.new("developement")) + + # See above + method_file, method_line = subject.method(:include_references).source_location + explain_stacktrace_includes = [ + "#{method_file}:#{method_line + 5}:in `include_references'", + Thread.current.backtrace[1].gsub(/:\d*:/) { |sub| ":#{sub.tr(":", "").to_i + 7}:" } + ] + + expect(subject).to receive(:warn).with("There was an issue with the Rbac filter without references!") + expect(subject).to receive(:warn).with("Consider trying to fix this edge case in Rbac::Filterer! Error Below:") + expect(subject).to receive(:warn).with(explain_error_match) + expect(subject).to receive(:warn).with(array_including(explain_stacktrace_includes)) + resulting_scope + end + + it "warns that there was an issue in production mode" do + expect(Rails).to receive(:env).and_return(ActiveSupport::StringInquirer.new("production")) + + expect(subject).to receive(:warn).never + resulting_scope + end + end end context "if the include is polymorphic" do