mirror of
https://github.com/supermemoryai/supermemory.git
synced 2026-08-28 05:25:33 +00:00
fix(mcp): return 503 on auth-backend outages instead of invalid_token (#1591)
Cherry-picks #1587 from @Sravanjangam, plus the OAuth half on top. When the auth backend is slow or returns a 5xx, the MCP server currently answers `invalid_token`. That is the protocol's signal to discard the credential and re-authenticate, so a brief upstream blip logs every connected client out, and `sm_` API key users have no automatic way back. These requests now return 503 with `Retry-After: 5` so clients retry instead. His change covered the API key path only. This shares one `transientAuthErrorFor` helper between `validateApiKey` and `validateOAuthToken`, so a JWKS timeout or a 5xx also returns 503 on the OAuth path that Claude, Cursor and browser clients use. Genuinely bad tokens are unaffected: bad signature, expired, and no-matching-key still resolve to 401. Verified across all seven cases. Co-Authored-By: Sravanjangam <163002695+Sravanjangam@users.noreply.github.com>
This commit is contained in:
parent
3b0fc9c959
commit
6cae175852
3 changed files with 121 additions and 6 deletions
|
|
@ -1,6 +1,11 @@
|
|||
import { createLocalJWKSet, exportJWK, generateKeyPair, SignJWT } from "jose"
|
||||
import { afterEach, beforeAll, describe, expect, it, vi } from "vitest"
|
||||
import { fetchSession, validateApiKey, validateOAuthToken } from "./index"
|
||||
import {
|
||||
fetchSession,
|
||||
TransientAuthError,
|
||||
validateApiKey,
|
||||
validateOAuthToken,
|
||||
} from "./index"
|
||||
|
||||
const API_URL = "https://api.example.com"
|
||||
const ISSUER = `${API_URL}/api/auth`
|
||||
|
|
@ -180,4 +185,32 @@ describe("MCP authentication", () => {
|
|||
await expect(validateApiKey("not_a_key", API_URL)).resolves.toBeNull()
|
||||
expect(fetchSpy).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it("surfaces a 500 from the session endpoint as TransientAuthError, not invalid token", async () => {
|
||||
vi.spyOn(console, "error").mockImplementation(() => {})
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn().mockResolvedValue(new Response(null, { status: 500 })),
|
||||
)
|
||||
|
||||
await expect(
|
||||
validateApiKey("sm_outage_key_0123456789abcdef", API_URL),
|
||||
).rejects.toThrow(TransientAuthError)
|
||||
})
|
||||
|
||||
it("surfaces a session-endpoint timeout as TransientAuthError", async () => {
|
||||
vi.spyOn(console, "error").mockImplementation(() => {})
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn().mockRejectedValue(
|
||||
Object.assign(new Error("The operation was aborted"), {
|
||||
name: "TimeoutError",
|
||||
}),
|
||||
),
|
||||
)
|
||||
|
||||
await expect(
|
||||
validateApiKey("sm_timeout_key_0123456789abcd", API_URL),
|
||||
).rejects.toThrow(TransientAuthError)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -65,6 +65,46 @@ export function isApiKey(token: string): boolean {
|
|||
return API_KEY_PATTERN.test(token)
|
||||
}
|
||||
|
||||
// Upstream was unreachable, not the token being bad: reporting these as invalid_token makes clients discard working credentials.
|
||||
export class TransientAuthError extends Error {
|
||||
readonly status?: number
|
||||
|
||||
constructor(message: string, status?: number) {
|
||||
super(message)
|
||||
this.name = "TransientAuthError"
|
||||
this.status = status
|
||||
}
|
||||
}
|
||||
|
||||
const TRANSIENT_ERROR_NAMES = new Set([
|
||||
"AbortError",
|
||||
"TimeoutError",
|
||||
"JWKSTimeout",
|
||||
])
|
||||
|
||||
// ERR_JOSE_GENERIC is what jose throws when the JWKS endpoint answers non-200 or unparseable JSON.
|
||||
const TRANSIENT_JOSE_CODES = new Set(["ERR_JWKS_TIMEOUT", "ERR_JOSE_GENERIC"])
|
||||
|
||||
function transientAuthErrorFor(error: unknown): TransientAuthError | null {
|
||||
const status = (error as { status?: unknown } | null)?.status
|
||||
if (typeof status === "number" && status !== 401 && status !== 403) {
|
||||
return new TransientAuthError(`Session endpoint returned ${status}`, status)
|
||||
}
|
||||
if (error instanceof TypeError) {
|
||||
return new TransientAuthError(`Auth backend unreachable: ${error.message}`)
|
||||
}
|
||||
if (error instanceof Error && TRANSIENT_ERROR_NAMES.has(error.name)) {
|
||||
return new TransientAuthError(error.message)
|
||||
}
|
||||
const code = (error as { code?: unknown } | null)?.code
|
||||
if (typeof code === "string" && TRANSIENT_JOSE_CODES.has(code)) {
|
||||
return new TransientAuthError(
|
||||
`JWKS fetch failed: ${(error as Error).message}`,
|
||||
)
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
export async function validateApiKey(
|
||||
token: string,
|
||||
apiUrl: string,
|
||||
|
|
@ -93,6 +133,8 @@ export async function validateApiKey(
|
|||
return user
|
||||
} catch (error) {
|
||||
console.error("API key validation error:", error)
|
||||
const transient = transientAuthErrorFor(error)
|
||||
if (transient) throw transient
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
|
@ -142,6 +184,8 @@ export async function validateOAuthToken(
|
|||
}
|
||||
} catch (error) {
|
||||
console.error("OAuth token validation error:", error)
|
||||
const transient = transientAuthErrorFor(error)
|
||||
if (transient) throw transient
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import { Hono, type Context } from "hono"
|
|||
import { cors } from "hono/cors"
|
||||
import {
|
||||
isApiKey,
|
||||
TransientAuthError,
|
||||
validateApiKey,
|
||||
validateOAuthToken,
|
||||
type AuthUser,
|
||||
|
|
@ -47,7 +48,7 @@ app.use(
|
|||
allowMethods: ["GET", "POST", "DELETE", "OPTIONS"],
|
||||
// When omitted, Hono echoes Access-Control-Request-Headers. This keeps
|
||||
// modern Mcp-Method/Mcp-Name/Mcp-Param-* routing forward-compatible.
|
||||
exposeHeaders: ["WWW-Authenticate"],
|
||||
exposeHeaders: ["WWW-Authenticate", "Retry-After"],
|
||||
}),
|
||||
)
|
||||
|
||||
|
|
@ -128,6 +129,30 @@ function authInfoFor(
|
|||
}
|
||||
}
|
||||
|
||||
type AuthResolution =
|
||||
| { ok: true; user: AuthUser }
|
||||
| { ok: false; reason: "invalid" }
|
||||
| { ok: false; reason: "transient" }
|
||||
|
||||
// Keeps a transient upstream failure distinct from an invalid token.
|
||||
async function resolveAuthUser(
|
||||
token: string,
|
||||
apiUrl: string,
|
||||
mcpResource: string,
|
||||
): Promise<AuthResolution> {
|
||||
try {
|
||||
const user = isApiKey(token)
|
||||
? await validateApiKey(token, apiUrl)
|
||||
: await validateOAuthToken(token, apiUrl, mcpResource)
|
||||
return user ? { ok: true, user } : { ok: false, reason: "invalid" }
|
||||
} catch (error) {
|
||||
if (error instanceof TransientAuthError) {
|
||||
return { ok: false, reason: "transient" }
|
||||
}
|
||||
throw error
|
||||
}
|
||||
}
|
||||
|
||||
function unauthorizedResponse(
|
||||
resourceMetadataUrl: string,
|
||||
invalidToken = false,
|
||||
|
|
@ -181,10 +206,23 @@ async function handleMcpRequest(
|
|||
|
||||
if (!token) return unauthorizedResponse(resourceMetadataUrl)
|
||||
|
||||
const authUser = isApiKey(token)
|
||||
? await validateApiKey(token, apiUrl)
|
||||
: await validateOAuthToken(token, apiUrl, mcpResource)
|
||||
if (!authUser) return unauthorizedResponse(resourceMetadataUrl, true)
|
||||
const resolved = await resolveAuthUser(token, apiUrl, mcpResource)
|
||||
if (!resolved.ok && resolved.reason === "transient") {
|
||||
return Response.json(
|
||||
{
|
||||
jsonrpc: "2.0",
|
||||
error: {
|
||||
code: -32001,
|
||||
message:
|
||||
"Authentication backend temporarily unavailable, please retry",
|
||||
},
|
||||
id: null,
|
||||
},
|
||||
{ status: 503, headers: { "Retry-After": "5" } },
|
||||
)
|
||||
}
|
||||
if (!resolved.ok) return unauthorizedResponse(resourceMetadataUrl, true)
|
||||
const authUser = resolved.user
|
||||
|
||||
const actor: ActorContext = {
|
||||
userId: authUser.userId,
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue