From 3f89cce141503c1282c90738f2bea42fef349def Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 28 Jul 2026 16:11:28 -0700 Subject: [PATCH] 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. --- .../[id]/connectors/[connectorId]/route.ts | 44 ++++++----- .../add-connector-modal.tsx | 16 +++- apps/sim/lib/credentials/access.test.ts | 79 ++++++++++++++++++- apps/sim/lib/credentials/access.ts | 68 +++++++++++++++- .../lib/knowledge/connectors/sync-engine.ts | 35 +++++++- 5 files changed, 215 insertions(+), 27 deletions(-) diff --git a/apps/sim/app/api/knowledge/[id]/connectors/[connectorId]/route.ts b/apps/sim/app/api/knowledge/[id]/connectors/[connectorId]/route.ts index 33717fdcb39..3ff1d479cd0 100644 --- a/apps/sim/app/api/knowledge/[id]/connectors/[connectorId]/route.ts +++ b/apps/sim/app/api/knowledge/[id]/connectors/[connectorId]/route.ts @@ -1,12 +1,6 @@ import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit' import { db } from '@sim/db' -import { - document, - embedding, - knowledgeBase, - knowledgeConnector, - knowledgeConnectorSyncLog, -} from '@sim/db/schema' +import { document, embedding, knowledgeConnector, knowledgeConnectorSyncLog } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { and, desc, eq, inArray, isNull, sql } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' @@ -17,6 +11,7 @@ import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' import { hasWorkspaceLiveSyncAccess } from '@/lib/billing/core/subscription' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' +import { resolveCredentialTokenIdentity } from '@/lib/credentials/access' import { deleteDocumentStorageFiles } from '@/lib/knowledge/documents/service' import { cleanupUnusedTagDefinitions } from '@/lib/knowledge/tags/service' import { captureServerEvent } from '@/lib/posthog/server' @@ -157,16 +152,6 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Rout ) } - const kbRows = await db - .select({ userId: knowledgeBase.userId }) - .from(knowledgeBase) - .where(eq(knowledgeBase.id, knowledgeBaseId)) - .limit(1) - - if (kbRows.length === 0) { - return NextResponse.json({ error: 'Knowledge base not found' }, { status: 404 }) - } - let accessToken: string | null = null if (connectorConfig.auth.mode === 'apiKey') { if (!existing.encryptedApiKey) { @@ -183,9 +168,32 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Rout { status: 400 } ) } + const connectorWorkspaceId = writeCheck.knowledgeBase.workspaceId + if (!connectorWorkspaceId) { + return NextResponse.json( + { error: 'Knowledge base is missing workspace context' }, + { status: 409 } + ) + } + /** + * Resolve the credential's own account owner, not the knowledge base owner: + * workspace credentials are shared, and token reads are scoped to + * `account.userId`. + */ + const identity = await resolveCredentialTokenIdentity( + existing.credentialId, + connectorWorkspaceId + ) + if (!identity) { + return NextResponse.json( + { error: 'Credential is no longer usable in this workspace. Please reconnect it.' }, + { status: 400 } + ) + } accessToken = await refreshAccessTokenIfNeeded( existing.credentialId, - kbRows[0].userId, + // Service accounts mint their own token and ignore the acting user. + identity.kind === 'oauth' ? identity.userId : auth.userId, `patch-${connectorId}` ) } diff --git a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/add-connector-modal/add-connector-modal.tsx b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/add-connector-modal/add-connector-modal.tsx index 8738542fa6d..934e4b07011 100644 --- a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/add-connector-modal/add-connector-modal.tsx +++ b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/add-connector-modal/add-connector-modal.tsx @@ -92,7 +92,7 @@ export function AddConnectorModal({ ) const { - data: credentials = [], + data: rawCredentials = [], isLoading: credentialsLoading, refetch: refetchCredentials, } = useOAuthCredentials(connectorProviderId ?? undefined, { @@ -100,6 +100,20 @@ export function AddConnectorModal({ workspaceId, }) + /** + * The credential list also returns the provider's service accounts, but + * `ConnectorAuthConfig` has no service-account mode: the sync engine resolves + * connector tokens through `refreshAccessTokenIfNeeded`, which passes no scopes + * and drops the `cloudId`/`domain`/`authStyle` a service account resolves with. + * Offering them here would surface credentials no connector can authenticate + * with, so — like a workflow picker that has not opted in via + * `allowServiceAccounts` — list OAuth accounts only. + */ + const credentials = useMemo( + () => rawCredentials.filter((cred) => cred.type !== 'service_account'), + [rawCredentials] + ) + useCredentialRefreshTriggers(refetchCredentials, connectorProviderId ?? '', workspaceId) const effectiveCredentialId = diff --git a/apps/sim/lib/credentials/access.test.ts b/apps/sim/lib/credentials/access.test.ts index b5ae09c1418..1fc40f6e4ed 100644 --- a/apps/sim/lib/credentials/access.test.ts +++ b/apps/sim/lib/credentials/access.test.ts @@ -1,13 +1,15 @@ -import { credential, credentialMember } from '@sim/db/schema' +import { account, credential, credentialMember } from '@sim/db/schema' import { queueTableRows, resetDbChainMock } from '@sim/testing' import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' -const { mockCheckWorkspaceAccess } = vi.hoisted(() => ({ +const { mockCheckWorkspaceAccess, mockGetUserEntityPermissions } = vi.hoisted(() => ({ mockCheckWorkspaceAccess: vi.fn(), + mockGetUserEntityPermissions: vi.fn(), })) vi.mock('@/lib/workspaces/permissions/utils', () => ({ checkWorkspaceAccess: mockCheckWorkspaceAccess, + getUserEntityPermissions: mockGetUserEntityPermissions, resolveWorkspaceAccess: vi.fn(async (workspaceId: string, userId: string, provided?: any) => provided && provided.workspace?.id === workspaceId ? provided @@ -15,7 +17,7 @@ vi.mock('@/lib/workspaces/permissions/utils', () => ({ ), })) -import { getCredentialActorContext } from '@/lib/credentials/access' +import { getCredentialActorContext, resolveCredentialTokenIdentity } from '@/lib/credentials/access' afterAll(resetDbChainMock) @@ -88,3 +90,74 @@ describe('getCredentialActorContext', () => { expect(ctx.isAdmin).toBe(false) }) }) + +describe('resolveCredentialTokenIdentity', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + }) + + it('resolves the account owner even when it is not the caller', async () => { + queueTableRows(credential, [{ workspaceId: 'ws', type: 'oauth', accountId: 'acct1' }]) + queueTableRows(account, [{ userId: 'authorizer' }]) + mockGetUserEntityPermissions.mockResolvedValue('write') + + await expect(resolveCredentialTokenIdentity('c1', 'ws')).resolves.toEqual({ + kind: 'oauth', + userId: 'authorizer', + }) + }) + + it('rejects a credential belonging to another workspace', async () => { + queueTableRows(credential, [{ workspaceId: 'other-ws', type: 'oauth', accountId: 'acct1' }]) + + await expect(resolveCredentialTokenIdentity('c1', 'ws')).resolves.toBeNull() + }) + + it('reports service accounts as needing no user id', async () => { + queueTableRows(credential, [{ workspaceId: 'ws', type: 'service_account', accountId: null }]) + + await expect(resolveCredentialTokenIdentity('c1', 'ws')).resolves.toEqual({ + kind: 'service_account', + }) + }) + + it('rejects a service account from another workspace', async () => { + queueTableRows(credential, [ + { workspaceId: 'other-ws', type: 'service_account', accountId: null }, + ]) + + await expect(resolveCredentialTokenIdentity('c1', 'ws')).resolves.toBeNull() + }) + + it('rejects a credential type that is neither oauth nor a service account', async () => { + queueTableRows(credential, [{ workspaceId: 'ws', type: 'env_personal', accountId: null }]) + + await expect(resolveCredentialTokenIdentity('c1', 'ws')).resolves.toBeNull() + }) + + it('rejects when the owner no longer has workspace access', async () => { + queueTableRows(credential, [{ workspaceId: 'ws', type: 'oauth', accountId: 'acct1' }]) + queueTableRows(account, [{ userId: 'departed' }]) + mockGetUserEntityPermissions.mockResolvedValue(null) + + await expect(resolveCredentialTokenIdentity('c1', 'ws')).resolves.toBeNull() + }) + + it('falls back to a legacy raw account id when no credential row exists', async () => { + queueTableRows(account, [{ userId: 'legacy-owner' }]) + mockGetUserEntityPermissions.mockResolvedValue('admin') + + await expect(resolveCredentialTokenIdentity('acct-legacy', 'ws')).resolves.toEqual({ + kind: 'oauth', + userId: 'legacy-owner', + }) + }) + + it('returns null when the account row is missing', async () => { + queueTableRows(credential, [{ workspaceId: 'ws', type: 'oauth', accountId: 'acct1' }]) + + await expect(resolveCredentialTokenIdentity('c1', 'ws')).resolves.toBeNull() + expect(mockGetUserEntityPermissions).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/lib/credentials/access.ts b/apps/sim/lib/credentials/access.ts index 005a47fe65d..185d6206db6 100644 --- a/apps/sim/lib/credentials/access.ts +++ b/apps/sim/lib/credentials/access.ts @@ -1,8 +1,12 @@ import { db } from '@sim/db' -import { credential, credentialMember, credentialTypeEnum } from '@sim/db/schema' +import { account, credential, credentialMember, credentialTypeEnum } from '@sim/db/schema' import { and, eq, inArray } from 'drizzle-orm' import type { DbOrTx } from '@/lib/db/types' -import { resolveWorkspaceAccess, type WorkspaceAccess } from '@/lib/workspaces/permissions/utils' +import { + getUserEntityPermissions, + resolveWorkspaceAccess, + type WorkspaceAccess, +} from '@/lib/workspaces/permissions/utils' type ActiveCredentialMember = typeof credentialMember.$inferSelect type CredentialRecord = typeof credential.$inferSelect @@ -19,6 +23,66 @@ export const SHARED_CREDENTIAL_TYPES = credentialTypeEnum.enumValues.filter( (type) => type !== 'env_personal' ) +/** + * Which user a credential's token must be read as. + * + * Service-account credentials mint their own token and ignore the acting user + * entirely, so they carry no user id — callers pass their existing one through. + */ +export type CredentialTokenIdentity = + | { kind: 'service_account' } + | { kind: 'oauth'; userId: string } + +/** + * Resolves which user a credential's token must be read as, for background jobs + * that run without a request context (connector syncs, scheduled runs). + * + * Workspace-scoped OAuth credentials are shared, so the member who authorized one + * is frequently not the user driving the job. Token reads are scoped to + * `account.userId`, so a job passing its own user id resolves no token at all. + * Mirrors the ownership resolution in `authorizeCredentialUse`: the credential must + * belong to `workspaceId`, and its owner must still have access to that workspace. + * + * @returns the identity to read the token as, or `null` when the credential is + * unusable from this workspace (wrong workspace, missing account, owner lost access). + */ +export async function resolveCredentialTokenIdentity( + credentialId: string, + workspaceId: string +): Promise { + const [platformCredential] = await db + .select({ + workspaceId: credential.workspaceId, + type: credential.type, + accountId: credential.accountId, + }) + .from(credential) + .where(eq(credential.id, credentialId)) + .limit(1) + + if (platformCredential) { + if (platformCredential.workspaceId !== workspaceId) return null + if (platformCredential.type === 'service_account') return { kind: 'service_account' } + if (platformCredential.type !== 'oauth' || !platformCredential.accountId) return null + } + + // Credentials predating the workspace-scoped `credential` table are raw account ids. + const accountId = platformCredential?.accountId ?? credentialId + + const [accountRow] = await db + .select({ userId: account.userId }) + .from(account) + .where(eq(account.id, accountId)) + .limit(1) + + if (!accountRow) return null + + const ownerPerm = await getUserEntityPermissions(accountRow.userId, 'workspace', workspaceId) + if (ownerPerm === null) return null + + return { kind: 'oauth', userId: accountRow.userId } +} + /** Whether a credential is shared at the workspace level (i.e. not a personal env var). */ export function isSharedCredentialType(type: CredentialType): boolean { return type !== 'env_personal' diff --git a/apps/sim/lib/knowledge/connectors/sync-engine.ts b/apps/sim/lib/knowledge/connectors/sync-engine.ts index 65b0d04542f..8ca978ec829 100644 --- a/apps/sim/lib/knowledge/connectors/sync-engine.ts +++ b/apps/sim/lib/knowledge/connectors/sync-engine.ts @@ -17,6 +17,7 @@ import { type BillingAttributionSnapshot, } from '@/lib/billing/core/billing-attribution' import { getInternalApiBaseUrl } from '@/lib/core/utils/urls' +import { resolveCredentialTokenIdentity } from '@/lib/credentials/access' import type { DocumentData } from '@/lib/knowledge/documents/service' import { hardDeleteDocuments, processDocumentsWithQueue } from '@/lib/knowledge/documents/service' import { StorageService } from '@/lib/uploads' @@ -377,6 +378,10 @@ export function resolveTagMapping( * Resolves an access token for a connector based on its auth mode. * OAuth connectors refresh via the credential system; API key connectors * decrypt the key stored in the dedicated `encryptedApiKey` column. + * + * `userId` must be the user who owns the credential's OAuth account — not the + * knowledge base owner. Workspace-scoped credentials are routinely authorized by + * a different member, and token reads are scoped to `account.userId`. */ async function resolveAccessToken( connector: { credentialId: string | null; encryptedApiKey: string | null }, @@ -529,7 +534,31 @@ export async function executeSync( let syncExitedCleanly = false try { - let accessToken = await resolveAccessToken(connector, connectorConfig, userId) + /** + * OAuth credentials are workspace-scoped and shared, so the member who authorized + * one is often not the knowledge base owner. Resolve the credential's own account + * owner — token reads are scoped to `account.userId`, so passing the KB owner + * resolves no token at all. Resolved once here rather than inside + * `resolveAccessToken` so per-page refreshes don't repeat the lookup. + */ + let credentialUserId = userId + if (connectorConfig.auth.mode === 'oauth' && connector.credentialId) { + const identity = await resolveCredentialTokenIdentity( + connector.credentialId, + kbOwner.workspaceId + ) + if (!identity) { + throw new Error( + `Credential ${connector.credentialId} is not usable from workspace ${kbOwner.workspaceId} — reconnect the credential` + ) + } + // Service accounts mint their own token and ignore the acting user. + if (identity.kind === 'oauth') { + credentialUserId = identity.userId + } + } + + let accessToken = await resolveAccessToken(connector, connectorConfig, credentialUserId) const externalDocs: ExternalDocument[] = [] let cursor: string | undefined @@ -605,7 +634,7 @@ export async function executeSync( for (let pageNum = 0; hasMore && pageNum < MAX_PAGES; pageNum++) { if (pageNum > 0 && connectorConfig.auth.mode === 'oauth') { - accessToken = await resolveAccessToken(connector, connectorConfig, userId) + accessToken = await resolveAccessToken(connector, connectorConfig, credentialUserId) } const page = await connectorConfig.listDocuments( @@ -795,7 +824,7 @@ export async function executeSync( if (deferredOps.length > 0) { if (connectorConfig.auth.mode === 'oauth') { - accessToken = await resolveAccessToken(connector, connectorConfig, userId) + accessToken = await resolveAccessToken(connector, connectorConfig, credentialUserId) } const hydrated = await Promise.allSettled(