Let cloud tenant sessions reach companies they hold memberships in (#10524)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - On Paperclip Cloud, each stack authenticates its users to the tenant app through trusted headers (`resolveCloudTenantActor`), which seed a primary company for the stack > - That actor was pinned to exactly one company — the seeded primary — regardless of any other companies the user actually holds a membership in > - Companies created later (via the import flow, or company creation) write real membership rows for the user, but the pinned actor ignored them, so those companies showed up in listings yet returned "User does not have access to this company" when opened > - This pull request unions the pinned primary with the user's own active membership rows, exactly as a locally authenticated session already does > - The benefit is that a Cloud user can reach every company they belong to — most visibly, a company they just imported ## Linked Issues or Issue Description - Refs #10507 (Import/Export — imported companies were unreachable on Cloud stacks). No open issue; bug described above (companies visible in listing but unreachable; expected: reachable when the user holds an active membership). ## What Changed - Extracted the session path's own active-membership query into `loadActiveUserCompanyMemberships(db, userId)` (single-sourced; the session path now calls it too). - `resolveCloudTenantActor` unions its result with the pinned primary: `companyIds = [primary, ...others]`, memberships likewise, primary first. Strictly per-user; a membership-read failure degrades to primary-only (mirrors the existing fail-closed owner-elevation pattern). No change to owner instance-admin elevation, grant seeding, the stale instance-admin purge, or trusted-header validation. - Grants are seeded at membership creation across all flows (company create, invite/join, import), not per request — so no extra seeding was added here. ## Verification - `@paperclipai/server` typecheck clean. - `cloud-tenant-actor.test.ts` (+ union / other-user-excluded / inactive-excluded / no-rows-identical cases), `auth-session-route.test.ts` (route-level: trusted headers reach a unioned company through `assertCompanyAccess`), plus agent-auth, authz-company-access, cross-company-authz, portability-routes — 83 tests green. ## Risks - Low and tightly scoped: only widens a Cloud actor's reachable companies to those it already holds active memberships in; users without extra memberships, other users' rows, and owner elevation are all unaffected. Read failure fails closed to primary-only. ## Model Used - Claude Fable 5 (`claude-fable-5`, Anthropic), Claude Code CLI, extended thinking + tool use. ## 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 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
This commit is contained in:
Devin Foley
GitHub
parent
a5d252d058
commit
1c52f02d34
|
|
@ -1,8 +1,10 @@
|
|||
import express from "express";
|
||||
import request from "supertest";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { instanceUserRoles } from "@paperclipai/db";
|
||||
import { companyMemberships, instanceUserRoles } from "@paperclipai/db";
|
||||
import { actorMiddleware } from "../middleware/auth.js";
|
||||
import { errorHandler } from "../middleware/error-handler.js";
|
||||
import { assertCompanyAccess } from "../routes/authz.js";
|
||||
|
||||
function createSelectChain(rows: unknown[]) {
|
||||
return {
|
||||
|
|
@ -138,6 +140,83 @@ describe("actorMiddleware authenticated session profile", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("lets the cloud tenant actor through assertCompanyAccess for a company it holds a membership row in", async () => {
|
||||
process.env.PAPERCLIP_CLOUD_TENANT_SERVER_TOKEN = "tenant-token";
|
||||
// A company created on the instance after provisioning (e.g. by a company
|
||||
// import) — the user has a real membership row, but it is not the stack's
|
||||
// seeded primary company.
|
||||
const importedCompanyId = "33333333-3333-4333-8333-333333333333";
|
||||
const unrelatedCompanyId = "44444444-4444-4444-8444-444444444444";
|
||||
const insertChain = {
|
||||
values() {
|
||||
return insertChain;
|
||||
},
|
||||
onConflictDoUpdate() {
|
||||
return insertChain;
|
||||
},
|
||||
onConflictDoNothing() {
|
||||
return insertChain;
|
||||
},
|
||||
returning() {
|
||||
return Promise.resolve([{ companyId: "company-1", membershipRole: "member", status: "active" }]);
|
||||
},
|
||||
then(resolve: (value: unknown) => unknown) {
|
||||
return Promise.resolve(undefined).then(resolve);
|
||||
},
|
||||
};
|
||||
const db = {
|
||||
select: vi.fn(() => ({
|
||||
from: (table: unknown) => ({
|
||||
where: () =>
|
||||
Promise.resolve(
|
||||
table === companyMemberships
|
||||
? [{ companyId: importedCompanyId, membershipRole: "member", status: "active" }]
|
||||
: [],
|
||||
),
|
||||
}),
|
||||
})),
|
||||
insert: vi.fn(() => insertChain),
|
||||
delete: vi.fn(() => ({ where: () => Promise.resolve(undefined) })),
|
||||
} as any;
|
||||
const app = express();
|
||||
app.use(
|
||||
actorMiddleware(db, {
|
||||
deploymentMode: "authenticated",
|
||||
resolveSession: async () => null,
|
||||
}),
|
||||
);
|
||||
app.get("/companies/:companyId/resource", (req, res) => {
|
||||
assertCompanyAccess(req, req.params.companyId);
|
||||
res.json({ ok: true, companyIds: req.actor.companyIds });
|
||||
});
|
||||
app.post("/companies/:companyId/resource", (req, res) => {
|
||||
assertCompanyAccess(req, req.params.companyId);
|
||||
res.json({ ok: true });
|
||||
});
|
||||
app.use(errorHandler);
|
||||
|
||||
const cloudHeaders = {
|
||||
"x-paperclip-cloud-tenant-token": "tenant-token",
|
||||
"x-paperclip-cloud-user-id": "global-user-1",
|
||||
"x-paperclip-cloud-user-email": "owner@example.com",
|
||||
"x-paperclip-cloud-stack-id": "stack-alpha",
|
||||
"x-paperclip-cloud-stack-role": "member",
|
||||
};
|
||||
|
||||
// Reads and writes both reach the imported company through the real
|
||||
// membership row (write access also consults actor.memberships).
|
||||
const read = await request(app).get(`/companies/${importedCompanyId}/resource`).set(cloudHeaders);
|
||||
expect(read.status).toBe(200);
|
||||
expect(read.body.companyIds).toContain(importedCompanyId);
|
||||
|
||||
const write = await request(app).post(`/companies/${importedCompanyId}/resource`).set(cloudHeaders);
|
||||
expect(write.status).toBe(200);
|
||||
|
||||
// Companies the user holds no membership row in stay unreachable.
|
||||
const denied = await request(app).get(`/companies/${unrelatedCompanyId}/resource`).set(cloudHeaders);
|
||||
expect(denied.status).toBe(403);
|
||||
});
|
||||
|
||||
it("purges a stale instance_admin row so the session path stops elevating the cloud-tenant user", async () => {
|
||||
process.env.PAPERCLIP_CLOUD_TENANT_SERVER_TOKEN = "tenant-token";
|
||||
// Simulates a deployment that previously ran the pre-hardening cloud_tenant
|
||||
|
|
|
|||
|
|
@ -78,6 +78,28 @@ async function loadResponsibleUserMemberships(
|
|||
return user ? memberships : [];
|
||||
}
|
||||
|
||||
/**
|
||||
* The user's own active company memberships — the exact company scope a
|
||||
* locally authenticated session actor carries. Shared by the session path
|
||||
* and the Cloud trusted-header path so both resolve the same access set.
|
||||
*/
|
||||
async function loadActiveUserCompanyMemberships(db: Db, userId: string) {
|
||||
return db
|
||||
.select({
|
||||
companyId: companyMemberships.companyId,
|
||||
membershipRole: companyMemberships.membershipRole,
|
||||
status: companyMemberships.status,
|
||||
})
|
||||
.from(companyMemberships)
|
||||
.where(
|
||||
and(
|
||||
eq(companyMemberships.principalType, "user"),
|
||||
eq(companyMemberships.principalId, userId),
|
||||
eq(companyMemberships.status, "active"),
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
async function auditAgentJwtRunHeaderMismatch(
|
||||
db: Db,
|
||||
input: { companyId: string; agentId: string; claimRunId: string; headerRunId: string; method: string; url: string },
|
||||
|
|
@ -185,20 +207,7 @@ export function actorMiddleware(db: Db, opts: ActorMiddlewareOptions): RequestHa
|
|||
.from(instanceUserRoles)
|
||||
.where(and(eq(instanceUserRoles.userId, userId), eq(instanceUserRoles.role, "instance_admin")))
|
||||
.then((rows) => rows[0] ?? null),
|
||||
db
|
||||
.select({
|
||||
companyId: companyMemberships.companyId,
|
||||
membershipRole: companyMemberships.membershipRole,
|
||||
status: companyMemberships.status,
|
||||
})
|
||||
.from(companyMemberships)
|
||||
.where(
|
||||
and(
|
||||
eq(companyMemberships.principalType, "user"),
|
||||
eq(companyMemberships.principalId, userId),
|
||||
eq(companyMemberships.status, "active"),
|
||||
),
|
||||
),
|
||||
loadActiveUserCompanyMemberships(db, userId),
|
||||
]);
|
||||
req.actor = {
|
||||
type: "board",
|
||||
|
|
@ -518,17 +527,42 @@ export async function resolveCloudTenantActor(db: Db, req: Request): Promise<Exp
|
|||
grantedByUserId: null,
|
||||
});
|
||||
|
||||
// The stack's seeded company is only where Cloud provisioned this user.
|
||||
// Companies created afterwards on the instance (imports, in-app company
|
||||
// creation) attach real membership rows for the user, so union those with
|
||||
// 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,
|
||||
);
|
||||
} catch (err) {
|
||||
logger.warn(
|
||||
{ err, userId, stackId },
|
||||
"Failed to load cloud tenant user's company memberships; scoping actor to the stack's primary company",
|
||||
);
|
||||
}
|
||||
|
||||
return {
|
||||
type: "board",
|
||||
userId,
|
||||
userName,
|
||||
userEmail,
|
||||
companyIds: [companyId],
|
||||
memberships: [{
|
||||
companyId,
|
||||
membershipRole: membership.membershipRole,
|
||||
status: membership.status,
|
||||
}],
|
||||
companyIds: [companyId, ...additionalMemberships.map((row) => row.companyId)],
|
||||
memberships: [
|
||||
{
|
||||
companyId,
|
||||
membershipRole: membership.membershipRole,
|
||||
status: membership.status,
|
||||
},
|
||||
...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
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
import type { Request } from "express";
|
||||
import { and, eq } from "drizzle-orm";
|
||||
import type { Db } from "@paperclipai/db";
|
||||
import { authUsers, companies, companyMemberships, instanceSettings, instanceUserRoles } from "@paperclipai/db";
|
||||
import { resolveCloudTenantActor } from "./auth.js";
|
||||
|
|
@ -7,11 +8,14 @@ import { resolveCloudTenantActor } from "./auth.js";
|
|||
// Minimal fake Drizzle Db: records every table passed to .insert() / .delete() and
|
||||
// supports the chained call shapes used by resolveCloudTenantActor (values /
|
||||
// onConflictDo* / returning().then() / delete().where()), plus the
|
||||
// select().from(instanceSettings).where().then() read the owner-elevation flag
|
||||
// resolution performs through instanceSettingsService. The chain is awaitable so
|
||||
// select().from(table).where() reads: instanceSettings for the owner-elevation
|
||||
// flag resolution through instanceSettingsService, and companyMemberships for
|
||||
// the user's own membership rows (rows configurable via membershipQueryRows,
|
||||
// where-conditions captured in selectWheres). The chain is awaitable so
|
||||
// directly-awaited statements resolve.
|
||||
function createFakeDb(options: {
|
||||
membershipRow?: { companyId: string; membershipRole: string; status: string };
|
||||
membershipQueryRows?: Array<{ companyId: string; membershipRole: string | null; status: string }>;
|
||||
settingsRow?: Record<string, unknown> | null;
|
||||
selectThrows?: boolean;
|
||||
} = {}) {
|
||||
|
|
@ -31,6 +35,7 @@ function createFakeDb(options: {
|
|||
: options.settingsRow;
|
||||
const insertedTables: unknown[] = [];
|
||||
const deletedTables: unknown[] = [];
|
||||
const selectWheres: Array<{ table: unknown; condition: unknown }> = [];
|
||||
const chain: Record<string, unknown> = {};
|
||||
chain.values = () => chain;
|
||||
chain.onConflictDoUpdate = () => chain;
|
||||
|
|
@ -51,15 +56,23 @@ function createFakeDb(options: {
|
|||
if (options.selectThrows) throw new Error("select unavailable");
|
||||
return {
|
||||
from: (table: unknown) => ({
|
||||
where: () => ({
|
||||
then: (resolve: (v: unknown) => unknown) =>
|
||||
Promise.resolve(table === instanceSettings && settingsRow ? [settingsRow] : []).then(resolve),
|
||||
}),
|
||||
where: (condition: unknown) => {
|
||||
selectWheres.push({ table, condition });
|
||||
const rows =
|
||||
table === instanceSettings && settingsRow
|
||||
? [settingsRow]
|
||||
: table === companyMemberships
|
||||
? (options.membershipQueryRows ?? [])
|
||||
: [];
|
||||
return {
|
||||
then: (resolve: (v: unknown) => unknown) => Promise.resolve(rows).then(resolve),
|
||||
};
|
||||
},
|
||||
}),
|
||||
};
|
||||
},
|
||||
} as unknown as Db;
|
||||
return { db, insertedTables, deletedTables };
|
||||
return { db, insertedTables, deletedTables, selectWheres };
|
||||
}
|
||||
|
||||
function settingsRowWith(experimental: Record<string, unknown>) {
|
||||
|
|
@ -121,7 +134,7 @@ describe("resolveCloudTenantActor (shared-pool hardening)", () => {
|
|||
expect(insertedTables).not.toContain(instanceUserRoles);
|
||||
});
|
||||
|
||||
it("is scoped to exactly the one company from its stack", async () => {
|
||||
it("stays scoped to exactly the stack's company when the user has no other membership rows", async () => {
|
||||
const { db } = createFakeDb();
|
||||
const actor = await resolveCloudTenantActor(db, fakeReq(VALID_HEADERS));
|
||||
expect(actor!.companyIds).toHaveLength(1);
|
||||
|
|
@ -165,6 +178,59 @@ describe("resolveCloudTenantActor (shared-pool hardening)", () => {
|
|||
expect(actor?.memberships?.[0]?.membershipRole).toBe("member");
|
||||
});
|
||||
|
||||
describe("company membership union", () => {
|
||||
// The primary company id is derived from the stack id; resolve it once
|
||||
// through the same code path instead of duplicating the hash here.
|
||||
async function resolvePrimaryCompanyId() {
|
||||
const { db } = createFakeDb();
|
||||
const actor = await resolveCloudTenantActor(db, fakeReq(VALID_HEADERS));
|
||||
return actor!.companyIds![0]!;
|
||||
}
|
||||
|
||||
it("unions the pinned primary with the user's own active membership rows, primary first and deduped", async () => {
|
||||
const primaryCompanyId = await resolvePrimaryCompanyId();
|
||||
const { db } = createFakeDb({
|
||||
membershipQueryRows: [
|
||||
// The user's own row for the primary company comes back from the
|
||||
// query too — it must not appear twice.
|
||||
{ companyId: primaryCompanyId, membershipRole: "owner", status: "active" },
|
||||
{ companyId: "company-imported", membershipRole: "member", status: "active" },
|
||||
],
|
||||
});
|
||||
const actor = await resolveCloudTenantActor(db, fakeReq(VALID_HEADERS));
|
||||
expect(actor!.companyIds).toEqual([primaryCompanyId, "company-imported"]);
|
||||
expect(actor!.memberships).toEqual([
|
||||
{ companyId: primaryCompanyId, membershipRole: "owner", status: "active" },
|
||||
{ companyId: "company-imported", membershipRole: "member", status: "active" },
|
||||
]);
|
||||
});
|
||||
|
||||
it("loads memberships with the session path's exact own-user active-status filter", async () => {
|
||||
const { db, selectWheres } = createFakeDb();
|
||||
await resolveCloudTenantActor(db, fakeReq(VALID_HEADERS));
|
||||
const membershipSelects = selectWheres.filter((entry) => entry.table === companyMemberships);
|
||||
expect(membershipSelects).toHaveLength(1);
|
||||
// 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[0]!.condition).toEqual(
|
||||
and(
|
||||
eq(companyMemberships.principalType, "user"),
|
||||
eq(companyMemberships.principalId, "user-123"),
|
||||
eq(companyMemberships.status, "active"),
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
it("degrades to the primary company when the membership read fails", async () => {
|
||||
const { db } = createFakeDb({ selectThrows: true });
|
||||
const actor = await resolveCloudTenantActor(db, fakeReq(VALID_HEADERS));
|
||||
expect(actor).not.toBeNull();
|
||||
expect(actor!.companyIds).toHaveLength(1);
|
||||
expect(actor!.memberships).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe("owner instance-admin elevation (enableOwnerInstanceAdmin)", () => {
|
||||
it("elevates the owner while the flag is enabled, still without any role row", async () => {
|
||||
const { db, insertedTables, deletedTables } = createFakeDb({
|
||||
|
|
|
|||
Loading…
Reference in New Issue