Compare commits
2 Commits
66b77e747d
...
fd0de714a4
| Author | SHA1 | Date | |
|---|---|---|---|
| fd0de714a4 | |||
| c6b155520c |
@@ -213,7 +213,7 @@ class TestAdminDeleteProgram:
|
|||||||
def test_delete_program(
|
def test_delete_program(
|
||||||
self, client, super_admin_headers, admin_program, db
|
self, client, super_admin_headers, admin_program, db
|
||||||
):
|
):
|
||||||
"""Admin can delete any program."""
|
"""Admin can soft-delete any program."""
|
||||||
program_id = admin_program.id
|
program_id = admin_program.id
|
||||||
|
|
||||||
response = client.delete(
|
response = client.delete(
|
||||||
@@ -222,10 +222,21 @@ class TestAdminDeleteProgram:
|
|||||||
)
|
)
|
||||||
assert response.status_code == 204
|
assert response.status_code == 204
|
||||||
|
|
||||||
# Verify deleted
|
# Verify soft-deleted (hidden from normal queries)
|
||||||
deleted = db.get(LoyaltyProgram, program_id)
|
db.expire_all()
|
||||||
|
deleted = db.query(LoyaltyProgram).filter(LoyaltyProgram.id == program_id).first()
|
||||||
assert deleted is None
|
assert deleted is None
|
||||||
|
|
||||||
|
# But visible with include_deleted
|
||||||
|
visible = (
|
||||||
|
db.query(LoyaltyProgram)
|
||||||
|
.execution_options(include_deleted=True)
|
||||||
|
.filter(LoyaltyProgram.id == program_id)
|
||||||
|
.first()
|
||||||
|
)
|
||||||
|
assert visible is not None
|
||||||
|
assert visible.deleted_at is not None
|
||||||
|
|
||||||
def test_delete_nonexistent_program(
|
def test_delete_nonexistent_program(
|
||||||
self, client, super_admin_headers
|
self, client, super_admin_headers
|
||||||
):
|
):
|
||||||
|
|||||||
@@ -233,7 +233,7 @@ class TestMerchantDeleteProgram:
|
|||||||
def test_delete_program_success(
|
def test_delete_program_success(
|
||||||
self, client, loyalty_merchant_headers, loyalty_store_setup, db
|
self, client, loyalty_merchant_headers, loyalty_store_setup, db
|
||||||
):
|
):
|
||||||
"""Delete the merchant's loyalty program."""
|
"""Soft-delete the merchant's loyalty program."""
|
||||||
program_id = loyalty_store_setup["program"].id
|
program_id = loyalty_store_setup["program"].id
|
||||||
|
|
||||||
response = client.delete(
|
response = client.delete(
|
||||||
@@ -241,12 +241,23 @@ class TestMerchantDeleteProgram:
|
|||||||
)
|
)
|
||||||
assert response.status_code == 204
|
assert response.status_code == 204
|
||||||
|
|
||||||
# Verify program is deleted
|
# Verify soft-deleted (hidden from normal queries)
|
||||||
from app.modules.loyalty.models import LoyaltyProgram
|
from app.modules.loyalty.models import LoyaltyProgram
|
||||||
|
|
||||||
deleted = db.get(LoyaltyProgram, program_id)
|
db.expire_all()
|
||||||
|
deleted = db.query(LoyaltyProgram).filter(LoyaltyProgram.id == program_id).first()
|
||||||
assert deleted is None
|
assert deleted is None
|
||||||
|
|
||||||
|
# But visible with include_deleted
|
||||||
|
visible = (
|
||||||
|
db.query(LoyaltyProgram)
|
||||||
|
.execution_options(include_deleted=True)
|
||||||
|
.filter(LoyaltyProgram.id == program_id)
|
||||||
|
.first()
|
||||||
|
)
|
||||||
|
assert visible is not None
|
||||||
|
assert visible.deleted_at is not None
|
||||||
|
|
||||||
def test_delete_program_not_found(
|
def test_delete_program_not_found(
|
||||||
self, client, loyalty_merchant_headers_no_program
|
self, client, loyalty_merchant_headers_no_program
|
||||||
):
|
):
|
||||||
|
|||||||
200
docs/proposals/security-hardening-plan.md
Normal file
200
docs/proposals/security-hardening-plan.md
Normal file
@@ -0,0 +1,200 @@
|
|||||||
|
# Security Hardening Plan
|
||||||
|
|
||||||
|
Findings from a 360 security audit (2026-03-28), filtered against what is already deployed on Hetzner (Steps 1-25 in `docs/deployment/hetzner-server-setup.md`).
|
||||||
|
|
||||||
|
Items are ordered by priority. Each will be tackled individually upon prompt.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## HIGH Priority
|
||||||
|
|
||||||
|
### 1. Rate-limit login endpoints at app level
|
||||||
|
|
||||||
|
**Problem:** Cloudflare rate-limits `/api/` broadly at 100/10s, but login endpoints (`/api/v1/auth/login`, forgot-password, resend-verification) need much stricter limits. If Cloudflare is bypassed via direct IP or an attacker is behind CF, brute-force is possible.
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `app/modules/tenancy/routes/api/admin_auth.py`
|
||||||
|
- `app/modules/tenancy/routes/api/merchant_auth.py`
|
||||||
|
- `app/modules/tenancy/routes/api/store_auth.py`
|
||||||
|
- `app/modules/customers/routes/api/storefront.py`
|
||||||
|
|
||||||
|
**Fix:** Apply `@rate_limit(max_requests=5, window_seconds=300)` to all login and `@rate_limit(max_requests=3, window_seconds=3600)` to password reset/resend-verification endpoints.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 2. GraphQL template injection in Letzshop client
|
||||||
|
|
||||||
|
**Problem:** `QUERY_SHIPMENTS_PAGINATED_TEMPLATE.format(state=state)` injects the `state` parameter directly into the GraphQL query string instead of using GraphQL variables.
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `app/modules/marketplace/services/letzshop/client_service.py:798`
|
||||||
|
- `scripts/letzshop_introspect.py:487`
|
||||||
|
- `scripts/check_letzshop_shipment.py:97`
|
||||||
|
|
||||||
|
**Fix:** Refactor to use the `variables` parameter in the GraphQL request payload.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 3. SSRF in CSV processor
|
||||||
|
|
||||||
|
**Problem:** `requests.get(url, timeout=30)` accepts any URL with no domain whitelist or internal IP blocking. An admin could target internal Docker network IPs (172.x.x.x, 10.x.x.x) or cloud metadata endpoints.
|
||||||
|
|
||||||
|
**File:** `app/utils/csv_processor.py:102-106`
|
||||||
|
|
||||||
|
**Fix:** Add domain whitelist and block RFC1918/link-local IP ranges before making the request.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 4. Sentry PII exposure (GDPR)
|
||||||
|
|
||||||
|
**Problem:** `send_default_pii=True` sends user emails and IP addresses to Sentry (US-hosted SaaS). For EU-based SaaS with .lu domains, this needs GDPR consideration.
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `main.py:52`
|
||||||
|
- `app/core/celery_config.py` (same setting)
|
||||||
|
|
||||||
|
**Fix:** Set `send_default_pii=False` and configure Sentry data scrubbing rules, or document GDPR basis for the data transfer.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 5. Fail-fast on insecure production defaults
|
||||||
|
|
||||||
|
**Problem:** `validate_production_settings()` in `app/core/config.py:314-337` prints warnings but allows the app to start with `admin123`, placeholder JWT secret, wildcard CORS, or debug mode in production.
|
||||||
|
|
||||||
|
**File:** `app/core/config.py:314-337`
|
||||||
|
|
||||||
|
**Fix:** Raise `SystemExit` if any critical misconfiguration is detected when `APP_ENV=production`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 6. JWT secret fallback in code
|
||||||
|
|
||||||
|
**Problem:** `middleware/auth.py:69-70` silently falls back to `"your-secret-key-change-in-production-please"` if `JWT_SECRET_KEY` env var is missing. Production is properly configured (Step 10), but the code should not have a weak default.
|
||||||
|
|
||||||
|
**File:** `middleware/auth.py:69-70`
|
||||||
|
|
||||||
|
**Fix:** Raise an error if `JWT_SECRET_KEY` is not set (remove the default fallback).
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## MEDIUM Priority
|
||||||
|
|
||||||
|
### 7. Pin critical unpinned dependencies
|
||||||
|
|
||||||
|
**Problem:** Several security-sensitive packages use `>=` instead of exact pinning: `PyJWT>=2.0.0`, `google-auth>=2.0.0`, `Pillow>=10.0.0`, `boto3>=1.34.0`, `sentry-sdk>=2.0.0`.
|
||||||
|
|
||||||
|
**File:** `requirements.txt`
|
||||||
|
|
||||||
|
**Fix:** Pin to exact versions. Run `pip freeze` to capture current working versions.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 8. Pin Docker image tags
|
||||||
|
|
||||||
|
**Problem:** Monitoring stack images use `:latest` tag: Prometheus, Grafana, cAdvisor, Alertmanager, Redis Exporter, Gitea.
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `docker-compose.yml`
|
||||||
|
- `~/gitea/docker-compose.yml` (on server)
|
||||||
|
|
||||||
|
**Fix:** Pin to specific version tags (e.g., `prom/prometheus:v2.51.0`).
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 9. Replace `random` with `secrets` for order numbers
|
||||||
|
|
||||||
|
**Problem:** `random.choices()` and `random.randint()` are not cryptographically secure. Order numbers and store code suffixes should be unpredictable.
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- `app/modules/orders/services/order_service.py:70-79`
|
||||||
|
- `app/modules/marketplace/services/letzshop/store_sync_service.py:436, 464, 469`
|
||||||
|
|
||||||
|
**Fix:** Replace with `secrets.choice()` and `secrets.randbelow()`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 10. Switch rate limiter to Redis backend
|
||||||
|
|
||||||
|
**Problem:** In-memory `defaultdict(deque)` rate limiter doesn't persist across restarts and won't work with multiple Gunicorn workers.
|
||||||
|
|
||||||
|
**File:** `middleware/rate_limiter.py`
|
||||||
|
|
||||||
|
**Fix:** Use Redis (already in the stack) as the backing store for rate limit counters. Not urgent while running a single worker, but needed before horizontal scaling.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 11. Add Content-Security-Policy header
|
||||||
|
|
||||||
|
**Problem:** Security headers middleware sets HSTS, X-Frame-Options, X-Content-Type-Options, Referrer-Policy, Permissions-Policy — but no CSP. Cloudflare doesn't add one either.
|
||||||
|
|
||||||
|
**File:** `middleware/security_headers.py`
|
||||||
|
|
||||||
|
**Fix:** Add a restrictive CSP header. Start with `default-src 'self'` and add exceptions for CDN assets, inline scripts (Alpine.js), etc.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 12. Validate file download paths
|
||||||
|
|
||||||
|
**Problem:** `get_file_content(file_path)` reads arbitrary paths without verifying the path is under the expected upload directory.
|
||||||
|
|
||||||
|
**File:** `app/modules/messaging/services/message_attachment_service.py:213-222`
|
||||||
|
|
||||||
|
**Fix:** Resolve path with `Path.resolve()` and verify it starts with the expected base directory.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## LOW Priority
|
||||||
|
|
||||||
|
### 13. SVG upload sanitization
|
||||||
|
|
||||||
|
**Problem:** SVG files are allowed in media uploads. SVGs can contain embedded JavaScript (XSS vector).
|
||||||
|
|
||||||
|
**File:** `app/modules/cms/services/media_service.py`
|
||||||
|
|
||||||
|
**Fix:** Either disallow SVG uploads or sanitize with `defusedxml` / strip `<script>` tags.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 14. Monitor Gitea SSH (port 2222) with fail2ban
|
||||||
|
|
||||||
|
**Problem:** Port 2222 (Gitea SSH) is intentionally public but not monitored by fail2ban. The SSH jail watches `/var/log/auth.log` which only covers port 22.
|
||||||
|
|
||||||
|
**Fix:** Add a fail2ban jail watching Gitea's SSH logs, or configure Gitea to log failed auth attempts to a file fail2ban can parse.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 15. Email header injection protection
|
||||||
|
|
||||||
|
**Problem:** Email `To`/`From`/`Reply-To` headers are built from data that could contain newlines or control characters.
|
||||||
|
|
||||||
|
**File:** `app/modules/messaging/services/email_service.py:142-149`
|
||||||
|
|
||||||
|
**Fix:** Use `email.utils.formataddr()` for header construction.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 16. Mass assignment guard in user account update
|
||||||
|
|
||||||
|
**Problem:** `setattr()` loop over dict keys without explicit field whitelist. Currently gated by Pydantic schema, but fragile if the service is called from elsewhere.
|
||||||
|
|
||||||
|
**File:** `app/modules/tenancy/services/user_account_service.py:57-59`
|
||||||
|
|
||||||
|
**Fix:** Add explicit `ALLOWED_FIELDS` set check before `setattr()`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 17. Google service account JSON file permissions
|
||||||
|
|
||||||
|
**Problem:** The service account JSON key (Step 25) will be uploaded to the server. Needs restricted permissions.
|
||||||
|
|
||||||
|
**Fix:** Store at `/etc/orion/google-wallet-sa.json` with `chmod 600` and `chown appuser:appuser`. Ensure the path is not logged or included in error responses.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### 18. Strengthen password validation
|
||||||
|
|
||||||
|
**Problem:** Password policy only requires one letter + one digit, minimum 8 characters. No special character or entropy check.
|
||||||
|
|
||||||
|
**File:** `app/modules/tenancy/schemas/user_account.py:50-58`
|
||||||
|
|
||||||
|
**Fix:** Consider NIST-aligned requirements (minimum 12 chars, check against breached password lists) or at minimum require a special character.
|
||||||
Reference in New Issue
Block a user