Skip to content

fix(slack): only parse scoped user id for oauth credentials#4781

Merged
waleedlatif1 merged 2 commits into
stagingfrom
waleedlatif1/slack-scoped-userid-resolution
May 28, 2026
Merged

fix(slack): only parse scoped user id for oauth credentials#4781
waleedlatif1 merged 2 commits into
stagingfrom
waleedlatif1/slack-scoped-userid-resolution

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

Summary

  • Follow-up to feat(slack): scope private channel visibility to installing user #4779. The channel-privacy scoping looked up account.id using authz.resolvedCredentialId, but that value is an account.id only for OAuth credentials — the service_account path in credential-access.ts returns a credential.id. Guard the lookup on credentialType === 'oauth' so it's correct by construction.
  • Not reachable for Slack today (Slack has no service-account support), and the prior behavior was fail-safe (is_member fallback, no leak), but this makes intent explicit and avoids a misdirected account lookup if that ever changes.

Type of Change

  • Bug fix

Testing

Tested manually; tsc, biome, and check:api-validation pass

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)

@vercel
Copy link
Copy Markdown

vercel Bot commented May 28, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped May 28, 2026 11:06pm

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 28, 2026

PR Summary

Low Risk
Single conditional guard on an account lookup for private-channel filtering; no auth or token changes and prior behavior was already fail-safe.

Overview
The Slack channels API only resolves installing-user privacy scoping when the authorized credential is OAuth. It now runs the account table lookup (to parse the usr_ marker from accountId) only when authz.credentialType === 'oauth' and resolvedCredentialId is set, with comments clarifying that resolvedCredentialId is an account.id for OAuth but a credential.id for service accounts.

This avoids a wrong join if a non-OAuth path ever supplies resolvedCredentialId; behavior for Slack today is unchanged because service-account Slack credentials are not in play, and the prior miss would still fall back to bot is_member filtering.

Reviewed by Cursor Bugbot for commit 23645a1. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 28, 2026

Greptile Summary

This one-line fix guards the OAuth account lookup in the Slack channels route so it only runs when credentialType === 'oauth', preventing a misdirected DB query when resolvedCredentialId is a credential.id (service-account path) rather than an account.id (OAuth path).

  • Adds authz.credentialType === 'oauth' check before looking up account by authz.resolvedCredentialId, correctly scoping the guard to the path where resolvedCredentialId is an account.id.
  • As confirmed in credential-access.ts, the service-account path returns resolvedCredentialId: platformCredential.id (a credential ID, not an account ID), so the old guard could silently produce no accountRow and fall back to is_member — the fix makes the intent explicit and future-safe.

Confidence Score: 5/5

Safe to merge — the one-line guard is consistent with how credential-access.ts populates resolvedCredentialId and has no effect on today's Slack flows.

The change is a single conditional guard that adds an explicit type check before a DB lookup. Reading credential-access.ts confirms that every OAuth code path sets credentialType to 'oauth' (including the legacy account-ID path), so no valid OAuth flow is inadvertently blocked. The service-account path, which previously could cause a no-op account lookup, is now correctly excluded. The fail-safe fallback to is_member remains intact.

No files require special attention.

Important Files Changed

Filename Overview
apps/sim/app/api/tools/slack/channels/route.ts Added credentialType === 'oauth' guard before the account-ID DB lookup; change is correct and consistent with how credential-access.ts populates resolvedCredentialId for each credential type.

Sequence Diagram

sequenceDiagram
    participant Client
    participant SlackChannelsAPI
    participant authorizeCredentialUse
    participant DB

    Client->>SlackChannelsAPI: POST /api/tools/slack/channels
    SlackChannelsAPI->>authorizeCredentialUse: authorize(credentialId, workflowId)
    authorizeCredentialUse-->>SlackChannelsAPI: "{ credentialType, resolvedCredentialId, ... }"

    alt "credentialType === 'oauth' AND resolvedCredentialId set"
        SlackChannelsAPI->>DB: "SELECT accountId FROM account WHERE id = resolvedCredentialId"
        DB-->>SlackChannelsAPI: accountRow
        SlackChannelsAPI->>SlackChannelsAPI: parseScopedSlackUserId(accountRow.accountId)
        note over SlackChannelsAPI: scopedUserId extracted (or null for legacy credentials)
    else "credentialType === 'service_account' (skipped by guard)"
        note over SlackChannelsAPI: resolvedCredentialId is credential.id, NOT account.id — skip lookup
    end

    SlackChannelsAPI->>SlackAPI: conversations.list (all channels)
    SlackAPI-->>SlackChannelsAPI: channels[]

    opt scopedUserId present
        SlackChannelsAPI->>SlackAPI: "users.conversations?user=scopedUserId"
        SlackAPI-->>SlackChannelsAPI: allowedPrivateChannelIds
    end

    SlackChannelsAPI->>SlackChannelsAPI: filter channels by privacy + membership
    SlackChannelsAPI-->>Client: "{ channels }"
Loading

Reviews (2): Last reviewed commit: "chore(slack): trim guard comment" | Re-trigger Greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 23645a1. Configure here.

@waleedlatif1 waleedlatif1 merged commit d382df6 into staging May 28, 2026
14 checks passed
@waleedlatif1 waleedlatif1 deleted the waleedlatif1/slack-scoped-userid-resolution branch May 28, 2026 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant