From 0cad903d27c054f0d847e4e0abfdd91e70074dea Mon Sep 17 00:00:00 2001 From: Shohan RAHMAN Date: Fri, 30 Oct 2026 16:47:39 +0100 Subject: [PATCH] fix(agent): apply the scope of collections reached through a relation Read routes only fetched the scope of the collection they serve, so the columns a projection pulls from to-one relations ignored the scope set on those related collections, and a leaderboard aggregated its related collection under the root scope only. show, list, csv, list_related and csv_related now fetch the scope of every collection the projection reaches, and empty a related record that falls outside it, keeping the root row. The leaderboard intersects the aggregated collection's scope (and, for a many-to-many, the foreign collection's through the target relation) into its filter. Refs: PRD-907 Co-Authored-By: Claude Opus 5.5 --- .../routes/charts/charts.rb | 18 +- .../routes/resources/csv.rb | 9 +- .../routes/resources/list.rb | 7 +- .../routes/resources/related/csv_related.rb | 19 ++- .../routes/resources/related/list_related.rb | 19 ++- .../routes/resources/show.rb | 7 +- .../services/permissions.rb | 21 +++ .../utils/relation_scopes.rb | 52 ++++++ .../routes/charts/charts_spec.rb | 51 ++++++ .../related_collection_scopes_spec.rb | 161 ++++++++++++++++++ .../forest_admin_agent/spec/spec_helper.rb | 1 + 11 files changed, 346 insertions(+), 19 deletions(-) create mode 100644 packages/forest_admin_agent/lib/forest_admin_agent/utils/relation_scopes.rb create mode 100644 packages/forest_admin_agent/spec/lib/forest_admin_agent/security/related_collection_scopes_spec.rb diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/routes/charts/charts.rb b/packages/forest_admin_agent/lib/forest_admin_agent/routes/charts/charts.rb index f7e2c4346..6869ef3bc 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/routes/charts/charts.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/routes/charts/charts.rb @@ -159,6 +159,18 @@ def make_line(context, filter, args) LineChart.new(result).serialize end + # No record of an aggregation can be redacted afterwards, so each aggregated scope filters. + def intersect_scopes(context, filter, scoped) + scopes = scoped.map do |collection_name, prefix| + scope = context.permissions.get_scope(context.datasource.get_collection(collection_name)) + prefix ? scope&.nest(prefix) : scope + end + + filter.override( + condition_tree: ConditionTree::ConditionTreeFactory.intersect([filter.condition_tree, *scopes]) + ) + end + def make_leaderboard(context, filter, args) field = context.collection.schema[:fields][args[:params][:relationshipFieldName]] @@ -169,7 +181,7 @@ def make_leaderboard(context, filter, args) ) if inverse collection = field.foreign_collection - leaderboard_filter = filter.nest(inverse) + leaderboard_filter = intersect_scopes(context, filter.nest(inverse), [[field.foreign_collection, nil]]) aggregation = Aggregation.new( operation: args[:params][:aggregator], field: aggregate_field_name(args), @@ -189,7 +201,9 @@ def make_leaderboard(context, filter, args) ) if origin && target collection = field.through_collection - leaderboard_filter = filter.nest(origin) + leaderboard_filter = intersect_scopes( + context, filter.nest(origin), [[field.through_collection, nil], [field.foreign_collection, target]] + ) aggregation = Aggregation.new( operation: args[:params][:aggregator], field: aggregate_field_name(args) ? "#{target}:#{aggregate_field_name(args)}" : nil, diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/csv.rb b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/csv.rb index 7f493bbd8..2b1d51620 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/csv.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/csv.rb @@ -40,7 +40,14 @@ def handle_request(args = {}) filename_with_timestamp = filename.gsub('.csv', "_export_#{now}.csv") # Return streaming enumerator instead of full CSV string - list_records = ->(batch_filter) { context.collection.list(context.caller, batch_filter, projection) } + relation_scopes = context.permissions.relation_scopes(context.collection, projection) + list_records = lambda do |batch_filter| + relation_scopes.redact( + context.collection.list(context.caller, batch_filter, relation_scopes.with_scope_fields(projection)), + projection, + context.caller.timezone + ) + end { content: { diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/list.rb b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/list.rb index a6aff6ed5..6454dfdb7 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/list.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/list.rb @@ -23,7 +23,12 @@ def handle_request(args = {}) .override(page: QueryStringParser.parse_pagination(args)) projection = redacted_projection_with_pks(context, context.collection, args) - records = context.collection.list(context.caller, filter, projection) + relation_scopes = context.permissions.relation_scopes(context.collection, projection) + records = relation_scopes.redact( + context.collection.list(context.caller, filter, relation_scopes.with_scope_fields(projection)), + projection, + context.caller.timezone + ) { name: args[:params]['collection_name'], diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/related/csv_related.rb b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/related/csv_related.rb index b77032004..79ddaa65a 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/related/csv_related.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/related/csv_related.rb @@ -47,14 +47,19 @@ def handle_request(args = {}) filename_with_timestamp = "#{collection_name}_#{relation_name}_export_#{now}.csv" # Create a callable to fetch related records + relation_scopes = context.permissions.relation_scopes(context.child_collection, projection) list_records = lambda do |batch_filter| - ForestAdminDatasourceToolkit::Utils::Collection.list_relation( - context.collection, - primary_key_values, - relation_name, - context.caller, - batch_filter, - projection + relation_scopes.redact( + ForestAdminDatasourceToolkit::Utils::Collection.list_relation( + context.collection, + primary_key_values, + relation_name, + context.caller, + batch_filter, + relation_scopes.with_scope_fields(projection) + ), + projection, + context.caller.timezone ) end diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/related/list_related.rb b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/related/list_related.rb index 0c3dfc5c8..6c1ed9d9a 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/related/list_related.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/related/list_related.rb @@ -30,13 +30,18 @@ def handle_request(args = {}) .override(page: ForestAdminAgent::Utils::QueryStringParser.parse_pagination(args)) projection = redacted_projection_with_pks(context, context.child_collection, args) primary_key_values = Utils::Id.unpack_id(context.collection, args[:params]['id'], with_key: true) - records = Collection.list_relation( - context.collection, - primary_key_values, - args[:params]['relation_name'], - context.caller, - filter, - projection + relation_scopes = context.permissions.relation_scopes(context.child_collection, projection) + records = relation_scopes.redact( + Collection.list_relation( + context.collection, + primary_key_values, + args[:params]['relation_name'], + context.caller, + filter, + relation_scopes.with_scope_fields(projection) + ), + projection, + context.caller.timezone ) { diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/show.rb b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/show.rb index 00d342cd2..607d01b52 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/show.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/show.rb @@ -27,7 +27,12 @@ def handle_request(args = {}) projection = redacted_projection_with_pks(context, context.collection, args) - records = context.collection.list(context.caller, filter, projection) + relation_scopes = context.permissions.relation_scopes(context.collection, projection) + records = relation_scopes.redact( + context.collection.list(context.caller, filter, relation_scopes.with_scope_fields(projection)), + projection, + context.caller.timezone + ) raise Http::Exceptions::NotFoundError, 'Record does not exists' unless records.size.positive? diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/services/permissions.rb b/packages/forest_admin_agent/lib/forest_admin_agent/services/permissions.rb index 8cc3e50ab..ab0621016 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/services/permissions.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/services/permissions.rb @@ -3,6 +3,8 @@ module ForestAdminAgent module Services class Permissions + TO_ONE_RELATION_TYPES = %w[ManyToOne OneToOne].freeze + include ForestAdminAgent::Http::Exceptions include ForestAdminAgent::Utils include ForestAdminDatasourceToolkit::Exceptions @@ -292,6 +294,11 @@ def get_scope(collection) ContextVariablesInjector.inject_context_in_filter(scope, context_variables) end + # The caller's scope on each collection the projection reaches through a to-one relation. + def relation_scopes(collection, projection) + Utils::RelationScopes.new(collect_relation_scopes(collection, projection)) + end + def get_segments(collection, force_fetch: false) permissions = get_rendering_data(caller.rendering_id, force_fetch: force_fetch) @@ -313,6 +320,20 @@ def get_team(rendering_id) private + def collect_relation_scopes(collection, projection, prefix = nil) + projection.relations.flat_map do |name, sub_projection| + relation = collection.schema[:fields][name] + next [] unless TO_ONE_RELATION_TYPES.include?(relation.type) + + foreign_collection = collection.datasource.get_collection(relation.foreign_collection) + path = prefix ? "#{prefix}:#{name}" : name + scope = get_scope(foreign_collection) + nested = collect_relation_scopes(foreign_collection, sub_projection, path) + + scope ? [Utils::RelationScopes::RelationScope.new(path, foreign_collection, scope), *nested] : nested + end + end + def cached(store, key, &fetch) @memo.fetch([store, key]) { @memo[[store, key]] = store.get_or_set(key, &fetch) } end diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/utils/relation_scopes.rb b/packages/forest_admin_agent/lib/forest_admin_agent/utils/relation_scopes.rb new file mode 100644 index 000000000..aceb905b2 --- /dev/null +++ b/packages/forest_admin_agent/lib/forest_admin_agent/utils/relation_scopes.rb @@ -0,0 +1,52 @@ +module ForestAdminAgent + module Utils + # The scope of a collection reached through a to-one relation empties the related record rather + # than filtering the root rows: a scope on `accounts` restricts which accounts are seen, never + # which `cards` exist. + class RelationScopes + RelationScope = Struct.new(:path, :collection, :scope) + + def initialize(scopes = []) + @scopes = scopes + end + + def with_scope_fields(projection) + scope_fields = @scopes.map { |relation| relation.scope.projection.nest(prefix: relation.path) } + + projection.union(*scope_fields) + end + + def redact(records, projection, timezone) + return records if @scopes.empty? + + out_of_scope_paths = records.map { |record| out_of_scope_paths(record, timezone) } + + projection.apply(records).each_with_index do |record, index| + out_of_scope_paths[index].each { |path| self.class.clear(record, path) } + end + end + + def self.clear(record, path) + relation, rest = path.split(':', 2) + + if rest.nil? + record[relation] = nil + elsif record[relation] + clear(record[relation], rest) + end + end + + private + + def out_of_scope_paths(record, timezone) + stringified = ForestAdminDatasourceToolkit::Utils::HashHelper.convert_keys(record, :to_s) + + @scopes.filter_map do |relation_scope| + related = ForestAdminDatasourceToolkit::Utils::Record.field_value(stringified, relation_scope.path) + + relation_scope.path if related && !relation_scope.scope.match(related, relation_scope.collection, timezone) + end + end + end + end +end diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/charts/charts_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/charts/charts_spec.rb index d4cc5d5b4..2ef2c66ff 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/charts/charts_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/charts/charts_spec.rb @@ -524,6 +524,57 @@ module Charts ) end + describe 'with scopes on the aggregated collections' do + let(:scopes) do + { + 'review' => ConditionTreeLeaf.new('author', Operators::EQUAL, 'Isaac Asimov'), + 'book_review' => ConditionTreeLeaf.new('review_id', Operators::EQUAL, 7) + } + end + + before do + allow(permissions).to receive(:get_scope) { |collection| scopes[collection.name] } + allow(@datasource.get_collection('book')).to receive(:datasource).and_return(@datasource) + end + + def aggregated_condition_tree(collection_name) + captured = nil + allow(@datasource.get_collection(collection_name)).to receive(:aggregate) do |_caller, filter, *| + captured = filter + [] + end + chart.handle_request(args) + + captured.condition_tree.to_h + end + + it 'applies the scope of the aggregated OneToMany collection' do + args[:params] = args[:params].merge({ labelFieldName: 'title', relationshipFieldName: 'bookReviews', + aggregator: 'Count', aggregateFieldName: 'id', + sourceCollectionName: 'book', type: 'Leaderboard' }) + + expect(aggregated_condition_tree('review')).to eq( + { field: 'author', operator: Operators::EQUAL, value: 'Isaac Asimov' } + ) + end + + it 'applies the scopes of the through and foreign collections of a ManyToMany' do + args[:params] = args[:params].merge({ labelFieldName: 'year', relationshipFieldName: 'reviews', + aggregator: 'Count', aggregateFieldName: 'id', + sourceCollectionName: 'book', type: 'Leaderboard' }) + + expect(aggregated_condition_tree('book_review')).to eq( + { + aggregator: 'And', + conditions: [ + { field: 'review_id', operator: Operators::EQUAL, value: 7 }, + { field: 'review:author', operator: Operators::EQUAL, value: 'Isaac Asimov' } + ] + } + ) + end + end + # A count names no path back to the collection it counts, so nothing above sees it. An empty # `aggregateFieldName` is a count too: `can_chart?` cannot tell it from an absent one. it 'asserts browse on the counted collection when a OneToMany count names no field' do diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/security/related_collection_scopes_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/security/related_collection_scopes_spec.rb new file mode 100644 index 000000000..2b4be0fc7 --- /dev/null +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/security/related_collection_scopes_spec.rb @@ -0,0 +1,161 @@ +require 'spec_helper' + +module ForestAdminAgent + module Services + include ForestAdminDatasourceToolkit + include ForestAdminDatasourceToolkit::Schema + include ForestAdminDatasourceToolkit::Components::Query + include ForestAdminDatasourceToolkit::Components::Query::ConditionTree + + describe Permissions do + include_context 'with caller' + + let(:datasource) { build_datasource_with_collections(collections) } + let(:cards) { datasource.get_collection('cards') } + + let(:collections) do + [ + build_collection( + name: 'cards', + schema: { + fields: { + 'id' => build_numeric_primary_key, + 'account_id' => build_column(column_type: 'Number'), + 'holder_id' => build_column(column_type: 'Number'), + 'account' => build_many_to_one(foreign_collection: 'accounts', foreign_key: 'account_id'), + 'holder' => build_many_to_one(foreign_collection: 'holders', foreign_key: 'holder_id') + } + } + ), + build_collection( + name: 'accounts', + schema: { + fields: { + 'id' => build_numeric_primary_key, + 'iban' => build_column(column_type: 'String'), + 'organization_id' => build_column(column_type: 'Number'), + 'organization' => build_many_to_one(foreign_collection: 'organizations', foreign_key: 'organization_id') + } + } + ), + build_collection( + name: 'organizations', + schema: { + fields: { + 'id' => build_numeric_primary_key, + 'name' => build_column(column_type: 'String'), + 'tier' => build_column(column_type: 'String') + } + } + ), + build_collection( + name: 'holders', + schema: { + fields: { + 'id' => build_numeric_primary_key, + 'national_id' => build_column(column_type: 'String'), + 'region' => build_column(column_type: 'String') + } + } + ) + ] + end + + let(:scopes) do + { + 'accounts' => Nodes::ConditionTreeLeaf.new('organization:name', Operators::EQUAL, 'OwnOrg'), + 'holders' => Nodes::ConditionTreeLeaf.new('region', Operators::EQUAL, 'EU') + } + end + + let(:permissions) do + permissions = described_class.new(caller) + allow(permissions).to receive(:get_scope) { |collection| scopes[collection.name] } + + permissions + end + + let(:cards_records) do + [ + { 'id' => 1, 'account' => { 'id' => 10, 'iban' => 'FR76', 'organization' => { 'name' => 'OwnOrg' } }, + 'holder' => { 'id' => 100, 'national_id' => 'EU-1', 'region' => 'EU' } }, + { 'id' => 2, 'account' => { 'id' => 20, 'iban' => 'DE22', 'organization' => { 'name' => 'AnotherTenant' } }, + 'holder' => { 'id' => 100, 'national_id' => 'EU-1', 'region' => 'EU' } }, + { 'id' => 3, 'account' => { 'id' => 30, 'iban' => 'FR14', 'organization' => { 'name' => 'OwnOrg' } }, + 'holder' => { 'id' => 300, 'national_id' => 'US-3', 'region' => 'US' } }, + { 'id' => 4, 'account' => nil, + 'holder' => { 'id' => 100, 'national_id' => 'EU-1', 'region' => 'EU' } } + ] + end + + let(:projection) { Projection.new(%w[id account:id account:iban holder:id holder:national_id]) } + + describe '#relation_scopes' do + it 'fetches the scope of every collection the projection reaches through a relation' do + fetched = [] + allow(permissions).to receive(:get_scope) { |collection| fetched << collection.name and nil } + + permissions.relation_scopes(cards, Projection.new(%w[id account:organization:name holder:national_id])) + + expect(fetched).to contain_exactly('accounts', 'organizations', 'holders') + end + + it 'fetches no scope when the projection reaches no relation' do + fetched = [] + allow(permissions).to receive(:get_scope) { |collection| fetched << collection.name and nil } + + permissions.relation_scopes(cards, Projection.new(%w[id])) + + expect(fetched).to be_empty + end + + it 'adds the fields each scope reads, nested under its relation' do + relation_scopes = permissions.relation_scopes(cards, projection) + + expect(relation_scopes.with_scope_fields(projection)).to eq( + %w[id account:id account:iban holder:id holder:national_id account:organization:name holder:region] + ) + end + + it 'keeps every card and empties the related records outside their scope' do + relation_scopes = permissions.relation_scopes(cards, projection) + + expect(relation_scopes.redact(cards_records, projection, 'Europe/Paris')).to eq( + [ + { 'id' => 1, 'account' => { 'id' => 10, 'iban' => 'FR76' }, + 'holder' => { 'id' => 100, 'national_id' => 'EU-1' } }, + { 'id' => 2, 'account' => nil, 'holder' => { 'id' => 100, 'national_id' => 'EU-1' } }, + { 'id' => 3, 'account' => { 'id' => 30, 'iban' => 'FR14' }, 'holder' => nil }, + { 'id' => 4, 'account' => nil, 'holder' => { 'id' => 100, 'national_id' => 'EU-1' } } + ] + ) + end + + it 'empties a record reached through several relations' do + scopes['organizations'] = Nodes::ConditionTreeLeaf.new('tier', Operators::EQUAL, 'gold') + scopes.delete('accounts') + deep_projection = Projection.new(%w[id account:organization:name]) + relation_scopes = permissions.relation_scopes(cards, deep_projection) + records = [ + { 'id' => 1, 'account' => { 'organization' => { 'name' => 'OwnOrg', 'tier' => 'gold' } } }, + { 'id' => 2, 'account' => { 'organization' => { 'name' => 'OwnOrg', 'tier' => 'silver' } } } + ] + + expect(relation_scopes.redact(records, deep_projection, 'Europe/Paris')).to eq( + [ + { 'id' => 1, 'account' => { 'organization' => { 'name' => 'OwnOrg' } } }, + { 'id' => 2, 'account' => { 'organization' => nil } } + ] + ) + end + + it 'returns the records untouched when no relation is scoped' do + scopes.clear + relation_scopes = permissions.relation_scopes(cards, projection) + + expect(relation_scopes.redact(cards_records, projection, 'Europe/Paris')).to be(cards_records) + end + end + end + end +end diff --git a/packages/forest_admin_agent/spec/spec_helper.rb b/packages/forest_admin_agent/spec/spec_helper.rb index c659ee7fe..8287b901a 100644 --- a/packages/forest_admin_agent/spec/spec_helper.rb +++ b/packages/forest_admin_agent/spec/spec_helper.rb @@ -61,6 +61,7 @@ read_guard_calls[:search_extended] << options[:search_extended] if options.key?(:search_extended) end allow(permissions).to receive(:assert_can_read_usages) + allow(permissions).to receive(:relation_scopes).and_return(ForestAdminAgent::Utils::RelationScopes.new) allow(permissions).to receive(:redact_projection) do |collection, projection, **options| read_guard_calls[:projections] << { collection: collection.name,