Fix Redis fail-open behavior for auth revocation and throttling

[osama1998H]

Summary

• fail closed on Redis errors for JWT blacklist checks and /api/v1/auth/* rate limiting
• return 503 via a shared service-unavailable error path instead of allowing protected/auth-sensitive traffic through
• stop logout, logout-all, and password-change flows before mutating DB-backed auth state when access-token blacklisting fails
• add regression coverage for middleware and auth-service Redis failure scenarios
• update Swagger output and security/scaling docs to reflect the new strict-auth behavior and mark security finding 8 as fixed

Testing

• go test ./...
• make lint
• make swag
• make docker-build
• docker compose up --build -d
• verified logs for app, postgres, redis, and mailhog
• exercised curl flows for health/readiness, login, protected route access, logout revocation, Redis outage 503 behavior, and Redis recovery

Summary by CodeRabbit

• Bug Fixes
  • Auth and rate-limited auth endpoints now fail closed: return 503 Service Unavailable when backend blacklist or rate-limit checks cannot be performed.
• Documentation
  • Updated deployment, scaling, and security docs and API spec to reflect the new 503/resilience behavior and preserved non-auth public routes.
• Tests
  • Added regression tests covering blacklist and rate-limit outage behaviors and 503 mappings.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0bc0c3ed-0648-4de0-81e2-642ad7de32b5

📥 Commits

Reviewing files that changed from the base of the PR and between 0e389b5 and e0f7a87.

📒 Files selected for processing (2)

• internal/api/middleware/auth.go
• internal/api/middleware/auth_test.go

---

📝 Walkthrough

Walkthrough

Auth and rate-limit middleware now fail closed on Redis errors: auth and auth-protected routes return HTTP 503 when blacklist or rate-limit checks fail. Swagger, handler docs, errors, tests, service logic, and router wiring were updated to reflect and enforce this behavior.

Changes

Cohort / File(s)	Summary
Documentation docs/horizontal-scaling.md, docs/security-report.md	Documented fail-closed behavior for auth/rate-limit during Redis outages; updated security findings and remediation/verification guidance.
OpenAPI / Generated Docs docs/swagger.json, docs/swagger.yaml, docs/docs.go	Added 503 responses (schema: internal_api_handlers.SwaggerErrorResponse) to multiple POST/PUT /api/v1/auth/* endpoints with endpoint-specific descriptions.
Handler Swagger Docs internal/api/handlers/auth.go	Added // @failure 503 {object} SwaggerErrorResponse annotations to auth handler comment blocks.
Error mapping & tests internal/domain/errors.go, internal/api/handlers/response.go, internal/api/handlers/response_test.go	Introduced ErrServiceUnavailable sentinel and mapped it to HTTP 503; added unit test for 503 mapping.
JWT middleware & tests internal/api/middleware/auth.go, internal/api/middleware/auth_test.go	JWTAuth signature changed to accept tokenBlacklistChecker and *slog.Logger; blacklist lookup errors now return 503 (fail-closed). Added typed-nil handling and tests for blacklisted and lookup-failure cases.
Rate-limit middleware & tests internal/api/middleware/ratelimit.go, internal/api/middleware/ratelimit_test.go	RateLimit now accepts logger; on rate-limit store errors, auth-paths (/api/v1/auth/*) return 503 while non-auth paths continue. Added tests for error scenarios and enforcement.
Auth service & Redis-sensitive tests internal/service/auth.go, internal/service/auth_redis_test.go	Replaced concrete cache with accessTokenBlacklistWriter interface; blacklistAccessToken now returns errors and callers (Logout, LogoutAll, ChangePassword) propagate service-unavailable errors. Added tests ensuring no DB state mutation when blacklist writes fail.
Router wiring internal/api/router.go	Updated middleware instantiation to pass logger into RateLimit and JWTAuth.

Sequence Diagram

sequenceDiagram
    participant Client
    participant RateLimit as RateLimit Middleware
    participant Redis as Redis
    participant JWTAuth as JWTAuth Middleware
    participant Handler as Auth Handler
    participant Service as Auth Service

    Client->>RateLimit: Request /api/v1/auth/login
    RateLimit->>Redis: IncrRateLimit()
    alt Redis Available
        Redis-->>RateLimit: Success
        RateLimit->>JWTAuth: Next
    else Redis Unavailable
        Redis-->>RateLimit: Error
        RateLimit-->>Client: 503 Service Unavailable
    end

    JWTAuth->>Redis: IsTokenBlacklisted(tokenID)
    alt Blacklisted
        Redis-->>JWTAuth: true
        JWTAuth-->>Client: 401 Unauthorized
    else Lookup Error
        Redis-->>JWTAuth: Error
        JWTAuth-->>Client: 503 Service Unavailable
    else Not Blacklisted
        Redis-->>JWTAuth: false
        JWTAuth->>Handler: Proceed
        Handler->>Service: Logout/ChangePassword...
        Service->>Redis: BlacklistToken(tokenID)
        alt Write Success
            Redis-->>Service: Success
            Service-->>Handler: OK
            Handler-->>Client: 200/204 OK
        else Write Failure
            Redis-->>Service: Error
            Service-->>Handler: ErrServiceUnavailable
            Handler-->>Client: 503 Service Unavailable
        end
    end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• Fix refresh token bearer authentication bypass #12: Overlaps with middleware changes to internal/api/middleware/auth.go (token verification/signature and middleware behavior).
• feat: add Swagger/OpenAPI documentation and interactive UI #10: Related to Swagger/handler annotation updates and added 503 responses in generated docs.

Suggested labels

size/XXL

Poem

🐰 Hop, hop — the cache went still,

I thumped my foot upon the hill.
No open doors where risks may creep,
503s guard the auth heap.
Rebound soon — and then we'll leap!

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description provides a summary of changes and testing performed, but is missing several required template sections including Type of change, Related issues, Changes (bullet list), and a formal Checklist.	Add the missing template sections: specify the type of change (appears to be a bug fix), reference any related GitHub issue, provide a bulleted Changes list, and complete the checklist with relevant items checked.
Docstring Coverage	⚠️ Warning	Docstring coverage is 15.91% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: fixing Redis fail-open behavior for auth operations, which is the core focus of the PR's changes across middleware, auth service, and documentation.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix-security-report-item-8

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}