diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/routes/abstract_authenticated_route.rb b/packages/forest_admin_agent/lib/forest_admin_agent/routes/abstract_authenticated_route.rb index abdc1fc55..56447849c 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/routes/abstract_authenticated_route.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/routes/abstract_authenticated_route.rb @@ -12,6 +12,26 @@ def build(args = {}) context end + # Never refused: a write must not 403 because the row it just wrote carries a relation the + # caller cannot read. + def redacted_full_projection(context) + all = ForestAdminDatasourceToolkit::Components::Query::ProjectionFactory.all(context.collection) + + context.permissions.redact_projection(context.collection, all, named_by_caller: false) + end + + # +with_pks+ runs after the redaction on purpose. It only re-adds keys for relations the + # redaction kept a path through, and the serializer needs those keys to emit the readable + # column behind them — dropping them would take the permitted path down with them. A relation + # the redaction emptied contributes no path, so nothing is re-added for it. + def redacted_projection_with_pks(context, collection, args) + requested = Utils::QueryStringParser.parse_requested_projection(collection, args) + + context.permissions.redact_projection( + collection, requested[:projection], named_by_caller: requested[:named_by_caller] + ).with_pks(collection) + end + def format_attributes(args, collection) record = args[:params][:data][:attributes] || {} 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 ef3dfa9d4..d99cc2557 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 @@ -28,15 +28,14 @@ def setup_routes def handle_request(args = {}) context = build(args) context.permissions.can_chart?(args[:params]) + condition_tree = ForestAdminAgent::Utils::QueryStringParser.parse_condition_tree( + context.collection, args + ) + context.permissions.assert_can_read_query_fields(context.collection, condition_tree: condition_tree) type = validate_and_get_type(args[:params][:type]) filter = Filter.new( condition_tree: ConditionTreeFactory.intersect( - [ - context.permissions.get_scope(context.collection), - ForestAdminAgent::Utils::QueryStringParser.parse_condition_tree( - context.collection, args - ) - ] + [context.permissions.get_scope(context.collection), condition_tree] ) ) @@ -47,6 +46,15 @@ def handle_request(args = {}) private + # An empty `aggregateFieldName` is a count, and `can_chart?` cannot tell it from an absent + # one — `sanitize_chart_parameters` drops both before hashing. Normalising here keeps the + # aggregation and the guards that read it from disagreeing on what a count is. + def aggregate_field_name(args) + field = args[:params][:aggregateFieldName] + + field.nil? || field.to_s.empty? ? nil : field + end + def validate_and_get_type(type) chart_types = %w[Value Objective Pie Line Leaderboard] unless chart_types.include?(type) @@ -89,9 +97,13 @@ def make_objective(context, filter, args) def make_pie(context, filter, args) group_field = args[:params][:groupByFieldName] + assert_can_read_aggregated_fields( + context, context.collection, + [['group a chart by', group_field], ['aggregate a chart on', aggregate_field_name(args)]] + ) aggregation = Aggregation.new( operation: args[:params][:aggregator], - field: args[:params][:aggregateFieldName], + field: aggregate_field_name(args), groups: group_field ? [{ field: group_field }] : [] ) @@ -102,6 +114,11 @@ def make_pie(context, filter, args) def make_line(context, filter, args) group_by_field_name = args[:params][:groupByFieldName] + assert_can_read_aggregated_fields( + context, context.collection, + [['group a chart by', group_by_field_name], + ['aggregate a chart on', aggregate_field_name(args)]] + ) time_range = args[:params][:timeRange] filter_only_with_values = filter.override( condition_tree: ConditionTree::ConditionTreeFactory.intersect( @@ -116,7 +133,7 @@ def make_line(context, filter, args) filter_only_with_values, Aggregation.new( operation: args[:params][:aggregator], - field: args[:params][:aggregateFieldName], + field: aggregate_field_name(args), groups: [{ field: group_by_field_name, operation: time_range }] ) ) @@ -151,7 +168,7 @@ def make_leaderboard(context, filter, args) leaderboard_filter = filter.nest(inverse) aggregation = Aggregation.new( operation: args[:params][:aggregator], - field: args[:params][:aggregateFieldName], + field: aggregate_field_name(args), groups: [{ field: "#{inverse}:#{args[:params][:labelFieldName]}" }] ) end @@ -171,13 +188,25 @@ def make_leaderboard(context, filter, args) leaderboard_filter = filter.nest(origin) aggregation = Aggregation.new( operation: args[:params][:aggregator], - field: args[:params][:aggregateFieldName] ? "#{target}:#{args[:params][:aggregateFieldName]}" : nil, + field: aggregate_field_name(args) ? "#{target}:#{aggregate_field_name(args)}" : nil, groups: [{ field: "#{origin}:#{args[:params][:labelFieldName]}" }] ) end end if collection && leaderboard_filter && aggregation + assert_can_read_aggregated_fields( + context, context.datasource.get_collection(collection), + [['group a leaderboard by', aggregation.groups[0][:field]], + ['aggregate a leaderboard on', aggregation.field]] + ) + + # A count exposes the cardinality of the relation, which `/relationships//count` + # puts behind `browse`. No path names it, so nothing above sees it. + if aggregation.field.nil? + context.permissions.can?(:browse, context.datasource.get_collection(field.foreign_collection)) + end + rows = context.datasource.get_collection(collection).aggregate( context.caller, leaderboard_filter, @@ -200,12 +229,33 @@ def make_leaderboard(context, filter, args) end def compute_value(context, filter, args) + assert_can_read_aggregated_fields( + context, context.collection, + [['aggregate a chart on', aggregate_field_name(args)]] + ) aggregation = Aggregation.new(operation: args[:params][:aggregator], - field: args[:params][:aggregateFieldName]) + field: aggregate_field_name(args)) result = context.collection.aggregate(context.caller, filter, aggregation) result[0]['value'] || 0 end + + # The permission root stays the chart's own collection, which the leaderboard call site does + # not share with the collection its paths resolve against. + def assert_can_read_aggregated_fields(context, path_collection, fields) + usages = fields.reject { |_action, path| path.nil? || path.to_s.empty? } + .map do |action, path| + { + action: action, + path: path, + collections: ForestAdminDatasourceToolkit::Utils::FieldPath.leaf_collection_names( + path_collection, path + ) + } + end + + context.permissions.assert_can_read_usages(context.collection.name, usages) + end end end end diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/count.rb b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/count.rb index 1dd8e2ece..85ecee003 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/count.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/count.rb @@ -19,16 +19,26 @@ def handle_request(args = {}) context.permissions.can?(:browse, context.collection) if context.collection.is_countable? + condition_tree = ForestAdminAgent::Utils::QueryStringParser.parse_condition_tree( + context.collection, args + ) + search = QueryStringParser.parse_search(context.collection, args) + search_extended = QueryStringParser.parse_search_extended(args) + context.permissions.assert_can_read_query_fields( + context.collection, + condition_tree: condition_tree, search: search, search_extended: search_extended + ) + filter = ForestAdminDatasourceToolkit::Components::Query::Filter.new( condition_tree: ConditionTreeFactory.intersect( [ context.permissions.get_scope(context.collection), parse_query_segment(context.collection, args, context.permissions, context.caller), - ForestAdminAgent::Utils::QueryStringParser.parse_condition_tree(context.collection, args) + condition_tree ] ), - search: QueryStringParser.parse_search(context.collection, args), - search_extended: QueryStringParser.parse_search_extended(args), + search: search, + search_extended: search_extended, segment: QueryStringParser.parse_segment(context.collection, args) ) aggregation = ForestAdminDatasourceToolkit::Components::Query::Aggregation.new(operation: 'Count') 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 9c028f78a..dd45c45fa 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 @@ -22,25 +22,38 @@ def handle_request(args = {}) context = build(args) context.permissions.can?(:browse, context.collection) context.permissions.can?(:export, context.collection) + + condition_tree = QueryStringParser.parse_condition_tree(context.collection, args) + search = QueryStringParser.parse_search(context.collection, args) + search_extended = QueryStringParser.parse_search_extended(args) + sort = QueryStringParser.parse_sort(context.collection, args) + context.permissions.assert_can_read_query_fields( + context.collection, + condition_tree: condition_tree, sort: sort, search: search, search_extended: search_extended + ) + filter = ForestAdminDatasourceToolkit::Components::Query::Filter.new( condition_tree: ConditionTreeFactory.intersect( [ context.permissions.get_scope(context.collection), parse_query_segment(context.collection, args, context.permissions, context.caller), - QueryStringParser.parse_condition_tree( - context.collection, args - ) + condition_tree ] ), - search: QueryStringParser.parse_search(context.collection, args), - search_extended: QueryStringParser.parse_search_extended(args), - sort: QueryStringParser.parse_sort(context.collection, args), + search: search, + search_extended: search_extended, + sort: sort, segment: QueryStringParser.parse_segment(context.collection, args) ) - projection = QueryStringParser.parse_projection_from_request(context.collection, args) + requested = QueryStringParser.parse_requested_projection(context.collection, args) + projection = context.permissions.redact_projection( + context.collection, + requested[:projection], + named_by_caller: requested[:named_by_caller] + ) filename = args[:params][:filename] || args[:params]['collection_name'] filename += '.csv' unless /\.csv$/i.match?(filename) - header = args[:params][:header] + header = Utils::CsvGenerator.filter_header(args[:params][:header], requested[:projection], projection) # Generate timestamp for filename now = Time.now.strftime('%Y%m%d_%H%M%S') 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 3a0ab05d0..57f99132a 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 @@ -18,22 +18,31 @@ def handle_request(args = {}) context = build(args) context.permissions.can?(:browse, context.collection) + condition_tree = QueryStringParser.parse_condition_tree(context.collection, args) + search = QueryStringParser.parse_search(context.collection, args) + search_extended = QueryStringParser.parse_search_extended(args) + sort = QueryStringParser.parse_sort(context.collection, args) + context.permissions.assert_can_read_query_fields( + context.collection, + condition_tree: condition_tree, sort: sort, search: search, search_extended: search_extended + ) + filter = ForestAdminDatasourceToolkit::Components::Query::Filter.new( condition_tree: ConditionTreeFactory.intersect( [ context.permissions.get_scope(context.collection), - QueryStringParser.parse_condition_tree(context.collection, args), + condition_tree, parse_query_segment(context.collection, args, context.permissions, context.caller) ] ), page: QueryStringParser.parse_pagination(args), - search: QueryStringParser.parse_search(context.collection, args), - search_extended: QueryStringParser.parse_search_extended(args), - sort: QueryStringParser.parse_sort(context.collection, args), + search: search, + search_extended: search_extended, + sort: sort, segment: QueryStringParser.parse_segment(context.collection, args) ) - projection = QueryStringParser.parse_projection_with_pks(context.collection, args) + projection = redacted_projection_with_pks(context, context.collection, args) records = context.collection.list(context.caller, filter, projection) { 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 f653a5bf6..294df1d89 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 @@ -23,18 +23,26 @@ def handle_request(args = {}) context = build(args) context.permissions.can?(:browse, context.child_collection) context.permissions.can?(:export, context.child_collection) + condition_tree = ForestAdminAgent::Utils::QueryStringParser.parse_condition_tree( + context.child_collection, args + ) + context.permissions.assert_can_read_query_fields( + context.child_collection, condition_tree: condition_tree + ) filter = ForestAdminDatasourceToolkit::Components::Query::Filter.new( condition_tree: ConditionTreeFactory.intersect( - [ - context.permissions.get_scope(context.child_collection), - ForestAdminAgent::Utils::QueryStringParser.parse_condition_tree(context.child_collection, args) - ] + [context.permissions.get_scope(context.child_collection), condition_tree] ) ) - projection = ForestAdminAgent::Utils::QueryStringParser.parse_projection_from_request( + requested = ForestAdminAgent::Utils::QueryStringParser.parse_requested_projection( context.child_collection, args ) + projection = context.permissions.redact_projection( + context.child_collection, + requested[:projection], + named_by_caller: requested[:named_by_caller] + ) # Get the parent record primary keys primary_key_values = Utils::Id.unpack_id(context.collection, args[:params]['id'], with_key: true) @@ -43,7 +51,9 @@ def handle_request(args = {}) # Generate timestamp for filename now = Time.now.strftime('%Y%m%d_%H%M%S') collection_name = args.dig(:params, 'collection_name') - header = args.dig(:params, 'header') + header = ForestAdminAgent::Utils::CsvGenerator.filter_header( + args.dig(:params, 'header'), requested[:projection], projection + ) filename_with_timestamp = "#{collection_name}_#{relation_name}_export_#{now}.csv" # Create a callable to fetch related records 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 58479d308..5b906a308 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 @@ -23,19 +23,22 @@ def setup_routes def handle_request(args = {}) context = build(args) context.permissions.can?(:browse, context.child_collection) + condition_tree = ForestAdminAgent::Utils::QueryStringParser.parse_condition_tree( + context.child_collection, args + ) + sort = ForestAdminAgent::Utils::QueryStringParser.parse_sort(context.child_collection, args) + context.permissions.assert_can_read_query_fields( + context.child_collection, condition_tree: condition_tree, sort: sort + ) filter = ForestAdminDatasourceToolkit::Components::Query::Filter.new( condition_tree: ConditionTreeFactory.intersect( - [ - context.permissions.get_scope(context.child_collection), - ForestAdminAgent::Utils::QueryStringParser.parse_condition_tree(context.child_collection, args) - ] + [context.permissions.get_scope(context.child_collection), condition_tree] ), page: ForestAdminAgent::Utils::QueryStringParser.parse_pagination(args), - sort: ForestAdminAgent::Utils::QueryStringParser.parse_sort(context.child_collection, args) + sort: sort ) - projection = ForestAdminAgent::Utils::QueryStringParser.parse_projection_with_pks(context.child_collection, - 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, 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 dc3856610..00d342cd2 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 @@ -25,7 +25,7 @@ def handle_request(args = {}) condition_tree: ConditionTree::ConditionTreeFactory.intersect([condition_tree, scope]) ) - projection = QueryStringParser.parse_projection_with_pks(context.collection, args) + projection = redacted_projection_with_pks(context, context.collection, args) records = context.collection.list(context.caller, filter, projection) diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/store.rb b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/store.rb index 90dd1a21c..1984a82a1 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/store.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/store.rb @@ -25,7 +25,8 @@ def handle_request(args = {}) filter = ForestAdminDatasourceToolkit::Components::Query::Filter.new( condition_tree: ConditionTree::ConditionTreeFactory.match_ids(context.collection, [id]) ) - records = context.collection.list(context.caller, filter, ProjectionFactory.all(context.collection)) + projection = redacted_full_projection(context) + records = context.collection.list(context.caller, filter, projection) { name: args[:params]['collection_name'], diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/update.rb b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/update.rb index 507b0f23e..7b046f685 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/update.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/update.rb @@ -26,7 +26,8 @@ def handle_request(args = {}) drop_relationships!(args) data = format_attributes(args, context.collection) context.collection.update(context.caller, filter, data) - records = context.collection.list(context.caller, filter, ProjectionFactory.all(context.collection)) + projection = redacted_full_projection(context) + records = context.collection.list(context.caller, filter, projection) { name: args[:params]['collection_name'], diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/update_field.rb b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/update_field.rb index 443602a4d..1cbaa874f 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/update_field.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/update_field.rb @@ -51,7 +51,8 @@ def handle_request(args = {}) ) context.collection.update(context.caller, filter, { field_name => updated_array }) - records = context.collection.list(context.caller, filter, ProjectionFactory.all(context.collection)) + projection = redacted_full_projection(context) + records = context.collection.list(context.caller, filter, projection) { name: args[:params]['collection_name'], @@ -93,7 +94,8 @@ def fetch_record(primary_key_values, context) filter = ForestAdminDatasourceToolkit::Components::Query::Filter.new( condition_tree: ConditionTree::ConditionTreeFactory.intersect([condition_tree, scope]) ) - records = context.collection.list(context.caller, filter, ProjectionFactory.all(context.collection)) + projection = redacted_full_projection(context) + records = context.collection.list(context.caller, filter, projection) raise Http::Exceptions::NotFoundError, 'Record not found' unless records&.any? 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 ca591cfb5..4ce0306a9 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 @@ -53,6 +53,83 @@ def can?(action, collection, allow_fetch: false) is_allowed end + # +root_collection_name+ is pinned to readable and never looked up: +browse+ already gates a + # listing, +read+ a get, and the signed hash a chart. + # + # Answered once per collection for the whole request: +redact_projection+ and + # +assert_can_read_query_fields+ both ask on a listing, and a chart asks three times. + def read_permissions(root_collection_name, collection_names) + to_check = collection_names.uniq.reject { |name| name == root_collection_name } + allowed = { root_collection_name => true } + + return allowed if to_check.empty? + + # 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. + return allowed.merge(to_check.to_h { |name| [name, true] }) unless permission_system? + + @read_permissions ||= {} + missing = to_check - @read_permissions.keys + @read_permissions.merge!(fetch_read_permissions(missing)) unless missing.empty? + + allowed.merge(@read_permissions.slice(*to_check)) + end + + # An unnamed field is dropped rather than refused: the default expansion covers every column + # of every to-one relation, so refusing would turn an ordinary listing into a 403 for a caller + # that asked for nothing. + def redact_projection(collection, projection, named_by_caller:) + owners = projection.to_h do |path| + [path, ForestAdminDatasourceToolkit::Utils::FieldPath.leaf_collection_names(collection, path)] + end + allowed = read_permissions(collection.name, owners.values.flatten) + readable = ->(path) { readable_leaves?(owners[path], allowed) } + + if named_by_caller + denied = projection.reject { |path| readable.call(path) } + + unless denied.empty? + fields = denied.map { |path| "'#{path}' from #{leaf_label(owners[path])}" } + raise ForbiddenError, "You are not allowed to read #{fields.join(", ")}." + end + end + + ForestAdminDatasourceToolkit::Components::Query::Projection.new(projection.select { |path| readable.call(path) }) + end + + # Refused rather than redacted: dropping a condition widens the result set and dropping a sort + # clause silently reorders it, while both leak the value they touch anyway — a `starts_with` + # filter answers one guess per request without returning a column of its own. + # + # The route passes the components it will actually apply, already parsed. A component it drops + # is simply one it does not pass — a count carries no sort, a chart neither sort nor search — so + # nothing has to be declared alongside the query and then kept in step with it. What is + # authorised here is what the filter carries, not a second parse of the same parameters. + def assert_can_read_query_fields(collection, condition_tree: nil, sort: nil, search: nil, + search_extended: false) + usages = [] + + # `projection` collects the leaf fields without touching the tree. The route applies this very + # instance next, so a traversal that rebuilt a branch — as `for_each_leaf` does — would leave + # the guard deciding what runs. + condition_tree&.projection&.each { |path| usages << usage('filter on', collection, path) } + sort&.each { |clause| usages << usage('sort on', collection, clause[:field]) } + collect_search_usages(collection, search, search_extended, usages) + + assert_can_read_usages(collection.name, usages) + end + + def assert_can_read_usages(root_collection_name, usages) + allowed = read_permissions(root_collection_name, usages.flat_map { |usage| usage[:collections] }) + denied = usages.find { |usage| !readable_leaves?(usage[:collections], allowed) } + + return unless denied + + raise ForbiddenError, + "You cannot #{denied[:action]} '#{denied[:path]}': you are not allowed to read " \ + "#{leaf_label(denied[:collections])}." + end + def can_chart?(parameters) attributes = sanitize_chart_parameters(parameters.deep_symbolize_keys) hash_request = "#{attributes[:type]}:#{array_hash(attributes)}" @@ -188,6 +265,103 @@ def get_team(rendering_id) private + # An empty list of leaves resolves to no collection at all — a polymorphic relation declaring + # no `foreign_collections`. `[].all?` would allow it unconditionally, which is the one answer + # this guard must never give by default, so it counts as denied. + def readable_leaves?(names, allowed) + names.any? && names.all? { |name| allowed[name] } + end + + def leaf_label(names) + names.empty? ? 'an unresolved polymorphic relation' : "the '#{names.join("' or '")}' collection" + end + + def fetch_read_permissions(names) + user_data = get_user_data(caller.id) + collections_data = get_collections_permissions_data + results = names.to_h { |name| [name, read_allowed?(collections_data, name, user_data)] } + + return results if results.values.all? || !refetch_denied_reads?(names, collections_data) + + @read_permissions_refetched = true + refetched = get_collections_permissions_data(force_fetch: true) + + names.to_h { |name| [name, read_allowed?(refetched, name, user_data)] } + end + + # `can?` refetches the whole environment on every denial. Denial is the steady state of this + # check rather than the exception, so paying that per denial would cost a permission fetch, and + # a cache eviction every other in-flight request reads through, on each page load. + # + # Two things make a denial worth one fetch. Without `instant_cache_refresh` nothing else keeps + # the cache fresh — this is the gate node puts its own refetch behind — so a role granted `read` + # would otherwise stay redacted until the cache expires. With the channel up, `refresh-roles` + # evicts on a role change, and the only denial staleness still explains is a collection the + # payload has never heard of. + def refetch_denied_reads?(names, collections_data) + return false if @read_permissions_refetched + + !instant_cache_refresh? || names.any? { |name| !collections_data.key?(name.to_sym) } + end + + def instant_cache_refresh? + Facades::Container.config_from_cache[:instant_cache_refresh] == true + end + + def read_allowed?(collections_data, collection_name, user_data) + return false unless user_data_valid?(user_data) + + collection_key = collection_name.to_sym + return false unless collection_exists?(collections_data, collection_key, collection_name, user_data) + + role_ids = get_role_ids_for_action(collections_data, collection_key, :read, collection_name, user_data) + return false unless role_ids + + check_user_permission(role_ids, user_data, :read, collection_name) + end + + def usage(action, collection, path) + { + action: action, + path: path, + collections: ForestAdminDatasourceToolkit::Utils::FieldPath.leaf_collection_names(collection, path) + } + end + + def collect_search_usages(collection, search, search_extended, usages) + return if search.nil? || !collection.respond_to?(:searched_fields) + + searched = collection.searched_fields(search, search_extended) + + return if searched.nil? + + published = collection.datasource.collections + + searched.each do |field| + assert_search_target_exposed(field, published) + usages << { action: 'search on', path: field[:path], collections: field[:collections] } + end + end + + # `searched_fields` answers below the publication layer — deliberately, so a field hidden by + # renaming above it is still checked — so it can name a collection `remove_collection` took out + # of the API. An extended search does reach through to it: the condition is built below + # publication, which has already passed the filter down and cannot strip it. + # + # That collection is absent from the permission payload too, so asking `read_allowed?` answers + # "denied" for every role, admins included, and no grant can lift it. So it is refused as what + # it is — a column the agent does not expose — which points at `disable_search` or publishing + # the collection again rather than at a permission to grant. + def assert_search_target_exposed(field, published) + unexposed = field[:collections].reject { |name| published.key?(name) } + + return if unexposed.empty? + + raise ForbiddenError, + "You cannot search on '#{field[:path]}': the '#{unexposed.join("' or '")}' collection " \ + 'is not exposed by this agent.' + end + def permission_allowed?(collections_data, collection, action, user_data) return false unless user_data_valid?(user_data) diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/utils/csv_generator.rb b/packages/forest_admin_agent/lib/forest_admin_agent/utils/csv_generator.rb index e351f89f6..fcbe12bf5 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/utils/csv_generator.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/utils/csv_generator.rb @@ -3,6 +3,46 @@ module ForestAdminAgent module Utils class CsvGenerator + # Labels are positionally aligned with the requested projection, so dropping a field without + # dropping its label shifts every later value under the wrong heading. + # The header carries one label per exported column, so a label survives exactly when the + # projection path at its index did. + # + # A count that does not match the requested projection cannot be mapped onto columns — a to-one + # relation expanded into several paths, or a polymorphic type field appended — and there is then + # no header to hand back. Returning the original would leave `CsvGeneratorStream.parse_header` + # to check it against the *kept* count instead, so labels that happen to match the redacted + # column count would be accepted, printing each surviving column under the label of whatever + # preceded it. `nil` is what makes the stream fall back to the field paths. + # + # Handing it back untouched is only safe on the branch above, where nothing was redacted: there + # the two counts are the same number, so the stream's own check cannot reach a different answer. + def self.filter_header(header, requested, kept) + return header if header.nil? || kept.size == requested.size + + labels = parse_header_labels(header) + + return nil unless labels&.size == requested.size + + labels.each_with_index + .select { |_label, index| kept.include?(requested[index]) } + .map(&:first) + end + + # The front joins the labels with commas; a JSON array comes from older callers. + def self.parse_header_labels(header) + return header if header.is_a?(Array) + return nil unless header.is_a?(String) + + parsed = begin + JSON.parse(header) + rescue JSON::ParserError + nil + end + + parsed.is_a?(Array) ? parsed : header.split(',', -1) + end + def self.generate(records, projection) data = {} projection.each do |schema_field| diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/utils/csv_generator_stream.rb b/packages/forest_admin_agent/lib/forest_admin_agent/utils/csv_generator_stream.rb index bc01cbab2..b3431b3ba 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/utils/csv_generator_stream.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/utils/csv_generator_stream.rb @@ -16,8 +16,9 @@ class CsvGeneratorStream def self.stream(header, filter, projection, list_records, limit_export_size = nil) Enumerator.new do |yielder| # Yield header row first (client receives immediately) - header_array = parse_header(header, projection) - yielder << "#{header_array.join(",")}\n" + # Escaped like any other row: the labels are the caller's, and one carrying a comma or a + # quote would otherwise emit a header row wider than the data under it. + yielder << CSV.generate_line(parse_header(header, projection)) offset = 0 @@ -58,23 +59,14 @@ def self.stream(header, filter, projection, list_records, limit_export_size = ni end # Parse header parameter into an array - # @param header [String, Array, nil] Header as JSON string, array, or nil - # @param projection [Projection] Field projection (fallback if header is invalid) - # @return [Array] Header fields + # @param header [String, Array, nil] Header as a comma-joined string, JSON array, array or nil + # @param projection [Projection] Field projection + # @return [Array] The labels when there is exactly one per exported column, the field + # paths otherwise — a header that cannot be mapped onto the columns would mislabel them def self.parse_header(header, projection) - case header - when Array - header - when String - return projection.to_a if header.empty? + labels = CsvGenerator.parse_header_labels(header) - JSON.parse(header) - else - projection.to_a - end - rescue JSON::ParserError - # Fallback to projection if JSON parsing fails - projection.to_a + labels&.size == projection.size ? labels : projection.to_a end # Generate CSV row from record data diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/utils/query_string_parser.rb b/packages/forest_admin_agent/lib/forest_admin_agent/utils/query_string_parser.rb index 97c584165..07301f4b0 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/utils/query_string_parser.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/utils/query_string_parser.rb @@ -66,6 +66,22 @@ def self.parse_projection_from_request(collection, args) parse_projection_from_header(collection, args) || parse_projection(collection, args) end + # Both halves read the same `fields` param the same way on purpose: an empty + # `fields[]=` means "every column", so it is not named and must take the redaction + # path rather than a 403. + def self.parse_requested_projection(collection, args) + from_header = parse_projection_from_header(collection, args) + + return { projection: from_header, named_by_caller: true } if from_header + + fields = args.dig(:params, :fields, collection.name) + + { + projection: parse_projection(collection, args), + named_by_caller: !(fields.nil? || fields == '') + } + end + def self.add_polymorphic_type_fields(collection, requested_field_names) polymorphic_relations = collection.schema[:fields].select { |_, field| field.type == 'PolymorphicManyToOne' } 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 24c2165f4..8cc33f867 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 @@ -15,6 +15,7 @@ module Charts describe Charts do include_context 'with caller' + include_context 'with readable related collections' subject(:chart) { described_class.new } let(:args) do { @@ -129,6 +130,23 @@ module Charts expect(chart.routes.length).to eq 1 end + # A chart applies neither sort nor search, so checking either would refuse a request the + # denied field cannot reach. + it 'checks only the filter, against its own collection' do + args[:params] = args[:params].merge({ + aggregateFieldName: 'price', + aggregator: 'Sum', + sourceCollectionName: 'book', + type: 'Value', + timezone: 'Europe/Paris' + }) + allow(@datasource.get_collection('book')).to receive(:aggregate).and_return([{ 'value' => 10, 'group' => [] }]) + + chart.handle_request(args) + + expect(read_guard_calls[:query_fields]).to eq([{ collection: 'book', applies: %i[filter] }]) + end + it 'throw an error when request has a bad chart type' do args[:params][:type] = 'unknown_type' @@ -488,6 +506,73 @@ module Charts ) 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 + args[:params] = args[:params].merge({ + labelFieldName: 'author', + relationshipFieldName: 'bookReviews', + aggregator: 'Count', + aggregateFieldName: '', + sourceCollectionName: 'book', + type: 'Leaderboard', + timezone: 'Europe/Paris' + }) + allow(permissions).to receive(:can?) + allow(@datasource.get_collection('book')).to receive(:datasource).and_return(@datasource) + allow(@datasource.get_collection('review')).to receive(:aggregate).and_return([]) + + chart.handle_request(args) + + expect(permissions).to have_received(:can?) do |action, collection| + expect(action).to eq(:browse) + expect(collection.name).to eq('review') + end + end + + it 'asserts browse on the foreign collection when a ManyToMany count names no field' do + args[:params] = args[:params].merge({ + labelFieldName: 'year', + relationshipFieldName: 'reviews', + aggregator: 'Count', + sourceCollectionName: 'book', + type: 'Leaderboard', + timezone: 'Europe/Paris' + }) + allow(permissions).to receive(:can?) + allow(@datasource.get_collection('book')).to receive(:datasource).and_return(@datasource) + allow(@datasource.get_collection('book_review')).to receive(:aggregate).and_return([]) + + chart.handle_request(args) + + expect(permissions).to have_received(:can?) do |action, collection| + expect(action).to eq(:browse) + expect(collection.name).to eq('review') + end + end + + it 'refuses a count leaderboard the caller cannot browse the counted collection for' do + args[:params] = args[:params].merge({ + labelFieldName: 'author', + relationshipFieldName: 'bookReviews', + aggregator: 'Count', + aggregateFieldName: '', + sourceCollectionName: 'book', + type: 'Leaderboard', + timezone: 'Europe/Paris' + }) + allow(permissions).to receive(:can?).and_raise( + ForestAdminAgent::Http::Exceptions::ForbiddenError.new( + "You don't have permission to browse this collection." + ) + ) + allow(@datasource.get_collection('book')).to receive(:datasource).and_return(@datasource) + + expect { chart.handle_request(args) }.to raise_error( + ForestAdminAgent::Http::Exceptions::ForbiddenError + ) + end + it 'throw a ForestException when the request is not filled correctly' do args[:params] = args[:params].merge({ relationshipFieldName: 'unknown_relation', diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/count_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/count_spec.rb index 7711a53fd..89d1bb663 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/count_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/count_spec.rb @@ -11,6 +11,7 @@ module Resources describe Count do include_context 'with caller' + include_context 'with readable related collections' subject(:count) { described_class.new } let(:args) do { @@ -48,6 +49,15 @@ module Resources expect(count.routes.length).to eq 1 end + # A count applies no sort, so checking one would refuse a request the denied field cannot + # reach — while its filter and search are the sharper half and stay checked. + it 'checks only the query components it applies, against its own collection' do + ForestAdminAgent::Facades::Container.datasource.get_collection('user').enable_count + count.handle_request(args) + + expect(read_guard_calls[:query_fields]).to eq([{ collection: 'user', applies: %i[filter search] }]) + end + context 'when collection is countable' do it 'return an serialized content' do ForestAdminAgent::Facades::Container.datasource.get_collection('user').enable_count diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/csv_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/csv_spec.rb index 58390e30c..b1e0b0fc6 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/csv_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/csv_spec.rb @@ -13,6 +13,7 @@ module Resources describe Csv do include_context 'with caller' + include_context 'with readable related collections' subject(:csv) { described_class.new } let(:args) do { @@ -60,6 +61,16 @@ module Resources expect(csv.routes.length).to eq 1 end + it 'checks every query component it applies, against its own collection' do + allow(csv_generator_stream).to receive(:stream).and_return([].to_enum) + csv.handle_request(args) + + expect(read_guard_calls[:query_fields]).to eq( + [{ collection: 'user', applies: %i[filter sort search] }] + ) + expect(read_guard_calls[:projections]).to eq([{ collection: 'user', named_by_caller: false }]) + end + context 'when call csv' do it 'returns a streaming export csv' do # Create a mock enumerator that yields CSV data diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/list_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/list_spec.rb index 81dcb1b55..13d0529eb 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/list_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/list_spec.rb @@ -14,6 +14,7 @@ module Resources describe List do include_context 'with caller' + include_context 'with readable related collections' subject(:list) { described_class.new } let(:args) do { @@ -74,6 +75,40 @@ module Resources expect(list.routes.length).to eq 1 end + it 'checks every query component it applies, against its own collection' do + list.handle_request(args) + + expect(read_guard_calls[:query_fields]).to eq( + [{ collection: 'user', applies: %i[filter sort search] }] + ) + end + + it 'refuses a projection the caller named on its own collection' do + args[:params][:fields] = { 'user' => 'id,first_name' } + list.handle_request(args) + + expect(read_guard_calls[:projections]).to eq([{ collection: 'user', named_by_caller: true }]) + end + + it 'redacts rather than refuses the expansion the caller never asked for' do + list.handle_request(args) + + expect(read_guard_calls[:projections]).to eq([{ collection: 'user', named_by_caller: false }]) + end + + it 'answers a denied query field with a 403 rather than a listing' do + allow(permissions).to receive(:assert_can_read_query_fields).and_raise( + ForestAdminAgent::Http::Exceptions::ForbiddenError.new( + "You cannot filter on 'category:label': you are not allowed to read the 'category' collection." + ) + ) + + expect { list.handle_request(args) }.to raise_error( + ForestAdminAgent::Http::Exceptions::ForbiddenError, + /You cannot filter on 'category:label'/ + ) + end + it 'return an serialized content' do result = list.handle_request(args) @@ -168,6 +203,7 @@ module Resources describe List, 'with a projection deeper than one relation' do include_context 'with caller' + include_context 'with readable related collections' subject(:list) { described_class.new } let(:permissions) { instance_double(ForestAdminAgent::Services::Permissions) } diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/related/csv_related_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/related/csv_related_spec.rb index dc160d9f7..5e67841ed 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/related/csv_related_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/related/csv_related_spec.rb @@ -14,6 +14,7 @@ module Related describe CsvRelated do include_context 'with caller' + include_context 'with readable related collections' subject(:csv) { described_class.new } let(:args) do { @@ -79,6 +80,17 @@ module Related expect(csv.routes.length).to eq 1 end + it 'checks the components it applies against the foreign collection' do + args[:params]['relation_name'] = 'category' + args[:params]['id'] = 1 + allow(ForestAdminDatasourceToolkit::Utils::Collection).to receive(:list_relation).and_return([]) + + csv.handle_request(args) + + expect(read_guard_calls[:query_fields]).to eq([{ collection: 'category', applies: %i[filter] }]) + expect(read_guard_calls[:projections]).to eq([{ collection: 'category', named_by_caller: false }]) + end + context 'when call csv' do it 'returns a streaming export csv of the related collection' do args[:params]['relation_name'] = 'category' diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/related/list_related_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/related/list_related_spec.rb index ab2d270ae..30288e707 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/related/list_related_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/related/list_related_spec.rb @@ -14,6 +14,7 @@ module Related describe ListRelated do include_context 'with caller' + include_context 'with readable related collections' subject(:list) { described_class.new } let(:args) do { @@ -79,6 +80,21 @@ module Related expect(list.routes.length).to eq 1 end + # The only routes resolving against the foreign collection rather than their own, and a + # related listing applies no search — so neither is checked against the parent. + it 'checks the components it applies against the foreign collection' do + args[:params]['relation_name'] = 'category' + args[:params]['id'] = 1 + allow(ForestAdminDatasourceToolkit::Utils::Collection).to receive(:list_relation).and_return([]) + + list.handle_request(args) + + expect(read_guard_calls[:query_fields]).to eq( + [{ collection: 'category', applies: %i[filter sort] }] + ) + expect(read_guard_calls[:projections]).to eq([{ collection: 'category', named_by_caller: false }]) + end + context 'when call without filters' do it 'call list_relation with expected args' do args[:params]['relation_name'] = 'category' @@ -192,6 +208,7 @@ module Related describe ListRelated, 'with a projection deeper than one relation' do include_context 'with caller' + include_context 'with readable related collections' subject(:list) { described_class.new } let(:permissions) { instance_double(ForestAdminAgent::Services::Permissions) } let(:args) do diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/show_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/show_spec.rb index cffabf9e2..8392e51b8 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/show_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/show_spec.rb @@ -11,6 +11,7 @@ module Resources describe Show do include_context 'with caller' + include_context 'with readable related collections' subject(:show) { described_class.new } let(:args) do { @@ -73,6 +74,13 @@ def respond_to?(arg) allow(@datasource.get_collection('user')).to receive(:list).and_return([User.new(1, 'foo', 'foo')]) end + it 'redacts the projection it serves against its own collection' do + args[:params]['id'] = 1 + show.handle_request(args) + + expect(read_guard_calls[:projections]).to eq([{ collection: 'user', named_by_caller: false }]) + end + it 'return an serialized content' do args[:params]['id'] = 1 result = show.handle_request(args) diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/store_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/store_spec.rb index 7038a8255..8bb9571d2 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/store_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/store_spec.rb @@ -11,6 +11,7 @@ module Resources describe Store do include_context 'with caller' + include_context 'with readable related collections' subject(:store) { described_class.new } let(:args) do { @@ -99,6 +100,9 @@ def respond_to?(arg) 'links' => { 'self' => '/forest/book/1' } } ) + # Redacted, never refused: a write must not 403 because the row it just wrote carries a + # relation the caller cannot read. Same helper serves `update` and `update_field`. + expect(read_guard_calls[:projections]).to eq([{ collection: 'book', named_by_caller: false }]) end it 'includes null attributes in the returned result' do diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/update_field_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/update_field_spec.rb index 1a226d49e..82866d247 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/update_field_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/update_field_spec.rb @@ -10,6 +10,7 @@ module Resources describe UpdateField do include_context 'with caller' + include_context 'with readable related collections' subject(:update_field) { described_class.new } let(:args) do diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/update_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/update_spec.rb index 47534d9a7..eb1a107a5 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/update_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/update_spec.rb @@ -11,6 +11,7 @@ module Resources describe Update do include_context 'with caller' + include_context 'with readable related collections' subject(:update) { described_class.new } let(:args) do { diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/security/related_read_permissions_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/security/related_read_permissions_spec.rb new file mode 100644 index 000000000..7c6b2f241 --- /dev/null +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/security/related_read_permissions_spec.rb @@ -0,0 +1,447 @@ +require 'spec_helper' + +# Drives a real Permissions service rather than a double, so the guards themselves are under test +# and not the stubs the route specs install. +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, + 'pan_last4' => build_column(column_type: 'String'), + 'account_id' => build_column(column_type: 'Number'), + 'holder_id' => build_column(column_type: 'Number'), + 'holder_type' => build_column(column_type: 'String'), + 'account' => build_many_to_one(foreign_collection: 'accounts', foreign_key: 'account_id'), + 'holder' => Relations::PolymorphicManyToOneSchema.new( + foreign_key: 'holder_id', + foreign_key_type_field: 'holder_type', + foreign_collections: %w[persons companies], + foreign_key_targets: { 'persons' => 'id', 'companies' => 'id' } + ) + } + } + ), + build_collection( + name: 'accounts', + schema: { + fields: { + 'id' => build_numeric_primary_key, + 'iban' => build_column(column_type: 'String', filter_operators: [Operators::EQUAL]), + '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', filter_operators: [Operators::EQUAL]) } } + ), + build_collection( + name: 'persons', + schema: { fields: { 'id' => build_numeric_primary_key, 'national_id' => build_column(column_type: 'String') } } + ), + build_collection( + name: 'companies', + schema: { fields: { 'id' => build_numeric_primary_key, 'siret' => build_column(column_type: 'String') } } + ) + ] + end + + # `cards` is always readable: the route asserts browse or read on it before any of this runs. + def build_permissions(readable) + permissions = described_class.new(caller) + allow(permissions).to receive_messages( + permission_system?: true, + get_user_data: { id: 1, roleId: 7 }, + get_collections_permissions_data: (%w[cards] + readable).to_h { |name| [name.to_sym, { read: [7] }] } + .merge( + (%w[accounts organizations persons companies] - readable) + .to_h { |name| [name.to_sym, { read: [] }] } + ) + ) + + permissions + end + + describe 'without a permission system' do + it 'keeps every path' do + permissions = described_class.new(caller) + allow(permissions).to receive(:permission_system?).and_return(false) + + projection = permissions.redact_projection( + cards, Projection.new(%w[id account:iban holder:*]), named_by_caller: true + ) + + expect(projection).to eq(%w[id account:iban holder:*]) + end + + it 'refuses no filter' do + permissions = described_class.new(caller) + allow(permissions).to receive(:permission_system?).and_return(false) + condition_tree = Nodes::ConditionTreeLeaf.new('account:iban', Operators::EQUAL, 'FR76') + + expect { permissions.assert_can_read_query_fields(cards, condition_tree: condition_tree) } + .not_to raise_error + end + end + + describe '#redact_projection' do + it 'refuses a field the caller named on a collection it cannot read' do + permissions = build_permissions([]) + + expect do + permissions.redact_projection(cards, Projection.new(%w[id account:iban]), named_by_caller: true) + end.to raise_error( + ForestAdminAgent::Http::Exceptions::ForbiddenError, + "You are not allowed to read 'account:iban' from the 'accounts' collection." + ) + end + + it 'names every offending path in one message so a client retries once' do + permissions = build_permissions([]) + + expect do + permissions.redact_projection( + cards, Projection.new(%w[id account:iban account:organization:name]), named_by_caller: true + ) + end.to raise_error(/'account:iban'.+'accounts'.+'account:organization:name'.+'organizations'/) + end + + it 'drops the path instead when the caller never named it' do + permissions = build_permissions([]) + + projection = permissions.redact_projection( + cards, Projection.new(%w[id pan_last4 account:iban]), named_by_caller: false + ) + + expect(projection).to eq(%w[id pan_last4]) + end + + it 'traverses a collection it cannot read to reach a column it can' do + permissions = build_permissions(%w[organizations]) + + projection = permissions.redact_projection( + cards, Projection.new(%w[id account:organization:name]), named_by_caller: true + ) + + expect(projection).to eq(%w[id account:organization:name]) + end + + it 'keeps a polymorphic relation whose every target is readable' do + permissions = build_permissions(%w[persons companies]) + + projection = permissions.redact_projection(cards, Projection.new(%w[id holder:*]), named_by_caller: false) + + expect(projection).to eq(%w[id holder:*]) + end + + it 'refuses a polymorphic relation when a single target is denied' do + permissions = build_permissions(%w[persons]) + + expect do + permissions.redact_projection(cards, Projection.new(%w[id holder:*]), named_by_caller: true) + end.to raise_error( + ForestAdminAgent::Http::Exceptions::ForbiddenError, + "You are not allowed to read 'holder:*' from the 'persons' or 'companies' collection." + ) + end + + it 'leaves nothing for `with_pks` to re-add once the redaction emptied a relation' do + permissions = build_permissions([]) + + projection = permissions.redact_projection( + cards, Projection.new(%w[id account:iban]), named_by_caller: false + ).with_pks(cards) + + expect(projection).to eq(%w[id]) + end + + # The key of a collection the caller cannot read stays, because the serializer needs it to + # emit the readable column behind it — dropping it would take the permitted path down too. + it 'keeps the key `with_pks` needs to serialize a path the redaction kept' do + permissions = build_permissions(%w[organizations]) + + projection = permissions.redact_projection( + cards, Projection.new(%w[id account:organization:name]), named_by_caller: true + ).with_pks(cards) + + expect(projection).to include('account:id', 'account:organization:id') + end + + context 'when a polymorphic relation declares no target at all' do + let(:orphan_cards) do + build_datasource_with_collections( + [ + build_collection( + name: 'cards', + schema: { + fields: { + 'id' => build_numeric_primary_key, + 'holder_id' => build_column(column_type: 'Number'), + 'holder_type' => build_column(column_type: 'String'), + 'holder' => Relations::PolymorphicManyToOneSchema.new( + foreign_key: 'holder_id', + foreign_key_type_field: 'holder_type', + foreign_collections: [], + foreign_key_targets: {} + ) + } + } + ) + ] + ).get_collection('cards') + end + + it 'refuses the path the caller named rather than allowing what resolves to nothing' do + permissions = build_permissions([]) + + expect do + permissions.redact_projection(orphan_cards, Projection.new(%w[id holder:*]), named_by_caller: true) + end.to raise_error( + ForestAdminAgent::Http::Exceptions::ForbiddenError, + "You are not allowed to read 'holder:*' from an unresolved polymorphic relation." + ) + end + + it 'drops the path from the default expansion' do + permissions = build_permissions([]) + + projection = permissions.redact_projection( + orphan_cards, Projection.new(%w[id holder:*]), named_by_caller: false + ) + + expect(projection).to eq(%w[id]) + end + end + + it 'drops a polymorphic relation with a denied target from the default expansion' do + permissions = build_permissions(%w[persons]) + + projection = permissions.redact_projection( + cards, ProjectionFactory.all(cards), named_by_caller: false + ) + + expect(projection).not_to include('holder:*') + expect(projection).to include('id', 'pan_last4', 'holder_type') + end + end + + describe '#assert_can_read_query_fields' do + def leaf(field) + Nodes::ConditionTreeLeaf.new(field, Operators::EQUAL, 'FR76') + end + + def searchable_cards(searched) + double = instance_double( + ForestAdminDatasourceToolkit::Decorators::CollectionDecorator, + name: 'cards', + datasource: datasource + ) + allow(double).to receive(:searched_fields).and_return(searched) + + double + end + + it 'refuses a filter on a collection the caller cannot read' do + permissions = build_permissions([]) + + expect { permissions.assert_can_read_query_fields(cards, condition_tree: leaf('account:iban')) } + .to raise_error( + ForestAdminAgent::Http::Exceptions::ForbiddenError, + "You cannot filter on 'account:iban': you are not allowed to read the 'accounts' collection." + ) + end + + it 'reaches every leaf of a branch, not only the first' do + permissions = build_permissions([]) + tree = Nodes::ConditionTreeBranch.new('And', [leaf('id'), leaf('account:iban')]) + + expect { permissions.assert_can_read_query_fields(cards, condition_tree: tree) } + .to raise_error(ForestAdminAgent::Http::Exceptions::ForbiddenError, /'account:iban'/) + end + + # The route applies this very instance next, so a traversal that rebuilt a branch — as + # `for_each_leaf` does — would leave the guard deciding what actually runs. + it 'leaves the tree the route is about to apply untouched' do + permissions = build_permissions(%w[accounts]) + tree = Nodes::ConditionTreeBranch.new('And', [leaf('account:iban'), leaf('id')]) + + permissions.assert_can_read_query_fields(cards, condition_tree: tree) + + expect(tree.conditions.map(&:field)).to eq(%w[account:iban id]) + end + + it 'refuses a sort on a collection the caller cannot read' do + permissions = build_permissions([]) + sort = [{ field: 'account:iban', ascending: false }] + + expect { permissions.assert_can_read_query_fields(cards, sort: sort) } + .to raise_error( + ForestAdminAgent::Http::Exceptions::ForbiddenError, + "You cannot sort on 'account:iban': you are not allowed to read the 'accounts' collection." + ) + end + + it 'refuses whatever the stack says the search will reach' do + permissions = build_permissions([]) + collection = searchable_cards([{ path: 'holder:national_id', collections: ['persons'] }]) + + expect { permissions.assert_can_read_query_fields(collection, search: 'martin') } + .to raise_error( + ForestAdminAgent::Http::Exceptions::ForbiddenError, + "You cannot search on 'holder:national_id': you are not allowed to read the 'persons' collection." + ) + end + + it 'accepts a search once every collection the stack names is readable' do + permissions = build_permissions(%w[persons]) + collection = searchable_cards([{ path: 'holder:national_id', collections: ['persons'] }]) + + expect { permissions.assert_can_read_query_fields(collection, search: 'martin') }.not_to raise_error + end + + it 'passes the extended flag on to the layer, since it decides what the search reaches' do + permissions = build_permissions(%w[persons]) + collection = searchable_cards([]) + + permissions.assert_can_read_query_fields(collection, search: 'martin', search_extended: true) + + expect(collection).to have_received(:searched_fields).with('martin', true) + end + + # An extended search reaches a removed collection: the condition is built below publication, + # which has already passed the filter down. Refused as unexposed rather than as denied — no + # role can be granted `read` on a collection absent from the permission payload. + it 'refuses a search reaching a collection the agent does not expose' do + permissions = build_permissions(%w[accounts]) + collection = searchable_cards([{ path: 'account:iban', collections: ['accounts'] }]) + published = datasource.collections.except('accounts') + allow(datasource).to receive(:collections).and_return(published) + + expect { permissions.assert_can_read_query_fields(collection, search: 'martin') } + .to raise_error( + ForestAdminAgent::Http::Exceptions::ForbiddenError, + "You cannot search on 'account:iban': the 'accounts' collection is not exposed by this agent." + ) + end + + it 'tells an unexposed collection apart from one the role cannot read' do + permissions = build_permissions([]) + collection = searchable_cards([{ path: 'account:iban', collections: ['accounts'] }]) + + expect { permissions.assert_can_read_query_fields(collection, search: 'martin') } + .to raise_error( + ForestAdminAgent::Http::Exceptions::ForbiddenError, + "You cannot search on 'account:iban': you are not allowed to read the 'accounts' collection." + ) + end + + # A replaced search: the handler picks the fields, the caller only supplies the text. + it 'serves the request when the stack cannot say what a search reaches' do + permissions = build_permissions([]) + + expect { permissions.assert_can_read_query_fields(searchable_cards(nil), search: 'martin') } + .not_to raise_error + end + + it 'checks nothing on a collection that cannot answer what a search reaches' do + permissions = build_permissions([]) + + expect { permissions.assert_can_read_query_fields(cards, search: 'martin') }.not_to raise_error + end + + it 'accepts a filter once the collection it reaches is readable' do + permissions = build_permissions(%w[accounts]) + + expect { permissions.assert_can_read_query_fields(cards, condition_tree: leaf('account:iban')) } + .not_to raise_error + end + + # Which components a route applies is now expressed by what it passes, so there is no flag to + # keep in step with the query: `spec/lib/forest_admin_agent/routes` pins that per route. + it 'checks nothing at all when the route passes no component' do + permissions = build_permissions([]) + + expect { permissions.assert_can_read_query_fields(cards) }.not_to raise_error + end + end + + describe '#read_permissions' do + def with_instant_cache_refresh(enabled) + config = ForestAdminAgent::Facades::Container.config_from_cache.merge(instant_cache_refresh: enabled) + allow(ForestAdminAgent::Facades::Container).to receive(:config_from_cache).and_return(config) + end + + it 'answers a collection once for the whole request' do + with_instant_cache_refresh(true) + permissions = build_permissions(%w[accounts]) + + permissions.read_permissions('cards', %w[accounts]) + permissions.read_permissions('cards', %w[accounts]) + + expect(permissions).to have_received(:get_collections_permissions_data).once + end + + # Denial is the steady state here, and `refresh-roles` on the SSE channel already evicts on a + # role change: a role that simply lacks `read` must not cost a permission fetch, and a cache + # eviction every other in-flight request reads through, on each page load. + it 'does not refetch for a denial the cached payload accounts for while the channel is up' do + with_instant_cache_refresh(true) + permissions = build_permissions([]) + + expect(permissions.read_permissions('cards', %w[accounts])).to eq( + { 'cards' => true, 'accounts' => false } + ) + expect(permissions).not_to have_received(:get_collections_permissions_data).with(force_fetch: true) + end + + # `instant_cache_refresh` defaults to production only, so outside it nothing else keeps the + # cache fresh and a newly granted `read` would stay redacted until the cache expires. + it 'refetches a denial when no channel keeps the cache fresh' do + with_instant_cache_refresh(false) + permissions = build_permissions([]) + + permissions.read_permissions('cards', %w[accounts]) + + expect(permissions).to have_received(:get_collections_permissions_data).with(force_fetch: true).once + end + + it 'refetches once for the whole request when the payload does not know a collection' do + with_instant_cache_refresh(true) + permissions = build_permissions([]) + + permissions.read_permissions('cards', %w[not_in_payload]) + permissions.read_permissions('cards', %w[another_one_missing]) + + expect(permissions).to have_received(:get_collections_permissions_data).with(force_fetch: true).once + end + + it 'answers without a fetch once the permission system is absent' do + permissions = described_class.new(caller) + allow(permissions).to receive_messages(permission_system?: false) + + expect(permissions.read_permissions('cards', %w[accounts])).to eq( + { 'cards' => true, 'accounts' => true } + ) + end + end + end + end +end diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/utils/csv_generator_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/utils/csv_generator_spec.rb index dd1c038de..22b41c1db 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/utils/csv_generator_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/utils/csv_generator_spec.rb @@ -72,6 +72,78 @@ module Utils "id,last_name,first_name,email,active,created_at,updated_at,address\n1,Skywalker,Luke,luke@sw.com,true,2024-05-21T00:00:00.000Z,2024-05-21T00:00:00.000Z,Tatooine\n2,Solo,Han,han@sw.com,true,2024-05-21T00:00:00.000Z,2024-05-21T00:00:00.000Z,Corellia\n3,Organa,Leia,leia@sw.com,true,2024-05-21T00:00:00.000Z,2024-05-21T00:00:00.000Z,Alderaan\n4,Kenobi,Obi-Wan,obiwan@sw.com,false,2024-05-21T00:00:00.000Z,2024-05-21T00:00:00.000Z,Stewjon\n" end + describe 'filter_header' do + let(:requested) { Projection.new(%w[id last_name address:planet]) } + + it 'hands the header back untouched when the redaction dropped nothing' do + expect(described_class.filter_header('Id,Last name,Planet', requested, requested)) + .to eq('Id,Last name,Planet') + end + + it 'returns nothing when the caller sent no header' do + expect(described_class.filter_header(nil, requested, Projection.new(%w[id]))).to be_nil + end + + it 'drops the label of a path the redaction removed from a comma-joined header' do + kept = Projection.new(%w[id last_name]) + + expect(described_class.filter_header('Id,Last name,Planet', requested, kept)) + .to eq(['Id', 'Last name']) + end + + it 'drops the label of a path the redaction removed from a JSON header' do + kept = Projection.new(%w[id address:planet]) + + expect(described_class.filter_header('["Id","Last name","Planet"]', requested, kept)) + .to eq(%w[Id Planet]) + end + + it 'drops the label of a path the redaction removed from an array of labels' do + kept = Projection.new(%w[address:planet]) + + expect(described_class.filter_header(['Id', 'Last name', 'Planet'], requested, kept)) + .to eq(['Planet']) + end + + # `fields[books]=author,id,title` with `fields[author]=firstName,lastName` expands into four + # paths against three labels: filtering by position drops "Id" and keeps "Title", leaving one + # label for two data columns, under the wrong name. + it 'discards a JSON header the expanded projection has outgrown' do + expanded = Projection.new(%w[author:firstName author:lastName id title]) + kept = Projection.new(%w[id title]) + + expect(described_class.filter_header('["Author","Id","Title"]', expanded, kept)).to be_nil + end + + it 'discards a header that carries no label for each requested path' do + # `fields[users]=address,id` expands `address` into two paths, so no label sits at the + # index of a path and dropping one by position would mislabel the rest. + expanded = Projection.new(%w[address:planet address:city id]) + + expect(described_class.filter_header('Address,Id', expanded, Projection.new(%w[id]))).to be_nil + end + + # An unmappable header must not be handed back: the stream re-checks the label count against + # the *kept* projection, so a header matching that count by coincidence would be accepted and + # print each surviving column under the label of whatever preceded it. Here `holder:*` is + # denied while the type field appended next to it survives, and 'Holder' would head + # `holder_type`. + it 'discards a header whose label count coincides with the kept projection' do + requested = Projection.new(%w[pan_last4 holder:* holder_type]) + kept = Projection.new(%w[pan_last4 holder_type]) + + expect(described_class.filter_header('PAN,Holder', requested, kept)).to be_nil + end + + # The one branch where handing the header back is safe: both counts are the same number, so + # the stream's own check cannot reach a different answer. + it 'keeps the header the caller sent when the redaction dropped no column' do + requested = Projection.new(%w[pan_last4 holder_type]) + + expect(described_class.filter_header('PAN,Type', requested, requested)).to eq('PAN,Type') + end + end + describe 'generate' do it 'generates a CSV string' do csv = described_class.generate(records, projection) diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/utils/csv_generator_stream_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/utils/csv_generator_stream_spec.rb index 4ab550568..63ff9ef46 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/utils/csv_generator_stream_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/utils/csv_generator_stream_spec.rb @@ -60,6 +60,24 @@ module Utils expect(csv_output).to start_with("id,first_name,last_name\n") end + + it 'labels the columns from a comma-joined header, which is what the front sends' do + enumerator = described_class.stream('Id,First name,Last name', filter, projection, list_records) + + expect(enumerator.to_a.join).to start_with("Id,First name,Last name\n") + end + + it 'escapes a label carrying a comma instead of widening the header row' do + enumerator = described_class.stream(['Id', 'Last, first', 'Name'], filter, projection, list_records) + + expect(enumerator.to_a.join).to start_with(%(Id,"Last, first",Name\n)) + end + + it 'uses the projection when the header carries no label for each exported column' do + enumerator = described_class.stream('Id,First name', filter, projection, list_records) + + expect(enumerator.to_a.join).to start_with("id,first_name,last_name\n") + end end it 'streams CSV data with header and records' do diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/utils/query_string_parser_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/utils/query_string_parser_spec.rb index b4eae8dba..cd7ee0144 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/utils/query_string_parser_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/utils/query_string_parser_spec.rb @@ -712,6 +712,80 @@ module Utils end end + describe 'parse_requested_projection' do + let(:collection) do + datasource = Datasource.new + collection_person = Collection.new(datasource, 'Person') + collection_person.add_fields( + { + 'id' => ColumnSchema.new(column_type: 'Number', is_primary_key: true), + 'name' => ColumnSchema.new(column_type: 'String') + } + ) + collection = Collection.new(datasource, 'Book') + collection.add_fields( + { + 'id' => ColumnSchema.new(column_type: 'Number', is_primary_key: true), + 'title' => ColumnSchema.new(column_type: 'String'), + 'author_id' => ColumnSchema.new(column_type: 'Number'), + 'author' => Relations::ManyToOneSchema.new( + foreign_key: 'author_id', + foreign_key_target: 'id', + foreign_collection: 'Person' + ) + } + ) + + datasource.add_collection(collection) + datasource.add_collection(collection_person) + + return collection + end + + it 'reports a projection header as named by the caller' do + args = { headers: { 'HTTP_FOREST_PROJECTION' => 'title,author:name' }, params: {} } + + expect(described_class.parse_requested_projection(collection, args)).to eq( + { projection: Projection.new(%w[title author:name]), named_by_caller: true } + ) + end + + it 'reports a fields parameter as named by the caller' do + args = { headers: {}, params: { fields: { 'Book' => 'id,title' } } } + + expect(described_class.parse_requested_projection(collection, args)).to eq( + { projection: Projection.new(%w[id title]), named_by_caller: true } + ) + end + + it 'reports an absent fields parameter as not named by the caller' do + args = { headers: {}, params: {} } + + expect(described_class.parse_requested_projection(collection, args)).to eq( + { projection: Projection.new(%w[id title author_id author:id author:name]), named_by_caller: false } + ) + end + + it 'reports an empty fields parameter as not named by the caller' do + args = { headers: {}, params: { fields: { 'Book' => '' } } } + + expect(described_class.parse_requested_projection(collection, args)).to eq( + { projection: Projection.new(%w[id title author_id author:id author:name]), named_by_caller: false } + ) + end + + it 'prefers the header over a fields parameter naming other columns' do + args = { + headers: { 'HTTP_FOREST_PROJECTION' => 'title' }, + params: { fields: { 'Book' => '' } } + } + + expect(described_class.parse_requested_projection(collection, args)).to eq( + { projection: Projection.new(%w[title]), named_by_caller: true } + ) + end + end + describe 'parse_projection_with_pks' do let(:collection) do datasource = Datasource.new diff --git a/packages/forest_admin_agent/spec/spec_helper.rb b/packages/forest_admin_agent/spec/spec_helper.rb index 91a64768f..edf93e0f9 100644 --- a/packages/forest_admin_agent/spec/spec_helper.rb +++ b/packages/forest_admin_agent/spec/spec_helper.rb @@ -32,6 +32,41 @@ let(:caller) { build_caller } end +# Stubs the read guards for a route spec and records how the route wired them, so a spec can pin the +# collection each guard resolves against, the query components the route says it applies, and whether +# a denial is a 403 or a redaction. The guards themselves are exercised against a real Permissions in +# spec/lib/forest_admin_agent/security. +# +# The collection is recorded by name rather than matched on: RSpec renders an argument mismatch by +# inspecting it, and a collection reaches its datasource, which reaches every collection, so the +# inspect never finishes and the suite hangs instead of failing. Leaving a guard unstubbed hangs the +# same way. + +# `applies` is derived from the keys the route actually passed, so it cannot drift from the query the +# route builds: a component it drops is one it does not pass. +READ_GUARD_QUERY_KEYS = { filter: :condition_tree, sort: :sort, search: :search }.freeze + +RSpec.shared_context 'with readable related collections' do + let(:read_guard_calls) { { query_fields: [], projections: [] } } + + before do + allow(permissions).to receive(:assert_can_read_query_fields) do |collection, **options| + read_guard_calls[:query_fields] << { + collection: collection.name, + applies: READ_GUARD_QUERY_KEYS.select { |_name, key| options.key?(key) }.keys + } + end + allow(permissions).to receive(:assert_can_read_usages) + allow(permissions).to receive(:redact_projection) do |collection, projection, **options| + read_guard_calls[:projections] << { + collection: collection.name, + named_by_caller: options[:named_by_caller] + } + projection + end + end +end + RSpec.configure do |config| config.include ForestAdminTestToolkit::Factory::Caller config.include ForestAdminTestToolkit::Factory::Collection diff --git a/packages/forest_admin_datasource_customizer/lib/forest_admin_datasource_customizer/decorators/search/search_collection_decorator.rb b/packages/forest_admin_datasource_customizer/lib/forest_admin_datasource_customizer/decorators/search/search_collection_decorator.rb index 1f894f18a..775d34567 100644 --- a/packages/forest_admin_datasource_customizer/lib/forest_admin_datasource_customizer/decorators/search/search_collection_decorator.rb +++ b/packages/forest_admin_datasource_customizer/lib/forest_admin_datasource_customizer/decorators/search/search_collection_decorator.rb @@ -56,8 +56,43 @@ def refine_filter(caller, filter) filter end + # Answers against +@child_collection+, which is what the search actually reads: a field + # hidden by the publication or renaming layers above is still searched. + # + # The term is run through the same +build_condition+ selection +refine_filter+ applies, so a + # field the search cannot match — a number column for a word, a uuid column for anything + # else — is left out rather than reported as reached. + # + # +nil+ whenever this layer does not choose the fields — a replacer is installed, or the + # child collection searches natively — because then no enumeration made here is true. + def searched_fields(search, extended) + return nil unless enumerable_search? + return [] if insignificant_search?(search) + + get_fields(extended).filter_map do |path, schema| + searched_field(path) if build_condition(path, schema, search) + end + end + private + def enumerable_search? + @replacer.nil? && !@child_collection.schema[:searchable] + end + + def insignificant_search?(search) + search.nil? || search.strip.empty? + end + + def searched_field(path) + { + path: path, + collections: ForestAdminDatasourceToolkit::Utils::FieldPath.leaf_collection_names( + @child_collection, path + ) + } + end + def default_replacer(search, extended) searchable_fields = get_fields(extended) diff --git a/packages/forest_admin_datasource_customizer/spec/lib/forest_admin_datasource_customizer/decorators/search/searched_fields_spec.rb b/packages/forest_admin_datasource_customizer/spec/lib/forest_admin_datasource_customizer/decorators/search/searched_fields_spec.rb new file mode 100644 index 000000000..24142426e --- /dev/null +++ b/packages/forest_admin_datasource_customizer/spec/lib/forest_admin_datasource_customizer/decorators/search/searched_fields_spec.rb @@ -0,0 +1,88 @@ +require 'spec_helper' + +module ForestAdminDatasourceCustomizer + module Decorators + module Search + include ForestAdminDatasourceToolkit::Schema + include ForestAdminDatasourceToolkit::Components::Query::ConditionTree + + describe SearchCollectionDecorator do + subject(:decorated) { described_class.new(datasource.get_collection('cards'), datasource) } + + let(:datasource) do + build_datasource_with_collections( + [ + build_collection( + name: 'cards', + schema: { + fields: { + 'id' => build_numeric_primary_key, + 'pan_last4' => build_column(column_type: 'String', filter_operators: [Operators::I_CONTAINS]), + 'holder_id' => build_column(column_type: 'Number'), + 'holder' => build_many_to_one(foreign_collection: 'holders', foreign_key: 'holder_id') + } + } + ), + build_collection( + name: 'holders', + schema: { + fields: { + 'id' => build_numeric_primary_key, + 'national_id' => build_column(column_type: 'String', filter_operators: [Operators::I_CONTAINS]) + } + } + ) + ] + ) + end + + describe '#searched_fields' do + it 'reports the columns of the collection itself when the search is not extended' do + expect(decorated.searched_fields('martin', false)).to contain_exactly( + { path: 'pan_last4', collections: ['cards'] } + ) + end + + it 'reports the collection a relation column belongs to when the search is extended' do + expect(decorated.searched_fields('martin', true)).to contain_exactly( + { path: 'pan_last4', collections: ['cards'] }, + { path: 'holder:national_id', collections: ['holders'] } + ) + end + + it 'leaves out a column the term cannot match, which the search builds no condition for' do + expect(decorated.searched_fields('martin', true).map { |field| field[:path] }) + .not_to include('id', 'holder:id') + end + + it 'reports a numeric column once the term is a number the search will compare it to' do + expect(decorated.searched_fields('42', true)).to contain_exactly( + { path: 'id', collections: ['cards'] }, + { path: 'pan_last4', collections: ['cards'] }, + { path: 'holder:id', collections: ['holders'] }, + { path: 'holder:national_id', collections: ['holders'] } + ) + end + + it 'reaches nothing for a search the stack discards as insignificant' do + expect(decorated.searched_fields(' ', true)).to eq([]) + end + + # Reading it as "reaches nothing" would let a replaced search through unchecked. + it 'answers nothing it can be sure of once a replacer chooses the fields' do + decorated.replace_search(->(search, _extended, _context) { { field: 'id', operator: 'equal', value: search } }) + + expect(decorated.searched_fields('martin', true)).to be_nil + end + + it 'answers nothing it can be sure of when the datasource searches natively' do + child = datasource.get_collection('cards') + allow(child).to receive(:schema).and_return(child.schema.merge(searchable: true)) + + expect(described_class.new(child, datasource).searched_fields('martin', true)).to be_nil + end + end + end + end + end +end diff --git a/packages/forest_admin_datasource_toolkit/lib/forest_admin_datasource_toolkit/decorators/collection_decorator.rb b/packages/forest_admin_datasource_toolkit/lib/forest_admin_datasource_toolkit/decorators/collection_decorator.rb index e72573de9..b6ffd92c4 100644 --- a/packages/forest_admin_datasource_toolkit/lib/forest_admin_datasource_toolkit/decorators/collection_decorator.rb +++ b/packages/forest_admin_datasource_toolkit/lib/forest_admin_datasource_toolkit/decorators/collection_decorator.rb @@ -77,6 +77,14 @@ def render_chart(caller, name, record_id, parameters = {}) @child_collection.render_chart(caller, name, record_id, parameters) end + # Which fields a search will actually reach, and the collection each one ends on. +nil+ means + # the collection cannot say, which a caller must read as "unknown", never as "none". + def searched_fields(search, extended) + return nil unless @child_collection.is_a?(CollectionDecorator) + + @child_collection.searched_fields(search, extended) + end + protected def mark_schema_as_dirty diff --git a/packages/forest_admin_datasource_toolkit/lib/forest_admin_datasource_toolkit/utils/field_path.rb b/packages/forest_admin_datasource_toolkit/lib/forest_admin_datasource_toolkit/utils/field_path.rb new file mode 100644 index 000000000..6fc54bf16 --- /dev/null +++ b/packages/forest_admin_datasource_toolkit/lib/forest_admin_datasource_toolkit/utils/field_path.rb @@ -0,0 +1,52 @@ +module ForestAdminDatasourceToolkit + module Utils + class FieldPath + POLYMORPHIC_MANY_TO_ONE = 'PolymorphicManyToOne'.freeze + + # The collections whose column a path ends on — the ones a read permission applies to. + # Collections crossed on the way are joins, not read targets, so they are not returned. + # + # Several names come back for a path ending on a polymorphic relation: it carries no + # discriminant, so any record may resolve to any of its targets and none can be ruled out. The + # list is empty for a polymorphic relation declaring no target at all, which the caller counts + # as denied — an empty list is a path that resolves to nothing, not a path that needs nothing. + # + # A prefix naming no relation raises rather than falling back to +collection+. The caller pins + # the collection it asked about to readable, so falling back would turn "this path does not + # resolve" into "this path is allowed". + # + # A path with no prefix at all is a column of +collection+ itself, and so is allowed. Two + # customisations reach a related collection through one deliberately: +import_field+, and an + # +add_field+ whose dependencies cross a relation, both register a root-level column that the + # operator and sorting layers rewrite into a relation path below this guard. Like a scope or a + # +replace_search+, they are the customer's own choice of what a role reaches through a column + # it can read. + def self.leaf_collection_names(collection, path) + index = path.index(':') + + return [collection.name] if index.nil? + + relation = relation_at(collection, path[0...index]) + + return relation.foreign_collections if relation.type == POLYMORPHIC_MANY_TO_ONE + + leaf_collection_names( + collection.datasource.get_collection(relation.foreign_collection), + path[(index + 1)..] + ) + end + + def self.relation_at(collection, name) + field = collection.schema[:fields][name] + + if field.nil? || field.type == 'Column' + raise Exceptions::ForestException, "Relation not found: '#{collection.name}.#{name}'" + end + + field + end + + private_class_method :relation_at + end + end +end diff --git a/packages/forest_admin_datasource_toolkit/spec/lib/forest_admin_datasource_toolkit/utils/field_path_spec.rb b/packages/forest_admin_datasource_toolkit/spec/lib/forest_admin_datasource_toolkit/utils/field_path_spec.rb new file mode 100644 index 000000000..d3489718e --- /dev/null +++ b/packages/forest_admin_datasource_toolkit/spec/lib/forest_admin_datasource_toolkit/utils/field_path_spec.rb @@ -0,0 +1,93 @@ +require 'spec_helper' + +module ForestAdminDatasourceToolkit + module Utils + include ForestAdminDatasourceToolkit::Schema + + describe FieldPath do + subject(:cards) { datasource.get_collection('cards') } + + let(:datasource) do + build_datasource_with_collections( + [ + build_collection( + name: 'cards', + schema: { + fields: { + 'id' => ColumnSchema.new(is_primary_key: true, column_type: 'Number'), + 'pan_last4' => ColumnSchema.new(column_type: 'String'), + 'account_id' => ColumnSchema.new(column_type: 'Number'), + 'holder_id' => ColumnSchema.new(column_type: 'Number'), + 'account' => Relations::ManyToOneSchema.new( + foreign_collection: 'accounts', foreign_key: 'account_id', foreign_key_target: 'id' + ), + 'certificate' => Relations::PolymorphicOneToOneSchema.new( + origin_key: 'owner_id', + origin_key_target: 'id', + foreign_collection: 'certificates', + origin_type_field: 'owner_type', + origin_type_value: 'Card' + ), + 'holder' => Relations::PolymorphicManyToOneSchema.new( + foreign_key: 'holder_id', + foreign_key_type_field: 'holder_type', + foreign_collections: %w[persons companies], + foreign_key_targets: { 'persons' => 'id', 'companies' => 'id' } + ) + } + } + ), + build_collection( + name: 'accounts', + schema: { + fields: { + 'id' => ColumnSchema.new(is_primary_key: true, column_type: 'Number'), + 'organization_id' => ColumnSchema.new(column_type: 'Number'), + 'organization' => Relations::ManyToOneSchema.new( + foreign_collection: 'organizations', foreign_key: 'organization_id', foreign_key_target: 'id' + ) + } + } + ), + build_collection(name: 'organizations', schema: { fields: { 'id' => ColumnSchema.new(is_primary_key: true, column_type: 'Number') } }), + build_collection(name: 'certificates', schema: { fields: { 'id' => ColumnSchema.new(is_primary_key: true, column_type: 'Number') } }), + build_collection(name: 'persons', schema: { fields: { 'id' => ColumnSchema.new(is_primary_key: true, column_type: 'Number') } }), + build_collection(name: 'companies', schema: { fields: { 'id' => ColumnSchema.new(is_primary_key: true, column_type: 'Number') } }) + ] + ) + end + + describe '.leaf_collection_names' do + it 'answers the collection itself for one of its own columns' do + expect(described_class.leaf_collection_names(cards, 'pan_last4')).to eq(['cards']) + end + + it 'answers only the collection a path ends on, not the ones it crosses' do + expect(described_class.leaf_collection_names(cards, 'account:organization:id')).to eq(['organizations']) + end + + it 'answers every target of a polymorphic many-to-one' do + expect(described_class.leaf_collection_names(cards, 'holder:*')).to eq(%w[persons companies]) + end + + it 'answers the single target of a polymorphic one-to-one' do + expect(described_class.leaf_collection_names(cards, 'certificate:id')).to eq(['certificates']) + end + + it 'raises rather than falling back when the prefix is a column' do + expect { described_class.leaf_collection_names(cards, 'pan_last4:id') }.to raise_error( + ForestAdminDatasourceToolkit::Exceptions::ForestException, + "Relation not found: 'cards.pan_last4'" + ) + end + + it 'raises when the prefix names nothing at all' do + expect { described_class.leaf_collection_names(cards, 'unknown:id') }.to raise_error( + ForestAdminDatasourceToolkit::Exceptions::ForestException, + "Relation not found: 'cards.unknown'" + ) + end + end + end + end +end