diff --git a/Api.SeaHavenIndustries/Controllers/UserController.cs b/Api.SeaHavenIndustries/Controllers/UserController.cs index 1540c5c..144896b 100644 --- a/Api.SeaHavenIndustries/Controllers/UserController.cs +++ b/Api.SeaHavenIndustries/Controllers/UserController.cs @@ -41,7 +41,8 @@ namespace Api.SeaHavenIndustries.Controllers Name = user.Name, Email = user.Email, Contact = user.Contact, - Role = user.Role + Role = user.Role, + AccountId = user.AccountId }; var outcome = await _userService.AddUserAsync(dto, cancellationToken); if (!outcome.Success) @@ -68,7 +69,8 @@ namespace Api.SeaHavenIndustries.Controllers Id = user.Id, Name = user.Name, Email = user.Email, - Role = user.Role + Role = user.Role, + AccountId = user.AccountId }; var succeeded = await _userService.EditUserAsync(dto, cancellationToken); if (!succeeded) diff --git a/Api.SeaHavenIndustries/DTOs/User_DTO.cs b/Api.SeaHavenIndustries/DTOs/User_DTO.cs index 8d1e68c..4a624c8 100644 --- a/Api.SeaHavenIndustries/DTOs/User_DTO.cs +++ b/Api.SeaHavenIndustries/DTOs/User_DTO.cs @@ -6,6 +6,7 @@ public string? Email { get; set; } public string? Contact { get; set; } public string? Role { get; set; } + public int? AccountId { get; set; } } public class EditUser_DTO { @@ -13,5 +14,6 @@ public string? Name { get; set; } public string? Email { get; set; } public string? Role { get; set; } + public int? AccountId { get; set; } } } diff --git a/SeaHaven.DataServices/Helpers/WorkOrderBoardQueryFilters.cs b/SeaHaven.DataServices/Helpers/WorkOrderBoardQueryFilters.cs index 098d533..ba41941 100644 --- a/SeaHaven.DataServices/Helpers/WorkOrderBoardQueryFilters.cs +++ b/SeaHaven.DataServices/Helpers/WorkOrderBoardQueryFilters.cs @@ -9,16 +9,11 @@ namespace SeaHaven.DataServices.Helpers => query.Where(w => w.istemplate != true && (w.IsDeleted != true || w.IsDeleted == null)); /// - /// When is set, restrict to that CRM account. - /// When null, no account filter (org-wide staff without an account claim). + /// Restricts to the given CRM account. Callers with org-wide scope must not + /// invoke this method (use only). /// - public static IQueryable ApplyAccountScope(IQueryable query, int? accountId) - { - if (!accountId.HasValue) - return query; - - return query.Where(w => w.AccountId == accountId.Value); - } + public static IQueryable ApplyAccountScope(IQueryable query, int accountId) + => query.Where(w => w.AccountId == accountId); /// /// Applies assignee filters. When is true and diff --git a/SeaHaven.DataServices/Implementation/WorkOrderDetailDataService.cs b/SeaHaven.DataServices/Implementation/WorkOrderDetailDataService.cs index 9a704b9..212c13a 100644 --- a/SeaHaven.DataServices/Implementation/WorkOrderDetailDataService.cs +++ b/SeaHaven.DataServices/Implementation/WorkOrderDetailDataService.cs @@ -18,10 +18,13 @@ namespace SeaHaven.DataServices.Implementation int workOrderId, CancellationToken cancellationToken = default, int? accountId = null) - => WorkOrderBoardQueryFilters.ApplyAccountScope( - WorkOrderBoardQueryFilters.ApplyBaseScope(_context.workOrders.AsNoTracking()), - accountId) - .AnyAsync(w => w.Id == workOrderId, cancellationToken); + { + var query = WorkOrderBoardQueryFilters.ApplyBaseScope(_context.workOrders.AsNoTracking()); + if (accountId.HasValue) + query = WorkOrderBoardQueryFilters.ApplyAccountScope(query, accountId.Value); + + return query.AnyAsync(w => w.Id == workOrderId, cancellationToken); + } public async Task GetExtendedFieldsAsync(int workOrderId) { @@ -98,10 +101,11 @@ namespace SeaHaven.DataServices.Implementation CancellationToken cancellationToken = default, int? accountId = null) { - return await WorkOrderBoardQueryFilters.ApplyAccountScope( - WorkOrderBoardQueryFilters.ApplyBaseScope(_context.workOrders.AsNoTracking()), - accountId) - .FirstOrDefaultAsync(w => w.Id == workOrderId, cancellationToken); + var query = WorkOrderBoardQueryFilters.ApplyBaseScope(_context.workOrders.AsNoTracking()); + if (accountId.HasValue) + query = WorkOrderBoardQueryFilters.ApplyAccountScope(query, accountId.Value); + + return await query.FirstOrDefaultAsync(w => w.Id == workOrderId, cancellationToken); } } } diff --git a/SeaHaven.DataServices/Implementation/WorkOrderMediaDataService.cs b/SeaHaven.DataServices/Implementation/WorkOrderMediaDataService.cs index 6923dc9..a1b887c 100644 --- a/SeaHaven.DataServices/Implementation/WorkOrderMediaDataService.cs +++ b/SeaHaven.DataServices/Implementation/WorkOrderMediaDataService.cs @@ -18,19 +18,25 @@ namespace SeaHaven.DataServices.Implementation int workOrderId, int? accountId, CancellationToken cancellationToken) - => WorkOrderBoardQueryFilters.ApplyAccountScope( - WorkOrderBoardQueryFilters.ApplyBaseScope(_context.workOrders.AsNoTracking()), - accountId) - .FirstOrDefaultAsync(w => w.Id == workOrderId, cancellationToken); + { + var query = WorkOrderBoardQueryFilters.ApplyBaseScope(_context.workOrders.AsNoTracking()); + if (accountId.HasValue) + query = WorkOrderBoardQueryFilters.ApplyAccountScope(query, accountId.Value); + + return query.FirstOrDefaultAsync(w => w.Id == workOrderId, cancellationToken); + } public Task GetTrackedWorkOrderAsync( int workOrderId, int? accountId, CancellationToken cancellationToken) - => WorkOrderBoardQueryFilters.ApplyAccountScope( - WorkOrderBoardQueryFilters.ApplyBaseScope(_context.workOrders), - accountId) - .FirstOrDefaultAsync(w => w.Id == workOrderId, cancellationToken); + { + var query = WorkOrderBoardQueryFilters.ApplyBaseScope(_context.workOrders); + if (accountId.HasValue) + query = WorkOrderBoardQueryFilters.ApplyAccountScope(query, accountId.Value); + + return query.FirstOrDefaultAsync(w => w.Id == workOrderId, cancellationToken); + } public Task GetTrackedAttachmentAsync(int mediaId, int workOrderId, CancellationToken cancellationToken) => _context.workOrderAttachments.FirstOrDefaultAsync( diff --git a/SeaHaven.DataServices/Interfaces/IWorkOrderMediaDataService.cs b/SeaHaven.DataServices/Interfaces/IWorkOrderMediaDataService.cs index 2e74bf3..f8db574 100644 --- a/SeaHaven.DataServices/Interfaces/IWorkOrderMediaDataService.cs +++ b/SeaHaven.DataServices/Interfaces/IWorkOrderMediaDataService.cs @@ -4,7 +4,7 @@ namespace SeaHaven.DataServices.Interfaces { public interface IWorkOrderMediaDataService { - /// AsNoTracking base+account scoped load for pre-mutation auth (does not pollute the change tracker). + /// AsNoTracking base (+ optional account) scoped load for pre-mutation auth. Task GetWorkOrderForMediaAuthAsync( int workOrderId, int? accountId, diff --git a/SeaHaven.Services/DTOs/IdentityDTOs.cs b/SeaHaven.Services/DTOs/IdentityDTOs.cs index 94df6dc..9afc76c 100644 --- a/SeaHaven.Services/DTOs/IdentityDTOs.cs +++ b/SeaHaven.Services/DTOs/IdentityDTOs.cs @@ -31,6 +31,7 @@ namespace SeaHaven.Services.DTOs public string? Email { get; set; } public string? Contact { get; set; } public string? Role { get; set; } + public int? AccountId { get; set; } } public class AddUserOutcomeDTO @@ -45,6 +46,7 @@ namespace SeaHaven.Services.DTOs public string? Name { get; set; } public string? Email { get; set; } public string? Role { get; set; } + public int? AccountId { get; set; } } public class UserListRowDTO diff --git a/SeaHaven.Services/Helpers/SeaHavenClaimTypes.cs b/SeaHaven.Services/Helpers/SeaHavenClaimTypes.cs index 094e8dc..51437cb 100644 --- a/SeaHaven.Services/Helpers/SeaHavenClaimTypes.cs +++ b/SeaHaven.Services/Helpers/SeaHavenClaimTypes.cs @@ -5,5 +5,23 @@ namespace SeaHaven.Services.Helpers { /// CRM account id from ApplicationUser.AccountId (never from request body). public const string AccountId = "account_id"; + + /// Explicit org-wide media scope; value . + public const string OrgScope = "org_scope"; + + /// Signed org-wide elevation (Admin without AccountId). + public const string OrgScopeAll = "all"; + } + + /// Resolved media tenant scope from signed claims (fail-closed when Missing). + public abstract record MediaAccountScope + { + private MediaAccountScope() { } + + public sealed record Account(int AccountId) : MediaAccountScope; + + public sealed record OrgWide : MediaAccountScope; + + public sealed record Missing : MediaAccountScope; } } diff --git a/SeaHaven.Services/Helpers/WorkOrderMediaAuthorization.cs b/SeaHaven.Services/Helpers/WorkOrderMediaAuthorization.cs index 77ecf57..2c8de60 100644 --- a/SeaHaven.Services/Helpers/WorkOrderMediaAuthorization.cs +++ b/SeaHaven.Services/Helpers/WorkOrderMediaAuthorization.cs @@ -6,12 +6,11 @@ namespace SeaHaven.Services.Helpers { /// /// Claims-derived authorization for work-order media read/mutations. - /// Callers must resolve the work order via ApplyBaseScope plus - /// ApplyAccountScope when the principal carries - /// . Staff may access any - /// resulting work order; technicians only when - /// matches the actor. Delete is staff-only. - /// Staff without an account claim remain org-wide (base scope only). + /// Scope is fail-closed: callers need a valid + /// or explicit =. + /// Absence of scope does not elevate. Staff may access any resulting work order; + /// technicians only when matches the actor. + /// Delete is staff-only. /// public static class WorkOrderMediaAuthorization { @@ -23,18 +22,39 @@ namespace SeaHaven.Services.Helpers "Supervisor" }; - public static int? ResolveAccountId(ClaimsPrincipal user) + public static MediaAccountScope ResolveMediaScope(ClaimsPrincipal user) { - var raw = user?.FindFirstValue(SeaHavenClaimTypes.AccountId); - if (string.IsNullOrWhiteSpace(raw)) - return null; + if (user is null) + return new MediaAccountScope.Missing(); - return int.TryParse(raw, out var accountId) ? accountId : null; + var accountRaw = user.FindFirstValue(SeaHavenClaimTypes.AccountId); + if (!string.IsNullOrWhiteSpace(accountRaw)) + { + if (!int.TryParse(accountRaw, out var accountId) || accountId <= 0) + return new MediaAccountScope.Missing(); + + return new MediaAccountScope.Account(accountId); + } + + var orgScope = user.FindFirstValue(SeaHavenClaimTypes.OrgScope); + if (string.Equals(orgScope, SeaHavenClaimTypes.OrgScopeAll, StringComparison.Ordinal)) + return new MediaAccountScope.OrgWide(); + + return new MediaAccountScope.Missing(); + } + + public static void EnsureHasMediaScope(ClaimsPrincipal user) + { + if (ResolveMediaScope(user) is MediaAccountScope.Missing) + { + throw Forbidden("You are not allowed to access work order media without account scope."); + } } public static void EnsureCanRead(ClaimsPrincipal user, string? actorId) { EnsureAuthenticated(user, actorId, "You are not allowed to view work order media."); + EnsureHasMediaScope(user); if (IsStaff(user) || user.IsInRole("User")) return; @@ -45,6 +65,7 @@ namespace SeaHaven.Services.Helpers public static void EnsureCanMutate(ClaimsPrincipal user, string? actorId) { EnsureAuthenticated(user, actorId); + EnsureHasMediaScope(user); if (IsStaff(user) || user.IsInRole("User")) return; @@ -55,6 +76,7 @@ namespace SeaHaven.Services.Helpers public static void EnsureCanDelete(ClaimsPrincipal user, string? actorId) { EnsureAuthenticated(user, actorId); + EnsureHasMediaScope(user); // Technician (User) may upload/categorize assigned media but not delete. if (IsStaff(user)) @@ -64,7 +86,7 @@ namespace SeaHaven.Services.Helpers } /// - /// Caller-scope check after a base+account scoped work-order load. + /// Caller-scope check after a base (+ account when scoped) work-order load. /// Staff: any resulting work order. /// Technician: only when assigned to the caller. /// Out of caller scope → NotFound (no disclosure). diff --git a/SeaHaven.Services/Implementation/AuthenticationService.cs b/SeaHaven.Services/Implementation/AuthenticationService.cs index 878004f..d86f19a 100644 --- a/SeaHaven.Services/Implementation/AuthenticationService.cs +++ b/SeaHaven.Services/Implementation/AuthenticationService.cs @@ -57,6 +57,13 @@ namespace SeaHaven.Services.Implementation SeaHavenClaimTypes.AccountId, user.AccountId.Value.ToString())); } + else if (userRoles.Contains("Admin")) + { + // Explicit signed org-wide elevation — never elevate via absence of account_id. + authClaims.Add(new Claim( + SeaHavenClaimTypes.OrgScope, + SeaHavenClaimTypes.OrgScopeAll)); + } var token = GetToken(authClaims); return new LoginResultDTO { diff --git a/SeaHaven.Services/Implementation/UserService.cs b/SeaHaven.Services/Implementation/UserService.cs index 454e619..61a2e58 100644 --- a/SeaHaven.Services/Implementation/UserService.cs +++ b/SeaHaven.Services/Implementation/UserService.cs @@ -44,6 +44,7 @@ namespace SeaHaven.Services.Implementation UserName = dto.Email, FirstName = dto.Name, Email = dto.Email, + AccountId = dto.AccountId }; var exist = await _userDataService.GetByIdAsync(model.Id); @@ -92,6 +93,7 @@ namespace SeaHaven.Services.Implementation exist1.LastName = model.LastName; exist1.Contact = model.Contact; exist1.PhoneNumber = model.PhoneNumber; + exist1.AccountId = dto.AccountId; await _userDataService.UpdateUserAsync(exist1, cancellationToken); @@ -119,6 +121,7 @@ namespace SeaHaven.Services.Implementation exist.CreatedDate = DateTime.Now; exist.UniqueName = "Active"; exist.PhoneNumber = dto.Role; + exist.AccountId = dto.AccountId; var existingRole = await _userManager.GetRolesAsync(exist); if (existingRole != null && existingRole.Any()) diff --git a/SeaHaven.Services/Implementation/WorkOrderMediaService.cs b/SeaHaven.Services/Implementation/WorkOrderMediaService.cs index 3fa214c..5d40998 100644 --- a/SeaHaven.Services/Implementation/WorkOrderMediaService.cs +++ b/SeaHaven.Services/Implementation/WorkOrderMediaService.cs @@ -33,12 +33,12 @@ namespace SeaHaven.Services.Implementation CancellationToken cancellationToken = default) { WorkOrderMediaAuthorization.EnsureCanRead(user, actorId); - var accountId = WorkOrderMediaAuthorization.ResolveAccountId(user); + var accountFilter = ResolveAccountFilter(user); - if (!await _detailData.ExistsAsync(workOrderId, cancellationToken, accountId)) + if (!await _detailData.ExistsAsync(workOrderId, cancellationToken, accountFilter)) return null; - var workOrder = await _detailData.GetWorkOrderForMediaAsync(workOrderId, cancellationToken, accountId); + var workOrder = await _detailData.GetWorkOrderForMediaAsync(workOrderId, cancellationToken, accountFilter); if (workOrder == null) return null; @@ -259,8 +259,8 @@ namespace SeaHaven.Services.Implementation string actorId, CancellationToken cancellationToken) { - var accountId = WorkOrderMediaAuthorization.ResolveAccountId(user); - var workOrder = await _mediaData.GetWorkOrderForMediaAuthAsync(workOrderId, accountId, cancellationToken); + var accountFilter = ResolveAccountFilter(user); + var workOrder = await _mediaData.GetWorkOrderForMediaAuthAsync(workOrderId, accountFilter, cancellationToken); if (workOrder == null) throw new WorkOrderBoardValidationException("NotFound", "Work order not found."); @@ -278,9 +278,9 @@ namespace SeaHaven.Services.Implementation string actorId, CancellationToken cancellationToken) { - var accountId = WorkOrderMediaAuthorization.ResolveAccountId(user); + var accountFilter = ResolveAccountFilter(user); // Fresh tracked load (not the AsNoTracking pre-check entity). - var workOrder = await _mediaData.GetTrackedWorkOrderAsync(workOrderId, accountId, cancellationToken); + var workOrder = await _mediaData.GetTrackedWorkOrderAsync(workOrderId, accountFilter, cancellationToken); if (workOrder == null) throw new WorkOrderBoardValidationException("NotFound", "Work order not found."); @@ -292,6 +292,22 @@ namespace SeaHaven.Services.Implementation return workOrder; } + /// + /// Account filter for data queries. Null means org-wide (skip ApplyAccountScope). + /// Call only after EnsureCan* has verified scope is not Missing. + /// + private static int? ResolveAccountFilter(ClaimsPrincipal user) + { + return WorkOrderMediaAuthorization.ResolveMediaScope(user) switch + { + MediaAccountScope.Account account => account.AccountId, + MediaAccountScope.OrgWide => null, + _ => throw new WorkOrderBoardValidationException( + "Forbidden", + "You are not allowed to access work order media without account scope.") + }; + } + private async Task SaveMediaAsync(CancellationToken cancellationToken) { try diff --git a/SeaHavenIndustries.Tests/WorkOrderMediaConcurrencyRelationalTests.cs b/SeaHavenIndustries.Tests/WorkOrderMediaConcurrencyRelationalTests.cs index 006fcb4..b39d3d6 100644 --- a/SeaHavenIndustries.Tests/WorkOrderMediaConcurrencyRelationalTests.cs +++ b/SeaHavenIndustries.Tests/WorkOrderMediaConcurrencyRelationalTests.cs @@ -189,7 +189,8 @@ public class WorkOrderMediaConcurrencyRelationalTests new[] { new Claim(ClaimTypes.NameIdentifier, actorId), - new Claim(ClaimTypes.Role, "Admin") + new Claim(ClaimTypes.Role, "Admin"), + new Claim(SeaHaven.Services.Helpers.SeaHavenClaimTypes.OrgScope, SeaHaven.Services.Helpers.SeaHavenClaimTypes.OrgScopeAll) }, "Test")); diff --git a/SeaHavenIndustries.Tests/WorkOrderPhase6Tests.cs b/SeaHavenIndustries.Tests/WorkOrderPhase6Tests.cs index 68722d6..4336f45 100644 --- a/SeaHavenIndustries.Tests/WorkOrderPhase6Tests.cs +++ b/SeaHavenIndustries.Tests/WorkOrderPhase6Tests.cs @@ -593,20 +593,39 @@ public class WorkOrderMediaServiceTests private static ClaimsPrincipal AuthenticatedUser( string actorId = "actor-1", string role = "Admin", - int? accountId = null) + int? accountId = null, + bool omitScopeClaims = false) { var claims = new List { new Claim(ClaimTypes.NameIdentifier, actorId), new Claim(ClaimTypes.Role, role) }; - if (accountId.HasValue) - claims.Add(new Claim(SeaHavenClaimTypes.AccountId, accountId.Value.ToString())); + if (!omitScopeClaims) + { + if (accountId.HasValue) + claims.Add(new Claim(SeaHavenClaimTypes.AccountId, accountId.Value.ToString())); + else if (role == "Admin") + claims.Add(new Claim(SeaHavenClaimTypes.OrgScope, SeaHavenClaimTypes.OrgScopeAll)); + } var identity = new ClaimsIdentity(claims, authenticationType: "Test"); return new ClaimsPrincipal(identity); } + private static ClaimsPrincipal AuthenticatedWithMalformedAccountClaim(string actorId = "actor-1") + { + var identity = new ClaimsIdentity( + new[] + { + new Claim(ClaimTypes.NameIdentifier, actorId), + new Claim(ClaimTypes.Role, "Admin"), + new Claim(SeaHavenClaimTypes.AccountId, "not-an-int") + }, + authenticationType: "Test"); + return new ClaimsPrincipal(identity); + } + private static ClaimsPrincipal AuthenticatedWithoutRole(string actorId = "actor-1") { var identity = new ClaimsIdentity( @@ -652,6 +671,7 @@ public class WorkOrderMediaServiceTests context.workOrders.Add(new WorkOrder { Id = 1, + AccountId = 1, AssignTo = "tech-1", BeforPhotoAttachment = "https://example.com/before.jpg" }); @@ -659,7 +679,7 @@ public class WorkOrderMediaServiceTests var media = await service.GetMediaAsync( 1, - AuthenticatedUser("tech-1", "User"), + AuthenticatedUser("tech-1", "User", accountId: 1), "tech-1"); Assert.NotNull(media); @@ -673,13 +693,14 @@ public class WorkOrderMediaServiceTests context.workOrders.Add(new WorkOrder { Id = 1, + AccountId = 1, AssignTo = "other-tech", BeforPhotoAttachment = "https://example.com/before.jpg" }); await context.SaveChangesAsync(); var ex = await Assert.ThrowsAsync(() => - service.GetMediaAsync(1, AuthenticatedUser("tech-1", "User"), "tech-1")); + service.GetMediaAsync(1, AuthenticatedUser("tech-1", "User", accountId: 1), "tech-1")); Assert.Equal("NotFound", ex.Code); } @@ -921,6 +942,7 @@ public class WorkOrderMediaServiceTests context.workOrders.Add(new WorkOrder { Id = 1, + AccountId = 1, AssignTo = "other-tech", LifecycleStatus = LifecycleStatus.Scheduled, RowVersion = new byte[] { 1, 0, 0, 0, 0, 0, 0, 1 } @@ -932,7 +954,7 @@ public class WorkOrderMediaServiceTests 1, WorkOrderMediaCategory.Extra, "https://example.com/photo.jpg", - AuthenticatedUser("tech-1", "User"), + AuthenticatedUser("tech-1", "User", accountId: 1), "tech-1")); Assert.Equal("NotFound", ex.Code); @@ -945,6 +967,7 @@ public class WorkOrderMediaServiceTests context.workOrders.Add(new WorkOrder { Id = 1, + AccountId = 1, AssignTo = "tech-1", LifecycleStatus = LifecycleStatus.Scheduled, RowVersion = new byte[] { 1, 0, 0, 0, 0, 0, 0, 1 } @@ -955,7 +978,7 @@ public class WorkOrderMediaServiceTests 1, null, "https://example.com/photo.jpg", - AuthenticatedUser("tech-1", "User"), + AuthenticatedUser("tech-1", "User", accountId: 1), "tech-1"); Assert.True(media.Id > 0); @@ -969,6 +992,7 @@ public class WorkOrderMediaServiceTests context.workOrders.Add(new WorkOrder { Id = 1, + AccountId = 1, AssignTo = "tech-1", LifecycleStatus = LifecycleStatus.Scheduled, RowVersion = new byte[] { 1, 0, 0, 0, 0, 0, 0, 1 } @@ -987,7 +1011,7 @@ public class WorkOrderMediaServiceTests 1, 10, ToVersion(context.workOrders.Single()), - AuthenticatedUser("tech-1", "User"), + AuthenticatedUser("tech-1", "User", accountId: 1), "tech-1")); Assert.Equal("Forbidden", ex.Code); @@ -1201,7 +1225,7 @@ public class WorkOrderMediaServiceTests } [Fact] - public async Task GetMedia_StaffWithoutAccountClaim_OrgWide_Succeeds() + public async Task GetMedia_StaffWithOrgScopeClaim_Succeeds() { var (context, service) = CreateSut(); context.workOrders.Add(new WorkOrder @@ -1217,6 +1241,65 @@ public class WorkOrderMediaServiceTests Assert.NotNull(media); } + [Fact] + public async Task GetMedia_StaffWithoutScopeClaim_ThrowsForbidden() + { + var (context, service) = CreateSut(); + context.workOrders.Add(new WorkOrder + { + Id = 1, + AccountId = 99, + BeforPhotoAttachment = "https://example.com/before.jpg" + }); + await context.SaveChangesAsync(); + + var ex = await Assert.ThrowsAsync(() => + service.GetMediaAsync(1, AuthenticatedUser(omitScopeClaims: true), "actor-1")); + + Assert.Equal("Forbidden", ex.Code); + } + + [Fact] + public async Task GetMedia_MalformedAccountClaim_ThrowsForbidden() + { + var (context, service) = CreateSut(); + context.workOrders.Add(new WorkOrder + { + Id = 1, + AccountId = 10, + BeforPhotoAttachment = "https://example.com/before.jpg" + }); + await context.SaveChangesAsync(); + + var ex = await Assert.ThrowsAsync(() => + service.GetMediaAsync(1, AuthenticatedWithMalformedAccountClaim(), "actor-1")); + + Assert.Equal("Forbidden", ex.Code); + } + + [Fact] + public async Task AddMedia_StaffWithoutScopeClaim_ThrowsForbidden() + { + 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(); + + var ex = await Assert.ThrowsAsync(() => + service.AddMediaAsync( + 1, + WorkOrderMediaCategory.Extra, + "https://example.com/photo.jpg", + AuthenticatedUser(role: "Dispatcher", omitScopeClaims: true), + "actor-1")); + + Assert.Equal("Forbidden", ex.Code); + } + [Fact] public async Task AddMedia_StaffWithAccountClaim_CrossAccount_ThrowsNotFound() { @@ -1303,13 +1386,14 @@ public class WorkOrderMediaServiceTests context.workOrders.Add(new WorkOrder { Id = 1, + AccountId = 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"); + await service.EnsureCanMutateMediaAsync(1, AuthenticatedUser("tech-1", "User", accountId: 1), "tech-1"); var wo = await context.workOrders.SingleAsync(w => w.Id == 1); wo.AssignTo = "other-tech"; @@ -1321,7 +1405,7 @@ public class WorkOrderMediaServiceTests 1, WorkOrderMediaCategory.Extra, "https://example.com/photo.jpg", - AuthenticatedUser("tech-1", "User"), + AuthenticatedUser("tech-1", "User", accountId: 1), "tech-1")); Assert.Equal("NotFound", ex.Code); diff --git a/docs/adr/0001-work-order-single-org-scope.md b/docs/adr/0001-work-order-single-org-scope.md index fce6dbc..6b35e78 100644 --- a/docs/adr/0001-work-order-single-org-scope.md +++ b/docs/adr/0001-work-order-single-org-scope.md @@ -2,11 +2,15 @@ ## Status -**Superseded** (2026-08-06) by the SH-221 media slice in PR #47: +**Superseded** (2026-08-06) by the SH-221 media slice in PR #47, with +**fail-closed** account scope (follow-up on the same PR): - `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 +- JWT `account_id` when `ApplicationUser.AccountId` is set +- JWT `org_scope=all` when Admin has no AccountId (explicit signed elevation) +- Media loads: `ApplyBaseScope` + `ApplyAccountScope(int)` when account-scoped; + org-wide path skips account filter +- Missing/malformed scope → **Forbidden** (absence of claim does not elevate) 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. @@ -14,41 +18,33 @@ remainder of [SH-221](https://luby-us.atlassian.net/browse/SH-221) lands. ## Context (historical) SH-116 requires that cross-tenant, unauthorized, and out-of-scope media access -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. - -## Decision (historical — Proposed interim) - -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. +be rejected without metadata disclosure. An interim Proposed ADR allowed +org-wide staff access via absence of an account claim; that path was rejected +in review (fail-open) and replaced by the contract below. ## Current media contract (superseding) 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. +2. **Account boundary** = claim `account_id` → `WorkOrder.AccountId == claim`. +3. **Org-wide** = claim `org_scope=all` only (issued to Admin without + AccountId). Not inferred from missing `account_id`. +4. **Fail-closed** = no valid account or org-scope claim → Forbidden. +5. **Authorization at service entry**: staff roles may read/mutate any resulting + work order; role `User` only when `AssignTo == actorId`; delete staff-only. +6. **User lifecycle** persists `AccountId` on create/edit so non-Admin principals + can receive `account_id`. ## Consequences -- Cross-account media tests are required for principals that carry `account_id`. +- Cross-account and missing-scope media tests are required. +- Dispatcher/Manager/Supervisor/User without AccountId cannot access media until + AccountId is assigned (or they are Admin with `org_scope=all`). - 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 -None for media (superseded). Hard rule **server-derived tenant scope** -(`ARCHITECTURE_AND_CODE_QUALITY.md` §2) is enforced for media via `account_id`. +None for media. Hard rule **server-derived tenant scope** +(`ARCHITECTURE_AND_CODE_QUALITY.md` §2) is enforced via claims. ## Review / expiry @@ -61,5 +57,5 @@ by **2027-02-04**. - SH-221 — Server-derived tenant/customer scope for Work Order domain - PR: Sea-Haven-Industries/shoc-backend#47 - `WorkOrderBoardQueryFilters.ApplyBaseScope` / `ApplyAccountScope` -- `WorkOrderMediaAuthorization` +- `WorkOrderMediaAuthorization` / `SeaHavenClaimTypes` - `ARCHITECTURE_AND_CODE_QUALITY.md` §2, §10