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",