Skip to content

Add OAuth 2.1 authorization module#181

Merged
RafaelPo merged 5 commits intomainfrom
feat/oauth-auth
Feb 21, 2026
Merged

Add OAuth 2.1 authorization module#181
RafaelPo merged 5 commits intomainfrom
feat/oauth-auth

Conversation

@RafaelPo
Copy link
Contributor

@RafaelPo RafaelPo commented Feb 21, 2026

Summary

  • Adds EveryRowAuthProvider implementing the full OAuth 2.1 Authorization Server flow (dynamic client registration, PKCE authorization, token exchange/refresh) with user login delegated to Supabase Google OAuth
  • Adds SupabaseTokenVerifier for JWKS-based JWT verification with a Redis-backed token deny-list
  • Auth codes and refresh tokens are consumed atomically via GETDEL to prevent replay and race conditions; client registration is rate-limited; refresh tokens enforce OAuth 2.1 §6.3 scope narrowing
  • Uses module-level http_settings singleton from config.py; URL fields are stripped of trailing slashes via HttpSettings validator
  • Adds PyJWT[crypto] and httpx as dependencies; uses TYPE_CHECKING import for redis.asyncio to keep stdio mode lightweight

New files

  • everyrow-mcp/src/everyrow_mcp/auth.py — OAuth provider + token verifier (540 lines)
  • everyrow-mcp/tests/test_auth.py — 31 tests covering JWT verification, deny-list propagation, required claims, JWKS lock concurrency, atomic code consumption, scope narrowing, redirect URI validation, rate limiting, token revocation, client ID mismatch, and input length validation (717 lines)

Key design decisions

  • http_settings singleton (not passed via constructor) — provider methods reference http_settings.* directly
  • JWT verify first, then deny-list check — avoids a Redis hit on garbage tokens
  • _issue_token_response shared helper — DRYs exchange_authorization_code and exchange_refresh_token
  • _supabase_token_request shared helper — DRYs the two Supabase /token calls
  • aclose() on provider for clean httpx shutdown
  • Cookies hardcode secure=True (HTTPS-only in production)
  • Deny-list errors propagate (no fail-open) — if Redis is down, verification fails

PR context

PR 3 of 5 in the HTTP transport split. Depends on PR1 (#179, foundation utilities) and PR2 (#180, Redis infrastructure), both already merged. Does not touch app.py, tools.py, server.py, or models.py.

Test plan

  • All 96 tests pass (uv run pytest tests/ -x)
  • Pre-commit hooks pass (format, lint, typecheck)
  • CI green
  • Reviewer confirms auth.py imports resolve correctly against merged PR1/PR2 modules (config.py, redis_utils.py)

🤖 Generated with Claude Code

@RafaelPo RafaelPo force-pushed the feat/oauth-auth branch 3 times, most recently from b1ba1d8 to e400049 Compare February 21, 2026 14:26
- EveryRowAuthProvider: full OAuth 2.1 AS with 3-leg PKCE chain
  (Google -> Supabase -> MCP Server -> Claude)
- SupabaseTokenVerifier: JWKS-based JWT verification with token deny-list
- Redis-backed storage for auth codes, refresh tokens, client registrations
- Atomic GETDEL operations to prevent token replay/race conditions
- Per-IP rate limiting on handle_start and handle_callback routes
- Access token revocation via deny-list integration
- Scope narrowing on refresh per OAuth 2.1 spec
- Input length validation on auth codes and refresh tokens
- Unconditional redirect_uri validation
- JWKS fetch timeout (10s) to prevent lock contention
- All TTLs and rate limits configurable via pydantic-settings with bounds validation

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ttpSettings

Replace all self._settings / self.supabase_url / self.mcp_server_url /
self.supabase_anon_key references in EveryRowAuthProvider with the
http_settings module-level singleton from config.py. Add _strip_trailing_slash
field validator to HttpSettings for URL fields. Simplify constructor to only
take redis + token_verifier. Update tests to match.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@RafaelPo RafaelPo requested a review from rgambee February 21, 2026 15:47
RafaelPo and others added 2 commits February 21, 2026 16:14
- Add back from __future__ import annotations (fixes Python 3.12 CI)
- Make _supabase_redirect_url and _validate_redirect_url @staticmethod
- Extract _supabase_token_request to DRY _exchange_supabase_code / _refresh_supabase_token
- Extract _create_authorisation_code and _create_callback_redirect_response
- Hardcode secure=True on auth cookies
- Type pending param in _create_callback_redirect_response

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Extract _is_revoked, _get_signing_key, _decode_jwt from verify_token
- Keep deny-list check after JWT verification (avoid Redis hit on garbage tokens)
- Extract _issue_token_response to DRY exchange_authorization_code / exchange_refresh_token
- Extract _create_authorisation_code and _create_callback_redirect_response
- Hardcode secure=True on auth cookies

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@RafaelPo
Copy link
Contributor Author

Migration to Supabase OAuth 2.1

This auth module currently implements a PKCE relay: we sit between Claude and Supabase, generating our own PKCE pair for the Supabase leg and managing auth codes / refresh tokens in Redis. Supabase is working on native OAuth 2.1 support, which would let Claude talk directly to Supabase as the authorization server — eliminating the relay and most of this module.

Not yet viable for two reasons:

  1. Still in beta
  2. Requires a consent screen — Supabase OAuth 2.1 mandates that users explicitly approve scopes before a token is issued. We don't currently have a consent UI, so we'd need to design and host one. (Needs to be double-checked)

@RafaelPo RafaelPo merged commit 6810ed4 into main Feb 21, 2026
6 checks passed
@RafaelPo RafaelPo deleted the feat/oauth-auth branch February 21, 2026 19:28
RafaelPo added a commit that referenced this pull request Feb 21, 2026
Resolve conflicts keeping feat/http-transport versions for all source
and test files (main got simplified subsets via PRs #179-#181; this
branch has the more evolved implementations). Keep redis[hiredis] and
ARG002 lint ignore from main.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
RafaelPo added a commit that referenced this pull request Feb 21, 2026
Keep main's canonical versions from PRs #179-#181 (foundation utilities,
Redis infrastructure, OAuth auth). Adapt http-transport code to use main's
RedisStore API (state.store.method() instead of state.method()).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@hnykda
Copy link
Contributor

hnykda commented Feb 23, 2026

OK, looks good!

Some claude's code review, which I think is valid but I don't see us doing these suggestions, it's good.

Details

  Code Review of #181

  What's good

  - GETDEL for auth codes and refresh tokens — correct one-time-use semantics, prevents replay
  - PKCE end-to-end — our server generates its own PKCE pair for the Supabase leg, separate from the client's PKCE
  - JWT verify-first, then deny-list — avoids a Redis hit on garbage tokens
  - _UNSAFE_decode_server_jwt naming — loudly signals the danger zone; usage is actually safe since the token just came from Supabase over HTTPS, never from
  the client
  - Cookie scoping — path="/auth/callback", httponly, secure
  - Key injection prevention — build_key() sanitizes colons
  - Test coverage — 31 tests covering JWT, deny-list, JWKS concurrency, atomic consumption, scope narrowing, rate limiting

  Issues found

  1. Refresh token rotation is incomplete (Critical)

  load_refresh_token() (line 474) uses GETDEL — good, the old token is consumed. But look at the flow: the MCP SDK calls load_refresh_token() first, then
  exchange_refresh_token(). If load_refresh_token succeeds but exchange_refresh_token fails (e.g., Supabase is down), the old refresh token is already deleted
  and the user is locked out with no recovery path.

  This is actually the correct OAuth 2.1 behavior (rotate on use), but worth noting as an operational risk. Consider whether you want a brief grace period or
  retry mechanism.

  2. Auth code expiration not checked in application code (Medium)

  load_authorization_code() (line 407) relies entirely on Redis TTL for expiration. If Redis TTL drifts or fails to expire the key, an expired code could be
  accepted. Add a defensive check:

  if code_obj.expires_at and code_obj.expires_at < time.time():
      return None

  3. Rate limiting has a sliding-window reset bug (Medium)

  Lines 202-208: the pipeline calls INCR then EXPIRE on every request. This resets the TTL window each time, so a sustained attacker at exactly the rate limit
  never gets blocked — the window keeps sliding forward. Fix: only set the TTL on the first increment (if count == 1), or use a Lua script for atomicity.

  4. Revocation TTL may be shorter than JWT lifetime (Medium)

  revocation_ttl defaults to 3600s, but Supabase JWTs can also be valid for up to 3600s. If a token is revoked near the start of its lifetime, the deny-list
  entry expires at the same time as the JWT — tight margin. If there's any clock skew, a revoked token could become usable again. Consider setting the
  revocation TTL to the token's remaining lifetime (exp - now) plus a small buffer.

  5. exc_info=True in JWT error logging (Low-Medium)

  Line 109: logger.debug("JWT verification failed", exc_info=True) — depending on the PyJWT version, the exception message could contain token fragments. Safer
   to log just the exception type:

  logger.debug("JWT verification failed: %s", type(e).__name__)

  6. samesite="lax" on the state cookie (Low)

  Lines 363, 402: OAuth 2.1 best practice is samesite="strict" for state cookies. lax allows the cookie to be sent on top-level navigations from cross-site
  contexts, which is slightly weaker.

  7. No validation of Supabase response structure (Low)

  _supabase_token_request() (line 523) accesses data["access_token"] directly — a KeyError if Supabase returns an unexpected response. Wrapping in a try/except
   or using .get() with validation would be more resilient.

  Test gaps

  - No test for auth code expiration boundary (time-based)
  - No test for what happens when Supabase returns malformed JSON
  - No test verifying cookie security attributes on the response object
  - Rate limiting boundary test exists but doesn't test the sliding-window reset issue

  ---
  Summary

  The architecture is sound — the OAuth 2.1 AS pattern is the right approach given MCP's requirements, Redis is justified for multi-pod state, and the code is
  well-structured. The most actionable items are:

  1. Add defensive expiration check on auth code load
  2. Fix rate limiting to not reset the window on every request
  3. Align revocation TTL with actual JWT remaining lifetime
  4. Consider the refresh token loss scenario if Supabase fails mid-exchange

  None of these are blocking for the overall series, but items 1-3 should be addressed before #183 ships to production.

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.

2 participants