From 416af7c57f396001ebfc9abe3384753de5a99bc7 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sun, 2 Aug 2026 20:30:52 -0700 Subject: [PATCH 1/2] fix(credentials): capture the correct provider identity on connect and rotate Attio OAuth recorded an arbitrary workspace member instead of the authorizing user, so two members connecting under one Sim user collapsed into a single account row via the stale-sibling dedupe. Notion read `profile.person.email`, which never exists on a bot token. Synthetic connector emails were minted on live third-party domains. Google service-account rotation left the credential labeled with the old key's client_email and skipped audit metadata entirely. Box and Salesforce identity lookups failed silently with no logger in either file. Service-account principals are now a single ServiceAccountPrincipal union (user / tenant / lookup_failed / null) mirrored centrally into both audit and stored metadata, so a principal can no longer be captured and forgotten, and "which account is this credential?" is answerable from SQL. --- apps/sim/app/api/credentials/route.test.ts | 3 +- apps/sim/app/api/credentials/route.ts | 5 +- apps/sim/lib/auth/auth.ts | 207 +++++++++--- apps/sim/lib/auth/connector-email.test.ts | 79 +++++ apps/sim/lib/auth/connector-email.ts | 61 ++++ .../credentials/atlassian-service-account.ts | 12 +- .../minters/box.test.ts | 35 +- .../client-credential-accounts/minters/box.ts | 71 +++-- .../minters/salesforce.test.ts | 26 +- .../minters/salesforce.ts | 66 +++- .../minters/zoho-desk.test.ts | 7 +- .../minters/zoho-desk.ts | 8 +- .../minters/zoom.test.ts | 3 +- .../minters/zoom.ts | 6 +- .../client-credential-accounts/server.ts | 15 +- apps/sim/lib/credentials/display-name.ts | 12 + .../credentials/orchestration/index.test.ts | 298 ++++++++++++++++++ .../lib/credentials/orchestration/index.ts | 174 ++++++---- apps/sim/lib/credentials/principal.ts | 62 ++++ .../service-account-secret.test.ts | 41 ++- .../lib/credentials/service-account-secret.ts | 62 +++- .../token-service-accounts/errors.ts | 14 + .../token-service-accounts/server.ts | 15 +- .../validators/airtable.test.ts | 10 +- .../validators/airtable.ts | 6 +- .../validators/asana.test.ts | 6 +- .../validators/asana.ts | 7 +- .../validators/attio.test.ts | 4 +- .../validators/attio.ts | 15 +- .../validators/calcom.test.ts | 4 +- .../validators/calcom.ts | 8 +- .../validators/claude-platform.ts | 5 + .../validators/clickup.ts | 7 +- .../validators/hubspot.test.ts | 9 +- .../validators/hubspot.ts | 18 +- .../validators/linear.test.ts | 3 +- .../validators/linear.ts | 5 +- .../validators/monday.test.ts | 5 +- .../validators/monday.ts | 5 +- .../validators/notion.test.ts | 13 +- .../validators/notion.ts | 8 +- .../validators/pipedrive.test.ts | 6 +- .../validators/pipedrive.ts | 4 +- .../validators/shopify.test.ts | 26 +- .../validators/shopify.ts | 40 ++- .../validators/trello.test.ts | 4 +- .../validators/trello.ts | 11 +- .../validators/wealthbox.test.ts | 4 +- .../validators/wealthbox.ts | 16 +- .../validators/webflow.test.ts | 6 +- .../validators/webflow.ts | 6 +- 51 files changed, 1273 insertions(+), 270 deletions(-) create mode 100644 apps/sim/lib/auth/connector-email.test.ts create mode 100644 apps/sim/lib/auth/connector-email.ts create mode 100644 apps/sim/lib/credentials/orchestration/index.test.ts create mode 100644 apps/sim/lib/credentials/principal.ts diff --git a/apps/sim/app/api/credentials/route.test.ts b/apps/sim/app/api/credentials/route.test.ts index 6127ba4b162..fb90c407a9c 100644 --- a/apps/sim/app/api/credentials/route.test.ts +++ b/apps/sim/app/api/credentials/route.test.ts @@ -140,7 +140,8 @@ describe('POST /api/credentials', () => { providerId: 'zoom-service-account', encryptedServiceAccountKey: 'encrypted-blob', displayName: 'Zoom account acct_123', - auditMetadata: { zoomAccountId: 'acct_123' }, + auditMetadata: { principalKind: 'tenant', principalId: 'acct_123' }, + principal: { kind: 'tenant', id: 'acct_123' }, }) const req = createMockRequest('POST', { diff --git a/apps/sim/app/api/credentials/route.ts b/apps/sim/app/api/credentials/route.ts index 74b3b6337f1..b8484be2dac 100644 --- a/apps/sim/app/api/credentials/route.ts +++ b/apps/sim/app/api/credentials/route.ts @@ -654,9 +654,12 @@ export const POST = withRouteHandler(async (request: NextRequest) => { resourceName: resolvedDisplayName, description: `Created ${type} credential "${resolvedDisplayName}"`, metadata: { + // Provider metadata spreads first so this route's own keys stay + // authoritative and can never be shadowed, matching the update path in + // `lib/credentials/orchestration`. + ...extraAuditMetadata, credentialType: type, providerId: resolvedProviderId, - ...extraAuditMetadata, }, request, }) diff --git a/apps/sim/lib/auth/auth.ts b/apps/sim/lib/auth/auth.ts index e7efb1a0e28..a4df054e65c 100644 --- a/apps/sim/lib/auth/auth.ts +++ b/apps/sim/lib/auth/auth.ts @@ -33,6 +33,7 @@ import { } from '@/components/emails' import { getAccessControlConfig, isEmailBlockedByAccessControl } from '@/lib/auth/access-control' import { createAnonymousSession, ensureAnonymousUserExists } from '@/lib/auth/anonymous' +import { syntheticConnectorEmail } from '@/lib/auth/connector-email' import { getRequestedSignInProviderId, isSignInProviderAllowed } from '@/lib/auth/constants' import { getSessionCookieCacheVersion } from '@/lib/auth/security-policy' import { clampExpiryForSession } from '@/lib/auth/session-policy' @@ -113,6 +114,45 @@ import { deriveZohoDeskBaseFromApiDomain } from '@/tools/zoho_desk/host-allowlis const logger = createLogger('Auth') +/** + * Shape of `GET https://api.notion.com/v1/users/me` for an OAuth integration token. + * @see https://developers.notion.com/reference/get-self + */ +interface NotionSelfResponse { + id: string + name?: string | null + bot?: { + owner?: + | { type: 'user'; user?: { id: string; name?: string | null; person?: { email?: string } } } + | { type: 'workspace'; workspace: true } + } +} + +/** + * Shape of `GET https://api.attio.com/v2/self` (the Identify endpoint). + * @see https://docs.attio.com/rest-api/endpoint-reference/meta/identify + */ +interface AttioSelfResponse { + active?: boolean + authorized_by_workspace_member_id?: string | null + workspace_id?: string + workspace_name?: string +} + +/** + * Shape of `GET https://api.attio.com/v2/workspace_members/{id}`. + * @see https://docs.attio.com/rest-api/endpoint-reference/workspace-members/get-a-workspace-member + */ +interface AttioWorkspaceMemberResponse { + data?: { + id: { workspace_id: string; workspace_member_id: string } + first_name?: string | null + last_name?: string | null + email_address?: string | null + avatar_url?: string | null + } +} + /** * Extracts user info from a Microsoft ID token JWT instead of calling Graph API /me. * This avoids 403 errors for external tenant users whose admin hasn't consented to Graph API scopes. @@ -1814,7 +1854,7 @@ export const auth = betterAuth({ const email = data.email && typeof data.email === 'string' ? data.email - : `wealthbox-${userId}@wealthbox.user` + : syntheticConnectorEmail('wealthbox', userId) const name = data.name || data.full_name || data.username || 'Wealthbox User' return { @@ -1845,7 +1885,7 @@ export const auth = betterAuth({ return { id: `wealthbox-${tokenHash}-${generateId()}`, name: 'Wealthbox User', - email: `wealthbox-${tokenHash}@wealthbox.user`, + email: syntheticConnectorEmail('wealthbox', tokenHash), emailVerified: false, createdAt: now, updatedAt: now, @@ -1962,7 +2002,7 @@ export const auth = betterAuth({ return { id: `${(data.user_id || data.hub_id).toString()}-${generateId()}`, name: data.user || 'HubSpot User', - email: data.user || `hubspot-${data.hub_id}@hubspot.com`, + email: data.user || syntheticConnectorEmail('hubspot', data.hub_id), emailVerified: true, image: undefined, createdAt: new Date(), @@ -2016,7 +2056,8 @@ export const auth = betterAuth({ return { id: `${(data.user_id || data.sub).toString()}-${generateId()}`, name: data.name || 'Salesforce User', - email: data.email || `salesforce-${data.user_id}@salesforce.com`, + email: + data.email || syntheticConnectorEmail('salesforce', data.user_id ?? data.sub), emailVerified: data.email_verified === true, image: data.picture || undefined, createdAt: new Date(), @@ -2172,7 +2213,7 @@ export const auth = betterAuth({ return { id: `${zuid}-${generateId()}`, name: profile.Display_Name || 'Zoho User', - email: profile.Email || `zoho-${zuid}@zoho.user`, + email: profile.Email || syntheticConnectorEmail('zoho', zuid), emailVerified: Boolean(profile.Email), createdAt: now, updatedAt: now, @@ -2230,7 +2271,7 @@ export const auth = betterAuth({ return { id: `${profile.data.id.toString()}-${generateId()}`, name: profile.data.name || 'X User', - email: `${profile.data.username}@x.com`, + email: syntheticConnectorEmail('x', profile.data.username ?? profile.data.id), image: profile.data.profile_image_url, emailVerified: profile.data.verified || false, createdAt: now, @@ -2333,7 +2374,7 @@ export const auth = betterAuth({ return { id: `${user.open_id}-${generateId()}`, name: user.display_name || 'TikTok User', - email: `${user.open_id}@tiktok.user`, + email: syntheticConnectorEmail('tiktok', user.open_id), image: user.avatar_url || undefined, emailVerified: false, createdAt: now, @@ -2384,7 +2425,7 @@ export const auth = betterAuth({ return { id: `${profile.account_id.toString()}-${generateId()}`, name: profile.name || profile.display_name || 'Confluence User', - email: profile.email || `${profile.account_id}@atlassian.com`, + email: profile.email || syntheticConnectorEmail('confluence', profile.account_id), image: profile.picture || undefined, emailVerified: true, createdAt: now, @@ -2435,7 +2476,7 @@ export const auth = betterAuth({ return { id: `${profile.account_id.toString()}-${generateId()}`, name: profile.name || profile.display_name || 'Jira User', - email: profile.email || `${profile.account_id}@atlassian.com`, + email: profile.email || syntheticConnectorEmail('jira', profile.account_id), image: profile.picture || undefined, emailVerified: true, createdAt: now, @@ -2485,7 +2526,7 @@ export const auth = betterAuth({ return { id: `${data.id.toString()}-${generateId()}`, name: data.email ? data.email.split('@')[0] : 'Airtable User', - email: data.email || `${data.id}@airtable.user`, + email: data.email || syntheticConnectorEmail('airtable', data.id), emailVerified: !!data.email, createdAt: now, updatedAt: now, @@ -2528,14 +2569,27 @@ export const auth = betterAuth({ return null } - const profile = await response.json() + const profile: NotionSelfResponse = await response.json() const now = new Date() + /** + * An OAuth integration token always resolves to a bot user, so the + * top-level `person` is never present and the top-level `name` is the + * integration's own name ("Sim"), not the human's. The authorizing + * human — and their email — live under `bot.owner.user`, which is + * only populated when `bot.owner.type === 'user'` (a workspace-owned + * internal integration reports `{ type: 'workspace' }` instead). + * @see https://developers.notion.com/reference/get-self + */ + const ownerUser = profile.bot?.owner?.type === 'user' ? profile.bot.owner.user : null + const stableId = ownerUser?.id || profile.id + const ownerEmail = ownerUser?.person?.email + return { - id: `${(profile.bot?.owner?.user?.id || profile.id).toString()}-${generateId()}`, - name: profile.name || profile.bot?.owner?.user?.name || 'Notion User', - email: profile.person?.email || `${profile.id}@notion.user`, - emailVerified: !!profile.person?.email, + id: `${stableId}-${generateId()}`, + name: ownerUser?.name || profile.name || 'Notion User', + email: ownerEmail || syntheticConnectorEmail('notion', stableId), + emailVerified: !!ownerEmail, createdAt: now, updatedAt: now, } @@ -2586,7 +2640,7 @@ export const auth = betterAuth({ return { id: `${user.id.toString()}-${generateId()}`, name: user.name || 'Monday.com User', - email: user.email || `${user.id}@monday.user`, + email: user.email || syntheticConnectorEmail('monday', user.id), emailVerified: !!user.email, createdAt: now, updatedAt: now, @@ -2636,7 +2690,7 @@ export const auth = betterAuth({ return { id: `${data.id.toString()}-${generateId()}`, name: data.name || 'Reddit User', - email: `${data.name}@reddit.user`, + email: syntheticConnectorEmail('reddit', data.name ?? data.id), image: data.icon_img || undefined, emailVerified: false, createdAt: now, @@ -2685,7 +2739,7 @@ export const auth = betterAuth({ return { id: `${user.id.toString()}-${generateId()}`, name: user.username || 'ClickUp User', - email: user.email || `${user.id}@clickup.user`, + email: user.email || syntheticConnectorEmail('clickup', user.id), emailVerified: !!user.email, createdAt: now, updatedAt: now, @@ -2756,7 +2810,7 @@ export const auth = betterAuth({ return { id: `${viewer.id.toString()}-${generateId()}`, - email: viewer.email, + email: viewer.email || syntheticConnectorEmail('linear', viewer.id), name: viewer.name, emailVerified: true, createdAt: new Date(), @@ -2781,44 +2835,90 @@ export const auth = betterAuth({ redirectURI: `${getBaseUrl()}/api/auth/oauth2/callback/attio`, getUserInfo: async (tokens) => { try { - const response = await fetch('https://api.attio.com/v2/workspace_members', { - headers: { - Authorization: `Bearer ${tokens.accessToken}`, - }, + /** + * Resolve the *authorizing* member, not an arbitrary one. Listing + * `/v2/workspace_members` returns every member of the workspace in no + * defined order, so taking `data[0]` records a stranger's id as the + * account's stable external id — which then collapses two different + * Attio members into one account row via the stale-sibling dedupe in + * the `account.create.after` hook. + * + * `/v2/self` requires no scope and reports who authorized the token. + * @see https://docs.attio.com/rest-api/endpoint-reference/meta/identify + */ + const selfResponse = await fetch('https://api.attio.com/v2/self', { + headers: { Authorization: `Bearer ${tokens.accessToken}` }, }) - if (!response.ok) { - const errorText = await response.text() - logger.error('Attio API error:', { - status: response.status, - statusText: response.statusText, + if (!selfResponse.ok) { + const errorText = await selfResponse.text().catch(() => '') + logger.error('Attio /v2/self error:', { + status: selfResponse.status, + statusText: selfResponse.statusText, body: errorText, }) - throw new Error(`Attio API error: ${response.status} ${response.statusText}`) + return null } - const { data } = await response.json() + const self: AttioSelfResponse = await selfResponse.json() + const memberId = self.authorized_by_workspace_member_id - if (!data || data.length === 0) { - throw new Error('No workspace members found in Attio response') + if (!memberId) { + logger.error('Attio /v2/self returned no authorizing workspace member', { + active: self.active, + workspaceId: self.workspace_id, + }) + return null } - const member = data[0] + /** + * Fetch that member by id rather than listing and filtering. Requires + * `user_management:read`, which Sim always requests for Attio. + * @see https://docs.attio.com/rest-api/endpoint-reference/workspace-members/get-a-workspace-member + */ + const memberResponse = await fetch( + `https://api.attio.com/v2/workspace_members/${encodeURIComponent(memberId)}`, + { headers: { Authorization: `Bearer ${tokens.accessToken}` } } + ) + + if (!memberResponse.ok) { + const errorText = await memberResponse.text().catch(() => '') + logger.error('Attio workspace member fetch error:', { + status: memberResponse.status, + statusText: memberResponse.statusText, + body: errorText, + }) + return null + } + + const { data: member }: AttioWorkspaceMemberResponse = await memberResponse.json() + + if (!member) { + logger.error('Attio workspace member not found', { memberId }) + return null + } + + const email = member.email_address + const fullName = `${member.first_name ?? ''} ${member.last_name ?? ''}`.trim() return { id: `${member.id.workspace_member_id}-${generateId()}`, - email: member.email_address, - name: - `${member.first_name ?? ''} ${member.last_name ?? ''}`.trim() || - member.email_address, - emailVerified: true, + email: email || syntheticConnectorEmail('attio', member.id.workspace_member_id), + name: fullName || email || 'Attio User', + emailVerified: Boolean(email), createdAt: new Date(), updatedAt: new Date(), image: member.avatar_url || undefined, } } catch (error) { + /** + * Return null rather than rethrowing: Better Auth's `handleUserInfo` + * does not wrap `getUserInfo`, so a throw escapes the callback route + * as a raw 500 with no way back into the app, while null redirects + * with `user_info_is_missing`. + */ logger.error('Error in Attio getUserInfo:', error) - throw error + return null } }, }, @@ -2854,8 +2954,8 @@ export const auth = betterAuth({ return { id: `${data.id}-${generateId()}`, - email: data.login, - name: data.name || data.login, + email: data.login || syntheticConnectorEmail('box', data.id), + name: data.name || data.login || 'Box User', emailVerified: true, createdAt: new Date(), updatedAt: new Date(), @@ -2962,7 +3062,7 @@ export const auth = betterAuth({ return { id: `${profile.gid.toString()}-${generateId()}`, name: profile.name || 'Asana User', - email: profile.email || `${profile.gid}@asana.user`, + email: profile.email || syntheticConnectorEmail('asana', profile.gid), image: profile.photo?.image_128x128 || undefined, emailVerified: !!profile.email, createdAt: now, @@ -3042,7 +3142,7 @@ export const auth = betterAuth({ return { id: `${uniqueId}-${generateId()}`, name: teamName, - email: `${uniqueId}@slack.bot`, + email: syntheticConnectorEmail('slack', uniqueId), emailVerified: false, createdAt: new Date(), updatedAt: new Date(), @@ -3092,7 +3192,7 @@ export const auth = betterAuth({ return { id: `${uniqueId}-${generateId()}`, name: data.user_name || 'Webflow User', - email: `${uniqueId.replace(/[^a-zA-Z0-9]/g, '')}@webflow.user`, + email: syntheticConnectorEmail('webflow', userId), emailVerified: false, createdAt: now, updatedAt: now, @@ -3139,8 +3239,8 @@ export const auth = betterAuth({ return { id: `${profile.sub}-${generateId()}`, name: profile.name || 'LinkedIn User', - email: profile.email || `${profile.sub}@linkedin.user`, - emailVerified: profile.email_verified || true, + email: profile.email || syntheticConnectorEmail('linkedin', profile.sub), + emailVerified: true, image: profile.picture || undefined, createdAt: new Date(), updatedAt: new Date(), @@ -3190,7 +3290,7 @@ export const auth = betterAuth({ id: `${profile.id.toString()}-${generateId()}`, name: `${profile.first_name || ''} ${profile.last_name || ''}`.trim() || 'Zoom User', - email: profile.email || `${profile.id}@zoom.user`, + email: profile.email || syntheticConnectorEmail('zoom', profile.id), emailVerified: profile.verified === 1, image: profile.pic_url || undefined, createdAt: new Date(), @@ -3238,7 +3338,7 @@ export const auth = betterAuth({ return { id: `${profile.id.toString()}-${generateId()}`, name: profile.display_name || 'Spotify User', - email: profile.email || `${profile.id}@spotify.user`, + email: profile.email || syntheticConnectorEmail('spotify', profile.id), emailVerified: true, image: profile.images?.[0]?.url || undefined, createdAt: new Date(), @@ -3286,7 +3386,12 @@ export const auth = betterAuth({ return { id: `${profile.ID?.toString() || profile.id?.toString()}-${generateId()}`, name: profile.display_name || profile.username || 'WordPress User', - email: profile.email || `${profile.username}@wordpress.com`, + email: + profile.email || + syntheticConnectorEmail( + 'wordpress', + profile.username ?? profile.ID ?? profile.id + ), emailVerified: profile.email_verified || false, image: profile.avatar_URL || undefined, createdAt: new Date(), @@ -3344,7 +3449,7 @@ export const auth = betterAuth({ return { id: `${data.sub}-${generateId()}`, name: data.name || accountName, - email: data.email || `${data.sub}@docusign.com`, + email: data.email || syntheticConnectorEmail('docusign', data.sub), emailVerified: true, image: undefined, createdAt: new Date(), @@ -3395,7 +3500,7 @@ export const auth = betterAuth({ return { id: `${profile.id?.toString()}-${generateId()}`, name: profile.name || 'Cal.com User', - email: profile.email || `${profile.id}@cal.com`, + email: profile.email || syntheticConnectorEmail('calcom', profile.id), emailVerified: true, createdAt: new Date(), updatedAt: new Date(), diff --git a/apps/sim/lib/auth/connector-email.test.ts b/apps/sim/lib/auth/connector-email.test.ts new file mode 100644 index 00000000000..a9941c70a40 --- /dev/null +++ b/apps/sim/lib/auth/connector-email.test.ts @@ -0,0 +1,79 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { syntheticConnectorEmail } from '@/lib/auth/connector-email' + +describe('syntheticConnectorEmail', () => { + it('namespaces the address by provider and identity', () => { + expect(syntheticConnectorEmail('attio', 'abc123')).toBe('attio-abc123@connectors.sim.invalid') + }) + + it('always lands on the RFC 2606 reserved .invalid TLD', () => { + const providers: Array<[string, string]> = [ + ['x', 'someuser'], + ['hubspot', '12345'], + ['salesforce', '005xx'], + ['docusign', 'sub-1'], + ['calcom', '77'], + ['atlassian', 'acct'], + ['wordpress', 'blogger'], + ] + for (const [provider, id] of providers) { + const email = syntheticConnectorEmail(provider, id) + expect(email.endsWith('@connectors.sim.invalid')).toBe(true) + } + }) + + it('never emits a live third-party domain', () => { + const email = syntheticConnectorEmail('x', 'jack') + expect(email).not.toMatch(/@(x|hubspot|docusign|cal|salesforce|atlassian|wordpress)\.com$/) + }) + + it('distinguishes the same external id across providers', () => { + expect(syntheticConnectorEmail('zoom', '42')).not.toBe(syntheticConnectorEmail('spotify', '42')) + }) + + it('is deterministic for the same input', () => { + expect(syntheticConnectorEmail('monday', 99)).toBe(syntheticConnectorEmail('monday', 99)) + }) + + it('accepts numeric identifiers', () => { + expect(syntheticConnectorEmail('monday', 99)).toBe('monday-99@connectors.sim.invalid') + }) + + it('strips characters that are illegal in an unquoted local part', () => { + expect(syntheticConnectorEmail('slack', 'T123-usr_U456')).toBe( + 'slack-T123-usr_U456@connectors.sim.invalid' + ) + expect(syntheticConnectorEmail('reddit', 'some user!@#')).toBe( + 'reddit-someuser@connectors.sim.invalid' + ) + }) + + it('keeps the local part inside the RFC 5321 64-character limit', () => { + const email = syntheticConnectorEmail('a'.repeat(100), 'b'.repeat(100)) + const [localPart] = email.split('@') + expect(localPart.length).toBeLessThanOrEqual(64) + }) + + it('does not leave a dot or hyphen at either edge of a truncated segment', () => { + const email = syntheticConnectorEmail('wealthbox', `${'c'.repeat(29)}...tail`) + const [localPart] = email.split('@') + expect(localPart.endsWith('.')).toBe(false) + expect(localPart.startsWith('.')).toBe(false) + }) + + it('falls back to placeholders rather than emitting an empty local part', () => { + expect(syntheticConnectorEmail('notion', undefined)).toBe( + 'notion-unknown@connectors.sim.invalid' + ) + expect(syntheticConnectorEmail('notion', '')).toBe('notion-unknown@connectors.sim.invalid') + expect(syntheticConnectorEmail('', '')).toBe('connector-unknown@connectors.sim.invalid') + expect(syntheticConnectorEmail('!!!', '###')).toBe('connector-unknown@connectors.sim.invalid') + }) + + it('always returns a truthy value, which is what Better Auth 1.6.23 requires', () => { + expect(syntheticConnectorEmail('', undefined)).toBeTruthy() + }) +}) diff --git a/apps/sim/lib/auth/connector-email.ts b/apps/sim/lib/auth/connector-email.ts new file mode 100644 index 00000000000..6f9725a61fe --- /dev/null +++ b/apps/sim/lib/auth/connector-email.ts @@ -0,0 +1,61 @@ +/** RFC 2606 §2 reserved TLD — permanently unregistrable and unroutable. */ +const SYNTHETIC_EMAIL_DOMAIN = 'connectors.sim.invalid' + +/** Longest local-part segment kept, so the address stays under the 64-char RFC 5321 limit. */ +const MAX_SEGMENT_LENGTH = 30 + +/** + * Reduce an arbitrary upstream identifier to characters that are unambiguously + * legal in an unquoted email local part. + */ +function sanitizeLocalPart(value: string): string { + return ( + value + .replace(/[^a-zA-Z0-9._-]/g, '') + .slice(0, MAX_SEGMENT_LENGTH) + // RFC 5321 `dot-string` is `Atom *("." Atom)`, so a run of separators is not + // a legal local part — and stripping illegal characters readily creates one. + .replace(/[._-]{2,}/g, '-') + .replace(/^[._-]+|[._-]+$/g, '') + ) +} + +/** + * Synthetic placeholder email for an OAuth connector identity. + * + * Many connector providers either never expose an email (X, Slack bot tokens, + * TikTok, Reddit, Webflow) or expose one only when an optional scope was + * granted. Better Auth still demands one: in `better-auth@1.6.23`, + * `dist/plugins/generic-oauth/routes.mjs` hard-rejects a falsy `email` returned + * from `getUserInfo` by throwing a redirect to `?error=email_is_missing`. There + * is no option to disable that guard, so every `getUserInfo` must return a + * truthy address or the connect flow dies at the callback. + * + * The value is never persisted. Sim's connectors go through the session-bound + * `oauth2.link` path, the `account` table has no email column, and + * `updateUserInfoOnLink` is unset — so Better Auth reads the address, satisfies + * its own guard, and discards it. It is never shown to a user, never mailed to, + * and never matched against a real account. + * + * The domain is `.invalid`, reserved by RFC 2606 §2 precisely so that it can + * never be registered or routed. Earlier code synthesized addresses on live + * third-party domains (`@x.com`, `@salesforce.com`, `@atlassian.com`, …), which + * are owned by other companies and could in principle resolve to a real + * mailbox. + * + * Delete this helper and return the upstream email directly once Better Auth + * relaxes the guard (tracked in better-auth issue #9124, slated for v2). + * + * @param providerId - Connector provider id, e.g. `'attio'`. Namespaces the + * address so two providers reporting the same external id do not collide. + * @param stableId - Stable external identifier for the connected identity + * (workspace member id, account id, username, …). Falsy or fully-unsupported + * values degrade to `unknown`; uniqueness is best-effort because the address + * is discarded either way. + * @returns An RFC 5321-shaped address on a permanently unroutable domain. + */ +export function syntheticConnectorEmail(providerId: string, stableId?: string | number): string { + const provider = sanitizeLocalPart(providerId) || 'connector' + const identity = sanitizeLocalPart(stableId == null ? '' : String(stableId)) || 'unknown' + return `${provider}-${identity}@${SYNTHETIC_EMAIL_DOMAIN}` +} diff --git a/apps/sim/lib/credentials/atlassian-service-account.ts b/apps/sim/lib/credentials/atlassian-service-account.ts index 1d78cd8bd6b..fa4381d31a9 100644 --- a/apps/sim/lib/credentials/atlassian-service-account.ts +++ b/apps/sim/lib/credentials/atlassian-service-account.ts @@ -80,7 +80,16 @@ async function assertAtlassianResponseOk( export async function validateAtlassianServiceAccount( apiToken: string, domain: string -): Promise<{ accountId: string; displayName: string; cloudId: string }> { +): Promise<{ + accountId: string + displayName: string + cloudId: string + /** + * Only present when the site's profile-visibility settings expose it to the + * calling token; absence is never a validation failure. + */ + emailAddress?: string +}> { assertAtlassianCloudHost(domain) const tenantInfoRes = await fetch(`https://${domain}/_edge/tenant_info`, { @@ -123,5 +132,6 @@ export async function validateAtlassianServiceAccount( accountId: myself.accountId, displayName: myself.displayName || myself.emailAddress || domain, cloudId, + ...(myself.emailAddress ? { emailAddress: myself.emailAddress } : {}), } } diff --git a/apps/sim/lib/credentials/client-credential-accounts/minters/box.test.ts b/apps/sim/lib/credentials/client-credential-accounts/minters/box.test.ts index f380e8d8742..eedc622da87 100644 --- a/apps/sim/lib/credentials/client-credential-accounts/minters/box.test.ts +++ b/apps/sim/lib/credentials/client-credential-accounts/minters/box.test.ts @@ -67,6 +67,7 @@ describe('mintBoxServiceAccountToken', () => { .mockResolvedValueOnce(jsonResponse(200, { access_token: 'box-access', expires_in: 3600 })) .mockResolvedValueOnce( jsonResponse(200, { + id: '33445566', name: 'Sim Automation', login: 'AutomationUser_123_abc@boxdevedition.com', }) @@ -79,14 +80,13 @@ describe('mintBoxServiceAccountToken', () => { expiresInSeconds: 3600, identity: { displayName: 'Sim Automation', - auditMetadata: { - boxEnterpriseId: '1234567', - boxServiceAccountLogin: 'AutomationUser_123_abc@boxdevedition.com', - }, - storedMetadata: { - enterpriseId: '1234567', - serviceAccountLogin: 'AutomationUser_123_abc@boxdevedition.com', + principal: { + kind: 'user', + id: '33445566', + label: 'AutomationUser_123_abc@boxdevedition.com', }, + auditMetadata: { boxEnterpriseId: '1234567' }, + storedMetadata: { enterpriseId: '1234567' }, }, }) expect(mockFetch).toHaveBeenCalledTimes(2) @@ -94,7 +94,7 @@ describe('mintBoxServiceAccountToken', () => { expectIdentityCall() }) - it('still succeeds with a fallback identity when users/me fails', async () => { + it('marks the principal as lookup_failed when users/me fails', async () => { mockFetch .mockResolvedValueOnce(jsonResponse(200, { access_token: 'box-access', expires_in: 2400 })) .mockResolvedValueOnce(jsonResponse(500, { message: 'boom' })) @@ -105,7 +105,22 @@ describe('mintBoxServiceAccountToken', () => { expect(result.expiresInSeconds).toBe(2400) expect(result.identity).toEqual({ displayName: 'Box enterprise 1234567', + principal: { kind: 'lookup_failed', reason: 'HTTP 500' }, auditMetadata: { boxEnterpriseId: '1234567' }, + storedMetadata: { enterpriseId: '1234567' }, + }) + }) + + it('marks the principal as lookup_failed when users/me omits the user id', async () => { + mockFetch + .mockResolvedValueOnce(jsonResponse(200, { access_token: 'box-access', expires_in: 3600 })) + .mockResolvedValueOnce(jsonResponse(200, { name: 'Sim Automation' })) + + const result = await mintBoxServiceAccountToken(FIELDS) + + expect(result.identity?.principal).toEqual({ + kind: 'lookup_failed', + reason: 'response missing user id', }) }) @@ -118,6 +133,10 @@ describe('mintBoxServiceAccountToken', () => { expect(result.accessToken).toBe('box-access') expect(result.identity?.displayName).toBe('Box enterprise 1234567') + expect(result.identity?.principal).toEqual({ + kind: 'lookup_failed', + reason: 'provider_unavailable (HTTP 502)', + }) }) it('throws invalid_credentials on 400 invalid_client', async () => { diff --git a/apps/sim/lib/credentials/client-credential-accounts/minters/box.ts b/apps/sim/lib/credentials/client-credential-accounts/minters/box.ts index 720631d072d..04cb73b2f85 100644 --- a/apps/sim/lib/credentials/client-credential-accounts/minters/box.ts +++ b/apps/sim/lib/credentials/client-credential-accounts/minters/box.ts @@ -1,3 +1,5 @@ +import { createLogger } from '@sim/logger' +import { getErrorMessage } from '@sim/utils/errors' import type { ClientCredentialAccountFields, ClientCredentialAccountIdentity, @@ -8,10 +10,15 @@ import { fetchProvider, isTransientProviderStatus, parseProviderJson, + providerFailureReason, readProviderErrorSnippet, TokenServiceAccountValidationError, } from '@/lib/credentials/token-service-accounts/errors' +const logger = createLogger('BoxServiceAccountMinter') + +const IDENTITY_STEP = 'box_identity' + const BOX_TOKEN_URL = 'https://api.box.com/oauth2/token' const BOX_CURRENT_USER_URL = 'https://api.box.com/2.0/users/me' @@ -20,7 +27,14 @@ interface BoxTokenResponse { expires_in?: number } +/** + * `id`, `name`, and `login` are all in the standard field set `GET /2.0/users/me` + * returns without a `fields` parameter, so capturing the Service Account's user + * id costs no extra request. + * @see https://developer.box.com/reference/get-users-me/ + */ interface BoxCurrentUserResponse { + id?: string name?: string login?: string } @@ -53,40 +67,61 @@ function boxErrorHint(body: string): string | undefined { /** * Best-effort identity lookup for the app's Service Account user. A failure - * never fails the mint — the caller falls back to an Enterprise-ID-derived - * display name. + * never fails the mint — the credential degrades to an Enterprise-ID-derived + * display name with a `lookup_failed` principal, so the audit record shows the + * identity was not captured rather than implying none exists. */ async function fetchBoxServiceAccountIdentity( accessToken: string, orgId: string ): Promise { - const fallback: ClientCredentialAccountIdentity = { + const degraded = (reason: string): ClientCredentialAccountIdentity => ({ displayName: `Box enterprise ${orgId}`, + principal: { kind: 'lookup_failed', reason }, auditMetadata: { boxEnterpriseId: orgId }, - } + storedMetadata: { enterpriseId: orgId }, + }) try { const res = await fetchProvider( BOX_CURRENT_USER_URL, { headers: { Authorization: `Bearer ${accessToken}`, Accept: 'application/json' } }, - 'box_identity' + IDENTITY_STEP ) - if (!res.ok) return fallback - const user = await parseProviderJson(res, 'box_identity') + if (!res.ok) { + logger.warn('Box service-account identity lookup failed', { + step: IDENTITY_STEP, + status: res.status, + enterpriseId: orgId, + }) + return degraded(`HTTP ${res.status}`) + } + const user = await parseProviderJson(res, IDENTITY_STEP) + const id = typeof user.id === 'string' && user.id ? user.id : undefined const login = typeof user.login === 'string' && user.login ? user.login : undefined const name = typeof user.name === 'string' && user.name ? user.name : undefined - return { - displayName: name ?? login ?? fallback.displayName, - auditMetadata: { - boxEnterpriseId: orgId, - ...(login ? { boxServiceAccountLogin: login } : {}), - }, - storedMetadata: { + if (!id) { + logger.warn('Box service-account identity response carried no user id', { + step: IDENTITY_STEP, + status: res.status, enterpriseId: orgId, - ...(login ? { serviceAccountLogin: login } : {}), - }, + }) + return degraded('response missing user id') } - } catch { - return fallback + return { + displayName: name ?? login ?? `Box enterprise ${orgId}`, + // The Service Account is a real Box user; `enterpriseId` is shared by + // every app in the enterprise and so is kept as separate context. + principal: { kind: 'user', id, ...(login ? { label: login } : {}) }, + auditMetadata: { boxEnterpriseId: orgId }, + storedMetadata: { enterpriseId: orgId }, + } + } catch (error) { + logger.warn('Box service-account identity lookup threw', { + step: IDENTITY_STEP, + enterpriseId: orgId, + error: getErrorMessage(error), + }) + return degraded(providerFailureReason(error)) } } diff --git a/apps/sim/lib/credentials/client-credential-accounts/minters/salesforce.test.ts b/apps/sim/lib/credentials/client-credential-accounts/minters/salesforce.test.ts index 6e0aad65f22..997f692206b 100644 --- a/apps/sim/lib/credentials/client-credential-accounts/minters/salesforce.test.ts +++ b/apps/sim/lib/credentials/client-credential-accounts/minters/salesforce.test.ts @@ -78,6 +78,7 @@ describe('mintSalesforceServiceAccountToken', () => { name: 'Integration User', preferred_username: 'integration@yourorg.com', organization_id: '00Dxx0000000001EAA', + user_id: '005xx000001Sv6DAAS', }) ) @@ -90,16 +91,19 @@ describe('mintSalesforceServiceAccountToken', () => { grantedScopes: ['api'], identity: { displayName: 'Integration User', + principal: { + kind: 'user', + id: '005xx000001Sv6DAAS', + label: 'integration@yourorg.com', + }, auditMetadata: { salesforceMyDomainHost: HOST, salesforceOrgId: '00Dxx0000000001EAA', - salesforceRunAsUsername: 'integration@yourorg.com', }, storedMetadata: { myDomainHost: HOST, instanceUrl: INSTANCE_URL, orgId: '00Dxx0000000001EAA', - runAsUsername: 'integration@yourorg.com', grantedScopes: 'api', }, }, @@ -259,7 +263,7 @@ describe('mintSalesforceServiceAccountToken', () => { }) }) - it('falls back to a host-derived identity when the userinfo call fails', async () => { + it('marks the principal as lookup_failed when the userinfo call throws', async () => { mockFetch .mockResolvedValueOnce( jsonResponse(200, { access_token: 'sf-access', instance_url: INSTANCE_URL }) @@ -271,11 +275,27 @@ describe('mintSalesforceServiceAccountToken', () => { expect(result.accessToken).toBe('sf-access') expect(result.identity).toEqual({ displayName: `Salesforce ${HOST}`, + principal: { kind: 'lookup_failed', reason: 'provider_unavailable (HTTP 502)' }, auditMetadata: { salesforceMyDomainHost: HOST }, storedMetadata: { myDomainHost: HOST, instanceUrl: INSTANCE_URL }, }) }) + it('marks the principal as lookup_failed when userinfo omits user_id', async () => { + mockFetch + .mockResolvedValueOnce( + jsonResponse(200, { access_token: 'sf-access', instance_url: INSTANCE_URL }) + ) + .mockResolvedValueOnce(jsonResponse(200, { name: 'Integration User' })) + + const result = await mintSalesforceServiceAccountToken(FIELDS) + + expect(result.identity?.principal).toEqual({ + kind: 'lookup_failed', + reason: 'response missing user_id', + }) + }) + it('ignores a non-Salesforce instance_url and falls back to the validated host', async () => { mockFetch .mockResolvedValueOnce( diff --git a/apps/sim/lib/credentials/client-credential-accounts/minters/salesforce.ts b/apps/sim/lib/credentials/client-credential-accounts/minters/salesforce.ts index 8928cf9d9c6..0c87a47d2a0 100644 --- a/apps/sim/lib/credentials/client-credential-accounts/minters/salesforce.ts +++ b/apps/sim/lib/credentials/client-credential-accounts/minters/salesforce.ts @@ -1,3 +1,5 @@ +import { createLogger } from '@sim/logger' +import { getErrorMessage } from '@sim/utils/errors' import { normalizeSalesforceMyDomainHost, SALESFORCE_MY_DOMAIN_HOST_REGEX, @@ -8,10 +10,12 @@ import type { ClientCredentialAccountMintOptions, ClientCredentialAccountMintResult, } from '@/lib/credentials/client-credential-accounts/server' +import { userPrincipal } from '@/lib/credentials/principal' import { fetchProvider, isTransientProviderStatus, parseProviderJson, + providerFailureReason, readProviderErrorSnippet, TokenServiceAccountValidationError, } from '@/lib/credentials/token-service-accounts/errors' @@ -24,16 +28,29 @@ import { */ const SALESFORCE_TOKEN_TTL_SECONDS = 600 +const IDENTITY_STEP = 'salesforce_identity' + +const logger = createLogger('SalesforceServiceAccountMinter') + interface SalesforceTokenResponse { access_token?: string instance_url?: string scope?: string } +/** + * `/services/oauth2/userinfo` returns `user_id`, `organization_id`, + * `preferred_username`, and `name` in the same call the display name already + * needs, so capturing the run-as user id costs no extra request. `sub` is + * deliberately unused — Salesforce documents it as the UserInfo endpoint URL, + * not a subject identifier. + * @see https://help.salesforce.com/s/articleView?id=sf.remoteaccess_using_userinfo_endpoint.htm&type=5 + */ interface SalesforceUserinfoResponse { name?: string preferred_username?: string organization_id?: string + user_id?: string } /** @@ -91,27 +108,37 @@ function salesforceTokenTtlSeconds(accessToken: string): number { /** * Best-effort identity lookup for the run-as integration user via the - * standard userinfo endpoint. A failure never fails the mint — the caller - * falls back to a host-derived display name. + * standard userinfo endpoint. A failure never fails the mint — the credential + * degrades to a host-derived display name with a `lookup_failed` principal, so + * the audit record shows the identity was not captured rather than implying + * none exists. */ async function fetchSalesforceIdentity( accessToken: string, instanceUrl: string, host: string ): Promise { - const fallback: ClientCredentialAccountIdentity = { + const degraded = (reason: string): ClientCredentialAccountIdentity => ({ displayName: `Salesforce ${host}`, + principal: { kind: 'lookup_failed', reason }, auditMetadata: { salesforceMyDomainHost: host }, storedMetadata: { myDomainHost: host, instanceUrl }, - } + }) try { const res = await fetchProvider( `${instanceUrl}/services/oauth2/userinfo`, { headers: { Authorization: `Bearer ${accessToken}`, Accept: 'application/json' } }, - 'salesforce_identity' + IDENTITY_STEP ) - if (!res.ok) return fallback - const user = await parseProviderJson(res, 'salesforce_identity') + if (!res.ok) { + logger.warn('Salesforce run-as identity lookup failed', { + step: IDENTITY_STEP, + status: res.status, + host, + }) + return degraded(`HTTP ${res.status}`) + } + const user = await parseProviderJson(res, IDENTITY_STEP) const username = typeof user.preferred_username === 'string' && user.preferred_username ? user.preferred_username @@ -121,22 +148,37 @@ async function fetchSalesforceIdentity( typeof user.organization_id === 'string' && user.organization_id ? user.organization_id : undefined + const userId = typeof user.user_id === 'string' && user.user_id ? user.user_id : undefined + if (!userId) { + logger.warn('Salesforce userinfo response carried no user_id', { + step: IDENTITY_STEP, + status: res.status, + host, + }) + return degraded('response missing user_id') + } return { - displayName: name ?? username ?? fallback.displayName, + displayName: name ?? username ?? `Salesforce ${host}`, + // The 18-char user id is immutable; `preferred_username` is renameable, + // so it is only a label. + principal: userPrincipal(userId, username), auditMetadata: { salesforceMyDomainHost: host, ...(orgId ? { salesforceOrgId: orgId } : {}), - ...(username ? { salesforceRunAsUsername: username } : {}), }, storedMetadata: { myDomainHost: host, instanceUrl, ...(orgId ? { orgId } : {}), - ...(username ? { runAsUsername: username } : {}), }, } - } catch { - return fallback + } catch (error) { + logger.warn('Salesforce run-as identity lookup threw', { + step: IDENTITY_STEP, + host, + error: getErrorMessage(error), + }) + return degraded(providerFailureReason(error)) } } diff --git a/apps/sim/lib/credentials/client-credential-accounts/minters/zoho-desk.test.ts b/apps/sim/lib/credentials/client-credential-accounts/minters/zoho-desk.test.ts index 839e452202a..6bf48fd65c1 100644 --- a/apps/sim/lib/credentials/client-credential-accounts/minters/zoho-desk.test.ts +++ b/apps/sim/lib/credentials/client-credential-accounts/minters/zoho-desk.test.ts @@ -130,12 +130,9 @@ describe('mintZohoDeskServiceAccountToken', () => { grantedScopes: ['Desk.tickets.READ', 'Desk.contacts.READ'], identity: { displayName: 'Zoho Desk org 600123456', - auditMetadata: { - zohoDeskSoid: 'ZohoDesk.600123456', - zohoDeskClientId: 'zoho-cid', - }, + principal: { kind: 'tenant', id: 'ZohoDesk.600123456' }, + auditMetadata: { zohoDeskClientId: 'zoho-cid' }, storedMetadata: { - soid: 'ZohoDesk.600123456', apiDomain: 'https://desk.zoho.com', dataCenter: 'us', grantedScopes: 'Desk.tickets.READ Desk.contacts.READ', diff --git a/apps/sim/lib/credentials/client-credential-accounts/minters/zoho-desk.ts b/apps/sim/lib/credentials/client-credential-accounts/minters/zoho-desk.ts index 35f3f6f7963..7ceff1748a9 100644 --- a/apps/sim/lib/credentials/client-credential-accounts/minters/zoho-desk.ts +++ b/apps/sim/lib/credentials/client-credential-accounts/minters/zoho-desk.ts @@ -12,6 +12,7 @@ import type { ClientCredentialAccountMintOptions, ClientCredentialAccountMintResult, } from '@/lib/credentials/client-credential-accounts/server' +import { tenantPrincipal } from '@/lib/credentials/principal' import { fetchProvider, isTransientProviderStatus, @@ -255,7 +256,7 @@ export async function mintZohoDeskServiceAccountToken( return { accessToken: payload.access_token, expiresInSeconds, apiDomain, grantedScopes } } - const storedMetadata: Record = { soid, apiDomain, dataCenter: dataCenter.id } + const storedMetadata: Record = { apiDomain, dataCenter: dataCenter.id } if (grantedScopes?.length) { storedMetadata.grantedScopes = grantedScopes.join(' ') } @@ -267,7 +268,10 @@ export async function mintZohoDeskServiceAccountToken( grantedScopes, identity: { displayName: `Zoho Desk org ${fields.orgId.trim()}`, - auditMetadata: { zohoDeskSoid: soid, zohoDeskClientId: fields.clientId }, + // The Self Client grant is scoped to the organization (`soid`) and never + // hits the Accounts profile endpoint, so no agent identity exists here. + principal: tenantPrincipal(soid), + auditMetadata: { zohoDeskClientId: fields.clientId }, storedMetadata, }, } diff --git a/apps/sim/lib/credentials/client-credential-accounts/minters/zoom.test.ts b/apps/sim/lib/credentials/client-credential-accounts/minters/zoom.test.ts index dcfd2822a14..afa900ed610 100644 --- a/apps/sim/lib/credentials/client-credential-accounts/minters/zoom.test.ts +++ b/apps/sim/lib/credentials/client-credential-accounts/minters/zoom.test.ts @@ -74,7 +74,8 @@ describe('mintZoomServiceAccountToken', () => { grantedScopes: ['meeting:read:meeting:admin', 'user:read:user:admin'], identity: { displayName: 'Zoom account AbCdEf123', - auditMetadata: { zoomAccountId: 'AbCdEf123', zoomClientId: 'zoom-cid' }, + principal: { kind: 'tenant', id: 'AbCdEf123' }, + auditMetadata: { zoomClientId: 'zoom-cid' }, storedMetadata: { apiUrl: 'https://api.zoom.us', grantedScopes: 'meeting:read:meeting:admin user:read:user:admin', diff --git a/apps/sim/lib/credentials/client-credential-accounts/minters/zoom.ts b/apps/sim/lib/credentials/client-credential-accounts/minters/zoom.ts index 978409ae0ee..218eee37c1e 100644 --- a/apps/sim/lib/credentials/client-credential-accounts/minters/zoom.ts +++ b/apps/sim/lib/credentials/client-credential-accounts/minters/zoom.ts @@ -3,6 +3,7 @@ import type { ClientCredentialAccountMintOptions, ClientCredentialAccountMintResult, } from '@/lib/credentials/client-credential-accounts/server' +import { tenantPrincipal } from '@/lib/credentials/principal' import { fetchProvider, isTransientProviderStatus, @@ -124,7 +125,10 @@ export async function mintZoomServiceAccountToken( grantedScopes, identity: { displayName: `Zoom account ${fields.orgId}`, - auditMetadata: { zoomAccountId: fields.orgId, zoomClientId: fields.clientId }, + // A Server-to-Server app authenticates as the account, not as a Zoom + // user; the grant exposes no user identifier at all. + principal: tenantPrincipal(fields.orgId), + auditMetadata: { zoomClientId: fields.clientId }, ...(Object.keys(storedMetadata).length > 0 ? { storedMetadata } : {}), }, } diff --git a/apps/sim/lib/credentials/client-credential-accounts/server.ts b/apps/sim/lib/credentials/client-credential-accounts/server.ts index 44b216f8406..2a1f6bd2214 100644 --- a/apps/sim/lib/credentials/client-credential-accounts/server.ts +++ b/apps/sim/lib/credentials/client-credential-accounts/server.ts @@ -11,6 +11,7 @@ import { mintBoxServiceAccountToken } from '@/lib/credentials/client-credential- import { mintSalesforceServiceAccountToken } from '@/lib/credentials/client-credential-accounts/minters/salesforce' import { mintZohoDeskServiceAccountToken } from '@/lib/credentials/client-credential-accounts/minters/zoho-desk' import { mintZoomServiceAccountToken } from '@/lib/credentials/client-credential-accounts/minters/zoom' +import type { ServiceAccountPrincipal } from '@/lib/credentials/principal' /** Raw fields a client-credential minter receives (already trimmed). */ export interface ClientCredentialAccountFields { @@ -33,11 +34,21 @@ export interface ClientCredentialAccountFields { export interface ClientCredentialAccountIdentity { /** Default display name when the user didn't provide one. */ displayName: string - /** Non-secret identifiers recorded in the audit log (e.g. account/enterprise id). */ + /** + * Identity the minted token acts as, or `null` when the provider exposes + * none. Required (never optional) so a new minter cannot be written without + * deciding. `verifyAndBuildServiceAccountSecret` mirrors it into both + * `auditMetadata` and `storedMetadata`, so minters must not repeat it. + */ + principal: ServiceAccountPrincipal | null + /** + * Non-secret identifiers recorded in the audit log that are NOT the + * principal (e.g. the enterprise id behind a service-account user). + */ auditMetadata: Record /** * Non-secret metadata persisted inside the encrypted blob alongside the - * credentials (e.g. regional API host, service-account login) for debugging. + * credentials (e.g. regional API host, granted scopes) for debugging. */ storedMetadata?: Record } diff --git a/apps/sim/lib/credentials/display-name.ts b/apps/sim/lib/credentials/display-name.ts index 9b946f31e56..24f47acb279 100644 --- a/apps/sim/lib/credentials/display-name.ts +++ b/apps/sim/lib/credentials/display-name.ts @@ -47,3 +47,15 @@ export function defaultCredentialDisplayName( } return base } + +/** + * Display name for a custom Slack bot credential. + * + * Lives in this leaf module because two callers must derive it identically — + * the secret builder that sets it at connect time, and the update path that + * compares against it to tell a stale system-derived label from one a user + * typed. A copied literal would silently break that comparison. + */ +export function slackCustomBotDisplayName(teamName?: string | null): string { + return teamName || 'Slack bot' +} diff --git a/apps/sim/lib/credentials/orchestration/index.test.ts b/apps/sim/lib/credentials/orchestration/index.test.ts new file mode 100644 index 00000000000..18e0cfb0d1d --- /dev/null +++ b/apps/sim/lib/credentials/orchestration/index.test.ts @@ -0,0 +1,298 @@ +/** + * @vitest-environment node + */ +import { dbChainMockFns, queueTableRows, resetDbChainMock, schemaMock } from '@sim/testing' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { + mockRecordAudit, + mockGetCredentialActorContext, + mockDecryptSecret, + mockVerifyAndBuildServiceAccountSecret, + mockIsClientCredentialAccountProviderId, +} = vi.hoisted(() => ({ + mockRecordAudit: vi.fn(), + mockGetCredentialActorContext: vi.fn(), + mockDecryptSecret: vi.fn(), + mockVerifyAndBuildServiceAccountSecret: vi.fn(), + mockIsClientCredentialAccountProviderId: vi.fn(() => false), +})) + +vi.mock('@sim/audit', () => ({ + AuditAction: { CREDENTIAL_UPDATED: 'credential.updated' }, + AuditResourceType: { CREDENTIAL: 'credential' }, + recordAudit: mockRecordAudit, +})) +vi.mock('@/lib/credentials/access', () => ({ + getCredentialActorContext: mockGetCredentialActorContext, +})) +vi.mock('@/lib/core/security/encryption', () => ({ decryptSecret: mockDecryptSecret })) +vi.mock('@/lib/credentials/service-account-secret', () => ({ + verifyAndBuildServiceAccountSecret: mockVerifyAndBuildServiceAccountSecret, + ServiceAccountSecretError: class ServiceAccountSecretError extends Error {}, +})) +vi.mock('@/lib/credentials/client-credential-accounts/descriptors', () => ({ + isClientCredentialAccountProviderId: mockIsClientCredentialAccountProviderId, +})) +vi.mock('@/lib/credentials/deletion', () => ({ deleteCredential: vi.fn() })) +vi.mock('@/lib/credentials/environment', () => ({ + deleteWorkspaceEnvCredentials: vi.fn(), + syncPersonalEnvCredentialsForUser: vi.fn(), +})) +vi.mock('@/lib/credentials/atlassian-service-account', () => ({ + AtlassianValidationError: class AtlassianValidationError extends Error {}, +})) +vi.mock('@/lib/credentials/token-service-accounts/errors', () => ({ + TokenServiceAccountValidationError: class TokenServiceAccountValidationError extends Error {}, +})) +vi.mock('@/lib/posthog/server', () => ({ captureServerEvent: vi.fn() })) + +import { performUpdateCredential } from '@/lib/credentials/orchestration' + +const OLD_EMAIL = 'old-sa@old-project.iam.gserviceaccount.com' +const NEW_EMAIL = 'new-sa@new-project.iam.gserviceaccount.com' + +const NEW_GOOGLE_KEY = JSON.stringify({ + type: 'service_account', + client_email: NEW_EMAIL, + private_key: 'pk', + project_id: 'new-project', +}) + +/** Points `getCredentialActorContext` at an admin-accessible credential row. */ +function mockCredential(overrides: Record = {}) { + mockGetCredentialActorContext.mockResolvedValue({ + credential: { + id: 'cred-1', + workspaceId: 'ws-1', + type: 'service_account', + providerId: 'google-service-account', + displayName: OLD_EMAIL, + ...overrides, + }, + hasWorkspaceAccess: true, + isAdmin: true, + }) +} + +/** Queues the stored (pre-rotation) secret blob for the orchestration's read. */ +function mockStoredBlob(blob: unknown) { + queueTableRows(schemaMock.credential, [{ key: 'stored-cipher' }]) + mockDecryptSecret.mockResolvedValue({ decrypted: JSON.stringify(blob) }) +} + +/** + * The `set(...)` payload of the credential UPDATE — always the first mutation, + * ahead of the Slack bot-user-id propagation to webhooks. + */ +function updatePayload(): Record { + const call = dbChainMockFns.set.mock.calls[0] + return (call?.[0] ?? {}) as Record +} + +/** The metadata recorded on the CREDENTIAL_UPDATED audit entry. */ +function auditMetadata(): Record { + const call = mockRecordAudit.mock.calls.at(-1) + return ((call?.[0] as { metadata?: Record })?.metadata ?? {}) as Record< + string, + unknown + > +} + +describe('performUpdateCredential — service-account secret rotation', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + mockIsClientCredentialAccountProviderId.mockReturnValue(false) + mockVerifyAndBuildServiceAccountSecret.mockResolvedValue({ + providerId: 'google-service-account', + encryptedServiceAccountKey: 'new-cipher', + displayName: NEW_EMAIL, + auditMetadata: { principalKind: 'user', principalId: NEW_EMAIL }, + }) + }) + + it('re-labels a Google credential whose name is still the previous key identity', async () => { + mockCredential() + mockStoredBlob({ type: 'service_account', client_email: OLD_EMAIL }) + + const result = await performUpdateCredential({ + credentialId: 'cred-1', + userId: 'user-1', + serviceAccountJson: NEW_GOOGLE_KEY, + }) + + expect(result.success).toBe(true) + expect(updatePayload().displayName).toBe(NEW_EMAIL) + expect(updatePayload().encryptedServiceAccountKey).toBe('new-cipher') + expect(result.updatedFields).toContain('displayName') + expect(result.previousDisplayName).toBe(OLD_EMAIL) + }) + + it('keeps a label the user typed instead of the derived identity', async () => { + mockCredential({ displayName: 'Prod billing exporter' }) + mockStoredBlob({ type: 'service_account', client_email: OLD_EMAIL }) + + const result = await performUpdateCredential({ + credentialId: 'cred-1', + userId: 'user-1', + serviceAccountJson: NEW_GOOGLE_KEY, + }) + + expect(result.success).toBe(true) + expect(updatePayload()).not.toHaveProperty('displayName') + expect(result.updatedFields).not.toContain('displayName') + }) + + it('lets an explicit displayName in the same request win over the derived one', async () => { + mockCredential() + mockStoredBlob({ type: 'service_account', client_email: OLD_EMAIL }) + + await performUpdateCredential({ + credentialId: 'cred-1', + userId: 'user-1', + displayName: 'Renamed by admin', + serviceAccountJson: NEW_GOOGLE_KEY, + }) + + expect(updatePayload().displayName).toBe('Renamed by admin') + // The stored blob is never read when the caller already named the credential. + expect(mockDecryptSecret).not.toHaveBeenCalled() + }) + + it('leaves the label alone when the stored blob carries no recoverable identity', async () => { + mockCredential({ providerId: 'atlassian-service-account', displayName: 'Acme Jira' }) + mockVerifyAndBuildServiceAccountSecret.mockResolvedValue({ + providerId: 'atlassian-service-account', + encryptedServiceAccountKey: 'new-cipher', + displayName: 'Other Site', + auditMetadata: { atlassianCloudId: 'cloud-2' }, + }) + + await performUpdateCredential({ + credentialId: 'cred-1', + userId: 'user-1', + apiToken: 'tok', + domain: 'other.atlassian.net', + }) + + expect(updatePayload()).not.toHaveProperty('displayName') + expect(mockDecryptSecret).not.toHaveBeenCalled() + }) + + it('re-labels a Slack custom bot that still carries its previous team name', async () => { + mockCredential({ providerId: 'slack-custom-bot', displayName: 'Old Team' }) + mockStoredBlob({ type: 'slack_custom_bot', teamName: 'Old Team', teamId: 'T1' }) + mockVerifyAndBuildServiceAccountSecret.mockResolvedValue({ + providerId: 'slack-custom-bot', + encryptedServiceAccountKey: 'new-cipher', + displayName: 'New Team', + auditMetadata: { slackTeamId: 'T2' }, + botUserId: 'U2', + }) + + await performUpdateCredential({ + credentialId: 'cred-1', + userId: 'user-1', + botToken: 'xoxb-new', + signingSecret: 'sig', + }) + + expect(updatePayload().displayName).toBe('New Team') + }) + + it('merges the rebuilt secret audit metadata into the CREDENTIAL_UPDATED entry', async () => { + mockCredential() + mockStoredBlob({ type: 'service_account', client_email: OLD_EMAIL }) + + await performUpdateCredential({ + credentialId: 'cred-1', + userId: 'user-1', + serviceAccountJson: NEW_GOOGLE_KEY, + }) + + expect(auditMetadata()).toMatchObject({ + credentialType: 'service_account', + principalKind: 'user', + principalId: NEW_EMAIL, + }) + expect(auditMetadata().updatedFields).toEqual( + expect.arrayContaining(['displayName', 'encryptedServiceAccountKey']) + ) + }) + + it('never lets provider audit metadata shadow the orchestration keys', async () => { + mockCredential({ providerId: 'atlassian-service-account', displayName: 'Acme Jira' }) + mockVerifyAndBuildServiceAccountSecret.mockResolvedValue({ + providerId: 'atlassian-service-account', + encryptedServiceAccountKey: 'new-cipher', + displayName: 'Acme Jira', + auditMetadata: { credentialType: 'spoofed', updatedFields: 'spoofed' }, + }) + + await performUpdateCredential({ credentialId: 'cred-1', userId: 'user-1', apiToken: 'tok' }) + + expect(auditMetadata().credentialType).toBe('service_account') + expect(auditMetadata().updatedFields).toEqual(['encryptedServiceAccountKey']) + }) + + it('omits secret audit metadata on a metadata-only update', async () => { + mockCredential() + + await performUpdateCredential({ + credentialId: 'cred-1', + userId: 'user-1', + description: 'Billing exports', + }) + + expect(mockVerifyAndBuildServiceAccountSecret).not.toHaveBeenCalled() + expect(auditMetadata()).toEqual({ + credentialType: 'service_account', + updatedFields: ['description'], + }) + }) + + it('carries the stored dataCenter forward for a client-credential reconnect', async () => { + mockCredential({ providerId: 'zoho-desk-service-account', displayName: 'Acme Desk' }) + mockIsClientCredentialAccountProviderId.mockReturnValue(true) + mockStoredBlob({ type: 'client_credential_account', dataCenter: 'eu' }) + mockVerifyAndBuildServiceAccountSecret.mockResolvedValue({ + providerId: 'zoho-desk-service-account', + encryptedServiceAccountKey: 'new-cipher', + displayName: 'Acme Desk', + auditMetadata: { zohoOrgId: 'org-1' }, + }) + + await performUpdateCredential({ + credentialId: 'cred-1', + userId: 'user-1', + clientId: 'cid', + clientSecret: 'csec', + orgId: 'org-1', + }) + + expect(mockVerifyAndBuildServiceAccountSecret).toHaveBeenCalledWith( + 'zoho-desk-service-account', + expect.objectContaining({ dataCenter: 'eu' }) + ) + }) + + it('surfaces a rebuild failure as a validation error and writes nothing', async () => { + mockCredential() + mockStoredBlob({ type: 'service_account', client_email: OLD_EMAIL }) + const { ServiceAccountSecretError } = await import('@/lib/credentials/service-account-secret') + mockVerifyAndBuildServiceAccountSecret.mockRejectedValue( + new ServiceAccountSecretError('Invalid service account JSON') + ) + + const result = await performUpdateCredential({ + credentialId: 'cred-1', + userId: 'user-1', + serviceAccountJson: '{}', + }) + + expect(result).toMatchObject({ success: false, errorCode: 'validation' }) + expect(dbChainMockFns.update).not.toHaveBeenCalled() + expect(mockRecordAudit).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/lib/credentials/orchestration/index.ts b/apps/sim/lib/credentials/orchestration/index.ts index ea69218e38c..b36ca844047 100644 --- a/apps/sim/lib/credentials/orchestration/index.ts +++ b/apps/sim/lib/credentials/orchestration/index.ts @@ -5,11 +5,12 @@ import { createLogger } from '@sim/logger' import { generateId } from '@sim/utils/id' import { and, eq, sql } from 'drizzle-orm' import type { NextRequest } from 'next/server' -import { decryptSecret, encryptSecret } from '@/lib/core/security/encryption' +import { decryptSecret } from '@/lib/core/security/encryption' import { getCredentialActorContext } from '@/lib/credentials/access' import { AtlassianValidationError } from '@/lib/credentials/atlassian-service-account' import { isClientCredentialAccountProviderId } from '@/lib/credentials/client-credential-accounts/descriptors' import { type CredentialDeleteReason, deleteCredential } from '@/lib/credentials/deletion' +import { slackCustomBotDisplayName } from '@/lib/credentials/display-name' import { deleteWorkspaceEnvCredentials, syncPersonalEnvCredentialsForUser, @@ -19,18 +20,40 @@ import { verifyAndBuildServiceAccountSecret, } from '@/lib/credentials/service-account-secret' import { TokenServiceAccountValidationError } from '@/lib/credentials/token-service-accounts/errors' +import { + GOOGLE_SERVICE_ACCOUNT_PROVIDER_ID, + SLACK_CUSTOM_BOT_PROVIDER_ID, + SLACK_CUSTOM_BOT_SECRET_TYPE, +} from '@/lib/oauth/types' import { captureServerEvent } from '@/lib/posthog/server' const logger = createLogger('CredentialOrchestration') /** - * Read the `dataCenter` already stored in a service-account credential's - * encrypted blob. Used on reconnect so a non-secret regional selector survives a - * secret rotation that does not resubmit it. Returns undefined on any failure - - * a blob that cannot be read must not block the reconnect, and the provider's - * own default then applies. + * Google's stored blob is the raw GCP JSON key, whose own `type` discriminator + * is `service_account`. + */ +const GOOGLE_SERVICE_ACCOUNT_KEY_TYPE = 'service_account' + +/** + * Provider ids whose credential `displayName` is derived from the secret's own + * principal at create time AND whose principal is recoverable from the stored + * blob. Only for these can a reconnect tell a stale derived label apart from a + * name the user typed. An empty provider id is a legacy Google service account + * (the original flow predates multi-provider support). + */ +const IDENTITY_DERIVED_DISPLAY_NAME_PROVIDERS: ReadonlySet = new Set([ + GOOGLE_SERVICE_ACCOUNT_PROVIDER_ID, + SLACK_CUSTOM_BOT_PROVIDER_ID, + '', +]) + +/** + * Read and decrypt a service-account credential's stored secret blob. Returns + * null on any failure - a blob that cannot be read must never block a + * reconnect; each caller degrades to the behaviour it had without the blob. */ -async function readStoredDataCenter(credentialId: string): Promise { +async function readStoredSecretBlob(credentialId: string): Promise | null> { try { const rows = await db .select({ key: credential.encryptedServiceAccountKey }) @@ -38,13 +61,41 @@ async function readStoredDataCenter(credentialId: string): Promise) : null } catch { - return undefined + return null + } +} + +/** + * The `dataCenter` already stored in a service-account blob. Used on reconnect + * so a non-secret regional selector survives a secret rotation that does not + * resubmit it; undefined lets the provider's own default apply. + */ +function readStoredDataCenter(blob: Record | null): string | undefined { + const dataCenter = blob?.dataCenter + return typeof dataCenter === 'string' && dataCenter ? dataCenter : undefined +} + +/** + * Recompute the display name that `verifyAndBuildServiceAccountSecret` derived + * from the *stored* secret, so a reconnect can tell whether the current label + * is still the previous principal or a name the user deliberately typed. + * Returns undefined when the blob does not carry its own identity, in which + * case the label must be left alone. + */ +function deriveStoredDisplayName(blob: Record | null): string | undefined { + if (!blob) return undefined + if (blob.type === SLACK_CUSTOM_BOT_SECRET_TYPE) { + return slackCustomBotDisplayName(typeof blob.teamName === 'string' ? blob.teamName : undefined) } + if (blob.type === GOOGLE_SERVICE_ACCOUNT_KEY_TYPE && typeof blob.client_email === 'string') { + return blob.client_email || undefined + } + return undefined } export type CredentialOrchestrationErrorCode = @@ -125,34 +176,13 @@ export async function performUpdateCredential( ) { updates.displayName = params.displayName } - if (params.serviceAccountJson !== undefined && access.credential.type === 'service_account') { - let parsedJson: Record - try { - parsedJson = JSON.parse(params.serviceAccountJson) - } catch { - return { success: false, error: 'Invalid JSON format', errorCode: 'validation' } - } - if ( - parsedJson.type !== 'service_account' || - typeof parsedJson.client_email !== 'string' || - typeof parsedJson.private_key !== 'string' || - typeof parsedJson.project_id !== 'string' - ) { - return { - success: false, - error: 'Invalid service account JSON key', - errorCode: 'validation', - } - } - const { encrypted } = await encryptSecret(params.serviceAccountJson) - updates.encryptedServiceAccountKey = encrypted - } - - // Reconnect: rotate a service-account secret (Slack, Atlassian, or any - // token-paste provider) in place. The - // secret is re-verified against the provider and re-encrypted; the display - // name is preserved (the user may have renamed it). + // Reconnect: rotate a service-account secret (Google JSON key, Slack, + // Atlassian, or any token-paste / client-credential provider) in place. The + // secret is re-verified against the provider and re-encrypted through the + // same builder the create path uses, so the rotation also yields the new + // principal's derived display name and audit metadata. const hasRotationSecret = + params.serviceAccountJson !== undefined || params.signingSecret !== undefined || params.botToken !== undefined || params.apiToken !== undefined || @@ -162,38 +192,61 @@ export async function performUpdateCredential( params.orgId !== undefined || params.dataCenter !== undefined let rotatedSlackBotUserId: string | undefined + let rotatedAuditMetadata: Record | undefined if (hasRotationSecret && access.credential.type === 'service_account') { + const providerId = access.credential.providerId ?? '' + // A reconnect rebuilds the secret blob from the submitted fields only, and // the modal never prefills (secrets are never echoed back). For an actual // secret that is correct - the admin retypes it. But a non-secret selector // like the Zoho data center would be silently dropped, moving an EU/IN/AU // credential back to the US accounts server. Carry the stored value forward // when the caller did not supply one. - // Scoped to the providers that actually have a dataCenter field, so no - // other service-account reconnect (Slack, Atlassian, every token-paste - // provider) pays for a DB read plus a decrypt it can never use. - const carriedDataCenter = - params.dataCenter === undefined && - isClientCredentialAccountProviderId(access.credential.providerId ?? '') - ? await readStoredDataCenter(access.credential.id) - : params.dataCenter + const needsStoredDataCenter = + params.dataCenter === undefined && isClientCredentialAccountProviderId(providerId) + + // Rotating to a key that belongs to a different principal makes an + // identity-derived label (a Google `client_email`, a Slack team name) + // actively wrong about who the credential authenticates as. Re-derive it - + // but only when the stored label is still the previous principal, so a + // name the user deliberately typed always wins. An explicit `displayName` + // in this same request wins outright and skips the read entirely. + const needsStoredIdentity = + params.displayName === undefined && IDENTITY_DERIVED_DISPLAY_NAME_PROVIDERS.has(providerId) + + // One read + decrypt at most, and only for the providers that can use it. + const storedBlob = + needsStoredDataCenter || needsStoredIdentity + ? await readStoredSecretBlob(access.credential.id) + : null try { - const secret = await verifyAndBuildServiceAccountSecret( - access.credential.providerId ?? '', - { - signingSecret: params.signingSecret, - botToken: params.botToken, - apiToken: params.apiToken, - domain: params.domain, - clientId: params.clientId, - clientSecret: params.clientSecret, - orgId: params.orgId, - dataCenter: carriedDataCenter, - } - ) + const secret = await verifyAndBuildServiceAccountSecret(providerId, { + signingSecret: params.signingSecret, + botToken: params.botToken, + apiToken: params.apiToken, + domain: params.domain, + serviceAccountJson: params.serviceAccountJson, + clientId: params.clientId, + clientSecret: params.clientSecret, + orgId: params.orgId, + dataCenter: needsStoredDataCenter ? readStoredDataCenter(storedBlob) : params.dataCenter, + }) updates.encryptedServiceAccountKey = secret.encryptedServiceAccountKey rotatedSlackBotUserId = secret.botUserId + rotatedAuditMetadata = secret.auditMetadata + + if (needsStoredIdentity) { + const previousIdentity = deriveStoredDisplayName(storedBlob) + if ( + previousIdentity !== undefined && + previousIdentity === access.credential.displayName && + secret.displayName && + secret.displayName !== previousIdentity + ) { + updates.displayName = secret.displayName + } + } } catch (error) { if (error instanceof ServiceAccountSecretError) { return { success: false, error: error.message, errorCode: 'validation' } @@ -260,7 +313,10 @@ export async function performUpdateCredential( resourceId: params.credentialId, resourceName: access.credential.displayName, description: `Updated ${access.credential.type} credential "${access.credential.displayName}"`, + // Provider metadata first: the orchestration's own keys stay authoritative + // and can never be shadowed by a builder's audit payload. metadata: { + ...rotatedAuditMetadata, credentialType: access.credential.type, updatedFields, }, diff --git a/apps/sim/lib/credentials/principal.ts b/apps/sim/lib/credentials/principal.ts new file mode 100644 index 00000000000..3339b00ec1a --- /dev/null +++ b/apps/sim/lib/credentials/principal.ts @@ -0,0 +1,62 @@ +/** + * Provider-identity primitives for service-account credentials. + * + * Deliberately a leaf module: the token and client-credential registries both + * need these, and `service-account-secret` imports values from both registries. + * Defining them there would close a runtime import cycle. + */ + +/** + * Provider identity captured while verifying a service-account credential. + * + * `tenant` exists because several providers can only ever report an + * org/workspace/site-level identifier (Attio, Shopify, Webflow, Zoom, Zoho + * Desk) — callers must never present those as the human actor behind the + * credential. `lookup_failed` records that the provider does expose a + * principal but the lookup did not complete, which is distinct from a + * provider that exposes no principal at all (`null`). + */ +export type ServiceAccountPrincipal = + | { kind: 'user'; id: string; label?: string } + | { kind: 'tenant'; id: string; label?: string } + | { kind: 'lookup_failed'; reason: string } + +/** + * The human actor a credential authenticates as. + * + * `label` accepts null/undefined because provider payloads routinely type an + * optional email or username that way, and is dropped when empty so + * {@link serviceAccountPrincipalMetadata} never emits a blank key. + */ +export function userPrincipal(id: string, label?: string | null): ServiceAccountPrincipal { + return { kind: 'user', id, ...(label ? { label } : {}) } +} + +/** + * An org/workspace/site-level identifier, for the providers that expose no + * actor at all. Kept distinct from {@link userPrincipal} so callers can never + * present a tenant id as the person behind the credential. + */ +export function tenantPrincipal(id: string, label?: string | null): ServiceAccountPrincipal { + return { kind: 'tenant', id, ...(label ? { label } : {}) } +} + +/** + * Flattens a principal into the string map mirrored into both `auditMetadata` + * (queryable on `audit_log.metadata`) and `storedMetadata` (inside the + * encrypted blob). Applied centrally by the builders below so no provider can + * capture a principal and forget to surface it. + */ +export function serviceAccountPrincipalMetadata( + principal: ServiceAccountPrincipal | null +): Record { + if (principal === null) return { principalKind: 'none' } + if (principal.kind === 'lookup_failed') { + return { principalKind: 'lookup_failed', principalLookupError: principal.reason } + } + return { + principalKind: principal.kind, + principalId: principal.id, + ...(principal.label ? { principalLabel: principal.label } : {}), + } +} diff --git a/apps/sim/lib/credentials/service-account-secret.test.ts b/apps/sim/lib/credentials/service-account-secret.test.ts index 27fa472f15f..b874432a680 100644 --- a/apps/sim/lib/credentials/service-account-secret.test.ts +++ b/apps/sim/lib/credentials/service-account-secret.test.ts @@ -104,6 +104,7 @@ describe('verifyAndBuildServiceAccountSecret', () => { accountId: 'acc-1', displayName: 'Jira Bot', cloudId: 'cloud-1', + emailAddress: 'bot@acme.com', }) const result = await verifyAndBuildServiceAccountSecret(ATLASSIAN_SERVICE_ACCOUNT_PROVIDER_ID, { apiToken: 'tok', @@ -112,6 +113,9 @@ describe('verifyAndBuildServiceAccountSecret', () => { expect(result.providerId).toBe(ATLASSIAN_SERVICE_ACCOUNT_PROVIDER_ID) expect(result.displayName).toBe('Jira Bot') expect(result.auditMetadata.atlassianCloudId).toBe('cloud-1') + expect(result.principal).toEqual({ kind: 'user', id: 'acc-1', label: 'bot@acme.com' }) + expect(result.auditMetadata.principalId).toBe('acc-1') + expect(result.auditMetadata.principalLabel).toBe('bot@acme.com') const blob = JSON.parse(result.encryptedServiceAccountKey) expect(blob).toMatchObject({ apiToken: 'tok', @@ -127,17 +131,32 @@ describe('verifyAndBuildServiceAccountSecret', () => { }) it('validates and encrypts a Google service-account JSON key', async () => { - const json = JSON.stringify({ type: 'service_account', client_email: 'svc@proj.iam' }) + const json = JSON.stringify({ + type: 'service_account', + client_email: 'svc@proj.iam', + project_id: 'proj', + }) const result = await verifyAndBuildServiceAccountSecret('google-service-account', { serviceAccountJson: json, }) expect(result.providerId).toBe('google-service-account') expect(result.displayName).toBe('svc@proj.iam') expect(result.encryptedServiceAccountKey).toBe(json) + expect(result.principal).toEqual({ kind: 'user', id: 'svc@proj.iam' }) + expect(result.auditMetadata).toEqual({ + googleClientEmail: 'svc@proj.iam', + googleProjectId: 'proj', + principalKind: 'user', + principalId: 'svc@proj.iam', + }) }) it('accepts a legacy Google create with an empty providerId', async () => { - const json = JSON.stringify({ type: 'service_account', client_email: 'svc@proj.iam' }) + const json = JSON.stringify({ + type: 'service_account', + client_email: 'svc@proj.iam', + project_id: 'proj', + }) const result = await verifyAndBuildServiceAccountSecret('', { serviceAccountJson: json }) expect(result.providerId).toBe('google-service-account') }) @@ -157,6 +176,7 @@ describe('verifyAndBuildServiceAccountSecret', () => { expiresInSeconds: 3600, identity: { displayName: 'Zoom account acc-1', + principal: { kind: 'tenant', id: 'acc-1' }, auditMetadata: { zoomAccountId: 'acc-1' }, storedMetadata: { apiUrl: 'https://api.zoom.us' }, }, @@ -168,7 +188,11 @@ describe('verifyAndBuildServiceAccountSecret', () => { }) expect(result.providerId).toBe('zoom-service-account') expect(result.displayName).toBe('Zoom account acc-1') - expect(result.auditMetadata).toEqual({ zoomAccountId: 'acc-1' }) + expect(result.auditMetadata).toEqual({ + zoomAccountId: 'acc-1', + principalKind: 'tenant', + principalId: 'acc-1', + }) expect(mockClientCredentialMinter).toHaveBeenCalledWith({ clientId: 'cid', clientSecret: 'csec', @@ -181,7 +205,11 @@ describe('verifyAndBuildServiceAccountSecret', () => { clientId: 'cid', clientSecret: 'csec', orgId: 'acc-1', - metadata: { apiUrl: 'https://api.zoom.us' }, + metadata: { + apiUrl: 'https://api.zoom.us', + principalKind: 'tenant', + principalId: 'acc-1', + }, }) }) @@ -193,9 +221,10 @@ describe('verifyAndBuildServiceAccountSecret', () => { orgId: '999', }) expect(result.displayName).toBe('Box 999') - expect(result.auditMetadata).toEqual({}) + expect(result.principal).toBeNull() + expect(result.auditMetadata).toEqual({ principalKind: 'none' }) const blob = JSON.parse(result.encryptedServiceAccountKey) - expect(blob.metadata).toBeUndefined() + expect(blob.metadata).toEqual({ principalKind: 'none' }) }) it('throws when client-credential required fields are missing, without minting', async () => { diff --git a/apps/sim/lib/credentials/service-account-secret.ts b/apps/sim/lib/credentials/service-account-secret.ts index 900462b6e27..d6b678d4a17 100644 --- a/apps/sim/lib/credentials/service-account-secret.ts +++ b/apps/sim/lib/credentials/service-account-secret.ts @@ -15,6 +15,11 @@ import { type ClientCredentialAccountSecretBlob, getClientCredentialAccountMinter, } from '@/lib/credentials/client-credential-accounts/server' +import { slackCustomBotDisplayName } from '@/lib/credentials/display-name' +import { + type ServiceAccountPrincipal, + serviceAccountPrincipalMetadata, +} from '@/lib/credentials/principal' import { getTokenServiceAccountDescriptor, isTokenServiceAccountProviderId, @@ -52,6 +57,12 @@ export interface ServiceAccountSecretResult { encryptedServiceAccountKey: string displayName: string auditMetadata: Record + /** + * Provider principal behind the credential, or `null` when the provider + * exposes none. Required (never optional) so a new provider cannot be added + * without deciding what identity it captures. + */ + principal: ServiceAccountPrincipal | null /** Slack custom bot: the derived bot user id (for reaction self-drop). */ botUserId?: string } @@ -78,12 +89,20 @@ async function buildAtlassianServiceAccountSecret( } const normalizedDomain = normalizeAtlassianDomain(domain) const validation = await validateAtlassianServiceAccount(apiToken, normalizedDomain) + const principal: ServiceAccountPrincipal = { + kind: 'user', + id: validation.accountId, + ...(validation.emailAddress ? { label: validation.emailAddress } : {}), + } + // `atlassianAccountId` stays at the blob's top level: `getAtlassianServiceAccountSecret` + // in `app/api/auth/oauth/utils.ts` reads it there on every existing credential. const blob = JSON.stringify({ type: ATLASSIAN_SERVICE_ACCOUNT_SECRET_TYPE, apiToken, domain: normalizedDomain, cloudId: validation.cloudId, atlassianAccountId: validation.accountId, + metadata: serviceAccountPrincipalMetadata(principal), }) const { encrypted } = await encryptSecret(blob) return { @@ -93,7 +112,9 @@ async function buildAtlassianServiceAccountSecret( auditMetadata: { atlassianDomain: normalizedDomain, atlassianCloudId: validation.cloudId, + ...serviceAccountPrincipalMetadata(principal), }, + principal, } } @@ -123,6 +144,11 @@ async function buildSlackCustomBotSecret( `Could not verify the Slack bot token: ${getErrorMessage(error)}` ) } + // `auth.test` returns the bot user only for bot tokens; a token without one + // is workspace-scoped, so the team is the finest identity available. + const principal: ServiceAccountPrincipal = botUserId + ? { kind: 'user', id: botUserId } + : { kind: 'tenant', id: teamId, ...(teamName ? { label: teamName } : {}) } const blob = JSON.stringify({ type: SLACK_CUSTOM_BOT_SECRET_TYPE, signingSecret, @@ -130,13 +156,15 @@ async function buildSlackCustomBotSecret( teamId, botUserId, teamName, + metadata: serviceAccountPrincipalMetadata(principal), }) const { encrypted } = await encryptSecret(blob) return { providerId: SLACK_CUSTOM_BOT_PROVIDER_ID, encryptedServiceAccountKey: encrypted, - displayName: teamName || 'Slack bot', - auditMetadata: { slackTeamId: teamId }, + displayName: slackCustomBotDisplayName(teamName), + auditMetadata: { slackTeamId: teamId, ...serviceAccountPrincipalMetadata(principal) }, + principal, botUserId, } } @@ -161,12 +189,23 @@ async function buildGoogleServiceAccountSecret( getValidationErrorMessage(jsonParseResult.error, 'Invalid service account JSON') ) } + const { client_email: clientEmail, project_id: projectId } = jsonParseResult.data + // `client_email` is the principal a Google service account authenticates as + // (its `unique_id` is not guaranteed to be present in a downloaded key). + const principal: ServiceAccountPrincipal = { kind: 'user', id: clientEmail } + // The blob stays the verbatim GCP key — every consumer parses it as one — so + // the principal is mirrored into the audit metadata only. const { encrypted } = await encryptSecret(serviceAccountJson) return { providerId: GOOGLE_SERVICE_ACCOUNT_PROVIDER_ID, encryptedServiceAccountKey: encrypted, - displayName: jsonParseResult.data.client_email, - auditMetadata: {}, + displayName: clientEmail, + auditMetadata: { + googleClientEmail: clientEmail, + googleProjectId: projectId, + ...serviceAccountPrincipalMetadata(principal), + }, + principal, } } @@ -197,19 +236,21 @@ async function buildTokenServiceAccountSecret( ) } const validation = await validator({ apiToken, domain }) + const principalMetadata = serviceAccountPrincipalMetadata(validation.principal) const blob: TokenServiceAccountSecretBlob = { type: TOKEN_SERVICE_ACCOUNT_SECRET_TYPE, providerId, apiToken, ...(requiresDomain ? { domain: validation.normalizedDomain ?? domain } : {}), - ...(validation.storedMetadata ? { metadata: validation.storedMetadata } : {}), + metadata: { ...validation.storedMetadata, ...principalMetadata }, } const { encrypted } = await encryptSecret(JSON.stringify(blob)) return { providerId, encryptedServiceAccountKey: encrypted, displayName: validation.displayName, - auditMetadata: validation.auditMetadata, + auditMetadata: { ...validation.auditMetadata, ...principalMetadata }, + principal: validation.principal, } } @@ -246,6 +287,10 @@ async function buildClientCredentialAccountSecret( ) } const mint = await minter({ clientId, clientSecret, orgId, dataCenter }) + // `identity` is absent only on the `skipIdentity` execution-time path, which + // never reaches this builder; treat it as "no principal captured". + const principal = mint.identity?.principal ?? null + const principalMetadata = serviceAccountPrincipalMetadata(principal) const blob: ClientCredentialAccountSecretBlob = { type: CLIENT_CREDENTIAL_ACCOUNT_SECRET_TYPE, providerId, @@ -253,14 +298,15 @@ async function buildClientCredentialAccountSecret( clientSecret, orgId, ...(dataCenter ? { dataCenter } : {}), - ...(mint.identity?.storedMetadata ? { metadata: mint.identity.storedMetadata } : {}), + metadata: { ...mint.identity?.storedMetadata, ...principalMetadata }, } const { encrypted } = await encryptSecret(JSON.stringify(blob)) return { providerId, encryptedServiceAccountKey: encrypted, displayName: mint.identity?.displayName ?? `${descriptor.serviceLabel} ${orgId}`, - auditMetadata: mint.identity?.auditMetadata ?? {}, + auditMetadata: { ...mint.identity?.auditMetadata, ...principalMetadata }, + principal, } } diff --git a/apps/sim/lib/credentials/token-service-accounts/errors.ts b/apps/sim/lib/credentials/token-service-accounts/errors.ts index abf0b003b96..3e6ec4cbed6 100644 --- a/apps/sim/lib/credentials/token-service-accounts/errors.ts +++ b/apps/sim/lib/credentials/token-service-accounts/errors.ts @@ -1,3 +1,4 @@ +import { getErrorMessage } from '@sim/utils/errors' import { truncate } from '@sim/utils/string' /** @@ -23,6 +24,19 @@ export class TokenServiceAccountValidationError extends Error { const ERROR_SNIPPET_MAX_LENGTH = 500 +/** + * Short, stable description of a failed best-effort provider call, for callers + * that degrade instead of throwing. `TokenServiceAccountValidationError`'s + * message is only its code, so the status is appended to keep the reason + * diagnosable. + */ +export function providerFailureReason(error: unknown): string { + if (error instanceof TokenServiceAccountValidationError) { + return `${error.code} (HTTP ${error.status})` + } + return getErrorMessage(error, 'request failed') +} + /** * Transient statuses a provider token/verification endpoint can return that * say nothing about the submitted credentials (throttling, request timeout) — diff --git a/apps/sim/lib/credentials/token-service-accounts/server.ts b/apps/sim/lib/credentials/token-service-accounts/server.ts index a7a693b1ca0..4fee7e16e40 100644 --- a/apps/sim/lib/credentials/token-service-accounts/server.ts +++ b/apps/sim/lib/credentials/token-service-accounts/server.ts @@ -1,3 +1,4 @@ +import type { ServiceAccountPrincipal } from '@/lib/credentials/principal' import { AIRTABLE_SERVICE_ACCOUNT_PROVIDER_ID, ASANA_SERVICE_ACCOUNT_PROVIDER_ID, @@ -44,11 +45,21 @@ export interface TokenServiceAccountFields { export interface TokenServiceAccountValidationResult { /** Default display name when the user didn't provide one. */ displayName: string - /** Non-secret identifiers recorded in the audit log (e.g. portal/workspace id). */ + /** + * Identity the token authenticates as, or `null` when the provider exposes + * none. Required (never optional) so a new validator cannot be written + * without deciding. `verifyAndBuildServiceAccountSecret` mirrors it into both + * `auditMetadata` and `storedMetadata`, so validators must not repeat it. + */ + principal: ServiceAccountPrincipal | null + /** + * Non-secret identifiers recorded in the audit log that are NOT the + * principal (e.g. the org id behind a user principal). + */ auditMetadata: Record /** * Non-secret metadata persisted inside the encrypted blob alongside the - * token (e.g. normalized store domain, portal id) for later debugging. + * token (e.g. normalized store domain, granted scopes) for later debugging. */ storedMetadata?: Record /** Normalized domain to persist instead of the raw user input (when collected). */ diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/airtable.test.ts b/apps/sim/lib/credentials/token-service-accounts/validators/airtable.test.ts index b1f4797b509..b71becb916b 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/airtable.test.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/airtable.test.ts @@ -37,8 +37,9 @@ describe('validateAirtableServiceAccount', () => { expect(result).toEqual({ displayName: 'svc@example.com', - auditMetadata: { airtableUserId: 'usrABC123' }, - storedMetadata: { userId: 'usrABC123', scopes: 'data.records:read' }, + principal: { kind: 'user', id: 'usrABC123', label: 'svc@example.com' }, + auditMetadata: {}, + storedMetadata: { scopes: 'data.records:read' }, }) expect(mockFetch).toHaveBeenCalledWith('https://api.airtable.com/v0/meta/whoami', { headers: { @@ -55,8 +56,9 @@ describe('validateAirtableServiceAccount', () => { const result = await validateAirtableServiceAccount({ apiToken: 'pat456.secret' }) expect(result.displayName).toBe('Airtable user usrXYZ789') - expect(result.auditMetadata).toEqual({ airtableUserId: 'usrXYZ789' }) - expect(result.storedMetadata).toEqual({ userId: 'usrXYZ789' }) + expect(result.principal).toEqual({ kind: 'user', id: 'usrXYZ789' }) + expect(result.auditMetadata).toEqual({}) + expect(result.storedMetadata).toEqual({}) }) it('throws invalid_credentials on 401', async () => { diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/airtable.ts b/apps/sim/lib/credentials/token-service-accounts/validators/airtable.ts index ccab65691eb..c70ed3c4b1b 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/airtable.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/airtable.ts @@ -1,3 +1,4 @@ +import { userPrincipal } from '@/lib/credentials/principal' import { fetchProvider, parseProviderJson, @@ -47,14 +48,15 @@ export async function validateAirtableServiceAccount( }) } - const storedMetadata: Record = { userId: whoami.id } + const storedMetadata: Record = {} if (whoami.scopes) { storedMetadata.scopes = whoami.scopes.join(' ') } return { displayName: whoami.email ?? `Airtable user ${whoami.id}`, - auditMetadata: { airtableUserId: whoami.id }, + principal: userPrincipal(whoami.id, whoami.email), + auditMetadata: {}, storedMetadata, } } diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/asana.test.ts b/apps/sim/lib/credentials/token-service-accounts/validators/asana.test.ts index 6e950a55002..c1c57c7f815 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/asana.test.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/asana.test.ts @@ -35,8 +35,8 @@ describe('validateAsanaServiceAccount', () => { expect(result).toEqual({ displayName: 'Sim Integration', - auditMetadata: { asanaUserGid: '12345' }, - storedMetadata: { userGid: '12345', email: 'bot@example.com' }, + principal: { kind: 'user', id: '12345', label: 'bot@example.com' }, + auditMetadata: {}, }) expect(mockFetch).toHaveBeenCalledWith( 'https://app.asana.com/api/1.0/users/me?opt_fields=gid,name,email', @@ -62,7 +62,7 @@ describe('validateAsanaServiceAccount', () => { const gidOnly = await validateAsanaServiceAccount({ apiToken: 'token-2' }) expect(gidOnly.displayName).toBe('Asana user 999') - expect(gidOnly.storedMetadata).toEqual({ userGid: '999' }) + expect(gidOnly.principal).toEqual({ kind: 'user', id: '999' }) }) it('maps 401 to invalid_credentials', async () => { diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/asana.ts b/apps/sim/lib/credentials/token-service-accounts/validators/asana.ts index c138258ee35..e35f0adff37 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/asana.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/asana.ts @@ -1,3 +1,4 @@ +import { userPrincipal } from '@/lib/credentials/principal' import { fetchProvider, parseProviderJson, @@ -51,12 +52,10 @@ export async function validateAsanaServiceAccount( const name = body.data?.name const email = body.data?.email - const storedMetadata: Record = { userGid: gid } - if (email) storedMetadata.email = email return { displayName: name || email || `Asana user ${gid}`, - auditMetadata: { asanaUserGid: gid }, - storedMetadata, + principal: userPrincipal(gid, email), + auditMetadata: {}, } } diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/attio.test.ts b/apps/sim/lib/credentials/token-service-accounts/validators/attio.test.ts index 7c193f063e5..92002f1752c 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/attio.test.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/attio.test.ts @@ -60,8 +60,8 @@ describe('validateAttioServiceAccount', () => { }) expect(result).toEqual({ displayName: 'Acme CRM', - auditMetadata: { attioWorkspaceId: 'ws-123' }, - storedMetadata: { workspaceId: 'ws-123', workspaceSlug: 'acme-crm' }, + principal: { kind: 'tenant', id: 'ws-123', label: 'acme-crm' }, + auditMetadata: {}, }) }) diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/attio.ts b/apps/sim/lib/credentials/token-service-accounts/validators/attio.ts index 1969439c4ac..c0b7ccc1d0d 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/attio.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/attio.ts @@ -69,14 +69,15 @@ export async function validateAttioServiceAccount( }) } - const storedMetadata: Record = { workspaceId: self.workspace_id } - if (self.workspace_slug) { - storedMetadata.workspaceSlug = self.workspace_slug - } - + // An Attio workspace access token is not bound to a member, so the workspace + // is the finest identity the token can ever report. return { displayName: self.workspace_name || 'Attio workspace', - auditMetadata: { attioWorkspaceId: self.workspace_id }, - storedMetadata, + principal: { + kind: 'tenant', + id: self.workspace_id, + ...(self.workspace_slug ? { label: self.workspace_slug } : {}), + }, + auditMetadata: {}, } } diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/calcom.test.ts b/apps/sim/lib/credentials/token-service-accounts/validators/calcom.test.ts index cbd22a5b1a2..78b0b7bbcd2 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/calcom.test.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/calcom.test.ts @@ -36,8 +36,8 @@ describe('validateCalcomServiceAccount', () => { expect(result).toEqual({ displayName: 'sim-bot', - auditMetadata: { calcomUserId: '42' }, - storedMetadata: { userId: '42', email: 'bot@example.com' }, + principal: { kind: 'user', id: '42', label: 'sim-bot' }, + auditMetadata: {}, }) expect(mockFetch).toHaveBeenCalledWith('https://api.cal.com/v2/me', { headers: { diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/calcom.ts b/apps/sim/lib/credentials/token-service-accounts/validators/calcom.ts index c536bb43b1e..bd8cc59ab07 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/calcom.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/calcom.ts @@ -1,3 +1,4 @@ +import { userPrincipal } from '@/lib/credentials/principal' import { fetchProvider, parseProviderJson, @@ -54,12 +55,11 @@ export async function validateCalcomServiceAccount( const userId = String(body.data.id) const username = body.data.username const email = body.data.email - const storedMetadata: Record = { userId } - if (email) storedMetadata.email = email + const label = username || email return { displayName: username || email || 'Cal.com account', - auditMetadata: { calcomUserId: userId }, - storedMetadata, + principal: userPrincipal(userId, label), + auditMetadata: {}, } } diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/claude-platform.ts b/apps/sim/lib/credentials/token-service-accounts/validators/claude-platform.ts index f457d6be29d..566834f4fd2 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/claude-platform.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/claude-platform.ts @@ -35,8 +35,13 @@ export async function validateClaudePlatformServiceAccount( await throwForProviderResponse(res, 'agents_list') const suffix = fields.apiToken.slice(-4) + // Explicitly no principal: the Managed Agents API exposes no whoami endpoint + // and no workspace identifier on any response, so nothing about the key's + // owner is knowable at connect time. This is a provider limitation, not a + // failed lookup — see `ServiceAccountPrincipal`. return { displayName: `Claude Platform (…${suffix})`, + principal: null, auditMetadata: {}, } } diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/clickup.ts b/apps/sim/lib/credentials/token-service-accounts/validators/clickup.ts index 129fbb1fb02..6facd5d4b28 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/clickup.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/clickup.ts @@ -1,3 +1,4 @@ +import { userPrincipal } from '@/lib/credentials/principal' import { fetchProvider, parseProviderJson, @@ -65,9 +66,11 @@ export async function validateClickupServiceAccount( }) } + const label = user.username || user.email + return { displayName: user.username || user.email || 'ClickUp account', - auditMetadata: { clickupUserId: String(user.id) }, - storedMetadata: { userId: String(user.id) }, + principal: userPrincipal(String(user.id), label), + auditMetadata: {}, } } diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/hubspot.test.ts b/apps/sim/lib/credentials/token-service-accounts/validators/hubspot.test.ts index 1259b11ac0a..c37e2ad1a53 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/hubspot.test.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/hubspot.test.ts @@ -74,8 +74,9 @@ describe('validateHubspotServiceAccount', () => { expect(result).toEqual({ displayName: 'HubSpot portal 12345', + principal: { kind: 'user', id: '111' }, auditMetadata: { hubspotHubId: '12345' }, - storedMetadata: { hubId: '12345', appId: '222', userId: '111' }, + storedMetadata: { hubId: '12345', appId: '222' }, }) expect(mockFetch).toHaveBeenCalledTimes(1) @@ -91,8 +92,8 @@ describe('validateHubspotServiceAccount', () => { expect(result).toEqual({ displayName: 'HubSpot portal 123', - auditMetadata: { hubspotHubId: '123' }, - storedMetadata: { hubId: '123' }, + principal: { kind: 'tenant', id: '123' }, + auditMetadata: {}, }) expect(mockFetch).toHaveBeenCalledTimes(2) @@ -127,8 +128,8 @@ describe('validateHubspotServiceAccount', () => { expect(result).toEqual({ displayName: 'HubSpot private app', + principal: null, auditMetadata: {}, - storedMetadata: {}, }) expect(mockFetch).toHaveBeenCalledTimes(2) diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/hubspot.ts b/apps/sim/lib/credentials/token-service-accounts/validators/hubspot.ts index 457710e32c8..c22452e5b55 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/hubspot.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/hubspot.ts @@ -1,3 +1,4 @@ +import { tenantPrincipal, userPrincipal } from '@/lib/credentials/principal' import { fetchProvider, parseProviderJson, @@ -41,10 +42,12 @@ async function verifyViaAccountInfo( 'account_info' ) if (res.status === 403) { + // The token is live but the app cannot read account info, so neither the + // portal nor the creating user is knowable on this path. return { displayName: 'HubSpot private app', + principal: null, auditMetadata: {}, - storedMetadata: {}, } } await throwForProviderResponse(res, 'account_info') @@ -53,8 +56,10 @@ async function verifyViaAccountInfo( const hubId = typeof info?.portalId === 'number' ? String(info.portalId) : undefined return { displayName: hubId ? `HubSpot portal ${hubId}` : 'HubSpot private app', - auditMetadata: hubId ? { hubspotHubId: hubId } : {}, - storedMetadata: hubId ? { hubId } : {}, + // This route never reports the private app's creating user, so the portal + // is the finest identity available here. + principal: hubId ? tenantPrincipal(hubId) : null, + auditMetadata: {}, } } @@ -113,10 +118,15 @@ export async function validateHubspotServiceAccount( const storedMetadata: Record = { hubId } if (typeof tokenInfo.appId === 'number') storedMetadata.appId = String(tokenInfo.appId) - if (typeof tokenInfo.userId === 'number') storedMetadata.userId = String(tokenInfo.userId) return { displayName: `HubSpot portal ${hubId}`, + // `userId` is the HubSpot user the private app acts on behalf of; it is the + // actor, while `hubId` is only the portal it lives in. + principal: + typeof tokenInfo.userId === 'number' + ? userPrincipal(String(tokenInfo.userId)) + : tenantPrincipal(hubId), auditMetadata: { hubspotHubId: hubId }, storedMetadata, } diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/linear.test.ts b/apps/sim/lib/credentials/token-service-accounts/validators/linear.test.ts index 3007d161370..37ef8fc156c 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/linear.test.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/linear.test.ts @@ -39,8 +39,9 @@ describe('validateLinearServiceAccount', () => { expect(result).toEqual({ displayName: 'Acme', + principal: { kind: 'user', id: 'viewer-1', label: 'jane@acme.com' }, auditMetadata: { linearOrganizationId: 'org-1' }, - storedMetadata: { viewerId: 'viewer-1', organizationId: 'org-1' }, + storedMetadata: { organizationId: 'org-1' }, }) const [url, init] = mockFetch.mock.calls[0] as [string, RequestInit] diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/linear.ts b/apps/sim/lib/credentials/token-service-accounts/validators/linear.ts index ea4e297f374..46c98aef7b0 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/linear.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/linear.ts @@ -1,3 +1,4 @@ +import { userPrincipal } from '@/lib/credentials/principal' import { fetchProvider, parseProviderJson, @@ -127,15 +128,17 @@ export async function validateLinearServiceAccount( } const organization = payload.data?.organization - const storedMetadata: Record = { viewerId: viewer.id } + const storedMetadata: Record = {} const auditMetadata: Record = {} if (organization?.id) { storedMetadata.organizationId = organization.id auditMetadata.linearOrganizationId = organization.id } + const label = viewer.email || viewer.name || undefined return { displayName: organization?.name || viewer.name || viewer.email || 'Linear workspace', + principal: userPrincipal(viewer.id, label), auditMetadata, storedMetadata, } diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/monday.test.ts b/apps/sim/lib/credentials/token-service-accounts/validators/monday.test.ts index e96e7006b72..590d425c79d 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/monday.test.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/monday.test.ts @@ -38,8 +38,9 @@ describe('validateMondayServiceAccount', () => { expect(result).toEqual({ displayName: 'Acme', + principal: { kind: 'user', id: '12345', label: 'jane@example.com' }, auditMetadata: { mondayAccountId: '987' }, - storedMetadata: { accountId: '987', accountSlug: 'acme', userId: '12345' }, + storedMetadata: { accountId: '987', accountSlug: 'acme' }, }) expect(mockFetch).toHaveBeenCalledWith('https://api.monday.com/v2', { method: 'POST', @@ -159,6 +160,6 @@ describe('validateMondayServiceAccount', () => { ) const result = await validateMondayServiceAccount({ apiToken: 'token' }) expect(result.displayName).toBe('Acme') - expect(result.storedMetadata?.userId).toBe('77') + expect(result.principal).toEqual({ kind: 'user', id: '77', label: 'Bot User' }) }) }) diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/monday.ts b/apps/sim/lib/credentials/token-service-accounts/validators/monday.ts index 9ff853d71ff..75fa3fdbe9e 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/monday.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/monday.ts @@ -1,3 +1,4 @@ +import { userPrincipal } from '@/lib/credentials/principal' import { fetchProvider, parseProviderJson, @@ -115,7 +116,7 @@ export async function validateMondayServiceAccount( const userId = String(me.id) const accountId = account?.id != null ? String(account.id) : '' - const storedMetadata: Record = { accountId, userId } + const storedMetadata: Record = { accountId } if (account?.slug) { storedMetadata.accountSlug = account.slug } @@ -123,9 +124,11 @@ export async function validateMondayServiceAccount( if (accountId) { auditMetadata.mondayAccountId = accountId } + const label = me.email || me.name return { displayName: account?.name || me.name || me.email || `monday user ${userId}`, + principal: userPrincipal(userId, label), auditMetadata, storedMetadata, } diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/notion.test.ts b/apps/sim/lib/credentials/token-service-accounts/validators/notion.test.ts index fad5f227b30..adffbfb5a1f 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/notion.test.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/notion.test.ts @@ -39,8 +39,9 @@ describe('validateNotionServiceAccount', () => { expect(result).toEqual({ displayName: 'Ops Integration', - auditMetadata: { notionBotId: 'bot-123' }, - storedMetadata: { botId: 'bot-123', workspaceName: 'Acme Workspace' }, + principal: { kind: 'user', id: 'bot-123', label: 'Ops Integration' }, + auditMetadata: {}, + storedMetadata: { workspaceName: 'Acme Workspace' }, }) expect(mockFetch).toHaveBeenCalledWith('https://api.notion.com/v1/users/me', { headers: { @@ -66,11 +67,9 @@ describe('validateNotionServiceAccount', () => { const result = await validateNotionServiceAccount({ apiToken: 'secret_legacy' }) expect(result.displayName).toBe('Acme Workspace') - expect(result.auditMetadata).toEqual({ notionBotId: 'bot-456' }) - expect(result.storedMetadata).toEqual({ - botId: 'bot-456', - workspaceName: 'Acme Workspace', - }) + expect(result.principal).toEqual({ kind: 'user', id: 'bot-456' }) + expect(result.auditMetadata).toEqual({}) + expect(result.storedMetadata).toEqual({ workspaceName: 'Acme Workspace' }) }) it('throws invalid_credentials on 401', async () => { diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/notion.ts b/apps/sim/lib/credentials/token-service-accounts/validators/notion.ts index 4321ba5f3c4..e2b75762e48 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/notion.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/notion.ts @@ -1,3 +1,4 @@ +import { userPrincipal } from '@/lib/credentials/principal' import { fetchProvider, parseProviderJson, @@ -56,14 +57,17 @@ export async function validateNotionServiceAccount( } const workspaceName = me.bot?.workspace_name || undefined - const storedMetadata: Record = { botId: me.id } + const storedMetadata: Record = {} if (workspaceName) { storedMetadata.workspaceName = workspaceName } return { displayName: me.name || workspaceName || 'Notion integration', - auditMetadata: { notionBotId: me.id }, + // The integration authenticates as its own bot user, which is the actor + // recorded on every page/database change it makes. + principal: userPrincipal(me.id, me.name), + auditMetadata: {}, storedMetadata, } } diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/pipedrive.test.ts b/apps/sim/lib/credentials/token-service-accounts/validators/pipedrive.test.ts index 1e73129649b..4e75faeb57f 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/pipedrive.test.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/pipedrive.test.ts @@ -48,8 +48,9 @@ describe('validatePipedriveServiceAccount', () => { expect(result).toEqual({ displayName: 'Jane Doe (Acme Inc)', + principal: { kind: 'user', id: '42', label: 'Jane Doe' }, auditMetadata: { pipedriveCompanyId: '777' }, - storedMetadata: { userId: '42', companyId: '777', companyDomain: 'acme' }, + storedMetadata: { companyId: '777', companyDomain: 'acme' }, }) expect(mockFetch).toHaveBeenCalledTimes(1) @@ -67,7 +68,8 @@ describe('validatePipedriveServiceAccount', () => { const result = await validatePipedriveServiceAccount(FIELDS) expect(result.displayName).toBe('Pipedrive company 777') - expect(result.storedMetadata).toEqual({ userId: '42', companyId: '777' }) + expect(result.principal).toEqual({ kind: 'user', id: '42' }) + expect(result.storedMetadata).toEqual({ companyId: '777' }) }) it('throws invalid_credentials on 401', async () => { diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/pipedrive.ts b/apps/sim/lib/credentials/token-service-accounts/validators/pipedrive.ts index 3fa20e90ec6..66da8628dfa 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/pipedrive.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/pipedrive.ts @@ -1,3 +1,4 @@ +import { userPrincipal } from '@/lib/credentials/principal' import { fetchProvider, parseProviderJson, @@ -62,7 +63,7 @@ export async function validatePipedriveServiceAccount( const companyDomain = typeof user.company_domain === 'string' && user.company_domain ? user.company_domain : undefined - const storedMetadata: Record = { userId: String(user.id) } + const storedMetadata: Record = {} if (companyId) storedMetadata.companyId = companyId if (companyDomain) storedMetadata.companyDomain = companyDomain @@ -76,6 +77,7 @@ export async function validatePipedriveServiceAccount( return { displayName, + principal: userPrincipal(String(user.id), userName), auditMetadata: companyId ? { pipedriveCompanyId: companyId } : {}, storedMetadata, } diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/shopify.test.ts b/apps/sim/lib/credentials/token-service-accounts/validators/shopify.test.ts index b53fc10cff3..36eff828aac 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/shopify.test.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/shopify.test.ts @@ -44,8 +44,8 @@ describe('validateShopifyServiceAccount', () => { expect(result).toEqual({ displayName: 'Acme Store', - auditMetadata: { shopifyShopDomain: 'acme-store.myshopify.com' }, - storedMetadata: { shopDomain: 'acme-store.myshopify.com', shopName: 'Acme Store' }, + principal: { kind: 'tenant', id: 'acme-store.myshopify.com', label: 'Acme Store' }, + auditMetadata: {}, normalizedDomain: 'acme-store.myshopify.com', }) expect(mockFetch).toHaveBeenCalledWith( @@ -159,6 +159,28 @@ describe('validateShopifyServiceAccount', () => { }) }) + it('does not blame the credential when an auth-shaped error accompanies a populated shop', async () => { + mockFetch.mockResolvedValueOnce( + jsonResponse(200, { + data: { shop: { name: 'My Store', myshopifyDomain: 'my-store.myshopify.com' } }, + errors: [ + { message: 'Access denied for email field', extensions: { code: 'ACCESS_DENIED' } }, + ], + }) + ) + /** + * A per-field scope denial is not evidence the token is invalid. Reporting + * it as `invalid_credentials` would tell an admin to replace a working + * credential; only a response with no `shop` at all indicts the token. + */ + await expect( + validateShopifyServiceAccount({ apiToken: 'shpat_good', domain: 'my-store.myshopify.com' }) + ).rejects.toMatchObject({ + name: 'TokenServiceAccountValidationError', + code: 'provider_unavailable', + }) + }) + it('normalizes a pasted admin URL down to the bare store host', async () => { mockFetch.mockResolvedValueOnce( jsonResponse(200, { diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/shopify.ts b/apps/sim/lib/credentials/token-service-accounts/validators/shopify.ts index e0a2f9a605d..4d18a625334 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/shopify.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/shopify.ts @@ -18,6 +18,14 @@ import { SHOPIFY_API_VERSION } from '@/tools/shopify/constants' */ const SHOPIFY_HOST_REGEX = /^[a-z0-9][a-z0-9-]*\.myshopify\.com$/ +/** + * Every selected field must stay scope-free. `hasShopifyAuthError` treats an + * auth-shaped GraphQL error as a rejected token, and that is only sound while + * this query cannot partially fail: adding a scoped field (anything guarded by + * `read_*`) makes Shopify answer a token missing that scope with HTTP 200, + * a populated `shop`, AND an `ACCESS_DENIED` error — a working credential that + * must not be rejected. Revisit that check before adding any field here. + */ const SHOP_QUERY = '{ shop { name myshopifyDomain } }' interface ShopifyGraphqlError { @@ -100,19 +108,29 @@ export async function validateShopifyServiceAccount( const payload = await parseProviderJson(res, 'shop_query') + // The auth heuristic only fires when the query returned nothing at all: an + // auth-shaped error alongside a populated `shop` is a partial-scope failure, + // not a rejected token, and blaming the credential there would be wrong. const shop = payload.data?.shop - if (hasShopifyAuthError(payload.errors)) { - throw new TokenServiceAccountValidationError('invalid_credentials', 401, { + if (!shop) { + if (hasShopifyAuthError(payload.errors)) { + throw new TokenServiceAccountValidationError('invalid_credentials', 401, { + step: 'shop_query', + domain, + reason: 'auth-shaped GraphQL error in 200 response', + }) + } + throw new TokenServiceAccountValidationError('provider_unavailable', 502, { step: 'shop_query', domain, - reason: 'auth-shaped GraphQL error in 200 response', + reason: payload.errors ? 'GraphQL errors in response' : 'missing shop in response', }) } - if (payload.errors || !shop) { + if (payload.errors) { throw new TokenServiceAccountValidationError('provider_unavailable', 502, { step: 'shop_query', domain, - reason: payload.errors ? 'GraphQL errors in response' : 'missing shop in response', + reason: 'GraphQL errors in response', }) } @@ -122,13 +140,17 @@ export async function validateShopifyServiceAccount( ? normalizeShopifyDomain(shop.myshopifyDomain) : undefined const canonicalDomain = apiDomain && SHOPIFY_HOST_REGEX.test(apiDomain) ? apiDomain : domain - const storedMetadata: Record = { shopDomain: canonicalDomain } - if (shopName) storedMetadata.shopName = shopName + // A custom-app Admin API token belongs to the app, not to a staff member, so + // the store is the finest identity it can ever report. return { displayName: shopName ?? canonicalDomain, - auditMetadata: { shopifyShopDomain: canonicalDomain }, - storedMetadata, + principal: { + kind: 'tenant', + id: canonicalDomain, + ...(shopName ? { label: shopName } : {}), + }, + auditMetadata: {}, normalizedDomain: canonicalDomain, } } diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/trello.test.ts b/apps/sim/lib/credentials/token-service-accounts/validators/trello.test.ts index 314da932186..82c76793363 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/trello.test.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/trello.test.ts @@ -46,8 +46,8 @@ describe('validateTrelloServiceAccount', () => { expect(result).toEqual({ displayName: 'Sim Bot', - auditMetadata: { trelloMemberId: 'abc123' }, - storedMetadata: { memberId: 'abc123', username: 'simbot' }, + principal: { kind: 'user', id: 'abc123', label: 'simbot' }, + auditMetadata: {}, }) const [url] = mockFetch.mock.calls[0] diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/trello.ts b/apps/sim/lib/credentials/token-service-accounts/validators/trello.ts index 209a43e8b2e..500a8c6b537 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/trello.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/trello.ts @@ -1,4 +1,5 @@ import { env } from '@/lib/core/config/env' +import { userPrincipal } from '@/lib/credentials/principal' import { fetchProvider, parseProviderJson, @@ -79,14 +80,12 @@ export async function validateTrelloServiceAccount( }) } - const storedMetadata: Record = { memberId: member.id } - if (typeof member.username === 'string' && member.username) { - storedMetadata.username = member.username - } + const username = + typeof member.username === 'string' && member.username ? member.username : undefined return { displayName: member.fullName || member.username || `Trello member ${member.id}`, - auditMetadata: { trelloMemberId: member.id }, - storedMetadata, + principal: userPrincipal(member.id, username), + auditMetadata: {}, } } diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/wealthbox.test.ts b/apps/sim/lib/credentials/token-service-accounts/validators/wealthbox.test.ts index 50f8739d879..153faef2706 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/wealthbox.test.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/wealthbox.test.ts @@ -43,8 +43,8 @@ describe('validateWealthboxServiceAccount', () => { expect(result).toEqual({ displayName: 'Bill Jones', - auditMetadata: { wealthboxUserId: '42' }, - storedMetadata: { userId: '42', email: 'bill@example.com' }, + principal: { kind: 'user', id: '42', label: 'bill@example.com' }, + auditMetadata: {}, }) expect(mockFetch).toHaveBeenCalledTimes(1) diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/wealthbox.ts b/apps/sim/lib/credentials/token-service-accounts/validators/wealthbox.ts index 94e85821f33..ac97ca76474 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/wealthbox.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/wealthbox.ts @@ -1,3 +1,4 @@ +import { userPrincipal } from '@/lib/credentials/principal' import { fetchProvider, parseProviderJson, @@ -94,12 +95,11 @@ export async function validateWealthboxServiceAccount( const userId = typeof me.current_user?.id === 'number' ? String(me.current_user.id) : undefined const email = me.email || me.current_user?.email - const auditMetadata: Record = {} - if (userId) auditMetadata.wealthboxUserId = userId - - const storedMetadata: Record = {} - if (userId) storedMetadata.userId = userId - if (email) storedMetadata.email = email - - return { displayName, auditMetadata, storedMetadata } + // `/v1/me` omits `current_user` for some token types; without it Wealthbox + // reports no identifier of any kind on this response. + return { + displayName, + principal: userId ? userPrincipal(userId, email) : null, + auditMetadata: {}, + } } diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/webflow.test.ts b/apps/sim/lib/credentials/token-service-accounts/validators/webflow.test.ts index 1cb3f9018c8..109f862070b 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/webflow.test.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/webflow.test.ts @@ -35,8 +35,8 @@ describe('validateWebflowServiceAccount', () => { expect(result).toEqual({ displayName: 'Acme Marketing', - auditMetadata: { webflowSiteId: 'site123' }, - storedMetadata: { siteId: 'site123', siteName: 'Acme Marketing' }, + principal: { kind: 'tenant', id: 'site123', label: 'Acme Marketing' }, + auditMetadata: {}, }) expect(mockFetch).toHaveBeenCalledWith('https://api.webflow.com/v2/sites', { headers: { @@ -55,7 +55,7 @@ describe('validateWebflowServiceAccount', () => { const result = await validateWebflowServiceAccount({ apiToken: 'wf-token' }) expect(result.displayName).toBe('acme') - expect(result.storedMetadata).toEqual({ siteId: 'site456', siteName: 'acme' }) + expect(result.principal).toEqual({ kind: 'tenant', id: 'site456', label: 'acme' }) }) it('throws invalid_credentials on 401', async () => { diff --git a/apps/sim/lib/credentials/token-service-accounts/validators/webflow.ts b/apps/sim/lib/credentials/token-service-accounts/validators/webflow.ts index 034558532b2..4da3aeba131 100644 --- a/apps/sim/lib/credentials/token-service-accounts/validators/webflow.ts +++ b/apps/sim/lib/credentials/token-service-accounts/validators/webflow.ts @@ -1,3 +1,4 @@ +import { tenantPrincipal } from '@/lib/credentials/principal' import { fetchProvider, parseProviderJson, @@ -51,9 +52,10 @@ export async function validateWebflowServiceAccount( const displayName = site.displayName || site.shortName || 'Webflow site' + // A site API token is bound to a site, never to a Webflow user. return { displayName, - auditMetadata: { webflowSiteId: site.id }, - storedMetadata: { siteId: site.id, siteName: displayName }, + principal: tenantPrincipal(site.id, displayName), + auditMetadata: {}, } } From bd79ba1f8ec0b1d78facf8eedde7feb9e0a68d07 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sun, 2 Aug 2026 22:03:38 -0700 Subject: [PATCH 2/2] fix(credentials): keep the provider-reported name when identity lookup degrades MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Box and Salesforce returned early on a missing user id, discarding a `name` or `login` the response did carry and relabeling the credential to the enterprise or host fallback. Only the principal should degrade; the human label still beats an id-derived string. Also notes the Salesforce `openid` scope in the connect help text. The client credentials minter sends no scope parameter — effective scopes come from the customer's Connected App — so without `openid` the userinfo lookup can 403 and the run-as user silently never reaches the audit record. --- .../client-credential-accounts/descriptors.ts | 2 +- .../client-credential-accounts/minters/box.test.ts | 3 +++ .../client-credential-accounts/minters/box.ts | 12 +++++++++--- .../minters/salesforce.test.ts | 3 +++ .../client-credential-accounts/minters/salesforce.ts | 12 +++++++++--- 5 files changed, 25 insertions(+), 7 deletions(-) diff --git a/apps/sim/lib/credentials/client-credential-accounts/descriptors.ts b/apps/sim/lib/credentials/client-credential-accounts/descriptors.ts index f2439cabd38..ef83609cee2 100644 --- a/apps/sim/lib/credentials/client-credential-accounts/descriptors.ts +++ b/apps/sim/lib/credentials/client-credential-accounts/descriptors.ts @@ -274,7 +274,7 @@ export const CLIENT_CREDENTIAL_ACCOUNT_DESCRIPTORS: Record< ], docsUrl: 'https://docs.sim.ai/integrations/salesforce-service-account', helpText: - 'The Connected App must have "Enable Client Credentials Flow" checked with a "Run As" integration user set under Edit Policies — every call executes with that user\'s permissions, and deactivating or freezing the user stops all runs.', + 'The Connected App must have "Enable Client Credentials Flow" checked with a "Run As" integration user set under Edit Policies — every call executes with that user\'s permissions, and deactivating or freezing the user stops all runs. Selecting the "openid" scope lets Sim record which run-as user the credential authenticates as; without it the connection still works but the identity is not captured.', }, [ZOHO_DESK_SERVICE_ACCOUNT_PROVIDER_ID]: { providerId: ZOHO_DESK_SERVICE_ACCOUNT_PROVIDER_ID, diff --git a/apps/sim/lib/credentials/client-credential-accounts/minters/box.test.ts b/apps/sim/lib/credentials/client-credential-accounts/minters/box.test.ts index eedc622da87..7b09b9b2ad5 100644 --- a/apps/sim/lib/credentials/client-credential-accounts/minters/box.test.ts +++ b/apps/sim/lib/credentials/client-credential-accounts/minters/box.test.ts @@ -122,6 +122,9 @@ describe('mintBoxServiceAccountToken', () => { kind: 'lookup_failed', reason: 'response missing user id', }) + // Only the principal degrades — a name that did come back still beats the + // Enterprise-ID fallback, so the credential does not lose its label. + expect(result.identity?.displayName).toBe('Sim Automation') }) it('still succeeds when the identity request itself throws', async () => { diff --git a/apps/sim/lib/credentials/client-credential-accounts/minters/box.ts b/apps/sim/lib/credentials/client-credential-accounts/minters/box.ts index 04cb73b2f85..25e3b609080 100644 --- a/apps/sim/lib/credentials/client-credential-accounts/minters/box.ts +++ b/apps/sim/lib/credentials/client-credential-accounts/minters/box.ts @@ -75,8 +75,14 @@ async function fetchBoxServiceAccountIdentity( accessToken: string, orgId: string ): Promise { - const degraded = (reason: string): ClientCredentialAccountIdentity => ({ - displayName: `Box enterprise ${orgId}`, + /** + * `label` keeps whatever human name the lookup did return. A response can + * carry `name`/`login` but no `id` — the principal is then unusable, but the + * label still beats the Enterprise-ID fallback, so only the principal + * degrades and the credential does not silently lose its name. + */ + const degraded = (reason: string, label?: string): ClientCredentialAccountIdentity => ({ + displayName: label ?? `Box enterprise ${orgId}`, principal: { kind: 'lookup_failed', reason }, auditMetadata: { boxEnterpriseId: orgId }, storedMetadata: { enterpriseId: orgId }, @@ -105,7 +111,7 @@ async function fetchBoxServiceAccountIdentity( status: res.status, enterpriseId: orgId, }) - return degraded('response missing user id') + return degraded('response missing user id', name ?? login) } return { displayName: name ?? login ?? `Box enterprise ${orgId}`, diff --git a/apps/sim/lib/credentials/client-credential-accounts/minters/salesforce.test.ts b/apps/sim/lib/credentials/client-credential-accounts/minters/salesforce.test.ts index 997f692206b..0b874321280 100644 --- a/apps/sim/lib/credentials/client-credential-accounts/minters/salesforce.test.ts +++ b/apps/sim/lib/credentials/client-credential-accounts/minters/salesforce.test.ts @@ -294,6 +294,9 @@ describe('mintSalesforceServiceAccountToken', () => { kind: 'lookup_failed', reason: 'response missing user_id', }) + // Only the principal degrades — a name that did come back still beats the + // host fallback, so the credential does not lose its label. + expect(result.identity?.displayName).toBe('Integration User') }) it('ignores a non-Salesforce instance_url and falls back to the validated host', async () => { diff --git a/apps/sim/lib/credentials/client-credential-accounts/minters/salesforce.ts b/apps/sim/lib/credentials/client-credential-accounts/minters/salesforce.ts index 0c87a47d2a0..e8e702c98c7 100644 --- a/apps/sim/lib/credentials/client-credential-accounts/minters/salesforce.ts +++ b/apps/sim/lib/credentials/client-credential-accounts/minters/salesforce.ts @@ -118,8 +118,14 @@ async function fetchSalesforceIdentity( instanceUrl: string, host: string ): Promise { - const degraded = (reason: string): ClientCredentialAccountIdentity => ({ - displayName: `Salesforce ${host}`, + /** + * `label` keeps whatever human name userinfo did return. A response can carry + * `name`/`preferred_username` but no `user_id` — the principal is then + * unusable, but the label still beats the host fallback, so only the + * principal degrades and the credential does not silently lose its name. + */ + const degraded = (reason: string, label?: string): ClientCredentialAccountIdentity => ({ + displayName: label ?? `Salesforce ${host}`, principal: { kind: 'lookup_failed', reason }, auditMetadata: { salesforceMyDomainHost: host }, storedMetadata: { myDomainHost: host, instanceUrl }, @@ -155,7 +161,7 @@ async function fetchSalesforceIdentity( status: res.status, host, }) - return degraded('response missing user_id') + return degraded('response missing user_id', name ?? username) } return { displayName: name ?? username ?? `Salesforce ${host}`,