From 0af7d294a8675c568a441a332a3cb190e3f97e93 Mon Sep 17 00:00:00 2001 From: RainbowBird Date: Tue, 28 Apr 2026 16:56:51 +0800 Subject: [PATCH] feat(auth): oidc jwt bearer plugin & linked social accounts (#1753) Co-authored-by: Liet Blue <127093491+lietblue@users.noreply.github.com> --- .../src/libs/auth-plugins/oidc-jwt-bearer.ts | 275 +++++++++++++ apps/server/src/libs/auth.ts | 99 ++++- apps/server/src/libs/gravatar.ts | 82 ++++ apps/server/src/middlewares/auth.ts | 5 - apps/server/src/routes/oidc/token-auth.ts | 25 +- apps/ui-server-auth/package.json | 1 + .../ui-server-auth/src/modules/auth-client.ts | 78 ++++ .../src/modules/profile.test.ts | 23 +- apps/ui-server-auth/src/modules/profile.ts | 136 ++++--- .../src/modules/sign-in.test.ts | 35 ++ apps/ui-server-auth/src/modules/sign-in.ts | 14 +- apps/ui-server-auth/src/pages/profile.vue | 370 ++++++++++++++++-- packages/i18n/src/locales/en/server/auth.yaml | 30 +- packages/i18n/src/locales/en/settings.yaml | 28 ++ .../i18n/src/locales/zh-Hans/server/auth.yaml | 30 +- .../i18n/src/locales/zh-Hans/settings.yaml | 28 ++ .../account/account-settings-page.vue | 298 +++++++++++++- packages/stage-ui/src/composables/index.ts | 1 + .../src/composables/use-linked-accounts.ts | 250 ++++++++++++ pnpm-lock.yaml | 3 + 20 files changed, 1678 insertions(+), 133 deletions(-) create mode 100644 apps/server/src/libs/auth-plugins/oidc-jwt-bearer.ts create mode 100644 apps/server/src/libs/gravatar.ts create mode 100644 apps/ui-server-auth/src/modules/auth-client.ts create mode 100644 packages/stage-ui/src/composables/use-linked-accounts.ts diff --git a/apps/server/src/libs/auth-plugins/oidc-jwt-bearer.ts b/apps/server/src/libs/auth-plugins/oidc-jwt-bearer.ts new file mode 100644 index 000000000..6338b61cc --- /dev/null +++ b/apps/server/src/libs/auth-plugins/oidc-jwt-bearer.ts @@ -0,0 +1,275 @@ +import type { BetterAuthPlugin } from 'better-auth' +import type { JSONWebKeySet } from 'jose' + +import type { Env } from '../env' + +import { createHmac } from 'node:crypto' + +import { createAuthMiddleware } from 'better-auth/api' +import { createLocalJWKSet, jwtVerify } from 'jose' + +/** + * Bridge plugin that lets better-auth's `sessionMiddleware` accept the + * RS256 JWT access tokens minted by our own oauthProvider plugin, instead + * of only the HMAC-signed session tokens that the stock {@link bearer} + * plugin understands. + * + * Use when: + * - The same Hono app hosts both the OIDC IdP (oauthProvider) and the + * resource server (`/api/v1/*`, `/api/auth/*`). Stage-web / Electron / + * Pocket clients carry an OIDC JWT for everything; without this plugin + * their `Authorization: Bearer ` is silently rejected by every + * `/api/auth/*` endpoint that needs `c.context.session`. + * + * Why a plugin (vs. per-route shims): + * - The `before` hook fires before `sessionMiddleware`, so a single + * translation here lets every better-auth endpoint (current + future) + * accept JWTs. Per-route shims would have to be rewritten for each new + * endpoint we expose to OIDC clients. + * + * Architecture mismatch this paves over: + * - better-auth's official OIDC story assumes the IdP and the resource + * server are different processes / different trust domains. The IdP + * issues JWTs for *external* RSes; the IdP itself only authenticates + * its own admin / profile API via cookies + HMAC bearer. Hosting both + * in one process is uncommon upstream, hence the gap. + * + * Mechanism: + * 1. Detect a JWT-shaped Bearer token (3 base64url segments). + * 2. Verify it via the local JWKS endpoint (the same RS256 keys our + * oauthProvider plugin signs with). If verification fails, bail out + * so the stock {@link bearer} plugin can still try its HMAC path. + * 3. Mint a short-lived bridge `session` row (5 min TTL via the + * `override.expiresAt` parameter on `internalAdapter.createSession`). + * Reusing an existing OIDC-flow session would seem cheaper, but it + * would let a refreshed-after-sign-out JWT silently keep working + * until its own TTL — minting anew avoids that surprise. + * 4. Sign the session token the same way better-auth's bearer plugin + * does (`serializeSignedCookie('', token, secret)` then strip the `=`), + * inject it as the `better-auth.session_token` cookie on the request + * headers, and let `sessionMiddleware` resolve from there as if a + * real cookie had been sent. + * + * NOTICE: + * - We intentionally only run on JWT-shaped tokens. HMAC tokens (no `.`s + * in the obvious places, or fail JWKS verify) are passed through to + * the stock {@link bearer} plugin so the existing better-auth-only + * clients keep working. + * - The bridge session table grows by one row per JWT-authed `/api/auth/*` + * request. With a 5-minute TTL the steady-state size is bounded; if + * that becomes load-bearing we can swap in a per-jti cache. + * - Mirror of `bearer()`'s cookie injection trick: + * node_modules/better-auth/dist/plugins/bearer/index.mjs L26-58. + * Removal condition: better-auth ships a first-party way to verify + * externally-signed JWTs against a JWKS for its own session resolution. + */ +export function oidcJwtBearer(env: Env): BetterAuthPlugin { + // JWT shape: three base64url segments separated by dots. Catches the + // happy path without us decoding; downstream JWKS verify is the real + // gate. Anything that fails this regex falls through to bearer(). + const JWT_SHAPE_RE = /^[\w-]+\.[\w-]+\.[\w-]+$/ + + // Bridge session lifetime. Long enough to span an OAuth round-trip + // (link-social → provider → callback) on slow networks; short enough + // that an unused row TTL-prunes quickly. + const BRIDGE_SESSION_TTL_MS = 5 * 60 * 1000 + + // Process-local JWKS cache. + // + // NOTICE: + // Why local (not `createRemoteJWKSet`): we are the JWKS endpoint. Using + // jose's remote variant would loopback-fetch `/api/auth/jwks` on the same + // process, which both costs 5+ seconds in slow-DB environments AND + // contends for the same Postgres connection pool we're already inside — + // observed as 5s `Connection terminated due to connection timeout` from + // the `jwks` SELECT during a JWT-authed `/api/auth/list-accounts`. We + // read the jwks table directly via the better-auth adapter and assemble + // the JWKS in-process. Cached for 60s so steady-state traffic is + // effectively no-op; rotations propagate within a minute. + // Source: better-auth jwt plugin endpoint that builds the same shape: + // node_modules/better-auth/dist/plugins/jwt/index.mjs L102-129. + // Removal condition: never — local JWKS is the right primitive when the + // server *is* the IdP. Only revisit if jwks ever moves out of process. + const JWKS_TTL_MS = 60 * 1000 + let cachedKeySet: ReturnType | null = null + let cachedAt = 0 + + interface JwkRow { + id: string + publicKey: string + alg?: string + crv?: string + expiresAt?: Date | null + } + + /** + * Build (or reuse) the local JWKS resolver from rows in the `jwks` table. + * + * Use when: + * - About to verify a JWT inside this plugin and we need an up-to-date + * `JWKSLike` callable for `jwtVerify`. + * + * Expects: + * - `c.context.adapter.findMany({ model: 'jwks' })` returns a list of + * rows shaped like {@link JwkRow}. + * + * Returns: + * - The same `createLocalJWKSet` callable on cache hit; freshly assembled + * one on miss / expiry. Returns `null` if no keys are present (better + * to bail than to lock everyone out — bearer() may still succeed). + */ + async function getOrLoadJWKS( + adapter: { findMany: (args: { model: string }) => Promise }, + ): Promise | null> { + if (cachedKeySet && Date.now() - cachedAt < JWKS_TTL_MS) + return cachedKeySet + + const rows = await adapter.findMany({ model: 'jwks' }) as JwkRow[] + const now = Date.now() + const keys = rows + .filter(row => !row.expiresAt || row.expiresAt.getTime() > now) + // NOTICE: + // `JSON.parse` is intentionally not wrapped in try/catch. The + // `publicKey` column is written exclusively by better-auth's jwt + // plugin via `JSON.stringify(publicWebKey)` (see + // node_modules/@better-auth/core/dist/plugins/jwt/utils.mjs L30, L50) + // — i.e. data we ourselves serialised. A parse failure means the + // table is corrupt or upstream's serialisation contract has + // shifted, and both are exactly the cases that should fail loud + // (5xx + alert) rather than be silently skipped, which would + // 401 every JWT signed with the dropped key and bury the root + // cause. PR #1753 review suggested adding the try/catch; declined + // for the reason above. + .map((row) => { + const publicKey = JSON.parse(row.publicKey) as Record + return { + ...(row.alg ? { alg: row.alg } : {}), + ...(row.crv ? { crv: row.crv } : {}), + ...publicKey, + kid: row.id, + } + }) + + if (keys.length === 0) + return null + + const jwks: JSONWebKeySet = { keys: keys as JSONWebKeySet['keys'] } + cachedKeySet = createLocalJWKSet(jwks) + cachedAt = Date.now() + return cachedKeySet + } + + /** + * Inline copy of `better-call`'s `signCookieValue`. + * + * Use when: + * - Producing a session-token cookie value that the stock {@link bearer} + * plugin would also accept on the verify path. + * + * Format: + * - HMAC-SHA-256 the raw value with `secret`, base64-encode the digest, + * join as `value.signature`, then URI-encode. Mirrors the upstream + * recipe at node_modules/better-call/dist/crypto.mjs L27-32. + * + * Why inline (not import from better-call): better-call is a transitive + * via better-auth, not a direct dep of apps/server. Inlining a 3-line + * helper avoids polluting package.json with what is, semantically, an + * internal of better-auth's bearer flow. + */ + function signCookieValue(value: string, secret: string): string { + const signature = createHmac('sha256', secret).update(value).digest('base64') + return encodeURIComponent(`${value}.${signature}`) + } + + return { + id: 'oidc-jwt-bearer', + hooks: { + before: [ + { + // Same matcher shape as bearer(). Run only when an Authorization + // header is present so we don't pay the cost on cookie-only flows. + matcher(context) { + return Boolean( + context.request?.headers.get('authorization') + ?? context.headers?.get('authorization'), + ) + }, + handler: createAuthMiddleware(async (c) => { + const incomingHeaders = c.request?.headers ?? c.headers + if (!incomingHeaders) + return + + const authHeader = incomingHeaders.get('authorization') + if (!authHeader) + return + + const lower = authHeader.slice(0, 7).toLowerCase() + if (lower !== 'bearer ') + return + + const token = authHeader.slice(7).trim() + if (!token || !JWT_SHAPE_RE.test(token)) + return + + // Verify against our own JWKS, read directly from DB (no + // self-fetch). If it isn't ours (signature mismatch, wrong + // issuer, expired) we silently skip and let bearer() try — + // that path is the only one that knows how to accept + // HMAC-signed better-auth session tokens. + const adapter = c.context.adapter as { + findMany: (args: { model: string }) => Promise + } + const keySet = await getOrLoadJWKS(adapter) + if (!keySet) + return + + let userId: string + try { + const { payload } = await jwtVerify(token, keySet, { + issuer: `${env.API_SERVER_URL}/api/auth`, + audience: env.API_SERVER_URL, + }) + if (typeof payload.sub !== 'string') + return + userId = payload.sub + } + catch { + return + } + + // Mint a bridge session bound to this user. The override sets + // a short TTL so abandoned bridge rows self-prune; the second + // arg `undefined` keeps `dontRememberMe` at its default. + const expiresAt = new Date(Date.now() + BRIDGE_SESSION_TTL_MS) + const bridgeSession = await c.context.internalAdapter.createSession( + userId, + undefined, + { expiresAt }, + ) + if (!bridgeSession?.token) + return + + // Format the session token exactly like bearer() expects it + // when the cookie comes back in (see plugin source above). + const signedValue = signCookieValue(bridgeSession.token, c.context.secret) + + const cookieName = c.context.authCookies.sessionToken.name + const newCookieEntry = `${cookieName}=${signedValue}` + + // Clone headers so we don't mutate the caller's. Append our + // cookie to whatever was already there (mostly nothing for + // Bearer-only stage-web; possibly other cookies in mixed flows). + const newHeaders = new Headers(incomingHeaders) + const existingCookie = newHeaders.get('cookie') + newHeaders.set( + 'cookie', + existingCookie ? `${existingCookie}; ${newCookieEntry}` : newCookieEntry, + ) + + return { context: { headers: newHeaders } } + }), + }, + ], + }, + } +} diff --git a/apps/server/src/libs/auth.ts b/apps/server/src/libs/auth.ts index ed8ccda82..6478377fa 100644 --- a/apps/server/src/libs/auth.ts +++ b/apps/server/src/libs/auth.ts @@ -9,11 +9,13 @@ import { oauthProvider } from '@better-auth/oauth-provider' import { betterAuth } from 'better-auth' import { drizzleAdapter } from 'better-auth/adapters/drizzle' import { createAuthMiddleware } from 'better-auth/api' +import { deleteSessionCookie } from 'better-auth/cookies' import { bearer, jwt, magicLink } from 'better-auth/plugins' import { eq } from 'drizzle-orm' import { ApiError } from '../utils/error' import { getAuthTrustedOrigins, getTrustedOrigin } from '../utils/origin' +import { oidcJwtBearer } from './auth-plugins/oidc-jwt-bearer' import * as authSchema from '../schemas/accounts' @@ -349,6 +351,14 @@ export function createAuth(db: Database, env: Env, email?: EmailService, metrics plugins: [ bearer(), jwt(), + // NOTICE: + // Bridges OIDC JWT access tokens (RS256, signed by our oauthProvider) + // into a real better-auth session so `sessionMiddleware` and every + // downstream `/api/auth/*` endpoint accept them. Must run after + // bearer() so we don't intercept HMAC session tokens that bearer() + // already handles. See libs/auth-plugins/oidc-jwt-bearer.ts for the + // architectural mismatch this paves over. + oidcJwtBearer(env), magicLink({ // NOTICE: better-auth's magic-link callback receives a server-side // verification URL ({baseURL}/magic-link/verify?token=...&callbackURL=...). @@ -388,6 +398,23 @@ export function createAuth(db: Database, env: Env, email?: EmailService, metrics async sendResetPassword({ user, url }) { await requireEmailService(email).sendPasswordReset({ to: user.email, url }) }, + // NOTICE: + // Why: clicking the password-reset link is itself proof that the user + // controls the address, so emailVerified must be true after a successful + // reset. Without this, social-only users who later set a password via + // "forgot password" stay stuck with emailVerified=false (better-auth's + // /reset-password handler only writes the password, see + // node_modules/better-auth/dist/api/routes/password.mjs L120-166) and + // get rejected on the next email/password sign-in by + // `requireEmailVerification: true`. + // Removal condition: better-auth flips emailVerified itself on reset. + async onPasswordReset({ user }) { + if (user.emailVerified) + return + await db.update(authSchema.user) + .set({ emailVerified: true, updatedAt: new Date() }) + .where(eq(authSchema.user.id, user.id)) + }, }, emailVerification: { @@ -432,12 +459,29 @@ export function createAuth(db: Database, env: Env, email?: EmailService, metrics // table. Without DB-backed sessions the FK INSERT fails when issuing tokens. storeSessionInDatabase: true, - // NOTICE: keep a short-lived signed session cache cookie so follow-up - // session reads avoid hitting the database on every request. - cookieCache: { - enabled: true, - maxAge: 60 * 5, - }, + // NOTICE: + // cookieCache is intentionally OFF. + // + // Why: with cookieCache enabled, the signed sessionData cookie keeps a + // "valid session" view for up to maxAge seconds even after the DB row is + // gone. /oauth2/end-session deletes the DB session row but does not + // expire that cookie (oauth-provider/dist/index.mjs L1069-1090 only calls + // internalAdapter.deleteSession). The next /oauth2/authorize then reads + // the cached session via getSessionFromCtx (better-auth/dist/api/routes/session.mjs L93+), + // binds an authorization code to the deleted session.id, and /oauth2/token + // fails with `invalid_request: session no longer exists` + // (oauth-provider/dist/index.mjs L557-567) — locking users out for the + // entire cookieCache TTL window after each logout. + // + // Trade-off: every getSession / authorize now hits the DB once. With the + // current AIRI flow the cost is negligible: cookie-based /get-session is + // only used by ui-server-auth pages, and /oauth2/authorize is rare. + // Bearer-token sessions (the hot path for stage-web/electron/pocket) bypass + // this entirely via libs/request-auth.ts. + // + // Removal condition: oauth-provider's end-session itself clears session + // cookies upstream, OR cookieCache TTL is reduced to a window short + // enough that "session no longer exists" is not user-visible. }, baseURL: env.API_SERVER_URL, @@ -459,10 +503,36 @@ export function createAuth(db: Database, env: Env, email?: EmailService, metrics google: { clientId: env.AUTH_GOOGLE_CLIENT_ID, clientSecret: env.AUTH_GOOGLE_CLIENT_SECRET, + // NOTICE: + // Why: better-auth's google provider already maps email_verified + // through, but a stale Google profile that omits the claim falls + // through to undefined → false. Default to true defensively so the + // requireEmailVerification gate in emailAndPassword above doesn't + // reject legitimate Google-OAuth users on a follow-up password sign-in. + // Source: node_modules/@better-auth/core/dist/social-providers/google.mjs L95. + // Removal condition: never — Google emails are always verified before + // OAuth issuance, so the override is correct in all cases. + mapProfileToUser: profile => ({ + emailVerified: profile.email_verified ?? true, + }), }, github: { clientId: env.AUTH_GITHUB_CLIENT_ID, clientSecret: env.AUTH_GITHUB_CLIENT_SECRET, + // NOTICE: + // Why: better-auth derives emailVerified from the GitHub /user/emails + // response, but `emails.find(e => e.email === profile.email)?.verified` + // returns undefined when GitHub returns the noreply proxy email + // (`+@users.noreply.github.com`) which is not present in + // the /user/emails list. The result is `?? false`, leaving brand-new + // GitHub users at emailVerified=false → blocked from email/password + // sign-in after a password reset. + // Source: node_modules/@better-auth/core/dist/social-providers/github.mjs L77. + // Removal condition: GitHub OAuth itself enforces a verified email + // before authorization, so forcing true is safe and matches the + // upstream invariant. Drop only if better-auth fixes the noreply + // lookup. + mapProfileToUser: () => ({ emailVerified: true }), }, }, @@ -489,6 +559,23 @@ export function createAuth(db: Database, env: Env, email?: EmailService, metrics throw ctx.redirect(url.toString()) } } + + // NOTICE: + // OIDC RP-Initiated Logout (/oauth2/end-session) only deletes the DB session + // row via internalAdapter.deleteSession; it does NOT expire the + // sessionToken / sessionData cookies. With cookieCache.enabled=true the + // signed sessionData cookie keeps a stale "valid session" view alive + // for up to maxAge seconds. The next /oauth2/authorize then picks up + // the cached old session, binds the auth code to a now-deleted + // session.id, and /oauth2/token fails with "session no longer exists". + // We mirror /sign-out's deleteSessionCookie call here so RP-Initiated + // Logout fully invalidates client-visible session state. + // Source: node_modules/@better-auth/oauth-provider/dist/index.mjs L1069-1090 + // (deleteSession only) vs node_modules/better-auth/dist/api/routes/sign-out.mjs L19-27. + // Removal condition: oauth-provider's end-session itself starts clearing + // session cookies upstream. + if (ctx.path === '/oauth2/end-session') + deleteSessionCookie(ctx) }), }, diff --git a/apps/server/src/libs/gravatar.ts b/apps/server/src/libs/gravatar.ts new file mode 100644 index 000000000..69985e902 --- /dev/null +++ b/apps/server/src/libs/gravatar.ts @@ -0,0 +1,82 @@ +/** + * Gravatar fallback URL builder for the server side. + * + * Use when: + * - Decorating session/profile responses so every client (web, Electron, + * mobile) receives a usable avatar URL even if no provider supplied an + * `image` and the user never uploaded one. Computing on the server keeps + * the hashing implementation in one place and lets future swaps (e.g. + * hosting our own avatars or proxying through a CDN) happen without + * touching every client. + * + * Background: + * - Gravatar's modern API hashes the trimmed/lowercased email with SHA-256. + * Docs: https://docs.gravatar.com/api/avatars/hash/. + */ + +import { createHash } from 'node:crypto' + +const GRAVATAR_BASE_URL = 'https://www.gravatar.com/avatar/' + +/** + * Default-image keyword for Gravatar's `d` query parameter. + * + * `identicon` deterministically generates a geometric pattern from the email + * hash so users with no Gravatar still get a unique-looking avatar. Switch + * to `mp` (mystery person) when a neutral silhouette is preferred. + */ +const DEFAULT_FALLBACK = 'identicon' + +/** + * Default rendered size in pixels. Profile avatar slot is rendered at + * 96px logical, so 200px gives sharp output on retina displays. + */ +const DEFAULT_SIZE = 200 + +interface GravatarOptions { + /** + * Default image keyword to serve when the email has no Gravatar profile. + * + * @default 'identicon' + */ + fallback?: 'identicon' | 'monsterid' | 'wavatar' | 'retro' | 'robohash' | 'mp' | '404' + /** + * Output square size in pixels. + * + * @default 200 + */ + size?: number +} + +/** + * Build a Gravatar avatar URL from an email address. + * + * Use when: + * - Decorating an API response that exposes a user; falls back to a + * personalised placeholder when no real avatar is on file. + * + * Expects: + * - `email` is non-empty. Empty/whitespace emails return `null` so the + * caller can decide whether to omit the field entirely. + * + * Returns: + * - Full HTTPS Gravatar URL or `null` when input is unusable. + * + * Before: + * - "Hello@Example.COM " + * + * After: + * - "https://www.gravatar.com/avatar/973dfe463ec85785f5f95af5ba3906eedb2d931c24e69824a89ea65dba4e813b?d=identicon&s=200" + */ +export function buildGravatarUrl(email: string, options: GravatarOptions = {}): string | null { + const trimmed = email.trim().toLowerCase() + if (!trimmed) + return null + + const hash = createHash('sha256').update(trimmed).digest('hex') + + const url = new URL(hash, GRAVATAR_BASE_URL) + url.searchParams.set('d', options.fallback ?? DEFAULT_FALLBACK) + url.searchParams.set('s', String(options.size ?? DEFAULT_SIZE)) + return url.toString() +} diff --git a/apps/server/src/middlewares/auth.ts b/apps/server/src/middlewares/auth.ts index 83cc818af..210a704cb 100644 --- a/apps/server/src/middlewares/auth.ts +++ b/apps/server/src/middlewares/auth.ts @@ -4,13 +4,9 @@ import type { createAuth } from '../libs/auth' import type { Env } from '../libs/env' import type { HonoEnv } from '../types/hono' -import { useLogger } from '@guiiai/logg' - import { resolveRequestAuth } from '../libs/request-auth' import { createUnauthorizedError } from '../utils/error' -const logger = useLogger('auth') - type AuthInstance = ReturnType /** @@ -60,7 +56,6 @@ export function sessionMiddleware(auth: AuthInstance, env: Env): MiddlewareHandl export const authGuard: MiddlewareHandler = async (c, next) => { const user = c.get('user') if (!user) { - logger.withFields({ path: c.req.path, method: c.req.method }).debug('Unauthorized request blocked') throw createUnauthorizedError() } await next() diff --git a/apps/server/src/routes/oidc/token-auth.ts b/apps/server/src/routes/oidc/token-auth.ts index 419ce991b..228b0423c 100644 --- a/apps/server/src/routes/oidc/token-auth.ts +++ b/apps/server/src/routes/oidc/token-auth.ts @@ -4,6 +4,7 @@ import type { HonoEnv } from '../../types/hono' import { Hono } from 'hono' +import { buildGravatarUrl } from '../../libs/gravatar' import { resolveRequestAuth } from '../../libs/request-auth' export interface OIDCTokenAuthRouteDeps { @@ -15,7 +16,29 @@ export function createOIDCTokenAuthRoute(deps: OIDCTokenAuthRouteDeps) { return new Hono() .on(['GET', 'POST'], '/get-session', async (c) => { const session = await resolveRequestAuth(deps.auth, deps.env, c.req.raw.headers) - return c.json(session) + if (!session) + return c.json(null) + + // NOTICE: + // Avatar fallback to Gravatar happens here so every client (web, + // Electron, mobile, future SSR) renders the same picture without + // re-implementing SHA-256 hashing or Gravatar URL conventions. The + // DB only stores user-set / provider-set images; the fallback is + // computed at response time so a future swap (DiceBear, self-hosted + // proxy) is a one-line change here. + // + // We intentionally do NOT carry an `imageSource` flag — the URL + // itself is the signal: anything starting with + // `https://www.gravatar.com/avatar/` is the fallback, anything else + // is manual / provider-set. Skipping the flag keeps the API surface + // small and the server free of redundant state. If we ever change + // the fallback provider, both this file and the client-side prefix + // check must be updated together. + // Removal condition: avatar storage moves off-band (e.g. CDN) and + // `user.image` becomes the canonical URL for every user. + const image = session.user.image || buildGravatarUrl(session.user.email) + + return c.json({ ...session, user: { ...session.user, image } }) }) .post('/sign-out', async (c) => { // NOTICE: JWT access tokens are self-contained and expire naturally. diff --git a/apps/ui-server-auth/package.json b/apps/ui-server-auth/package.json index aa5b701bc..89720b5d2 100644 --- a/apps/ui-server-auth/package.json +++ b/apps/ui-server-auth/package.json @@ -34,6 +34,7 @@ "@vueuse/core": "^14.2.1", "@vueuse/shared": "^14.2.1", "animejs": "^4.3.6", + "better-auth": "catalog:", "colorjs.io": "^0.6.1", "culori": "^4.0.2", "date-fns": "^4.1.0", diff --git a/apps/ui-server-auth/src/modules/auth-client.ts b/apps/ui-server-auth/src/modules/auth-client.ts new file mode 100644 index 000000000..68e3b4c17 --- /dev/null +++ b/apps/ui-server-auth/src/modules/auth-client.ts @@ -0,0 +1,78 @@ +/** + * Better-auth client factory for the auth-only SPA (`apps/ui-server-auth`). + * + * Use when: + * - Calling any `/api/auth/*` endpoint from the auth UI (profile read/write, + * sign-in / sign-up, password reset, linked accounts management). Lets us + * reuse better-auth's typed client surface instead of re-deriving response + * shapes from `unknown` JSON in N hand-written wrappers. + * + * Why a separate factory (vs. importing the singleton in + * `packages/stage-ui/src/libs/auth.ts`): + * - Stage-UI's client is configured for **Bearer-only** access (`credentials: + * 'omit'` so cookies don't tag along with OIDC JWTs). It also injects a + * Bearer token from the auth store on every request — nonsense in this + * app, since the auth UI is the page the cookie was *just* set on. + * - This client uses the better-auth defaults (cookies via + * `credentials: 'include'`) and skips the Bearer header. That matches + * what the auth UI actually has at hand. + * + * Test seam: + * - Pass `fetchImpl` to substitute `globalThis.fetch`. Better-auth wires it + * as `customFetchImpl` (see node_modules/better-auth/dist/client/config.mjs + * L+: the spread of `restOfFetchOptions` happens after the default, so a + * user-supplied value wins). Production callers omit `fetchImpl` and we + * memoise per `apiServerUrl` so we don't rebuild on every render. + * + * Removal condition: better-auth ships a hosted typed client for OIDC IdP + * setups where one process is both IdP and resource server. Until then, + * one factory per credential mode is the cleanest contract. + */ + +import { createAuthClient } from 'better-auth/vue' + +export interface AuthClientArgs { + apiServerUrl: string + /** + * Optional fetch override for tests. When provided we *do not* memoise so + * every test case can install its own mock without bleed-through. + */ + fetchImpl?: typeof fetch +} + +const cache = new Map>() + +/** + * Build (or reuse) a better-auth client pointed at the given server. + * + * Use when: + * - Any module needs to call `/api/auth/*` from the auth UI. + * + * Expects: + * - `apiServerUrl` is a fully-qualified origin (e.g. `https://api.airi.test` + * or `http://localhost:3000`). Trailing slash optional; better-auth + * normalises. + * + * Returns: + * - A typed client whose methods (`getSession`, `updateUser`, `listAccounts`, + * etc.) match the better-auth endpoint surface. Tokens / cookies handled + * via `credentials: 'include'` defaults. + */ +export function getAuthClient(args: AuthClientArgs): ReturnType { + if (args.fetchImpl) { + // Tests: never cache, never share. The injected fetchImpl is the whole + // point of the call. + return createAuthClient({ + baseURL: args.apiServerUrl, + fetchOptions: { customFetchImpl: args.fetchImpl }, + }) + } + + const cached = cache.get(args.apiServerUrl) + if (cached) + return cached + + const client = createAuthClient({ baseURL: args.apiServerUrl }) + cache.set(args.apiServerUrl, client) + return client +} diff --git a/apps/ui-server-auth/src/modules/profile.test.ts b/apps/ui-server-auth/src/modules/profile.test.ts index c4c72265b..9c082a85b 100644 --- a/apps/ui-server-auth/src/modules/profile.test.ts +++ b/apps/ui-server-auth/src/modules/profile.test.ts @@ -39,11 +39,17 @@ describe('ui-server-auth profile flow helpers', () => { }, }) + // NOTICE: + // We assert on the URL only, not on the fetch options shape. better-auth + // client builds the request via better-fetch, which decorates the + // options with framework metadata (plugins, jsonParser, signal, etc.). + // Pinning option fields ties the test to better-auth internals; the URL + // and the parsed result are the behavioural contract we actually care + // about. + // better-fetch passes a URL object, not a string, so coerce before + // matching. URL.toString() yields the canonical href. expect(fetchImpl).toHaveBeenCalledTimes(1) - expect(fetchImpl).toHaveBeenCalledWith( - 'https://api.airi.test/api/auth/get-session', - expect.objectContaining({ method: 'GET', credentials: 'include' }), - ) + expect(String(fetchImpl.mock.calls[0]?.[0])).toBe('https://api.airi.test/api/auth/get-session') }) it('returns user=null when better-auth reports no session', async () => { @@ -112,14 +118,13 @@ describe('ui-server-auth profile flow helpers', () => { })).rejects.toThrow('Invalid current password') }) - it('posts to /sign-out with credentials included', async () => { + it('hits /sign-out via the auth client', async () => { const fetchImpl = vi.fn(async () => jsonResponse({ success: true })) await signOut({ apiServerUrl: 'https://api.airi.test', fetchImpl }) - expect(fetchImpl).toHaveBeenCalledWith( - 'https://api.airi.test/api/auth/sign-out', - expect.objectContaining({ method: 'POST', credentials: 'include' }), - ) + // See URL-coercion + URL-only rationale above the get-session assertion. + expect(fetchImpl).toHaveBeenCalledTimes(1) + expect(String(fetchImpl.mock.calls[0]?.[0])).toBe('https://api.airi.test/api/auth/sign-out') }) }) diff --git a/apps/ui-server-auth/src/modules/profile.ts b/apps/ui-server-auth/src/modules/profile.ts index c32f0dfc4..652f3f72e 100644 --- a/apps/ui-server-auth/src/modules/profile.ts +++ b/apps/ui-server-auth/src/modules/profile.ts @@ -1,26 +1,32 @@ /** - * Account profile flows backed by better-auth's built-in user routes. + * Account profile flows backed by better-auth's typed Vue client. * * Use when: * - Driving the profile page in `apps/ui-server-auth` (load current user, - * update display name, change password, sign out). + * update display name / avatar, change password, sign out). * - * Each function shares the {@link AuthFetchBase} contract via auth-fetch.ts; - * see that module for HTTP-level expectations (credentials, error parsing). + * Why this delegates to {@link getAuthClient} instead of hand-rolling + * `fetch` wrappers: better-auth already returns typed payloads. Mapping + * `unknown` JSON to a hand-written interface duplicated work the upstream + * client already does and produced a layer of defensive `typeof x === + * 'string' ? x : ''` casts that were brittle and noisy. See + * `auth-client.ts` for the rationale on the cookie-based credentials mode. */ import type { AuthFetchBase } from './auth-fetch' import { errorMessageFrom } from '@moeru/std' -import { getAuthJSON, postAuthJSON } from './auth-fetch' +import { getAuthClient } from './auth-client' /** - * Subset of the better-auth `user` row needed to render the profile page. + * Trimmed view of the better-auth `user` row exposed via `/get-session`. * - * Mirrors the shape returned by `/api/auth/get-session`; extra fields are - * ignored intentionally so this module doesn't drift if better-auth adds - * unrelated columns. + * Mirrors the better-auth `User` shape but flattens `createdAt` to a + * string (or null) for ergonomic rendering — better-auth's client returns + * `Date`, but the profile page formats it via `Intl.DateTimeFormat` which + * accepts both. We keep the string projection so consumers don't have to + * worry about Date-vs-string drift across the ui-server-auth boundary. */ export interface ProfileUser { id: string @@ -29,7 +35,13 @@ export interface ProfileUser { email: string /** True once the user clicked the verification link sent on sign-up. */ emailVerified: boolean - /** Avatar URL — usually populated by social providers; may be empty. */ + /** + * Avatar URL. Server decorates this so it's always populated for + * signed-in users: provider-set / user-uploaded URL when present, or a + * Gravatar fallback derived server-side from the email hash. The UI + * detects the fallback by URL prefix + * (`https://www.gravatar.com/avatar/`). + */ image: string | null /** ISO timestamp from `created_at`. */ createdAt: string | null @@ -64,42 +76,35 @@ interface ChangePasswordArgs extends AuthFetchBase { } /** - * Read the current session from `/api/auth/get-session`. + * Read the current session via the typed better-auth client. * * Use when: * - Bootstrapping the profile page; decides whether to render the form or * bounce the user to the sign-in page. * - * Expects: - * - Browser sends the better-auth session cookie (`credentials: include`). - * * Returns: - * - `user: null` when there's no active session (better-auth returns an empty - * body for unauthenticated GETs). + * - `user: null` for unauthenticated requests (better-auth client returns + * `null` data, not an error, in that case). * - {@link CurrentSessionResult} with the trimmed user fields otherwise. */ export async function getCurrentSession(args: AuthFetchBase): Promise { - return getAuthJSON(args, '/get-session', (data) => { - // NOTICE: - // better-auth returns either `null` or an empty object for an - // unauthenticated GET to `/get-session`, not a 401. Treat both as - // "no session" so the caller can branch on user === null without a - // separate try/catch. - // Source: node_modules/better-auth/dist/api/routes/session.mjs (`getSession`) - if (!data || typeof data !== 'object' || !('user' in data) || !data.user) - return { user: null } + const client = getAuthClient(args) + const { data, error } = await client.getSession() + if (error) + throw new Error(error.message ?? `Auth request failed (${error.status ?? 'unknown'})`) + if (!data?.user) + return { user: null } - const raw = (data as { user: unknown }).user as Record - const user: ProfileUser = { - id: typeof raw.id === 'string' ? raw.id : '', - name: typeof raw.name === 'string' ? raw.name : '', - email: typeof raw.email === 'string' ? raw.email : '', - emailVerified: Boolean(raw.emailVerified), - image: typeof raw.image === 'string' ? raw.image : null, - createdAt: typeof raw.createdAt === 'string' ? raw.createdAt : null, - } - return { user } - }) + return { + user: { + id: data.user.id, + name: data.user.name, + email: data.user.email, + emailVerified: data.user.emailVerified, + image: data.user.image ?? null, + createdAt: toIsoString(data.user.createdAt), + }, + } } /** @@ -111,18 +116,17 @@ export async function getCurrentSession(args: AuthFetchBase): Promise { - const body: Record = {} + const client = getAuthClient(args) + const body: { name?: string, image?: string | null } = {} if (args.name !== undefined) body.name = args.name if (args.image !== undefined) body.image = args.image - - await postAuthJSON(args, '/update-user', body, () => undefined) + const { error } = await client.updateUser(body) + if (error) + throw new Error(error.message ?? 'updateUser failed') } /** @@ -135,27 +139,20 @@ export async function updateUserProfile(args: UpdateUserProfileArgs): Promise { - await postAuthJSON( - args, - '/change-password', - { - currentPassword: args.currentPassword, - newPassword: args.newPassword, - revokeOtherSessions: args.revokeOtherSessions ?? true, - }, - () => undefined, - ) + const client = getAuthClient(args) + const { error } = await client.changePassword({ + currentPassword: args.currentPassword, + newPassword: args.newPassword, + revokeOtherSessions: args.revokeOtherSessions ?? true, + }) + if (error) + throw new Error(error.message ?? 'changePassword failed') } /** - * Sign the current user out via `/api/auth/sign-out`. + * Sign the current user out via better-auth's `/sign-out` endpoint. * * Use when: * - User clicks "Sign out" on the profile page. @@ -166,9 +163,30 @@ export async function changePassword(args: ChangePasswordArgs): Promise { * page after this resolves. */ export async function signOut(args: AuthFetchBase): Promise { - await postAuthJSON(args, '/sign-out', {}, () => undefined) + const client = getAuthClient(args) + const { error } = await client.signOut() + if (error) + throw new Error(error.message ?? 'signOut failed') } export function describeProfileError(error: unknown): string { return errorMessageFrom(error) ?? 'Unexpected error' } + +/** + * Normalise better-auth's `Date | string | null | undefined` createdAt into + * the ISO string the rest of the UI expects. + * + * Before: + * - `new Date('2025-04-01T00:00:00.000Z')` / `'2025-04-01T00:00:00.000Z'` / `null` + * + * After: + * - `'2025-04-01T00:00:00.000Z'` / `'2025-04-01T00:00:00.000Z'` / `null` + */ +function toIsoString(value: unknown): string | null { + if (value instanceof Date) + return value.toISOString() + if (typeof value === 'string') + return value + return null +} diff --git a/apps/ui-server-auth/src/modules/sign-in.test.ts b/apps/ui-server-auth/src/modules/sign-in.test.ts index d6c745d95..346d36227 100644 --- a/apps/ui-server-auth/src/modules/sign-in.test.ts +++ b/apps/ui-server-auth/src/modules/sign-in.test.ts @@ -23,6 +23,41 @@ describe('ui-server-auth sign-in flow helpers', () => { }) }) + // ROOT CAUSE: + // + // Production hit `/api/auth/oauth2/authorize?token=4HEMlnagmOESfes99kW5nNmZ` + // and got a VALIDATION_ERROR for missing client_id / response_type. + // + // The 24-char `token=...` is the format better-auth's password-reset + // callback appends to redirectTo (password.mjs L65 generateId(24), L118 + // redirectCallback adds `?token=`). When such a token-only URL + // landed on /auth/sign-in (e.g. via a stale or misconfigured reset email), + // the previous filter only stripped `provider`/`prompt` and treated any + // remaining query as an OIDC handoff — synthesizing + // /api/auth/oauth2/authorize?token=... and trapping the user on a 422. + // + // Fix: require both `client_id` and `response_type` before treating the + // query as an OIDC continuation; otherwise fall back to '/'. + it('ignores stray non-OIDC query params (Issue: production reset-password token leaking into authorize URL)', () => { + expect(createServerSignInContext( + 'https://auth.airi.test/sign-in?token=4HEMlnagmOESfes99kW5nNmZ', + 'https://api.airi.test', + )).toEqual({ + callbackURL: '/', + requestedProvider: null, + }) + }) + + it('still falls back when the OIDC handoff is partial (client_id without response_type)', () => { + expect(createServerSignInContext( + 'https://auth.airi.test/sign-in?client_id=airi-stage-web&scope=openid', + 'https://api.airi.test', + )).toEqual({ + callbackURL: '/', + requestedProvider: null, + }) + }) + it('posts the selected provider and callback URL to the social sign-in endpoint', async () => { const fetchImpl = vi.fn(async () => { return new Response(JSON.stringify({ url: 'https://accounts.example.test/oauth/google' }), { diff --git a/apps/ui-server-auth/src/modules/sign-in.ts b/apps/ui-server-auth/src/modules/sign-in.ts index 0bb9730de..083c22a3b 100644 --- a/apps/ui-server-auth/src/modules/sign-in.ts +++ b/apps/ui-server-auth/src/modules/sign-in.ts @@ -22,7 +22,19 @@ export function createServerSignInContext(currentUrl: string, apiServerUrl: stri oidcParams.delete('provider') oidcParams.delete('prompt') - if (!oidcParams.size) { + // NOTICE: + // Only synthesize an OIDC authorize callback when the page query genuinely + // looks like an OIDC handoff. Without this guard, a stray `?token=...` — + // e.g. the 24-char password-reset token better-auth appends when it + // redirects through redirectTo (better-auth/dist/api/routes/password.mjs L65, L118) + // back into /auth/sign-in — would synthesize + // `/api/auth/oauth2/authorize?token=...` as the callback. The OIDC zod + // schema then rejects it for missing client_id / response_type + // (oauth-provider/dist/index.mjs L2808-2826) and the user sees a + // VALIDATION_ERROR instead of the sign-in form. + // Removal condition: redirectTo origins are exhaustively scoped so reset / + // verification redirects can never land on /auth/sign-in carrying a `token`. + if (!oidcParams.has('client_id') || !oidcParams.has('response_type')) { return { callbackURL: '/', requestedProvider, diff --git a/apps/ui-server-auth/src/pages/profile.vue b/apps/ui-server-auth/src/pages/profile.vue index 6ec4919f0..ea8723e1d 100644 --- a/apps/ui-server-auth/src/pages/profile.vue +++ b/apps/ui-server-auth/src/pages/profile.vue @@ -1,12 +1,16 @@