fix: code

Author: arnaud-moncel

packages/forest_admin_agent/lib/forest_admin_agent/builder/agent_factory.rb

@@ -22,8 +22,6 @@ def initialize
 
       def setup(options)
         @options = options
-        Services::Permissions.skip_relation_read_permissions =
-          options.to_h[:skip_relation_read_permissions] == true
         @has_env_secret = !options.to_h[:env_secret].nil?
         @customizer = ForestAdminDatasourceCustomizer::DatasourceCustomizer.new
         build_container
@@ -345,8 +343,10 @@ def should_skip_schema_update?
       end
 
       # An auditor reading the boot log should see the weakened posture without reading the config.
+      # Read from the options `setup` was handed: `Facades::Container` resolves against
+      # `AgentFactory.instance`, which a host subclassing this factory has not populated.
       def warn_relation_read_permissions_skipped
-        return unless Services::Permissions.skip_relation_read_permissions?
+        return unless @options.to_h[:skip_relation_read_permissions] == true
 
         @logger.log(
           'Warn',

packages/forest_admin_agent/lib/forest_admin_agent/routes/capabilities/collections.rb

@@ -69,7 +69,7 @@ def handle_request(args = {})
                 canUseProjectionViaHeaderOnList: true,
                 canUseMultipleFieldsProjectionOnRelation: true,
                 canUseAuditTrail: audit_trail_enabled?,
-                checksRelationReadPermissions: !Services::Permissions.skip_relation_read_permissions?
+                checksRelationReadPermissions: !skip_relation_read_permissions?
               }
             },
             status: 200
@@ -81,6 +81,10 @@ def handle_request(args = {})
         # True only where the store the record-history route reads from exists — the same lookup that route
         # mounts itself on, so the capability cannot drift from what the routes actually serve. The front gates
         # its History tab on this.
+        def skip_relation_read_permissions?
+          ForestAdminAgent::Facades::Container.config_from_cache[:skip_relation_read_permissions] == true
+        end
+
         def audit_trail_enabled?
           !::ForestAdminAgent::AuditTrail.store.nil?
         end

packages/forest_admin_agent/lib/forest_admin_agent/routes/charts/charts.rb

@@ -55,6 +55,10 @@ def aggregate_field_name(args)
           field.nil? || field.to_s.empty? ? nil : field
         end
 
+        def skip_relation_read_permissions?
+          ForestAdminAgent::Facades::Container.config_from_cache[:skip_relation_read_permissions] == true
+        end
+
         def validate_and_get_type(type)
           chart_types = %w[Value Objective Pie Line Leaderboard]
           unless chart_types.include?(type)
@@ -204,7 +208,7 @@ def make_leaderboard(context, filter, args)
             # A count exposes the cardinality of the relation, which `/relationships/<name>/count`
             # puts behind `browse`. No path names it, so nothing above sees it. It arrived with the
             # related-read checks, so it goes away with them.
-            if aggregation.field.nil? && !Services::Permissions.skip_relation_read_permissions?
+            if aggregation.field.nil? && !skip_relation_read_permissions?
               context.permissions.can?(:browse, context.datasource.get_collection(field.foreign_collection))
             end
 

packages/forest_admin_agent/lib/forest_admin_agent/services/permissions.rb

@@ -35,20 +35,6 @@ def self.invalidate_cache(id_cache = nil)
         ForestAdminAgent::Facades::Container.logger.log('Info', "Invalidating #{id_cache} cache..")
       end
 
-      # Set once from the options `AgentFactory#setup` was handed, rather than resolved from
-      # `Facades::Container`: that answers from `AgentFactory.instance`, so a host subclassing the
-      # factory — `ForestAdminRpcAgent::Agent` has its own singleton — would leave the option
-      # ignored and the checks silently on. A class method so the chart and capabilities routes can
-      # ask without an instance, and so the route specs faking this service with an
-      # `instance_double` do not each have to stub one more message.
-      class << self
-        attr_writer :skip_relation_read_permissions
-      end
-
-      def self.skip_relation_read_permissions?
-        @skip_relation_read_permissions == true
-      end
-
       def can?(action, collection, allow_fetch: false)
         return true unless permission_system?
 
@@ -81,7 +67,7 @@ def read_permissions(root_collection_name, collection_names)
         # An absent permission system is not a denial: `can?` allows everything there, and answering
         # anything else would redact every relation on a deployment that granted nothing to check.
         # `skip_relation_read_permissions` is the operator asking for that same answer on purpose.
-        if self.class.skip_relation_read_permissions? || !permission_system?
+        if skip_relation_read_permissions? || !permission_system?
           return allowed.merge(to_check.to_h { |name| [name, true] })
         end
 
@@ -290,6 +276,10 @@ def readable_leaves?(names, allowed)
         names.any? && names.all? { |name| allowed[name] }
       end
 
+      def skip_relation_read_permissions?
+        Facades::Container.config_from_cache[:skip_relation_read_permissions] == true
+      end
+
       def leaf_label(names)
         names.empty? ? 'an unresolved polymorphic relation' : "the '#{names.join("' or '")}' collection"
       end
@@ -374,7 +364,7 @@ def assert_extended_search_checkable(collection, search_extended)
         return unless describes_own_search?(collection)
         return unless permission_system?
         # Refused ahead of `read_permissions`, so the skip has to be read here as well.
-        return if self.class.skip_relation_read_permissions?
+        return if skip_relation_read_permissions?
 
         raise ForbiddenError,
               "You cannot run an extended search on the '#{collection.name}' collection: the fields " \

packages/forest_admin_agent/spec/lib/forest_admin_agent/builder/agent_factory_spec.rb

@@ -900,13 +900,12 @@ module Builder
       end
 
       describe 'warning that the relation read checks are off' do
-        after { Services::Permissions.skip_relation_read_permissions = false }
-
+        # `@options` is what `setup` holds when it calls this, so the spec hands over the same thing.
         def boot_with(skip)
           instance = described_class.instance
           logger = instance_spy(Services::LoggerService)
           instance.instance_variable_set(:@logger, logger)
-          Services::Permissions.skip_relation_read_permissions = skip
+          instance.instance_variable_set(:@options, { skip_relation_read_permissions: skip })
 
           instance.send(:warn_relation_read_permissions_skipped)
 
@@ -925,46 +924,6 @@ def boot_with(skip)
           expect(boot_with(false)).not_to have_received(:log)
         end
       end
-
-      # `Facades::Container` resolves against `AgentFactory.instance`, so a host that subclasses
-      # this factory — `ForestAdminRpcAgent::Agent`, with its own singleton — has no container
-      # there. Reading the option through the facade left it ignored on exactly those hosts, which
-      # kept the checks on while the operator had asked for them off.
-      describe 'carrying skip_relation_read_permissions out of the options' do
-        let(:options) do
-          {
-            auth_secret: 'cba803d01a4d43b55010cab41fa1ea1f1f51a95e',
-            env_secret: '89719c6d8e2e2de2694c2f220fe2dbf02d5289487364daf1e4c6b13733ed0cdb',
-            is_production: false,
-            schema_path: File.join('tmp', '.forestadmin-schema.json')
-          }
-        end
-
-        after { Services::Permissions.skip_relation_read_permissions = false }
-
-        it 'takes the value setup was handed' do
-          described_class.instance.setup(options.merge(skip_relation_read_permissions: true))
-
-          expect(Services::Permissions.skip_relation_read_permissions?).to be true
-        end
-
-        it 'defaults to keeping the checks on when the host declares nothing' do
-          Services::Permissions.skip_relation_read_permissions = true
-
-          described_class.instance.setup(options)
-
-          expect(Services::Permissions.skip_relation_read_permissions?).to be false
-        end
-
-        it 'answers without resolving the base facade container' do
-          allow(Facades::Container).to receive(:config_from_cache).and_raise(
-            NoMethodError, "undefined method `resolve' for nil"
-          )
-          Services::Permissions.skip_relation_read_permissions = true
-
-          expect(Services::Permissions.skip_relation_read_permissions?).to be true
-        end
-      end
     end
   end
 end

packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/capabilities/collections_spec.rb

@@ -95,11 +95,12 @@ module Capabilities
           # The frontend prunes projections on this flag, so announcing it wrong either hides
           # columns the agent serves or lets the frontend ask for ones it refuses.
           it 'announces the relation read checks as off once the option turns them off' do
-            ForestAdminAgent::Services::Permissions.skip_relation_read_permissions = true
+            configured = ForestAdminAgent::Facades::Container.config_from_cache.merge(
+              skip_relation_read_permissions: true
+            )
+            allow(ForestAdminAgent::Facades::Container).to receive(:config_from_cache).and_return(configured)
 
             expect(result[:content][:agentCapabilities][:checksRelationReadPermissions]).to be false
-          ensure
-            ForestAdminAgent::Services::Permissions.skip_relation_read_permissions = false
           end
 
           it 'returns agentCapabilities' do

packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/charts/charts_spec.rb

@@ -581,8 +581,10 @@ module Charts
                                                   type: 'Leaderboard',
                                                   timezone: 'Europe/Paris'
                                                 })
-            allow(ForestAdminAgent::Services::Permissions).to receive(:skip_relation_read_permissions?)
-              .and_return(true)
+            configured = ForestAdminAgent::Facades::Container.config_from_cache.merge(
+              skip_relation_read_permissions: true
+            )
+            allow(ForestAdminAgent::Facades::Container).to receive(:config_from_cache).and_return(configured)
             allow(permissions).to receive(:can?)
             allow(@datasource.get_collection('book')).to receive(:datasource).and_return(@datasource)
             review = @datasource.get_collection('review')

packages/forest_admin_agent/spec/lib/forest_admin_agent/security/related_read_permissions_spec.rb

@@ -619,9 +619,10 @@ def with_instant_cache_refresh(enabled)
       # asked by the route through `can?`, and still answered.
       describe 'with skip_relation_read_permissions' do
         def unchecked_permissions(readable = [])
-          allow(described_class).to receive(:skip_relation_read_permissions?).and_return(true)
+          permissions = build_permissions(readable)
+          allow(permissions).to receive(:skip_relation_read_permissions?).and_return(true)
 
-          build_permissions(readable)
+          permissions
         end
 
         def searchable_cards(searched, search_handler: false)