Skip to content

Commit fab9bc7

Browse files
committed
fix(oauth): key Notion connections on their workspace
A Notion token reaches one workspace, but Sim keyed Notion accounts on the person alone, so a second workspace on the same login was refused. Record the workspace as the tenant. A connection saved before then takes on its workspace when the person reconnects, and must be reconnected before another workspace is added so it is never overwritten. Other providers keep treating a missing tenant as distinct.
1 parent 8fb0c9a commit fab9bc7

7 files changed

Lines changed: 97 additions & 7 deletions

File tree

‎apps/docs/content/docs/search/connect-your-account.mdx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ To connect another account of the same app, such as Google Drive for two differe
2626

2727
- Slack accounts must belong to your organization's connected workspace.
2828
- 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, and Search covers every site that login can reach.
29+
- 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.
2930
- Zoom, Lucid, and Databricks allow one account, because Sim cannot yet tell their accounts apart.
3031
- 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.
3132

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,8 @@ describe('userinfo-backed managed OAuth connectors', () => {
276276
type: 'user',
277277
user: { id: 'user-1', name: 'Person', person: { email: 'person@example.com' } },
278278
},
279+
workspace_id: 'workspace-1',
280+
workspace_name: 'Engineering',
279281
},
280282
})
281283

@@ -286,6 +288,7 @@ describe('userinfo-backed managed OAuth connectors', () => {
286288

287289
expect(identity).toMatchObject({
288290
providerSubjectId: 'user-1',
291+
providerTenantId: 'workspace-1',
289292
email: 'person@example.com',
290293
emailVerified: true,
291294
})

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -905,7 +905,12 @@ const USER_INFO_MANAGED_OAUTH_CONNECTORS = new Map<string, () => ManagedOAuthCon
905905
/** Notion only exposes `person.email` for a confirmed workspace member. */
906906
emailVerified: true,
907907
},
908-
{ displayName: user.name, avatarUrl: user.avatar_url }
908+
/** A Notion token reaches one workspace, so one person's workspaces are separate accounts. */
909+
{
910+
displayName: user.name,
911+
avatarUrl: user.avatar_url,
912+
providerTenantId: bot.workspace_id,
913+
}
909914
)
910915
},
911916
}),

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

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ const DRIVE = optionFor('google-drive')
9494
const GITHUB = optionFor('github-repositories')
9595
const SLACK = optionFor('slack')
9696
const CONFLUENCE = optionFor('confluence')
97+
const NOTION = optionFor('notion')
9798

9899
function connect(
99100
code: string,
@@ -111,7 +112,7 @@ function connect(
111112
email,
112113
enrollmentStatus: 'completed',
113114
option,
114-
options: [DRIVE, GITHUB, SLACK, CONFLUENCE],
115+
options: [DRIVE, GITHUB, SLACK, CONFLUENCE, NOTION],
115116
}
116117
const attempt: CredentialGroupOAuthAttempt = {
117118
userId,
@@ -189,7 +190,7 @@ beforeAll(async () => {
189190
organizationId,
190191
publicId: generateId(),
191192
name: 'Connected accounts',
192-
options: [DRIVE, GITHUB, SLACK, CONFLUENCE],
193+
options: [DRIVE, GITHUB, SLACK, CONFLUENCE, NOTION],
193194
})
194195
await db.insert(member).values({
195196
id: generateId(),
@@ -358,6 +359,56 @@ describe('adding another site to an Atlassian login', () => {
358359
})
359360
})
360361

362+
describe('an account recorded before its tenant was known', () => {
363+
it('stays separate on a provider that always recorded its tenant', async () => {
364+
const legacy = await connect('person', { kind: 'create' })
365+
expect(
366+
await failureOf(
367+
connect('person@workspace-a', { kind: 'reconnect', credentialId: legacy.credentialId })
368+
)
369+
).toBe('account_mismatch')
370+
})
371+
372+
it('takes on its tenant when reconnected or refreshed', async () => {
373+
const legacy = await connect('person', { kind: 'create' }, NOTION)
374+
375+
await expect(
376+
connect(
377+
'person@workspace-a',
378+
{ kind: 'reconnect', credentialId: legacy.credentialId },
379+
NOTION
380+
)
381+
).resolves.toMatchObject({ created: false, credentialId: legacy.credentialId })
382+
expect(await rows()).toMatchObject([{ subject: 'person', tenant: 'workspace-a' }])
383+
})
384+
385+
it('takes on its tenant through an invitation link', async () => {
386+
const legacy = await connect('person', { kind: 'create' }, NOTION)
387+
await expect(connect('person@workspace-a', undefined, NOTION)).resolves.toMatchObject({
388+
credentialId: legacy.credentialId,
389+
})
390+
expect(await rows()).toMatchObject([{ tenant: 'workspace-a' }])
391+
})
392+
393+
it('must be reconnected before another tenant of the same login is added, so it is never overwritten', async () => {
394+
const legacy = await connect('person', { kind: 'create' }, NOTION)
395+
const [before] = await rows()
396+
397+
expect(await failureOf(connect('person@workspace-b', { kind: 'create' }, NOTION))).toBe(
398+
'account_identity_required'
399+
)
400+
expect(await rows()).toEqual([before])
401+
402+
await connect(
403+
'person@workspace-a',
404+
{ kind: 'reconnect', credentialId: legacy.credentialId },
405+
NOTION
406+
)
407+
await connect('person@workspace-b', { kind: 'create' }, NOTION)
408+
expect((await rows()).map((row) => row.tenant).sort()).toEqual(['workspace-a', 'workspace-b'])
409+
})
410+
})
411+
361412
describe('adding an account that needs reconnecting', () => {
362413
it('refreshes it instead of calling it already connected', async () => {
363414
const first = await connect('work@first-tenant.test', { kind: 'create' })

‎apps/sim/lib/credential-groups/account-rules.ts‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,14 @@ export function resolveAccountWrite<Account extends ConnectedAccount>({
3535
accounts,
3636
identity,
3737
grantSpansSites = false,
38+
tenantRecordedLater = false,
3839
}: {
3940
intent?: CredentialGroupConnectionIntent
4041
limit: number
4142
/** Connecting the same login again extends its grant, so it refreshes that account. */
4243
grantSpansSites?: boolean
44+
/** An account saved without a tenant may be this identity's, at a tenant not yet recorded. */
45+
tenantRecordedLater?: boolean
4346
accounts: Account[]
4447
identity: { subject: string; tenant: string | null } | null
4548
}): Account | undefined {
@@ -55,6 +58,14 @@ export function resolveAccountWrite<Account extends ConnectedAccount>({
5558
const disconnectedUnidentified = accounts.find(
5659
(account) => account.revokedAt && account.providerSubjectId === null
5760
)
61+
/** Saved before its provider reported a tenant: the same person, at a tenant still unknown. */
62+
const tenantUnknown =
63+
tenantRecordedLater && identity && identity.tenant !== null && !existing
64+
? accounts.find(
65+
(account) =>
66+
account.providerSubjectId === identity.subject && account.providerTenantId === null
67+
)
68+
: undefined
5869

5970
if (intent?.kind === 'reconnect') {
6071
const target = accounts.find((account) => account.id === intent.credentialId)
@@ -64,7 +75,11 @@ export function resolveAccountWrite<Account extends ConnectedAccount>({
6475
409
6576
)
6677
if (identity) {
67-
if (existing ? existing.id !== target.id : target.providerSubjectId !== null)
78+
if (
79+
existing
80+
? existing.id !== target.id
81+
: target.providerSubjectId !== null && target.id !== tenantUnknown?.id
82+
)
6883
throw accountConflict('account_mismatch')
6984
} else if (
7085
target.providerSubjectId !== null &&
@@ -83,13 +98,15 @@ export function resolveAccountWrite<Account extends ConnectedAccount>({
8398
return existing
8499
}
85100
if (live.length > 0 && !identity) throw accountConflict('account_unverified')
86-
if (hasUnidentifiedLive) throw accountConflict('account_identity_required')
101+
if (hasUnidentifiedLive || (tenantUnknown && !tenantUnknown.revokedAt))
102+
throw accountConflict('account_identity_required')
87103
if (live.length >= limit) throw accountConflict('account_limit_reached')
88-
return existing ?? (live.length === 0 ? disconnectedUnidentified : undefined)
104+
return existing ?? (live.length === 0 ? (tenantUnknown ?? disconnectedUnidentified) : undefined)
89105
}
90106

91107
if (existing && !existing.revokedAt) return existing
92-
if (live.length === 0) return existing ?? disconnectedUnidentified
108+
if (live.length === 0) return existing ?? tenantUnknown ?? disconnectedUnidentified
109+
if (tenantUnknown && !tenantUnknown.revokedAt) return tenantUnknown
93110
if (live.length === 1 && !existing && (!identity || hasUnidentifiedLive)) return live[0]
94111
throw accountConflict(identity ? 'account_mismatch' : 'account_unverified')
95112
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import { getCredentialGroupProviderAdapter } from '@/lib/credential-groups/provi
3636
import {
3737
type CredentialGroupProvider,
3838
credentialGroupGrantSpansSites,
39+
credentialGroupTenantRecordedLater,
3940
getCredentialGroupProviderService,
4041
isCredentialGroupProvider,
4142
} from '@/lib/credential-groups/providers'
@@ -249,6 +250,7 @@ async function persistGrant(
249250
accounts,
250251
identity: { subject: grant.providerSubjectId, tenant: grant.providerTenantId },
251252
grantSpansSites: credentialGroupGrantSpansSites(adapter.provider),
253+
tenantRecordedLater: credentialGroupTenantRecordedLater(adapter.provider),
252254
})
253255

254256
let refreshToken = grant.refreshToken

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,12 @@ interface CredentialGroupProviderSupport {
5757
* again adds a site to the existing account rather than a second account.
5858
*/
5959
grantSpansSites?: true
60+
/**
61+
* Sim began recording this provider's tenant after accounts were saved without one. Such an
62+
* account takes on its tenant when the same person reconnects; elsewhere a missing tenant never
63+
* matches another.
64+
*/
65+
tenantRecordedLater?: true
6066
}
6167

6268
const CREDENTIAL_GROUP_PROVIDER_SUPPORT: Record<
@@ -196,6 +202,7 @@ const CREDENTIAL_GROUP_PROVIDER_SUPPORT: Record<
196202
configuration: 'oauth',
197203
},
198204
notion: {
205+
tenantRecordedLater: true,
199206
serviceId: 'notion',
200207
description: 'Let each person connect one Notion account',
201208
configuration: 'oauth',
@@ -269,6 +276,10 @@ export function credentialGroupGrantSpansSites(provider: CredentialGroupProvider
269276
return CREDENTIAL_GROUP_PROVIDER_SUPPORT[provider].grantSpansSites === true
270277
}
271278

279+
export function credentialGroupTenantRecordedLater(provider: CredentialGroupProvider): boolean {
280+
return CREDENTIAL_GROUP_PROVIDER_SUPPORT[provider].tenantRecordedLater === true
281+
}
282+
272283
export function getCredentialGroupProviderId(provider: CredentialGroupProvider): string {
273284
return getCredentialGroupProviderService(provider).providerId
274285
}

0 commit comments

Comments
 (0)