fix(credentials): let workspace admins disconnect a teammate's OAuth credential - #6737
Conversation
…credential
Disconnecting an OAuth credential routed through POST /api/auth/oauth/disconnect,
a credential *user* operation scoped to the acting user's own `account` rows. A
workspace or org admin acting on a teammate's connection matched no accounts, so
the call returned `{ credentials: [] }` and the route still answered 200 — the UI
navigated away and the credential was still there. Not a denial, a silent no-op.
Route every type through the workspace-scoped credential delete instead, which
authorizes against credential admin and already resolves workspace and org admins
as derived credential admins for shared credential types.
That path only deleted the `credential` row, so send `oauth` through
`deleteCredentialRecord` — the manager that also tears down a credential's secret
source — and teach it to revoke the backing `account` grant once no credential
references it. Scoped by account id, not owner: the caller is already authorized
against the credential, and the grant belongs to the teammate.
Also make RoleLockTooltip layout-transparent. It wrapped locked controls in an
`inline-flex` div, which let the chip shrink to its label while unwrapped
controls stretched to the member row's fixed role track — so a credential's own
members list rendered Admin at two different widths. A `grid` wrapper stretches
like the unwrapped control, aligning the credential, secrets, and skills member
lists that share the row.
Follow-ups from review of the previous commit. - Collapse the delete use case's remaining `service_account` carve-out. Both ternary arms reached the same `deleteConnectionCredential` tail, but the carve-out skipped `deleteCredentialRecord`'s Slack custom-bot guard — and custom bots are exactly the type it guards, so the single-delete surface could orphan a credential group that the batch path refuses to. - Make `deleteOrphanedOAuthAccount` one conditional statement instead of a read then a write. `credential.accountId` is ON DELETE CASCADE, so a credential racing the gap would have been reaped by Postgres without `clearCredentialRefs` running, stranding its id in workflow state. - Give oauth its own branch in `deleteCredentialRecord`, matching the shape the env types already use, rather than a conditional tail after the return. - Point `handleReconnectCredential` at the shared helper; it carried its own copy of the same orphan-grant rule. - Invalidate the OAuth connections query on credential delete. The removed disconnect hook owned that invalidation, and the detail page reads it. This also retires the hand-dispatched `oauth-credentials-updated` event from the delete path; the connect/reconnect path still dispatches it for its listener. - Delete `useDisconnectOAuthService`, now callerless. - Trim comments to the behavior rather than the bug that motivated it.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Backend teardown is unified on Client/cache: Reviewed by Cursor Bugbot for commit d1d76bc. Configure here. |
Greptile SummaryThe PR consolidates credential disconnection onto the workspace-authorized deletion path and safely removes orphaned OAuth account grants while preserving shared grants.
Confidence Score: 5/5The PR appears safe to merge, with no concrete blocking or independently actionable non-blocking issues identified. The consolidated path retains canonical credential-admin authorization, performs type-specific cleanup, protects shared OAuth grants with an atomic all-reference check, and refreshes the affected client caches.
|
| Filename | Overview |
|---|---|
| apps/sim/lib/credentials/deletion.ts | Adds an atomic conditional deletion for OAuth account grants after all credential references are gone. |
| apps/sim/lib/credentials/orchestration/index.ts | Extends canonical record deletion to clean up orphaned OAuth grants while retaining type-specific guards and environment cleanup. |
| apps/sim/lib/credentials/application/service-account.ts | Consolidates all permitted credential types onto the canonical deletion manager without changing principal-specific type policy. |
| apps/sim/lib/credentials/draft-hooks.ts | Reuses the atomic orphan-account helper when reconnecting a credential to a different OAuth account. |
| apps/sim/app/workspace/[workspaceId]/integrations/connected/[credentialId]/connected-credential-detail.tsx | Routes every credential disconnect through the workspace-scoped credential deletion mutation. |
| apps/sim/hooks/queries/credentials.ts | Invalidates the matching OAuth connections query after credential deletion. |
| apps/sim/components/permissions/role-lock.tsx | Makes the locked-control tooltip wrapper stretch within its parent role track. |
Sequence Diagram
sequenceDiagram
participant UI as Credential detail UI
participant API as DELETE /api/credentials/:id
participant UC as Authorized delete use case
participant Manager as Credential record manager
participant DB as PostgreSQL
UI->>API: Disconnect credential
API->>UC: Principal + credential ID
UC->>UC: Load canonical scope and require credential admin
UC->>Manager: deleteCredentialRecord
Manager->>DB: Clear credential references
Manager->>DB: Delete credential row
alt OAuth credential with account ID
Manager->>DB: Delete account WHERE NOT EXISTS credential reference
end
Manager-->>UC: Authoritative deletion result
UC-->>API: Audit and result
API-->>UI: Success
UI->>UI: Invalidate credential and OAuth connection queries
Reviews (1): Last reviewed commit: "refactor(credentials): consolidate crede..." | Re-trigger Greptile
Summary
POST /api/auth/oauth/disconnect, a credential user operation scoped to the acting user's ownaccountrows. A workspace or org admin acting on a credential another user created matched no accounts, so it returned{ credentials: [] }and the route still answered 200 — the UI navigated away and the credential was still there. Not a denial, a silent no-op.credentialrow, so sendoauththroughdeleteCredentialRecord— the manager that also tears down a credential's secret source — and have it delete the backingaccountgrant once no credential references it. Scoped by account id, not owner: the caller is already authorized against the credential. A grant still used by a credential in another workspace survives.service_accountcarve-out. Both arms reached the same tail, but the carve-out skipped the Slack custom-bot credential-group guard — and custom bots are exactly the type it guards, so the single-delete surface could orphan a credential group that the batch path refuses to.deleteOrphanedOAuthAccountis one conditional statement rather than a read then a write.credential.accountIdisON DELETE CASCADE, so a credential racing the gap would be reaped by Postgres withoutclearCredentialRefsrunning, stranding its id in workflow state.oauth-credentials-updatedevent from the delete path; connect/reconnect still dispatches it. Deletes the now-callerlessuseDisconnectOAuthService.RoleLockTooltiplayout-transparent. It wrapped locked controls in aninline-flexdiv, letting the chip shrink to its label while unwrapped controls stretched to the member row's fixed role track — so one members list rendered the same role at two widths. Agridwrapper takes the width its parent offers and passes it through, aligning the credential, secrets, and skills member lists that share the row.Type of Change
Testing
bun run type-checkclean;bun run check:api-validationpasseslib/credentials,hooks,app/api/credentials,app/api/auth/oauthFollow-ups (not in this PR)
POST /api/auth/oauth/disconnectand its use case now have no caller. Left in place rather than removing an authenticated endpoint in a bugfix PR.performDeleteCredentialin credential orchestration also appears to have no remaining callers.MemberRow's role track is120pxwhen the viewer can manage and200pxwhen not — a width difference across viewers, never within one list.Checklist