From e572786b1ce0211d72a433c75c2d06397e50a379 Mon Sep 17 00:00:00 2001 From: Arthur Bassi Date: Wed, 5 Aug 2026 09:57:14 -0300 Subject: [PATCH] ~docs(work-orders): demote ADR 0001 to Proposed pending CODEOWNERS [SH-116] --- .../Helpers/WorkOrderMediaAuthorization.cs | 21 ++-- .../WorkOrderPhase6Tests.cs | 24 ++--- docs/adr/0001-work-order-single-org-scope.md | 102 +++++++++++++----- 3 files changed, 100 insertions(+), 47 deletions(-) diff --git a/SeaHaven.Services/Helpers/WorkOrderMediaAuthorization.cs b/SeaHaven.Services/Helpers/WorkOrderMediaAuthorization.cs index 2af3080..0dd25f7 100644 --- a/SeaHaven.Services/Helpers/WorkOrderMediaAuthorization.cs +++ b/SeaHaven.Services/Helpers/WorkOrderMediaAuthorization.cs @@ -6,11 +6,12 @@ namespace SeaHaven.Services.Helpers { /// /// Claims-derived authorization for work-order media read/mutations. - /// 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. + /// Callers must already have resolved the work order via + /// ApplyBaseScope (non-deleted, non-template). Staff may access any + /// such work order; technicians only when + /// matches the actor. This is not tenant/customer isolation — that + /// hard rule remains open pending + /// docs/adr/0001-work-order-single-org-scope.md (Proposed). /// public static class WorkOrderMediaAuthorization { @@ -54,8 +55,8 @@ namespace SeaHaven.Services.Helpers } /// - /// Caller-scope check after a base-scoped (in-org) work-order load. - /// Staff: any in-org work order already resolved via ApplyBaseScope. + /// Caller-scope check after an ApplyBaseScope work-order load. + /// Staff: any base-scoped work order (interim; no tenant key — ADR 0001 Proposed). /// Technician: only when assigned to the caller. /// Out of caller scope → NotFound (no disclosure). /// @@ -64,9 +65,9 @@ namespace SeaHaven.Services.Helpers 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. + // ApplyBaseScope already filtered the load. Staff may reach any such + // work order (board-aligned interim). Not a tenant boundary — see + // ADR 0001 (Proposed). if (IsStaff(user)) return; diff --git a/SeaHavenIndustries.Tests/WorkOrderPhase6Tests.cs b/SeaHavenIndustries.Tests/WorkOrderPhase6Tests.cs index 0d2f6fc..d1e46bc 100644 --- a/SeaHavenIndustries.Tests/WorkOrderPhase6Tests.cs +++ b/SeaHavenIndustries.Tests/WorkOrderPhase6Tests.cs @@ -732,11 +732,11 @@ public class WorkOrderMediaServiceTests } /// - /// Out-of-organization-scope (single-org analogue of SH-116 cross-tenant): - /// deleted work orders are outside ApplyBaseScope → null / no media disclosure. + /// ApplyBaseScope filter: deleted work orders → null / no media disclosure. + /// (Not a cross-tenant test — no tenant key exists yet.) /// [Fact] - public async Task GetMedia_StaffOnDeletedWorkOrder_ReturnsNull_OutOfOrgScope() + public async Task GetMedia_StaffOnDeletedWorkOrder_ReturnsNull_OutsideBaseScope() { var (context, service) = CreateSut(); context.workOrders.Add(new WorkOrder @@ -753,10 +753,10 @@ public class WorkOrderMediaServiceTests } /// - /// Out-of-organization-scope: template work orders are outside ApplyBaseScope. + /// ApplyBaseScope filter: template work orders are excluded. /// [Fact] - public async Task GetMedia_StaffOnTemplateWorkOrder_ReturnsNull_OutOfOrgScope() + public async Task GetMedia_StaffOnTemplateWorkOrder_ReturnsNull_OutsideBaseScope() { var (context, service) = CreateSut(); context.workOrders.Add(new WorkOrder @@ -773,7 +773,7 @@ public class WorkOrderMediaServiceTests } [Fact] - public async Task GetMedia_MissingWorkOrder_ReturnsNull_OutOfOrgScope() + public async Task GetMedia_MissingWorkOrder_ReturnsNull_OutsideBaseScope() { var (_, service) = CreateSut(); @@ -989,10 +989,10 @@ public class WorkOrderMediaServiceTests } /// - /// Out-of-organization-scope mutation: missing id → NotFound (no disclosure). + /// ApplyBaseScope mutation: missing id → NotFound (no disclosure). /// [Fact] - public async Task EnsureCanMutateMedia_MissingWorkOrder_ThrowsNotFound_OutOfOrgScope() + public async Task EnsureCanMutateMedia_MissingWorkOrder_ThrowsNotFound_OutsideBaseScope() { var (_, service) = CreateSut(); var ex = await Assert.ThrowsAsync(() => @@ -1001,10 +1001,10 @@ public class WorkOrderMediaServiceTests } /// - /// Out-of-organization-scope mutation: deleted WO outside ApplyBaseScope → NotFound. + /// ApplyBaseScope mutation: deleted WO → NotFound. /// [Fact] - public async Task EnsureCanMutateMedia_DeletedWorkOrder_ThrowsNotFound_OutOfOrgScope() + public async Task EnsureCanMutateMedia_DeletedWorkOrder_ThrowsNotFound_OutsideBaseScope() { var (context, service) = CreateSut(); context.workOrders.Add(new WorkOrder @@ -1023,10 +1023,10 @@ public class WorkOrderMediaServiceTests } /// - /// Out-of-organization-scope mutation: template WO outside ApplyBaseScope → NotFound. + /// ApplyBaseScope mutation: template WO → NotFound. /// [Fact] - public async Task EnsureCanMutateMedia_TemplateWorkOrder_ThrowsNotFound_OutOfOrgScope() + public async Task EnsureCanMutateMedia_TemplateWorkOrder_ThrowsNotFound_OutsideBaseScope() { 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 index 4abacc7..eadaaf9 100644 --- a/docs/adr/0001-work-order-single-org-scope.md +++ b/docs/adr/0001-work-order-single-org-scope.md @@ -2,7 +2,16 @@ ## Status -Accepted — 2026-08-04 +**Proposed — awaiting approval** (2026-08-05). + +This ADR is **not** self-accepted by the PR author. Approval is required from: + +- CODEOWNERS team `@Sea-Haven-Industries/internal-dev` (see `.github/CODEOWNERS`) +- SH-116 product owner + +Until approved (or until real tenant enforcement lands), this document records a +**pending** exception request against the hard rule below — it does not waive +the rule on its own. ## Context @@ -13,18 +22,26 @@ 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. +Verified against the current codebase: -## Decision +- There is no `TenantId` / `CustomerId` column on `WorkOrder`, + `WorkOrderAttachments`, or `ApplicationUser`. +- `WorkOrder.Customer` is free-text (`nvarchar`), not a FK. +- JWT issuance (`AuthenticationService.GetToken`) emits only `Name`, + `NameIdentifier`, `Jti`, and `Role` — no tenant/customer claim. +- Board, search, and detail already treat the deployment as a single + organization via `ApplyBaseScope` (non-deleted, non-template). -Work-order media authorization matches the board: +Introducing a multi-tenant key requires product modeling plus a schema +migration, which is out of scope for the SH-116 media contract and blocked by +`AGENTS.md` (no migrations / product behavior without explicit instruction). -1. **Organization / “tenant” boundary** = `ApplyBaseScope` in the data layer +## Decision (proposed) + +Until real tenant enforcement exists, work-order media authorization matches +the board: + +1. **Organization 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 @@ -34,38 +51,73 @@ Work-order media authorization matches the board: 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. +This is **not** a substitute for SH-116 cross-tenant isolation. It is the +interim behavior while the exception is under review or real tenant keys land. + +## Accepted risk (while Proposed / if Accepted) + +Any authenticated **staff** principal who knows a numeric work-order id can +read or mutate media for that work order, provided it passes `ApplyBaseScope`. +There is no server-derived tenant/customer boundary separating staff access +across customers. Deleted / template / missing ids are **not** a cross-tenant +test; they only prove the `ApplyBaseScope` filter. ## 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. +- Tests for deleted, template, and missing work-order ids cover + `ApplyBaseScope` rejection only — they must not be labeled as SH-116 + cross-tenant coverage. - 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. + board for this interim state. +- Merge of PR #47 that relies on this ADR requires either: + 1. formal approval of this ADR by the approvers listed in Status, or + 2. landing of real tenant enforcement (see below). -## Excepted / clarified rule +## Path to real enforcement + +Candidate design for the deferred multi-tenant work (tracked in the linked +Jira ticket): + +1. **Tenant key** — reuse the existing `Accounts` entity (CRM customer) as the + customer boundary; add `WorkOrder.AccountId` (FK) and + `ApplicationUser.AccountId` (or an equivalent membership table). +2. **Claim** — emit a server-derived `account_id` (or equivalent) claim in + `AuthenticationService.GetToken` from the authenticated user’s account + membership; never accept account id from body/query as trust source. +3. **Data filter** — extend `ApplyBaseScope` (or a sibling filter) so board, + detail, search, and media loads restrict by the claim-derived account + scope; staff may still be broader if product defines org-wide roles, but + that must be an explicit claims rule, not “any numeric id”. +4. **Backfill** — map free-text `WorkOrder.Customer` strings to `Accounts` + rows where possible; unresolved rows need a product decision (block, + orphan bucket, or manual remapping). +5. **Tests** — add true cross-tenant rejection tests (staff/tech of account A + cannot read or mutate media of a work order owned by account B) with stable + `NotFound` / `Forbidden` and no metadata disclosure. + +## Excepted 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. +Requested clarification while this ADR is Proposed/Accepted: in the work-order +domain, the interim 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 a temporary +product/architecture gap until superseded by the path above. ## Review / expiry Re-review by **2027-02-04**, or earlier if product introduces -`TenantId`/`CustomerId` on work orders or JWT claims. +`TenantId`/`CustomerId`/`AccountId` on work orders or JWT claims, or when the +linked multi-tenant ticket closes. ## References - SH-116 — Completion document: fields + media categorization +- SH-221 — Server-derived tenant/customer scope for Work Order domain (deferred enforcement) - PR that relies on this ADR: Sea-Haven-Industries/shoc-backend#47 - `WorkOrderBoardQueryFilters.ApplyBaseScope` - `WorkOrderMediaAuthorization` +- `ARCHITECTURE_AND_CODE_QUALITY.md` §2, §10 +- `REVIEW_AND_PR_FRAMEWORK.md` §7, §8