From 5b08f1879be0f17b82f3062e98b75da8fc54f986 Mon Sep 17 00:00:00 2001 From: AminDhouib Date: Sun, 13 Sep 2026 12:14:30 -0400 Subject: [PATCH] feat(mcp): 30d access tokens, 365d refresh, 5min rotation grace Claude Code was re-authorizing against /api/mcp roughly daily. The MCP OAuth TTLs were already env-driven but defaulted conservatively, and rotation had no grace window at all. access token 24h -> 720h (30 days) DOKPLOY_MCP_ACCESS_TOKEN_HOURS refresh token 180d -> 365d (sliding) DOKPLOY_MCP_REFRESH_TOKEN_DAYS rotation grace 0s -> 300s DOKPLOY_MCP_REFRESH_GRACE_SECONDS (new) The grace window is the substantive change. better-auth rotates by inserting a new row and leaving the consumed one fully alive, so the fork retired it immediately after a successful refresh. That is the strictest option but it strands clients: a dropped refresh response or two racing requests leave the client holding a token that no longer exists, and the only way out is a browser re-auth. PostHog hit exactly this with MCP clients and responded by disabling rotation for them outright; Google, Okta and Cognito issue non-rotating refresh tokens for the same reason. deleteConsumedRefreshToken becomes consumeRotatedRefreshToken, which clamps the consumed row with LEAST() instead of deleting it. LEAST only ever shortens, so a row already expiring sooner keeps its own expiry, and one with a NULL refresh expiry ends up bounded rather than open. The daily purge reaps them either way. Setting the grace to 0 restores the previous delete-at-once behaviour, and the helper takes the value as an injectable argument because the vitest config statically defines process.env. Auth-code TTL is untouched at the plugin default of 600s. Documents all four MCP knobs in apps/dokploy/.env.example, which previously mentioned none of them, and corrects the README. --- README.md | 2 +- apps/dokploy/.env.example | 23 ++++++ .../__test__/mcp/mcp-oauth-service.test.ts | 26 +++++- .../__test__/mcp/mcp-oauth-tokens.test.ts | 27 ++++-- packages/server/src/lib/auth.ts | 8 +- packages/server/src/services/mcp-oauth.ts | 82 +++++++++++++++++-- 6 files changed, 147 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 066113f554c..01fefc096b7 100644 --- a/README.md +++ b/README.md @@ -133,7 +133,7 @@ Every item above is ported 1:1 and credited to its original upstream author. See - **MCP server hosted inside Dokploy** — `POST /api/mcp` serves all 664 API procedures as MCP tools (same names as `@dokploy/mcp`) over Streamable HTTP, so 50 Claude Code sessions share one HTTPS endpoint instead of spawning 50 local processes; `claude mcp add --transport http --scope user dokploy https:///api/mcp` and `/mcp → Authenticate` is the whole setup ([#203](https://github.com/DevinoSolutions/dokploy-community/pull/203)) - **OAuth 2.1 instead of API keys** — PKCE, dynamic client registration and RFC 9728 discovery via better-auth's in-core `mcp` plugin, hardened with a consent-proof gate on the authorize endpoint, a loopback/https-only redirect-URI policy, refresh-token rotation cleanup, and a bounded request body ([#203](https://github.com/DevinoSolutions/dokploy-community/pull/203)) - **Scoped grants** — the consent page lets you check off exactly what a client may do: `dokploy:read`, `deploy`, `services:write`, `services:delete`, `projects:write`, `projects:delete`, `backups`, `admin` (delete + admin off by default); credential-store queries and raw file reads require `admin`; every tool still runs the caller's role checks ([#203](https://github.com/DevinoSolutions/dokploy-community/pull/203)) -- **Minimal re-authentication** — 24-hour access tokens with a 180-day sliding refresh (`DOKPLOY_MCP_ACCESS_TOKEN_HOURS` / `DOKPLOY_MCP_REFRESH_TOKEN_DAYS`), daily purge of expired rows, `DOKPLOY_MCP_DISABLED=true` kill switch, and a Settings → Profile card listing authorized clients with one-click revoke ([#203](https://github.com/DevinoSolutions/dokploy-community/pull/203)) +- **Minimal re-authentication** — 30-day access tokens with a 365-day sliding refresh and a 5-minute grace window on rotated refresh tokens (`DOKPLOY_MCP_ACCESS_TOKEN_HOURS` / `DOKPLOY_MCP_REFRESH_TOKEN_DAYS` / `DOKPLOY_MCP_REFRESH_GRACE_SECONDS`), daily purge of expired rows, `DOKPLOY_MCP_DISABLED=true` kill switch, and a Settings → Profile card listing authorized clients with one-click revoke ([#203](https://github.com/DevinoSolutions/dokploy-community/pull/203)) ### New in v0.30.3-community.3 diff --git a/apps/dokploy/.env.example b/apps/dokploy/.env.example index 34efe4cf475..22009d6e71c 100644 --- a/apps/dokploy/.env.example +++ b/apps/dokploy/.env.example @@ -3,3 +3,26 @@ PORT=3000 NODE_ENV=development # TRAEFIK_PORT=7080 # TRAEFIK_SWARM_MODE=true # Deploy Traefik as a Swarm service instead of a standalone container. Required on macOS with Docker Desktop gvisor networking. + +# ─── Remote MCP OAuth (optional) ───────────────────────────────────────────── +# How long an MCP client (Claude Code, Cursor) stays signed in to POST /api/mcp +# before it has to re-authorize in a browser. All optional — the defaults below +# are what the server uses when these are unset. Changing one needs a restart: +# the values are read once when better-auth is constructed. +# +# Access-token lifetime in hours. Default 720 (30 days). +# DOKPLOY_MCP_ACCESS_TOKEN_HOURS=720 +# +# Refresh-token lifetime in days, sliding: the window restarts on every +# refresh, so an active client effectively never has to re-authorize. +# Default 365. +# DOKPLOY_MCP_REFRESH_TOKEN_DAYS=365 +# +# How long a rotated refresh token stays usable after being consumed, in +# seconds. Covers a dropped refresh response or two racing requests, either of +# which would otherwise force a browser re-auth. Set 0 to revoke immediately. +# Default 300. +# DOKPLOY_MCP_REFRESH_GRACE_SECONDS=300 +# +# Kill switch: removes the MCP endpoint and its daily token purge entirely. +# DOKPLOY_MCP_DISABLED=true diff --git a/apps/dokploy/__test__/mcp/mcp-oauth-service.test.ts b/apps/dokploy/__test__/mcp/mcp-oauth-service.test.ts index 3e704197293..0e2991f3d1f 100644 --- a/apps/dokploy/__test__/mcp/mcp-oauth-service.test.ts +++ b/apps/dokploy/__test__/mcp/mcp-oauth-service.test.ts @@ -9,6 +9,7 @@ let webServerSettingsRow: { host?: string | null } | undefined; const { db } = await import("@dokploy/server/db"); const { getMcpAccessTokenSeconds, + getMcpRefreshGraceSeconds, getMcpRefreshTokenSeconds, isAllowedRedirectUri, isMcpDisabled, @@ -21,9 +22,10 @@ const findFirst = vi.mocked(db.query.member.findFirst); // The vitest config statically `define`s `process.env`, so tests pass an // explicit env object instead of mutating process.env. describe("mcp-oauth env knobs", () => { - it("defaults to 24h access / 180d refresh", () => { - expect(getMcpAccessTokenSeconds({})).toBe(24 * 3600); - expect(getMcpRefreshTokenSeconds({})).toBe(180 * 86400); + it("defaults to 30d access / 365d refresh / 5min grace", () => { + expect(getMcpAccessTokenSeconds({})).toBe(720 * 3600); + expect(getMcpRefreshTokenSeconds({})).toBe(365 * 86400); + expect(getMcpRefreshGraceSeconds({})).toBe(300); }); it("honours positive integer overrides and ignores garbage", () => { @@ -32,7 +34,23 @@ describe("mcp-oauth env knobs", () => { ).toBe(6 * 3600); expect( getMcpRefreshTokenSeconds({ DOKPLOY_MCP_REFRESH_TOKEN_DAYS: "-3" }), - ).toBe(180 * 86400); + ).toBe(365 * 86400); + }); + + // Unlike the TTLs, 0 is a real setting here: it means "revoke at once". + it("accepts a zero grace period but still rejects garbage", () => { + expect( + getMcpRefreshGraceSeconds({ DOKPLOY_MCP_REFRESH_GRACE_SECONDS: "0" }), + ).toBe(0); + expect( + getMcpRefreshGraceSeconds({ DOKPLOY_MCP_REFRESH_GRACE_SECONDS: "30" }), + ).toBe(30); + expect( + getMcpRefreshGraceSeconds({ DOKPLOY_MCP_REFRESH_GRACE_SECONDS: "-1" }), + ).toBe(300); + expect( + getMcpRefreshGraceSeconds({ DOKPLOY_MCP_REFRESH_GRACE_SECONDS: "nope" }), + ).toBe(300); }); it("isMcpDisabled only for the literal string true", () => { diff --git a/apps/dokploy/__test__/mcp/mcp-oauth-tokens.test.ts b/apps/dokploy/__test__/mcp/mcp-oauth-tokens.test.ts index 4b4fe70dd5d..0c6ef526884 100644 --- a/apps/dokploy/__test__/mcp/mcp-oauth-tokens.test.ts +++ b/apps/dokploy/__test__/mcp/mcp-oauth-tokens.test.ts @@ -2,8 +2,8 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; const { db } = await import("@dokploy/server/db"); const { + consumeRotatedRefreshToken, createConsentProof, - deleteConsumedRefreshToken, findMcpAccessToken, findOAuthApplicationByClientId, listMcpAuthorizations, @@ -19,6 +19,7 @@ const findFirst = vi.mocked(db.query.oauthAccessToken.findFirst); const findMany = vi.mocked(db.query.oauthAccessToken.findMany); const dbDelete = vi.mocked(db.delete); const dbInsert = vi.mocked(db.insert); +const dbUpdate = vi.mocked(db.update); const basePayload = { userId: "user-1", @@ -145,13 +146,29 @@ describe("findOAuthApplicationByClientId", () => { describe("token hygiene", () => { // mockClear, not mockReset: the setup's `db.delete` implementation returns // the query chain and must survive between cases. - beforeEach(() => dbDelete.mockClear()); + beforeEach(() => { + dbDelete.mockClear(); + dbUpdate.mockClear(); + }); + + it("consumeRotatedRefreshToken ignores an empty token", async () => { + await consumeRotatedRefreshToken(""); + expect(dbUpdate).not.toHaveBeenCalled(); + expect(dbDelete).not.toHaveBeenCalled(); + }); - it("deleteConsumedRefreshToken skips an empty token and deletes a real one", async () => { - await deleteConsumedRefreshToken(""); + // The default grace window keeps the row alive but clamps its expiry, so a + // client retrying a dropped refresh response still succeeds. + it("consumeRotatedRefreshToken clamps the row instead of deleting it", async () => { + await consumeRotatedRefreshToken("refresh-1"); + expect(dbUpdate).toHaveBeenCalledTimes(1); expect(dbDelete).not.toHaveBeenCalled(); - await deleteConsumedRefreshToken("refresh-1"); + }); + + it("consumeRotatedRefreshToken deletes outright when the grace period is 0", async () => { + await consumeRotatedRefreshToken("refresh-1", 0); expect(dbDelete).toHaveBeenCalledTimes(1); + expect(dbUpdate).not.toHaveBeenCalled(); }); it("purgeExpiredMcpTokens deletes dead tokens and abandoned registrations", async () => { diff --git a/packages/server/src/lib/auth.ts b/packages/server/src/lib/auth.ts index d34cac10393..7d22870aacc 100644 --- a/packages/server/src/lib/auth.ts +++ b/packages/server/src/lib/auth.ts @@ -22,7 +22,7 @@ import { getUserByToken, } from "../services/admin"; import { - deleteConsumedRefreshToken, + consumeRotatedRefreshToken, DOKPLOY_MCP_SCOPE_IDS, evaluateMcpAuthorizeGate, evaluateMcpRegisterBody, @@ -195,7 +195,9 @@ const createBetterAuth = () => }), after: createAuthMiddleware(async (ctx) => { // Refresh rotation: the plugin inserts a new row and leaves the - // consumed refresh token alive. Delete it so it cannot be replayed. + // consumed refresh token alive for its whole remaining window. + // Clamp it to a short grace window so it cannot be replayed later + // but an in-flight retry still succeeds. if (ctx.path !== "/mcp/token") return; const rawBody = ctx.body as unknown; const body = @@ -211,7 +213,7 @@ const createBetterAuth = () => if (!succeeded) return; const consumed = body.refresh_token; if (typeof consumed === "string" && consumed) { - await deleteConsumedRefreshToken(consumed); + await consumeRotatedRefreshToken(consumed); } }), }, diff --git a/packages/server/src/services/mcp-oauth.ts b/packages/server/src/services/mcp-oauth.ts index 11ace977804..35333bf409e 100644 --- a/packages/server/src/services/mcp-oauth.ts +++ b/packages/server/src/services/mcp-oauth.ts @@ -1,6 +1,16 @@ import { createHmac, timingSafeEqual } from "node:crypto"; import type { IncomingHttpHeaders } from "node:http"; -import { and, asc, desc, eq, isNull, lt, notExists, or } from "drizzle-orm"; +import { + and, + asc, + desc, + eq, + isNull, + lt, + notExists, + or, + sql, +} from "drizzle-orm"; import { scheduleJob } from "node-schedule"; import { db } from "../db"; import { @@ -27,8 +37,9 @@ export const MCP_PLUGIN_AUTHORIZE_PATH = "/api/auth/mcp/authorize"; */ export { DOKPLOY_MCP_SCOPE_IDS, type DokployMcpScope } from "./mcp-scopes"; -const DEFAULT_ACCESS_TOKEN_HOURS = 24; -const DEFAULT_REFRESH_TOKEN_DAYS = 180; +const DEFAULT_ACCESS_TOKEN_HOURS = 24 * 30; // 30 days +const DEFAULT_REFRESH_TOKEN_DAYS = 365; +const DEFAULT_REFRESH_GRACE_SECONDS = 300; // 5 minutes /** Env reads take an explicit env object so tests never mutate process.env. */ type Env = Record; @@ -40,7 +51,15 @@ const positiveIntEnv = (env: Env, name: string, fallback: number) => { return Number.isFinite(parsed) && parsed > 0 ? parsed : fallback; }; -/** Access-token lifetime in seconds (DOKPLOY_MCP_ACCESS_TOKEN_HOURS, default 24). */ +/** Like positiveIntEnv but accepts 0, which is meaningful for the grace window. */ +const nonNegativeIntEnv = (env: Env, name: string, fallback: number) => { + const raw = env[name]; + if (!raw) return fallback; + const parsed = Number.parseInt(raw, 10); + return Number.isFinite(parsed) && parsed >= 0 ? parsed : fallback; +}; + +/** Access-token lifetime in seconds (DOKPLOY_MCP_ACCESS_TOKEN_HOURS, default 720 = 30 days). */ export const getMcpAccessTokenSeconds = (env: Env = process.env) => positiveIntEnv( env, @@ -48,7 +67,7 @@ export const getMcpAccessTokenSeconds = (env: Env = process.env) => DEFAULT_ACCESS_TOKEN_HOURS, ) * 3600; -/** Refresh-token lifetime in seconds (DOKPLOY_MCP_REFRESH_TOKEN_DAYS, default 180). Slides on every refresh. */ +/** Refresh-token lifetime in seconds (DOKPLOY_MCP_REFRESH_TOKEN_DAYS, default 365). Slides on every refresh. */ export const getMcpRefreshTokenSeconds = (env: Env = process.env) => positiveIntEnv( env, @@ -56,6 +75,17 @@ export const getMcpRefreshTokenSeconds = (env: Env = process.env) => DEFAULT_REFRESH_TOKEN_DAYS, ) * 86400; +/** + * How long a rotated refresh token stays usable after being consumed + * (DOKPLOY_MCP_REFRESH_GRACE_SECONDS, default 300). 0 revokes immediately. + */ +export const getMcpRefreshGraceSeconds = (env: Env = process.env) => + nonNegativeIntEnv( + env, + "DOKPLOY_MCP_REFRESH_GRACE_SECONDS", + DEFAULT_REFRESH_GRACE_SECONDS, + ); + /** Kill switch: DOKPLOY_MCP_DISABLED=true removes the endpoint and the purge job. */ export const isMcpDisabled = (env: Env = process.env) => env.DOKPLOY_MCP_DISABLED === "true"; @@ -354,11 +384,47 @@ export const evaluateMcpAuthorizeGate = ({ // Token hygiene // --------------------------------------------------------------------------- -/** Called after a successful refresh: the consumed refresh token must die. */ -export const deleteConsumedRefreshToken = async (refreshToken: string) => { +/** + * Called after a successful refresh. better-auth rotates by inserting a new + * row and leaving the consumed one alive for its whole remaining window, so + * the old refresh token stays replayable until something retires it. + * + * Retiring it instantly is the strictest option, but it strands clients: a + * dropped response or a racing second request leaves the client holding a + * token that no longer exists, and the only way out is a browser re-auth. + * PostHog hit exactly this with MCP clients and responded by disabling + * rotation for them outright; Google, Okta and Cognito issue non-rotating + * refresh tokens for the same reason. We keep rotation but clamp the consumed + * row to a short grace window, so a retry inside it still succeeds. + * + * LEAST() only ever shortens the row: one already expiring sooner keeps its + * own expiry, and the daily purge reaps it either way. LEAST ignores NULL, so + * a row with no recorded refresh expiry also ends up bounded by the window + * rather than staying open. Set DOKPLOY_MCP_REFRESH_GRACE_SECONDS=0 to restore + * immediate revocation. + */ +export const consumeRotatedRefreshToken = async ( + refreshToken: string, + // Injected like the env helpers above: the vitest config statically + // `define`s process.env, so tests cannot stub it. + graceSeconds: number = getMcpRefreshGraceSeconds(), +) => { if (!refreshToken) return; + + if (graceSeconds <= 0) { + await db + .delete(oauthAccessToken) + .where(eq(oauthAccessToken.refreshToken, refreshToken)); + return; + } + + const until = new Date(Date.now() + graceSeconds * 1000); await db - .delete(oauthAccessToken) + .update(oauthAccessToken) + .set({ + accessTokenExpiresAt: sql`LEAST(${oauthAccessToken.accessTokenExpiresAt}, ${until}::timestamp)`, + refreshTokenExpiresAt: sql`LEAST(${oauthAccessToken.refreshTokenExpiresAt}, ${until}::timestamp)`, + }) .where(eq(oauthAccessToken.refreshToken, refreshToken)); };