diff --git a/apps/docs/content/docs/en/platform/enterprise/session-policies.mdx b/apps/docs/content/docs/en/platform/enterprise/session-policies.mdx index fb26c2f0a32..903af6ed14e 100644 --- a/apps/docs/content/docs/en/platform/enterprise/session-policies.mdx +++ b/apps/docs/content/docs/en/platform/enterprise/session-policies.mdx @@ -27,7 +27,9 @@ Use this to enforce periodic re-authentication — for example, a value of `168` ### Idle timeout -Signs a member out after this many hours without activity. Activity extends the session, so members who use Sim regularly stay signed in; dormant sessions expire. +Signs a member out after a period without activity. Activity extends the session, so members who use Sim regularly stay signed in; dormant sessions expire. + +Activity is recorded at most once per 24 hours, so the actual sign-out lands between (value − 24) and (value) hours after a member's last use — never later. A value of `48` means a dormant session ends somewhere between 24 and 48 hours after last activity. Accepts 48 to 8760 hours. The 48-hour minimum exists because session activity is recorded at most once per day — a shorter window could sign out members who are actively working. @@ -72,5 +74,10 @@ Use this after a security incident, an offboarding wave, or before tightening a answer: 'Session activity is recorded at most once per 24 hours for performance. The minimum is twice that window so a member who stays active always registers activity before the idle limit can expire their session.', }, + { + question: 'Why was a member signed out earlier than the idle timeout I set?', + answer: + 'Because activity is recorded at most once per 24 hours, the last recorded activity can be up to a day older than a member\'s actual last use. Sign-out therefore lands between (value − 24) and (value) hours after they stop working. It never lands later than the value you set, so the policy is never exceeded.', + }, ]} /> diff --git a/apps/sim/app/api/organizations/[id]/session-policy/route.test.ts b/apps/sim/app/api/organizations/[id]/session-policy/route.test.ts index c8722a51154..57f1ac01ea4 100644 --- a/apps/sim/app/api/organizations/[id]/session-policy/route.test.ts +++ b/apps/sim/app/api/organizations/[id]/session-policy/route.test.ts @@ -21,11 +21,10 @@ const { mockIsEnterprise, mockEagerClamp, mockRecordAudit } = vi.hoisted(() => ( vi.mock('@/lib/auth/session-policy', () => ({ eagerClampOrgSessions: mockEagerClamp, - invalidateSessionPolicyCache: vi.fn(), })) vi.mock('@/lib/auth/security-policy', () => ({ - invalidateSecurityPolicyVersionCache: vi.fn(), + setSecurityPolicyVersion: vi.fn(), })) vi.mock('@/lib/billing/core/subscription', () => ({ @@ -128,7 +127,7 @@ describe('session policy route', () => { it('saves the policy, eagerly clamps sessions, and bumps the version', async () => { queueTableRows(member, [{ role: 'owner' }]) queueTableRows(organization, [{ name: 'Acme' }]) - dbChainMockFns.returning.mockResolvedValueOnce([{ id: ORG_ID }]) + dbChainMockFns.returning.mockResolvedValueOnce([{ id: ORG_ID, securityPolicyVersion: 2 }]) const response = await PUT( putRequest({ maxSessionHours: 72, idleTimeoutHours: 48 }), @@ -154,7 +153,7 @@ describe('session policy route', () => { it('clearing both fields still saves and delegates the no-op to the clamp', async () => { queueTableRows(member, [{ role: 'owner' }]) queueTableRows(organization, [{ name: 'Acme' }]) - dbChainMockFns.returning.mockResolvedValueOnce([{ id: ORG_ID }]) + dbChainMockFns.returning.mockResolvedValueOnce([{ id: ORG_ID, securityPolicyVersion: 2 }]) const response = await PUT( putRequest({ maxSessionHours: null, idleTimeoutHours: null }), diff --git a/apps/sim/app/api/organizations/[id]/session-policy/route.ts b/apps/sim/app/api/organizations/[id]/session-policy/route.ts index d3ce9d53bc3..79795d111e8 100644 --- a/apps/sim/app/api/organizations/[id]/session-policy/route.ts +++ b/apps/sim/app/api/organizations/[id]/session-policy/route.ts @@ -9,8 +9,8 @@ import { type NextRequest, NextResponse } from 'next/server' import { updateOrganizationSessionPolicyContract } from '@/lib/api/contracts/organization' import { parseRequest, validationErrorResponse } from '@/lib/api/server' import { getSession } from '@/lib/auth' -import { invalidateSecurityPolicyVersionCache } from '@/lib/auth/security-policy' -import { eagerClampOrgSessions, invalidateSessionPolicyCache } from '@/lib/auth/session-policy' +import { setSecurityPolicyVersion } from '@/lib/auth/security-policy' +import { eagerClampOrgSessions } from '@/lib/auth/session-policy' import { isOrganizationOnEnterprisePlan } from '@/lib/billing/core/subscription' import { isBillingEnabled } from '@/lib/core/config/env-flags' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' @@ -150,7 +150,7 @@ export const PUT = withRouteHandler( updatedAt: new Date(), }) .where(eq(organization.id, organizationId)) - .returning({ id: organization.id }) + .returning({ securityPolicyVersion: organization.securityPolicyVersion }) if (!row) return null await eagerClampOrgSessions(organizationId, merged, tx) return row @@ -160,8 +160,7 @@ export const PUT = withRouteHandler( return NextResponse.json({ error: 'Organization not found' }, { status: 404 }) } - invalidateSessionPolicyCache(organizationId) - invalidateSecurityPolicyVersionCache(organizationId) + setSecurityPolicyVersion(organizationId, updated.securityPolicyVersion) logger.info('Updated organization session policy', { organizationId }) diff --git a/apps/sim/app/api/organizations/[id]/sessions/revoke/route.test.ts b/apps/sim/app/api/organizations/[id]/sessions/revoke/route.test.ts new file mode 100644 index 00000000000..d550102ad0d --- /dev/null +++ b/apps/sim/app/api/organizations/[id]/sessions/revoke/route.test.ts @@ -0,0 +1,247 @@ +/** + * @vitest-environment node + */ +import { member, organization, session as sessionTable } from '@sim/db/schema' +import { + authMockFns, + createMockRequest, + dbChainMockFns, + queueTableRows, + resetDbChainMock, + resetEnvFlagsMock, + setEnvFlags, +} from '@sim/testing' +import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockIsEnterprise, mockRecordAudit, mockSetVersion } = vi.hoisted(() => ({ + mockIsEnterprise: vi.fn(), + mockRecordAudit: vi.fn(), + mockSetVersion: vi.fn(), +})) + +vi.mock('@/lib/auth/security-policy', () => ({ + setSecurityPolicyVersion: mockSetVersion, +})) + +vi.mock('@/lib/billing/core/subscription', () => ({ + isOrganizationOnEnterprisePlan: mockIsEnterprise, +})) + +vi.mock('@sim/audit', () => ({ + recordAudit: mockRecordAudit, + AuditAction: { ORGANIZATION_SESSIONS_REVOKED: 'organization.sessions.revoked' }, + AuditResourceType: { ORGANIZATION: 'organization' }, +})) + +import { POST } from '@/app/api/organizations/[id]/sessions/revoke/route' + +const mockGetSession = authMockFns.mockGetSession + +const ORG_ID = 'org-1' +const CALLER_TOKEN = 'tok-caller' +const routeContext = { params: Promise.resolve({ id: ORG_ID }) } + +interface MockCondition { + type: string + left?: unknown + right?: unknown + column?: unknown + conditions?: MockCondition[] +} + +/** Flattens the nested `and(...)` the route builds into a flat condition list. */ +function flattenConditions(condition: MockCondition): MockCondition[] { + if (condition.type !== 'and') return [condition] + return (condition.conditions ?? []).flatMap(flattenConditions) +} + +/** + * The condition list passed to the session DELETE's `.where(...)`. Identified by + * its `inArray` over `session.user_id` rather than by call position, since the + * version-bump UPDATE issues its own `.where(...)` on the same shared spy. + */ +function deleteConditions(): MockCondition[] { + expect(dbChainMockFns.delete).toHaveBeenCalledWith(sessionTable) + const match = dbChainMockFns.where.mock.calls + .map(([arg]) => flattenConditions(arg as MockCondition)) + .find((conditions) => + conditions.some( + (condition) => condition.type === 'inArray' && condition.column === sessionTable.userId + ) + ) + expect(match).toBeDefined() + return match as MockCondition[] +} + +function authenticateAs(overrides: Record = {}) { + mockGetSession.mockResolvedValue({ + user: { id: 'user-1', name: 'Admin', email: 'admin@acme.dev' }, + session: { token: CALLER_TOKEN, ...overrides }, + }) +} + +beforeAll(() => { + setEnvFlags({ isBillingEnabled: true }) +}) + +afterAll(resetEnvFlagsMock) + +describe('org sessions revoke route', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + authenticateAs() + mockIsEnterprise.mockResolvedValue(true) + }) + + describe('authorization', () => { + it('returns 401 when unauthenticated', async () => { + mockGetSession.mockResolvedValue(null) + const response = await POST(createMockRequest('POST'), routeContext) + expect(response.status).toBe(401) + expect(dbChainMockFns.delete).not.toHaveBeenCalled() + }) + + it('returns 403 for non-members', async () => { + queueTableRows(member, []) + const response = await POST(createMockRequest('POST'), routeContext) + expect(response.status).toBe(403) + expect(dbChainMockFns.delete).not.toHaveBeenCalled() + }) + + it('returns 403 for members without an admin role', async () => { + queueTableRows(member, [{ role: 'member' }]) + const response = await POST(createMockRequest('POST'), routeContext) + expect(response.status).toBe(403) + expect(dbChainMockFns.delete).not.toHaveBeenCalled() + }) + + it('returns 403 for non-enterprise organizations', async () => { + queueTableRows(member, [{ role: 'owner' }]) + mockIsEnterprise.mockResolvedValue(false) + const response = await POST(createMockRequest('POST'), routeContext) + expect(response.status).toBe(403) + expect(dbChainMockFns.delete).not.toHaveBeenCalled() + }) + + it('returns 404 when the organization does not exist', async () => { + queueTableRows(member, [{ role: 'owner' }]) + queueTableRows(organization, []) + const response = await POST(createMockRequest('POST'), routeContext) + expect(response.status).toBe(404) + expect(dbChainMockFns.delete).not.toHaveBeenCalled() + }) + + it('allows admins as well as owners', async () => { + queueTableRows(member, [{ role: 'admin' }]) + queueTableRows(organization, [{ name: 'Acme' }]) + dbChainMockFns.returning + .mockResolvedValueOnce([{ id: 's-1' }]) + .mockResolvedValueOnce([{ securityPolicyVersion: 5 }]) + const response = await POST(createMockRequest('POST'), routeContext) + expect(response.status).toBe(200) + }) + }) + + describe('revocation', () => { + beforeEach(() => { + queueTableRows(member, [{ role: 'owner' }]) + queueTableRows(organization, [{ name: 'Acme' }]) + }) + + it('reports the revoked count, bumps the version, and publishes it', async () => { + dbChainMockFns.returning + .mockResolvedValueOnce([{ id: 's-1' }, { id: 's-2' }]) + .mockResolvedValueOnce([{ securityPolicyVersion: 5 }]) + + const response = await POST(createMockRequest('POST'), routeContext) + + expect(response.status).toBe(200) + expect(await response.json()).toEqual({ success: true, data: { revokedSessions: 2 } }) + expect(dbChainMockFns.set).toHaveBeenCalledWith( + expect.objectContaining({ securityPolicyVersion: expect.anything() }) + ) + expect(mockSetVersion).toHaveBeenCalledWith(ORG_ID, 5) + expect(mockRecordAudit).toHaveBeenCalledWith( + expect.objectContaining({ + action: 'organization.sessions.revoked', + resourceId: ORG_ID, + metadata: { revokedSessions: 2 }, + }) + ) + }) + + it("never deletes the caller's own session", async () => { + dbChainMockFns.returning + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([{ securityPolicyVersion: 5 }]) + await POST(createMockRequest('POST'), routeContext) + + expect(deleteConditions()).toContainEqual( + expect.objectContaining({ type: 'ne', right: CALLER_TOKEN }) + ) + }) + + it('spares impersonation sessions', async () => { + dbChainMockFns.returning + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([{ securityPolicyVersion: 5 }]) + await POST(createMockRequest('POST'), routeContext) + + expect(deleteConditions()).toContainEqual( + expect.objectContaining({ type: 'isNull', column: sessionTable.impersonatedBy }) + ) + }) + + it('scopes the delete to members of the organization', async () => { + dbChainMockFns.returning + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([{ securityPolicyVersion: 5 }]) + await POST(createMockRequest('POST'), routeContext) + + expect(deleteConditions()).toContainEqual( + expect.objectContaining({ type: 'inArray', column: sessionTable.userId }) + ) + }) + + it("also spares the impersonator's own sessions when the caller is impersonating", async () => { + authenticateAs({ impersonatedBy: 'platform-admin-1' }) + dbChainMockFns.returning + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([{ securityPolicyVersion: 5 }]) + + await POST(createMockRequest('POST'), routeContext) + + expect(deleteConditions()).toContainEqual( + expect.objectContaining({ + type: 'ne', + left: sessionTable.userId, + right: 'platform-admin-1', + }) + ) + }) + + it('adds no impersonator exclusion for an ordinary caller', async () => { + dbChainMockFns.returning + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([{ securityPolicyVersion: 5 }]) + await POST(createMockRequest('POST'), routeContext) + + const userIdExclusions = deleteConditions().filter( + (condition) => condition.type === 'ne' && condition.left === sessionTable.userId + ) + expect(userIdExclusions).toHaveLength(0) + }) + + it('singularizes the audit description for a single session', async () => { + dbChainMockFns.returning + .mockResolvedValueOnce([{ id: 's-1' }]) + .mockResolvedValueOnce([{ securityPolicyVersion: 5 }]) + await POST(createMockRequest('POST'), routeContext) + + expect(mockRecordAudit).toHaveBeenCalledWith( + expect.objectContaining({ description: 'Revoked 1 member session' }) + ) + }) + }) +}) diff --git a/apps/sim/app/api/organizations/[id]/sessions/revoke/route.ts b/apps/sim/app/api/organizations/[id]/sessions/revoke/route.ts index 2731d2882ac..9a38d9db778 100644 --- a/apps/sim/app/api/organizations/[id]/sessions/revoke/route.ts +++ b/apps/sim/app/api/organizations/[id]/sessions/revoke/route.ts @@ -8,7 +8,7 @@ import { type NextRequest, NextResponse } from 'next/server' import { revokeOrganizationSessionsContract } from '@/lib/api/contracts/organization' import { parseRequest } from '@/lib/api/server' import { getSession } from '@/lib/auth' -import { invalidateSecurityPolicyVersionCache } from '@/lib/auth/security-policy' +import { setSecurityPolicyVersion } from '@/lib/auth/security-policy' import { isOrganizationOnEnterprisePlan } from '@/lib/billing/core/subscription' import { isBillingEnabled } from '@/lib/core/config/env-flags' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' @@ -81,7 +81,7 @@ export const POST = withRouteHandler( // Delete and version bump commit atomically: a bump failure must roll the // delete back, or members would stay authenticated from the cookie cache // for up to 24h with their DB sessions already gone. - const revoked = await db.transaction(async (tx) => { + const { revoked, version } = await db.transaction(async (tx) => { const deleted = await tx .delete(sessionTable) .where( @@ -99,13 +99,14 @@ export const POST = withRouteHandler( ) ) .returning({ id: sessionTable.id }) - await tx + const [bumped] = await tx .update(organization) .set({ securityPolicyVersion: sql`${organization.securityPolicyVersion} + 1` }) .where(eq(organization.id, organizationId)) - return deleted + .returning({ securityPolicyVersion: organization.securityPolicyVersion }) + return { revoked: deleted, version: bumped?.securityPolicyVersion } }) - invalidateSecurityPolicyVersionCache(organizationId) + if (version !== undefined) setSecurityPolicyVersion(organizationId, version) logger.info('Revoked organization sessions', { organizationId, diff --git a/apps/sim/ee/session-policy/components/session-policy-settings.tsx b/apps/sim/ee/session-policy/components/session-policy-settings.tsx index d3b6a1cb977..b5304a7ed80 100644 --- a/apps/sim/ee/session-policy/components/session-policy-settings.tsx +++ b/apps/sim/ee/session-policy/components/session-policy-settings.tsx @@ -190,7 +190,7 @@ export function SessionPolicySettings({ organizationId }: SessionPolicySettingsP diff --git a/apps/sim/lib/auth/auth.ts b/apps/sim/lib/auth/auth.ts index 167e02112dd..0da42cf078b 100644 --- a/apps/sim/lib/auth/auth.ts +++ b/apps/sim/lib/auth/auth.ts @@ -34,8 +34,12 @@ import { import { getAccessControlConfig, isEmailBlockedByAccessControl } from '@/lib/auth/access-control' import { createAnonymousSession, ensureAnonymousUserExists } from '@/lib/auth/anonymous' import { getRequestedSignInProviderId, isSignInProviderAllowed } from '@/lib/auth/constants' -import { getSessionCookieCacheVersion } from '@/lib/auth/security-policy' -import { clampExpiryForSession } from '@/lib/auth/session-policy' +import { + getMemberOrganizationId, + getSessionCookieCacheVersion, + invalidateMembershipCache, +} from '@/lib/auth/security-policy' +import { applySessionPolicyToNewMember, clampExpiryForSession } from '@/lib/auth/session-policy' import { guardSubscriptionPlanWrites } from '@/lib/auth/stripe-adapter-guard' import { sendPlanWelcomeEmail } from '@/lib/billing' import { @@ -661,36 +665,68 @@ export const auth = betterAuth({ } } + // Membership is resolved ONCE, here, and the clamp is handed the + // result. Resolving it in two places let the catch below branch on a + // stale local while the clamp had actually resolved the org from + // cache — failing open on a path documented as failing closed. + let membershipOrgId: string | null try { - // Find the first organization this user is a member of - const members = await db - .select({ organizationId: schema.member.organizationId }) - .from(schema.member) - .where(eq(schema.member.userId, session.userId)) - .limit(1) - - if (members.length > 0) { - logger.info('Found organization for user', { - userId: session.userId, - organizationId: members[0].organizationId, - }) - - const expiresAt = await clampExpiryForSession(session, members[0].organizationId) - - return { - data: { - ...session, - expiresAt, - activeOrganizationId: members[0].organizationId, - }, - } - } - logger.info('No organizations found for user', { + membershipOrgId = await getMemberOrganizationId(session.userId) + logger.info( + membershipOrgId ? 'Found organization for user' : 'No organizations found for user', + { userId: session.userId, organizationId: membershipOrgId ?? undefined } + ) + } catch (error) { + // The governing org is genuinely unknown, so a member is + // indistinguishable from the majority of users who belong to no org. + // Failing closed would take authentication down for everyone to + // protect a policy most of them do not have. Allow, unclamped: the + // after-hook above re-applies the policy once membership resolves, + // and the update hook clamps retroactively from `createdAt`. + logger.error('Could not resolve organization for new session; session not clamped', { + error, userId: session.userId, }) return { data: session } + } + + try { + const expiresAt = await clampExpiryForSession(session, membershipOrgId) + return { + data: { + ...session, + // Only ever narrows an existing expiry — never introduces an + // `expiresAt: undefined` key that would blank out the value + // Better Auth already put on the row. + ...(expiresAt ? { expiresAt } : {}), + ...(membershipOrgId ? { activeOrganizationId: membershipOrgId } : {}), + }, + } } catch (error) { - logger.error('Error setting active organization', { + // Membership already resolved above, so reaching here means only + // the POLICY read failed. For a member that is fail-closed: this + // user is definitely governed by an org, so the "would break + // sign-in for non-members too" argument does not apply. The blast + // radius is org members during a window where the `organization` + // read fails but the `member` read just succeeded — narrow, loud, + // and self-clearing. Minting a full-length session for a member + // whose policy we could not read is the one outcome this feature + // exists to prevent. + if (membershipOrgId) { + logger.error('Refusing session: org session policy could not be resolved', { + error, + userId: session.userId, + organizationId: membershipOrgId, + }) + throw new APIError('INTERNAL_SERVER_ERROR', { + message: 'Could not verify your organization session policy. Please try again.', + }) + } + + // Confirmed non-member: the clamp short-circuits before any policy + // read for a null org, so this is defence in depth rather than a + // live path. A user with no org has no policy to violate, so allow. + logger.error('Error clamping new session to org policy; session not clamped', { error, userId: session.userId, }) @@ -711,11 +747,24 @@ export const auth = betterAuth({ if (!data.expiresAt) return { data } const current = ctx?.context?.session?.session if (!current) return { data } - const expiresAt = await clampExpiryForSession({ - ...current, - expiresAt: new Date(data.expiresAt), - }) - return { data: { ...data, expiresAt } } + try { + const expiresAt = await clampExpiryForSession({ + ...current, + expiresAt: new Date(data.expiresAt), + }) + return { data: { ...data, expiresAt } } + } catch (error) { + // Refuse to EXTEND when the policy cannot be read: keeping the + // session's current expiry leaves the member signed in without + // granting them a fresh 30 days a policy might have forbidden. + logger.error('Error re-clamping refreshed session; leaving expiry unchanged', { + error, + userId: current.userId, + }) + // Normalized like the happy path above: Better Auth context values + // can cross a serialization boundary, and the column is a timestamp. + return { data: { ...data, expiresAt: new Date(current.expiresAt) } } + } }, }, }, @@ -1016,6 +1065,47 @@ export const auth = betterAuth({ return }), + /** + * Re-applies the org session policy to a session that was created before + * its `member` row existed. + * + * SSO JIT provisioning does exactly that: the callback creates the session + * first and only then writes `member` straight through the adapter + * (`@better-auth/sso` calls `handleOAuthUserInfo` before + * `assignOrganizationFromProvider`, and bypasses the organization plugin's + * `addMember`, so no member hook fires). The session-create hook therefore + * sees no membership and mints Better Auth's full 30-day expiry — and + * because a 30-day expiry reads as "not due for refresh", the sliding + * refresh would not re-clamp it for another day. On a first SSO sign-in + * into an org with session bounds, that is a real policy bypass. + * + * Runs only for sessions created WITHOUT an org (the sole case that can be + * affected) and is best-effort — a failure here must never break sign-in. + */ + after: createAuthMiddleware(async (ctx) => { + const created = ctx.context.newSession + if (!created?.session || created.session.activeOrganizationId) return + if ((created.session as { impersonatedBy?: string | null }).impersonatedBy) return + + try { + // Drop the negative membership entry the create hook just cached, so + // the row written moments ago by the provisioning path is visible. + invalidateMembershipCache(created.user.id) + const organizationId = await getMemberOrganizationId(created.user.id) + if (!organizationId) return + + logger.info('Applying org session policy to a session created pre-membership', { + userId: created.user.id, + organizationId, + }) + await applySessionPolicyToNewMember(created.user.id, organizationId) + } catch (error) { + logger.error('Failed to apply org session policy after session creation', { + error, + userId: created.user.id, + }) + } + }), }, plugins: [ ...(env.TURNSTILE_SECRET_KEY diff --git a/apps/sim/lib/auth/security-policy.test.ts b/apps/sim/lib/auth/security-policy.test.ts new file mode 100644 index 00000000000..770fbdc1c75 --- /dev/null +++ b/apps/sim/lib/auth/security-policy.test.ts @@ -0,0 +1,157 @@ +/** + * @vitest-environment node + */ +import { member, organization } from '@sim/db/schema' +import { + dbChainMockFns, + queueTableRows, + resetDbChainMock, + resetEnvFlagsMock, + setEnvFlags, +} from '@sim/testing' +import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' +import { + getMemberOrganizationId, + getSecurityPolicyVersion, + getSessionCookieCacheVersion, + invalidateMembershipCache, + setSecurityPolicyVersion, +} from '@/lib/auth/security-policy' + +/** Module-level caches persist across cases, so every case uses a fresh id. */ +let idCounter = 0 +const nextOrgId = () => `org-${++idCounter}` +const nextUserId = () => `user-${++idCounter}` + +afterAll(resetEnvFlagsMock) + +describe('security policy', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + setEnvFlags({ isOrganizationsEnabled: true }) + }) + + describe('getSecurityPolicyVersion', () => { + it('caches the version and serves a newly published one without a re-read', async () => { + const orgId = nextOrgId() + queueTableRows(organization, [{ version: 3 }]) + + expect(await getSecurityPolicyVersion(orgId)).toBe(3) + expect(await getSecurityPolicyVersion(orgId)).toBe(3) + expect(dbChainMockFns.select).toHaveBeenCalledTimes(1) + + setSecurityPolicyVersion(orgId, 4) + expect(await getSecurityPolicyVersion(orgId)).toBe(4) + // Published authoritatively by the writer — no extra read needed. + expect(dbChainMockFns.select).toHaveBeenCalledTimes(1) + }) + + it('returns the default without querying for an org-less session', async () => { + expect(await getSecurityPolicyVersion(null)).toBe(1) + expect(dbChainMockFns.select).not.toHaveBeenCalled() + }) + + it('defaults an unknown organization to version 1', async () => { + queueTableRows(organization, []) + expect(await getSecurityPolicyVersion(nextOrgId())).toBe(1) + }) + + it('never lets a late read serve or store a superseded version', async () => { + const orgId = nextOrgId() + setSecurityPolicyVersion(orgId, 10) + + // A read that started before the bump resolves late. It must neither be + // returned nor cached, or cookies would stay matched to the old version + // and revoked sessions would keep serving from the cookie cache. + queueTableRows(organization, [{ version: 9 }]) + expect(await getSecurityPolicyVersion(orgId)).toBe(10) + expect(await getSecurityPolicyVersion(orgId)).toBe(10) + }) + + it('ignores a published version older than what is cached', async () => { + const orgId = nextOrgId() + setSecurityPolicyVersion(orgId, 7) + setSecurityPolicyVersion(orgId, 6) + expect(await getSecurityPolicyVersion(orgId)).toBe(7) + }) + + it('prefers a version published mid-flight over the default when the read fails', async () => { + const orgId = nextOrgId() + // A revoke/policy save publishes the bump while this read is in flight, + // then the read fails. Returning the default would reproduce the exact + // version a pre-bump cookie carries and keep it matching. + dbChainMockFns.limit.mockImplementationOnce(() => { + setSecurityPolicyVersion(orgId, 6) + throw new Error('connection reset') + }) + expect(await getSecurityPolicyVersion(orgId)).toBe(6) + }) + + it('falls back to the default when the read fails and nothing is cached', async () => { + dbChainMockFns.limit.mockImplementationOnce(() => { + throw new Error('connection reset') + }) + // Erring low only costs a cookie mismatch and a database session read — + // it can never suppress a revocation the way a stale-high value would. + expect(await getSecurityPolicyVersion(nextOrgId())).toBe(1) + }) + }) + + describe('getMemberOrganizationId', () => { + it('returns null without querying for an anonymous session', async () => { + expect(await getMemberOrganizationId(null)).toBeNull() + expect(dbChainMockFns.select).not.toHaveBeenCalled() + }) + + it('caches a positive membership and re-reads after invalidation', async () => { + const userId = nextUserId() + queueTableRows(member, [{ organizationId: 'org-a' }]) + + expect(await getMemberOrganizationId(userId)).toBe('org-a') + expect(await getMemberOrganizationId(userId)).toBe('org-a') + expect(dbChainMockFns.select).toHaveBeenCalledTimes(1) + + invalidateMembershipCache(userId) + queueTableRows(member, [{ organizationId: 'org-b' }]) + expect(await getMemberOrganizationId(userId)).toBe('org-b') + }) + + it('propagates a read failure so callers can tell it apart from "no org"', async () => { + dbChainMockFns.limit.mockImplementationOnce(() => { + throw new Error('connection reset') + }) + await expect(getMemberOrganizationId(nextUserId())).rejects.toThrow('connection reset') + }) + }) + + describe('getSessionCookieCacheVersion', () => { + it('embeds the org id so two orgs never share a version string', async () => { + const userId = nextUserId() + const orgId = nextOrgId() + queueTableRows(member, [{ organizationId: orgId }]) + queueTableRows(organization, [{ version: 4 }]) + + expect(await getSessionCookieCacheVersion({ userId })).toBe(`${orgId}:4`) + }) + + it('returns the static default for org-less sessions', async () => { + queueTableRows(member, []) + expect(await getSessionCookieCacheVersion({ userId: nextUserId() })).toBe('none') + }) + + it('never throws on a failed membership read', async () => { + dbChainMockFns.limit.mockImplementationOnce(() => { + throw new Error('connection reset') + }) + // Runs on every authenticated request — a throw here would 500 the app. + expect(await getSessionCookieCacheVersion({ userId: nextUserId() })).toBe('none') + }) + + it('costs no lookups when organizations are disabled for the deployment', async () => { + setEnvFlags({ isOrganizationsEnabled: false }) + expect(await getSessionCookieCacheVersion({ userId: nextUserId() })).toBe('none') + expect(dbChainMockFns.select).not.toHaveBeenCalled() + }) + }) +}) diff --git a/apps/sim/lib/auth/security-policy.ts b/apps/sim/lib/auth/security-policy.ts index a8019ea0971..33c3d250e58 100644 --- a/apps/sim/lib/auth/security-policy.ts +++ b/apps/sim/lib/auth/security-policy.ts @@ -2,6 +2,8 @@ import { db } from '@sim/db' import { member, organization } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { eq } from 'drizzle-orm' +import { LRUCache } from 'lru-cache' +import { isOrganizationsEnabled } from '@/lib/core/config/env-flags' const logger = createLogger('SecurityPolicy') @@ -11,17 +13,60 @@ const logger = createLogger('SecurityPolicy') * bound on org-wide session-revocation latency: a version bump changes the * cookie-cache version, and every cached session cookie in the org falls * through to a DB read within one TTL. + * + * Only the VERSION is cached. It is read on every authenticated request, and + * Better Auth offers no cross-instance invalidation for cookie caches — its own + * guidance for immediate revocation is to disable cookie caching entirely — so + * a short TTL is the deliberate trade this feature makes. The session POLICY is + * never cached; it is read fresh at enforcement time (see `session-policy.ts`), + * which is what keeps a stale version from ever pairing with a stale policy. */ -export const SECURITY_POLICY_VERSION_CACHE_TTL_MS = 60 * 1000 +const SECURITY_POLICY_VERSION_CACHE_TTL_MS = 60 * 1000 +const SECURITY_POLICY_VERSION_CACHE_MAX_ENTRIES = 5_000 + +const MEMBERSHIP_CACHE_TTL_MS = 60 * 1000 +const MEMBERSHIP_CACHE_MAX_ENTRIES = 20_000 + +/** + * Negative (non-member) membership results use a much shorter TTL than + * positive ones: a user's cached `null` would otherwise let them dodge a new + * org's policy for the full TTL after joining through ANY path — including + * ones outside this codebase (Better Auth SSO JIT provisioning writes `member` + * rows straight through the adapter, and this app registers no `member` + * database hook that could observe it). Positive results change only through + * leave/transfer, which invalidate explicitly. + */ +const NEGATIVE_MEMBERSHIP_CACHE_TTL_MS = 15 * 1000 const DEFAULT_VERSION = 1 -interface VersionCacheEntry { - version: number - fetchedAt: number +const versionCache = new LRUCache({ + max: SECURITY_POLICY_VERSION_CACHE_MAX_ENTRIES, + ttl: SECURITY_POLICY_VERSION_CACHE_TTL_MS, +}) + +/** + * Boxed because a resolved non-member is a cached VALUE, not a cache miss, and + * `LRUCache` constrains values to non-nullish types — so `null` cannot be + * stored directly and would be indistinguishable from a miss if it could. + */ +interface MembershipCacheEntry { + organizationId: string | null } -const versionCache = new Map() +const membershipCache = new LRUCache({ + max: MEMBERSHIP_CACHE_MAX_ENTRIES, + ttl: MEMBERSHIP_CACHE_TTL_MS, +}) + +async function readStoredVersion(organizationId: string): Promise { + const [row] = await db + .select({ version: organization.securityPolicyVersion }) + .from(organization) + .where(eq(organization.id, organizationId)) + .limit(1) + return row?.version ?? DEFAULT_VERSION +} /** * Resolves the org's security-policy version — the shared monotonic counter @@ -29,6 +74,13 @@ const versionCache = new Map() * policies (session policies today; IP allowlisting and MFA enforcement are * planned consumers): any feature that needs cached session cookies to * re-validate bumps this one counter. + * + * A failed read prefers whatever this process last knew over the default. The + * default is not the safe fallback it looks like: it is the version a + * never-bumped org carries, so returning it can MATCH a pre-bump cookie and + * keep a just-revoked session serving from the cookie cache. A retained or + * freshly published value can only be equal or higher, so it either forces the + * same revalidation or more of it. */ export async function getSecurityPolicyVersion( organizationId: string | null | undefined @@ -36,49 +88,57 @@ export async function getSecurityPolicyVersion( if (!organizationId) return DEFAULT_VERSION const cached = versionCache.get(organizationId) - if (cached && Date.now() - cached.fetchedAt < SECURITY_POLICY_VERSION_CACHE_TTL_MS) { - return cached.version - } + if (cached !== undefined) return cached try { - const [row] = await db - .select({ version: organization.securityPolicyVersion }) - .from(organization) - .where(eq(organization.id, organizationId)) - .limit(1) - - const version = row?.version ?? DEFAULT_VERSION - versionCache.set(organizationId, { version, fetchedAt: Date.now() }) + const version = await readStoredVersion(organizationId) + + // Re-check after the await. The counter only ever increments, so a value + // that landed while this read was in flight is newer — neither store nor + // return ours, or a late read would re-serve a pre-bump version and keep + // cookies matched past a revocation. + // + // This narrows the window; it cannot close it. A read still in flight when + // the newer entry expires finds no floor to lose against, and no amount of + // re-reading fixes that — the replacement read can be slow across a later + // bump in exactly the same way. Closing it needs a floor that outlives the + // cache, which means cross-process state (see the deferred Redis + // invalidation follow-up), not more retries here. The residual is bounded: + // a stale read that wins serves a pre-bump version for at most one more + // TTL, so worst-case propagation is two TTLs rather than one — a slower + // instance of the latency this feature already documents, not a new class + // of failure. + const concurrent = versionCache.get(organizationId) + if (concurrent !== undefined && concurrent > version) return concurrent + + versionCache.set(organizationId, version) return version } catch (error) { - logger.error('Failed to resolve security policy version; using default', { - organizationId, - error, - }) - return DEFAULT_VERSION + logger.error('Failed to resolve security policy version', { organizationId, error }) + // A publish, or a concurrent read, may have landed while this one was in + // flight. Prefer it — the default could reproduce exactly the version a + // pre-bump cookie carries, leaving a revoked session on the cookie cache. + return versionCache.get(organizationId) ?? DEFAULT_VERSION } } -/** Drops the cached version for an org so the next read is fresh. */ -export function invalidateSecurityPolicyVersionCache(organizationId: string): void { - versionCache.delete(organizationId) -} - -interface MembershipCacheEntry { - organizationId: string | null - fetchedAt: number -} - -const membershipCache = new Map() - /** - * Negative (non-member) membership results use a much shorter TTL than - * positive ones: a user's cached `null` would otherwise let them dodge a new - * org's policy for the full TTL after joining through ANY path — including - * ones outside this codebase (Better Auth SSO JIT provisioning). Positive - * results change only through leave/transfer, which invalidate explicitly. + * Seeds THIS process with the version a write just committed. Other instances + * still pick it up on their own next read, so org-wide propagation remains + * bounded by {@link SECURITY_POLICY_VERSION_CACHE_TTL_MS} — this only removes + * the calling instance's own re-read. + * + * Deliberately a SET rather than a delete. Deleting would leave no floor for the + * monotonic guard above, so a read that started before the bump could land + * afterwards and re-seed the pre-bump version for a full TTL — exactly the + * cookie-invalidation delay the bump exists to avoid. Callers already have the + * new value from their `RETURNING` clause, so this also saves a redundant read. */ -const NEGATIVE_MEMBERSHIP_CACHE_TTL_MS = 15 * 1000 +export function setSecurityPolicyVersion(organizationId: string, version: number): void { + const current = versionCache.get(organizationId) + if (current !== undefined && current > version) return + versionCache.set(organizationId, version) +} /** Drops the cached membership for a user (call when they join/leave an org). */ export function invalidateMembershipCache(userId: string): void { @@ -86,12 +146,17 @@ export function invalidateMembershipCache(userId: string): void { } /** - * Resolves the org a user belongs to (users belong to at most one org), - * served from a short TTL cache. Org security policies govern MEMBERS, not - * just sessions that happen to carry an `activeOrganizationId` — a session - * created before the user joined an org has none, and without this fallback - * such sessions would dodge cookie-cache invalidation (and therefore - * org-wide revocation) for up to the 24h cookie lifetime. + * Resolves the org a user belongs to (users belong to at most one org — see the + * `member_user_id_unique` index), served from a short TTL cache. Org security + * policies govern MEMBERS, not just sessions that happen to carry an + * `activeOrganizationId` — a session created before the user joined an org has + * none, and without this fallback such sessions would dodge cookie-cache + * invalidation (and therefore org-wide revocation) for up to the 24h cookie + * lifetime. + * + * Throws if the lookup fails. Callers that can tolerate an unknown membership + * use {@link getMemberOrganizationIdSafe}; the session-create path deliberately + * does not, so a failed read cannot be mistaken for "not in an org". */ export async function getMemberOrganizationId( userId: string | null | undefined @@ -99,23 +164,29 @@ export async function getMemberOrganizationId( if (!userId) return null const cached = membershipCache.get(userId) - if (cached) { - const ttl = cached.organizationId - ? SECURITY_POLICY_VERSION_CACHE_TTL_MS - : NEGATIVE_MEMBERSHIP_CACHE_TTL_MS - if (Date.now() - cached.fetchedAt < ttl) return cached.organizationId - } + if (cached) return cached.organizationId + + const [row] = await db + .select({ organizationId: member.organizationId }) + .from(member) + .where(eq(member.userId, userId)) + .limit(1) + + const organizationId = row?.organizationId ?? null + membershipCache.set( + userId, + { organizationId }, + { ttl: organizationId ? MEMBERSHIP_CACHE_TTL_MS : NEGATIVE_MEMBERSHIP_CACHE_TTL_MS } + ) + return organizationId +} +/** {@link getMemberOrganizationId}, treating a failed lookup as org-less. */ +async function getMemberOrganizationIdSafe( + userId: string | null | undefined +): Promise { try { - const [row] = await db - .select({ organizationId: member.organizationId }) - .from(member) - .where(eq(member.userId, userId)) - .limit(1) - - const organizationId = row?.organizationId ?? null - membershipCache.set(userId, { organizationId, fetchedAt: Date.now() }) - return organizationId + return await getMemberOrganizationId(userId) } catch (error) { logger.error('Failed to resolve org membership; treating session as org-less', { userId, @@ -127,18 +198,24 @@ export async function getMemberOrganizationId( /** * Cookie-cache version for a session, consumed by Better Auth's - * `session.cookieCache.version`. Embeds the member org's security-policy - * version so bumps propagate to cached cookies. Resolved from the user's - * MEMBERSHIP, never the session's `activeOrganizationId` — that field goes - * stale on join/leave/transfer (it is only written at session creation), and - * a stale org here would let cookies dodge the destination org's version - * bumps for up to the 24h cookie lifetime. Sessions of non-members use the - * static default. + * `session.cookieCache.version` on EVERY authenticated request. Embeds the + * member org's security-policy version so bumps propagate to cached cookies. + * Resolved from the user's MEMBERSHIP, never the session's + * `activeOrganizationId` — that field goes stale on join/leave/transfer (it is + * only written at session creation), and a stale org here would let cookies + * dodge the destination org's version bumps for up to the 24h cookie lifetime. + * Sessions of non-members — and every session when organizations are disabled + * for the deployment — use the static default and cost no lookups. + * + * Never throws: this runs on every request, and a failed membership read here + * yields the default version, whose only effect is a cookie mismatch and a + * database session read. */ export async function getSessionCookieCacheVersion(session: { userId?: string | null }): Promise { - const organizationId = await getMemberOrganizationId(session.userId) + if (!isOrganizationsEnabled) return 'none' + const organizationId = await getMemberOrganizationIdSafe(session.userId) if (!organizationId) return 'none' // The org id is part of the version so moving between orgs always changes // the string — two orgs whose counters happen to hold the same number must diff --git a/apps/sim/lib/auth/session-policy.test.ts b/apps/sim/lib/auth/session-policy.test.ts index 6484f179b91..8dfc7ebec83 100644 --- a/apps/sim/lib/auth/session-policy.test.ts +++ b/apps/sim/lib/auth/session-policy.test.ts @@ -1,9 +1,30 @@ /** * @vitest-environment node */ -import { describe, expect, it } from 'vitest' +import { member, organization } from '@sim/db/schema' +import { + dbChainMockFns, + queueTableRows, + resetDbChainMock, + resetEnvFlagsMock, + setEnvFlags, +} from '@sim/testing' +import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' import { MIN_IDLE_TIMEOUT_HOURS } from '@/lib/api/contracts/organization' -import { clampSessionExpiry, type ResolvedSessionPolicy } from '@/lib/auth/session-policy' +import { + clampExpiryForSession, + clampSessionExpiry, + getSessionPolicy, + type ResolvedSessionPolicy, +} from '@/lib/auth/session-policy' + +const { mockResolveEnterprisePlan } = vi.hoisted(() => ({ + mockResolveEnterprisePlan: vi.fn(), +})) + +vi.mock('@/lib/billing/core/subscription', () => ({ + resolveOrganizationEnterprisePlan: mockResolveEnterprisePlan, +})) const HOUR_MS = 60 * 60 * 1000 @@ -82,3 +103,165 @@ describe('clampSessionExpiry', () => { expect(result.getTime()).toBe(shortProposal.getTime()) }) }) + +/** Module-level caches persist across cases, so every case uses a fresh id. */ +let idCounter = 0 +const nextOrgId = () => `sp-org-${++idCounter}` + +afterAll(resetEnvFlagsMock) + +describe('getSessionPolicy', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + setEnvFlags({ isBillingEnabled: true, isOrganizationsEnabled: true }) + mockResolveEnterprisePlan.mockResolvedValue(true) + }) + + it('returns no policy for an org-less session without touching the database', async () => { + expect(await getSessionPolicy(null)).toEqual({ + maxSessionHours: null, + idleTimeoutHours: null, + }) + expect(dbChainMockFns.select).not.toHaveBeenCalled() + }) + + it('skips the entitlement check when the org has no bounds stored', async () => { + queueTableRows(organization, [{ settings: null }]) + expect(await getSessionPolicy(nextOrgId())).toEqual({ + maxSessionHours: null, + idleTimeoutHours: null, + }) + expect(mockResolveEnterprisePlan).not.toHaveBeenCalled() + }) + + it('stops enforcing stored bounds for an org that is no longer entitled', async () => { + queueTableRows(organization, [{ settings: { maxSessionHours: 8, idleTimeoutHours: null } }]) + mockResolveEnterprisePlan.mockResolvedValue(false) + + expect(await getSessionPolicy(nextOrgId())).toEqual({ + maxSessionHours: null, + idleTimeoutHours: null, + }) + }) + + it('keeps enforcing when the entitlement check itself fails', async () => { + queueTableRows(organization, [{ settings: { maxSessionHours: 8, idleTimeoutHours: null } }]) + // Stored bounds are only writable by an entitled org, so a failed plan read + // must not be mistaken for a downgrade and silently disable enforcement. + mockResolveEnterprisePlan.mockRejectedValue(new Error('billing unavailable')) + + expect(await getSessionPolicy(nextOrgId())).toEqual({ + maxSessionHours: 8, + idleTimeoutHours: null, + }) + }) +}) + +describe('clampExpiryForSession', () => { + const createdAt = new Date('2026-07-22T00:00:00Z') + const proposed = new Date('2026-08-21T00:00:00Z') + + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + setEnvFlags({ isBillingEnabled: true, isOrganizationsEnabled: true }) + mockResolveEnterprisePlan.mockResolvedValue(true) + }) + + it('exempts impersonation sessions from the clamp', async () => { + const result = await clampExpiryForSession({ + userId: 'user-1', + impersonatedBy: 'platform-admin-1', + createdAt, + expiresAt: proposed, + }) + + expect(result?.getTime()).toBe(proposed.getTime()) + // Exempt before any lookup — impersonation must not even resolve a policy. + expect(dbChainMockFns.select).not.toHaveBeenCalled() + }) + + it('returns undefined when the session carries no expiry', async () => { + expect(await clampExpiryForSession({ userId: 'user-1', createdAt })).toBeUndefined() + }) + + it('leaves non-member sessions untouched', async () => { + queueTableRows(member, []) + const result = await clampExpiryForSession({ + userId: 'user-1', + createdAt, + expiresAt: proposed, + }) + expect(result?.getTime()).toBe(proposed.getTime()) + }) + + it('clamps against the org resolved from membership', async () => { + const orgId = nextOrgId() + queueTableRows(member, [{ organizationId: orgId }]) + queueTableRows(organization, [{ settings: { maxSessionHours: 24, idleTimeoutHours: null } }]) + + const result = await clampExpiryForSession({ + userId: `user-${orgId}`, + createdAt, + expiresAt: proposed, + }) + + expect(result?.getTime()).toBe(createdAt.getTime() + 24 * 60 * 60 * 1000) + }) + + it('skips the membership lookup when the caller already resolved it', async () => { + const orgId = nextOrgId() + queueTableRows(organization, [{ settings: { maxSessionHours: 24, idleTimeoutHours: null } }]) + + const result = await clampExpiryForSession( + { userId: 'user-fresh', createdAt, expiresAt: proposed }, + orgId + ) + + expect(result?.getTime()).toBe(createdAt.getTime() + 24 * 60 * 60 * 1000) + // Only the organization row was read; membership came from the caller. + expect(dbChainMockFns.select).toHaveBeenCalledTimes(1) + }) + + it('normalizes ISO string dates crossing the hook serialization boundary', async () => { + const orgId = nextOrgId() + queueTableRows(organization, [{ settings: { maxSessionHours: 24, idleTimeoutHours: null } }]) + + const result = await clampExpiryForSession( + { + userId: 'user-iso', + createdAt: createdAt.toISOString(), + expiresAt: proposed.toISOString(), + }, + orgId + ) + + expect(result?.getTime()).toBe(createdAt.getTime() + 24 * 60 * 60 * 1000) + }) + + it('reads the policy fresh on every call so a saved policy is never missed', async () => { + const orgId = nextOrgId() + queueTableRows(organization, [{ settings: null }]) + await clampExpiryForSession({ userId: 'u', createdAt, expiresAt: proposed }, orgId) + + // A policy saved on another process between the two calls. Nothing is + // cached, so the second clamp already sees it. + queueTableRows(organization, [{ settings: { maxSessionHours: 24, idleTimeoutHours: null } }]) + const result = await clampExpiryForSession( + { userId: 'u', createdAt, expiresAt: proposed }, + orgId + ) + + expect(result?.getTime()).toBe(createdAt.getTime() + 24 * 60 * 60 * 1000) + }) + + it('propagates a policy read failure so the hook can refuse to extend', async () => { + dbChainMockFns.limit.mockImplementationOnce(() => { + throw new Error('connection reset') + }) + await expect( + clampExpiryForSession({ userId: 'u', createdAt, expiresAt: proposed }, nextOrgId()) + ).rejects.toThrow('connection reset') + }) +}) diff --git a/apps/sim/lib/auth/session-policy.ts b/apps/sim/lib/auth/session-policy.ts index 484423dfde3..a0956442fef 100644 --- a/apps/sim/lib/auth/session-policy.ts +++ b/apps/sim/lib/auth/session-policy.ts @@ -1,18 +1,13 @@ import { db } from '@sim/db' -import type { SessionPolicySettings } from '@sim/db/schema' import { organization } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { eq, sql } from 'drizzle-orm' import { MIN_IDLE_TIMEOUT_HOURS } from '@/lib/api/contracts/organization' import { getMemberOrganizationId, invalidateMembershipCache } from '@/lib/auth/security-policy' -import { isOrganizationOnEnterprisePlan } from '@/lib/billing/core/subscription' -import { isBillingEnabled } from '@/lib/core/config/env-flags' +import { resolveOrganizationEnterprisePlan } from '@/lib/billing/core/subscription' const logger = createLogger('SessionPolicy') -/** How long a resolved org session policy is served from process memory. */ -export const SESSION_POLICY_CACHE_TTL_MS = 60 * 1000 - const HOUR_MS = 60 * 60 * 1000 export interface ResolvedSessionPolicy { @@ -20,67 +15,86 @@ export interface ResolvedSessionPolicy { idleTimeoutHours: number | null } -interface PolicyCacheEntry { - policy: ResolvedSessionPolicy - fetchedAt: number -} - -const policyCache = new Map() - const NO_POLICY: ResolvedSessionPolicy = { maxSessionHours: null, idleTimeoutHours: null, } /** - * Resolves the EFFECTIVE session policy for an organization, served from a - * short TTL cache. Returns a no-op policy for personal (org-less) sessions - * and — mirroring data-retention's plan-gated effective settings — for - * hosted orgs no longer on an Enterprise plan: stored limits stop enforcing - * automatically on downgrade, since the enterprise-gated settings UI can no - * longer manage them. + * Whether stored session bounds should be enforced for this org. Mirroring + * data-retention's plan-gated effective settings, a hosted org that leaves the + * Enterprise plan stops enforcing its stored limits automatically, since the + * enterprise-gated settings UI can no longer manage them. + * + * Uses the PROPAGATING plan resolver so a failed read is not mistaken for a + * downgrade. `isOrganizationOnEnterprisePlan` swallows its errors and returns + * `false`, which is correct for feature gating (fail closed) but would silently + * stop ENFORCING here — the fail-open trap the PII-redaction path documents in + * `lib/logs/execution/logger.ts`. Callers only reach this when bounds are + * actually stored, so non-enterprise orgs never pay for it. */ -export async function getSessionPolicy( - organizationId: string | null | undefined -): Promise { - if (!organizationId) return NO_POLICY - - const cached = policyCache.get(organizationId) - if (cached && Date.now() - cached.fetchedAt < SESSION_POLICY_CACHE_TTL_MS) { - return cached.policy - } - +async function isEntitledToEnforce(organizationId: string): Promise { try { - const [row] = await db - .select({ settings: organization.sessionPolicySettings }) - .from(organization) - .where(eq(organization.id, organizationId)) - .limit(1) - - const settings: SessionPolicySettings = row?.settings ?? {} - const hasBounds = Boolean(settings.maxSessionHours || settings.idleTimeoutHours) - const isEntitled = - !hasBounds || !isBillingEnabled || (await isOrganizationOnEnterprisePlan(organizationId)) - const policy: ResolvedSessionPolicy = isEntitled - ? { - maxSessionHours: settings.maxSessionHours ?? null, - idleTimeoutHours: settings.idleTimeoutHours ?? null, - } - : NO_POLICY - policyCache.set(organizationId, { policy, fetchedAt: Date.now() }) - return policy + return await resolveOrganizationEnterprisePlan(organizationId) } catch (error) { - logger.error('Failed to resolve session policy; applying no policy', { + logger.error('Enterprise entitlement check failed; enforcing stored session policy', { organizationId, error, }) - return NO_POLICY + return true } } -/** Drops the cached policy for an org so the next read is fresh. */ -export function invalidateSessionPolicyCache(organizationId: string): void { - policyCache.delete(organizationId) +/** + * Resolves the EFFECTIVE session policy for an organization, read FRESH from + * the organization row on every call. + * + * Deliberately uncached, matching how the sibling enterprise setting resolves + * its stored rules at enforcement time (`lib/logs/execution/logger.ts`, + * `lib/workflows/executor/execution-core.ts`). This is not a hot path: it runs + * on sign-in and on a sliding session refresh, which the 24h cookie cache + * limits to roughly once per user per day. + * + * Cost is one indexed lookup for an org with no bounds stored — the common + * case. An org that DOES store bounds additionally pays the entitlement check + * (owner lookup + block status + subscription), so about four reads. That is + * affordable per sign-in, but it lands all at once for every member when a + * policy save or org-wide revocation invalidates the whole org's cookie caches + * together; watch it if enterprise orgs grow much larger. + * + * A cache here is what made an earlier version of this code incorrect. Better + * Auth rewrites `expiresAt` to `now + 30d` on every refresh, so a refresh that + * clamps against a stale policy silently un-does a tightening — and because a + * 30-day expiry then reads as "not due for refresh", nothing re-clamps it for + * another day. Reading fresh removes that failure mode by construction rather + * than trying to keep two caches coherent. + * + * Throws if the read fails. Callers pick their own failure posture: the session + * UPDATE hook keeps the current expiry rather than extending it, and the CREATE + * hook refuses the sign-in when the governing org is known but allows it when + * membership itself could not be resolved. + */ +export async function getSessionPolicy( + organizationId: string | null | undefined +): Promise { + if (!organizationId) return NO_POLICY + + const [row] = await db + .select({ settings: organization.sessionPolicySettings }) + .from(organization) + .where(eq(organization.id, organizationId)) + .limit(1) + + const settings = row?.settings ?? {} + const bounds: ResolvedSessionPolicy = { + maxSessionHours: settings.maxSessionHours ?? null, + idleTimeoutHours: settings.idleTimeoutHours ?? null, + } + + if (!bounds.maxSessionHours && !bounds.idleTimeoutHours) return NO_POLICY + if (!(await isEntitledToEnforce(organizationId))) return NO_POLICY + + return bounds } /** diff --git a/apps/sim/lib/billing/core/subscription.ts b/apps/sim/lib/billing/core/subscription.ts index 5ab950c0eb8..a1855c05a3a 100644 --- a/apps/sim/lib/billing/core/subscription.ts +++ b/apps/sim/lib/billing/core/subscription.ts @@ -427,26 +427,43 @@ export async function isEnterpriseOrgAdminOrOwner(userId: string): Promise { - try { - if (!isBillingEnabled) { - return true - } +export async function resolveOrganizationEnterprisePlan(organizationId: string): Promise { + if (!isBillingEnabled) { + return true + } - if (isAccessControlEnabled && !isHosted) { - return true - } + if (isAccessControlEnabled && !isHosted) { + return true + } - if (await isOrganizationBillingBlocked(organizationId)) { - return false - } + if (await isOrganizationBillingBlocked(organizationId)) { + return false + } + + // `onError: 'throw'` is load-bearing: the default swallows a read failure into + // `null`, which reads as "no usable subscription" and would make this resolver + // report a downgrade on a transient error — exactly the conflation it exists + // to avoid. `isOrganizationOnEnterprisePlan` still turns that throw back into + // `false` for feature gating. + const orgSub = await getOrganizationSubscriptionUsable(organizationId, { onError: 'throw' }) - const orgSub = await getOrganizationSubscriptionUsable(organizationId) + return !!orgSub && checkEnterprisePlan(orgSub) +} - return !!orgSub && checkEnterprisePlan(orgSub) +/** + * Check if an organization has an enterprise plan + * Used for Access Control (Permission Groups) feature gating + */ +export async function isOrganizationOnEnterprisePlan(organizationId: string): Promise { + try { + return await resolveOrganizationEnterprisePlan(organizationId) } catch (error) { logger.error('Error checking organization enterprise plan status', { error, organizationId }) return false