diff --git a/server/src/__tests__/auth-session-route.test.ts b/server/src/__tests__/auth-session-route.test.ts index 0ad9fe91a57..a04be97a268 100644 --- a/server/src/__tests__/auth-session-route.test.ts +++ b/server/src/__tests__/auth-session-route.test.ts @@ -83,9 +83,11 @@ describe("actorMiddleware authenticated session profile", () => { const inserts: Array<{ values: Record }> = []; const db = { insert: vi.fn(() => { + let own: { values: Record } | undefined; const chain = { values(values: Record) { - inserts.push({ values }); + own = { values }; + inserts.push(own); return chain; }, onConflictDoUpdate() { @@ -96,9 +98,9 @@ describe("actorMiddleware authenticated session profile", () => { }, returning() { return Promise.resolve([{ - companyId: inserts.at(-1)?.values.companyId, - membershipRole: inserts.at(-1)?.values.membershipRole, - status: inserts.at(-1)?.values.status, + companyId: own?.values.companyId, + membershipRole: own?.values.membershipRole, + status: own?.values.status, }]); }, }; @@ -143,15 +145,18 @@ describe("actorMiddleware authenticated session profile", () => { expect(res.body.companyIds[0]).toMatch(/^[0-9a-f-]{36}$/); // authUsers, companies, companyMemberships, the role-default // principalPermissionGrants, and the lazily initialized instance setting. + // The instance setting is read concurrently with the tenant sync, so + // match inserts by content rather than position. expect(inserts).toHaveLength(5); - expect(inserts[0]?.values).toMatchObject({ - id: "global-user-1", - email: "owner@example.com", - emailVerified: true, - }); - expect(inserts[1]?.values).toMatchObject({ - name: "Purple Rain", - }); + expect(inserts.map((insert) => insert.values)).toEqual(expect.arrayContaining([ + expect.objectContaining({ + id: "global-user-1", + email: "owner@example.com", + emailVerified: true, + }), + expect.objectContaining({ name: "Purple Rain" }), + expect.objectContaining({ singletonKey: "default" }), + ])); }); it("lets the cloud tenant actor through assertCompanyAccess for a company it holds a membership row in", async () => { diff --git a/server/src/__tests__/cloud-tenant-actor-round-trips.test.ts b/server/src/__tests__/cloud-tenant-actor-round-trips.test.ts new file mode 100644 index 00000000000..b17fa75f6ca --- /dev/null +++ b/server/src/__tests__/cloud-tenant-actor-round-trips.test.ts @@ -0,0 +1,140 @@ +import { resolveExternalTestDatabaseUrl } from "./helpers/external-test-database.js"; +import net from "node:net"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { sql } from "drizzle-orm"; +import { + activityLog, + authUsers, + companies, + companyMemberships, + createDb, + instanceUserRoles, + principalPermissionGrants, +} from "@paperclipai/db"; +import { + getEmbeddedPostgresTestSupport, + startEmbeddedPostgresTestDatabase, +} from "./helpers/embedded-postgres.js"; +import { cloudActorHeaderSourceFromHeaders, resolveCloudTenantActor } from "../middleware/auth.js"; + +// Cloud tenant requests resolve their actor on every request, static files +// included. On a loaded database each round trip waits, so the steady-state +// resolution (tenant sync debounced) must cost one round trip, not one per +// statement. This proxy delays every client-to-server write by a fixed amount +// so the number of sequential round trips shows up as wall time. + +const external = resolveExternalTestDatabaseUrl("PAPERCLIP_CLOUD_PROVISIONING_TEST_DATABASE_URL"); +const embeddedPostgresSupport = external ? { supported: true, reason: undefined } : await getEmbeddedPostgresTestSupport(); +const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; + +const SERVER_TOKEN = "test-server-token"; +const ROUND_TRIP_DELAY_MS = 150; + +function startDelayProxy(target: URL, delayMs: number) { + const sockets = new Set(); + const server = net.createServer((client) => { + const upstream = net.connect(Number(target.port || 5432), target.hostname); + sockets.add(client); + sockets.add(upstream); + let releaseAt = 0; + client.on("data", (chunk) => { + // Preserve byte order while delaying each write by at least delayMs. + releaseAt = Math.max(Date.now() + delayMs, releaseAt); + setTimeout(() => upstream.write(chunk), releaseAt - Date.now()); + }); + upstream.on("data", (chunk) => client.write(chunk)); + const close = () => { client.destroy(); upstream.destroy(); }; + client.on("close", close); + upstream.on("close", close); + client.on("error", close); + upstream.on("error", close); + }); + return new Promise<{ url: string; close: () => Promise }>((resolve) => { + server.listen(0, "127.0.0.1", () => { + const { port } = server.address() as net.AddressInfo; + const proxied = new URL(target.toString()); + proxied.hostname = "127.0.0.1"; + proxied.port = String(port); + resolve({ + url: proxied.toString(), + close: () => new Promise((done) => { + for (const socket of sockets) socket.destroy(); + server.close(() => done()); + }), + }); + }); + }); +} + +describeEmbeddedPostgres("cloud tenant actor round trips", () => { + let tempDb: Awaited> | null = null; + let directDb!: ReturnType; + let slowDb!: ReturnType; + let proxy!: Awaited>; + + beforeAll(async () => { + if (!external) tempDb = await startEmbeddedPostgresTestDatabase("paperclip-tenant-round-trips-"); + const url = external ?? tempDb!.connectionString; + directDb = createDb(url); + proxy = await startDelayProxy(new URL(url), ROUND_TRIP_DELAY_MS); + slowDb = createDb(proxy.url); + process.env.PAPERCLIP_CLOUD_TENANT_SERVER_TOKEN = SERVER_TOKEN; + }, 30_000); + + afterAll(async () => { + delete process.env.PAPERCLIP_CLOUD_TENANT_SERVER_TOKEN; + await directDb?.delete(activityLog); + await directDb?.delete(principalPermissionGrants); + await directDb?.delete(companyMemberships); + await directDb?.delete(companies); + await directDb?.delete(instanceUserRoles); + await directDb?.delete(authUsers); + await slowDb?.$client.end({ timeout: 1 }); + await directDb?.$client.end({ timeout: 1 }); + await proxy?.close(); + await tempDb?.cleanup(); + }); + + it("resolves a synced tenant actor in about one database round trip", async () => { + const headers = cloudActorHeaderSourceFromHeaders({ + "x-paperclip-cloud-tenant-token": SERVER_TOKEN, + "x-paperclip-cloud-user-id": "user-round-trips", + "x-paperclip-cloud-user-email": "user-round-trips@example.com", + "x-paperclip-cloud-stack-id": "stack-round-trips", + // Owners also read the instance settings for owner elevation. + "x-paperclip-cloud-stack-role": "owner", + }); + + // First request provisions the tenant; its writes are debounced afterwards. + const first = await resolveCloudTenantActor(slowDb, headers); + expect(first?.type).toBe("board"); + // Open enough pooled connections that connection setup is not measured. + await Promise.all(Array.from({ length: 4 }, () => slowDb.execute(sql`select 1`))); + + // Measure one statement through the same proxy as the baseline, so a slow + // runner scales both sides of the comparison. + const statementMs: number[] = []; + for (let attempt = 0; attempt < 3; attempt++) { + const statementStarted = performance.now(); + await slowDb.execute(sql`select 1`); + statementMs.push(performance.now() - statementStarted); + } + const oneStatementMs = statementMs.sort((a, b) => a - b)[1]!; + + const started = performance.now(); + const actor = await resolveCloudTenantActor(slowDb, headers); + const elapsedMs = performance.now() - started; + + expect(actor).toMatchObject({ + type: "board", + userId: "user-round-trips", + source: "cloud_tenant", + isInstanceAdmin: false, + }); + expect(actor?.type === "board" ? actor.companyIds : []).toHaveLength(1); + // `select 1` is one delayed write; a parameterized statement is two. Three + // sequential statements (purge, memberships, owner elevation) take about + // 6 × the baseline; concurrent statements take about 2 ×. + expect(elapsedMs).toBeLessThan(oneStatementMs * 4); + }, 30_000); +}); diff --git a/server/src/middleware/auth.ts b/server/src/middleware/auth.ts index 92ccddcf508..60ca593d153 100644 --- a/server/src/middleware/auth.ts +++ b/server/src/middleware/auth.ts @@ -622,83 +622,90 @@ async function resolveCloudTenantActorOnce( // the BetterAuth session path, board API keys, and the authorization // service's own instanceUserRoles lookup — so actively purge them on every // trusted-header authentication instead of merely no longer inserting them. - await db - .delete(instanceUserRoles) - .where(and(eq(instanceUserRoles.userId, userId), eq(instanceUserRoles.role, "instance_admin"))); - - await syncCloudTenantWrites(db, userId, syncFingerprint, async () => { - const now = new Date(); + // + // Every request runs this path, static files included. The purge and the + // tenant sync stay ordered; the two reads below are independent of them, so + // they run concurrently. A loaded database then costs one round trip per + // request rather than three in a row. + const purgeAndSync = (async () => { await db - .insert(authUsers) - .values({ - id: userId, - name: userName, - email: userEmail, - emailVerified: true, - image: null, - createdAt: now, - updatedAt: now, - }) - .onConflictDoUpdate({ - target: authUsers.id, - set: { name: userName, email: userEmail, emailVerified: true, updatedAt: now }, - setWhere: sql`${authUsers.name} is distinct from ${userName} - or ${authUsers.email} is distinct from ${userEmail} - or ${authUsers.emailVerified} is distinct from true`, - }); + .delete(instanceUserRoles) + .where(and(eq(instanceUserRoles.userId, userId), eq(instanceUserRoles.role, "instance_admin"))); - await insertCloudTenantCompany(db, { companyId, companyName, now }); - if (paperclipCompanyName) { - await repairCloudTenantCompanyName(db, { - companyId, - paperclipCompanyId, - paperclipCompanyName, - now, - }); - } - // Derive any legacy prefix from the repaired name, preserving the helper's - // exact legacy-marker guards and the operator's own edits. - await repairCloudTenantCompanyProvisionDefaults(db, { companyId, stackId, now }); - - effectiveMembership = await db.transaction(async tx => { - const [existing] = await tx.select({ - status: companyMemberships.status, - membershipRole: companyMemberships.membershipRole, - }).from(companyMemberships).where(and( - eq(companyMemberships.companyId, companyId), - eq(companyMemberships.principalType, "user"), - eq(companyMemberships.principalId, userId), - )).for("update"); - // Serialize authority restoration and its notice while retaining the - // guarded upsert that leaves unchanged memberships untouched. - if (!existing || existing.status !== "active" || existing.membershipRole === "viewer") { - await notifyChatCompanyAuthorityChange(tx, { companyId }); - await notifyMuseContactVerification(tx, { companyId, operatorId: userId }); - } - return tx - .insert(companyMemberships) - .values({ companyId, principalType: "user", principalId: userId, status: "active", membershipRole, updatedAt: now }) - .onConflictDoUpdate({ - target: [companyMemberships.companyId, companyMemberships.principalType, companyMemberships.principalId], - set: { status: "active", membershipRole, updatedAt: now }, - setWhere: sql`${companyMemberships.status} is distinct from 'active' - or ${companyMemberships.membershipRole} is distinct from ${membershipRole}`, + await syncCloudTenantWrites(db, userId, syncFingerprint, async () => { + const now = new Date(); + await db + .insert(authUsers) + .values({ + id: userId, + name: userName, + email: userEmail, + emailVerified: true, + image: null, + createdAt: now, + updatedAt: now, }) - .returning({ companyId: companyMemberships.companyId, membershipRole: companyMemberships.membershipRole, status: companyMemberships.status }) - // PostgreSQL returns no row for an unchanged ON CONFLICT ... WHERE. - // That condition itself proves the existing role/status match the input. - .then((rows) => rows[0] ?? { companyId, membershipRole, status: "active" }); - }); + .onConflictDoUpdate({ + target: authUsers.id, + set: { name: userName, email: userEmail, emailVerified: true, updatedAt: now }, + setWhere: sql`${authUsers.name} is distinct from ${userName} + or ${authUsers.email} is distinct from ${userEmail} + or ${authUsers.emailVerified} is distinct from true`, + }); - // An unchanged membership can still lack default grants after a partial - // sync or an older deployment; keep this repair inside every actual sync. - await ensureHumanRoleDefaultGrants(db, { - companyId, - principalId: userId, - membershipRole: effectiveMembership.membershipRole ?? membershipRole, - grantedByUserId: null, + await insertCloudTenantCompany(db, { companyId, companyName, now }); + if (paperclipCompanyName) { + await repairCloudTenantCompanyName(db, { + companyId, + paperclipCompanyId, + paperclipCompanyName, + now, + }); + } + // Derive any legacy prefix from the repaired name, preserving the helper's + // exact legacy-marker guards and the operator's own edits. + await repairCloudTenantCompanyProvisionDefaults(db, { companyId, stackId, now }); + + effectiveMembership = await db.transaction(async tx => { + const [existing] = await tx.select({ + status: companyMemberships.status, + membershipRole: companyMemberships.membershipRole, + }).from(companyMemberships).where(and( + eq(companyMemberships.companyId, companyId), + eq(companyMemberships.principalType, "user"), + eq(companyMemberships.principalId, userId), + )).for("update"); + // Serialize authority restoration and its notice while retaining the + // guarded upsert that leaves unchanged memberships untouched. + if (!existing || existing.status !== "active" || existing.membershipRole === "viewer") { + await notifyChatCompanyAuthorityChange(tx, { companyId }); + await notifyMuseContactVerification(tx, { companyId, operatorId: userId }); + } + return tx + .insert(companyMemberships) + .values({ companyId, principalType: "user", principalId: userId, status: "active", membershipRole, updatedAt: now }) + .onConflictDoUpdate({ + target: [companyMemberships.companyId, companyMemberships.principalType, companyMemberships.principalId], + set: { status: "active", membershipRole, updatedAt: now }, + setWhere: sql`${companyMemberships.status} is distinct from 'active' + or ${companyMemberships.membershipRole} is distinct from ${membershipRole}`, + }) + .returning({ companyId: companyMemberships.companyId, membershipRole: companyMemberships.membershipRole, status: companyMemberships.status }) + // PostgreSQL returns no row for an unchanged ON CONFLICT ... WHERE. + // That condition itself proves the existing role/status match the input. + .then((rows) => rows[0] ?? { companyId, membershipRole, status: "active" }); + }); + + // An unchanged membership can still lack default grants after a partial + // sync or an older deployment; keep this repair inside every actual sync. + await ensureHumanRoleDefaultGrants(db, { + companyId, + principalId: userId, + membershipRole: effectiveMembership.membershipRole ?? membershipRole, + grantedByUserId: null, + }); }); - }); + })(); // The stack's seeded company is only where Cloud provisioned this user. // Companies created afterwards on the instance (imports, in-app company @@ -706,21 +713,42 @@ async function resolveCloudTenantActorOnce( // the pinned primary — the same active-membership scope a locally // authenticated session actor carries. Strictly this user's own rows; the // membership-creating flows seed their own permission grants, so nothing - // needs seeding per request here. A read failure degrades to the pinned - // primary instead of blocking authentication, mirroring the fail-closed - // owner-elevation resolution below. - let additionalMemberships: { companyId: string; membershipRole: string | null; status: string }[] = - []; - try { - additionalMemberships = (await loadActiveUserCompanyMemberships(db, userId)).filter( - (row) => row.companyId !== companyId, + // needs seeding per request here. The sync above writes only the primary + // company's membership, which this read excludes, so it need not wait for + // the sync. A read failure degrades to the pinned primary instead of + // blocking authentication, mirroring the fail-closed owner-elevation + // resolution below. + const additionalMembershipsRead: Promise<{ companyId: string; membershipRole: string | null; status: string }[]> = + loadActiveUserCompanyMemberships(db, userId).then( + (rows) => rows.filter((row) => row.companyId !== companyId), + (err) => { + logger.warn( + { err, userId, stackId }, + "Failed to load cloud tenant user's company memberships; scoping actor to the stack's primary company", + ); + return []; + }, ); - } catch (err) { - logger.warn( - { err, userId, stackId }, - "Failed to load cloud tenant user's company memberships; scoping actor to the stack's primary company", - ); - } + // Computed per request, never persisted: the stack owner is elevated to + // instance admin of their own dedicated instance only while the + // `enableOwnerInstanceAdmin` flag is on. Non-owner stack roles stay + // company-scoped. Turning the flag off de-elevates on the next request — + // there is no role row to clean up. + const ownerInstanceAdminRead = resolveOwnerInstanceAdmin(db, stackRole); + + // Settle every started statement before an error leaves: the caller stops + // counting authentication work for idle and drain checks when this throws, + // and the owner-settings read can insert the missing settings row. + const [synced, membershipsRead, ownerRead] = await Promise.allSettled([ + purgeAndSync, + additionalMembershipsRead, + ownerInstanceAdminRead, + ]); + if (synced.status === "rejected") throw synced.reason; + if (membershipsRead.status === "rejected") throw membershipsRead.reason; + if (ownerRead.status === "rejected") throw ownerRead.reason; + const additionalMemberships = membershipsRead.value; + const isInstanceAdmin = ownerRead.value; return { type: "board", @@ -736,12 +764,7 @@ async function resolveCloudTenantActorOnce( }, ...additionalMemberships, ], - // Computed per request, never persisted: the stack owner is elevated to - // instance admin of their own dedicated instance only while the - // `enableOwnerInstanceAdmin` flag is on. Non-owner stack roles stay - // company-scoped. Turning the flag off de-elevates on the next request — - // there is no role row to clean up. - isInstanceAdmin: await resolveOwnerInstanceAdmin(db, stackRole), + isInstanceAdmin, canManageExperimentalSettings: stackRole === "admin", source: "cloud_tenant", }; diff --git a/server/src/middleware/cloud-tenant-actor.test.ts b/server/src/middleware/cloud-tenant-actor.test.ts index 8aaca4e5990..c0c13cc6c42 100644 --- a/server/src/middleware/cloud-tenant-actor.test.ts +++ b/server/src/middleware/cloud-tenant-actor.test.ts @@ -30,6 +30,7 @@ function createFakeDb(options: { settingsRow?: Record | null; selectThrows?: boolean; beforeWrite?: (table: unknown) => Promise; + beforeRead?: (table: unknown) => Promise; unchangedMembership?: boolean; } = {}) { const settingsRow = @@ -101,7 +102,7 @@ function createFakeDb(options: { const query = options.selectThrows && table === companyMemberships && !locked ? Promise.reject(new Error("select unavailable")) : Promise.resolve(rows); - return query.then(resolve, reject); + return Promise.resolve().then(() => options.beforeRead?.(table)).then(() => query).then(resolve, reject); }, }; return result; @@ -183,6 +184,26 @@ describe("resolveCloudTenantActor (shared-pool hardening)", () => { expect(actor!.source).toBe("cloud_tenant"); }); + it("settles its concurrent reads before a failed purge leaves authentication", async () => { + let settingsReadStarted = false; + let settingsReadSettled = false; + const { db } = createFakeDb({ + beforeWrite: async (table) => { + if (table === instanceUserRoles) throw new Error("purge failed"); + }, + beforeRead: async (table) => { + if (table !== instanceSettings) return; + settingsReadStarted = true; + await new Promise((resolve) => setTimeout(resolve, 50)); + settingsReadSettled = true; + }, + }); + await expect(resolveCloudTenantActor(db, fakeReq(VALID_HEADERS))).rejects.toThrow("purge failed"); + // No started read may still be running once the error surfaces. + expect(settingsReadStarted).toBe(true); + expect(settingsReadSettled).toBe(true); + }); + it("purges stale instance_admin rows left by pre-hardening deployments", async () => { const { db, deletedTables } = createFakeDb(); const actor = await resolveCloudTenantActor(db, fakeReq(VALID_HEADERS)); @@ -478,12 +499,15 @@ describe("resolveCloudTenantActor (shared-pool hardening)", () => { await resolveCloudTenantActor(db, fakeReq(VALID_HEADERS)); const membershipSelects = selectWheres.filter((entry) => entry.table === companyMemberships); expect(membershipSelects).toHaveLength(2); - // The first read locks the primary membership before its upsert; the - // second loads the user's active memberships for request authorization. - // Rows for other users and rows in any non-active status are excluded - // by the query itself: the filter binds exactly this user id and the - // "active" status — the same condition the session path applies. - expect(membershipSelects[1]!.condition).toEqual( + // One read locks the primary membership before its upsert; the other, + // which runs concurrently, loads the user's active memberships for + // request authorization. Rows for other users and rows in any + // non-active status are excluded by the query itself: the filter binds + // exactly this user id and the "active" status — the same condition + // the session path applies. + const authorizationReads = membershipSelects.filter((entry) => !entry.locked); + expect(authorizationReads).toHaveLength(1); + expect(authorizationReads[0]!.condition).toEqual( and( eq(companyMemberships.principalType, "user"), eq(companyMemberships.principalId, "user-123"),