From 9156107e725f961fb7408dea4499ac1119b31257 Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Tue, 22 Sep 2026 23:09:47 -0300 Subject: [PATCH 1/6] fix(uplifts): enforce request permission at service boundary --- .../Implementation/WorkOrderUpliftService.cs | 31 +++++++ .../WorkOrderBoardCancelServiceTests.cs | 8 ++ .../WorkOrderUpliftServiceTests.cs | 83 +++++++++++++++++++ 3 files changed, 122 insertions(+) diff --git a/SeaHaven.Services/Implementation/WorkOrderUpliftService.cs b/SeaHaven.Services/Implementation/WorkOrderUpliftService.cs index 6a2c388..261a8c5 100644 --- a/SeaHaven.Services/Implementation/WorkOrderUpliftService.cs +++ b/SeaHaven.Services/Implementation/WorkOrderUpliftService.cs @@ -4,6 +4,7 @@ using Data.SeaHavenIndustries.Enums; using Microsoft.Extensions.Options; using SeaHaven.DataServices.Interfaces; using SeaHaven.Services.Configuration; +using SeaHaven.Services.Constants; using SeaHaven.Services.DTOs; using SeaHaven.Services.Exceptions; using SeaHaven.Services.Helpers; @@ -19,6 +20,8 @@ namespace SeaHaven.Services.Implementation private readonly IWorkOrderDetailDataService _detailData; private readonly IWorkOrderAccountResolver _accountResolver; private readonly IUserDataService _userData; + private readonly ITeamPermissionOverrideDataService _permissionOverrideData; + private readonly ITeamPermissionPolicy _permissionPolicy; private readonly TimeProvider _timeProvider; private readonly ApprovalsOptions _approvalsOptions; @@ -28,6 +31,8 @@ namespace SeaHaven.Services.Implementation IWorkOrderDetailDataService detailData, IWorkOrderAccountResolver accountResolver, IUserDataService userData, + ITeamPermissionOverrideDataService permissionOverrideData, + ITeamPermissionPolicy permissionPolicy, TimeProvider timeProvider, IOptions approvalsOptions) { @@ -36,6 +41,8 @@ namespace SeaHaven.Services.Implementation _detailData = detailData; _accountResolver = accountResolver; _userData = userData; + _permissionOverrideData = permissionOverrideData; + _permissionPolicy = permissionPolicy; _timeProvider = timeProvider; _approvalsOptions = approvalsOptions.Value; } @@ -61,6 +68,8 @@ namespace SeaHaven.Services.Implementation if (!await HasWorkOrderAccessAsync(workOrderId, user, cancellationToken)) return null; + await EnsureCanRequestUpliftAsync(user, cancellationToken); + if (request.Amount <= 0) throw new InvalidOperationException("Uplift amount must be greater than zero"); @@ -82,6 +91,28 @@ namespace SeaHaven.Services.Implementation cancellationToken); } + private async Task EnsureCanRequestUpliftAsync( + ClaimsPrincipal user, + CancellationToken cancellationToken) + { + var userId = user.FindFirstValue(ClaimTypes.NameIdentifier); + var permissionUser = string.IsNullOrWhiteSpace(userId) + ? null + : await _permissionOverrideData.GetUserAsync(userId, cancellationToken); + var roleName = user.IsInRole("Admin") + ? "Admin" + : user.FindFirstValue(ClaimTypes.Role); + + if (!_permissionPolicy.IsAllowed( + roleName, + TeamPermissionKeys.RequestUplifts, + permissionUser?.Overrides)) + { + throw new UpliftForbiddenException( + "Your role can't request uplifts on this work order."); + } + } + private async Task CreateLockedAsync( int workOrderId, decimal amount, diff --git a/SeaHavenIndustries.Tests/WorkOrderBoardCancelServiceTests.cs b/SeaHavenIndustries.Tests/WorkOrderBoardCancelServiceTests.cs index e797a68..c910604 100644 --- a/SeaHavenIndustries.Tests/WorkOrderBoardCancelServiceTests.cs +++ b/SeaHavenIndustries.Tests/WorkOrderBoardCancelServiceTests.cs @@ -203,6 +203,8 @@ public class WorkOrderBoardCancelServiceTests new WorkOrderDetailDataService(context), WorkOrderAccountTestHelpers.Resolver(context), new UserDataService(context), + new TeamPermissionOverrideDataService(context), + new TeamPermissionPolicy(), TimeProvider.System, Options.Create(new ApprovalsOptions())); var cancel = new WorkOrderBoardCancelService(mutationData, boardService, audit, uplifts, new PassThroughUpliftData()); @@ -264,6 +266,8 @@ public class WorkOrderBoardCancelServiceTests new WorkOrderDetailDataService(context), WorkOrderAccountTestHelpers.Resolver(context), new UserDataService(context), + new TeamPermissionOverrideDataService(context), + new TeamPermissionPolicy(), TimeProvider.System, Options.Create(new ApprovalsOptions())); var cancel = new WorkOrderBoardCancelService( @@ -332,6 +336,8 @@ public class WorkOrderBoardCancelServiceTests new WorkOrderDetailDataService(createContext), WorkOrderAccountTestHelpers.Resolver(createContext), new UserDataService(createContext), + new TeamPermissionOverrideDataService(createContext), + new TeamPermissionPolicy(), TimeProvider.System, Options.Create(new ApprovalsOptions())); var boardData = new WorkOrderBoardDataService(cancelContext); @@ -345,6 +351,8 @@ public class WorkOrderBoardCancelServiceTests new WorkOrderDetailDataService(cancelContext), WorkOrderAccountTestHelpers.Resolver(cancelContext), new UserDataService(cancelContext), + new TeamPermissionOverrideDataService(cancelContext), + new TeamPermissionPolicy(), TimeProvider.System, Options.Create(new ApprovalsOptions())); var cancel = new WorkOrderBoardCancelService( diff --git a/SeaHavenIndustries.Tests/WorkOrderUpliftServiceTests.cs b/SeaHavenIndustries.Tests/WorkOrderUpliftServiceTests.cs index 7ec8856..bb63d73 100644 --- a/SeaHavenIndustries.Tests/WorkOrderUpliftServiceTests.cs +++ b/SeaHavenIndustries.Tests/WorkOrderUpliftServiceTests.cs @@ -4,6 +4,7 @@ using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Options; using SeaHaven.DataServices.Implementation; using SeaHaven.Services.Configuration; +using SeaHaven.Services.Constants; using SeaHaven.Services.DTOs; using SeaHaven.Services.Implementation; using SeaHaven.Services.Interfaces; @@ -37,6 +38,8 @@ public sealed class WorkOrderUpliftServiceTests new WorkOrderDetailDataService(context), WorkOrderAccountTestHelpers.Resolver(context), new UserDataService(context), + new TeamPermissionOverrideDataService(context), + new TeamPermissionPolicy(), TimeProvider.System, Options.Create(NewOptions())); } @@ -123,6 +126,86 @@ public sealed class WorkOrderUpliftServiceTests Assert.Equal(1400m, context.Dispatches.Single(d => d.Id == 10).NTEAmount); } + [Fact] + public async Task CreateAsync_Admin_RemainsUnrestricted() + { + await using var context = CreateContext(); + var (workOrder, _) = await SeedWorkOrderAsync(context); + var service = NewService(context); + + var created = await service.CreateAsync( + workOrder.Id, + new CreateWorkOrderUpliftRequestDto { Amount = 400m }, + Dispatcher(), + CancellationToken.None); + + Assert.NotNull(created); + } + + [Theory] + [InlineData("Dispatcher", true)] + [InlineData("Scheduler", false)] + public async Task CreateAsync_UsesRoleDefaultForRequestPermission(string role, bool allowed) + { + await using var context = CreateContext(); + var (workOrder, dispatch) = await SeedWorkOrderAsync(context); + var service = NewService(context); + + var act = () => service.CreateAsync( + workOrder.Id, + new CreateWorkOrderUpliftRequestDto { Amount = 400m }, + WorkOrderAccountTestHelpers.AccountUser("dispatcher-1", 1, role), + CancellationToken.None); + + if (allowed) + { + Assert.NotNull(await act()); + return; + } + + var exception = await Assert.ThrowsAsync(act); + Assert.Equal("Your role can't request uplifts on this work order.", exception.Message); + Assert.Empty(context.DispatchUpliftRequests); + Assert.Equal(1000m, dispatch.NTEAmount); + } + + [Theory] + [InlineData("Scheduler", UserPermissionState.Allow, true)] + [InlineData("Dispatcher", UserPermissionState.Deny, false)] + public async Task CreateAsync_AppliesExplicitRequestPermissionOverride( + string role, + UserPermissionState overrideState, + bool allowed) + { + await using var context = CreateContext(); + var (workOrder, dispatch) = await SeedWorkOrderAsync(context); + context.UserPermissionOverrides.Add(new UserPermissionOverride + { + UserId = "dispatcher-1", + PermissionKey = TeamPermissionKeys.RequestUplifts, + State = overrideState, + }); + await context.SaveChangesAsync(); + var service = NewService(context); + + var act = () => service.CreateAsync( + workOrder.Id, + new CreateWorkOrderUpliftRequestDto { Amount = 400m }, + WorkOrderAccountTestHelpers.AccountUser("dispatcher-1", 1, role), + CancellationToken.None); + + if (allowed) + { + Assert.NotNull(await act()); + return; + } + + var exception = await Assert.ThrowsAsync(act); + Assert.Equal("Your role can't request uplifts on this work order.", exception.Message); + Assert.Empty(context.DispatchUpliftRequests); + Assert.Equal(1000m, dispatch.NTEAmount); + } + [Fact] public async Task CreateAsync_PmAmountAboveCap_CreatesPendingRequest() { From ff6a5945f1381151ad71da5d8f1a9e836d36af7f Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Tue, 22 Sep 2026 23:24:17 -0300 Subject: [PATCH 2/6] fix(uplifts): honor current permissions and denial contract --- .../VendorPortalControllerTests.cs | 32 ++++++++++- .../Controllers/VendorPortalController.cs | 10 ++++ SeaHaven.Services/DTOs/UpliftDTOs.cs | 3 + .../Implementation/WorkOrderUpliftService.cs | 19 ++++--- .../WorkOrderBoardCancelServiceTests.cs | 8 +++ .../WorkOrderUpliftServiceTests.cs | 57 +++++++++++++++++-- 6 files changed, 115 insertions(+), 14 deletions(-) diff --git a/Api.SeaHavenIndustries.Tests/VendorPortalControllerTests.cs b/Api.SeaHavenIndustries.Tests/VendorPortalControllerTests.cs index dd9e40e..0e48da5 100644 --- a/Api.SeaHavenIndustries.Tests/VendorPortalControllerTests.cs +++ b/Api.SeaHavenIndustries.Tests/VendorPortalControllerTests.cs @@ -306,7 +306,37 @@ public class VendorPortalControllerTests var result = await controller.RequestUplift(10, new VendorPortalController.UpliftRequestBody { RequestedNTE = 100m }); - result.Should().BeOfType(); + var badRequest = result.Should().BeOfType().Subject; + var response = badRequest.Value.Should().BeOfType().Subject; + response.Message.Should().Contain("reference"); + response.Message.Should().NotContain("Requested NTE must be greater than the current NTE"); + } + + [Fact] + public async Task RequestUplift_Forbidden_Returns403WithExactPermissionMessage() + { + var service = new Mock(); + service.Setup(x => x.ResolveSessionAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync(Session); + service.Setup(x => x.RequestUpliftAsync( + Session, + 10, + 500m, + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .ThrowsAsync(new UpliftForbiddenException("internal permission detail")); + var controller = NewController(service); + + var result = await controller.RequestUplift( + 10, + new VendorPortalController.UpliftRequestBody { RequestedNTE = 500m }); + + var forbidden = result.Should().BeOfType().Subject; + forbidden.StatusCode.Should().Be(StatusCodes.Status403Forbidden); + var response = forbidden.Value.Should().BeOfType().Subject; + response.Message.Should().Be("Your role can't request uplifts on this work order."); } [Fact] diff --git a/Api.SeaHavenIndustries/Controllers/VendorPortalController.cs b/Api.SeaHavenIndustries/Controllers/VendorPortalController.cs index 39dd90b..9fdeed4 100644 --- a/Api.SeaHavenIndustries/Controllers/VendorPortalController.cs +++ b/Api.SeaHavenIndustries/Controllers/VendorPortalController.cs @@ -74,6 +74,16 @@ namespace Api.SeaHavenIndustries.Controllers { return NotFound(new Response { Status = "Error", Message = "Dispatch not found" }); } + catch (UpliftForbiddenException) + { + return StatusCode( + StatusCodes.Status403Forbidden, + new Response + { + Status = "Error", + Message = UpliftForbiddenException.RequestUpliftsDeniedMessage + }); + } catch (InvalidOperationException ex) { return BadRequest(new Response { Status = "Error", Message = _logger.Sanitize(ex, "The requested action could not be completed for this dispatch") }); diff --git a/SeaHaven.Services/DTOs/UpliftDTOs.cs b/SeaHaven.Services/DTOs/UpliftDTOs.cs index 24cde20..d93dd63 100644 --- a/SeaHaven.Services/DTOs/UpliftDTOs.cs +++ b/SeaHaven.Services/DTOs/UpliftDTOs.cs @@ -104,6 +104,9 @@ namespace SeaHaven.Services.DTOs public class UpliftForbiddenException : Exception { + public const string RequestUpliftsDeniedMessage = + "Your role can't request uplifts on this work order."; + public UpliftForbiddenException(string message) : base(message) { } } diff --git a/SeaHaven.Services/Implementation/WorkOrderUpliftService.cs b/SeaHaven.Services/Implementation/WorkOrderUpliftService.cs index 261a8c5..f5548ee 100644 --- a/SeaHaven.Services/Implementation/WorkOrderUpliftService.cs +++ b/SeaHaven.Services/Implementation/WorkOrderUpliftService.cs @@ -99,17 +99,20 @@ namespace SeaHaven.Services.Implementation var permissionUser = string.IsNullOrWhiteSpace(userId) ? null : await _permissionOverrideData.GetUserAsync(userId, cancellationToken); - var roleName = user.IsInRole("Admin") - ? "Admin" - : user.FindFirstValue(ClaimTypes.Role); - if (!_permissionPolicy.IsAllowed( - roleName, - TeamPermissionKeys.RequestUplifts, - permissionUser?.Overrides)) + if (permissionUser is null) { throw new UpliftForbiddenException( - "Your role can't request uplifts on this work order."); + UpliftForbiddenException.RequestUpliftsDeniedMessage); + } + + if (!_permissionPolicy.IsAllowed( + permissionUser.RoleName, + TeamPermissionKeys.RequestUplifts, + permissionUser.Overrides)) + { + throw new UpliftForbiddenException( + UpliftForbiddenException.RequestUpliftsDeniedMessage); } } diff --git a/SeaHavenIndustries.Tests/WorkOrderBoardCancelServiceTests.cs b/SeaHavenIndustries.Tests/WorkOrderBoardCancelServiceTests.cs index c910604..9f1e3f6 100644 --- a/SeaHavenIndustries.Tests/WorkOrderBoardCancelServiceTests.cs +++ b/SeaHavenIndustries.Tests/WorkOrderBoardCancelServiceTests.cs @@ -1,5 +1,6 @@ using Data.SeaHavenIndustries; using Data.SeaHavenIndustries.Enums; +using Microsoft.AspNetCore.Identity; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Options; using SeaHaven.DataServices.Implementation; @@ -304,6 +305,13 @@ public class WorkOrderBoardCancelServiceTests FirstName = "Alex", LastName = "Dispatcher", }); + var adminRole = new IdentityRole("Admin"); + seed.Roles.Add(adminRole); + seed.UserRoles.Add(new IdentityUserRole + { + UserId = "dispatcher-1", + RoleId = adminRole.Id, + }); seed.Vendors.Add(new Vendor { Id = 1, CompanyName = "Acme HVAC" }); seed.Dispatches.Add(new Dispatch { diff --git a/SeaHavenIndustries.Tests/WorkOrderUpliftServiceTests.cs b/SeaHavenIndustries.Tests/WorkOrderUpliftServiceTests.cs index bb63d73..86a7613 100644 --- a/SeaHavenIndustries.Tests/WorkOrderUpliftServiceTests.cs +++ b/SeaHavenIndustries.Tests/WorkOrderUpliftServiceTests.cs @@ -1,5 +1,6 @@ using Data.SeaHavenIndustries; using Data.SeaHavenIndustries.Enums; +using Microsoft.AspNetCore.Identity; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Options; using SeaHaven.DataServices.Implementation; @@ -49,7 +50,8 @@ public sealed class WorkOrderUpliftServiceTests private static async Task<(WorkOrder WorkOrder, Dispatch Dispatch)> SeedWorkOrderAsync( ApplicationDbContext context, - WorkOrderType type = WorkOrderType.PM) + WorkOrderType type = WorkOrderType.PM, + string role = "Admin") { await WorkOrderAccountTestHelpers.EnsureAccountAsync(context); context.Users.Add(new ApplicationUser @@ -59,6 +61,13 @@ public sealed class WorkOrderUpliftServiceTests FirstName = "Alex", LastName = "Dispatcher", }); + var identityRole = new IdentityRole(role); + context.Roles.Add(identityRole); + context.UserRoles.Add(new IdentityUserRole + { + UserId = "dispatcher-1", + RoleId = identityRole.Id, + }); context.Vendors.Add(new Vendor { Id = 1, CompanyName = "Acme HVAC" }); context.Dispatches.Add(new Dispatch { @@ -127,7 +136,7 @@ public sealed class WorkOrderUpliftServiceTests } [Fact] - public async Task CreateAsync_Admin_RemainsUnrestricted() + public async Task CreateAsync_CurrentDatabaseAdmin_RemainsUnrestricted() { await using var context = CreateContext(); var (workOrder, _) = await SeedWorkOrderAsync(context); @@ -136,19 +145,57 @@ public sealed class WorkOrderUpliftServiceTests var created = await service.CreateAsync( workOrder.Id, new CreateWorkOrderUpliftRequestDto { Amount = 400m }, - Dispatcher(), + WorkOrderAccountTestHelpers.AccountUser("dispatcher-1", 1, "Scheduler"), CancellationToken.None); Assert.NotNull(created); } + [Fact] + public async Task CreateAsync_StaleAdminClaim_DeniesWhenDatabaseRoleIsNotAdmin() + { + await using var context = CreateContext(); + var (workOrder, dispatch) = await SeedWorkOrderAsync(context, role: "Dispatcher"); + var service = NewService(context); + + var exception = await Assert.ThrowsAsync(() => service.CreateAsync( + workOrder.Id, + new CreateWorkOrderUpliftRequestDto { Amount = 400m }, + Dispatcher(), + CancellationToken.None)); + + Assert.Equal(UpliftForbiddenException.RequestUpliftsDeniedMessage, exception.Message); + Assert.Empty(context.DispatchUpliftRequests); + Assert.Equal(1000m, dispatch.NTEAmount); + } + + [Fact] + public async Task CreateAsync_MissingDatabaseUser_DeniesEvenWithAdminClaim() + { + await using var context = CreateContext(); + var (workOrder, dispatch) = await SeedWorkOrderAsync(context); + context.Users.Remove(context.Users.Single(user => user.Id == "dispatcher-1")); + await context.SaveChangesAsync(); + var service = NewService(context); + + var exception = await Assert.ThrowsAsync(() => service.CreateAsync( + workOrder.Id, + new CreateWorkOrderUpliftRequestDto { Amount = 400m }, + Dispatcher("dispatcher-1"), + CancellationToken.None)); + + Assert.Equal(UpliftForbiddenException.RequestUpliftsDeniedMessage, exception.Message); + Assert.Empty(context.DispatchUpliftRequests); + Assert.Equal(1000m, dispatch.NTEAmount); + } + [Theory] [InlineData("Dispatcher", true)] [InlineData("Scheduler", false)] public async Task CreateAsync_UsesRoleDefaultForRequestPermission(string role, bool allowed) { await using var context = CreateContext(); - var (workOrder, dispatch) = await SeedWorkOrderAsync(context); + var (workOrder, dispatch) = await SeedWorkOrderAsync(context, role: role); var service = NewService(context); var act = () => service.CreateAsync( @@ -178,7 +225,7 @@ public sealed class WorkOrderUpliftServiceTests bool allowed) { await using var context = CreateContext(); - var (workOrder, dispatch) = await SeedWorkOrderAsync(context); + var (workOrder, dispatch) = await SeedWorkOrderAsync(context, role: role); context.UserPermissionOverrides.Add(new UserPermissionOverride { UserId = "dispatcher-1", From f4284badf2499514af66cc8d643986038047654c Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Tue, 22 Sep 2026 23:30:14 -0300 Subject: [PATCH 3/6] fix(uplifts): place forbidden handler on request endpoint --- .../Controllers/VendorPortalController.cs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Api.SeaHavenIndustries/Controllers/VendorPortalController.cs b/Api.SeaHavenIndustries/Controllers/VendorPortalController.cs index 9fdeed4..ec48a68 100644 --- a/Api.SeaHavenIndustries/Controllers/VendorPortalController.cs +++ b/Api.SeaHavenIndustries/Controllers/VendorPortalController.cs @@ -74,16 +74,6 @@ namespace Api.SeaHavenIndustries.Controllers { return NotFound(new Response { Status = "Error", Message = "Dispatch not found" }); } - catch (UpliftForbiddenException) - { - return StatusCode( - StatusCodes.Status403Forbidden, - new Response - { - Status = "Error", - Message = UpliftForbiddenException.RequestUpliftsDeniedMessage - }); - } catch (InvalidOperationException ex) { return BadRequest(new Response { Status = "Error", Message = _logger.Sanitize(ex, "The requested action could not be completed for this dispatch") }); @@ -231,6 +221,16 @@ namespace Api.SeaHavenIndustries.Controllers { return NotFound(new Response { Status = "Error", Message = "Dispatch not found" }); } + catch (UpliftForbiddenException) + { + return StatusCode( + StatusCodes.Status403Forbidden, + new Response + { + Status = "Error", + Message = UpliftForbiddenException.RequestUpliftsDeniedMessage + }); + } catch (InvalidOperationException ex) { return BadRequest(new Response { Status = "Error", Message = _logger.Sanitize(ex, "The requested action could not be completed for this dispatch") }); From e1632bc3bf9e2709aba656390a944a11ace93597 Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Tue, 22 Sep 2026 23:35:07 -0300 Subject: [PATCH 4/6] fix(uplifts): expose only allowlisted denial copy --- .../VendorPortalControllerTests.cs | 32 +------------ .../WorkOrderUpliftControllerTests.cs | 47 +++++++++++++++++++ .../Controllers/VendorPortalController.cs | 10 ---- .../Controllers/WorkOrderDetailController.cs | 5 +- 4 files changed, 52 insertions(+), 42 deletions(-) diff --git a/Api.SeaHavenIndustries.Tests/VendorPortalControllerTests.cs b/Api.SeaHavenIndustries.Tests/VendorPortalControllerTests.cs index 0e48da5..dd9e40e 100644 --- a/Api.SeaHavenIndustries.Tests/VendorPortalControllerTests.cs +++ b/Api.SeaHavenIndustries.Tests/VendorPortalControllerTests.cs @@ -306,37 +306,7 @@ public class VendorPortalControllerTests var result = await controller.RequestUplift(10, new VendorPortalController.UpliftRequestBody { RequestedNTE = 100m }); - var badRequest = result.Should().BeOfType().Subject; - var response = badRequest.Value.Should().BeOfType().Subject; - response.Message.Should().Contain("reference"); - response.Message.Should().NotContain("Requested NTE must be greater than the current NTE"); - } - - [Fact] - public async Task RequestUplift_Forbidden_Returns403WithExactPermissionMessage() - { - var service = new Mock(); - service.Setup(x => x.ResolveSessionAsync(It.IsAny(), It.IsAny())) - .ReturnsAsync(Session); - service.Setup(x => x.RequestUpliftAsync( - Session, - 10, - 500m, - It.IsAny(), - It.IsAny(), - It.IsAny(), - It.IsAny())) - .ThrowsAsync(new UpliftForbiddenException("internal permission detail")); - var controller = NewController(service); - - var result = await controller.RequestUplift( - 10, - new VendorPortalController.UpliftRequestBody { RequestedNTE = 500m }); - - var forbidden = result.Should().BeOfType().Subject; - forbidden.StatusCode.Should().Be(StatusCodes.Status403Forbidden); - var response = forbidden.Value.Should().BeOfType().Subject; - response.Message.Should().Be("Your role can't request uplifts on this work order."); + result.Should().BeOfType(); } [Fact] diff --git a/Api.SeaHavenIndustries.Tests/WorkOrderUpliftControllerTests.cs b/Api.SeaHavenIndustries.Tests/WorkOrderUpliftControllerTests.cs index b3cdbfc..8dabbcd 100644 --- a/Api.SeaHavenIndustries.Tests/WorkOrderUpliftControllerTests.cs +++ b/Api.SeaHavenIndustries.Tests/WorkOrderUpliftControllerTests.cs @@ -78,6 +78,53 @@ public sealed class WorkOrderUpliftControllerTests Assert.Equal(12, created.Id); } + [Fact] + public async Task CreateUplift_RequestPermissionDenied_ReturnsExact403Message() + { + var service = new Mock(); + service.Setup(x => x.CreateAsync( + 7, + It.IsAny(), + It.IsAny(), + It.IsAny())) + .ThrowsAsync(new UpliftForbiddenException(UpliftForbiddenException.RequestUpliftsDeniedMessage)); + + var controller = NewController(service, "Dispatcher"); + var result = await controller.CreateUplift( + 7, + new CreateWorkOrderUpliftRequestDto { Amount = 750m }, + CancellationToken.None); + + var forbidden = result.Should().BeOfType().Subject; + forbidden.StatusCode.Should().Be(StatusCodes.Status403Forbidden); + var response = forbidden.Value.Should().BeOfType().Subject; + response.Message.Should().Be(UpliftForbiddenException.RequestUpliftsDeniedMessage); + } + + [Fact] + public async Task CreateUplift_UnrelatedForbiddenException_ReturnsSanitized403() + { + var service = new Mock(); + service.Setup(x => x.CreateAsync( + 7, + It.IsAny(), + It.IsAny(), + It.IsAny())) + .ThrowsAsync(new UpliftForbiddenException("SECRET-internal-tier-detail")); + + var controller = NewController(service, "Dispatcher"); + var result = await controller.CreateUplift( + 7, + new CreateWorkOrderUpliftRequestDto { Amount = 750m }, + CancellationToken.None); + + var forbidden = result.Should().BeOfType().Subject; + forbidden.StatusCode.Should().Be(StatusCodes.Status403Forbidden); + var response = forbidden.Value.Should().BeOfType().Subject; + response.Message.Should().NotContain("SECRET-internal-tier-detail"); + response.Message.Should().Contain("reference"); + } + [Fact] public async Task CancelUplift_NotFound_Returns404() { diff --git a/Api.SeaHavenIndustries/Controllers/VendorPortalController.cs b/Api.SeaHavenIndustries/Controllers/VendorPortalController.cs index ec48a68..39dd90b 100644 --- a/Api.SeaHavenIndustries/Controllers/VendorPortalController.cs +++ b/Api.SeaHavenIndustries/Controllers/VendorPortalController.cs @@ -221,16 +221,6 @@ namespace Api.SeaHavenIndustries.Controllers { return NotFound(new Response { Status = "Error", Message = "Dispatch not found" }); } - catch (UpliftForbiddenException) - { - return StatusCode( - StatusCodes.Status403Forbidden, - new Response - { - Status = "Error", - Message = UpliftForbiddenException.RequestUpliftsDeniedMessage - }); - } catch (InvalidOperationException ex) { return BadRequest(new Response { Status = "Error", Message = _logger.Sanitize(ex, "The requested action could not be completed for this dispatch") }); diff --git a/Api.SeaHavenIndustries/Controllers/WorkOrderDetailController.cs b/Api.SeaHavenIndustries/Controllers/WorkOrderDetailController.cs index d975b32..be0a708 100644 --- a/Api.SeaHavenIndustries/Controllers/WorkOrderDetailController.cs +++ b/Api.SeaHavenIndustries/Controllers/WorkOrderDetailController.cs @@ -170,7 +170,10 @@ namespace Api.SeaHavenIndustries.Controllers } catch (UpliftForbiddenException ex) { - return StatusCode(StatusCodes.Status403Forbidden, new Response { Status = "Error", Message = _logger.Sanitize(ex, "You are not authorized to perform this action") }); + var message = ex.Message == UpliftForbiddenException.RequestUpliftsDeniedMessage + ? UpliftForbiddenException.RequestUpliftsDeniedMessage + : _logger.Sanitize(ex, "You are not authorized to perform this action"); + return StatusCode(StatusCodes.Status403Forbidden, new Response { Status = "Error", Message = message }); } catch (InvalidOperationException ex) { From 50a2cbab5fd912287eaaab81aba6e6ef5e20c1cc Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Tue, 22 Sep 2026 23:39:51 -0300 Subject: [PATCH 5/6] test(uplifts): correct current-role denial fixtures --- SeaHavenIndustries.Tests/WorkOrderUpliftServiceTests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/SeaHavenIndustries.Tests/WorkOrderUpliftServiceTests.cs b/SeaHavenIndustries.Tests/WorkOrderUpliftServiceTests.cs index 86a7613..1740334 100644 --- a/SeaHavenIndustries.Tests/WorkOrderUpliftServiceTests.cs +++ b/SeaHavenIndustries.Tests/WorkOrderUpliftServiceTests.cs @@ -155,7 +155,7 @@ public sealed class WorkOrderUpliftServiceTests public async Task CreateAsync_StaleAdminClaim_DeniesWhenDatabaseRoleIsNotAdmin() { await using var context = CreateContext(); - var (workOrder, dispatch) = await SeedWorkOrderAsync(context, role: "Dispatcher"); + var (workOrder, dispatch) = await SeedWorkOrderAsync(context, role: "Scheduler"); var service = NewService(context); var exception = await Assert.ThrowsAsync(() => service.CreateAsync( @@ -174,6 +174,7 @@ public sealed class WorkOrderUpliftServiceTests { await using var context = CreateContext(); var (workOrder, dispatch) = await SeedWorkOrderAsync(context); + context.UserRoles.RemoveRange(context.UserRoles.Where(userRole => userRole.UserId == "dispatcher-1")); context.Users.Remove(context.Users.Single(user => user.Id == "dispatcher-1")); await context.SaveChangesAsync(); var service = NewService(context); From b116e71d168eb08abf41485ffd65f5f0ecbef3ee Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Thu, 24 Sep 2026 22:31:07 -0300 Subject: [PATCH 6/6] test(uplifts): grant the SH-393 ownership fixture actor RequestUplifts Uplift creation now checks the actor's RequestUplifts permission, so the ownership tests construct the service with the permission services and seed their actor as an Admin. --- .../WorkOrderUpliftDispatchOwnershipTests.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/SeaHavenIndustries.Tests/WorkOrderUpliftDispatchOwnershipTests.cs b/SeaHavenIndustries.Tests/WorkOrderUpliftDispatchOwnershipTests.cs index 840fd28..adba1a7 100644 --- a/SeaHavenIndustries.Tests/WorkOrderUpliftDispatchOwnershipTests.cs +++ b/SeaHavenIndustries.Tests/WorkOrderUpliftDispatchOwnershipTests.cs @@ -1,5 +1,6 @@ using Data.SeaHavenIndustries; using Data.SeaHavenIndustries.Enums; +using Microsoft.AspNetCore.Identity; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Options; using SeaHaven.DataServices.Implementation; @@ -31,6 +32,11 @@ public sealed class WorkOrderUpliftDispatchOwnershipTests var context = new ApplicationDbContext(options); WorkOrderAccountTestHelpers.SeedBoardCreateScope(context); context.Vendors.Add(new Vendor { Id = 5, CompanyName = "Acme HVAC" }); + // SH-327: uplift creation checks the actor's RequestUplifts permission. + var adminRole = new IdentityRole("Admin"); + context.Roles.Add(adminRole); + context.Users.Add(new ApplicationUser { Id = "actor-1", UserName = "actor-1" }); + context.UserRoles.Add(new IdentityUserRole { UserId = "actor-1", RoleId = adminRole.Id }); context.SaveChanges(); return context; } @@ -67,6 +73,8 @@ public sealed class WorkOrderUpliftDispatchOwnershipTests new WorkOrderDetailDataService(context), WorkOrderAccountTestHelpers.Resolver(context), new UserDataService(context), + new TeamPermissionOverrideDataService(context), + new TeamPermissionPolicy(), TimeProvider.System, Options.Create(new ApprovalsOptions {