Skip to content

Commit 7abd55f

Browse files
committed
fix(auth): end OAuth grants on SCIM address change and add sign out other devices
1 parent f79d322 commit 7abd55f

5 files changed

Lines changed: 124 additions & 10 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/settings/components/general/general.tsx‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
Button,
77
Chip,
88
ChipCombobox,
9+
ChipConfirmModal,
910
ChipModal,
1011
ChipModalBody,
1112
ChipModalError,
@@ -17,9 +18,11 @@ import {
1718
Label,
1819
Switch,
1920
Tooltip,
21+
toast,
2022
} from '@sim/emcn'
2123
import { Camera, Check, CircleInfo, Pencil } from '@sim/emcn/icons'
2224
import { createLogger } from '@sim/logger'
25+
import { getErrorMessage } from '@sim/utils/errors'
2326
import dynamic from 'next/dynamic'
2427
import Image from 'next/image'
2528
import { useRouter } from 'next/navigation'
@@ -47,6 +50,7 @@ import { useBrandConfig } from '@/ee/whitelabeling'
4750
import { useGeneralSettings, useUpdateGeneralSetting } from '@/hooks/queries/general-settings'
4851
import {
4952
useResetPassword,
53+
useSignOutOtherDevices,
5054
useUpdateUserProfile,
5155
useUserProfile,
5256
} from '@/hooks/queries/user-profile'
@@ -118,6 +122,8 @@ export function General() {
118122
})
119123
const [showResetPasswordModal, setShowResetPasswordModal] = useState(false)
120124
const resetPassword = useResetPassword()
125+
const [showSignOutOtherDevicesModal, setShowSignOutOtherDevicesModal] = useState(false)
126+
const signOutOtherDevices = useSignOutOtherDevices()
121127

122128
const [showDeleteAccountModal, setShowDeleteAccountModal] = useState(false)
123129

@@ -221,6 +227,14 @@ export function General() {
221227
)
222228
}
223229

230+
const confirmSignOutOtherDevices = () => {
231+
signOutOtherDevices.mutate(undefined, {
232+
onSuccess: () => toast.success('Signed out of other devices'),
233+
onError: (error) => toast.error(getErrorMessage(error)),
234+
onSettled: () => setShowSignOutOtherDevicesModal(false),
235+
})
236+
}
237+
224238
const handleThemeChange = async (value: string) => {
225239
await updateSetting.mutateAsync({ key: 'theme', value: value as 'system' | 'light' | 'dark' })
226240
}
@@ -288,6 +302,11 @@ export function General() {
288302
...(session?.user?.id && !isAuthDisabled
289303
? [
290304
{ id: 'sign-out', text: 'Sign out', onSelect: () => signOutAndRedirect(router.push) },
305+
{
306+
id: 'sign-out-other-devices',
307+
text: 'Sign out other devices',
308+
onSelect: () => setShowSignOutOtherDevicesModal(true),
309+
},
291310
{
292311
id: 'reset-password',
293312
text: 'Reset password',
@@ -640,6 +659,19 @@ export function General() {
640659
/>
641660
</ChipModal>
642661

662+
<ChipConfirmModal
663+
open={showSignOutOtherDevicesModal}
664+
onOpenChange={setShowSignOutOtherDevicesModal}
665+
title='Sign out other devices'
666+
text='This device and your authorized apps stay signed in.'
667+
confirm={{
668+
label: 'Sign out',
669+
onClick: confirmSignOutOtherDevices,
670+
pending: signOutOtherDevices.isPending,
671+
pendingLabel: 'Signing out...',
672+
}}
673+
/>
674+
643675
<DeleteAccountModal
644676
open={showDeleteAccountModal}
645677
onOpenChange={setShowDeleteAccountModal}

‎apps/sim/ee/scim/lib/application/users/provision-user.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import type { ScimUserAttributes } from '@sim/db/schema'
44
import { createLogger } from '@sim/logger'
55
import { APIError } from 'better-auth/api'
66
import { auth } from '@/lib/auth'
7+
import { revokeUserOAuthGrants } from '@/lib/auth/oauth-access-token'
78
import { applySessionPolicyToNewMember } from '@/lib/auth/session-policy'
89
import { syncUsageLimitsFromSubscription } from '@/lib/billing/core/usage'
910
import { ensureUserInOrganizationTx } from '@/lib/billing/organizations/membership'
@@ -184,8 +185,9 @@ export const provisionScimUser = defineAuthorizedScimUseCase({
184185
email,
185186
name: accountName(attributes),
186187
})
187-
if (emailChanged && attributes.active) {
188+
if (emailChanged) {
188189
await revokeUserSessionsTx(tx, { userId, organizationId: context.organizationId })
190+
await revokeUserOAuthGrants(userId, tx)
189191
}
190192
/** A relinked account may still carry the suspension a lost deprovisioning left behind. */
191193
if (attributes.active) await unsuspendMemberTx(tx, { userId, source: 'scim' })

‎apps/sim/ee/scim/lib/application/users/update-user.ts‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { db } from '@sim/db'
33
import type { ScimUserAttributes } from '@sim/db/schema'
44
import { normalizeEmail } from '@sim/utils/string'
55
import type { ScimPatchOperation } from '@/lib/api/contracts/scim'
6+
import { revokeUserOAuthGrants } from '@/lib/auth/oauth-access-token'
67
import { acquireOrganizationUserMutationLocks } from '@/lib/billing/organizations/membership'
78
import type { DbOrTx, DbTransaction } from '@/lib/db/types'
89
import type { StoppedRun } from '@/lib/mothership/async-runs/repository'
@@ -86,16 +87,15 @@ async function applyUserUpdate(
8687
})
8788

8889
/**
89-
* An address change ends the sessions established under the old one. A
90-
* deactivation in the same request revokes them itself, so this only runs
91-
* when nothing else will.
90+
* An address change ends the sessions and OAuth grants established under
91+
* the old one. Sessions go first: issuance references its session, so no
92+
* grant can be minted behind the sweep.
9293
*/
93-
if (!deactivated) {
94-
await revokeUserSessionsTx(tx, {
95-
userId: current.userId,
96-
organizationId: context.organizationId,
97-
})
98-
}
94+
await revokeUserSessionsTx(tx, {
95+
userId: current.userId,
96+
organizationId: context.organizationId,
97+
})
98+
await revokeUserOAuthGrants(current.userId, tx)
9999
} else if (nextName !== current.name) {
100100
await syncAccountIdentityTx(tx, { userId: current.userId, name: nextName })
101101
}

‎apps/sim/hooks/queries/user-profile.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
type UpdateUserProfileBody,
99
updateUserProfileContract,
1010
} from '@/lib/api/contracts/user'
11+
import { client } from '@/lib/auth/client'
1112
import {
1213
mapUserProfileResponse,
1314
USER_PROFILE_STALE_TIME,
@@ -88,3 +89,16 @@ export function useResetPassword() {
8889
},
8990
})
9091
}
92+
93+
/**
94+
* Ends every session except this one. Better Auth's plugin endpoint is called through the auth
95+
* client, as no Sim contract types it.
96+
*/
97+
export function useSignOutOtherDevices() {
98+
return useMutation({
99+
mutationFn: async () => {
100+
const { error } = await client.revokeOtherSessions()
101+
if (error) throw new Error(error.message ?? 'Failed to sign out other devices')
102+
},
103+
})
104+
}

‎apps/sim/scripts/test-scim-e2e.ts‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,72 @@ async function run() {
543543
}
544544
)
545545

546+
await check(
547+
'An address change ends sessions and OAuth grants issued under the old one',
548+
async () => {
549+
const carolEmail = `carol@${primary.domain}`
550+
const created = await scim(token, '/Users', {
551+
method: 'POST',
552+
expected: 201,
553+
body: {
554+
schemas: [USER_SCHEMA],
555+
userName: carolEmail,
556+
active: true,
557+
name: { givenName: 'Carol', familyName: 'Example' },
558+
emails: [{ value: carolEmail, type: 'work', primary: true }],
559+
},
560+
})
561+
const carolId = string(created.body.id)
562+
const userId = string(
563+
(await sql`select user_id from scim_user where id = ${carolId}`)[0]?.user_id
564+
)
565+
const sessionId = generateId()
566+
const clientId = generateId()
567+
await sql`insert into session (id, token, user_id, expires_at, created_at, updated_at)
568+
values (${sessionId}, ${generateShortId()}, ${userId}, now() + interval '1 day', now(), now())`
569+
await sql`insert into oauth_client (id, client_id, name, public, token_endpoint_auth_method, grant_types, redirect_uris)
570+
values (${clientId}, ${clientId}, 'SCIM E2E OAuth client', true, 'none',
571+
${['authorization_code', 'refresh_token']}, ${['http://127.0.0.1/callback']})`
572+
try {
573+
await sql`insert into oauth_consent (id, client_id, user_id, scopes, created_at, updated_at)
574+
values (${generateId()}, ${clientId}, ${userId}, ${['api:read']}, now(), now())`
575+
await sql`insert into oauth_token_family (id, client_id, user_id, session_id, created_at, expires_at)
576+
values (${generateId()}, ${clientId}, ${userId}, ${sessionId}, now(), now() + interval '1 day')`
577+
await sql`insert into oauth_access_token (id, token, client_id, user_id, scopes, created_at, expires_at)
578+
values (${generateId()}, ${generateShortId()}, ${clientId}, ${userId}, ${['api:read']},
579+
now(), now() + interval '1 hour')`
580+
581+
await patch(token, `/Users/${carolId}`, [
582+
{ op: 'replace', path: 'name.givenName', value: 'Caroline' },
583+
])
584+
assert.equal(
585+
(await sql`select id from oauth_access_token where user_id = ${userId}`).length,
586+
1
587+
)
588+
589+
await patch(token, `/Users/${carolId}`, [
590+
{
591+
op: 'replace',
592+
path: 'emails[type eq "work"].value',
593+
value: `carol-renamed@${primary.domain}`,
594+
},
595+
])
596+
597+
assert.equal((await sql`select id from session where user_id = ${userId}`).length, 0)
598+
assert.equal(
599+
(await sql`select id from oauth_token_family where user_id = ${userId}`).length,
600+
0
601+
)
602+
assert.equal(
603+
(await sql`select id from oauth_access_token where user_id = ${userId}`).length,
604+
0
605+
)
606+
} finally {
607+
await sql`delete from oauth_client where client_id = ${clientId}`
608+
}
609+
}
610+
)
611+
546612
await check(
547613
'Group CRUD supports Entra member PATCH, duplicates, foreign IDs, and membership-only PUT versions',
548614
async () => {

0 commit comments

Comments
 (0)