Skip to content

Commit 5ba954d

Browse files
committed
fix(auth): clean up replayed authorization-code grants without stranding token families
Better Auth 1.7 revokes a replayed code's tokens by authorizationCodeId outside any transaction. The adapter guard now deletes the grant's token families (cascading to its tokens) and refuses an access-token insert whose refresh token a concurrent replay already removed as invalid_grant instead of a 500.
1 parent 2120b33 commit 5ba954d

3 files changed

Lines changed: 144 additions & 18 deletions

File tree

‎apps/sim/app/api/auth/oauth2/token/route.integration.ts‎

Lines changed: 69 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -586,35 +586,89 @@ describe('OAuth token route in PostgreSQL', () => {
586586
.where(eq(schema.oauthTokenFamily.id, secondRefresh?.familyId ?? 'missing'))
587587
).toHaveLength(0)
588588

589-
const racingVerifier = `${testId}-racing-verifier-with-more-than-forty-three-characters`
590-
const racingCode = await issueAuthorizationCode(racingVerifier)
591-
const exchangeSameCode = () =>
589+
const exchangeCode = (code: string, verifier: string) =>
592590
exchangeToken(
593591
createFormRequest(
594592
'/api/auth/oauth2/token',
595593
new URLSearchParams({
596594
grant_type: 'authorization_code',
597595
client_id: provider.SIM_CLI_CLIENT_ID,
598-
code: racingCode,
599-
code_verifier: racingVerifier,
596+
code,
597+
code_verifier: verifier,
600598
redirect_uri: redirectUri,
601599
})
602600
)
603601
)
604-
const racingResponses = await Promise.all([exchangeSameCode(), exchangeSameCode()])
605-
expect(racingResponses.map((response) => response.status).sort()).toEqual([200, 400])
606-
const rejectedExchange = racingResponses.find((response) => response.status === 400)
607-
await expect(rejectedExchange?.json()).resolves.toMatchObject({ error: 'invalid_grant' })
608-
expect(
609-
await db
602+
const familyIdsForUser = () =>
603+
db
610604
.select({ id: schema.oauthTokenFamily.id })
611605
.from(schema.oauthTokenFamily)
612606
.where(eq(schema.oauthTokenFamily.userId, userId))
613-
).toHaveLength(1)
614607

615-
const activeTokens = (await racingResponses
616-
.find((response) => response.status === 200)
617-
?.json()) as TokenResponseBody
608+
const racingVerifier = `${testId}-racing-verifier-with-more-than-forty-three-characters`
609+
const racingCode = await issueAuthorizationCode(racingVerifier)
610+
const racingResponses = await Promise.all([
611+
exchangeCode(racingCode, racingVerifier),
612+
exchangeCode(racingCode, racingVerifier),
613+
])
614+
// The replay revokes whatever the first exchange has issued so far, so which of the two
615+
// succeeds depends on interleaving; neither may fail as a server error or strand a family.
616+
const racingStatuses = racingResponses.map((response) => response.status)
617+
expect(racingStatuses).toContain(400)
618+
expect(racingStatuses.every((status) => status === 200 || status === 400)).toBe(true)
619+
for (const response of racingResponses.filter(({ status }) => status === 400)) {
620+
await expect(response.json()).resolves.toMatchObject({ error: 'invalid_grant' })
621+
}
622+
const refreshFamilyIds = new Set(
623+
(
624+
await db
625+
.select({ familyId: schema.oauthRefreshToken.familyId })
626+
.from(schema.oauthRefreshToken)
627+
.where(eq(schema.oauthRefreshToken.userId, userId))
628+
).map(({ familyId }) => familyId)
629+
)
630+
expect((await familyIdsForUser()).every(({ id }) => refreshFamilyIds.has(id))).toBe(true)
631+
for (const response of racingResponses.filter(({ status }) => status === 200)) {
632+
const { refresh_token } = (await response.json()) as TokenResponseBody
633+
await revokeToken(
634+
createFormRequest(
635+
'/api/auth/oauth2/revoke',
636+
new URLSearchParams({ client_id: provider.SIM_CLI_CLIENT_ID, token: refresh_token })
637+
)
638+
)
639+
}
640+
expect(await familyIdsForUser()).toHaveLength(0)
641+
642+
const replayVerifier = `${testId}-replay-verifier-with-more-than-forty-three-characters`
643+
const replayCode = await issueAuthorizationCode(replayVerifier)
644+
const firstExchange = await exchangeCode(replayCode, replayVerifier)
645+
expect(firstExchange.status).toBe(200)
646+
const firstGrant = (await firstExchange.json()) as TokenResponseBody
647+
expect(await familyIdsForUser()).toHaveLength(1)
648+
const replayedExchange = await exchangeCode(replayCode, replayVerifier)
649+
expect(replayedExchange.status).toBe(400)
650+
await expect(replayedExchange.json()).resolves.toMatchObject({ error: 'invalid_grant' })
651+
expect(await familyIdsForUser()).toHaveLength(0)
652+
const refreshAfterReplay = await exchangeToken(
653+
createFormRequest(
654+
'/api/auth/oauth2/token',
655+
new URLSearchParams({
656+
grant_type: 'refresh_token',
657+
client_id: provider.SIM_CLI_CLIENT_ID,
658+
refresh_token: firstGrant.refresh_token,
659+
})
660+
)
661+
)
662+
expect(refreshAfterReplay.status).toBe(400)
663+
664+
const activeVerifier = `${testId}-active-verifier-with-more-than-forty-three-characters`
665+
const activeExchange = await exchangeCode(
666+
await issueAuthorizationCode(activeVerifier),
667+
activeVerifier
668+
)
669+
expect(activeExchange.status).toBe(200)
670+
const activeTokens = (await activeExchange.json()) as TokenResponseBody
671+
expect(await familyIdsForUser()).toHaveLength(1)
618672
const withheldVerifier = `${testId}-withheld-verifier-with-more-than-forty-three-characters`
619673
const withheldCode = await issueAuthorizationCode(withheldVerifier)
620674
const consentUrl = await createAuthorizeUrl(withheldVerifier)

‎apps/sim/lib/auth/oauth-provider-adapter-guard.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,26 @@ describe('guardOAuthProviderWrites', () => {
112112
expect(mocks.insert).not.toHaveBeenCalled()
113113
})
114114

115+
it('refuses an access token whose refresh token a concurrent replay revoked', async () => {
116+
const base = adapter()
117+
vi.mocked(base.create).mockRejectedValueOnce(
118+
Object.assign(new Error('Failed query'), {
119+
cause: Object.assign(new Error('violates foreign key constraint'), {
120+
code: '23503',
121+
constraint_name: 'oauth_access_token_refresh_id_oauth_refresh_token_id_fk',
122+
}),
123+
})
124+
)
125+
const guarded = guardOAuthProviderWrites(base)
126+
127+
await expect(
128+
guarded.create({
129+
model: 'oauthAccessToken',
130+
data: { userId: 'user-1', scopes: ['api:read'], refreshId: 'revoked' },
131+
})
132+
).rejects.toMatchObject({ body: { error: 'invalid_grant' } })
133+
})
134+
115135
it.each(['oauthAccessToken', 'oauthRefreshToken'])(
116136
'persists the verified audience on %s and ignores injected resource fields',
117137
async (model) => {

‎apps/sim/lib/auth/oauth-provider-adapter-guard.ts‎

Lines changed: 55 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
import { AsyncLocalStorage } from 'node:async_hooks'
22
import type { db } from '@sim/db'
3-
import { oauthConsent, oauthTokenFamily } from '@sim/db/schema'
3+
import { oauthConsent, oauthRefreshToken, oauthTokenFamily } from '@sim/db/schema'
4+
import { getPostgresConstraintName, getPostgresErrorCode } from '@sim/utils/errors'
45
import { generateId } from '@sim/utils/id'
56
import type { drizzleAdapter } from 'better-auth/adapters/drizzle'
67
import { APIError } from 'better-auth/api'
7-
import { inArray } from 'drizzle-orm'
8+
import { eq, inArray } from 'drizzle-orm'
89
import { getAuthPool } from '@/lib/auth/database-context'
910
import { getOAuthIssuedResource } from '@/lib/auth/oauth-resource'
1011
import { capabilityRefusal } from '@/lib/permission-groups/capabilities'
@@ -61,6 +62,30 @@ export async function withOAuthProviderIssuanceCompensation<T extends Response>(
6162
}
6263
}
6364

65+
const ACCESS_TOKEN_REFRESH_FK = 'oauth_access_token_refresh_id_oauth_refresh_token_id_fk'
66+
67+
/**
68+
* Better Auth revokes a replayed code's grant by deleting its tokens by `authorizationCodeId`,
69+
* which would leave Sim's token family behind. Deleting the families instead cascades to their
70+
* refresh tokens and the access tokens bound to them.
71+
*/
72+
async function deleteFamiliesIssuedForAuthorizationCode(
73+
database: AuthDatabase,
74+
authorizationCodeId: string
75+
): Promise<void> {
76+
await database
77+
.delete(oauthTokenFamily)
78+
.where(
79+
inArray(
80+
oauthTokenFamily.id,
81+
database
82+
.select({ id: oauthRefreshToken.familyId })
83+
.from(oauthRefreshToken)
84+
.where(eq(oauthRefreshToken.authorizationCodeId, authorizationCodeId))
85+
)
86+
)
87+
}
88+
6489
function requireConsentInsert(data: Record<string, unknown>): OAuthConsentInsert {
6590
if (
6691
typeof data.clientId !== 'string' ||
@@ -106,6 +131,18 @@ export function guardOAuthProviderWrites(
106131
): BetterAuthAdapter {
107132
return {
108133
...adapter,
134+
deleteMany: async (input) => {
135+
const [condition, ...rest] = input.where
136+
if (
137+
input.model === 'oauthRefreshToken' &&
138+
rest.length === 0 &&
139+
condition?.field === 'authorizationCodeId' &&
140+
typeof condition.value === 'string'
141+
) {
142+
await deleteFamiliesIssuedForAuthorizationCode(database, condition.value)
143+
}
144+
return adapter.deleteMany(input)
145+
},
109146
create: async (input) => {
110147
/**
111148
* permission-group-enforced: oauth_apps.use — recheck the canonical token
@@ -133,7 +170,22 @@ export function guardOAuthProviderWrites(
133170
}
134171

135172
if (input.model !== 'oauthConsent') {
136-
const created = await adapter.create(input)
173+
const created = await adapter.create(input).catch((error: unknown) => {
174+
// Issuance is not transactional: a concurrent replay can revoke the grant between the
175+
// refresh and access inserts, so the access insert's foreign key fails. Refuse it the
176+
// way the replay is refused rather than as a server error.
177+
if (
178+
input.model === 'oauthAccessToken' &&
179+
getPostgresErrorCode(error) === '23503' &&
180+
getPostgresConstraintName(error) === ACCESS_TOKEN_REFRESH_FK
181+
) {
182+
throw new APIError('BAD_REQUEST', {
183+
error: 'invalid_grant',
184+
error_description: 'The authorization grant was revoked.',
185+
})
186+
}
187+
throw error
188+
})
137189
if (input.model === 'oauthRefreshToken') {
138190
const id = (created as { id?: unknown }).id
139191
if (typeof id !== 'string' || !id) {

0 commit comments

Comments
 (0)