From 64db3fa050b5759eec66c99209fb76edb391a411 Mon Sep 17 00:00:00 2001 From: Juan Castro Date: Tue, 1 Sep 2026 12:07:47 -0400 Subject: [PATCH] feat: add audit_team_membership and share.holder_group_id Insert-only record of what a workspace administrator did to an account, shaped like \`audit_user_to_group_permissions\` after 0019: nullable FK beside a NOT NULL \`_keep\` column. The FKs are ON DELETE SET NULL, never CASCADE, so hard-deleting an account cannot erase the record of the resets performed on it. Two indexes rather than one. The member's own view is the only place a reset becomes visible to the account it was performed on, so (user_id_keep, id) is a read path, not an optimisation. \`share.holder_group_id\` mirrors \`holder_user_id\` from 0067. The existing unique index does not constrain team shares at all -- it leads with \`holder_user_id\`, which is NULL on every team share, and NULLs are distinct -- so the group-scoped unique index is what prevents duplicates. Drops the \`role\` column from the specified DDL: it contradicted the settled single-administrator model. --- .../database/SqliteDatabaseClient.test.ts | 120 +++++++++++++++++- .../clients/database/SqliteDatabaseClient.ts | 1 + .../migrations/mysql/mysql_mig_34.sql | 84 ++++++++++++ .../migrations/postgres/postgres_mig_23.sql | 57 +++++++++ .../0079_team-audit-and-group-shares.sql | 66 ++++++++++ 5 files changed, 327 insertions(+), 1 deletion(-) create mode 100644 src/backend/clients/database/migrations/mysql/mysql_mig_34.sql create mode 100644 src/backend/clients/database/migrations/postgres/postgres_mig_23.sql create mode 100644 src/backend/clients/database/migrations/sqlite/0079_team-audit-and-group-shares.sql diff --git a/src/backend/clients/database/SqliteDatabaseClient.test.ts b/src/backend/clients/database/SqliteDatabaseClient.test.ts index 715b3af8b..72eca95fe 100644 --- a/src/backend/clients/database/SqliteDatabaseClient.test.ts +++ b/src/backend/clients/database/SqliteDatabaseClient.test.ts @@ -27,7 +27,7 @@ import { DatabaseClientFactory } from './index.js'; import { SqliteDatabaseClient } from './SqliteDatabaseClient.js'; /** Highest schema version the migration table can reach. */ -const CURRENT_SCHEMA_VERSION = 74; +const CURRENT_SCHEMA_VERSION = 75; /** * These suites migrate real files on disk. Idle they finish in well under a @@ -191,6 +191,124 @@ describe('SqliteDatabaseClient — boot and migrations', { timeout: DISK_MIGRATI await expect(insert()).rejects.toThrow(/UNIQUE/iu); }); + it('applies the audit table and group-share columns from 0079', async () => { + const columns = (await client.read( + "SELECT name FROM pragma_table_info('audit_team_membership')", + )) as { name: string }[]; + expect(columns.map((r) => r.name)).toEqual([ + 'id', + 'group_id', + 'group_id_keep', + 'user_id', + 'user_id_keep', + 'actor_user_id', + 'action', + 'reason', + 'created_at', + ]); + + const shareColumns = (await client.read( + "SELECT name FROM pragma_table_info('share')", + )) as { name: string }[]; + expect(shareColumns.map((r) => r.name)).toContain('holder_group_id'); + + const indexes = (await client.read( + "SELECT name FROM sqlite_master WHERE type = 'index' " + + 'AND (name LIKE ? OR name LIKE ?)', + ['idx_audit_team_membership%', 'idx_share_holder_group%'], + )) as { name: string }[]; + expect(indexes.map((r) => r.name).sort()).toEqual([ + // The `_fk` three stop a user or group delete scanning the audit table. + 'idx_audit_team_membership_actor_fk', + 'idx_audit_team_membership_group', + 'idx_audit_team_membership_group_fk', + 'idx_audit_team_membership_user', + 'idx_audit_team_membership_user_fk', + 'idx_share_holder_group', + 'idx_share_holder_group_entry_issuer', + ]); + }); + + it('keeps an audit row after its group is deleted, blanking only the FK', async () => { + // 0043 leaves `PRAGMA foreign_keys = ON`, so this is real behaviour. + const [group] = (await client.read( + 'SELECT MIN(`id`) AS id FROM `group`', + )) as { id: number }[]; + const [user] = (await client.read( + 'SELECT MIN(`id`) AS id FROM `user`', + )) as { id: number }[]; + + await client.write( + 'INSERT INTO `audit_team_membership` ' + + '(`group_id`, `group_id_keep`, `user_id`, `user_id_keep`, ' + + '`actor_user_id`, `action`) VALUES (?, ?, ?, ?, ?, ?)', + [group.id, group.id, user.id, user.id, user.id, 'reset_member_password'], + ); + + await client.write('DELETE FROM `group` WHERE `id` = ?', [group.id]); + + await expect( + client.read( + 'SELECT `group_id`, `group_id_keep`, `action` FROM `audit_team_membership`', + ), + ).resolves.toEqual([ + { + group_id: null, + group_id_keep: group.id, + action: 'reset_member_password', + }, + ]); + }); + + it('constrains team shares that the user-holder index cannot', async () => { + const [user] = (await client.read( + 'SELECT MIN(`id`) AS id FROM `user`', + )) as { id: number }[]; + const groups = (await client.read( + 'SELECT `id` FROM `group` ORDER BY `id` LIMIT 2', + )) as { id: number }[]; + + await client.write( + 'INSERT INTO `fsentries` (`uuid`, `name`, `user_id`, `modified`) ' + + 'VALUES (?, ?, ?, ?)', + ['11111111-1111-4111-8111-111111111111', 'shared.txt', user.id, 0], + ); + const [entry] = (await client.read( + 'SELECT `id` FROM `fsentries` WHERE `uuid` = ?', + ['11111111-1111-4111-8111-111111111111'], + )) as { id: number }[]; + + const insertShare = (uid: string, groupId: number) => + client.write( + 'INSERT INTO `share` ' + + '(`uid`, `issuer_user_id`, `recipient_email`, `holder_user_id`, ' + + '`holder_group_id`, `fsentry_id`) VALUES (?, ?, ?, NULL, ?, ?)', + [uid, user.id, 'team@test.local', groupId, entry.id], + ); + + await insertShare('share-team-a', groups[0].id); + // Different group, same file and issuer -- allowed. + await insertShare('share-team-b', groups[1].id); + // Both have `holder_user_id` NULL, so the user-holder index permitted them. + await expect(insertShare('share-team-c', groups[0].id)).rejects.toThrow( + /UNIQUE/iu, + ); + }); + + it('matches a handle case-insensitively on lookup, not just on insert', async () => { + // Index-only NOCASE would leave `WHERE handle = ?` case-sensitive here while + // mysql's utf8mb4_unicode_ci column matched -- one query, two behaviours. + await client.write( + 'UPDATE `group` SET `handle` = ? WHERE `id` = (SELECT MIN(`id`) FROM `group`)', + ['design-team'], + ); + await expect( + client.read('SELECT `handle` FROM `group` WHERE `handle` = ?', [ + 'Design-Team', + ]), + ).resolves.toEqual([{ handle: 'design-team' }]); + }); + it('runs the javascript migrations, not just the .sql ones', async () => { // The `system` user only exists because 0025 (a .dbmig.js file) ran // inside the migration VM. diff --git a/src/backend/clients/database/SqliteDatabaseClient.ts b/src/backend/clients/database/SqliteDatabaseClient.ts index 338f78edd..7dce5ff9a 100644 --- a/src/backend/clients/database/SqliteDatabaseClient.ts +++ b/src/backend/clients/database/SqliteDatabaseClient.ts @@ -108,6 +108,7 @@ const AVAILABLE_MIGRATIONS: [number, string[]][] = [ [71, ['0076_event-handlers.sql']], [72, ['0077_teams.sql']], [73, ['0078_jct-user-group-pair-unique.sql']], + [74, ['0079_team-audit-and-group-shares.sql']], ]; export class SqliteDatabaseClient extends AbstractDatabaseClient { diff --git a/src/backend/clients/database/migrations/mysql/mysql_mig_34.sql b/src/backend/clients/database/migrations/mysql/mysql_mig_34.sql new file mode 100644 index 000000000..75b614efa --- /dev/null +++ b/src/backend/clients/database/migrations/mysql/mysql_mig_34.sql @@ -0,0 +1,84 @@ +-- Copyright (C) 2024-present Puter Technologies Inc. +-- +-- This file is part of Puter. +-- +-- Puter is free software: you can redistribute it and/or modify +-- it under the terms of the GNU Affero General Public License as published +-- by the Free Software Foundation, either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU Affero General Public License for more details. +-- +-- You should have received a copy of the GNU Affero General Public License +-- along with this program. If not, see . + +-- See sqlite/0076. Types follow mysql_mig_1; the guarded procedure follows mysql_mig_22. + +CREATE TABLE IF NOT EXISTS `audit_team_membership` ( + `id` int unsigned NOT NULL AUTO_INCREMENT, + `group_id` int unsigned DEFAULT NULL, + `group_id_keep` int unsigned NOT NULL, + `user_id` int unsigned DEFAULT NULL, + `user_id_keep` int unsigned NOT NULL, + `actor_user_id` int unsigned DEFAULT NULL, + `action` varchar(255) COLLATE utf8mb4_unicode_ci NOT NULL, + `reason` varchar(255) COLLATE utf8mb4_unicode_ci DEFAULT NULL, + `created_at` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP, + PRIMARY KEY (`id`), + KEY `idx_audit_team_membership_group` (`group_id_keep`, `id`), + KEY `idx_audit_team_membership_user` (`user_id_keep`, `id`), + KEY `idx_audit_team_membership_group_fk` (`group_id`), + KEY `idx_audit_team_membership_user_fk` (`user_id`), + KEY `idx_audit_team_membership_actor_fk` (`actor_user_id`), + -- SET NULL, never CASCADE: deleting an account must not erase what was done to it. + CONSTRAINT `fk_audit_team_membership_group` FOREIGN KEY (`group_id`) + REFERENCES `group` (`id`) ON DELETE SET NULL ON UPDATE CASCADE, + CONSTRAINT `fk_audit_team_membership_user` FOREIGN KEY (`user_id`) + REFERENCES `user` (`id`) ON DELETE SET NULL ON UPDATE CASCADE, + CONSTRAINT `fk_audit_team_membership_actor` FOREIGN KEY (`actor_user_id`) + REFERENCES `user` (`id`) ON DELETE SET NULL ON UPDATE CASCADE +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; + +CALL _puter_add_col('share', 'holder_group_id', '`holder_group_id` int unsigned DEFAULT NULL'); + +DROP PROCEDURE IF EXISTS _puter_add_group_share_constraints; + +DELIMITER // +CREATE PROCEDURE _puter_add_group_share_constraints() +BEGIN + IF NOT EXISTS ( + SELECT 1 FROM INFORMATION_SCHEMA.STATISTICS + WHERE TABLE_SCHEMA = DATABASE() + AND TABLE_NAME = 'share' + AND INDEX_NAME = 'idx_share_holder_group' + ) THEN + ALTER TABLE `share` ADD INDEX `idx_share_holder_group` (`holder_group_id`, `id`); + END IF; + -- Team shares leave `holder_user_id` NULL, so the existing unique index binds none. + IF NOT EXISTS ( + SELECT 1 FROM INFORMATION_SCHEMA.STATISTICS + WHERE TABLE_SCHEMA = DATABASE() + AND TABLE_NAME = 'share' + AND INDEX_NAME = 'idx_share_holder_group_entry_issuer' + ) THEN + ALTER TABLE `share` ADD UNIQUE INDEX `idx_share_holder_group_entry_issuer` + (`holder_group_id`, `fsentry_id`, `issuer_user_id`); + END IF; + IF NOT EXISTS ( + SELECT 1 FROM INFORMATION_SCHEMA.TABLE_CONSTRAINTS + WHERE TABLE_SCHEMA = DATABASE() + AND TABLE_NAME = 'share' + AND CONSTRAINT_NAME = 'share_holder_group_fk' + ) THEN + ALTER TABLE `share` ADD CONSTRAINT `share_holder_group_fk` + FOREIGN KEY (`holder_group_id`) REFERENCES `group` (`id`) + ON DELETE CASCADE ON UPDATE CASCADE; + END IF; +END// +DELIMITER ; + +CALL _puter_add_group_share_constraints(); +DROP PROCEDURE IF EXISTS _puter_add_group_share_constraints; diff --git a/src/backend/clients/database/migrations/postgres/postgres_mig_23.sql b/src/backend/clients/database/migrations/postgres/postgres_mig_23.sql new file mode 100644 index 000000000..ef8b90d05 --- /dev/null +++ b/src/backend/clients/database/migrations/postgres/postgres_mig_23.sql @@ -0,0 +1,57 @@ +-- Copyright (C) 2024-present Puter Technologies Inc. +-- +-- This file is part of Puter. +-- +-- Puter is free software: you can redistribute it and/or modify +-- it under the terms of the GNU Affero General Public License as published +-- by the Free Software Foundation, either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU Affero General Public License for more details. +-- +-- You should have received a copy of the GNU Affero General Public License +-- along with this program. If not, see . + +-- See sqlite/0076. Types follow `audit_user_to_group_permissions` in postgres_mig_1. + +CREATE TABLE IF NOT EXISTS audit_team_membership ( + id integer GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY, + + -- SET NULL, never CASCADE: deleting an account must not erase what was done to it. + group_id integer REFERENCES "group" (id) ON DELETE SET NULL ON UPDATE CASCADE, + group_id_keep integer NOT NULL, + user_id integer REFERENCES "user" (id) ON DELETE SET NULL ON UPDATE CASCADE, + user_id_keep integer NOT NULL, + actor_user_id integer REFERENCES "user" (id) ON DELETE SET NULL ON UPDATE CASCADE, + + action varchar(255) NOT NULL, + reason varchar(255), + created_at timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP +); + +CREATE INDEX IF NOT EXISTS idx_audit_team_membership_group + ON audit_team_membership (group_id_keep, id); + +CREATE INDEX IF NOT EXISTS idx_audit_team_membership_user + ON audit_team_membership (user_id_keep, id); + +-- SET NULL has to find the child rows; postgres does not index FK columns for you. +CREATE INDEX IF NOT EXISTS idx_audit_team_membership_group_fk + ON audit_team_membership (group_id); +CREATE INDEX IF NOT EXISTS idx_audit_team_membership_user_fk + ON audit_team_membership (user_id); +CREATE INDEX IF NOT EXISTS idx_audit_team_membership_actor_fk + ON audit_team_membership (actor_user_id); + +ALTER TABLE share ADD COLUMN IF NOT EXISTS holder_group_id integer + REFERENCES "group" (id) ON DELETE CASCADE ON UPDATE CASCADE; + +CREATE INDEX IF NOT EXISTS idx_share_holder_group + ON share (holder_group_id, id); + +-- Default NULLS DISTINCT, matching the other engines, so this binds only team shares. +CREATE UNIQUE INDEX IF NOT EXISTS idx_share_holder_group_entry_issuer + ON share (holder_group_id, fsentry_id, issuer_user_id); diff --git a/src/backend/clients/database/migrations/sqlite/0079_team-audit-and-group-shares.sql b/src/backend/clients/database/migrations/sqlite/0079_team-audit-and-group-shares.sql new file mode 100644 index 000000000..73b226cda --- /dev/null +++ b/src/backend/clients/database/migrations/sqlite/0079_team-audit-and-group-shares.sql @@ -0,0 +1,66 @@ +-- Copyright (C) 2024-present Puter Technologies Inc. +-- +-- This file is part of Puter. +-- +-- Puter is free software: you can redistribute it and/or modify +-- it under the terms of the GNU Affero General Public License as published +-- by the Free Software Foundation, either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU Affero General Public License for more details. +-- +-- You should have received a copy of the GNU Affero General Public License +-- along with this program. If not, see . + +-- Insert-only record of what an admin did to an account, plus `share.holder_group_id`. + +CREATE TABLE IF NOT EXISTS `audit_team_membership` ( + "id" INTEGER PRIMARY KEY AUTOINCREMENT, + + -- Nullable FK beside a NOT NULL `_keep`, as `audit_user_to_group_permissions` since 0019. + "group_id" INTEGER DEFAULT NULL, + "group_id_keep" INTEGER NOT NULL, + "user_id" INTEGER DEFAULT NULL, + "user_id_keep" INTEGER NOT NULL, + "actor_user_id" INTEGER DEFAULT NULL, + + "action" TEXT NOT NULL, + "reason" TEXT DEFAULT NULL, + "created_at" TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + + -- SET NULL, never CASCADE: deleting an account must not erase what was done to it. + FOREIGN KEY("group_id") REFERENCES "group" ("id") ON DELETE SET NULL ON UPDATE CASCADE, + FOREIGN KEY("user_id") REFERENCES "user" ("id") ON DELETE SET NULL ON UPDATE CASCADE, + FOREIGN KEY("actor_user_id") REFERENCES "user" ("id") ON DELETE SET NULL ON UPDATE CASCADE +); + +-- The admin's view: one workspace, newest first. +CREATE INDEX IF NOT EXISTS `idx_audit_team_membership_group` + ON `audit_team_membership` (`group_id_keep`, `id`); + +-- The member's view; the only place a reset becomes visible to its subject. +CREATE INDEX IF NOT EXISTS `idx_audit_team_membership_user` + ON `audit_team_membership` (`user_id_keep`, `id`); + +-- Without these, SET NULL scans the audit table on every user or group delete. +CREATE INDEX IF NOT EXISTS `idx_audit_team_membership_group_fk` + ON `audit_team_membership` (`group_id`); +CREATE INDEX IF NOT EXISTS `idx_audit_team_membership_user_fk` + ON `audit_team_membership` (`user_id`); +CREATE INDEX IF NOT EXISTS `idx_audit_team_membership_actor_fk` + ON `audit_team_membership` (`actor_user_id`); + +-- Mirrors `holder_user_id` from 0067: a `share` row is a listing entry, not the grant. +ALTER TABLE `share` ADD COLUMN `holder_group_id` INTEGER DEFAULT NULL + REFERENCES `group` (`id`) ON DELETE CASCADE ON UPDATE CASCADE; + +CREATE INDEX IF NOT EXISTS `idx_share_holder_group` + ON `share` (`holder_group_id`, `id`); + +-- Team shares leave `holder_user_id` NULL, and NULLs are distinct, so the existing +-- unique index binds none of them. +CREATE UNIQUE INDEX IF NOT EXISTS `idx_share_holder_group_entry_issuer` + ON `share` (`holder_group_id`, `fsentry_id`, `issuer_user_id`);