[HYPER-178] use upstream OAuth consent UI instead of broken auth-service reimplementation

.changeset/consent-upstream-oauth-ui.md

@@ -0,0 +1,35 @@
+---
+'epds': patch
+---
+
+The permissions shown on the sign-in approval screen now match what the app actually asked for.
+
+**Affects:** End users, Client app developers, Operators
+
+**End users:** When you sign in to a third-party app through ePDS and
+are asked to approve what the app can do with your account, the list
+you see now reflects the permissions that particular app actually
+requested. Previously the screen always showed the same hard-coded
+list ("Read and write posts", "Access your profile", "Manage your
+follows") no matter which app you were signing in to, which was
+misleading. The approval screen itself also now looks and behaves
+like the standard AT Protocol consent screen used elsewhere in the
+ecosystem.
+
+**Client app developers:** The consent screen rendered at
+`/oauth/authorize` is now the stock `@atproto/oauth-provider`
+`consent-view.tsx`, driven by the real `scope` / `permissionSets`
+your client requests. The previous auth-service implementation
+ignored the requested scopes entirely. After OTP verification and
+(for new users) account creation, `epds-callback` now binds the
+device session via `upsertDeviceAccount()` and redirects through
+`/oauth/authorize`, so the upstream `oauthMiddleware` runs
+`provider.authorize()` — including `checkConsentRequired()` — against
+the actual request. Clients that only need scopes the user has
+already approved will now be auto-approved instead of being shown a
+redundant consent screen.
+
+**Operators:** On upgrade, the `client_logins` table is dropped by
+migration v8. Consent state now lives in the upstream provider's
+`authorizedClients` tracking, so nothing else needs to move. No
+configuration changes are required.

.env.example

@@ -85,6 +85,14 @@ PDS_EMAIL_SMTP_URL=smtp://localhost:1025
 [email protected]
 PDS_BLOBSTORE_DISK_LOCATION=/data/blobs
 
+# Trusted OAuth clients — comma-separated client_id URLs.
+# PDS_OAUTH_TRUSTED_CLIENTS=https://app.example/client-metadata.json
+
+# Skip consent on sign-up for trusted clients that request it.
+# Requires PDS_OAUTH_TRUSTED_CLIENTS and client metadata with
+# "epds_skip_consent_on_signup": true. Default: false.
+# PDS_SIGNUP_ALLOW_CONSENT_SKIP=false
+
 # Invite code for automated account creation (ePDS creates accounts on first login).
 # Required when PDS_INVITE_REQUIRED is true (the default).
 # Generate with:

.github/workflows/e2e-pr.yml

@@ -468,19 +468,22 @@ jobs:
           EPDS_URL="https://certified-apppds-core-${ENV_SLUG}.up.railway.app"
           AUTH_URL="https://certified-appauth-service-${ENV_SLUG}.up.railway.app"
           DEMO_URL="https://certified-appdemo-${ENV_SLUG}.up.railway.app"
+          DEMO_UNTRUSTED_URL="https://certified-appdemo-untrusted-${ENV_SLUG}.up.railway.app"
           MAILPIT_URL="https://mailpit-${ENV_SLUG}.up.railway.app"
 
           echo
           echo "Service URLs:"
-          echo "  PDS:     $EPDS_URL"
-          echo "  Auth:    $AUTH_URL"
-          echo "  Demo:    $DEMO_URL"
-          echo "  Mailpit: $MAILPIT_URL"
+          echo "  PDS:            $EPDS_URL"
+          echo "  Auth:           $AUTH_URL"
+          echo "  Demo:           $DEMO_URL"
+          echo "  Demo untrusted: $DEMO_UNTRUSTED_URL"
+          echo "  Mailpit:        $MAILPIT_URL"
 
           {
             echo "epds_url=$EPDS_URL"
             echo "auth_url=$AUTH_URL"
             echo "demo_url=$DEMO_URL"
+            echo "demo_untrusted_url=$DEMO_UNTRUSTED_URL"
             echo "mailpit_url=$MAILPIT_URL"
           } >> "$GITHUB_OUTPUT"
 
@@ -492,9 +495,10 @@ jobs:
           EPDS_URL: ${{ steps.urls.outputs.epds_url }}
           AUTH_URL: ${{ steps.urls.outputs.auth_url }}
           DEMO_URL: ${{ steps.urls.outputs.demo_url }}
+          DEMO_UNTRUSTED_URL: ${{ steps.urls.outputs.demo_untrusted_url }}
           MAILPIT_URL: ${{ steps.urls.outputs.mailpit_url }}
         run: |
-          echo "Health-checking 4 Railway services (5 minutes per service)..."
+          echo "Health-checking 5 Railway services (5 minutes per service)..."
 
           check() {
             local LABEL=$1
@@ -514,19 +518,21 @@ jobs:
             echo "::error::$LABEL ($URL) timed out after 5 minutes"
             exit 1
           }
-          check "PDS"     "${EPDS_URL}/health"
-          check "Auth"    "${AUTH_URL}/health"
-          check "Demo"    "${DEMO_URL}"
-          check "Mailpit" "${MAILPIT_URL}/readyz"
+          check "PDS"            "${EPDS_URL}/health"
+          check "Auth"           "${AUTH_URL}/health"
+          check "Demo"           "${DEMO_URL}"
+          check "Demo untrusted" "${DEMO_UNTRUSTED_URL}"
+          check "Mailpit"        "${MAILPIT_URL}/readyz"
 
           echo
-          echo "All 4 services reachable."
+          echo "All 5 services reachable."
 
       - name: Run e2e suite
         env:
           E2E_PDS_URL: ${{ steps.urls.outputs.epds_url }}
           E2E_AUTH_URL: ${{ steps.urls.outputs.auth_url }}
           E2E_DEMO_URL: ${{ steps.urls.outputs.demo_url }}
+          E2E_DEMO_UNTRUSTED_URL: ${{ steps.urls.outputs.demo_untrusted_url }}
           E2E_MAILPIT_URL: ${{ steps.urls.outputs.mailpit_url }}
           E2E_MAILPIT_USER: ${{ secrets.E2E_MAILPIT_USER }}
           E2E_MAILPIT_PASS: ${{ secrets.E2E_MAILPIT_PASS }}

AGENTS.md

@@ -47,6 +47,29 @@ pnpm vitest run packages/shared
 Tests live in `packages/<name>/src/__tests__/`. There is no per-package test
 script — all tests are run from the root via vitest.
 
+### End-to-end tests in CI
+
+The e2e suite lives in `e2e/` and its feature files in `features/`. Normally
+the `E2E tests` workflow (`.github/workflows/e2e-pr.yml`) runs itself off
+Railway's `deployment_status` webhook — no action needed on an ordinary PR.
+
+To manually trigger it against a Railway environment (for e2e-only changes
+that don't cause a rebuild, or to re-run without a new commit), **always
+pass both `--ref` and `-f env_name`**:
+
+```bash
+gh workflow run e2e-pr.yml \
+  --ref <your-branch> \
+  -f env_name="ePDS / ePDS-pr-<N>"
+```
+
+`--ref` controls which version of the feature files, step definitions, and
+workflow YAML get checked out. Without it, `gh workflow run` defaults to
+`main` and you'll silently test old code against the right environment.
+See [`e2e/README.md`](e2e/README.md#running-the-ci-e2e-job-against-a-railway-environment)
+for details (env-name formats, URL derivation, how to handle missing Railway
+domains).
+
 ### Writing Tests
 
 Before designing or writing new tests, read
@@ -76,6 +99,31 @@ Follow these guidelines when adding tests:
 - **Ratchet thresholds** — after improving coverage, bump the thresholds in
   `vitest.config.ts` so coverage cannot regress.
 
+### Coverage Ratcheting Policy
+
+Coverage thresholds in `vitest.config.ts` must **only ever increase**.
+When a PR increases coverage above the current thresholds, the thresholds
+must be ratcheted up to the new floor (rounded down to the nearest integer)
+as part of the same PR. This ensures coverage can never regress.
+
+```bash
+pnpm test:coverage   # check current coverage vs thresholds
+```
+
+After confirming coverage exceeds thresholds, update `vitest.config.ts`:
+
+```ts
+thresholds: {
+  statements: <new floor>,
+  branches: <new floor>,
+  functions: <new floor>,
+  lines: <new floor>,
+},
+```
+
+**Never lower thresholds.** If a change removes tested code (e.g. deleting
+a feature), add tests for other code to compensate.
+
 ## Docker
 
 ```bash

docs/design/consent-flow-fix.md

@@ -0,0 +1,193 @@
+# Design: Fix Consent Flow to Use Upstream OAuth UI
+
+## Problem
+
+The ePDS consent page (`packages/auth-service/src/routes/consent.ts`) is a
+broken reimplementation of the consent UI that already exists in the upstream
+`@atproto/oauth-provider-ui` package. Specifically:
+
+1. **Hard-coded permissions** — the consent page shows "Read and write posts",
+   "Access your profile", "Manage your follows" regardless of what OAuth scopes
+   the client actually requested.
+
+2. **Ignores OAuth scopes entirely** — never reads `parameters.scope` from the
+   PAR request. A client requesting `transition:chat.bsky` would still see
+   generic permissions.
+
+3. **Separate consent tracking** — uses its own `client_logins` table
+   (`hasClientLogin` / `recordClientLogin`) instead of the atproto provider's
+   built-in `authorizedClients` tracking, which already handles scope-level
+   consent (re-prompting when new scopes are requested).
+
+4. **Missing consent on signup** — the new-user flow (handle picker →
+   `epds-callback`) bypasses `complete.ts` step 5c, so `recordClientLogin` is
+   never called. The user sees the consent page on their second login instead
+   of their first.
+
+## Root Cause
+
+The `epds-callback` endpoint bypasses the stock OAuth middleware entirely. It
+calls `requestManager.get()` and `requestManager.setAuthorized()` directly
+instead of going through `provider.authorize()`, which is what the stock PDS
+uses (via `oauthMiddleware` in `auth-routes.ts`).
+
+The stock PDS delegates the full authorization UI to `oauthMiddleware` from
+`@atproto/oauth-provider`, which serves a React-based UI from the
+`@atproto/oauth-provider-ui` package. This UI includes a proper consent view
+(`consent-view.tsx`) that displays actual requested scopes/permissionSets.
+
+## Fix: Hand Back to the Stock OAuth Flow After Authentication
+
+After the auth-service authenticates the user (OTP) and pds-core creates the
+account (if new), redirect back through the stock `/oauth/authorize` endpoint
+instead of calling `setAuthorized()` directly.
+
+### Flow Change
+
+**Current flow (broken consent):**
+
+```text
+OTP verify → /auth/complete → check consent (auth-service) →
+  → show broken consent page (auth-service) →
+  → /oauth/epds-callback → setAuthorized() → redirect to client
+```
+
+**Fixed flow:**
+
+```text
+OTP verify → /auth/complete →
+  → /oauth/epds-callback → create account if needed, upsertDeviceAccount →
+  → redirect to /oauth/authorize?request_uri=... (stock middleware) →
+  → stock middleware calls provider.authorize() →
+  → if consent needed: renders upstream consent-view.tsx with real scopes →
+  → if no consent needed: auto-approves (SSO match) →
+  → redirect to client
+```
+
+### Implementation Steps
+
+#### Step 1: Modify `epds-callback` to redirect through stock OAuth flow
+
+In `packages/pds-core/src/index.ts`, after account creation and
+`upsertDeviceAccount`, instead of calling `requestManager.setAuthorized()`
+and building the redirect URL ourselves:
+
+- Redirect to `/oauth/authorize?request_uri=<requestUri>&client_id=<clientId>`
+- The stock `oauthMiddleware` will handle this request, find the existing
+  device session (just created via `upsertDeviceAccount`), check consent via
+  `provider.authorize()`, and either auto-approve or show the upstream
+  consent UI.
+
+The `epds-callback` handler should **stop calling**:
+
+- `requestManager.setAuthorized()`
+- `provider.checkConsentRequired()`
+- `provider.accountManager.setAuthorizedClient()`
+
+These are all handled internally by the stock middleware when it processes the
+`/oauth/authorize` redirect.
+
+#### Step 2: Remove auth-service consent page
+
+Delete or disable:
+
+- `packages/auth-service/src/routes/consent.ts`
+- The consent route registration in `packages/auth-service/src/index.ts`
+- The `needsConsent` check in `packages/auth-service/src/routes/complete.ts`
+  (step 5b) — the auth-service no longer decides whether consent is needed
+- `ctx.db.hasClientLogin()` and `ctx.db.recordClientLogin()` methods
+- The `client_login` table (add a migration to drop it)
+
+#### Step 3: Simplify `complete.ts`
+
+`/auth/complete` no longer needs to check consent. Its only job is:
+
+1. Verify the auth flow cookie and better-auth session
+2. For new users → redirect to `/auth/choose-handle`
+3. For existing users → redirect to `/oauth/epds-callback` (which then
+   redirects through the stock OAuth flow)
+
+#### Step 4: Verify the stock middleware handles the redirect correctly
+
+The key assumption to verify: when `oauthMiddleware` receives
+`/oauth/authorize?request_uri=...` and finds an existing device session
+(created by `upsertDeviceAccount` moments earlier), it should:
+
+- Call `provider.authorize()` which returns `sessions` with
+  `consentRequired` and `loginRequired` flags
+- Since `loginRequired` should be false (device session just created) and
+  `consentRequired` depends on whether this client was previously authorized,
+  it should either auto-approve or show consent
+- The `permissionSets` from the requested scopes will be displayed correctly
+
+**Risk**: the stock middleware might require a full browser login flow (cookies,
+CSRF) rather than just a device session. This needs to be tested. If the
+middleware requires its own session state, we may need to ensure that the
+device session created by `upsertDeviceAccount` is sufficient for the
+middleware to recognize the user as authenticated.
+
+### Consent-Skip on Sign-Up
+
+For trusted clients, a new user flow can optionally skip the consent screen
+entirely on initial sign-up. This requires all three conditions:
+
+1. **`PDS_SIGNUP_ALLOW_CONSENT_SKIP=true`** in the PDS environment
+2. **Client is trusted** — listed in `PDS_OAUTH_TRUSTED_CLIENTS`
+3. **Client metadata** includes `"epds_skip_consent_on_signup": true`
+
+When all conditions are met, `epds-callback` issues the authorization code
+directly (via `requestManager.setAuthorized()`) and records the client's
+scopes as authorized (via `setAuthorizedClient()`). The browser goes straight
+to the client app without any intermediate screen.
+
+When consent-skip does NOT apply (e.g. untrusted client, env var disabled),
+new users are redirected to `/oauth/authorize?prompt=consent`, which skips
+the account selection screen (unnecessary for a just-created account) and
+shows the upstream consent UI directly.
+
+Existing users always go through the normal `/oauth/authorize` flow (no
+`prompt` override) which auto-approves or shows consent as appropriate.
+
+### What This Fixes
+
+- Consent page shows actual requested scopes (from upstream `consent-view.tsx`)
+- Consent tracking uses the atproto provider's `authorizedClients` system
+  (scope-aware, re-prompts for new scopes)
+- No more hard-coded "Read and write posts" etc.
+- No more separate `client_login` table
+- New users see consent at the right time (determined by the provider)
+- Trusted clients can skip consent on sign-up (opt-in, triple-gated)
+- Removes ~250 lines of auth-service code (`consent.ts` + DB methods)
+
+### Research Findings (Resolved Questions)
+
+1. **Device identification** — the stock middleware uses `dev-id` and `ses-id`
+   HTTP cookies (set by `deviceManager.load()` with ~10-year expiry). Since
+   the browser carries these cookies on both the original `/oauth/authorize`
+   visit and the redirect back from `epds-callback`, the deviceId will match.
+   `upsertDeviceAccount(deviceId, sub)` creates the device-account association
+   that `authorize()` finds via `listDeviceAccounts(deviceId)`.
+
+2. **PAR request state** — `requestManager.get()` called WITHOUT a deviceId
+   (as we now do in `epds-callback`) does NOT bind a deviceId to the request.
+   When the stock middleware subsequently calls `get(requestUri, deviceId,
+clientId)`, it binds the browser's deviceId for the first time. This is
+   the expected PAR → redirect → authorize flow.
+
+3. **Auto-approve conditions** — `provider.authorize()` auto-approves when:
+   - `prompt=none` with a single matching session (no login/consent required)
+   - No explicit prompt + `login_hint` matching one session (no login/consent)
+   - For unauthenticated clients (`token_endpoint_auth_method: 'none'`),
+     `requestManager.validate()` forces `prompt: 'consent'`, so consent is
+     always shown — this is correct behavior.
+
+4. **Consent UI rendering** — the stock middleware serves a React SPA from
+   `@atproto/oauth-provider-ui`. It injects hydration data (requestUri,
+   clientMetadata, scope, permissionSets, sessions with consentRequired flags)
+   as `window.__authorizeData`. The SPA calls back to
+   `/oauth/authorize/api/consent` which internally calls `setAuthorized()`.
+
+5. **AS metadata override** — the redirect from `epds-callback` to
+   `/oauth/authorize` is a 303 redirect on the same pds-core host. The AS
+   metadata's `authorization_endpoint` (pointing to the auth service) is
+   irrelevant here since the browser follows the redirect directly.