fix(credentials): capture the correct provider identity on connect and rotate (#6201)

* fix(credentials): capture the correct provider identity on connect and rotate

Attio OAuth recorded an arbitrary workspace member instead of the authorizing
user, so two members connecting under one Sim user collapsed into a single
account row via the stale-sibling dedupe. Notion read `profile.person.email`,
which never exists on a bot token. Synthetic connector emails were minted on
live third-party domains. Google service-account rotation left the credential
labeled with the old key's client_email and skipped audit metadata entirely.
Box and Salesforce identity lookups failed silently with no logger in either
file.

Service-account principals are now a single ServiceAccountPrincipal union
(user / tenant / lookup_failed / null) mirrored centrally into both audit and
stored metadata, so a principal can no longer be captured and forgotten, and
"which account is this credential?" is answerable from SQL.

* fix(credentials): keep the provider-reported name when identity lookup degrades

Box and Salesforce returned early on a missing user id, discarding a `name` or
`login` the response did carry and relabeling the credential to the enterprise
or host fallback. Only the principal should degrade; the human label still
beats an id-derived string.

Also notes the Salesforce `openid` scope in the connect help text. The client
credentials minter sends no scope parameter — effective scopes come from the
customer's Connected App — so without `openid` the userinfo lookup can 403 and
the run-as user silently never reaches the audit record.

Author: waleedlatif1

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', {

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,
     })

apps/sim/lib/auth/auth.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()
+  })
+})

apps/sim/lib/auth/connector-email.test.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}`
+}

apps/sim/lib/auth/connector-email.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 } : {}),
   }
 }

apps/sim/lib/credentials/atlassian-service-account.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,

apps/sim/lib/credentials/client-credential-accounts/descriptors.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,22 +80,21 @@ 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)
     expectMintCall()
     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 () => {

apps/sim/lib/credentials/client-credential-accounts/minters/box.test.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<ClientCredentialAccountIdentity> {
-  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<BoxCurrentUserResponse>(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<BoxCurrentUserResponse>(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))
   }
 }