Skip to content

auth: centralize token revocation checks and resolve rate-limit client addresses - #750

Merged
madfam-io merged 10 commits into
mainfrom
fix/access-token-revocation
Oct 28, 2026
Merged

madfam-io merged 10 commits into
mainfrom
fix/access-token-revocation

Conversation

@madfam-io

Copy link
Copy Markdown
Collaborator

Summary

  • Revocation: every bearer-authentication path (get_current_user and its optional/GraphQL variants, /authorize, token exchange, and the service boundaries) now shares one revocation check: a single Redis EXISTS over the token's jti and session-family keys. Session access tokens carry the session family, so revoking a session also applies to tokens issued before a refresh.
  • Redis unavailable: the existing fail-closed policy is kept (503 with Retry-After). Sign-out is exempt.
  • Rate limits: limiters key on the client address resolved through TRUSTED_PROXIES, the same resolver the sign-in link limits already use, instead of the socket peer.
  • Deploy: TRUSTED_PROXIES adds the pod network (10.42.0.0/16) to the localhost defaults. The NetworkPolicy admits only tunnel and application pods to the API port.

Tests

  • test_access_token_revocation.py: 16 tests.
  • Rate-limit key tests: 12.
  • Authorize-flow tests now get a per-test Redis: 41/41.
  • Remaining unit-suite failures are the same set as on main.

Follow-ups

  • The request log and global middleware still read X-Forwarded-For without the trusted-proxy check.
  • The lockout counter policy after a lock expires.

🤖 Generated with Claude Code

aldoruizluna and others added 4 commits October 27, 2026 13:28
Bearer verification now goes through one revocation-list read shared by
get_current_user, the issuer's session reads at /authorize, token exchange
and the service-token boundaries. Session access tokens carry the session's
rotation family so session-level revocation reaches them. Sign-out and
session revocation endpoints get dedicated dependencies so they keep
working while Redis is unavailable.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
All slowapi limiters now use one key function that resolves the client
address through TRUSTED_PROXIES, the same resolution the sign-in link
limits already use (IPv6 per /64).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Rate-limit keys now resolve the client address through TRUSTED_PROXIES.
Trust the pod network so the tunnel's forwarded client address is used.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

📚 Documentation checks found issues. Please review the maintenance script output.

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Code quality checks found issues. Please review the maintenance script output.

@github-actions

Copy link
Copy Markdown
Contributor

Security Scan Summary

Date: Tue Oct 27 23:12:40 UTC 2026
Commit: cf9b7bf46c5486036ea27c907e69dd341d94a981

Scan Results

  • CodeQL: success
  • Python Security: success
  • JavaScript Security: success
  • Secrets Scan: success
  • Container Scan: success
  • DAST ZAP: skipped

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

📚 Documentation checks found issues. Please review the maintenance script output.

@github-actions

Copy link
Copy Markdown
Contributor

🎭 Playwright E2E Test Results

  • Total: 11
  • Passed: 11 ✅
  • Failed: 0 ❌
  • Flaky: 0 ⚠️
  • Skipped: 0 ⏭️

Full Report

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Code quality checks found issues. Please review the maintenance script output.

@github-actions

Copy link
Copy Markdown
Contributor

Security Scan Summary

Date: Tue Oct 27 23:17:24 UTC 2026
Commit: e7cd06b

Scan Results

  • CodeQL: success
  • Python Security: success
  • JavaScript Security: success
  • Secrets Scan: success
  • Container Scan: success
  • DAST ZAP: skipped

@github-actions

Copy link
Copy Markdown
Contributor

🎭 Playwright E2E Test Results

  • Total: 11
  • Passed: 11 ✅
  • Failed: 0 ❌
  • Flaky: 0 ⚠️
  • Skipped: 0 ⏭️

Full Report

@github-actions

Copy link
Copy Markdown
Contributor

📚 Documentation checks found issues. Please review the maintenance script output.

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Code quality checks found issues. Please review the maintenance script output.

@github-actions

Copy link
Copy Markdown
Contributor

Security Scan Summary

Date: Wed Oct 28 00:18:59 UTC 2026
Commit: d8e14ae

Scan Results

  • CodeQL: success
  • Python Security: success
  • JavaScript Security: success
  • Secrets Scan: success
  • Container Scan: success
  • DAST ZAP: skipped

@github-actions

Copy link
Copy Markdown
Contributor

🎭 Playwright E2E Test Results

  • Total: 11
  • Passed: 11 ✅
  • Failed: 0 ❌
  • Flaky: 0 ⚠️
  • Skipped: 0 ⏭️

Full Report

…on fixture override

is_revoked called strict_exists on a missing client, which answered 500.
A missing or non-strict client now raises RedisUnavailableError, so strict
paths answer 503 like any outage. The invitation Postgres fixture also
overrides the get_redis that app.dependencies declares (tests/conftest.py
swaps the module attribute after it was imported). 25/25 pass locally
against a synthetic PostgreSQL + Redis.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

📚 Documentation checks found issues. Please review the maintenance script output.

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Code quality checks found issues. Please review the maintenance script output.

@github-actions

Copy link
Copy Markdown
Contributor

Security Scan Summary

Date: Wed Oct 28 04:16:17 UTC 2026
Commit: 0774514

Scan Results

  • CodeQL: success
  • Python Security: success
  • JavaScript Security: success
  • Secrets Scan: success
  • Container Scan: success
  • DAST ZAP: skipped

@github-actions

Copy link
Copy Markdown
Contributor

🎭 Playwright E2E Test Results

  • Total: 11
  • Passed: 11 ✅
  • Failed: 0 ❌
  • Flaky: 0 ⚠️
  • Skipped: 0 ⏭️

Full Report

…akeRedis in tests

verify_unrevoked_token received the raw FakeRedis that tests/conftest.py
hands out, which has no strict read, and that single instance was bound to
the first test's event loop (later tests: 'bound to a different event
loop' -> 500). is_revoked now wraps a raw client in ResilientRedisClient
(still fails closed on no client), and get_fake_redis returns one instance
per running loop. Full unit suite: the two mail-boundary failures are gone;
remaining local failures are environment-only (xmlsec, defusedxml, old
cryptography) and identical without this change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

📚 Documentation checks found issues. Please review the maintenance script output.

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Code quality checks found issues. Please review the maintenance script output.

… set

CI lints only changed files. Import-order and dict-comprehension fixes
(ruff --fix) and removal of the first mock_redis fixture, which the later
definition already shadowed at import (no behaviour change).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

📚 Documentation checks found issues. Please review the maintenance script output.

@github-actions

Copy link
Copy Markdown
Contributor

Security Scan Summary

Date: Wed Oct 28 05:54:00 UTC 2026
Commit: 57efd77

Scan Results

  • CodeQL: success
  • Python Security: success
  • JavaScript Security: success
  • Secrets Scan: success
  • Container Scan: success
  • DAST ZAP: skipped

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Code quality checks found issues. Please review the maintenance script output.

if _fake_redis_instance is None:
_fake_redis_instance = fakeredis.aioredis.FakeRedis(decode_responses=True)
return _fake_redis_instance
import asyncio
@github-actions

Copy link
Copy Markdown
Contributor

Security Scan Summary

Date: Wed Oct 28 05:56:47 UTC 2026
Commit: fbbc9a0

Scan Results

  • CodeQL: success
  • Python Security: success
  • JavaScript Security: success
  • Secrets Scan: success
  • Container Scan: success
  • DAST ZAP: skipped

@github-actions

Copy link
Copy Markdown
Contributor

🎭 Playwright E2E Test Results

  • Total: 11
  • Passed: 11 ✅
  • Failed: 0 ❌
  • Flaky: 0 ⚠️
  • Skipped: 0 ⏭️

Full Report

@github-actions

Copy link
Copy Markdown
Contributor

🎭 Playwright E2E Test Results

  • Total: 11
  • Passed: 11 ✅
  • Failed: 0 ❌
  • Flaky: 0 ⚠️
  • Skipped: 0 ⏭️

Full Report

Strict token-revocation reads need a reachable Redis. The invitation
proofs connect to 127.0.0.1:6379 in CI; without a service every bearer
request in them failed with ConnectionError -> 500. Mirrors the api-tests
job's Redis service.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

📚 Documentation checks found issues. Please review the maintenance script output.

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Code quality checks found issues. Please review the maintenance script output.

@github-actions

Copy link
Copy Markdown
Contributor

Security Scan Summary

Date: Wed Oct 28 09:46:01 UTC 2026
Commit: 8eddaa5

Scan Results

  • CodeQL: success
  • Python Security: success
  • JavaScript Security: success
  • Secrets Scan: success
  • Container Scan: success
  • DAST ZAP: skipped

@github-actions

Copy link
Copy Markdown
Contributor

🎭 Playwright E2E Test Results

  • Total: 11
  • Passed: 11 ✅
  • Failed: 0 ❌
  • Flaky: 0 ⚠️
  • Skipped: 0 ⏭️

Full Report

@madfam-io
madfam-io merged commit abca35f into main Oct 28, 2026
36 checks passed
@madfam-io
madfam-io deleted the fix/access-token-revocation branch October 28, 2026 10:37
github-actions Bot pushed a commit that referenced this pull request Oct 28, 2026
Reason: manual:Owner-approved: #746 critical dependency fixes + #750 auth hardening (shared revocation checks, trusted client address for rate limits)
Promoted-by: madfam-io
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants