mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 05:11:49 +01:00
perf(auth): resolve a Cloud tenant actor in one database round trip (#20390)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - On Paperclip Cloud, each request resolves its board actor from trusted Cloud headers in `actorMiddleware` > - This runs for every request after the liveness probe, also for static files such as `favicon.ico` and `index.html` > - The resolution ran three database statements one after another: the stale `instance_admin` purge, the membership read and the owner-elevation read > - On a loaded database, each statement waits. Three sequential waits make every request slow, also a request that needs no data > - This pull request keeps the purge and the tenant sync in order and runs the two reads at the same time as them > - The benefit is one database wait per request instead of three ## Linked Issues or Issue Description **What existing behavior does this improve?** Cloud tenant actor resolution in `server/src/middleware/auth.ts` (`resolveCloudTenantActorOnce`). **Subsystem affected** Server authentication middleware (Cloud trusted-header path). **Current behavior** Measured on the hosted staging instance on 2026-11-09 from a signed-in browser, 45 paired samples in 40 s: - `/api/liveness` (mounted before actor resolution): client median 92 ms, p90 255 ms, server time ≤ 1 ms. - `/favicon.ico` with a session (after actor resolution): client median 413 ms, p90 1,751 ms, max 4.6 s. - `/api/cli-auth/me`: server median 346 ms, p90 1,528 ms. The same Node process answers liveness quickly, so the event loop is not blocked. The database is busy (6–8 of 8 vCPU for the whole day), and trivial primary-key reads waited 100–280 ms each. Three sequential reads explain the ~320 ms median gap between liveness and favicon. **Proposed behavior** The purge and the debounced tenant sync stay ordered. The membership read and the owner-elevation read run at the same time as them. The sync writes only the primary company's membership, and the membership read excludes that company, so the result does not change. **Reason and benefit** A synced request costs one database wait instead of three. With 150 ms of injected database latency, a synced resolution goes from 914 ms to 310 ms (3 runs each, same machine). **Breaking changes** None. ## What Changed - `resolveCloudTenantActorOnce` starts the purge-and-sync, the additional-membership read and `resolveOwnerInstanceAdmin` together and awaits them with `Promise.all`. A membership read failure still degrades to the primary company. The owner-elevation read still fails closed. - New regression test `server/src/__tests__/cloud-tenant-actor-round-trips.test.ts`. It connects through a TCP proxy that delays every client write by 150 ms and asserts that a synced resolution finishes in less than 4 × the delay. Before the change it takes about 6 × (914 ms); after the change about 2 × (310 ms). - `auth-session-route.test.ts`: the fake database binds each `returning()` to its own insert, and the test matches inserts by content instead of position, because the instance-settings insert can now happen in parallel with the tenant sync. ## Verification - `cd server && npx vitest run src/__tests__/cloud-tenant-actor-round-trips.test.ts` — red on `origin/master` (914 ms, 913 ms, 916 ms), green with the change (310 ms). - `cd server && npx vitest run src/__tests__/cloud-tenant-company-provisioning.test.ts src/__tests__/auth-session-route.test.ts src/__tests__/liveness.test.ts src/__tests__/cloud-warm-standby.test.ts src/middleware/cloud-tenant-actor.test.ts` - `pnpm verify` (selected tests), and CI. - After deploy: compare `/favicon.ico` and `/api/cli-auth/me` client time with `/api/liveness` on staging. Expect the median gap to drop from about 3 database waits to about 1. ## Risks - Low risk. Authorization does not change: the same statements run and the request waits for all of them before it continues. - Each request now uses up to three pooled connections at the same time instead of one. The staging pool limit is 500, so this is not a concern there. Smaller pools reach their limit sooner under a burst. - The diff re-indents the purge-and-sync block. The statements in it do not change. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, browser, code execution). Extended reasoning on. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
a9d0946e64
commit
99a6daf5bc
4 files changed
+303
-111
No files matched your search
@@ -83,9 +83,11 @@ describe("actorMiddleware authenticated session profile", () => {
|
||||
const inserts: Array<{ values: Record<string, unknown> }> = [];
|
||||
const db = {
|
||||
insert: vi.fn(() => {
|
||||
let own: { values: Record<string, unknown> } | undefined;
|
||||
const chain = {
|
||||
values(values: Record<string, unknown>) {
|
||||
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 () => {
|
||||
|
||||
@@ -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<net.Socket>();
|
||||
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<void> }>((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<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | null = null;
|
||||
let directDb!: ReturnType<typeof createDb>;
|
||||
let slowDb!: ReturnType<typeof createDb>;
|
||||
let proxy!: Awaited<ReturnType<typeof startDelayProxy>>;
|
||||
|
||||
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);
|
||||
});
|
||||
+115
-92
@@ -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",
|
||||
};
|
||||
|
||||
@@ -30,6 +30,7 @@ function createFakeDb(options: {
|
||||
settingsRow?: Record<string, unknown> | null;
|
||||
selectThrows?: boolean;
|
||||
beforeWrite?: (table: unknown) => Promise<void>;
|
||||
beforeRead?: (table: unknown) => Promise<void>;
|
||||
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"),
|
||||
|
||||
Reference in new issue
Block a user