feat(scopes): enforce scopes restrictions on a wider range of requests (#488)

Author: larcin

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    forest_liana (7.0.0-beta.4)
+    forest_liana (7.0.0.pre.beta.4)
       arel-helpers
       bcrypt
       forestadmin-jsonapi-serializers (>= 0.14.0)
@@ -252,4 +252,4 @@ DEPENDENCIES
   useragent
 
 BUNDLED WITH
-   2.1.4
+   2.2.3

app/controllers/forest_liana/actions_controller.rb

@@ -1,5 +1,5 @@
 module ForestLiana
-  class ActionsController < ForestLiana::BaseController
+  class ActionsController < ApplicationController
 
     def get_smart_action_hook_request
       begin
@@ -17,7 +17,7 @@ def get_collection(collection_name)
     def get_action(collection_name)
       collection = get_collection(collection_name)
       begin
-         collection.actions.find {|action| ActiveSupport::Inflector.parameterize(action.name) == params[:action_name]}
+        collection.actions.find {|action| ActiveSupport::Inflector.parameterize(action.name) == params[:action_name]}
       rescue => error
         FOREST_LOGGER.error "Smart Action get action retrieval error: #{error}"
         nil

app/controllers/forest_liana/associations_controller.rb

@@ -10,7 +10,7 @@ class AssociationsController < ForestLiana::ApplicationController
 
     def index
       begin
-        getter = HasManyGetter.new(@resource, @association, params)
+        getter = HasManyGetter.new(@resource, @association, params, forest_user)
         getter.perform
 
         respond_to do |format|
@@ -25,7 +25,7 @@ def index
 
     def count
       begin
-        getter = HasManyGetter.new(@resource, @association, params)
+        getter = HasManyGetter.new(@resource, @association, params, forest_user)
         getter.count
 
         render serializer: nil, json: { count: getter.records_count }

app/controllers/forest_liana/resources_controller.rb

@@ -29,7 +29,7 @@ def index
           return head :forbidden unless checker.is_authorized?
         end
 
-        getter = ForestLiana::ResourcesGetter.new(@resource, params)
+        getter = ForestLiana::ResourcesGetter.new(@resource, params, forest_user)
         getter.perform
 
         respond_to do |format|
@@ -63,7 +63,7 @@ def count
         )
         return head :forbidden unless checker.is_authorized?
 
-        getter = ForestLiana::ResourcesGetter.new(@resource, params)
+        getter = ForestLiana::ResourcesGetter.new(@resource, params, forest_user)
         getter.count
 
         render serializer: nil, json: { count: getter.records_count }
@@ -89,10 +89,12 @@ def show
         checker = ForestLiana::PermissionsChecker.new(@resource, 'readEnabled', @rendering_id, user_id: forest_user['id'])
         return head :forbidden unless checker.is_authorized?
 
-        getter = ForestLiana::ResourceGetter.new(@resource, params)
+        getter = ForestLiana::ResourceGetter.new(@resource, params, forest_user)
         getter.perform
 
         render serializer: nil, json: render_record_jsonapi(getter.record)
+      rescue ActiveRecord::RecordNotFound
+        render serializer: nil, json: { status: 404 }, status: :not_found
       rescue => error
         FOREST_LOGGER.error "Record Show error: #{error}\n#{format_stacktrace(error)}"
         internal_server_error
@@ -127,7 +129,7 @@ def update
         checker = ForestLiana::PermissionsChecker.new(@resource, 'editEnabled', @rendering_id, user_id: forest_user['id'])
         return head :forbidden unless checker.is_authorized?
 
-        updater = ForestLiana::ResourceUpdater.new(@resource, params)
+        updater = ForestLiana::ResourceUpdater.new(@resource, params, forest_user)
         updater.perform
 
         if updater.errors
@@ -149,7 +151,14 @@ def destroy
       checker = ForestLiana::PermissionsChecker.new(@resource, 'deleteEnabled', @rendering_id, user_id: forest_user['id'])
       return head :forbidden unless checker.is_authorized?
 
-      @resource.destroy(params[:id]) if @resource.exists?(params[:id])
+      collection_name = ForestLiana.name_for(@resource)
+      scoped_records = ForestLiana::ScopeManager.apply_scopes_on_records(@resource, forest_user, collection_name, params[:timezone])
+
+      unless scoped_records.exists?(params[:id])
+        return render serializer: nil, json: { status: 404 }, status: :not_found
+      end
+
+      scoped_records.destroy(params[:id])
 
       head :no_content
     rescue => error
@@ -161,7 +170,7 @@ def destroy_bulk
       checker = ForestLiana::PermissionsChecker.new(@resource, 'deleteEnabled', @rendering_id, user_id: forest_user['id'])
       return head :forbidden unless checker.is_authorized?
 
-      ids = ForestLiana::ResourcesGetter.get_ids_from_request(params)
+      ids = ForestLiana::ResourcesGetter.get_ids_from_request(params, forest_user)
       @resource.destroy(ids) if ids&.any?
 
       head :no_content

app/controllers/forest_liana/scopes_controller.rb

@@ -0,0 +1,20 @@
+module ForestLiana
+  class ScopesController < ForestLiana::ApplicationController
+    def invalidate_scope_cache
+      begin
+        rendering_id = params[:renderingId]
+
+        unless rendering_id
+          FOREST_LOGGER.error 'Missing renderingId'
+          return render serializer: nil, json: { status: 400 }, status: :bad_request
+        end
+
+        ForestLiana::ScopeManager.invalidate_scope_cache(rendering_id)
+        return render serializer: nil, json: { status: 200 }, status: :ok
+      rescue => error
+        FOREST_LOGGER.error "Error during scope cache invalidation: #{error.message}"
+        render serializer: nil, json: {status: 500 }, status: :internal_server_error
+      end
+    end
+  end
+end

app/controllers/forest_liana/smart_actions_controller.rb

@@ -1,9 +1,9 @@
 module ForestLiana
   class SmartActionsController < ForestLiana::ApplicationController
     if Rails::VERSION::MAJOR < 4
-      before_filter :check_permission_for_smart_route
+      before_filter :smart_action_pre_perform_checks
     else
-      before_action :check_permission_for_smart_route
+      before_action :smart_action_pre_perform_checks
     end
 
     private
@@ -17,6 +17,41 @@ def get_smart_action_request
       end
     end
 
+    def smart_action_pre_perform_checks
+      check_permission_for_smart_route
+      ensure_record_ids_in_scope
+    end
+
+    def ensure_record_ids_in_scope
+      begin
+        attributes = get_smart_action_request
+
+        # if performing a `selectAll` let the `get_ids_from_request` handle the scopes
+        return if attributes[:all_records]
+
+        resource = find_resource(attributes[:collection_name])
+
+        # user is using the composite_primary_keys gem
+        if resource.primary_key.kind_of?(Array)
+          # TODO: handle primary keys
+          return
+        end
+
+        filter = JSON.generate({ 'field' => resource.primary_key, 'operator' => 'in', 'value' => attributes[:ids] })
+
+        resources_getter = ForestLiana::ResourcesGetter.new(resource, { :filters => filter, :timezone => attributes[:timezone] }, forest_user)
+
+        # resources getter will return records inside the scope. if the length differs then ids are out of scope
+        return if resources_getter.count == attributes[:ids].length
+
+        # target records are out of scope
+        render serializer: nil, json: { error: 'Smart Action: target record not found' }, status: :bad_request
+      rescue => error
+        FOREST_LOGGER.error "Smart Action: #{error}\n#{format_stacktrace(error)}"
+        render serializer: nil, json: { error: 'Smart Action: failed to evaluate permissions' }, status: :internal_server_error
+      end
+    end
+
     def check_permission_for_smart_route
       begin
 
@@ -58,7 +93,8 @@ def find_resource(collection_name)
     # smart action permissions are retrieved from the action's endpoint and http_method
     def get_smart_action_request_info
       {
-        endpoint: request.fullpath,
+        # trim query params to get the endpoint
+        endpoint: request.fullpath.split('?').first,
         http_method: request.request_method
       }
     end

app/controllers/forest_liana/stats_controller.rb

@@ -27,15 +27,15 @@ class StatsController < ForestLiana::ApplicationController
     def get
       case params[:type]
       when CHART_TYPE_VALUE
-        stat = ValueStatGetter.new(@resource, params)
+        stat = ValueStatGetter.new(@resource, params, forest_user)
       when CHART_TYPE_PIE
-        stat = PieStatGetter.new(@resource, params)
+        stat = PieStatGetter.new(@resource, params, forest_user)
       when CHART_TYPE_LINE
-        stat = LineStatGetter.new(@resource, params)
+        stat = LineStatGetter.new(@resource, params, forest_user)
       when CHART_TYPE_OBJECTIVE
-        stat = ObjectiveStatGetter.new(@resource, params)
+        stat = ObjectiveStatGetter.new(@resource, params, forest_user)
       when CHART_TYPE_LEADERBOARD
-        stat = LeaderboardStatGetter.new(@resource, params)
+        stat = LeaderboardStatGetter.new(@resource, params, forest_user)
       end
 
       stat.perform

app/services/forest_liana/filters_parser.rb

@@ -109,13 +109,15 @@ def parse_condition_without_smart_field(condition)
       parsed_value = parse_value(operator, value)
       field_and_operator = "#{parsed_field} #{parsed_operator}"
 
+      # parenthesis around the parsed_value are required to make the `IN` operator work
+      # and have no side effects on other requests
       if Rails::VERSION::MAJOR < 5
-        "#{field_and_operator} #{ActiveRecord::Base.sanitize(parsed_value)}"
+        "#{field_and_operator} (#{ActiveRecord::Base.sanitize(parsed_value)})"
       # NOTICE: sanitize method as been removed in Rails 5.1 and sanitize_sql introduced in Rails 5.2.
       elsif Rails::VERSION::MAJOR == 5 && Rails::VERSION::MINOR == 1
-        "#{field_and_operator} #{ActiveRecord::Base.connection.quote(parsed_value)}"
+        "#{field_and_operator} (#{ActiveRecord::Base.connection.quote(parsed_value)})"
       else
-        ActiveRecord::Base.sanitize_sql(["#{field_and_operator} ?", parsed_value])
+        ActiveRecord::Base.sanitize_sql(["#{field_and_operator} (?)", parsed_value])
       end
     end
 
@@ -147,14 +149,16 @@ def parse_operator(operator)
         'IS'
       when 'present'
         'IS NOT'
+      when 'in'
+        'IN'
       else
         raise_unknown_operator_error(operator)
       end
     end
 
     def parse_value(operator, value)
       case operator
-      when 'not', 'greater_than', 'less_than', 'not_equal', 'equal', 'before', 'after'
+      when 'not', 'greater_than', 'less_than', 'not_equal', 'equal', 'before', 'after', 'in'
         value
       when 'contains', 'not_contains'
         "%#{value}%"

app/services/forest_liana/has_many_dissociator.rb

@@ -1,5 +1,5 @@
 module ForestLiana
-  class HasManyDissociator
+  class HasManyDissociator < ForestLiana::ApplicationController
     def initialize(resource, association, params)
       @resource = resource
       @association = association
@@ -17,7 +17,7 @@ def perform
       if @data.is_a?(Array)
         record_ids = @data.map { |record| record[:id] }
       elsif @data.dig('attributes').present?
-        record_ids = ForestLiana::ResourcesGetter.get_ids_from_request(@params)
+        record_ids = ForestLiana::ResourcesGetter.get_ids_from_request(@params, forest_user)
       else
         record_ids = Array.new
       end

app/services/forest_liana/has_many_getter.rb

@@ -4,7 +4,7 @@ class HasManyGetter < BaseGetter
     attr_reader :includes
     attr_reader :records_count
 
-    def initialize(resource, association, params)
+    def initialize(resource, association, params, forest_user)
       @resource = resource
       @association = association
       @params = params
@@ -13,7 +13,7 @@ def initialize(resource, association, params)
       @collection = get_collection(@collection_name)
       compute_includes()
       includes_symbols = @includes.map { |include| include.to_sym }
-      @search_query_builder = SearchQueryBuilder.new(@params, includes_symbols, @collection)
+      @search_query_builder = SearchQueryBuilder.new(@params, includes_symbols, @collection, forest_user)
 
       prepare_query()
     end