diff --git a/docs/gotchas.md b/docs/gotchas.md index 1065b43..108a645 100644 --- a/docs/gotchas.md +++ b/docs/gotchas.md @@ -1348,6 +1348,14 @@ for h in resp.points: # ← .points không phải iterable trực tiếp **Bài học:** derived-invariant phải re-establish ở **MỌI mutation đổi tập-nguồn — kể cả DELETE + cascade-delete**, không chỉ write/select. **Enumeration writer trong spec KHÔNG đáng tin** — grep độc-lập MỌI `.Remove(` chạm entity chứa field-nguồn (S87/S88 cardinality class mở rộng lên tầng mutation-path: writer-miss ≡ read-site-miss). Chỉ **verify adversarial-refute** bắt (invest/review/implement đều sót vì cùng theo enumeration). Liên-quan #72/#73 (cardinality) + `feedback_cardinality_change_grep_consumers`. +### 82. Menu-flag permission grant ≠ quyền API thật — controller gate GET bằng `[Authorize]` trần (Session 118) + +**Triệu chứng:** S118 phân quyền role Procurement — grant menu `Suppliers` (R+C+U) tưởng "menu hiện = làm được mọi thứ". THỰC TẾ 2 chiều lệch: (1) `PUT/DELETE /suppliers` gate `[Authorize(Roles="Admin,CatalogManager")]` → PRO vẫn **403 khi Sửa/Xóa NCC** dù CanUpdate=1 (chỉ Publish/Import theo policy `Suppliers.Update` là ăn); nút Sửa hiện nhưng bấm lỗi. (2) Ngược lại `ReportsController` chỉ `[Authorize]` **trần (KHÔNG policy)** → menu bị S92 ẩn nhưng **MỌI user đăng nhập** vẫn gọi `GET /reports/dashboard` lấy tổng giá trị HĐ + top NCC/dự án theo giá trị (gõ URL/API trực tiếp, bỏ qua menu). + +**Cơ chế:** Menu-tree (`GetMyMenuTree`) filter theo Permission `CanRead` → CHỈ đổi HIỂN THỊ menu FE. Hiệu-lực API THẬT chỉ ở endpoint mang `[Authorize(Policy="X.Y")]` (map qua `MenuPermissionHandler`). Nhiều controller master gate GET bằng `[Authorize]` trần (mở mọi authed user, chủ ý S59) + gate write bằng `[Authorize(Roles=...)]` (KHÔNG theo menu-key). Nên grant "R/C/U" phần lớn = FE-button-visibility; delta API thật ≪ mong đợi. + +**Fix:** (1) Endpoint lộ data nhạy → gate `[Authorize(Policy="Reports.Read")]` per-method (S118 `dashboard` + `contracts/export`; `my-dashboard` giữ mở vì scope theo `currentUser`). (2) Quy tắc: TRƯỚC khi tin hiệu-lực 1 grant → **grep authz-attribute của CHÍNH controller đích** (`[Authorize(Policy=...)]` vs `[Authorize(Roles=...)]` vs `[Authorize]` trần). **Bài học:** grant permission = 2 tầng ĐỘC-LẬP: menu-display (Permission table) ⟂ API-authz (controller attribute) — verify CẢ HAI. reviewer (adversarial) bắt được vì grep authz thật; em-main-solo tin "menu-flag=quyền" thì sót. Liên-quan #44 (silent 403). + --- ## Checklist debug bug mới diff --git a/src/Backend/SolutionErp.Api/Controllers/ReportsController.cs b/src/Backend/SolutionErp.Api/Controllers/ReportsController.cs index e832999..e24bd4f 100644 --- a/src/Backend/SolutionErp.Api/Controllers/ReportsController.cs +++ b/src/Backend/SolutionErp.Api/Controllers/ReportsController.cs @@ -14,15 +14,24 @@ namespace SolutionErp.Api.Controllers; [Authorize] public class ReportsController(IMediator mediator) : ControllerBase { + // [S118 — reviewer catch] Báo cáo TOÀN CÔNG TY (tổng giá trị HĐ, top NCC/dự án theo giá trị, + // xu hướng 12 tháng) = nhạy cảm → gate Reports.Read (owner chốt: chỉ Admin, khớp S92 "HĐ chỉ + // Admin thấy"). Hiện chỉ Admin có Reports.Read → non-admin 403. Mở cho role khác = cấp Reports.Read + // qua ma trận (không sửa code). Trước S118: chỉ [Authorize] trần → mọi user đăng nhập gọi được. [HttpGet("dashboard")] + [Authorize(Policy = "Reports.Read")] public async Task> Dashboard(CancellationToken ct) => Ok(await mediator.Send(new GetDashboardStatsQuery(), ct)); + // GIỮ MỞ (chỉ [Authorize] class-level): my-dashboard scope theo currentUser.UserId — mỗi user + // chỉ thấy số của CHÍNH MÌNH (draft/pending/SLA/giá trị draft của mình). Không lộ toàn công ty. [HttpGet("my-dashboard")] public async Task> MyDashboard(CancellationToken ct) => Ok(await mediator.Send(new GetMyDashboardQuery(), ct)); + // [S118 — reviewer catch] Xuất TOÀN BỘ HĐ ra Excel = nhạy cảm → gate Reports.Read (chỉ Admin, S92). [HttpGet("contracts/export")] + [Authorize(Policy = "Reports.Read")] public async Task ExportContracts( [FromQuery] ContractPhase? phase, [FromQuery] Guid? supplierId, diff --git a/tests/SolutionErp.Infrastructure.Tests/Api/AuthorizePolicyRegressionTests.cs b/tests/SolutionErp.Infrastructure.Tests/Api/AuthorizePolicyRegressionTests.cs index fd80e66..d713d80 100644 --- a/tests/SolutionErp.Infrastructure.Tests/Api/AuthorizePolicyRegressionTests.cs +++ b/tests/SolutionErp.Infrastructure.Tests/Api/AuthorizePolicyRegressionTests.cs @@ -200,4 +200,71 @@ public class AuthorizePolicyRegressionTests attr.Should().NotBeNull("POST satellite CreateWorkHistory phải có action-level policy"); attr!.Policy.Should().Be("Hrm_HoSo.Create"); } + + // =================================================================== + // S118 (2026-07-14) — Reviewer catch: ReportsController lỗ hổng lộ tài chính toàn công ty. + // + // Trước S118: cả controller chỉ [Authorize] class-level trần → MỌI user đăng nhập gọi được + // GET /reports/dashboard (tổng giá trị HĐ, top NCC/dự án theo giá trị, xu hướng 12 tháng) + + // GET /reports/contracts/export (xuất TOÀN BỘ HĐ ra Excel) → lộ tài chính HĐ toàn công ty. + // + // Fix (owner chốt, khớp S92 "HĐ chỉ Admin thấy"): 2 endpoint company-wide gate Policy + // "Reports.Read" (hiện chỉ Admin có) — mở role khác = cấp qua ma trận, KHÔNG sửa code. + // my-dashboard CỐ Ý giữ mở (scope theo currentUser — mỗi user chỉ thấy số của chính mình). + // + // Regression coverage: nếu ai gỡ Policy khỏi 2 endpoint company-wide → tái lộ tài chính; + // hoặc thêm nhầm Policy vào my-dashboard → khóa dashboard CÁ NHÂN của role không-Admin. + // Cả 2 hướng đều FAIL test ngay, không cần UAT reproduce. + // =================================================================== + + [Fact] + public void ReportsController_ClassLevel_AuthorizeOnly_NoPolicy_NoRoles() + { + // Class-level [Authorize] trần (any-authenticated) — gate company-wide đặt PER-ACTION. + // Nếu hardcode Policy/Roles ở class-level sẽ khóa luôn my-dashboard cá nhân (gotcha #44). + var attr = GetClassLevelAuthorize(typeof(ReportsController)); + + attr.Should().NotBeNull("ReportsController phải có [Authorize] class-level chặn anonymous"); + attr!.Policy.Should().BeNull( + "class-level KHÔNG được hardcode Policy — my-dashboard cần mở cho mọi authenticated. " + + "Gate company-wide đặt action-level (Reports.Read)."); + attr.Roles.Should().BeNull("class-level KHÔNG được hardcode Roles"); + } + + [Fact] + public void ReportsController_Dashboard_GET_RequiresReportsReadPolicy() + { + // GET /reports/dashboard = báo cáo TOÀN CÔNG TY → phải gate Reports.Read (chỉ Admin, S118). + var attr = GetActionAuthorize(typeof(ReportsController), nameof(ReportsController.Dashboard)); + + attr.Should().NotBeNull("GET dashboard (báo cáo toàn công ty) phải có [Authorize(Policy = ...)]"); + attr!.Policy.Should().Be("Reports.Read", + "dashboard company-wide (tổng giá trị HĐ, top NCC/dự án, xu hướng 12 tháng) chỉ Admin — " + + "gate Reports.Read (S118 vá lỗ hổng lộ tài chính). Gỡ Policy = tái lộ toàn công ty."); + } + + [Fact] + public void ReportsController_ExportContracts_GET_RequiresReportsReadPolicy() + { + // GET /reports/contracts/export = xuất TOÀN BỘ HĐ ra Excel → gate Reports.Read (chỉ Admin, S118). + var attr = GetActionAuthorize(typeof(ReportsController), nameof(ReportsController.ExportContracts)); + + attr.Should().NotBeNull("GET contracts/export (xuất toàn bộ HĐ) phải có [Authorize(Policy = ...)]"); + attr!.Policy.Should().Be("Reports.Read", + "export toàn bộ HĐ ra Excel = nhạy cảm → gate Reports.Read chỉ Admin (S118)."); + } + + [Fact] + public void ReportsController_MyDashboard_GET_StaysOpen_NoActionPolicy() + { + // GET /reports/my-dashboard CỐ Ý giữ mở (chỉ inherit class-level [Authorize] any-authenticated) + // — scope theo currentUser.UserId, mỗi user chỉ thấy số của CHÍNH MÌNH, không lộ toàn công ty. + // Nếu ai thêm [Authorize(Policy=...)] action-level → khóa nhầm dashboard CÁ NHÂN của mọi role + // không có Reports.Read (403 silent). Cho phép bare [Authorize] (vô hại) nhưng CHẶN mọi Policy. + var attr = GetActionAuthorize(typeof(ReportsController), nameof(ReportsController.MyDashboard)); + + (attr?.Policy).Should().BeNull( + "GET my-dashboard KHÔNG được gate Policy — mỗi user chỉ thấy số của chính mình. " + + "Thêm policy sẽ khóa dashboard cá nhân của role không có Reports.Read (S118 giữ mở có chủ đích)."); + } }