fix(credentials): authorize shared credentials without requiring a workflow by waleedlatif1 · Pull Request #6571 · simstudioai/sim · GitHub
Skip to content

fix(credentials): authorize shared credentials without requiring a workflow - #6571

Merged
waleedlatif1 merged 1 commit into
stagingfrom
fix/kb-connector-credential-access
Aug 11, 2026
Merged

fix(credentials): authorize shared credentials without requiring a workflow#6571
waleedlatif1 merged 1 commit into
stagingfrom
fix/kb-connector-credential-access

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • authorizeCredentialUse could only reach its member-based sharing branch when a workflowId was supplied, so non-workflow surfaces (KB connectors, credential management) fell through to an owner-only path and rejected everyone but the user who ran the OAuth flow
  • a workflow now only pins which workspace a legacy account id resolves through — it never grants access on its own
  • legacy raw account.ids resolve through whichever workspace credential rows the caller can actually reach, instead of owner-only
  • extracted canUseCredential and replaced the predicate that was hand-inlined at five sites
  • reused resolveCredentialTokenIdentity for owner resolution instead of keeping a second local copy of the same invariant
  • the workflow-pinned path still cannot fall through to the owner path and cross a workspace boundary

Reported against RI - MDS: a workspace admin who did not run the SharePoint OAuth flow got Credential not found setting up a KB connector. The literal error came from the prod-only owner-scoped getCredential(…, auth.userId) in the connector-create route, which staging already replaced in #5273 — this PR fixes the systemic half underneath it.

Net effect: access is now decided by one rule everywhere — active credential member, or derived credential admin (workspace admin, for shared types). Membership stays explicit; a plain workspace member still needs a grant, same as in workflows.

Type of Change

  • Bug fix

Testing

New lib/auth/credential-access.test.ts (13 cases) covering the non-owner admin/member paths, workspace-loss on both requester and owner, the workflow-pinning invariant, and the unshared-account-stays-private case. Each guard test verified to fail against the pre-fix behavior. Full suites green across lib/auth, lib/credentials, lib/oauth, lib/knowledge, app/api/auth, app/api/credentials, app/api/knowledge, app/api/tools, executor.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@waleedlatif1
fix(credentials): authorize shared credentials without requiring a wo…
5f6bb43 
…rkflow

`authorizeCredentialUse` could only reach its member-based sharing branch when a
`workflowId` was supplied, so non-workflow surfaces — knowledge base connectors,
credential management — fell through to an owner-only path and rejected everyone
but the user who ran the OAuth flow.

A workflow now only pins which workspace a legacy account id resolves through; it
never grants access on its own. Access itself is decided by one rule everywhere:
active credential member, or derived credential admin.

- resolve legacy account ids through whichever workspace credential rows the
  caller can reach, instead of an owner-only fallback
- extract `canUseCredential` and replace the predicate hand-inlined at five sites
- reuse `resolveCredentialTokenIdentity` for owner resolution instead of a second
  local copy of the same invariant
- keep the workflow-pinned path from crossing a workspace boundary
@vercel

vercel Bot commented Aug 11, 2026

Copy link
Copy Markdown

@cursor

cursor Bot commented Aug 11, 2026 •
edited
Loading

Copy link
Copy Markdown

PR Summary

High Risk
Changes credential authorization logic across shared OAuth/service-account access paths. Incorrect checks could over-grant or deny credential use.

Overview
Fixes a bug where authorizeCredentialUse only applied shared credential membership checks when a workflowId was present. Non-workflow surfaces (KB connectors, credential management) fell through to an owner-only path and rejected everyone except the user who ran OAuth.

Access is now one rule everywhere: active credential member or derived credential admin. A workflowId only pins which workspace a legacy account id resolves through — it never grants access on its own. Legacy raw account ids resolve through reachable workspace credential rows, while unshared accounts stay private to their owner.

Also extracts canUseCredential to replace the duplicated use-check predicate across API routes and Vertex credential resolution, and adds coverage for the non-owner admin/member paths.

Reviewed by Cursor Bugbot for commit 5f6bb43. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR centralizes credential-use authorization and permits shared OAuth credentials to resolve without a workflow while preserving workflow workspace pinning and owner-only access for unshared legacy accounts.

  • Adds canUseCredential as the shared active-member or derived-admin predicate.
  • Resolves legacy account IDs through caller-accessible workspace credential rows.
  • Reuses canonical OAuth token identity resolution and expands authorization tests.

Confidence Score: 5/5

The PR appears safe to merge, with workflow boundaries, workspace access, credential membership, derived administration, and OAuth owner validation preserved.

The refactor consolidates existing credential-use checks and adds non-workflow shared-credential resolution without exposing an unsupported owner fallback or changing the resolved OAuth token identity.

Important Files Changed
Filename Overview
apps/sim/lib/auth/credential-access.ts Refactors credential authorization around canonical actor context and token identity resolution, adding accessible-workspace lookup for legacy account IDs without weakening workflow pinning.
apps/sim/lib/credentials/access.ts Adds a focused credential-use predicate that preserves the existing workspace-access and active-member-or-admin rule.
apps/sim/lib/auth/credential-access.test.ts Adds coverage for shared admins and members, workspace-access loss, workflow confinement, owner fallback, and unshared legacy accounts.
apps/sim/executor/utils/vertex-credential.ts Replaces a duplicated authorization condition with the shared predicate without changing behavior.
apps/sim/app/api/auth/oauth/credentials/route.ts Applies the shared credential-use predicate to OAuth credential listing paths.
apps/sim/app/api/credentials/[id]/route.ts Applies the shared credential-use predicate when retrieving an individual credential.

Flowchart

 
%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Credential-use request] --> B{Credential ID matches workspace credential?}
  B -->|Yes| C{Workflow pins another workspace?}
  C -->|Yes| X[Deny]
  C -->|No| D{Workspace access and member or admin?}
  D -->|No| X
  D -->|Yes| E{Credential type}
  E -->|Service account| F[Authorize workspace credential]
  E -->|OAuth| G[Resolve canonical token owner]
  G --> H{Owner retains workspace access?}
  H -->|No| X
  H -->|Yes| I[Authorize OAuth account]
  B -->|No: legacy account ID| J[Find wrapping OAuth credential rows]
  J --> K{Workflow supplied?}
  K -->|Yes| L[Restrict rows to workflow workspace]
  K -->|No| M[Consider all caller-reachable rows]
  L --> N{Usable row and valid token identity?}
  M --> N
  N -->|Yes| I
  N -->|No, workflow pinned| X
  N -->|No workflow and no usable row| O{Caller owns raw account?}
  O -->|Yes| I
  O -->|No| X
Loading

Reviews (1): Last reviewed commit: "fix(credentials): authorize shared crede..." | Re-trigger Greptile

@waleedlatif1
waleedlatif1 merged commit 3595fa2 into staging Aug 11, 2026
30 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/kb-connector-credential-access branch August 11, 2026 23:36
@waleedlatif1 waleedlatif1 mentioned this pull request Aug 12, 2026
Merged

This branch was previously deployed

1 inactive deployment
Preview — 5f6bb433 Deployed Aug 11, 2026 by vercel[bot]
View deployment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@waleedlatif1