Skip to content

Commit b9d8047

Browse files
committed
refactor(auth): use the shared LRUCache instead of hand-rolled eviction
`lru-cache` is already a dependency and already the house pattern for a bounded, TTL'd, process-local cache — six modules use it, and `lib/environment/utils.ts` uses the same `*_CACHE_MAX_ENTRIES` / `*_CACHE_TTL_MS` constant naming this file had adopted. The bespoke `prune`/`touch` pair, the low-water-mark ratio, the per-entry `fetchedAt` bookkeeping, and the manual TTL comparisons were all reimplementing it, less well. Replaced with two `LRUCache` instances: 41 lines in, 91 out. Behavior is preserved, including the asymmetric membership TTL (a negative result is written with a shorter per-entry `ttl` rather than being branched on at read time). Membership entries are boxed in an object because `LRUCache` constrains values to non-nullish types, and a resolved non-member has to stay distinguishable from a cache miss. The only logic that stays hand-written is the monotonic version guard, which is domain-specific rather than cache machinery: the counter only increments, so a read that resolves lower than what is cached started earlier and must be discarded rather than served.
1 parent 3b76628 commit b9d8047

1 file changed

Lines changed: 41 additions & 91 deletions

File tree

apps/sim/lib/auth/security-policy.ts

Lines changed: 41 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { db } from '@sim/db'
22
import { member, organization } from '@sim/db/schema'
33
import { createLogger } from '@sim/logger'
44
import { eq } from 'drizzle-orm'
5+
import { LRUCache } from 'lru-cache'
56
import { isOrganizationsEnabled } from '@/lib/core/config/env-flags'
67

78
const logger = createLogger('SecurityPolicy')
@@ -21,62 +22,42 @@ const logger = createLogger('SecurityPolicy')
2122
* which is what keeps a stale version from ever pairing with a stale policy.
2223
*/
2324
export const SECURITY_POLICY_VERSION_CACHE_TTL_MS = 60 * 1000
25+
const SECURITY_POLICY_VERSION_CACHE_MAX_ENTRIES = 5_000
2426

25-
/**
26-
* Entry caps. Both maps only ever grow through explicit writes, so without a
27-
* bound a long-lived process accumulates one entry per distinct org/user it has
28-
* ever served.
29-
*/
30-
const MAX_VERSION_CACHE_ENTRIES = 5_000
31-
const MAX_MEMBERSHIP_CACHE_ENTRIES = 20_000
27+
const MEMBERSHIP_CACHE_TTL_MS = 60 * 1000
28+
const MEMBERSHIP_CACHE_MAX_ENTRIES = 20_000
3229

3330
/**
34-
* Fraction of the cap an over-capacity prune evicts down to. Trimming to a low
35-
* water mark rather than exactly to the cap is what keeps eviction amortized
36-
* O(1): pruning back to the cap would leave the very next insert over again, so
37-
* every subsequent write would rescan the whole map.
31+
* Negative (non-member) membership results use a much shorter TTL than
32+
* positive ones: a user's cached `null` would otherwise let them dodge a new
33+
* org's policy for the full TTL after joining through ANY path — including
34+
* ones outside this codebase (Better Auth SSO JIT provisioning writes `member`
35+
* rows straight through the adapter, and this app registers no `member`
36+
* database hook that could observe it). Positive results change only through
37+
* leave/transfer, which invalidate explicitly.
3838
*/
39-
const PRUNE_TARGET_RATIO = 0.9
39+
const NEGATIVE_MEMBERSHIP_CACHE_TTL_MS = 15 * 1000
4040

4141
const DEFAULT_VERSION = 1
4242

43+
const versionCache = new LRUCache<string, number>({
44+
max: SECURITY_POLICY_VERSION_CACHE_MAX_ENTRIES,
45+
ttl: SECURITY_POLICY_VERSION_CACHE_TTL_MS,
46+
})
47+
4348
/**
44-
* Evicts down to a low water mark, expired entries first. Map iteration follows
45-
* insertion order and {@link touch} re-inserts on every refresh, so whatever
46-
* remains at the head is the least recently refreshed.
49+
* Boxed because a resolved non-member is a cached VALUE, not a cache miss, and
50+
* `LRUCache` constrains values to non-nullish types — so `null` cannot be
51+
* stored directly and would be indistinguishable from a miss if it could.
4752
*/
48-
function prune(cache: Map<string, { fetchedAt: number }>, maxEntries: number): void {
49-
if (cache.size <= maxEntries) return
50-
const target = Math.floor(maxEntries * PRUNE_TARGET_RATIO)
51-
const cutoff = Date.now() - SECURITY_POLICY_VERSION_CACHE_TTL_MS
52-
for (const [key, entry] of cache) {
53-
if (cache.size <= target) break
54-
if (entry.fetchedAt < cutoff) cache.delete(key)
55-
}
56-
for (const key of cache.keys()) {
57-
if (cache.size <= target) break
58-
cache.delete(key)
59-
}
60-
}
61-
62-
/** Re-inserts so insertion order tracks recency of refresh, then prunes. */
63-
function touch<T extends { fetchedAt: number }>(
64-
cache: Map<string, T>,
65-
key: string,
66-
entry: T,
67-
maxEntries: number
68-
): void {
69-
cache.delete(key)
70-
cache.set(key, entry)
71-
prune(cache, maxEntries)
72-
}
73-
74-
interface VersionCacheEntry {
75-
version: number
76-
fetchedAt: number
53+
interface MembershipCacheEntry {
54+
organizationId: string | null
7755
}
7856

79-
const versionCache = new Map<string, VersionCacheEntry>()
57+
const membershipCache = new LRUCache<string, MembershipCacheEntry>({
58+
max: MEMBERSHIP_CACHE_MAX_ENTRIES,
59+
ttl: MEMBERSHIP_CACHE_TTL_MS,
60+
})
8061

8162
/**
8263
* Resolves the org's security-policy version — the shared monotonic counter
@@ -95,9 +76,7 @@ export async function getSecurityPolicyVersion(
9576
if (!organizationId) return DEFAULT_VERSION
9677

9778
const cached = versionCache.get(organizationId)
98-
if (cached && Date.now() - cached.fetchedAt < SECURITY_POLICY_VERSION_CACHE_TTL_MS) {
99-
return cached.version
100-
}
79+
if (cached !== undefined) return cached
10180

10281
try {
10382
const [row] = await db
@@ -107,19 +86,14 @@ export async function getSecurityPolicyVersion(
10786
.limit(1)
10887

10988
const version = row?.version ?? DEFAULT_VERSION
110-
// The counter only ever increments, so a read resolving LOWER than what is
111-
// already cached started before the newer one. Neither store it nor return
112-
// it: a late value would re-serve a pre-bump version, keeping cookies
113-
// matched and letting revoked sessions stay on the cookie cache.
114-
const current = versionCache.get(organizationId)
115-
if (current && current.version > version) return current.version
116-
117-
touch(
118-
versionCache,
119-
organizationId,
120-
{ version, fetchedAt: Date.now() },
121-
MAX_VERSION_CACHE_ENTRIES
122-
)
89+
// Re-check after the await. The counter only ever increments, so a value
90+
// that landed while this read was in flight is newer — neither store nor
91+
// return ours, or a late read would re-serve a pre-bump version and keep
92+
// cookies matched past a revocation.
93+
const concurrent = versionCache.get(organizationId)
94+
if (concurrent !== undefined && concurrent > version) return concurrent
95+
96+
versionCache.set(organizationId, version)
12397
return version
12498
} catch (error) {
12599
logger.error('Failed to resolve security policy version; using default', {
@@ -141,28 +115,10 @@ export async function getSecurityPolicyVersion(
141115
*/
142116
export function setSecurityPolicyVersion(organizationId: string, version: number): void {
143117
const current = versionCache.get(organizationId)
144-
if (current && current.version > version) return
145-
touch(versionCache, organizationId, { version, fetchedAt: Date.now() }, MAX_VERSION_CACHE_ENTRIES)
146-
}
147-
148-
interface MembershipCacheEntry {
149-
organizationId: string | null
150-
fetchedAt: number
118+
if (current !== undefined && current > version) return
119+
versionCache.set(organizationId, version)
151120
}
152121

153-
const membershipCache = new Map<string, MembershipCacheEntry>()
154-
155-
/**
156-
* Negative (non-member) membership results use a much shorter TTL than
157-
* positive ones: a user's cached `null` would otherwise let them dodge a new
158-
* org's policy for the full TTL after joining through ANY path — including
159-
* ones outside this codebase (Better Auth SSO JIT provisioning writes `member`
160-
* rows straight through the adapter, and this app registers no `member`
161-
* database hook that could observe it). Positive results change only through
162-
* leave/transfer, which invalidate explicitly.
163-
*/
164-
const NEGATIVE_MEMBERSHIP_CACHE_TTL_MS = 15 * 1000
165-
166122
/** Drops the cached membership for a user (call when they join/leave an org). */
167123
export function invalidateMembershipCache(userId: string): void {
168124
membershipCache.delete(userId)
@@ -187,12 +143,7 @@ export async function getMemberOrganizationId(
187143
if (!userId) return null
188144

189145
const cached = membershipCache.get(userId)
190-
if (cached) {
191-
const ttl = cached.organizationId
192-
? SECURITY_POLICY_VERSION_CACHE_TTL_MS
193-
: NEGATIVE_MEMBERSHIP_CACHE_TTL_MS
194-
if (Date.now() - cached.fetchedAt < ttl) return cached.organizationId
195-
}
146+
if (cached) return cached.organizationId
196147

197148
const [row] = await db
198149
.select({ organizationId: member.organizationId })
@@ -201,11 +152,10 @@ export async function getMemberOrganizationId(
201152
.limit(1)
202153

203154
const organizationId = row?.organizationId ?? null
204-
touch(
205-
membershipCache,
155+
membershipCache.set(
206156
userId,
207-
{ organizationId, fetchedAt: Date.now() },
208-
MAX_MEMBERSHIP_CACHE_ENTRIES
157+
{ organizationId },
158+
organizationId ? undefined : { ttl: NEGATIVE_MEMBERSHIP_CACHE_TTL_MS }
209159
)
210160
return organizationId
211161
}

0 commit comments

Comments
 (0)