Skip to content

Commit 64c75cc

Browse files
amal66claude
amal66
and
claude
committed
feat: project role ladder + capability matrix over the org access branches
Give the three access branches a Drive-style role ladder instead of raw ok/isOwner flags: row owner → owner, shared_with email → editor, org owner/admin → manager, plain org member → viewer. A single capability matrix (lib/permissions.ts) maps roles to what routes may do — view, content.edit, docs.organize, structure.manage, members.manage, container.delete — and every project/document/review write route now declares the capability it needs instead of hand-rolling an owner check. This makes the ADR's 'org membership grants visibility, not ownership' promise real: plain org members are read-only (previously the org branch returned ok:true and most write routes gated on nothing beyond ok), and org owner/admins can curate content (manage folders, sharing, review structure) without being able to delete containers they don't own. Notable tightenings, all fail-closed: - folder rename/move/delete, doc-set/column edits on reviews, and clear-cells are manager+ (generalising the owner-only folder-delete gate that landed upstream in open-legal-products#193) - version pushes, edit resolution, chat, and review generation are editor+ (org viewers excluded) - project PATCH (metadata + sharing) is manager+, so org admins can manage without owning; project/review DELETE stays owner-only - GET /projects/:id and /people now go through checkProjectAccess (the roster previously 404'd for org members who could read the project) Detail responses expose access_role alongside is_owner so the client can render per-role affordances. can() is exhaustively unit-tested (role × capability), and route suites cover the new gates. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent fb90b47 commit 64c75cc

12 files changed

Lines changed: 435 additions & 76 deletions

File tree

‎backend/src/__tests__/integration/projectChat.routes.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ describe("POST /projects/:projectId/chat", () => {
103103
checkProjectAccess.mockResolvedValue({
104104
ok: true,
105105
isOwner: true,
106+
projectRole: "owner",
106107
project: { id: "p1", user_id: "u1", shared_with: null },
107108
});
108109
});

‎backend/src/__tests__/integration/projects.routes.test.ts‎

Lines changed: 74 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,12 +86,16 @@ vi.mock("../../middleware/auth", () => ({
8686

8787
// Every export of lib/access must be present — other routers (chat, documents,
8888
// downloads, tabular) import from it at app load.
89-
vi.mock("../../lib/access", () => ({
89+
vi.mock("../../lib/access", async (importOriginal) => ({
90+
...(await importOriginal<typeof import("../../lib/access")>()),
9091
checkProjectAccess: (...args: unknown[]) => checkProjectAccess(...args),
9192
ensureDocAccess: vi.fn(async () => ({ ok: true, isOwner: true })),
9293
ensureReviewAccess: vi.fn(async () => ({ ok: true, isOwner: true })),
9394
filterAccessibleDocumentIds: vi.fn(async (ids: string[]) => ids),
9495
listAccessibleProjectIds: vi.fn(async () => []),
96+
getOrgRole: vi.fn(async () => null),
97+
getPersonalOrgId: vi.fn(async () => null),
98+
resolveContentOrgId: vi.fn(async () => null),
9599
}));
96100

97101
// user router imports all four cleanup helpers at module load.
@@ -121,6 +125,7 @@ describe("projects.routes", () => {
121125
checkProjectAccess.mockResolvedValue({
122126
ok: true,
123127
isOwner: true,
128+
projectRole: "owner",
124129
project: { id: "p1", user_id: "u1", shared_with: null },
125130
});
126131
deleteUserProjects.mockResolvedValue(1);
@@ -261,6 +266,7 @@ describe("projects.routes", () => {
261266
});
262267

263268
it("returns 404 when the caller is neither owner nor shared", async () => {
269+
checkProjectAccess.mockResolvedValue({ ok: false });
264270
supabaseState.tables.projects = {
265271
data: {
266272
id: "p1",
@@ -276,7 +282,17 @@ describe("projects.routes", () => {
276282
expect(res.body.detail).toBe("Project not found");
277283
});
278284

279-
it("grants access to a shared member (is_owner false)", async () => {
285+
it("grants access to a shared member (is_owner false, editor role)", async () => {
286+
checkProjectAccess.mockResolvedValue({
287+
ok: true,
288+
isOwner: false,
289+
projectRole: "editor",
290+
project: {
291+
id: "p1",
292+
user_id: "someone-else",
293+
shared_with: ["u1@test.local"],
294+
},
295+
});
280296
supabaseState.tables.projects = {
281297
data: {
282298
id: "p1",
@@ -291,7 +307,11 @@ describe("projects.routes", () => {
291307
const res = await request(app).get("/projects/p1").set(...AUTH);
292308

293309
expect(res.status).toBe(200);
294-
expect(res.body).toMatchObject({ id: "p1", is_owner: false });
310+
expect(res.body).toMatchObject({
311+
id: "p1",
312+
is_owner: false,
313+
access_role: "editor",
314+
});
295315
});
296316

297317
it("returns 200 with documents/folders/is_owner when owned", async () => {
@@ -320,6 +340,57 @@ describe("projects.routes", () => {
320340
});
321341
});
322342

343+
// ── DELETE /projects/:projectId/folders/:folderId (role ladder) ──────
344+
// Folder deletion cascades into nested documents, so it is manager+:
345+
// owner and org owner/admin pass; shared editors and org viewers do not.
346+
describe("DELETE /projects/:projectId/folders/:folderId", () => {
347+
const roleAccess = (projectRole: string) => ({
348+
ok: true,
349+
isOwner: projectRole === "owner",
350+
projectRole,
351+
project: { id: "p1", user_id: "u1", shared_with: null },
352+
});
353+
354+
beforeEach(() => {
355+
supabaseState.tables.project_subfolders = {
356+
data: [{ id: "f1", parent_folder_id: null }],
357+
error: null,
358+
};
359+
supabaseState.tables.documents = { data: [], error: null };
360+
});
361+
362+
it("allows the owner (204)", async () => {
363+
const res = await request(app)
364+
.delete("/projects/p1/folders/f1")
365+
.set(...AUTH);
366+
expect(res.status).toBe(204);
367+
});
368+
369+
it("allows an org owner/admin (manager) (204)", async () => {
370+
checkProjectAccess.mockResolvedValue(roleAccess("manager"));
371+
const res = await request(app)
372+
.delete("/projects/p1/folders/f1")
373+
.set(...AUTH);
374+
expect(res.status).toBe(204);
375+
});
376+
377+
it("blocks a shared editor (404)", async () => {
378+
checkProjectAccess.mockResolvedValue(roleAccess("editor"));
379+
const res = await request(app)
380+
.delete("/projects/p1/folders/f1")
381+
.set(...AUTH);
382+
expect(res.status).toBe(404);
383+
});
384+
385+
it("blocks a plain org member (viewer) (404)", async () => {
386+
checkProjectAccess.mockResolvedValue(roleAccess("viewer"));
387+
const res = await request(app)
388+
.delete("/projects/p1/folders/f1")
389+
.set(...AUTH);
390+
expect(res.status).toBe(404);
391+
});
392+
});
393+
323394
// ── GET /projects/:projectId/documents (checkProjectAccess guard) ─────
324395
describe("GET /projects/:projectId/documents", () => {
325396
it("returns 404 when checkProjectAccess denies access", async () => {

‎backend/src/__tests__/integration/tabular.routes.test.ts‎

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -97,13 +97,17 @@ vi.mock("../../middleware/auth", () => ({
9797
next(),
9898
}));
9999

100-
vi.mock("../../lib/access", () => ({
100+
vi.mock("../../lib/access", async (importOriginal) => ({
101+
...(await importOriginal<typeof import("../../lib/access")>()),
101102
ensureReviewAccess: (...args: unknown[]) => ensureReviewAccess(...args),
102103
checkProjectAccess: (...args: unknown[]) => checkProjectAccess(...args),
103104
filterAccessibleDocumentIds: (...args: unknown[]) =>
104105
filterAccessibleDocumentIds(...args),
105106
ensureDocAccess: vi.fn(async () => ({ ok: true, isOwner: true })),
106107
listAccessibleProjectIds: vi.fn(async () => []),
108+
getOrgRole: vi.fn(async () => null),
109+
getPersonalOrgId: vi.fn(async () => null),
110+
resolveContentOrgId: vi.fn(async () => null),
107111
}));
108112

109113
vi.mock("../../lib/userSettings", () => ({
@@ -128,10 +132,15 @@ describe("tabular.routes", () => {
128132
vi.clearAllMocks();
129133
resetSupabaseState();
130134
// Default: caller is the owner with full access.
131-
ensureReviewAccess.mockResolvedValue({ ok: true, isOwner: true });
135+
ensureReviewAccess.mockResolvedValue({
136+
ok: true,
137+
isOwner: true,
138+
projectRole: "owner",
139+
});
132140
checkProjectAccess.mockResolvedValue({
133141
ok: true,
134142
isOwner: true,
143+
projectRole: "owner",
135144
project: { id: "p1", user_id: "u1", shared_with: null },
136145
});
137146
// Default: every requested doc is accessible (identity passthrough).
@@ -352,20 +361,24 @@ describe("tabular.routes", () => {
352361
expect(res.body.detail).toBe("Review not found");
353362
});
354363

355-
it("returns 403 when a non-owner edits columns_config", async () => {
364+
it("returns 403 when an editor (shared member) edits columns_config", async () => {
356365
supabaseState.tables.tabular_reviews = {
357366
data: { id: "r1", user_id: "other", project_id: "p1" },
358367
error: null,
359368
};
360-
ensureReviewAccess.mockResolvedValue({ ok: true, isOwner: false });
369+
ensureReviewAccess.mockResolvedValue({
370+
ok: true,
371+
isOwner: false,
372+
projectRole: "editor",
373+
});
361374

362375
const res = await request(app)
363376
.patch("/tabular-review/r1")
364377
.set(...AUTH)
365378
.send({ columns_config: [{ index: 0, name: "X", prompt: "p" }] });
366379

367380
expect(res.status).toBe(403);
368-
expect(res.body.detail).toBe("Only the review owner can change columns");
381+
expect(res.body.detail).toBe("Only a review manager can change columns");
369382
});
370383
});
371384

@@ -424,6 +437,26 @@ describe("tabular.routes", () => {
424437
expect(res.body.detail).toBe("Review not found");
425438
});
426439

440+
it("returns 403 for an editor — clearing cells is manager+", async () => {
441+
supabaseState.tables.tabular_reviews = {
442+
data: { id: "r1", user_id: "other", project_id: "p1" },
443+
error: null,
444+
};
445+
ensureReviewAccess.mockResolvedValue({
446+
ok: true,
447+
isOwner: false,
448+
projectRole: "editor",
449+
});
450+
451+
const res = await request(app)
452+
.post("/tabular-review/r1/clear-cells")
453+
.set(...AUTH)
454+
.send({ document_ids: ["d1"] });
455+
456+
expect(res.status).toBe(403);
457+
expect(res.body.detail).toBe("Only a review manager can clear cells");
458+
});
459+
427460
it("returns 204 on success", async () => {
428461
supabaseState.tables.tabular_reviews = {
429462
data: { id: "r1", user_id: "u1", project_id: null },

‎backend/src/lib/__tests__/access.test.ts‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -199,25 +199,51 @@ describe("org RBAC access", () => {
199199
],
200200
});
201201

202-
it("grants an org member read access without ownership", async () => {
202+
it("grants an org member read access without ownership (viewer)", async () => {
203203
await expect(
204204
checkProjectAccess("proj-a", "carol", "carol@example.com", db),
205205
).resolves.toMatchObject({
206206
ok: true,
207207
isOwner: false,
208208
role: "member",
209209
canManage: false,
210+
projectRole: "viewer",
210211
});
211212
});
212213

213-
it("marks org owners/admins as able to manage", async () => {
214+
it("marks org owners/admins as able to manage (manager)", async () => {
214215
await expect(
215216
checkProjectAccess("proj-a", "dave", "dave@example.com", db),
216217
).resolves.toMatchObject({
217218
ok: true,
218219
isOwner: false,
219220
role: "admin",
220221
canManage: true,
222+
projectRole: "manager",
223+
});
224+
});
225+
226+
it("derives owner and editor roles on the non-org branches", async () => {
227+
await expect(
228+
checkProjectAccess("proj-a", "alice", "alice@example.com", db),
229+
).resolves.toMatchObject({ ok: true, projectRole: "owner" });
230+
231+
const sharedDb = makeDb({
232+
projects: [
233+
{
234+
id: "proj-s",
235+
user_id: "alice",
236+
shared_with: ["eve@example.com"],
237+
org_id: null,
238+
},
239+
],
240+
});
241+
await expect(
242+
checkProjectAccess("proj-s", "eve", "eve@example.com", sharedDb),
243+
).resolves.toMatchObject({
244+
ok: true,
245+
isOwner: false,
246+
projectRole: "editor",
221247
});
222248
});
223249

‎backend/src/lib/__tests__/permissions.test.ts‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import { describe, expect, it } from "vitest";
2+
import { can, type Capability, type ProjectRole } from "../permissions";
3+
4+
// The full role × capability matrix, asserted cell by cell so any edit to
5+
// the policy table is a visible diff here.
6+
const EXPECTED: Record<ProjectRole, Record<Capability, boolean>> = {
7+
viewer: {
8+
"project.view": true,
9+
"content.edit": false,
10+
"docs.organize": false,
11+
"structure.manage": false,
12+
"members.manage": false,
13+
"container.delete": false,
14+
},
15+
editor: {
16+
"project.view": true,
17+
"content.edit": true,
18+
"docs.organize": true,
19+
"structure.manage": false,
20+
"members.manage": false,
21+
"container.delete": false,
22+
},
23+
manager: {
24+
"project.view": true,
25+
"content.edit": true,
26+
"docs.organize": true,
27+
"structure.manage": true,
28+
"members.manage": true,
29+
"container.delete": false,
30+
},
31+
owner: {
32+
"project.view": true,
33+
"content.edit": true,
34+
"docs.organize": true,
35+
"structure.manage": true,
36+
"members.manage": true,
37+
"container.delete": true,
38+
},
39+
};
40+
41+
describe("permissions matrix", () => {
42+
for (const [role, caps] of Object.entries(EXPECTED)) {
43+
for (const [capability, allowed] of Object.entries(caps)) {
44+
it(`${role} ${allowed ? "can" : "cannot"} ${capability}`, () => {
45+
expect(
46+
can(role as ProjectRole, capability as Capability),
47+
).toBe(allowed);
48+
});
49+
}
50+
}
51+
52+
it("fails closed on missing or unknown roles", () => {
53+
expect(can(null, "project.view")).toBe(false);
54+
expect(can(undefined, "project.view")).toBe(false);
55+
expect(can("admin" as ProjectRole, "project.view")).toBe(false);
56+
});
57+
});

0 commit comments

Comments
 (0)