fix(expense): open category read to all authed users; statement lookups via FirstOrDefaultAsync

Final-review findings:
- ExpenseCategoriesController was finance-only at the class level, but the member
  self-service reimbursement form reads the category list to populate its dropdown,
  so members got 403 and could not submit. Open GET to any authenticated user;
  keep group/subcategory writes finance-only (mirrors MinistriesController).
  Verified live with a member-role account: reads 200, writes 403, self-submit 200.
- MonthlyStatementService Update/Finalize now use FirstOrDefaultAsync for
  convention consistency with the rest of the service layer.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Chris Chen
2026-05-29 19:14:18 -07:00
parent e1f99158aa
commit 95fa37ebdf
2 changed files with 10 additions and 3 deletions
@@ -7,7 +7,8 @@ namespace ROLAC.API.Controllers;
[ApiController] [ApiController]
[Route("api/expense-categories")] [Route("api/expense-categories")]
[Authorize(Roles = "finance,super_admin")] [Authorize] // read (GetAll) is open to any authenticated user — the member self-service
// reimbursement form needs the category list. Write actions are finance-only below.
public class ExpenseCategoriesController : ControllerBase public class ExpenseCategoriesController : ControllerBase
{ {
private readonly IExpenseCategoryService _svc; private readonly IExpenseCategoryService _svc;
@@ -18,26 +19,32 @@ public class ExpenseCategoriesController : ControllerBase
=> Ok(await _svc.GetAllAsync(includeInactive)); => Ok(await _svc.GetAllAsync(includeInactive));
[HttpPost("groups")] [HttpPost("groups")]
[Authorize(Roles = "finance,super_admin")]
public async Task<IActionResult> CreateGroup([FromBody] CreateExpenseGroupRequest r) public async Task<IActionResult> CreateGroup([FromBody] CreateExpenseGroupRequest r)
=> Ok(new { id = await _svc.CreateGroupAsync(r) }); => Ok(new { id = await _svc.CreateGroupAsync(r) });
[HttpPut("groups/{id:int}")] [HttpPut("groups/{id:int}")]
[Authorize(Roles = "finance,super_admin")]
public async Task<IActionResult> UpdateGroup(int id, [FromBody] UpdateExpenseGroupRequest r) public async Task<IActionResult> UpdateGroup(int id, [FromBody] UpdateExpenseGroupRequest r)
{ try { await _svc.UpdateGroupAsync(id, r); return NoContent(); } catch (KeyNotFoundException) { return NotFound(); } } { try { await _svc.UpdateGroupAsync(id, r); return NoContent(); } catch (KeyNotFoundException) { return NotFound(); } }
[HttpDelete("groups/{id:int}")] [HttpDelete("groups/{id:int}")]
[Authorize(Roles = "finance,super_admin")]
public async Task<IActionResult> DeactivateGroup(int id) public async Task<IActionResult> DeactivateGroup(int id)
{ try { await _svc.DeactivateGroupAsync(id); return NoContent(); } catch (KeyNotFoundException) { return NotFound(); } } { try { await _svc.DeactivateGroupAsync(id); return NoContent(); } catch (KeyNotFoundException) { return NotFound(); } }
[HttpPost("subcategories")] [HttpPost("subcategories")]
[Authorize(Roles = "finance,super_admin")]
public async Task<IActionResult> CreateSub([FromBody] CreateExpenseSubCategoryRequest r) public async Task<IActionResult> CreateSub([FromBody] CreateExpenseSubCategoryRequest r)
{ try { return Ok(new { id = await _svc.CreateSubCategoryAsync(r) }); } catch (KeyNotFoundException) { return NotFound(); } } { try { return Ok(new { id = await _svc.CreateSubCategoryAsync(r) }); } catch (KeyNotFoundException) { return NotFound(); } }
[HttpPut("subcategories/{id:int}")] [HttpPut("subcategories/{id:int}")]
[Authorize(Roles = "finance,super_admin")]
public async Task<IActionResult> UpdateSub(int id, [FromBody] UpdateExpenseSubCategoryRequest r) public async Task<IActionResult> UpdateSub(int id, [FromBody] UpdateExpenseSubCategoryRequest r)
{ try { await _svc.UpdateSubCategoryAsync(id, r); return NoContent(); } catch (KeyNotFoundException) { return NotFound(); } } { try { await _svc.UpdateSubCategoryAsync(id, r); return NoContent(); } catch (KeyNotFoundException) { return NotFound(); } }
[HttpDelete("subcategories/{id:int}")] [HttpDelete("subcategories/{id:int}")]
[Authorize(Roles = "finance,super_admin")]
public async Task<IActionResult> DeactivateSub(int id) public async Task<IActionResult> DeactivateSub(int id)
{ try { await _svc.DeactivateSubCategoryAsync(id); return NoContent(); } catch (KeyNotFoundException) { return NotFound(); } } { try { await _svc.DeactivateSubCategoryAsync(id); return NoContent(); } catch (KeyNotFoundException) { return NotFound(); } }
} }
@@ -51,7 +51,7 @@ public class MonthlyStatementService : IMonthlyStatementService
public async Task UpdateAsync(int id, UpdateMonthlyStatementRequest r) public async Task UpdateAsync(int id, UpdateMonthlyStatementRequest r)
{ {
var s = await _db.MonthlyStatements.FindAsync(id) var s = await _db.MonthlyStatements.FirstOrDefaultAsync(x => x.Id == id)
?? throw new KeyNotFoundException($"MonthlyStatement {id} not found."); ?? throw new KeyNotFoundException($"MonthlyStatement {id} not found.");
if (s.IsFinalized) throw new InvalidOperationException("Statement is finalized and cannot be modified."); if (s.IsFinalized) throw new InvalidOperationException("Statement is finalized and cannot be modified.");
s.OpeningBalance = r.OpeningBalance; s.TotalOtherIncome = r.TotalOtherIncome; s.OpeningBalance = r.OpeningBalance; s.TotalOtherIncome = r.TotalOtherIncome;
@@ -62,7 +62,7 @@ public class MonthlyStatementService : IMonthlyStatementService
public async Task FinalizeAsync(int id) public async Task FinalizeAsync(int id)
{ {
var s = await _db.MonthlyStatements.FindAsync(id) var s = await _db.MonthlyStatements.FirstOrDefaultAsync(x => x.Id == id)
?? throw new KeyNotFoundException($"MonthlyStatement {id} not found."); ?? throw new KeyNotFoundException($"MonthlyStatement {id} not found.");
s.IsFinalized = true; s.FinalizedAt = DateTimeOffset.UtcNow; s.FinalizedBy = CurrentUserId; s.IsFinalized = true; s.FinalizedAt = DateTimeOffset.UtcNow; s.FinalizedBy = CurrentUserId;
await _db.SaveChangesAsync(); await _db.SaveChangesAsync();