oauth improvements

src/lib/api/client.ts

@@ -48,8 +48,8 @@ export interface RequestOptions {
   method?: 'GET' | 'POST' | 'PUT' | 'PATCH' | 'DELETE'
   query?: Query
   body?: unknown
-  /** Send as multipart instead of JSON (media uploads). */
+  /** Send as browser-encoded form data instead of JSON. */
-  form?: FormData
+  form?: FormData | URLSearchParams
   signal?: AbortSignal
   /** Override the instance token for this call. */
   token?: string | null
@@ -216,6 +216,25 @@ export class ApiClient {
     return data
   }
 
+  /**
+   * Post an `application/x-www-form-urlencoded` body.
+   *
+   * OAuth endpoints are specified as form endpoints. Letting fetch set the
+   * header for URLSearchParams also keeps this a CORS-safelisted request.
+   */
+  async postForm<T>(
+    path: string,
+    fields: Record<string, string | undefined>,
+    options: RequestOptions = {},
+  ): Promise<T> {
+    const form = new URLSearchParams()
+    for (const [key, value] of Object.entries(fields)) {
+      if (value !== undefined) form.set(key, value)
+    }
+    const { data } = await this.raw<T>(path, { ...options, method: 'POST', form })
+    return data
+  }
+
   async patch<T>(path: string, body?: unknown, options: RequestOptions = {}): Promise<T> {
     const { data } = await this.raw<T>(path, { ...options, method: 'PATCH', body })
     return data

src/lib/api/oauth.test.ts

@@ -0,0 +1,206 @@
+import { afterEach, describe, expect, it, vi } from 'vitest'
+import {
+  authorizationUrl,
+  beginLogin,
+  completeLogin,
+  ensureApp,
+  pkceChallenge,
+  redirectUri,
+  revoke,
+  SCOPES,
+} from './oauth'
+import type { OAuthApp } from './types'
+
+const APP_KEY = 'plspace:oauth:apps'
+const PENDING_KEY = 'plspace:oauth:pending'
+
+function response(body: unknown, status = 200): Response {
+  return new Response(body === null ? null : JSON.stringify(body), {
+    status,
+    headers: { 'Content-Type': 'application/json' },
+  })
+}
+
+function app(overrides: Partial<OAuthApp> = {}): OAuthApp {
+  return {
+    id: 'app-1',
+    name: 'plspace',
+    client_id: 'client-id',
+    client_secret: 'client-secret',
+    redirect_uris: [redirectUri()],
+    ...overrides,
+  }
+}
+
+function saveApp(value = app()): void {
+  localStorage.setItem(
+    APP_KEY,
+    JSON.stringify({
+      'social.example': { ...value, plspace_redirect_uri: redirectUri() },
+    }),
+  )
+}
+
+function savePending(overrides: Record<string, unknown> = {}): void {
+  sessionStorage.setItem(
+    PENDING_KEY,
+    JSON.stringify({
+      host: 'social.example',
+      state: 'expected-state',
+      verifier: 'pkce-verifier',
+      createdAt: Date.now(),
+      returnTo: '#/timeline/home',
+      ...overrides,
+    }),
+  )
+}
+
+afterEach(() => {
+  vi.unstubAllGlobals()
+})
+
+describe('OAuth request compatibility', () => {
+  it('produces the RFC 7636 S256 challenge', async () => {
+    await expect(
+      pkceChallenge('dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk'),
+    ).resolves.toBe('E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM')
+  })
+
+  it('builds a Mastodon-compatible authorization URL with state and PKCE', () => {
+    const url = new URL(
+      authorizationUrl('social.example', app(), 'oauth-state', 'pkce-challenge'),
+    )
+
+    expect(url.origin).toBe('https://social.example')
+    expect(url.pathname).toBe('/oauth/authorize')
+    expect(url.searchParams.get('client_id')).toBe('client-id')
+    expect(url.searchParams.get('redirect_uri')).toBe(redirectUri())
+    expect(url.searchParams.get('response_type')).toBe('code')
+    expect(url.searchParams.get('scope')).toBe(SCOPES)
+    expect(url.searchParams.get('state')).toBe('oauth-state')
+    expect(url.searchParams.get('code_challenge')).toBe('pkce-challenge')
+    expect(url.searchParams.get('code_challenge_method')).toBe('S256')
+  })
+
+  it('registers once and understands the Mastodon 4.3 redirect_uris response', async () => {
+    const fetchMock = vi.fn().mockResolvedValue(response(app()))
+    vi.stubGlobal('fetch', fetchMock)
+
+    await ensureApp('social.example')
+    await ensureApp('social.example')
+
+    expect(fetchMock).toHaveBeenCalledOnce()
+    const [url, options] = fetchMock.mock.calls[0] as [string, RequestInit]
+    expect(url).toBe('https://social.example/api/v1/apps')
+    expect(JSON.parse(String(options.body))).toMatchObject({
+      client_name: 'plspace',
+      redirect_uris: redirectUri(),
+      scopes: SCOPES,
+    })
+  })
+
+  it('persists one state-bound PKCE attempt before navigating away', async () => {
+    vi.stubGlobal('fetch', vi.fn().mockResolvedValue(response(app())))
+    const navigate = vi.fn()
+
+    await beginLogin('social.example', '#/mail', navigate)
+
+    expect(navigate).toHaveBeenCalledOnce()
+    const url = new URL(navigate.mock.calls[0][0])
+    const pending = JSON.parse(String(sessionStorage.getItem(PENDING_KEY))) as Record<
+      string,
+      unknown
+    >
+    expect(pending).toMatchObject({
+      host: 'social.example',
+      returnTo: '#/mail',
+      state: url.searchParams.get('state'),
+    })
+    expect(typeof pending.verifier).toBe('string')
+    expect(typeof pending.createdAt).toBe('number')
+    expect(url.searchParams.get('code_challenge')).toBeTruthy()
+    expect(url.searchParams.get('code_challenge_method')).toBe('S256')
+  })
+
+  it('exchanges a code as URL-encoded form data and accepts Pleroma extras', async () => {
+    saveApp()
+    savePending()
+    window.history.replaceState(
+      {},
+      '',
+      '/?code=authorization-code&state=expected-state&iss=https%3A%2F%2Fsocial.example',
+    )
+    const fetchMock = vi.fn().mockResolvedValue(
+      response({
+        access_token: 'access-token',
+        token_type: 'Bearer',
+        scope: SCOPES,
+        created_at: 123,
+        id: 42,
+        me: 'https://social.example/users/alice',
+      }),
+    )
+    vi.stubGlobal('fetch', fetchMock)
+
+    await expect(completeLogin()).resolves.toEqual({
+      host: 'social.example',
+      token: 'access-token',
+      returnTo: '#/timeline/home',
+    })
+
+    expect(fetchMock).toHaveBeenCalledOnce()
+    const [url, options] = fetchMock.mock.calls[0] as [string, RequestInit]
+    expect(url).toBe('https://social.example/oauth/token')
+    expect(options.body).toBeInstanceOf(URLSearchParams)
+    expect(Object.fromEntries(new URLSearchParams(String(options.body)))).toEqual({
+      grant_type: 'authorization_code',
+      client_id: 'client-id',
+      client_secret: 'client-secret',
+      redirect_uri: redirectUri(),
+      code: 'authorization-code',
+      code_verifier: 'pkce-verifier',
+    })
+    expect(window.location.search).toBe('')
+  })
+
+  it('does not retry a PKCE-bound code without its verifier', async () => {
+    saveApp()
+    savePending()
+    window.history.replaceState({}, '', '/?code=bad-code&state=expected-state')
+    const fetchMock = vi
+      .fn()
+      .mockResolvedValue(response({ error: 'invalid_grant' }, 400))
+    vi.stubGlobal('fetch', fetchMock)
+
+    await expect(completeLogin()).rejects.toThrow('invalid_grant')
+    expect(fetchMock).toHaveBeenCalledOnce()
+  })
+
+  it('rejects expired attempts before exchanging the code', async () => {
+    saveApp()
+    savePending({ createdAt: Date.now() - 11 * 60 * 1000 })
+    window.history.replaceState({}, '', '/?code=old-code&state=expected-state')
+    const fetchMock = vi.fn()
+    vi.stubGlobal('fetch', fetchMock)
+
+    await expect(completeLogin()).rejects.toThrow('expired')
+    expect(fetchMock).not.toHaveBeenCalled()
+  })
+
+  it('revokes with URL-encoded form data', async () => {
+    saveApp()
+    const fetchMock = vi.fn().mockResolvedValue(response({}))
+    vi.stubGlobal('fetch', fetchMock)
+
+    await revoke('social.example', 'access-token')
+
+    const [url, options] = fetchMock.mock.calls[0] as [string, RequestInit]
+    expect(url).toBe('https://social.example/oauth/revoke')
+    expect(options.body).toBeInstanceOf(URLSearchParams)
+    expect(Object.fromEntries(new URLSearchParams(String(options.body)))).toEqual({
+      client_id: 'client-id',
+      client_secret: 'client-secret',
+      token: 'access-token',
+    })
+  })
+})

src/lib/api/oauth.ts

@@ -3,16 +3,15 @@
  *
  * A static frontend has no backend to keep a client secret, so it registers a
  * throwaway app per instance (`POST /api/v1/apps`) and uses the authorization
- * code flow, upgrading to PKCE when the server advertises support (Mastodon
+ * code flow with S256 PKCE. Current Pleroma supports these parameters, while
- * 4.3+). Pleroma and older Mastodon ignore the PKCE params, hence the retry
+ * older OAuth implementations ignore unknown authorization parameters.
- * without them rather than a hard requirement.
  *
  * The registered app credentials and the resulting token live in localStorage;
  * they are per-origin and per-instance, and are exactly as sensitive as being
  * logged in on this browser.
  */
 
-import { ApiClient, ApiError, normalizeHost } from './client'
+import { ApiClient, normalizeHost } from './client'
 import type { OAuthApp, OAuthToken } from './types'
 
 export const APP_NAME = 'plspace'
@@ -21,11 +20,18 @@ export const SCOPES = 'read write follow'
 
 const APP_KEY = 'plspace:oauth:apps'
 const PENDING_KEY = 'plspace:oauth:pending'
+const PENDING_MAX_AGE_MS = 10 * 60 * 1000
+
+interface StoredOAuthApp extends OAuthApp {
+  /** Redirect registered by this client, independent of server response shape. */
+  plspace_redirect_uri?: string
+}
 
 interface PendingAuth {
   host: string
   verifier?: string
   state: string
+  createdAt: number
   /** Route to land on after a successful exchange. */
   returnTo: string
 }
@@ -39,9 +45,10 @@ export function redirectUri(): string {
   return `${window.location.origin}${window.location.pathname}`
 }
 
-function readApps(): Record<string, OAuthApp> {
+function readApps(): Record<string, StoredOAuthApp> {
   try {
-    return JSON.parse(localStorage.getItem(APP_KEY) ?? '{}') as Record<string, OAuthApp>
+    const value = JSON.parse(localStorage.getItem(APP_KEY) ?? '{}') as unknown
+    return value && typeof value === 'object' ? (value as Record<string, StoredOAuthApp>) : {}
   } catch {
     return {}
   }
@@ -49,17 +56,23 @@ function readApps(): Record<string, OAuthApp> {
 
 function writeApp(host: string, app: OAuthApp): void {
   const apps = readApps()
-  apps[host] = app
+  apps[host] = { ...app, plspace_redirect_uri: redirectUri() }
   localStorage.setItem(APP_KEY, JSON.stringify(apps))
 }
 
+function appHasRedirect(app: StoredOAuthApp, redirect: string): boolean {
+  if (app.plspace_redirect_uri === redirect) return true
+  if (app.redirect_uris?.includes(redirect)) return true
+  return app.redirect_uri?.split(/\s+/).includes(redirect) ?? false
+}
+
 /** Register (or reuse) an OAuth app on `host`. */
 export async function ensureApp(host: string): Promise<OAuthApp> {
   const key = normalizeHost(host)
   const cached = readApps()[key]
   // Re-register if the deployment moved: a stale redirect_uri fails at /oauth/authorize
   // with an opaque error page, which is miserable to debug.
-  if (cached && cached.redirect_uri === redirectUri()) return cached
+  if (cached?.client_id && cached.client_secret && appHasRedirect(cached, redirectUri())) return cached
 
   const client = new ApiClient(key)
   const app = await client.post<OAuthApp>('/api/v1/apps', {
@@ -68,6 +81,9 @@ export async function ensureApp(host: string): Promise<OAuthApp> {
     scopes: SCOPES,
     website: APP_WEBSITE,
   })
+  if (!app?.client_id || !app.client_secret) {
+    throw new Error('The server returned an invalid OAuth app registration.')
+  }
   writeApp(key, app)
   return app
 }
@@ -85,13 +101,37 @@ function base64url(buffer: ArrayBuffer | Uint8Array): string {
   return btoa(binary).replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, '')
 }
 
-async function challengeFor(verifier: string): Promise<string> {
+export async function pkceChallenge(verifier: string): Promise<string> {
   const digest = await crypto.subtle.digest('SHA-256', new TextEncoder().encode(verifier))
   return base64url(digest)
 }
 
+export function authorizationUrl(
+  host: string,
+  app: Pick<OAuthApp, 'client_id'>,
+  state: string,
+  codeChallenge?: string,
+): string {
+  const params = new URLSearchParams({
+    client_id: app.client_id,
+    redirect_uri: redirectUri(),
+    response_type: 'code',
+    scope: SCOPES,
+    state,
+  })
+  if (codeChallenge) {
+    params.set('code_challenge', codeChallenge)
+    params.set('code_challenge_method', 'S256')
+  }
+  return `https://${normalizeHost(host)}/oauth/authorize?${params.toString()}`
+}
+
 /** Kick off the redirect to the instance's consent screen. */
-export async function beginLogin(host: string, returnTo = '#/'): Promise<void> {
+export async function beginLogin(
+  host: string,
+  returnTo = '#/',
+  navigate: (url: string) => void = (url) => window.location.assign(url),
+): Promise<void> {
   const key = normalizeHost(host)
   const app = await ensureApp(key)
 
@@ -100,23 +140,18 @@ export async function beginLogin(host: string, returnTo = '#/'): Promise<void> {
   // authorization-code flow there rather than failing to log in at all.
   const canPkce = Boolean(crypto.subtle)
   const verifier = canPkce ? randomString(48) : undefined
+  const codeChallenge = verifier ? await pkceChallenge(verifier) : undefined
 
-  const pending: PendingAuth = { host: key, verifier, state, returnTo }
+  const pending: PendingAuth = {
+    host: key,
+    verifier,
+    state,
+    createdAt: Date.now(),
+    returnTo: returnTo.startsWith('#/') ? returnTo : '#/',
+  }
   sessionStorage.setItem(PENDING_KEY, JSON.stringify(pending))
 
-  const params = new URLSearchParams({
+  navigate(authorizationUrl(key, app, state, codeChallenge))
-    client_id: app.client_id,
-    redirect_uri: redirectUri(),
-    response_type: 'code',
-    scope: SCOPES,
-    state,
-  })
-  if (verifier) {
-    params.set('code_challenge', await challengeFor(verifier))
-    params.set('code_challenge_method', 'S256')
-  }
-
-  window.location.assign(`https://${key}/oauth/authorize?${params.toString()}`)
 }
 
 export interface CompletedLogin {
@@ -141,48 +176,76 @@ export async function completeLogin(): Promise<CompletedLogin | null> {
   sessionStorage.removeItem(PENDING_KEY)
   clearOAuthParams()
 
-  if (error) {
-    throw new Error(url.searchParams.get('error_description') ?? `Authorization failed: ${error}`)
-  }
   if (!rawPending) {
     throw new Error('No sign-in was in progress in this tab. Please start again.')
   }
 
-  const pending = JSON.parse(rawPending) as PendingAuth
+  const pending = parsePending(rawPending)
   if (pending.state !== state) {
     throw new Error('Sign-in state did not match. Please start again.')
   }
+  if (Date.now() - pending.createdAt > PENDING_MAX_AGE_MS) {
+    throw new Error('This sign-in attempt expired. Please start again.')
+  }
+  if (error) {
+    throw new Error(url.searchParams.get('error_description') ?? `Authorization failed: ${error}`)
+  }
 
   const app = readApps()[pending.host]
   if (!app) throw new Error('Lost the app registration for this server. Please start again.')
 
   const client = new ApiClient(pending.host)
-  const body: Record<string, string> = {
+  const body = {
     grant_type: 'authorization_code',
     client_id: app.client_id,
     client_secret: app.client_secret,
     redirect_uri: redirectUri(),
-    scope: SCOPES,
     code: code!,
+    code_verifier: pending.verifier,
   }
-  if (pending.verifier) body.code_verifier = pending.verifier
 
-  let token: OAuthToken
+  // A code issued with a PKCE challenge is bound to its verifier. Retrying a
-  try {
+  // failed exchange without it is both invalid and liable to consume the code.
-    token = await client.post<OAuthToken>('/oauth/token', body)
+  const token = await client.postForm<OAuthToken>('/oauth/token', body)
-  } catch (cause) {
+  if (!token || typeof token.access_token !== 'string' || !token.access_token) {
-    // Servers that don't implement PKCE reject the unexpected code_verifier.
+    throw new Error('The server returned an invalid OAuth token response.')
-    if (pending.verifier && cause instanceof ApiError && cause.status === 400) {
-      delete body.code_verifier
-      token = await client.post<OAuthToken>('/oauth/token', body)
-    } else {
-      throw cause
-    }
   }
 
   return { host: pending.host, token: token.access_token, returnTo: pending.returnTo || '#/' }
 }
 
+function parsePending(raw: string): PendingAuth {
+  let value: unknown
+  try {
+    value = JSON.parse(raw)
+  } catch {
+    throw new Error('The saved sign-in attempt is invalid. Please start again.')
+  }
+  if (!value || typeof value !== 'object') {
+    throw new Error('The saved sign-in attempt is invalid. Please start again.')
+  }
+  const pending = value as Partial<PendingAuth>
+  if (
+    typeof pending.host !== 'string' ||
+    !normalizeHost(pending.host) ||
+    typeof pending.state !== 'string' ||
+    !pending.state ||
+    typeof pending.createdAt !== 'number' ||
+    !Number.isFinite(pending.createdAt) ||
+    typeof pending.returnTo !== 'string' ||
+    (pending.verifier !== undefined && typeof pending.verifier !== 'string')
+  ) {
+    throw new Error('The saved sign-in attempt is invalid. Please start again.')
+  }
+  return {
+    host: normalizeHost(pending.host),
+    state: pending.state,
+    verifier: pending.verifier,
+    createdAt: pending.createdAt,
+    returnTo: pending.returnTo.startsWith('#/') ? pending.returnTo : '#/',
+  }
+}
+
 /** Strip `?code=…&state=…` so a refresh doesn't try to redeem a spent code. */
 function clearOAuthParams(): void {
   const url = new URL(window.location.href)
@@ -197,7 +260,7 @@ export async function revoke(host: string, token: string): Promise<void> {
   const app = readApps()[normalizeHost(host)]
   if (!app) return
   try {
-    await new ApiClient(host).post('/oauth/revoke', {
+    await new ApiClient(host).postForm('/oauth/revoke', {
       client_id: app.client_id,
       client_secret: app.client_secret,
       token,

src/lib/api/types.ts

@@ -283,9 +283,12 @@ export interface OAuthApp {
   id: string
   name: string
   website?: string | null
-  redirect_uri: string
+  /** Deprecated by Mastodon 4.3, but still returned by older servers. */
+  redirect_uri?: string
+  redirect_uris?: string[]
   client_id: string
   client_secret: string
+  client_secret_expires_at?: number
   vapid_key?: string
 }
 

src/test/setup.ts

@@ -1,7 +1,9 @@
 import '@testing-library/jest-dom/vitest'
+import { beforeEach } from 'vitest'
 
+function memoryStorage(): Storage {
   const values = new Map<string, string>()
-const memoryStorage: Storage = {
+  return {
     get length() {
       return values.size
     },
@@ -15,8 +17,19 @@ const memoryStorage: Storage = {
       values.set(key, String(value))
     },
   }
+}
 
 Object.defineProperty(globalThis, 'localStorage', {
   configurable: true,
-  value: memoryStorage,
+  value: memoryStorage(),
+})
+Object.defineProperty(globalThis, 'sessionStorage', {
+  configurable: true,
+  value: memoryStorage(),
+})
+
+beforeEach(() => {
+  localStorage.clear()
+  sessionStorage.clear()
+  window.history.replaceState({}, '', '/')
 })

vite.config.ts

@@ -22,6 +22,9 @@ export default defineConfig({
   },
   test: {
     environment: 'jsdom',
+    environmentOptions: {
+      jsdom: { url: 'https://plspace.test/' },
+    },
     setupFiles: ['./src/test/setup.ts'],
   },
 })