256 lines
12 KiB
Markdown
256 lines
12 KiB
Markdown
# API Architecture Audit — GoodGo POS System
|
|
|
|
**Auditor:** API Architect
|
|
**Date:** 2026-03-20
|
|
**Scope:** API design consistency, OpenAPI specs, versioning, Traefik routing, endpoint conventions
|
|
**Services Audited:** 26 microservices, 121 controllers
|
|
|
|
---
|
|
|
|
## Executive Summary
|
|
|
|
The GoodGo platform has a solid architectural foundation rooted in Clean Architecture and CQRS, but the API layer suffers from **fragmented versioning, inconsistent response formats, and incomplete gateway routing**. The IAM service sets a high standard that the majority of other services have not followed. These gaps create compounding developer experience issues: the POS frontend already compensates with a "4-format smart deserializer" — a symptom that the API contract is not being enforced. Fixing response format and versioning consistency is the highest-leverage action available.
|
|
|
|
---
|
|
|
|
## Critical Issues
|
|
|
|
### CRIT-1: Response Format Fragmentation
|
|
|
|
Three incompatible response patterns exist across the platform, making contract-first SDK generation impossible and forcing clients to implement defensive deserialization.
|
|
|
|
**Pattern A — IAM Service (correct):**
|
|
```json
|
|
{ "success": true, "data": { ... } }
|
|
{ "success": false, "error": { "code": "INVALID_TOKEN", "message": "...", "details": { ... } } }
|
|
```
|
|
Source: `services/iam-service-net/src/IamService.API/Application/Common/ApiResponse.cs`
|
|
|
|
**Pattern B — Order / Catalog / Template (partially correct):**
|
|
```json
|
|
{ "success": true, "data": { ... } }
|
|
{ "success": false, "error": { "code": "...", "message": "..." } }
|
|
```
|
|
Using anonymous objects rather than typed `ApiResponse<T>`.
|
|
- `services/order-service-net/src/OrderService.API/Controllers/OrdersController.cs:136,139`
|
|
- `services/_template_dot_net/src/MyService.API/Controllers/SamplesController.cs:38-87`
|
|
|
|
**Pattern C — Merchant Service (broken):**
|
|
```json
|
|
{ ... } // raw entity, no wrapper
|
|
{ "message": "Merchant profile not found..." } // no success/error structure
|
|
```
|
|
Source: `services/merchant-service-net/src/MerchantService.API/Controllers/MerchantsController.cs:44,46`
|
|
|
|
**Impact:** Frontend `PosDataService` implements 4-format deserialization to compensate. SDK auto-generation (openapi-generator/orval) is blocked. Error codes are not surfaced to the UI consistently.
|
|
|
|
---
|
|
|
|
### CRIT-2: API Versioning Inconsistency
|
|
|
|
Three versioning patterns found across the platform. Only IAM and Catalog services are version-future-proof.
|
|
|
|
**Pattern A — `Asp.Versioning` with dynamic URL + header (correct):**
|
|
```csharp
|
|
[ApiVersion("1.0")]
|
|
[Route("api/v{version:apiVersion}/auth")]
|
|
```
|
|
Services: `iam-service-net`, `catalog-service-net`, `storage-service-net`
|
|
Config: `services/iam-service-net/src/IamService.API/Program.cs:46-59`
|
|
|
|
**Pattern B — Hardcoded `api/v1` without `[ApiVersion]` attribute:**
|
|
```csharp
|
|
[Route("api/v1/merchants")] // no version negotiation
|
|
```
|
|
Services: `merchant-service-net`, `order-service-net`, `inventory-service-net`, `wallet-service-net`, `booking-service-net`
|
|
- `services/merchant-service-net/src/MerchantService.API/Controllers/MerchantsController.cs:18-19`
|
|
- `services/order-service-net/src/OrderService.API/Controllers/OrdersController.cs:17-18`
|
|
- `services/inventory-service-net/src/InventoryService.API/Controllers/InventoryController.cs:18`
|
|
|
|
**Pattern C — No version at all:**
|
|
```csharp
|
|
[Route("api/[controller]")]
|
|
```
|
|
Services: `chat-service-net`, some `fnb-engine-net` controllers
|
|
- `services/chat-service-net/src/ChatService.API/Controllers/MessagesController.cs:14`
|
|
|
|
**Impact:** No backward-compatibility guarantee. A v2 rollout requires touching all clients simultaneously. `X-Api-Version` header negotiation only works for Pattern A services.
|
|
|
|
---
|
|
|
|
### CRIT-3: Missing Traefik Gateway Routes
|
|
|
|
11 services (out of 26) have no entry in `infra/traefik/dynamic/routes.yml`. These services are either inaccessible from outside the Docker network or require clients to hardcode internal URLs — both are unacceptable.
|
|
|
|
**Missing services:**
|
|
- `chat-service-net` (SignalR `/hubs/chat` also not routed)
|
|
- `promotion-service-net`
|
|
- `social-service-net`
|
|
- `mining-service-net`
|
|
- `mission-service-net`
|
|
- `ads-manager-service-net`
|
|
- `ads-serving-service-net`
|
|
- `ads-tracking-service-net`
|
|
- `ads-billing-service-net`
|
|
- `ads-analytics-service-net`
|
|
- `mkt-facebook-service-net`, `mkt-whatsapp-service-net`, `mkt-x-service-net`, `mkt-zalo-service-net`
|
|
|
|
**Note:** The `CLAUDE.md` lists these services with full implementation. Their absence from the gateway config suggests the routes file is out of sync with actual deployment.
|
|
|
|
---
|
|
|
|
## Warnings
|
|
|
|
### WARN-1: OpenAPI Spec Coverage is Incomplete
|
|
|
|
No `.yaml` or `.json` OpenAPI spec files exist in the repository. Swagger UI is configured on some services but not all:
|
|
|
|
| Service | Swagger Configured |
|
|
|---|---|
|
|
| `iam-service-net` | ✅ Full (with SwaggerTag, SwaggerOperation, SwaggerResponse) |
|
|
| `order-service-net` | ✅ Basic |
|
|
| `catalog-service-net` | ✅ With versioning support |
|
|
| `merchant-service-net` | ❌ Not found |
|
|
| `inventory-service-net` | ❌ Not found |
|
|
| `chat-service-net` | ❌ Not found |
|
|
| Ads/Marketing services | ❌ Not found |
|
|
|
|
No central API documentation aggregator is configured. Traefik does not expose a unified Swagger UI. As a result, there is no way for frontend teams or external integrators to discover the full API surface without reading source code.
|
|
|
|
---
|
|
|
|
### WARN-2: Authorization Pattern Fragmentation
|
|
|
|
Three authorization patterns are in use with no consistent policy layer:
|
|
|
|
- **Attribute-based:** `[Authorize]` — checks token existence only
|
|
`services/merchant-service-net/.../MerchantsController.cs:20`
|
|
- **Role-based:** `[Authorize(Roles = "Admin,SuperAdmin")]`
|
|
`services/merchant-service-net/.../Admin/AdminMerchantsController.cs:18`
|
|
- **AllowAnonymous:** health endpoints
|
|
`services/wallet-service-net/.../HealthController.cs:13`
|
|
|
|
**No Policy-Based Authorization** (`[Authorize(Policy = "...")]`) is used anywhere. JWT scope validation is disabled in the IAM service configuration:
|
|
```csharp
|
|
ValidateAudience = false, // services/iam-service-net/src/IamService.API/Program.cs
|
|
```
|
|
|
|
This means a token issued for one service's scope is silently accepted by all services.
|
|
|
|
---
|
|
|
|
### WARN-3: Health Check Implementation is Non-Standard
|
|
|
|
Only `wallet-service-net` has an explicit `HealthController` with `/api/v1/health/live` and `/api/v1/health/ready`. The majority of services rely on implicit framework behavior and register health checks in `Program.cs` without exposing a documented endpoint.
|
|
|
|
The docker-compose healthcheck command (`curl -f http://localhost:8080/health/live`) works only if each service individually exposes that path — which is inconsistent.
|
|
|
|
No standardized health check response format exists. Kubernetes readiness/liveness probes in staging (`deployments/staging/kubernetes/`) point to `/health/ready` and `/health/live` — if a service doesn't expose these, probes will fail silently.
|
|
|
|
---
|
|
|
|
### WARN-4: SignalR Hub Route Fragmentation
|
|
|
|
The Order service registers a WebSocket hub (`/hubs/pos`) and includes query-string token handling:
|
|
```csharp
|
|
// services/order-service-net/src/OrderService.API/Program.cs:216-226
|
|
var accessToken = context.Request.Query["access_token"];
|
|
if (!string.IsNullOrEmpty(accessToken) && path.StartsWithSegments("/hubs"))
|
|
context.Token = accessToken;
|
|
```
|
|
|
|
Traefik routes `/hubs/pos` to the order-service with a dedicated `hub-ratelimit` middleware. However, `chat-service-net` likely has its own hub (`/hubs/chat`) which is **not routed through Traefik at all**.
|
|
|
|
CORS headers on the `hub-ratelimit` middleware also need to be verified — WebSocket upgrades require permissive CORS preflight handling.
|
|
|
|
---
|
|
|
|
### WARN-5: Rate Limit Middlewares Not Verified
|
|
|
|
`routes.yml` references three rate limit middleware names (`auth-ratelimit`, `api-ratelimit`, `payment-ratelimit`, `hub-ratelimit`) but the corresponding definitions in `infra/traefik/dynamic/middlewares.yml` were not verified to be complete or consistent with the referenced names. A typo or missing definition causes Traefik to silently drop the middleware.
|
|
|
|
---
|
|
|
|
## Improvements
|
|
|
|
### IMP-1: Extract `ApiResponse<T>` to Shared Package
|
|
|
|
Move `services/iam-service-net/src/IamService.API/Application/Common/ApiResponse.cs` to a NuGet package (`GoodGo.Shared.ApiResponse`) referenced by all services. This enforces uniform serialization at compile time.
|
|
|
|
```
|
|
packages/
|
|
GoodGo.Shared.ApiResponse/
|
|
ApiResponse.cs
|
|
ApiError.cs
|
|
PaginationInfo.cs
|
|
```
|
|
|
|
### IMP-2: Standardize API Versioning via `_template_dot_net`
|
|
|
|
Update `services/_template_dot_net/src/MyService.API/Program.cs` to include `Asp.Versioning` configuration as the reference default. All services should include the same versioning setup during scaffolding:
|
|
|
|
```csharp
|
|
builder.Services.AddApiVersioning(options => {
|
|
options.DefaultApiVersion = new ApiVersion(1, 0);
|
|
options.AssumeDefaultVersionWhenUnspecified = true;
|
|
options.ReportApiVersions = true;
|
|
options.ApiVersionReader = ApiVersionReader.Combine(
|
|
new UrlSegmentApiVersionReader(),
|
|
new HeaderApiVersionReader("X-Api-Version"));
|
|
});
|
|
```
|
|
|
|
### IMP-3: Add a HealthController to `_template_dot_net`
|
|
|
|
Create a standard `HealthController` in the template that all services inherit:
|
|
- `GET /health/live` — always returns 200 (container alive)
|
|
- `GET /health/ready` — checks PostgreSQL + Redis connections, returns 200/503
|
|
|
|
### IMP-4: OpenAPI Spec Export in CI/CD
|
|
|
|
Add a CI job that runs `dotnet swagger tofile` for each service and stores the output as a build artifact. This creates the foundation for:
|
|
- Client SDK generation via `orval` or `openapi-generator`
|
|
- Contract testing via `pact.js`
|
|
- Central API documentation via Redoc or Scalar
|
|
|
|
### IMP-5: Add Policy-Based Authorization
|
|
|
|
Define authorization policies in a shared middleware and register them per-service. Minimum set:
|
|
- `RequireAuthenticated` — replaces bare `[Authorize]`
|
|
- `RequireMerchantAccess` — validates `merchant_id` claim matches route param
|
|
- `RequireAdminRole` — replaces `[Authorize(Roles = "Admin,SuperAdmin")]`
|
|
|
|
Enable audience validation once scopes are defined per service.
|
|
|
|
### IMP-6: Complete Traefik Routes with Route Priority Planning
|
|
|
|
Add all 11 missing services to `infra/traefik/dynamic/routes.yml`. Follow the existing pattern and assign route prefixes:
|
|
|
|
| Service | Suggested Prefix |
|
|
|---|---|
|
|
| `promotion-service-net` | `/api/v1/promotions`, `/api/v1/vouchers` |
|
|
| `social-service-net` | `/api/v1/social`, `/api/v1/feed` |
|
|
| `mission-service-net` | `/api/v1/missions`, `/api/v1/rewards` |
|
|
| `mining-service-net` | `/api/v1/analytics` (internal only, consider IP allowlist) |
|
|
| `chat-service-net` | `/api/v1/chat`, `/hubs/chat` |
|
|
| `ads-*-service-net` | `/api/v1/ads/*` (sub-paths per service) |
|
|
| `mkt-*-service-net` | `/api/v1/mkt/*` (sub-paths per channel) |
|
|
|
|
---
|
|
|
|
## Action Items
|
|
|
|
| Priority | Action | Owner | Files |
|
|
|---|---|---|---|
|
|
| P0 | Unify response format — extract `ApiResponse<T>` to shared package | Backend Lead | `services/iam-service-net/.../ApiResponse.cs` → `packages/GoodGo.Shared.ApiResponse/` |
|
|
| P0 | Fix `merchant-service-net` controllers to use `ApiResponse<T>` wrapper | Backend Dev | `services/merchant-service-net/.../MerchantsController.cs` |
|
|
| P0 | Add missing Traefik routes for all 11 unrouted services | DevOps | `infra/traefik/dynamic/routes.yml` |
|
|
| P1 | Standardize API versioning — update `_template_dot_net` and migrate non-IAM services | Backend Lead | `services/_template_dot_net/src/MyService.API/Program.cs` |
|
|
| P1 | Add `HealthController` to `_template_dot_net`, backfill all services | Backend Dev | `services/_template_dot_net/src/MyService.API/Controllers/HealthController.cs` |
|
|
| P1 | Add `AddSwaggerGen` to all services missing it; export specs in CI | DevOps | All missing `Program.cs` files |
|
|
| P2 | Define and implement Policy-Based Authorization with scope validation | IAM Dev | `services/iam-service-net/` + all service `Program.cs` |
|
|
| P2 | Verify `middlewares.yml` has all 4 rate limit middleware definitions | DevOps | `infra/traefik/dynamic/middlewares.yml` |
|
|
| P2 | Route `/hubs/chat` through Traefik with correct CORS/upgrade config | DevOps | `infra/traefik/dynamic/routes.yml` |
|
|
| P3 | Set up OpenAPI spec export in CI and configure Redoc/Scalar for central docs | DevOps | `.github/workflows/` |
|
|
| P3 | Add contract tests (Pact.js) for cross-service boundaries | QA | `tests/ContractTests/` |
|