Skip to content

Commit 5d2a0dd

Browse files
stakachstakach
committed
feat(modules): support subsystem listing
1 parent 22ae2af commit 5d2a0dd

2 files changed

Lines changed: 162 additions & 19 deletions

File tree

‎spec/controllers/modules_spec.cr‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -727,6 +727,120 @@ module PlaceOS::Api
727727

728728
zone.destroy
729729
end
730+
731+
# ---- read scoping ----
732+
733+
# A non-logic (device) module referenced by a control system scoped to
734+
# `zone`. Device modules are reachable via the system's `modules` array,
735+
# so the module derives its zones from that system.
736+
device_in_zone = ->(zone : Model::Zone) {
737+
driver = Model::Generator.driver(role: Model::Driver::Role::Device).save!
738+
mod = Model::Generator.module(driver: driver).save!
739+
cs = Model::Generator.control_system
740+
cs.zones = [zone.id.as(String)]
741+
cs.modules = [mod.id.as(String)]
742+
cs.save!
743+
{cs, mod}
744+
}
745+
746+
it "allows show for a support user with Read on the module's zone" do
747+
authority = Model::Authority.find_by_domain("localhost").not_nil!
748+
user, headers = Spec::Authentication.authentication(sys_admin: false, support: false)
749+
750+
zone = Model::Generator.zone.save!
751+
cs, mod = device_in_zone.call(zone)
752+
753+
group = Model::Generator.group(authority: authority, subsystems: ["support"]).save!
754+
Model::Generator.group_user(user: user, group: group, permissions: Model::Permissions::Read).save!
755+
Model::Generator.group_zone(group: group, zone: zone, permissions: Model::Permissions::Read).save!
756+
757+
result = client.get(path: "#{Modules.base_route}#{mod.id}", headers: headers)
758+
result.status_code.should eq 200
759+
760+
mod.destroy
761+
cs.destroy
762+
zone.destroy
763+
end
764+
765+
it "rejects show for a support user without a grant on the module's zone" do
766+
authority = Model::Authority.find_by_domain("localhost").not_nil!
767+
user, headers = Spec::Authentication.authentication(sys_admin: false, support: false)
768+
769+
zone = Model::Generator.zone.save!
770+
cs, mod = device_in_zone.call(zone)
771+
772+
# group exists but has no GroupZone reach to the module's zone
773+
group = Model::Generator.group(authority: authority, subsystems: ["support"]).save!
774+
Model::Generator.group_user(user: user, group: group, permissions: Model::Permissions::Read).save!
775+
776+
result = client.get(path: "#{Modules.base_route}#{mod.id}", headers: headers)
777+
result.status_code.should eq 403
778+
779+
mod.destroy
780+
cs.destroy
781+
zone.destroy
782+
end
783+
784+
it "allows index?control_system_id= with Read on the system zones and lists the modules" do
785+
authority = Model::Authority.find_by_domain("localhost").not_nil!
786+
user, headers = Spec::Authentication.authentication(sys_admin: false, support: false)
787+
788+
zone = Model::Generator.zone.save!
789+
cs, mod = device_in_zone.call(zone)
790+
791+
group = Model::Generator.group(authority: authority, subsystems: ["support"]).save!
792+
Model::Generator.group_user(user: user, group: group, permissions: Model::Permissions::Read).save!
793+
Model::Generator.group_zone(group: group, zone: zone, permissions: Model::Permissions::Read).save!
794+
795+
result = client.get(path: "#{Modules.base_route}?control_system_id=#{cs.id}", headers: headers)
796+
result.status_code.should eq 200
797+
JSON.parse(result.body).as_a.map(&.["id"].as_s).should contain(mod.id.as(String))
798+
799+
mod.destroy
800+
cs.destroy
801+
zone.destroy
802+
end
803+
804+
it "rejects index?control_system_id= for a support user without a grant" do
805+
authority = Model::Authority.find_by_domain("localhost").not_nil!
806+
user, headers = Spec::Authentication.authentication(sys_admin: false, support: false)
807+
808+
zone = Model::Generator.zone.save!
809+
cs, mod = device_in_zone.call(zone)
810+
811+
group = Model::Generator.group(authority: authority, subsystems: ["support"]).save!
812+
Model::Generator.group_user(user: user, group: group, permissions: Model::Permissions::Read).save!
813+
814+
result = client.get(path: "#{Modules.base_route}?control_system_id=#{cs.id}", headers: headers)
815+
result.status_code.should eq 403
816+
817+
mod.destroy
818+
cs.destroy
819+
zone.destroy
820+
end
821+
822+
it "gates the whole module list: 403 with no accessible zones, 200 (scoped) with a grant" do
823+
authority = Model::Authority.find_by_domain("localhost").not_nil!
824+
user, headers = Spec::Authentication.authentication(sys_admin: false, support: false)
825+
826+
zone = Model::Generator.zone.save!
827+
cs, mod = device_in_zone.call(zone)
828+
829+
# no group/grant => no accessible zones => 403
830+
client.get(path: Modules.base_route, headers: headers).status_code.should eq 403
831+
832+
# grant a reachable zone => allowed (result set is zone-scoped; contents
833+
# depend on ES indexing, so we only assert the gate opens)
834+
group = Model::Generator.group(authority: authority, subsystems: ["support"]).save!
835+
Model::Generator.group_user(user: user, group: group, permissions: Model::Permissions::Read).save!
836+
Model::Generator.group_zone(group: group, zone: zone, permissions: Model::Permissions::Read).save!
837+
838+
client.get(path: Modules.base_route, headers: headers).status_code.should eq 200
839+
840+
mod.destroy
841+
cs.destroy
842+
zone.destroy
843+
end
730844
end
731845
end
732846
end

‎src/placeos-rest-api/controllers/modules.cr‎

Lines changed: 48 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,10 @@ module PlaceOS::Api
2222
before_action :can_write, only: [:create, :update, :destroy, :remove]
2323

2424
before_action :check_admin, except: [:index, :create, :update, :destroy, :state, :show, :ping, :start, :stop]
25-
# start/stop are gated by the support-subsystem Operate check in-action
26-
# (see `start`/`stop`), so they're intentionally absent here.
27-
before_action :check_support, only: [:state, :show, :ping, :show_error]
25+
# start/stop are gated by the support-subsystem Operate check in-action;
26+
# show is gated by `check_show_permissions` (support-subsystem Read on the
27+
# module's zones) — so both are intentionally absent here.
28+
before_action :check_support, only: [:state, :ping, :show_error]
2829

2930
###############################################################################################
3031

@@ -63,29 +64,56 @@ module PlaceOS::Api
6364
zones.uniq
6465
end
6566

67+
# Reading a single module: admin/support JWT and the legacy org_zone path
68+
# bypass; otherwise a "support" subsystem user needs Read on the module's
69+
# zones. A module with no derivable zones stays admin/support only.
70+
@[AC::Route::Filter(:before_action, only: [:show])]
71+
def check_show_permissions
72+
ensure_support_access!(zones_for_module(current_module), ::PlaceOS::Model::Permissions::Read)
73+
end
74+
6675
@[AC::Route::Filter(:before_action, only: [:index])]
6776
def check_view_permissions(
6877
@[AC::Param::Info(description: "only return modules running in this system (query params are ignored if this is provided)", example: "sys-1234")]
6978
control_system_id : String? = nil,
7079
)
80+
# admin/support JWT see everything (no scoping)
7181
return if user_support?
7282

73-
# find the org zone
74-
authority = current_authority.as(::PlaceOS::Model::Authority)
75-
@org_zone_id = org_zone_id = authority.config["org_zone"]?.try(&.as_s?)
76-
raise Error::Forbidden.new unless org_zone_id
83+
org_zone = support_org_zone_id
7784

7885
if control_system_id
7986
zones = ::PlaceOS::Model::ControlSystem.find!(control_system_id).zones
80-
raise Error::Forbidden.new unless zones.includes?(org_zone_id)
81-
raise Error::Forbidden.new unless check_access(current_user.groups, zones).can_manage?
82-
else
83-
access = check_access(current_user.groups, [org_zone_id])
84-
raise Error::Forbidden.new unless access.can_manage?
87+
# "support" subsystem: Read on the system's zones.
88+
return if support_subsystem_grants?(zones, ::PlaceOS::Model::Permissions::Read)
89+
# legacy org_zone path: the system must include the org_zone and the
90+
# user must be able to manage it.
91+
return if org_zone && zones.includes?(org_zone) && check_access(current_user.groups, zones).can_manage?
92+
raise Error::Forbidden.new
93+
end
94+
95+
# Whole-list view: instead of a flat allow/deny, resolve the zone set the
96+
# caller may see and scope the result set to it (see `index`).
97+
# legacy org_zone manager -> systems containing the org_zone.
98+
if org_zone && check_access(current_user.groups, [org_zone]).can_manage?
99+
@module_scope_zones = [org_zone]
100+
return
101+
end
102+
103+
# "support" subsystem -> systems whose zones the caller can reach.
104+
accessible = support_accessible_zone_ids
105+
unless accessible.empty?
106+
@module_scope_zones = accessible
107+
return
85108
end
109+
110+
raise Error::Forbidden.new
86111
end
87112

88-
getter org_zone_id : String? = nil
113+
# Zone ids used to scope the module list for non-admin/non-support callers
114+
# (legacy org_zone, or the caller's reachable "support" zones). Left nil for
115+
# admin/support callers, who see everything.
116+
getter module_scope_zones : Array(String)? = nil
89117

90118
# Response helpers
91119
###############################################################################################
@@ -147,18 +175,19 @@ module PlaceOS::Api
147175

148176
# TODO:: we can remove this once there is a tenant_id field on modules
149177
# which will make this much simpler to filter
150-
if filter_zone_id = org_zone_id
151-
# we only want to show modules in use by systems that include this zone
178+
if scope_zones = module_scope_zones
179+
# we only want to show modules in use by systems within these zones
152180
no_logic = true
153181

154182
# find all the non-logic modules that this user can access
155-
# 1. grabs all the module ids in the systems of the provided org zone
156-
# 2. select distinct modules ids which are not logic modules (99)
183+
# 1. grab all the module ids in systems whose zones overlap scope_zones
184+
# 2. select distinct module ids which are not logic modules (99)
185+
placeholders = (1..scope_zones.size).map { |i| "$#{i}" }.join(", ")
157186
sql_query = %[
158187
WITH matching_rows AS (
159188
SELECT unnest(modules) AS module_id
160189
FROM sys
161-
WHERE $1 = ANY(zones)
190+
WHERE zones && ARRAY[#{placeholders}]::text[]
162191
)
163192
164193
SELECT ARRAY_AGG(DISTINCT m.module_id)
@@ -168,7 +197,7 @@ module PlaceOS::Api
168197
]
169198

170199
module_ids = PgORM::Database.connection do |conn|
171-
conn.query_one(sql_query, args: [filter_zone_id], &.read(Array(String)?))
200+
conn.query_one(sql_query, args: scope_zones.map(&.as(PgORM::Value)), &.read(Array(String)?))
172201
end || [] of String
173202

174203
query.must({

0 commit comments

Comments
 (0)