Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ Run `devspace init` to create both files. `devspace config set publicBaseUrl
"oauth": {
"accessTokenTtlSeconds": 3600,
"refreshTokenTtlSeconds": 2592000,
"refreshTokenGraceSeconds": 0,
"scopes": ["devspace"],
"allowedResourceUrls": [],
"allowedRedirectHosts": ["chatgpt.com", "localhost", "127.0.0.1"],
Expand All @@ -79,6 +80,19 @@ Omitted sections and keys use the defaults shown above. An empty
`workspaces.allowedRoots` uses the current working directory. Unknown keys are
rejected so spelling mistakes cannot silently alter behavior.

`oauth.refreshTokenGraceSeconds` optionally allows a rotated refresh token to be
retried for 0–600 seconds. The default, `0`, consumes the token immediately.
For a trusted host that issues concurrent refreshes or retries after losing a
response, set it to a bounded window such as `600` and restart DevSpace. On first
use, the old token's expiry is shortened to the earlier of its original expiry
and the grace deadline; retries never extend that deadline. New refresh tokens
keep the configured `oauth.refreshTokenTtlSeconds`. The deadline persists across
server restarts without a database migration.

Enabling this option allows anyone holding the rotated token to reuse it during
the window. Keep it disabled unless that compatibility behavior is needed;
client, scope, resource, and token revocation checks still apply.

`oauth.allowedResourceUrls` accepts exact alternate MCP resource URLs for
clients that connect through a resource alias, such as a secure MCP tunnel.
The normal `server.publicBaseUrl` `/mcp` resource remains allowed automatically.
Expand Down
7 changes: 7 additions & 0 deletions schema/v1/devspace.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,13 @@
"exclusiveMinimum": 0,
"maximum": 9007199254740991
},
"refreshTokenGraceSeconds": {
"default": 0,
"description": "Allow refresh-token retries for up to this many seconds after rotation. Zero consumes tokens immediately.",
"type": "integer",
"minimum": 0,
"maximum": 600
},
"scopes": {
"default": [
"devspace"
Expand Down
12 changes: 12 additions & 0 deletions src/config-schema.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,18 @@ assert.throws(
/Unrecognized key/,
);

assert.equal(devspaceConfigSchema.parse({ configVersion: 1 }).oauth.refreshTokenGraceSeconds, 0);
for (const refreshTokenGraceSeconds of [0, 1, 600]) {
assert.equal(devspaceConfigSchema.parse({
configVersion: 1, oauth: { refreshTokenGraceSeconds },
}).oauth.refreshTokenGraceSeconds, refreshTokenGraceSeconds);
}
for (const refreshTokenGraceSeconds of [-1, 601, 1.5, "600"]) {
assert.equal(devspaceConfigSchema.safeParse({
configVersion: 1, oauth: { refreshTokenGraceSeconds },
}).success, false);
}

for (const url of [
"https://tunnel.example.com/v1/mcp/tunnel_123",
"http://localhost:7676/mcp",
Expand Down
2 changes: 2 additions & 0 deletions src/config-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ const loggingConfigSchema = z.object({
const oauthConfigSchema = z.object({
accessTokenTtlSeconds: z.number().int().positive().default(60 * 60),
refreshTokenTtlSeconds: z.number().int().positive().default(30 * 24 * 60 * 60),
refreshTokenGraceSeconds: z.number().int().min(0).max(600).default(0)
.describe("Allow refresh-token retries for up to this many seconds after rotation. Zero consumes tokens immediately."),
scopes: z.array(z.string().trim().min(1)).min(1).default(["devspace"]),
allowedResourceUrls: z.array(z.string().trim().url().refine((value) => {
const url = URL.parse(value);
Expand Down
3 changes: 3 additions & 0 deletions src/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ try {
providers: [],
});
assert.deepEqual(defaults.oauth.allowedResourceUrls, []);
assert.equal(defaults.oauth.refreshTokenGraceSeconds, 0);
assert.deepEqual(defaults.logging, {
level: "info",
format: "json",
Expand Down Expand Up @@ -72,6 +73,7 @@ try {
oauth: {
accessTokenTtlSeconds: 120,
refreshTokenTtlSeconds: 240,
refreshTokenGraceSeconds: 60,
scopes: ["devspace", "admin"],
allowedResourceUrls: ["https://tunnel.example.com/v1/mcp/tunnel_123"],
allowedRedirectHosts: ["chatgpt.com", "example.com"],
Expand Down Expand Up @@ -106,6 +108,7 @@ try {
assert.equal(configured.subagents.instructions, "preload");
assert.equal(configured.oauth.ownerToken, "persisted-owner-token-long-enough");
assert.equal(configured.oauth.accessTokenTtlSeconds, 120);
assert.equal(configured.oauth.refreshTokenGraceSeconds, 60);
assert.deepEqual(configured.oauth.scopes, ["devspace", "admin"]);
assert.deepEqual(configured.oauth.allowedResourceUrls, [
"https://tunnel.example.com/v1/mcp/tunnel_123",
Expand Down
1 change: 1 addition & 0 deletions src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export function loadConfig(env: NodeJS.ProcessEnv = process.env): ServerConfig {
),
accessTokenTtlSeconds: stored.oauth.accessTokenTtlSeconds,
refreshTokenTtlSeconds: stored.oauth.refreshTokenTtlSeconds,
refreshTokenGraceSeconds: stored.oauth.refreshTokenGraceSeconds,
scopes: stored.oauth.scopes,
allowedResourceUrls: stored.oauth.allowedResourceUrls,
allowedRedirectHosts: stored.oauth.allowedRedirectHosts,
Expand Down
3 changes: 2 additions & 1 deletion src/oauth-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export interface OAuthConfig {
ownerToken: string;
accessTokenTtlSeconds: number;
refreshTokenTtlSeconds: number;
refreshTokenGraceSeconds?: number;
scopes: string[];
allowedResourceUrls: string[];
allowedRedirectHosts: string[];
Expand Down Expand Up @@ -128,7 +129,7 @@ export class SingleUserOAuthProvider implements OAuthServerProvider {
this.allowedResourceUrls = new Set(
config.allowedResourceUrls.map((url) => resourceUrlFromServerUrl(url).href),
);
this.oauthStore = new SqliteOAuthStore(stateDir);
this.oauthStore = new SqliteOAuthStore(stateDir, config.refreshTokenGraceSeconds ?? 0);
this.clientsStore = new SqliteOAuthClientsStore(this.oauthStore, config.allowedRedirectHosts);
}

Expand Down
218 changes: 218 additions & 0 deletions src/oauth-refresh-grace.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,218 @@
import assert from "node:assert/strict";
import { createHash } from "node:crypto";
import { mkdtemp, rm } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { AccessDeniedError, InvalidGrantError } from "@modelcontextprotocol/sdk/server/auth/errors.js";
import { SingleUserOAuthProvider } from "./oauth-provider.js";
import type { OAuthConfig } from "./oauth-provider.js";
import { SqliteOAuthStore } from "./oauth-store.js";

const root = await mkdtemp(join(tmpdir(), "devspace-oauth-grace-test-"));
const baseConfig: OAuthConfig = {
ownerToken: "test-owner-token-that-is-long-enough",
accessTokenTtlSeconds: 3600,
refreshTokenTtlSeconds: 2592000,
scopes: ["devspace", "read"],
allowedResourceUrls: ["https://tunnel.example.com/v1/mcp/tunnel_123"],
allowedRedirectHosts: ["chatgpt.com"],
};
const serverUrl = new URL("https://agent.example.com/mcp");
const resource = new URL(baseConfig.allowedResourceUrls[0]!);
const otherResource = new URL("https://other.example.com/mcp");
const redirectUri = "https://chatgpt.com/connector_platform_oauth_redirect";
const realDateNow = Date.now;

try {
await testDefaultAndZeroGrace();
await testConcurrentReuseAndRestartPersistence();
await testGraceExpiryAndOriginalTtl();
await testGraceDoesNotRelaxRefreshPolicy();
} finally {
Date.now = realDateNow;
await rm(root, { recursive: true, force: true });
}

async function testDefaultAndZeroGrace(): Promise<void> {
const defaultDir = join(root, "default");
const zeroDir = join(root, "zero");
for (const [stateDir, config] of [[defaultDir, baseConfig], [zeroDir, { ...baseConfig, refreshTokenGraceSeconds: 0 }]] as const) {
const provider = new SingleUserOAuthProvider(config, serverUrl, stateDir);
try {
const { client, tokens } = await issueInitialTokens(provider);
const rotated = await provider.exchangeRefreshToken(client, tokens.refresh_token);
await assert.rejects(provider.exchangeRefreshToken(client, tokens.refresh_token), InvalidGrantError);
assert.ok(rotated.refresh_token);
} finally {
provider.close();
}
}
}

async function testConcurrentReuseAndRestartPersistence(): Promise<void> {
const stateDir = join(root, "concurrent-restart");
const config = { ...baseConfig, refreshTokenGraceSeconds: 600 };
const baseSeconds = 2_000_000_500;
setNowSeconds(baseSeconds);
try {
const first = new SingleUserOAuthProvider(config, serverUrl, stateDir);
const { client, tokens } = await issueInitialTokens(first);
first.close();

setNowSeconds(baseSeconds + 20);
const left = new SingleUserOAuthProvider(config, serverUrl, stateDir);
const right = new SingleUserOAuthProvider(config, serverUrl, stateDir);
try {
const [leftTokens, rightTokens] = await Promise.all([
left.exchangeRefreshToken(client, tokens.refresh_token),
right.exchangeRefreshToken(client, tokens.refresh_token),
]);
Comment on lines +66 to +69

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Refresh calls do not overlap

Both refresh calls complete their synchronous SQLite work one after the other, despite being passed to Promise.all. The test checks reuse through two connections, but cannot catch regressions that appear only when rotations contend for a database write lock. This test-coverage gap is non-blocking.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Artifacts

Executable refresh-rotation comparison script

  • The authored script runs the provider calls with same-event-loop tracing and with an independent process holding a SQLite write lock, enabling a direct comparison.

Original refresh-grace test passing

  • The repository's refresh-grace test was run unchanged and passed, showing that its assertions do not require overlapping rotations.

Same-event-loop refresh calls running sequentially

  • Two provider calls in the test's `Promise.all` structure produced distinct tokens, while the trace showed the first transaction exited before the second read began.

Independent-process refresh calls encountering a SQLite lock

  • A separate process held an uncommitted rotation while the second call waited 914 ms for its write lock, showing contention absent from the original test.

View artifacts

T-Rex Ran code and verified through T-Rex

assert.ok(leftTokens.refresh_token);
assert.ok(rightTokens.refresh_token);
assert.notEqual(leftTokens.refresh_token, rightTokens.refresh_token);
} finally {
left.close();
right.close();
}

const reopened = new SingleUserOAuthProvider(config, serverUrl, stateDir);
try {
assert.equal(readRefreshExpiry(stateDir, tokens.refresh_token), baseSeconds + 20 + 600);
const retried = await reopened.exchangeRefreshToken(client, tokens.refresh_token);
assert.ok(retried.refresh_token);
assert.equal((await reopened.verifyAccessToken(retried.access_token)).clientId, client.client_id);
await reopened.revokeToken(client, { token: retried.refresh_token });
await assert.rejects(reopened.exchangeRefreshToken(client, retried.refresh_token), InvalidGrantError);

setNowSeconds(baseSeconds + 20 + 600);
const atBoundary = await reopened.exchangeRefreshToken(client, tokens.refresh_token);
assert.ok(atBoundary.refresh_token, "the 600-second grace token is accepted at its exact expiry second");
setNowSeconds(baseSeconds + 20 + 601);
await assert.rejects(reopened.exchangeRefreshToken(client, tokens.refresh_token), InvalidGrantError);
} finally {
reopened.close();
}
} finally {
Date.now = realDateNow;
}
}

async function testGraceExpiryAndOriginalTtl(): Promise<void> {
const stateDir = join(root, "expiry");
const config = { ...baseConfig, refreshTokenGraceSeconds: 10, refreshTokenTtlSeconds: 30 };
const baseSeconds = 2_000_000_000;
setNowSeconds(baseSeconds);
const provider = new SingleUserOAuthProvider(config, serverUrl, stateDir);
try {
const { client, tokens } = await issueInitialTokens(provider);
const oldExpiry = baseSeconds + 30;
assert.equal((await provider.verifyAccessToken(tokens.access_token)).expiresAt, baseSeconds + 3600);
assert.equal(readRefreshExpiry(stateDir, tokens.refresh_token), oldExpiry);

setNowSeconds(baseSeconds + 5);
const firstRotation = await provider.exchangeRefreshToken(client, tokens.refresh_token);
assert.equal(readRefreshExpiry(stateDir, tokens.refresh_token), baseSeconds + 15);
const firstNewRefreshExpiry = readRefreshExpiry(stateDir, firstRotation.refresh_token!);
assert.equal(firstNewRefreshExpiry, baseSeconds + 5 + 30);
assert.equal((await provider.verifyAccessToken(firstRotation.access_token)).expiresAt, baseSeconds + 5 + 3600);

setNowSeconds(baseSeconds + 9);
const secondRotation = await provider.exchangeRefreshToken(client, tokens.refresh_token);
assert.equal(readRefreshExpiry(stateDir, tokens.refresh_token), baseSeconds + 15);
assert.equal(readRefreshExpiry(stateDir, secondRotation.refresh_token!), baseSeconds + 9 + 30);

setNowSeconds(baseSeconds + 15);
const atBoundary = await provider.exchangeRefreshToken(client, tokens.refresh_token);
assert.ok(atBoundary.refresh_token, "the grace token is accepted at its exact expiry second");
assert.equal(readRefreshExpiry(stateDir, tokens.refresh_token), baseSeconds + 15);

setNowSeconds(baseSeconds + 16);
await assert.rejects(provider.exchangeRefreshToken(client, tokens.refresh_token), InvalidGrantError);
} finally {
provider.close();
Date.now = realDateNow;
}

const shortTtlDir = join(root, "short-original-ttl");
const shortTtlConfig = { ...baseConfig, refreshTokenGraceSeconds: 10, refreshTokenTtlSeconds: 6 };
setNowSeconds(baseSeconds);
const shortTtlProvider = new SingleUserOAuthProvider(shortTtlConfig, serverUrl, shortTtlDir);
try {
const { client, tokens } = await issueInitialTokens(shortTtlProvider);
assert.equal(readRefreshExpiry(shortTtlDir, tokens.refresh_token), baseSeconds + 6);
setNowSeconds(baseSeconds + 3);
await shortTtlProvider.exchangeRefreshToken(client, tokens.refresh_token);
assert.equal(
readRefreshExpiry(shortTtlDir, tokens.refresh_token),
baseSeconds + 6,
"grace must not extend a refresh token past its original earlier expiry",
);
setNowSeconds(baseSeconds + 6);
await shortTtlProvider.exchangeRefreshToken(client, tokens.refresh_token);
setNowSeconds(baseSeconds + 7);
await assert.rejects(shortTtlProvider.exchangeRefreshToken(client, tokens.refresh_token), InvalidGrantError);
} finally {
shortTtlProvider.close();
Date.now = realDateNow;
}
}

async function testGraceDoesNotRelaxRefreshPolicy(): Promise<void> {
const stateDir = join(root, "policy");
const config = { ...baseConfig, refreshTokenGraceSeconds: 600 };
const provider = new SingleUserOAuthProvider(config, serverUrl, stateDir);
try {
const { client, tokens } = await issueInitialTokens(provider);
const otherClient = await provider.clientsStore.registerClient?.({ redirect_uris: [redirectUri] });
assert.ok(otherClient);
await provider.exchangeRefreshToken(client, tokens.refresh_token);

await assert.rejects(provider.exchangeRefreshToken(otherClient, tokens.refresh_token), InvalidGrantError);
await assert.rejects(provider.exchangeRefreshToken(client, tokens.refresh_token, ["write"]), AccessDeniedError);
await assert.rejects(
provider.exchangeRefreshToken(client, tokens.refresh_token, undefined, otherResource),
InvalidGrantError,
);
} finally {
provider.close();
}
}

async function issueInitialTokens(provider: SingleUserOAuthProvider) {
const client = await provider.clientsStore.registerClient?.({
redirect_uris: [redirectUri],
client_name: "Test client",
});
assert.ok(client);
const code = `test-code-${Math.random()}`;
provider["codes"].set(code, {
clientId: client.client_id,
params: {
redirectUri,
codeChallenge: "challenge",
scopes: ["devspace"],
resource,
},
expiresAtMs: Date.now() + 60_000,
});
const tokens = await provider.exchangeAuthorizationCode(client, code, undefined, redirectUri, resource);
assert.ok(tokens.refresh_token);
return { client, tokens: { ...tokens, refresh_token: tokens.refresh_token } };
}

function readRefreshExpiry(stateDir: string, token: string): number | undefined {
const store = new SqliteOAuthStore(stateDir);
try {
return store.getRefreshToken(hashToken(token))?.expiresAt;
} finally {
store.close();
}
}

function hashToken(token: string): string {
return createHash("sha256").update(token).digest("base64url");
}

function setNowSeconds(seconds: number): void {
Date.now = () => seconds * 1000;
}
14 changes: 10 additions & 4 deletions src/oauth-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ function redirectHostAllowed(redirectUri: string, allowedHosts: string[]): boole
export class SqliteOAuthStore {
private readonly database: DatabaseHandle;

constructor(stateDir: string) {
constructor(stateDir: string, private readonly refreshTokenGraceSeconds = 0) {
this.database = openDatabase(stateDir);
this.deleteExpiredTokens(Math.floor(Date.now() / 1000));
}
Expand Down Expand Up @@ -142,9 +142,15 @@ export class SqliteOAuthStore {
saveTokenPair(pair: PersistedTokenPair, consumedRefreshTokenHash?: string): boolean {
const save = this.database.sqlite.transaction(() => {
if (consumedRefreshTokenHash) {
const result = this.database.sqlite
.prepare("delete from oauth_refresh_tokens where token_hash = ?")
.run(consumedRefreshTokenHash);
const now = Math.floor(Date.now() / 1000);
// Retried rotations must not extend the original grace deadline or TTL.
const result = this.refreshTokenGraceSeconds > 0
? this.database.sqlite
.prepare("update oauth_refresh_tokens set expires_at = min(expires_at, ?) where token_hash = ? and expires_at >= ?")
.run(now + this.refreshTokenGraceSeconds, consumedRefreshTokenHash, now)
: this.database.sqlite
.prepare("delete from oauth_refresh_tokens where token_hash = ?")
.run(consumedRefreshTokenHash);
if (result.changes !== 1) return false;
}

Expand Down