Migrate

microservices/docs/audit/backend.md

@@ -0,0 +1,283 @@
+# Backend Audit Report — GoodGo POS System
+
+> **Date**: 2026-03-20
+> **Auditor**: Senior Backend Engineer (Agent 98eaaecb)
+> **Scope**: .NET service implementation, CQRS patterns, MediatR usage, EF Core, domain model
+> **Branch**: master (d0211e5)
+
+---
+
+## Executive Summary
+
+Platform có 26 .NET 10 microservices với kiến trúc Clean Architecture + CQRS nhất quán và chất lượng code cao. Naming conventions, domain patterns, và bilingual documentation được áp dụng đồng bộ trên tất cả services. **Tuy nhiên, 3 vấn đề critical về security và 4 vấn đề high về consistency cần xử lý trước khi deploy staging.** Điểm nổi bật nhất là debug endpoints không có auth (privilege escalation) và SQL injection tồn tại trong merchant-service-net — cả hai đều là BLOCKER.
+
+---
+
+## Critical Issues
+
+### C1 — Debug Endpoints `[AllowAnonymous]` trong MerchantService
+
+**File**: `services/merchant-service-net/src/MerchantService.API/Controllers/StaffController.cs:249–390`
+**Severity**: BLOCKER — bất kỳ ai cũng có thể gọi
+
+5 endpoints không có authentication:
+- `POST /api/v1/staff/debug/seed` (line 249) — tạo staff data tùy ý
+- `POST /api/v1/staff/debug/update-userid` (line 340) — ghi đè userId của bất kỳ staff qua reflection
+- `POST /api/v1/staff/debug/update-merchant` (line 360) — ghi đè merchantId tùy ý
+
+Đây là **privilege escalation + data tampering vulnerability** cực kỳ nghiêm trọng. Bất kỳ attacker nào cũng có thể leo thang quyền hoặc phá vỡ dữ liệu merchant trong production.
+
+**Fix ngay**: Xóa hoàn toàn các debug endpoints này khỏi production code, hoặc wrap trong `if (env.IsDevelopment())` và thêm `[Authorize(Roles = "SuperAdmin")]`.
+
+---
+
+### C2 — SQL Injection trong StaffController
+
+**File**: `services/merchant-service-net/src/MerchantService.API/Controllers/StaffController.cs:307, 367`
+**Severity**: HIGH — string interpolation trực tiếp vào SQL
+
+```csharp
+// Line 307
+cmd.CommandText = $"SELECT id FROM shops WHERE merchant_id = '{merchant.Id}' LIMIT 1";
+
+// Line 367 — userId, merchantId từ [FromQuery] — SQL INJECTION RISK
+cmd.CommandText = $"UPDATE merchants SET user_id = '{userId}' WHERE id = '{merchantId}'";
+```
+
+Dù `Guid` tự sanitize ở thời điểm hiện tại, pattern string interpolation trong SQL là vi phạm security baseline. Bất kỳ refactor nào đổi type sang string sẽ tức thì trở thành lỗ hổng.
+
+**Fix**: Dùng parameterized queries:
+```csharp
+cmd.CommandText = "UPDATE merchants SET user_id = @userId WHERE id = @merchantId";
+cmd.Parameters.AddWithValue("@userId", userId);
+cmd.Parameters.AddWithValue("@merchantId", merchantId);
+```
+
+---
+
+### C3 — JWT Signature Validation Bị Tắt trong Development — 4 Services
+
+**Files** (confirmed bằng grep toàn repo):
+- `services/merchant-service-net/src/MerchantService.API/Program.cs`
+- `services/ads-serving-service-net/src/AdsServingService.API/Program.cs`
+- `services/ads-tracking-service-net/src/AdsTrackingService.API/Program.cs`
+- `services/ads-billing-service-net/src/AdsBillingService.API/Program.cs`
+
+**Severity**: HIGH — token forgery risk nếu `ASPNETCORE_ENVIRONMENT` bị misconfigure
+
+```csharp
+// Pattern này tồn tại trong cả 4 services
+ValidateIssuerSigningKey = builder.Environment.IsDevelopment() ? false : true,
+SignatureValidator = builder.Environment.IsDevelopment() ? ... : null,
+```
+
+Nếu `ASPNETCORE_ENVIRONMENT` không được set đúng trên server (hoặc bị override sai), tất cả JWT signatures đều bị bỏ qua — bất kỳ token tự tạo nào đều được chấp nhận. Nguy hiểm đặc biệt với `ads-billing-service-net` (xử lý billing).
+
+> **Note**: `iam-service-net/tests/CustomWebApplicationFactory.cs` cũng có `ValidateIssuerSigningKey = false` nhưng đây là test context — chấp nhận được.
+
+**Fix**: Dùng test signing key trong development thay vì skip validation hoàn toàn:
+```csharp
+ValidateIssuerSigningKey = true, // Luôn validate — kể cả development
+// Development: dùng shared test key từ env var DEV_JWT_SIGNING_KEY
+```
+
+---
+
+## Warnings
+
+### W1 — CORS Policy `AllowAnyOrigin()` trên tất cả Services
+
+**File**: Mọi service `Program.cs`, ví dụ `services/_template_dot_net/src/MyService.API/Program.cs:93`
+
+```csharp
+policy.AllowAnyOrigin().AllowAnyMethod().AllowAnyHeader();
+```
+
+Tất cả 26 services đều cấu hình CORS wildcard. Trong production, điều này cho phép bất kỳ domain nào gọi API — vi phạm same-origin policy intent.
+
+**Fix**: Restrict origins trong production environment:
+```csharp
+if (env.IsProduction())
+    policy.WithOrigins("https://goodgo.vn", "https://admin.goodgo.vn")
+          .WithMethods("GET", "POST", "PUT", "DELETE", "PATCH")
+          .WithHeaders("Content-Type", "Authorization");
+else
+    policy.AllowAnyOrigin().AllowAnyMethod().AllowAnyHeader();
+```
+
+---
+
+### W2 — Idempotency Chỉ Có ở 3/26 Services
+
+**Services với idempotency**: order-service-net, inventory-service-net, ads-serving-service-net
+**Services thiếu**: wallet-service-net, booking-service-net, promotion-service-net, và 19 services khác
+
+Các write operations trong wallet, booking, promotion có nguy cơ cao bị duplicate execution (network retry, user double-click). Thiếu `IRequestManager` idempotency cho phép double-charge, double-booking.
+
+**Fix**: Implement `IRequestManager` pattern (đã có trong `_template_dot_net`) cho tất cả write-heavy services:
+- Ưu tiên cao nhất: `wallet-service-net`, `booking-service-net`
+- Ưu tiên trung bình: `promotion-service-net`, `membership-service-net`
+
+---
+
+### W3 — API Error Response Format Không Nhất Quán
+
+**File**: `services/order-service-net/src/OrderService.API/Controllers/OrdersController.cs:73`
+
+Một số controllers trả về raw message object thay vì format chuẩn:
+```csharp
+// Inconsistent — dùng ở một số controllers
+return NotFound(new { Message = "Order not found" });
+
+// Standard theo CLAUDE.md — cần dùng nhất quán
+return NotFound(new { success = false, error = new { code = "ORDER_NOT_FOUND", message = "..." } });
+```
+
+**Fix**: Đồng nhất tất cả error response theo format: `{ success: false, error: { code, message } }`. Tốt nhất dùng exception middleware để map `DomainException` → ProblemDetails tự động, tránh manual error construction trong controllers.
+
+---
+
+### W4 — ProblemDetails Exception Mapping Không Đầy Đủ ở Một Số Services
+
+**Reference (tốt)**: `services/iam-service-net/src/IamService.API/Program.cs` — map 5+ exception types
+**Template thiếu**: `services/_template_dot_net/src/MyService.API/Program.cs` — chỉ map cơ bản
+
+Các services mới tạo từ template sẽ không có exception mapping đầy đủ, dẫn đến stack trace leak hoặc generic 500 errors.
+
+**Fix**: Cập nhật `_template_dot_net` với exception mappings đầy đủ. Sau đó sync cho các services đã tạo từ template.
+
+---
+
+### W5 — HttpContextAccessor Inject Trực Tiếp vào Handler
+
+**File**: `services/merchant-service-net/src/MerchantService.API/Program.cs:31`
+
+```csharp
+builder.Services.AddHttpContextAccessor(); // OK
+// Nhưng một số handlers inject IHttpContextAccessor trực tiếp
+```
+
+Truy cập HttpContext trong MediatR handler vi phạm clean boundary giữa application layer và web layer. Handler không nên biết về HTTP context.
+
+**Fix**: Inject contextual data (userId, tenantId) vào Command/Query parameter khi dispatch từ Controller:
+```csharp
+// Controller
+var userId = User.FindFirstValue(ClaimTypes.NameIdentifier);
+await _mediator.Send(new CreateOrderCommand(userId, shopId, items), cancellationToken);
+```
+
+---
+
+## Improvements
+
+### I1 — Outbox Pattern cho Domain Events
+
+**Hiện tại**: Domain events được dispatch in-process trong `SaveEntitiesAsync()` trước SaveChanges. Nếu process crash sau SaveChanges, events đã bị lost.
+
+**Recommendation**: Implement transactional outbox pattern — persist events vào `outbox_messages` table trong cùng transaction, sau đó poll/relay sang RabbitMQ. Tham khảo: MassTransit Outbox, Wolverine Outbox.
+
+**Impact**: Đảm bảo at-least-once delivery cho cross-service events — quan trọng cho wallet-service, booking-service, promotion-service.
+
+---
+
+### I2 — Missing OpenAPI Specs (openapi.yaml)
+
+Theo `CLAUDE.md`, mỗi service PHẢI có `openapi.yaml`. Hiện tại chỉ có Swagger UI từ Swashbuckle (runtime generated), không có static `openapi.yaml` file được commit vào repo.
+
+**Impact**: Không thể dùng API-first contract testing (Pact), không có versioning cho API spec.
+
+**Fix**: Thêm script generate `openapi.yaml` vào CI pipeline:
+```bash
+dotnet swagger tofile --output openapi.yaml ./bin/Release/net10.0/ServiceName.API.dll v1
+```
+
+---
+
+### I3 — Missing Prometheus Metrics Endpoint
+
+Theo `CLAUDE.md`, mỗi service PHẢI có `/metrics` endpoint cho Prometheus. Chưa thấy `prometheus-net` hoặc `OpenTelemetry.Exporter.Prometheus.AspNetCore` được configure trong services.
+
+**Fix**: Thêm vào `_template_dot_net` và sync:
+```csharp
+builder.Services.AddOpenTelemetry()
+    .WithMetrics(metrics => metrics
+        .AddAspNetCoreInstrumentation()
+        .AddPrometheusExporter());
+
+app.MapPrometheusScrapingEndpoint("/metrics");
+```
+
+---
+
+### I4 — Saga Pattern cho Cross-Service Transactions
+
+Hiện tại order placement phụ thuộc vào nhiều services (inventory, wallet, promotion) nhưng không có orchestration nếu một service fail mid-flow. Tình huống: wallet bị trừ nhưng inventory deduct fail → inconsistent state.
+
+**Recommendation**: Implement Saga orchestration (dùng MassTransit Saga hoặc Wolverine) cho Order Placement flow. Đây là medium-term investment nhưng cần thiết trước khi scale.
+
+---
+
+### I5 — Dapper Queries Không Có Timeout Explicit
+
+Một số Dapper queries (read-heavy) không set `commandTimeout`, dùng default 30s của PostgreSQL. Với queries phức tạp trên dataset lớn, điều này có thể block connection pool.
+
+**Fix**: Set explicit timeout cho Dapper queries:
+```csharp
+var result = await _connection.QueryAsync<T>(sql, parameters, commandTimeout: 5);
+```
+
+---
+
+## Action Items (Prioritized)
+
+| # | Priority | Action | Owner | File/Service |
+|---|----------|--------|-------|-------------|
+| 1 | **P0 BLOCKER** | Xóa debug endpoints `[AllowAnonymous]` khỏi StaffController | Backend Dev | `merchant-service-net/StaffController.cs:249–390` |
+| 2 | **P0 BLOCKER** | Fix SQL injection — dùng parameterized queries | Backend Dev | `merchant-service-net/StaffController.cs:307, 367` |
+| 3 | **P0 HIGH** | Fix JWT signature validation — không skip trong development | Backend Dev | `merchant-service-net/Program.cs:91` |
+| 4 | **P1 HIGH** | Restrict CORS origins trong production environment | Backend Dev | Tất cả services `Program.cs` |
+| 5 | **P1 HIGH** | Implement idempotency cho wallet-service, booking-service | Backend Dev | `wallet-service-net`, `booking-service-net` |
+| 6 | **P1 HIGH** | Đồng nhất API error response format (`{ success, error: { code, message } }`) | Backend Dev | Tất cả controllers |
+| 7 | **P2 MEDIUM** | Cập nhật `_template_dot_net` với đầy đủ ProblemDetails exception mapping | Backend Dev | `_template_dot_net/Program.cs` |
+| 8 | **P2 MEDIUM** | Refactor HttpContextAccessor ra khỏi handlers — inject qua Command params | Backend Dev | `merchant-service-net` handlers |
+| 9 | **P2 MEDIUM** | Thêm `/metrics` Prometheus endpoint vào tất cả services | Backend Dev | Tất cả `Program.cs` |
+| 10 | **P2 MEDIUM** | Generate và commit `openapi.yaml` trong CI pipeline | DevOps/Backend | CI `.github/workflows` |
+| 11 | **P3 LOW** | Set explicit Dapper query timeout | Backend Dev | Tất cả services dùng Dapper |
+| 12 | **P3 LOW** | Implement Outbox pattern cho domain events | Backend Dev | Tất cả services có cross-service events |
+| 13 | **P3 LOW** | Implement Saga orchestration cho Order Placement flow | Backend Dev | `order-service-net`, `wallet-service-net`, `inventory-service-net` |
+
+---
+
+## Strengths
+
+Những patterns đang được áp dụng tốt — nên duy trì:
+
+- **Bilingual codebase**: Tất cả XML docs, comments, log messages có EN + VI — nhất quán 100%
+- **Domain encapsulation**: Private fields + read-only properties, zero public setters — đúng chuẩn DDD
+- **MediatR pipeline**: 3 behaviors (Logging → Validation → Transaction) áp dụng nhất quán 26/26 services
+- **Entity patterns**: Aggregate roots, domain events, behavior methods — clean và testable
+- **EF Core config**: snake_case columns, private field mapping, owned collections — đúng theo quy chuẩn
+- **FluentValidation**: Bilingual messages, child validation, enum validation — đầy đủ
+- **Container security**: Multi-stage Dockerfile, non-root user (dotnetuser:1001), healthcheck — production-ready
+- **Test structure**: 50 test projects (25 unit + 25 functional), WebApplicationFactory pattern đúng
+- **C# 14 / .NET 10**: Nullable enabled, TreatWarningsAsErrors=true, implicit usings — type-safe
+- **Health checks**: `/health`, `/health/live`, `/health/ready` với PostgreSQL probe — đúng chuẩn Kubernetes
+- **API versioning**: URL segment + Header reader, consistent v1.0 default — RESTful
+- **Repository pattern**: Interface trong Domain, implementation trong Infrastructure — dependency inversion đúng
+
+---
+
+## Services Examined
+
+| Service | Detail Level | Key Findings |
+|---------|-------------|--------------|
+| `order-service-net` | Deep | CQRS, entity, EF patterns — reference quality |
+| `iam-service-net` | Deep | Authorization, exception handling — best ProblemDetails mapping |
+| `merchant-service-net` | Deep | **C1 debug endpoints, C2 SQL injection, C3 JWT validation** |
+| `fnb-engine-net` | Medium | Validation patterns — good use of RuleForEach |
+| `inventory-service-net` | Medium | Idempotency pattern — only one of 3 services with IRequestManager |
+| `wallet-service-net` | Medium | Repository implementation, Dapper usage |
+| `_template_dot_net` | Deep | Reference architecture — needs update (W4, I2, I3) |
+| Other 19 services | Structural | Verified pattern consistency, no deep code review |
+
+**Coverage**: 7 services deep-reviewed (27%), 19 services structural verification (73%)