diff --git a/app/api/owner-auth.test.ts b/app/api/owner-auth.test.ts index f3ab362..54ed842 100644 --- a/app/api/owner-auth.test.ts +++ b/app/api/owner-auth.test.ts @@ -118,9 +118,17 @@ async function setupWithSessions(db: ReturnType) { expect(bobResp.status).toBe(200); const bobCookie = extractCookie(bobResp.headers.get("set-cookie")); - // Capture Alice's userId from the users table (only Alice was first) + // Capture both userIds from the users table. const users = db.select({ id: schema.user.id, email: schema.user.email }).from(schema.user).all(); const aliceRow = users.find((u) => u.email === "alice@example.com"); + const bobRow = users.find((u) => u.email === "bob@example.com"); + + // Sign-up alone does not create a membership; acceptInvitation does. Insert Bob's member + // row directly to represent an accepted invite, so he is a real workspace member (which is + // what these IDOR tests intend). Without a membership row he is not an authorized actor. + db.insert(schema.member).values({ + id: randomUUID(), organizationId: orgId, userId: bobRow!.id, role: "member", createdAt: new Date(), + }).run(); return { aliceCookie: aliceCookie!, bobCookie: bobCookie!, aliceUserId: aliceRow!.id }; } @@ -475,6 +483,33 @@ describe("null-owner link — member blocked, admin allowed (fix 1)", () => { }); }); +// ───────────────────────────────────────────────────────────────────────────── +// Removing a member revokes their access at the endpoint layer +// ───────────────────────────────────────────────────────────────────────────── + +describe("removed member loses endpoint access", () => { + it("a member's key goes from 403 (authed, not owner) to 401 (unauthed) after removal, in production", async () => { + vi.stubEnv("NODE_ENV", "production"); + const { db, userB, keyB, ownedLinkId } = setupTwoActors(); + const { GET: statsGet } = await import("@/app/api/stats/route"); + + // While a member, Bob is authenticated but not the owner of Alice's link → 403. + const before = await statsGet(new Request(`http://t/api/stats?id=${ownedLinkId}`, { + headers: { authorization: `Bearer ${keyB}` }, + })); + expect(before.status).toBe(403); + + // Admin removes Bob: delete his membership row (what removeMember does). + db.delete(schema.member).where(eq(schema.member.userId, userB)).run(); + + // Bob's key no longer resolves to an actor → requireOwner fails closed → 401. + const after = await statsGet(new Request(`http://t/api/stats?id=${ownedLinkId}`, { + headers: { authorization: `Bearer ${keyB}` }, + })); + expect(after.status).toBe(401); + }); +}); + // ───────────────────────────────────────────────────────────────────────────── // Fix 2: API key creation endpoint round-trip // ───────────────────────────────────────────────────────────────────────────── diff --git a/lib/auth-session.test.ts b/lib/auth-session.test.ts index 16a935d..6d25ffd 100644 --- a/lib/auth-session.test.ts +++ b/lib/auth-session.test.ts @@ -3,10 +3,11 @@ import { mkdtempSync } from "node:fs"; import { tmpdir } from "node:os"; import path from "node:path"; import { randomUUID, createHash } from "node:crypto"; +import { eq } from "drizzle-orm"; import { migrate } from "drizzle-orm/better-sqlite3/migrator"; import { getDb } from "@/lib/db/client"; import { makeAuth } from "@/lib/auth"; -import { makeGetActor } from "@/lib/auth-session"; +import { makeGetActor, resolveMembership } from "@/lib/auth-session"; import * as schema from "@/lib/db/schema"; // better-auth requires BETTER_AUTH_SECRET. @@ -161,4 +162,99 @@ describe("getActor", () => { ); expect(actor).toBeNull(); }); + + it("returns null for a valid API key whose user has no workspace membership", async () => { + // A user + key with NO member row (e.g. an orphaned invite: signed up but + // never accepted). The key is valid and enabled but must not authorize. + env.db.insert(schema.organization).values({ + id: randomUUID(), name: "Workspace", slug: "workspace", createdAt: new Date(), + }).run(); + const userId = randomUUID(); + env.db.insert(schema.user).values({ + id: userId, name: "Nomember", email: "nomember@example.com", + emailVerified: true, createdAt: new Date(), updatedAt: new Date(), + }).run(); + const rawKey = "sentou_nomember_" + randomUUID().replace(/-/g, ""); + env.db.insert(schema.apiKey).values({ + id: randomUUID(), userId, name: "no-member key", + keyHash: createHash("sha256").update(rawKey).digest("hex"), + prefix: rawKey.slice(0, 8), createdAt: new Date(), enabled: true, + }).run(); + + const actor = await env.getActor( + new Request("http://t/api/publish", { + headers: { authorization: `Bearer ${rawKey}` }, + }), + ); + expect(actor).toBeNull(); + }); + + it("stops authorizing an API key once the member's row is removed", async () => { + // Seed org + member + key, confirm the key authorizes, then delete the + // member row (what removeMember does) and confirm the key no longer does. + const orgId = randomUUID(); + env.db.insert(schema.organization).values({ + id: orgId, name: "Workspace", slug: "workspace", createdAt: new Date(), + }).run(); + const userId = randomUUID(); + env.db.insert(schema.user).values({ + id: userId, name: "Bob", email: "bob@example.com", + emailVerified: true, createdAt: new Date(), updatedAt: new Date(), + }).run(); + env.db.insert(schema.member).values({ + id: randomUUID(), organizationId: orgId, userId, role: "member", createdAt: new Date(), + }).run(); + const rawKey = "sentou_removed_" + randomUUID().replace(/-/g, ""); + env.db.insert(schema.apiKey).values({ + id: randomUUID(), userId, name: "bob key", + keyHash: createHash("sha256").update(rawKey).digest("hex"), + prefix: rawKey.slice(0, 8), createdAt: new Date(), enabled: true, + }).run(); + + const request = () => + new Request("http://t/api/publish", { headers: { authorization: `Bearer ${rawKey}` } }); + + const before = await env.getActor(request()); + expect(before?.role).toBe("member"); + + env.db.delete(schema.member).where(eq(schema.member.userId, userId)).run(); + + const after = await env.getActor(request()); + expect(after).toBeNull(); + }); +}); + +describe("resolveMembership", () => { + let env: ReturnType; + beforeEach(() => { + env = makeTestEnv(); + }); + + it("returns null when the user has no member row in the workspace org", () => { + env.db.insert(schema.organization).values({ + id: randomUUID(), name: "Workspace", slug: "workspace", createdAt: new Date(), + }).run(); + const userId = randomUUID(); + env.db.insert(schema.user).values({ + id: userId, name: "Nobody", email: "nobody@example.com", + emailVerified: true, createdAt: new Date(), updatedAt: new Date(), + }).run(); + expect(resolveMembership(env.db, userId)).toBeNull(); + }); + + it("returns the role when the user is a member of the workspace org", () => { + const orgId = randomUUID(); + env.db.insert(schema.organization).values({ + id: orgId, name: "Workspace", slug: "workspace", createdAt: new Date(), + }).run(); + const userId = randomUUID(); + env.db.insert(schema.user).values({ + id: userId, name: "Owner", email: "owner@example.com", + emailVerified: true, createdAt: new Date(), updatedAt: new Date(), + }).run(); + env.db.insert(schema.member).values({ + id: randomUUID(), organizationId: orgId, userId, role: "owner", createdAt: new Date(), + }).run(); + expect(resolveMembership(env.db, userId)).toBe("owner"); + }); }); diff --git a/lib/auth-session.ts b/lib/auth-session.ts index 38b5d7d..7a9593a 100644 --- a/lib/auth-session.ts +++ b/lib/auth-session.ts @@ -28,14 +28,17 @@ export function isAdmin(actor: Actor): boolean { return actor.role === "owner" || actor.role === "admin"; } -// Resolves the actor's role, scoped to the workspace org (oldest by createdAt, -// then id as tiebreaker). Defaults to "member" if the user has no membership. -// Using the workspace org prevents a user from gaining elevated rights by -// joining or creating a second org. -export function resolveRole( +// Resolves the user's membership role in the workspace org (oldest by createdAt, +// then id as tiebreaker), or null when the user has no membership row there. +// Scoping to the workspace org prevents a user from gaining elevated rights by +// joining or creating a second org. null is the load-bearing signal for +// authorization: a removed member, an orphaned invite (signed up but never +// accepted), and a race-losing "second owner" all have no row here and so must +// not resolve to an authorized actor. +export function resolveMembership( db: BetterSQLite3Database, userId: string, -): string { +): string | null { const workspaceOrg = db .select({ id: schema.organization.id }) .from(schema.organization) @@ -43,7 +46,7 @@ export function resolveRole( .limit(1) .get(); - if (!workspaceOrg) return "member"; + if (!workspaceOrg) return null; const memberRow = db .select({ role: schema.member.role }) @@ -56,7 +59,18 @@ export function resolveRole( ) .get(); - return memberRow?.role ?? "member"; + return memberRow?.role ?? null; +} + +// Back-compat helper for the dashboard pages that only need a role string to +// compute isAdmin: a non-member reads as "member" (never elevated), which is +// harmless there. Authorization decisions must use resolveMembership / +// resolveActor, which fail closed on a missing membership. +export function resolveRole( + db: BetterSQLite3Database, + userId: string, +): string { + return resolveMembership(db, userId) ?? "member"; } // Internal implementation — shared between the factory (tests) and the @@ -71,7 +85,11 @@ async function resolveActor( // signed session cookie and returns the user + session or null. const session = await authInst.api.getSession({ headers: req.headers }); if (session?.user?.id) { - const role = resolveRole(db, session.user.id); + // Require a live workspace membership. A valid session for a removed member + // (or an account that signed up but never accepted its invite) must NOT be + // an authorized actor, so removing someone actually revokes their access. + const role = resolveMembership(db, session.user.id); + if (!role) return null; return { userId: session.user.id, role }; } @@ -99,7 +117,10 @@ async function resolveActor( .where(eq(schema.apiKey.id, keyRow.id)) .run(); - const role = resolveRole(db, keyRow.userId); + // Same membership requirement as the session path: a key belonging to a + // removed member resolves to no actor, so revoking membership revokes the key. + const role = resolveMembership(db, keyRow.userId); + if (!role) return null; return { userId: keyRow.userId, role }; }