diff --git a/src/lib/orchestrators/membership.ts b/src/lib/orchestrators/membership.ts index 4b56ffc..cced30d 100644 --- a/src/lib/orchestrators/membership.ts +++ b/src/lib/orchestrators/membership.ts @@ -1,3 +1,4 @@ +import type { Client } from "@atcute/client"; import * as TID from "@atcute/tid"; import { deleteRecord, @@ -21,6 +22,23 @@ import { ForbiddenError, NotFoundError, ValidationError } from "../errors.ts"; import { t } from "../i18n/index.ts"; import { withPdsError } from "./helpers.ts"; +// A membership record lives in the repo of whoever granted it, and only that +// repo's owner may delete it. Best-effort, so revoking access locally never +// hangs on a PDS write we are not entitled to make. +async function deleteOwnMembershipRecord( + agent: Client, + sessionDid: string, + atUri: string, +): Promise { + const parsed = parseAtUri(atUri); + if (!parsed || parsed.did !== sessionDid) return; + try { + await deleteRecord(agent, parsed.did, COLLECTIONS.membership, parsed.rkey); + } catch (err) { + console.warn("Failed to delete membership record from PDS:", err); + } +} + export async function requestAccessAction( ctx: WikiRequestContext, ): Promise { @@ -118,20 +136,7 @@ export async function changeMemberRoleAction( const session = ctx.session; const agent = session ? getAgent(session) : null; if (agent && session) { - // Best-effort: delete the old PDS record if we own it. - const parsed = parseAtUri(existing.at_uri); - if (parsed && parsed.did === session.did) { - try { - await deleteRecord( - agent, - parsed.did, - COLLECTIONS.membership, - parsed.rkey, - ); - } catch (err) { - console.warn("Failed to delete old membership record from PDS:", err); - } - } + await deleteOwnMembershipRecord(agent, session.did, existing.at_uri); await withPdsError("change member role", async () => { await writeMembershipRecord( @@ -206,18 +211,8 @@ export async function deleteMemberAction( const session = ctx.session; const agent = session ? getAgent(session) : null; - if (agent) { - const parsed = parseAtUri(atUri); - if (parsed) { - await withPdsError("remove member", async () => { - await deleteRecord( - agent, - parsed.did, - COLLECTIONS.membership, - parsed.rkey, - ); - }); - } + if (agent && session) { + await deleteOwnMembershipRecord(agent, session.did, atUri); } deleteMembership(ctx.wiki.at_uri, memberDid); diff --git a/tests/lib/orchestrators/membership.test.ts b/tests/lib/orchestrators/membership.test.ts index e3d50df..52bce58 100644 --- a/tests/lib/orchestrators/membership.test.ts +++ b/tests/lib/orchestrators/membership.test.ts @@ -490,8 +490,7 @@ describe("deleteMemberAction", () => { ).rejects.toBeInstanceOf(NotFoundError); }); - test("throws PdsWriteError on PDS delete failure", async () => { - // Setup: add a member + test("revokes locally even when the PDS delete fails", async () => { const db = getDb(); db.run( `INSERT OR REPLACE INTO memberships (wiki_at_uri, wiki_slug, did, role, at_uri, created_at) @@ -510,9 +509,40 @@ describe("deleteMemberAction", () => { throw new Error("PDS delete failed"); }); - await expect( - deleteMemberAction(makeCtx(), "did:plc:pdsfailremove"), - ).rejects.toBeInstanceOf(PdsWriteError); + await deleteMemberAction(makeCtx(), "did:plc:pdsfailremove"); + + const row = db + .query("SELECT * FROM memberships WHERE wiki_slug = ? AND did = ?") + .get(WIKI_SLUG, "did:plc:pdsfailremove"); + expect(row).toBeNull(); + }); + + test("removes a member the owner granted, without touching their repo", async () => { + // The record lives in OWNER_DID's repo; the acting admin cannot delete it + // there, and attempting to must not block the local revocation. + const db = getDb(); + db.run( + `INSERT OR REPLACE INTO memberships (wiki_at_uri, wiki_slug, did, role, at_uri, created_at) + VALUES (?, ?, ?, ?, ?, ?)`, + [ + wiki.at_uri, + WIKI_SLUG, + "did:plc:granted-by-owner", + "contributor", + `at://${OWNER_DID}/wiki.lichen.membership/owner-granted-tid`, + "2026-01-01T00:00:00.000Z", + ], + ); + + mockDeleteRecord.mockClear(); + + await deleteMemberAction(makeCtx(), "did:plc:granted-by-owner"); + + expect(mockDeleteRecord).not.toHaveBeenCalled(); + const row = db + .query("SELECT * FROM memberships WHERE wiki_slug = ? AND did = ?") + .get(WIKI_SLUG, "did:plc:granted-by-owner"); + expect(row).toBeNull(); }); test("skips PDS when no session", async () => {