Skip to content

Commit 8ca4ff1

Browse files
committed
refactor(auth): one email fold, in SQL and TypeScript, and no reads of the dead column
An address was folded five different ways across the codebase: three SQL spellings — `lower(x)`, `lower(trim(x))`, `lower(btrim(x))` — plus reads of `user.normalized_email`, plus inline `trim().toLowerCase()`. Only one of the SQL forms matches the expression the new index is built on, so the others were sequential scans of `user` wearing the costume of an indexed lookup. `normalized_email` turned out to be populated after all — for a fifth of accounts, by a signup plugin removed in June, using Gmail dot-and-tag stripping. That is the right function for deduplicating signups and the wrong one for identity: it merges addresses a mail provider may route to different people, and it stops at the day the plugin left. Every read of the column is gone. The column itself stays for one release, because Better Auth selects every schema column and dropping it while the previous release still serves would break sign-in; the drop is the follow-up, and the repo's drop audit will name the argless reads that must be fixed first. `foldedEmail` now lives in the schema beside the index that indexes it, so the predicate and the index are one expression by construction. Its TypeScript twin is `normalizeEmail` from `@sim/utils/string`, which the new access code now uses instead of inlining the fold. The index is no longer unique. Production holds thirteen addresses that collide once folded — real, verified, active accounts — so a unique build would fail the deploy by design. Access resolution refuses to bind an ambiguous address, which keeps either account from reading the other's documents until the pairs are merged and the index can be promoted. The directory-sync cron joins docker/crontab; the parity audit caught that Helm alone was not enough.
1 parent 822625b commit 8ca4ff1

19 files changed

Lines changed: 111 additions & 126 deletions

File tree

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,18 @@
11
import { db } from '@sim/db'
2-
import { user } from '@sim/db/schema'
3-
import { eq, or } from 'drizzle-orm'
2+
import { foldedEmail, user } from '@sim/db/schema'
3+
import { normalizeEmail } from '@sim/utils/string'
4+
import { eq } from 'drizzle-orm'
45
import type { NextRequest } from 'next/server'
56
import type { AdminMutationActor } from '@/lib/admin/dashboard'
67

78
export async function getAdminAuditActor(request: NextRequest): Promise<AdminMutationActor> {
8-
const email = request.headers.get('x-admin-email')?.trim().toLowerCase()
9+
const rawEmail = request.headers.get('x-admin-email')
10+
const email = rawEmail ? normalizeEmail(rawEmail) : ''
911
if (!email) return { id: null, name: 'Admin API', email: null }
1012
const [admin] = await db
1113
.select({ id: user.id, name: user.name, email: user.email })
1214
.from(user)
13-
.where(or(eq(user.email, email), eq(user.normalizedEmail, email)))
15+
.where(eq(foldedEmail(user.email), email))
1416
.limit(1)
1517
return admin ?? { id: null, name: 'Admin Panel', email }
1618
}

apps/sim/connectors/confluence/permissions.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { createLogger } from '@sim/logger'
2+
import { normalizeEmail } from '@sim/utils/string'
23
import type {
34
ConfluencePrincipal,
45
ConfluenceRestriction,
@@ -189,7 +190,7 @@ export async function resolveUserEmails(
189190
accessToken
190191
)
191192
for (const entry of body.results ?? []) {
192-
const email = entry.email?.trim().toLowerCase()
193+
const email = entry.email ? normalizeEmail(entry.email) : ''
193194
if (entry.accountId && email) emails.set(entry.accountId, email)
194195
}
195196
} catch (error) {

apps/sim/connectors/google-drive/directory.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { createLogger } from '@sim/logger'
2+
import { normalizeEmail } from '@sim/utils/string'
23
import { canonicalGroupId } from '@/lib/knowledge/access/tokens'
34
import type {
45
ConnectorDirectory,
@@ -35,7 +36,7 @@ const MAX_GROUP_NESTING_DEPTH = 10
3536
*/
3637
export function googleWorkspaceDomain(adminEmail: unknown): string | undefined {
3738
if (typeof adminEmail !== 'string') return undefined
38-
const domain = adminEmail.trim().toLowerCase().split('@')[1]
39+
const domain = normalizeEmail(adminEmail).split('@')[1]
3940
return domain || undefined
4041
}
4142

@@ -143,7 +144,7 @@ export async function listGroupMembers(
143144
)
144145

145146
for (const member of members) {
146-
const email = member.email?.trim().toLowerCase()
147+
const email = member.email ? normalizeEmail(member.email) : ''
147148
if (!email) continue
148149
if (member.status && member.status.toUpperCase() !== 'ACTIVE') continue
149150

apps/sim/lib/auth/sso/application/admit-sso-user.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit'
22
import { db } from '@sim/db'
33
import {
44
account,
5+
foldedEmail,
56
invitation,
67
member,
78
permissions,
@@ -13,7 +14,7 @@ import {
1314
import { createLogger } from '@sim/logger'
1415
import { normalizeSSODomain } from '@sim/utils/sso-domain'
1516
import { normalizeEmail } from '@sim/utils/string'
16-
import { and, desc, eq, gt, inArray, isNull, sql } from 'drizzle-orm'
17+
import { and, desc, eq, gt, inArray, isNull } from 'drizzle-orm'
1718
import { applySessionPolicyToNewMember } from '@/lib/auth/session-policy'
1819
import { ssoJitAdmissionOperation } from '@/lib/auth/sso/application/operations'
1920
import { syncUsageLimitsFromSubscription } from '@/lib/billing/core/usage'
@@ -184,7 +185,7 @@ async function runAdmissionTransaction(
184185
eq(invitation.organizationId, provider.organizationId),
185186
eq(invitation.status, 'pending'),
186187
gt(invitation.expiresAt, new Date()),
187-
sql`lower(trim(${invitation.email})) = ${normalizedEmail}`
188+
eq(foldedEmail(invitation.email), normalizedEmail)
188189
)
189190
)
190191
.limit(1),

apps/sim/lib/billing/enterprise-owner-claim.test.ts

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -325,21 +325,11 @@ describe('Enterprise future-owner claims', () => {
325325
condition.type === 'eq' && condition.left === user.id && condition.right === 'owner-1'
326326
)
327327
).toBe(true)
328-
const emailScope = updateConditions.find((condition) => condition.type === 'or')
329-
const emailConditions = Array.isArray(emailScope?.conditions) ? emailScope.conditions : []
330-
expect(
331-
emailConditions.some(
332-
(condition) =>
333-
condition?.type === 'eq' &&
334-
condition.left === user.normalizedEmail &&
335-
condition.right === request.ownerEmail
336-
)
337-
).toBe(true)
338-
expect(
339-
emailConditions.filter(
340-
(condition) => condition?.type === 'eq' && condition.right === request.ownerEmail
341-
)
342-
).toHaveLength(2)
328+
/** The folded address is one `eq`, not an OR over a dead column and an ad-hoc fold. */
329+
const emailConditions = updateConditions.filter(
330+
(condition) => condition?.type === 'eq' && condition.right === request.ownerEmail
331+
)
332+
expect(emailConditions).toHaveLength(1)
343333
expect(mocks.createOrganization).not.toHaveBeenCalled()
344334
expect(mocks.enqueue).not.toHaveBeenCalled()
345335
})

apps/sim/lib/billing/enterprise-owner-claim.ts

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { AuditAction, AuditResourceType, recordAuditOnce } from '@sim/audit'
22
import { db } from '@sim/db'
3-
import { member, outboxEvent, user, workspace } from '@sim/db/schema'
3+
import { foldedEmail, member, outboxEvent, user, workspace } from '@sim/db/schema'
44
import { createLogger } from '@sim/logger'
55
import { safeCompare } from '@sim/security/compare'
66
import { generateId } from '@sim/utils/id'
@@ -297,9 +297,7 @@ async function assertOwnerEmailHasNoAccount(ownerEmail: string): Promise<void> {
297297
const [existingUser] = await db
298298
.select({ id: user.id })
299299
.from(user)
300-
.where(
301-
or(eq(user.normalizedEmail, ownerEmail), eq(sql<string>`lower(${user.email})`, ownerEmail))
302-
)
300+
.where(eq(foldedEmail(user.email), ownerEmail))
303301
.limit(1)
304302
if (existingUser) {
305303
throw new EnterpriseProvisioningError(
@@ -499,12 +497,7 @@ export async function createEnterpriseOwnerClaim(
499497
const [accountCreatedDuringReview] = await tx
500498
.select({ id: user.id })
501499
.from(user)
502-
.where(
503-
or(
504-
eq(user.normalizedEmail, normalized.ownerEmail),
505-
eq(sql<string>`lower(${user.email})`, normalized.ownerEmail)
506-
)
507-
)
500+
.where(eq(foldedEmail(user.email), normalized.ownerEmail))
508501
.limit(1)
509502
if (accountCreatedDuringReview) {
510503
throw new EnterpriseProvisioningError(
@@ -968,13 +961,7 @@ export async function acceptEnterpriseOwnerClaim(params: {
968961
.update(user)
969962
.set({ emailVerified: true, updatedAt: new Date() })
970963
.where(
971-
and(
972-
eq(user.id, params.userId),
973-
or(
974-
eq(user.normalizedEmail, payload.request.ownerEmail),
975-
eq(sql<string>`lower(trim(${user.email}))`, payload.request.ownerEmail)
976-
)
977-
)
964+
and(eq(user.id, params.userId), eq(foldedEmail(user.email), payload.request.ownerEmail))
978965
)
979966
.returning({ id: user.id })
980967
if (!verifiedOwner) {

apps/sim/lib/billing/webhooks/enterprise.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit'
22
import { db } from '@sim/db'
3-
import { organization, outboxEvent, session, subscription, user } from '@sim/db/schema'
3+
import { foldedEmail, organization, outboxEvent, session, subscription, user } from '@sim/db/schema'
44
import { createLogger } from '@sim/logger'
55
import { generateId } from '@sim/utils/id'
66
import { isRecordLike } from '@sim/utils/object'
7+
import { normalizeEmail } from '@sim/utils/string'
78
import { and, eq, inArray, sql } from 'drizzle-orm'
89
import type Stripe from 'stripe'
910
import { getEmailSubject, renderEnterpriseSubscriptionEmail } from '@/components/emails'
@@ -532,7 +533,7 @@ async function reconcileManualEnterpriseSubscription(
532533
requestedByUserId
533534
? eq(user.id, requestedByUserId)
534535
: requestedByEmail
535-
? eq(user.normalizedEmail, requestedByEmail.toLowerCase())
536+
? eq(foldedEmail(user.email), normalizeEmail(requestedByEmail))
536537
: eq(user.stripeCustomerId, stripeCustomerId)
537538
)
538539
.limit(1)

apps/sim/lib/invitations/direct-grant.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { AuditAction, AuditResourceType, recordAudit, recordAuditOnce } from '@sim/audit'
22
import { db } from '@sim/db'
33
import {
4+
foldedEmail,
45
invitation,
56
invitationWorkspaceGrant,
67
member,
@@ -12,7 +13,7 @@ import { permissionSatisfies } from '@sim/platform-authz/workspace'
1213
import { generateId } from '@sim/utils/id'
1314
import { isRecordLike } from '@sim/utils/object'
1415
import { normalizeEmail } from '@sim/utils/string'
15-
import { and, eq, sql } from 'drizzle-orm'
16+
import { and, eq } from 'drizzle-orm'
1617
import type { NextRequest } from 'next/server'
1718
import {
1819
acquireOrganizationUserMutationLocks,
@@ -94,7 +95,7 @@ async function getPendingWorkspaceInvitationIds(
9495
.innerJoin(invitationWorkspaceGrant, eq(invitationWorkspaceGrant.invitationId, invitation.id))
9596
.where(
9697
and(
97-
sql`lower(${invitation.email}) = ${normalizedEmail}`,
98+
eq(foldedEmail(invitation.email), normalizedEmail),
9899
eq(invitation.status, 'pending'),
99100
eq(invitationWorkspaceGrant.workspaceId, workspaceId)
100101
)

apps/sim/lib/invitations/workspace-invitations.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,15 @@
11
import { AuditAction, AuditResourceType, recordAudit, recordAuditOnce } from '@sim/audit'
22
import { db } from '@sim/db'
3-
import { type InvitationMembershipIntent, member, permissions, user } from '@sim/db/schema'
3+
import {
4+
foldedEmail,
5+
type InvitationMembershipIntent,
6+
member,
7+
permissions,
8+
user,
9+
} from '@sim/db/schema'
410
import { isOrgAdminRole, permissionSatisfies } from '@sim/platform-authz/workspace'
511
import { normalizeEmail } from '@sim/utils/string'
6-
import { and, eq, inArray, sql } from 'drizzle-orm'
12+
import { and, eq, inArray } from 'drizzle-orm'
713
import type { NextRequest } from 'next/server'
814
import { isOrganizationOwnerOrAdmin } from '@/lib/billing/core/organization'
915
import {
@@ -479,7 +485,7 @@ export async function createWorkspaceInvitation({
479485
const existingUser = await db
480486
.select({ id: user.id })
481487
.from(user)
482-
.where(sql`lower(${user.email}) = ${normalizedEmail}`)
488+
.where(eq(foldedEmail(user.email), normalizedEmail))
483489
.then((rows) => rows[0])
484490

485491
const existingMembership = existingUser ? await getUserOrganization(existingUser.id) : null

apps/sim/lib/knowledge/access/drive-permissions.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { normalizeEmail } from '@sim/utils/string'
12
import { groupToken, sortAccessTokens, userToken } from '@/lib/knowledge/access/tokens'
23
import { LINK_ACCESS_TOKEN, PUBLIC_ACCESS_TOKEN } from '@/lib/knowledge/access/types'
34

@@ -50,7 +51,7 @@ export const CLOSED_OPEN_SHARING: OpenSharingPolicy = Object.freeze({
5051
* decided by the reader's own email domain rather than a second predicate.
5152
*/
5253
export function domainGroupId(domain: string): string {
53-
return `domain:${domain.trim().toLowerCase()}`
54+
return `domain:${normalizeEmail(domain)}`
5455
}
5556

5657
export interface DriveAclInput {

0 commit comments

Comments
 (0)