diff --git a/Api.SeaHavenIndustries.Tests/UpliftRevokeEndpointRulesTests.cs b/Api.SeaHavenIndustries.Tests/UpliftRevokeEndpointRulesTests.cs new file mode 100644 index 0000000..41a45df --- /dev/null +++ b/Api.SeaHavenIndustries.Tests/UpliftRevokeEndpointRulesTests.cs @@ -0,0 +1,276 @@ +using System.Security.Claims; +using Api.SeaHavenIndustries.Controllers; +using Data.SeaHavenIndustries; +using Data.SeaHavenIndustries.Enums; +using FluentAssertions; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Logging; +using Moq; +using SeaHaven.DataServices.Implementation; +using SeaHaven.Services.Configuration; +using SeaHaven.Services.DTOs; +using SeaHaven.Services.Helpers; +using SeaHaven.Services.Implementation; +using SeaHaven.Services.Interfaces; +using Xunit; + +namespace Api.SeaHavenIndustries.Tests; + +/// +/// Revoke rules exercised end to end through both revoke endpoints (the Uplift +/// Approvals route and the work-order route) against the real services, so a +/// refusal is observed as the HTTP result and the unchanged stored state. +/// +public sealed class UpliftRevokeEndpointRulesTests +{ + private const int WorkOrderId = 1; + private const int DispatchId = 10; + private const int AutoApprovedId = 100; + private const int AdminApprovedId = 101; + + public enum RevokeRoute + { + UpliftApprovals, + WorkOrder, + } + + private static ApplicationDbContext CreateContext() + { + var options = new DbContextOptionsBuilder() + .UseInMemoryDatabase(Guid.NewGuid().ToString()) + .Options; + return new ApplicationDbContext(options); + } + + private static ClaimsPrincipal OrgWideUser(string userId, string role) => + new(new ClaimsIdentity( + new[] + { + new Claim(ClaimTypes.NameIdentifier, userId), + new Claim(ClaimTypes.Role, role), + new Claim(SeaHavenClaimTypes.OrgScope, SeaHavenClaimTypes.OrgScopeAll), + }, + "test")); + + private static async Task SeedAsync(ApplicationDbContext context) + { + context.Accounts.Add(new Accounts { Id = 1, Name = "Acme Corp", IsDeleted = false }); + context.Users.Add(new ApplicationUser { Id = "admin-1", UserName = "admin-1", FirstName = "Ada", LastName = "Admin" }); + context.Users.Add(new ApplicationUser { Id = "dispatcher-1", UserName = "dispatcher-1", FirstName = "Dee", LastName = "Dispatcher" }); + context.Vendors.Add(new Vendor { Id = 1, CompanyName = "Acme HVAC" }); + context.Dispatches.Add(new Dispatch + { + Id = DispatchId, + VendorId = 1, + WorkOrderId = WorkOrderId, + NTEAmount = 2200m, + DispatchNumber = "DIS-10", + Status = "Scheduled", + }); + context.workOrders.Add(new WorkOrder + { + Id = WorkOrderId, + InternalWONumber = "10000000001", + PrimaryDispatchId = DispatchId, + AccountId = 1, + WorkOrderType = WorkOrderType.PM, + LifecycleStatus = LifecycleStatus.Scheduled, + }); + // The admin filed this one themselves and it auto-approved within the allowance. + context.DispatchUpliftRequests.Add(new DispatchUpliftRequest + { + Id = AutoApprovedId, + DispatchId = DispatchId, + CurrentNTE = 1000m, + RequestedNTE = 400m, + Status = UpliftStatus.NoApprovalRequired, + RequiredTier = 0, + NotificationStatus = "Sent", + createdby = "admin-1", + CreatedDate = DateTime.UtcNow.AddHours(-2), + }); + context.DispatchUpliftRequests.Add(new DispatchUpliftRequest + { + Id = AdminApprovedId, + DispatchId = DispatchId, + CurrentNTE = 1400m, + RequestedNTE = 800m, + Status = UpliftStatus.Approved, + RequiredTier = 1, + NotificationStatus = "Sent", + createdby = "dispatcher-1", + CreatedDate = DateTime.UtcNow.AddHours(-1), + DecidedAt = DateTime.UtcNow.AddMinutes(-30), + DecidedByUserId = "admin-1", + }); + await context.SaveChangesAsync(); + } + + private static WorkOrderUpliftService NewWorkOrderUpliftService(ApplicationDbContext context) => + new( + new UpliftDataService(context), + new DispatchDataService(context), + new WorkOrderDetailDataService(context), + new WorkOrderAccountResolver(new AccountDataService(context), new LocationDataService(context)), + new UserDataService(context), + new TeamPermissionOverrideDataService(context), + new TeamPermissionPolicy(), + TimeProvider.System, + Microsoft.Extensions.Options.Options.Create(new ApprovalsOptions())); + + private static async Task RevokeAsync( + ApplicationDbContext context, + RevokeRoute route, + ClaimsPrincipal user, + int upliftId, + string? reason) + { + var workOrderFlow = NewWorkOrderUpliftService(context); + var httpContext = new DefaultHttpContext { User = user }; + + if (route == RevokeRoute.WorkOrder) + { + var controller = new WorkOrderDetailController( + Mock.Of(), + Mock.Of(), + workOrderFlow, + Mock.Of>()) + { + ControllerContext = new ControllerContext { HttpContext = httpContext }, + }; + return await controller.RevokeUplift( + WorkOrderId, + upliftId, + new RevokeWorkOrderUpliftRequestDto { Reason = reason }, + CancellationToken.None); + } + + var upliftService = new UpliftService( + new UpliftDataService(context), + new DispatchDataService(context), + Mock.Of(), + TimeProvider.System, + Microsoft.Extensions.Options.Options.Create(new ApprovalsOptions()), + workOrderFlow); + var approvals = new UpliftController(upliftService, Mock.Of>()) + { + ControllerContext = new ControllerContext { HttpContext = httpContext }, + }; + return await approvals.Revoke( + upliftId, + new UpliftController.DecisionRequest { Note = reason }, + CancellationToken.None); + } + + private static async Task AssertUnchangedAsync(ApplicationDbContext context, int upliftId, string status) + { + var stored = await context.DispatchUpliftRequests.AsNoTracking().SingleAsync(u => u.Id == upliftId); + stored.Status.Should().Be(status); + stored.DecisionNote.Should().BeNull(); + (await context.Dispatches.AsNoTracking().SingleAsync(d => d.Id == DispatchId)).NTEAmount.Should().Be(2200m); + (await context.WorkOrderAuditLogs.AsNoTracking().AnyAsync(a => a.Action == "uplift_revoke")).Should().BeFalse(); + } + + [Fact] + public async Task WorkOrderRoute_AdminRevokingAutoApprovedUpliftTheyRequested_IsForbiddenAndChangesNothing() + { + await using var context = CreateContext(); + await SeedAsync(context); + + var result = await RevokeAsync( + context, + RevokeRoute.WorkOrder, + OrgWideUser("admin-1", "Admin"), + AutoApprovedId, + "Wrong quote attached"); + + var refused = result.Should().BeOfType().Subject; + refused.StatusCode.Should().Be(StatusCodes.Status403Forbidden); + refused.Value.Should().BeOfType().Which.Message + .Should().StartWith("You are not authorized to perform this action"); + await AssertUnchangedAsync(context, AutoApprovedId, UpliftStatus.NoApprovalRequired); + } + + [Fact] + public async Task ApprovalsRoute_AdminRevokingAutoApprovedUplift_IsRefusedAndChangesNothing() + { + await using var context = CreateContext(); + await SeedAsync(context); + + var result = await RevokeAsync( + context, + RevokeRoute.UpliftApprovals, + OrgWideUser("admin-1", "Admin"), + AutoApprovedId, + "Wrong quote attached"); + + var refused = result.Should().BeOfType().Subject; + refused.Value.Should().BeOfType().Which.Message + .Should().StartWith("This uplift request cannot be revoked"); + await AssertUnchangedAsync(context, AutoApprovedId, UpliftStatus.NoApprovalRequired); + } + + [Theory] + [InlineData(RevokeRoute.UpliftApprovals)] + [InlineData(RevokeRoute.WorkOrder)] + public async Task AdminRevokingAdminApprovedUpliftWithReason_Succeeds(RevokeRoute route) + { + await using var context = CreateContext(); + await SeedAsync(context); + + var result = await RevokeAsync( + context, + route, + OrgWideUser("admin-1", "Admin"), + AdminApprovedId, + " Approved against the wrong quote "); + + result.Should().BeOfType(); + var stored = await context.DispatchUpliftRequests.AsNoTracking().SingleAsync(u => u.Id == AdminApprovedId); + stored.Status.Should().Be(UpliftStatus.Revoked); + stored.DecisionNote.Should().Be("Approved against the wrong quote"); + (await context.Dispatches.AsNoTracking().SingleAsync(d => d.Id == DispatchId)).NTEAmount.Should().Be(1400m); + var audit = await context.WorkOrderAuditLogs.AsNoTracking().SingleAsync(a => a.Action == "uplift_revoke"); + audit.OldValue.Should().Be(UpliftStatus.Approved); + audit.NewValue.Should().Be(UpliftStatus.Revoked); + } + + [Theory] + [InlineData(RevokeRoute.UpliftApprovals)] + [InlineData(RevokeRoute.WorkOrder)] + public async Task AdminRevokingAdminApprovedUpliftWithoutReason_IsRefused(RevokeRoute route) + { + await using var context = CreateContext(); + await SeedAsync(context); + + var result = await RevokeAsync(context, route, OrgWideUser("admin-1", "Admin"), AdminApprovedId, " "); + + result.Should().BeOfType(); + await AssertUnchangedAsync(context, AdminApprovedId, UpliftStatus.Approved); + } + + [Fact] + public async Task WorkOrderRoute_DispatcherRevokingOwnAutoApprovedUpliftWithoutReason_Succeeds() + { + await using var context = CreateContext(); + await SeedAsync(context); + var own = await context.DispatchUpliftRequests.SingleAsync(u => u.Id == AutoApprovedId); + own.createdby = "dispatcher-1"; + await context.SaveChangesAsync(); + + var result = await RevokeAsync( + context, + RevokeRoute.WorkOrder, + OrgWideUser("dispatcher-1", "Dispatcher"), + AutoApprovedId, + null); + + result.Should().BeOfType(); + var stored = await context.DispatchUpliftRequests.AsNoTracking().SingleAsync(u => u.Id == AutoApprovedId); + stored.Status.Should().Be(UpliftStatus.Revoked); + stored.DecisionNote.Should().BeNull(); + (await context.Dispatches.AsNoTracking().SingleAsync(d => d.Id == DispatchId)).NTEAmount.Should().Be(1000m); + } +} diff --git a/SeaHaven.Services/Implementation/WorkOrderUpliftService.cs b/SeaHaven.Services/Implementation/WorkOrderUpliftService.cs index 0980e0d..87a1834 100644 --- a/SeaHaven.Services/Implementation/WorkOrderUpliftService.cs +++ b/SeaHaven.Services/Implementation/WorkOrderUpliftService.cs @@ -273,6 +273,11 @@ namespace SeaHaven.Services.Implementation if (canonical == UpliftStatus.NoApprovalRequired) { + // An admin revoke overturns a human decision, and an auto-approval has none, + // so admins are refused here even when they filed the request themselves. + if (user.IsInRole("Admin")) + throw new UpliftForbiddenException("Admins can revoke only admin-approved uplifts"); + if (string.IsNullOrWhiteSpace(userId) || !string.Equals(req.createdby, userId, StringComparison.Ordinal)) { diff --git a/SeaHavenIndustries.Tests/WorkOrderUpliftDispatchOwnershipTests.cs b/SeaHavenIndustries.Tests/WorkOrderUpliftDispatchOwnershipTests.cs index adba1a7..3708240 100644 --- a/SeaHavenIndustries.Tests/WorkOrderUpliftDispatchOwnershipTests.cs +++ b/SeaHavenIndustries.Tests/WorkOrderUpliftDispatchOwnershipTests.cs @@ -212,7 +212,7 @@ public sealed class WorkOrderUpliftDispatchOwnershipTests row.Id, created!.Id, new RevokeWorkOrderUpliftRequestDto(), - WorkOrderAccountTestHelpers.AccountUser(), + WorkOrderAccountTestHelpers.AccountUser("actor-1", 1, "Dispatcher"), CancellationToken.None); Assert.Equal("revoked", revoked!.Status); diff --git a/SeaHavenIndustries.Tests/WorkOrderUpliftServiceTests.cs b/SeaHavenIndustries.Tests/WorkOrderUpliftServiceTests.cs index 390beaf..9685f70 100644 --- a/SeaHavenIndustries.Tests/WorkOrderUpliftServiceTests.cs +++ b/SeaHavenIndustries.Tests/WorkOrderUpliftServiceTests.cs @@ -48,6 +48,11 @@ public sealed class WorkOrderUpliftServiceTests private static ClaimsPrincipal Dispatcher(string userId = "dispatcher-1") => WorkOrderAccountTestHelpers.OrgWideAdmin(userId); + // Same user as Dispatcher(), without the Admin role: the only kind of caller that + // may revoke an auto-approved uplift, and only their own. + private static ClaimsPrincipal DispatcherRoleOnly(string userId = "dispatcher-1") + => WorkOrderAccountTestHelpers.AccountUser(userId, 1, "Dispatcher"); + private static async Task<(WorkOrder WorkOrder, Dispatch Dispatch)> SeedWorkOrderAsync( ApplicationDbContext context, WorkOrderType type = WorkOrderType.PM, @@ -585,7 +590,7 @@ public sealed class WorkOrderUpliftServiceTests workOrder.Id, created.Id, new RevokeWorkOrderUpliftRequestDto(), - Dispatcher(), + DispatcherRoleOnly(), CancellationToken.None); // SH-196: revoking frees the allowance, so it must release the NTE too. Otherwise @@ -724,7 +729,7 @@ public sealed class WorkOrderUpliftServiceTests workOrder.Id, created!.Id, new RevokeWorkOrderUpliftRequestDto(), - Dispatcher(), + DispatcherRoleOnly(), CancellationToken.None); Assert.Equal("revoked", revoked!.Status); @@ -733,6 +738,35 @@ public sealed class WorkOrderUpliftServiceTests .SumAutoApprovedAmountForWorkOrderAsync(workOrder.Id, CancellationToken.None)); } + [Fact] + public async Task RevokeAsync_AdminOwnerRevokingAutoApproved_IsForbiddenAndKeepsAllowanceConsumed() + { + 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, Notes = "Within limit" }, + Dispatcher(), + CancellationToken.None); + Assert.Equal("auto_approved", created!.Status); + + await Assert.ThrowsAsync(() => service.RevokeAsync( + workOrder.Id, + created.Id, + new RevokeWorkOrderUpliftRequestDto { Reason = "Wrong quote" }, + Dispatcher(), + CancellationToken.None)); + + var stored = context.DispatchUpliftRequests.Single(u => u.Id == created.Id); + Assert.Equal(UpliftStatus.NoApprovalRequired, stored.Status); + Assert.Null(stored.DecisionNote); + Assert.Equal(1400m, context.Dispatches.Single(d => d.Id == 10).NTEAmount); + Assert.Equal(400m, await new UpliftDataService(context) + .SumAutoApprovedAmountForWorkOrderAsync(workOrder.Id, CancellationToken.None)); + } + [Theory] [InlineData(LifecycleStatus.Completed)] [InlineData(LifecycleStatus.Canceled)] @@ -760,7 +794,7 @@ public sealed class WorkOrderUpliftServiceTests workOrder.Id, 100, new RevokeWorkOrderUpliftRequestDto(), - Dispatcher(), + DispatcherRoleOnly(), CancellationToken.None)); Assert.Contains("work order", ex.Message, StringComparison.OrdinalIgnoreCase); }