forgejo_admin/basketball-api

Fork 1

Replace pal-e-auth with Keycloak OIDC JWT validation #78

Merged

forgejo_admin merged 2 commits from 77-replace-pal-e-auth-with-keycloak-oidc-jw into main

2026-03-14 18:55:04 +00:00

forgejo_admin commented

2026-03-14 16:35:43 +00:00

Owner

Summary

Replace the custom pal-e-auth-ldraney library with direct Keycloak JWT validation via JWKS. Tokens are validated using RS256 public keys from the realm's JWKS endpoint -- no client secret needed in basketball-api.

Changes

src/basketball_api/auth.py (NEW) -- User dataclass, get_current_user async dependency (validates JWTs via JWKS with key rotation support), require_role factory for role-checking dependencies
src/basketball_api/config.py -- Replace jwt_secret_key, google_client_id, google_client_secret with single keycloak_realm_url setting (default: in-cluster Keycloak service)
src/basketball_api/main.py -- Remove all pal_e_auth imports (AuthConfig, auth_router), _build_auth_config(), app.state.auth_config, and auth router registration
src/basketball_api/routes/admin.py -- Import User, require_role from basketball_api.auth
src/basketball_api/routes/roster.py -- Same import update
src/basketball_api/routes/tryouts.py -- Same import update
pyproject.toml -- Remove pal-e-auth-ldraney>=0.1.0, add python-jose[cryptography]>=3.3 and httpx>=0.27
k8s/deployment.yaml -- Remove pal-e-auth-secrets Secret references (jwt-secret-key, google-client-id, google-client-secret), add BASKETBALL_KEYCLOAK_REALM_URL env var
tests/conftest.py -- Remove Google OAuth env vars, remove _auth_config usage, set BASKETBALL_KEYCLOAK_REALM_URL for tests
tests/test_admin.py -- Import User from basketball_api.auth
tests/test_tryouts.py -- Import User from basketball_api.auth, remove _auth_config import and usage

Test Plan

All 157 existing tests pass (pytest tests/ -v)
Protected endpoints return 401 without valid JWT (verified by existing auth tests)
Public endpoints work without auth header
Ruff check and format pass clean
Zero remaining pal_e_auth / pal-e-auth references in src/, tests/, k8s/, pyproject.toml

Review Checklist

No secrets committed
No unnecessary file changes
Commit messages are descriptive

Closes #77
plan-pal-e-platform -- Phase 5 (Keycloak IdP integration)

## Summary Replace the custom `pal-e-auth-ldraney` library with direct Keycloak JWT validation via JWKS. Tokens are validated using RS256 public keys from the realm's JWKS endpoint -- no client secret needed in basketball-api. ## Changes - **`src/basketball_api/auth.py`** (NEW) -- `User` dataclass, `get_current_user` async dependency (validates JWTs via JWKS with key rotation support), `require_role` factory for role-checking dependencies - **`src/basketball_api/config.py`** -- Replace `jwt_secret_key`, `google_client_id`, `google_client_secret` with single `keycloak_realm_url` setting (default: in-cluster Keycloak service) - **`src/basketball_api/main.py`** -- Remove all `pal_e_auth` imports (`AuthConfig`, `auth_router`), `_build_auth_config()`, `app.state.auth_config`, and auth router registration - **`src/basketball_api/routes/admin.py`** -- Import `User`, `require_role` from `basketball_api.auth` - **`src/basketball_api/routes/roster.py`** -- Same import update - **`src/basketball_api/routes/tryouts.py`** -- Same import update - **`pyproject.toml`** -- Remove `pal-e-auth-ldraney>=0.1.0`, add `python-jose[cryptography]>=3.3` and `httpx>=0.27` - **`k8s/deployment.yaml`** -- Remove `pal-e-auth-secrets` Secret references (jwt-secret-key, google-client-id, google-client-secret), add `BASKETBALL_KEYCLOAK_REALM_URL` env var - **`tests/conftest.py`** -- Remove Google OAuth env vars, remove `_auth_config` usage, set `BASKETBALL_KEYCLOAK_REALM_URL` for tests - **`tests/test_admin.py`** -- Import `User` from `basketball_api.auth` - **`tests/test_tryouts.py`** -- Import `User` from `basketball_api.auth`, remove `_auth_config` import and usage ## Test Plan - [x] All 157 existing tests pass (`pytest tests/ -v`) - [x] Protected endpoints return 401 without valid JWT (verified by existing auth tests) - [x] Public endpoints work without auth header - [x] Ruff check and format pass clean - [x] Zero remaining `pal_e_auth` / `pal-e-auth` references in src/, tests/, k8s/, pyproject.toml ## Review Checklist - [x] No secrets committed - [x] No unnecessary file changes - [x] Commit messages are descriptive ## Related - Closes #77 - `plan-pal-e-platform` -- Phase 5 (Keycloak IdP integration)

forgejo_admin added 1 commit

2026-03-14 16:35:43 +00:00

Replace pal-e-auth with Keycloak OIDC JWT validation

ci/woodpecker/pr/woodpecker Pipeline was successful

Details

85b0783e07

Swap the custom pal-e-auth-ldraney library for direct Keycloak JWT
validation via JWKS so the auth chain is unified through Keycloak IdP.
No client secret needed -- tokens are validated using RS256 public keys
from the realm's JWKS endpoint.

- Create src/basketball_api/auth.py with User dataclass, get_current_user,
  and require_role dependencies
- Replace jwt_secret_key/google_client_id/google_client_secret config
  with single keycloak_realm_url setting
- Remove AuthConfig, auth_router, and app.state.auth_config from main.py
- Update admin.py, roster.py, tryouts.py imports
- Replace pal-e-auth-ldraney dep with python-jose[cryptography] + httpx
- Replace pal-e-auth-secrets k8s Secret with BASKETBALL_KEYCLOAK_REALM_URL
- Update test fixtures to remove Google OAuth env vars and auth_config

All 157 tests pass. Ruff clean.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented

2026-03-14 16:36:20 +00:00

Author

Owner

Self-Review

Reviewed all 11 changed files (195 additions, 59 deletions).

Findings

No blocking issues.

Area	Status	Notes
auth.py (new)	OK	JWKS caching with key rotation retry, RS256 validation, proper 401/403 responses
config.py	OK	3 settings replaced by 1 (`keycloak_realm_url`), sensible in-cluster default
main.py	OK	All pal-e-auth imports, `_build_auth_config()`, `auth_router`, `app.state.auth_config` removed. Stripe lifespan preserved
Route files	OK	Import-only changes (`pal_e_auth` -> `basketball_api.auth`)
pyproject.toml	OK	`pal-e-auth-ldraney` replaced with `python-jose[cryptography]` + `httpx`
k8s/deployment.yaml	OK	3 secret-ref env vars replaced by 1 plain-text in-cluster URL. No secrets in value
Tests	OK	Auth env vars updated, `_auth_config` references removed, mock pattern preserved
Ruff	OK	Check and format both pass clean
Tests	OK	All 157 tests pass

Nit (non-blocking): httpx>=0.27 now appears in both main dependencies and [project.optional-dependencies] dev. Harmless (pip deduplicates), but could remove from dev deps in a follow-up.

Verdict: Ready for human review.

## Self-Review Reviewed all 11 changed files (195 additions, 59 deletions). ### Findings **No blocking issues.** | Area | Status | Notes | |------|--------|-------| | auth.py (new) | OK | JWKS caching with key rotation retry, RS256 validation, proper 401/403 responses | | config.py | OK | 3 settings replaced by 1 (`keycloak_realm_url`), sensible in-cluster default | | main.py | OK | All pal-e-auth imports, `_build_auth_config()`, `auth_router`, `app.state.auth_config` removed. Stripe lifespan preserved | | Route files | OK | Import-only changes (`pal_e_auth` -> `basketball_api.auth`) | | pyproject.toml | OK | `pal-e-auth-ldraney` replaced with `python-jose[cryptography]` + `httpx` | | k8s/deployment.yaml | OK | 3 secret-ref env vars replaced by 1 plain-text in-cluster URL. No secrets in value | | Tests | OK | Auth env vars updated, `_auth_config` references removed, mock pattern preserved | | Ruff | OK | Check and format both pass clean | | Tests | OK | All 157 tests pass | **Nit (non-blocking):** `httpx>=0.27` now appears in both main `dependencies` and `[project.optional-dependencies] dev`. Harmless (pip deduplicates), but could remove from dev deps in a follow-up. ### Verdict: Ready for human review.

forgejo_admin commented

2026-03-14 16:38:13 +00:00

Author

Owner

PR #78 Review

BLOCKERS

README.md not updated -- stale auth references remain.
Two locations in /README.md still reference the removed pal-e-auth library and its infrastructure:
- Line 54: pal-e-auth-secrets K8s secret is listed under Prerequisites.
- Line 34: /auth/google endpoint is listed in the endpoint table (this route was removed with auth_router).
- Line 70: Auth section references pal-e-auth library and /auth/google.
These must be updated to reflect the Keycloak OIDC flow. The Prerequisites should list BASKETBALL_KEYCLOAK_REALM_URL (or note it defaults to the in-cluster URL). The endpoint table should remove /auth/google. The Auth section should describe Keycloak realm token validation.

NITS

verify_aud: False is intentional but undocumented. The jwt.decode() call disables audience verification (line 133 of auth.py). This is a reasonable choice since basketball-api accepts any token from the westside-basketball realm regardless of client, but a code comment explaining the rationale would help future reviewers understand this is deliberate and not an oversight.
Module-level _jwks_cache is synchronous. The _fetch_jwks() function uses synchronous httpx.get() inside an async def get_current_user() dependency. This blocks the event loop during the initial JWKS fetch (and during rotation retries). With a single Keycloak instance on the same cluster the latency is negligible, but if you later move to an external IdP or experience network delays, consider switching to httpx.AsyncClient. Not blocking for current deployment topology.
Test mocks set mock_user.role (singular) which is not a field on the new User dataclass. In tests/test_admin.py (line 45) and tests/test_tryouts.py (line 108), the mock sets .role = "admin" but the new User dataclass has .roles (list). This works because MagicMock allows arbitrary attributes and the route code never reads .role directly (the role check is inside require_role() which is overridden). Still, updating the mocks to set .roles = ["admin"] would be more accurate and prevent confusion.

SOP COMPLIANCE

Branch named after issue (77-replace-pal-e-auth-with-keycloak-oidc-jw references issue #77)
PR body has: Summary, Changes, Test Plan, Related
Related section references the plan slug (plan-pal-e-platform)
Closes #77 present in PR body
No secrets committed
No .env files committed
No unnecessary file changes (scope is tight -- 11 files, all directly related)
Commit messages are descriptive
README.md not updated (stale pal-e-auth references -- see BLOCKER #1)

SECURITY NOTES (all positive)

RS256 algorithm hardcoded -- no algorithm confusion attack vector.
kid matching with rotation retry -- handles Keycloak key rotation correctly.
Issuer verification enabled -- tokens from other realms are rejected.
JWKS cached at module level with cache-bust-and-retry on unknown kid -- good balance of performance and rotation handling.
HTTPBearer(auto_error=False) with explicit None check -- clean 401 for missing tokens.
401 for auth failures, 403 for insufficient roles -- correct HTTP semantics.

VERDICT: NOT APPROVED

The README blocker must be fixed. The stale pal-e-auth-secrets prerequisite and /auth/google endpoint reference will mislead anyone deploying or onboarding to this codebase. Once the README is updated to reflect Keycloak auth, this is ready to merge.

## PR #78 Review ### BLOCKERS 1. **README.md not updated -- stale auth references remain.** Two locations in `/README.md` still reference the removed `pal-e-auth` library and its infrastructure: - Line 54: `pal-e-auth-secrets` K8s secret is listed under Prerequisites. - Line 34: `/auth/google` endpoint is listed in the endpoint table (this route was removed with `auth_router`). - Line 70: Auth section references `pal-e-auth` library and `/auth/google`. These must be updated to reflect the Keycloak OIDC flow. The Prerequisites should list `BASKETBALL_KEYCLOAK_REALM_URL` (or note it defaults to the in-cluster URL). The endpoint table should remove `/auth/google`. The Auth section should describe Keycloak realm token validation. ### NITS 1. **`verify_aud: False` is intentional but undocumented.** The `jwt.decode()` call disables audience verification (line 133 of `auth.py`). This is a reasonable choice since basketball-api accepts any token from the `westside-basketball` realm regardless of client, but a code comment explaining the rationale would help future reviewers understand this is deliberate and not an oversight. 2. **Module-level `_jwks_cache` is synchronous.** The `_fetch_jwks()` function uses synchronous `httpx.get()` inside an `async def get_current_user()` dependency. This blocks the event loop during the initial JWKS fetch (and during rotation retries). With a single Keycloak instance on the same cluster the latency is negligible, but if you later move to an external IdP or experience network delays, consider switching to `httpx.AsyncClient`. Not blocking for current deployment topology. 3. **Test mocks set `mock_user.role` (singular) which is not a field on the new `User` dataclass.** In `tests/test_admin.py` (line 45) and `tests/test_tryouts.py` (line 108), the mock sets `.role = "admin"` but the new `User` dataclass has `.roles` (list). This works because `MagicMock` allows arbitrary attributes and the route code never reads `.role` directly (the role check is inside `require_role()` which is overridden). Still, updating the mocks to set `.roles = ["admin"]` would be more accurate and prevent confusion. ### SOP COMPLIANCE - [x] Branch named after issue (`77-replace-pal-e-auth-with-keycloak-oidc-jw` references issue #77) - [x] PR body has: Summary, Changes, Test Plan, Related - [x] Related section references the plan slug (`plan-pal-e-platform`) - [x] `Closes #77` present in PR body - [x] No secrets committed - [x] No `.env` files committed - [x] No unnecessary file changes (scope is tight -- 11 files, all directly related) - [x] Commit messages are descriptive - [ ] README.md not updated (stale `pal-e-auth` references -- see BLOCKER #1) ### SECURITY NOTES (all positive) - RS256 algorithm hardcoded -- no algorithm confusion attack vector. - `kid` matching with rotation retry -- handles Keycloak key rotation correctly. - Issuer verification enabled -- tokens from other realms are rejected. - JWKS cached at module level with cache-bust-and-retry on unknown `kid` -- good balance of performance and rotation handling. - `HTTPBearer(auto_error=False)` with explicit `None` check -- clean 401 for missing tokens. - 401 for auth failures, 403 for insufficient roles -- correct HTTP semantics. ### VERDICT: NOT APPROVED The README blocker must be fixed. The stale `pal-e-auth-secrets` prerequisite and `/auth/google` endpoint reference will mislead anyone deploying or onboarding to this codebase. Once the README is updated to reflect Keycloak auth, this is ready to merge.

forgejo_admin added 1 commit

2026-03-14 16:40:48 +00:00

Fix QA findings: remove stale pal-e-auth references, add auth.py comments, fix test mocks

ci/woodpecker/pr/woodpecker Pipeline was successful

Details

058bd6a3c0

BLOCKER fixes:
- README: remove /auth/google endpoint row (route was removed)
- README: replace pal-e-auth-secrets prereq with BASKETBALL_KEYCLOAK_REALM_URL env var
- README: rewrite Auth section to describe Keycloak OIDC JWT validation via JWKS

NIT fixes:
- auth.py: add comment explaining why verify_aud is False (Keycloak audience varies)
- auth.py: add comment explaining why synchronous httpx is acceptable for JWKS fetch
- test_admin.py, test_tryouts.py: fix mock_user.role -> mock_user.roles to match User dataclass

Closes #77

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>