mirror of
https://github.com/Sea-Haven-Industries/shoc-backend.git
synced 2026-09-30 08:23:12 +00:00
!fix(work-orders): fail-closed media account scope with org_scope claim [SH-221]
This commit is contained in:
parent
fdc315d8fe
commit
ea2dedf579
15 changed files with 245 additions and 87 deletions
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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; }
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -9,16 +9,11 @@ namespace SeaHaven.DataServices.Helpers
|
|||
=> 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).
|
||||
/// Restricts to the given CRM account. Callers with org-wide scope must not
|
||||
/// invoke this method (use <see cref="ApplyBaseScope"/> only).
|
||||
/// </summary>
|
||||
public static IQueryable<WorkOrder> ApplyAccountScope(IQueryable<WorkOrder> query, int? accountId)
|
||||
{
|
||||
if (!accountId.HasValue)
|
||||
return query;
|
||||
|
||||
return query.Where(w => w.AccountId == accountId.Value);
|
||||
}
|
||||
public static IQueryable<WorkOrder> ApplyAccountScope(IQueryable<WorkOrder> query, int accountId)
|
||||
=> query.Where(w => w.AccountId == accountId);
|
||||
|
||||
/// <summary>
|
||||
/// Applies assignee filters. When <paramref name="myWorkOrders"/> is true and
|
||||
|
|
|
|||
|
|
@ -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<WorkOrderDetailExtendedFields?> 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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<WorkOrder?> 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<WorkOrderAttachments?> GetTrackedAttachmentAsync(int mediaId, int workOrderId, CancellationToken cancellationToken)
|
||||
=> _context.workOrderAttachments.FirstOrDefaultAsync(
|
||||
|
|
|
|||
|
|
@ -4,7 +4,7 @@ namespace SeaHaven.DataServices.Interfaces
|
|||
{
|
||||
public interface IWorkOrderMediaDataService
|
||||
{
|
||||
/// <summary>AsNoTracking base+account scoped load for pre-mutation auth (does not pollute the change tracker).</summary>
|
||||
/// <summary>AsNoTracking base (+ optional account) scoped load for pre-mutation auth.</summary>
|
||||
Task<WorkOrder?> GetWorkOrderForMediaAuthAsync(
|
||||
int workOrderId,
|
||||
int? accountId,
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -5,5 +5,23 @@ namespace SeaHaven.Services.Helpers
|
|||
{
|
||||
/// <summary>CRM account id from <c>ApplicationUser.AccountId</c> (never from request body).</summary>
|
||||
public const string AccountId = "account_id";
|
||||
|
||||
/// <summary>Explicit org-wide media scope; value <see cref="OrgScopeAll"/>.</summary>
|
||||
public const string OrgScope = "org_scope";
|
||||
|
||||
/// <summary>Signed org-wide elevation (Admin without AccountId).</summary>
|
||||
public const string OrgScopeAll = "all";
|
||||
}
|
||||
|
||||
/// <summary>Resolved media tenant scope from signed claims (fail-closed when Missing).</summary>
|
||||
public abstract record MediaAccountScope
|
||||
{
|
||||
private MediaAccountScope() { }
|
||||
|
||||
public sealed record Account(int AccountId) : MediaAccountScope;
|
||||
|
||||
public sealed record OrgWide : MediaAccountScope;
|
||||
|
||||
public sealed record Missing : MediaAccountScope;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -6,12 +6,11 @@ namespace SeaHaven.Services.Helpers
|
|||
{
|
||||
/// <summary>
|
||||
/// Claims-derived authorization for work-order media read/mutations.
|
||||
/// 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).
|
||||
/// Scope is fail-closed: callers need a valid <see cref="SeaHavenClaimTypes.AccountId"/>
|
||||
/// or explicit <see cref="SeaHavenClaimTypes.OrgScope"/>=<see cref="SeaHavenClaimTypes.OrgScopeAll"/>.
|
||||
/// Absence of scope does not elevate. Staff may access any resulting work order;
|
||||
/// technicians only when <see cref="WorkOrder.AssignTo"/> matches the actor.
|
||||
/// Delete is staff-only.
|
||||
/// </summary>
|
||||
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
|
|||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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).
|
||||
|
|
|
|||
|
|
@ -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
|
||||
{
|
||||
|
|
|
|||
|
|
@ -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())
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Account filter for data queries. Null means org-wide (skip ApplyAccountScope).
|
||||
/// Call only after EnsureCan* has verified scope is not Missing.
|
||||
/// </summary>
|
||||
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
|
||||
|
|
|
|||
|
|
@ -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"));
|
||||
|
||||
|
|
|
|||
|
|
@ -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<Claim>
|
||||
{
|
||||
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<WorkOrderBoardValidationException>(() =>
|
||||
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<WorkOrderBoardValidationException>(() =>
|
||||
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<WorkOrderBoardValidationException>(() =>
|
||||
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<WorkOrderBoardValidationException>(() =>
|
||||
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);
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue