Skip to content

Commit 68bef59

Browse files
committed
fix(integrations): bound recorded sites and keep them when a reconnect cannot list them
1 parent b5e415d commit 68bef59

7 files changed

Lines changed: 106 additions & 29 deletions

File tree

‎apps/sim/lib/api/contracts/organization-accounts.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ import { organizationIdSchema, workspaceIdSchema } from '@/lib/api/contracts/pri
1818
import { defineRouteContract } from '@/lib/api/contracts/types'
1919
import { ORGANIZATION_CREDENTIAL_TYPES } from '@/lib/credential-groups/credential-types'
2020
import {
21+
ACCOUNT_SITE_LIMIT,
22+
ACCOUNT_SITE_NAME_MAX_LENGTH,
23+
ACCOUNT_SITE_URL_MAX_LENGTH,
2124
ORGANIZATION_ACCOUNT_WORKSPACE_LIMIT,
2225
ORGANIZATION_VIEWER_ACCOUNT_LIMIT,
2326
} from '@/lib/credential-groups/limits'
@@ -80,8 +83,13 @@ export const getOrganizationAccountsContract = defineRouteContract({
8083
optionId: z.string().min(1).max(128),
8184
status: z.enum(['active', 'needs_reauth']),
8285
sites: z
83-
.array(z.object({ name: z.string().max(512), url: z.string().max(2048) }))
84-
.max(50),
86+
.array(
87+
z.object({
88+
name: z.string().max(ACCOUNT_SITE_NAME_MAX_LENGTH),
89+
url: z.string().max(ACCOUNT_SITE_URL_MAX_LENGTH),
90+
})
91+
)
92+
.max(ACCOUNT_SITE_LIMIT),
8593
})
8694
)
8795
.max(ORGANIZATION_VIEWER_ACCOUNT_LIMIT)

‎apps/sim/lib/auth/connectors/managed-oauth.ts‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { OAuth2Client, type TokenPayload } from 'google-auth-library'
77
import { createRemoteJWKSet, type JWTPayload, jwtVerify } from 'jose'
88
import { buildConnectorProviders } from '@/lib/auth/connectors/providers'
99
import { readResponseJsonWithLimit } from '@/lib/core/utils/stream-limits'
10+
import { boundedAccountSites } from '@/lib/credential-groups/account-sites'
1011
import { getDocusignOAuthUrl } from '@/lib/oauth/docusign'
1112
import { verifyGitHubRepositoriesIdentity } from '@/lib/oauth/github-repositories'
1213
import { deriveMicrosoftEmailVerified, mapMicrosoftProfileToUser } from '@/lib/oauth/microsoft'
@@ -26,7 +27,6 @@ const ATLASSIAN_USER_INFO_URL = 'https://api.atlassian.com/me'
2627
const ATLASSIAN_USER_INFO_MAX_BYTES = 256 * 1024
2728
const ATLASSIAN_USER_INFO_TIMEOUT_MS = 10_000
2829
const ATLASSIAN_SITES_URL = 'https://api.atlassian.com/oauth/token/accessible-resources'
29-
const ATLASSIAN_SITE_LIMIT = 50
3030
const MICROSOFT_JWKS_URL = 'https://login.microsoftonline.com/common/discovery/v2.0/keys'
3131
const MICROSOFT_OIDC_USER_INFO_URL = 'https://graph.microsoft.com/oidc/userinfo'
3232
const MICROSOFT_OIDC_USER_INFO_MAX_BYTES = 256 * 1024
@@ -216,14 +216,12 @@ async function listAtlassianSites(accessToken: string): Promise<ManagedOAuthSite
216216
label: 'Atlassian accessible resources response',
217217
})
218218
if (!response.ok || !Array.isArray(body)) return undefined
219-
return body
220-
.flatMap((entry) => {
219+
return boundedAccountSites(
220+
body.map((entry) => {
221221
const site = toRecord(entry)
222-
const name = typeof site.name === 'string' ? site.name.trim() : ''
223-
const url = typeof site.url === 'string' ? httpsOrigin(site.url) : null
224-
return name && url ? [{ name, url }] : []
222+
return { name: site.name, url: typeof site.url === 'string' ? httpsOrigin(site.url) : null }
225223
})
226-
.slice(0, ATLASSIAN_SITE_LIMIT)
224+
)
227225
} catch {
228226
return undefined
229227
}

‎apps/sim/lib/credential-groups/__integration__/multiple-provider-accounts.integration.ts‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ vi.mock('@/lib/credential-groups/scoped-availability', () => ({
4242

4343
/** Each grant carries its own token, as a real exchange does, so a saved grant is identifiable. */
4444
let grants = 0
45+
/** Sites the next grant reports, or `undefined` when its site listing failed. */
46+
let grantSites: Array<{ name: string; url: string }> | undefined
4547

4648
vi.mock('@/lib/credential-groups/provider-registry', async (importOriginal) => {
4749
const { getCredentialGroupProviderId } = await import('@/lib/credential-groups/providers')
@@ -64,7 +66,7 @@ vi.mock('@/lib/credential-groups/provider-registry', async (importOriginal) => {
6466
providerSubjectId: subject,
6567
providerTenantId: tenant || null,
6668
displayName: code,
67-
metadata: { email: code },
69+
metadata: { email: code, ...(grantSites ? { sites: grantSites } : {}) },
6870
accessToken: `token-for-${code}-${(grants += 1)}`,
6971
grantedScopes: POLICY.requiredScopes,
7072
accessTokenExpiresAt: null,
@@ -230,6 +232,7 @@ beforeAll(async () => {
230232
})
231233

232234
beforeEach(async () => {
235+
grantSites = undefined
233236
await db.delete(credential).where(eq(credential.organizationId, organizationId))
234237
})
235238

@@ -383,6 +386,52 @@ describe('adding another site to an Atlassian login', () => {
383386
expect((await viewer())?.sites).toEqual([{ name: 'Acme', url: 'https://acme.example.net' }])
384387
})
385388

389+
it('never lets oversized stored sites break the account list', async () => {
390+
const account = await connect('person@atlassian', { kind: 'create' }, CONFLUENCE)
391+
const site = (index: number) => ({
392+
name: `Site ${index}`,
393+
url: `https://s${index}.example.net`,
394+
})
395+
await db
396+
.update(credential)
397+
.set({
398+
providerMetadata: sql`${JSON.stringify({
399+
email: 'person@atlassian',
400+
sites: [
401+
{ name: 'x'.repeat(600), url: 'https://long-name.example.net' },
402+
{ name: 'Long URL', url: `https://example.net/${'x'.repeat(2100)}` },
403+
...Array.from({ length: 60 }, (_, index) => site(index)),
404+
],
405+
})}::jsonb`,
406+
})
407+
.where(eq(credential.id, account.credentialId))
408+
409+
const [viewer] = await listViewerOrganizationAccounts({ organizationId, userId })
410+
expect(viewer?.sites).toHaveLength(50)
411+
expect(viewer?.sites[0]).toEqual(site(0))
412+
})
413+
414+
it('keeps the recorded sites when a reconnect cannot list them', async () => {
415+
grantSites = [{ name: 'Acme', url: 'https://acme.example.net' }]
416+
const account = await connect('person@atlassian', { kind: 'create' }, CONFLUENCE)
417+
const reconnect = () =>
418+
connect(
419+
'person@atlassian',
420+
{ kind: 'reconnect', credentialId: account.credentialId },
421+
CONFLUENCE
422+
)
423+
const sites = async () =>
424+
(await listViewerOrganizationAccounts({ organizationId, userId }))[0]?.sites
425+
426+
grantSites = undefined
427+
await reconnect()
428+
expect(await sites()).toEqual([{ name: 'Acme', url: 'https://acme.example.net' }])
429+
430+
grantSites = []
431+
await reconnect()
432+
expect(await sites()).toEqual([])
433+
})
434+
386435
it('still adds a different Atlassian login as its own account', async () => {
387436
await connect('person@atlassian', { kind: 'create' }, CONFLUENCE)
388437
await connect('other@atlassian', { kind: 'create' }, CONFLUENCE)
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import type { ManagedOAuthSite } from '@sim/db/schema'
2+
import { toArray, toRecord } from '@sim/utils/object'
3+
import {
4+
ACCOUNT_SITE_LIMIT,
5+
ACCOUNT_SITE_NAME_MAX_LENGTH,
6+
ACCOUNT_SITE_URL_MAX_LENGTH,
7+
} from '@/lib/credential-groups/limits'
8+
9+
/**
10+
* Keeps the well-formed sites within the recorded bounds. Site lists are display data from a
11+
* provider, so a bad entry is dropped rather than allowed to fail the account it belongs to.
12+
*/
13+
export function boundedAccountSites(value: unknown): ManagedOAuthSite[] {
14+
return toArray(value)
15+
.flatMap((entry) => {
16+
const site = toRecord(entry)
17+
const name = typeof site.name === 'string' ? site.name.trim() : ''
18+
const url = typeof site.url === 'string' ? site.url : ''
19+
return name &&
20+
url &&
21+
name.length <= ACCOUNT_SITE_NAME_MAX_LENGTH &&
22+
url.length <= ACCOUNT_SITE_URL_MAX_LENGTH
23+
? [{ name, url }]
24+
: []
25+
})
26+
.slice(0, ACCOUNT_SITE_LIMIT)
27+
}

‎apps/sim/lib/credential-groups/limits.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,7 @@ export const CREDENTIAL_GROUP_KNOWLEDGE_CONNECTOR_ACCESS_LIMIT = 32
99
export const ORGANIZATION_ACCOUNT_INDEXING_SOURCE_LIMIT = 100
1010
/** Live accounts one member may connect per provider; bounds a search's per-account fan-out. */
1111
export const CREDENTIAL_GROUP_ACCOUNTS_PER_PROVIDER_LIMIT = 5
12+
/** Bounds on the sites recorded for one account, enforced wherever they are written or read. */
13+
export const ACCOUNT_SITE_LIMIT = 50
14+
export const ACCOUNT_SITE_NAME_MAX_LENGTH = 512
15+
export const ACCOUNT_SITE_URL_MAX_LENGTH = 2048

‎apps/sim/lib/credential-groups/oauth.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,7 @@ async function persistGrant(
231231
id: credential.id,
232232
revokedAt: credential.revokedAt,
233233
managedOauthStatus: credential.managedOauthStatus,
234+
providerMetadata: credential.providerMetadata,
234235
providerSubjectId: credential.providerSubjectId,
235236
providerTenantId: credential.providerTenantId,
236237
encryptedOauthTokenSet: credential.encryptedOauthTokenSet,
@@ -287,7 +288,11 @@ async function persistGrant(
287288
providerTenantId: grant.providerTenantId,
288289
managedOauthStatus: 'active' as const,
289290
grantedScopes: grant.grantedScopes,
290-
providerMetadata: grant.metadata,
291+
// A site listing that failed reports no sites; keep the account's last known list.
292+
providerMetadata:
293+
grant.metadata.sites === undefined && existing?.providerMetadata?.sites
294+
? { ...grant.metadata, sites: existing.providerMetadata.sites }
295+
: grant.metadata,
291296
encryptedOauthTokenSet,
292297
grantedAt: now,
293298
revokedAt: null,

‎apps/sim/lib/credential-groups/viewer-accounts.ts‎

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,11 @@
11
import { db } from '@sim/db'
2-
import {
3-
credential,
4-
credentialGroup,
5-
credentialGroupEnrollment,
6-
type ManagedOAuthSite,
7-
} from '@sim/db/schema'
8-
import { toArray, toRecord } from '@sim/utils/object'
2+
import { credential, credentialGroup, credentialGroupEnrollment } from '@sim/db/schema'
3+
import { toRecord } from '@sim/utils/object'
94
import { and, eq, inArray, isNull, type SQL } from 'drizzle-orm'
105
import { resourceScopeCondition } from '@/lib/core/resource-scope.server'
6+
import { boundedAccountSites } from '@/lib/credential-groups/account-sites'
117
import { ORGANIZATION_VIEWER_ACCOUNT_LIMIT } from '@/lib/credential-groups/limits'
128

13-
/** Stored metadata is provider-shaped JSON, so only well-formed entries are shown. */
14-
function recordedSites(metadata: unknown): ManagedOAuthSite[] {
15-
return toArray(toRecord(metadata).sites).flatMap((entry) => {
16-
const site = toRecord(entry)
17-
return typeof site.name === 'string' && typeof site.url === 'string'
18-
? [{ name: site.name, url: site.url }]
19-
: []
20-
})
21-
}
22-
239
/** Own grant metadata remains manageable even while provider setup or enrollment is disabled. */
2410
export async function listViewerOrganizationAccounts(input: {
2511
organizationId: string
@@ -70,7 +56,7 @@ export async function listViewerOrganizationAccounts(input: {
7056
optionId: account.optionId,
7157
providerId: account.providerId,
7258
status: account.status,
73-
sites: recordedSites(metadata),
59+
sites: boundedAccountSites(toRecord(metadata).sites),
7460
}
7561
})
7662
}

0 commit comments

Comments
 (0)