Skip to content

Commit 98d3a44

Browse files
dcramercodexclaude
authored
fix(cloudflare): Stabilize MCP OAuth refresh reuse (#882)
Tighten the Cloudflare MCP OAuth refresh path so we stop logging users out when we can still safely reuse the upstream Sentry token. The main behavioral change is to distinguish locally valid cached tokens, probe-validated cached tokens, definitively invalid upstream tokens, and indeterminate verification failures. Legacy grants without a refresh token remain usable while the embedded upstream access token still works instead of being revoked immediately. This also expands the MCP OAuth coverage around `/oauth/callback`, `/oauth/token`, and `/mcp`, and upgrades the Cloudflare worker test stack to the current `vitest-pool-workers` / Wrangler APIs. That upgrade makes the previous opaque local startup failure explicit, but this Ubuntu 20.04 WSL environment still cannot run the worker pool because newer `workerd` now requires a newer glibc. CI on `ubuntu-latest` is the intended verification path for these tests. I did not add a separate workflow because the existing workspace `test:ci` job already runs `@sentry/mcp-cloudflare`. --------- Co-authored-by: OpenAI Codex <noreply@openai.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent fe1d7ae commit 98d3a44

21 files changed

Lines changed: 1206 additions & 609 deletions

‎docs/cloudflare/oauth-architecture.md‎

Lines changed: 43 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -63,13 +63,16 @@ sequenceDiagram
6363
6464
Note over Client,SentryAPI: Token Refresh
6565
Client->>MCPOAuth: POST /oauth/token<br/>(MCP refresh_token)
66-
MCPOAuth->>MCPOAuth: Check Sentry token expiry
67-
alt Sentry token still valid
66+
MCPOAuth->>MCPOAuth: Check cached Sentry token expiry
67+
alt Sentry token still valid locally
6868
MCPOAuth-->>Client: New MCP token<br/>(reusing cached Sentry token)
69-
else Sentry token expired
70-
MCPOAuth->>SentryOAuth: Refresh Sentry token
71-
SentryOAuth-->>MCPOAuth: New Sentry tokens
72-
MCPOAuth-->>Client: New MCP token<br/>(with new Sentry tokens)
69+
else Local expiry uncertain
70+
MCPOAuth->>SentryAPI: Probe with cached Sentry token
71+
alt Probe succeeds
72+
MCPOAuth-->>Client: New MCP token<br/>(reusing cached Sentry token)
73+
else Probe fails or token invalid
74+
MCPOAuth-->>Client: Re-authentication required
75+
end
7376
end
7477
```
7578

@@ -111,7 +114,7 @@ The integration with Sentry OAuth happens through:
111114
1. **Authorization redirect** - After MCP consent, redirect to Sentry OAuth
112115
2. **Code exchange** - Exchange Sentry auth code for tokens
113116
3. **Token storage** - Store Sentry tokens in MCP token props
114-
4. **Token refresh** - Use Sentry refresh tokens to get new access tokens
117+
4. **Token reuse on refresh** - Re-issue MCP tokens while the cached Sentry access token is still usable
115118

116119
## Key Concepts
117120

@@ -248,22 +251,27 @@ const { redirectTo } = await c.env.OAUTH_PROVIDER.completeAuthorization({
248251

249252
## Token Refresh Implementation
250253

251-
### Dual Refresh Token System
254+
### MCP Refresh Model
252255

253-
The system maintains two separate refresh flows:
256+
The system only refreshes MCP tokens. It does not rotate upstream Sentry OAuth
257+
tokens anymore.
254258

255-
1. **MCP Token Refresh**: When MCP clients need new MCP access tokens
256-
2. **Sentry Token Refresh**: When Sentry access tokens expire (handled internally)
259+
1. **MCP Token Refresh**: MCP clients exchange an MCP refresh token for a new MCP access token
260+
2. **Sentry Token Reuse**: The worker keeps reusing the cached Sentry access token while it is still valid
261+
3. **Stale grant rejection**: Grants missing required stored props like `refreshToken` are treated as stale and revoked
262+
4. **Re-auth on expiry**: Once the cached Sentry access token is no longer usable, the client must complete OAuth again
257263

258264
### MCP Token Refresh Flow
259265

260266
When an MCP client's token expires:
261267

262268
1. Client sends refresh request to MCP OAuth: `POST /oauth/token` with MCP refresh token
263269
2. MCP OAuth invokes `tokenExchangeCallback` function
264-
3. Callback checks if cached Sentry token is still valid (with 2-minute safety window)
265-
4. If Sentry token is valid, returns new MCP token with cached Sentry token
266-
5. If Sentry token expired, refreshes with Sentry OAuth and updates storage
270+
3. Callback checks if cached Sentry token is still valid (with a 2-minute safety window)
271+
4. If the local expiry is still safely in the future, it returns a new MCP token immediately
272+
5. If the local expiry is stale or near expiry, it probes Sentry with the cached access token
273+
6. If the probe succeeds, it returns a new MCP token using the same cached Sentry token
274+
7. If the probe shows the token is invalid, or the worker cannot verify validity, the client must re-authenticate
267275

268276
### Token Exchange Callback Implementation
269277

@@ -275,10 +283,8 @@ export async function tokenExchangeCallback(options, env) {
275283
return undefined;
276284
}
277285

278-
// Extract Sentry refresh token from MCP token props
279-
const sentryRefreshToken = options.props.refreshToken;
280-
if (!sentryRefreshToken) {
281-
throw new Error("No Sentry refresh token available in stored props");
286+
if (!options.props.refreshToken) {
287+
return undefined;
282288
}
283289

284290
// Smart caching: Check if Sentry token is still valid
@@ -296,40 +302,33 @@ export async function tokenExchangeCallback(options, env) {
296302
}
297303
}
298304

299-
// Sentry token expired - refresh with Sentry OAuth
300-
const [sentryTokens, errorResponse] = await refreshAccessToken({
301-
client_id: env.SENTRY_CLIENT_ID,
302-
client_secret: env.SENTRY_CLIENT_SECRET,
303-
refresh_token: sentryRefreshToken,
304-
upstream_url: "https://sentry.io/oauth/token/",
305+
// Local expiry is not enough to trust the token, so probe upstream.
306+
const api = new SentryApiService({
307+
accessToken: props.accessToken,
308+
host: env.SENTRY_HOST || "sentry.io",
305309
});
310+
await api.getAuthenticatedUser();
306311

307-
// Update MCP token props with new Sentry tokens
308312
return {
309-
newProps: {
310-
...options.props,
311-
accessToken: sentryTokens.access_token, // New Sentry access token
312-
refreshToken: sentryTokens.refresh_token, // New Sentry refresh token
313-
accessTokenExpiresAt: Date.now() + sentryTokens.expires_in * 1000,
314-
},
315-
accessTokenTTL: sentryTokens.expires_in,
313+
newProps: { ...options.props },
314+
accessTokenTTL: 60 * 60,
316315
};
317316
}
318317
```
319318

320319
### Error Scenarios
321320

322-
1. **Missing Sentry Refresh Token**:
323-
- Error: "No Sentry refresh token available in stored props"
324-
- Resolution: Client must re-authenticate through full OAuth flow
321+
1. **Legacy grant missing Sentry refresh token**:
322+
- Behavior: The refresh exchange immediately stops returning new MCP tokens
323+
- Resolution: The next `/mcp` request revokes the stale grant and requires a clean re-authentication flow
325324

326-
2. **Sentry Refresh Token Invalid**:
327-
- Error: Sentry OAuth returns 401/400
328-
- Resolution: Client must re-authenticate with both MCP and Sentry
325+
2. **Cached Sentry token invalid**:
326+
- Error: Sentry probe returns 400/401
327+
- Resolution: Client must re-authenticate with MCP and Sentry
329328

330-
3. **Network Failures**:
331-
- Error: Cannot reach Sentry OAuth endpoint
332-
- Resolution: Retry with exponential backoff or re-authenticate
329+
3. **Verification indeterminate**:
330+
- Error: Network failure, timeout, or upstream 5xx while probing validity
331+
- Resolution: The current implementation fails closed and requires re-authentication
333332

334333
The 2-minute safety window prevents edge cases with clock skew and processing delays between MCP and Sentry.
335334

@@ -340,7 +339,7 @@ The 2-minute safety window prevents edge cases with clock skew and processing de
340339
3. **Dual consent**: Users approve both MCP permissions and Sentry access
341340
4. **Scope enforcement**: Both MCP and Sentry scopes limit access
342341
5. **Token expiration**: Both MCP and Sentry tokens have expiry times
343-
6. **Refresh token rotation**: Sentry issues new refresh tokens on each refresh
342+
6. **Fail-closed verification**: If the worker cannot verify token validity confidently, it requires re-authentication rather than extending access speculatively
344343

345344
## Discovery Endpoints
346345

@@ -376,14 +375,14 @@ The MCP Server then uses the Sentry access token from context to make Sentry API
376375
### Benefits of the Dual OAuth Approach
377376

378377
1. **Security isolation**: MCP clients never see Sentry tokens directly
379-
2. **Token management**: MCP can refresh Sentry tokens transparently
378+
2. **Token management**: MCP can re-issue its own tokens while reusing cached Sentry credentials
380379
3. **Permission layering**: MCP permissions separate from Sentry API scopes
381380
4. **Client flexibility**: MCP clients don't need to understand Sentry OAuth
382381

383382
### Why Not Direct Sentry OAuth?
384383

385384
If MCP clients used Sentry OAuth directly:
386-
- Clients would need to manage Sentry token refresh
385+
- Clients would need to manage Sentry token lifetime and re-authentication directly
387386
- No way to add MCP-specific permissions
388387
- Clients would have raw Sentry API access (security risk)
389388
- No centralized token management

‎docs/releases/cloudflare.md‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -98,19 +98,27 @@ const mcpHandler: ExportedHandler<Env> = {
9898

9999
## OAuth Provider Setup
100100

101-
Configure the OAuth provider with required scopes:
101+
Configure the OAuth provider with required scopes and MCP token refresh
102+
settings:
102103

103104
```typescript
104105
const oAuthProvider = new OAuthProvider({
105-
clientId: env.SENTRY_CLIENT_ID,
106-
clientSecret: env.SENTRY_CLIENT_SECRET,
107-
oauthUrl: `https://${env.SENTRY_HOST}/api/0/authorize/`,
108-
tokenUrl: `https://${env.SENTRY_HOST}/api/0/token/`,
109-
redirectUrl: `${new URL(request.url).origin}/auth/sentry/callback`,
110-
scope: ["org:read", "project:read", "issue:read", "issue:write"]
106+
apiRoute: "/mcp",
107+
apiHandler: sentryMcpHandler,
108+
defaultHandler: app,
109+
authorizeEndpoint: "/oauth/authorize",
110+
tokenEndpoint: "/oauth/token",
111+
clientRegistrationEndpoint: "/oauth/register",
112+
tokenExchangeCallback: (options) => tokenExchangeCallback(options, env),
113+
scopesSupported: Object.keys(SCOPES),
114+
refreshTokenTTL: 30 * 24 * 60 * 60,
111115
});
112116
```
113117

118+
`tokenExchangeCallback` does not refresh upstream Sentry OAuth tokens. It
119+
re-issues MCP access tokens while the cached Sentry access token is still
120+
usable, and otherwise requires the client to re-authenticate.
121+
114122
## Deployment Commands
115123

116124
### Local Development

‎docs/security.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ MCP Client → MCP Server → Sentry OAuth → Sentry API
2525

2626
3. **Token Management**
2727
- Access tokens encrypted in KV storage
28+
- MCP refresh reuses cached Sentry access tokens while they remain valid
2829
- Tokens scoped to organizations
2930

3031
## Implementation Patterns

‎packages/mcp-cloudflare/package.json‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,7 @@
44
"private": true,
55
"type": "module",
66
"license": "FSL-1.1-ALv2",
7-
"files": [
8-
"./dist/*"
9-
],
7+
"files": ["./dist/*"],
108
"exports": {
119
".": {
1210
"types": "./dist/index.ts",
@@ -43,7 +41,8 @@
4341
"urlpattern-polyfill": "^10.1.0",
4442
"vite": "catalog:",
4543
"vitest": "catalog:",
46-
"wrangler": "4.59.2"
44+
"miniflare": "catalog:",
45+
"wrangler": "4.80.0"
4746
},
4847
"dependencies": {
4948
"@ai-sdk/mcp": "catalog:",

‎packages/mcp-cloudflare/src/server/index.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@ const {
99
mockCheckRateLimit,
1010
} = vi.hoisted(() => {
1111
const mockOAuthProviderFetch = vi.fn();
12-
const MockOAuthProvider = vi
13-
.fn()
14-
.mockImplementation(() => ({ fetch: mockOAuthProviderFetch }));
12+
const MockOAuthProvider = vi.fn(function MockOAuthProvider() {
13+
return { fetch: mockOAuthProviderFetch };
14+
});
1515
const mockGetClientIp = vi.fn<(request: Request) => string | null>(
1616
() => null,
1717
);

‎packages/mcp-cloudflare/src/server/lib/mcp-handler.test.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,6 @@ describe("MCP Handler", () => {
134134
expect(response.headers.get("WWW-Authenticate")).toContain(
135135
"invalid_token",
136136
);
137-
// Revocation is dispatched via ctx.waitUntil — verify it was scheduled
138137
expect(ctx.waitUntil).toHaveBeenCalled();
139138
});
140139

‎packages/mcp-cloudflare/src/server/lib/mcp-handler.ts‎

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import type { ServerContext } from "@sentry/mcp-core/types";
1616
import { createMcpHandler } from "agents/mcp";
1717
import { CfWorkerJsonSchemaValidator } from "@modelcontextprotocol/sdk/validation/cfworker";
1818
import * as Sentry from "@sentry/cloudflare";
19+
import type { WorkerProps } from "../types";
1920
import type { Env } from "../types";
2021
import {
2122
checkRateLimit,
@@ -31,6 +32,14 @@ type OAuthExecutionContext = ExecutionContext & {
3132
props?: Record<string, unknown>;
3233
};
3334

35+
function escapeAuthenticateHeaderValue(value: string): string {
36+
return value
37+
.replaceAll("\\", "\\\\")
38+
.replaceAll('"', '\\"')
39+
.replaceAll("\r", "")
40+
.replaceAll("\n", "");
41+
}
42+
3443
/**
3544
* Revokes the OAuth grant for the given user/client pair in the background,
3645
* then returns a 401 response prompting re-authorization.
@@ -41,6 +50,7 @@ function revokeStaleGrant(
4150
userId: string,
4251
clientId: string,
4352
logLabel: string,
53+
errorDescription = "Token requires re-authorization",
4454
): Response {
4555
ctx.waitUntil(
4656
(async () => {
@@ -63,8 +73,7 @@ function revokeStaleGrant(
6373
{
6474
status: 401,
6575
headers: {
66-
"WWW-Authenticate":
67-
'Bearer realm="Sentry MCP", error="invalid_token", error_description="Token requires re-authorization"',
76+
"WWW-Authenticate": `Bearer realm="Sentry MCP", error="invalid_token", error_description="${escapeAuthenticateHeaderValue(errorDescription)}"`,
6877
},
6978
},
7079
);
@@ -113,14 +122,16 @@ const mcpHandler: ExportedHandler<Env> = {
113122
throw new Error("No authentication context available");
114123
}
115124

116-
const userId = oauthCtx.props.id as string;
117-
const accessToken = oauthCtx.props.accessToken as string;
118-
const clientId = oauthCtx.props.clientId as string;
125+
const rawProps = oauthCtx.props as Partial<WorkerProps>;
126+
127+
const userId = rawProps.id as string;
128+
const accessToken = rawProps.accessToken as string;
129+
const clientId = rawProps.clientId as string;
119130
const sentryHost = env.SENTRY_HOST || "sentry.io";
120131

121132
// Parse and validate granted skills (primary authorization method)
122133
// Legacy tokens without grantedSkills are no longer supported
123-
if (!oauthCtx.props.grantedSkills) {
134+
if (!rawProps.grantedSkills) {
124135
logWarn("Legacy token without grantedSkills detected - revoking grant", {
125136
loggerScope: ["cloudflare", "mcp-handler"],
126137
extra: { clientId, userId },
@@ -130,7 +141,7 @@ const mcpHandler: ExportedHandler<Env> = {
130141

131142
// Grants created before refreshToken was stored in props are stale and
132143
// can no longer be silently refreshed. Revoke and force clean re-auth.
133-
if (!oauthCtx.props.refreshToken) {
144+
if (!rawProps.refreshToken) {
134145
Sentry.metrics.count("mcp.oauth.grant_revoked", 1, {
135146
attributes: { reason: "missing_refresh_token" },
136147
});
@@ -143,8 +154,22 @@ const mcpHandler: ExportedHandler<Env> = {
143154
);
144155
}
145156

157+
if (rawProps.upstreamTokenInvalid) {
158+
Sentry.metrics.count("mcp.oauth.grant_revoked", 1, {
159+
attributes: { reason: "invalid_upstream_token" },
160+
});
161+
return revokeStaleGrant(
162+
ctx,
163+
env,
164+
userId,
165+
clientId,
166+
"stale grant (invalid upstream token)",
167+
"Upstream authorization is no longer valid",
168+
);
169+
}
170+
146171
const { valid: validSkills, invalid: invalidSkills } = parseSkills(
147-
oauthCtx.props.grantedSkills as string[],
172+
rawProps.grantedSkills as string[],
148173
);
149174

150175
if (invalidSkills.length > 0) {
@@ -161,11 +186,11 @@ const mcpHandler: ExportedHandler<Env> = {
161186
logWarn("Authorization rejected: No valid skills in token", {
162187
loggerScope: ["cloudflare", "mcp-handler"],
163188
extra: {
164-
clientId: oauthCtx.props.clientId,
165-
userId: oauthCtx.props.id,
166-
rawGrantedSkills: oauthCtx.props.grantedSkills,
167-
rawGrantedSkillsType: typeof oauthCtx.props.grantedSkills,
168-
rawGrantedSkillsIsArray: Array.isArray(oauthCtx.props.grantedSkills),
189+
clientId,
190+
userId: rawProps.id,
191+
rawGrantedSkills: rawProps.grantedSkills,
192+
rawGrantedSkillsType: typeof rawProps.grantedSkills,
193+
rawGrantedSkillsIsArray: Array.isArray(rawProps.grantedSkills),
169194
},
170195
});
171196
return new Response(

0 commit comments

Comments
 (0)