From 861652143501fd4567cddb5ad33f727988ce306a Mon Sep 17 00:00:00 2001 From: Aidan Date: Sat, 11 Apr 2026 23:36:14 -0400 Subject: [PATCH 1/2] add WWW-Authenticate header when token is invalid --- packages/pds/src/middleware/auth.ts | 4 ++ packages/pds/test/xrpc.test.ts | 91 ++++++++++++++++++++++++++++- plans/complete/oauth-provider.md | 4 ++ 3 files changed, 98 insertions(+), 1 deletion(-) diff --git a/packages/pds/src/middleware/auth.ts b/packages/pds/src/middleware/auth.ts index e3376588..a8815177 100644 --- a/packages/pds/src/middleware/auth.ts +++ b/packages/pds/src/middleware/auth.ts @@ -42,6 +42,10 @@ export async function requireAuth( message: "Invalid OAuth access token", }, 401, + { + "WWW-Authenticate": + 'DPoP error="invalid_token", error_description="Invalid access token"', + }, ); } diff --git a/packages/pds/test/xrpc.test.ts b/packages/pds/test/xrpc.test.ts index c7b84297..9343002c 100644 --- a/packages/pds/test/xrpc.test.ts +++ b/packages/pds/test/xrpc.test.ts @@ -1,6 +1,43 @@ import { describe, it, expect } from "vitest"; import { env, worker } from "./helpers"; - +import { + SignJWT, + base64url, + calculateJwkThumbprint, + exportJWK, + generateKeyPair, +} from "jose"; + + +async function createValidDpopProof(options: { + accessToken: string; + method: string; + url: string; +}): Promise<{ dpopProof: string; dpopJkt: string }> { + const { accessToken, method, url } = options; + const { privateKey, publicKey } = await generateKeyPair("ES256"); + const publicJwk = await exportJWK(publicKey); + delete publicJwk.key_ops; + delete publicJwk.ext; + + const tokenHash = await crypto.subtle.digest( + "SHA-256", + new TextEncoder().encode(accessToken), + ); + const ath = base64url.encode(new Uint8Array(tokenHash)); + const dpopProof = await new SignJWT({ + jti: crypto.randomUUID(), + htm: method, + htu: url, + iat: Math.floor(Date.now() / 1000), + ath, + }) + .setProtectedHeader({ typ: "dpop+jwt", alg: "ES256", jwk: publicJwk }) + .sign(privateKey); + const dpopJkt = await calculateJwkThumbprint(publicJwk, "sha256"); + + return { dpopProof, dpopJkt }; +} describe("XRPC Endpoints", () => { describe("Health Check", () => { it("should return status and version", async () => { @@ -105,6 +142,58 @@ describe("XRPC Endpoints", () => { }); }); + + it("returns DPoP invalid_token challenge for expired OAuth access tokens", async () => { + const now = Date.now(); + const id = env.ACCOUNT.idFromName("account"); + const stub = env.ACCOUNT.get(id); + const accessToken = "expired-dpop-access-token"; + const requestUrl = "http://pds.test/xrpc/com.atproto.repo.createRecord"; + const { dpopProof, dpopJkt } = await createValidDpopProof({ + accessToken, + method: "POST", + url: requestUrl, + }); + + await stub.rpcSaveTokens({ + accessToken, + refreshToken: "expired-dpop-refresh-token", + clientId: "did:web:app.example", + sub: env.DID, + scope: "atproto", + dpopJkt, + issuedAt: now - 2 * 60 * 60 * 1000, + expiresAt: now - 60 * 1000, + revoked: false, + }); + + const response = await worker.fetch( + new Request(requestUrl, { + method: "POST", + headers: { + "Content-Type": "application/json", + Authorization: `DPoP ${accessToken}`, + DPoP: dpopProof, + }, + body: JSON.stringify({ + repo: env.DID, + collection: "app.bsky.feed.post", + record: { + $type: "app.bsky.feed.post", + text: "Should fail with OAuth challenge", + createdAt: new Date().toISOString(), + }, + }), + }), + env, + ); + + expect(response.status).toBe(401); + expect(response.headers.get("WWW-Authenticate")).toContain( + 'error="invalid_token"', + ); + expect(response.headers.get("WWW-Authenticate")).toContain("DPoP"); + }); it("should accept request with valid token", async () => { const response = await worker.fetch( new Request("http://pds.test/xrpc/com.atproto.repo.createRecord", { diff --git a/plans/complete/oauth-provider.md b/plans/complete/oauth-provider.md index d07b95db..06178f9c 100644 --- a/plans/complete/oauth-provider.md +++ b/plans/complete/oauth-provider.md @@ -87,6 +87,10 @@ packages/pds/src/ - Bearer tokens (session JWTs) - DPoP tokens (OAuth access tokens) +## Post-Completion Updates + +- ✅ PDS auth middleware now returns DPoP `WWW-Authenticate` invalid_token challenges on 401 responses so OAuth clients can trigger automatic refresh. + ## OAuth Endpoints | Endpoint | Method | Description | From 091b46932f3656d9d66a5b77a4ee351762746f21 Mon Sep 17 00:00:00 2001 From: Aidan Date: Sun, 12 Apr 2026 00:09:19 -0400 Subject: [PATCH 2/2] set separate refresh expires at on oauth tokens --- packages/oauth-provider/src/storage.ts | 11 +-- packages/oauth-provider/src/tokens.ts | 13 ++- packages/pds/src/oauth-storage.ts | 111 ++++++++++++++++++++----- packages/pds/test/storage.test.ts | 103 +++++++++++++++++++++++ packages/pds/test/xrpc.test.ts | 3 +- plans/complete/oauth-provider.md | 1 + 6 files changed, 211 insertions(+), 31 deletions(-) diff --git a/packages/oauth-provider/src/storage.ts b/packages/oauth-provider/src/storage.ts index 89645a32..1ea8d8a9 100644 --- a/packages/oauth-provider/src/storage.ts +++ b/packages/oauth-provider/src/storage.ts @@ -41,8 +41,10 @@ export interface TokenData { dpopJkt?: string; /** Issuance timestamp (Unix ms) */ issuedAt: number; - /** Expiration timestamp (Unix ms) */ - expiresAt: number; + /** Access token expiration timestamp (Unix ms) */ + accessExpiresAt: number; + /** Refresh token expiration timestamp (Unix ms) */ + refreshExpiresAt: number; /** Whether the token has been revoked */ revoked?: boolean; } @@ -258,7 +260,7 @@ export class InMemoryOAuthStorage implements OAuthStorage { async getTokenByAccess(accessToken: string): Promise { const data = this.tokens.get(accessToken); if (!data) return null; - if (data.revoked || Date.now() > data.expiresAt) { + if (data.revoked || Date.now() > data.accessExpiresAt) { return null; } return data; @@ -269,8 +271,7 @@ export class InMemoryOAuthStorage implements OAuthStorage { if (!accessToken) return null; const data = this.tokens.get(accessToken); if (!data) return null; - if (data.revoked) return null; - // Refresh tokens don't use accessToken expiresAt + if (data.revoked || Date.now() > data.refreshExpiresAt) return null; return data; } diff --git a/packages/oauth-provider/src/tokens.ts b/packages/oauth-provider/src/tokens.ts index baf7c37c..5b8909da 100644 --- a/packages/oauth-provider/src/tokens.ts +++ b/packages/oauth-provider/src/tokens.ts @@ -85,6 +85,7 @@ export function generateTokens(options: GenerateTokensOptions): { scope, dpopJkt, accessTokenTtl = ACCESS_TOKEN_TTL, + refreshTokenTtl = REFRESH_TOKEN_TTL, } = options; const accessToken = generateRandomToken(32); @@ -99,7 +100,8 @@ export function generateTokens(options: GenerateTokensOptions): { scope, dpopJkt, issuedAt: now, - expiresAt: now + accessTokenTtl, + accessExpiresAt: now + accessTokenTtl, + refreshExpiresAt: now + refreshTokenTtl, revoked: false, }; @@ -120,12 +122,14 @@ export function generateTokens(options: GenerateTokensOptions): { * @param existingData The existing token data * @param rotateRefreshToken Whether to generate a new refresh token * @param accessTokenTtl Custom access token TTL in ms + * @param refreshTokenTtl Custom refresh token TTL in ms (used when rotating) * @returns Updated tokens and token data */ export function refreshTokens( existingData: TokenData, rotateRefreshToken: boolean = false, accessTokenTtl: number = ACCESS_TOKEN_TTL, + refreshTokenTtl: number = REFRESH_TOKEN_TTL, ): { tokens: GeneratedTokens; tokenData: TokenData; @@ -141,7 +145,10 @@ export function refreshTokens( accessToken, refreshToken, issuedAt: now, - expiresAt: now + accessTokenTtl, + accessExpiresAt: now + accessTokenTtl, + refreshExpiresAt: rotateRefreshToken + ? now + refreshTokenTtl + : existingData.refreshExpiresAt, }; const tokens: GeneratedTokens = { @@ -214,7 +221,7 @@ export function isTokenValid(tokenData: TokenData): boolean { if (tokenData.revoked) { return false; } - if (Date.now() > tokenData.expiresAt) { + if (Date.now() > tokenData.accessExpiresAt) { return false; } return true; diff --git a/packages/pds/src/oauth-storage.ts b/packages/pds/src/oauth-storage.ts index 2a729a48..7719441f 100644 --- a/packages/pds/src/oauth-storage.ts +++ b/packages/pds/src/oauth-storage.ts @@ -1,9 +1,10 @@ -import type { - AuthCodeData, - ClientMetadata, - OAuthStorage, - PARData, - TokenData, +import { + type AuthCodeData, + type ClientMetadata, + type OAuthStorage, + type PARData, + type TokenData, + REFRESH_TOKEN_TTL, } from "@getcirrus/oauth-provider"; /** @@ -41,13 +42,11 @@ export class SqliteOAuthStorage implements OAuthStorage { scope TEXT NOT NULL, dpop_jkt TEXT, issued_at INTEGER NOT NULL, - expires_at INTEGER NOT NULL, + access_expires_at INTEGER NOT NULL, + refresh_expires_at INTEGER NOT NULL, revoked INTEGER NOT NULL DEFAULT 0 ); - CREATE INDEX IF NOT EXISTS idx_tokens_refresh ON oauth_tokens(refresh_token); - CREATE INDEX IF NOT EXISTS idx_tokens_sub ON oauth_tokens(sub); - CREATE INDEX IF NOT EXISTS idx_tokens_expires ON oauth_tokens(expires_at); -- Cached client metadata CREATE TABLE IF NOT EXISTS oauth_clients ( @@ -86,8 +85,10 @@ export class SqliteOAuthStorage implements OAuthStorage { CREATE INDEX IF NOT EXISTS idx_challenges_created ON oauth_webauthn_challenges(created_at); `); - // Migration: add columns for client auth metadata if missing + // Migrations for older OAuth schema versions this.migrateClientTable(); + this.migrateTokensTable(); + this.ensureTokenIndexes(); } private migrateClientTable(): void { @@ -106,6 +107,67 @@ export class SqliteOAuthStorage implements OAuthStorage { } } + private migrateTokensTable(): void { + const columns = this.sql + .exec("PRAGMA table_info(oauth_tokens)") + .toArray() + .map((r) => r.name as string); + + if (columns.length === 0) return; + if ( + columns.includes("access_expires_at") && + columns.includes("refresh_expires_at") + ) { + return; + } + + this.sql.exec(` + CREATE TABLE oauth_tokens_new ( + access_token TEXT PRIMARY KEY, + refresh_token TEXT NOT NULL UNIQUE, + client_id TEXT NOT NULL, + sub TEXT NOT NULL, + scope TEXT NOT NULL, + dpop_jkt TEXT, + issued_at INTEGER NOT NULL, + access_expires_at INTEGER NOT NULL, + refresh_expires_at INTEGER NOT NULL, + revoked INTEGER NOT NULL DEFAULT 0 + ); + `); + + this.sql.exec( + `INSERT INTO oauth_tokens_new ( + access_token, refresh_token, client_id, sub, scope, dpop_jkt, issued_at, access_expires_at, refresh_expires_at, revoked + ) + SELECT + access_token, refresh_token, client_id, sub, scope, dpop_jkt, issued_at, + expires_at AS access_expires_at, + MAX(expires_at, issued_at + ?) AS refresh_expires_at, + revoked + FROM oauth_tokens`, + REFRESH_TOKEN_TTL, + ); + + this.sql.exec("DROP TABLE oauth_tokens"); + this.sql.exec("ALTER TABLE oauth_tokens_new RENAME TO oauth_tokens"); + } + + private ensureTokenIndexes(): void { + this.sql.exec( + "CREATE INDEX IF NOT EXISTS idx_tokens_refresh ON oauth_tokens(refresh_token)", + ); + this.sql.exec( + "CREATE INDEX IF NOT EXISTS idx_tokens_sub ON oauth_tokens(sub)", + ); + this.sql.exec( + "CREATE INDEX IF NOT EXISTS idx_tokens_access_expires ON oauth_tokens(access_expires_at)", + ); + this.sql.exec( + "CREATE INDEX IF NOT EXISTS idx_tokens_refresh_expires ON oauth_tokens(refresh_expires_at)", + ); + } + /** * Clean up expired entries. Should be called periodically. */ @@ -113,7 +175,7 @@ export class SqliteOAuthStorage implements OAuthStorage { const now = Date.now(); this.sql.exec("DELETE FROM oauth_auth_codes WHERE expires_at < ?", now); this.sql.exec( - "DELETE FROM oauth_tokens WHERE expires_at < ? AND revoked = 0", + "DELETE FROM oauth_tokens WHERE refresh_expires_at < ?", now, ); this.sql.exec("DELETE FROM oauth_par_requests WHERE expires_at < ?", now); @@ -189,8 +251,8 @@ export class SqliteOAuthStorage implements OAuthStorage { async saveTokens(data: TokenData): Promise { this.sql.exec( `INSERT INTO oauth_tokens - (access_token, refresh_token, client_id, sub, scope, dpop_jkt, issued_at, expires_at, revoked) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)`, + (access_token, refresh_token, client_id, sub, scope, dpop_jkt, issued_at, access_expires_at, refresh_expires_at, revoked) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, data.accessToken, data.refreshToken, data.clientId, @@ -198,7 +260,8 @@ export class SqliteOAuthStorage implements OAuthStorage { data.scope, data.dpopJkt ?? null, data.issuedAt, - data.expiresAt, + data.accessExpiresAt, + data.refreshExpiresAt, data.revoked ? 1 : 0, ); } @@ -206,7 +269,7 @@ export class SqliteOAuthStorage implements OAuthStorage { async getTokenByAccess(accessToken: string): Promise { const rows = this.sql .exec( - `SELECT access_token, refresh_token, client_id, sub, scope, dpop_jkt, issued_at, expires_at, revoked + `SELECT access_token, refresh_token, client_id, sub, scope, dpop_jkt, issued_at, access_expires_at, refresh_expires_at, revoked FROM oauth_tokens WHERE access_token = ?`, accessToken, ) @@ -216,9 +279,9 @@ export class SqliteOAuthStorage implements OAuthStorage { const row = rows[0]!; const revoked = Boolean(row.revoked); - const expiresAt = row.expires_at as number; + const accessExpiresAt = row.access_expires_at as number; - if (revoked || Date.now() > expiresAt) { + if (revoked || Date.now() > accessExpiresAt) { return null; } @@ -230,7 +293,8 @@ export class SqliteOAuthStorage implements OAuthStorage { scope: row.scope as string, dpopJkt: (row.dpop_jkt as string) ?? undefined, issuedAt: row.issued_at as number, - expiresAt, + accessExpiresAt, + refreshExpiresAt: row.refresh_expires_at as number, revoked, }; } @@ -238,7 +302,7 @@ export class SqliteOAuthStorage implements OAuthStorage { async getTokenByRefresh(refreshToken: string): Promise { const rows = this.sql .exec( - `SELECT access_token, refresh_token, client_id, sub, scope, dpop_jkt, issued_at, expires_at, revoked + `SELECT access_token, refresh_token, client_id, sub, scope, dpop_jkt, issued_at, access_expires_at, refresh_expires_at, revoked FROM oauth_tokens WHERE refresh_token = ?`, refreshToken, ) @@ -249,7 +313,9 @@ export class SqliteOAuthStorage implements OAuthStorage { const row = rows[0]!; const revoked = Boolean(row.revoked); - if (revoked) return null; + const refreshExpiresAt = row.refresh_expires_at as number; + + if (revoked || Date.now() > refreshExpiresAt) return null; return { accessToken: row.access_token as string, @@ -259,7 +325,8 @@ export class SqliteOAuthStorage implements OAuthStorage { scope: row.scope as string, dpopJkt: (row.dpop_jkt as string) ?? undefined, issuedAt: row.issued_at as number, - expiresAt: row.expires_at as number, + accessExpiresAt: row.access_expires_at as number, + refreshExpiresAt, revoked, }; } diff --git a/packages/pds/test/storage.test.ts b/packages/pds/test/storage.test.ts index 1400f1ac..8e1c4ea1 100644 --- a/packages/pds/test/storage.test.ts +++ b/packages/pds/test/storage.test.ts @@ -5,6 +5,8 @@ import { encode, cidForCbor, type LexValue } from "@atproto/lex-cbor"; import { BlockMap, CidSet } from "@atproto/repo"; import { AccountDurableObject } from "../src/account-do"; import { SqliteRepoStorage } from "../src/storage"; +import { SqliteOAuthStorage } from "../src/oauth-storage"; +import { REFRESH_TOKEN_TTL } from "@getcirrus/oauth-provider"; // Helper to create a CID from data async function createCid( @@ -332,3 +334,104 @@ describe("AccountDurableObject", () => { }); }); }); + + +describe("SqliteOAuthStorage", () => { + it("preserves refresh-valid tokens when access token has expired", async () => { + const id = env.ACCOUNT.newUniqueId(); + const stub = env.ACCOUNT.get(id); + + await runInDurableObject(stub, async (instance: AccountDurableObject) => { + const oauthStorage = await instance.getOAuthStorage(); + const now = Date.now(); + const tokenData = { + accessToken: "expired-access-token", + refreshToken: "still-valid-refresh-token", + clientId: "did:web:app.example", + sub: env.DID, + scope: "atproto", + issuedAt: now - 2 * 60 * 60 * 1000, + accessExpiresAt: now - 60 * 1000, + refreshExpiresAt: now + 24 * 60 * 60 * 1000, + revoked: false, + }; + + await oauthStorage.saveTokens(tokenData); + oauthStorage.cleanup(); + + const tokenByRefresh = await oauthStorage.getTokenByRefresh( + tokenData.refreshToken, + ); + expect(tokenByRefresh).not.toBeNull(); + expect(tokenByRefresh?.refreshToken).toBe(tokenData.refreshToken); + expect(await oauthStorage.getTokenByAccess(tokenData.accessToken)).toBeNull(); + }); + }); + + it("migrates legacy oauth_tokens schema without failing on missing new columns", async () => { + const id = env.ACCOUNT.newUniqueId(); + const stub = env.ACCOUNT.get(id); + + await runInDurableObject(stub, async (instance: AccountDurableObject) => { + const sql = (instance as unknown as { ctx: { storage: { sql: SqlStorage } } }) + .ctx.storage.sql; + const now = Date.now(); + const issuedAt = now - 2 * 60 * 60 * 1000; + const expiresAt = now - 60 * 1000; + + sql.exec(` + CREATE TABLE oauth_tokens ( + access_token TEXT PRIMARY KEY, + refresh_token TEXT NOT NULL UNIQUE, + client_id TEXT NOT NULL, + sub TEXT NOT NULL, + scope TEXT NOT NULL, + dpop_jkt TEXT, + issued_at INTEGER NOT NULL, + expires_at INTEGER NOT NULL, + revoked INTEGER NOT NULL DEFAULT 0 + ); + `); + + sql.exec( + `INSERT INTO oauth_tokens + (access_token, refresh_token, client_id, sub, scope, dpop_jkt, issued_at, expires_at, revoked) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)`, + "legacy-access", + "legacy-refresh", + "did:web:app.example", + env.DID, + "atproto", + null, + issuedAt, + expiresAt, + 0, + ); + + const oauthStorage = new SqliteOAuthStorage(sql); + expect(() => oauthStorage.initSchema()).not.toThrow(); + + const columns = sql + .exec("PRAGMA table_info(oauth_tokens)") + .toArray() + .map((row) => row.name as string); + + expect(columns).toContain("access_expires_at"); + expect(columns).toContain("refresh_expires_at"); + expect(columns).not.toContain("expires_at"); + + const migratedRow = sql + .exec( + `SELECT access_expires_at, refresh_expires_at FROM oauth_tokens WHERE access_token = ?`, + "legacy-access", + ) + .one(); + + expect(migratedRow).toBeDefined(); + expect(migratedRow.access_expires_at as number).toBe(expiresAt); + expect(migratedRow.refresh_expires_at as number).toBe( + Math.max(expiresAt, issuedAt + REFRESH_TOKEN_TTL), + ); + }); + }); +}); diff --git a/packages/pds/test/xrpc.test.ts b/packages/pds/test/xrpc.test.ts index 9343002c..f6a9c68f 100644 --- a/packages/pds/test/xrpc.test.ts +++ b/packages/pds/test/xrpc.test.ts @@ -163,7 +163,8 @@ describe("XRPC Endpoints", () => { scope: "atproto", dpopJkt, issuedAt: now - 2 * 60 * 60 * 1000, - expiresAt: now - 60 * 1000, + accessExpiresAt: now - 60 * 1000, + refreshExpiresAt: now + 30 * 24 * 60 * 60 * 1000, revoked: false, }); diff --git a/plans/complete/oauth-provider.md b/plans/complete/oauth-provider.md index 06178f9c..b6bfe573 100644 --- a/plans/complete/oauth-provider.md +++ b/plans/complete/oauth-provider.md @@ -90,6 +90,7 @@ packages/pds/src/ ## Post-Completion Updates - ✅ PDS auth middleware now returns DPoP `WWW-Authenticate` invalid_token challenges on 401 responses so OAuth clients can trigger automatic refresh. +- ✅ OAuth token storage now uses separate access/refresh expiries, and cleanup prunes by refresh expiry so refresh remains possible after access expiry. ## OAuth Endpoints