diff --git a/apps/docs/content/docs/search/connect-your-account.mdx b/apps/docs/content/docs/search/connect-your-account.mdx index 34978bde2ff..ed708ab47e9 100644 --- a/apps/docs/content/docs/search/connect-your-account.mdx +++ b/apps/docs/content/docs/search/connect-your-account.mdx @@ -25,6 +25,8 @@ After your admin adds a source in **Settings → Sources**, connect your own acc To connect another account of the same app, such as Google Drive for two different work accounts, open the app and choose **Add account**, then pick the other account when the provider asks. You can connect up to five accounts per app, and Search looks across all of them. Open an account to see its tool permissions when the app supports them; each account's menu has **Reconnect** and **Disconnect**. - Slack accounts must belong to your organization's connected workspace. +- For another Confluence or Jira site on the same Atlassian login, choose **Add account** or **Reconnect** and pick the site. The site joins your existing account. Search reads up to four sites per Atlassian account in one search and notes when it leaves sites out. +- Each Notion workspace is its own account, so add one per workspace. If Sim asks you to reconnect first, reconnect your existing Notion account once, then add the other workspace. - Zoom, Lucid, and Databricks allow one account, because Sim cannot yet tell their accounts apart. - If the provider signs you straight back into a connected account, sign in to the other account with the provider first, then choose **Add account** again. diff --git a/apps/sim/lib/auth/connectors/managed-oauth.test.ts b/apps/sim/lib/auth/connectors/managed-oauth.test.ts index 6875ea9cc11..041d992b97b 100644 --- a/apps/sim/lib/auth/connectors/managed-oauth.test.ts +++ b/apps/sim/lib/auth/connectors/managed-oauth.test.ts @@ -276,6 +276,8 @@ describe('userinfo-backed managed OAuth connectors', () => { type: 'user', user: { id: 'user-1', name: 'Person', person: { email: 'person@example.com' } }, }, + workspace_id: 'workspace-1', + workspace_name: 'Engineering', }, }) @@ -286,6 +288,7 @@ describe('userinfo-backed managed OAuth connectors', () => { expect(identity).toMatchObject({ providerSubjectId: 'user-1', + providerTenantId: 'workspace-1', email: 'person@example.com', emailVerified: true, }) diff --git a/apps/sim/lib/auth/connectors/managed-oauth.ts b/apps/sim/lib/auth/connectors/managed-oauth.ts index 36f3fb0baf5..b10118070bd 100644 --- a/apps/sim/lib/auth/connectors/managed-oauth.ts +++ b/apps/sim/lib/auth/connectors/managed-oauth.ts @@ -905,7 +905,12 @@ const USER_INFO_MANAGED_OAUTH_CONNECTORS = new Map ManagedOAuthCon /** Notion only exposes `person.email` for a confirmed workspace member. */ emailVerified: true, }, - { displayName: user.name, avatarUrl: user.avatar_url } + /** A Notion token reaches one workspace, so one person's workspaces are separate accounts. */ + { + displayName: user.name, + avatarUrl: user.avatar_url, + providerTenantId: bot.workspace_id, + } ) }, }), diff --git a/apps/sim/lib/credential-groups/__integration__/multiple-provider-accounts.integration.ts b/apps/sim/lib/credential-groups/__integration__/multiple-provider-accounts.integration.ts index 602bc70e6db..4cd6b31a801 100644 --- a/apps/sim/lib/credential-groups/__integration__/multiple-provider-accounts.integration.ts +++ b/apps/sim/lib/credential-groups/__integration__/multiple-provider-accounts.integration.ts @@ -39,6 +39,9 @@ vi.mock('@/lib/credential-groups/scoped-availability', () => ({ isScopedCredentialGroupsAvailable: async () => true, })) +/** Each grant carries its own token, as a real exchange does, so a saved grant is identifiable. */ +let grants = 0 + vi.mock('@/lib/credential-groups/provider-registry', async (importOriginal) => { const { getCredentialGroupProviderId } = await import('@/lib/credential-groups/providers') return { @@ -61,7 +64,7 @@ vi.mock('@/lib/credential-groups/provider-registry', async (importOriginal) => { providerTenantId: tenant || null, displayName: code, metadata: { email: code }, - accessToken: `token-for-${code}`, + accessToken: `token-for-${code}-${(grants += 1)}`, grantedScopes: POLICY.requiredScopes, accessTokenExpiresAt: null, refreshTokenExpiresAt: null, @@ -93,6 +96,8 @@ const optionFor = (provider: CredentialGroupProvider) => ({ const DRIVE = optionFor('google-drive') const GITHUB = optionFor('github-repositories') const SLACK = optionFor('slack') +const CONFLUENCE = optionFor('confluence') +const NOTION = optionFor('notion') function connect( code: string, @@ -110,7 +115,7 @@ function connect( email, enrollmentStatus: 'completed', option, - options: [DRIVE, GITHUB, SLACK], + options: [DRIVE, GITHUB, SLACK, CONFLUENCE, NOTION], } const attempt: CredentialGroupOAuthAttempt = { userId, @@ -188,7 +193,7 @@ beforeAll(async () => { organizationId, publicId: generateId(), name: 'Connected accounts', - options: [DRIVE, GITHUB, SLACK], + options: [DRIVE, GITHUB, SLACK, CONFLUENCE, NOTION], }) await db.insert(member).values({ id: generateId(), @@ -251,7 +256,7 @@ describe('connecting several accounts of one provider', () => { status: 'active', }) expect(await decryptManagedOAuthTokenSet(stored?.tokens ?? '')).toMatchObject({ - accessToken: 'token-for-work@second-tenant.test', + accessToken: expect.stringMatching(/^token-for-work@second-tenant\.test-/), }) }) @@ -337,6 +342,78 @@ describe('reconnecting one of several accounts', () => { }) }) +describe('adding another site to an Atlassian login', () => { + it('keeps the new grant on the existing account instead of refusing it', async () => { + const first = await connect('person@atlassian', { kind: 'create' }, CONFLUENCE) + const saved = async () => + (await decryptManagedOAuthTokenSet((await rows())[0]?.tokens ?? '')).accessToken + const firstToken = await saved() + + const added = await connect('person@atlassian', { kind: 'create' }, CONFLUENCE) + + expect(added).toMatchObject({ created: false, credentialId: first.credentialId }) + expect(await rows()).toHaveLength(1) + expect(await saved()).toMatch(/^token-for-person@atlassian-/) + expect(await saved()).not.toBe(firstToken) + }) + + it('still adds a different Atlassian login as its own account', async () => { + await connect('person@atlassian', { kind: 'create' }, CONFLUENCE) + await connect('other@atlassian', { kind: 'create' }, CONFLUENCE) + expect(await rows()).toHaveLength(2) + }) +}) + +describe('an account recorded before its tenant was known', () => { + it('stays separate on a provider that always recorded its tenant', async () => { + const legacy = await connect('person', { kind: 'create' }) + expect( + await failureOf( + connect('person@workspace-a', { kind: 'reconnect', credentialId: legacy.credentialId }) + ) + ).toBe('account_mismatch') + }) + + it('takes on its tenant when reconnected or refreshed', async () => { + const legacy = await connect('person', { kind: 'create' }, NOTION) + + await expect( + connect( + 'person@workspace-a', + { kind: 'reconnect', credentialId: legacy.credentialId }, + NOTION + ) + ).resolves.toMatchObject({ created: false, credentialId: legacy.credentialId }) + expect(await rows()).toMatchObject([{ subject: 'person', tenant: 'workspace-a' }]) + }) + + it('takes on its tenant through an invitation link', async () => { + const legacy = await connect('person', { kind: 'create' }, NOTION) + await expect(connect('person@workspace-a', undefined, NOTION)).resolves.toMatchObject({ + credentialId: legacy.credentialId, + }) + expect(await rows()).toMatchObject([{ tenant: 'workspace-a' }]) + }) + + it('must be reconnected before another tenant of the same login is added, so it is never overwritten', async () => { + const legacy = await connect('person', { kind: 'create' }, NOTION) + const [before] = await rows() + + expect(await failureOf(connect('person@workspace-b', { kind: 'create' }, NOTION))).toBe( + 'account_identity_required' + ) + expect(await rows()).toEqual([before]) + + await connect( + 'person@workspace-a', + { kind: 'reconnect', credentialId: legacy.credentialId }, + NOTION + ) + await connect('person@workspace-b', { kind: 'create' }, NOTION) + expect((await rows()).map((row) => row.tenant).sort()).toEqual(['workspace-a', 'workspace-b']) + }) +}) + describe('adding an account that needs reconnecting', () => { it('refreshes it instead of calling it already connected', async () => { const first = await connect('work@first-tenant.test', { kind: 'create' }) diff --git a/apps/sim/lib/credential-groups/account-rules.ts b/apps/sim/lib/credential-groups/account-rules.ts index ef4fea3b4a2..a0f15652fdf 100644 --- a/apps/sim/lib/credential-groups/account-rules.ts +++ b/apps/sim/lib/credential-groups/account-rules.ts @@ -34,9 +34,15 @@ export function resolveAccountWrite({ limit, accounts, identity, + grantSpansSites = false, + tenantRecordedLater = false, }: { intent?: CredentialGroupConnectionIntent limit: number + /** Connecting the same login again extends its grant, so it refreshes that account. */ + grantSpansSites?: boolean + /** An account saved without a tenant may be this identity's, at a tenant not yet recorded. */ + tenantRecordedLater?: boolean accounts: Account[] identity: { subject: string; tenant: string | null } | null }): Account | undefined { @@ -52,6 +58,14 @@ export function resolveAccountWrite({ const disconnectedUnidentified = accounts.find( (account) => account.revokedAt && account.providerSubjectId === null ) + /** Saved before its provider reported a tenant: the same person, at a tenant still unknown. */ + const tenantUnknown = + tenantRecordedLater && identity && identity.tenant !== null && !existing + ? accounts.find( + (account) => + account.providerSubjectId === identity.subject && account.providerTenantId === null + ) + : undefined if (intent?.kind === 'reconnect') { const target = accounts.find((account) => account.id === intent.credentialId) @@ -61,7 +75,11 @@ export function resolveAccountWrite({ 409 ) if (identity) { - if (existing ? existing.id !== target.id : target.providerSubjectId !== null) + if ( + existing + ? existing.id !== target.id + : target.providerSubjectId !== null && target.id !== tenantUnknown?.id + ) throw accountConflict('account_mismatch') } else if ( target.providerSubjectId !== null && @@ -75,18 +93,20 @@ export function resolveAccountWrite({ if (intent?.kind === 'create') { if (existing && !existing.revokedAt) { - if (existing.managedOauthStatus === 'active') + if (existing.managedOauthStatus === 'active' && !grantSpansSites) throw accountConflict('account_already_connected') return existing } if (live.length > 0 && !identity) throw accountConflict('account_unverified') - if (hasUnidentifiedLive) throw accountConflict('account_identity_required') + if (hasUnidentifiedLive || (tenantUnknown && !tenantUnknown.revokedAt)) + throw accountConflict('account_identity_required') if (live.length >= limit) throw accountConflict('account_limit_reached') - return existing ?? (live.length === 0 ? disconnectedUnidentified : undefined) + return existing ?? (live.length === 0 ? (tenantUnknown ?? disconnectedUnidentified) : undefined) } if (existing && !existing.revokedAt) return existing - if (live.length === 0) return existing ?? disconnectedUnidentified + if (live.length === 0) return existing ?? tenantUnknown ?? disconnectedUnidentified + if (tenantUnknown && !tenantUnknown.revokedAt) return tenantUnknown if (live.length === 1 && !existing && (!identity || hasUnidentifiedLive)) return live[0] throw accountConflict(identity ? 'account_mismatch' : 'account_unverified') } diff --git a/apps/sim/lib/credential-groups/oauth.ts b/apps/sim/lib/credential-groups/oauth.ts index 9363c01e6a5..7853425c85e 100644 --- a/apps/sim/lib/credential-groups/oauth.ts +++ b/apps/sim/lib/credential-groups/oauth.ts @@ -35,6 +35,8 @@ import { import { getCredentialGroupProviderAdapter } from '@/lib/credential-groups/provider-registry' import { type CredentialGroupProvider, + credentialGroupGrantSpansSites, + credentialGroupTenantRecordedLater, getCredentialGroupProviderService, isCredentialGroupProvider, } from '@/lib/credential-groups/providers' @@ -247,6 +249,8 @@ async function persistGrant( limit: CREDENTIAL_GROUP_ACCOUNTS_PER_PROVIDER_LIMIT, accounts, identity: { subject: grant.providerSubjectId, tenant: grant.providerTenantId }, + grantSpansSites: credentialGroupGrantSpansSites(adapter.provider), + tenantRecordedLater: credentialGroupTenantRecordedLater(adapter.provider), }) let refreshToken = grant.refreshToken diff --git a/apps/sim/lib/credential-groups/providers.ts b/apps/sim/lib/credential-groups/providers.ts index e7d0fb36cdd..1a8276cbbf7 100644 --- a/apps/sim/lib/credential-groups/providers.ts +++ b/apps/sim/lib/credential-groups/providers.ts @@ -52,6 +52,17 @@ interface CredentialGroupProviderSupport { serviceId: string description: string configuration: 'oauth' | 'slack_custom_bot' + /** + * One login holds a single grant that gains each site it consents to, so connecting that login + * again adds a site to the existing account rather than a second account. + */ + grantSpansSites?: true + /** + * Sim began recording this provider's tenant after accounts were saved without one. Such an + * account takes on its tenant when the same person reconnects; elsewhere a missing tenant never + * matches another. + */ + tenantRecordedLater?: true } const CREDENTIAL_GROUP_PROVIDER_SUPPORT: Record< @@ -127,11 +138,13 @@ const CREDENTIAL_GROUP_PROVIDER_SUPPORT: Record< serviceId: 'confluence', description: 'Let each person connect one Confluence account', configuration: 'oauth', + grantSpansSites: true, }, jira: { serviceId: 'jira', description: 'Let each person connect one Jira account', configuration: 'oauth', + grantSpansSites: true, }, airtable: { serviceId: 'airtable', @@ -189,6 +202,7 @@ const CREDENTIAL_GROUP_PROVIDER_SUPPORT: Record< configuration: 'oauth', }, notion: { + tenantRecordedLater: true, serviceId: 'notion', description: 'Let each person connect one Notion account', configuration: 'oauth', @@ -258,6 +272,16 @@ export function getCredentialGroupProviderService( return service } +/** Whether connecting the same login again refreshes its existing account, adding sites to it. */ +export function credentialGroupGrantSpansSites(provider: CredentialGroupProvider): boolean { + return CREDENTIAL_GROUP_PROVIDER_SUPPORT[provider].grantSpansSites === true +} + +/** Whether an account saved without a tenant may take one on when the same person reconnects. */ +export function credentialGroupTenantRecordedLater(provider: CredentialGroupProvider): boolean { + return CREDENTIAL_GROUP_PROVIDER_SUPPORT[provider].tenantRecordedLater === true +} + export function getCredentialGroupProviderId(provider: CredentialGroupProvider): string { return getCredentialGroupProviderService(provider).providerId }