mirror of
https://github.com/Sea-Haven-Industries/shoc-backend.git
synced 2026-10-01 11:03:14 +00:00
!feat(work-orders): enforce media account scope and AddMedia freshness [SH-221]
This commit is contained in:
parent
e572786b1c
commit
fdc315d8fe
16 changed files with 4308 additions and 132 deletions
|
|
@ -136,6 +136,24 @@ namespace Data.SeaHavenIndustries
|
|||
.HasForeignKey(w => w.PrimaryDispatchId)
|
||||
.OnDelete(DeleteBehavior.Restrict);
|
||||
|
||||
builder.Entity<WorkOrder>()
|
||||
.HasOne(w => w.Account)
|
||||
.WithMany()
|
||||
.HasForeignKey(w => w.AccountId)
|
||||
.OnDelete(DeleteBehavior.Restrict);
|
||||
|
||||
builder.Entity<WorkOrder>()
|
||||
.HasIndex(w => w.AccountId);
|
||||
|
||||
builder.Entity<ApplicationUser>()
|
||||
.HasOne(u => u.Account)
|
||||
.WithMany()
|
||||
.HasForeignKey(u => u.AccountId)
|
||||
.OnDelete(DeleteBehavior.Restrict);
|
||||
|
||||
builder.Entity<ApplicationUser>()
|
||||
.HasIndex(u => u.AccountId);
|
||||
|
||||
builder.Entity<VendorCompany>()
|
||||
.HasIndex(c => c.NormalizedName)
|
||||
.IsUnique();
|
||||
|
|
@ -284,6 +302,10 @@ namespace Data.SeaHavenIndustries
|
|||
public string? Initials { get; set; }
|
||||
public string? Color { get; set; }
|
||||
public int? Type { get; set; } // 1 for users 0 for admin
|
||||
/// <summary>Optional CRM account membership for server-derived media scope (SH-221).</summary>
|
||||
public int? AccountId { get; set; }
|
||||
[ForeignKey(nameof(AccountId))]
|
||||
public virtual Accounts? Account { get; set; }
|
||||
public ICollection<Template>? Templates { get; set; }
|
||||
public ICollection<WorkOrder>? WorkOrders { get; set; }
|
||||
}
|
||||
|
|
|
|||
3814
Data.SeaHavenIndustries/Migrations/20260806123838_SH221_WorkOrderAccountScope.Designer.cs
generated
Normal file
3814
Data.SeaHavenIndustries/Migrations/20260806123838_SH221_WorkOrderAccountScope.Designer.cs
generated
Normal file
File diff suppressed because it is too large
Load diff
|
|
@ -0,0 +1,110 @@
|
|||
using Microsoft.EntityFrameworkCore.Migrations;
|
||||
|
||||
#nullable disable
|
||||
|
||||
namespace Data.SeaHavenIndustries.Migrations
|
||||
{
|
||||
/// <inheritdoc />
|
||||
public partial class SH221_WorkOrderAccountScope : Migration
|
||||
{
|
||||
/// <inheritdoc />
|
||||
protected override void Up(MigrationBuilder migrationBuilder)
|
||||
{
|
||||
migrationBuilder.AddColumn<int>(
|
||||
name: "AccountId",
|
||||
table: "workOrders",
|
||||
type: "int",
|
||||
nullable: true);
|
||||
|
||||
migrationBuilder.AddColumn<int>(
|
||||
name: "AccountId",
|
||||
table: "AspNetUsers",
|
||||
type: "int",
|
||||
nullable: true);
|
||||
|
||||
migrationBuilder.CreateIndex(
|
||||
name: "IX_workOrders_AccountId",
|
||||
table: "workOrders",
|
||||
column: "AccountId");
|
||||
|
||||
migrationBuilder.CreateIndex(
|
||||
name: "IX_AspNetUsers_AccountId",
|
||||
table: "AspNetUsers",
|
||||
column: "AccountId");
|
||||
|
||||
migrationBuilder.AddForeignKey(
|
||||
name: "FK_AspNetUsers_Accounts_AccountId",
|
||||
table: "AspNetUsers",
|
||||
column: "AccountId",
|
||||
principalTable: "Accounts",
|
||||
principalColumn: "Id",
|
||||
onDelete: ReferentialAction.Restrict);
|
||||
|
||||
migrationBuilder.AddForeignKey(
|
||||
name: "FK_workOrders_Accounts_AccountId",
|
||||
table: "workOrders",
|
||||
column: "AccountId",
|
||||
principalTable: "Accounts",
|
||||
principalColumn: "Id",
|
||||
onDelete: ReferentialAction.Restrict);
|
||||
|
||||
// Best-effort backfill: unique Accounts.Name match against WorkOrder.Customer.
|
||||
migrationBuilder.Sql(@"
|
||||
;WITH UniqueAccounts AS (
|
||||
SELECT
|
||||
a.[Id],
|
||||
a.[Name]
|
||||
FROM [Accounts] a
|
||||
WHERE a.[Name] IS NOT NULL
|
||||
AND LTRIM(RTRIM(a.[Name])) <> ''
|
||||
AND (a.[IsDeleted] IS NULL OR a.[IsDeleted] = 0)
|
||||
AND a.[Name] IN (
|
||||
SELECT a2.[Name]
|
||||
FROM [Accounts] a2
|
||||
WHERE a2.[Name] IS NOT NULL
|
||||
AND LTRIM(RTRIM(a2.[Name])) <> ''
|
||||
AND (a2.[IsDeleted] IS NULL OR a2.[IsDeleted] = 0)
|
||||
GROUP BY a2.[Name]
|
||||
HAVING COUNT(*) = 1
|
||||
)
|
||||
)
|
||||
UPDATE wo
|
||||
SET wo.[AccountId] = ua.[Id]
|
||||
FROM [workOrders] wo
|
||||
INNER JOIN UniqueAccounts ua
|
||||
ON ua.[Name] = wo.[Customer]
|
||||
WHERE wo.[AccountId] IS NULL
|
||||
AND wo.[Customer] IS NOT NULL
|
||||
AND LTRIM(RTRIM(wo.[Customer])) <> '';
|
||||
");
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
protected override void Down(MigrationBuilder migrationBuilder)
|
||||
{
|
||||
migrationBuilder.DropForeignKey(
|
||||
name: "FK_AspNetUsers_Accounts_AccountId",
|
||||
table: "AspNetUsers");
|
||||
|
||||
migrationBuilder.DropForeignKey(
|
||||
name: "FK_workOrders_Accounts_AccountId",
|
||||
table: "workOrders");
|
||||
|
||||
migrationBuilder.DropIndex(
|
||||
name: "IX_workOrders_AccountId",
|
||||
table: "workOrders");
|
||||
|
||||
migrationBuilder.DropIndex(
|
||||
name: "IX_AspNetUsers_AccountId",
|
||||
table: "AspNetUsers");
|
||||
|
||||
migrationBuilder.DropColumn(
|
||||
name: "AccountId",
|
||||
table: "workOrders");
|
||||
|
||||
migrationBuilder.DropColumn(
|
||||
name: "AccountId",
|
||||
table: "AspNetUsers");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -188,6 +188,9 @@ namespace Data.SeaHavenIndustries.Migrations
|
|||
b.Property<int>("AccessFailedCount")
|
||||
.HasColumnType("int");
|
||||
|
||||
b.Property<int?>("AccountId")
|
||||
.HasColumnType("int");
|
||||
|
||||
b.Property<string>("Color")
|
||||
.HasColumnType("nvarchar(max)");
|
||||
|
||||
|
|
@ -270,6 +273,8 @@ namespace Data.SeaHavenIndustries.Migrations
|
|||
|
||||
b.HasKey("Id");
|
||||
|
||||
b.HasIndex("AccountId");
|
||||
|
||||
b.HasIndex("NormalizedEmail")
|
||||
.HasDatabaseName("EmailIndex");
|
||||
|
||||
|
|
@ -2314,6 +2319,9 @@ namespace Data.SeaHavenIndustries.Migrations
|
|||
|
||||
SqlServerPropertyBuilderExtensions.UseIdentityColumn(b.Property<int>("Id"));
|
||||
|
||||
b.Property<int?>("AccountId")
|
||||
.HasColumnType("int");
|
||||
|
||||
b.Property<string>("AfterPhotoAttachment")
|
||||
.HasColumnType("nvarchar(max)");
|
||||
|
||||
|
|
@ -2553,6 +2561,8 @@ namespace Data.SeaHavenIndustries.Migrations
|
|||
|
||||
b.HasKey("Id");
|
||||
|
||||
b.HasIndex("AccountId");
|
||||
|
||||
b.HasIndex("ExternalWorkOrderId")
|
||||
.IsUnique()
|
||||
.HasFilter("[ExternalWorkOrderId] IS NOT NULL AND [ExternalWorkOrderId] <> ''");
|
||||
|
|
@ -3107,6 +3117,16 @@ namespace Data.SeaHavenIndustries.Migrations
|
|||
b.Navigation("Contact");
|
||||
});
|
||||
|
||||
modelBuilder.Entity("Data.SeaHavenIndustries.ApplicationUser", b =>
|
||||
{
|
||||
b.HasOne("Data.SeaHavenIndustries.Accounts", "Account")
|
||||
.WithMany()
|
||||
.HasForeignKey("AccountId")
|
||||
.OnDelete(DeleteBehavior.Restrict);
|
||||
|
||||
b.Navigation("Account");
|
||||
});
|
||||
|
||||
modelBuilder.Entity("Data.SeaHavenIndustries.Assets", b =>
|
||||
{
|
||||
b.HasOne("Data.SeaHavenIndustries.Accounts", "Account")
|
||||
|
|
@ -3506,6 +3526,11 @@ namespace Data.SeaHavenIndustries.Migrations
|
|||
|
||||
modelBuilder.Entity("Data.SeaHavenIndustries.WorkOrder", b =>
|
||||
{
|
||||
b.HasOne("Data.SeaHavenIndustries.Accounts", "Account")
|
||||
.WithMany()
|
||||
.HasForeignKey("AccountId")
|
||||
.OnDelete(DeleteBehavior.Restrict);
|
||||
|
||||
b.HasOne("Data.SeaHavenIndustries.ApplicationUser", "AssignToUser")
|
||||
.WithMany("WorkOrders")
|
||||
.HasForeignKey("AssignTo")
|
||||
|
|
@ -3521,6 +3546,8 @@ namespace Data.SeaHavenIndustries.Migrations
|
|||
.HasForeignKey("PrimaryDispatchId")
|
||||
.OnDelete(DeleteBehavior.Restrict);
|
||||
|
||||
b.Navigation("Account");
|
||||
|
||||
b.Navigation("AssignToUser");
|
||||
|
||||
b.Navigation("Locations");
|
||||
|
|
|
|||
|
|
@ -23,6 +23,10 @@ namespace Data.SeaHavenIndustries
|
|||
public string? WorkerOrderTitle { get; set; }
|
||||
public string? Description { get; set; }
|
||||
public string? Customer { get; set; }
|
||||
/// <summary>CRM account (customer) boundary for server-derived media/tenant scope (SH-221).</summary>
|
||||
public int? AccountId { get; set; }
|
||||
[ForeignKey(nameof(AccountId))]
|
||||
public virtual Accounts? Account { get; set; }
|
||||
public string? SiteCode { get; set; }
|
||||
public string? Building { get; set; }
|
||||
public string? Severity { get; set; }
|
||||
|
|
|
|||
|
|
@ -8,6 +8,18 @@ namespace SeaHaven.DataServices.Helpers
|
|||
public static IQueryable<WorkOrder> ApplyBaseScope(IQueryable<WorkOrder> query)
|
||||
=> query.Where(w => w.istemplate != true && (w.IsDeleted != true || w.IsDeleted == null));
|
||||
|
||||
/// <summary>
|
||||
/// When <paramref name="accountId"/> is set, restrict to that CRM account.
|
||||
/// When null, no account filter (org-wide staff without an account claim).
|
||||
/// </summary>
|
||||
public static IQueryable<WorkOrder> ApplyAccountScope(IQueryable<WorkOrder> query, int? accountId)
|
||||
{
|
||||
if (!accountId.HasValue)
|
||||
return query;
|
||||
|
||||
return query.Where(w => w.AccountId == accountId.Value);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Applies assignee filters. When <paramref name="myWorkOrders"/> is true and
|
||||
/// <paramref name="currentUserId"/> is set, that takes precedence and
|
||||
|
|
|
|||
|
|
@ -14,8 +14,13 @@ namespace SeaHaven.DataServices.Implementation
|
|||
_context = context;
|
||||
}
|
||||
|
||||
public Task<bool> ExistsAsync(int workOrderId, CancellationToken cancellationToken = default)
|
||||
=> WorkOrderBoardQueryFilters.ApplyBaseScope(_context.workOrders.AsNoTracking())
|
||||
public Task<bool> ExistsAsync(
|
||||
int workOrderId,
|
||||
CancellationToken cancellationToken = default,
|
||||
int? accountId = null)
|
||||
=> WorkOrderBoardQueryFilters.ApplyAccountScope(
|
||||
WorkOrderBoardQueryFilters.ApplyBaseScope(_context.workOrders.AsNoTracking()),
|
||||
accountId)
|
||||
.AnyAsync(w => w.Id == workOrderId, cancellationToken);
|
||||
|
||||
public async Task<WorkOrderDetailExtendedFields?> GetExtendedFieldsAsync(int workOrderId)
|
||||
|
|
@ -90,9 +95,12 @@ namespace SeaHaven.DataServices.Implementation
|
|||
|
||||
public async Task<WorkOrder?> GetWorkOrderForMediaAsync(
|
||||
int workOrderId,
|
||||
CancellationToken cancellationToken = default)
|
||||
CancellationToken cancellationToken = default,
|
||||
int? accountId = null)
|
||||
{
|
||||
return await WorkOrderBoardQueryFilters.ApplyBaseScope(_context.workOrders.AsNoTracking())
|
||||
return await WorkOrderBoardQueryFilters.ApplyAccountScope(
|
||||
WorkOrderBoardQueryFilters.ApplyBaseScope(_context.workOrders.AsNoTracking()),
|
||||
accountId)
|
||||
.FirstOrDefaultAsync(w => w.Id == workOrderId, cancellationToken);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
using Data.SeaHavenIndustries;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
using SeaHaven.DataServices.Helpers;
|
||||
using SeaHaven.DataServices.Interfaces;
|
||||
|
||||
namespace SeaHaven.DataServices.Implementation
|
||||
|
|
@ -13,10 +14,23 @@ namespace SeaHaven.DataServices.Implementation
|
|||
_context = context;
|
||||
}
|
||||
|
||||
public Task<WorkOrder?> GetTrackedWorkOrderAsync(int workOrderId, CancellationToken cancellationToken)
|
||||
=> _context.workOrders.FirstOrDefaultAsync(
|
||||
w => w.Id == workOrderId && w.istemplate != true && (w.IsDeleted != true || w.IsDeleted == null),
|
||||
cancellationToken);
|
||||
public Task<WorkOrder?> GetWorkOrderForMediaAuthAsync(
|
||||
int workOrderId,
|
||||
int? accountId,
|
||||
CancellationToken cancellationToken)
|
||||
=> WorkOrderBoardQueryFilters.ApplyAccountScope(
|
||||
WorkOrderBoardQueryFilters.ApplyBaseScope(_context.workOrders.AsNoTracking()),
|
||||
accountId)
|
||||
.FirstOrDefaultAsync(w => w.Id == workOrderId, cancellationToken);
|
||||
|
||||
public Task<WorkOrder?> GetTrackedWorkOrderAsync(
|
||||
int workOrderId,
|
||||
int? accountId,
|
||||
CancellationToken cancellationToken)
|
||||
=> WorkOrderBoardQueryFilters.ApplyAccountScope(
|
||||
WorkOrderBoardQueryFilters.ApplyBaseScope(_context.workOrders),
|
||||
accountId)
|
||||
.FirstOrDefaultAsync(w => w.Id == workOrderId, cancellationToken);
|
||||
|
||||
public Task<WorkOrderAttachments?> GetTrackedAttachmentAsync(int mediaId, int workOrderId, CancellationToken cancellationToken)
|
||||
=> _context.workOrderAttachments.FirstOrDefaultAsync(
|
||||
|
|
|
|||
|
|
@ -6,7 +6,10 @@ namespace SeaHaven.DataServices.Interfaces
|
|||
{
|
||||
public interface IWorkOrderDetailDataService
|
||||
{
|
||||
Task<bool> ExistsAsync(int workOrderId, CancellationToken cancellationToken = default);
|
||||
Task<bool> ExistsAsync(
|
||||
int workOrderId,
|
||||
CancellationToken cancellationToken = default,
|
||||
int? accountId = null);
|
||||
Task<WorkOrderDetailExtendedFields?> GetExtendedFieldsAsync(int workOrderId);
|
||||
Task<IReadOnlyList<Comments>> GetCommentsAsync(int workOrderId);
|
||||
Task<IReadOnlyList<WorkOrderAuditLog>> GetAuditLogsAsync(int workOrderId, int? limit = null);
|
||||
|
|
@ -16,7 +19,8 @@ namespace SeaHaven.DataServices.Interfaces
|
|||
Task<IReadOnlyList<DispatchSignoffRow>> GetDispatchSignoffsAsync(int workOrderId);
|
||||
Task<WorkOrder?> GetWorkOrderForMediaAsync(
|
||||
int workOrderId,
|
||||
CancellationToken cancellationToken = default);
|
||||
CancellationToken cancellationToken = default,
|
||||
int? accountId = null);
|
||||
}
|
||||
|
||||
public interface ICompletionDocTemplateDataService
|
||||
|
|
|
|||
|
|
@ -4,7 +4,17 @@ namespace SeaHaven.DataServices.Interfaces
|
|||
{
|
||||
public interface IWorkOrderMediaDataService
|
||||
{
|
||||
Task<WorkOrder?> GetTrackedWorkOrderAsync(int workOrderId, CancellationToken cancellationToken);
|
||||
/// <summary>AsNoTracking base+account scoped load for pre-mutation auth (does not pollute the change tracker).</summary>
|
||||
Task<WorkOrder?> GetWorkOrderForMediaAuthAsync(
|
||||
int workOrderId,
|
||||
int? accountId,
|
||||
CancellationToken cancellationToken);
|
||||
|
||||
Task<WorkOrder?> GetTrackedWorkOrderAsync(
|
||||
int workOrderId,
|
||||
int? accountId,
|
||||
CancellationToken cancellationToken);
|
||||
|
||||
Task<WorkOrderAttachments?> GetTrackedAttachmentAsync(int mediaId, int workOrderId, CancellationToken cancellationToken);
|
||||
void TrackAttachment(WorkOrderAttachments attachment);
|
||||
void SetExpectedWorkOrderVersion(WorkOrder workOrder, byte[] version);
|
||||
|
|
|
|||
9
SeaHaven.Services/Helpers/SeaHavenClaimTypes.cs
Normal file
9
SeaHaven.Services/Helpers/SeaHavenClaimTypes.cs
Normal file
|
|
@ -0,0 +1,9 @@
|
|||
namespace SeaHaven.Services.Helpers
|
||||
{
|
||||
/// <summary>Server-derived claim type names emitted at token issuance.</summary>
|
||||
public static class SeaHavenClaimTypes
|
||||
{
|
||||
/// <summary>CRM account id from <c>ApplicationUser.AccountId</c> (never from request body).</summary>
|
||||
public const string AccountId = "account_id";
|
||||
}
|
||||
}
|
||||
|
|
@ -6,12 +6,12 @@ namespace SeaHaven.Services.Helpers
|
|||
{
|
||||
/// <summary>
|
||||
/// Claims-derived authorization for work-order media read/mutations.
|
||||
/// Callers must already have resolved the work order via
|
||||
/// <c>ApplyBaseScope</c> (non-deleted, non-template). Staff may access any
|
||||
/// such work order; technicians only when <see cref="WorkOrder.AssignTo"/>
|
||||
/// matches the actor. This is <b>not</b> tenant/customer isolation — that
|
||||
/// hard rule remains open pending
|
||||
/// <c>docs/adr/0001-work-order-single-org-scope.md</c> (Proposed).
|
||||
/// Callers must resolve the work order via <c>ApplyBaseScope</c> plus
|
||||
/// <c>ApplyAccountScope</c> when the principal carries
|
||||
/// <see cref="SeaHavenClaimTypes.AccountId"/>. Staff may access any
|
||||
/// resulting work order; technicians only when <see cref="WorkOrder.AssignTo"/>
|
||||
/// matches the actor. Delete is staff-only.
|
||||
/// Staff without an account claim remain org-wide (base scope only).
|
||||
/// </summary>
|
||||
public static class WorkOrderMediaAuthorization
|
||||
{
|
||||
|
|
@ -23,6 +23,15 @@ namespace SeaHaven.Services.Helpers
|
|||
"Supervisor"
|
||||
};
|
||||
|
||||
public static int? ResolveAccountId(ClaimsPrincipal user)
|
||||
{
|
||||
var raw = user?.FindFirstValue(SeaHavenClaimTypes.AccountId);
|
||||
if (string.IsNullOrWhiteSpace(raw))
|
||||
return null;
|
||||
|
||||
return int.TryParse(raw, out var accountId) ? accountId : null;
|
||||
}
|
||||
|
||||
public static void EnsureCanRead(ClaimsPrincipal user, string? actorId)
|
||||
{
|
||||
EnsureAuthenticated(user, actorId, "You are not allowed to view work order media.");
|
||||
|
|
@ -55,8 +64,8 @@ namespace SeaHaven.Services.Helpers
|
|||
}
|
||||
|
||||
/// <summary>
|
||||
/// Caller-scope check after an <c>ApplyBaseScope</c> work-order load.
|
||||
/// Staff: any base-scoped work order (interim; no tenant key — ADR 0001 Proposed).
|
||||
/// Caller-scope check after a base+account scoped work-order load.
|
||||
/// Staff: any resulting work order.
|
||||
/// Technician: only when assigned to the caller.
|
||||
/// Out of caller scope → NotFound (no disclosure).
|
||||
/// </summary>
|
||||
|
|
@ -65,9 +74,6 @@ namespace SeaHaven.Services.Helpers
|
|||
string actorId,
|
||||
WorkOrder workOrder)
|
||||
{
|
||||
// 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;
|
||||
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ using Microsoft.IdentityModel.Tokens;
|
|||
using SeaHaven.DataServices.Interfaces;
|
||||
using SeaHaven.Services.Configuration;
|
||||
using SeaHaven.Services.DTOs;
|
||||
using SeaHaven.Services.Helpers;
|
||||
using SeaHaven.Services.Interfaces;
|
||||
using System.IdentityModel.Tokens.Jwt;
|
||||
using System.Security.Claims;
|
||||
|
|
@ -50,6 +51,12 @@ namespace SeaHaven.Services.Implementation
|
|||
{
|
||||
authClaims.Add(new Claim(ClaimTypes.Role, userRole));
|
||||
}
|
||||
if (user.AccountId.HasValue)
|
||||
{
|
||||
authClaims.Add(new Claim(
|
||||
SeaHavenClaimTypes.AccountId,
|
||||
user.AccountId.Value.ToString()));
|
||||
}
|
||||
var token = GetToken(authClaims);
|
||||
return new LoginResultDTO
|
||||
{
|
||||
|
|
|
|||
|
|
@ -33,13 +33,12 @@ namespace SeaHaven.Services.Implementation
|
|||
CancellationToken cancellationToken = default)
|
||||
{
|
||||
WorkOrderMediaAuthorization.EnsureCanRead(user, actorId);
|
||||
var accountId = WorkOrderMediaAuthorization.ResolveAccountId(user);
|
||||
|
||||
// Organization scope: ApplyBaseScope via Exists / GetWorkOrderForMedia.
|
||||
// Outside org (deleted/template/missing) → null (no disclosure). ADR 0001.
|
||||
if (!await _detailData.ExistsAsync(workOrderId, cancellationToken))
|
||||
if (!await _detailData.ExistsAsync(workOrderId, cancellationToken, accountId))
|
||||
return null;
|
||||
|
||||
var workOrder = await _detailData.GetWorkOrderForMediaAsync(workOrderId, cancellationToken);
|
||||
var workOrder = await _detailData.GetWorkOrderForMediaAsync(workOrderId, cancellationToken, accountId);
|
||||
if (workOrder == null)
|
||||
return null;
|
||||
|
||||
|
|
@ -56,7 +55,8 @@ namespace SeaHaven.Services.Implementation
|
|||
CancellationToken cancellationToken = default)
|
||||
{
|
||||
WorkOrderMediaAuthorization.EnsureCanMutate(user, actorId);
|
||||
await GetMutableWorkOrderAsync(workOrderId, user, actorId!, cancellationToken);
|
||||
// AsNoTracking pre-check so the subsequent AddMediaAsync load is not stale-cached.
|
||||
await GetMutableWorkOrderForAuthAsync(workOrderId, user, actorId!, cancellationToken);
|
||||
}
|
||||
|
||||
public async Task<WorkOrderMediaFileDto> AddMediaAsync(
|
||||
|
|
@ -253,14 +253,34 @@ namespace SeaHaven.Services.Implementation
|
|||
await SaveMediaAsync(cancellationToken);
|
||||
}
|
||||
|
||||
private async Task<WorkOrder> GetMutableWorkOrderForAuthAsync(
|
||||
int workOrderId,
|
||||
ClaimsPrincipal user,
|
||||
string actorId,
|
||||
CancellationToken cancellationToken)
|
||||
{
|
||||
var accountId = WorkOrderMediaAuthorization.ResolveAccountId(user);
|
||||
var workOrder = await _mediaData.GetWorkOrderForMediaAuthAsync(workOrderId, accountId, cancellationToken);
|
||||
if (workOrder == null)
|
||||
throw new WorkOrderBoardValidationException("NotFound", "Work order not found.");
|
||||
|
||||
WorkOrderMediaAuthorization.EnsureWorkOrderInCallerScope(user, actorId, workOrder);
|
||||
|
||||
if (WorkOrderBoardMutationRules.IsReadOnly(workOrder.LifecycleStatus))
|
||||
throw new WorkOrderBoardValidationException("ReadOnly", "Work order is read-only in its current status.");
|
||||
|
||||
return workOrder;
|
||||
}
|
||||
|
||||
private async Task<WorkOrder> GetMutableWorkOrderAsync(
|
||||
int workOrderId,
|
||||
ClaimsPrincipal user,
|
||||
string actorId,
|
||||
CancellationToken cancellationToken)
|
||||
{
|
||||
// Organization scope via ApplyBaseScope: deleted/template → NotFound (ADR 0001).
|
||||
var workOrder = await _mediaData.GetTrackedWorkOrderAsync(workOrderId, cancellationToken);
|
||||
var accountId = WorkOrderMediaAuthorization.ResolveAccountId(user);
|
||||
// Fresh tracked load (not the AsNoTracking pre-check entity).
|
||||
var workOrder = await _mediaData.GetTrackedWorkOrderAsync(workOrderId, accountId, cancellationToken);
|
||||
if (workOrder == null)
|
||||
throw new WorkOrderBoardValidationException("NotFound", "Work order not found.");
|
||||
|
||||
|
|
|
|||
|
|
@ -590,15 +590,20 @@ public class WorkOrderMediaServiceTests
|
|||
private static string ToVersion(WorkOrder workOrder)
|
||||
=> Convert.ToBase64String(workOrder.RowVersion ?? new byte[] { 1, 0, 0, 0, 0, 0, 0, 1 });
|
||||
|
||||
private static ClaimsPrincipal AuthenticatedUser(string actorId = "actor-1", string role = "Admin")
|
||||
private static ClaimsPrincipal AuthenticatedUser(
|
||||
string actorId = "actor-1",
|
||||
string role = "Admin",
|
||||
int? accountId = null)
|
||||
{
|
||||
var identity = new ClaimsIdentity(
|
||||
new[]
|
||||
{
|
||||
new Claim(ClaimTypes.NameIdentifier, actorId),
|
||||
new Claim(ClaimTypes.Role, role)
|
||||
},
|
||||
authenticationType: "Test");
|
||||
var claims = new List<Claim>
|
||||
{
|
||||
new Claim(ClaimTypes.NameIdentifier, actorId),
|
||||
new Claim(ClaimTypes.Role, role)
|
||||
};
|
||||
if (accountId.HasValue)
|
||||
claims.Add(new Claim(SeaHavenClaimTypes.AccountId, accountId.Value.ToString()));
|
||||
|
||||
var identity = new ClaimsIdentity(claims, authenticationType: "Test");
|
||||
return new ClaimsPrincipal(identity);
|
||||
}
|
||||
|
||||
|
|
@ -1159,6 +1164,168 @@ public class WorkOrderMediaServiceTests
|
|||
|
||||
Assert.Equal("NotFound", ex.Code);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GetMedia_StaffWithAccountClaim_CrossAccount_ReturnsNull()
|
||||
{
|
||||
var (context, service) = CreateSut();
|
||||
context.workOrders.Add(new WorkOrder
|
||||
{
|
||||
Id = 1,
|
||||
AccountId = 20,
|
||||
BeforPhotoAttachment = "https://example.com/before.jpg"
|
||||
});
|
||||
await context.SaveChangesAsync();
|
||||
|
||||
var media = await service.GetMediaAsync(1, AuthenticatedUser(accountId: 10), "actor-1");
|
||||
|
||||
Assert.Null(media);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GetMedia_StaffWithAccountClaim_SameAccount_Succeeds()
|
||||
{
|
||||
var (context, service) = CreateSut();
|
||||
context.workOrders.Add(new WorkOrder
|
||||
{
|
||||
Id = 1,
|
||||
AccountId = 10,
|
||||
BeforPhotoAttachment = "https://example.com/before.jpg"
|
||||
});
|
||||
await context.SaveChangesAsync();
|
||||
|
||||
var media = await service.GetMediaAsync(1, AuthenticatedUser(accountId: 10), "actor-1");
|
||||
|
||||
Assert.NotNull(media);
|
||||
Assert.Contains(media!, m => m.Category == WorkOrderMediaCategory.Before);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GetMedia_StaffWithoutAccountClaim_OrgWide_Succeeds()
|
||||
{
|
||||
var (context, service) = CreateSut();
|
||||
context.workOrders.Add(new WorkOrder
|
||||
{
|
||||
Id = 1,
|
||||
AccountId = 99,
|
||||
BeforPhotoAttachment = "https://example.com/before.jpg"
|
||||
});
|
||||
await context.SaveChangesAsync();
|
||||
|
||||
var media = await service.GetMediaAsync(1, AuthenticatedUser(), "actor-1");
|
||||
|
||||
Assert.NotNull(media);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task AddMedia_StaffWithAccountClaim_CrossAccount_ThrowsNotFound()
|
||||
{
|
||||
var (context, service) = CreateSut();
|
||||
context.workOrders.Add(new WorkOrder
|
||||
{
|
||||
Id = 1,
|
||||
AccountId = 20,
|
||||
LifecycleStatus = LifecycleStatus.Scheduled,
|
||||
RowVersion = new byte[] { 1, 0, 0, 0, 0, 0, 0, 1 }
|
||||
});
|
||||
await context.SaveChangesAsync();
|
||||
|
||||
var ex = await Assert.ThrowsAsync<WorkOrderBoardValidationException>(() =>
|
||||
service.AddMediaAsync(
|
||||
1,
|
||||
WorkOrderMediaCategory.Extra,
|
||||
"https://example.com/photo.jpg",
|
||||
AuthenticatedUser(accountId: 10),
|
||||
"actor-1"));
|
||||
|
||||
Assert.Equal("NotFound", ex.Code);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task AddMedia_TechnicianAssigned_CrossAccount_ThrowsNotFound()
|
||||
{
|
||||
var (context, service) = CreateSut();
|
||||
context.workOrders.Add(new WorkOrder
|
||||
{
|
||||
Id = 1,
|
||||
AccountId = 20,
|
||||
AssignTo = "tech-1",
|
||||
LifecycleStatus = LifecycleStatus.Scheduled,
|
||||
RowVersion = new byte[] { 1, 0, 0, 0, 0, 0, 0, 1 }
|
||||
});
|
||||
await context.SaveChangesAsync();
|
||||
|
||||
var ex = await Assert.ThrowsAsync<WorkOrderBoardValidationException>(() =>
|
||||
service.AddMediaAsync(
|
||||
1,
|
||||
WorkOrderMediaCategory.Extra,
|
||||
"https://example.com/photo.jpg",
|
||||
AuthenticatedUser("tech-1", "User", accountId: 10),
|
||||
"tech-1"));
|
||||
|
||||
Assert.Equal("NotFound", ex.Code);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task AddMedia_AfterPreCheck_BecomesReadOnly_ThrowsReadOnly_OnFreshLoad()
|
||||
{
|
||||
var (context, service) = CreateSut();
|
||||
context.workOrders.Add(new WorkOrder
|
||||
{
|
||||
Id = 1,
|
||||
LifecycleStatus = LifecycleStatus.Scheduled,
|
||||
RowVersion = new byte[] { 1, 0, 0, 0, 0, 0, 0, 1 }
|
||||
});
|
||||
await context.SaveChangesAsync();
|
||||
|
||||
await service.EnsureCanMutateMediaAsync(1, AuthenticatedUser(), "actor-1");
|
||||
|
||||
var wo = await context.workOrders.SingleAsync(w => w.Id == 1);
|
||||
wo.LifecycleStatus = LifecycleStatus.Completed;
|
||||
await context.SaveChangesAsync();
|
||||
context.ChangeTracker.Clear();
|
||||
|
||||
var ex = await Assert.ThrowsAsync<WorkOrderBoardValidationException>(() =>
|
||||
service.AddMediaAsync(
|
||||
1,
|
||||
WorkOrderMediaCategory.Extra,
|
||||
"https://example.com/photo.jpg",
|
||||
AuthenticatedUser(),
|
||||
"actor-1"));
|
||||
|
||||
Assert.Equal("ReadOnly", ex.Code);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task AddMedia_AfterPreCheck_AssignmentChanged_ThrowsNotFound_OnFreshLoad()
|
||||
{
|
||||
var (context, service) = CreateSut();
|
||||
context.workOrders.Add(new WorkOrder
|
||||
{
|
||||
Id = 1,
|
||||
AssignTo = "tech-1",
|
||||
LifecycleStatus = LifecycleStatus.Scheduled,
|
||||
RowVersion = new byte[] { 1, 0, 0, 0, 0, 0, 0, 1 }
|
||||
});
|
||||
await context.SaveChangesAsync();
|
||||
|
||||
await service.EnsureCanMutateMediaAsync(1, AuthenticatedUser("tech-1", "User"), "tech-1");
|
||||
|
||||
var wo = await context.workOrders.SingleAsync(w => w.Id == 1);
|
||||
wo.AssignTo = "other-tech";
|
||||
await context.SaveChangesAsync();
|
||||
context.ChangeTracker.Clear();
|
||||
|
||||
var ex = await Assert.ThrowsAsync<WorkOrderBoardValidationException>(() =>
|
||||
service.AddMediaAsync(
|
||||
1,
|
||||
WorkOrderMediaCategory.Extra,
|
||||
"https://example.com/photo.jpg",
|
||||
AuthenticatedUser("tech-1", "User"),
|
||||
"tech-1"));
|
||||
|
||||
Assert.Equal("NotFound", ex.Code);
|
||||
}
|
||||
}
|
||||
|
||||
public class WorkOrderMediaFileRulesTests
|
||||
|
|
|
|||
|
|
@ -2,122 +2,64 @@
|
|||
|
||||
## Status
|
||||
|
||||
**Proposed — awaiting approval** (2026-08-05).
|
||||
**Superseded** (2026-08-06) by the SH-221 media slice in PR #47:
|
||||
|
||||
This ADR is **not** self-accepted by the PR author. Approval is required from:
|
||||
- `WorkOrder.AccountId` / `ApplicationUser.AccountId` schema keys
|
||||
- JWT `account_id` claim emitted from `ApplicationUser.AccountId`
|
||||
- Media loads filter via `ApplyBaseScope` + `ApplyAccountScope` when the claim is present
|
||||
|
||||
- CODEOWNERS team `@Sea-Haven-Industries/internal-dev` (see `.github/CODEOWNERS`)
|
||||
- SH-116 product owner
|
||||
Board, detail, and search outside media still use base scope only until the
|
||||
remainder of [SH-221](https://luby-us.atlassian.net/browse/SH-221) lands.
|
||||
|
||||
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 (historical)
|
||||
|
||||
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.
|
||||
be rejected without metadata disclosure. When this ADR was Proposed, the
|
||||
work-order domain had no `TenantId` / `CustomerId` / `AccountId` on
|
||||
`WorkOrder` or `ApplicationUser`, and JWT issuance emitted only identity/role
|
||||
claims. Media authorization matched the board via `ApplyBaseScope` plus
|
||||
role/assignee checks.
|
||||
|
||||
Verified against the current codebase:
|
||||
## Decision (historical — Proposed interim)
|
||||
|
||||
- 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).
|
||||
Until real tenant enforcement existed, work-order media authorization matched
|
||||
the board: `ApplyBaseScope` + claims-derived roles/assignee. That interim is
|
||||
no longer the media contract.
|
||||
|
||||
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).
|
||||
## Current media contract (superseding)
|
||||
|
||||
## 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
|
||||
(`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 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.
|
||||
1. **Organization boundary** = `ApplyBaseScope` (non-deleted, non-template).
|
||||
2. **Account boundary** = when the principal has claim `account_id`, media
|
||||
queries require `WorkOrder.AccountId == claim`. Cross-account → stable
|
||||
`NotFound` / null (no disclosure).
|
||||
3. **Org-wide staff** = staff principals **without** `account_id` keep base
|
||||
scope only (explicit claims rule).
|
||||
4. **Authorization at service entry** from claims: staff roles may read/mutate
|
||||
any resulting work order; role `User` only when `AssignTo == actorId`;
|
||||
delete remains staff-only.
|
||||
|
||||
## Consequences
|
||||
|
||||
- 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 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).
|
||||
|
||||
## 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.
|
||||
- Cross-account media tests are required for principals that carry `account_id`.
|
||||
- Board/search/detail without account filtering remain a SH-221 follow-up.
|
||||
- This ADR no longer grants an exception to §2 for media; the claim+FK path is
|
||||
the enforcement.
|
||||
|
||||
## Excepted rule
|
||||
|
||||
Hard rule: **server-derived tenant scope**
|
||||
(`ARCHITECTURE_AND_CODE_QUALITY.md` §2).
|
||||
|
||||
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.
|
||||
None for media (superseded). Hard rule **server-derived tenant scope**
|
||||
(`ARCHITECTURE_AND_CODE_QUALITY.md` §2) is enforced for media via `account_id`.
|
||||
|
||||
## Review / expiry
|
||||
|
||||
Re-review by **2027-02-04**, or earlier if product introduces
|
||||
`TenantId`/`CustomerId`/`AccountId` on work orders or JWT claims, or when the
|
||||
linked multi-tenant ticket closes.
|
||||
Re-review when SH-221 closes remaining board/search/detail account filters, or
|
||||
by **2027-02-04**.
|
||||
|
||||
## 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`
|
||||
- SH-221 — Server-derived tenant/customer scope for Work Order domain
|
||||
- PR: Sea-Haven-Industries/shoc-backend#47
|
||||
- `WorkOrderBoardQueryFilters.ApplyBaseScope` / `ApplyAccountScope`
|
||||
- `WorkOrderMediaAuthorization`
|
||||
- `ARCHITECTURE_AND_CODE_QUALITY.md` §2, §10
|
||||
- `REVIEW_AND_PR_FRAMEWORK.md` §7, §8
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue