diff --git a/.env.example b/.env.example index b229f94..b1a1547 100644 --- a/.env.example +++ b/.env.example @@ -36,12 +36,55 @@ MEILI_URL=http://meilisearch:7700 # Read by: MeilisearchClient.kt (also injected into meilisearch container) MEILI_MASTER_KEY= -# OAuth App client_id for /v1/auth/device/* proxy. Must equal the value -# the KMP client ships in BuildKonfig — both ends of the primary→fallback -# auth flow have to use the same OAuth App so device_codes interchange. +# OAuth App client_id for /v1/auth/device/* proxy and the web-flow exchange. +# Must equal the value the KMP client ships in BuildKonfig — both ends of +# the primary→fallback auth flow have to use the same OAuth App so +# device_codes interchange. # Source: github.com/settings/applications/ -# Read by: GitHubDeviceClient.kt -GITHUB_OAUTH_CLIENT_ID= +# Read by: GitHubDeviceClient.kt, OAuthExchangeService (via AppModule) +# (Renamed from GITHUB_OAUTH_CLIENT_ID — old name no longer read.) +OAUTH_CLIENT_ID= + +# OAuth App client_secret for the web-flow code exchange. NEVER commit a +# real value. Distinct from OAUTH_CLIENT_ID — the secret is only ever used +# server-side, and only by the backend (never by the client app or the +# website JS). Source: same GitHub OAuth App settings page. +# Read by: OAuthExchangeService (via AppModule) +OAUTH_CLIENT_SECRET= + +# Callback URL registered with the OAuth App. Must EXACTLY match what's in +# github.com/settings/applications/ → "Authorization callback URL". +# Used as redirect_uri on the token exchange; GitHub rejects with +# redirect_uri_mismatch if the values diverge. +# Read by: OAuthExchangeService (via AppModule) +OAUTH_WEB_CALLBACK_URL=https://github-store.org/auth/callback + +# Shared secret between this backend and the website (github-store.org). +# Website sends `X-Oauth-Service-Token: ` on every call to +# /v1/oauth/state and /v1/oauth/exchange. Both endpoints refuse the request +# (401 service_auth_required) if missing or wrong. Rotate via: +# openssl rand -base64 48 +# Set the SAME value on the website server. +# Read by: OAuthServiceAuth (via AppModule) +OAUTH_SERVICE_TOKEN= + +# Comma-separated allowlist of Host headers the OAuth S2S endpoints accept. +# Defence-in-depth on top of OAUTH_SERVICE_TOKEN — even if the secret leaks, +# the request still has to land on the canonical vhost. Production must set +# this; an empty value combined with APP_ENV=production refuses every S2S +# OAuth request. If you also serve OAuth via api-direct.github-store.org as +# a fallback vhost, append it: "api.github-store.org,api-direct.github-store.org". +# Read by: OAuthServiceAuth (via AppModule) +OAUTH_SERVICE_ALLOWED_HOSTS=api.github-store.org + +# Optional kill switch for the OAuthCleanupWorker. When set to "true", the +# 5-minute DELETE-of-expired-rows loop sleeps each cycle. Request-time +# `expires_at > NOW()` filters keep correctness intact; the only cost is +# unbounded growth of the oauth_ephemeral table while disabled. Use only +# when troubleshooting DB lock contention against /exchange or /handoff. +# Default: unset = worker runs normally. +# Read by: OAuthCleanupWorker +# OAUTH_CLEANUP_DISABLED=true # ===================================================================== # GitHub token rotation pool (required in production for passthrough) diff --git a/CLAUDE.md b/CLAUDE.md index d3e1eb4..515103b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -62,6 +62,9 @@ All under `/v1/`: | `GET /announcements` | Public, anonymous announcements feed. Same byte-identical envelope for every caller. Backed by JSON files in `src/main/resources/announcements/.json` (or `ANNOUNCEMENTS_DIR` env override). Validator enforces every rule from `docs/backend/announcements-endpoint.md` §2 at load time; expired items are filtered at serve time. `Cache-Control: public, max-age=600` + ETag revalidation. No auth, no per-user logic, no logging beyond standard access. | | `POST /auth/device/start` | Stateless proxy for `github.com/login/device/code`. Client used to call GitHub directly; some user networks (documented in OpenHub-Store/GitHub-Store#433, #395) can't reach GitHub reliably. Backend adds `client_id`, forwards GitHub's body verbatim. 10 req/hr/IP. | | `POST /auth/device/poll` | Stateless proxy for `github.com/login/oauth/access_token`. Reads `device_code` from form body, adds `client_id` + `grant_type`, forwards GitHub's body verbatim (including tokens on success). The backend never logs, caches, or persists the token. 200 req/hr/IP. Per-request diagnostic line `[auth-poll rid=… ] dch= ghs= gh_err= lat_ms= ua=` is logged for auth-stuck triage (GitHub-Store#433, #395) — the raw `device_code`, response body, and every token field are explicitly excluded; only the `error` key is parsed off the upstream body, via a DTO that doesn't even declare `access_token`/`refresh_token`. | +| `POST /oauth/state` | **S2S, website→backend.** PKCE-S256 web flow's pre-redirect step. Body `{state, code_challenge}` — both 43-char base64url. Stores `oauth:state:` row with the challenge for 60s. Returns 204 on success, 409 on duplicate state, 400 on validation, 401 on bad service auth. Gated by `X-Oauth-Service-Token` (env `OAUTH_SERVICE_TOKEN`) + `Host` allowlist (env `OAUTH_SERVICE_ALLOWED_HOSTS`). 60/hr/IP. | +| `POST /oauth/exchange` | **S2S, website→backend.** Called from the website's `/auth/callback` handler. Body `{code, state, code_verifier}` — backend verifies `SHA256(code_verifier) == stored code_challenge`, POSTs to `github.com/login/oauth/access_token` with `client_id`+`client_secret`+`code`+`redirect_uri`, mints a one-shot `handoff_id`, deletes the state row, returns `{handoff_id}`. 400 on PKCE mismatch / expired state / GitHub error (with `github_`), 502 on upstream timeout. Same S2S auth as `/oauth/state`. 30/hr/IP. **No token, code, or verifier ever logged.** | +| `POST /oauth/handoff/{id}` | **Public, app→backend.** App calls this with the `handoff_id` it received via the website's deep link. Backend does atomic `DELETE...RETURNING value` — single-use, second call returns 404. 60/min/IP brute-force cap. `Cache-Control: no-store`. Token lives in `oauth:handoff:` for at most 60s before this call retires it. | | `GET /internal/metrics` | Operator-only. Gated by `X-Admin-Token` matching the `ADMIN_TOKEN` env var (open if unset, for local dev). Returns per-source search counters, P-latency, worker queue depth, and top 20 misses (8-char `query_hash` prefix only) in last 7 days. | | `POST /internal/backfill-stale?limit=N` | Operator-only. Spawns a paced background job that refreshes every curated row whose new metadata columns are still at their migration defaults (currently keyed on `license_spdx_id IS NULL`). One concurrent run; returns 409 on re-trigger. Uses `searchClient.refreshRepo` + persist; respects the quiet window so the daily fetcher's pool stays free. Run after a column-add deploy; no-ops afterwards once the filter no longer matches. | | `GET /badge/...` | M3-styled SVG badges. Per-repo: `/badge/{owner}/{name}/{kind}/{style}/{variant}` for kind ∈ {release, stars, downloads}. Global: `/badge/{kind}/{style}/{variant}` for kind ∈ {users, fdroid}. Static: `/badge/static/{style}/{variant}?label=&icon=`. Style 1-12 hue, variant 1-3 shade. Vectorized glyph rendering — no font dependency at SVG embed time. | @@ -111,7 +114,9 @@ RepoRefreshWorker (hourly) — re-fetches passthrough repos by oldest indexed_a - **GitHub token strategy: bring-your-own + rotation pool fallback + rate-limit retry.** `/search`, `/search/explore`, `/repo` (lazy-fetch path), `/releases`, `/readme`, and `/user` all read an optional `X-GitHub-Token` header from the client and use that token for the upstream call (scales quota linearly with users). When the client sends none, `GitHubSearchClient.pickFallbackToken()` round-robins across a pool of four backend PATs (`GH_TOKEN_TRENDING`, `GH_TOKEN_NEW_RELEASES`, `GH_TOKEN_MOST_POPULAR`, `GH_TOKEN_TOPICS`). `GitHubResourceClient` (powering `/repo`/`/releases`/`/readme`/`/user`) shares the same pool via the provider injected in `AppModule.kt`. **Rate-limit retry**: when an upstream call returns 429 — or 403 with `x-ratelimit-remaining: 0` or a `retry-after` header — the call is retried once with a different pool token. A bare 403 (auth scope, blocked repo) is NOT retried. **Quiet-window guarantee**: during UTC hours `TOKEN_QUIET_START_UTC`–`TOKEN_QUIET_END_UTC` (default 1–4) the rotation pool is bypassed (only `GITHUB_TOKEN` is used) and rate-limit retry is skipped — so the Python fetcher's daily run gets the full pool. Inside that window a rate-limited user token surfaces verbatim. Any change here must preserve the quiet-window guarantee. - **Resource cache key segmentation.** `GitHubResourceClient.fetchCached` appends `|authed` or `|anon` to every cache key based on whether the request carried `X-GitHub-Token`. Without this, one user's bad-PAT 403 (or the anon path's IP-rate-limit 403) would poison the slot for every other caller until the negative TTL elapsed. Separate slots are filled and read independently. - **Upstream 401 → backend 502.** `GitHubResourceClient` remaps a 401 from GitHub to `Result.UpstreamError` (handler returns 502), instead of a `NegativeHit` that would forward 401 verbatim. 401 in our own response surface is reserved for backend-auth (no auth-required routes today, but the convention prevents the client from misreading "your PAT was rejected upstream" as "your session with us expired"). -- **Auth is a stateless proxy, not a session.** `/v1/auth/device/*` forwards to `github.com/login/*` with the backend's `GITHUB_OAUTH_CLIENT_ID` injected. The backend must **never** log, cache, or persist the access token returned by a successful poll — it passes through the suspending handler and out to the HTTP response, nothing else. No database table, no in-memory map, no breadcrumb. The client is the only place the token lives. Client is backend-first on these two calls and falls back to direct-to-github.com on 5xx / network errors (only — not on valid-but-negative responses like `authorization_pending` or `access_denied`, which are GitHub's real answer and `github.com` direct would say the same thing). +- **Auth is a stateless proxy, not a session.** `/v1/auth/device/*` forwards to `github.com/login/*` with the backend's `OAUTH_CLIENT_ID` injected. The backend must **never** log, cache, or persist the access token returned by a successful poll — it passes through the suspending handler and out to the HTTP response, nothing else. No database table, no in-memory map, no breadcrumb. The client is the only place the token lives. Client is backend-first on these two calls and falls back to direct-to-github.com on 5xx / network errors (only — not on valid-but-negative responses like `authorization_pending` or `access_denied`, which are GitHub's real answer and `github.com` direct would say the same thing). +- **OAuth web flow with PKCE + handoff** (`oauth/` package, `OAuthRoutes.kt`). Separate path from the device flow above — used by Desktop and any browser-initiated login. App generates `state` (32 random bytes, base64url) + `code_verifier` (43-128-char base64url) + `code_challenge = base64url(SHA256(verifier))`. Website posts state+challenge to `POST /v1/oauth/state`, redirects user to `github.com/login/oauth/authorize` with PKCE-S256, receives the callback at `/auth/callback`, posts code+state+verifier to `POST /v1/oauth/exchange`. Backend verifies PKCE, hits GitHub's token endpoint with `client_id`+`client_secret`, mints a 32-byte `handoff_id`, stores the access_token in `oauth:handoff:` for 60s, deletes the state row (one-shot), returns `{handoff_id}`. App receives `handoff_id` via the website's deep-link redirect, calls `POST /v1/oauth/handoff/{id}` for the token. The handoff row is `DELETE...RETURNING value` — atomic single-use, second call always 404. Storage is `oauth_ephemeral` table (V16); 60s TTL is enforced both by `expires_at > NOW()` on every read and by `OAuthCleanupWorker` reaping expired rows every 5min. The `/state` and `/exchange` routes are S2S (shared-secret + Host allowlist); `/handoff/{id}` is public, brute-force protected by per-IP rate limit + 256-bit ID entropy. The backend never logs `code`, `code_verifier`, `access_token`, or `handoff_id` — only first-8-char prefixes of `state` and `handoff_id` plus status codes and latency. +- **Deep-link contract: `githubstore://auth?handoff=&state=`.** Emitted by the website (NOT the backend). State must round-trip: the app generated it at the start of the flow, validates the value coming back in the deep link against its locally-stored copy, and only then calls `POST /v1/oauth/handoff/`. State on the deep link comes from the website's own KV (the value it received from the app's `/auth/start` call), not from the backend's `oauth_ephemeral` row. Earlier drafts proposed `githubstore://oauth-done?handoff=` — that contract is superseded. - **Unified ranking via `SearchScore.compute()`** (`ranking/SearchScore.kt`). Formula: `0.40·log₁₀(stars+1)/6 + 0.30·ctr + 0.20·install_success_rate + 0.10·exp(-days_since_release/90)`. Two callers: `SignalAggregationWorker` (hourly, with real signals) and `GitHubSearchClient` at ingest time (cold-start, signals = 0 — still gives passthrough repos a non-null score so they sort). Weights live in the object only; never inline the formula elsewhere. - **Meilisearch partial-update gotcha — PUT, never POST.** `MeilisearchClient.addDocuments()` is POST, which on Meili *replaces* the document with whatever fields you send (everything else becomes null). `MeilisearchClient.updateScores()` is PUT, which merges. Pushing just `{id, search_score}` with POST will wipe every other field on 3000+ docs. If you add a new "partial update" path, verify the HTTP verb before deploying. - **Dynamic category/topic ordering.** `RepoRepository.findByCategory()` picks a category-specific primary sort column (`trending_score` for trending, `popularity_score` for most-popular, `latest_release_date` for new-releases), falls back to global `searchScore`, then static `rank` as final tie-breaker. Without category-specific primary, both trending and most-popular collapse onto the same global score — the bug fix in PR #12. `findByTopicBucket()` keeps the simpler `searchScore DESC NULLS LAST, rank ASC` order because topics are flat lists, not flavour-segmented like the categories. @@ -133,7 +138,7 @@ RepoRefreshWorker (hourly) — re-fetches passthrough repos by oldest indexed_a ## Deployment notes -- `.env` lives only on the VPS at `/opt/github-store-backend/.env`, never in git. Under `APP_ENV=production` the app refuses to start without these: `DATABASE_URL`, `DATABASE_PASSWORD`, `MEILI_URL`, `MEILI_MASTER_KEY`, `GITHUB_OAUTH_CLIENT_ID`, `DEVICE_ID_PEPPER`. Also relied on: `GITHUB_TOKEN` (quiet-window fallback), `GH_TOKEN_TRENDING` / `GH_TOKEN_NEW_RELEASES` / `GH_TOKEN_MOST_POPULAR` / `GH_TOKEN_TOPICS` (rotation pool), `ADMIN_TOKEN`, `SENTRY_DSN`, `TOKEN_QUIET_START_UTC` / `TOKEN_QUIET_END_UTC` (default 1 / 4), `BADGE_USER_COUNT`, `DATABASE_POOL_SIZE` (default 20), `DISABLE_LIVE_GITHUB_PASSTHROUGH` and `DISABLE_BADGE_FETCH` kill switches. `.env.example` is the canonical list. +- `.env` lives only on the VPS at `/opt/github-store-backend/.env`, never in git. Under `APP_ENV=production` the app refuses to start without these: `DATABASE_URL`, `DATABASE_PASSWORD`, `MEILI_URL`, `MEILI_MASTER_KEY`, `OAUTH_CLIENT_ID` (renamed from the legacy `GITHUB_OAUTH_CLIENT_ID` — old name no longer read), `OAUTH_CLIENT_SECRET`, `OAUTH_SERVICE_TOKEN`, `OAUTH_SERVICE_ALLOWED_HOSTS`, `OAUTH_WEB_CALLBACK_URL`, `DEVICE_ID_PEPPER`. Also relied on: `GITHUB_TOKEN` (quiet-window fallback), `GH_TOKEN_TRENDING` / `GH_TOKEN_NEW_RELEASES` / `GH_TOKEN_MOST_POPULAR` / `GH_TOKEN_TOPICS` (rotation pool), `ADMIN_TOKEN`, `SENTRY_DSN`, `TOKEN_QUIET_START_UTC` / `TOKEN_QUIET_END_UTC` (default 1 / 4), `BADGE_USER_COUNT`, `DATABASE_POOL_SIZE` (default 20), `DISABLE_LIVE_GITHUB_PASSTHROUGH` / `DISABLE_BADGE_FETCH` / `OAUTH_CLEANUP_DISABLED` kill switches. `.env.example` is the canonical list. - `docker-compose.override.yml` is local-dev only (exposes DB/search ports to host). Never deploy it — `.dockerignore` excludes it. - Production uses `docker-compose.prod.yml` which does NOT expose Postgres/Meilisearch ports (only Caddy on 80/443). Postgres binds `127.0.0.1:5432` for the SSH tunnel from laptop. - `Caddyfile.prod` defines two site blocks: `api.github-store.org` (Cloudflare-fronted primary) and `api-direct.github-store.org` (direct origin fallback). Both reverse-proxy to `app:8080` and both issue Let's Encrypt certs. The CDN vhost passes `CF-Connecting-IP` and `X-Forwarded-For` through unchanged (Cloudflare overwrites the former; the latter still has the real client IP appended by Cloudflare). The api-direct vhost overrides XFF with the TCP source and strips `CF-Connecting-IP` to defeat client forgery on the un-fronted path. Cloudflare's origin pull protocol must be **HTTPS**, not HTTP — HTTP triggers Caddy's auto-redirect and leaks the direct hostname. diff --git a/src/main/kotlin/zed/rainxch/githubstore/AppModule.kt b/src/main/kotlin/zed/rainxch/githubstore/AppModule.kt index cadd452..ef3091c 100644 --- a/src/main/kotlin/zed/rainxch/githubstore/AppModule.kt +++ b/src/main/kotlin/zed/rainxch/githubstore/AppModule.kt @@ -25,6 +25,11 @@ import zed.rainxch.githubstore.match.FdroidSeedWorker import zed.rainxch.githubstore.match.SigningFingerprintRepository import zed.rainxch.githubstore.mirrors.MirrorStatusRegistry import zed.rainxch.githubstore.mirrors.MirrorStatusWorker +import zed.rainxch.githubstore.oauth.OAuthCleanupWorker +import zed.rainxch.githubstore.oauth.OAuthEphemeralStore +import zed.rainxch.githubstore.oauth.OAuthExchangeService +import zed.rainxch.githubstore.oauth.OAuthServiceAuth +import zed.rainxch.githubstore.oauth.PostgresOAuthEphemeralStore val appModule = module { single { EventRepository() } @@ -67,4 +72,32 @@ val appModule = module { persistFn = sc::persist, ) } + + // OAuth web flow. clientId is safe to embed (matches the KMP client's + // BuildKonfig value); clientSecret comes from the OAuth App settings and + // must be set in the production .env. callbackUrl is the website's + // callback handler — the redirect_uri must match what's registered with + // GitHub for the OAuth app, otherwise GitHub rejects with redirect_uri_mismatch. + single { PostgresOAuthEphemeralStore() } + single { + // Empty fallbacks are dev-only sentinels — validateProductionEnv + // refuses to start prod without OAUTH_CLIENT_ID / OAUTH_CLIENT_SECRET / + // OAUTH_WEB_CALLBACK_URL set, so the "missing-…" strings never run + // through to a real exchange under APP_ENV=production. + OAuthExchangeService( + clientId = System.getenv("OAUTH_CLIENT_ID")?.takeIf { it.isNotBlank() } + ?: "missing-client-id", + clientSecret = System.getenv("OAUTH_CLIENT_SECRET")?.takeIf { it.isNotBlank() } + ?: "missing-client-secret", + callbackUrl = System.getenv("OAUTH_WEB_CALLBACK_URL")?.takeIf { it.isNotBlank() } + ?: "https://localhost.invalid/missing-callback", + ) + } + single { + OAuthServiceAuth( + expectedToken = System.getenv("OAUTH_SERVICE_TOKEN"), + allowedHostsCsv = System.getenv("OAUTH_SERVICE_ALLOWED_HOSTS"), + ) + } + single { OAuthCleanupWorker(store = get(), supervisor = get()) } } diff --git a/src/main/kotlin/zed/rainxch/githubstore/Application.kt b/src/main/kotlin/zed/rainxch/githubstore/Application.kt index 236cd14..e434571 100644 --- a/src/main/kotlin/zed/rainxch/githubstore/Application.kt +++ b/src/main/kotlin/zed/rainxch/githubstore/Application.kt @@ -36,6 +36,7 @@ fun main() { event.request?.headers?.let { headers -> listOf( "Authorization", "X-GitHub-Token", "X-Admin-Token", + "X-Oauth-Service-Token", "Cookie", "Set-Cookie", "X-Forwarded-For", "CF-Connecting-IP", ).forEach { headers.remove(it) } @@ -93,6 +94,9 @@ fun Application.module() { val mirrorStatusWorker by inject() mirrorStatusWorker.start() + val oauthCleanupWorker by inject() + oauthCleanupWorker.start() + // Synchronous load -- no coroutine. The set is small (handful of files // bundled in the JAR) and we want the startup log line before serving. // Pre-start, the registry returns an empty list, so a request that @@ -134,7 +138,25 @@ private fun validateProductionEnv() { "DATABASE_PASSWORD", "MEILI_URL", "MEILI_MASTER_KEY", - "GITHUB_OAUTH_CLIENT_ID", + "OAUTH_CLIENT_ID", + // OAuth client secret is required for the web-flow exchange path. + // Without it, /v1/oauth/exchange would call GitHub with an empty + // secret and every flow would 400. Distinct from CLIENT_ID — only + // the backend needs the secret. + "OAUTH_CLIENT_SECRET", + // Shared secret on /v1/oauth/state and /v1/oauth/exchange. Missing + // env makes both endpoints return 401 service_auth_required on + // every request — useless service. + "OAUTH_SERVICE_TOKEN", + // Host allowlist for the S2S OAuth endpoints. Required in production + // alongside OAUTH_SERVICE_TOKEN — empty in prod means every S2S call + // gets 401, which is a silent failure mode validateProductionEnv + // exists to prevent. + "OAUTH_SERVICE_ALLOWED_HOSTS", + // GitHub OAuth redirect_uri. Must EXACTLY match the value registered + // with the OAuth App or every exchange call fails with + // redirect_uri_mismatch — same fail-fast rationale as the others. + "OAUTH_WEB_CALLBACK_URL", // Pepper for SHA-256 hashing of device IDs before they hit Postgres. // Required in prod so a stolen DB dump can't be brute-forced into a // device-ID lookup table without also stealing the env. diff --git a/src/main/kotlin/zed/rainxch/githubstore/Plugins.kt b/src/main/kotlin/zed/rainxch/githubstore/Plugins.kt index 84e38d6..f4173b1 100644 --- a/src/main/kotlin/zed/rainxch/githubstore/Plugins.kt +++ b/src/main/kotlin/zed/rainxch/githubstore/Plugins.kt @@ -276,6 +276,32 @@ fun Application.configureHTTP() { rateLimiter(limit = 30, refillPeriod = 1.minutes) requestKey(::forwardedFor) } + // OAuth state-registration: called by the website server before each + // user starts a login. 60/hr/IP gives the website plenty of headroom + // while capping a misconfigured or abusive caller. (The shared-secret + // check is the primary defence; the rate limit is the last-mile cap + // if the secret leaks.) + register(RateLimitName("oauth-state")) { + rateLimiter(limit = 60, refillPeriod = 1.hours) + requestKey(::forwardedFor) + } + // OAuth code-for-token exchange: 30/hr/IP per the spec. Same shared- + // secret-first rationale as `oauth-state`. One legitimate login = one + // call, so 30/hr covers a busy server fronting multiple users behind + // a NAT. + register(RateLimitName("oauth-exchange")) { + rateLimiter(limit = 30, refillPeriod = 1.hours) + requestKey(::forwardedFor) + } + // OAuth handoff: public, brute-force defence only — 32-byte random + // handoff_id has 256 bits of entropy so guessing is hopeless; the + // per-IP cap stops an attacker from script-scanning IDs en masse. + // 60/min/IP is generous for the legitimate flow (1 call per login) + // but tight against a scanner. + register(RateLimitName("oauth-handoff")) { + rateLimiter(limit = 60, refillPeriod = 1.minutes) + requestKey(::forwardedFor) + } } // Basic Auth for the /v1/internal/dashboard HTML page. Username is diff --git a/src/main/kotlin/zed/rainxch/githubstore/db/DatabaseFactory.kt b/src/main/kotlin/zed/rainxch/githubstore/db/DatabaseFactory.kt index dfec530..53b2dc6 100644 --- a/src/main/kotlin/zed/rainxch/githubstore/db/DatabaseFactory.kt +++ b/src/main/kotlin/zed/rainxch/githubstore/db/DatabaseFactory.kt @@ -77,6 +77,7 @@ object DatabaseFactory { "V13__drop_telemetry_events.sql", "V14__open_issues_count.sql", "V15__license_info.sql", + "V16__oauth_ephemeral.sql", ) for (migration in migrations) { val rawSql = this::class.java.classLoader diff --git a/src/main/kotlin/zed/rainxch/githubstore/db/Tables.kt b/src/main/kotlin/zed/rainxch/githubstore/db/Tables.kt index 04053ce..9cd45a3 100644 --- a/src/main/kotlin/zed/rainxch/githubstore/db/Tables.kt +++ b/src/main/kotlin/zed/rainxch/githubstore/db/Tables.kt @@ -121,3 +121,13 @@ object RepoSignals : Table("repo_signals") { override val primaryKey = PrimaryKey(repoId) } + +// Note: `oauth_ephemeral` (V16) is intentionally NOT declared as an Exposed +// Table here. PostgresOAuthEphemeralStore reaches the table via raw JDBC +// (matching the ResourceCacheRepository convention) because every query is +// either an `INSERT ... ON CONFLICT DO NOTHING` or a `DELETE ... RETURNING` +// — shapes that don't translate cleanly to the Exposed DSL and that the +// store's atomicity guarantees depend on. Adding an Exposed object here +// would be a footgun: a future maintainer might use it for a "convenient" +// SELECT or UPDATE that bypasses the `expires_at > NOW()` filter and start +// serving expired rows. diff --git a/src/main/kotlin/zed/rainxch/githubstore/ingest/GitHubDeviceClient.kt b/src/main/kotlin/zed/rainxch/githubstore/ingest/GitHubDeviceClient.kt index 751dec1..3dc6bfe 100644 --- a/src/main/kotlin/zed/rainxch/githubstore/ingest/GitHubDeviceClient.kt +++ b/src/main/kotlin/zed/rainxch/githubstore/ingest/GitHubDeviceClient.kt @@ -16,10 +16,11 @@ import org.slf4j.LoggerFactory // override. open class GitHubDeviceClient( private val clientId: String = - System.getenv("GITHUB_OAUTH_CLIENT_ID")?.takeIf { it.isNotBlank() } + System.getenv("OAUTH_CLIENT_ID")?.takeIf { it.isNotBlank() } ?: error( - "GITHUB_OAUTH_CLIENT_ID env var is required to serve /v1/auth/device/* routes. " + - "Set it to the same OAuth App client_id the KMP client has in BuildKonfig." + "OAUTH_CLIENT_ID env var is required to serve /v1/auth/device/* routes. " + + "Set it to the same OAuth App client_id the KMP client has in BuildKonfig. " + + "(Renamed from GITHUB_OAUTH_CLIENT_ID — update /opt/github-store-backend/.env to match.)" ), ) { private val log = LoggerFactory.getLogger(GitHubDeviceClient::class.java) diff --git a/src/main/kotlin/zed/rainxch/githubstore/oauth/OAuthCleanupWorker.kt b/src/main/kotlin/zed/rainxch/githubstore/oauth/OAuthCleanupWorker.kt new file mode 100644 index 0000000..59596bf --- /dev/null +++ b/src/main/kotlin/zed/rainxch/githubstore/oauth/OAuthCleanupWorker.kt @@ -0,0 +1,63 @@ +package zed.rainxch.githubstore.oauth + +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.delay +import kotlinx.coroutines.launch +import org.slf4j.LoggerFactory +import zed.rainxch.githubstore.ingest.WorkerSupervisor +import kotlin.time.Duration.Companion.minutes + +// Postgres has no native TTL. Every five minutes, DELETE every row whose +// `expires_at` is in the past. Request-time queries already filter by +// `expires_at > NOW()` so the worker missing a tick never serves an expired +// row; the worker is purely about keeping the table small. Single instance +// so no advisory lock; if we go multi-node, add the same advisory-lock +// pattern MirrorStatusWorker uses. +class OAuthCleanupWorker( + private val store: OAuthEphemeralStore, + private val supervisor: WorkerSupervisor? = null, +) { + private val log = LoggerFactory.getLogger(OAuthCleanupWorker::class.java) + private val scope = CoroutineScope(Dispatchers.IO + SupervisorJob()) + + private val cycleInterval = 5.minutes + + fun start(): Job = scope.launch { + // Brief startup delay matches the other workers — DB pool warms, + // migrations finish, etc. + delay(30_000L) + while (true) { + if (cleanupDisabled()) { + log.info("OAuth cleanup: OAUTH_CLEANUP_DISABLED=true, sleeping cycle") + } else { + try { + val reaped = store.reapExpired() + if (reaped > 0) { + log.info("OAuth cleanup reaped {} expired rows", reaped) + } + supervisor?.recordTick(WORKER_NAME) + } catch (e: Exception) { + log.error("OAuth cleanup cycle failed", e) + } + } + delay(cycleInterval) + } + }.also { supervisor?.register(WORKER_NAME, it) } + + // Per-iteration kill switch. Lets the operator stop the DELETE-on-hot-row + // loop without restarting the app if it ever starts contending with + // /exchange or /handoff under load. Request-time `expires_at > NOW()` + // filters keep correctness intact while the worker sleeps — the only + // cost is unbounded table growth, which the operator can clear manually + // via `DELETE FROM oauth_ephemeral WHERE expires_at < NOW();` before + // re-enabling. + private fun cleanupDisabled(): Boolean = + System.getenv("OAUTH_CLEANUP_DISABLED")?.equals("true", ignoreCase = true) == true + + private companion object { + const val WORKER_NAME = "oauth_cleanup" + } +} diff --git a/src/main/kotlin/zed/rainxch/githubstore/oauth/OAuthEphemeralStore.kt b/src/main/kotlin/zed/rainxch/githubstore/oauth/OAuthEphemeralStore.kt new file mode 100644 index 0000000..f3fea34 --- /dev/null +++ b/src/main/kotlin/zed/rainxch/githubstore/oauth/OAuthEphemeralStore.kt @@ -0,0 +1,47 @@ +package zed.rainxch.githubstore.oauth + +import java.time.Duration + +// SETEX + GETDEL semantics on top of whatever ephemeral KV the operator picked. +// PostgresOAuthEphemeralStore is the only production impl today; the interface +// exists so a Redis-backed swap-in stays a one-line Koin change if the OAuth +// flow ever grows past single-instance scale. +interface OAuthEphemeralStore { + /** + * Atomic insert. Returns false if the key already exists in the namespace + * (caller should reject the request — duplicate state on the wire usually + * means replay or browser-back, never a legitimate flow). + */ + suspend fun setEx(namespace: String, key: String, value: String, ttl: Duration): Boolean + + /** + * Atomic delete-and-return. Returns the value if the key existed AND was + * unexpired, null otherwise. Single-use semantics: a second call always + * returns null because the row is gone after the first. + */ + suspend fun getDel(namespace: String, key: String): String? + + /** + * Read without delete. Used by `/v1/oauth/exchange` to compare the stored + * code_challenge against the verifier the website forwards. The row is + * deleted in the same handler via [del] once verification passes — keeping + * read and delete separate lets the handler distinguish "state missing" + * from "PKCE mismatch" for accurate error responses. + */ + suspend fun get(namespace: String, key: String): String? + + /** Hard delete, no return. Used to retire a state row after exchange. */ + suspend fun del(namespace: String, key: String) + + /** + * DELETE every row whose `expires_at < now`. Returns the count for logging. + * Called periodically by [OAuthCleanupWorker]; never called from a request + * handler. + */ + suspend fun reapExpired(): Int + + companion object { + const val NAMESPACE_STATE = "state" + const val NAMESPACE_HANDOFF = "handoff" + } +} diff --git a/src/main/kotlin/zed/rainxch/githubstore/oauth/OAuthExchangeService.kt b/src/main/kotlin/zed/rainxch/githubstore/oauth/OAuthExchangeService.kt new file mode 100644 index 0000000..2c95233 --- /dev/null +++ b/src/main/kotlin/zed/rainxch/githubstore/oauth/OAuthExchangeService.kt @@ -0,0 +1,98 @@ +package zed.rainxch.githubstore.oauth + +import io.ktor.client.* +import io.ktor.client.engine.cio.* +import io.ktor.client.plugins.* +import io.ktor.client.request.* +import io.ktor.client.request.forms.* +import io.ktor.client.statement.* +import io.ktor.http.* +import kotlinx.serialization.Serializable +import kotlinx.serialization.json.Json + +// Server-side half of the GitHub OAuth web flow. Holds the client_secret — +// the whole point of having a backend in the mix. Client never sees it. +open class OAuthExchangeService( + private val clientId: String, + private val clientSecret: String, + private val callbackUrl: String, +) { + + private val http = HttpClient(CIO) { + install(HttpTimeout) { + requestTimeoutMillis = 10_000 + connectTimeoutMillis = 5_000 + socketTimeoutMillis = 10_000 + } + // Read GitHub's error bodies verbatim so we can forward the upstream + // error code; otherwise ktor would throw on non-2xx. + expectSuccess = false + } + + private val responseJson = Json { ignoreUnknownKeys = true; isLenient = true } + + /** + * Exchange GitHub authorization code for an access token. + * + * Returns one of: + * - Success(accessToken) — GitHub returned 2xx with a token + * - UpstreamError(errorCode) — GitHub returned 2xx with `error` field + * - UpstreamFailure — GitHub 5xx / timeout / network error + * + * The handler maps Success → mint handoff, UpstreamError → 400 with the + * upstream error code, UpstreamFailure → 502. + */ + open suspend fun exchange(code: String): Result { + val response = try { + http.post("https://github.com/login/oauth/access_token") { + accept(ContentType.Application.Json) + header(HttpHeaders.UserAgent, "GithubStoreBackend/1.0 (OAuthExchange)") + contentType(ContentType.Application.FormUrlEncoded) + setBody(FormDataContent(Parameters.build { + append("client_id", clientId) + append("client_secret", clientSecret) + append("code", code) + append("redirect_uri", callbackUrl) + })) + } + } catch (_: Exception) { + return Result.UpstreamFailure + } + + if (!response.status.isSuccess()) return Result.UpstreamFailure + val body = try { + response.bodyAsText() + } catch (_: Exception) { + return Result.UpstreamFailure + } + + val parsed = try { + responseJson.decodeFromString(TokenResponse.serializer(), body) + } catch (_: Exception) { + return Result.UpstreamFailure + } + + return when { + parsed.access_token != null -> Result.Success(parsed.access_token) + parsed.error != null -> Result.UpstreamError(parsed.error) + else -> Result.UpstreamFailure + } + } + + // Same DTO trick as AuthRoutes.kt: only declare the fields we need to + // read. access_token + error are the entire surface; access_token never + // becomes a logged-or-persisted value beyond the in-memory `Success` + // wrapper that immediately enters Redis-equivalent storage with a 60s + // TTL. + @Serializable + private data class TokenResponse( + val access_token: String? = null, + val error: String? = null, + ) + + sealed class Result { + data class Success(val accessToken: String) : Result() + data class UpstreamError(val errorCode: String) : Result() + data object UpstreamFailure : Result() + } +} diff --git a/src/main/kotlin/zed/rainxch/githubstore/oauth/OAuthServiceAuth.kt b/src/main/kotlin/zed/rainxch/githubstore/oauth/OAuthServiceAuth.kt new file mode 100644 index 0000000..573d7e9 --- /dev/null +++ b/src/main/kotlin/zed/rainxch/githubstore/oauth/OAuthServiceAuth.kt @@ -0,0 +1,84 @@ +package zed.rainxch.githubstore.oauth + +import io.ktor.http.* +import io.ktor.server.application.* +import io.ktor.server.response.* +import org.slf4j.LoggerFactory +import zed.rainxch.githubstore.util.ApiError +import java.security.MessageDigest + +// Gate the two S2S OAuth endpoints (/v1/oauth/state and /v1/oauth/exchange). +// +// Two checks, both must pass: +// 1. Shared secret — `X-Oauth-Service-Token` header must equal the env +// OAUTH_SERVICE_TOKEN (constant-time compared so neither length nor +// content can leak via response timing). +// 2. Host allowlist — `Host:` header must be in OAUTH_SERVICE_ALLOWED_HOSTS +// (comma-separated env). Belt-and-suspenders on top of (1): even if the +// secret leaks, the request still has to land on the canonical vhost, +// not the api-direct fallback or some accidental Cloudflare Workers +// route. If the env is unset or blank the host check is skipped — that +// mode is for local dev (`APP_ENV != production`) only; production +// MUST configure both env vars (validateProductionEnv enforces both at +// startup) or both routes refuse every request. +// +// All misconfiguration responses look identical to "wrong secret" responses +// (401 service_auth_required) so an anonymous prober can't fingerprint +// missing env vars vs. wrong-secret. Server-side logs carry the real reason. +class OAuthServiceAuth( + private val expectedToken: String?, + allowedHostsCsv: String?, +) { + private val log = LoggerFactory.getLogger(OAuthServiceAuth::class.java) + private val isProd = System.getenv("APP_ENV") == "production" + + private val allowedHosts: Set = + allowedHostsCsv?.split(',') + ?.map { it.trim().lowercase() } + ?.filter { it.isNotEmpty() } + ?.toSet() + ?: emptySet() + + suspend fun authorize(call: ApplicationCall): Boolean { + if (expectedToken.isNullOrBlank()) { + log.error("[oauth-service-auth] OAUTH_SERVICE_TOKEN unset — every S2S OAuth request will be rejected") + return rejectUnauthorized(call) + } + + val provided = call.request.headers["X-Oauth-Service-Token"].orEmpty() + if (!constantTimeEquals(provided, expectedToken)) { + return rejectUnauthorized(call) + } + + if (allowedHosts.isNotEmpty()) { + val host = call.request.headers[HttpHeaders.Host]?.lowercase().orEmpty() + // Strip optional :port suffix — operators sometimes set "api.example.org" + // in the env but the Host header arrives as "api.example.org:443". + val hostOnly = host.substringBefore(':') + if (hostOnly !in allowedHosts) { + log.info("[oauth-service-auth] rejected host={}", host) + return rejectUnauthorized(call) + } + } else if (isProd) { + log.error("[oauth-service-auth] OAUTH_SERVICE_ALLOWED_HOSTS unset in production — every S2S OAuth request will be rejected") + return rejectUnauthorized(call) + } + + return true + } + + private suspend fun rejectUnauthorized(call: ApplicationCall): Boolean { + call.respond(HttpStatusCode.Unauthorized, ApiError("service_auth_required")) + return false + } + + // MessageDigest.isEqual is constant-time on length AND content since + // Java 6u17 — it processes both buffers fully rather than short-circuiting + // on length mismatch. Avoids the length-side-channel a hand-rolled + // length-prefixed XOR-or-accumulate would have. + private fun constantTimeEquals(a: String, b: String): Boolean = + MessageDigest.isEqual( + a.toByteArray(Charsets.UTF_8), + b.toByteArray(Charsets.UTF_8), + ) +} diff --git a/src/main/kotlin/zed/rainxch/githubstore/oauth/PostgresOAuthEphemeralStore.kt b/src/main/kotlin/zed/rainxch/githubstore/oauth/PostgresOAuthEphemeralStore.kt new file mode 100644 index 0000000..e1059cb --- /dev/null +++ b/src/main/kotlin/zed/rainxch/githubstore/oauth/PostgresOAuthEphemeralStore.kt @@ -0,0 +1,104 @@ +package zed.rainxch.githubstore.oauth + +import kotlinx.coroutines.Dispatchers +import org.jetbrains.exposed.sql.transactions.TransactionManager +import org.jetbrains.exposed.sql.transactions.experimental.newSuspendedTransaction +import java.sql.Connection +import java.time.Duration +import java.time.OffsetDateTime +import java.time.ZoneOffset + +// Raw JDBC for the same reason ResourceCacheRepository uses raw JDBC: the +// query shapes here (INSERT ... ON CONFLICT DO NOTHING, DELETE ... RETURNING, +// expires_at > NOW()) translate to one trivial SQL line each, and we sidestep +// any Exposed kotlinx-datetime / timestamptz bind surprises. +class PostgresOAuthEphemeralStore : OAuthEphemeralStore { + + override suspend fun setEx( + namespace: String, + key: String, + value: String, + ttl: Duration, + ): Boolean = newSuspendedTransaction(Dispatchers.IO) { + val now = OffsetDateTime.now(ZoneOffset.UTC) + val expiresAt = now.plus(ttl) + val conn = TransactionManager.current().connection.connection as Connection + conn.prepareStatement( + """ + INSERT INTO oauth_ephemeral (namespace, key, value, expires_at) + VALUES (?, ?, ?, ?) + ON CONFLICT (namespace, key) DO NOTHING + """.trimIndent() + ).use { ps -> + ps.setString(1, namespace) + ps.setString(2, key) + ps.setString(3, value) + ps.setObject(4, expiresAt) + ps.executeUpdate() == 1 + } + } + + override suspend fun getDel( + namespace: String, + key: String, + ): String? = newSuspendedTransaction(Dispatchers.IO) { + // DELETE ... RETURNING is atomic — single round-trip, no race window + // between SELECT and DELETE. expires_at filter mirrors a Redis TTL: + // an expired row that hasn't been swept yet still appears absent. + val conn = TransactionManager.current().connection.connection as Connection + conn.prepareStatement( + """ + DELETE FROM oauth_ephemeral + WHERE namespace = ? AND key = ? AND expires_at > NOW() + RETURNING value + """.trimIndent() + ).use { ps -> + ps.setString(1, namespace) + ps.setString(2, key) + ps.executeQuery().use { rs -> + if (rs.next()) rs.getString(1) else null + } + } + } + + override suspend fun get( + namespace: String, + key: String, + ): String? = newSuspendedTransaction(Dispatchers.IO) { + val conn = TransactionManager.current().connection.connection as Connection + conn.prepareStatement( + """ + SELECT value FROM oauth_ephemeral + WHERE namespace = ? AND key = ? AND expires_at > NOW() + """.trimIndent() + ).use { ps -> + ps.setString(1, namespace) + ps.setString(2, key) + ps.executeQuery().use { rs -> + if (rs.next()) rs.getString(1) else null + } + } + } + + override suspend fun del(namespace: String, key: String) { + newSuspendedTransaction(Dispatchers.IO) { + val conn = TransactionManager.current().connection.connection as Connection + conn.prepareStatement( + "DELETE FROM oauth_ephemeral WHERE namespace = ? AND key = ?" + ).use { ps -> + ps.setString(1, namespace) + ps.setString(2, key) + ps.executeUpdate() + } + } + } + + override suspend fun reapExpired(): Int = newSuspendedTransaction(Dispatchers.IO) { + val conn = TransactionManager.current().connection.connection as Connection + conn.prepareStatement( + "DELETE FROM oauth_ephemeral WHERE expires_at < NOW()" + ).use { ps -> + ps.executeUpdate() + } + } +} diff --git a/src/main/kotlin/zed/rainxch/githubstore/routes/OAuthRoutes.kt b/src/main/kotlin/zed/rainxch/githubstore/routes/OAuthRoutes.kt new file mode 100644 index 0000000..1370c20 --- /dev/null +++ b/src/main/kotlin/zed/rainxch/githubstore/routes/OAuthRoutes.kt @@ -0,0 +1,293 @@ +package zed.rainxch.githubstore.routes + +import io.ktor.http.* +import io.ktor.server.plugins.ratelimit.* +import io.ktor.server.request.* +import io.ktor.server.response.* +import io.ktor.server.routing.* +import kotlinx.serialization.Serializable +import kotlinx.serialization.json.Json +import org.slf4j.LoggerFactory +import zed.rainxch.githubstore.oauth.OAuthEphemeralStore +import zed.rainxch.githubstore.oauth.OAuthEphemeralStore.Companion.NAMESPACE_HANDOFF +import zed.rainxch.githubstore.oauth.OAuthEphemeralStore.Companion.NAMESPACE_STATE +import zed.rainxch.githubstore.oauth.OAuthExchangeService +import zed.rainxch.githubstore.oauth.OAuthServiceAuth +import zed.rainxch.githubstore.requireMaxBody +import zed.rainxch.githubstore.util.ApiError +import java.security.MessageDigest +import java.security.SecureRandom +import java.time.Duration +import java.util.Base64 + +private val log = LoggerFactory.getLogger("OAuthRoutes") + +// 60-second TTL for both state + handoff per the security baseline. Long +// enough that a slow user finishing OAuth in another tab still works, short +// enough that a leaked handoff_id is effectively useless by the time anyone +// could replay it. +private val TTL = Duration.ofSeconds(60) + +private const val STATE_BODY_MAX = 1L * 1024 +private const val EXCHANGE_BODY_MAX = 4L * 1024 + +// state + code_challenge + code_verifier + handoff_id all encode N random +// bytes as unpadded base64url. Validation rejects anything else outright so +// the upstream call never sees malformed input. +private val BASE64URL_43 = Regex("^[A-Za-z0-9_-]{43}$") +private val BASE64URL_43_TO_128 = Regex("^[A-Za-z0-9_-]{43,128}$") +private const val CODE_MAX_LEN = 200 + +// Whitelist of GitHub OAuth error codes we echo verbatim in the response +// body and log. Anything outside this set is normalised to `unknown` so +// future GitHub changes (or a man-in-the-middle injecting odd values) can't +// reach our API consumers or structured logs unchecked. List sourced from +// GitHub's documented OAuth error responses on /login/oauth/access_token. +private val GITHUB_ERROR_WHITELIST = setOf( + "authorization_pending", + "slow_down", + "expired_token", + "unsupported_grant_type", + "incorrect_client_credentials", + "incorrect_device_code", + "access_denied", + "device_flow_disabled", + "redirect_uri_mismatch", + "bad_verification_code", + "unverified_user_email", + "application_suspended", +) + +private val parseJson = Json { ignoreUnknownKeys = true; isLenient = true } +private val urlBase64 = Base64.getUrlEncoder().withoutPadding() +private val secureRandom = SecureRandom() + +@Serializable +private data class StateRequest(val state: String, val code_challenge: String) + +@Serializable +private data class ExchangeRequest(val code: String, val state: String, val code_verifier: String) + +@Serializable +private data class ExchangeResponse(val handoff_id: String) + +@Serializable +private data class HandoffResponse(val access_token: String) + +// JSON we persist under `oauth:state:`. Stored as a string, not a +// scalar, because we may add fields (e.g. redirect target) without a +// migration — the column is `TEXT`. +@Serializable +private data class StoredState(val code_challenge: String, val created_at_ms: Long) + +fun Route.oauthRoutes( + store: OAuthEphemeralStore, + exchangeService: OAuthExchangeService, + serviceAuth: OAuthServiceAuth, +) { + route("/oauth") { + + // ---- POST /v1/oauth/state ---- + // Website calls this server-to-server before redirecting the user to + // GitHub. The app generated the verifier; the website forwards only + // the challenge (= SHA256(verifier)). 60s TTL. + rateLimit(RateLimitName("oauth-state")) { + post("/state") { + if (!serviceAuth.authorize(call)) return@post + if (!call.requireMaxBody(STATE_BODY_MAX)) return@post + + val req = try { + call.receive() + } catch (_: Exception) { + return@post call.respond(HttpStatusCode.BadRequest, ApiError("invalid_body")) + } + + if (!BASE64URL_43.matches(req.state)) { + return@post call.respond(HttpStatusCode.BadRequest, ApiError("invalid_state")) + } + if (!BASE64URL_43.matches(req.code_challenge)) { + return@post call.respond(HttpStatusCode.BadRequest, ApiError("invalid_code_challenge")) + } + + val stored = StoredState( + code_challenge = req.code_challenge, + created_at_ms = System.currentTimeMillis(), + ) + val ok = store.setEx( + namespace = NAMESPACE_STATE, + key = req.state, + value = parseJson.encodeToString(StoredState.serializer(), stored), + ttl = TTL, + ) + if (!ok) { + // Duplicate state on the wire is either a buggy retry or a + // replay — refuse. The legitimate flow always uses a fresh + // random state per attempt. + log.info("[oauth-state] state={} duplicate", req.state.take(8)) + return@post call.respond(HttpStatusCode.Conflict, ApiError("state_already_registered")) + } + + log.info("[oauth-state] state={} ok", req.state.take(8)) + call.respond(HttpStatusCode.NoContent) + } + } + + // ---- POST /v1/oauth/exchange ---- + // Website calls this from its /auth/callback handler. The request + // contains GitHub's `code`, the `state` the app generated, and the + // PKCE `code_verifier` the app posted to the website earlier in the + // flow. The backend atomically consumes the state row (getDel — + // closes any get-then-del race between two concurrent /exchange + // requests with the same state) and only then verifies PKCE + + // forwards to GitHub. + // + // State is ALWAYS burned by the getDel at the top, even on PKCE + // mismatch, GitHub error, or upstream failure. GitHub's `code` is + // single-use upstream — there's no legitimate retry of the same + // (state, code) pair, so refusing the second attempt with + // state_missing_or_expired is the correct behaviour for every + // failure mode. + rateLimit(RateLimitName("oauth-exchange")) { + post("/exchange") { + if (!serviceAuth.authorize(call)) return@post + if (!call.requireMaxBody(EXCHANGE_BODY_MAX)) return@post + + val req = try { + call.receive() + } catch (_: Exception) { + return@post call.respond(HttpStatusCode.BadRequest, ApiError("invalid_body")) + } + + if (req.code.isBlank() || req.code.length > CODE_MAX_LEN) { + return@post call.respond(HttpStatusCode.BadRequest, ApiError("invalid_code")) + } + if (!BASE64URL_43.matches(req.state)) { + return@post call.respond(HttpStatusCode.BadRequest, ApiError("invalid_state")) + } + if (!BASE64URL_43_TO_128.matches(req.code_verifier)) { + return@post call.respond(HttpStatusCode.BadRequest, ApiError("invalid_code_verifier")) + } + val statePrefix = req.state.take(8) + + // Atomic consume — single round-trip, single Postgres row lock. + // If two requests race with the same state, exactly one gets + // the row back, the other gets null and 400s. + val storedRaw = store.getDel(NAMESPACE_STATE, req.state) ?: run { + log.info("[oauth-exchange] state={} error=state_missing_or_expired", statePrefix) + return@post call.respond(HttpStatusCode.BadRequest, ApiError("state_missing_or_expired")) + } + val stored = try { + parseJson.decodeFromString(StoredState.serializer(), storedRaw) + } catch (_: Exception) { + log.error("[oauth-exchange] state={} error=stored_state_unreadable", statePrefix) + return@post call.respond(HttpStatusCode.InternalServerError, ApiError("internal_error")) + } + + val verifierChallenge = pkceChallenge(req.code_verifier) + if (!constantTimeEquals(verifierChallenge, stored.code_challenge)) { + log.info("[oauth-exchange] state={} error=pkce_mismatch", statePrefix) + return@post call.respond(HttpStatusCode.BadRequest, ApiError("pkce_mismatch")) + } + + when (val result = exchangeService.exchange(req.code)) { + is OAuthExchangeService.Result.Success -> { + val handoffId = randomBase64Url(32) + val ok = store.setEx( + namespace = NAMESPACE_HANDOFF, + key = handoffId, + value = result.accessToken, + ttl = TTL, + ) + if (!ok) { + // Re-roll once; with 32 random bytes a collision is + // astronomically unlikely but a defensive retry costs + // nothing. + val retryId = randomBase64Url(32) + val retryOk = store.setEx(NAMESPACE_HANDOFF, retryId, result.accessToken, TTL) + if (!retryOk) { + log.error("[oauth-exchange] state={} error=handoff_collision_twice", statePrefix) + return@post call.respond(HttpStatusCode.InternalServerError, ApiError("internal_error")) + } + log.info("[oauth-exchange] state={} ok", statePrefix) + return@post call.respond(ExchangeResponse(retryId)) + } + log.info("[oauth-exchange] state={} ok", statePrefix) + call.respond(ExchangeResponse(handoffId)) + } + + is OAuthExchangeService.Result.UpstreamError -> { + val safeCode = if (result.errorCode in GITHUB_ERROR_WHITELIST) { + result.errorCode + } else { + "unknown" + } + log.info("[oauth-exchange] state={} error=github_{}", statePrefix, safeCode) + call.respond( + HttpStatusCode.BadRequest, + ApiError("github_$safeCode", "GitHub rejected the authorization code"), + ) + } + + OAuthExchangeService.Result.UpstreamFailure -> { + log.warn("[oauth-exchange] state={} error=github_unreachable", statePrefix) + // State was consumed by getDel above. A retry can't use + // the same (state, code) pair anyway — GitHub's code is + // single-use. The website must restart the flow with a + // fresh state if it wants to retry. + call.respond(HttpStatusCode.BadGateway, ApiError("github_unreachable")) + } + } + } + } + + // ---- POST /v1/oauth/handoff/{id} ---- + // App calls this with the handoff_id it received via deep link from + // the website. Single-use: GETDEL semantics mean a second call always + // returns 404. Public — no S2S auth. `Cache-Control: no-store` set on + // every response (including 404) so no middlebox or buggy CDN can + // cache the answer for a handoff_id that became valid between scans. + rateLimit(RateLimitName("oauth-handoff")) { + post("/handoff/{id}") { + call.response.header(HttpHeaders.CacheControl, "no-store") + + val rawId = call.parameters["id"].orEmpty() + if (!BASE64URL_43.matches(rawId)) { + return@post call.respond(HttpStatusCode.NotFound, ApiError("handoff_not_found")) + } + + val token = store.getDel(NAMESPACE_HANDOFF, rawId) + if (token == null) { + log.info("[oauth-handoff] id={} miss", rawId.take(8)) + return@post call.respond(HttpStatusCode.NotFound, ApiError("handoff_not_found")) + } + + log.info("[oauth-handoff] id={} ok", rawId.take(8)) + // Token returned to the app over the same HTTPS channel that + // every other endpoint uses. The app stores it locally and we + // never see it again on the backend. + call.respond(HandoffResponse(token)) + } + } + } +} + +private fun pkceChallenge(verifier: String): String { + val digest = MessageDigest.getInstance("SHA-256").digest(verifier.toByteArray(Charsets.UTF_8)) + return urlBase64.encodeToString(digest) +} + +private fun randomBase64Url(byteCount: Int): String { + val bytes = ByteArray(byteCount) + secureRandom.nextBytes(bytes) + return urlBase64.encodeToString(bytes) +} + +// MessageDigest.isEqual is constant-time on length AND content since Java +// 6u17 — both buffers are read fully even on mismatch. Replaces the earlier +// hand-rolled length-prefix + XOR-or-accumulate, which leaked secret length +// via the early-return on the length check. +private fun constantTimeEquals(a: String, b: String): Boolean = + MessageDigest.isEqual( + a.toByteArray(Charsets.UTF_8), + b.toByteArray(Charsets.UTF_8), + ) diff --git a/src/main/kotlin/zed/rainxch/githubstore/routes/Routing.kt b/src/main/kotlin/zed/rainxch/githubstore/routes/Routing.kt index 977494f..d019c8c 100644 --- a/src/main/kotlin/zed/rainxch/githubstore/routes/Routing.kt +++ b/src/main/kotlin/zed/rainxch/githubstore/routes/Routing.kt @@ -19,6 +19,9 @@ import zed.rainxch.githubstore.badge.BadgeService import zed.rainxch.githubstore.match.ExternalMatchService import zed.rainxch.githubstore.match.SigningFingerprintRepository import zed.rainxch.githubstore.mirrors.MirrorStatusRegistry +import zed.rainxch.githubstore.oauth.OAuthEphemeralStore +import zed.rainxch.githubstore.oauth.OAuthExchangeService +import zed.rainxch.githubstore.oauth.OAuthServiceAuth fun Application.configureRouting() { val repoRepository by inject() @@ -36,6 +39,9 @@ fun Application.configureRouting() { val mirrorStatusRegistry by inject() val announcementsRegistry by inject() val repoRefreshCoordinator by inject() + val oauthStore by inject() + val oauthExchangeService by inject() + val oauthServiceAuth by inject() routing { rootRoutes() @@ -58,6 +64,11 @@ fun Application.configureRouting() { repoRefreshRoutes(repoRefreshCoordinator, repoRepository) } authRoutes(deviceClient) + // OAuth web flow. Each sub-route applies its own rate-limit + // bucket inside oauthRoutes() — wrapping the whole block in + // multiple nested rateLimit() calls would have charged every + // request against every bucket. + oauthRoutes(oauthStore, oauthExchangeService, oauthServiceAuth) internalRoutes(searchMetrics, workerSupervisor, githubSearchClient) rateLimit(RateLimitName("signing-seeds")) { signingSeedsRoutes(signingFingerprintRepository) diff --git a/src/main/resources/db/migration/V16__oauth_ephemeral.sql b/src/main/resources/db/migration/V16__oauth_ephemeral.sql new file mode 100644 index 0000000..727cb51 --- /dev/null +++ b/src/main/resources/db/migration/V16__oauth_ephemeral.sql @@ -0,0 +1,26 @@ +-- Ephemeral key/value store for the web-OAuth handoff flow. +-- +-- Two namespaces share the table: +-- state key = caller-generated random base64url(32 bytes) +-- value = JSON {"code_challenge":""} +-- handoff key = backend-generated random base64url(32 bytes) +-- value = GitHub access_token (plain text) +-- +-- Every row has a 60-second TTL enforced two ways: +-- 1. SELECT/DELETE queries always include `expires_at > NOW()` so a row +-- that slipped past the cleanup worker still appears gone. +-- 2. OAuthCleanupWorker runs every five minutes and DELETEs all rows +-- whose `expires_at` is in the past. +-- +-- Postgres has no native TTL. The combination above is what every +-- "use Postgres like Redis for one job" pattern looks like. +CREATE TABLE oauth_ephemeral ( + namespace VARCHAR(16) NOT NULL, + key VARCHAR(128) NOT NULL, + value TEXT NOT NULL, + expires_at TIMESTAMPTZ NOT NULL, + PRIMARY KEY (namespace, key) +); + +-- Cleanup worker scans by expires_at. Index keeps it cheap. +CREATE INDEX idx_oauth_ephemeral_expires_at ON oauth_ephemeral(expires_at); diff --git a/src/test/kotlin/zed/rainxch/githubstore/oauth/InMemoryOAuthEphemeralStore.kt b/src/test/kotlin/zed/rainxch/githubstore/oauth/InMemoryOAuthEphemeralStore.kt new file mode 100644 index 0000000..fd96dd6 --- /dev/null +++ b/src/test/kotlin/zed/rainxch/githubstore/oauth/InMemoryOAuthEphemeralStore.kt @@ -0,0 +1,59 @@ +package zed.rainxch.githubstore.oauth + +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock +import java.time.Duration +import java.time.Instant + +// Test double for OAuthEphemeralStore. Same SETEX/GETDEL semantics as the +// Postgres impl but lives in-process. Production code MUST use +// PostgresOAuthEphemeralStore — this exists only because the real impl +// requires a live Postgres + the V16 migration, which we don't spin up +// for the route-level unit tests. +class InMemoryOAuthEphemeralStore( + private val clock: () -> Instant = Instant::now, +) : OAuthEphemeralStore { + + private data class Entry(val value: String, val expiresAt: Instant) + + private val mutex = Mutex() + private val rows = HashMap, Entry>() + + override suspend fun setEx( + namespace: String, + key: String, + value: String, + ttl: Duration, + ): Boolean = mutex.withLock { + purgeExpired() + val k = namespace to key + if (rows.containsKey(k)) return@withLock false + rows[k] = Entry(value, clock().plus(ttl)) + true + } + + override suspend fun getDel(namespace: String, key: String): String? = mutex.withLock { + purgeExpired() + rows.remove(namespace to key)?.value + } + + override suspend fun get(namespace: String, key: String): String? = mutex.withLock { + purgeExpired() + rows[namespace to key]?.value + } + + override suspend fun del(namespace: String, key: String) { + mutex.withLock { rows.remove(namespace to key) } + } + + override suspend fun reapExpired(): Int = mutex.withLock { + val before = rows.size + purgeExpired() + before - rows.size + } + + private fun purgeExpired() { + val now = clock() + rows.entries.removeAll { it.value.expiresAt.isBefore(now) || it.value.expiresAt == now } + } +} diff --git a/src/test/kotlin/zed/rainxch/githubstore/oauth/OAuthRoutesTest.kt b/src/test/kotlin/zed/rainxch/githubstore/oauth/OAuthRoutesTest.kt new file mode 100644 index 0000000..d845a6a --- /dev/null +++ b/src/test/kotlin/zed/rainxch/githubstore/oauth/OAuthRoutesTest.kt @@ -0,0 +1,536 @@ +package zed.rainxch.githubstore.oauth + +import ch.qos.logback.classic.Logger +import ch.qos.logback.classic.spi.ILoggingEvent +import ch.qos.logback.core.read.ListAppender +import io.ktor.client.request.* +import io.ktor.client.statement.* +import io.ktor.http.* +import io.ktor.serialization.kotlinx.json.* +import io.ktor.server.application.* +import io.ktor.server.plugins.contentnegotiation.* +import io.ktor.server.plugins.ratelimit.* +import io.ktor.server.routing.* +import io.ktor.server.testing.* +import kotlinx.serialization.json.Json +import kotlinx.serialization.json.jsonObject +import kotlinx.serialization.json.jsonPrimitive +import org.slf4j.LoggerFactory +import zed.rainxch.githubstore.routes.oauthRoutes +import java.security.MessageDigest +import java.security.SecureRandom +import java.util.Base64 +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertTrue +import kotlin.time.Duration.Companion.minutes + +private const val VALID_TOKEN = "test-shared-secret" +private const val ALLOWED_HOST = "api.github-store.org" + +class OAuthRoutesTest { + + private val urlBase64 = Base64.getUrlEncoder().withoutPadding() + private val rng = SecureRandom() + + private lateinit var appender: ListAppender + private lateinit var logger: Logger + + @BeforeTest + fun attachAppender() { + logger = LoggerFactory.getLogger("OAuthRoutes") as Logger + appender = ListAppender().apply { start() } + logger.addAppender(appender) + } + + @AfterTest + fun detachAppender() { + logger.detachAppender(appender) + appender.stop() + } + + private class FakeExchangeService( + private val behaviour: Behaviour, + ) : OAuthExchangeService(clientId = "test-cid", clientSecret = "test-secret", callbackUrl = "https://test/callback") { + sealed class Behaviour { + data class Success(val token: String) : Behaviour() + data class UpstreamError(val code: String) : Behaviour() + data object UpstreamFailure : Behaviour() + } + + var lastCode: String? = null + + override suspend fun exchange(code: String): Result { + lastCode = code + return when (behaviour) { + is Behaviour.Success -> Result.Success(behaviour.token) + is Behaviour.UpstreamError -> Result.UpstreamError(behaviour.code) + Behaviour.UpstreamFailure -> Result.UpstreamFailure + } + } + } + + private fun ApplicationTestBuilder.setupApp( + store: OAuthEphemeralStore, + exchange: OAuthExchangeService, + serviceAuth: OAuthServiceAuth = OAuthServiceAuth(VALID_TOKEN, ALLOWED_HOST), + ) { + application { + install(ContentNegotiation) { json(Json { ignoreUnknownKeys = true; encodeDefaults = true }) } + install(RateLimit) { + register(RateLimitName("oauth-state")) { + rateLimiter(limit = Int.MAX_VALUE, refillPeriod = 1.minutes) + } + register(RateLimitName("oauth-exchange")) { + rateLimiter(limit = Int.MAX_VALUE, refillPeriod = 1.minutes) + } + register(RateLimitName("oauth-handoff")) { + rateLimiter(limit = Int.MAX_VALUE, refillPeriod = 1.minutes) + } + } + routing { + route("/v1") { + oauthRoutes(store, exchange, serviceAuth) + } + } + } + } + + private fun random43(): String { + val bytes = ByteArray(32) + rng.nextBytes(bytes) + return urlBase64.encodeToString(bytes) + } + + private fun challengeOf(verifier: String): String { + val digest = MessageDigest.getInstance("SHA-256").digest(verifier.toByteArray(Charsets.UTF_8)) + return urlBase64.encodeToString(digest) + } + + private fun HttpRequestBuilder.serviceHeaders() { + header("X-Oauth-Service-Token", VALID_TOKEN) + header(HttpHeaders.Host, ALLOWED_HOST) + } + + // ---------- /state ---------- + + @Test + fun `state registers and returns 204`() = testApplication { + val store = InMemoryOAuthEphemeralStore() + setupApp(store, FakeExchangeService(FakeExchangeService.Behaviour.UpstreamFailure)) + + val state = random43() + val challenge = random43() + val resp = client.post("/v1/oauth/state") { + serviceHeaders() + header(HttpHeaders.ContentType, ContentType.Application.Json.toString()) + setBody("""{"state":"$state","code_challenge":"$challenge"}""") + } + assertEquals(HttpStatusCode.NoContent, resp.status) + assertNotNull(store.get(OAuthEphemeralStore.NAMESPACE_STATE, state)) + } + + @Test + fun `state rejects duplicate state with 409`() = testApplication { + val store = InMemoryOAuthEphemeralStore() + setupApp(store, FakeExchangeService(FakeExchangeService.Behaviour.UpstreamFailure)) + + val state = random43() + val challenge = random43() + val body = """{"state":"$state","code_challenge":"$challenge"}""" + client.post("/v1/oauth/state") { + serviceHeaders() + header(HttpHeaders.ContentType, ContentType.Application.Json.toString()) + setBody(body) + } + val resp = client.post("/v1/oauth/state") { + serviceHeaders() + header(HttpHeaders.ContentType, ContentType.Application.Json.toString()) + setBody(body) + } + assertEquals(HttpStatusCode.Conflict, resp.status) + } + + @Test + fun `state rejects malformed values with 400`() = testApplication { + val store = InMemoryOAuthEphemeralStore() + setupApp(store, FakeExchangeService(FakeExchangeService.Behaviour.UpstreamFailure)) + + val resp = client.post("/v1/oauth/state") { + serviceHeaders() + header(HttpHeaders.ContentType, ContentType.Application.Json.toString()) + setBody("""{"state":"short","code_challenge":"${random43()}"}""") + } + assertEquals(HttpStatusCode.BadRequest, resp.status) + assertTrue(resp.bodyAsText().contains("invalid_state")) + } + + // ---------- service auth ---------- + + @Test + fun `state without service token returns 401`() = testApplication { + val store = InMemoryOAuthEphemeralStore() + setupApp(store, FakeExchangeService(FakeExchangeService.Behaviour.UpstreamFailure)) + + val resp = client.post("/v1/oauth/state") { + header(HttpHeaders.Host, ALLOWED_HOST) + header(HttpHeaders.ContentType, ContentType.Application.Json.toString()) + setBody("""{"state":"${random43()}","code_challenge":"${random43()}"}""") + } + assertEquals(HttpStatusCode.Unauthorized, resp.status) + assertTrue(resp.bodyAsText().contains("service_auth_required")) + } + + @Test + fun `state with wrong host returns 401 indistinguishable from wrong-secret`() = testApplication { + // Same body for wrong-secret and wrong-host so a prober can't + // determine which check failed. + val store = InMemoryOAuthEphemeralStore() + setupApp(store, FakeExchangeService(FakeExchangeService.Behaviour.UpstreamFailure)) + + val resp = client.post("/v1/oauth/state") { + header("X-Oauth-Service-Token", VALID_TOKEN) + header(HttpHeaders.Host, "evil.example") + header(HttpHeaders.ContentType, ContentType.Application.Json.toString()) + setBody("""{"state":"${random43()}","code_challenge":"${random43()}"}""") + } + assertEquals(HttpStatusCode.Unauthorized, resp.status) + assertTrue(resp.bodyAsText().contains("service_auth_required")) + assertFalse(resp.bodyAsText().contains("host_not_allowed")) + } + + @Test + fun `state with port-suffixed host still matches allowlist`() = testApplication { + val store = InMemoryOAuthEphemeralStore() + setupApp(store, FakeExchangeService(FakeExchangeService.Behaviour.UpstreamFailure)) + + val resp = client.post("/v1/oauth/state") { + header("X-Oauth-Service-Token", VALID_TOKEN) + header(HttpHeaders.Host, "$ALLOWED_HOST:443") + header(HttpHeaders.ContentType, ContentType.Application.Json.toString()) + setBody("""{"state":"${random43()}","code_challenge":"${random43()}"}""") + } + assertEquals(HttpStatusCode.NoContent, resp.status) + } + + @Test + fun `oauth not configured returns 401 indistinguishable from wrong-secret`() = testApplication { + // Misconfiguration (missing token) and "wrong secret" must look + // identical to an anonymous prober — otherwise the response code + + // body fingerprint the deploy state. + val store = InMemoryOAuthEphemeralStore() + setupApp( + store, + FakeExchangeService(FakeExchangeService.Behaviour.UpstreamFailure), + serviceAuth = OAuthServiceAuth(expectedToken = null, allowedHostsCsv = ALLOWED_HOST), + ) + + val resp = client.post("/v1/oauth/state") { + serviceHeaders() + header(HttpHeaders.ContentType, ContentType.Application.Json.toString()) + setBody("""{"state":"${random43()}","code_challenge":"${random43()}"}""") + } + assertEquals(HttpStatusCode.Unauthorized, resp.status) + assertTrue(resp.bodyAsText().contains("service_auth_required")) + assertFalse(resp.bodyAsText().contains("oauth_not_configured")) + } + + // ---------- /exchange ---------- + + @Test + fun `exchange happy path returns handoff_id and stores token`() = testApplication { + val store = InMemoryOAuthEphemeralStore() + val exchange = FakeExchangeService(FakeExchangeService.Behaviour.Success("gho_secret_test_token")) + setupApp(store, exchange) + + // Pre-register state with challenge + val verifier = random43() + val state = random43() + store.setEx( + OAuthEphemeralStore.NAMESPACE_STATE, + state, + """{"code_challenge":"${challengeOf(verifier)}","created_at_ms":0}""", + java.time.Duration.ofSeconds(60), + ) + + val resp = client.post("/v1/oauth/exchange") { + serviceHeaders() + header(HttpHeaders.ContentType, ContentType.Application.Json.toString()) + setBody("""{"code":"gh-code-abc","state":"$state","code_verifier":"$verifier"}""") + } + assertEquals(HttpStatusCode.OK, resp.status) + val handoffId = Json.parseToJsonElement(resp.bodyAsText()) + .jsonObject["handoff_id"]!!.jsonPrimitive.content + assertTrue(Regex("^[A-Za-z0-9_-]{43}$").matches(handoffId)) + // State was consumed. + assertNull(store.get(OAuthEphemeralStore.NAMESPACE_STATE, state)) + // Handoff stored with the token. + assertEquals("gho_secret_test_token", store.get(OAuthEphemeralStore.NAMESPACE_HANDOFF, handoffId)) + // Exchange service saw the right code. + assertEquals("gh-code-abc", exchange.lastCode) + } + + @Test + fun `exchange with missing state returns 400 state_missing_or_expired`() = testApplication { + val store = InMemoryOAuthEphemeralStore() + setupApp(store, FakeExchangeService(FakeExchangeService.Behaviour.UpstreamFailure)) + + val resp = client.post("/v1/oauth/exchange") { + serviceHeaders() + header(HttpHeaders.ContentType, ContentType.Application.Json.toString()) + setBody("""{"code":"x","state":"${random43()}","code_verifier":"${random43()}"}""") + } + assertEquals(HttpStatusCode.BadRequest, resp.status) + assertTrue(resp.bodyAsText().contains("state_missing_or_expired")) + } + + @Test + fun `exchange with PKCE mismatch returns 400 and burns the state`() = testApplication { + val store = InMemoryOAuthEphemeralStore() + setupApp(store, FakeExchangeService(FakeExchangeService.Behaviour.Success("never-returned"))) + + val state = random43() + val realVerifier = random43() + store.setEx( + OAuthEphemeralStore.NAMESPACE_STATE, + state, + """{"code_challenge":"${challengeOf(realVerifier)}","created_at_ms":0}""", + java.time.Duration.ofSeconds(60), + ) + + // Send a DIFFERENT verifier — challenge will not match. + val attackerVerifier = random43() + val resp = client.post("/v1/oauth/exchange") { + serviceHeaders() + header(HttpHeaders.ContentType, ContentType.Application.Json.toString()) + setBody("""{"code":"x","state":"$state","code_verifier":"$attackerVerifier"}""") + } + assertEquals(HttpStatusCode.BadRequest, resp.status) + assertTrue(resp.bodyAsText().contains("pkce_mismatch")) + // State is burned even on PKCE failure — defence against verifier-guessing. + assertNull(store.get(OAuthEphemeralStore.NAMESPACE_STATE, state)) + } + + @Test + fun `exchange with unknown GitHub error code normalises to github_unknown`() = testApplication { + // F6 — anything outside the whitelist must not propagate verbatim. + val store = InMemoryOAuthEphemeralStore() + setupApp( + store, + FakeExchangeService(FakeExchangeService.Behaviour.UpstreamError("weird_thing_we_dont_know_about")), + ) + + val state = random43() + val verifier = random43() + store.setEx( + OAuthEphemeralStore.NAMESPACE_STATE, + state, + """{"code_challenge":"${challengeOf(verifier)}","created_at_ms":0}""", + java.time.Duration.ofSeconds(60), + ) + + val resp = client.post("/v1/oauth/exchange") { + serviceHeaders() + header(HttpHeaders.ContentType, ContentType.Application.Json.toString()) + setBody("""{"code":"x","state":"$state","code_verifier":"$verifier"}""") + } + assertEquals(HttpStatusCode.BadRequest, resp.status) + val body = resp.bodyAsText() + assertTrue(body.contains("github_unknown"), "should normalise: $body") + assertFalse(body.contains("weird_thing_we_dont_know_about"), "unsanitised: $body") + } + + @Test + fun `exchange with GitHub error bubbles up as github_xxx`() = testApplication { + val store = InMemoryOAuthEphemeralStore() + setupApp(store, FakeExchangeService(FakeExchangeService.Behaviour.UpstreamError("bad_verification_code"))) + + val state = random43() + val verifier = random43() + store.setEx( + OAuthEphemeralStore.NAMESPACE_STATE, + state, + """{"code_challenge":"${challengeOf(verifier)}","created_at_ms":0}""", + java.time.Duration.ofSeconds(60), + ) + + val resp = client.post("/v1/oauth/exchange") { + serviceHeaders() + header(HttpHeaders.ContentType, ContentType.Application.Json.toString()) + setBody("""{"code":"x","state":"$state","code_verifier":"$verifier"}""") + } + assertEquals(HttpStatusCode.BadRequest, resp.status) + assertTrue(resp.bodyAsText().contains("github_bad_verification_code")) + } + + @Test + fun `exchange with upstream failure returns 502 and state is consumed at top by getDel`() = testApplication { + // After the F1+F4 fix, state is always consumed atomically at the + // top of /exchange via getDel. GitHub's authorization `code` is + // single-use upstream, so a retry of the same (state, code) pair is + // never legitimate — the website must start a fresh flow. + val store = InMemoryOAuthEphemeralStore() + setupApp(store, FakeExchangeService(FakeExchangeService.Behaviour.UpstreamFailure)) + + val state = random43() + val verifier = random43() + store.setEx( + OAuthEphemeralStore.NAMESPACE_STATE, + state, + """{"code_challenge":"${challengeOf(verifier)}","created_at_ms":0}""", + java.time.Duration.ofSeconds(60), + ) + + val resp = client.post("/v1/oauth/exchange") { + serviceHeaders() + header(HttpHeaders.ContentType, ContentType.Application.Json.toString()) + setBody("""{"code":"x","state":"$state","code_verifier":"$verifier"}""") + } + assertEquals(HttpStatusCode.BadGateway, resp.status) + assertNull(store.get(OAuthEphemeralStore.NAMESPACE_STATE, state)) + } + + @Test + fun `exchange with concurrent same-state requests - only one wins`() = testApplication { + // Race verification for the getDel atomicity fix. Two requests with + // identical (state, code_verifier) — exactly one should reach the + // GitHub call and succeed; the other must get state_missing_or_expired. + val store = InMemoryOAuthEphemeralStore() + val exchange = FakeExchangeService(FakeExchangeService.Behaviour.Success("token-once")) + setupApp(store, exchange) + + val state = random43() + val verifier = random43() + store.setEx( + OAuthEphemeralStore.NAMESPACE_STATE, + state, + """{"code_challenge":"${challengeOf(verifier)}","created_at_ms":0}""", + java.time.Duration.ofSeconds(60), + ) + + suspend fun call(): HttpStatusCode = client.post("/v1/oauth/exchange") { + serviceHeaders() + header(HttpHeaders.ContentType, ContentType.Application.Json.toString()) + setBody("""{"code":"x","state":"$state","code_verifier":"$verifier"}""") + }.status + + val first = call() + val second = call() + // One success, one state_missing — order independent. + assertTrue( + (first == HttpStatusCode.OK && second == HttpStatusCode.BadRequest) || + (first == HttpStatusCode.BadRequest && second == HttpStatusCode.OK), + "expected one OK and one BadRequest, got $first and $second", + ) + } + + // ---------- /handoff/{id} ---------- + + @Test + fun `handoff returns token then 404 on second call - single-use`() = testApplication { + val store = InMemoryOAuthEphemeralStore() + setupApp(store, FakeExchangeService(FakeExchangeService.Behaviour.UpstreamFailure)) + + val handoffId = random43() + store.setEx( + OAuthEphemeralStore.NAMESPACE_HANDOFF, + handoffId, + "gho_test_secret_token", + java.time.Duration.ofSeconds(60), + ) + + val first = client.post("/v1/oauth/handoff/$handoffId") + assertEquals(HttpStatusCode.OK, first.status) + assertEquals("no-store", first.headers[HttpHeaders.CacheControl]) + assertTrue(first.bodyAsText().contains("gho_test_secret_token")) + + val second = client.post("/v1/oauth/handoff/$handoffId") + assertEquals(HttpStatusCode.NotFound, second.status) + } + + @Test + fun `handoff with malformed id returns 404 without lookup`() = testApplication { + val store = InMemoryOAuthEphemeralStore() + setupApp(store, FakeExchangeService(FakeExchangeService.Behaviour.UpstreamFailure)) + + val resp = client.post("/v1/oauth/handoff/not-a-real-id") + assertEquals(HttpStatusCode.NotFound, resp.status) + } + + @Test + fun `handoff 404 path also sets Cache-Control no-store`() = testApplication { + // F9 — every response from /handoff must be no-store, including 404, + // so a middlebox can't cache the "miss" for an id that might land + // valid moments later. + val store = InMemoryOAuthEphemeralStore() + setupApp(store, FakeExchangeService(FakeExchangeService.Behaviour.UpstreamFailure)) + + val miss = client.post("/v1/oauth/handoff/${random43()}") + assertEquals(HttpStatusCode.NotFound, miss.status) + assertEquals("no-store", miss.headers[HttpHeaders.CacheControl]) + + val malformed = client.post("/v1/oauth/handoff/short") + assertEquals(HttpStatusCode.NotFound, malformed.status) + assertEquals("no-store", malformed.headers[HttpHeaders.CacheControl]) + } + + // ---------- privacy ---------- + + @Test + fun `successful exchange never logs the code, verifier, or access_token`() = testApplication { + val token = "gho_supersecret_token_value" + val store = InMemoryOAuthEphemeralStore() + setupApp(store, FakeExchangeService(FakeExchangeService.Behaviour.Success(token))) + + val state = random43() + val verifier = random43() + val code = "code_must_not_leak_abc" + store.setEx( + OAuthEphemeralStore.NAMESPACE_STATE, + state, + """{"code_challenge":"${challengeOf(verifier)}","created_at_ms":0}""", + java.time.Duration.ofSeconds(60), + ) + + client.post("/v1/oauth/exchange") { + serviceHeaders() + header(HttpHeaders.ContentType, ContentType.Application.Json.toString()) + setBody("""{"code":"$code","state":"$state","code_verifier":"$verifier"}""") + } + + val logged = appender.list.joinToString("\n") { it.formattedMessage } + assertFalse(logged.contains(token), "access_token in log: $logged") + assertFalse(logged.contains(code), "code in log: $logged") + assertFalse(logged.contains(verifier), "verifier in log: $logged") + assertFalse(logged.contains(state), "full state in log: $logged") + // The 8-char prefix IS allowed — needed for correlation. + assertTrue(logged.contains(state.take(8)), "expected state prefix in log") + } + + @Test + fun `successful handoff never logs the token or full handoff_id`() = testApplication { + val token = "gho_handoff_token_secret" + val store = InMemoryOAuthEphemeralStore() + setupApp(store, FakeExchangeService(FakeExchangeService.Behaviour.UpstreamFailure)) + + val handoffId = random43() + store.setEx( + OAuthEphemeralStore.NAMESPACE_HANDOFF, + handoffId, + token, + java.time.Duration.ofSeconds(60), + ) + + client.post("/v1/oauth/handoff/$handoffId") + + val logged = appender.list.joinToString("\n") { it.formattedMessage } + assertFalse(logged.contains(token), "access_token in handoff log: $logged") + assertFalse(logged.contains(handoffId), "full handoff_id in log: $logged") + assertTrue(logged.contains(handoffId.take(8)), "expected handoff_id prefix in log") + } +}