Skip to content

Commit 474d73f

Browse files
waleedlatif1claude
andcommitted
fix(sso): rely on lower(domain) match for conflict detection, drop dead in-memory recheck
Address PR review: the SQL `lower(domain) = <normalized>` predicate already excludes rows that the in-memory `normalizeSSODomain(...) === domain` recheck claimed to catch, making that recheck dead/misleading code. Match on the canonical lower-cased domain and filter purely by ownership. Malformed legacy values (wildcards, schemes, ports) never match an email domain at sign-in, so excluding them is not a gap. Test DB mock now applies the lower() predicate so the casing-variant case is genuinely exercised. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 6b860ec commit 474d73f

2 files changed

Lines changed: 9 additions & 5 deletions

File tree

apps/sim/app/api/auth/sso/register/route.test.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,14 @@ const {
3737

3838
function makeBuilder(rows: any[]): any {
3939
const thenable: any = Promise.resolve(rows)
40-
thenable.where = () => makeBuilder(rows)
40+
thenable.where = (condition: any) => {
41+
const values = condition?.values
42+
if (Array.isArray(values) && values.length > 0) {
43+
const target = String(values[values.length - 1]).toLowerCase()
44+
return makeBuilder(rows.filter((r) => String(r.domain ?? '').toLowerCase() === target))
45+
}
46+
return makeBuilder(rows)
47+
}
4148
thenable.limit = () => Promise.resolve(rows)
4249
thenable.orderBy = () => Promise.resolve(rows)
4350
return thenable

apps/sim/app/api/auth/sso/register/route.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,15 +83,12 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
8383

8484
const existingProviders = await db
8585
.select({
86-
domain: ssoProvider.domain,
8786
userId: ssoProvider.userId,
8887
organizationId: ssoProvider.organizationId,
8988
})
9089
.from(ssoProvider)
9190
.where(sql`lower(${ssoProvider.domain}) = ${domain}`)
92-
const conflictingProvider = existingProviders.find(
93-
(provider) => normalizeSSODomain(provider.domain) === domain && !isOwnedByCaller(provider)
94-
)
91+
const conflictingProvider = existingProviders.find((provider) => !isOwnedByCaller(provider))
9592

9693
if (conflictingProvider) {
9794
logger.warn('Rejected SSO registration for domain owned by another tenant', {

0 commit comments

Comments
 (0)