mirror of
https://github.com/Sea-Haven-Industries/shoc-backend.git
synced 2026-09-30 06:03:12 +00:00
~docs(work-orders): demote ADR 0001 to Proposed pending CODEOWNERS [SH-116]
This commit is contained in:
parent
8ff4ab1742
commit
e572786b1c
3 changed files with 100 additions and 47 deletions
|
|
@ -6,11 +6,12 @@ namespace SeaHaven.Services.Helpers
|
||||||
{
|
{
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Claims-derived authorization for work-order media read/mutations.
|
/// Claims-derived authorization for work-order media read/mutations.
|
||||||
/// Organization scope matches the board: callers must already have resolved
|
/// Callers must already have resolved the work order via
|
||||||
/// the work order via <c>ApplyBaseScope</c> (non-deleted, non-template).
|
/// <c>ApplyBaseScope</c> (non-deleted, non-template). Staff may access any
|
||||||
/// Staff may access any in-org work order; technicians only when
|
/// such work order; technicians only when <see cref="WorkOrder.AssignTo"/>
|
||||||
/// <see cref="WorkOrder.AssignTo"/> matches the actor. See
|
/// matches the actor. This is <b>not</b> tenant/customer isolation — that
|
||||||
/// <c>docs/adr/0001-work-order-single-org-scope.md</c>.
|
/// hard rule remains open pending
|
||||||
|
/// <c>docs/adr/0001-work-order-single-org-scope.md</c> (Proposed).
|
||||||
/// </summary>
|
/// </summary>
|
||||||
public static class WorkOrderMediaAuthorization
|
public static class WorkOrderMediaAuthorization
|
||||||
{
|
{
|
||||||
|
|
@ -54,8 +55,8 @@ namespace SeaHaven.Services.Helpers
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Caller-scope check after a base-scoped (in-org) work-order load.
|
/// Caller-scope check after an <c>ApplyBaseScope</c> work-order load.
|
||||||
/// Staff: any in-org work order already resolved via <c>ApplyBaseScope</c>.
|
/// Staff: any base-scoped work order (interim; no tenant key — ADR 0001 Proposed).
|
||||||
/// Technician: only when assigned to the caller.
|
/// Technician: only when assigned to the caller.
|
||||||
/// Out of caller scope → NotFound (no disclosure).
|
/// Out of caller scope → NotFound (no disclosure).
|
||||||
/// </summary>
|
/// </summary>
|
||||||
|
|
@ -64,9 +65,9 @@ namespace SeaHaven.Services.Helpers
|
||||||
string actorId,
|
string actorId,
|
||||||
WorkOrder workOrder)
|
WorkOrder workOrder)
|
||||||
{
|
{
|
||||||
// Organization boundary is enforced by the data-layer ApplyBaseScope
|
// ApplyBaseScope already filtered the load. Staff may reach any such
|
||||||
// load that produced <paramref name="workOrder"/>. Staff are authorized
|
// work order (board-aligned interim). Not a tenant boundary — see
|
||||||
// for any in-org work order (same as the board). See ADR 0001.
|
// ADR 0001 (Proposed).
|
||||||
if (IsStaff(user))
|
if (IsStaff(user))
|
||||||
return;
|
return;
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -732,11 +732,11 @@ public class WorkOrderMediaServiceTests
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Out-of-organization-scope (single-org analogue of SH-116 cross-tenant):
|
/// ApplyBaseScope filter: deleted work orders → null / no media disclosure.
|
||||||
/// deleted work orders are outside ApplyBaseScope → null / no media disclosure.
|
/// (Not a cross-tenant test — no tenant key exists yet.)
|
||||||
/// </summary>
|
/// </summary>
|
||||||
[Fact]
|
[Fact]
|
||||||
public async Task GetMedia_StaffOnDeletedWorkOrder_ReturnsNull_OutOfOrgScope()
|
public async Task GetMedia_StaffOnDeletedWorkOrder_ReturnsNull_OutsideBaseScope()
|
||||||
{
|
{
|
||||||
var (context, service) = CreateSut();
|
var (context, service) = CreateSut();
|
||||||
context.workOrders.Add(new WorkOrder
|
context.workOrders.Add(new WorkOrder
|
||||||
|
|
@ -753,10 +753,10 @@ public class WorkOrderMediaServiceTests
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Out-of-organization-scope: template work orders are outside ApplyBaseScope.
|
/// ApplyBaseScope filter: template work orders are excluded.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
[Fact]
|
[Fact]
|
||||||
public async Task GetMedia_StaffOnTemplateWorkOrder_ReturnsNull_OutOfOrgScope()
|
public async Task GetMedia_StaffOnTemplateWorkOrder_ReturnsNull_OutsideBaseScope()
|
||||||
{
|
{
|
||||||
var (context, service) = CreateSut();
|
var (context, service) = CreateSut();
|
||||||
context.workOrders.Add(new WorkOrder
|
context.workOrders.Add(new WorkOrder
|
||||||
|
|
@ -773,7 +773,7 @@ public class WorkOrderMediaServiceTests
|
||||||
}
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public async Task GetMedia_MissingWorkOrder_ReturnsNull_OutOfOrgScope()
|
public async Task GetMedia_MissingWorkOrder_ReturnsNull_OutsideBaseScope()
|
||||||
{
|
{
|
||||||
var (_, service) = CreateSut();
|
var (_, service) = CreateSut();
|
||||||
|
|
||||||
|
|
@ -989,10 +989,10 @@ public class WorkOrderMediaServiceTests
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Out-of-organization-scope mutation: missing id → NotFound (no disclosure).
|
/// ApplyBaseScope mutation: missing id → NotFound (no disclosure).
|
||||||
/// </summary>
|
/// </summary>
|
||||||
[Fact]
|
[Fact]
|
||||||
public async Task EnsureCanMutateMedia_MissingWorkOrder_ThrowsNotFound_OutOfOrgScope()
|
public async Task EnsureCanMutateMedia_MissingWorkOrder_ThrowsNotFound_OutsideBaseScope()
|
||||||
{
|
{
|
||||||
var (_, service) = CreateSut();
|
var (_, service) = CreateSut();
|
||||||
var ex = await Assert.ThrowsAsync<WorkOrderBoardValidationException>(() =>
|
var ex = await Assert.ThrowsAsync<WorkOrderBoardValidationException>(() =>
|
||||||
|
|
@ -1001,10 +1001,10 @@ public class WorkOrderMediaServiceTests
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Out-of-organization-scope mutation: deleted WO outside ApplyBaseScope → NotFound.
|
/// ApplyBaseScope mutation: deleted WO → NotFound.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
[Fact]
|
[Fact]
|
||||||
public async Task EnsureCanMutateMedia_DeletedWorkOrder_ThrowsNotFound_OutOfOrgScope()
|
public async Task EnsureCanMutateMedia_DeletedWorkOrder_ThrowsNotFound_OutsideBaseScope()
|
||||||
{
|
{
|
||||||
var (context, service) = CreateSut();
|
var (context, service) = CreateSut();
|
||||||
context.workOrders.Add(new WorkOrder
|
context.workOrders.Add(new WorkOrder
|
||||||
|
|
@ -1023,10 +1023,10 @@ public class WorkOrderMediaServiceTests
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Out-of-organization-scope mutation: template WO outside ApplyBaseScope → NotFound.
|
/// ApplyBaseScope mutation: template WO → NotFound.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
[Fact]
|
[Fact]
|
||||||
public async Task EnsureCanMutateMedia_TemplateWorkOrder_ThrowsNotFound_OutOfOrgScope()
|
public async Task EnsureCanMutateMedia_TemplateWorkOrder_ThrowsNotFound_OutsideBaseScope()
|
||||||
{
|
{
|
||||||
var (context, service) = CreateSut();
|
var (context, service) = CreateSut();
|
||||||
context.workOrders.Add(new WorkOrder
|
context.workOrders.Add(new WorkOrder
|
||||||
|
|
|
||||||
|
|
@ -2,7 +2,16 @@
|
||||||
|
|
||||||
## Status
|
## 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
|
## 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
|
principal and the data layer, never from client-supplied body/query as the
|
||||||
source of truth.
|
source of truth.
|
||||||
|
|
||||||
The work-order domain does not model `TenantId` / `CustomerId` on `WorkOrder`
|
Verified against the current codebase:
|
||||||
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
|
- 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
|
(`istemplate != true` and not deleted). Lookups outside that set resolve as
|
||||||
missing → `NotFound` / null (no disclosure).
|
missing → `NotFound` / null (no disclosure).
|
||||||
2. **Authorization at service entry** from claims: staff roles
|
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;
|
3. Scope is never taken from request body or query as the trust source;
|
||||||
`actorId` and roles come from the authenticated principal.
|
`actorId` and roles come from the authenticated principal.
|
||||||
|
|
||||||
This satisfies server-derived scope for the current single-org deployment.
|
This is **not** a substitute for SH-116 cross-tenant isolation. It is the
|
||||||
True multi-tenant isolation remains deferred until product models a tenant key
|
interim behavior while the exception is under review or real tenant keys land.
|
||||||
and emits a matching claim.
|
|
||||||
|
## 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
|
## Consequences
|
||||||
|
|
||||||
- Out-of-organization-scope tests cover deleted, template, and missing work
|
- Tests for deleted, template, and missing work-order ids cover
|
||||||
order ids (GET and mutations) as the single-org analogue of SH-116
|
`ApplyBaseScope` rejection only — they must not be labeled as SH-116
|
||||||
“cross-tenant” rejection.
|
cross-tenant coverage.
|
||||||
- Staff org-wide access by numeric id remains intentional and aligned with the
|
- Staff org-wide access by numeric id remains intentional and aligned with the
|
||||||
board; it is not a substitute for future multi-tenant keys.
|
board for this interim state.
|
||||||
- Reviewers of media PRs should cite this ADR when evaluating tenant-scope
|
- Merge of PR #47 that relies on this ADR requires either:
|
||||||
findings against SH-116.
|
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**
|
Hard rule: **server-derived tenant scope**
|
||||||
(`ARCHITECTURE_AND_CODE_QUALITY.md` §2).
|
(`ARCHITECTURE_AND_CODE_QUALITY.md` §2).
|
||||||
|
|
||||||
Clarification: in the work-order domain, the server-derived scope key is the
|
Requested clarification while this ADR is Proposed/Accepted: in the work-order
|
||||||
organization boundary enforced by `ApplyBaseScope` plus claims-derived
|
domain, the interim server-derived scope key is the organization boundary
|
||||||
role/assignee — not a `TenantId`/`CustomerId` column. Absence of a multi-tenant
|
enforced by `ApplyBaseScope` plus claims-derived role/assignee — not a
|
||||||
key is an accepted product/architecture state until superseded.
|
`TenantId`/`CustomerId` column. Absence of a multi-tenant key is a temporary
|
||||||
|
product/architecture gap until superseded by the path above.
|
||||||
|
|
||||||
## Review / expiry
|
## Review / expiry
|
||||||
|
|
||||||
Re-review by **2027-02-04**, or earlier if product introduces
|
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
|
## References
|
||||||
|
|
||||||
- SH-116 — Completion document: fields + media categorization
|
- 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
|
- PR that relies on this ADR: Sea-Haven-Industries/shoc-backend#47
|
||||||
- `WorkOrderBoardQueryFilters.ApplyBaseScope`
|
- `WorkOrderBoardQueryFilters.ApplyBaseScope`
|
||||||
- `WorkOrderMediaAuthorization`
|
- `WorkOrderMediaAuthorization`
|
||||||
|
- `ARCHITECTURE_AND_CODE_QUALITY.md` §2, §10
|
||||||
|
- `REVIEW_AND_PR_FRAMEWORK.md` §7, §8
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue