From 8ff4ab174213305379fec154a9436ae0635178a6 Mon Sep 17 00:00:00 2001 From: Arthur Bassi Date: Tue, 4 Aug 2026 16:07:26 -0300 Subject: [PATCH] fix(work-orders): document single-org media scope (ADR 0001) Clarify SH-116 tenant scope as board-aligned ApplyBaseScope + claims, and add out-of-org-scope GET/mutation tests for deleted/template/missing WOs. --- .../Helpers/WorkOrderMediaAuthorization.cs | 16 +++-- .../Implementation/WorkOrderMediaService.cs | 4 +- .../WorkOrderPhase6Tests.cs | 66 ++++++++++++++++- docs/adr/0001-work-order-single-org-scope.md | 71 +++++++++++++++++++ 4 files changed, 149 insertions(+), 8 deletions(-) create mode 100644 docs/adr/0001-work-order-single-org-scope.md diff --git a/SeaHaven.Services/Helpers/WorkOrderMediaAuthorization.cs b/SeaHaven.Services/Helpers/WorkOrderMediaAuthorization.cs index 5034a0e..2af3080 100644 --- a/SeaHaven.Services/Helpers/WorkOrderMediaAuthorization.cs +++ b/SeaHaven.Services/Helpers/WorkOrderMediaAuthorization.cs @@ -6,8 +6,11 @@ namespace SeaHaven.Services.Helpers { /// /// Claims-derived authorization for work-order media read/mutations. - /// True multi-tenant CustomerId/TenantId is not modeled on WorkOrder/JWT; - /// scope is role + Assigned () for Technician. + /// Organization scope matches the board: callers must already have resolved + /// the work order via ApplyBaseScope (non-deleted, non-template). + /// Staff may access any in-org work order; technicians only when + /// matches the actor. See + /// docs/adr/0001-work-order-single-org-scope.md. /// public static class WorkOrderMediaAuthorization { @@ -51,14 +54,19 @@ namespace SeaHaven.Services.Helpers } /// - /// Staff: any in-scope (non-deleted/non-template) work order. - /// Technician: only work orders assigned to the caller. Out-of-scope → NotFound (no disclosure). + /// Caller-scope check after a base-scoped (in-org) work-order load. + /// Staff: any in-org work order already resolved via ApplyBaseScope. + /// Technician: only when assigned to the caller. + /// Out of caller scope → NotFound (no disclosure). /// public static void EnsureWorkOrderInCallerScope( ClaimsPrincipal user, string actorId, WorkOrder workOrder) { + // Organization boundary is enforced by the data-layer ApplyBaseScope + // load that produced . Staff are authorized + // for any in-org work order (same as the board). See ADR 0001. if (IsStaff(user)) return; diff --git a/SeaHaven.Services/Implementation/WorkOrderMediaService.cs b/SeaHaven.Services/Implementation/WorkOrderMediaService.cs index f224388..a75df3e 100644 --- a/SeaHaven.Services/Implementation/WorkOrderMediaService.cs +++ b/SeaHaven.Services/Implementation/WorkOrderMediaService.cs @@ -34,6 +34,8 @@ namespace SeaHaven.Services.Implementation { WorkOrderMediaAuthorization.EnsureCanRead(user, actorId); + // Organization scope: ApplyBaseScope via Exists / GetWorkOrderForMedia. + // Outside org (deleted/template/missing) → null (no disclosure). ADR 0001. if (!await _detailData.ExistsAsync(workOrderId, cancellationToken)) return null; @@ -257,7 +259,7 @@ namespace SeaHaven.Services.Implementation string actorId, CancellationToken cancellationToken) { - // Base-scoped lookup: deleted/template work orders surface as NotFound (no disclosure). + // Organization scope via ApplyBaseScope: deleted/template → NotFound (ADR 0001). var workOrder = await _mediaData.GetTrackedWorkOrderAsync(workOrderId, cancellationToken); if (workOrder == null) throw new WorkOrderBoardValidationException("NotFound", "Work order not found."); diff --git a/SeaHavenIndustries.Tests/WorkOrderPhase6Tests.cs b/SeaHavenIndustries.Tests/WorkOrderPhase6Tests.cs index a2e467f..0d2f6fc 100644 --- a/SeaHavenIndustries.Tests/WorkOrderPhase6Tests.cs +++ b/SeaHavenIndustries.Tests/WorkOrderPhase6Tests.cs @@ -731,6 +731,57 @@ public class WorkOrderMediaServiceTests service.GetMediaAsync(1, AuthenticatedUser(), "actor-1", cts.Token)); } + /// + /// Out-of-organization-scope (single-org analogue of SH-116 cross-tenant): + /// deleted work orders are outside ApplyBaseScope → null / no media disclosure. + /// + [Fact] + public async Task GetMedia_StaffOnDeletedWorkOrder_ReturnsNull_OutOfOrgScope() + { + var (context, service) = CreateSut(); + context.workOrders.Add(new WorkOrder + { + Id = 1, + IsDeleted = true, + BeforPhotoAttachment = "https://example.com/before.jpg" + }); + await context.SaveChangesAsync(); + + var media = await service.GetMediaAsync(1, AuthenticatedUser(), "actor-1"); + + Assert.Null(media); + } + + /// + /// Out-of-organization-scope: template work orders are outside ApplyBaseScope. + /// + [Fact] + public async Task GetMedia_StaffOnTemplateWorkOrder_ReturnsNull_OutOfOrgScope() + { + var (context, service) = CreateSut(); + context.workOrders.Add(new WorkOrder + { + Id = 1, + istemplate = true, + BeforPhotoAttachment = "https://example.com/before.jpg" + }); + await context.SaveChangesAsync(); + + var media = await service.GetMediaAsync(1, AuthenticatedUser(), "actor-1"); + + Assert.Null(media); + } + + [Fact] + public async Task GetMedia_MissingWorkOrder_ReturnsNull_OutOfOrgScope() + { + var (_, service) = CreateSut(); + + var media = await service.GetMediaAsync(99, AuthenticatedUser(), "actor-1"); + + Assert.Null(media); + } + [Fact] public async Task DeleteMedia_ReadOnlyWorkOrder_Throws() { @@ -937,8 +988,11 @@ public class WorkOrderMediaServiceTests Assert.Equal("Forbidden", ex.Code); } + /// + /// Out-of-organization-scope mutation: missing id → NotFound (no disclosure). + /// [Fact] - public async Task EnsureCanMutateMedia_MissingWorkOrder_ThrowsNotFound() + public async Task EnsureCanMutateMedia_MissingWorkOrder_ThrowsNotFound_OutOfOrgScope() { var (_, service) = CreateSut(); var ex = await Assert.ThrowsAsync(() => @@ -946,8 +1000,11 @@ public class WorkOrderMediaServiceTests Assert.Equal("NotFound", ex.Code); } + /// + /// Out-of-organization-scope mutation: deleted WO outside ApplyBaseScope → NotFound. + /// [Fact] - public async Task EnsureCanMutateMedia_DeletedWorkOrder_ThrowsNotFound() + public async Task EnsureCanMutateMedia_DeletedWorkOrder_ThrowsNotFound_OutOfOrgScope() { var (context, service) = CreateSut(); context.workOrders.Add(new WorkOrder @@ -965,8 +1022,11 @@ public class WorkOrderMediaServiceTests Assert.Equal("NotFound", ex.Code); } + /// + /// Out-of-organization-scope mutation: template WO outside ApplyBaseScope → NotFound. + /// [Fact] - public async Task EnsureCanMutateMedia_TemplateWorkOrder_ThrowsNotFound() + public async Task EnsureCanMutateMedia_TemplateWorkOrder_ThrowsNotFound_OutOfOrgScope() { var (context, service) = CreateSut(); context.workOrders.Add(new WorkOrder diff --git a/docs/adr/0001-work-order-single-org-scope.md b/docs/adr/0001-work-order-single-org-scope.md new file mode 100644 index 0000000..4abacc7 --- /dev/null +++ b/docs/adr/0001-work-order-single-org-scope.md @@ -0,0 +1,71 @@ +# ADR 0001: Work-order media uses single-org scope (board-aligned) + +## Status + +Accepted — 2026-08-04 + +## Context + +SH-116 requires that cross-tenant, unauthorized, and out-of-scope media access +be rejected without metadata disclosure. The repository hard rule +(`ARCHITECTURE_AND_CODE_QUALITY.md` §2) requires **server-derived tenant +scope**: filtering by tenant/customer/owner comes from the authenticated +principal and the data layer, never from client-supplied body/query as the +source of truth. + +The work-order domain does not model `TenantId` / `CustomerId` on `WorkOrder` +or on JWT claims. The board, search, and detail paths already treat the +deployment as a single organization: staff see any work order that passes +`ApplyBaseScope` (non-deleted, non-template). Introducing a multi-tenant key +would require product modeling plus a schema migration, which is out of scope +for the SH-116 media contract. + +## Decision + +Work-order media authorization matches the board: + +1. **Organization / “tenant” boundary** = `ApplyBaseScope` in the data layer + (`istemplate != true` and not deleted). Lookups outside that set resolve as + missing → `NotFound` / null (no disclosure). +2. **Authorization at service entry** from claims: staff roles + (`Admin`, `Manager`, `Dispatcher`, `Supervisor`) may read/mutate any + in-org work order; role `User` (technician) only when + `WorkOrder.AssignTo == actorId`; delete remains staff-only. +3. Scope is never taken from request body or query as the trust source; + `actorId` and roles come from the authenticated principal. + +This satisfies server-derived scope for the current single-org deployment. +True multi-tenant isolation remains deferred until product models a tenant key +and emits a matching claim. + +## Consequences + +- Out-of-organization-scope tests cover deleted, template, and missing work + order ids (GET and mutations) as the single-org analogue of SH-116 + “cross-tenant” rejection. +- Staff org-wide access by numeric id remains intentional and aligned with the + board; it is not a substitute for future multi-tenant keys. +- Reviewers of media PRs should cite this ADR when evaluating tenant-scope + findings against SH-116. + +## Excepted / clarified rule + +Hard rule: **server-derived tenant scope** +(`ARCHITECTURE_AND_CODE_QUALITY.md` §2). + +Clarification: in the work-order domain, the server-derived scope key is the +organization boundary enforced by `ApplyBaseScope` plus claims-derived +role/assignee — not a `TenantId`/`CustomerId` column. Absence of a multi-tenant +key is an accepted product/architecture state until superseded. + +## Review / expiry + +Re-review by **2027-02-04**, or earlier if product introduces +`TenantId`/`CustomerId` on work orders or JWT claims. + +## References + +- SH-116 — Completion document: fields + media categorization +- PR that relies on this ADR: Sea-Haven-Industries/shoc-backend#47 +- `WorkOrderBoardQueryFilters.ApplyBaseScope` +- `WorkOrderMediaAuthorization`