Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions apps/sim/hooks/queries/mcp.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
useAllowedMcpDomains,
useForceRefreshMcpTools,
useMcpServers,
useMcpToolServers,
useMcpToolsQuery,
useStoredMcpTools,
} from '@/hooks/queries/mcp'
Expand Down Expand Up @@ -118,6 +119,75 @@ class FakeEventSource {
close(): void {}
}

describe('useMcpToolServers', () => {
beforeEach(() => {
vi.clearAllMocks()
})

it('lists ordinary workspace servers when no managed connections are available', async () => {
const sharedServer = server('shared-server')
mockServers([
sharedServer,
server('managed-canonical-server', { credentialGroupId: 'group-1' }),
])

const hook = renderHookWithClient(() => useMcpToolServers(WORKSPACE_ID))
await flush()

expect(hook.getResult()).toEqual({ data: [sharedServer], isLoading: false, error: null })
hook.unmount()
})

it('includes allowed managed connections alongside ordinary servers', async () => {
const sharedServer = server('shared-server')
const managedServer = server('mcp-cg-123456789012345678901', {
name: 'Fireflies — person@example.com',
managedConnectorId: 'fireflies',
authType: 'oauth',
url: undefined,
})
mockRequestJson.mockImplementation(async (contract) => {
if (contract === listMcpServersContract) {
return { success: true, data: { servers: [sharedServer] } }
}
if (contract === listManagedMcpCatalogContract) return { servers: [managedServer], tools: [] }
throw new Error('Unexpected MCP request')
})

const hook = renderHookWithClient(() => useMcpToolServers(WORKSPACE_ID))
await flush()

expect(hook.getResult()).toEqual({
data: [sharedServer, managedServer],
isLoading: false,
error: null,
})
hook.unmount()
})

it.each([
{ name: 'shared servers', failingContract: listMcpServersContract },
{ name: 'managed catalog', failingContract: listManagedMcpCatalogContract },
])('keeps unrelated $name errors visible', async ({ failingContract }) => {
const error = new Error('MCP request failed')
mockRequestJson.mockImplementation(async (contract) => {
if (contract === failingContract) throw error
if (contract === listMcpServersContract) {
return { success: true, data: { servers: [server('shared-server')] } }
}
if (contract === listManagedMcpCatalogContract) return { servers: [], tools: [] }
throw new Error('Unexpected MCP request')
})

const hook = renderHookWithClient(() => useMcpToolServers(WORKSPACE_ID))
await flush()

expect(hook.getResult().error).toBe(error)
expect(hook.getResult().isLoading).toBe(false)
hook.unmount()
})
})

describe('useMcpToolsQuery', () => {
beforeEach(() => {
vi.clearAllMocks()
Expand Down
31 changes: 31 additions & 0 deletions apps/sim/lib/credential-groups/application/authorization.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,37 @@ describe('requireCredentialGroupCredentialAccess', () => {
await expect(requireAccess(executorPrincipal())).rejects.toMatchObject({ code: 'not_found' })
})

it('requires a live connector grant to execute a managed MCP credential', async () => {
mocks.loadBinding.mockResolvedValue(null)
const managedContext = { ...context, credentialType: 'mcp:fireflies' as const }
const principal = executorPrincipal()
const requireManagedAccess = () =>
requireCredentialGroupCredentialAccess(
principal,
managedContext,
credentialOperations.useManagedMcp.resourcePolicy
)

await expect(requireManagedAccess()).resolves.toBeUndefined()

mocks.requirePolicy.mockResolvedValue(storedPolicy([]))
await expect(requireManagedAccess()).rejects.toMatchObject({ code: 'forbidden' })

mocks.requirePolicy.mockResolvedValue({
document: buildOrganizationAccountAccessPolicy('group-1', [
{
workspaceId: context.workspaceId,
access: { mode: 'selected', credentialTypes: ['oauth:gmail'] },
},
]),
})
await expect(requireManagedAccess()).rejects.toMatchObject({ code: 'forbidden' })

mocks.requirePolicy.mockResolvedValue(storedPolicy())
mocks.isAvailable.mockResolvedValue(false)
await expect(requireManagedAccess()).rejects.toMatchObject({ code: 'not_found' })
})

it('rejects inconsistent Sim and external subject assertions before loading policy', async () => {
const simPrincipal = executorPrincipal()
simPrincipal.subjectUserId = 'user-2'
Expand Down
118 changes: 103 additions & 15 deletions apps/sim/lib/mcp/application/managed-connections.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/** @vitest-environment node */
import type { SessionPrincipal } from '@sim/auth/principal'
import { dbChainMockFns, queueTableRows, resetDbChainMock, schemaMock } from '@sim/testing'
import { eq } from 'drizzle-orm'
import { eq, inArray } from 'drizzle-orm'
import { beforeEach, describe, expect, it, vi } from 'vitest'

const mocks = vi.hoisted(() => ({
Expand All @@ -10,7 +10,8 @@ const mocks = vi.hoisted(() => ({
group: vi.fn(),
workspace: vi.fn(),
permission: vi.fn(),
requireAccess: vi.fn(),
scopedAvailable: vi.fn(),
policy: vi.fn(),
}))
vi.mock('@/lib/billing/core/workspace-access', () => ({
getWorkspaceOwnerSubscriptionAccess: mocks.billing,
Expand All @@ -21,16 +22,22 @@ vi.mock('@/lib/credential-groups/availability', () => ({
vi.mock('@/lib/credential-groups/credentials', () => ({
loadScopedAccountsCredentialListContext: mocks.group,
}))
vi.mock('@/lib/credential-groups/application/organization-workspace-access', () => ({
requireOrganizationAccountsWorkspaceAccess: mocks.requireAccess,
vi.mock('@/lib/credential-groups/scoped-availability', () => ({
isScopedCredentialGroupsAvailable: mocks.scopedAvailable,
}))
vi.mock('@/lib/resource-policies/repository', () => ({
requireResourcePolicy: mocks.policy,
}))
vi.mock('@/lib/mcp/application/context', () => ({ resolveMcpWorkspaceContext: mocks.workspace }))
vi.mock('@sim/platform-authz/workspace', () => ({
permissionSatisfies: (permission: string | null) => permission !== null,
resolveEffectiveWorkspacePermission: mocks.permission,
}))

import { buildOrganizationAccountAccessPolicy } from '@/lib/credential-groups/application/workspace-access-policy'
import {
buildOrganizationAccountAccessPolicy,
organizationAccountAccessPolicyCodec,
} from '@/lib/credential-groups/application/workspace-access-policy'
import { listManagedMcpConnectionsUseCase } from '@/lib/mcp/application/managed-connections'

const principal: SessionPrincipal = { kind: 'session', userId: 'user-1', sessionId: 'session-1' }
Expand All @@ -53,6 +60,7 @@ describe('managed MCP connection catalog', () => {
resetDbChainMock()
mocks.billing.mockResolvedValue({ organizationId: 'org-1' })
mocks.available.mockResolvedValue(true)
mocks.scopedAvailable.mockResolvedValue(true)
mocks.group.mockResolvedValue({ credentialGroupId: 'group-1' })
mocks.workspace.mockResolvedValue({
workspaceId: 'workspace-1',
Expand All @@ -61,11 +69,11 @@ describe('managed MCP connection catalog', () => {
billedAccountUserId: 'owner-1',
})
mocks.permission.mockResolvedValue('read')
mocks.requireAccess.mockResolvedValue(
buildOrganizationAccountAccessPolicy('group-1', [
mocks.policy.mockResolvedValue({
document: buildOrganizationAccountAccessPolicy('group-1', [
{ workspaceId: 'workspace-1', access: { mode: 'all' } },
])
)
]),
})
})

it('uses organization ownership and workspace access before exposing credential operations', async () => {
Expand All @@ -78,11 +86,12 @@ describe('managed MCP connection catalog', () => {
])
const result = await listManagedMcpConnectionsUseCase.execute({ principal, input })
expect(mocks.group).toHaveBeenCalledWith({ kind: 'organization', organizationId: 'org-1' })
expect(mocks.requireAccess).toHaveBeenCalledWith(
expect(mocks.policy).toHaveBeenCalledWith(
expect.objectContaining({
organizationId: 'org-1',
credentialGroupId: 'group-1',
workspaceId: 'workspace-1',
resourceType: 'credential_group',
resourceId: 'group-1',
codec: organizationAccountAccessPolicyCodec,
})
)
expect(eq).toHaveBeenCalledWith(schemaMock.credential.organizationId, 'org-1')
Expand All @@ -95,14 +104,93 @@ describe('managed MCP connection catalog', () => {
})
})

it('denies revoked workspace access before reading credentials', async () => {
mocks.requireAccess.mockRejectedValue(new Error('Workspace access revoked'))
it('returns an empty catalog when organization connected accounts are not configured', async () => {
mocks.group.mockResolvedValue(null)
await expect(listManagedMcpConnectionsUseCase.execute({ principal, input })).resolves.toEqual({
servers: [],
tools: [],
})
expect(mocks.policy).not.toHaveBeenCalled()
expect(dbChainMockFns.from).not.toHaveBeenCalled()
})

it.each(['workspace', 'organization'])(
'returns an empty catalog when %s availability is disabled',
async (scope) => {
const available = scope === 'workspace' ? mocks.available : mocks.scopedAvailable
available.mockResolvedValue(false)
await expect(listManagedMcpConnectionsUseCase.execute({ principal, input })).resolves.toEqual(
{
servers: [],
tools: [],
}
)
expect(mocks.policy).not.toHaveBeenCalled()
expect(dbChainMockFns.from).not.toHaveBeenCalled()
}
)

it.each([
{ name: 'no grants', grants: [] },
{
name: 'another workspace only',
grants: [{ workspaceId: 'other-workspace', access: { mode: 'all' as const } }],
},
{
name: 'OAuth only',
grants: [
{
workspaceId: input.workspaceId,
access: { mode: 'selected' as const, credentialTypes: ['oauth:gmail' as const] },
},
],
},
])('returns an empty catalog without an MCP workspace grant: $name', async ({ grants }) => {
mocks.policy.mockResolvedValue({
document: buildOrganizationAccountAccessPolicy('group-1', grants),
})
await expect(listManagedMcpConnectionsUseCase.execute({ principal, input })).resolves.toEqual({
servers: [],
tools: [],
})
expect(dbChainMockFns.from).not.toHaveBeenCalled()
})

it('only queries connectors granted to this workspace', async () => {
mocks.policy.mockResolvedValue({
document: buildOrganizationAccountAccessPolicy('group-1', [
{
workspaceId: input.workspaceId,
access: { mode: 'selected', credentialTypes: ['mcp:fireflies'] },
},
]),
})
await listManagedMcpConnectionsUseCase.execute({ principal, input })
expect(inArray).toHaveBeenCalledWith(schemaMock.mcpServers.managedConnectorId, ['fireflies'])
})

it('rejects callers without workspace access before checking catalog availability', async () => {
mocks.permission.mockResolvedValue(null)
await expect(listManagedMcpConnectionsUseCase.execute({ principal, input })).rejects.toThrow(
'revoked'
'Insufficient workspace permissions'
)
expect(mocks.billing).not.toHaveBeenCalled()
expect(mocks.policy).not.toHaveBeenCalled()
expect(dbChainMockFns.from).not.toHaveBeenCalled()
})

it.each(['scopedAvailable', 'policy'] as const)(
'propagates %s failures before reading credentials',
async (dependency) => {
const error = new Error('Database unavailable')
mocks[dependency].mockRejectedValue(error)
await expect(listManagedMcpConnectionsUseCase.execute({ principal, input })).rejects.toBe(
error
)
expect(dbChainMockFns.from).not.toHaveBeenCalled()
}
)

it.each([
[Array.from({ length: 501 }, () => metadata), 'connection limit'],
[[{ ...metadata, toolSnapshotBytes: 6 * 1024 * 1024 }], 'metadata limit'],
Expand Down
21 changes: 15 additions & 6 deletions apps/sim/lib/mcp/application/managed-connections.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,21 @@ import { credential, credentialGroup, credentialGroupEnrollment, mcpServers } fr
import { and, asc, eq, inArray, isNotNull, isNull, or, sql } from 'drizzle-orm'
import { getWorkspaceOwnerSubscriptionAccess } from '@/lib/billing/core/workspace-access'
import { defineAuthorizedWorkspaceUseCase } from '@/lib/core/application'
import { requireOrganizationAccountsWorkspaceAccess } from '@/lib/credential-groups/application/organization-workspace-access'
import { organizationAccountPolicyAllowsWorkspace } from '@/lib/credential-groups/application/workspace-access-policy'
import {
organizationAccountAccessPolicyCodec,
organizationAccountPolicyAllowsWorkspace,
} from '@/lib/credential-groups/application/workspace-access-policy'
import { isCredentialGroupsAvailable } from '@/lib/credential-groups/availability'
import { loadScopedAccountsCredentialListContext } from '@/lib/credential-groups/credentials'
import {
getManagedMcpConnector,
MANAGED_MCP_CONNECTOR_IDS,
} from '@/lib/credential-groups/managed-mcp-connectors'
import { isScopedCredentialGroupsAvailable } from '@/lib/credential-groups/scoped-availability'
import { resolveMcpWorkspaceContext } from '@/lib/mcp/application/context'
import { mcpServerOperations } from '@/lib/mcp/application/operations'
import type { McpToolSchema } from '@/lib/mcp/types'
import { requireResourcePolicy } from '@/lib/resource-policies/repository'

const MAX_MANAGED_MCP_CONNECTIONS = 500
const MAX_MANAGED_MCP_CATALOG_BYTES = 5 * 1024 * 1024
Expand Down Expand Up @@ -52,13 +56,18 @@ export const listManagedMcpConnectionsUseCase = defineAuthorizedWorkspaceUseCase
organizationId,
})
if (!group) return { servers: [], tools: [] }
const policy = await requireOrganizationAccountsWorkspaceAccess({
...context,
if (!(await isScopedCredentialGroupsAvailable({ kind: 'organization', organizationId }))) {
return { servers: [], tools: [] }
}
const policy = await requireResourcePolicy({
organizationId,
credentialGroupId: group.credentialGroupId,
resourceType: 'credential_group',
resourceId: group.credentialGroupId,
codec: organizationAccountAccessPolicyCodec,
})
/** Catalogs omit unavailable credentials; execution still requires explicit workspace access. */
const allowedConnectorIds = MANAGED_MCP_CONNECTOR_IDS.filter((id) =>
organizationAccountPolicyAllowsWorkspace(policy, context.workspaceId, `mcp:${id}`)
organizationAccountPolicyAllowsWorkspace(policy.document, context.workspaceId, `mcp:${id}`)
)
if (!allowedConnectorIds.length) return { servers: [], tools: [] }
const managedCatalogScope = () =>
Expand Down
Loading