Skip to content

Commit 1839da4

Browse files
committed
fix(knowledge): bound permission pagination and member cleanup
1 parent 5206a22 commit 1839da4

8 files changed

Lines changed: 497 additions & 75 deletions

File tree

‎apps/docs/content/docs/search/confluence.mdx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ In **Sync history**, **Continuing** means a healthy listing needs another batch.
142142
| Space picker is empty or fails | Check the domain, account's space access, and `read:space:confluence` scope. Manual space keys are also supported. |
143143
| Service-account validation fails | Check token expiry, site, Confluence app access, and the full scope list above, including `read:confluence-user`. |
144144
| Content syncs but Search is empty | Connect your personal Confluence identity. Check permission/directory sync errors and group-read scopes. |
145-
| **Some permissions could not be verified** | Open the source's **Sync history**. Check the service account's space, page, and directory access. If access is correct and the warning persists, ask your operator to inspect the connector run's permission errors. Do not broaden sharing to clear the warning. |
145+
| **Permission verification incomplete** | Open the source's **Sync history**. Check the service account's space, page, and directory access. If access is correct and the warning persists, ask your operator to inspect the connector run's permission errors, including incomplete or repeated permission pages. Documents without verified access stay hidden; do not broaden sharing to clear the warning. |
146146
| A new page, blog post, or label is missing | Confluence search can take time to update. Once the content appears in Confluence search with the selected label, sync again. |
147147
| A restricted page is missing | Both your account and the crawling account need access to the page and its ancestors. |
148148
| Embedded content is missing | Index the referenced page separately; remote macro output is excluded. |

‎apps/sim/app/o/[organizationId]/settings/integrations/sources/[connectorId]/source-detail.test.tsx‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { createRoot, type Root } from 'react-dom/client'
55
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
66
import { ApiClientError } from '@/lib/api/client/errors'
77
import type { ConnectorData } from '@/lib/api/contracts/knowledge/connectors'
8+
import { SOURCE_PERMISSION_ERROR } from '@/lib/knowledge/connectors/sync-limits'
89
import type { ConnectorActionsOptions } from '@/app/workspace/[workspaceId]/knowledge/[id]/components/connectors-section/use-connector-actions'
910

1011
const mocks = vi.hoisted(() => ({
@@ -257,6 +258,21 @@ describe('organization source detail navigation', () => {
257258
)
258259
})
259260

261+
it.each(['active', 'pending', 'syncing'] as const)(
262+
'keeps the safe permission warning visible while a source is %s',
263+
async (status) => {
264+
mocks.detail.mockReturnValue({
265+
data: { ...connector, accessMode: 'admin', status, lastSyncError: SOURCE_PERMISSION_ERROR },
266+
})
267+
await render()
268+
expect(container.textContent).toContain('Permission verification incomplete')
269+
expect(container.textContent).toContain(SOURCE_PERMISSION_ERROR)
270+
expect(container.textContent).not.toContain(
271+
'Review the connection settings and try syncing again.'
272+
)
273+
}
274+
)
275+
260276
it.each(['', '?view=settings', '?view=history'])(
261277
'shows integration deactivation independently of source sync state at %s',
262278
async (searchParams) => {

‎apps/sim/app/o/[organizationId]/settings/integrations/sources/[connectorId]/source-detail.tsx‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { useSettingsUnsavedGuard } from '@/components/settings/use-settings-unsa
1212
import { isApiClientError } from '@/lib/api/client/errors'
1313
import type { ConnectorData, ConnectorDetailData } from '@/lib/api/contracts/knowledge/connectors'
1414
import type { ResourceScope } from '@/lib/core/resource-scope'
15+
import { SOURCE_PERMISSION_ERROR } from '@/lib/knowledge/connectors/sync-limits'
1516
import { organizationRoutes } from '@/lib/navigation/paths'
1617
import { describeSearchSource } from '@/lib/sim-search/source-identity'
1718
import { SEARCH_DEBOUNCE_MS } from '@/lib/url-state'
@@ -186,6 +187,7 @@ function SourceDetailContent({
186187
? describeSearchSource(meta, connector.sourceConfig) || meta.name
187188
: 'Connection'
188189
const { effectiveStatus, lastSyncError } = getConnectorSyncState(connector)
190+
const permissionsIncomplete = lastSyncError === SOURCE_PERMISSION_ERROR
189191
const status =
190192
effectiveStatus === 'paused'
191193
? 'Sync paused'
@@ -271,12 +273,23 @@ function SourceDetailContent({
271273
>
272274
{integrationFeedback}
273275
<SourceNavigation view={view} onViewChange={onViewChange} />
274-
{effectiveStatus === 'active' && lastSyncError && (
275-
<SettingsResourceRow
276-
title='Some connection updates are incomplete'
277-
description='Review the connection settings and try syncing again.'
278-
/>
279-
)}
276+
{lastSyncError &&
277+
(effectiveStatus === 'active' ||
278+
(permissionsIncomplete &&
279+
(effectiveStatus === 'pending' || effectiveStatus === 'syncing'))) && (
280+
<SettingsResourceRow
281+
title={
282+
permissionsIncomplete
283+
? 'Permission verification incomplete'
284+
: 'Some connection updates are incomplete'
285+
}
286+
description={
287+
permissionsIncomplete
288+
? SOURCE_PERMISSION_ERROR
289+
: 'Review the connection settings and try syncing again.'
290+
}
291+
/>
292+
)}
280293
<ConnectorRecovery
281294
connector={connector}
282295
knowledgeBaseId={connector.knowledgeBaseId}

‎apps/sim/connectors/confluence/permissions.test.ts‎

Lines changed: 132 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
* @vitest-environment node
33
*/
44
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
import { MAX_ACL_TOKENS } from '@/lib/knowledge/access/tokens'
56
import {
67
getReadRestriction,
78
listAncestorIds,
@@ -292,20 +293,22 @@ describe('listSpaceReadPrincipals', () => {
292293
'rejects a repeated or missing cursor without publishing partial permissions: %s',
293294
async (next) => {
294295
mockFetch
295-
.mockResolvedValueOnce(jsonResponse({ results: [], _links: { next: '?cursor=next' } }))
296-
.mockResolvedValueOnce(jsonResponse({ results: [], _links: { next } }))
296+
.mockResolvedValueOnce(
297+
jsonResponse({ results: [{ id: 'first' }], _links: { next: '?cursor=next' } })
298+
)
299+
.mockResolvedValueOnce(jsonResponse({ results: [{ id: 'second' }], _links: { next } }))
297300

298301
await expect(listSpaceReadPrincipals(CLOUD, 'token', 'space-1')).rejects.toThrow(
299-
'invalid or repeated space permissions continuation'
302+
'invalid or repeated permission continuation'
300303
)
301304
expect(mockFetch).toHaveBeenCalledTimes(2)
302305
}
303306
)
304307

305308
it('rejects a cursor cycle rather than making a hundred repeated requests', async () => {
306-
for (const cursor of ['first', 'second', 'first']) {
309+
for (const [page, cursor] of ['first', 'second', 'first'].entries()) {
307310
mockFetch.mockResolvedValueOnce(
308-
jsonResponse({ results: [], _links: { next: `?cursor=${cursor}` } })
311+
jsonResponse({ results: [{ id: String(page) }], _links: { next: `?cursor=${cursor}` } })
309312
)
310313
}
311314
await expect(listSpaceReadPrincipals(CLOUD, 'token', 'space-1')).rejects.toThrow('repeated')
@@ -315,7 +318,7 @@ describe('listSpaceReadPrincipals', () => {
315318
it('rejects a malformed collection instead of treating it as a verified empty grant', async () => {
316319
mockFetch.mockResolvedValueOnce(jsonResponse({}))
317320
await expect(listSpaceReadPrincipals(CLOUD, 'token', 'space-1')).rejects.toThrow(
318-
'invalid space permissions'
321+
'invalid permission page'
319322
)
320323
})
321324

@@ -325,6 +328,7 @@ describe('listSpaceReadPrincipals', () => {
325328
jsonResponse({
326329
results: [
327330
{
331+
id: String(page),
328332
principal: { type: 'user', id: 'reader' },
329333
operation: { key: 'read', targetType: 'space' },
330334
},
@@ -333,9 +337,129 @@ describe('listSpaceReadPrincipals', () => {
333337
})
334338
)
335339
await expect(listSpaceReadPrincipals(CLOUD, 'token', 'space-1')).rejects.toThrow(
336-
'exceeded 100 pages (100 entries)'
340+
'exceeded 1000 pages (1000 entries)'
337341
)
338-
expect(mockFetch).toHaveBeenCalledTimes(100)
342+
expect(mockFetch).toHaveBeenCalledTimes(1000)
343+
})
344+
345+
it('reads past 25,000 assignments and only publishes readers after the final page', async () => {
346+
let page = 0
347+
mockFetch.mockImplementation(async () => {
348+
const current = page++
349+
return jsonResponse({
350+
results:
351+
current < 100
352+
? Array.from({ length: 250 }, (_, index) => ({
353+
id: `${current}-${index}`,
354+
principal: { type: 'user', id: 'editor' },
355+
operation: { key: 'create', targetType: 'page' },
356+
}))
357+
: [
358+
{
359+
id: 'last',
360+
principal: { type: 'group', id: 'readers' },
361+
operation: { key: 'read', targetType: 'space' },
362+
},
363+
],
364+
_links: current < 100 ? { next: `?cursor=${page}` } : {},
365+
})
366+
})
367+
await expect(listSpaceReadPrincipals(CLOUD, 'token', 'large-space')).resolves.toEqual([
368+
{ kind: 'group', id: 'readers' },
369+
])
370+
expect(mockFetch).toHaveBeenCalledTimes(101)
371+
})
372+
373+
it('rejects repeated assignments even when cursors and record order change', async () => {
374+
const reader = {
375+
id: 'one',
376+
principal: { type: 'user', id: 'reader' },
377+
operation: { key: 'read', targetType: 'space' },
378+
}
379+
mockFetch
380+
.mockResolvedValueOnce(
381+
jsonResponse({ results: [reader, { id: 'two' }], _links: { next: '?cursor=one' } })
382+
)
383+
.mockResolvedValueOnce(
384+
jsonResponse({ results: [{ id: 'two' }, reader], _links: { next: '?cursor=two' } })
385+
)
386+
await expect(listSpaceReadPrincipals(CLOUD, 'token', 'space')).rejects.toThrow(
387+
'repeated a permission page'
388+
)
389+
expect(mockFetch).toHaveBeenCalledTimes(2)
390+
})
391+
392+
it('follows an empty permission page that has a continuation', async () => {
393+
mockFetch.mockResolvedValueOnce(jsonResponse({ results: [], _links: { next: '?cursor=next' } }))
394+
mockFetch.mockResolvedValueOnce(
395+
jsonResponse({
396+
results: [
397+
{
398+
principal: { type: 'group', id: 'readers' },
399+
operation: { key: 'read', targetType: 'space' },
400+
},
401+
],
402+
})
403+
)
404+
await expect(listSpaceReadPrincipals(CLOUD, 'token', 'space')).resolves.toEqual([
405+
{ kind: 'group', id: 'readers' },
406+
])
407+
expect(mockFetch).toHaveBeenCalledTimes(2)
408+
})
409+
410+
it('bounds retained readers without truncating a large grant', async () => {
411+
let page = 0
412+
mockFetch.mockImplementation(async () =>
413+
jsonResponse({
414+
results: Array.from({ length: 250 }, (_, index) => ({
415+
principal: { type: 'user', id: `reader-${page}-${index}` },
416+
operation: { key: 'read', targetType: 'space' },
417+
})),
418+
_links: { next: `?cursor=${++page}` },
419+
})
420+
)
421+
await expect(listSpaceReadPrincipals(CLOUD, 'token', 'space')).rejects.toThrow(
422+
'document permission limit'
423+
)
424+
expect(mockFetch).toHaveBeenCalledTimes(Math.floor(MAX_ACL_TOKENS / 250) + 1)
425+
})
426+
427+
it('rejects an oversized permission response before accepting its readers', async () => {
428+
mockFetch.mockResolvedValueOnce(jsonResponse({ results: [{ id: 'x'.repeat(1024 * 1024) }] }))
429+
await expect(listSpaceReadPrincipals(CLOUD, 'token', 'space')).rejects.toThrow(
430+
'exceeds maximum size'
431+
)
432+
})
433+
434+
it('does not return a partial reader list when a later permission page fails', async () => {
435+
mockFetch
436+
.mockResolvedValueOnce(
437+
jsonResponse({
438+
results: [
439+
{
440+
principal: { type: 'user', id: 'reader' },
441+
operation: { key: 'read', targetType: 'space' },
442+
},
443+
],
444+
_links: { next: '?cursor=next' },
445+
})
446+
)
447+
.mockResolvedValueOnce(jsonResponse({}, 403))
448+
await expect(listSpaceReadPrincipals(CLOUD, 'token', 'space')).rejects.toThrow('403')
449+
expect(mockFetch).toHaveBeenCalledTimes(2)
450+
})
451+
452+
it('refuses an oversized continuation before issuing another request', async () => {
453+
mockFetch.mockResolvedValueOnce(
454+
jsonResponse({
455+
results: [],
456+
_links: { next: `?cursor=${'x'.repeat(8193)}` },
457+
})
458+
)
459+
await expect(listSpaceReadPrincipals(CLOUD, 'token', 'space')).rejects.toThrow(
460+
'invalid or repeated permission continuation'
461+
)
462+
expect(mockFetch).toHaveBeenCalledTimes(1)
339463
})
340464
})
341465

0 commit comments

Comments
 (0)