# 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(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%)