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