diff --git a/Api.SeaHavenIndustries.Tests/WorkOrderUpliftControllerTests.cs b/Api.SeaHavenIndustries.Tests/WorkOrderUpliftControllerTests.cs index 3784558..2b52ed2 100644 --- a/Api.SeaHavenIndustries.Tests/WorkOrderUpliftControllerTests.cs +++ b/Api.SeaHavenIndustries.Tests/WorkOrderUpliftControllerTests.cs @@ -79,6 +79,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 CreateUplift_ConcurrentActiveRequest_ReturnsConflict() { diff --git a/Api.SeaHavenIndustries.Tests/WorkOrderUpliftServiceConflictTests.cs b/Api.SeaHavenIndustries.Tests/WorkOrderUpliftServiceConflictTests.cs index 5366d8c..55d1372 100644 --- a/Api.SeaHavenIndustries.Tests/WorkOrderUpliftServiceConflictTests.cs +++ b/Api.SeaHavenIndustries.Tests/WorkOrderUpliftServiceConflictTests.cs @@ -1,6 +1,7 @@ using Data.SeaHavenIndustries; using Data.SeaHavenIndustries.Enums; using Moq; +using SeaHaven.DataServices.Dto; using SeaHaven.DataServices.Exceptions; using SeaHaven.DataServices.Interfaces; using SeaHaven.Services.Configuration; @@ -89,6 +90,10 @@ public sealed class WorkOrderUpliftServiceConflictTests var userData = new Mock(); userData.Setup(x => x.GetDisplayNamesByIdsAsync(It.IsAny>())) .ReturnsAsync(new Dictionary()); + // SH-327: the requester must hold RequestUplifts before the create reaches the insert. + var permissionData = new Mock(); + permissionData.Setup(x => x.GetUserAsync("dispatcher-1", It.IsAny())) + .ReturnsAsync(new TeamPermissionUserData { UserId = "dispatcher-1", RoleName = "Dispatcher" }); var service = new WorkOrderUpliftService( upliftData.Object, @@ -96,6 +101,8 @@ public sealed class WorkOrderUpliftServiceConflictTests detailData.Object, accountResolver.Object, userData.Object, + permissionData.Object, + new TeamPermissionPolicy(), TimeProvider.System, Microsoft.Extensions.Options.Options.Create(new ApprovalsOptions { diff --git a/Api.SeaHavenIndustries/Controllers/WorkOrderDetailController.cs b/Api.SeaHavenIndustries/Controllers/WorkOrderDetailController.cs index 742eaaf..edbafbc 100644 --- a/Api.SeaHavenIndustries/Controllers/WorkOrderDetailController.cs +++ b/Api.SeaHavenIndustries/Controllers/WorkOrderDetailController.cs @@ -174,7 +174,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) { 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 6910036..0980e0d 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"); @@ -89,6 +98,31 @@ namespace SeaHaven.Services.Implementation } } + 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); + + if (permissionUser is null) + { + throw new UpliftForbiddenException( + UpliftForbiddenException.RequestUpliftsDeniedMessage); + } + + if (!_permissionPolicy.IsAllowed( + permissionUser.RoleName, + TeamPermissionKeys.RequestUplifts, + permissionUser.Overrides)) + { + throw new UpliftForbiddenException( + UpliftForbiddenException.RequestUpliftsDeniedMessage); + } + } + private async Task CreateLockedAsync( int workOrderId, decimal amount, diff --git a/SeaHavenIndustries.Tests/WorkOrderBoardCancelServiceTests.cs b/SeaHavenIndustries.Tests/WorkOrderBoardCancelServiceTests.cs index 8e4fb7b..aad09c3 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; @@ -203,6 +204,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 +267,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( @@ -300,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 { @@ -332,6 +344,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 +359,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/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 { diff --git a/SeaHavenIndustries.Tests/WorkOrderUpliftServiceTests.cs b/SeaHavenIndustries.Tests/WorkOrderUpliftServiceTests.cs index 3ebb7c1..390beaf 100644 --- a/SeaHavenIndustries.Tests/WorkOrderUpliftServiceTests.cs +++ b/SeaHavenIndustries.Tests/WorkOrderUpliftServiceTests.cs @@ -1,9 +1,11 @@ using Data.SeaHavenIndustries; using Data.SeaHavenIndustries.Enums; +using Microsoft.AspNetCore.Identity; 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 +39,8 @@ public sealed class WorkOrderUpliftServiceTests new WorkOrderDetailDataService(context), WorkOrderAccountTestHelpers.Resolver(context), new UserDataService(context), + new TeamPermissionOverrideDataService(context), + new TeamPermissionPolicy(), TimeProvider.System, Options.Create(NewOptions())); } @@ -47,6 +51,7 @@ public sealed class WorkOrderUpliftServiceTests private static async Task<(WorkOrder WorkOrder, Dispatch Dispatch)> SeedWorkOrderAsync( ApplicationDbContext context, WorkOrderType type = WorkOrderType.PM, + string role = "Admin", int? primaryDispatchId = 10) { await WorkOrderAccountTestHelpers.EnsureAccountAsync(context); @@ -57,6 +62,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 { @@ -124,6 +136,22 @@ public sealed class WorkOrderUpliftServiceTests Assert.Equal(1400m, context.Dispatches.Single(d => d.Id == 10).NTEAmount); } + [Fact] + public async Task CreateAsync_CurrentDatabaseAdmin_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 }, + WorkOrderAccountTestHelpers.AccountUser("dispatcher-1", 1, "Scheduler"), + CancellationToken.None); + + Assert.NotNull(created); + } + [Fact] public async Task CreateAsync_WithoutPrimaryDispatch_UsesWorkOrderDispatch() { @@ -174,6 +202,109 @@ public sealed class WorkOrderUpliftServiceTests Assert.Equal(workOrder.Id, audit.WorkOrderId); } + [Fact] + public async Task CreateAsync_StaleAdminClaim_DeniesWhenDatabaseRoleIsNotAdmin() + { + await using var context = CreateContext(); + var (workOrder, dispatch) = await SeedWorkOrderAsync(context, role: "Scheduler"); + 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.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); + + 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, role: role); + 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, role: role); + 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() {