pass erased flag properly; add tests
REUSE Compliance Check / reuse-compliance-check (push) Waiting to run
CI / Build (push) Waiting to run
CI / Docker (push) Blocked by required conditions
CI / Docker (subpath /admin) (push) Blocked by required conditions
CI / CDN (push) Blocked by required conditions
CI / Github Release (push) Blocked by required conditions

This commit is contained in:
Aine
2026-05-31 14:53:14 +01:00
parent 8e9827a6ce
commit 5cc13cbba9
5 changed files with 124 additions and 5 deletions
+15
View File
@@ -403,4 +403,19 @@ describe("dataProvider", () => {
expect(vi.mocked(fetch).mock.calls[0]?.[0]).toContain("from=0");
expect(vi.mocked(fetch).mock.calls[1]?.[0]).toContain("from=250");
});
it("update skips the profile PUT when the user was erased (meta.userErased)", async () => {
const result = await dataProvider.update("users", {
id: "@bob:hs",
previousData: { id: "@bob:hs", deactivated: false, erased: false },
data: { id: "@bob:hs", deactivated: false, erased: false, displayname: "x" },
meta: { userErased: true },
});
// No HTTP request fired — the erased account would have returned M_UNKNOWN.
expect(fetch).not.toHaveBeenCalled();
expect(result.data.id).toBe("@bob:hs");
expect(result.data.deactivated).toBe(true);
expect(result.data.erased).toBe(true);
});
});
+6
View File
@@ -576,6 +576,12 @@ const baseDataProvider: SynapseDataProvider = {
update: async (resource, params) => {
log.debug("update", resource, params.id);
// Erase is terminal: the lifecycle beforeUpdate already deactivated + GDPR-erased the user and
// set this flag. Skip the profile PUT (PUT /v2/users would recreate the just-erased profile)
// and echo back the erased record.
if (resource === "users" && params.meta?.userErased) {
return { data: { ...params.previousData, ...params.data, id: params.id, deactivated: true, erased: true } };
}
const { res, baseUrl } = resolveResource(resource);
const endpoint_url = baseUrl + res.path;
+90 -1
View File
@@ -43,7 +43,7 @@ const makeBase = (): MockBase => ({
shadowBanUser: vi.fn().mockResolvedValue({}),
uploadMedia: vi.fn().mockResolvedValue({ content_uri: "mxc://h/m" }),
setRateLimits: vi.fn().mockResolvedValue({}),
eraseUser: vi.fn().mockResolvedValue({}),
eraseUser: vi.fn().mockResolvedValue({ success: true }),
update: vi.fn().mockResolvedValue({ data: { id: "@u:hs" } }),
});
@@ -364,3 +364,92 @@ describe("getMASUsersAsMainResource.delete", () => {
expect(JSON.parse(calls[0].body || "{}")).toEqual({ deactivated: true });
});
});
describe("lifecycle.beforeUpdate — Synapse-only mode, erase guard", () => {
// Regression for the catastrophic bug: deactivated + erased are present on every user record,
// so the old `!== undefined` guard fired eraseUser on every save. The guard must only fire when
// both flags are explicitly true AND actually changed.
it("does NOT erase on a routine edit (displayname change, deactivated/erased unchanged)", async () => {
const base = makeBase();
const wrap = wrapWithLifecycle(base as any);
await wrap.update("users", {
id: "@bob:hs",
previousData: { id: "@bob:hs", displayname: "Bob", deactivated: false, erased: false },
data: { id: "@bob:hs", displayname: "Bobby", deactivated: false, erased: false },
});
expect(base.eraseUser).not.toHaveBeenCalled();
// The profile PUT proceeds normally — no skip signal set.
expect(base.update).toHaveBeenCalledTimes(1);
expect(base.update.mock.calls[0][1].meta?.userErased).toBeFalsy();
});
it("does NOT erase when only deactivating (deactivated true, erased false) — deactivate flows to the PUT", async () => {
const base = makeBase();
const wrap = wrapWithLifecycle(base as any);
await wrap.update("users", {
id: "@bob:hs",
previousData: { id: "@bob:hs", deactivated: false, erased: false },
data: { id: "@bob:hs", deactivated: true, erased: false },
});
expect(base.eraseUser).not.toHaveBeenCalled();
// deactivated stays in data so the base PUT carries it (plain deactivate, no erase).
expect(base.update.mock.calls[0][1].data.deactivated).toBe(true);
});
it("erases exactly once when the operator sets deactivated+erased (false → true), then signals skip", async () => {
const base = makeBase();
const wrap = wrapWithLifecycle(base as any);
await wrap.update("users", {
id: "@bob:hs",
previousData: { id: "@bob:hs", deactivated: false, erased: false },
data: { id: "@bob:hs", deactivated: true, erased: true },
});
expect(base.eraseUser).toHaveBeenCalledTimes(1);
expect(base.eraseUser).toHaveBeenCalledWith("@bob:hs");
// beforeUpdate can't cancel the base update; it signals it to skip the doomed PUT instead.
expect(base.update.mock.calls[0][1].meta.userErased).toBe(true);
});
it("idempotency: erase fires exactly once across two sequential saves (second sees already-erased state)", async () => {
const base = makeBase();
const wrap = wrapWithLifecycle(base as any);
// First save: operator deactivates + erases.
await wrap.update("users", {
id: "@bob:hs",
previousData: { id: "@bob:hs", deactivated: false, erased: false },
data: { id: "@bob:hs", deactivated: true, erased: true },
});
// Second save: the record is now erased; RA's previousData reflects it, so no re-fire.
await wrap.update("users", {
id: "@bob:hs",
previousData: { id: "@bob:hs", deactivated: true, erased: true },
data: { id: "@bob:hs", deactivated: true, erased: true, displayname: "x" },
});
expect(base.eraseUser).toHaveBeenCalledTimes(1);
});
it("rejects without faking success when eraseUser reports failure, and skips the PUT", async () => {
const base = makeBase();
base.eraseUser.mockResolvedValueOnce({ success: false, error: "boom" });
const wrap = wrapWithLifecycle(base as any);
await expect(
wrap.update("users", {
id: "@bob:hs",
previousData: { id: "@bob:hs", deactivated: false, erased: false },
data: { id: "@bob:hs", deactivated: true, erased: true },
})
).rejects.toThrow("boom");
// The erase failed → the account still exists → we must not silently proceed.
expect(base.update).not.toHaveBeenCalled();
});
});
+12 -4
View File
@@ -116,7 +116,6 @@ export const wrapWithLifecycle = (base: SynapseDataProvider): SynapseDataProvide
const erased = params.data.erased;
const previousErased = params.previousData?.erased;
if (rates) {
await dataProvider.setRateLimits(params.id, rates);
delete params.data.rates;
@@ -137,9 +136,18 @@ export const wrapWithLifecycle = (base: SynapseDataProvider): SynapseDataProvide
erased === true &&
(deactivated !== previousDeactivated || erased !== previousErased)
) {
await (dataProvider as SynapseDataProvider).eraseUser(params.id);
delete params.data.deactivated;
delete params.data.erased;
const result = await (dataProvider as SynapseDataProvider).eraseUser(params.id);
if (!result.success) {
return Promise.reject(new Error(result.error || "Failed to erase user"));
}
// Erase is terminal for this save: skip the follow-up profile PUT. PUT /v2/users is
// create-or-recreate, so letting it run would recreate the profile we just erased.
// Re-editing an already-erased user does not re-enter this branch (the change check above
// is false when both flags are unchanged), so recreating an erased user via a normal edit
// still works. Note: returning here exits beforeUpdate only — react-admin still runs any
// registered beforeSave on params.data; there is none on "users" today, keep it that way.
params.meta = { ...params.meta, userErased: true };
return params;
}
if (avatarErase) {
+1
View File
@@ -71,6 +71,7 @@ export const synapseResourceMap = {
is_guest: !!u.is_guest,
admin: !!u.admin,
deactivated: !!u.deactivated,
erased: !!u.erased,
shadow_banned: !!u.shadow_banned,
// Normalize across Synapse user endpoints before the value reaches the UI.
creation_ts_ms: normalizeTS(u.creation_ts),