diff --git a/API/ROLAC.API.Tests/Services/ExpenseServiceTests.cs b/API/ROLAC.API.Tests/Services/ExpenseServiceTests.cs index 0a985e3..a927cfa 100644 --- a/API/ROLAC.API.Tests/Services/ExpenseServiceTests.cs +++ b/API/ROLAC.API.Tests/Services/ExpenseServiceTests.cs @@ -55,6 +55,16 @@ public class ExpenseServiceTests return new ExpenseService(db, http.Object, fs); } + // Builds a service whose principal carries ONLY the "sub" claim (no NameIdentifier), + // mirroring the real JWT (NameClaimType="sub", MapInboundClaims=false). + private static ExpenseService SvcWithSubClaim(AppDbContext db, FakeStorage fs, string userId) + { + var http = new Mock(); + var ctx = new DefaultHttpContext { User = new(new ClaimsIdentity(new[] { new Claim("sub", userId) })) }; + http.Setup(x => x.HttpContext).Returns(ctx); + return new ExpenseService(db, http.Object, fs); + } + private static CreateExpenseRequest Reimb() => new() { Type = "StaffReimbursement", MinistryId = 1, CategoryGroupId = 1, SubCategoryId = 1, @@ -69,6 +79,24 @@ public class ExpenseServiceTests ExpenseDate = r.ExpenseDate, Notes = r.Notes, }; + [Fact] + public async Task Create_Reimbursement_ResolvesUserId_FromSubClaim() + { + // Regression: the real JWT exposes the user id as "sub", not ClaimTypes.NameIdentifier. + // SubmittedBy must be the sub value (not "system"), or the self-ownership guard breaks. + var db = BuildDb("ignored"); + db.Ministries.Add(new Ministry { Id = 1, Name_en = "Worship" }); + db.ExpenseCategoryGroups.Add(new ExpenseCategoryGroup { Id = 1, Name_en = "Equipment" }); + db.ExpenseSubCategories.Add(new ExpenseSubCategory { Id = 1, GroupId = 1, Name_en = "Purchase" }); + await db.SaveChangesAsync(); + var svc = SvcWithSubClaim(db, new FakeStorage(), "user-guid-123"); + + var id = await svc.CreateAsync(Reimb(), isFinance: false); + + var e = await db.Expenses.FindAsync(id); + Assert.Equal("user-guid-123", e!.SubmittedBy); + } + [Fact] public async Task Create_Vendor_AsFinance_IsImmediatelyPaid() { diff --git a/API/ROLAC.API/Controllers/ExpensesController.cs b/API/ROLAC.API/Controllers/ExpensesController.cs index 93c7bc7..b0107c6 100644 --- a/API/ROLAC.API/Controllers/ExpensesController.cs +++ b/API/ROLAC.API/Controllers/ExpensesController.cs @@ -1,3 +1,4 @@ +using System.Security.Claims; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using ROLAC.API.DTOs.Expense; @@ -16,6 +17,10 @@ public class ExpensesController : ControllerBase private bool IsFinance() => User.IsInRole("finance") || User.IsInRole("super_admin"); private bool CanViewAll() => IsFinance() || User.IsInRole("pastor"); + // User id lives in the "sub" claim (NameClaimType="sub"); NameIdentifier is absent at runtime. + private string CurrentUserId() => + User.FindFirstValue(ClaimTypes.NameIdentifier) ?? User.FindFirstValue("sub") ?? ""; + [HttpGet] public async Task GetPaged( [FromQuery] int page = 1, [FromQuery] int pageSize = 20, [FromQuery] string? search = null, @@ -29,8 +34,7 @@ public class ExpensesController : ControllerBase [HttpGet("mine")] public async Task GetMine([FromQuery] string? status = null, [FromQuery] int page = 1, [FromQuery] int pageSize = 20) { - var uid = User.FindFirst(System.Security.Claims.ClaimTypes.NameIdentifier)!.Value; - return Ok(await _svc.GetMineAsync(uid, status, page, pageSize)); + return Ok(await _svc.GetMineAsync(CurrentUserId(), status, page, pageSize)); } [HttpGet("{id:int}")] @@ -38,8 +42,7 @@ public class ExpensesController : ControllerBase { var dto = await _svc.GetByIdAsync(id); if (dto is null) return NotFound(); - var uid = User.FindFirst(System.Security.Claims.ClaimTypes.NameIdentifier)!.Value; - if (!CanViewAll() && dto.SubmittedBy != uid) return Forbid(); + if (!CanViewAll() && dto.SubmittedBy != CurrentUserId()) return Forbid(); return Ok(dto); } diff --git a/API/ROLAC.API/Services/ExpenseService.cs b/API/ROLAC.API/Services/ExpenseService.cs index dac185d..769e8e4 100644 --- a/API/ROLAC.API/Services/ExpenseService.cs +++ b/API/ROLAC.API/Services/ExpenseService.cs @@ -17,8 +17,13 @@ public class ExpenseService : IExpenseService public ExpenseService(AppDbContext db, IHttpContextAccessor http, IFileStorage storage) { _db = db; _http = http; _storage = storage; } + // The JWT carries the user id in the "sub" claim (NameClaimType="sub", MapInboundClaims=false), + // so ClaimTypes.NameIdentifier is absent at runtime. Check NameIdentifier first (unit tests set it), + // then fall back to "sub" (real tokens). Required for the self-ownership guard to work in production. private string CurrentUserId => - _http.HttpContext?.User.FindFirstValue(ClaimTypes.NameIdentifier) ?? "system"; + _http.HttpContext?.User.FindFirstValue(ClaimTypes.NameIdentifier) + ?? _http.HttpContext?.User.FindFirstValue("sub") + ?? "system"; public async Task> GetPagedAsync( int page, int pageSize, string? search, int? ministryId, diff --git a/API/ROLAC.API/Services/MonthlyStatementService.cs b/API/ROLAC.API/Services/MonthlyStatementService.cs index 222b8a8..4ba1669 100644 --- a/API/ROLAC.API/Services/MonthlyStatementService.cs +++ b/API/ROLAC.API/Services/MonthlyStatementService.cs @@ -12,8 +12,11 @@ public class MonthlyStatementService : IMonthlyStatementService private readonly IHttpContextAccessor _http; public MonthlyStatementService(AppDbContext db, IHttpContextAccessor http) { _db = db; _http = http; } + // See ExpenseService: the user id lives in the "sub" claim at runtime; NameIdentifier is for tests. private string CurrentUserId => - _http.HttpContext?.User.FindFirstValue(ClaimTypes.NameIdentifier) ?? "system"; + _http.HttpContext?.User.FindFirstValue(ClaimTypes.NameIdentifier) + ?? _http.HttpContext?.User.FindFirstValue("sub") + ?? "system"; public async Task> GetAllAsync(int? year) {