Skip to content

Commit 0ff7e28

Browse files
committed
fix(oauth): record revoked refresh tokens durably and surface them as reconnect
A refresh token the provider revoked (invalid_grant and kin) was only flagged in Redis for an hour, so every scheduled run kept failing with a generic error logged at ERROR three times, and the provider was asked again each hour, indefinitely. - account gains refresh_revoked_at/_code/_token_hash (expand-only, nullable). The record holds only while its hash fingerprints the stored refresh token, so a writer storing a new chain supersedes it; every reconnect path also clears it explicitly, since a provider can reauthorize without issuing a new refresh token. - The record is written only on rows still holding the rejected token, so a refresh that lost a race to a newer chain cannot mark the live credential revoked. Revocations no longer use the Redis dead flag, which now holds only app-registration faults. - A recorded revocation answers without the provider; one re-probe a day lets rejections that clear without a reconnect recover. A successful refresh clears it; a re-probe that fails for a passing reason keeps it. - refreshTokenIfNeeded throws CredentialRevokedError; token resolution returns code OAUTH_CREDENTIAL_REVOKED (POST and GET) and logs it once at WARN; the executor, tool and connector sites no longer log it at ERROR.
1 parent 722510e commit 0ff7e28

22 files changed

Lines changed: 31633 additions & 54 deletions

‎apps/sim/app/api/auth/oauth/token/route.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,10 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
217217
})
218218

219219
if (!result.ok) {
220-
return NextResponse.json({ error: result.error }, { status: result.status })
220+
return NextResponse.json(
221+
{ ...(result.code ? { code: result.code } : {}), error: result.error },
222+
{ status: result.status }
223+
)
221224
}
222225

223226
return NextResponse.json(result.token, { status: 200 })

‎apps/sim/app/api/auth/oauth/utils.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ describe('OAuth Utils', () => {
153153
])
154154
mockRefreshOAuthToken.mockResolvedValueOnce({
155155
ok: false,
156-
errorCode: 'token_revoked',
156+
errorCode: 'invalid_client',
157157
})
158158
mockSelectChain([])
159159

@@ -164,7 +164,7 @@ describe('OAuth Utils', () => {
164164
const installationIdentity = getOAuthRefreshCoordinationIdentity('slack:T08CM6ZNYBE')
165165
expect(fakeRedis.set).toHaveBeenCalledWith(
166166
`oauth:dead:${installationIdentity}`,
167-
'token_revoked',
167+
'invalid_client',
168168
'EX',
169169
3600
170170
)
@@ -182,7 +182,7 @@ describe('OAuth Utils', () => {
182182
])
183183
mockRefreshOAuthToken.mockResolvedValueOnce({
184184
ok: false,
185-
errorCode: 'token_revoked',
185+
errorCode: 'invalid_client',
186186
})
187187
mockSelectChain([{ moved: new Date() }])
188188

‎apps/sim/executor/utils/credential-token.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { executeCopilotCredentialUseCase } from '@/lib/mothership/application/ex
77
import { resolveCopilotOrganizationPersonalToken } from '@/lib/mothership/application/resolve-organization-personal-token'
88
import { projectAssistantConnectedAccountTool } from '@/lib/mothership/assistant/connected-account-tool'
99
import type { CopilotExecutionContext } from '@/lib/mothership/auth/application-delegation'
10+
import { CredentialRevokedError, OAUTH_CREDENTIAL_REVOKED } from '@/lib/oauth/credential-revoked'
1011
import {
1112
type CredentialTokenPayload,
1213
resolveCredentialAccessToken,
@@ -124,14 +125,17 @@ export async function resolveExecutorCredentialToken(
124125
})
125126

126127
if (!result.ok) {
128+
const message = `Failed to obtain credential for ${params.toolLabel ?? credentialId}: ${result.error}`
129+
// The resolver already logged the revocation once, at WARN, where it was classified.
130+
if (result.code === OAUTH_CREDENTIAL_REVOKED) {
131+
throw new CredentialRevokedError(message)
132+
}
127133
logger.error(`[${requestId}] Credential token resolution failed`, {
128134
status: result.status,
129135
credentialId,
130136
code: result.code,
131137
})
132-
throw new Error(
133-
`Failed to obtain credential for ${params.toolLabel ?? credentialId}: ${result.error}`
134-
)
138+
throw new Error(message)
135139
}
136140

137141
return result.token

‎apps/sim/lib/auth/auth.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ import { validateSignupEmailMx } from '@/lib/messaging/email/validation.server'
148148
import { isEmailVerificationEffectivelyEnabled } from '@/lib/messaging/email/verification'
149149
import { scheduleLifecycleEmail } from '@/lib/messaging/lifecycle'
150150
import { APP_ENTRY_PATH } from '@/lib/navigation/paths'
151+
import { clearOAuthRefreshFailure } from '@/lib/oauth/credential-service'
151152
import {
152153
getMicrosoftRefreshTokenExpiry,
153154
isMicrosoftProvider,
@@ -701,6 +702,26 @@ export const auth = betterAuth({
701702
}
702703
},
703704
},
705+
update: {
706+
/**
707+
* Relinking an identity that already has a row updates it in place instead of creating
708+
* one, and keeps the old refresh token when the provider issues none, so the fresh
709+
* authorization has to clear the failures recorded against the old grant explicitly.
710+
*/
711+
after: async (account, context) => {
712+
const path = context?.path
713+
if (!path?.startsWith('/oauth2/callback/') && !path?.startsWith('/callback/')) return
714+
try {
715+
await clearOAuthRefreshFailure(account.id)
716+
} catch (error) {
717+
logger.error('[account.update.after] Failed to clear recorded refresh failures', {
718+
accountId: account.id,
719+
providerId: account.providerId,
720+
error,
721+
})
722+
}
723+
},
724+
},
704725
},
705726
session: {
706727
create: {

‎apps/sim/lib/credentials/draft-hooks.test.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ describe('handleCreateCredentialFromDraft', () => {
3838
queueTableRows(schemaMock.credential, [
3939
{ id: 'credential-existing', displayName: 'Existing Gmail' },
4040
])
41+
queueTableRows(schemaMock.account, [
42+
{ providerId: 'google-email', providerAccountId: 'subject' },
43+
])
4144

4245
await handleCreateCredentialFromDraft({
4346
draft: {
@@ -99,6 +102,7 @@ describe('handleReconnectCredential', () => {
99102
{ id: 'credential-1', accountId: null, displayName: 'Renamed Gmail' },
100103
])
101104
queueTableRows(schemaMock.credential, [])
105+
queueTableRows(schemaMock.account, [{ providerId: 'gmail', providerAccountId: 'subject-new' }])
102106
queueTableRows(schemaMock.account, [{ providerId: 'gmail', accountId: 'subject-new' }])
103107

104108
await handleReconnectCredential({

‎apps/sim/lib/credentials/draft-hooks.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { generateId } from '@sim/utils/id'
77
import { and, eq, sql } from 'drizzle-orm'
88
import { deleteOrphanedOAuthAccount } from '@/lib/credentials/deletion'
99
import { resumeConnectorsAfterCredentialReconnect } from '@/lib/knowledge/connectors/credential-recovery'
10-
import { clearOAuthRefreshDeadFlag } from '@/lib/oauth/refresh-coordination'
10+
import { clearOAuthRefreshFailure } from '@/lib/oauth/credential-service'
1111
import { captureServerEvent } from '@/lib/posthog/server'
1212

1313
const logger = createLogger('CredentialDraftHooks')
@@ -75,7 +75,7 @@ export async function handleCreateCredentialFromDraft(params: {
7575
.set({ updatedAt: now })
7676
.where(eq(schema.credential.id, existingCredential.id))
7777

78-
await clearOAuthRefreshDeadFlag(accountId)
78+
await clearOAuthRefreshFailure(accountId)
7979
await resumeConnectorsAfterCredentialReconnect(accountId, now)
8080

8181
recordAudit({
@@ -110,7 +110,7 @@ export async function handleCreateCredentialFromDraft(params: {
110110
accountId,
111111
})
112112

113-
await clearOAuthRefreshDeadFlag(accountId)
113+
await clearOAuthRefreshFailure(accountId)
114114

115115
recordAudit({
116116
workspaceId: draft.workspaceId,
@@ -210,7 +210,7 @@ export async function handleReconnectCredential(params: {
210210
}
211211
)
212212

213-
await clearOAuthRefreshDeadFlag(newAccountId)
213+
await clearOAuthRefreshFailure(newAccountId)
214214
await resumeConnectorsAfterCredentialReconnect(newAccountId, now)
215215

216216
recordAudit({

‎apps/sim/lib/credentials/organization-draft.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { pendingCredentialDraft } from '@sim/db/schema'
22
import { dbChainMockFns, hasMockCondition, resetDbChainMock } from '@sim/testing'
33
import { auditMock } from '@sim/testing/mocks/audit.mock'
4+
import { authOAuthUtilsMock } from '@sim/testing/mocks/auth-oauth-utils.mock'
45
import {
56
organizationMembershipMock,
67
organizationMembershipMockFns,
@@ -9,7 +10,6 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
910

1011
const hoisted = vi.hoisted(() => ({
1112
context: vi.fn(),
12-
clear: vi.fn(),
1313
deleteOrphan: vi.fn(),
1414
}))
1515
vi.mock('@sim/audit', () => auditMock)
@@ -18,7 +18,7 @@ vi.mock('@/lib/credentials/organization', () => ({
1818
getCredentialCreationOrganizationContext: hoisted.context,
1919
}))
2020
vi.mock('@/lib/credentials/deletion', () => ({ deleteOrphanedOAuthAccount: hoisted.deleteOrphan }))
21-
vi.mock('@/lib/oauth/refresh-coordination', () => ({ clearOAuthRefreshDeadFlag: hoisted.clear }))
21+
vi.mock('@/lib/oauth/credential-service', () => authOAuthUtilsMock)
2222

2323
import { completeOrganizationCredentialDraft } from '@/lib/credentials/organization-draft'
2424

‎apps/sim/lib/credentials/organization-draft.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { resourceScopeCondition } from '@/lib/core/resource-scope.server'
99
import { deleteOrphanedOAuthAccount } from '@/lib/credentials/deletion'
1010
import { getCredentialCreationOrganizationContext } from '@/lib/credentials/organization'
1111
import { resumeConnectorsAfterCredentialReconnect } from '@/lib/knowledge/connectors/credential-recovery'
12-
import { clearOAuthRefreshDeadFlag } from '@/lib/oauth/refresh-coordination'
12+
import { clearOAuthRefreshFailure } from '@/lib/oauth/credential-service'
1313

1414
/** Completes the exact draft bound to the authenticated provider callback, rechecking current ownership under membership locks. */
1515
export async function completeOrganizationCredentialDraft(input: {
@@ -124,7 +124,7 @@ export async function completeOrganizationCredentialDraft(input: {
124124
oldAccountId: existing?.accountId,
125125
}
126126
})
127-
await clearOAuthRefreshDeadFlag(input.accountId)
127+
await clearOAuthRefreshFailure(input.accountId)
128128
if (result.reconnected) await resumeConnectorsAfterCredentialReconnect(input.accountId, now)
129129
recordAudit({
130130
actorId: input.userId,

‎apps/sim/lib/knowledge/connectors/sync-engine.ts‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -819,18 +819,23 @@ async function resolveAccessToken(
819819
})
820820

821821
if (!resolved) {
822-
logger.error(`[${requestId}] Connector credential resolved no access token`, {
823-
credentialId: connector.credentialId,
824-
userId,
825-
authMode: connectorConfig.auth.mode,
826-
})
827822
const revocationError =
828823
connectorConfig.auth.mode === 'oauth' && connector.credentialId
829824
? await getCredentialRevocationError(connector.credentialId)
830825
: null
826+
const logContext = {
827+
credentialId: connector.credentialId,
828+
userId,
829+
authMode: connectorConfig.auth.mode,
830+
}
831831
if (revocationError && connector.credentialId) {
832+
logger.warn(`[${requestId}] Connector credential was revoked by the source`, {
833+
...logContext,
834+
errorCode: revocationError,
835+
})
832836
throw new ConnectorCredentialRevokedError(connector.credentialId, revocationError)
833837
}
838+
logger.error(`[${requestId}] Connector credential resolved no access token`, logContext)
834839
throw new Error(`Failed to obtain access token for credential ${connector.credentialId}`)
835840
}
836841

0 commit comments

Comments
 (0)