Skip to content

Commit b26ea1e

Browse files
TheodoreSpeaksTheodoreSpeaks
committed
fix(credential-groups): refresh selectors and validate credential ownership
1 parent b10cfc3 commit b26ea1e

9 files changed

Lines changed: 227 additions & 23 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/settings/navigation.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ describe('unified settings navigation', () => {
3333
{ id: 'organization', label: 'Members', section: 'organization' },
3434
{ id: 'usage', label: 'Usage tracking', section: 'organization' },
3535
{ id: 'secrets', label: 'Secrets', section: 'workspace' },
36-
{ id: 'connected-accounts', label: 'Connected accounts', section: 'organization' },
36+
{ id: 'connected-accounts', label: 'Credential Groups', section: 'organization' },
3737
{ id: 'custom-tools', label: 'Custom tools', section: 'workspace' },
3838
{ id: 'mcp', label: 'MCP tools', section: 'workspace' },
3939
{ id: 'apikeys', label: 'Sim API keys', section: 'workspace' },

‎apps/sim/ee/credential-groups/components/organization-workspace-grant-modal.tsx‎

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,33 @@ import { isOrganizationCredentialType } from '@/lib/credential-groups/credential
1717
type Grant = OrganizationAccountWorkspaceAccess['grants'][number]
1818
const ALL_INTEGRATIONS = 'all'
1919

20-
type OrganizationWorkspaceGrantModalProps = {
20+
interface OrganizationWorkspaceGrantModalBaseProps {
2121
credentialTypes: OrganizationAccountWorkspaceAccess['credentialTypes']
2222
disabled: boolean
2323
error?: string
2424
onSave: (grant: Grant) => void
2525
onClose: () => void
26-
} & (
27-
| { mode: 'create'; workspaces: OrganizationAccountWorkspaceAccess['workspaces'] }
28-
| { mode: 'edit'; grant: Grant; workspaceName: string; onRemove: () => void }
29-
)
26+
}
27+
28+
interface CreateOrganizationWorkspaceGrantModalProps
29+
extends OrganizationWorkspaceGrantModalBaseProps {
30+
mode: 'create'
31+
workspaces: OrganizationAccountWorkspaceAccess['workspaces']
32+
}
33+
34+
interface EditOrganizationWorkspaceGrantModalProps
35+
extends OrganizationWorkspaceGrantModalBaseProps {
36+
mode: 'edit'
37+
grant: Grant
38+
workspaceName: string
39+
onRemove: () => void
40+
}
41+
42+
type OrganizationWorkspaceGrantModalProps =
43+
| CreateOrganizationWorkspaceGrantModalProps
44+
| EditOrganizationWorkspaceGrantModalProps
3045

46+
/** All integrations is an explicit grant; an empty picker selection never grants access. */
3147
export function OrganizationWorkspaceGrantModal(props: OrganizationWorkspaceGrantModalProps) {
3248
const { credentialTypes, disabled, error, onSave, onClose } = props
3349
const [workspaceId, setWorkspaceId] = useState(

‎apps/sim/hooks/queries/organization-accounts.test.tsx‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import {
2727
import { slackSearchKeys } from '@/hooks/queries/slack-search'
2828
import { knowledgeKeys } from '@/hooks/queries/utils/knowledge-keys'
2929
import { searchSourceKeys } from '@/hooks/queries/utils/search-source-keys'
30+
import { selectorKeys, selectorQueryRoots } from '@/hooks/queries/utils/selector-keys'
3031

3132
describe('personal account disconnect', () => {
3233
it.each([true, false])(
@@ -163,6 +164,20 @@ describe('organization account setup updates', () => {
163164
const other = slackSearchKeys.manifest('org-2', 'Sim Search')
164165
const overview = searchSourceKeys.organizationOverview('org-1')
165166
const otherOverview = searchSourceKeys.organizationOverview('org-2')
167+
const providerSelectors = [
168+
selectorKeys.scoped(
169+
'workspace.credentialGroupProviders',
170+
{ kind: 'workspace', workspaceId: 'workspace-1' },
171+
'block-1'
172+
),
173+
selectorKeys.scoped(
174+
'workspace.organizationMcpProviders',
175+
{ kind: 'workspace', workspaceId: 'workspace-1' },
176+
'block-2'
177+
),
178+
[...selectorQueryRoots.workflowSearchReplace, 'workflow-1'],
179+
]
180+
for (const key of providerSelectors) client.setQueryData(key, { options: ['cached'] })
166181
for (const key of [current, renamed, other]) client.setQueryData(key, { existingApp: 'A1' })
167182
for (const key of [overview, otherOverview]) client.setQueryData(key, { providers: [] })
168183
try {
@@ -200,6 +215,8 @@ describe('organization account setup updates', () => {
200215
expect(client.getQueryState(other)?.isInvalidated).toBe(false)
201216
expect(client.getQueryState(overview)?.isInvalidated).toBe(success)
202217
expect(client.getQueryState(otherOverview)?.isInvalidated).toBe(false)
218+
for (const key of providerSelectors)
219+
expect(client.getQueryState(key)?.isInvalidated).toBe(success)
203220
} finally {
204221
await act(async () => root.unmount())
205222
client.clear()

‎apps/sim/hooks/queries/organization-accounts.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import { slackSearchKeys } from '@/hooks/queries/slack-search'
4343
import { mcpKeys } from '@/hooks/queries/utils/mcp-keys'
4444
import { resetOrganizationSearchAccess } from '@/hooks/queries/utils/reset-organization-search-access'
4545
import { searchSourceKeys } from '@/hooks/queries/utils/search-source-keys'
46+
import { invalidateSelectorQueries } from '@/hooks/queries/utils/selector-keys'
4647

4748
export const ORGANIZATION_ACCOUNTS_STALE_TIME = 30_000
4849

@@ -65,6 +66,9 @@ export function useDisconnectPersonalOrganizationAccount(organizationId: string)
6566
onSuccess: async () => {
6667
await Promise.all([
6768
resetOrganizationSearchAccess(queryClient, organizationId),
69+
queryClient.invalidateQueries({ queryKey: personalCredentialKeys.lists() }),
70+
queryClient.invalidateQueries({ queryKey: mcpKeys.managedCatalog() }),
71+
invalidateSelectorQueries(queryClient),
6872
queryClient.invalidateQueries({
6973
queryKey: organizationAccountsKeys.detail(organizationId),
7074
}),
@@ -149,6 +153,7 @@ export function useConfigureOrganizationMcp() {
149153
queryClient.invalidateQueries({ queryKey: organizationAccountsKeys.workspaces() }),
150154
queryClient.invalidateQueries({ queryKey: personalCredentialKeys.lists() }),
151155
queryClient.invalidateQueries({ queryKey: mcpKeys.managedCatalog() }),
156+
invalidateSelectorQueries(queryClient),
152157
]),
153158
})
154159
}
@@ -177,6 +182,7 @@ export function useUpdateOrganizationAccounts() {
177182
queryClient.invalidateQueries({ queryKey: organizationAccountsKeys.workspaces() }),
178183
queryClient.invalidateQueries({ queryKey: personalCredentialKeys.lists() }),
179184
queryClient.invalidateQueries({ queryKey: mcpKeys.managedCatalog() }),
185+
invalidateSelectorQueries(queryClient),
180186
queryClient.invalidateQueries({
181187
queryKey: slackSearchKeys.organizationManifests(organizationId),
182188
}),
@@ -245,6 +251,7 @@ export function useUpdateOrganizationAccountWorkspaceAccess() {
245251
queryClient.invalidateQueries({ queryKey: organizationAccountsKeys.workspaces() }),
246252
queryClient.invalidateQueries({ queryKey: personalCredentialKeys.lists() }),
247253
queryClient.invalidateQueries({ queryKey: mcpKeys.managedCatalog() }),
254+
invalidateSelectorQueries(queryClient),
248255
]),
249256
})
250257
}
@@ -334,6 +341,9 @@ export function useRevokeOrganizationAccountEnrollment() {
334341
onSuccess: (_, { organizationId }) =>
335342
Promise.all([
336343
resetOrganizationSearchAccess(queryClient, organizationId),
344+
queryClient.invalidateQueries({ queryKey: personalCredentialKeys.lists() }),
345+
queryClient.invalidateQueries({ queryKey: mcpKeys.managedCatalog() }),
346+
invalidateSelectorQueries(queryClient),
337347
queryClient.invalidateQueries({
338348
queryKey: organizationAccountsKeys.detail(organizationId),
339349
}),
@@ -359,6 +369,7 @@ export function useAddOrganizationAccountMcpProvider() {
359369
queryClient.invalidateQueries({ queryKey: organizationAccountsKeys.workspaces() }),
360370
queryClient.invalidateQueries({ queryKey: personalCredentialKeys.lists() }),
361371
queryClient.invalidateQueries({ queryKey: mcpKeys.managedCatalog() }),
372+
invalidateSelectorQueries(queryClient),
362373
]),
363374
})
364375
}
@@ -383,6 +394,7 @@ export function useRemoveOrganizationAccountMcpProvider() {
383394
queryClient.invalidateQueries({ queryKey: organizationAccountsKeys.workspaces() }),
384395
queryClient.invalidateQueries({ queryKey: personalCredentialKeys.lists() }),
385396
queryClient.invalidateQueries({ queryKey: mcpKeys.managedCatalog() }),
397+
invalidateSelectorQueries(queryClient),
386398
]),
387399
})
388400
}

‎apps/sim/lib/credential-groups/application/workspace-organization-accounts.test.ts‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
/** @vitest-environment node */
2+
import { queueTableRows, resetDbChainMock, schemaMock } from '@sim/testing'
3+
import { beforeEach, describe, expect, it, vi } from 'vitest'
4+
5+
const mocks = vi.hoisted(() => ({ group: vi.fn(), policy: vi.fn() }))
6+
vi.mock('@/lib/credential-groups/application/context', () => ({
7+
resolveCredentialGroupWorkspaceContext: async () => ({
8+
workspaceId: 'workspace-1',
9+
workspaceOrganizationId: 'org-1',
10+
allowPersonalApiKeys: true,
11+
}),
12+
}))
13+
vi.mock('@sim/platform-authz/workspace', () => ({
14+
resolveEffectiveWorkspacePermission: vi.fn().mockResolvedValue('read'),
15+
permissionSatisfies: (permission: string, required: string) => permission === required,
16+
}))
17+
vi.mock('@/lib/credential-groups/credentials', () => ({
18+
loadScopedAccountsCredentialListContext: mocks.group,
19+
}))
20+
vi.mock('@/lib/credential-groups/scoped-availability', () => ({
21+
isScopedCredentialGroupsAvailable: vi.fn().mockResolvedValue(true),
22+
}))
23+
vi.mock('@/lib/resource-policies/repository', () => ({ requireResourcePolicy: mocks.policy }))
24+
25+
import { buildOrganizationAccountAccessPolicy } from '@/lib/credential-groups/application/workspace-access-policy'
26+
import { getWorkspaceOrganizationAccounts } from '@/lib/credential-groups/application/workspace-organization-accounts'
27+
28+
function read() {
29+
return getWorkspaceOrganizationAccounts.execute({
30+
principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' },
31+
input: { workspaceId: 'workspace-1' },
32+
})
33+
}
34+
35+
describe('workspace organization provider projection', () => {
36+
beforeEach(() => {
37+
vi.clearAllMocks()
38+
resetDbChainMock()
39+
queueTableRows(schemaMock.organization, [{ name: 'Organization' }])
40+
queueTableRows(schemaMock.member, [{ role: 'member' }])
41+
queueTableRows(schemaMock.mcpServers, [{ connectorId: 'fireflies' }])
42+
mocks.group.mockResolvedValue({
43+
credentialGroupId: 'group-1',
44+
status: 'active',
45+
options: [
46+
{ provider: 'gmail', status: 'active' },
47+
{ provider: 'google-calendar', status: 'active' },
48+
{ provider: 'retired-provider', status: 'disabled' },
49+
],
50+
})
51+
mocks.policy.mockResolvedValue({
52+
document: buildOrganizationAccountAccessPolicy('group-1', [
53+
{ workspaceId: 'workspace-1', access: { mode: 'all' } },
54+
]),
55+
})
56+
})
57+
58+
it('ignores disabled legacy options before validating active providers', async () => {
59+
const result = await read()
60+
expect(result.providers.map(({ id }) => id)).toEqual(['google-email', 'google-calendar'])
61+
expect(result.mcpProviders.map(({ id }) => id)).toEqual(['fireflies'])
62+
})
63+
64+
it('projects only credential types allowed for the current workspace', async () => {
65+
mocks.policy.mockResolvedValue({
66+
document: buildOrganizationAccountAccessPolicy('group-1', [
67+
{
68+
workspaceId: 'workspace-1',
69+
access: { mode: 'selected', credentialTypes: ['oauth:gmail'] },
70+
},
71+
]),
72+
})
73+
const result = await read()
74+
expect(result.allowed).toBe(true)
75+
expect(result.providers.map(({ id }) => id)).toEqual(['google-email'])
76+
expect(result.mcpProviders).toEqual([])
77+
})
78+
79+
it('fails fast for an unregistered active provider', async () => {
80+
mocks.group.mockResolvedValue({
81+
credentialGroupId: 'group-1',
82+
status: 'active',
83+
options: [{ provider: 'unknown', status: 'active' }],
84+
})
85+
await expect(read()).rejects.toThrow('Unsupported organization provider: unknown')
86+
})
87+
})

‎apps/sim/lib/credential-groups/application/workspace-organization-accounts.ts‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -78,15 +78,13 @@ export const getWorkspaceOrganizationAccounts = defineAuthorizedWorkspaceUseCase
7878
if (!result.allowed) return result
7979
result.providers = group.options
8080
.filter((option) => {
81+
if (option.status !== 'active') return false
8182
if (!isCredentialGroupProvider(option.provider))
8283
throw new Error(`Unsupported organization provider: ${option.provider}`)
83-
return (
84-
option.status === 'active' &&
85-
organizationAccountPolicyAllowsWorkspace(
86-
policy.document,
87-
context.workspaceId,
88-
`oauth:${option.provider}`
89-
)
84+
return organizationAccountPolicyAllowsWorkspace(
85+
policy.document,
86+
context.workspaceId,
87+
`oauth:${option.provider}`
9088
)
9189
})
9290
.map((option) => {

‎apps/sim/lib/credentials/application/personal-credentials.test.ts‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,14 @@ describe('personal credential application access', () => {
117117
}
118118
mocks.listTokens.mockResolvedValue([token])
119119
queueTableRows(schemaMock.credential, [
120-
{ ...token, organizationId: null, groupId: 'legacy-group' },
120+
{
121+
...token,
122+
organizationId: null,
123+
workspaceId: 'workspace-1',
124+
groupId: 'legacy-group',
125+
groupOrganizationId: null,
126+
groupWorkspaceId: 'workspace-1',
127+
},
121128
])
122129
const result = await listPersonalCredentials.execute({
123130
principal,
@@ -142,7 +149,14 @@ describe('personal credential application access', () => {
142149
const managed = { ...personalCredential, providerId: 'slack', type: 'managed_oauth' as const }
143150
mocks.listPersonal.mockResolvedValue([managed])
144151
queueTableRows(schemaMock.credential, [
145-
{ ...managed, organizationId: null, groupId: 'legacy-group' },
152+
{
153+
...managed,
154+
organizationId: null,
155+
workspaceId: 'workspace-1',
156+
groupId: 'legacy-group',
157+
groupOrganizationId: null,
158+
groupWorkspaceId: 'workspace-1',
159+
},
146160
])
147161

148162
const result = await authorizePersonalCredential.execute({

‎apps/sim/lib/credentials/application/workspace-account-visibility.test.ts‎

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,14 @@ const entries = [
1818
{ id: 'calendar', type: 'managed_oauth', providerId: 'google-calendar' },
1919
{ id: 'token', type: 'personal_token', providerId: 'gitlab' },
2020
]
21-
const bindings = entries
22-
.slice(1)
23-
.map((entry) => ({ ...entry, organizationId: 'org', groupId: 'group' }))
21+
const bindings = entries.slice(1).map((entry) => ({
22+
...entry,
23+
organizationId: 'org',
24+
workspaceId: null,
25+
groupId: 'group',
26+
groupOrganizationId: 'org',
27+
groupWorkspaceId: null,
28+
}))
2429

2530
beforeEach(() => {
2631
vi.clearAllMocks()
@@ -50,7 +55,10 @@ describe('workspace organization credential visibility', () => {
5055
expect(dbChainMockFns.select).toHaveBeenCalledExactlyOnceWith({
5156
id: schemaMock.credential.id,
5257
organizationId: schemaMock.credential.organizationId,
53-
groupId: schemaMock.credentialGroupEnrollment.credentialGroupId,
58+
workspaceId: schemaMock.credential.workspaceId,
59+
groupId: schemaMock.credentialGroup.id,
60+
groupOrganizationId: schemaMock.credentialGroup.organizationId,
61+
groupWorkspaceId: schemaMock.credentialGroup.workspaceId,
5462
providerId: schemaMock.credential.providerId,
5563
type: schemaMock.credential.type,
5664
})
@@ -87,13 +95,48 @@ describe('workspace organization credential visibility', () => {
8795
it('preserves independently managed workspace accounts', async () => {
8896
queueTableRows(
8997
schemaMock.credential,
90-
bindings.map((binding) => ({ ...binding, organizationId: null }))
98+
bindings.map((binding) => ({
99+
...binding,
100+
organizationId: null,
101+
workspaceId: 'ws',
102+
groupOrganizationId: null,
103+
groupWorkspaceId: 'ws',
104+
}))
91105
)
92106
expect(await filterWorkspaceAccountCredentials(context, entries)).toEqual(entries)
93107
expect(mocks.available).not.toHaveBeenCalled()
94108
expect(mocks.policy).not.toHaveBeenCalled()
95109
})
96110

111+
it.each([
112+
{ groupOrganizationId: 'other-org', groupWorkspaceId: null },
113+
{ groupOrganizationId: null, groupWorkspaceId: 'ws' },
114+
])('rejects mismatched group ownership before loading policy: %j', async (owner) => {
115+
queueTableRows(
116+
schemaMock.credential,
117+
bindings.map((binding) => ({ ...binding, ...owner }))
118+
)
119+
await expect(filterWorkspaceAccountCredentials(context, entries)).rejects.toThrow(
120+
'Credential and enrollment group owners do not match'
121+
)
122+
expect(mocks.policy).not.toHaveBeenCalled()
123+
})
124+
125+
it('hides independently managed credentials that moved to another workspace', async () => {
126+
queueTableRows(
127+
schemaMock.credential,
128+
bindings.map((binding) => ({
129+
...binding,
130+
organizationId: null,
131+
workspaceId: 'other-ws',
132+
groupOrganizationId: null,
133+
groupWorkspaceId: 'other-ws',
134+
}))
135+
)
136+
expect(await filterWorkspaceAccountCredentials(context, entries)).toEqual([entries[0]])
137+
expect(mocks.policy).not.toHaveBeenCalled()
138+
})
139+
97140
it('throws for malformed policy or a changed canonical provider instead of granting access', async () => {
98141
queueTableRows(schemaMock.credential, bindings)
99142
mocks.policy.mockRejectedValueOnce(new Error('Malformed policy'))

0 commit comments

Comments
 (0)