From 9f3c678bd516c450863cf719bcd791f742cdd42b Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Fri, 2 Oct 2026 20:20:53 +1000 Subject: [PATCH] feat(zones): PPT-2649 search every zone the user can reach --- OPENAPI_DOC.yml | 7 + spec/controllers/zones_spec.cr | 181 ++++++++++++++++++++++ src/placeos-rest-api/controllers/zones.cr | 32 +++- 3 files changed, 214 insertions(+), 6 deletions(-) diff --git a/OPENAPI_DOC.yml b/OPENAPI_DOC.yml index 7889aab3..f72bf004 100644 --- a/OPENAPI_DOC.yml +++ b/OPENAPI_DOC.yml @@ -41982,6 +41982,13 @@ paths: example: "true" schema: type: boolean + - name: descendants + in: query + description: with group_id, include anchor zones and their descendants, excluding + denied subtrees + example: "true" + schema: + type: boolean - name: q in: query description: filters results by the given text; words match as prefixes against diff --git a/spec/controllers/zones_spec.cr b/spec/controllers/zones_spec.cr index 56643c38..b18c2baa 100644 --- a/spec/controllers/zones_spec.cr +++ b/spec/controllers/zones_spec.cr @@ -268,6 +268,187 @@ module PlaceOS::Api result = client.get("#{Zones.base_route}?#{params}", headers: headers) result.status_code.should eq 403 end + it "lists and searches only the user's signage group reach" do + clear_group_tables + authority = Model::Authority.find_by_domain("localhost").not_nil! + user, headers = Spec::Authentication.authentication(sys_admin: false, support: false) + anchor = Model::Generator.zone.save! + child = Model::Generator.zone + child.name = "ReachSearchMatch" + child.parent_id = anchor.id + child.save! + grandchild = Model::Generator.zone + grandchild.parent_id = child.id + grandchild.save! + unrelated = Model::Generator.zone + unrelated.name = "ReachSearchMatch Outside" + unrelated.save! + group = Model::Generator.group(authority: authority, subsystems: ["signage"]).save! + Model::Generator.group_user(user: user, group: group, permissions: Model::Permissions::Read).save! + Model::Generator.group_zone(group: group, zone: anchor, permissions: Model::Permissions::Read).save! + + params = HTTP::Params.encode({"limit" => "1000", "include_children_count" => "true"}) + result = client.get("#{Zones.base_route}?#{params}", headers: headers) + result.status_code.should eq 200 + zones = Array(Hash(String, JSON::Any)).from_json(result.body) + zones.map(&.["id"].as_s).sort!.should eq [anchor.id, child.id, grandchild.id].compact.sort! + zones.find! { |z| z["id"].as_s == anchor.id }["children_count"].as_i.should eq 1 + + params = HTTP::Params.encode({"q" => "ReachSearchMatch", "limit" => "1000"}) + result = client.get("#{Zones.base_route}?#{params}", headers: headers) + result.status_code.should eq 200 + Array(Hash(String, JSON::Any)).from_json(result.body).map(&.["id"].as_s).should eq [child.id] + ensure + grandchild.try(&.destroy) + child.try(&.destroy) + anchor.try(&.destroy) + unrelated.try(&.destroy) + end + + it "lists and searches only the user's support group reach" do + clear_group_tables + authority = Model::Authority.find_by_domain("localhost").not_nil! + user, headers = Spec::Authentication.authentication(sys_admin: false, support: false) + anchor = Model::Generator.zone.save! + child = Model::Generator.zone + child.name = "ReachSearchMatch" + child.parent_id = anchor.id + child.save! + grandchild = Model::Generator.zone + grandchild.parent_id = child.id + grandchild.save! + unrelated = Model::Generator.zone + unrelated.name = "ReachSearchMatch Outside" + unrelated.save! + group = Model::Generator.group(authority: authority, subsystems: ["support"]).save! + Model::Generator.group_user(user: user, group: group, permissions: Model::Permissions::Read).save! + Model::Generator.group_zone(group: group, zone: anchor, permissions: Model::Permissions::Read).save! + + params = HTTP::Params.encode({"limit" => "1000", "include_children_count" => "true"}) + result = client.get("#{Zones.base_route}?#{params}", headers: headers) + result.status_code.should eq 200 + zones = Array(Hash(String, JSON::Any)).from_json(result.body) + zones.map(&.["id"].as_s).sort!.should eq [anchor.id, child.id, grandchild.id].compact.sort! + zones.find! { |z| z["id"].as_s == anchor.id }["children_count"].as_i.should eq 1 + + params = HTTP::Params.encode({"q" => "ReachSearchMatch", "limit" => "1000"}) + result = client.get("#{Zones.base_route}?#{params}", headers: headers) + result.status_code.should eq 200 + Array(Hash(String, JSON::Any)).from_json(result.body).map(&.["id"].as_s).should eq [child.id] + ensure + grandchild.try(&.destroy) + child.try(&.destroy) + anchor.try(&.destroy) + unrelated.try(&.destroy) + end + + it "removes denied subtrees from unfiltered group reach" do + clear_group_tables + authority = Model::Authority.find_by_domain("localhost").not_nil! + user, headers = Spec::Authentication.authentication(sys_admin: false, support: false) + anchor = Model::Generator.zone.save! + denied = Model::Generator.zone + denied.parent_id = anchor.id + denied.save! + descendant = Model::Generator.zone + descendant.parent_id = denied.id + descendant.save! + group = Model::Generator.group(authority: authority, subsystems: ["signage"]).save! + Model::Generator.group_user(user: user, group: group, permissions: Model::Permissions::Read).save! + Model::Generator.group_zone(group: group, zone: anchor, permissions: Model::Permissions::Read).save! + deny_row = Model::Generator.group_zone(group: group, zone: denied, permissions: Model::Permissions::Read) + deny_row.deny = true + deny_row.save! + + result = client.get(Zones.base_route, headers: headers) + result.status_code.should eq 200 + Array(Hash(String, JSON::Any)).from_json(result.body).map(&.["id"].as_s).should eq [anchor.id] + ensure + descendant.try(&.destroy) + denied.try(&.destroy) + anchor.try(&.destroy) + end + + it "rejects unfiltered callers with no group reach, including unrelated subsystems" do + clear_group_tables + authority = Model::Authority.find_by_domain("localhost").not_nil! + user, headers = Spec::Authentication.authentication(sys_admin: false, support: false) + client.get(Zones.base_route, headers: headers).status_code.should eq 403 + client.get("#{Zones.base_route}?descendants=true", headers: headers).status_code.should eq 403 + + anchor = Model::Generator.zone.save! + group = Model::Generator.group(authority: authority, subsystems: ["bookings"]).save! + Model::Generator.group_user(user: user, group: group, permissions: Model::Permissions::Read).save! + Model::Generator.group_zone(group: group, zone: anchor, permissions: Model::Permissions::Read).save! + client.get(Zones.base_route, headers: headers).status_code.should eq 403 + ensure + anchor.try(&.destroy) + end + + it "keeps support listings unscoped and ignores descendants without a group" do + clear_group_tables + first = Model::Generator.zone.save! + unrelated = Model::Generator.zone.save! + params = HTTP::Params.encode({"limit" => "1000", "descendants" => "true"}) + result = client.get("#{Zones.base_route}?#{params}", headers: Spec::Authentication.headers) + result.status_code.should eq 200 + ids = Array(Hash(String, JSON::Any)).from_json(result.body).map(&.["id"].as_s) + ids.should contain(first.id) + ids.should contain(unrelated.id) + ensure + first.try(&.destroy) + unrelated.try(&.destroy) + end + + it "expands group anchors in SQL and excludes denied subtrees" do + clear_group_tables + authority = Model::Authority.find_by_domain("localhost").not_nil! + user, headers = Spec::Authentication.authentication(sys_admin: false, support: false) + anchor = Model::Generator.zone.save! + child = Model::Generator.zone + child.parent_id = anchor.id + child.save! + grandchild = Model::Generator.zone + grandchild.parent_id = child.id + grandchild.save! + denied = Model::Generator.zone + denied.parent_id = anchor.id + denied.save! + denied_child = Model::Generator.zone + denied_child.parent_id = denied.id + denied_child.save! + unrelated = Model::Generator.zone.save! + group = Model::Generator.group(authority: authority).save! + Model::Generator.group_user(user: user, group: group, permissions: Model::Permissions::Read).save! + Model::Generator.group_zone(group: group, zone: anchor, permissions: Model::Permissions::Read).save! + deny_row = Model::Generator.group_zone(group: group, zone: denied, permissions: Model::Permissions::Read) + deny_row.deny = true + deny_row.save! + + params = HTTP::Params.encode({"group_id" => group.id.to_s, "descendants" => "true", "limit" => "1000"}) + result = client.get("#{Zones.base_route}?#{params}", headers: headers) + result.status_code.should eq 200 + zones = Array(Hash(String, JSON::Any)).from_json(result.body) + zones.map(&.["id"].as_s).sort!.should eq [anchor.id, child.id, grandchild.id].compact.sort! + zones.all? { |z| z["children_count"]?.try(&.as_i?) }.should be_true + + params = HTTP::Params.encode({"group_id" => group.id.to_s}) + result = client.get("#{Zones.base_route}?#{params}", headers: headers) + result.status_code.should eq 200 + Array(Hash(String, JSON::Any)).from_json(result.body).map(&.["id"].as_s).sort!.should eq [anchor.id, denied.id].compact.sort! + + params = HTTP::Params.encode({"parent_id" => anchor.id.as(String), "descendants" => "true"}) + result = client.get("#{Zones.base_route}?#{params}", headers: headers) + result.status_code.should eq 200 + Array(Hash(String, JSON::Any)).from_json(result.body).map(&.["id"].as_s).sort!.should eq [child.id, denied.id].compact.sort! + ensure + grandchild.try(&.destroy) + child.try(&.destroy) + denied_child.try(&.destroy) + denied.try(&.destroy) + anchor.try(&.destroy) + unrelated.try(&.destroy) + end end describe "tags", tags: "search" do diff --git a/src/placeos-rest-api/controllers/zones.cr b/src/placeos-rest-api/controllers/zones.cr index 151f1607..0379678a 100644 --- a/src/placeos-rest-api/controllers/zones.cr +++ b/src/placeos-rest-api/controllers/zones.cr @@ -160,10 +160,9 @@ module PlaceOS::Api group_id : UUID? = nil, @[AC::Param::Info(description: "include children_count for each zone (useful for tree views)", example: "true")] include_children_count : Bool = false, + @[AC::Param::Info(description: "with group_id, include anchor zones and their descendants, excluding denied subtrees", example: "true")] + descendants : Bool = false, ) : Array(::PlaceOS::Model::Zone) - # Group-anchor filter: resolve the GroupZone anchors first so we - # can short-circuit on an empty list (and so the ES query never - # ends up unconstrained). group_zone_ids = nil if group_id unless user_support? @@ -190,7 +189,9 @@ module PlaceOS::Api query = ::PlaceOS::Model::Zone.all - if group_zone_ids + if group_id && descendants + query = query.where("id IN (#{group_descendants_sql})", group_id.to_s) + elsif group_zone_ids query = query.where(id: group_zone_ids) end @@ -215,8 +216,10 @@ module PlaceOS::Api # Limit results to zones containing ALL of the passed tags if (filter_tags = tags) && !filter_tags.empty? query = query.where("tags @> #{sql_array(filter_tags)}", filter_tags) - elsif group_zone_ids.nil? - raise Error::Forbidden.new unless parent_id || user_support? + elsif group_zone_ids.nil? && parent_id.nil? && !user_support? + scope = accessible_zones_scope_sql([SIGNAGE_SUBSYSTEM, SUPPORT_SUBSYSTEM], ::PlaceOS::Model::Permissions::Read) + raise Error::Forbidden.new unless scope + query = query.where("id IN (SELECT zone_id FROM #{scope} AS v(zone_id))", [] of ::PgORM::Value) end results = paginate_search(query, ::PlaceOS::Model::Zone.table_name) @@ -229,6 +232,23 @@ module PlaceOS::Api results end + private def group_descendants_sql : String + <<-SQL + WITH RECURSIVE anchors AS ( + SELECT zone_id, deny FROM group_zones WHERE group_id = ?::uuid + ), denied(zone_id) AS ( + SELECT zone_id FROM anchors WHERE deny + UNION + SELECT z.id FROM "zone" z INNER JOIN denied d ON z.parent_id = d.zone_id + ), reachable(zone_id) AS ( + SELECT zone_id FROM anchors WHERE NOT deny + UNION + SELECT z.id FROM "zone" z INNER JOIN reachable r ON z.parent_id = r.zone_id + ) + SELECT zone_id FROM reachable EXCEPT SELECT zone_id FROM denied + SQL + end + # Helper to add children counts to zones (GROUP BY over parent_id, # replacing the Elasticsearch terms aggregation) private def add_children_counts(zones : Array(::PlaceOS::Model::Zone))