From 899da0eb4fd7e6a87c4705d4691ac6fc314beb48 Mon Sep 17 00:00:00 2001 From: Arthur Bassi Date: Tue, 4 Aug 2026 10:21:47 -0300 Subject: [PATCH] fix(work-orders): enforce media auth and base scope on mutations Require an authenticated ClaimsPrincipal at service entry and filter tracked work orders with board base scope so deleted/template rows surface as NotFound without disclosure. --- .../Controllers/WorkOrderMediaController.cs | 8 +- .../WorkOrderMediaDataService.cs | 4 +- .../Implementation/WorkOrderMediaService.cs | 29 ++++-- .../Interfaces/IWorkOrderMediaService.cs | 10 +- .../WorkOrderPhase6Tests.cs | 91 +++++++++++++++++-- 5 files changed, 117 insertions(+), 25 deletions(-) diff --git a/Api.SeaHavenIndustries/Controllers/WorkOrderMediaController.cs b/Api.SeaHavenIndustries/Controllers/WorkOrderMediaController.cs index 2d85749..b8d9967 100644 --- a/Api.SeaHavenIndustries/Controllers/WorkOrderMediaController.cs +++ b/Api.SeaHavenIndustries/Controllers/WorkOrderMediaController.cs @@ -53,10 +53,10 @@ namespace Api.SeaHavenIndustries.Controllers { WorkOrderMediaFileRules.EnsureAllowed(file); var actorId = User.FindFirstValue(ClaimTypes.NameIdentifier); - await _workOrderMediaService.EnsureCanMutateMediaAsync(id, actorId, cancellationToken); + await _workOrderMediaService.EnsureCanMutateMediaAsync(id, User, actorId, cancellationToken); fileUrl = await _fileStorage.SaveFileAsync(file); - var media = await _workOrderMediaService.AddMediaAsync(id, category, fileUrl, actorId, cancellationToken); + var media = await _workOrderMediaService.AddMediaAsync(id, category, fileUrl, User, actorId, cancellationToken); return Ok(media); } catch (WorkOrderBoardValidationException ex) when (ex.Code == "NotFound") @@ -94,7 +94,7 @@ namespace Api.SeaHavenIndustries.Controllers { var actorId = User.FindFirstValue(ClaimTypes.NameIdentifier); var media = await _workOrderMediaService.UpdateMediaCategoryAsync( - id, mediaId, category, workOrderVersion, actorId, cancellationToken); + id, mediaId, category, workOrderVersion, User, actorId, cancellationToken); return Ok(media); } catch (WorkOrderBoardValidationException ex) when (ex.Code == "NotFound") @@ -126,7 +126,7 @@ namespace Api.SeaHavenIndustries.Controllers try { var actorId = User.FindFirstValue(ClaimTypes.NameIdentifier); - await _workOrderMediaService.DeleteMediaAsync(id, mediaId, workOrderVersion, actorId, cancellationToken); + await _workOrderMediaService.DeleteMediaAsync(id, mediaId, workOrderVersion, User, actorId, cancellationToken); return NoContent(); } catch (WorkOrderBoardValidationException ex) when (ex.Code == "NotFound") diff --git a/SeaHaven.DataServices/Implementation/WorkOrderMediaDataService.cs b/SeaHaven.DataServices/Implementation/WorkOrderMediaDataService.cs index a6e481e..7a81f01 100644 --- a/SeaHaven.DataServices/Implementation/WorkOrderMediaDataService.cs +++ b/SeaHaven.DataServices/Implementation/WorkOrderMediaDataService.cs @@ -14,7 +14,9 @@ namespace SeaHaven.DataServices.Implementation } public Task GetTrackedWorkOrderAsync(int workOrderId, CancellationToken cancellationToken) - => _context.workOrders.FirstOrDefaultAsync(w => w.Id == workOrderId, cancellationToken); + => _context.workOrders.FirstOrDefaultAsync( + w => w.Id == workOrderId && w.istemplate != true && (w.IsDeleted != true || w.IsDeleted == null), + cancellationToken); public Task GetTrackedAttachmentAsync(int mediaId, int workOrderId, CancellationToken cancellationToken) => _context.workOrderAttachments.FirstOrDefaultAsync( diff --git a/SeaHaven.Services/Implementation/WorkOrderMediaService.cs b/SeaHaven.Services/Implementation/WorkOrderMediaService.cs index 7c7cf9d..9d9df01 100644 --- a/SeaHaven.Services/Implementation/WorkOrderMediaService.cs +++ b/SeaHaven.Services/Implementation/WorkOrderMediaService.cs @@ -1,3 +1,4 @@ +using System.Security.Claims; using Data.SeaHavenIndustries; using Data.SeaHavenIndustries.Enums; using SeaHaven.DataServices.Interfaces; @@ -41,10 +42,11 @@ namespace SeaHaven.Services.Implementation public async Task EnsureCanMutateMediaAsync( int workOrderId, + ClaimsPrincipal user, string? actorId, CancellationToken cancellationToken = default) { - EnsureAuthenticatedActor(actorId); + EnsureAuthenticatedCaller(user, actorId); await GetMutableWorkOrderAsync(workOrderId, cancellationToken); } @@ -52,10 +54,11 @@ namespace SeaHaven.Services.Implementation int workOrderId, WorkOrderMediaCategory? category, string fileUrl, + ClaimsPrincipal user, string? actorId, CancellationToken cancellationToken = default) { - EnsureAuthenticatedActor(actorId); + EnsureAuthenticatedCaller(user, actorId); var resolvedCategory = category ?? WorkOrderMediaCategory.Extra; var workOrder = await GetMutableWorkOrderAsync(workOrderId, cancellationToken); @@ -132,10 +135,11 @@ namespace SeaHaven.Services.Implementation int mediaId, WorkOrderMediaCategory category, string? workOrderVersion, + ClaimsPrincipal user, string? actorId, CancellationToken cancellationToken = default) { - EnsureAuthenticatedActor(actorId); + EnsureAuthenticatedCaller(user, actorId); if (mediaId <= 0) throw new WorkOrderBoardValidationException("InvalidMedia", "Legacy media cannot be categorized via this endpoint."); @@ -209,10 +213,11 @@ namespace SeaHaven.Services.Implementation int workOrderId, int mediaId, string? workOrderVersion, + ClaimsPrincipal user, string? actorId, CancellationToken cancellationToken = default) { - EnsureAuthenticatedActor(actorId); + EnsureAuthenticatedCaller(user, actorId); if (mediaId <= 0) throw new WorkOrderBoardValidationException("InvalidMedia", "Legacy media cannot be deleted via this endpoint."); @@ -241,9 +246,7 @@ namespace SeaHaven.Services.Implementation private async Task GetMutableWorkOrderAsync(int workOrderId, CancellationToken cancellationToken) { - if (!await _detailData.ExistsAsync(workOrderId)) - throw new WorkOrderBoardValidationException("NotFound", "Work order not found."); - + // Base-scoped lookup: deleted/template work orders surface as NotFound (no disclosure). var workOrder = await _mediaData.GetTrackedWorkOrderAsync(workOrderId, cancellationToken); if (workOrder == null) throw new WorkOrderBoardValidationException("NotFound", "Work order not found."); @@ -266,10 +269,16 @@ namespace SeaHaven.Services.Implementation _mediaData.SetExpectedWorkOrderVersion(workOrder, expected); } - private static void EnsureAuthenticatedActor(string? actorId) + private static void EnsureAuthenticatedCaller(ClaimsPrincipal user, string? actorId) { - if (string.IsNullOrWhiteSpace(actorId)) - throw new WorkOrderBoardValidationException("Forbidden", "You are not allowed to mutate work order media."); + if (user is null + || !(user.Identity?.IsAuthenticated ?? false) + || string.IsNullOrWhiteSpace(actorId)) + { + throw new WorkOrderBoardValidationException( + "Forbidden", + "You are not allowed to mutate work order media."); + } } private static byte[]? ParseRowVersion(string? base64) diff --git a/SeaHaven.Services/Interfaces/IWorkOrderMediaService.cs b/SeaHaven.Services/Interfaces/IWorkOrderMediaService.cs index fe1bbe1..13bb4c9 100644 --- a/SeaHaven.Services/Interfaces/IWorkOrderMediaService.cs +++ b/SeaHaven.Services/Interfaces/IWorkOrderMediaService.cs @@ -1,3 +1,4 @@ +using System.Security.Claims; using Data.SeaHavenIndustries.Enums; using SeaHaven.Services.DTOs; @@ -10,12 +11,17 @@ namespace SeaHaven.Services.Interfaces /// /// Authorizes the caller and validates the work order is mutable before any blob storage write. /// - Task EnsureCanMutateMediaAsync(int workOrderId, string? actorId, CancellationToken cancellationToken = default); + Task EnsureCanMutateMediaAsync( + int workOrderId, + ClaimsPrincipal user, + string? actorId, + CancellationToken cancellationToken = default); Task AddMediaAsync( int workOrderId, WorkOrderMediaCategory? category, string fileUrl, + ClaimsPrincipal user, string? actorId, CancellationToken cancellationToken = default); @@ -24,6 +30,7 @@ namespace SeaHaven.Services.Interfaces int mediaId, WorkOrderMediaCategory category, string? workOrderVersion, + ClaimsPrincipal user, string? actorId, CancellationToken cancellationToken = default); @@ -31,6 +38,7 @@ namespace SeaHaven.Services.Interfaces int workOrderId, int mediaId, string? workOrderVersion, + ClaimsPrincipal user, string? actorId, CancellationToken cancellationToken = default); } diff --git a/SeaHavenIndustries.Tests/WorkOrderPhase6Tests.cs b/SeaHavenIndustries.Tests/WorkOrderPhase6Tests.cs index 087d084..2ace8eb 100644 --- a/SeaHavenIndustries.Tests/WorkOrderPhase6Tests.cs +++ b/SeaHavenIndustries.Tests/WorkOrderPhase6Tests.cs @@ -1,3 +1,4 @@ +using System.Security.Claims; using System.Text; using Data.SeaHavenIndustries; using Data.SeaHavenIndustries.Enums; @@ -589,6 +590,17 @@ 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") + { + var identity = new ClaimsIdentity( + new[] { new Claim(ClaimTypes.NameIdentifier, actorId) }, + authenticationType: "Test"); + return new ClaimsPrincipal(identity); + } + + private static ClaimsPrincipal UnauthenticatedUser() + => new ClaimsPrincipal(new ClaimsIdentity()); + [Fact] public async Task GetMedia_IncludesLegacyBeforeAfterAndExtra() { @@ -636,7 +648,7 @@ public class WorkOrderMediaServiceTests await context.SaveChangesAsync(); var ex = await Assert.ThrowsAsync(() => - service.DeleteMediaAsync(1, 10, ToVersion(context.workOrders.Single()), "actor-1")); + service.DeleteMediaAsync(1, 10, ToVersion(context.workOrders.Single()), AuthenticatedUser(), "actor-1")); Assert.Equal("ReadOnly", ex.Code); } @@ -654,7 +666,7 @@ public class WorkOrderMediaServiceTests await context.SaveChangesAsync(); var ex = await Assert.ThrowsAsync(() => - service.AddMediaAsync(1, WorkOrderMediaCategory.Completion, "https://example.com/doc.pdf", "actor-1")); + service.AddMediaAsync(1, WorkOrderMediaCategory.Completion, "https://example.com/doc.pdf", AuthenticatedUser(), "actor-1")); Assert.Equal("UseCompletionDocEndpoint", ex.Code); } @@ -671,7 +683,7 @@ public class WorkOrderMediaServiceTests }); await context.SaveChangesAsync(); - var media = await service.AddMediaAsync(1, null, "https://example.com/photo.jpg", "actor-1"); + var media = await service.AddMediaAsync(1, null, "https://example.com/photo.jpg", AuthenticatedUser(), "actor-1"); Assert.True(media.Id > 0); Assert.Equal(WorkOrderMediaCategory.Extra, media.Category); @@ -692,7 +704,30 @@ public class WorkOrderMediaServiceTests await context.SaveChangesAsync(); var ex = await Assert.ThrowsAsync(() => - service.AddMediaAsync(1, WorkOrderMediaCategory.Extra, "https://example.com/photo.jpg", null)); + service.AddMediaAsync(1, WorkOrderMediaCategory.Extra, "https://example.com/photo.jpg", AuthenticatedUser(), null)); + + Assert.Equal("Forbidden", ex.Code); + } + + [Fact] + public async Task AddMedia_UnauthenticatedCaller_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", + UnauthenticatedUser(), + "actor-1")); Assert.Equal("Forbidden", ex.Code); } @@ -702,7 +737,45 @@ public class WorkOrderMediaServiceTests { var (_, service) = CreateSut(); var ex = await Assert.ThrowsAsync(() => - service.EnsureCanMutateMediaAsync(99, "actor-1")); + service.EnsureCanMutateMediaAsync(99, AuthenticatedUser(), "actor-1")); + Assert.Equal("NotFound", ex.Code); + } + + [Fact] + public async Task EnsureCanMutateMedia_DeletedWorkOrder_ThrowsNotFound() + { + var (context, service) = CreateSut(); + context.workOrders.Add(new WorkOrder + { + Id = 1, + LifecycleStatus = LifecycleStatus.Scheduled, + IsDeleted = true, + RowVersion = new byte[] { 1, 0, 0, 0, 0, 0, 0, 1 } + }); + await context.SaveChangesAsync(); + + var ex = await Assert.ThrowsAsync(() => + service.EnsureCanMutateMediaAsync(1, AuthenticatedUser(), "actor-1")); + + Assert.Equal("NotFound", ex.Code); + } + + [Fact] + public async Task EnsureCanMutateMedia_TemplateWorkOrder_ThrowsNotFound() + { + var (context, service) = CreateSut(); + context.workOrders.Add(new WorkOrder + { + Id = 1, + LifecycleStatus = LifecycleStatus.Scheduled, + istemplate = true, + RowVersion = new byte[] { 1, 0, 0, 0, 0, 0, 0, 1 } + }); + await context.SaveChangesAsync(); + + var ex = await Assert.ThrowsAsync(() => + service.EnsureCanMutateMediaAsync(1, AuthenticatedUser(), "actor-1")); + Assert.Equal("NotFound", ex.Code); } @@ -727,7 +800,7 @@ public class WorkOrderMediaServiceTests await context.SaveChangesAsync(); var media = await service.UpdateMediaCategoryAsync( - 1, 10, WorkOrderMediaCategory.Before, ToVersion(wo), "actor-1"); + 1, 10, WorkOrderMediaCategory.Before, ToVersion(wo), AuthenticatedUser(), "actor-1"); Assert.Equal(-1, media.Id); Assert.Equal(WorkOrderMediaCategory.Before, media.Category); @@ -758,7 +831,7 @@ public class WorkOrderMediaServiceTests var stale = Convert.ToBase64String(new byte[] { 1, 0, 0, 0, 0, 0, 0, 1 }); var ex = await Assert.ThrowsAsync(() => - service.UpdateMediaCategoryAsync(1, 10, WorkOrderMediaCategory.Extra, stale, "actor-1")); + service.UpdateMediaCategoryAsync(1, 10, WorkOrderMediaCategory.Extra, stale, AuthenticatedUser(), "actor-1")); Assert.Equal("ConcurrencyConflict", ex.Code); } @@ -790,7 +863,7 @@ public class WorkOrderMediaServiceTests var version = ToVersion(context.workOrders.Single(w => w.Id == 1)); var ex = await Assert.ThrowsAsync(() => - service.UpdateMediaCategoryAsync(1, 10, WorkOrderMediaCategory.Extra, version, "actor-1")); + service.UpdateMediaCategoryAsync(1, 10, WorkOrderMediaCategory.Extra, version, AuthenticatedUser(), "actor-1")); Assert.Equal("NotFound", ex.Code); } @@ -817,7 +890,7 @@ public class WorkOrderMediaServiceTests await context.SaveChangesAsync(); var ex = await Assert.ThrowsAsync(() => - service.UpdateMediaCategoryAsync(1, 10, WorkOrderMediaCategory.Before, ToVersion(wo), "actor-1")); + service.UpdateMediaCategoryAsync(1, 10, WorkOrderMediaCategory.Before, ToVersion(wo), AuthenticatedUser(), "actor-1")); Assert.Equal("NotFound", ex.Code); }