diff --git a/docs/project-lifecycle.md b/docs/project-lifecycle.md index edf3bc59..2f44f0fd 100644 --- a/docs/project-lifecycle.md +++ b/docs/project-lifecycle.md @@ -6,12 +6,12 @@ ## Status -| # | Slice | Size | State | -| --- | --- | --- | --- | -| 1 | `GET /v1/projects` — list | S | in progress | -| 2 | `POST /v1/projects` — create | M | not started | -| 3 | Web: switcher + create dialog | M | not started | -| 4 | Org switching (follow-up) | M–L | not started, blocked on v1.45.0 adoption | +| # | Slice | Size | State | +| --- | ----------------------------- | ---- | ---------------------------------------- | +| 1 | `GET /v1/projects` — list | S | in progress | +| 2 | `POST /v1/projects` — create | M | not started | +| 3 | Web: switcher + create dialog | M | not started | +| 4 | Org switching (follow-up) | M–L | not started, blocked on v1.45.0 adoption | ## The gap in one line @@ -19,16 +19,16 @@ Everything below the UI is already multi-project. What's missing is the ability ## What already works — do not rebuild -| Layer | State | -| --- | --- | -| `Project` model | Multi-project ready: `slug` with `@@unique([organizationId, slug])`, `createdByUserId`, org FK | -| `ProjectAccess` | User + group bindings, `owner`/`member` role, cascade deletes | -| Ownership of data | `Agent`, `Secret`, `AppConnection`, `PolicyRuleV2`, `ApiKey`, `Budget`, `AuditLog` all FK to `Project` | -| Gateway | Fully project-aware — `agent.project_id` drives secrets, connections and policy resolution | -| API keys | Already `scope: "project" | "organization"` with a `projectId` FK | -| **Switching (transport)** | `resolveProjectId` already honours `x-project-id`, gated by `canAccessProjectAsUser` | -| **Switching (authz)** | `canAccessProjectAsUser` + `hasProjectBinding` — built and tested in #13 | -| Management | `renameProject`, `deleteProject`, `listProjectAccess`, `setProjectAccess` — all shipped | +| Layer | State | +| ------------------------- | ------------------------------------------------------------------------------------------------------ | +| `Project` model | Multi-project ready: `slug` with `@@unique([organizationId, slug])`, `createdByUserId`, org FK | +| `ProjectAccess` | User + group bindings, `owner`/`member` role, cascade deletes | +| Ownership of data | `Agent`, `Secret`, `AppConnection`, `PolicyRuleV2`, `ApiKey`, `Budget`, `AuditLog` all FK to `Project` | +| Gateway | Fully project-aware — `agent.project_id` drives secrets, connections and policy resolution | +| API keys | Already `scope: "project" \| "organization"` with a `projectId` FK | +| **Switching (transport)** | `resolveProjectId` already honours `x-project-id`, gated by `canAccessProjectAsUser` | +| **Switching (authz)** | `canAccessProjectAsUser` + `hasProjectBinding` — built and tested in #13 | +| Management | `renameProject`, `deleteProject`, `listProjectAccess`, `setProjectAccess` — all shipped | Multi-project is not a new capability. It is already happening: `ensureMemberDefaultProject` creates one project per invited member, which is exactly why `projectNameSchema` is deliberately non-unique per org. @@ -51,11 +51,11 @@ Multi-project is not a new capability. It is already happening: `ensureMemberDef `proxy.ts` derives project from a `/p/` path prefix, but **strips that prefix entirely when `!CAPS.orgScopedUI`** — which covers `oss` and `onprem-slim`. On those editions the only ways to set project context today are the `?projectId=` query bridge (scoped to `/app-connect`) and the `findUserDefaultProject` fallback. **A switcher on OSS currently has no transport.** -| Option | Assessment | -| --- | --- | -| **A. Default-project cookie**, read by `proxy.ts` → `x-project-id` | **Recommended.** Keeps flat editions flat (no URL change), works on every edition, and mirrors a precedent upstream is *actively building*: `DEFAULT_ORG_COOKIE = "onecli-default-org"` plus `readDefaultOrgCookie()` land in `navigation.ts` in **v1.45.0** (absent from our v1.44.0 base). Define `onecli-default-project` alongside it in the same file, honouring upstream's own comment there — *"One definition so writer and reader can't drift."* Server still validates via `canAccessProjectAsUser`, so the cookie is a hint, never authority. **Note the extension**: upstream's cookie is read client-side only (`document.cookie`); ours must also be read server-side in `proxy.ts` to become transport. | -| B. Enable `/p/` namespacing on flat editions | Changes every URL on OSS, touches proxy + nav + every link. Large blast radius for a feature that doesn't need it. | -| C. Extend the `?projectId=` query bridge | Ugly to carry across navigations; the bridge exists for a popup with no path, not general use. | +| Option | Assessment | +| ------------------------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| **A. Default-project cookie**, read by `proxy.ts` → `x-project-id` | **Recommended.** Keeps flat editions flat (no URL change), works on every edition, and mirrors a precedent upstream is _actively building_: `DEFAULT_ORG_COOKIE = "onecli-default-org"` plus `readDefaultOrgCookie()` land in `navigation.ts` in **v1.45.0** (absent from our v1.44.0 base). Define `onecli-default-project` alongside it in the same file, honouring upstream's own comment there — _"One definition so writer and reader can't drift."_ Server still validates via `canAccessProjectAsUser`, so the cookie is a hint, never authority. **Note the extension**: upstream's cookie is read client-side only (`document.cookie`); ours must also be read server-side in `proxy.ts` to become transport. | +| B. Enable `/p/` namespacing on flat editions | Changes every URL on OSS, touches proxy + nav + every link. Large blast radius for a feature that doesn't need it. | +| C. Extend the `?projectId=` query bridge | Ugly to carry across navigations; the bridge exists for a popup with no path, not general use. | ### 2. Does `GET /` match upstream's contract? @@ -74,7 +74,7 @@ Options: any active org member, or admins only. **Recommend any active member**, ## Invariants that must not break 1. **`findUserDefaultProject` and `hasResolvableProjectExcluding` MUST stay in sync.** The comment in `organization-service.ts` is emphatic: drift is a lockout — a user whose last project is deleted resolves no project and session auth 401s them everywhere. Adding create/list touches project resolution; both predicates must be re-checked together. -2. **Default-project resolution is stable under creation.** Arm 1 is "oldest project you created" (`orderBy: createdAt asc`), so creating a second project does not silently move your default. Preserve that — and note it means there is currently no way to *change* your default, which Decision 1's cookie would effectively provide. +2. **Default-project resolution is stable under creation.** Arm 1 is "oldest project you created" (`orderBy: createdAt asc`), so creating a second project does not silently move your default. Preserve that — and note it means there is currently no way to _change_ your default, which Decision 1's cookie would effectively provide. 3. **Guard G — a project must always keep an owner.** Creation must seed the creator's `owner` binding in the same transaction, or a freshly created project is immediately unmanageable. 4. **Slug uniqueness per org.** `@@unique([organizationId, slug])` — creation needs slug generation with collision handling (`slugify` already exists in `organization-service.ts`). A P2002 race must surface as a 409, matching the group-create precedent. 5. **Audit + gateway invalidation.** Per `CLAUDE.md`, create must use `withAudit`. `withAudit` handles gateway cache invalidation when `organizationId`/`projectId` are present. @@ -117,17 +117,17 @@ if (projectId) { organizationId = resolveOrganizationId(request, userId) // header only reached here ``` -And `resolveProjectId` falls back to `findUserDefaultProject` whenever `CAPS.tenancy !== "multi-org"`. On `oss` (`org-per-user`) and `onprem-slim` (`single-org-shared`) a project therefore *always* resolves — so **`x-organization-id` is dead on flat editions.** Org switching there is not a missing-UI problem; the resolution order defeats it. On `cloud` (`multi-org`) it already works, because no project header means `resolveProjectId` returns null and the org header gets its turn. +And `resolveProjectId` falls back to `findUserDefaultProject` whenever `CAPS.tenancy !== "multi-org"`. On `oss` (`org-per-user`) and `onprem-slim` (`single-org-shared`) a project therefore _always_ resolves — so **`x-organization-id` is dead on flat editions.** Org switching there is not a missing-UI problem; the resolution order defeats it. On `cloud` (`multi-org`) it already works, because no project header means `resolveProjectId` returns null and the org header gets its turn. This is why org switching is a genuine follow-up rather than a sibling of the project work: it changes the most security-sensitive resolution path in the app. ### How to fix it -| Option | Assessment | -| --- | --- | -| **A. Make the default-project fallback org-aware** — `findUserDefaultProject(userId, preferredOrgId?)`, with `preferredOrgId` from the org cookie/header | **Recommended.** Switching org then means "land on my default project *within that org*." Precedence is untouched, `x-organization-id` stops being dead without inverting anything, and org and project can never disagree because org is still derived from the winning project. Falls back to today's unfiltered behaviour when the selected org has no reachable project, so the lockout invariant holds. | -| B. Invert precedence — org header wins, project must belong to it | Touches the lockout invariant and the resolution path every request depends on. Also introduces a genuine mismatch class: `resolveProjectId` validates a project against *any* org the user belongs to, not the selected one, so a stale project header from org A would need explicit rejection under org B. | -| C. No org switcher — switch org implicitly by picking a project in it | Zero new mechanism, and the project switcher (PR 3) already spans orgs. But it's poor UX for an org where you hold no project binding yet, and it leaves `x-organization-id` dead. Reasonable interim if PR 4 slips. | +| Option | Assessment | +| -------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| **A. Make the default-project fallback org-aware** — `findUserDefaultProject(userId, preferredOrgId?)`, with `preferredOrgId` from the org cookie/header | **Recommended.** Switching org then means "land on my default project _within that org_." Precedence is untouched, `x-organization-id` stops being dead without inverting anything, and org and project can never disagree because org is still derived from the winning project. Falls back to today's unfiltered behaviour when the selected org has no reachable project, so the lockout invariant holds. | +| B. Invert precedence — org header wins, project must belong to it | Touches the lockout invariant and the resolution path every request depends on. Also introduces a genuine mismatch class: `resolveProjectId` validates a project against _any_ org the user belongs to, not the selected one, so a stale project header from org A would need explicit rejection under org B. | +| C. No org switcher — switch org implicitly by picking a project in it | Zero new mechanism, and the project switcher (PR 3) already spans orgs. But it's poor UX for an org where you hold no project binding yet, and it leaves `x-organization-id` dead. Reasonable interim if PR 4 slips. | **Option A keeps the invariant that org is always derived from the resolved project** — which is what makes the current design coherent. Preserve it. @@ -137,7 +137,7 @@ This is why org switching is a genuine follow-up rather than a sibling of the pr ## Scope of PR 4 -1. `GET /v1/organizations` — list orgs where the caller is an *active* member (`activeMembershipWhere`), returning id/name/slug/role. New file `routes/org/organizations.ts`, or extend `organization-service.ts` with `listUserOrganizations`. +1. `GET /v1/organizations` — list orgs where the caller is an _active_ member (`activeMembershipWhere`), returning id/name/slug/role. New file `routes/org/organizations.ts`, or extend `organization-service.ts` with `listUserOrganizations`. 2. `findUserDefaultProject(userId, preferredOrgId?)` — Decision A. **`hasResolvableProjectExcluding` must be updated in lockstep** (invariant 1); the two predicates are required to agree. 3. `proxy.ts` — read the org cookie into `x-organization-id` on flat editions, mirroring the project cookie from PR 3. 4. Web — `api/organizations.ts`, `use-organizations` hook, org switcher in the dashboard nav (alongside the project switcher), writing the cookie on switch. diff --git a/packages/api/src/routes/org/projects.test.ts b/packages/api/src/routes/org/projects.test.ts index 9768de89..8a39e75f 100644 --- a/packages/api/src/routes/org/projects.test.ts +++ b/packages/api/src/routes/org/projects.test.ts @@ -451,6 +451,33 @@ vi.mock("@onecli/db", () => { } return picked; }, + findMany: async ({ + where, + select, + }: { + where: ProjectWhere; + select?: Record; + orderBy?: unknown; + }) => { + // Always `createdAt asc, id asc` — the only ordering `listProjects` + // asks for, and the same one `findUserDefaultProject` uses. + const rows = store.projects + .slice() + .sort( + (a, b) => + a.createdAt.getTime() - b.createdAt.getTime() || + a.id.localeCompare(b.id), + ) + .filter((p) => matchesProject(p, where)); + if (!select) return rows.map((r) => ({ ...r })); + return rows.map((row) => { + const picked: Record = {}; + for (const key of Object.keys(select)) { + if (select[key]) picked[key] = row[key as keyof ProjectRow]; + } + return picked; + }); + }, count: async ({ where }: { where: ProjectWhere }) => store.projects.filter((p) => matchesProject(p, where)).length, updateMany: async ({ @@ -927,6 +954,15 @@ interface AccessBody { const get = (id: string, init: RequestInit = asAdmin) => app.request(`/v1/projects/${id}`, init); +const list = (init: RequestInit = asAdmin) => app.request("/v1/projects", init); + +/** Ids from a `GET /v1/projects` body, in response order. */ +const listIds = async (init: RequestInit = asAdmin): Promise => { + const res = await list(init); + expect(res.status).toBe(200); + return ((await res.json()) as ProjectBody[]).map((p) => p.id); +}; + const patch = (id: string, body: unknown, init: RequestInit = asAdmin) => app.request(`/v1/projects/${id}`, { ...init, @@ -947,6 +983,118 @@ const putAccess = (id: string, body: unknown, init: RequestInit = asAdmin) => body: JSON.stringify(body), }); +describe("GET /projects (list)", () => { + it("401s an unauthenticated caller", async () => { + expect((await list({})).status).toBe(401); + }); + + it("403s a project-scoped key — an agent credential must not enumerate projects", async () => { + expect((await list(asProjectKey)).status).toBe(403); + }); + + it("returns every project in the org to an admin, oldest first", async () => { + expect(await listIds()).toEqual(["proj-1", "proj-2", "proj-3", "proj-4"]); + }); + + it("never leaks another organization's projects", async () => { + expect(await listIds()).not.toContain("proj-x"); + store.sessionUserId = OUTSIDER; // admin of OTHER_ORG only + expect(await listIds({})).toEqual(["proj-x"]); + }); + + it("returns the whole org to an OWNER, not just their bindings", async () => { + // OWNER holds one binding (proj-2) but is org owner — the admin arm wins, + // so a project they hold no binding on (proj-4) must still be listed. + store.sessionUserId = OWNER; + expect(await listIds({})).toEqual(["proj-1", "proj-2", "proj-3", "proj-4"]); + }); + + it("returns ONLY bound projects to a plain member", async () => { + store.sessionUserId = MEMBER; // owner binding on proj-1 only + expect(await listIds({})).toEqual(["proj-1"]); + }); + + it("counts a GROUP binding, not just a direct one", async () => { + // MEMBER2: direct `member` row on proj-2, plus proj-1 through group g-a. + // Ordering is by createdAt, so the group-derived project comes first. + store.sessionUserId = MEMBER2; + expect(await listIds({})).toEqual(["proj-1", "proj-2"]); + }); + + it("omits a project with no bindings from a member's list", async () => { + // proj-4 is the legacy zero-binding shape: visible to admins, unreachable + // for a plain member, so it must not appear. + store.sessionUserId = MEMBER; + expect(await listIds({})).not.toContain("proj-4"); + }); + + it("401s a SUSPENDED member before the handler runs, binding notwithstanding", async () => { + // The suspension invariant, enforced a layer earlier than you might + // expect: session auth resolves the default project through + // `activeMembershipWhere`, which a suspended member fails, so no project + // AND no org resolve and the request 401s. MEMBER's owner binding on + // proj-1 never gets a chance to rescue them. + // + // `listProjects`'s own `if (!role) return []` arm is therefore + // defence-in-depth for direct service callers, not a path this route can + // reach — it exists so the function mirrors `canAccessProjectAsUser` arm + // for arm rather than relying on its caller to have gated first. + const row = store.members.find((m) => m.userId === MEMBER); + if (row) row.status = "suspended"; + store.sessionUserId = MEMBER; + expect((await list({})).status).toBe(401); + }); + + it("401s a non-member holding a stale binding", async () => { + // STRANGER has a real ProjectAccess row on proj-3 but no membership at + // all, so nothing resolves and auth rejects — the binding is inert. + store.sessionUserId = STRANGER; + expect((await list({})).status).toBe(401); + }); + + it("returns [] — not 403 — for a member with no bindings at all", async () => { + // There is no id to authorize against, so an empty list IS the answer. + store.projectAccess = store.projectAccess.filter( + (pa) => pa.userId !== MEMBER, + ); + store.sessionUserId = MEMBER; + const res = await list({}); + expect(res.status).toBe(200); + expect(await res.json()).toEqual([]); + }); + + it("returns the client Project shape and leaks no internal columns", async () => { + const res = await list(); + const [first] = (await res.json()) as ProjectBody[]; + expect(first).toEqual({ + id: "proj-1", + name: "Alpha", + slug: "alpha", + createdAt: expect.any(String), + }); + // `projectSelect` also reads createdByUserId (resolveAuthority needs it); + // `toProjectRow` must not pass it through, nor the org id. + expect(Object.keys(first ?? {})).not.toContain("createdByUserId"); + expect(Object.keys(first ?? {})).not.toContain("organizationId"); + }); + + it("agrees with GET /:projectId — everything listed is readable, and vice versa", async () => { + // The invariant `listProjects` exists to hold: it must mirror + // `canAccessProjectAsUser` arm for arm. Drift either way is a bug. + store.sessionUserId = MEMBER2; + const listed = new Set(await listIds({})); + for (const id of ["proj-1", "proj-2", "proj-3", "proj-4"]) { + const readable = (await get(id, {})).status === 200; + expect(listed.has(id)).toBe(readable); + } + }); + + it("reads nothing and audits nothing", async () => { + await list(); + expect(store.audits).toHaveLength(0); + }); +}); + describe("guard stack", () => { it("401s an unauthenticated caller on every route", async () => { expect((await get("proj-1", {})).status).toBe(401); diff --git a/packages/api/src/routes/org/projects.ts b/packages/api/src/routes/org/projects.ts index c6569691..a40034e6 100644 --- a/packages/api/src/routes/org/projects.ts +++ b/packages/api/src/routes/org/projects.ts @@ -8,6 +8,7 @@ import { canAccessProjectAsUser } from "../../middleware/auth/resolve"; import { deleteProject, getProject, + listProjects, renameProject, requireManageableProject, requireProject, @@ -97,6 +98,21 @@ export const ossProjectRoutes = () => { return project; }; + // GET /projects — every project the caller may use, for a switcher and for + // upstream's Get Started picker. Deliberately matches upstream's client + // contract (`Project[]`, optional `X-Organization-Id` override): upstream + // serves this route from its closed cloud backend, so the OSS build 404s it + // without this handler. + // + // No authorization here on purpose. Unlike every other route in this file + // there is no id to resolve, so there is nothing to 404 and nothing to 403 — + // `listProjects` IS the authorization, returning only what the caller may + // reach. A member of the org with no bindings correctly gets `[]`, not a 403. + app.get("/", async (c) => { + const auth = c.get("auth"); + return c.json(await listProjects(auth.organizationId, auth.userId)); + }); + // GET /projects/:projectId — the sharing page's name/slug source. Nothing // else in the API exposes a project's name (the session route returns only // `projectId`). diff --git a/packages/api/src/services/project-service.ts b/packages/api/src/services/project-service.ts index 71f90861..9a7787f5 100644 --- a/packages/api/src/services/project-service.ts +++ b/packages/api/src/services/project-service.ts @@ -6,6 +6,7 @@ import { hasResolvableProjectExcluding, } from "./organization-service"; import { invalidateGatewayCacheForKeys } from "../lib/gateway-invalidate"; +import { CAPS } from "../lib/env"; // Project administration: read, rename, delete. Three rules, same as // `org-group-service.ts`: @@ -159,6 +160,69 @@ export const getProject = async ( ): Promise => toProjectRow(await requireProject(organizationId, projectId)); +const listAllProjects = async (organizationId: string): Promise => + ( + await db.project.findMany({ + where: { organizationId }, + select: projectSelect, + orderBy: [{ createdAt: "asc" }, { id: "asc" }], + }) + ).map(toProjectRow); + +/** + * Every project in `organizationId` the caller may USE, oldest first. + * + * THIS MUST MIRROR `canAccessProjectAsUser` — it is that per-row predicate + * expressed as a query, arm for arm, and the two are required to agree. Drift + * either way is a bug with teeth: a project listed here but rejected by the + * usage gate leaks a name past its ProjectAccess bindings, and one allowed + * there but omitted here is a project the caller can reach but never discover. + * + * The arms, in the same order and for the same reasons as the gate: + * + * 1. No RBAC — the gate no-ops (allows), so the list is the whole org. + * 2. No role — suspended or not a member. Empty, and the binding arm is + * INSIDE this gate, so a suspended user's stale binding is never consulted + * (the suspension invariant). + * 3. Admin/owner — the whole org. + * 4. Otherwise — projects carrying a binding for this user, direct or through + * a group. `role` is deliberately not filtered: usage is role-blind, and a + * plain `member` binding is a full use grant. + * + * Ordering matches `findUserDefaultProject` (`createdAt asc, id asc`) so the + * project a caller lands on by default is the first one a switcher shows. + */ +export const listProjects = async ( + organizationId: string, + userId: string, +): Promise => { + if (!CAPS.rbac) return listAllProjects(organizationId); + + const resolver = getRoleResolver(); + const role = resolver + ? await resolver.getUserRole(userId, organizationId) + : null; + if (!role) return []; + if (ROLE_HIERARCHY[role] >= ROLE_HIERARCHY.admin) { + return listAllProjects(organizationId); + } + + return ( + await db.project.findMany({ + where: { + organizationId, + accessBindings: { + some: { + OR: [{ userId }, { group: { members: { some: { userId } } } }], + }, + }, + }, + select: projectSelect, + orderBy: [{ createdAt: "asc" }, { id: "asc" }], + }) + ).map(toProjectRow); +}; + /** * Rename. `name` ONLY — `slug` is immutable (it is write-only provenance, * never read by api/web/gateway, and it is `@@unique([organizationId, slug])`,