From 66d33ec294f265c85fda947150c52d5896c20ffc Mon Sep 17 00:00:00 2001 From: Peter Drier Date: Wed, 15 Apr 2026 04:30:47 +0200 Subject: [PATCH] Service-layer auth enforcement for budget mutations (#420) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 3c (final slice) of the first-class authorization transition plan. Budget mutations are now protected at the service boundary regardless of call path (controllers, background jobs, future API surfaces), mirroring the pattern established by #210 (role assignment). - BudgetOperationRequirement moves to Humans.Application.Authorization and gains a resource-free Manage static alongside the existing Edit. - New BudgetManageAuthorizationHandler succeeds for Admin / FinanceAdmin / SystemPrincipal on the Manage requirement (used for budget years, groups, categories, ticketing projection parameters, and ticketing sync jobs). - Existing BudgetAuthorizationHandler still handles Edit against a BudgetCategory resource (department-coordinator scoping) and now succeeds for SystemPrincipal. Uses IServiceProvider to lazily resolve IBudgetService, matching TeamAuthorizationHandler so the new BudgetService -> IAuthorizationService dependency does not introduce a DI cycle. - Every IBudgetService mutation method now accepts a ClaimsPrincipal and calls IAuthorizationService.AuthorizeAsync before touching the database. Unauthorized callers raise UnauthorizedAccessException. Line-item create/update/delete authorize against the loaded BudgetCategory (Edit); year/group/category/projection/sync mutations authorize against null (Manage). - BudgetController and FinanceController forward User to the service; the controller-level [Authorize(Policy = FinanceAdminOrAdmin)] and resource-based edit check stay as defense in depth. - TicketingBudgetService and ITicketingBudgetService forward ClaimsPrincipal through to BudgetService. TicketingBudgetSyncJob and DevelopmentBudgetSeeder pass SystemPrincipal.Instance. - RoleChecks is now an internal helper — part of the Phase 1 cleanup called out in docs/plans/2026-04-03-first-class-authorization-transition.md. ViewPolicies was already removed in earlier phases (no references remain outside that plan doc). Tests: - BudgetServiceTests updated for the new ctor, plus new cases covering unauthorized CreateLineItem / UpdateLineItem / DeleteLineItem / CreateYear / UpdateYearStatus / CreateGroup / CreateCategory paths (UnauthorizedAccessException thrown, no DB mutation) and an authorized FinanceAdmin path. - BudgetAuthorizationHandlerTests updated for the IServiceProvider constructor and exercises the new SystemPrincipal override plus the "Manage requirement not handled" guard. - New BudgetManageAuthorizationHandlerTests covers Admin, FinanceAdmin, SystemPrincipal, Board (denied), regular user (denied), unauthenticated user (denied), and the "Edit requirement not handled" guard. - docs/sections/Budget.md gains an Authorization note describing the two-layer defense. Closes #420 Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/sections/Budget.md | 9 + .../BudgetOperationRequirement.cs | 36 +++ .../Interfaces/IBudgetService.cs | 48 ++-- .../Interfaces/ITicketingBudgetService.cs | 10 +- .../Jobs/TicketingBudgetSyncJob.cs | 3 +- .../Services/BudgetService.cs | 128 ++++++++-- .../Services/TicketingBudgetService.cs | 9 +- .../AuthorizationPolicyExtensions.cs | 1 + .../BudgetAuthorizationHandler.cs | 36 ++- .../BudgetManageAuthorizationHandler.cs | 40 +++ .../BudgetOperationRequirement.cs | 20 -- src/Humans.Web/Authorization/RoleChecks.cs | 9 +- .../Controllers/BudgetController.cs | 7 +- .../Controllers/FinanceController.cs | 36 +-- .../Infrastructure/DevelopmentBudgetSeeder.cs | 40 +-- .../BudgetAuthorizationHandlerTests.cs | 35 ++- .../BudgetManageAuthorizationHandlerTests.cs | 102 ++++++++ .../Services/BudgetServiceTests.cs | 236 +++++++++++++++++- 18 files changed, 692 insertions(+), 113 deletions(-) create mode 100644 src/Humans.Application/Authorization/BudgetOperationRequirement.cs create mode 100644 src/Humans.Web/Authorization/Requirements/BudgetManageAuthorizationHandler.cs delete mode 100644 src/Humans.Web/Authorization/Requirements/BudgetOperationRequirement.cs create mode 100644 tests/Humans.Application.Tests/Authorization/BudgetManageAuthorizationHandlerTests.cs diff --git a/docs/sections/Budget.md b/docs/sections/Budget.md index 418740873..7c5112f0c 100644 --- a/docs/sections/Budget.md +++ b/docs/sections/Budget.md @@ -55,3 +55,12 @@ See `.claude/DESIGN_RULES.md` for the full rules. ### Current State **Compliant.** No violations found. Controllers do not inject DbContext. BudgetService only queries its own tables. Cache ownership is correct. + +### Authorization + +Budget mutations are guarded at two layers (defense in depth): + +1. **Controller layer** — `[Authorize(Policy = PolicyNames.FinanceAdminOrAdmin)]` on `FinanceController`, and resource-based `BudgetOperationRequirement.Edit` checks in `BudgetController` before calling line-item mutations. +2. **Service layer** — `BudgetService` mutation methods call `IAuthorizationService.AuthorizeAsync` internally, throwing `UnauthorizedAccessException` for unprivileged callers. Admin/group/category/year/projection mutations use `BudgetOperationRequirement.Manage` (FinanceAdmin / Admin / system-principal only). Line-item create/update/delete use `BudgetOperationRequirement.Edit` with the owning `BudgetCategory` resource — so department coordinators are scoped to their own categories, FinanceAdmin/Admin succeed on any category, and background jobs pass `SystemPrincipal.Instance`. + +Service-layer enforcement means that future call paths (background jobs, alternate UI surfaces, future API endpoints) cannot bypass authorization by skipping controller attributes. diff --git a/src/Humans.Application/Authorization/BudgetOperationRequirement.cs b/src/Humans.Application/Authorization/BudgetOperationRequirement.cs new file mode 100644 index 000000000..ef70c0e69 --- /dev/null +++ b/src/Humans.Application/Authorization/BudgetOperationRequirement.cs @@ -0,0 +1,36 @@ +using Microsoft.AspNetCore.Authorization; + +namespace Humans.Application.Authorization; + +/// +/// Resource-based authorization requirement for budget operations. +/// +/// +/// +/// +/// Edit is resource-scoped — pair it with a BudgetCategory to +/// authorize line-item create/update/delete. FinanceAdmin/Admin can edit any +/// category; department coordinators can edit categories linked to their team. +/// +/// +/// +/// +/// Manage is global — used for budget-wide admin operations +/// (budget year lifecycle, group/category management, projection parameters, +/// background sync jobs). Only FinanceAdmin/Admin and the system principal succeed. +/// +/// +/// +/// +public sealed class BudgetOperationRequirement : IAuthorizationRequirement +{ + public static readonly BudgetOperationRequirement Edit = new(nameof(Edit)); + public static readonly BudgetOperationRequirement Manage = new(nameof(Manage)); + + public string OperationName { get; } + + private BudgetOperationRequirement(string operationName) + { + OperationName = operationName; + } +} diff --git a/src/Humans.Application/Interfaces/IBudgetService.cs b/src/Humans.Application/Interfaces/IBudgetService.cs index c11128f75..2a0c760ef 100644 --- a/src/Humans.Application/Interfaces/IBudgetService.cs +++ b/src/Humans.Application/Interfaces/IBudgetService.cs @@ -1,3 +1,4 @@ +using System.Security.Claims; using Humans.Application.DTOs; using Humans.Domain.Entities; using Humans.Domain.Enums; @@ -7,6 +8,12 @@ namespace Humans.Application.Interfaces; /// /// Service for managing budget years, groups, categories, and line items. +/// +/// Mutation methods enforce authorization at the service boundary via +/// and throw +/// for unprivileged callers. Callers must +/// pass the current ; background jobs pass +/// . /// public interface IBudgetService { @@ -14,20 +21,21 @@ public interface IBudgetService Task> GetAllYearsAsync(bool includeArchived = false); Task GetYearByIdAsync(Guid id); Task GetActiveYearAsync(); - Task CreateYearAsync(string year, string name, Guid actorUserId); - Task UpdateYearStatusAsync(Guid yearId, BudgetYearStatus status, Guid actorUserId); - Task UpdateYearAsync(Guid yearId, string year, string name, Guid actorUserId); - Task DeleteYearAsync(Guid yearId, Guid actorUserId); - Task RestoreYearAsync(Guid yearId, Guid actorUserId); + Task CreateYearAsync(string year, string name, Guid actorUserId, ClaimsPrincipal principal); + Task UpdateYearStatusAsync(Guid yearId, BudgetYearStatus status, Guid actorUserId, ClaimsPrincipal principal); + Task UpdateYearAsync(Guid yearId, string year, string name, Guid actorUserId, ClaimsPrincipal principal); + Task DeleteYearAsync(Guid yearId, Guid actorUserId, ClaimsPrincipal principal); + Task RestoreYearAsync(Guid yearId, Guid actorUserId, ClaimsPrincipal principal); - Task SyncDepartmentsAsync(Guid budgetYearId, Guid actorUserId); - Task EnsureTicketingGroupAsync(Guid budgetYearId, Guid actorUserId); + Task SyncDepartmentsAsync(Guid budgetYearId, Guid actorUserId, ClaimsPrincipal principal); + Task EnsureTicketingGroupAsync(Guid budgetYearId, Guid actorUserId, ClaimsPrincipal principal); // Ticketing Projection Task GetTicketingProjectionAsync(Guid budgetGroupId); Task UpdateTicketingProjectionAsync(Guid budgetGroupId, LocalDate? startDate, LocalDate? eventDate, int initialSalesCount, decimal dailySalesRate, decimal averageTicketPrice, int vatRate, - decimal stripeFeePercent, decimal stripeFeeFixed, decimal ticketTailorFeePercent, Guid actorUserId); + decimal stripeFeePercent, decimal stripeFeeFixed, decimal ticketTailorFeePercent, + Guid actorUserId, ClaimsPrincipal principal); /// /// Sync ticket sales actuals (already aggregated per ISO week by the ticket side) @@ -40,6 +48,7 @@ Task UpdateTicketingProjectionAsync(Guid budgetGroupId, LocalDate? startDate, Lo Task SyncTicketingActualsAsync( Guid budgetYearId, IReadOnlyList weeklyActuals, + ClaimsPrincipal principal, CancellationToken ct = default); /// @@ -47,7 +56,10 @@ Task SyncTicketingActualsAsync( /// after projection parameters change so the projected lines reflect the new inputs. /// Returns the number of projected line items created. /// - Task RefreshTicketingProjectionsAsync(Guid budgetYearId, CancellationToken ct = default); + Task RefreshTicketingProjectionsAsync( + Guid budgetYearId, + ClaimsPrincipal principal, + CancellationToken ct = default); /// /// Compute virtual (non-persisted) weekly ticket projections for future weeks. @@ -63,21 +75,21 @@ Task> GetTicketingProjectionEntriesAsync( int GetActualTicketsSold(BudgetGroup ticketingGroup); // Budget Groups - Task CreateGroupAsync(Guid budgetYearId, string name, bool isRestricted, Guid actorUserId); - Task UpdateGroupAsync(Guid groupId, string name, int sortOrder, bool isRestricted, Guid actorUserId); - Task DeleteGroupAsync(Guid groupId, Guid actorUserId); + Task CreateGroupAsync(Guid budgetYearId, string name, bool isRestricted, Guid actorUserId, ClaimsPrincipal principal); + Task UpdateGroupAsync(Guid groupId, string name, int sortOrder, bool isRestricted, Guid actorUserId, ClaimsPrincipal principal); + Task DeleteGroupAsync(Guid groupId, Guid actorUserId, ClaimsPrincipal principal); // Budget Categories Task GetCategoryByIdAsync(Guid id); - Task CreateCategoryAsync(Guid budgetGroupId, string name, decimal allocatedAmount, ExpenditureType expenditureType, Guid? teamId, Guid actorUserId); - Task UpdateCategoryAsync(Guid categoryId, string name, decimal allocatedAmount, ExpenditureType expenditureType, Guid actorUserId); - Task DeleteCategoryAsync(Guid categoryId, Guid actorUserId); + Task CreateCategoryAsync(Guid budgetGroupId, string name, decimal allocatedAmount, ExpenditureType expenditureType, Guid? teamId, Guid actorUserId, ClaimsPrincipal principal); + Task UpdateCategoryAsync(Guid categoryId, string name, decimal allocatedAmount, ExpenditureType expenditureType, Guid actorUserId, ClaimsPrincipal principal); + Task DeleteCategoryAsync(Guid categoryId, Guid actorUserId, ClaimsPrincipal principal); // Budget Line Items Task GetLineItemByIdAsync(Guid id); - Task CreateLineItemAsync(Guid budgetCategoryId, string description, decimal amount, Guid? responsibleTeamId, string? notes, LocalDate? expectedDate, int vatRate, Guid actorUserId); - Task UpdateLineItemAsync(Guid lineItemId, string description, decimal amount, Guid? responsibleTeamId, string? notes, LocalDate? expectedDate, int vatRate, Guid actorUserId); - Task DeleteLineItemAsync(Guid lineItemId, Guid actorUserId); + Task CreateLineItemAsync(Guid budgetCategoryId, string description, decimal amount, Guid? responsibleTeamId, string? notes, LocalDate? expectedDate, int vatRate, Guid actorUserId, ClaimsPrincipal principal); + Task UpdateLineItemAsync(Guid lineItemId, string description, decimal amount, Guid? responsibleTeamId, string? notes, LocalDate? expectedDate, int vatRate, Guid actorUserId, ClaimsPrincipal principal); + Task DeleteLineItemAsync(Guid lineItemId, Guid actorUserId, ClaimsPrincipal principal); // Coordinator Task> GetEffectiveCoordinatorTeamIdsAsync(Guid userId); diff --git a/src/Humans.Application/Interfaces/ITicketingBudgetService.cs b/src/Humans.Application/Interfaces/ITicketingBudgetService.cs index 14dce950d..21f0ba07f 100644 --- a/src/Humans.Application/Interfaces/ITicketingBudgetService.cs +++ b/src/Humans.Application/Interfaces/ITicketingBudgetService.cs @@ -1,3 +1,4 @@ +using System.Security.Claims; using Humans.Application.DTOs; using Humans.Domain.Entities; @@ -11,13 +12,18 @@ public interface ITicketingBudgetService /// /// Sync completed weeks of ticket sales into budget line items from TicketTailor/Stripe data, /// then refresh projections for future weeks. + /// The is forwarded to for + /// service-boundary authorization. Background jobs pass + /// . /// - Task SyncActualsAsync(Guid budgetYearId, CancellationToken ct = default); + Task SyncActualsAsync(Guid budgetYearId, ClaimsPrincipal principal, CancellationToken ct = default); /// /// Refresh projected line items only (no actuals sync). Called after saving projection parameters. + /// The is forwarded to for + /// service-boundary authorization. /// - Task RefreshProjectionsAsync(Guid budgetYearId, CancellationToken ct = default); + Task RefreshProjectionsAsync(Guid budgetYearId, ClaimsPrincipal principal, CancellationToken ct = default); /// /// Compute projected line items for future weeks based on ticketing projection parameters diff --git a/src/Humans.Infrastructure/Jobs/TicketingBudgetSyncJob.cs b/src/Humans.Infrastructure/Jobs/TicketingBudgetSyncJob.cs index e4a41e32c..d5c5148ec 100644 --- a/src/Humans.Infrastructure/Jobs/TicketingBudgetSyncJob.cs +++ b/src/Humans.Infrastructure/Jobs/TicketingBudgetSyncJob.cs @@ -1,4 +1,5 @@ using Hangfire; +using Humans.Application.Authorization; using Humans.Application.Interfaces; using Microsoft.Extensions.Logging; @@ -38,7 +39,7 @@ public async Task ExecuteAsync(CancellationToken cancellationToken = default) try { - var count = await _ticketingBudgetService.SyncActualsAsync(activeYear.Id); + var count = await _ticketingBudgetService.SyncActualsAsync(activeYear.Id, SystemPrincipal.Instance, cancellationToken); _logger.LogInformation("Ticketing budget sync completed: {Count} line items synced", count); } catch (Exception ex) diff --git a/src/Humans.Infrastructure/Services/BudgetService.cs b/src/Humans.Infrastructure/Services/BudgetService.cs index ea5cb33bd..9f4a65eed 100644 --- a/src/Humans.Infrastructure/Services/BudgetService.cs +++ b/src/Humans.Infrastructure/Services/BudgetService.cs @@ -1,7 +1,10 @@ using System.Globalization; +using System.Security.Claims; +using Microsoft.AspNetCore.Authorization; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; using NodaTime; +using Humans.Application.Authorization; using Humans.Application.DTOs; using Humans.Application.Interfaces; using Humans.Domain.Entities; @@ -12,23 +15,66 @@ namespace Humans.Infrastructure.Services; /// /// Service for managing budget years, groups, categories, and line items with integrated audit logging. +/// +/// Mutation methods enforce authorization at the service boundary via +/// and throw +/// for unprivileged callers — protecting budget mutations regardless of call path +/// (controllers, background jobs, future API surfaces). /// public class BudgetService : IBudgetService { private readonly HumansDbContext _dbContext; + private readonly IAuthorizationService _authorizationService; private readonly IClock _clock; private readonly ILogger _logger; public BudgetService( HumansDbContext dbContext, + IAuthorizationService authorizationService, IClock clock, ILogger logger) { _dbContext = dbContext; + _authorizationService = authorizationService; _clock = clock; _logger = logger; } + private async Task AuthorizeManageAsync(ClaimsPrincipal principal, string operation) + { + ArgumentNullException.ThrowIfNull(principal); + + var result = await _authorizationService.AuthorizeAsync( + principal, null, BudgetOperationRequirement.Manage); + + if (!result.Succeeded) + { + _logger.LogWarning( + "Authorization denied for budget Manage operation '{Operation}': principal {Principal}", + operation, principal.Identity?.Name); + throw new UnauthorizedAccessException( + $"Caller is not authorized to perform budget operation '{operation}'."); + } + } + + private async Task AuthorizeEditAsync(ClaimsPrincipal principal, BudgetCategory category, string operation) + { + ArgumentNullException.ThrowIfNull(principal); + ArgumentNullException.ThrowIfNull(category); + + var result = await _authorizationService.AuthorizeAsync( + principal, category, BudgetOperationRequirement.Edit); + + if (!result.Succeeded) + { + _logger.LogWarning( + "Authorization denied for budget Edit operation '{Operation}' on category {CategoryId}: principal {Principal}", + operation, category.Id, principal.Identity?.Name); + throw new UnauthorizedAccessException( + $"Caller is not authorized to edit budget category {category.Id}."); + } + } + // ───────────────────────── Budget Years ───────────────────────── public async Task> GetAllYearsAsync(bool includeArchived = false) @@ -72,8 +118,10 @@ public async Task> GetAllYearsAsync(bool includeArchiv return await GetYearByIdAsync(activeYear.Id); } - public async Task CreateYearAsync(string year, string name, Guid actorUserId) + public async Task CreateYearAsync(string year, string name, Guid actorUserId, ClaimsPrincipal principal) { + await AuthorizeManageAsync(principal, nameof(CreateYearAsync)); + var now = _clock.GetCurrentInstant(); var budgetYear = new BudgetYear @@ -187,8 +235,10 @@ public async Task CreateYearAsync(string year, string name, Guid act return budgetYear; } - public async Task UpdateYearStatusAsync(Guid yearId, BudgetYearStatus status, Guid actorUserId) + public async Task UpdateYearStatusAsync(Guid yearId, BudgetYearStatus status, Guid actorUserId, ClaimsPrincipal principal) { + await AuthorizeManageAsync(principal, nameof(UpdateYearStatusAsync)); + var year = await _dbContext.BudgetYears.FindAsync(yearId) ?? throw new InvalidOperationException($"Budget year {yearId} not found"); @@ -226,8 +276,10 @@ public async Task UpdateYearStatusAsync(Guid yearId, BudgetYearStatus status, Gu yearId, oldStatus, status); } - public async Task UpdateYearAsync(Guid yearId, string year, string name, Guid actorUserId) + public async Task UpdateYearAsync(Guid yearId, string year, string name, Guid actorUserId, ClaimsPrincipal principal) { + await AuthorizeManageAsync(principal, nameof(UpdateYearAsync)); + var budgetYear = await _dbContext.BudgetYears.FindAsync(yearId) ?? throw new InvalidOperationException($"Budget year {yearId} not found"); @@ -254,8 +306,10 @@ public async Task UpdateYearAsync(Guid yearId, string year, string name, Guid ac await _dbContext.SaveChangesAsync(); } - public async Task DeleteYearAsync(Guid yearId, Guid actorUserId) + public async Task DeleteYearAsync(Guid yearId, Guid actorUserId, ClaimsPrincipal principal) { + await AuthorizeManageAsync(principal, nameof(DeleteYearAsync)); + var year = await _dbContext.BudgetYears.FindAsync(yearId) ?? throw new InvalidOperationException($"Budget year {yearId} not found"); @@ -279,8 +333,10 @@ public async Task DeleteYearAsync(Guid yearId, Guid actorUserId) _logger.LogInformation("Archived budget year {YearId} ({Year})", yearId, year.Year); } - public async Task RestoreYearAsync(Guid yearId, Guid actorUserId) + public async Task RestoreYearAsync(Guid yearId, Guid actorUserId, ClaimsPrincipal principal) { + await AuthorizeManageAsync(principal, nameof(RestoreYearAsync)); + var year = await _dbContext.BudgetYears.FindAsync(yearId) ?? throw new InvalidOperationException($"Budget year {yearId} not found"); @@ -303,8 +359,10 @@ public async Task RestoreYearAsync(Guid yearId, Guid actorUserId) _logger.LogInformation("Restored budget year {YearId} ({Year})", yearId, year.Year); } - public async Task SyncDepartmentsAsync(Guid budgetYearId, Guid actorUserId) + public async Task SyncDepartmentsAsync(Guid budgetYearId, Guid actorUserId, ClaimsPrincipal principal) { + await AuthorizeManageAsync(principal, nameof(SyncDepartmentsAsync)); + await EnsureYearNotClosedAsync(budgetYearId); var deptGroup = await _dbContext.BudgetGroups @@ -359,8 +417,10 @@ public async Task SyncDepartmentsAsync(Guid budgetYearId, Guid actorUserId) return budgetTeams.Count; } - public async Task EnsureTicketingGroupAsync(Guid budgetYearId, Guid actorUserId) + public async Task EnsureTicketingGroupAsync(Guid budgetYearId, Guid actorUserId, ClaimsPrincipal principal) { + await AuthorizeManageAsync(principal, nameof(EnsureTicketingGroupAsync)); + await EnsureYearNotClosedAsync(budgetYearId); var exists = await _dbContext.BudgetGroups @@ -433,8 +493,10 @@ public async Task EnsureTicketingGroupAsync(Guid budgetYearId, Guid actorU // ───────────────────────── Budget Groups ───────────────────────── - public async Task CreateGroupAsync(Guid budgetYearId, string name, bool isRestricted, Guid actorUserId) + public async Task CreateGroupAsync(Guid budgetYearId, string name, bool isRestricted, Guid actorUserId, ClaimsPrincipal principal) { + await AuthorizeManageAsync(principal, nameof(CreateGroupAsync)); + await EnsureYearNotClosedAsync(budgetYearId); var year = await _dbContext.BudgetYears.FindAsync(budgetYearId) @@ -471,8 +533,10 @@ public async Task CreateGroupAsync(Guid budgetYearId, string name, return group; } - public async Task UpdateGroupAsync(Guid groupId, string name, int sortOrder, bool isRestricted, Guid actorUserId) + public async Task UpdateGroupAsync(Guid groupId, string name, int sortOrder, bool isRestricted, Guid actorUserId, ClaimsPrincipal principal) { + await AuthorizeManageAsync(principal, nameof(UpdateGroupAsync)); + var group = await _dbContext.BudgetGroups.FindAsync(groupId) ?? throw new InvalidOperationException($"Budget group {groupId} not found"); @@ -508,8 +572,10 @@ public async Task UpdateGroupAsync(Guid groupId, string name, int sortOrder, boo await _dbContext.SaveChangesAsync(); } - public async Task DeleteGroupAsync(Guid groupId, Guid actorUserId) + public async Task DeleteGroupAsync(Guid groupId, Guid actorUserId, ClaimsPrincipal principal) { + await AuthorizeManageAsync(principal, nameof(DeleteGroupAsync)); + var group = await _dbContext.BudgetGroups.FindAsync(groupId) ?? throw new InvalidOperationException($"Budget group {groupId} not found"); @@ -547,8 +613,10 @@ public async Task DeleteGroupAsync(Guid groupId, Guid actorUserId) public async Task CreateCategoryAsync( Guid budgetGroupId, string name, decimal allocatedAmount, - ExpenditureType expenditureType, Guid? teamId, Guid actorUserId) + ExpenditureType expenditureType, Guid? teamId, Guid actorUserId, ClaimsPrincipal principal) { + await AuthorizeManageAsync(principal, nameof(CreateCategoryAsync)); + var group = await _dbContext.BudgetGroups.FindAsync(budgetGroupId) ?? throw new InvalidOperationException($"Budget group {budgetGroupId} not found"); @@ -587,8 +655,10 @@ public async Task CreateCategoryAsync( public async Task UpdateCategoryAsync( Guid categoryId, string name, decimal allocatedAmount, - ExpenditureType expenditureType, Guid actorUserId) + ExpenditureType expenditureType, Guid actorUserId, ClaimsPrincipal principal) { + await AuthorizeManageAsync(principal, nameof(UpdateCategoryAsync)); + var category = await _dbContext.BudgetCategories .Include(c => c.BudgetGroup) .FirstOrDefaultAsync(c => c.Id == categoryId) @@ -629,8 +699,10 @@ public async Task UpdateCategoryAsync( await _dbContext.SaveChangesAsync(); } - public async Task DeleteCategoryAsync(Guid categoryId, Guid actorUserId) + public async Task DeleteCategoryAsync(Guid categoryId, Guid actorUserId, ClaimsPrincipal principal) { + await AuthorizeManageAsync(principal, nameof(DeleteCategoryAsync)); + var category = await _dbContext.BudgetCategories .Include(c => c.BudgetGroup) .FirstOrDefaultAsync(c => c.Id == categoryId) @@ -662,15 +734,18 @@ public async Task DeleteCategoryAsync(Guid categoryId, Guid actorUserId) public async Task CreateLineItemAsync( Guid budgetCategoryId, string description, decimal amount, Guid? responsibleTeamId, string? notes, LocalDate? expectedDate, - int vatRate, Guid actorUserId) + int vatRate, Guid actorUserId, ClaimsPrincipal principal) { ValidateVatRate(vatRate); var category = await _dbContext.BudgetCategories .Include(c => c.BudgetGroup) + .ThenInclude(g => g!.BudgetYear) .FirstOrDefaultAsync(c => c.Id == budgetCategoryId) ?? throw new InvalidOperationException($"Budget category {budgetCategoryId} not found"); + await AuthorizeEditAsync(principal, category, nameof(CreateLineItemAsync)); + var budgetYearId = category.BudgetGroup!.BudgetYearId; await EnsureYearNotClosedAsync(budgetYearId); var now = _clock.GetCurrentInstant(); @@ -710,16 +785,19 @@ public async Task CreateLineItemAsync( public async Task UpdateLineItemAsync( Guid lineItemId, string description, decimal amount, Guid? responsibleTeamId, string? notes, LocalDate? expectedDate, - int vatRate, Guid actorUserId) + int vatRate, Guid actorUserId, ClaimsPrincipal principal) { ValidateVatRate(vatRate); var lineItem = await _dbContext.BudgetLineItems .Include(li => li.BudgetCategory) .ThenInclude(c => c!.BudgetGroup) + .ThenInclude(g => g!.BudgetYear) .FirstOrDefaultAsync(li => li.Id == lineItemId) ?? throw new InvalidOperationException($"Budget line item {lineItemId} not found"); + await AuthorizeEditAsync(principal, lineItem.BudgetCategory!, nameof(UpdateLineItemAsync)); + var budgetYearId = lineItem.BudgetCategory!.BudgetGroup!.BudgetYearId; await EnsureYearNotClosedAsync(budgetYearId); var now = _clock.GetCurrentInstant(); @@ -788,14 +866,17 @@ private static void ValidateVatRate(int vatRate) throw new ArgumentOutOfRangeException(nameof(vatRate), vatRate, "VAT rate must be between 0 and 21."); } - public async Task DeleteLineItemAsync(Guid lineItemId, Guid actorUserId) + public async Task DeleteLineItemAsync(Guid lineItemId, Guid actorUserId, ClaimsPrincipal principal) { var lineItem = await _dbContext.BudgetLineItems .Include(li => li.BudgetCategory) .ThenInclude(c => c!.BudgetGroup) + .ThenInclude(g => g!.BudgetYear) .FirstOrDefaultAsync(li => li.Id == lineItemId) ?? throw new InvalidOperationException($"Budget line item {lineItemId} not found"); + await AuthorizeEditAsync(principal, lineItem.BudgetCategory!, nameof(DeleteLineItemAsync)); + var budgetYearId = lineItem.BudgetCategory!.BudgetGroup!.BudgetYearId; await EnsureYearNotClosedAsync(budgetYearId); var now = _clock.GetCurrentInstant(); @@ -823,8 +904,11 @@ public async Task DeleteLineItemAsync(Guid lineItemId, Guid actorUserId) public async Task UpdateTicketingProjectionAsync( Guid budgetGroupId, LocalDate? startDate, LocalDate? eventDate, int initialSalesCount, decimal dailySalesRate, decimal averageTicketPrice, int vatRate, - decimal stripeFeePercent, decimal stripeFeeFixed, decimal ticketTailorFeePercent, Guid actorUserId) + decimal stripeFeePercent, decimal stripeFeeFixed, decimal ticketTailorFeePercent, Guid actorUserId, + ClaimsPrincipal principal) { + await AuthorizeManageAsync(principal, nameof(UpdateTicketingProjectionAsync)); + var group = await _dbContext.BudgetGroups.FindAsync(budgetGroupId) ?? throw new InvalidOperationException($"Budget group {budgetGroupId} not found"); @@ -1180,8 +1264,11 @@ public LocalDate ComputeVatSettlementDate(LocalDate expectedDate) public async Task SyncTicketingActualsAsync( Guid budgetYearId, IReadOnlyList weeklyActuals, + ClaimsPrincipal principal, CancellationToken ct = default) { + await AuthorizeManageAsync(principal, nameof(SyncTicketingActualsAsync)); + var ticketingGroup = await LoadTicketingGroupAsync(budgetYearId, ct); if (ticketingGroup is null) { @@ -1242,8 +1329,13 @@ public async Task SyncTicketingActualsAsync( return lineItemsCreated; } - public async Task RefreshTicketingProjectionsAsync(Guid budgetYearId, CancellationToken ct = default) + public async Task RefreshTicketingProjectionsAsync( + Guid budgetYearId, + ClaimsPrincipal principal, + CancellationToken ct = default) { + await AuthorizeManageAsync(principal, nameof(RefreshTicketingProjectionsAsync)); + var ticketingGroup = await LoadTicketingGroupAsync(budgetYearId, ct); if (ticketingGroup is null) return 0; diff --git a/src/Humans.Infrastructure/Services/TicketingBudgetService.cs b/src/Humans.Infrastructure/Services/TicketingBudgetService.cs index 5250c519c..3f2de959e 100644 --- a/src/Humans.Infrastructure/Services/TicketingBudgetService.cs +++ b/src/Humans.Infrastructure/Services/TicketingBudgetService.cs @@ -1,3 +1,4 @@ +using System.Security.Claims; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; using NodaTime; @@ -35,7 +36,7 @@ public TicketingBudgetService( _logger = logger; } - public async Task SyncActualsAsync(Guid budgetYearId, CancellationToken ct = default) + public async Task SyncActualsAsync(Guid budgetYearId, ClaimsPrincipal principal, CancellationToken ct = default) { try { @@ -82,7 +83,7 @@ public async Task SyncActualsAsync(Guid budgetYearId, CancellationToken ct // Delegate the BudgetLineItem / TicketingProjection mutations to // BudgetService, which owns those tables. - return await _budgetService.SyncTicketingActualsAsync(budgetYearId, weeklyActuals, ct); + return await _budgetService.SyncTicketingActualsAsync(budgetYearId, weeklyActuals, principal, ct); } catch (Exception ex) { @@ -91,11 +92,11 @@ public async Task SyncActualsAsync(Guid budgetYearId, CancellationToken ct } } - public async Task RefreshProjectionsAsync(Guid budgetYearId, CancellationToken ct = default) + public async Task RefreshProjectionsAsync(Guid budgetYearId, ClaimsPrincipal principal, CancellationToken ct = default) { try { - return await _budgetService.RefreshTicketingProjectionsAsync(budgetYearId, ct); + return await _budgetService.RefreshTicketingProjectionsAsync(budgetYearId, principal, ct); } catch (Exception ex) { diff --git a/src/Humans.Web/Authorization/AuthorizationPolicyExtensions.cs b/src/Humans.Web/Authorization/AuthorizationPolicyExtensions.cs index cc792d6d3..589906608 100644 --- a/src/Humans.Web/Authorization/AuthorizationPolicyExtensions.cs +++ b/src/Humans.Web/Authorization/AuthorizationPolicyExtensions.cs @@ -26,6 +26,7 @@ public static IServiceCollection AddHumansAuthorizationPolicies(this IServiceCol // Service-layer enforcement handlers (singleton — no scoped dependencies) services.AddSingleton(); + services.AddSingleton(); services.AddAuthorization(options => { diff --git a/src/Humans.Web/Authorization/Requirements/BudgetAuthorizationHandler.cs b/src/Humans.Web/Authorization/Requirements/BudgetAuthorizationHandler.cs index 62f7f0830..3dc483353 100644 --- a/src/Humans.Web/Authorization/Requirements/BudgetAuthorizationHandler.cs +++ b/src/Humans.Web/Authorization/Requirements/BudgetAuthorizationHandler.cs @@ -1,29 +1,34 @@ using System.Security.Claims; +using Humans.Application.Authorization; using Humans.Application.Interfaces; +using Humans.Domain.Constants; using Humans.Domain.Entities; using Microsoft.AspNetCore.Authorization; namespace Humans.Web.Authorization.Requirements; /// -/// Resource-based authorization handler for budget operations. -/// Evaluates whether a user can perform budget operations on a specific BudgetCategory. +/// Resource-based authorization handler for budget operations on a specific +/// (line-item create/update/delete). /// -/// Authorization logic: -/// - Admin: allow any category -/// - FinanceAdmin: allow any category +/// Authorization logic for : +/// - Admin / FinanceAdmin / system principal: allow any category /// - Department coordinator: allow only categories linked to their department /// - Everyone else: deny /// /// Also denies edits on restricted groups and deleted budget years for non-admin users. +/// +/// Uses to lazily resolve , +/// breaking the DI cycle: +/// BudgetService → IAuthorizationService → BudgetAuthorizationHandler → IBudgetService. /// public class BudgetAuthorizationHandler : AuthorizationHandler { - private readonly IBudgetService _budgetService; + private readonly IServiceProvider _serviceProvider; - public BudgetAuthorizationHandler(IBudgetService budgetService) + public BudgetAuthorizationHandler(IServiceProvider serviceProvider) { - _budgetService = budgetService; + _serviceProvider = serviceProvider; } protected override async Task HandleRequirementAsync( @@ -31,6 +36,18 @@ protected override async Task HandleRequirementAsync( BudgetOperationRequirement requirement, BudgetCategory resource) { + // Only this handler fires for the Edit requirement. Manage is handled by + // BudgetManageAuthorizationHandler with no resource. + if (!ReferenceEquals(requirement, BudgetOperationRequirement.Edit)) + return; + + // System principal (background jobs) is always allowed + if (SystemPrincipal.IsSystem(context.User)) + { + context.Succeed(requirement); + return; + } + // Admin and FinanceAdmin can edit any budget category if (RoleChecks.IsFinanceAdmin(context.User)) { @@ -55,7 +72,8 @@ protected override async Task HandleRequirementAsync( if (userIdClaim is null || !Guid.TryParse(userIdClaim.Value, out var userId)) return; - var coordinatorTeamIds = await _budgetService.GetEffectiveCoordinatorTeamIdsAsync(userId); + var budgetService = _serviceProvider.GetRequiredService(); + var coordinatorTeamIds = await budgetService.GetEffectiveCoordinatorTeamIdsAsync(userId); if (coordinatorTeamIds.Contains(resource.TeamId.Value)) { context.Succeed(requirement); diff --git a/src/Humans.Web/Authorization/Requirements/BudgetManageAuthorizationHandler.cs b/src/Humans.Web/Authorization/Requirements/BudgetManageAuthorizationHandler.cs new file mode 100644 index 000000000..36dae24ea --- /dev/null +++ b/src/Humans.Web/Authorization/Requirements/BudgetManageAuthorizationHandler.cs @@ -0,0 +1,40 @@ +using Humans.Application.Authorization; +using Microsoft.AspNetCore.Authorization; + +namespace Humans.Web.Authorization.Requirements; + +/// +/// Authorization handler for budget-wide management operations — resource-free. +/// +/// Fires only for and succeeds for: +/// - Admin or FinanceAdmin +/// - The system principal (background jobs) +/// +/// Everyone else is denied. Paired with the +/// handler — that one handles line-item edits with a +/// resource; this one handles admin mutations (budget years, groups, categories, +/// ticketing projections, sync jobs) where no category is in scope. +/// +public class BudgetManageAuthorizationHandler : AuthorizationHandler +{ + protected override Task HandleRequirementAsync( + AuthorizationHandlerContext context, + BudgetOperationRequirement requirement) + { + if (!ReferenceEquals(requirement, BudgetOperationRequirement.Manage)) + return Task.CompletedTask; + + if (SystemPrincipal.IsSystem(context.User)) + { + context.Succeed(requirement); + return Task.CompletedTask; + } + + if (RoleChecks.IsFinanceAdmin(context.User)) + { + context.Succeed(requirement); + } + + return Task.CompletedTask; + } +} diff --git a/src/Humans.Web/Authorization/Requirements/BudgetOperationRequirement.cs b/src/Humans.Web/Authorization/Requirements/BudgetOperationRequirement.cs deleted file mode 100644 index 8f8bbc8f0..000000000 --- a/src/Humans.Web/Authorization/Requirements/BudgetOperationRequirement.cs +++ /dev/null @@ -1,20 +0,0 @@ -using Microsoft.AspNetCore.Authorization; - -namespace Humans.Web.Authorization.Requirements; - -/// -/// Resource-based authorization requirement for budget operations. -/// Used with IAuthorizationService.AuthorizeAsync(User, resource, requirement) -/// where the resource is a BudgetCategory. -/// -public sealed class BudgetOperationRequirement : IAuthorizationRequirement -{ - public static readonly BudgetOperationRequirement Edit = new(nameof(Edit)); - - public string OperationName { get; } - - private BudgetOperationRequirement(string operationName) - { - OperationName = operationName; - } -} diff --git a/src/Humans.Web/Authorization/RoleChecks.cs b/src/Humans.Web/Authorization/RoleChecks.cs index ad366c84e..f0732faa3 100644 --- a/src/Humans.Web/Authorization/RoleChecks.cs +++ b/src/Humans.Web/Authorization/RoleChecks.cs @@ -3,7 +3,14 @@ namespace Humans.Web.Authorization; -public static class RoleChecks +/// +/// Internal helper for role-based predicates used by the Humans.Web authorization +/// layer (handlers, nav TagHelpers, controllers). Kept internal as part of the +/// first-class authorization transition — external callers should use +/// with +/// named policies or resource-based requirements instead. +/// +internal static class RoleChecks { private static readonly IReadOnlyList AdminAssignableRoles = [RoleNames.Admin, .. RoleNames.BoardManageableRoles]; diff --git a/src/Humans.Web/Controllers/BudgetController.cs b/src/Humans.Web/Controllers/BudgetController.cs index 6d8cf4356..594be0ac9 100644 --- a/src/Humans.Web/Controllers/BudgetController.cs +++ b/src/Humans.Web/Controllers/BudgetController.cs @@ -1,3 +1,4 @@ +using Humans.Application.Authorization; using Humans.Application.Interfaces; using Humans.Domain.Entities; using Humans.Web.Authorization; @@ -174,7 +175,7 @@ public async Task CreateLineItem(Guid budgetCategoryId, string de try { - await _budgetService.CreateLineItemAsync(budgetCategoryId, description, amount, responsibleTeamId, notes, nodaDate, vatRate, user.Id); + await _budgetService.CreateLineItemAsync(budgetCategoryId, description, amount, responsibleTeamId, notes, nodaDate, vatRate, user.Id, User); SetSuccess($"Line item '{description}' created."); } catch (Exception ex) @@ -203,7 +204,7 @@ public async Task UpdateLineItem(Guid id, string description, dec try { - await _budgetService.UpdateLineItemAsync(id, description, amount, responsibleTeamId, notes, nodaDate, vatRate, user.Id); + await _budgetService.UpdateLineItemAsync(id, description, amount, responsibleTeamId, notes, nodaDate, vatRate, user.Id, User); SetSuccess($"Line item '{description}' updated."); } catch (Exception ex) @@ -229,7 +230,7 @@ public async Task DeleteLineItem(Guid id, Guid budgetCategoryId) try { - await _budgetService.DeleteLineItemAsync(id, user.Id); + await _budgetService.DeleteLineItemAsync(id, user.Id, User); SetSuccess("Line item deleted."); } catch (Exception ex) diff --git a/src/Humans.Web/Controllers/FinanceController.cs b/src/Humans.Web/Controllers/FinanceController.cs index e7647bcce..56044ebc8 100644 --- a/src/Humans.Web/Controllers/FinanceController.cs +++ b/src/Humans.Web/Controllers/FinanceController.cs @@ -178,7 +178,7 @@ public async Task SyncDepartments(Guid id) try { - var count = await _budgetService.SyncDepartmentsAsync(id, user.Id); + var count = await _budgetService.SyncDepartmentsAsync(id, user.Id, User); if (count > 0) SetSuccess($"Synced {count} new department(s) into budget."); else @@ -208,7 +208,7 @@ public async Task CreateYear(string year, string name) try { - await _budgetService.CreateYearAsync(year, name, user.Id); + await _budgetService.CreateYearAsync(year, name, user.Id, User); SetSuccess($"Budget year '{name}' created."); return RedirectToAction(nameof(Admin)); } @@ -229,7 +229,7 @@ public async Task UpdateYearStatus(Guid id, BudgetYearStatus stat try { - await _budgetService.UpdateYearStatusAsync(id, status, user.Id); + await _budgetService.UpdateYearStatusAsync(id, status, user.Id, User); SetSuccess($"Budget year status updated to {status}."); return RedirectToAction(nameof(Admin)); } @@ -250,7 +250,7 @@ public async Task UpdateYear(Guid id, string year, string name) try { - await _budgetService.UpdateYearAsync(id, year, name, user.Id); + await _budgetService.UpdateYearAsync(id, year, name, user.Id, User); SetSuccess("Budget year updated."); return RedirectToAction(nameof(Admin)); } @@ -271,7 +271,7 @@ public async Task DeleteYear(Guid id) try { - await _budgetService.DeleteYearAsync(id, user.Id); + await _budgetService.DeleteYearAsync(id, user.Id, User); SetSuccess("Budget year deleted."); return RedirectToAction(nameof(Admin)); } @@ -292,7 +292,7 @@ public async Task CreateGroup(Guid budgetYearId, string name, boo try { - await _budgetService.CreateGroupAsync(budgetYearId, name, isRestricted, user.Id); + await _budgetService.CreateGroupAsync(budgetYearId, name, isRestricted, user.Id, User); SetSuccess($"Group '{name}' created."); return RedirectToAction(nameof(Admin)); } @@ -313,7 +313,7 @@ public async Task UpdateGroup(Guid id, string name, int sortOrder try { - await _budgetService.UpdateGroupAsync(id, name, sortOrder, isRestricted, user.Id); + await _budgetService.UpdateGroupAsync(id, name, sortOrder, isRestricted, user.Id, User); SetSuccess($"Group '{name}' updated."); return RedirectToAction(nameof(Admin)); } @@ -334,7 +334,7 @@ public async Task DeleteGroup(Guid id) try { - await _budgetService.DeleteGroupAsync(id, user.Id); + await _budgetService.DeleteGroupAsync(id, user.Id, User); SetSuccess("Group deleted."); return RedirectToAction(nameof(Admin)); } @@ -356,7 +356,7 @@ public async Task CreateCategory(Guid budgetGroupId, string name, try { - await _budgetService.CreateCategoryAsync(budgetGroupId, name, allocatedAmount, expenditureType, teamId, user.Id); + await _budgetService.CreateCategoryAsync(budgetGroupId, name, allocatedAmount, expenditureType, teamId, user.Id, User); SetSuccess($"Category '{name}' created."); return RedirectToAction(nameof(YearDetail), new { id = budgetYearId }); } @@ -378,7 +378,7 @@ public async Task UpdateCategory(Guid id, string name, decimal al try { - await _budgetService.UpdateCategoryAsync(id, name, allocatedAmount, expenditureType, user.Id); + await _budgetService.UpdateCategoryAsync(id, name, allocatedAmount, expenditureType, user.Id, User); SetSuccess($"Category '{name}' updated."); return RedirectToAction(nameof(CategoryDetail), new { id }); } @@ -399,7 +399,7 @@ public async Task DeleteCategory(Guid id, Guid budgetYearId) try { - await _budgetService.DeleteCategoryAsync(id, user.Id); + await _budgetService.DeleteCategoryAsync(id, user.Id, User); SetSuccess("Category deleted."); return RedirectToAction(nameof(YearDetail), new { id = budgetYearId }); } @@ -423,7 +423,7 @@ public async Task CreateLineItem(Guid budgetCategoryId, string de try { - await _budgetService.CreateLineItemAsync(budgetCategoryId, description, amount, responsibleTeamId, notes, nodaDate, vatRate, user.Id); + await _budgetService.CreateLineItemAsync(budgetCategoryId, description, amount, responsibleTeamId, notes, nodaDate, vatRate, user.Id, User); SetSuccess($"Line item '{description}' created."); return RedirectToAction(nameof(CategoryDetail), new { id = budgetCategoryId }); } @@ -447,7 +447,7 @@ public async Task UpdateLineItem(Guid id, string description, dec try { - await _budgetService.UpdateLineItemAsync(id, description, amount, responsibleTeamId, notes, nodaDate, vatRate, user.Id); + await _budgetService.UpdateLineItemAsync(id, description, amount, responsibleTeamId, notes, nodaDate, vatRate, user.Id, User); SetSuccess($"Line item '{description}' updated."); return RedirectToAction(nameof(CategoryDetail), new { id = budgetCategoryId }); } @@ -468,7 +468,7 @@ public async Task DeleteLineItem(Guid id, Guid budgetCategoryId) try { - await _budgetService.DeleteLineItemAsync(id, user.Id); + await _budgetService.DeleteLineItemAsync(id, user.Id, User); SetSuccess("Line item deleted."); return RedirectToAction(nameof(CategoryDetail), new { id = budgetCategoryId }); } @@ -489,7 +489,7 @@ public async Task EnsureTicketingGroup(Guid id) try { - var added = await _budgetService.EnsureTicketingGroupAsync(id, user.Id); + var added = await _budgetService.EnsureTicketingGroupAsync(id, user.Id, User); if (added) SetSuccess("Ticketing group added to this budget year."); else @@ -520,10 +520,10 @@ public async Task UpdateTicketingProjection(Guid groupId, DateTim { await _budgetService.UpdateTicketingProjectionAsync(groupId, nodaStart, nodaEvent, initialSalesCount, dailySalesRate, averageTicketPrice, vatRate, - stripeFeePercent, stripeFeeFixed, ticketTailorFeePercent, user.Id); + stripeFeePercent, stripeFeeFixed, ticketTailorFeePercent, user.Id, User); // Refresh projections after saving parameters (no actuals sync needed) - var count = await _ticketingBudgetService.RefreshProjectionsAsync(budgetYearId); + var count = await _ticketingBudgetService.RefreshProjectionsAsync(budgetYearId, User); SetSuccess($"Ticketing projection saved — {count} projected line item(s) generated."); } catch (Exception ex) @@ -541,7 +541,7 @@ public async Task SyncTicketingBudget(Guid yearId) { try { - var count = await _ticketingBudgetService.SyncActualsAsync(yearId); + var count = await _ticketingBudgetService.SyncActualsAsync(yearId, User); if (count > 0) SetSuccess($"Synced {count} ticketing line item(s) from ticket sales data."); else diff --git a/src/Humans.Web/Infrastructure/DevelopmentBudgetSeeder.cs b/src/Humans.Web/Infrastructure/DevelopmentBudgetSeeder.cs index 37d935fac..6e616243b 100644 --- a/src/Humans.Web/Infrastructure/DevelopmentBudgetSeeder.cs +++ b/src/Humans.Web/Infrastructure/DevelopmentBudgetSeeder.cs @@ -1,3 +1,4 @@ +using Humans.Application.Authorization; using Humans.Application.Extensions; using Humans.Application.Interfaces; using Humans.Domain.Entities; @@ -178,19 +179,21 @@ public async Task SeedAsync(Guid actorUserId, Cance var allYears = await _budgetService.GetAllYearsAsync(includeArchived: true); var budgetYear = allYears.FirstOrDefault(y => string.Equals(y.Year, budgetYearCode, StringComparison.Ordinal)); + var principal = SystemPrincipal.Instance; + if (budgetYear is null) { - budgetYear = await _budgetService.CreateYearAsync(budgetYearCode, budgetYearName, actorUserId); + budgetYear = await _budgetService.CreateYearAsync(budgetYearCode, budgetYearName, actorUserId, principal); } else if (budgetYear.IsDeleted) { - await _budgetService.RestoreYearAsync(budgetYear.Id, actorUserId); + await _budgetService.RestoreYearAsync(budgetYear.Id, actorUserId, principal); } - var departmentCategoriesSynced = await _budgetService.SyncDepartmentsAsync(budgetYear.Id, actorUserId); + var departmentCategoriesSynced = await _budgetService.SyncDepartmentsAsync(budgetYear.Id, actorUserId, principal); var groupsCreated = 0; - if (await _budgetService.EnsureTicketingGroupAsync(budgetYear.Id, actorUserId)) + if (await _budgetService.EnsureTicketingGroupAsync(budgetYear.Id, actorUserId, principal)) { groupsCreated++; } @@ -199,7 +202,7 @@ public async Task SeedAsync(Guid actorUserId, Cance var activatedBudgetYear = false; if (activeYear is null) { - await _budgetService.UpdateYearStatusAsync(budgetYear.Id, BudgetYearStatus.Active, actorUserId); + await _budgetService.UpdateYearStatusAsync(budgetYear.Id, BudgetYearStatus.Active, actorUserId, principal); activatedBudgetYear = true; } @@ -208,21 +211,21 @@ public async Task SeedAsync(Guid actorUserId, Cance ?? throw new InvalidOperationException($"Budget year {budgetYear.Id} not found after creation"); var departmentGroup = currentYear.Groups.Single(g => g.IsDepartmentGroup); - await _budgetService.UpdateGroupAsync(departmentGroup.Id, departmentGroup.Name, 0, departmentGroup.IsRestricted, actorUserId); + await _budgetService.UpdateGroupAsync(departmentGroup.Id, departmentGroup.Name, 0, departmentGroup.IsRestricted, actorUserId, principal); var sharedServicesGroup = currentYear.Groups.FirstOrDefault(g => string.Equals(g.Name, "Shared Services", StringComparison.Ordinal)); if (sharedServicesGroup is null) { - sharedServicesGroup = await _budgetService.CreateGroupAsync(budgetYear.Id, "Shared Services", false, actorUserId); + sharedServicesGroup = await _budgetService.CreateGroupAsync(budgetYear.Id, "Shared Services", false, actorUserId, principal); groupsCreated++; } - await _budgetService.UpdateGroupAsync(sharedServicesGroup.Id, sharedServicesGroup.Name, 1, sharedServicesGroup.IsRestricted, actorUserId); + await _budgetService.UpdateGroupAsync(sharedServicesGroup.Id, sharedServicesGroup.Name, 1, sharedServicesGroup.IsRestricted, actorUserId, principal); var ticketingGroup = currentYear.Groups.Single(g => g.IsTicketingGroup); - await _budgetService.UpdateGroupAsync(ticketingGroup.Id, ticketingGroup.Name, 2, ticketingGroup.IsRestricted, actorUserId); + await _budgetService.UpdateGroupAsync(ticketingGroup.Id, ticketingGroup.Name, 2, ticketingGroup.IsRestricted, actorUserId, principal); var categoriesCreated = 0; var lineItemsCreated = 0; @@ -245,7 +248,7 @@ public async Task SeedAsync(Guid actorUserId, Cance if (category is null) { category = await _budgetService.CreateCategoryAsync( - departmentGroup.Id, teamSeed.Name, teamSeed.AllocatedAmount, teamSeed.ExpenditureType, team.Id, actorUserId); + departmentGroup.Id, teamSeed.Name, teamSeed.AllocatedAmount, teamSeed.ExpenditureType, team.Id, actorUserId, principal); categoriesCreated++; } @@ -260,7 +263,7 @@ public async Task SeedAsync(Guid actorUserId, Cance if (category is null) { category = await _budgetService.CreateCategoryAsync( - sharedServicesGroup.Id, sharedSeed.Name, sharedSeed.AllocatedAmount, sharedSeed.ExpenditureType, null, actorUserId); + sharedServicesGroup.Id, sharedSeed.Name, sharedSeed.AllocatedAmount, sharedSeed.ExpenditureType, null, actorUserId, principal); categoriesCreated++; } @@ -275,7 +278,7 @@ public async Task SeedAsync(Guid actorUserId, Cance if (category is null) { category = await _budgetService.CreateCategoryAsync( - ticketingGroup.Id, ticketSeed.Name, ticketSeed.AllocatedAmount, ticketSeed.ExpenditureType, null, actorUserId); + ticketingGroup.Id, ticketSeed.Name, ticketSeed.AllocatedAmount, ticketSeed.ExpenditureType, null, actorUserId, principal); categoriesCreated++; } @@ -293,7 +296,8 @@ await _budgetService.UpdateTicketingProjectionAsync( stripeFeePercent: 1.50m, stripeFeeFixed: 0.25m, ticketTailorFeePercent: 3.00m, - actorUserId); + actorUserId, + principal); _logger.LogInformation( "Development budget seed completed for {BudgetYearCode}: teamsCreated={TeamsCreated}, teamsUpdated={TeamsUpdated}, categoriesCreated={CategoriesCreated}, lineItemsCreated={LineItemsCreated}", @@ -347,8 +351,10 @@ private async Task SeedLineItemsAsync( CancellationToken cancellationToken, Action onLineItemCreated) { + var principal = SystemPrincipal.Instance; + await _budgetService.UpdateCategoryAsync(category.Id, category.Name, - lineItems.Sum(li => li.Amount), category.ExpenditureType, actorUserId); + lineItems.Sum(li => li.Amount), category.ExpenditureType, actorUserId, principal); foreach (var lineItem in lineItems) { @@ -365,7 +371,8 @@ await _budgetService.CreateLineItemAsync( notes: lineItem.Notes, expectedDate: lineItem.ExpectedDate, vatRate: lineItem.VatRate, - actorUserId); + actorUserId, + principal); onLineItemCreated(); continue; @@ -379,7 +386,8 @@ await _budgetService.UpdateLineItemAsync( notes: lineItem.Notes, expectedDate: lineItem.ExpectedDate, vatRate: lineItem.VatRate, - actorUserId); + actorUserId, + principal); } } diff --git a/tests/Humans.Application.Tests/Authorization/BudgetAuthorizationHandlerTests.cs b/tests/Humans.Application.Tests/Authorization/BudgetAuthorizationHandlerTests.cs index d93e1c5cd..0879af40c 100644 --- a/tests/Humans.Application.Tests/Authorization/BudgetAuthorizationHandlerTests.cs +++ b/tests/Humans.Application.Tests/Authorization/BudgetAuthorizationHandlerTests.cs @@ -1,10 +1,12 @@ using System.Security.Claims; using AwesomeAssertions; +using Humans.Application.Authorization; using Humans.Application.Interfaces; using Humans.Domain.Constants; using Humans.Domain.Entities; using Humans.Web.Authorization.Requirements; using Microsoft.AspNetCore.Authorization; +using Microsoft.Extensions.DependencyInjection; using NSubstitute; using Xunit; @@ -26,12 +28,43 @@ public sealed class BudgetAuthorizationHandlerTests public BudgetAuthorizationHandlerTests() { - _handler = new BudgetAuthorizationHandler(_budgetService); + var services = new ServiceCollection(); + services.AddSingleton(_budgetService); + var provider = services.BuildServiceProvider(); + _handler = new BudgetAuthorizationHandler(provider); _budgetService.GetEffectiveCoordinatorTeamIdsAsync(UserId) .Returns(new HashSet { CoordinatorTeamId }); } + // --- System principal override (background jobs) --- + + [Fact] + public async Task SystemPrincipal_CanEditAnyCategory() + { + var category = CreateCategory(OtherTeamId, isRestricted: true, isDeleted: true); + + var result = await EvaluateAsync(SystemPrincipal.Instance, category); + + result.Should().BeTrue(); + } + + [Fact] + public async Task ManageRequirement_NotHandled_ByEditHandler() + { + // The Edit handler only fires for the Edit requirement. When the Manage + // requirement is supplied with a BudgetCategory resource, this handler + // must not succeed — Manage is handled by BudgetManageAuthorizationHandler. + var user = CreateUserWithRoles(RoleNames.FinanceAdmin); + var category = CreateCategory(OtherTeamId); + var requirement = BudgetOperationRequirement.Manage; + var context = new AuthorizationHandlerContext([requirement], user, category); + + await _handler.HandleAsync(context); + + context.HasSucceeded.Should().BeFalse(); + } + // --- Admin override --- [Fact] diff --git a/tests/Humans.Application.Tests/Authorization/BudgetManageAuthorizationHandlerTests.cs b/tests/Humans.Application.Tests/Authorization/BudgetManageAuthorizationHandlerTests.cs new file mode 100644 index 000000000..0a27862ad --- /dev/null +++ b/tests/Humans.Application.Tests/Authorization/BudgetManageAuthorizationHandlerTests.cs @@ -0,0 +1,102 @@ +using System.Security.Claims; +using AwesomeAssertions; +using Humans.Application.Authorization; +using Humans.Domain.Constants; +using Humans.Web.Authorization.Requirements; +using Microsoft.AspNetCore.Authorization; +using Xunit; + +namespace Humans.Application.Tests.Authorization; + +/// +/// Unit tests for BudgetManageAuthorizationHandler — evaluates +/// for budget-wide admin mutations +/// (budget year lifecycle, groups, categories, projection parameters, sync jobs). +/// +public sealed class BudgetManageAuthorizationHandlerTests +{ + private readonly BudgetManageAuthorizationHandler _handler = new(); + + [Fact] + public async Task Admin_CanManage() + { + var user = CreateUserWithRoles(RoleNames.Admin); + var result = await EvaluateAsync(user); + result.Should().BeTrue(); + } + + [Fact] + public async Task FinanceAdmin_CanManage() + { + var user = CreateUserWithRoles(RoleNames.FinanceAdmin); + var result = await EvaluateAsync(user); + result.Should().BeTrue(); + } + + [Fact] + public async Task SystemPrincipal_CanManage() + { + var result = await EvaluateAsync(SystemPrincipal.Instance); + result.Should().BeTrue(); + } + + [Fact] + public async Task Board_CannotManage() + { + var user = CreateUserWithRoles(RoleNames.Board); + var result = await EvaluateAsync(user); + result.Should().BeFalse(); + } + + [Fact] + public async Task RegularUser_CannotManage() + { + var user = CreateUserWithRoles(); + var result = await EvaluateAsync(user); + result.Should().BeFalse(); + } + + [Fact] + public async Task UnauthenticatedUser_CannotManage() + { + var user = new ClaimsPrincipal(new ClaimsIdentity()); + var result = await EvaluateAsync(user); + result.Should().BeFalse(); + } + + [Fact] + public async Task EditRequirement_NotHandled_ByManageHandler() + { + // Even an Admin should not have this handler succeed the Edit requirement — + // that requirement is scoped to BudgetCategory resources via a different handler. + var user = CreateUserWithRoles(RoleNames.Admin); + var requirement = BudgetOperationRequirement.Edit; + var context = new AuthorizationHandlerContext([requirement], user, resource: null); + + await _handler.HandleAsync(context); + + context.HasSucceeded.Should().BeFalse(); + } + + private async Task EvaluateAsync(ClaimsPrincipal user) + { + var requirement = BudgetOperationRequirement.Manage; + var context = new AuthorizationHandlerContext([requirement], user, resource: null); + await _handler.HandleAsync(context); + return context.HasSucceeded; + } + + private static ClaimsPrincipal CreateUserWithRoles(params string[] roles) + { + var claims = new List + { + new(ClaimTypes.NameIdentifier, Guid.NewGuid().ToString()), + new(ClaimTypes.Name, "user@example.com") + }; + foreach (var role in roles) + { + claims.Add(new Claim(ClaimTypes.Role, role)); + } + return new ClaimsPrincipal(new ClaimsIdentity(claims, "TestAuth")); + } +} diff --git a/tests/Humans.Application.Tests/Services/BudgetServiceTests.cs b/tests/Humans.Application.Tests/Services/BudgetServiceTests.cs index 92ac93d59..a40069854 100644 --- a/tests/Humans.Application.Tests/Services/BudgetServiceTests.cs +++ b/tests/Humans.Application.Tests/Services/BudgetServiceTests.cs @@ -1,10 +1,16 @@ +using System.Security.Claims; using AwesomeAssertions; +using Humans.Application.Authorization; +using Humans.Domain.Constants; using Humans.Domain.Entities; +using Humans.Domain.Enums; using Humans.Infrastructure.Data; using Humans.Infrastructure.Services; +using Microsoft.AspNetCore.Authorization; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging.Abstractions; using NodaTime.Testing; +using NSubstitute; using Xunit; namespace Humans.Application.Tests.Services; @@ -12,7 +18,9 @@ namespace Humans.Application.Tests.Services; public class BudgetServiceTests : IDisposable { private readonly HumansDbContext _dbContext; + private readonly IAuthorizationService _authorizationService = Substitute.For(); private readonly BudgetService _service; + private readonly ClaimsPrincipal _financeAdmin; public BudgetServiceTests() { @@ -21,10 +29,20 @@ public BudgetServiceTests() .Options; _dbContext = new HumansDbContext(options); + + // Default: authorize succeeds. Tests for authorization denial override this per-test. + _authorizationService.AuthorizeAsync( + Arg.Any(), Arg.Any(), + Arg.Any>()) + .Returns(AuthorizationResult.Success()); + _service = new BudgetService( _dbContext, + _authorizationService, new FakeClock(NodaTime.Instant.FromUtc(2026, 3, 31, 12, 0)), NullLogger.Instance); + + _financeAdmin = CreateUserWithRoles(RoleNames.FinanceAdmin); } public void Dispose() @@ -48,7 +66,8 @@ public async Task CreateLineItemAsync_rejects_vat_rates_outside_0_to_21(int vatR null, null, vatRate, - Guid.NewGuid()); + Guid.NewGuid(), + _financeAdmin); await act.Should().ThrowAsync() .WithMessage("*between 0 and 21*"); @@ -79,12 +98,211 @@ public async Task UpdateLineItemAsync_rejects_vat_rates_outside_0_to_21(int vatR null, null, vatRate, - Guid.NewGuid()); + Guid.NewGuid(), + _financeAdmin); await act.Should().ThrowAsync() .WithMessage("*between 0 and 21*"); } + // ───────────────────────── Authorization enforcement ───────────────────────── + + [Fact] + public async Task CreateLineItemAsync_Unauthorized_ThrowsBeforeMutating() + { + var category = await SeedCategoryAsync(); + _authorizationService.AuthorizeAsync( + Arg.Any(), Arg.Any(), + Arg.Any>()) + .Returns(AuthorizationResult.Failed()); + + var unprivileged = new ClaimsPrincipal(new ClaimsIdentity()); + + var act = () => _service.CreateLineItemAsync( + category.Id, "Blocked", 100m, null, null, null, 0, Guid.NewGuid(), unprivileged); + + await act.Should().ThrowAsync(); + (await _dbContext.BudgetLineItems.CountAsync()).Should().Be(0); + } + + [Fact] + public async Task UpdateLineItemAsync_Unauthorized_ThrowsAndDoesNotMutate() + { + var category = await SeedCategoryAsync(); + var lineItem = new BudgetLineItem + { + Id = Guid.NewGuid(), + BudgetCategoryId = category.Id, + Description = "Existing", + Amount = 100m, + VatRate = 0 + }; + _dbContext.BudgetLineItems.Add(lineItem); + await _dbContext.SaveChangesAsync(); + + _authorizationService.AuthorizeAsync( + Arg.Any(), Arg.Any(), + Arg.Any>()) + .Returns(AuthorizationResult.Failed()); + + var unprivileged = new ClaimsPrincipal(new ClaimsIdentity()); + + var act = () => _service.UpdateLineItemAsync( + lineItem.Id, "Updated", 999m, null, null, null, 0, Guid.NewGuid(), unprivileged); + + await act.Should().ThrowAsync(); + var reloaded = await _dbContext.BudgetLineItems.FindAsync(lineItem.Id); + reloaded!.Description.Should().Be("Existing"); + reloaded.Amount.Should().Be(100m); + } + + [Fact] + public async Task DeleteLineItemAsync_Unauthorized_ThrowsAndDoesNotRemove() + { + var category = await SeedCategoryAsync(); + var lineItem = new BudgetLineItem + { + Id = Guid.NewGuid(), + BudgetCategoryId = category.Id, + Description = "Existing", + Amount = 100m, + VatRate = 0 + }; + _dbContext.BudgetLineItems.Add(lineItem); + await _dbContext.SaveChangesAsync(); + + _authorizationService.AuthorizeAsync( + Arg.Any(), Arg.Any(), + Arg.Any>()) + .Returns(AuthorizationResult.Failed()); + + var unprivileged = new ClaimsPrincipal(new ClaimsIdentity()); + + var act = () => _service.DeleteLineItemAsync(lineItem.Id, Guid.NewGuid(), unprivileged); + + await act.Should().ThrowAsync(); + (await _dbContext.BudgetLineItems.CountAsync()).Should().Be(1); + } + + [Fact] + public async Task CreateYearAsync_Unauthorized_Throws() + { + _authorizationService.AuthorizeAsync( + Arg.Any(), Arg.Any(), + Arg.Any>()) + .Returns(AuthorizationResult.Failed()); + + var unprivileged = new ClaimsPrincipal(new ClaimsIdentity()); + + var act = () => _service.CreateYearAsync("2027", "Budget 2027", Guid.NewGuid(), unprivileged); + + await act.Should().ThrowAsync(); + (await _dbContext.BudgetYears.CountAsync()).Should().Be(0); + } + + [Fact] + public async Task UpdateYearStatusAsync_Unauthorized_Throws() + { + var year = new BudgetYear + { + Id = Guid.NewGuid(), + Year = "2026", + Name = "Budget 2026", + Status = BudgetYearStatus.Draft + }; + _dbContext.BudgetYears.Add(year); + await _dbContext.SaveChangesAsync(); + + _authorizationService.AuthorizeAsync( + Arg.Any(), Arg.Any(), + Arg.Any>()) + .Returns(AuthorizationResult.Failed()); + + var unprivileged = new ClaimsPrincipal(new ClaimsIdentity()); + + var act = () => _service.UpdateYearStatusAsync(year.Id, BudgetYearStatus.Active, Guid.NewGuid(), unprivileged); + + await act.Should().ThrowAsync(); + var reloaded = await _dbContext.BudgetYears.FindAsync(year.Id); + reloaded!.Status.Should().Be(BudgetYearStatus.Draft); + } + + [Fact] + public async Task CreateGroupAsync_Unauthorized_Throws() + { + var year = new BudgetYear + { + Id = Guid.NewGuid(), + Year = "2026", + Name = "Budget 2026" + }; + _dbContext.BudgetYears.Add(year); + await _dbContext.SaveChangesAsync(); + + _authorizationService.AuthorizeAsync( + Arg.Any(), Arg.Any(), + Arg.Any>()) + .Returns(AuthorizationResult.Failed()); + + var unprivileged = new ClaimsPrincipal(new ClaimsIdentity()); + + var act = () => _service.CreateGroupAsync(year.Id, "Gear", false, Guid.NewGuid(), unprivileged); + + await act.Should().ThrowAsync(); + (await _dbContext.BudgetGroups.CountAsync()).Should().Be(0); + } + + [Fact] + public async Task CreateCategoryAsync_Unauthorized_Throws() + { + var year = new BudgetYear { Id = Guid.NewGuid(), Year = "2026", Name = "Budget 2026" }; + var group = new BudgetGroup { Id = Guid.NewGuid(), BudgetYearId = year.Id, Name = "General" }; + _dbContext.BudgetYears.Add(year); + _dbContext.BudgetGroups.Add(group); + await _dbContext.SaveChangesAsync(); + + _authorizationService.AuthorizeAsync( + Arg.Any(), Arg.Any(), + Arg.Any>()) + .Returns(AuthorizationResult.Failed()); + + var unprivileged = new ClaimsPrincipal(new ClaimsIdentity()); + + var act = () => _service.CreateCategoryAsync( + group.Id, "Supplies", 100m, ExpenditureType.OpEx, null, Guid.NewGuid(), unprivileged); + + await act.Should().ThrowAsync(); + (await _dbContext.BudgetCategories.CountAsync()).Should().Be(0); + } + + [Fact] + public async Task CreateYearAsync_AuthorizedFinanceAdmin_Succeeds() + { + var year = await _service.CreateYearAsync("2027", "Budget 2027", Guid.NewGuid(), _financeAdmin); + + year.Should().NotBeNull(); + year.Year.Should().Be("2027"); + (await _dbContext.BudgetYears.CountAsync()).Should().BeGreaterThan(0); + + await _authorizationService.Received().AuthorizeAsync( + _financeAdmin, Arg.Is(r => r == null), Arg.Any>()); + } + + [Fact] + public async Task CreateLineItemAsync_PassesCategoryAsResource() + { + var category = await SeedCategoryAsync(); + + await _service.CreateLineItemAsync( + category.Id, "Valid", 100m, null, null, null, 0, Guid.NewGuid(), _financeAdmin); + + // Verify the resource passed to AuthorizeAsync is the loaded BudgetCategory + await _authorizationService.Received().AuthorizeAsync( + _financeAdmin, + Arg.Is(o => o != null && ((BudgetCategory)o).Id == category.Id), + Arg.Any>()); + } + private async Task SeedCategoryAsync() { var year = new BudgetYear @@ -115,4 +333,18 @@ private async Task SeedCategoryAsync() return category; } + + private static ClaimsPrincipal CreateUserWithRoles(params string[] roles) + { + var claims = new List + { + new(ClaimTypes.NameIdentifier, Guid.NewGuid().ToString()), + new(ClaimTypes.Name, "finance@example.com") + }; + foreach (var role in roles) + { + claims.Add(new Claim(ClaimTypes.Role, role)); + } + return new ClaimsPrincipal(new ClaimsIdentity(claims, "TestAuth")); + } }