mirror of
https://github.com/Sea-Haven-Industries/shoc-backend.git
synced 2026-09-30 16:33:12 +00:00
Merge pull request #168 from Sea-Haven-Industries/fix/sh-327-request-uplifts-permission
fix(uplifts): enforce request permission at service boundary
This commit is contained in:
commit
59b8cdaf7d
8 changed files with 250 additions and 1 deletions
|
|
@ -79,6 +79,53 @@ public sealed class WorkOrderUpliftControllerTests
|
|||
Assert.Equal(12, created.Id);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task CreateUplift_RequestPermissionDenied_ReturnsExact403Message()
|
||||
{
|
||||
var service = new Mock<IWorkOrderUpliftService>();
|
||||
service.Setup(x => x.CreateAsync(
|
||||
7,
|
||||
It.IsAny<CreateWorkOrderUpliftRequestDto>(),
|
||||
It.IsAny<ClaimsPrincipal>(),
|
||||
It.IsAny<CancellationToken>()))
|
||||
.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<ObjectResult>().Subject;
|
||||
forbidden.StatusCode.Should().Be(StatusCodes.Status403Forbidden);
|
||||
var response = forbidden.Value.Should().BeOfType<Response>().Subject;
|
||||
response.Message.Should().Be(UpliftForbiddenException.RequestUpliftsDeniedMessage);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task CreateUplift_UnrelatedForbiddenException_ReturnsSanitized403()
|
||||
{
|
||||
var service = new Mock<IWorkOrderUpliftService>();
|
||||
service.Setup(x => x.CreateAsync(
|
||||
7,
|
||||
It.IsAny<CreateWorkOrderUpliftRequestDto>(),
|
||||
It.IsAny<ClaimsPrincipal>(),
|
||||
It.IsAny<CancellationToken>()))
|
||||
.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<ObjectResult>().Subject;
|
||||
forbidden.StatusCode.Should().Be(StatusCodes.Status403Forbidden);
|
||||
var response = forbidden.Value.Should().BeOfType<Response>().Subject;
|
||||
response.Message.Should().NotContain("SECRET-internal-tier-detail");
|
||||
response.Message.Should().Contain("reference");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task CreateUplift_ConcurrentActiveRequest_ReturnsConflict()
|
||||
{
|
||||
|
|
|
|||
|
|
@ -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<IUserDataService>();
|
||||
userData.Setup(x => x.GetDisplayNamesByIdsAsync(It.IsAny<IEnumerable<string>>()))
|
||||
.ReturnsAsync(new Dictionary<string, string>());
|
||||
// SH-327: the requester must hold RequestUplifts before the create reaches the insert.
|
||||
var permissionData = new Mock<ITeamPermissionOverrideDataService>();
|
||||
permissionData.Setup(x => x.GetUserAsync("dispatcher-1", It.IsAny<CancellationToken>()))
|
||||
.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
|
||||
{
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
{
|
||||
|
|
|
|||
|
|
@ -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) { }
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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> 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<WorkOrderUpliftDto?> CreateLockedAsync(
|
||||
int workOrderId,
|
||||
decimal amount,
|
||||
|
|
|
|||
|
|
@ -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<string>
|
||||
{
|
||||
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(
|
||||
|
|
|
|||
|
|
@ -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<string> { 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
|
||||
{
|
||||
|
|
|
|||
|
|
@ -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<string>
|
||||
{
|
||||
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<UpliftForbiddenException>(() => 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<UpliftForbiddenException>(() => 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<UpliftForbiddenException>(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<UpliftForbiddenException>(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()
|
||||
{
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue