Skip to content

Commit adc82a3

Browse files
committed
fix(auth): bound cleanup runtime and finish consent polish
1 parent c253788 commit adc82a3

11 files changed

Lines changed: 264 additions & 109 deletions

File tree

apps/docs/content/docs/api-reference/authentication.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ curl https://www.sim.ai/api/v2/workspaces \
103103
| Scope | Access |
104104
| --- | --- |
105105
| `api:read` | Read operations, including searches sent as POST requests |
106-
| `api:write` | Mutations and execution, including operations that can start external work |
106+
| `api:write` | Includes `api:read`, plus mutations and execution, including operations that can start external work |
107107
| `offline_access` | Refresh tokens for continued access after the access token expires |
108108

109109
Scopes limit what an application may do; your current workspace membership and role still apply. Each endpoint documents its required scope. Some GET endpoints that perform external discovery require `api:write`, so HTTP method alone does not determine the permission.

apps/docs/content/docs/platform/self-hosting/authentication.mdx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,14 +98,17 @@ returns 404 and the CLI falls back to the pairing-code handoff on its own.
9898
`DISABLE_AUTH=true` also forces the provider off because the authorization flow
9999
requires a real Better Auth user session.
100100

101-
Access tokens are opaque and last an hour; refresh tokens rotate on every use
102-
and expire after thirty days. Nothing is cached, so revoking a grant under
101+
Access tokens are opaque and last an hour; refresh tokens rotate on every use.
102+
Each login has a fixed thirty-day lifetime that refreshing does not extend.
103+
Nothing is cached, so revoking a grant under
103104
**Settings → Authorized apps** stops the app on its very next request.
104105

105106
### Registering an app
106107

107108
Dynamic client registration is switched off, so clients are created by an
108-
operator. The Sim CLI is seeded by the migration; register anything else with:
109+
operator. Both `db:migrate` and the development `db:push` command install the
110+
OAuth lifecycle triggers and register the Sim CLI. Repeating either command
111+
preserves existing grants and client customizations. Register other apps with:
109112

110113
```bash
111114
DATABASE_URL=… \

apps/sim/app/(auth)/oauth/consent/consent-view.tsx

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,8 @@ interface OAuthConsentViewProps {
4545
}
4646

4747
/**
48-
* The host the authorization code will be delivered to, or `null` when the
49-
* request names none this page can read.
50-
*
51-
* A registered `client_name` is whatever the client called itself, so for a
52-
* public client the destination is the one part of the request an impostor
53-
* cannot borrow. A loopback address is named plainly rather than shown as an
54-
* IP, because "this computer" is what it means to the person reading it.
48+
* Shows the redirect host beside the app name to help identify impersonation;
49+
* names loopback callbacks as this computer.
5550
*/
5651
function describeDestination(redirectUri: string | null): string | null {
5752
if (!redirectUri) return null
@@ -66,11 +61,8 @@ function describeDestination(redirectUri: string | null): string | null {
6661
}
6762

6863
/**
69-
* The consent card: which app is asking, what it will be able to do, and as
70-
* whom. Every decision here is a real grant, so the copy names the app and the
71-
* account rather than a generic "an application" — the page is the only place
72-
* a relayed or phished authorization can be caught, which is also why the
73-
* first-party CLI never skips it.
64+
* Always names the app and account, including for the CLI, so users can
65+
* recognize relayed authorization attempts.
7466
*/
7567
export function OAuthConsentView({
7668
refusal,

apps/sim/app/(auth)/oauth/consent/page.tsx

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,8 @@ export const metadata: Metadata = {
1414
export const dynamic = 'force-dynamic'
1515

1616
/**
17-
* The OAuth provider's `consentPage`. The plugin sends a signed-in user here with
18-
* the signed authorization query; the view shows who is asking and for what, and
19-
* the consent call carries that same query back so the plugin can mint a code
20-
* for exactly the request the user saw.
21-
*
22-
* A signed-out visitor (a stale tab, a shared link) goes through the same
23-
* bounce the plugin uses for its `loginPage`, which re-enters authorize after
24-
* sign-in and lands back here with a fresh signature.
17+
* Renders the plugin's signed request; signed-out visitors restart through the
18+
* login bridge to obtain a fresh authorization query.
2519
*/
2620
export default async function OAuthConsentPage({
2721
searchParams,
@@ -55,13 +49,15 @@ export default async function OAuthConsentPage({
5549
const authorizationRequestKey = refusal ? null : JSON.stringify(raw)
5650

5751
return (
58-
<OAuthConsentView
59-
refusal={refusal}
60-
clientId={params?.client_id ?? null}
61-
authorizationRequestKey={authorizationRequestKey}
62-
scope={params?.scope ?? null}
63-
redirectUri={params?.redirect_uri ?? null}
64-
email={session.user.email}
65-
/>
52+
<div className='[overflow-wrap:anywhere]'>
53+
<OAuthConsentView
54+
refusal={refusal}
55+
clientId={params?.client_id ?? null}
56+
authorizationRequestKey={authorizationRequestKey}
57+
scope={params?.scope ?? null}
58+
redirectUri={params?.redirect_uri ?? null}
59+
email={session.user.email}
60+
/>
61+
</div>
6662
)
6763
}

apps/sim/app/(auth)/oauth/consent/search-params.ts

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,8 @@
11
import { createSearchParamsCache, parseAsString } from 'nuqs/server'
22

33
/**
4-
* The authorize parameters the consent card renders. The rest of the signed
5-
* query (`state`, `sig`, `exp`, …) stays untouched in `window.location.search`,
6-
* which is what the auth client forwards verbatim on the consent call.
7-
*
8-
* Read-only for the life of the page, so there is no `urlKeys` companion and
9-
* no client-side `useQueryStates` — the server component reads them and passes
10-
* them down as props.
11-
*
12-
* Deliberately nullable rather than `.withDefault('')`: a missing `client_id`
13-
* is a malformed request the card must refuse, not a value to fall back on.
4+
* Read once for display; the auth client forwards the original signed query.
5+
* Nullable parsers preserve missing identifiers for rejection.
146
*/
157
const oauthConsentParsers = {
168
client_id: parseAsString,

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
66
import { runCleanupOAuthTokens } from '@/background/cleanup-oauth-tokens'
77

88
export const dynamic = 'force-dynamic'
9+
export const maxDuration = 60
910

1011
const logger = createLogger('CleanupOAuthTokensAPI')
1112

apps/sim/background/cleanup-oauth-tokens.test.ts

Lines changed: 81 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/**
22
* @vitest-environment node
33
*/
4-
import { beforeEach, describe, expect, it, vi } from 'vitest'
4+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
55

66
const mocks = vi.hoisted(() => ({
77
select: vi.fn(),
@@ -45,13 +45,18 @@ function selectChain(rows: unknown[], captured: unknown[]) {
4545

4646
describe('runCleanupOAuthTokens', () => {
4747
beforeEach(() => {
48-
vi.clearAllMocks()
48+
vi.resetAllMocks()
49+
vi.useFakeTimers({ toFake: ['Date'] })
4950
mocks.transaction.mockImplementation((work) =>
5051
work({ select: mocks.txSelect, delete: mocks.txDelete })
5152
)
5253
mocks.txSelect.mockImplementation(() => selectChain([], []))
5354
})
5455

56+
afterEach(() => {
57+
vi.useRealTimers()
58+
})
59+
5560
it('deletes exactly the expired rows it selected, and reports both counts', async () => {
5661
const deletedFrom: unknown[] = []
5762
mocks.select
@@ -116,30 +121,92 @@ describe('runCleanupOAuthTokens', () => {
116121
expect(OAUTH_TOKEN_RETENTION_DAYS).toBeGreaterThan(0)
117122
})
118123

119-
it('bounds family cascades independently of direct token pages and stops a full backlog', async () => {
124+
it('drains up to 50,000 families and access tokens with only ten families per transaction', async () => {
120125
const families = Array.from({ length: 10 }, (_, index) => ({
121126
id: `family-${index}`,
122127
clientId: 'client-1',
123128
sessionId: null,
124129
userId: 'user-1',
125130
consentId: null,
126131
}))
127-
for (let page = 0; page < 10; page += 1) {
128-
mocks.select.mockReturnValueOnce(selectChain(families, []))
129-
}
130-
mocks.select.mockReturnValueOnce(selectChain([], []))
132+
const accessTokens = Array.from({ length: 5_000 }, (_, index) => ({ id: `access-${index}` }))
133+
mocks.select.mockImplementation((fields: Record<string, unknown>) =>
134+
selectChain('clientId' in fields ? families : accessTokens, [])
135+
)
131136
mocks.txDelete.mockReturnValue({
132137
where: () => ({ returning: async () => families.map(({ id }) => ({ id })) }),
133138
})
139+
mocks.delete.mockReturnValue({
140+
where: () => ({ returning: async () => accessTokens }),
141+
})
134142

135143
await expect(runCleanupOAuthTokens()).resolves.toEqual({
136-
tokenFamilies: 100,
137-
accessTokens: 0,
144+
tokenFamilies: 50_000,
145+
accessTokens: 50_000,
138146
})
139-
expect(mocks.transaction).toHaveBeenCalledTimes(10)
140-
expect(mocks.limits.mock.calls.map(([limit]) => limit)).toEqual([
141-
...Array.from({ length: 10 }, () => 10),
142-
5_000,
143-
])
147+
expect(mocks.transaction).toHaveBeenCalledTimes(5_000)
148+
expect(mocks.delete).toHaveBeenCalledTimes(10)
149+
const limits = mocks.limits.mock.calls.map(([limit]) => limit)
150+
expect(limits.filter((limit) => limit === 10)).toHaveLength(5_000)
151+
expect(limits.filter((limit) => limit === 5_000)).toHaveLength(10)
152+
expect(limits.slice(0, 4)).toEqual([10, 5_000, 10, 5_000])
153+
})
154+
155+
it('stops at its deadline with committed progress from both backlogs', async () => {
156+
const startedAt = Date.now()
157+
const families = Array.from({ length: 10 }, (_, index) => ({
158+
id: `family-${index}`,
159+
clientId: 'client-1',
160+
sessionId: null,
161+
userId: 'user-1',
162+
consentId: null,
163+
}))
164+
const accessTokens = Array.from({ length: 5_000 }, (_, index) => ({ id: `access-${index}` }))
165+
mocks.select.mockImplementation((fields: Record<string, unknown>) =>
166+
selectChain('clientId' in fields ? families : accessTokens, [])
167+
)
168+
mocks.txDelete.mockReturnValue({
169+
where: () => ({ returning: async () => families.map(({ id }) => ({ id })) }),
170+
})
171+
mocks.delete.mockReturnValue({
172+
where: () => ({
173+
returning: async () => {
174+
vi.setSystemTime(startedAt + 45_000)
175+
return accessTokens
176+
},
177+
}),
178+
})
179+
180+
await expect(runCleanupOAuthTokens()).resolves.toEqual({
181+
tokenFamilies: 10,
182+
accessTokens: 5_000,
183+
})
184+
expect(mocks.transaction).toHaveBeenCalledOnce()
185+
expect(mocks.delete).toHaveBeenCalledOnce()
186+
expect(mocks.select).toHaveBeenCalledTimes(2)
187+
})
188+
189+
it('does not begin a delete when selection exhausts the deadline', async () => {
190+
const startedAt = Date.now()
191+
mocks.select.mockImplementation(() => {
192+
vi.setSystemTime(startedAt + 45_000)
193+
return selectChain(
194+
[
195+
{
196+
id: 'family-1',
197+
clientId: 'client-1',
198+
sessionId: null,
199+
userId: 'user-1',
200+
consentId: null,
201+
},
202+
],
203+
[]
204+
)
205+
})
206+
207+
await expect(runCleanupOAuthTokens()).resolves.toEqual({ tokenFamilies: 0, accessTokens: 0 })
208+
expect(mocks.transaction).not.toHaveBeenCalled()
209+
expect(mocks.delete).not.toHaveBeenCalled()
210+
expect(mocks.select).toHaveBeenCalledOnce()
144211
})
145212
})

apps/sim/background/cleanup-oauth-tokens.ts

Lines changed: 63 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,11 @@ export const OAUTH_TOKEN_RETENTION_DAYS = 7
2626
* batch small independently of direct access-token deletion.
2727
*/
2828
const OAUTH_FAMILY_SWEEP_LIMIT = 10
29+
const OAUTH_FAMILY_SWEEP_MAX_PAGES = 5_000
2930
const OAUTH_ACCESS_TOKEN_SWEEP_LIMIT = 5_000
30-
const OAUTH_TOKEN_SWEEP_MAX_PAGES = 10
31+
const OAUTH_ACCESS_TOKEN_SWEEP_MAX_PAGES = 10
32+
/** Stops admitting batches before the Helm cron's 60-second request timeout. */
33+
const OAUTH_TOKEN_SWEEP_BUDGET_MS = 45_000
3134

3235
export interface CleanupOAuthTokensResult {
3336
tokenFamilies: number
@@ -117,60 +120,79 @@ async function deleteExpiredFamilyBatch(
117120
* later reuse go unnoticed while descendants remained active. The family row
118121
* therefore owns retention and cascades every generation when it expires.
119122
*
120-
* Families go first and their refresh/access tokens follow by cascade. The
121-
* second pass catches expired access tokens for still-live families and access
122-
* tokens issued without a refresh grant. Both passes use bounded indexed pages.
123+
* Interleaves family and access-token pages so neither backlog prevents the
124+
* other from making progress before the deadline. Family deletion cascades its
125+
* remaining tokens; access pages also catch tokens from still-live families or
126+
* grants without refresh tokens. Both sweeps retain a 50,000-row run cap while
127+
* family transactions stay small enough to bound their descendant cascades.
123128
*/
124129
export async function runCleanupOAuthTokens(): Promise<CleanupOAuthTokensResult> {
125-
const cutoff = new Date(Date.now() - OAUTH_TOKEN_RETENTION_DAYS * 24 * 60 * 60 * 1000)
130+
const startedAt = Date.now()
131+
const deadline = startedAt + OAUTH_TOKEN_SWEEP_BUDGET_MS
132+
const cutoff = new Date(startedAt - OAUTH_TOKEN_RETENTION_DAYS * 24 * 60 * 60 * 1000)
126133
let tokenFamilies = 0
127134
let accessTokens = 0
135+
let moreFamilies = true
136+
let moreAccessTokens = true
128137

129-
for (let page = 0; page < OAUTH_TOKEN_SWEEP_MAX_PAGES; page += 1) {
130-
const staleFamilies = await db
131-
.select({
132-
id: oauthTokenFamily.id,
133-
clientId: oauthTokenFamily.clientId,
134-
sessionId: oauthTokenFamily.sessionId,
135-
userId: oauthTokenFamily.userId,
136-
consentId: oauthTokenFamily.consentId,
137-
})
138-
.from(oauthTokenFamily)
139-
.where(lt(oauthTokenFamily.expiresAt, cutoff))
140-
.orderBy(asc(oauthTokenFamily.expiresAt), asc(oauthTokenFamily.id))
141-
.limit(OAUTH_FAMILY_SWEEP_LIMIT)
142-
if (staleFamilies.length === 0) break
138+
for (let page = 0; page < OAUTH_FAMILY_SWEEP_MAX_PAGES; page += 1) {
139+
if (Date.now() >= deadline || (!moreFamilies && !moreAccessTokens)) break
143140

144-
tokenFamilies += await deleteExpiredFamilyBatch(staleFamilies, cutoff)
145-
if (staleFamilies.length < OAUTH_FAMILY_SWEEP_LIMIT) break
146-
}
141+
if (moreFamilies) {
142+
const staleFamilies = await db
143+
.select({
144+
id: oauthTokenFamily.id,
145+
clientId: oauthTokenFamily.clientId,
146+
sessionId: oauthTokenFamily.sessionId,
147+
userId: oauthTokenFamily.userId,
148+
consentId: oauthTokenFamily.consentId,
149+
})
150+
.from(oauthTokenFamily)
151+
.where(lt(oauthTokenFamily.expiresAt, cutoff))
152+
.orderBy(asc(oauthTokenFamily.expiresAt), asc(oauthTokenFamily.id))
153+
.limit(OAUTH_FAMILY_SWEEP_LIMIT)
154+
if (Date.now() >= deadline) break
147155

148-
for (let page = 0; page < OAUTH_TOKEN_SWEEP_MAX_PAGES; page += 1) {
149-
const staleAccess = await db
150-
.select({ id: oauthAccessToken.id })
151-
.from(oauthAccessToken)
152-
.where(lt(oauthAccessToken.expiresAt, cutoff))
153-
.orderBy(asc(oauthAccessToken.expiresAt), asc(oauthAccessToken.id))
154-
.limit(OAUTH_ACCESS_TOKEN_SWEEP_LIMIT)
155-
if (staleAccess.length === 0) break
156+
if (staleFamilies.length > 0) {
157+
tokenFamilies += await deleteExpiredFamilyBatch(staleFamilies, cutoff)
158+
}
159+
moreFamilies = staleFamilies.length === OAUTH_FAMILY_SWEEP_LIMIT
160+
}
156161

157-
const deleted = await db
158-
.delete(oauthAccessToken)
159-
.where(
160-
inArray(
161-
oauthAccessToken.id,
162-
staleAccess.map((row) => row.id)
163-
)
164-
)
165-
.returning({ id: oauthAccessToken.id })
166-
accessTokens += deleted.length
167-
if (staleAccess.length < OAUTH_ACCESS_TOKEN_SWEEP_LIMIT) break
162+
if (Date.now() >= deadline) break
163+
if (moreAccessTokens && page < OAUTH_ACCESS_TOKEN_SWEEP_MAX_PAGES) {
164+
const staleAccess = await db
165+
.select({ id: oauthAccessToken.id })
166+
.from(oauthAccessToken)
167+
.where(lt(oauthAccessToken.expiresAt, cutoff))
168+
.orderBy(asc(oauthAccessToken.expiresAt), asc(oauthAccessToken.id))
169+
.limit(OAUTH_ACCESS_TOKEN_SWEEP_LIMIT)
170+
if (Date.now() >= deadline) break
171+
172+
if (staleAccess.length > 0) {
173+
const deleted = await db
174+
.delete(oauthAccessToken)
175+
.where(
176+
inArray(
177+
oauthAccessToken.id,
178+
staleAccess.map((row) => row.id)
179+
)
180+
)
181+
.returning({ id: oauthAccessToken.id })
182+
accessTokens += deleted.length
183+
}
184+
moreAccessTokens =
185+
staleAccess.length === OAUTH_ACCESS_TOKEN_SWEEP_LIMIT &&
186+
page + 1 < OAUTH_ACCESS_TOKEN_SWEEP_MAX_PAGES
187+
}
168188
}
169189

170190
const result = { tokenFamilies, accessTokens }
171191
logger.info('Swept expired OAuth tokens', {
172192
...result,
173193
retentionDays: OAUTH_TOKEN_RETENTION_DAYS,
194+
elapsedMs: Date.now() - startedAt,
195+
deadlineReached: Date.now() >= deadline,
174196
})
175197
return result
176198
}

0 commit comments

Comments
 (0)