Skip to content

Commit 4e68846

Browse files
committed
fix(sso): close the fail-open paths an independent audit found
- Ask the entitlement read to throw rather than answer "not entitled" on an outage, so a blip cannot drop the requirement and cache that - Keep the membership read's old resilience for paths the requirement allows anyway, and fail closed only on the ones it governs - Say why a verification code was refused instead of calling it invalid - Report the requirement from fresh reads, so the settings surface cannot disagree with what sign-in enforces - Document that social sign-in is refused too, and how an owner without a password gets back in
1 parent b84724b commit 4e68846

10 files changed

Lines changed: 101 additions & 28 deletions

File tree

‎apps/docs/content/docs/platform/enterprise/sso.mdx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -324,10 +324,10 @@ SSO provisioning creates internal organization members but does not grant worksp
324324
By default members can sign in with a password, an email code, or your identity provider. To make single sign-on the only way in, open **Settings → Organization → Single sign-on → Sign-in** and set **Allowed sign-in methods** to **Single sign-on**. It becomes available once the organization has an identity provider on a verified domain.
325325

326326
- The requirement is checked when a session is created, so turning it on signs nobody out. Members keep working and meet the requirement at their next sign-in. To end current sessions too, use **Sign out all members** under **Settings → Organization → Security**.
327-
- Organization owners keep every sign-in method. If the identity provider breaks, an owner can still sign in with a password and turn the requirement back off.
327+
- Organization owners keep every sign-in method. If the identity provider breaks, an owner can still sign in with a password — setting one through **Forgot password** if they only ever signed in through the identity provider — and turn the requirement back off.
328328
- If the last identity provider is deleted or its domain verification lapses, the requirement stops being enforced instead of locking the organization out, and you can switch back to any method at any time.
329329
- Desktop app handoff from an already signed-in browser keeps working, because that session derives from one the requirement already admitted.
330-
- A member who tries a password or email code sees a message telling them to sign in through their identity provider.
330+
- A member who tries a password, an email code, or a social sign-in such as Google or GitHub sees a message telling them to sign in through their identity provider.
331331

332332
<Callout type="info">
333333
Turning the requirement off restores password and email sign-in immediately for everyone.
@@ -362,7 +362,7 @@ By default members can sign in with a password, an email code, or your identity
362362
},
363363
{
364364
question: "Can I still use email/password login after enabling SSO?",
365-
answer: "Yes, unless you require single sign-on. Enabling SSO does not disable password login on its own; set Allowed sign-in methods to Single sign-on to refuse password and email-code sign-in for members. Organization owners keep password sign-in as a way back in if the identity provider breaks."
365+
answer: "Yes, unless you require single sign-on. Enabling SSO does not disable password login on its own; set Allowed sign-in methods to Single sign-on to refuse password, email-code, and social sign-in for members. Organization owners keep password sign-in as a way back in if the identity provider breaks, and can set a password through Forgot password if they never had one."
366366
},
367367
{
368368
question: "A user already has an account with the same email — what happens when they sign in with SSO?",

‎apps/sim/app/(auth)/verify/use-verification.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { createLogger } from '@sim/logger'
55
import { normalizeEmail } from '@sim/utils/string'
66
import { useSearchParams } from 'next/navigation'
77
import { client, useSession } from '@/lib/auth/auth-client'
8+
import { SSO_REQUIRED_ERROR_CODE } from '@/lib/auth/constants'
89
import { validateCallbackUrl } from '@/lib/core/security/input-validation'
910
import { DEFAULT_POST_AUTH_ROUTE, POST_AUTH_REDIRECT_STORAGE_KEY } from '@/app/(auth)/auth-redirect'
1011

@@ -122,7 +123,14 @@ export function useVerification({
122123
}, 1000)
123124
} else {
124125
logger.info('Setting invalid OTP state - API error response')
125-
const message = 'Invalid verification code. Please check and try again.'
126+
/**
127+
* A refusal by policy — an organization requiring single sign-on — is not a bad code, and
128+
* telling the person to re-check their code sends them round a loop they cannot exit.
129+
*/
130+
const message =
131+
response?.error?.code === SSO_REQUIRED_ERROR_CODE && response.error.message
132+
? response.error.message
133+
: 'Invalid verification code. Please check and try again.'
126134
setStatus('error')
127135
setErrorMessage(message)
128136
logger.info('Error state after API error:', { errorMessage: message })

‎apps/sim/app/oauth-error/page.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { Metadata } from 'next'
2-
import { SSO_REQUIRED_ERROR_CODE, SSO_REQUIRED_MESSAGE } from '@/lib/auth/sso-policy'
2+
import { SSO_REQUIRED_ERROR_CODE, SSO_REQUIRED_MESSAGE } from '@/lib/auth/constants'
33
import { DesktopHandoffShell } from '@/app/desktop/components/desktop-handoff-shell'
44

55
export const metadata: Metadata = {

‎apps/sim/lib/auth/constants.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,3 +106,13 @@ export function applyRegistrationGate<T extends Record<string, RegistrationGate>
106106
/** The spread widens past what TypeScript can prove; the keys are unchanged. */
107107
return gated as T
108108
}
109+
110+
/**
111+
* How a sign-in refused by an organization's single sign-on requirement identifies itself. Here
112+
* rather than beside the policy so the sign-in and verification screens can recognize the refusal
113+
* without pulling the policy module — and its database dependencies — into the browser bundle.
114+
*/
115+
export const SSO_REQUIRED_ERROR_CODE = 'SSO_REQUIRED'
116+
117+
export const SSO_REQUIRED_MESSAGE =
118+
'Your organization requires single sign-on. Sign in through your identity provider.'

‎apps/sim/lib/auth/session-hooks.test.ts‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@ vi.mock('@/lib/auth/access-control', () => ({
1313
isEmailBlockedByAccessControl: isBlocked,
1414
}))
1515

16+
import { SSO_REQUIRED_MESSAGE } from '@/lib/auth/constants'
1617
import { runWithAuthDatabase } from '@/lib/auth/database-context'
1718
import { prepareSessionForCreation } from '@/lib/auth/session-hooks'
1819
import { invalidateSessionPolicyCache } from '@/lib/auth/session-policy'
19-
import { SSO_REQUIRED_MESSAGE } from '@/lib/auth/sso-policy'
2020

2121
const createdAt = new Date('2026-09-08T00:00:00Z')
2222
const session: Session = {
@@ -108,6 +108,34 @@ describe('prepareSessionForCreation', () => {
108108
).rejects.toThrow(SSO_REQUIRED_MESSAGE)
109109
})
110110

111+
it('still signs in through the identity provider when the membership read fails', async () => {
112+
setEnvFlags({ isBillingEnabled: false, isSsoEnabled: true })
113+
const { executor, limit } = transactionExecutor()
114+
limit.mockResolvedValueOnce([{ email: 'member@example.com', suspendedAt: null }])
115+
limit.mockRejectedValueOnce(new Error('connection reset'))
116+
117+
/** A database blip must not cost a sign-in the requirement would have allowed anyway. */
118+
await expect(
119+
runWithAuthDatabase(executor, () =>
120+
prepareSessionForCreation(session, { path: '/sso/callback/okta' })
121+
)
122+
).resolves.toEqual({ data: session })
123+
})
124+
125+
it('refuses a password sign-in when the membership itself cannot be read', async () => {
126+
setEnvFlags({ isBillingEnabled: false, isSsoEnabled: true })
127+
const { executor, limit } = transactionExecutor()
128+
limit.mockResolvedValueOnce([{ email: 'member@example.com', suspendedAt: null }])
129+
limit.mockRejectedValueOnce(new Error('connection reset'))
130+
131+
/** An unknown membership cannot be read as "no organization requires SSO of this person". */
132+
await expect(
133+
runWithAuthDatabase(executor, () =>
134+
prepareSessionForCreation(session, { path: '/sign-in/email' })
135+
)
136+
).rejects.toThrow('connection reset')
137+
})
138+
111139
it('refuses the sign-in when the requirement itself cannot be read', async () => {
112140
setEnvFlags({ isBillingEnabled: false, isSsoEnabled: true })
113141
const { executor, limit } = transactionExecutor()

‎apps/sim/lib/auth/session-hooks.ts‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { eq } from 'drizzle-orm'
66
import { getAccessControlConfig, isEmailBlockedByAccessControl } from '@/lib/auth/access-control'
77
import { getAuthDatabase } from '@/lib/auth/database-context'
88
import { clampExpiryForSession } from '@/lib/auth/session-policy'
9-
import { assertSsoRequirementSatisfied } from '@/lib/auth/sso-policy'
9+
import { assertSsoRequirementSatisfied, satisfiesSsoRequirement } from '@/lib/auth/sso-policy'
1010

1111
const logger = createLogger('SessionHooks')
1212

@@ -41,20 +41,32 @@ export async function prepareSessionForCreation<T extends Session>(
4141
})
4242
}
4343

44-
/** Users belong to at most one organization, the same assumption the expiry clamp below makes. */
45-
const [membership] = await executor
46-
.select({ organizationId: member.organizationId, role: member.role })
47-
.from(member)
48-
.where(eq(member.userId, session.userId))
49-
.limit(1)
44+
/**
45+
* A membership that cannot be read is not a membership that does not exist, so a failed lookup
46+
* refuses the sign-in methods an organization could be requiring against — and only those. Every
47+
* other path keeps the old behavior of continuing without an organization, so a database blip
48+
* does not cost a sign-in to people this setting has nothing to say about.
49+
*/
50+
let membership: { organizationId: string; role: string } | undefined
51+
try {
52+
/** Users belong to at most one organization, the same assumption the expiry clamp makes. */
53+
;[membership] = await executor
54+
.select({ organizationId: member.organizationId, role: member.role })
55+
.from(member)
56+
.where(eq(member.userId, session.userId))
57+
.limit(1)
58+
} catch (error) {
59+
if (!satisfiesSsoRequirement(context?.path)) throw error
60+
logger.error('Error reading organization membership', { error, userId: session.userId })
61+
return { data: session }
62+
}
5063

5164
if (!membership) return { data: session }
5265

5366
/**
5467
* Outside the fallback below on purpose: a requirement that cannot be read is not a requirement
5568
* that does not apply, and admitting a password sign-in because a lookup failed is exactly the
56-
* bypass the setting exists to prevent. The read runs on the transaction that is creating the
57-
* session, so a failure here means that write is failing too.
69+
* bypass the setting exists to prevent.
5870
*/
5971
await assertSsoRequirementSatisfied(
6072
{ userId: session.userId, ...membership },

‎apps/sim/lib/auth/sso-policy.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,11 @@ vi.mock('@/lib/auth/sso/verified-provider', () => ({
1818
hasSignInCapableSsoProvider: mockHasProvider,
1919
}))
2020

21+
import { SSO_REQUIRED_MESSAGE } from '@/lib/auth/constants'
2122
import {
2223
assertSsoRequirementSatisfied,
2324
invalidateSsoPolicyCache,
2425
isSsoRequiredForOrganization,
25-
SSO_REQUIRED_MESSAGE,
2626
satisfiesSsoRequirement,
2727
} from '@/lib/auth/sso-policy'
2828

‎apps/sim/lib/auth/sso-policy.ts‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { createLogger } from '@sim/logger'
44
import { APIError } from 'better-auth/api'
55
import { eq } from 'drizzle-orm'
66
import { LRUCache } from 'lru-cache'
7+
import { SSO_REQUIRED_ERROR_CODE, SSO_REQUIRED_MESSAGE } from '@/lib/auth/constants'
78
import { isSsoCallbackPath } from '@/lib/auth/sso/callback-provider'
89
import { hasSignInCapableSsoProvider } from '@/lib/auth/sso/verified-provider'
910
import { isOrganizationFeatureEntitled } from '@/lib/billing/core/subscription'
@@ -51,14 +52,26 @@ export async function isSsoRequiredForOrganization(
5152
.where(eq(organization.id, organizationId))
5253
.limit(1)
5354

55+
/**
56+
* `onError: 'throw'` because a swallowed read is a permissive answer here: an outage on the
57+
* subscription read would otherwise return "not entitled", drop the requirement, and — worse —
58+
* cache that for the whole TTL. A throw leaves the cache untouched and refuses the sign-in.
59+
*/
60+
/** Stored, entitled, and a provider that could satisfy it — the three terms, cheapest first. */
5461
const required =
5562
row?.requireSso === true &&
56-
(await isOrganizationFeatureEntitled(organizationId, isSsoEnabled, executor)) &&
63+
(await isOrganizationFeatureEntitled(organizationId, isSsoEnabled, executor, {
64+
onError: 'throw',
65+
})) &&
5766
(await hasSignInCapableSsoProvider(organizationId, executor))
5867
if (executor === db) requirementCache.set(organizationId, required)
5968
return required
6069
}
6170

71+
/**
72+
* Drops this process's copy. Other instances keep theirs until the TTL expires, so the TTL — not
73+
* this call — is what bounds how long a change takes to reach the whole fleet.
74+
*/
6275
export function invalidateSsoPolicyCache(organizationId: string): void {
6376
requirementCache.delete(organizationId)
6477
}
@@ -88,11 +101,6 @@ export function satisfiesSsoRequirement(path: string | undefined): boolean {
88101
return DERIVED_SESSION_PATHS.has(path) || isSsoCallbackPath(path)
89102
}
90103

91-
export const SSO_REQUIRED_ERROR_CODE = 'SSO_REQUIRED'
92-
93-
export const SSO_REQUIRED_MESSAGE =
94-
'Your organization requires single sign-on. Sign in through your identity provider.'
95-
96104
/**
97105
* Refuses a session that an organization's sign-in requirement does not allow. Owners keep every
98106
* sign-in method as a break-glass path, so a broken identity provider cannot lock an organization

‎apps/sim/lib/auth/sso/application/sso-requirement.ts‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { db } from '@sim/db'
33
import { organization } from '@sim/db/schema'
44
import { eq } from 'drizzle-orm'
55
import { hasSignInCapableSsoProvider } from '@/lib/auth/sso/verified-provider'
6-
import { invalidateSsoPolicyCache, isSsoRequiredForOrganization } from '@/lib/auth/sso-policy'
6+
import { invalidateSsoPolicyCache } from '@/lib/auth/sso-policy'
77
import { isOrganizationFeatureEntitled } from '@/lib/billing/core/subscription'
88
import { recordProjectedUseCaseAuditEntries } from '@/lib/core/application/authorized-workspace-use-case'
99
import type { OperationUseCase } from '@/lib/core/application/operation'
@@ -49,11 +49,17 @@ async function loadRequirement(organizationId: string): Promise<SsoRequirement>
4949
.limit(1)
5050
if (!org) throw new OrchestrationError('not_found', 'Organization not found')
5151

52-
const [hasVerifiedProvider, isEnforced] = await Promise.all([
52+
/** Read fresh rather than through the sign-in cache: an admin is waiting on this answer. */
53+
const [hasVerifiedProvider, entitled] = await Promise.all([
5354
hasSignInCapableSsoProvider(organizationId),
54-
isSsoRequiredForOrganization(organizationId),
55+
isOrganizationFeatureEntitled(organizationId, isSsoEnabled),
5556
])
56-
return { requireSso: org.requireSso, hasVerifiedProvider, isEnforced }
57+
/** The same three terms sign-in checks, so the surface reports what is actually enforced. */
58+
return {
59+
requireSso: org.requireSso,
60+
hasVerifiedProvider,
61+
isEnforced: org.requireSso && entitled && hasVerifiedProvider,
62+
}
5763
}
5864

5965
export const readSsoRequirement: OperationUseCase<

‎apps/sim/lib/billing/core/subscription.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -589,10 +589,11 @@ export const isOrganizationOnEnterprisePlan = cache(resolveOrganizationEnterpris
589589
export async function isOrganizationFeatureEntitled(
590590
organizationId: string,
591591
selfHostEntitlement: boolean,
592-
executor: DbOrTx = db
592+
executor: DbOrTx = db,
593+
options: { onError?: EnterprisePlanErrorPolicy } = {}
593594
): Promise<boolean> {
594595
if (!isBillingEnabled) return selfHostEntitlement
595-
return isOrganizationOnEnterprisePlan(organizationId, 'return-false', executor)
596+
return isOrganizationOnEnterprisePlan(organizationId, options.onError ?? 'return-false', executor)
596597
}
597598

598599
/**

0 commit comments

Comments
 (0)