fix(APIv2): RHINENG-11772 restrict search/sort to specific parents

app/controllers/concerns/error_handling.rb

@@ -74,6 +74,9 @@ module ErrorHandling
 
     rescue_from ScopedSearch::QueryNotSupported do |error|
       message = "Invalid parameter: #{error.message}"
+      # As we are using the validation to restrict searches for certain hierarchies, the error message
+      # for these should be adjusted to reflect the actual issue.
+      message.sub!(/Value '([^']*)' is not valid for field '([^']+)'/, "Field '\\2' is not searchable in this context")
       logger.info "#{message} (#{ScopedSearch::QueryNotSupported})"
       render_error message, status: :unprocessable_entity
     end

app/controllers/concerns/v2/collection.rb

@@ -48,6 +48,7 @@ def filter_by_tags(data)
         data.where('tags @> ?', tags.to_json)
       end
 
+      # rubocop:disable Metrics/AbcSize
       def search(data)
         return data if permitted_params[:filter].blank?
 
@@ -56,8 +57,14 @@ def search(data)
           raise ActionController::UnpermittedParameters.new(filter: permitted_params[:filter])
         end
 
+        # Pass the parents to the current thread context as there is no other way to access
+        # the parents from inside models. This is obviously an antipattern, but we are limited
+        # by scoped_search here and I have not found any better option.
+        Thread.current[:parents] = permitted_params[:parents]
+
         data.search_for(permitted_params[:filter])
       end
+      # rubocop:enable Metrics/AbcSize
 
       def sort(data)
         order_hash = data.klass.build_order_by(permitted_params[:sort_by])

app/models/concerns/v2/searchable.rb

@@ -7,10 +7,26 @@ module V2
   module Searchable
     extend ActiveSupport::Concern
 
+    # The validator is used to ensure that some fields are not available for searching under certain
+    # parent hierarchy combinations. It is expected that the controller passes the parents to the
+    # current thread context before calling seach_for.
+    module ParentValidator
+      def self.new(except, only)
+        lambda do |_|
+          parents = Thread.current[:parents].to_a
+
+          !(only.any? && !parents.intersect?(only)) && !(except.any? && parents.intersect?(except))
+        end
+      end
+    end
+
     class_methods do
       # This is a wrapper around scoped_search with some of our conventions around explicit-by-default
       # search and mandatory operator definitions. It also simplifies the usage of ext_method searches
       # by allowing them to be passed as blocks. The block's signature is the same as the ext_method's.
+      # Additionally, the `except_parents` and `only_parents` arrays can be used for search restriction
+      # to specific parent hierarchies.
+      #
       # For more info see: https://github.com/wvanbergen/scoped_search/wiki/search-definition
       #
       # Examples:
@@ -21,13 +37,14 @@ module Searchable
       #   { condition: 'first_name = ? OR last_name = ?', parameters: [val] }
       # end
       # ```
-      def searchable_by(field, operators, **args, &)
+      def searchable_by(field, operators, except_parents: [], only_parents: [], **args, &)
         if block_given?
           args[:ext_method] = "__find_by_#{field}".to_sym
           define_singleton_method(args[:ext_method], &)
         end
 
-        scoped_search on: field, operators: operators, only_explicit: true, **args
+        validator = ParentValidator.new(except_parents, only_parents)
+        scoped_search on: field, operators: operators, validator: validator, only_explicit: true, **args
       end
     end
   end

app/models/v2/policy.rb

@@ -33,7 +33,7 @@ class Policy < ApplicationRecord
     sortable_by :compliance_threshold
 
     searchable_by :title, %i[like unlike eq ne in notin]
-    searchable_by :os_major_version, %i[eq ne in notin] do |_key, op, val|
+    searchable_by :os_major_version, %i[eq ne in notin], except_parents: %i[systems] do |_key, op, val|
       bind = ['IN', 'NOT IN'].include?(op) ? '(?)' : '?'
 
       {

app/models/v2/report.rb

@@ -101,15 +101,15 @@ class Report < ApplicationRecord
     sortable_by :percent_compliant, 'aggregate_percent_compliant'
 
     searchable_by :title, %i[like unlike eq ne in notin]
-    searchable_by :os_major_version, %i[eq ne in notin] do |_key, op, val|
+    searchable_by :os_major_version, %i[eq ne in notin], except_parents: %i[system] do |_key, op, val|
       bind = ['IN', 'NOT IN'].include?(op) ? '(?)' : '?'
 
       {
         conditions: "security_guide.os_major_version #{op} #{bind}",
         parameter: [val.split.map(&:to_i)]
       }
     end
-    searchable_by :with_reported_systems, %i[eq] do |_key, _op, _val|
+    searchable_by :with_reported_systems, %i[eq], except_parents: %i[system] do |_key, _op, _val|
       ids = V2::Report.joins(:test_results).reselect(:id)
 
       { conditions: "v2_policies.id IN (#{ids.to_sql})" }

app/models/v2/system.rb

@@ -82,7 +82,7 @@ def self.first_group_name(table = arel_table)
 
     searchable_by :display_name, %i[eq neq like unlike]
 
-    searchable_by :os_major_version, %i[eq neq in notin] do |_key, op, val|
+    searchable_by :os_major_version, %i[eq neq in notin], except_parents: %i[policies reports] do |_key, op, val|
       {
         conditions: os_major_versions(val.split.map(&:to_i), %w[IN =].include?(op)).arel.where_sql.sub(/^where /i, '')
       }
@@ -102,7 +102,7 @@ def self.first_group_name(table = arel_table)
       { conditions: "inventory.hosts.id IN (#{ids.to_sql})" }
     end
 
-    searchable_by :never_reported, %i[eq] do |_key, _op, _val|
+    searchable_by :never_reported, %i[eq], only_parents: %i[reports] do |_key, _op, _val|
       ids = V2::TestResult.reselect(:system_id, :report_id)
 
       { conditions: "(inventory.hosts.id, reports.id) NOT IN (#{ids.to_sql})" }
@@ -114,14 +114,14 @@ def self.first_group_name(table = arel_table)
       { conditions: systems.arel.where_sql.gsub(/^where /i, '') }
     end
 
-    searchable_by :policies, %i[eq in] do |_key, _op, val|
+    searchable_by :policies, %i[eq in], except_parents: %i[policies reports] do |_key, _op, val|
       values = val.split.map(&:strip)
       ids = ::V2::PolicySystem.where(policy_id: values).select(:system_id)
 
       { conditions: "inventory.hosts.id IN (#{ids.to_sql})" }
     end
 
-    searchable_by :profile_ref_id, %i[neq notin] do |_key, _op, val|
+    searchable_by :profile_ref_id, %i[neq notin], except_parents: %i[policies reports] do |_key, _op, val|
       values = val.split.map(&:strip)
       ids = ::V2::PolicySystem.joins(policy: :profile).where(profile: { ref_id: values }).select(:system_id)
 

spec/integration/v2/policies_spec.rb

@@ -189,8 +189,8 @@
       operationId 'SystemsPolicies'
       content_types
       pagination_params_v2
-      sort_params_v2(V2::Policy)
-      search_params_v2(V2::Policy)
+      sort_params_v2(V2::Policy, except: %i[os_major_version total_system_count])
+      search_params_v2(V2::Policy, except: %i[os_major_version])
 
       parameter name: :system_id, in: :path, type: :string, required: true
 

spec/integration/v2/reports_spec.rb

@@ -208,8 +208,8 @@
       operationId 'SystemReports'
       content_types
       pagination_params_v2
-      sort_params_v2(V2::Report)
-      search_params_v2(V2::Report)
+      sort_params_v2(V2::Report, except: %i[os_major_version])
+      search_params_v2(V2::Report, except: %i[os_major_version with_reported_systems])
 
       parameter name: :system_id, in: :path, type: :string, required: true
 

spec/integration/v2/systems_spec.rb

@@ -19,7 +19,7 @@
       content_types
       pagination_params_v2
       sort_params_v2(V2::System)
-      search_params_v2(V2::System)
+      search_params_v2(V2::System, except: %i[never_reported])
 
       response '200', 'Lists Systems' do
         v2_collection_schema 'system'
@@ -149,8 +149,8 @@
       operationId 'PolicySystems'
       content_types
       pagination_params_v2
-      sort_params_v2(V2::System)
-      search_params_v2(V2::System)
+      sort_params_v2(V2::System, except: %i[os_major_version])
+      search_params_v2(V2::System, except: %i[never_reported os_major_version policies profile_ref_id])
 
       parameter name: :policy_id, in: :path, type: :string, required: true
 
@@ -163,19 +163,19 @@
       end
 
       response '200', 'Lists Systems assigned to a Policy' do
-        let(:sort_by) { ['os_major_version'] }
+        let(:sort_by) { ['os_minor_version'] }
         v2_collection_schema 'system'
 
-        after { |e| autogenerate_examples(e, 'List of Systems sorted by "os_major_version:asc"') }
+        after { |e| autogenerate_examples(e, 'List of Systems sorted by "os_minor_version:asc"') }
 
         run_test!
       end
 
       response '200', 'Lists Systems assigned to a Policy' do
-        let(:filter) { '(os_major_version=8)' }
+        let(:filter) { '(os_minor_version=0)' }
         v2_collection_schema 'system'
 
-        after { |e| autogenerate_examples(e, 'List of Systems filtered by "(os_major_version=8)"') }
+        after { |e| autogenerate_examples(e, 'List of Systems filtered by "(os_minor_version=0)"') }
 
         run_test!
       end
@@ -388,8 +388,8 @@
       operationId 'ReportSystems'
       content_types
       pagination_params_v2
-      sort_params_v2(V2::System)
-      search_params_v2(V2::System)
+      sort_params_v2(V2::System, except: %i[os_major_version])
+      search_params_v2(V2::System, except: %i[os_major_version policies profile_ref_id])
 
       parameter name: :report_id, in: :path, type: :string, required: true
 
@@ -402,19 +402,19 @@
       end
 
       response '200', 'Lists Systems assigned to a Report' do
-        let(:sort_by) { ['os_major_version'] }
+        let(:sort_by) { ['os_minor_version'] }
         v2_collection_schema 'system'
 
-        after { |e| autogenerate_examples(e, 'List of Systems sorted by "os_major_version:asc"') }
+        after { |e| autogenerate_examples(e, 'List of Systems sorted by "os_minor_version:asc"') }
 
         run_test!
       end
 
       response '200', 'Lists Systems assigned to a Report' do
-        let(:filter) { '(os_major_version=8)' }
+        let(:filter) { '(os_minor_version=0)' }
         v2_collection_schema 'system'
 
-        after { |e| autogenerate_examples(e, 'List of Systems filtered by "(os_major_version=8)"') }
+        after { |e| autogenerate_examples(e, 'List of Systems filtered by "(os_minor_version=0)"') }
 
         run_test!
       end

spec/swagger_helper.rb

@@ -119,15 +119,16 @@ def search_params
             schema: { type: :string }
 end
 
-def search_params_v2(model = nil)
+def search_params_v2(model = nil, except: [])
+  keys = model.scoped_search.fields.keys.reject { |key| key == :_____ || except.include?(key) }
+
   parameter name: :filter, in: :query, required: false,
             description: 'Query string to filter items by their attributes. ' \
               'Compliant with <a href="https://github.com/wvanbergen/scoped_search/wiki/Query-language" ' \
               'target="_blank" title="github.com/wvanbergen/scoped_search">scoped_search query language</a>. ' \
               'However, only `=` or `!=` (resp. `<>`) operators are supported.<br><br>' \
               "#{model.name.split('::').second.gsub(/([A-Z])/) { " #{Regexp.last_match(1)}" }.strip.pluralize} " \
-              'are searchable using attributes ' \
-              "#{model.scoped_search.fields.keys.reject { |k| k == :_____ }.map { |k| "`#{k}`" }.to_sentence}" \
+              "are searchable using attributes #{keys.map { |k| "`#{k}`" }.to_sentence}" \
               '<br><br>(e.g.: `(field_1=something AND field_2!="something else") OR field_3>40`)',
             schema: { type: :string }
 end
@@ -152,20 +153,22 @@ def sort_params(model = nil)
             }
 end
 
-def sort_params_v2(model = nil)
+def sort_params_v2(model = nil, except: [])
   parameter name: :sort_by, in: :query, required: false,
             description: 'Attribute and direction to sort the items by. ' \
               'Represented by an array of fields with an optional direction ' \
               '(`<key>:asc` or `<key>:desc`).<br><br>' \
               'If no direction is selected, `<key>:asc` is used by default.',
             schema: {
               type: :array,
-              items: { enum: sort_combinations(model) }
+              items: { enum: sort_combinations(model, except) }
             }
 end
 
-def sort_combinations(model)
-  fields = model.instance_variable_get(:@sortable_by).keys
+def sort_combinations(model, except = [])
+  fields = model.instance_variable_get(:@sortable_by).keys.reject do |field|
+    except.include?(field)
+  end
   fields + fields.flat_map do |field|
     %w[asc desc].map do |direction|
       [field, direction].join(':')