Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion apps/sim/app/api/auth/oauth/token/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,10 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
})

if (!result.ok) {
return NextResponse.json({ error: result.error }, { status: result.status })
return NextResponse.json(
{ ...(result.code ? { code: result.code } : {}), error: result.error },
{ status: result.status }
)
}

return NextResponse.json(result.token, { status: 200 })
Expand Down
14 changes: 11 additions & 3 deletions apps/sim/app/api/auth/oauth/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ describe('OAuth Utils', () => {
}

it('locks per installation and refreshes with the freshest sibling refresh token', async () => {
// The leader rereads the row under the lock.
mockSelectChain([])
mockSelectOrderedChain([
{ accessToken: 'stale-at', refreshToken: 'live-rt', accessTokenExpiresAt: past },
])
Expand Down Expand Up @@ -127,6 +129,8 @@ describe('OAuth Utils', () => {
})

it('returns the freshest sibling token without refreshing when it is still valid', async () => {
// The leader rereads the row under the lock.
mockSelectChain([])
mockSelectOrderedChain([
{ accessToken: 'sibling-at', refreshToken: 'live-rt', accessTokenExpiresAt: future },
])
Expand All @@ -148,12 +152,14 @@ describe('OAuth Utils', () => {
del: vi.fn().mockResolvedValue(1),
}
redisConfigMockFns.mockGetRedisClient.mockReturnValue(fakeRedis)
// The leader rereads the row under the lock.
mockSelectChain([])
mockSelectOrderedChain([
{ accessToken: 'stale-at', refreshToken: 'live-rt', accessTokenExpiresAt: past },
])
mockRefreshOAuthToken.mockResolvedValueOnce({
ok: false,
errorCode: 'token_revoked',
errorCode: 'invalid_client',
})
mockSelectChain([])

Expand All @@ -164,7 +170,7 @@ describe('OAuth Utils', () => {
const installationIdentity = getOAuthRefreshCoordinationIdentity('slack:T08CM6ZNYBE')
expect(fakeRedis.set).toHaveBeenCalledWith(
`oauth:dead:${installationIdentity}`,
'token_revoked',
'invalid_client',
'EX',
3600
)
Expand All @@ -177,12 +183,14 @@ describe('OAuth Utils', () => {
del: vi.fn().mockResolvedValue(1),
}
redisConfigMockFns.mockGetRedisClient.mockReturnValue(fakeRedis)
// The leader rereads the row under the lock.
mockSelectChain([])
mockSelectOrderedChain([
{ accessToken: 'stale-at', refreshToken: 'live-rt', accessTokenExpiresAt: past },
])
mockRefreshOAuthToken.mockResolvedValueOnce({
ok: false,
errorCode: 'token_revoked',
errorCode: 'invalid_client',
})
mockSelectChain([{ moved: new Date() }])

Expand Down
10 changes: 7 additions & 3 deletions apps/sim/executor/utils/credential-token.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { executeCopilotCredentialUseCase } from '@/lib/mothership/application/ex
import { resolveCopilotOrganizationPersonalToken } from '@/lib/mothership/application/resolve-organization-personal-token'
import { projectAssistantConnectedAccountTool } from '@/lib/mothership/assistant/connected-account-tool'
import type { CopilotExecutionContext } from '@/lib/mothership/auth/application-delegation'
import { CredentialRevokedError, OAUTH_CREDENTIAL_REVOKED } from '@/lib/oauth/credential-revoked'
import {
type CredentialTokenPayload,
resolveCredentialAccessToken,
Expand Down Expand Up @@ -124,14 +125,17 @@ export async function resolveExecutorCredentialToken(
})

if (!result.ok) {
const message = `Failed to obtain credential for ${params.toolLabel ?? credentialId}: ${result.error}`
// The resolver already logged the revocation once, at WARN, where it was classified.
if (result.code === OAUTH_CREDENTIAL_REVOKED) {
throw new CredentialRevokedError(message)
}
logger.error(`[${requestId}] Credential token resolution failed`, {
status: result.status,
credentialId,
code: result.code,
})
throw new Error(
`Failed to obtain credential for ${params.toolLabel ?? credentialId}: ${result.error}`
)
throw new Error(message)
}

return result.token
Expand Down
15 changes: 15 additions & 0 deletions apps/sim/lib/auth/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ import { validateSignupEmailMx } from '@/lib/messaging/email/validation.server'
import { isEmailVerificationEffectivelyEnabled } from '@/lib/messaging/email/verification'
import { scheduleLifecycleEmail } from '@/lib/messaging/lifecycle'
import { APP_ENTRY_PATH } from '@/lib/navigation/paths'
import { clearRecordedRevocation } from '@/lib/oauth/credential-service'
import {
getMicrosoftRefreshTokenExpiry,
isMicrosoftProvider,
Expand Down Expand Up @@ -701,6 +702,20 @@ export const auth = betterAuth({
}
},
},
update: {
/**
* Relinking an identity that already has a row updates it in place instead of creating
* one, and keeps the old refresh token when the provider issues none, so the fresh
* authorization has to clear the failures recorded against the old grant explicitly.
*/
after: async (account, context) => {
const path = context?.path
if (!path?.startsWith('/oauth2/callback/') && !path?.startsWith('/callback/')) return
// Fails the callback only if this statement fails, so a reconnect never reports
// success while the old revocation still blocks the credential.
await clearRecordedRevocation(account.id)
},
},
},
session: {
create: {
Expand Down
2 changes: 1 addition & 1 deletion apps/sim/lib/auth/connectors/managed-oauth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import { readResponseJsonWithLimit } from '@/lib/core/utils/stream-limits'
import { getDocusignOAuthUrl } from '@/lib/oauth/docusign'
import { verifyGitHubRepositoriesIdentity } from '@/lib/oauth/github-repositories'
import { deriveMicrosoftEmailVerified, mapMicrosoftProfileToUser } from '@/lib/oauth/microsoft'
import { isTerminalRefreshError } from '@/lib/oauth/refresh-error-codes'
import { SALESFORCE_LOGIN_HOSTS } from '@/lib/oauth/salesforce'
import { isTerminalRefreshError } from '@/lib/oauth/terminal-errors'
import { getCanonicalScopesForProvider, isScopeSatisfiedBy } from '@/lib/oauth/utils'
import { MONDAY_API_URL, MONDAY_API_VERSION } from '@/tools/monday/utils'

Expand Down
4 changes: 4 additions & 0 deletions apps/sim/lib/credentials/draft-hooks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ describe('handleCreateCredentialFromDraft', () => {
queueTableRows(schemaMock.credential, [
{ id: 'credential-existing', displayName: 'Existing Gmail' },
])
queueTableRows(schemaMock.account, [
{ providerId: 'google-email', providerAccountId: 'subject' },
])

await handleCreateCredentialFromDraft({
draft: {
Expand Down Expand Up @@ -99,6 +102,7 @@ describe('handleReconnectCredential', () => {
{ id: 'credential-1', accountId: null, displayName: 'Renamed Gmail' },
])
queueTableRows(schemaMock.credential, [])
queueTableRows(schemaMock.account, [{ providerId: 'gmail', providerAccountId: 'subject-new' }])
queueTableRows(schemaMock.account, [{ providerId: 'gmail', accountId: 'subject-new' }])

await handleReconnectCredential({
Expand Down
8 changes: 4 additions & 4 deletions apps/sim/lib/credentials/draft-hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { generateId } from '@sim/utils/id'
import { and, eq, sql } from 'drizzle-orm'
import { deleteOrphanedOAuthAccount } from '@/lib/credentials/deletion'
import { resumeConnectorsAfterCredentialReconnect } from '@/lib/knowledge/connectors/credential-recovery'
import { clearOAuthRefreshDeadFlag } from '@/lib/oauth/refresh-coordination'
import { clearOAuthRefreshFailure } from '@/lib/oauth/credential-service'
import { captureServerEvent } from '@/lib/posthog/server'

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

await clearOAuthRefreshDeadFlag(accountId)
await clearOAuthRefreshFailure(accountId)
await resumeConnectorsAfterCredentialReconnect(accountId, now)

recordAudit({
Expand Down Expand Up @@ -110,7 +110,7 @@ export async function handleCreateCredentialFromDraft(params: {
accountId,
})

await clearOAuthRefreshDeadFlag(accountId)
await clearOAuthRefreshFailure(accountId)

recordAudit({
workspaceId: draft.workspaceId,
Expand Down Expand Up @@ -210,7 +210,7 @@ export async function handleReconnectCredential(params: {
}
)

await clearOAuthRefreshDeadFlag(newAccountId)
await clearOAuthRefreshFailure(newAccountId)
await resumeConnectorsAfterCredentialReconnect(newAccountId, now)

recordAudit({
Expand Down
4 changes: 2 additions & 2 deletions apps/sim/lib/credentials/organization-draft.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { pendingCredentialDraft } from '@sim/db/schema'
import { dbChainMockFns, hasMockCondition, resetDbChainMock } from '@sim/testing'
import { auditMock } from '@sim/testing/mocks/audit.mock'
import { authOAuthUtilsMock } from '@sim/testing/mocks/auth-oauth-utils.mock'
import {
organizationMembershipMock,
organizationMembershipMockFns,
Expand All @@ -9,7 +10,6 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'

const hoisted = vi.hoisted(() => ({
context: vi.fn(),
clear: vi.fn(),
deleteOrphan: vi.fn(),
}))
vi.mock('@sim/audit', () => auditMock)
Expand All @@ -18,7 +18,7 @@ vi.mock('@/lib/credentials/organization', () => ({
getCredentialCreationOrganizationContext: hoisted.context,
}))
vi.mock('@/lib/credentials/deletion', () => ({ deleteOrphanedOAuthAccount: hoisted.deleteOrphan }))
vi.mock('@/lib/oauth/refresh-coordination', () => ({ clearOAuthRefreshDeadFlag: hoisted.clear }))
vi.mock('@/lib/oauth/credential-service', () => authOAuthUtilsMock)

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

Expand Down
4 changes: 2 additions & 2 deletions apps/sim/lib/credentials/organization-draft.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { resourceScopeCondition } from '@/lib/core/resource-scope.server'
import { deleteOrphanedOAuthAccount } from '@/lib/credentials/deletion'
import { getCredentialCreationOrganizationContext } from '@/lib/credentials/organization'
import { resumeConnectorsAfterCredentialReconnect } from '@/lib/knowledge/connectors/credential-recovery'
import { clearOAuthRefreshDeadFlag } from '@/lib/oauth/refresh-coordination'
import { clearOAuthRefreshFailure } from '@/lib/oauth/credential-service'

/** Completes the exact draft bound to the authenticated provider callback, rechecking current ownership under membership locks. */
export async function completeOrganizationCredentialDraft(input: {
Expand Down Expand Up @@ -124,7 +124,7 @@ export async function completeOrganizationCredentialDraft(input: {
oldAccountId: existing?.accountId,
}
})
await clearOAuthRefreshDeadFlag(input.accountId)
await clearOAuthRefreshFailure(input.accountId)
if (result.reconnected) await resumeConnectorsAfterCredentialReconnect(input.accountId, now)
recordAudit({
actorId: input.userId,
Expand Down
13 changes: 7 additions & 6 deletions apps/sim/lib/knowledge/connectors/sync-engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ import {
import { hardDeleteDocuments } from '@/lib/knowledge/documents/service'
import { getRetryAfterMs, isRateLimitError } from '@/lib/knowledge/documents/utils'
import { getCredentialTerminalRefreshError } from '@/lib/oauth/credential-service'
import { isCredentialRevocationError } from '@/lib/oauth/terminal-errors'
import { isCredentialRevocationError } from '@/lib/oauth/refresh-error-codes'
import { connectorHasAuthSource } from '@/connectors/auth'
import { CONNECTOR_REGISTRY } from '@/connectors/registry.server'
import type {
Expand Down Expand Up @@ -819,18 +819,19 @@ async function resolveAccessToken(
})

if (!resolved) {
logger.error(`[${requestId}] Connector credential resolved no access token`, {
credentialId: connector.credentialId,
userId,
authMode: connectorConfig.auth.mode,
})
const revocationError =
connectorConfig.auth.mode === 'oauth' && connector.credentialId
? await getCredentialRevocationError(connector.credentialId)
: null
// executeSync logs the revocation once when it unschedules the connector.
if (revocationError && connector.credentialId) {
throw new ConnectorCredentialRevokedError(connector.credentialId, revocationError)
}
logger.error(`[${requestId}] Connector credential resolved no access token`, {
credentialId: connector.credentialId,
userId,
authMode: connectorConfig.auth.mode,
})
throw new Error(`Failed to obtain access token for credential ${connector.credentialId}`)
}

Expand Down
Loading
Loading