Skip to content

Commit 859f7b3

Browse files
mattzcareymattzcarey
authored
build: upgrade workers OAuth provider to 0.8.0 (cloudflare#160)
* build: upgrade workers OAuth provider to 0.8.0 * refactor: revoke exact failed OAuth grant
1 parent 019abcf commit 859f7b3

4 files changed

Lines changed: 38 additions & 130 deletions

File tree

‎package-lock.json‎

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
"seed:prod": "tsx scripts/seed-r2.ts production"
2020
},
2121
"dependencies": {
22-
"@cloudflare/workers-oauth-provider": "^0.6.0",
22+
"@cloudflare/workers-oauth-provider": "^0.8.0",
2323
"@modelcontextprotocol/sdk": "^1.26.0",
2424
"hono": "^4.12.25",
2525
"zod": "^4.3.5"

‎src/auth/oauth-handler.ts‎

Lines changed: 6 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,8 @@ export interface RefreshGuardContext {
102102
userId?: string
103103
/** Downstream OAuth client id (from the token-exchange callback options). */
104104
clientId?: string
105+
/** Exact downstream grant being refreshed. */
106+
grantId?: string
105107
/**
106108
* Lazily builds OAuth helpers (via `getOAuthApi`). Only invoked on a terminal
107109
* `invalid_grant` so we don't construct the provider on every refresh.
@@ -134,35 +136,6 @@ function logRefreshTelemetry(event: {
134136
console.error(`[refresh-telemetry] ${JSON.stringify({ ...event, at: Date.now() })}`)
135137
}
136138

137-
/**
138-
* Kill every grant for this user+client. `completeAuthorization` revokes prior
139-
* grants for the same user+client by default, so in practice there is at most
140-
* one, but we loop defensively (and paginate). `revokeGrant` deletes all access
141-
* tokens for the grant and the grant record itself, which invalidates the
142-
* downstream refresh token too.
143-
*/
144-
async function revokeGrantsForClient(
145-
helpers: OAuthHelpers,
146-
userId: string,
147-
clientId: string
148-
): Promise<number> {
149-
let revoked = 0
150-
let cursor: string | undefined
151-
do {
152-
const page = await helpers.listUserGrants(userId, cursor ? { cursor } : undefined)
153-
for (const grant of page.items) {
154-
// Match on client AND user. listUserGrants is expected to scope by userId,
155-
// but double-check defensively so a provider bug can never let us revoke a
156-
// different user's grant for the same clientId.
157-
if (grant.clientId !== clientId || grant.userId !== userId) continue
158-
await helpers.revokeGrant(grant.id, userId)
159-
revoked++
160-
}
161-
cursor = page.cursor
162-
} while (cursor)
163-
return revoked
164-
}
165-
166139
async function getCachedRefreshFailure(
167140
kv: KVNamespace,
168141
failureKey: string
@@ -305,15 +278,12 @@ export async function guardRefreshTokenExchange(
305278
if (
306279
error.code === 'invalid_grant' &&
307280
context.userId &&
308-
context.clientId &&
281+
context.grantId &&
309282
context.getHelpers
310283
) {
311284
try {
312-
grantsRevoked = await revokeGrantsForClient(
313-
context.getHelpers(),
314-
context.userId,
315-
context.clientId
316-
)
285+
await context.getHelpers().revokeGrant(context.grantId, context.userId)
286+
grantsRevoked = 1
317287
} catch (revokeError) {
318288
console.error(
319289
'Refresh guard: failed to revoke grant after invalid_grant',
@@ -548,6 +518,7 @@ export async function handleTokenExchangeCallback(
548518
{
549519
userId: options.userId,
550520
clientId: options.clientId,
521+
grantId: options.grantId,
551522
getHelpers
552523
}
553524
)

‎tests/auth/oauth-handler.test.ts‎

Lines changed: 27 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {
2+
GrantType,
23
OAuthError as ProviderOAuthError,
34
type OAuthHelpers
45
} from '@cloudflare/workers-oauth-provider'
@@ -47,20 +48,10 @@ function deferred<T>(): {
4748
return { promise, resolve, reject }
4849
}
4950

50-
interface MockGrant {
51-
id: string
52-
clientId: string
53-
userId: string
54-
}
55-
56-
/**
57-
* Minimal OAuthHelpers mock backing the revoke-on-invalid_grant path. Only
58-
* listUserGrants/revokeGrant are exercised; cast to the full type since the
59-
* guard never touches the other members.
60-
*/
61-
function mockOAuthHelpers(grants: MockGrant[]) {
51+
/** Minimal OAuthHelpers mock backing the revoke-on-invalid_grant path. */
52+
function mockOAuthHelpers() {
6253
return {
63-
listUserGrants: vi.fn(async () => ({ items: grants as never[], cursor: undefined })),
54+
listUserGrants: vi.fn(),
6455
revokeGrant: vi.fn(async () => undefined)
6556
} as unknown as OAuthHelpers & {
6657
listUserGrants: ReturnType<typeof vi.fn>
@@ -510,29 +501,26 @@ describe('guardRefreshTokenExchange', () => {
510501
expect(refreshFn).toHaveBeenCalledTimes(1)
511502
})
512503

513-
it('revokes the grant for this user+client on upstream invalid_grant', async () => {
504+
it('revokes the exact grant on upstream invalid_grant', async () => {
514505
const kv = env.OAUTH_KV
515506
const refreshFn = vi
516507
.fn()
517508
.mockRejectedValueOnce(new OAuthError('invalid_grant', 'refresh token reused', 400))
518-
const helpers = mockOAuthHelpers([
519-
{ id: 'grant-keep', clientId: 'other-client', userId: 'user-1' },
520-
{ id: 'grant-kill', clientId: 'mcp-client', userId: 'user-1' }
521-
])
509+
const helpers = mockOAuthHelpers()
522510
const getHelpers = vi.fn(() => helpers)
523511

524512
await expectOAuthError(
525513
guardRefreshTokenExchange(kv, 'dead-token', refreshFn, {
526514
userId: 'user-1',
527515
clientId: 'mcp-client',
516+
grantId: 'grant-kill',
528517
getHelpers
529518
}),
530519
'invalid_grant',
531520
400
532521
)
533522

534-
// Only the matching user+client grant is killed; other clients untouched.
535-
expect(helpers.listUserGrants).toHaveBeenCalledWith('user-1', undefined)
523+
expect(helpers.listUserGrants).not.toHaveBeenCalled()
536524
expect(helpers.revokeGrant).toHaveBeenCalledTimes(1)
537525
expect(helpers.revokeGrant).toHaveBeenCalledWith('grant-kill', 'user-1')
538526
})
@@ -544,7 +532,7 @@ describe('guardRefreshTokenExchange', () => {
544532
.mockRejectedValueOnce(
545533
new OAuthError('temporarily_unavailable', 'rate limited', 429, { 'Retry-After': '30' })
546534
)
547-
const helpers = mockOAuthHelpers([{ id: 'grant-1', clientId: 'mcp-client', userId: 'user-1' }])
535+
const helpers = mockOAuthHelpers()
548536
const getHelpers = vi.fn(() => helpers)
549537

550538
await expectOAuthError(
@@ -566,7 +554,7 @@ describe('guardRefreshTokenExchange', () => {
566554
const refreshFn = vi
567555
.fn()
568556
.mockRejectedValueOnce(new OAuthError('invalid_client', 'bad client creds', 401))
569-
const helpers = mockOAuthHelpers([{ id: 'grant-1', clientId: 'mcp-client', userId: 'user-1' }])
557+
const helpers = mockOAuthHelpers()
570558
const getHelpers = vi.fn(() => helpers)
571559

572560
await expectOAuthError(
@@ -588,99 +576,45 @@ describe('guardRefreshTokenExchange', () => {
588576
const refreshFn = vi
589577
.fn()
590578
.mockRejectedValueOnce(new OAuthError('invalid_grant', 'refresh token reused', 400))
591-
const helpers = mockOAuthHelpers([
592-
{ id: 'grant-kill', clientId: 'mcp-client', userId: 'user-1' }
593-
])
579+
const helpers = mockOAuthHelpers()
594580
vi.mocked(helpers.revokeGrant).mockRejectedValueOnce(new Error('KV unavailable'))
595581

596582
await expectOAuthError(
597583
guardRefreshTokenExchange(kv, 'dead-token-revoke-fails', refreshFn, {
598584
userId: 'user-1',
599585
clientId: 'mcp-client',
586+
grantId: 'grant-kill',
600587
getHelpers: () => helpers
601588
}),
602589
'invalid_grant',
603590
400
604591
)
605592
})
606-
607-
it('paginates listUserGrants when revoking', async () => {
608-
const kv = env.OAUTH_KV
609-
const refreshFn = vi
610-
.fn()
611-
.mockRejectedValueOnce(new OAuthError('invalid_grant', 'refresh token reused', 400))
612-
const helpers = mockOAuthHelpers([])
613-
vi.mocked(helpers.listUserGrants)
614-
.mockResolvedValueOnce({
615-
items: [{ id: 'grant-a', clientId: 'mcp-client', userId: 'user-1' } as never],
616-
cursor: 'next'
617-
} as never)
618-
.mockResolvedValueOnce({
619-
items: [{ id: 'grant-b', clientId: 'mcp-client', userId: 'user-1' } as never],
620-
cursor: undefined
621-
} as never)
622-
623-
await expectOAuthError(
624-
guardRefreshTokenExchange(kv, 'paginated-token', refreshFn, {
625-
userId: 'user-1',
626-
clientId: 'mcp-client',
627-
getHelpers: () => helpers
628-
}),
629-
'invalid_grant',
630-
400
631-
)
632-
633-
expect(helpers.listUserGrants).toHaveBeenCalledTimes(2)
634-
expect(helpers.revokeGrant).toHaveBeenCalledWith('grant-a', 'user-1')
635-
expect(helpers.revokeGrant).toHaveBeenCalledWith('grant-b', 'user-1')
636-
})
637-
638-
it('never revokes another user grant for the same client (defense-in-depth)', async () => {
639-
const kv = env.OAUTH_KV
640-
const refreshFn = vi
641-
.fn()
642-
.mockRejectedValueOnce(new OAuthError('invalid_grant', 'refresh token reused', 400))
643-
// listUserGrants is supposed to scope by userId, but simulate a provider
644-
// returning a same-client grant belonging to a DIFFERENT user. We must not
645-
// revoke it — only the calling user's matching grant.
646-
const helpers = mockOAuthHelpers([
647-
{ id: 'grant-mine', clientId: 'mcp-client', userId: 'user-1' },
648-
{ id: 'grant-other-user', clientId: 'mcp-client', userId: 'user-2' }
649-
])
650-
651-
await expectOAuthError(
652-
guardRefreshTokenExchange(kv, 'cross-user-token', refreshFn, {
653-
userId: 'user-1',
654-
clientId: 'mcp-client',
655-
getHelpers: () => helpers
656-
}),
657-
'invalid_grant',
658-
400
659-
)
660-
661-
expect(helpers.revokeGrant).toHaveBeenCalledTimes(1)
662-
expect(helpers.revokeGrant).toHaveBeenCalledWith('grant-mine', 'user-1')
663-
expect(helpers.revokeGrant).not.toHaveBeenCalledWith('grant-other-user', 'user-1')
664-
})
665593
})
666594

667595
describe('handleTokenExchangeCallback', () => {
668596
const OAUTH_TOKEN_URL = 'https://dash.cloudflare.com/oauth2/token'
669597

670-
const refreshCallback = (refreshToken = 'old-refresh-token') =>
598+
const refreshCallback = (refreshToken = 'old-refresh-token', getHelpers?: () => OAuthHelpers) =>
671599
handleTokenExchangeCallback(
672600
{
673-
grantType: 'refresh_token',
601+
grantType: GrantType.REFRESH_TOKEN,
602+
clientId: 'mcp-client',
603+
userId: 'user-1',
604+
grantId: 'grant-exact',
605+
scope: [],
606+
requestedScope: [],
674607
props: {
675608
type: 'user_token',
676609
accessToken: 'old-access-token',
677610
user: { id: 'user-1', email: 'user@example.com' },
678611
accounts: [{ id: 'account-1', name: 'Account 1' }],
679612
refreshToken
680613
}
681-
} as never,
614+
},
682615
'client-id',
683-
'client-secret'
616+
'client-secret',
617+
getHelpers
684618
)
685619

686620
it('refreshes upstream tokens and returns updated auth props', async () => {
@@ -715,17 +649,20 @@ describe('handleTokenExchangeCallback', () => {
715649
expect(form?.get('refresh_token')).toBe('old-refresh-token')
716650
})
717651

718-
it('throws the local OAuthError that extends the provider OAuthError', async () => {
652+
it('revokes the callback grant when upstream returns invalid_grant', async () => {
719653
// Upstream 400 -> real refreshAuthToken maps it to invalid_grant.
720654
server.use(
721655
http.post(OAUTH_TOKEN_URL, () => HttpResponse.text('invalid grant', { status: 400 }))
722656
)
657+
const helpers = mockOAuthHelpers()
723658

724-
await expect(refreshCallback()).rejects.toMatchObject({
659+
await expect(refreshCallback('old-refresh-token', () => helpers)).rejects.toMatchObject({
725660
name: 'OAuthError',
726661
code: 'invalid_grant',
727662
statusCode: 400
728663
})
664+
expect(helpers.listUserGrants).not.toHaveBeenCalled()
665+
expect(helpers.revokeGrant).toHaveBeenCalledWith('grant-exact', 'user-1')
729666
})
730667

731668
it('preserves Retry-After on a local in-flight collision (429)', async () => {

0 commit comments

Comments
 (0)