feat: opt-in encrypted Redis store for OAuth proxy state (#52)

[ml-aixolotl]

Closes #52.

Adds an opt-in, encrypted Redis store for the OAuth proxy's state. With it, sign-ins survive container recreation, scale-to-zero and multiple replicas. A fixed JWT_SIGNING_KEY also lets you rotate the Entra secret without signing everyone out. With nothing set, the provider is built exactly as before.

Evidence (driven against real Entra + Graph; tokens redacted)

The client's own POST /token refresh_token grant, replayed verbatim:

Scenario	Before (main)	After (REDIS_URL set)
Refresh token issued before a restart, server restarted on an empty disk store (= container recreation)	401 {"error":"invalid_client","error_description":"Invalid client_id"}	200 {"access_token":"<redacted>","token_type":"Bearer","expires_in":5208,...}
Second replica (separate container, same BASE_URL + Redis) refreshing a token replica A issued	401 invalid_client ¹	200, then get_me with B's new token → OK
Server restarted with a rotated CLIENT_SECRET ¹	401 invalid_client (registration unknown)	401 invalid_grant: registration still readable (with JWT_SIGNING_KEY + STORAGE_ENCRYPTION_KEY set)
After the review fixes: rebuilt image with REDIS_URL + both keys, docker rm -f + new container, refresh	—	200, then get_me with the new token → OK
Real container: docker rm -f + docker run of the built image, then get_me with the access token issued before both restarts	—	get_me -> {'displayName': 'Martin Laukkanen', ...}

¹ Probed with a deliberately bogus refresh token: invalid_grant means a known client, invalid_client an unknown one. The rotation row therefore proves the registration survives a secret rotation, not a real refresh (with a rotated fake secret, Entra would reject the upstream refresh). FastMCP rotates the refresh token on every use, so reusing a consumed one returns invalid_grant by design.

What lands in Redis after a real sign-in (all entries __encrypted_data__, 0 in plaintext). TTLs are FastMCP's own:

mcp-refresh-tokens / mcp-upstream-tokens  ttl ≈ 2,592,000 s (30 days)
mcp-oauth-transactions                    ttl ≈ 900 s
mcp-oauth-proxy-clients                   no TTL (FastMCP stores registrations without one)

Root cause

src/auth_provider.py built AzureProvider without client_storage or jwt_signing_key. FastMCP then keeps all OAuth state in a FileTreeStore inside the container, at a path derived from CLIENT_SECRET. Recreating the container, scaling to zero, adding a replica, or rotating the secret each lose that state.

What changed

flowchart LR
    S[Settings] -->|REDIS_URL unset| D["AzureProvider(client_storage=None)<br/>FastMCP file store, as today"]
    S -->|REDIS_URL set| R["Redis.from_url(REDIS_URL)<br/>(keeps rediss:// TLS)"]
    R --> RS[RedisStore]
    K["STORAGE_ENCRYPTION_KEY<br/>or derived like FastMCP"] --> F[FernetEncryptionWrapper<br/>raise_on_decryption_error=False]
    RS --> F --> P["AzureProvider(client_storage=…,<br/>jwt_signing_key=JWT_SIGNING_KEY)"]

Loading

• src/config.py: adds REDIS_URL (RedisDsn: redis:///rediss:// only), JWT_SIGNING_KEY and STORAGE_ENCRYPTION_KEY (both SecretStr). hide_input_in_errors=True, so a malformed URL never prints the Redis password.
  • A bad Fernet key fails at startup with a clear error.
  • STORAGE_ENCRYPTION_KEY without REDIS_URL refuses to start, as decided at the gate.
  • env_ignore_empty=True applies to all settings: an empty VAR= now means "use the default" (e.g. ALLOWED_ORIGINS= falls back to the default rather than failing validation).
• src/auth_provider.py: _redis_client_storage() follows FastMCP's documented production pattern. With no explicit key, the encryption key is derived exactly as FastMCP derives its default store key, reusing its derive_jwt_key with the same salts.
• Trap avoided: RedisStore(url=...) rebuilds the client from host, port, db and password and drops TLS. rediss:// yields a plaintext Connection and fails against Azure Redis. So the client is built with Redis.from_url, which yields an SSLConnection, and that is pinned by a test.
• Reused: RedisStore and FernetEncryptionWrapper from the already-locked py-key-value-aio 0.4.4, which now gets the [redis] extra (adds redis). derive_jwt_key comes from FastMCP.
• Docs: README and .env.example explain when Redis is needed, that REDIS_URL is a credential, and that enabling it signs everyone out once (no migration, as decided at the gate).

Edge cases driven

• Invalid REDIS_URL → ValidationError: REDIS_URL — URL scheme should be 'redis' or 'rediss', password not echoed.
• Key without Redis → STORAGE_ENCRYPTION_KEY requires REDIS_URL.
• Entry encrypted with another key → a clean 401 invalid_client (reconnect) with no traceback.
• Redis down → 500 in 0.16 s, with a log naming redis.exceptions.ConnectionError ... :6380. Redis hung → 500 in 4.05 s (explicit 2 s timeouts + 1 retry; main's implicit defaults measured 5.0 s). Redis back → works again with no server restart. A 500 (retry) is deliberate here: a 401 would make clients discard their sign-in.
• JWT_SIGNING_KEY= (empty) → treated as unset. A key shorter than 32 characters or whitespace-only, or a non-numeric DB path, → startup error.
• Regression: list_tools → 18 tools, and list_my_tasks returns your tasks through the Redis-backed container.

Tests

• tests/test_config.py: defaults, redis/rediss accepted, invalid URL fails without leaking it, key-without-Redis, bad Fernet key.
• tests/test_auth_provider.py: no REDIS_URL → client_storage=None, jwt_signing_key=None. REDIS_URL → encrypted RedisStore (TLS kept for rediss://, DB index from the path). JWT_SIGNING_KEY forwarded. Encryption key: explicit, derived from the secret, and derived from JWT_SIGNING_KEY.
• tests/test_redis_storage_integration.py (real Redis; CI gains a redis:7-alpine service, and the test skips without REDIS_TEST_URL):
  • a client registered by one provider instance is found by a fresh one, and the key really lives in Redis (mutation-checked: returning the default store turns it red)
  • entries are encrypted and carry the TTL
  • a refresh token issued by one provider is redeemed by a fresh one (own empty FastMCP home = a recreated container) through FastMCP's real exchange_authorization_code / exchange_refresh_token, with only Entra's token endpoint stubbed (mutation-checked)
  • a rotated encryption key reads as a miss, not an exception
• The same refresh-after-restart flow was also driven against real Entra (evidence above).

/ponytail-review: one shrink applied (the consent-wiring test reuses the new helper, -14 lines). The Fernet validator and the FastMCP-mirroring key derivation are kept on purpose.

🤖 Generated with Claude Code

[Copilot]

Copilot review overview

🟡 Changes recommended

Blank signing keys create predictable encryption, and the required refresh-after-restart flow lacks integration coverage.

Review effort: Balanced
Findings: 1 · 1

Open (2)

• Reject blank JWT signing keys during settings validation · New
• Test refresh-token redemption after provider restart · New

What changed in this PR

Adds optional encrypted Redis persistence for OAuth proxy state, enabling resilient authentication across restarts and replicas.

Changes:

• Adds Redis-backed encrypted OAuth storage and fixed signing/encryption keys.
• Documents deployment configuration and migration behavior.
• Adds unit, integration, and CI Redis coverage.

File	Description
src/​config.py	Adds Redis and encryption settings.
src/​auth_provider.py	Configures encrypted Redis storage.
tests/​test_config.py	Tests storage configuration validation.
tests/​test_auth_provider.py	Tests provider storage wiring.
tests/​test_redis_storage_integration.py	Tests Redis persistence, encryption, and TTLs.
.github/​workflows/​ci.yml	Adds a Redis CI service.
pyproject.toml	Adds the Redis storage dependency.
uv.lock	Locks Redis dependencies.
README.md	Documents persistent OAuth storage.
.env.example	Adds example Redis settings.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[ml-aixolotl]

Dual-model review (Opus + Fable): findings and fixes

Both reviewers returned REQUEST_CHANGES. Each found a different HIGH. Both HIGHs and every consensus and MEDIUM finding are fixed in b3482bc and c1d3076.

#	Sev	Found by	Finding	Fix
1	HIGH	Opus	JWT_SIGNING_KEY= (the .env.example line uncommented, or an unset compose ${VAR}) became SecretStr(''). It is not None, so the storage key was derived from "", a publicly computable key. It is falsy, so JWT signing silently fell back to CLIENT_SECRET.	env_ignore_empty=True: empty means unset for every setting. JWT_SIGNING_KEY must be at least 32 characters. One consistent is not None check. Tests: test_empty_storage_setting_means_unset[*], test_short_jwt_signing_key_fails
2	HIGH	Fable	tests/test_auth_provider.py failed when run alone. The module executed twice under the mock, and the suite was green only through collection order. (The refactor in 711b16d introduced this.)	make_provider_kwargs imports before patching. uv run pytest tests/test_auth_provider.py → all pass when run alone
3	MEDIUM	consensus (Fable MEDIUM, Opus LOW)	Outage behaviour relied on redis-py's implicit timeouts and retries, and every tool call reads token state from Redis	Explicit socket_connect_timeout=2, socket_timeout=2, Retry(..., retries=1), pinned by test_redis_client_bounds_outage_latency. Measured with Redis hung: 500 in 4.05 s (main's defaults 5.0 s; a first attempt with 2 retries measured 6.2 s and was rejected)
4	MEDIUM	consensus (Opus MEDIUM, Fable LOW)	mcp-oauth-proxy-clients has no TTL (a FastMCP choice), so registrations and old-key leftovers accumulate in a shared Redis	Documented: give the store its own DB index or Redis, and use a non-silent eviction policy. Adding a TTL was not done: it would expire clients that are still active.
5	MEDIUM	Fable	The derived store key is coupled to FastMCP's internal salts and KDF	Kept (the issue's AC asks for a derived default), but fastmcp<4 is now bounded and the README and .env.example say set STORAGE_ENCRYPTION_KEY explicitly in production
6	MEDIUM	Opus	Azure access keys can contain /, which REDIS_URL rejects with an opaque error	README: percent-encoding one-liner
7	MEDIUM	Opus	Untested: a rotated key reading as a miss, and empty env values	Added test_rotated_encryption_key_reads_as_a_miss (real Redis; mutation-checked: raise_on_decryption_error=True turns it red) plus the empty-value tests
8	LOW	consensus	With JWT_SIGNING_KEY set, 1M-iteration PBKDF2 ran twice at import	Derived once and passed as bytes, which FastMCP uses verbatim. Same key.
9	LOW	consensus	repr(settings) showed the Redis password	REDIS_URL is Field(repr=False), tested
10	LOW	Opus	redis://h/abc silently used DB 0	Path must be a DB index, tested
11	LOW	Opus	Redis Cluster supports only DB 0	README: a non-clustered Redis is required
12	LOW	consensus	The key_value DeprecationWarning filter was package-wide	Narrowed to the setex message

Not changed, with reason:

• Integration tests don't close the clients they're handed (consensus LOW): the leak is test-only, and closing them means reaching into a private _client.
• Startup PING (Fable LOW): a lazy connection keeps /health and startup independent of Redis, and an outage now fails fast and is logged.
• MultiFernet rotation (Fable LOW): YAGNI until a rotation is planned.
• Shared test helpers in conftest (Fable LOW): cosmetic.

Verification after the fixes:

• uv run pytest: 282 passed with REDIS_TEST_URL, and real-Redis tests are skipped without it.
• tests/test_auth_provider.py passes when run alone.
• Rebuilt the Docker image and re-drove the Redis hung, down and recovered cases (above).
• Real Entra sign-in against the rebuilt image with REDIS_URL, JWT_SIGNING_KEY and STORAGE_ENCRYPTION_KEY set. This exercises the new bytes-forwarded signing key:
  • after docker rm -f and a new container, the pre-recreation refresh token gives POST /token → 200, and the new access token gives get_me → OK
  • real cross-replica refresh: replica B (separate container, same BASE_URL, same Redis) with the token replica A issued gives POST /token → 200, and B's new access token gives get_me → OK