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/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 f380e8d8742..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 @@ -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,8 +105,26 @@ 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', }) + // 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 () => { @@ -118,6 +136,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..25e3b609080 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,67 @@ 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 = { - 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 }, + }) 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', name ?? login) } - } 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..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 @@ -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,30 @@ 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', + }) + // 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 () => { 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..e8e702c98c7 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,43 @@ 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 = { - 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 }, - } + }) 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 +154,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', name ?? username) + } 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: {}, } }