Skip to content

Fix Redis fail-open behavior for auth revocation and throttling #20

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
osama1998H merged 2 commits into from
Mar 27, 2026
Merged

Fix Redis fail-open behavior for auth revocation and throttling #20

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0e389b5
Update docs for Redis fail-closed auth controls
osama1998H Mar 27, 2026
e0f7a87
Fix JWT auth typed-nil blacklist checker handling
osama1998H Mar 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions docs/docs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,12 @@ const docTemplate = `{
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
},
"503": {
"description": "Rate limiting temporarily unavailable",
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
}
}
}
Expand Down Expand Up @@ -383,6 +389,12 @@ const docTemplate = `{
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
},
"503": {
"description": "Authentication temporarily unavailable",
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
}
}
}
Expand Down Expand Up @@ -420,6 +432,12 @@ const docTemplate = `{
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
},
"503": {
"description": "Authentication temporarily unavailable",
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
}
}
}
Expand Down Expand Up @@ -477,6 +495,12 @@ const docTemplate = `{
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
},
"503": {
"description": "Authentication temporarily unavailable",
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
}
}
}
Expand Down Expand Up @@ -529,6 +553,12 @@ const docTemplate = `{
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
},
"503": {
"description": "Rate limiting temporarily unavailable",
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
}
}
}
Expand Down Expand Up @@ -569,6 +599,12 @@ const docTemplate = `{
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
},
"503": {
"description": "Rate limiting temporarily unavailable",
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
}
}
}
Expand Down Expand Up @@ -621,6 +657,12 @@ const docTemplate = `{
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
},
"503": {
"description": "Rate limiting temporarily unavailable",
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
}
}
}
Expand Down Expand Up @@ -673,6 +715,12 @@ const docTemplate = `{
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
},
"503": {
"description": "Rate limiting temporarily unavailable",
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
}
}
}
Expand Down
11 changes: 6 additions & 5 deletions docs/horizontal-scaling.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,11 @@ REDIS_URL=redis://:password@your-redis-host:6379/0
```

Redis is used for rate limiting and access token blacklisting. If Redis is temporarily unavailable:
- Rate limiting is **skipped** (requests are allowed — graceful degradation)
- Blacklist checks are **skipped** (already-revoked tokens may be accepted until Redis recovers)
- Public auth routes under `/api/v1/auth/*` fail closed with `503 Service Unavailable`
- JWT-protected routes fail closed during blacklist checks with `503 Service Unavailable`
- Non-auth public routes such as `/health`, `/ready`, and `/swagger/*` remain available

This means Redis is **not** a hard availability dependency, but it should be highly available in production.
This means Redis is part of UniAuth's auth control plane. It should be highly available in production, and `/ready` should be used to remove unhealthy instances from rotation quickly.

### Shared JWT Secret

Expand All @@ -74,11 +75,11 @@ When a user logs out or changes their password, UniAuth:
1. Revokes the session in PostgreSQL (invalidates the refresh token for all instances)
2. Blacklists the current access token JTI in Redis with the remaining TTL

The blacklist entry is checked by the `JWTAuth` middleware on every subsequent authenticated request across all instances. This ensures that a user who logs out cannot continue using their short-lived access token on a different instance.
The blacklist entry is checked by the `JWTAuth` middleware on every subsequent authenticated request across all instances. This ensures that a user who logs out cannot continue using their short-lived access token on a different instance. If Redis cannot be reached for that check, UniAuth now rejects the request with `503` instead of failing open.

### Access Token Expiry

Access tokens have a short TTL (default 15 minutes). Even without Redis, a blacklisted token expires naturally within that window. The Redis blacklist entry TTL matches the remaining token lifetime — it cleans itself up automatically.
Access tokens still have a short TTL (default 15 minutes), and the Redis blacklist entry TTL matches the remaining token lifetime so it cleans itself up automatically. During a Redis outage, protected requests fail closed until Redis recovers rather than accepting potentially revoked access tokens.

---

Expand Down
13 changes: 8 additions & 5 deletions docs/security-report.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
**1. Executive Summary**
This repo’s remaining main security problems are real and concentrated in operational hardening. The highest-risk remaining issue is now that Redis fail-open behavior can weaken revocation and throttling during outages, with weak JWT secret acceptance as the next tier of residual risk.
This repo’s remaining main security problems are now concentrated in configuration and deployment hardening. The highest-risk remaining issue is weak JWT secret acceptance, followed by the remaining lower-severity readiness and supply-chain hardening gaps.

Update (2026-03-28): findings 1, 2, 3, 4, 5, 6, and 7 have been fixed in the repo. The remaining findings below are still outstanding unless explicitly marked otherwise.
Update (2026-03-28): findings 1, 2, 3, 4, 5, 6, 7, and 8 have been fixed in the repo. The remaining findings below are still outstanding unless explicitly marked otherwise.

This report began as a read-only review of the codebase across auth/session logic, authorization/tenant boundaries, and config/deployment. Findings 1, 2, 3, 4, 5, 6, and 7 have since been remediated in the repo. `go test ./...` passed locally; `govulncheck` was not installed, so dependency-CVE verification remains open.
This report began as a read-only review of the codebase across auth/session logic, authorization/tenant boundaries, and config/deployment. Findings 1, 2, 3, 4, 5, 6, 7, and 8 have since been remediated in the repo. `go test ./...` passed locally; `govulncheck` was not installed, so dependency-CVE verification remains open.

**2. Architecture and Attack Surface**
- Entry points: public auth routes, `/health`, `/ready`, `/swagger/*`, and JWT-protected `/api/v1/*` in [router.go](/Users/osamamuhammed/uniauth/internal/api/router.go#L76).
Expand Down Expand Up @@ -48,6 +48,8 @@ This report began as a read-only review of the codebase across auth/session logi
Remediation implemented: `chi`'s unconditional `RealIP` middleware has been removed, a shared trusted-proxy-aware client IP resolver now populates request context before logging and rate limiting, auth handlers consume that resolved value for audit/session metadata, and `TRUSTED_PROXY_CIDRS` now explicitly defines which proxy CIDRs or IPs may supply forwarded client-IP headers. Regression coverage was added in [ratelimit_test.go](/Users/osamamuhammed/uniauth/internal/api/middleware/ratelimit_test.go#L1) and [config_test.go](/Users/osamamuhammed/uniauth/internal/config/config_test.go#L1).

8. **Revocation and throttling fail open when Redis is unavailable** — Medium, High confidence. Affected: [auth middleware](/Users/osamamuhammed/uniauth/internal/api/middleware/auth.go#L44), [auth service](/Users/osamamuhammed/uniauth/internal/service/auth.go#L313), [ratelimit middleware](/Users/osamamuhammed/uniauth/internal/api/middleware/ratelimit.go#L18). Evidence: blacklist lookup failures allow the request, blacklist writes are ignored, and rate limiting is skipped on Redis errors. Risk: during a Redis outage or partition, revoked access tokens remain usable until expiry and brute-force controls disappear. Preconditions: Redis outage/partition. Fix: decide explicitly which endpoints must fail closed, and at minimum fail closed for logout-sensitive auth decisions or degrade with alarms/feature flags. Verify with tests that simulate Redis failures. Category: session management / availability-hardening.
Status: Fixed.
Remediation implemented: Redis-backed auth controls now use a strict-auth fail-closed policy: `JWTAuth` returns `503` when blacklist lookups fail, `/api/v1/auth/*` requests return `503` when rate limiting cannot be enforced, and logout/password-change flows abort with `ErrServiceUnavailable` before mutating DB-backed auth state if the access-token blacklist write fails. Supporting docs were updated in [horizontal-scaling.md](/Users/osamamuhammed/uniauth/docs/horizontal-scaling.md#L1). Regression coverage was added in [auth_test.go](/Users/osamamuhammed/uniauth/internal/api/middleware/auth_test.go#L1), [ratelimit_test.go](/Users/osamamuhammed/uniauth/internal/api/middleware/ratelimit_test.go#L1), [auth_redis_test.go](/Users/osamamuhammed/uniauth/internal/service/auth_redis_test.go#L1), and [response_test.go](/Users/osamamuhammed/uniauth/internal/api/handlers/response_test.go#L1).

9. **JWT secret strength is documented but not enforced** — Medium, Medium confidence. Affected: [main.go](/Users/osamamuhammed/uniauth/cmd/server/main.go#L115), [config.go](/Users/osamamuhammed/uniauth/internal/config/config.go#L53), [docker-compose](/Users/osamamuhammed/uniauth/docker-compose.yml#L12). Evidence: startup only checks `JWT_SECRET != ""`; docs say “minimum 32 characters,” but code does not enforce it. Risk: weak or placeholder HMAC secrets make token forgery practical in misconfigured self-hosted deployments. Preconditions: operator sets a weak/default secret. Fix: reject short or known-placeholder secrets at startup. Verify with config validation tests. OWASP: A02 Cryptographic Failures.

Expand All @@ -67,7 +69,7 @@ This report began as a read-only review of the codebase across auth/session logi
- Completed hotfix 4: stop logging reset tokens and fail closed when reset-email delivery is unavailable. Complexity: Small. Regression risk: Low. Order: 4.
- Completed hardening 1: add webhook URL validation and outbound SSRF guards. Complexity: Medium. Regression risk: Medium. Order: 5.
- Completed hardening 2: replace naive forwarded-header trust with trusted-proxy-aware IP extraction and explicit proxy CIDR configuration. Complexity: Small-Medium. Regression risk: Medium. Order: 6.
- Short-term hardening 3: enforce JWT secret quality, sanitize `/ready`, and define Redis degraded-mode behavior. Complexity: Small. Regression risk: Low-Medium. Order: 7.
- Short-term hardening 3: enforce JWT secret quality and sanitize `/ready`. Complexity: Small. Regression risk: Low-Medium. Order: 7.
- Completed structural improvement 1: introduce reusable authorization middleware/policy mapping instead of ad hoc handler checks. Complexity: High. Regression risk: Medium.
- Completed structural improvement 2: add DB-level same-org enforcement for role assignments and auto-remove legacy cross-tenant links during migration. Complexity: Medium. Regression risk: Medium.
- Structural improvement 3: pin CI actions/artifacts/images and add `govulncheck` to CI. Complexity: Small. Regression risk: Low.
Expand All @@ -84,11 +86,12 @@ This report began as a read-only review of the codebase across auth/session logi
- Add startup/config tests that weak or placeholder `JWT_SECRET` values fail fast.
- Implemented: password reset email tests now confirm reset tokens and links are never written to logs in [email_test.go](/Users/osamamuhammed/uniauth/internal/service/email_test.go#L1).
- Implemented: auth reset-flow tests now confirm failed delivery deletes reset-token rows while successful delivery preserves a usable token in [auth_reset_test.go](/Users/osamamuhammed/uniauth/internal/service/auth_reset_test.go#L1).
- Implemented: Redis outage regression tests now confirm blacklist lookup failures return `503`, auth-route rate limiting fails closed with `503`, non-auth public routes remain available, and logout/password-change flows do not mutate DB-backed auth state when blacklist writes fail in [auth_test.go](/Users/osamamuhammed/uniauth/internal/api/middleware/auth_test.go#L1), [ratelimit_test.go](/Users/osamamuhammed/uniauth/internal/api/middleware/ratelimit_test.go#L1), and [auth_redis_test.go](/Users/osamamuhammed/uniauth/internal/service/auth_redis_test.go#L1).
- Add `govulncheck ./...` to CI and pin supply-chain artifacts.

**7. Open Questions / Assumptions**
- Deployments behind a reverse proxy must now set `TRUSTED_PROXY_CIDRS` explicitly; otherwise UniAuth will use the proxy peer IP for rate limiting and audit trails by design.
- I did not verify pod/network egress policy. Existing egress controls remain useful defense-in-depth on top of the new webhook validation and delivery guards.
- I could not verify dependency-level vulns because `govulncheck` is not installed in this workspace.

Quick note: 4 security issues remain open in the report: findings 8 and 9, plus the 2 lower-severity hardening items.
Quick note: 3 security issues remain open in the report: finding 9 plus the 2 lower-severity hardening items.
48 changes: 48 additions & 0 deletions docs/swagger.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,12 @@
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
},
"503": {
"description": "Rate limiting temporarily unavailable",
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
}
}
}
Expand Down Expand Up @@ -377,6 +383,12 @@
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
},
"503": {
"description": "Authentication temporarily unavailable",
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
}
}
}
Expand Down Expand Up @@ -414,6 +426,12 @@
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
},
"503": {
"description": "Authentication temporarily unavailable",
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
}
}
}
Expand Down Expand Up @@ -471,6 +489,12 @@
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
},
"503": {
"description": "Authentication temporarily unavailable",
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
}
}
}
Expand Down Expand Up @@ -523,6 +547,12 @@
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
},
"503": {
"description": "Rate limiting temporarily unavailable",
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
}
}
}
Expand Down Expand Up @@ -563,6 +593,12 @@
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
},
"503": {
"description": "Rate limiting temporarily unavailable",
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
}
}
}
Expand Down Expand Up @@ -615,6 +651,12 @@
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
},
"503": {
"description": "Rate limiting temporarily unavailable",
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
}
}
}
Expand Down Expand Up @@ -667,6 +709,12 @@
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
},
"503": {
"description": "Rate limiting temporarily unavailable",
"schema": {
"$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse"
}
}
}
}
Expand Down
Loading
Loading