Skip to content

Commit 3f89cce

Browse files
committed
fix(knowledge): resolve connector tokens as the credential owner, not the KB owner
Connector syncs read OAuth tokens as the knowledge base owner. Token reads are scoped to account.userId, so a shared workspace credential authorized by any other member resolved no token at all. Adds resolveCredentialTokenIdentity, matching the ownership resolution authorizeCredentialUse and getCredentialOwner already use, and applies it in the sync engine and the connector PATCH route. Also stops the connector credential picker from offering service accounts. The credential list returns them alongside OAuth accounts and the picker rendered them unfiltered (21 of 30 connectors affected), but no connector can authenticate with one: the sync engine passes no scopes (a Google service account throws) and drops the cloudId/domain/authStyle an Atlassian service account resolves with.
1 parent cb3611b commit 3f89cce

5 files changed

Lines changed: 215 additions & 27 deletions

File tree

apps/sim/app/api/knowledge/[id]/connectors/[connectorId]/route.ts

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,6 @@
11
import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit'
22
import { db } from '@sim/db'
3-
import {
4-
document,
5-
embedding,
6-
knowledgeBase,
7-
knowledgeConnector,
8-
knowledgeConnectorSyncLog,
9-
} from '@sim/db/schema'
3+
import { document, embedding, knowledgeConnector, knowledgeConnectorSyncLog } from '@sim/db/schema'
104
import { createLogger } from '@sim/logger'
115
import { and, desc, eq, inArray, isNull, sql } from 'drizzle-orm'
126
import { type NextRequest, NextResponse } from 'next/server'
@@ -17,6 +11,7 @@ import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
1711
import { hasWorkspaceLiveSyncAccess } from '@/lib/billing/core/subscription'
1812
import { generateRequestId } from '@/lib/core/utils/request'
1913
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
14+
import { resolveCredentialTokenIdentity } from '@/lib/credentials/access'
2015
import { deleteDocumentStorageFiles } from '@/lib/knowledge/documents/service'
2116
import { cleanupUnusedTagDefinitions } from '@/lib/knowledge/tags/service'
2217
import { captureServerEvent } from '@/lib/posthog/server'
@@ -157,16 +152,6 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Rout
157152
)
158153
}
159154

160-
const kbRows = await db
161-
.select({ userId: knowledgeBase.userId })
162-
.from(knowledgeBase)
163-
.where(eq(knowledgeBase.id, knowledgeBaseId))
164-
.limit(1)
165-
166-
if (kbRows.length === 0) {
167-
return NextResponse.json({ error: 'Knowledge base not found' }, { status: 404 })
168-
}
169-
170155
let accessToken: string | null = null
171156
if (connectorConfig.auth.mode === 'apiKey') {
172157
if (!existing.encryptedApiKey) {
@@ -183,9 +168,32 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Rout
183168
{ status: 400 }
184169
)
185170
}
171+
const connectorWorkspaceId = writeCheck.knowledgeBase.workspaceId
172+
if (!connectorWorkspaceId) {
173+
return NextResponse.json(
174+
{ error: 'Knowledge base is missing workspace context' },
175+
{ status: 409 }
176+
)
177+
}
178+
/**
179+
* Resolve the credential's own account owner, not the knowledge base owner:
180+
* workspace credentials are shared, and token reads are scoped to
181+
* `account.userId`.
182+
*/
183+
const identity = await resolveCredentialTokenIdentity(
184+
existing.credentialId,
185+
connectorWorkspaceId
186+
)
187+
if (!identity) {
188+
return NextResponse.json(
189+
{ error: 'Credential is no longer usable in this workspace. Please reconnect it.' },
190+
{ status: 400 }
191+
)
192+
}
186193
accessToken = await refreshAccessTokenIfNeeded(
187194
existing.credentialId,
188-
kbRows[0].userId,
195+
// Service accounts mint their own token and ignore the acting user.
196+
identity.kind === 'oauth' ? identity.userId : auth.userId,
189197
`patch-${connectorId}`
190198
)
191199
}

apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/add-connector-modal/add-connector-modal.tsx

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,14 +92,28 @@ export function AddConnectorModal({
9292
)
9393

9494
const {
95-
data: credentials = [],
95+
data: rawCredentials = [],
9696
isLoading: credentialsLoading,
9797
refetch: refetchCredentials,
9898
} = useOAuthCredentials(connectorProviderId ?? undefined, {
9999
enabled: Boolean(connectorConfig) && !isApiKeyMode,
100100
workspaceId,
101101
})
102102

103+
/**
104+
* The credential list also returns the provider's service accounts, but
105+
* `ConnectorAuthConfig` has no service-account mode: the sync engine resolves
106+
* connector tokens through `refreshAccessTokenIfNeeded`, which passes no scopes
107+
* and drops the `cloudId`/`domain`/`authStyle` a service account resolves with.
108+
* Offering them here would surface credentials no connector can authenticate
109+
* with, so — like a workflow picker that has not opted in via
110+
* `allowServiceAccounts` — list OAuth accounts only.
111+
*/
112+
const credentials = useMemo(
113+
() => rawCredentials.filter((cred) => cred.type !== 'service_account'),
114+
[rawCredentials]
115+
)
116+
103117
useCredentialRefreshTriggers(refetchCredentials, connectorProviderId ?? '', workspaceId)
104118

105119
const effectiveCredentialId =

apps/sim/lib/credentials/access.test.ts

Lines changed: 76 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,23 @@
1-
import { credential, credentialMember } from '@sim/db/schema'
1+
import { account, credential, credentialMember } from '@sim/db/schema'
22
import { queueTableRows, resetDbChainMock } from '@sim/testing'
33
import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
44

5-
const { mockCheckWorkspaceAccess } = vi.hoisted(() => ({
5+
const { mockCheckWorkspaceAccess, mockGetUserEntityPermissions } = vi.hoisted(() => ({
66
mockCheckWorkspaceAccess: vi.fn(),
7+
mockGetUserEntityPermissions: vi.fn(),
78
}))
89

910
vi.mock('@/lib/workspaces/permissions/utils', () => ({
1011
checkWorkspaceAccess: mockCheckWorkspaceAccess,
12+
getUserEntityPermissions: mockGetUserEntityPermissions,
1113
resolveWorkspaceAccess: vi.fn(async (workspaceId: string, userId: string, provided?: any) =>
1214
provided && provided.workspace?.id === workspaceId
1315
? provided
1416
: mockCheckWorkspaceAccess(workspaceId, userId)
1517
),
1618
}))
1719

18-
import { getCredentialActorContext } from '@/lib/credentials/access'
20+
import { getCredentialActorContext, resolveCredentialTokenIdentity } from '@/lib/credentials/access'
1921

2022
afterAll(resetDbChainMock)
2123

@@ -88,3 +90,74 @@ describe('getCredentialActorContext', () => {
8890
expect(ctx.isAdmin).toBe(false)
8991
})
9092
})
93+
94+
describe('resolveCredentialTokenIdentity', () => {
95+
beforeEach(() => {
96+
vi.clearAllMocks()
97+
resetDbChainMock()
98+
})
99+
100+
it('resolves the account owner even when it is not the caller', async () => {
101+
queueTableRows(credential, [{ workspaceId: 'ws', type: 'oauth', accountId: 'acct1' }])
102+
queueTableRows(account, [{ userId: 'authorizer' }])
103+
mockGetUserEntityPermissions.mockResolvedValue('write')
104+
105+
await expect(resolveCredentialTokenIdentity('c1', 'ws')).resolves.toEqual({
106+
kind: 'oauth',
107+
userId: 'authorizer',
108+
})
109+
})
110+
111+
it('rejects a credential belonging to another workspace', async () => {
112+
queueTableRows(credential, [{ workspaceId: 'other-ws', type: 'oauth', accountId: 'acct1' }])
113+
114+
await expect(resolveCredentialTokenIdentity('c1', 'ws')).resolves.toBeNull()
115+
})
116+
117+
it('reports service accounts as needing no user id', async () => {
118+
queueTableRows(credential, [{ workspaceId: 'ws', type: 'service_account', accountId: null }])
119+
120+
await expect(resolveCredentialTokenIdentity('c1', 'ws')).resolves.toEqual({
121+
kind: 'service_account',
122+
})
123+
})
124+
125+
it('rejects a service account from another workspace', async () => {
126+
queueTableRows(credential, [
127+
{ workspaceId: 'other-ws', type: 'service_account', accountId: null },
128+
])
129+
130+
await expect(resolveCredentialTokenIdentity('c1', 'ws')).resolves.toBeNull()
131+
})
132+
133+
it('rejects a credential type that is neither oauth nor a service account', async () => {
134+
queueTableRows(credential, [{ workspaceId: 'ws', type: 'env_personal', accountId: null }])
135+
136+
await expect(resolveCredentialTokenIdentity('c1', 'ws')).resolves.toBeNull()
137+
})
138+
139+
it('rejects when the owner no longer has workspace access', async () => {
140+
queueTableRows(credential, [{ workspaceId: 'ws', type: 'oauth', accountId: 'acct1' }])
141+
queueTableRows(account, [{ userId: 'departed' }])
142+
mockGetUserEntityPermissions.mockResolvedValue(null)
143+
144+
await expect(resolveCredentialTokenIdentity('c1', 'ws')).resolves.toBeNull()
145+
})
146+
147+
it('falls back to a legacy raw account id when no credential row exists', async () => {
148+
queueTableRows(account, [{ userId: 'legacy-owner' }])
149+
mockGetUserEntityPermissions.mockResolvedValue('admin')
150+
151+
await expect(resolveCredentialTokenIdentity('acct-legacy', 'ws')).resolves.toEqual({
152+
kind: 'oauth',
153+
userId: 'legacy-owner',
154+
})
155+
})
156+
157+
it('returns null when the account row is missing', async () => {
158+
queueTableRows(credential, [{ workspaceId: 'ws', type: 'oauth', accountId: 'acct1' }])
159+
160+
await expect(resolveCredentialTokenIdentity('c1', 'ws')).resolves.toBeNull()
161+
expect(mockGetUserEntityPermissions).not.toHaveBeenCalled()
162+
})
163+
})

apps/sim/lib/credentials/access.ts

Lines changed: 66 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
import { db } from '@sim/db'
2-
import { credential, credentialMember, credentialTypeEnum } from '@sim/db/schema'
2+
import { account, credential, credentialMember, credentialTypeEnum } from '@sim/db/schema'
33
import { and, eq, inArray } from 'drizzle-orm'
44
import type { DbOrTx } from '@/lib/db/types'
5-
import { resolveWorkspaceAccess, type WorkspaceAccess } from '@/lib/workspaces/permissions/utils'
5+
import {
6+
getUserEntityPermissions,
7+
resolveWorkspaceAccess,
8+
type WorkspaceAccess,
9+
} from '@/lib/workspaces/permissions/utils'
610

711
type ActiveCredentialMember = typeof credentialMember.$inferSelect
812
type CredentialRecord = typeof credential.$inferSelect
@@ -19,6 +23,66 @@ export const SHARED_CREDENTIAL_TYPES = credentialTypeEnum.enumValues.filter(
1923
(type) => type !== 'env_personal'
2024
)
2125

26+
/**
27+
* Which user a credential's token must be read as.
28+
*
29+
* Service-account credentials mint their own token and ignore the acting user
30+
* entirely, so they carry no user id — callers pass their existing one through.
31+
*/
32+
export type CredentialTokenIdentity =
33+
| { kind: 'service_account' }
34+
| { kind: 'oauth'; userId: string }
35+
36+
/**
37+
* Resolves which user a credential's token must be read as, for background jobs
38+
* that run without a request context (connector syncs, scheduled runs).
39+
*
40+
* Workspace-scoped OAuth credentials are shared, so the member who authorized one
41+
* is frequently not the user driving the job. Token reads are scoped to
42+
* `account.userId`, so a job passing its own user id resolves no token at all.
43+
* Mirrors the ownership resolution in `authorizeCredentialUse`: the credential must
44+
* belong to `workspaceId`, and its owner must still have access to that workspace.
45+
*
46+
* @returns the identity to read the token as, or `null` when the credential is
47+
* unusable from this workspace (wrong workspace, missing account, owner lost access).
48+
*/
49+
export async function resolveCredentialTokenIdentity(
50+
credentialId: string,
51+
workspaceId: string
52+
): Promise<CredentialTokenIdentity | null> {
53+
const [platformCredential] = await db
54+
.select({
55+
workspaceId: credential.workspaceId,
56+
type: credential.type,
57+
accountId: credential.accountId,
58+
})
59+
.from(credential)
60+
.where(eq(credential.id, credentialId))
61+
.limit(1)
62+
63+
if (platformCredential) {
64+
if (platformCredential.workspaceId !== workspaceId) return null
65+
if (platformCredential.type === 'service_account') return { kind: 'service_account' }
66+
if (platformCredential.type !== 'oauth' || !platformCredential.accountId) return null
67+
}
68+
69+
// Credentials predating the workspace-scoped `credential` table are raw account ids.
70+
const accountId = platformCredential?.accountId ?? credentialId
71+
72+
const [accountRow] = await db
73+
.select({ userId: account.userId })
74+
.from(account)
75+
.where(eq(account.id, accountId))
76+
.limit(1)
77+
78+
if (!accountRow) return null
79+
80+
const ownerPerm = await getUserEntityPermissions(accountRow.userId, 'workspace', workspaceId)
81+
if (ownerPerm === null) return null
82+
83+
return { kind: 'oauth', userId: accountRow.userId }
84+
}
85+
2286
/** Whether a credential is shared at the workspace level (i.e. not a personal env var). */
2387
export function isSharedCredentialType(type: CredentialType): boolean {
2488
return type !== 'env_personal'

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

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
type BillingAttributionSnapshot,
1818
} from '@/lib/billing/core/billing-attribution'
1919
import { getInternalApiBaseUrl } from '@/lib/core/utils/urls'
20+
import { resolveCredentialTokenIdentity } from '@/lib/credentials/access'
2021
import type { DocumentData } from '@/lib/knowledge/documents/service'
2122
import { hardDeleteDocuments, processDocumentsWithQueue } from '@/lib/knowledge/documents/service'
2223
import { StorageService } from '@/lib/uploads'
@@ -377,6 +378,10 @@ export function resolveTagMapping(
377378
* Resolves an access token for a connector based on its auth mode.
378379
* OAuth connectors refresh via the credential system; API key connectors
379380
* decrypt the key stored in the dedicated `encryptedApiKey` column.
381+
*
382+
* `userId` must be the user who owns the credential's OAuth account — not the
383+
* knowledge base owner. Workspace-scoped credentials are routinely authorized by
384+
* a different member, and token reads are scoped to `account.userId`.
380385
*/
381386
async function resolveAccessToken(
382387
connector: { credentialId: string | null; encryptedApiKey: string | null },
@@ -529,7 +534,31 @@ export async function executeSync(
529534
let syncExitedCleanly = false
530535

531536
try {
532-
let accessToken = await resolveAccessToken(connector, connectorConfig, userId)
537+
/**
538+
* OAuth credentials are workspace-scoped and shared, so the member who authorized
539+
* one is often not the knowledge base owner. Resolve the credential's own account
540+
* owner — token reads are scoped to `account.userId`, so passing the KB owner
541+
* resolves no token at all. Resolved once here rather than inside
542+
* `resolveAccessToken` so per-page refreshes don't repeat the lookup.
543+
*/
544+
let credentialUserId = userId
545+
if (connectorConfig.auth.mode === 'oauth' && connector.credentialId) {
546+
const identity = await resolveCredentialTokenIdentity(
547+
connector.credentialId,
548+
kbOwner.workspaceId
549+
)
550+
if (!identity) {
551+
throw new Error(
552+
`Credential ${connector.credentialId} is not usable from workspace ${kbOwner.workspaceId} — reconnect the credential`
553+
)
554+
}
555+
// Service accounts mint their own token and ignore the acting user.
556+
if (identity.kind === 'oauth') {
557+
credentialUserId = identity.userId
558+
}
559+
}
560+
561+
let accessToken = await resolveAccessToken(connector, connectorConfig, credentialUserId)
533562

534563
const externalDocs: ExternalDocument[] = []
535564
let cursor: string | undefined
@@ -605,7 +634,7 @@ export async function executeSync(
605634

606635
for (let pageNum = 0; hasMore && pageNum < MAX_PAGES; pageNum++) {
607636
if (pageNum > 0 && connectorConfig.auth.mode === 'oauth') {
608-
accessToken = await resolveAccessToken(connector, connectorConfig, userId)
637+
accessToken = await resolveAccessToken(connector, connectorConfig, credentialUserId)
609638
}
610639

611640
const page = await connectorConfig.listDocuments(
@@ -795,7 +824,7 @@ export async function executeSync(
795824

796825
if (deferredOps.length > 0) {
797826
if (connectorConfig.auth.mode === 'oauth') {
798-
accessToken = await resolveAccessToken(connector, connectorConfig, userId)
827+
accessToken = await resolveAccessToken(connector, connectorConfig, credentialUserId)
799828
}
800829

801830
const hydrated = await Promise.allSettled(

0 commit comments

Comments
 (0)