Add OAuth flow

[ScriptSmith]

No description provided.

[greptile-apps]

Greptile Summary

This PR adds a full OAuth 2.0 PKCE authorization-code flow that lets external apps obtain user-scoped Hadrian API keys after the user grants consent on a new hosted consent page, following the same pattern as OpenRouter's PKCE flow. The implementation is carefully security-conscious: the consent UI gates rendering on a server-side preflight check (closing the Deny-redirect bypass), codes are looked up before PKCE verification so a wrong verifier doesn't burn a legitimate code, and membership + key-count limits are re-validated at token exchange time. The remaining P2 findings are: check_owner_create_authz (RBAC permission check) is not repeatable at the public token endpoint due to architectural constraints; the flow defaults to enabled = true and will activate new public endpoints on any upgrade without operator opt-in; and a dead "[::1]" arm in the Rust loopback check.

Confidence Score: 4/5

Safe to merge; all P1s from prior rounds appear addressed and remaining findings are P2 style concerns.

No P0 or P1 issues found after reviewing the full implementation. Previously flagged critical issues (deny-redirect bypass, code burning before PKCE verification, RBAC TOCTOU) are all addressed. Remaining P2 items are: RBAC authz check not repeatable at the public token endpoint (architectural constraint), enabled = true by default activating new public endpoints on upgrade, a dead "[::1]" arm in the Rust loopback check, and a SQLite 3.35+ requirement for RETURNING. Score capped at 4/5 to reflect the open P2 items on a security-sensitive flow.

src/routes/oauth_public.rs (RBAC re-check gap), src/config/auth.rs (default-enabled behavior), src/routes/admin/oauth.rs (dead IPv6 branch)

Important Files Changed

Filename	Overview
src/services/oauth_pkce.rs	Core PKCE service: lookup-then-verify-then-consume ordering is correct and prevents the code-burn DoS; constant-time comparison via subtle is properly used.
src/routes/admin/oauth.rs	Authorize and preflight endpoints look solid; validate_callback_url correctly gates both paths. Minor: the "[::1]" arm in the loopback check is dead code since url::Url::host_str() strips brackets.
src/routes/oauth_public.rs	Public token endpoint now re-validates membership and key-count limits at exchange time; check_owner_create_authz (RBAC) is intentionally omitted due to missing auth context — worth documenting. derive_issuer correctly ignores X-Forwarded-* headers.
src/config/auth.rs	OAuthPkceConfig is well-validated; allow/deny list logic is correct and well-tested. Defaults to enabled = true, which activates new public endpoints on upgrade without operator opt-in.
ui/src/pages/OAuthAuthorizePage.tsx	Consent UI gates itself on a server-side preflight check before rendering — this correctly prevents the Deny button from redirecting to a policy-denied host. The two-step wizard (scopes → key details) is well-structured.
src/db/sqlite/oauth_authorization_codes.rs	SQLite implementation is correct; atomic consume uses UPDATE…RETURNING with expiry guard. Note: RETURNING requires SQLite ≥ 3.35, which may not be present on older distros.
src/db/postgres/oauth_authorization_codes.rs	Postgres implementation is correct; all operations use the write pool appropriately, and the atomic UPDATE…RETURNING for consume correctly guards on both used_at and expires_at.
src/jobs/oauth_code_cleanup.rs	Simple cleanup worker that fires first after 10 minutes (sleep-first avoids startup races) and loops indefinitely; errors are logged as warnings, not panics.
migrations_sqlx/postgres/20250101000000_initial.sql	Adds oauth_authorization_codes table and oauth_pkce_method enum idempotently; indexes on code, user_id, and expires_at are appropriate for lookup, ownership queries, and cleanup scans.
src/db/repos/oauth_authorization_codes.rs	Trait doc is now accurate: delete_stale removes all consumed codes (regardless of age) and expired-but-unclaimed codes before the cutoff — matches both DB implementations.

Sequence Diagram

sequenceDiagram
    participant App as External App
    participant Browser as User Browser
    participant UI as Hadrian UI /oauth/authorize
    participant Admin as Admin API /admin/v1/oauth/
    participant Public as Public API /oauth/token
    participant DB as Database

    App->>Browser: Redirect to /oauth/authorize?callback_url=...&code_challenge=...
    Browser->>UI: Load consent page
    UI->>Admin: GET /admin/v1/oauth/preflight?callback_url=... (authenticated)
    Admin->>Admin: validate_callback_url (scheme + allow/deny lists)
    Admin-->>UI: 200 OK / 403 Forbidden
    Note over UI: Renders consent UI only if preflight passes
    Browser->>UI: User clicks Authorize (with key options)
    UI->>Admin: POST /admin/v1/oauth/authorize (authenticated)
    Admin->>Admin: check_owner_create_authz + check_owner_create_limits
    Admin->>Admin: validate_callback_url (re-validated)
    Admin->>DB: INSERT oauth_authorization_codes
    Admin-->>UI: redirect_url with code
    UI->>Browser: window.location = redirect_url
    Browser->>App: GET callback_url?code=...
    App->>Public: POST /oauth/token with code and code_verifier
    Public->>DB: lookup_active(code) no mutation
    Public->>Public: derive_challenge(verifier) ct_eq stored challenge
    Public->>DB: consume(code) atomic UPDATE WHERE used_at IS NULL
    Public->>Admin: check_owner_membership_for_user (re-validated)
    Public->>Admin: check_owner_create_limits (re-validated)
    Public->>Public: services.api_keys.create
    Public-->>App: key, key_prefix, key_id

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/routes/admin/oauth.rs
Line: 54

Comment:
**Dead `"[::1]"` branch in loopback check**

`url::Url::host_str()` in Rust's `url` crate strips the square brackets from IPv6 literals, so the host string for `http://[::1]:8080/` is `"::1"`, never `"[::1]"`. The `"[::1]"` arm in the `matches!` macro is therefore unreachable — the IPv6 loopback is already handled by `"::1"`. This is not a security issue (HTTP to `::1` is still correctly allowed), but the dead branch is misleading.

```suggestion
    let is_loopback = matches!(host.as_str(), "localhost" | "127.0.0.1" | "::1");
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/routes/oauth_public.rs
Line: 159-170

Comment:
**`check_owner_create_authz` not re-run at exchange time**

The comment at line 159 correctly identifies the TOCTOU window and re-checks both membership (`check_owner_membership_for_user`) and key-count limits (`check_owner_create_limits`). However, the RBAC authorization check — `check_owner_create_authz` — that ran in `POST /admin/v1/oauth/authorize` is not repeated here.

In deployments with RBAC enabled (`auth.rbac.enabled = true`), a user's permission to create API keys for an org/team/project can be revoked independently of their membership. If that permission is revoked in the 10-minute (up to 60-minute) window between consent and exchange, the key will still be issued as long as the membership check passes.

This is an architectural constraint — the public token endpoint has no session/auth context to pass to `check_owner_create_authz` — so it may be intentional. Worth documenting explicitly in the code comment, or considering whether a lightweight "can this user still create keys for this owner?" DB-level check can be added without requiring a full auth context.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/config/auth.rs
Line: 2016-2017

Comment:
**OAuth PKCE flow enabled by default on all upgrades**

`OAuthPkceConfig` defaults to `enabled: true`. This means upgrading an existing Hadrian deployment automatically exposes two new public endpoints (`/oauth/token`, `/.well-known/oauth-authorization-server`) and two new authenticated admin endpoints without any operator opt-in action.

While the endpoints are secure by design, operators with strict surface-area policies may not expect this feature to light up on upgrade. Consider whether the default should be `false` (opt-in), or at minimum call this out prominently in the upgrade / changelog notes so operators aren't surprised.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/db/sqlite/oauth_authorization_codes.rs
Line: 143-154

Comment:
**`RETURNING` in SQLite requires 3.35.0+**

The `consume` implementation (line 121) uses `RETURNING`, which was introduced in SQLite 3.35 (March 2021). `delete_stale` doesn't use it, but it's worth noting this minimum SQLite version requirement in the docs or a startup check, since some Linux distros (e.g. Ubuntu 20.04) ship older SQLite versions. If `sqlx` enforces a minimum version check elsewhere this is already covered, but it isn't obvious from this file.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (5): Last reviewed commit: "Review fixes" | Re-trigger Greptile}

[ScriptSmith]

@greptile-apps

[ScriptSmith]

@greptile-apps

[ScriptSmith]

@greptile-apps