pos-system/microservices/docs/audit/security.md

# Security Audit Report — GoodGo POS System

**Auditor**: Security Engineer
**Date**: 2026-03-20
**Scope**: Auth implementation, OWASP risks, secrets management, CORS, dependencies
**Classification**: INTERNAL — CONFIDENTIAL

---

## Executive Summary

The GoodGo POS System demonstrates a solid security architecture in its design patterns (CQRS, Clean Architecture, MediatR pipeline with validation, account lockout, TOTP-based 2FA, comprehensive audit logging). However, there is one systemic critical failure that overrides all other positives: **real production credentials are hardcoded and tracked in git across 19+ microservices**. Until this is remediated, the entire platform is at risk of full compromise. Additionally, JWT tokens are stored in `localStorage` (XSS-extractable), CSP headers are absent, and the MCP server has a live bearer token committed to version control.

---

## Critical Issues

### CRIT-01 — Production Database Credentials Committed to Git (19 Services)

**Severity**: Critical
**CVSS**: 9.8 (AV:N/AC:L/PR:N/UI:N/S:C/C:H/I:H/A:H)
**Affected Components**: All 19 `.NET` microservices `appsettings.json`

**Description**: The Neon PostgreSQL production password `npg_Ssfy6HKO0cXI` and the Redis production password `Velik@2026` (with public IP `167.114.174.113`) are hardcoded in `appsettings.json` files that are tracked by git.

**Affected Files** (sample — all 19 services affected):
```
services/iam-service-net/src/IamService.API/appsettings.json:33
services/order-service-net/src/OrderService.API/appsettings.json:33
services/wallet-service-net/src/WalletService.API/appsettings.json:33
services/merchant-service-net/src/MerchantService.API/appsettings.json:33
services/chat-service-net/src/ChatService.API/appsettings.json:33
... (14 more services)
```

**Leaked Credentials**:
```
PostgreSQL: Host=ep-holy-glitter-a4hongg7-pooler.us-east-1.aws.neon.tech
            Username=neondb_owner | Password=npg_Ssfy6HKO0cXI

Redis:      Host=167.114.174.113:6379 (public IP)
            Password=Velik@2026

SMTP:       SmtpPassword=a469e9333580ef5dbb141f01e33864ef-51afd2db-6c014754
            (appsettings.json:54 in iam-service-net)

JWT Secret: goodgo-iam-service-secret-key-32chars!
            (appsettings.json:44 in iam-service-net)
```

**Impact**: Anyone with read access to this repository can authenticate directly to the production database and Redis. All customer data, merchant data, orders, wallets, and PII are at risk of exfiltration or destruction.

**Proof of Concept**:
```bash
psql "Host=ep-holy-glitter-a4hongg7-pooler.us-east-1.aws.neon.tech;Username=neondb_owner;Password=npg_Ssfy6HKO0cXI;Database=iam_service;SSL Mode=Require"
```

**Remediation**:
1. **Rotate all credentials immediately** — DB password, Redis password, SMTP key, JWT secret
2. Replace all `appsettings.json` values with environment variable placeholders:
   ```json
   "DefaultConnection": ""
   ```
3. Inject secrets at runtime via Kubernetes Secrets or HashiCorp Vault
4. Add `appsettings.*.json` pattern to `.gitignore` for non-Development configs, or use User Secrets locally
5. Run `git filter-repo` or BFG Repo Cleaner to purge secrets from git history

**Timeline**: **IMMEDIATE — within 24 hours**

---

### CRIT-02 — Active JWT Bearer Token Committed in MCP Server .env

**Severity**: Critical
**Affected Component**: `services/goodgo-mcp-server/.env:3`

**Description**: A live, signed JWT bearer token is committed to git history in the MCP server's `.env` file, which is tracked by git (`git ls-files` confirms).

```
API_TOKEN=eyJhbGciOiJSUzI1NiIsImtpZCI6IjQ3NjE1OTQ3...
```

Decoded payload includes: `sub`, `email`, `auth_time`, `jti` — a real user/service account token.

**Impact**: Any party with repository access can replay this token to authenticate as the associated service account until it expires or is revoked. The MCP server has 12 operational tools that can read/write F&B data.

**Remediation**:
1. Revoke the token immediately via IAM service
2. Remove `.env` from git tracking: add `services/goodgo-mcp-server/.env` to `.gitignore`
3. Purge from git history
4. Rotate the service account credentials

**Timeline**: **IMMEDIATE — within 24 hours**

---

### CRIT-03 — IdentityServer Using `AddDeveloperSigningCredential()` in All Environments

**Severity**: Critical
**Affected Component**: `services/iam-service-net/src/IamService.Infrastructure/DependencyInjection.cs:142`

**Description**: The code unconditionally calls `AddDeveloperSigningCredential()`, which generates a temporary RSA key on startup that is not persisted between restarts. This means:
- JWT tokens are invalidated on every service restart
- There is no certificate-based signing in production
- The signing key changes silently

```csharp
.AddDeveloperSigningCredential();  // Line 142 — applied in ALL environments
```

**Impact**: In production, this causes all active sessions to be invalidated on restart. More critically, a production RSA private key should be stored in a secret vault (Vault/K8s Secret), not re-generated on each boot.

**Remediation**:
```csharp
if (env.IsDevelopment())
    identityServer.AddDeveloperSigningCredential();
else
    identityServer.AddSigningCredential(LoadCertificateFromVault()); // HSM or K8s TLS secret
```

**Timeline**: Before next production deployment

---

## Warnings

### WARN-01 — JWT Tokens Stored in `localStorage` (XSS Risk)

**Severity**: High
**Affected Component**: `apps/web-client-tpos-net/src/WebClientTpos.Client/Services/AuthService.cs:147`

**Description**: Access tokens are stored in `localStorage` using role-specific keys (`aPOS_token_owner`, `aPOS_token_staff`):

```csharp
await _js.InvokeVoidAsync("localStorage.setItem", TokenKey(role), token.AccessToken);
```

`localStorage` is accessible by any JavaScript running on the same origin. If a single XSS vulnerability is introduced anywhere in the Blazor WASM app, an attacker can exfiltrate all tokens.

**Impact**: XSS → token theft → full account takeover.

**Remediation**:
- Use `HttpOnly` cookies for token storage (tokens never accessible to JS)
- If `localStorage` is retained for Blazor WASM constraints, implement strict CSP (see WARN-02) to minimize XSS risk
- Add token binding or short expiry (already 15 min — good) with refresh rotation

---

### WARN-02 — No Content-Security-Policy (CSP) Header

**Severity**: High
**Affected Component**: `infra/traefik/dynamic/middlewares.yml`

**Description**: The `secure-headers` middleware is missing `Content-Security-Policy`:

```yaml
secure-headers:
  headers:
    contentTypeNosniff: true   # ✓
    browserXssFilter: true     # ✓ (deprecated but present)
    frameDeny: true            # ✓
    stsSeconds: 31536000       # ✓
    # CSP: MISSING ✗
    # Referrer-Policy: MISSING ✗
    # Permissions-Policy: MISSING ✗
```

Without CSP, the browser has no instructions to block inline scripts, restrict `eval()`, or limit allowed resource origins — the primary defense against XSS exploitation.

**Remediation** — Add to `secure-headers` middleware:
```yaml
contentSecurityPolicy: >
  default-src 'self';
  script-src 'self' 'wasm-unsafe-eval';
  style-src 'self' 'unsafe-inline';
  img-src 'self' data: https:;
  connect-src 'self' https://api.goodgo.vn wss://;
  font-src 'self';
  object-src 'none';
  frame-ancestors 'none';
referrerPolicy: "strict-origin-when-cross-origin"
permissionsPolicy: "camera=(), microphone=(), geolocation=()"
```

Note: Blazor WASM requires `wasm-unsafe-eval` for .NET runtime compilation.

---

### WARN-03 — CORS `allowCredentials: true` with Multiple Origins

**Severity**: High
**Affected Component**: `infra/traefik/dynamic/middlewares.yml:27`

**Description**:
```yaml
cors:
  headers:
    accessControlAllowOriginList:
      - "http://localhost:3000"   # Development origins
      - "http://localhost:3001"
      - "https://goodgo.vn"      # Production origin
    accessControlAllowCredentials: true   # ← HIGH RISK
```

Combining `allowCredentials: true` with a list of origins that includes localhost development URLs is dangerous. If a developer's machine is compromised, the credentials flag allows the attacker to make authenticated cross-origin requests from any of these origins.

**Remediation**:
- Per-environment CORS config: production allows only `https://goodgo.vn`
- Development config in a separate file that is NOT applied to production
- `admin.goodgo.vn` should be added to production allowlist if used

---

### WARN-04 — `sslRedirect: false` in Shared Middleware Config

**Severity**: High
**Affected Component**: `infra/traefik/dynamic/middlewares.yml:5`

**Description**:
```yaml
sslRedirect: false  # EN: Disabled for local development
```

This file is used across all environments. If this same `middlewares.yml` is deployed to staging/production (common in this monorepo pattern), HTTPS redirection will be disabled.

**Remediation**:
- Separate `middlewares.yml` per environment, or use Traefik environment variable substitution
- Explicitly set `sslRedirect: true` in staging/production overlay

---

### WARN-05 — `Jwt__RequireHttpsMetadata=false` Across All Services

**Severity**: High
**Affected Component**: `deployments/local/docker-compose.yml` (lines ~219, 265, 372, 420, etc.)

**Description**: Every service in docker-compose overrides JWT metadata HTTPS requirement:
```yaml
- Jwt__RequireHttpsMetadata=false
```

This is acceptable for local Docker networking. However, if these environment variables are inadvertently propagated to staging/production K8s ConfigMaps, JWT validation will succeed over plain HTTP.

**Remediation**:
- Verify K8s ConfigMaps do NOT contain this override: `grep -r RequireHttpsMetadata deployments/staging/ deployments/production/`
- Add to deployment runbook: `RequireHttpsMetadata` must be `true` in staging and production

---

### WARN-06 — Traefik Dashboard Exposed Without Authentication

**Severity**: Medium
**Affected Component**: `deployments/local/docker-compose.yml:121`

**Description**:
```yaml
command:
  - "--api.insecure=true"
```

Traefik API/dashboard is accessible at `http://localhost:8080` with no authentication in local development. While local-only, this exposes full routing configuration, middleware definitions, and service topology.

**Remediation**:
- Use `--api.insecure=false` and enable `api.dashboard: true` with `BasicAuth` middleware
- Verify staging/production Traefik deployment does NOT have `--api.insecure=true`

---

### WARN-07 — Unauthenticated Ad Tracking Endpoints Without Rate Limiting

**Severity**: Medium
**Affected Component**: Routes for `/api/v1/pixels`, `/api/v1/conversions`, `/api/v1/ads/events`

**Description**: These endpoints are intentionally public (pixel tracking requires no auth) but have no documented rate limiting in the Traefik route labels. The only defined rate limits are `auth-ratelimit`, `payment-ratelimit`, `api-ratelimit`, and `hub-ratelimit`.

**Impact**: An attacker can spam fake conversion/pixel events to corrupt analytics, inflate ad billing, or DoS the tracking services.

**Remediation**:
- Apply a dedicated `tracking-ratelimit` (e.g., 200 req/min per IP) to these routes
- Implement HMAC signature validation on pixel requests with a short-lived nonce

---

### WARN-08 — AllowedHosts Wildcard in IAM Service

**Severity**: Medium
**Affected Component**: `services/iam-service-net/src/IamService.API/appsettings.json:79`

**Description**:
```json
"AllowedHosts": "*"
```

A wildcard `AllowedHosts` allows the IAM service (which issues JWT tokens) to respond to requests regardless of the `Host` header value. This can be exploited in DNS rebinding attacks or Host header injection to redirect token issuance.

**Remediation**:
```json
"AllowedHosts": "iam-service;localhost;127.0.0.1"
```
Production should explicitly list `iam.goodgo.vn` or the internal K8s service name.

---

### WARN-09 — K8s Staging Secrets File Contains Placeholders

**Severity**: Medium
**Affected Component**: `deployments/staging/kubernetes/secrets.yaml`

**Description**: The secrets manifest contains literal placeholder strings:
```yaml
Jwt__Secret: "PLACEHOLDER-staging-jwt-secret-min-32-chars"
```

If a CI/CD pipeline or developer applies this file directly without substitution, the platform will launch with predictable secrets.

**Remediation**:
- Use `secrets.yaml.example` pattern (like production) — remove the live staging `secrets.yaml` from git
- Adopt External Secrets Operator or Sealed Secrets for K8s secret management
- Add a CI pre-flight check that rejects `PLACEHOLDER` values in K8s secret manifests

---

### WARN-10 — TOTP Verification Window Allows 90-Second Replay

**Severity**: Low
**Affected Component**: `services/iam-service-net/src/IamService.Infrastructure/TwoFactor/TotpTwoFactorService.cs:86`

**Description**:
```csharp
new VerificationWindow(previous: 1, future: 1)
```

A tolerance of ±1 time step (30 seconds each) means a valid TOTP code has a 90-second effective validity window. Combined with no used-code tracking, a captured OTP could be replayed within 90 seconds.

**Remediation**:
- Track used TOTP codes in Redis with a 90-second TTL to prevent replay
- Consider reducing window to `previous: 0, future: 0` if clock skew is controlled

---

## Improvements

### IMP-01 — Implement Secrets Management Pipeline

Replace all hardcoded credentials with a proper secrets lifecycle:

```
Local Dev:    dotnet user-secrets / .env (gitignored)
CI/CD:        GitHub Actions Secrets → injected at build time
Staging/Prod: Kubernetes External Secrets Operator → HashiCorp Vault
              or AWS Secrets Manager / Azure Key Vault
```

Reference implementation pattern for each service:
```csharp
// Program.cs — replace appsettings.json secret loading
builder.Configuration.AddEnvironmentVariables();
// In K8s: environment variables injected from K8s Secrets
```

---

### IMP-02 — Add Serilog Sensitive Data Destructuring

Prevent accidental PII/credential logging:

```csharp
Log.Logger = new LoggerConfiguration()
    .Destructure.ByTransforming<ApplicationUser>(u => new { u.Id, u.Email })
    .Filter.ByExcluding(e => e.Properties.ContainsKey("password"))
    .WriteTo.Console()
    .CreateLogger();
```

---

### IMP-03 — Add Security Scanning to CI/CD Pipeline

```yaml
# .github/workflows/security.yml
- name: Secret scanning
  uses: gitleaks/gitleaks-action@v2

- name: Dependency audit (.NET)
  run: dotnet list package --vulnerable --include-transitive

- name: SAST
  uses: github/codeql-action/analyze@v3
  with:
    languages: csharp, javascript
```

---

### IMP-04 — Implement Refresh Token Rotation

Current refresh tokens have 7-day sliding window with no rotation. Add one-time-use refresh tokens:

```csharp
// In ResourceOwnerPasswordValidator or token endpoint
// Revoke old refresh token on each use
await _tokenRevocationService.RevokeAsync(oldRefreshToken);
var newRefreshToken = await _tokenService.IssueRefreshTokenAsync(user);
```

---

### IMP-05 — Add `Referrer-Policy` and `Permissions-Policy` Headers

Extend `secure-headers` middleware:
```yaml
customResponseHeaders:
  Referrer-Policy: "strict-origin-when-cross-origin"
  Permissions-Policy: "camera=(), microphone=(), geolocation=(self), payment=(self)"
  Cross-Origin-Opener-Policy: "same-origin"
  Cross-Origin-Resource-Policy: "same-site"
```

---

## Action Items

| Priority | ID | Action | Owner | Deadline |
|----------|----|--------|-------|----------|
| P0 | CRIT-01 | Rotate all production credentials, remove from git, purge history | DevOps + CTO | 24h |
| P0 | CRIT-02 | Revoke MCP server JWT token, remove `.env` from git tracking | DevOps | 24h |
| P0 | CRIT-03 | Replace `AddDeveloperSigningCredential()` with cert-based signing for production | Backend Lead | Before next deploy |
| P1 | WARN-01 | Evaluate `HttpOnly` cookie storage vs. localStorage for JWT tokens | Frontend Lead | 1 week |
| P1 | WARN-02 | Add CSP header to Traefik `secure-headers` middleware | DevOps | 1 week |
| P1 | WARN-03 | Separate CORS config by environment; remove localhost from production | DevOps | 1 week |
| P1 | WARN-04 | Create per-environment `middlewares.yml`; enforce `sslRedirect: true` in prod | DevOps | 1 week |
| P1 | WARN-05 | Audit K8s manifests for `RequireHttpsMetadata=false` — must not appear in staging/prod | DevOps | 1 week |
| P2 | WARN-06 | Disable Traefik insecure API, add BasicAuth to dashboard | DevOps | 2 weeks |
| P2 | WARN-07 | Apply rate limiting to tracking/pixel endpoints | Backend | 2 weeks |
| P2 | WARN-08 | Set explicit `AllowedHosts` in all service `appsettings.json` | Backend Lead | 2 weeks |
| P2 | WARN-09 | Remove staging `secrets.yaml` from git, adopt External Secrets Operator | DevOps | 2 weeks |
| P2 | WARN-10 | Add Redis-backed TOTP replay prevention | Backend | 2 weeks |
| P3 | IMP-01 | Implement full secrets management pipeline (Vault / External Secrets) | DevOps | 1 month |
| P3 | IMP-02 | Add Serilog sensitive data destructuring to all services | Backend | 1 month |
| P3 | IMP-03 | Add GitLeaks + CodeQL + dotnet vulnerability scan to CI/CD | DevOps | 1 month |
| P3 | IMP-04 | Implement refresh token rotation | Backend | 1 month |
| P3 | IMP-05 | Add Referrer-Policy and Permissions-Policy headers | DevOps | 1 month |

---

## Positive Findings

The following security controls are correctly implemented and should be maintained:

- ✅ **Account lockout**: 5 failed attempts → 15-minute lockout (`DependencyInjection.cs:93-96`)
- ✅ **TOTP 2FA**: OtpNet library, 160-bit secret, QR code provisioning, 10 recovery codes
- ✅ **Password hashing**: ASP.NET Core Identity PBKDF2/SHA256, enforced complexity rules
- ✅ **Audit logging**: 52 event types tracked with IP, UserAgent, actor, resource (`AuditEventType.cs`)
- ✅ **OTP hashing**: `SHA256.HashData()` used for phone/email OTPs — not stored in plaintext
- ✅ **Generic auth errors**: Login failures return generic messages to prevent username enumeration
- ✅ **Auth rate limiting**: 10 req/min on login/register/2FA endpoints
- ✅ **RBAC policies**: `RequireSuperAdmin`, `RequireAdmin`, `OwnerOrAdmin` custom handler
- ✅ **Health checks**: `/health/live` and `/health/ready` endpoints on all services
- ✅ **Short-lived access tokens**: 15-minute expiry
- ✅ **Security headers**: `X-Content-Type-Options`, `X-XSS-Protection`, `X-Frame-Options`, `HSTS`
- ✅ **Input validation**: FluentValidation in MediatR pipeline on all Commands

---

*Report generated by Security Engineer — TechBi / GoodGo Platform*
*Next audit scheduled: 2026-06-20*