diff --git a/CHANGELOG.md b/CHANGELOG.md index 92b1361..0895745 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). - fix(sql): lowercase existing zone names, and the zone in record names - fix: one random, self describing session row per login, expired ones swept at login, and mysql throttles the activity touch - fix: a token is valid only while its session row exists, so logout revokes it, and any authenticated request counts as activity +- fix: destroying a group, user or zone on json or toml takes its nameservers, sessions or records with it, as mysql does - fix: moving a user moves their permission rows to the new group ### [3.0.3] - 2026-07-27 diff --git a/lib/group/store/file.js b/lib/group/store/file.js index 76f244c..43961c9 100644 --- a/lib/group/store/file.js +++ b/lib/group/store/file.js @@ -3,6 +3,8 @@ import { idConflict } from '../../store/error.js' import GroupBase from './base.js' +import { cascade } from '../../references.js' + const defaultPermissions = { inherit: false, self_write: false, @@ -150,6 +152,8 @@ class GroupRepoFile extends GroupBase { } async destroy(args) { + if (!(await this._load()).some((g) => g.id === args.id)) return false + await cascade('group', args.id) return this.file.mutate('group', (groups) => removeRows(groups, (g) => g.id === args.id) > 0) } } diff --git a/lib/nameserver/store/file.js b/lib/nameserver/store/file.js index 8598c41..0d689c8 100644 --- a/lib/nameserver/store/file.js +++ b/lib/nameserver/store/file.js @@ -1,4 +1,4 @@ -import FileStore, { claimId, removeRows } from '../../store/file.js' +import FileStore, { claimId, destroyMatch, removeRows } from '../../store/file.js' import { idConflict } from '../../store/error.js' import NameserverBase from './base.js' @@ -85,10 +85,8 @@ class NameserverRepoFile extends NameserverBase { } async destroy(args) { - return this.file.mutate( - 'nameserver', - (nameservers) => removeRows(nameservers, (n) => n.id === args.id) > 0, - ) + const match = destroyMatch(args, ['id', 'gid']) + return this.file.mutate('nameserver', (nameservers) => removeRows(nameservers, match) > 0) } } diff --git a/lib/nameserver/store/mysql.js b/lib/nameserver/store/mysql.js index 3c04278..0be093b 100644 --- a/lib/nameserver/store/mysql.js +++ b/lib/nameserver/store/mysql.js @@ -95,9 +95,13 @@ class Nameserver extends NameserverBase { return r.changedRows === 1 } - async destroy(args) { - const r = await Mysql.execute(...Mysql.delete(`nt_nameserver`, { nt_nameserver_id: args.id })) - return r.affectedRows === 1 + async destroy({ id, gid }) { + if (id === undefined && gid === undefined) throw new Error('destroy() needs one of id, gid') + const where = {} + if (id !== undefined) where.nt_nameserver_id = id + if (gid !== undefined) where.nt_group_id = gid + const r = await Mysql.execute(...Mysql.delete(`nt_nameserver`, where)) + return r.affectedRows > 0 } disconnect() { diff --git a/lib/references.js b/lib/references.js new file mode 100644 index 0000000..80eef54 --- /dev/null +++ b/lib/references.js @@ -0,0 +1,28 @@ +// What a hard delete takes with it. mysql enforces each of these with a +// FOREIGN KEY; the file stores have none, so their destroy() calls cascade(). +const cascades = [ + { parent: 'group', child: 'nameserver', via: 'gid' }, + { parent: 'user', child: 'session', via: 'uid' }, + { parent: 'zone', child: 'zone_record', via: 'zid' }, +] + +// Imported when a cascade runs, so a child store loading its own parent does +// not close a circle at module load. +const childStores = { + nameserver: () => import('./nameserver/index.js'), + session: () => import('./session/index.js'), + zone_record: () => import('./zone_record/index.js'), +} + +export async function cascade(parent, id) { + for (const edge of cascades.filter((e) => e.parent === parent)) { + const store = (await childStores[edge.child]()).default + + // One call per edge, matched on the parent key at write time, so a child + // moved to another parent stays. Session's delete() is its hard delete. + const remove = typeof store.destroy === 'function' ? store.destroy : store.delete + await remove.call(store, { [edge.via]: id }) + } +} + +export default cascades diff --git a/lib/references.test.js b/lib/references.test.js new file mode 100644 index 0000000..cc9fc85 --- /dev/null +++ b/lib/references.test.js @@ -0,0 +1,247 @@ +import assert from 'node:assert/strict' +import { after, before, describe, it } from 'node:test' + +import { storeType } from './config.js' +import cascades from './references.js' + +import Group from './group/index.js' +import Nameserver from './nameserver/index.js' +import Session from './session/index.js' +import User from './user/index.js' +import Zone from './zone/index.js' +import ZoneRecord from './zone_record/index.js' + +const GID = 47101 +const ids = { + group: [47101, 47111, 47121], + user: [47102, 47112, 47122], + zone: [47103, 47113, 47123], + nameserver: [47104, 47114, 47124], + session: [47105, 47115, 47125], + zone_record: [47106, 47116, 47126], +} + +const entities = { + group: { store: Group, body: (id) => ({ id, parent_gid: 0, name: `g${id}.refgraph.test` }) }, + user: { + store: User, + body: (id) => ({ + id, + gid: GID, + username: `refgraph${id}`, + email: `refgraph${id}@example.com`, + password: 'Wh@tA-Decent#P6ssw0rd', + first_name: 'Ref', + last_name: 'Graph', + is_admin: false, + }), + }, + zone: { + store: Zone, + body: (id) => ({ + id, + gid: GID, + zone: `z${id}.refgraph.test`, + mailaddr: `hostmaster.z${id}.refgraph.test.`, + serial: 1, + refresh: 1, + retry: 2, + expire: 3, + minimum: 4, + ttl: 3600, + }), + }, + nameserver: { + store: Nameserver, + body: (id) => ({ id, name: `ns${id}.refgraph.test.`, address: '203.0.113.10', type: 'nsd', ttl: 3600 }), + }, + session: { store: Session, body: (id) => ({ id, session: `refgraph-${id}`, last_access: 1700000000 }) }, + zone_record: { + store: ZoneRecord, + body: (id) => ({ id, owner: `r${id}.refgraph.test.`, type: 'A', address: '203.0.113.9', ttl: 3600 }), + }, +} + +const softDeletes = (name) => typeof entities[name].store.destroy === 'function' +const rows = (found) => [found].flat().filter(Boolean).length + +async function present(name, id) { + const { store } = entities[name] + if (!softDeletes(name)) return rows(await store.get({ id })) + let n = 0 + for (const deleted of [false, true]) n += rows(await store.get({ id, deleted })) + return n +} + +async function remove(name, id) { + const { store } = entities[name] + return softDeletes(name) ? store.destroy({ id }) : store.delete({ id }) +} + +// A store may allocate its own id, so seed() reports the one it used. +async function seed(name, body) { + const returned = await entities[name].store.create(body) + return Number.isInteger(returned) ? returned : body.id +} + +async function reset() { + for (const uid of ids.user) await Session.delete({ uid }) + for (const name of ['zone_record', 'session', 'zone', 'nameserver', 'user', 'group']) { + for (const id of ids[name]) await remove(name, id) + } +} + +before(reset) + +after(async () => { + await reset() + for (const { store } of Object.values(entities)) await store.disconnect?.() +}) + +describe('cascade graph', () => { + it('declares the edges the constraints enforce', () => { + assert.deepEqual( + cascades.map((e) => `${e.parent} -> ${e.child}`).sort(), + ['group -> nameserver', 'user -> session', 'zone -> zone_record'], + 'an edge was added or dropped; the file stores follow this list', + ) + for (const edge of cascades) { + assert.ok(entities[edge.parent], `no fixture for parent ${edge.parent}`) + assert.ok(entities[edge.child], `no fixture for child ${edge.child}`) + assert.ok(edge.via, `${edge.parent} -> ${edge.child} declares no via`) + } + }) + + for (const edge of cascades) { + it(`destroying a ${edge.parent} takes its ${edge.child} rows (${edge.via})`, async () => { + await reset() + await seed('group', entities.group.body(GID)) + + const [doomed, spared] = ids[edge.parent] + if (edge.parent === 'group') await seed('group', entities.group.body(spared)) + else for (const id of [doomed, spared]) await seed(edge.parent, entities[edge.parent].body(id)) + + const [first, other, extra] = ids[edge.child] + const child = await seed(edge.child, { ...entities[edge.child].body(first), [edge.via]: doomed }) + const sibling = await seed(edge.child, { ...entities[edge.child].body(extra), [edge.via]: doomed }) + const control = await seed(edge.child, { ...entities[edge.child].body(other), [edge.via]: spared }) + + if (softDeletes(edge.child)) await entities[edge.child].store.delete({ id: sibling }) + + for (const [id, what] of [ + [child, 'child'], + [sibling, 'sibling'], + [control, 'control'], + ]) { + assert.equal(await present(edge.child, id), 1, `the ${what} did not seed`) + } + + assert.ok(await remove(edge.parent, doomed), `destroying the ${edge.parent} reported no change`) + assert.equal(await present(edge.parent, doomed), 0, `the ${edge.parent} outlived its own destroy`) + + assert.equal(await present(edge.child, child), 0, `the ${edge.child} outlived its ${edge.parent}`) + assert.equal(await present(edge.child, sibling), 0, `a soft-deleted ${edge.child} was left behind`) + assert.equal(await present(edge.child, control), 1, `an unrelated ${edge.child} was taken too`) + + // mysql reads a session through its user, so a surviving row would read + // as absent. Deleting it again reports whether it is really gone. + for (const id of [child, sibling]) { + assert.equal(await remove(edge.child, id), false, `a ${edge.child} row was still there to delete`) + } + }) + } + + it('leaves the parent in place when a cascade fails', async () => { + await reset() + await seed('group', entities.group.body(GID)) + const zid = ids.zone[0] + await seed('zone', entities.zone.body(zid)) + const rec = await seed('zone_record', { ...entities.zone_record.body(ids.zone_record[0]), zid }) + + if (storeType() === 'mysql') { + assert.ok(await Zone.destroy({ id: zid }), 'the zone did not destroy') + assert.equal(await present('zone_record', rec), 0, 'the constraint left the record') + return + } + + ZoneRecord.destroy = async () => { + throw new Error('cascade probe') + } + try { + await assert.rejects(() => Zone.destroy({ id: zid }), /cascade probe/) + } finally { + delete ZoneRecord.destroy + } + + assert.equal(await present('zone', zid), 1, 'the zone went while its records stayed') + assert.ok(await Zone.destroy({ id: zid }), 'the retry found no zone to destroy') + assert.equal(await present('zone_record', rec), 0, 'the record outlived the retry') + }) + + it('spares a record moved to another zone while the cascade runs', async () => { + await reset() + await seed('group', entities.group.body(GID)) + const [doomed, spared] = ids.zone + for (const id of [doomed, spared]) await seed('zone', entities.zone.body(id)) + const rec = await seed('zone_record', { ...entities.zone_record.body(ids.zone_record[0]), zid: doomed }) + + // mysql's constraint cascades inside the DELETE, so no move can land mid-way + if (storeType() === 'mysql') return + + // The move lands after any read and before any delete the cascade makes. + let moved = false + const move = async () => { + if (moved) return + moved = true + assert.ok(await ZoneRecord.put({ id: rec, zid: spared }), 'the record did not move') + } + const get = ZoneRecord.get.bind(ZoneRecord) + const destroy = ZoneRecord.destroy.bind(ZoneRecord) + ZoneRecord.get = async (...args) => { + const found = await get(...args) + await move() + return found + } + ZoneRecord.destroy = async (...args) => { + await move() + return destroy(...args) + } + try { + assert.ok(await Zone.destroy({ id: doomed }), 'the zone did not destroy') + } finally { + delete ZoneRecord.get + delete ZoneRecord.destroy + } + + assert.ok(moved, 'the cascade never reached the record store') + assert.equal(await present('zone_record', rec), 1, 'the moved record went with its old zone') + }) + + it("rewrites the record file once for all of a zone's records", async () => { + await reset() + await seed('group', entities.group.body(GID)) + const zid = ids.zone[0] + await seed('zone', entities.zone.body(zid)) + for (const id of ids.zone_record) await seed('zone_record', { ...entities.zone_record.body(id), zid }) + + if (storeType() === 'mysql') return + + let writes = 0 + const write = ZoneRecord.file._write.bind(ZoneRecord.file) + ZoneRecord.file._write = async (...args) => { + writes++ + return write(...args) + } + try { + assert.ok(await Zone.destroy({ id: zid }), 'the zone did not destroy') + } finally { + delete ZoneRecord.file._write + } + assert.equal(writes, 1, 'each record rewrote the file on its own') + }) + + it('refuses a destroy that names no row or parent', async () => { + await assert.rejects(() => Nameserver.destroy({}), /needs one of id, gid/) + await assert.rejects(() => ZoneRecord.destroy({}), /needs one of id, zid/) + }) +}) diff --git a/lib/store/file.js b/lib/store/file.js index f0ab9bd..108e88b 100644 --- a/lib/store/file.js +++ b/lib/store/file.js @@ -143,6 +143,13 @@ export class FileStore { } } +// destroy() takes an id, a parent key, or both; never every row +export function destroyMatch(args, keys) { + const given = keys.filter((k) => args[k] !== undefined) + if (given.length === 0) throw new Error(`destroy() needs one of ${keys.join(', ')}`) + return (row) => given.every((k) => row[k] === args[k]) +} + // in place, so a mutate() callback can drop rows export function removeRows(rows, match) { let kept = 0 diff --git a/lib/user/store/file.js b/lib/user/store/file.js index bc2aafb..88eb9fb 100644 --- a/lib/user/store/file.js +++ b/lib/user/store/file.js @@ -5,6 +5,8 @@ import Credentials from '../credentials.js' import UserBase from './base.js' import { splitQualifiedUsername } from '../qualified.js' +import { cascade } from '../../references.js' + const boolFields = ['is_admin', 'deleted'] // Read the group file directly (never via the Group module) to avoid circular imports. @@ -207,6 +209,8 @@ class UserRepoFile extends UserBase { } async destroy(args) { + if (!(await this._load()).some((u) => u.id === args.id)) return false + await cascade('user', args.id) return this.file.mutate('user', (users) => removeRows(users, (u) => u.id === args.id) > 0) } } diff --git a/lib/zone/store/file.js b/lib/zone/store/file.js index a697515..77dc493 100644 --- a/lib/zone/store/file.js +++ b/lib/zone/store/file.js @@ -3,6 +3,8 @@ import { idConflict } from '../../store/error.js' import ZoneBase, { canonicalZoneName, sortNameservers, zoneNameConflict } from './base.js' +import { cascade } from '../../references.js' + const zoneDefaults = { minimum: 3600, ttl: 3600, refresh: 86400, retry: 7200, expire: 1209600 } function assertZoneNameAvailable(zones, zone, excludeId) { @@ -174,6 +176,8 @@ class ZoneRepoFile extends ZoneBase { } async destroy(args) { + if (!(await this._load()).some((z) => z.id === args.id)) return false + await cascade('zone', args.id) return this.file.mutate('zone', (zones) => removeRows(zones, (z) => z.id === args.id) > 0) } diff --git a/lib/zone_record/store/file.js b/lib/zone_record/store/file.js index 18cac93..0f2fd14 100644 --- a/lib/zone_record/store/file.js +++ b/lib/zone_record/store/file.js @@ -1,4 +1,4 @@ -import FileStore, { claimId, removeRows } from '../../store/file.js' +import FileStore, { claimId, destroyMatch, removeRows } from '../../store/file.js' import { idConflict } from '../../store/error.js' import ZoneRecordBase, { checkPut, mergeChanges } from './base.js' @@ -115,7 +115,8 @@ class ZoneRecordRepoFile extends ZoneRecordBase { } async destroy(args) { - return this.file.mutate('zone_record', (records) => removeRows(records, (r) => r.id === args.id) > 0) + const match = destroyMatch(args, ['id', 'zid']) + return this.file.mutate('zone_record', (records) => removeRows(records, match) > 0) } } diff --git a/lib/zone_record/store/mysql.js b/lib/zone_record/store/mysql.js index 16f25e1..c35123a 100644 --- a/lib/zone_record/store/mysql.js +++ b/lib/zone_record/store/mysql.js @@ -214,9 +214,13 @@ class ZoneRecordMySQL extends ZoneRecordBase { return r.changedRows === 1 } - async destroy(args) { - const r = await Mysql.execute(...Mysql.delete(`nt_zone_record`, { nt_zone_record_id: args.id })) - return r.affectedRows === 1 + async destroy({ id, zid }) { + if (id === undefined && zid === undefined) throw new Error('destroy() needs one of id, zid') + const where = {} + if (id !== undefined) where.nt_zone_record_id = id + if (zid !== undefined) where.nt_zone_id = zid + const r = await Mysql.execute(...Mysql.delete(`nt_zone_record`, where)) + return r.affectedRows > 0 } disconnect() {