fix(credentials): capture the correct provider identity on connect and rotate - #6201
fix(credentials): capture the correct provider identity on connect and rotate#6201waleedlatif1 wants to merge 2 commits into
Conversation
…d 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.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryHigh Risk Overview OAuth connector callbacks now resolve the real authorizing user where APIs allow it (e.g. Attio via Service accounts adopt a required Credential update/create audit spreads provider metadata first so route keys cannot be shadowed. Google/Slack reconnect re-derives Reviewed by Cursor Bugbot for commit 416af7c. Configure here. |
Greptile SummaryThis PR standardizes connector and service-account identity capture while correcting Attio, Notion, and credential-rotation behavior.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains in the eligible follow-up review scope.
|
| Filename | Overview |
|---|---|
| apps/sim/lib/auth/auth.ts | Corrects provider identity traversal and adopts safe synthetic connector emails while preserving session-bound linking. |
| apps/sim/lib/auth/connector-email.ts | Centralizes deterministic placeholder addresses on the reserved .invalid domain. |
| apps/sim/lib/credentials/principal.ts | Defines and serializes the uniform required-and-nullable service-account principal model. |
| apps/sim/lib/credentials/service-account-secret.ts | Centralizes principal metadata in encrypted credential blobs and audit metadata for all builder variants. |
| apps/sim/lib/credentials/orchestration/index.ts | Rebuilds rotated credentials through shared verification while preserving explicit display-name precedence and audit authority. |
| apps/sim/lib/credentials/client-credential-accounts/minters/box.ts | Captures the Box service-account user and records identity lookup failures without rejecting token minting. |
| apps/sim/lib/credentials/client-credential-accounts/minters/salesforce.ts | Captures the Salesforce run-as user and distinguishes unavailable identity information from tenant-only credentials. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Connect or rotate credential] --> B[Verify provider secret]
B --> C[Resolve user or tenant identity]
C --> D[Build principal metadata]
D --> E[Encrypt credential blob]
D --> F[Write audit metadata]
E --> G[Store credential]
Reviews (2): Last reviewed commit: "fix(credentials): keep the provider-repo..." | Re-trigger Greptile
…p 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.
|
Second commit Scope is small: Box and Salesforce were returning early on a missing user id and discarding a @greptileai review bugbot run |
1 participant
Summary
data[0]off an unordered workspace-wide list. The wrong member id landed inaccount.accountId, and the stale-sibling dedupe keys on that prefix, so two Attio members connecting under one Sim user silently collapsed into one account row. Now resolves via/v2/self→/v2/workspace_members/{id}; no new scopesprofile.person.email, which never exists on a bot token, so every Notion connect fell through to a synthetic address. Fixed the traversal tobot.owner.user.person.emailand thenameprecedence (a bot's top-level name is always the integration's)@x.com,@hubspot.com,@salesforce.com,@atlassian.com). All 25 now route through one documented helper on.invalid(RFC 2606). Better Auth 1.6.23 hard-rejects a falsy email, so the placeholders are required — the helper explains why and when they can godisplayName, leaving the credential permanently labeled with the oldclient_email. Root cause was a hand-rolled branch bypassing the shared builder; deleting it also fixed rotation dropping audit metadataServiceAccountPrincipalunion (user / tenant / lookup_failed / null), required-and-nullable so a new provider can't compile without deciding, mirrored centrally into both audit and stored metadata.tenantexists because Attio, Shopify, Webflow, Zoom and Zoho Desk expose no actor at all;lookup_failedfinally distinguishes "the lookup broke" from "this provider has no identity"Type of Change
Testing
502 tests pass across the touched suites (3554/3555 repo-wide; the one failure is a pre-existing environmental
rg-on-PATH issue inexecutor/handlers/pi, untouched here). Typecheck,check:api-validationand biome clean.Every behavioral change was verified against the vendor's official docs. Not live-tested against real provider accounts — the Attio two-call flow and the Notion traversal each deserve one real connect before this goes out. Notion's fix is also inert until "user information with email addresses" is enabled in our Notion developer portal, which forces re-authorization and should be a separate decision.
Note for release: provider-named audit keys (
airtableUserId,trelloMemberId, …) are replaced by uniformprincipalId/principalKind/principalLabel. Historical rows keep the old keys, so any external drain filtering on them needs updating.Known limitations
displayNameand send it, which skips the re-derivation — it only fires if the user clears the name field first. The audit-metadata half of that fix does run on every rotation, so a rotated credential's new principal is always recorded; the stale label mostly persists. Worth a follow-up on the modals.principalLabelputs email addresses intoaudit_log.metadata, where previously only opaque ids lived. A deliberate data-classification change, not just a key rename.openidthe userinfo lookup can 403 and the run-as user degrades tolookup_failed. Non-fatal — the connection still works. Connect help text now says so.Verification notes
Every behavioral change was checked against vendor docs, and the three least-certain claims were independently re-verified: Salesforce's
subreally is documented as the UserInfo endpoint URL (so usinguser_idis correct), Attio'sauthorized_by_workspace_member_idis documented required for active tokens and this code path only ever sees user-authorized OAuth tokens, and Box'sidis a required property ofUser--Basein Box's own OpenAPI spec.Still not live-tested against real provider accounts — a real Attio connect and a real Notion connect are the remaining gap.
Checklist