Fix Two OAuth token refresh bugs

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<TokenData | null> {
 		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;
 	}
 

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;

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"',
+				},
 			);
 		}
 

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,14 +107,75 @@ 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.
 	 */
 	cleanup(): void {
 		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,24 +251,25 @@ export class SqliteOAuthStorage implements OAuthStorage {
 	async saveTokens(data: TokenData): Promise<void> {
 		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,
 			data.sub,
 			data.scope,
 			data.dpopJkt ?? null,
 			data.issuedAt,
-			data.expiresAt,
+			data.accessExpiresAt,
+			data.refreshExpiresAt,
 			data.revoked ? 1 : 0,
 		);
 	}
 
 	async getTokenByAccess(accessToken: string): Promise<TokenData | null> {
 		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,15 +293,16 @@ 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,
 		};
 	}
 
 	async getTokenByRefresh(refreshToken: string): Promise<TokenData | null> {
 		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,
 		};
 	}