Skip to content

Commit 7574cd7

Browse files
committed
fix(billing): drain expired identity digests hourly, record only verified emails
- Expired deleted-account digests were removed only while recording a later deletion, so with no later deletions they outlived the 30-day window. The hourly OAuth-token cleanup cron now drains them first in bounded batches (indexed expires_at order, SKIP LOCKED, batch and time caps); a failed purge is logged and never holds up the token sweep. - A deleted account's email is recorded only when it was verified, so an unverified signup can never cost the address's real owner the allowance. GitHub and Microsoft subjects are still recorded; matching is unchanged. - The admin credit grant creates a missing stats row with the same email check as every other row creation instead of assuming eligibility, looked up before its transaction.
1 parent e6d86b9 commit 7574cd7

6 files changed

Lines changed: 163 additions & 13 deletions

File tree

‎apps/sim/app/api/cron/cleanup-oauth-tokens/route.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { getErrorMessage } from '@sim/utils/errors'
33
import { type NextRequest, NextResponse } from 'next/server'
44
import { verifyCronAuth } from '@/lib/auth/internal'
55
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
6+
import { purgeExpiredDeletedAccountIdentities } from '@/lib/users/deleted-identity'
67
import { runCleanupOAuthTokens } from '@/background/cleanup-oauth-tokens'
78

89
export const dynamic = 'force-dynamic'
@@ -14,14 +15,22 @@ const logger = createLogger('CleanupOAuthTokensAPI')
1415
* Retention sweep for lapsed OAuth token rows. Issuance state does not gate
1516
* retention: a deployment that disables the provider must still drain rows
1617
* created while it was enabled.
18+
*
19+
* The same hourly run drains expired deleted-account identity digests first,
20+
* within its own short budget, so they never outlive their retention window. A
21+
* failed purge is logged and never holds up the token sweep.
1722
*/
1823
export const GET = withRouteHandler(async (request: NextRequest) => {
1924
const authError = verifyCronAuth(request, 'OAuth token cleanup')
2025
if (authError) return authError
2126

2227
try {
28+
const deletedAccountIdentities = await purgeExpiredDeletedAccountIdentities().catch((error) => {
29+
logger.error('Failed to purge expired deleted-account identities', { error })
30+
return null
31+
})
2332
const result = await runCleanupOAuthTokens()
24-
return NextResponse.json({ success: true, ...result })
33+
return NextResponse.json({ success: true, ...result, deletedAccountIdentities })
2534
} catch (error) {
2635
logger.error('Failed to sweep expired OAuth tokens', { error })
2736
return NextResponse.json(

‎apps/sim/lib/admin/dashboard-credit-grant.test.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ const hoistedMocks = vi.hoisted(() => ({
2121
billingSubscriptions: [] as unknown[],
2222
idempotencyCalls: [] as { namespace: string; requestFingerprint: string }[],
2323
moveWorkspace: vi.fn(),
24+
userHasDeletedAccountIdentity: vi.fn(async () => false),
2425
}))
2526

2627
vi.mock('@sim/audit', () => auditMock)
@@ -53,6 +54,9 @@ vi.mock('@/lib/billing/enterprise-outbox', () => ({
5354
resolveEnterpriseMetadataIntent: vi.fn(),
5455
}))
5556
vi.mock('@/lib/core/outbox/service', () => outboxServiceMock)
57+
vi.mock('@/lib/users/deleted-identity', () => ({
58+
userHasDeletedAccountIdentity: hoistedMocks.userHasDeletedAccountIdentity,
59+
}))
5660

5761
import { grantDashboardOrganizationBalance, grantDashboardUserBalance } from '@/lib/admin/dashboard'
5862

@@ -150,6 +154,30 @@ describe('grantDashboardUserBalance', () => {
150154
)
151155
})
152156

157+
it('creates a missing stats row without the free allowance for a returning deleted identity', async () => {
158+
mocks.userHasDeletedAccountIdentity.mockResolvedValueOnce(true)
159+
queueTableRows(user, [{ id: 'user-1' }])
160+
queueTableRows(userStats, [
161+
{ creditBalance: '0', currentUsageLimit: '0', freeTierEligible: false },
162+
])
163+
mocks.billingSubscriptions = [null, null]
164+
dbChainMockFns.returning.mockResolvedValueOnce([
165+
{ creditBalance: '0.005', currentUsageLimit: '0.005' },
166+
])
167+
168+
await grantDashboardUserBalance(
169+
'user-1',
170+
0.005,
171+
undefined,
172+
'4f0f5c4e-3a43-4f39-9d55-6b1b2f6f2a10',
173+
{ id: 'admin-1', name: 'Admin', email: 'admin@sim.ai' }
174+
)
175+
176+
expect(dbChainMockFns.values).toHaveBeenCalledWith(
177+
expect.objectContaining({ userId: 'user-1', freeTierEligible: false, currentUsageLimit: '0' })
178+
)
179+
})
180+
153181
it('adds above a higher custom limit for an entitled personal subscription', async () => {
154182
const personalSubscription = {
155183
referenceId: 'user-1',

‎apps/sim/lib/admin/dashboard.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ import { executeTransactionallyIdempotent } from '@/lib/core/idempotency/transac
7878
import { enqueueOutboxEvent } from '@/lib/core/outbox/service'
7979
import type { DbOrTx } from '@/lib/db/types'
8080
import { deliverRunStops } from '@/lib/mothership/request/application/stop-delivery'
81+
import { userHasDeletedAccountIdentity } from '@/lib/users/deleted-identity'
8182
import { ownedAttachableWorkspacesWhere } from '@/lib/workspaces/organization-workspaces'
8283

8384
interface PaginationInput {
@@ -1497,6 +1498,14 @@ export async function grantDashboardUserBalance(
14971498
actor: AdminMutationActor
14981499
) {
14991500
const normalizedReason = reason?.trim() || null
1501+
/**
1502+
* Decides the eligibility of a stats row this grant may have to create, with
1503+
* the same email check as every other row creation. Looked up before the
1504+
* transaction so it never holds a second pooled connection inside it.
1505+
*/
1506+
const freeTierEligibleIfCreated = !(await userHasDeletedAccountIdentity(userId, {
1507+
includeSignIns: false,
1508+
}))
15001509
const outcome = await db.transaction(async (tx) => {
15011510
await acquireUserBillingIdentityLock(tx, userId)
15021511
const [account] = await tx
@@ -1515,17 +1524,17 @@ export async function grantDashboardUserBalance(
15151524
.from(member)
15161525
.where(eq(member.userId, userId))
15171526
.limit(1)
1518-
// Only inserts when signup never created the row; that row keeps the column's default eligibility.
15191527
const initialUsageLimit =
15201528
initialMembership || isOrgScopedSubscription(initialSubscription, userId)
15211529
? null
1522-
: getPerUserMinimumLimit(initialSubscription, true).toString()
1530+
: getPerUserMinimumLimit(initialSubscription, freeTierEligibleIfCreated).toString()
15231531
await tx
15241532
.insert(userStats)
15251533
.values({
15261534
id: generateId(),
15271535
userId,
15281536
currentUsageLimit: initialUsageLimit,
1537+
freeTierEligible: freeTierEligibleIfCreated,
15291538
usageLimitUpdatedAt: new Date(),
15301539
})
15311540
.onConflictDoNothing({ target: userStats.userId })

‎apps/sim/lib/users/deleted-identity.integration.ts‎

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { account, deletedAccountIdentity, user, userStats } from '@sim/db/schema
88
import { authMock } from '@sim/testing/mocks/auth.mock'
99
import { envFlagsMock } from '@sim/testing/mocks/env-flags.mock'
1010
import { generateId } from '@sim/utils/id'
11+
import { generateRandomHex } from '@sim/utils/random'
1112
import { eq, inArray } from 'drizzle-orm'
1213
import { afterAll, describe, expect, it, vi } from 'vitest'
1314
import {
@@ -18,6 +19,7 @@ import {
1819
} from '@/lib/billing/core/usage'
1920
import { getFreeTierLimit } from '@/lib/billing/subscriptions/utils'
2021
import { deleteUserAccount } from '@/lib/users/account-deletion'
22+
import { purgeExpiredDeletedAccountIdentities } from '@/lib/users/deleted-identity'
2123

2224
vi.mock('@/lib/core/config/env-flags', () => ({
2325
...envFlagsMock,
@@ -34,15 +36,15 @@ interface SignIn {
3436
accountId: string
3537
}
3638

37-
async function insertUser(email: string): Promise<string> {
39+
async function insertUser(email: string, { emailVerified = true } = {}): Promise<string> {
3840
const id = generateId()
3941
userIds.push(id)
4042
const now = new Date()
4143
await db.insert(user).values({
4244
id,
4345
name: 'Deleted identity fixture',
4446
email,
45-
emailVerified: true,
47+
emailVerified,
4648
createdAt: now,
4749
updatedAt: now,
4850
})
@@ -290,6 +292,52 @@ describe('deleted account identities and the free allowance', () => {
290292
expect(Math.abs(rows[0].expiresAt.getTime() - (deletedAt + thirtyDays))).toBeLessThan(60_000)
291293
})
292294

295+
it('never records an unverified email, so its real owner keeps the allowance', async () => {
296+
const email = `unverified.${generateId()}@deleted-identity.test`
297+
const github = { providerId: 'github', accountId: `gh-${generateId()}` }
298+
const squatter = await insertUser(email, { emailVerified: false })
299+
await linkAccount(squatter, github)
300+
await handleNewUser(squatter, {})
301+
const written = await deleteOwnAccount(squatter)
302+
303+
const owner = await signUp(email)
304+
const sameGithub = await signUp(`unverified-gh.${generateId()}@deleted-identity.test`, github)
305+
306+
expect(written).toHaveLength(1)
307+
expect(await statsOf(owner)).toEqual({ freeTierEligible: true, limit: getFreeTierLimit() })
308+
expect(await statsOf(sameGithub)).toEqual({ freeTierEligible: false, limit: 0 })
309+
})
310+
311+
it('drains every expired digest on the scheduled purge and keeps live ones', async () => {
312+
const expired = Array.from({ length: 2_345 }, () => generateRandomHex(64))
313+
const live = Array.from({ length: 3 }, () => generateRandomHex(64))
314+
recordedHashes.push(...expired, ...live)
315+
const past = new Date(Date.now() - 60_000)
316+
const future = new Date(Date.now() + 60 * 60_000)
317+
for (let start = 0; start < expired.length; start += 500) {
318+
await db
319+
.insert(deletedAccountIdentity)
320+
.values(
321+
expired
322+
.slice(start, start + 500)
323+
.map((identityHash) => ({ identityHash, expiresAt: past }))
324+
)
325+
}
326+
await db
327+
.insert(deletedAccountIdentity)
328+
.values(live.map((identityHash) => ({ identityHash, expiresAt: future })))
329+
330+
const result = await purgeExpiredDeletedAccountIdentities()
331+
332+
const remaining = await db
333+
.select({ hash: deletedAccountIdentity.identityHash })
334+
.from(deletedAccountIdentity)
335+
.where(inArray(deletedAccountIdentity.identityHash, [...expired, ...live]))
336+
expect(result.drained).toBe(true)
337+
expect(result.deleted).toBeGreaterThanOrEqual(expired.length)
338+
expect(remaining.map((row) => row.hash).sort()).toEqual([...live].sort())
339+
})
340+
293341
it('records only sign-in identities, as keyed digests holding no address or subject', async () => {
294342
const email = `digest.${generateId()}@deleted-identity.test`
295343
const githubSubject = `gh-${generateId()}`

‎apps/sim/lib/users/deleted-identity.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ describe('recordDeletedAccountIdentities', () => {
1313
beforeEach(() => {
1414
resetDbChainMock()
1515
setEnvFlags({ isBillingEnabled: true })
16-
queueTableRows(schemaMock.user, [{ email: 'ada@example.com' }])
16+
queueTableRows(schemaMock.user, [{ email: 'ada@example.com', emailVerified: true }])
1717
})
1818

1919
afterEach(() => {

‎apps/sim/lib/users/deleted-identity.ts‎

Lines changed: 63 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { createLogger } from '@sim/logger'
44
import { hmacSha256Hex } from '@sim/security/hmac'
55
import { getErrorMessage } from '@sim/utils/errors'
66
import { normalizeEmail } from '@sim/utils/string'
7-
import { and, eq, gt, inArray, lte, sql } from 'drizzle-orm'
7+
import { and, asc, eq, gt, inArray, lte, sql } from 'drizzle-orm'
88
import { env } from '@/lib/core/config/env'
99
import { isBillingEnabled } from '@/lib/core/config/env-flags'
1010
import type { DbOrTx } from '@/lib/db/types'
@@ -42,9 +42,18 @@ const logger = createLogger('DeletedAccountIdentity')
4242
*/
4343
const DELETED_IDENTITY_RETENTION_MS = 30 * 24 * 60 * 60 * 1000
4444

45-
/** Expired rows removed per recorded deletion, which keeps the table bounded without a cron. */
45+
/** Expired rows a recorded deletion removes opportunistically on its way through. */
4646
const EXPIRED_PURGE_BATCH = 100
4747

48+
/** Rows per batch of the scheduled purge, each its own short transaction. */
49+
const SCHEDULED_PURGE_BATCH = 1_000
50+
51+
/** Batches one scheduled run may delete before leaving the rest to the next run. */
52+
const SCHEDULED_PURGE_MAX_BATCHES = 50
53+
54+
/** Stops admitting batches well inside the cron request's 60-second timeout. */
55+
const SCHEDULED_PURGE_BUDGET_MS = 10_000
56+
4857
const GMAIL_DOMAINS = new Set(['gmail.com', 'googlemail.com'])
4958

5059
/**
@@ -129,10 +138,13 @@ function identityHashes(identity: SignInIdentity): string[] {
129138
return [...new Set(preimages)].map((preimage) => hmacSha256Hex(preimage, key))
130139
}
131140

132-
/** A user's email and the social sign-in subjects currently linked to it. */
133-
async function loadSignInIdentity(executor: DbOrTx, userId: string): Promise<SignInIdentity> {
141+
/** A user's email (and whether it is verified) and the social sign-in subjects linked to it. */
142+
async function loadSignInIdentity(
143+
executor: DbOrTx,
144+
userId: string
145+
): Promise<SignInIdentity & { emailVerified: boolean }> {
134146
const [profile] = await executor
135-
.select({ email: user.email })
147+
.select({ email: user.email, emailVerified: user.emailVerified })
136148
.from(user)
137149
.where(eq(user.id, userId))
138150
.limit(1)
@@ -145,14 +157,20 @@ async function loadSignInIdentity(executor: DbOrTx, userId: string): Promise<Sig
145157
inArray(account.providerId, [...RECORDED_SIGN_IN_PROVIDER_IDS])
146158
)
147159
)
148-
return { email: profile?.email, oauthSubjects }
160+
return { email: profile?.email, emailVerified: profile?.emailVerified === true, oauthSubjects }
149161
}
150162

151163
/**
152164
* Records the sign-in identities of an account being deleted at its holder's
153165
* request. Runs inside the deletion transaction, before the `user` row and its
154166
* `account` rows cascade away, so the record and the erasure commit together.
155167
*
168+
* The email is recorded only when it was verified: an unverified address may
169+
* belong to someone else, who must not lose the allowance for an account they
170+
* never held. Its GitHub and Microsoft subjects are recorded either way. Where
171+
* email verification is off, password accounts are therefore recorded by
172+
* nothing (fails open).
173+
*
156174
* The write runs in a savepoint and never fails the deletion: an anti-abuse
157175
* record must not become a way to block an erasure request. On failure the
158176
* savepoint rolls back, nothing is recorded, and the deletion proceeds.
@@ -162,7 +180,8 @@ export async function recordDeletedAccountIdentities(tx: DbOrTx, userId: string)
162180

163181
try {
164182
await tx.transaction(async (savepoint) => {
165-
const hashes = identityHashes(await loadSignInIdentity(savepoint, userId))
183+
const { email, emailVerified, oauthSubjects } = await loadSignInIdentity(savepoint, userId)
184+
const hashes = identityHashes({ email: emailVerified ? email : null, oauthSubjects })
166185
if (hashes.length === 0) return
167186

168187
await savepoint.delete(deletedAccountIdentity).where(
@@ -242,3 +261,40 @@ export async function userHasDeletedAccountIdentity(
242261
return false
243262
}
244263
}
264+
265+
/**
266+
* Deletes every expired identity digest, so none outlives its retention window
267+
* even when no later deletion comes along to purge it. Drains in short batches
268+
* in `expires_at` order, skipping rows a concurrent deletion has locked, until
269+
* none remain, the batch cap is hit, or the time budget runs out; whatever is
270+
* left goes to the next run. Runs whether or not billing is enabled, since
271+
* deleting expired rows is always safe.
272+
*/
273+
export async function purgeExpiredDeletedAccountIdentities(): Promise<{
274+
deleted: number
275+
drained: boolean
276+
}> {
277+
const deadline = Date.now() + SCHEDULED_PURGE_BUDGET_MS
278+
let deleted = 0
279+
for (let batch = 0; batch < SCHEDULED_PURGE_MAX_BATCHES; batch += 1) {
280+
if (Date.now() >= deadline) return { deleted, drained: false }
281+
const removed = await db
282+
.delete(deletedAccountIdentity)
283+
.where(
284+
inArray(
285+
deletedAccountIdentity.identityHash,
286+
db
287+
.select({ identityHash: deletedAccountIdentity.identityHash })
288+
.from(deletedAccountIdentity)
289+
.where(lte(deletedAccountIdentity.expiresAt, sql`now()`))
290+
.orderBy(asc(deletedAccountIdentity.expiresAt))
291+
.limit(SCHEDULED_PURGE_BATCH)
292+
.for('update', { skipLocked: true })
293+
)
294+
)
295+
.returning({ identityHash: deletedAccountIdentity.identityHash })
296+
deleted += removed.length
297+
if (removed.length < SCHEDULED_PURGE_BATCH) return { deleted, drained: true }
298+
}
299+
return { deleted, drained: false }
300+
}

0 commit comments

Comments
 (0)