From cfb05a906f2a8014c87288d899b1286034a3f7dd Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Thu, 24 Sep 2026 22:34:32 -0300 Subject: [PATCH 1/3] fix(uplifts): Admin passes uplift approval checks regardless of tier config (SH-327) UserCanApprove only accepted roles listed in Approvals:Tier1Roles/Tier2Roles, so an environment whose config omits Admin denied every approval surface to admins: can-approve, per-row CanDecide, approve/reject/request-changes and evidence download. Admin now short-circuits the check; tier-role config still governs every other role. --- .../UpliftAdminApprovalTests.cs | 165 ++++++++++++++++++ .../Implementation/UpliftService.cs | 4 + 2 files changed, 169 insertions(+) create mode 100644 Api.SeaHavenIndustries.Tests/UpliftAdminApprovalTests.cs diff --git a/Api.SeaHavenIndustries.Tests/UpliftAdminApprovalTests.cs b/Api.SeaHavenIndustries.Tests/UpliftAdminApprovalTests.cs new file mode 100644 index 0000000..cbfc971 --- /dev/null +++ b/Api.SeaHavenIndustries.Tests/UpliftAdminApprovalTests.cs @@ -0,0 +1,165 @@ +using Data.SeaHavenIndustries; +using FluentAssertions; +using Microsoft.EntityFrameworkCore; +using SeaHaven.DataServices.Implementation; +using SeaHaven.Services.Configuration; +using SeaHaven.Services.DTOs; +using SeaHaven.Services.Implementation; +using SeaHaven.Services.Interfaces; +using System.Security.Claims; +using Xunit; + +namespace Api.SeaHavenIndustries.Tests; + +// SH-327: Admin passes every permission check regardless of stored configuration. +// Uplift approval authority is otherwise driven by Approvals:Tier1Roles/Tier2Roles, +// which may be empty in a deployed environment; Admin must still be able to decide. +public sealed class UpliftAdminApprovalTests +{ + private static ApplicationDbContext NewContext() + { + var options = new DbContextOptionsBuilder() + .UseInMemoryDatabase(Guid.NewGuid().ToString()) + .Options; + return new ApplicationDbContext(options); + } + + private static ClaimsPrincipal UserWithRoles(params string[] roles) + { + var claims = new List { new(ClaimTypes.NameIdentifier, "user-42") }; + claims.AddRange(roles.Select(r => new Claim(ClaimTypes.Role, r))); + return new ClaimsPrincipal(new ClaimsIdentity(claims, "Test")); + } + + private static UpliftService NewService(ApplicationDbContext context, ApprovalsOptions? options = null) => + new(new UpliftDataService(context), + new DispatchDataService(context), + new NoopDocumentStorage(), + TimeProvider.System, + Microsoft.Extensions.Options.Options.Create(options ?? new ApprovalsOptions())); + + private static async Task SeedPendingAsync(ApplicationDbContext context, int requiredTier) + { + var vendor = new Vendor { CompanyName = "Gateway", IsActive = true }; + var workOrder = new WorkOrder { WorkerOrderTitle = "Repair" }; + context.AddRange(vendor, workOrder); + await context.SaveChangesAsync(); + var dispatch = new Dispatch + { + VendorId = vendor.Id, + WorkOrderId = workOrder.Id, + Status = "Completed", + NTEAmount = 1000m, + DispatchNumber = "DIS-1" + }; + context.Dispatches.Add(dispatch); + await context.SaveChangesAsync(); + context.DispatchUpliftRequests.Add(new DispatchUpliftRequest + { + DispatchId = dispatch.Id, + CurrentNTE = 1000m, + RequestedNTE = 1800m, + VendorReason = "reason", + Status = UpliftStatus.Pending, + RequiredTier = requiredTier, + CreatedDate = DateTime.UtcNow + }); + await context.SaveChangesAsync(); + return dispatch; + } + + [Theory] + [InlineData(1)] + [InlineData(2)] + public void CanApprove_Admin_WithEmptyTierConfig_IsTrue(int tier) + { + using var context = NewContext(); + var service = NewService(context); + + service.CanApprove(UserWithRoles("Admin"), tier).Should().BeTrue(); + } + + [Theory] + [InlineData(1)] + [InlineData(2)] + public async Task List_Admin_WithEmptyTierConfig_CanDecidePendingRows(int tier) + { + using var context = NewContext(); + var dispatch = await SeedPendingAsync(context, tier); + var service = NewService(context); + + var queue = await service.ListAsync(UserWithRoles("Admin"), UpliftStatus.Pending, null, 1, 25, CancellationToken.None); + var forDispatch = await service.ListForDispatchAsync(UserWithRoles("Admin"), dispatch.Id, CancellationToken.None); + + queue.Items.Should().ContainSingle().Which.CanDecide.Should().BeTrue(); + forDispatch.Should().ContainSingle().Which.CanDecide.Should().BeTrue(); + } + + [Theory] + [InlineData(1)] + [InlineData(2)] + public async Task Approve_Admin_WithEmptyTierConfig_Succeeds(int tier) + { + using var context = NewContext(); + await SeedPendingAsync(context, tier); + var service = NewService(context); + + var result = await service.ApproveAsync(UserWithRoles("Admin"), 1, "ok", CancellationToken.None); + + result.Status.Should().Be(UpliftStatus.Approved); + context.Dispatches.Single().NTEAmount.Should().Be(1800m); + } + + [Fact] + public async Task Reject_Admin_WithEmptyTierConfig_Succeeds() + { + using var context = NewContext(); + await SeedPendingAsync(context, 2); + var service = NewService(context); + + var result = await service.RejectAsync(UserWithRoles("Admin"), 1, "no", CancellationToken.None); + + result.Status.Should().Be(UpliftStatus.Rejected); + context.Dispatches.Single().NTEAmount.Should().Be(1000m); + } + + [Theory] + [InlineData("Dispatcher")] + [InlineData("Scheduler")] + public async Task NonAdminRole_WithEmptyTierConfig_IsStillDenied(string role) + { + using var context = NewContext(); + await SeedPendingAsync(context, 1); + var service = NewService(context); + var user = UserWithRoles(role); + + service.CanApprove(user, 1).Should().BeFalse(); + service.CanApprove(user, 2).Should().BeFalse(); + var queue = await service.ListAsync(user, UpliftStatus.Pending, null, 1, 25, CancellationToken.None); + queue.Items.Should().ContainSingle().Which.CanDecide.Should().BeFalse(); + + var act = () => service.ApproveAsync(user, 1, "ok", CancellationToken.None); + + await act.Should().ThrowAsync(); + context.Dispatches.Single().NTEAmount.Should().Be(1000m); + context.DispatchUpliftRequests.Single().Status.Should().Be(UpliftStatus.Pending); + } + + [Fact] + public void Tier1OnlyRole_ApprovesTier1_ButNotTier2() + { + using var context = NewContext(); + var service = NewService(context, new ApprovalsOptions { Tier1Roles = new[] { "Approver" } }); + var user = UserWithRoles("Approver"); + + service.CanApprove(user, 1).Should().BeTrue(); + service.CanApprove(user, 2).Should().BeFalse(); + } + + private sealed class NoopDocumentStorage : IVendorDocumentStoragePort + { + public Task SaveAsync(int vendorId, int dispatchId, string storedFileName, Stream content, CancellationToken cancellationToken) => Task.CompletedTask; + public Stream OpenRead(int vendorId, int dispatchId, string storedFileName) => new MemoryStream(); + public void Delete(int vendorId, int dispatchId, string storedFileName) { } + } +} diff --git a/SeaHaven.Services/Implementation/UpliftService.cs b/SeaHaven.Services/Implementation/UpliftService.cs index 5459c36..801139e 100644 --- a/SeaHaven.Services/Implementation/UpliftService.cs +++ b/SeaHaven.Services/Implementation/UpliftService.cs @@ -335,8 +335,12 @@ namespace SeaHaven.Services.Implementation return UpliftEvidenceDownloadResultDTO.Ok(content, evidence.ContentType ?? "application/octet-stream", evidence.OriginalFileName ?? "evidence"); } + // SH-327: Admin passes every permission check regardless of stored configuration, + // so the tier-role lists only govern non-Admin approvers. private bool UserCanApprove(ClaimsPrincipal user, int requiredTier) { + if (user.IsInRole("Admin")) return true; + var roles = RolesForTier(requiredTier); foreach (var r in roles) { From 26b4aaf1b2e06767615c34f51019f291920f4e2e Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Thu, 24 Sep 2026 22:51:57 -0300 Subject: [PATCH 2/3] test(uplifts): cover Admin request-changes and evidence download with empty tier config (SH-327) --- .../UpliftAdminApprovalTests.cs | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/Api.SeaHavenIndustries.Tests/UpliftAdminApprovalTests.cs b/Api.SeaHavenIndustries.Tests/UpliftAdminApprovalTests.cs index cbfc971..e3fa547 100644 --- a/Api.SeaHavenIndustries.Tests/UpliftAdminApprovalTests.cs +++ b/Api.SeaHavenIndustries.Tests/UpliftAdminApprovalTests.cs @@ -123,6 +123,47 @@ public sealed class UpliftAdminApprovalTests context.Dispatches.Single().NTEAmount.Should().Be(1000m); } + [Fact] + public async Task RequestChanges_Admin_WithEmptyTierConfig_Succeeds() + { + using var context = NewContext(); + await SeedPendingAsync(context, 2); + var service = NewService(context); + + var result = await service.RequestChangesAsync(UserWithRoles("Admin"), 1, "more detail", CancellationToken.None); + + result.Status.Should().Be(UpliftStatus.ChangesRequested); + } + + [Fact] + public async Task EvidenceDownload_Admin_WithEmptyTierConfig_ReturnsFile() + { + using var context = NewContext(); + var dispatch = await SeedPendingAsync(context, 2); + context.VendorCompletionDocuments.Add(new VendorCompletionDocument + { + VendorId = dispatch.VendorId, + DispatchId = dispatch.Id, + WorkOrderId = dispatch.WorkOrderId!.Value, + OriginalFileName = "invoice.pdf", + StoredFileName = "evidence.bin", + ContentType = "application/pdf", + SizeBytes = 4, + ScanStatus = "Passed", + ReviewStatus = "Approved", + Purpose = "UpliftEvidence", + Version = 1 + }); + context.DispatchUpliftRequests.Single().EvidenceDocumentId = 1; + await context.SaveChangesAsync(); + var service = NewService(context); + + var result = await service.GetEvidenceForDownloadAsync(UserWithRoles("Admin"), 1, CancellationToken.None); + + result.Outcome.Should().Be(VendorDocumentDownloadOutcome.Ok); + result.FileName.Should().Be("invoice.pdf"); + } + [Theory] [InlineData("Dispatcher")] [InlineData("Scheduler")] From 1e2f39a5fa288fc68d2b4ed3ff9ef237570c2696 Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Thu, 24 Sep 2026 22:57:09 -0300 Subject: [PATCH 3/3] chore(uplifts): drop ticket key from source comments --- Api.SeaHavenIndustries.Tests/UpliftAdminApprovalTests.cs | 2 +- SeaHaven.Services/Implementation/UpliftService.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Api.SeaHavenIndustries.Tests/UpliftAdminApprovalTests.cs b/Api.SeaHavenIndustries.Tests/UpliftAdminApprovalTests.cs index e3fa547..7a5d6a7 100644 --- a/Api.SeaHavenIndustries.Tests/UpliftAdminApprovalTests.cs +++ b/Api.SeaHavenIndustries.Tests/UpliftAdminApprovalTests.cs @@ -11,7 +11,7 @@ using Xunit; namespace Api.SeaHavenIndustries.Tests; -// SH-327: Admin passes every permission check regardless of stored configuration. +// Admin passes every permission check regardless of stored configuration. // Uplift approval authority is otherwise driven by Approvals:Tier1Roles/Tier2Roles, // which may be empty in a deployed environment; Admin must still be able to decide. public sealed class UpliftAdminApprovalTests diff --git a/SeaHaven.Services/Implementation/UpliftService.cs b/SeaHaven.Services/Implementation/UpliftService.cs index 801139e..3cfed76 100644 --- a/SeaHaven.Services/Implementation/UpliftService.cs +++ b/SeaHaven.Services/Implementation/UpliftService.cs @@ -335,7 +335,7 @@ namespace SeaHaven.Services.Implementation return UpliftEvidenceDownloadResultDTO.Ok(content, evidence.ContentType ?? "application/octet-stream", evidence.OriginalFileName ?? "evidence"); } - // SH-327: Admin passes every permission check regardless of stored configuration, + // Admin passes every permission check regardless of stored configuration, // so the tier-role lists only govern non-Admin approvers. private bool UserCanApprove(ClaimsPrincipal user, int requiredTier) {