From 6fd6e944e6b1f41a90728aff041f2f015584d21f Mon Sep 17 00:00:00 2001 From: Milan Freml Date: Fri, 24 Feb 2023 14:04:57 +0100 Subject: [PATCH 1/2] [feat] Adding a few DB triggers to delete permissions when items are soft deleted - user - user_external_account - repo All of these would leave rows in the user_repo_permissions table, which is not desired. The user_external_account one was the most problematic from the point of view of behavior of authzQuery. This only affects the new unified permissions table, old behavior is not affected. --- .../down.sql | 8 ++++ .../metadata.yaml | 2 + .../up.sql | 38 +++++++++++++++++++ 3 files changed, 48 insertions(+) create mode 100644 migrations/frontend/1677242688_add_triggers_for_soft_deleted_perms_entities/down.sql create mode 100644 migrations/frontend/1677242688_add_triggers_for_soft_deleted_perms_entities/metadata.yaml create mode 100644 migrations/frontend/1677242688_add_triggers_for_soft_deleted_perms_entities/up.sql diff --git a/migrations/frontend/1677242688_add_triggers_for_soft_deleted_perms_entities/down.sql b/migrations/frontend/1677242688_add_triggers_for_soft_deleted_perms_entities/down.sql new file mode 100644 index 000000000000..6118a8c70cf4 --- /dev/null +++ b/migrations/frontend/1677242688_add_triggers_for_soft_deleted_perms_entities/down.sql @@ -0,0 +1,8 @@ +DROP TRIGGER IF EXISTS trig_delete_user_repo_permissions_on_repo_soft_delete ON repo; +DROP FUNCTION IF EXISTS delete_user_repo_permissions_on_repo_soft_delete; + +DROP TRIGGER IF EXISTS trig_delete_user_repo_permissions_on_external_account_soft_delete ON user_external_accounts; +DROP FUNCTION IF EXISTS delete_user_repo_permissions_on_external_account_soft_delete; + +DROP TRIGGER IF EXISTS trig_delete_user_repo_permissions_on_user_soft_delete ON users; +DROP FUNCTION IF EXISTS delete_user_repo_permissions_on_user_soft_delete; diff --git a/migrations/frontend/1677242688_add_triggers_for_soft_deleted_perms_entities/metadata.yaml b/migrations/frontend/1677242688_add_triggers_for_soft_deleted_perms_entities/metadata.yaml new file mode 100644 index 000000000000..675fc503afa0 --- /dev/null +++ b/migrations/frontend/1677242688_add_triggers_for_soft_deleted_perms_entities/metadata.yaml @@ -0,0 +1,2 @@ +name: add_triggers_for_soft_deleted_perms_entities +parents: [1677073533] diff --git a/migrations/frontend/1677242688_add_triggers_for_soft_deleted_perms_entities/up.sql b/migrations/frontend/1677242688_add_triggers_for_soft_deleted_perms_entities/up.sql new file mode 100644 index 000000000000..ffe784bb4f7c --- /dev/null +++ b/migrations/frontend/1677242688_add_triggers_for_soft_deleted_perms_entities/up.sql @@ -0,0 +1,38 @@ +CREATE OR REPLACE FUNCTION delete_user_repo_permissions_on_repo_soft_delete() RETURNS trigger + LANGUAGE plpgsql + AS $$ BEGIN + IF NEW.deleted_at IS NOT NULL AND OLD.deleted_at IS NULL THEN + DELETE FROM user_repo_permissions WHERE repo_id = NEW.id; + END IF; + RETURN NULL; + END +$$; + +DROP TRIGGER IF EXISTS trig_delete_user_repo_permissions_on_repo_soft_delete ON repo; +CREATE TRIGGER trig_delete_user_repo_permissions_on_repo_soft_delete AFTER UPDATE ON repo FOR EACH ROW EXECUTE FUNCTION delete_user_repo_permissions_on_repo_soft_delete(); + +CREATE OR REPLACE FUNCTION delete_user_repo_permissions_on_external_account_soft_delete() RETURNS trigger + LANGUAGE plpgsql + AS $$ BEGIN + IF NEW.deleted_at IS NOT NULL AND OLD.deleted_at IS NULL THEN + DELETE FROM user_repo_permissions WHERE user_id = OLD.user_id AND user_external_account_id = OLD.id; + END IF; + RETURN NULL; + END +$$; + +DROP TRIGGER IF EXISTS trig_delete_user_repo_permissions_on_external_account_soft_delete ON user_external_accounts; +CREATE TRIGGER trig_delete_user_repo_permissions_on_external_account_soft_delete AFTER UPDATE ON user_external_accounts FOR EACH ROW EXECUTE FUNCTION delete_user_repo_permissions_on_external_account_soft_delete(); + +CREATE OR REPLACE FUNCTION delete_user_repo_permissions_on_user_soft_delete() RETURNS trigger + LANGUAGE plpgsql + AS $$ BEGIN + IF NEW.deleted_at IS NOT NULL AND OLD.deleted_at IS NULL THEN + DELETE FROM user_repo_permissions WHERE user_id = OLD.id; + END IF; + RETURN NULL; + END +$$; + +DROP TRIGGER IF EXISTS trig_delete_user_repo_permissions_on_user_soft_delete ON users; +CREATE TRIGGER trig_delete_user_repo_permissions_on_user_soft_delete AFTER UPDATE ON users FOR EACH ROW EXECUTE FUNCTION delete_user_repo_permissions_on_user_soft_delete(); From 6dea8bd27a6a0cbd9d92170dcf1b0c563d8c5c49 Mon Sep 17 00:00:00 2001 From: Milan Freml Date: Fri, 24 Feb 2023 16:51:23 +0100 Subject: [PATCH 2/2] Go generate --- internal/database/schema.json | 27 +++++++++++++++++++++++- internal/database/schema.md | 4 ++++ migrations/frontend/squashed.sql | 36 ++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 1 deletion(-) diff --git a/internal/database/schema.json b/internal/database/schema.json index ad1af90ab990..a00e677b1da7 100755 --- a/internal/database/schema.json +++ b/internal/database/schema.json @@ -92,6 +92,18 @@ "Name": "delete_repo_ref_on_external_service_repos", "Definition": "CREATE OR REPLACE FUNCTION public.delete_repo_ref_on_external_service_repos()\n RETURNS trigger\n LANGUAGE plpgsql\nAS $function$\n BEGIN\n -- if a repo is soft-deleted, delete every row that references that repo\n IF (OLD.deleted_at IS NULL AND NEW.deleted_at IS NOT NULL) THEN\n DELETE FROM\n external_service_repos\n WHERE\n repo_id = OLD.id;\n END IF;\n\n RETURN OLD;\n END;\n$function$\n" }, + { + "Name": "delete_user_repo_permissions_on_external_account_soft_delete", + "Definition": "CREATE OR REPLACE FUNCTION public.delete_user_repo_permissions_on_external_account_soft_delete()\n RETURNS trigger\n LANGUAGE plpgsql\nAS $function$ BEGIN\n IF NEW.deleted_at IS NOT NULL AND OLD.deleted_at IS NULL THEN\n \tDELETE FROM user_repo_permissions WHERE user_id = OLD.user_id AND user_external_account_id = OLD.id;\n END IF;\n RETURN NULL;\n END\n$function$\n" + }, + { + "Name": "delete_user_repo_permissions_on_repo_soft_delete", + "Definition": "CREATE OR REPLACE FUNCTION public.delete_user_repo_permissions_on_repo_soft_delete()\n RETURNS trigger\n LANGUAGE plpgsql\nAS $function$ BEGIN\n IF NEW.deleted_at IS NOT NULL AND OLD.deleted_at IS NULL THEN\n \tDELETE FROM user_repo_permissions WHERE repo_id = NEW.id;\n END IF;\n RETURN NULL;\n END\n$function$\n" + }, + { + "Name": "delete_user_repo_permissions_on_user_soft_delete", + "Definition": "CREATE OR REPLACE FUNCTION public.delete_user_repo_permissions_on_user_soft_delete()\n RETURNS trigger\n LANGUAGE plpgsql\nAS $function$ BEGIN\n IF NEW.deleted_at IS NOT NULL AND OLD.deleted_at IS NULL THEN\n \tDELETE FROM user_repo_permissions WHERE user_id = OLD.id;\n END IF;\n RETURN NULL;\n END\n$function$\n" + }, { "Name": "func_configuration_policies_delete", "Definition": "CREATE OR REPLACE FUNCTION public.func_configuration_policies_delete()\n RETURNS trigger\n LANGUAGE plpgsql\nAS $function$\n BEGIN\n UPDATE configuration_policies_audit_logs\n SET record_deleted_at = NOW()\n WHERE policy_id IN (\n SELECT id FROM OLD\n );\n\n RETURN NULL;\n END;\n$function$\n" @@ -19585,6 +19597,10 @@ "Name": "trig_delete_repo_ref_on_external_service_repos", "Definition": "CREATE TRIGGER trig_delete_repo_ref_on_external_service_repos AFTER UPDATE OF deleted_at ON repo FOR EACH ROW EXECUTE FUNCTION delete_repo_ref_on_external_service_repos()" }, + { + "Name": "trig_delete_user_repo_permissions_on_repo_soft_delete", + "Definition": "CREATE TRIGGER trig_delete_user_repo_permissions_on_repo_soft_delete AFTER UPDATE ON repo FOR EACH ROW EXECUTE FUNCTION delete_user_repo_permissions_on_repo_soft_delete()" + }, { "Name": "trig_recalc_repo_statistics_on_repo_delete", "Definition": "CREATE TRIGGER trig_recalc_repo_statistics_on_repo_delete AFTER DELETE ON repo REFERENCING OLD TABLE AS oldtab FOR EACH STATEMENT EXECUTE FUNCTION recalc_repo_statistics_on_repo_delete()" @@ -22233,7 +22249,12 @@ "ConstraintDefinition": "FOREIGN KEY (user_id) REFERENCES users(id)" } ], - "Triggers": [] + "Triggers": [ + { + "Name": "trig_delete_user_repo_permissions_on_external_account_soft_dele", + "Definition": "CREATE TRIGGER trig_delete_user_repo_permissions_on_external_account_soft_dele AFTER UPDATE ON user_external_accounts FOR EACH ROW EXECUTE FUNCTION delete_user_repo_permissions_on_external_account_soft_delete()" + } + ] }, { "Name": "user_pending_permissions", @@ -23116,6 +23137,10 @@ } ], "Triggers": [ + { + "Name": "trig_delete_user_repo_permissions_on_user_soft_delete", + "Definition": "CREATE TRIGGER trig_delete_user_repo_permissions_on_user_soft_delete AFTER UPDATE ON users FOR EACH ROW EXECUTE FUNCTION delete_user_repo_permissions_on_user_soft_delete()" + }, { "Name": "trig_invalidate_session_on_password_change", "Definition": "CREATE TRIGGER trig_invalidate_session_on_password_change BEFORE UPDATE OF passwd ON users FOR EACH ROW EXECUTE FUNCTION invalidate_session_for_userid_on_password_change()" diff --git a/internal/database/schema.md b/internal/database/schema.md index e12c126faa7d..6f80fefa021d 100755 --- a/internal/database/schema.md +++ b/internal/database/schema.md @@ -3007,6 +3007,7 @@ Referenced by: Triggers: trig_create_zoekt_repo_on_repo_insert AFTER INSERT ON repo FOR EACH ROW EXECUTE FUNCTION func_insert_zoekt_repo() trig_delete_repo_ref_on_external_service_repos AFTER UPDATE OF deleted_at ON repo FOR EACH ROW EXECUTE FUNCTION delete_repo_ref_on_external_service_repos() + trig_delete_user_repo_permissions_on_repo_soft_delete AFTER UPDATE ON repo FOR EACH ROW EXECUTE FUNCTION delete_user_repo_permissions_on_repo_soft_delete() trig_recalc_repo_statistics_on_repo_delete AFTER DELETE ON repo REFERENCING OLD TABLE AS oldtab FOR EACH STATEMENT EXECUTE FUNCTION recalc_repo_statistics_on_repo_delete() trig_recalc_repo_statistics_on_repo_insert AFTER INSERT ON repo REFERENCING NEW TABLE AS newtab FOR EACH STATEMENT EXECUTE FUNCTION recalc_repo_statistics_on_repo_insert() trig_recalc_repo_statistics_on_repo_update AFTER UPDATE ON repo REFERENCING OLD TABLE AS oldtab NEW TABLE AS newtab FOR EACH STATEMENT EXECUTE FUNCTION recalc_repo_statistics_on_repo_update() @@ -3475,6 +3476,8 @@ Foreign-key constraints: "user_external_accounts_user_id_fkey" FOREIGN KEY (user_id) REFERENCES users(id) Referenced by: TABLE "user_repo_permissions" CONSTRAINT "user_repo_permissions_user_external_account_id_fkey" FOREIGN KEY (user_external_account_id) REFERENCES user_external_accounts(id) ON DELETE CASCADE +Triggers: + trig_delete_user_repo_permissions_on_external_account_soft_dele AFTER UPDATE ON user_external_accounts FOR EACH ROW EXECUTE FUNCTION delete_user_repo_permissions_on_external_account_soft_delete() ``` @@ -3668,6 +3671,7 @@ Referenced by: TABLE "webhooks" CONSTRAINT "webhooks_created_by_user_id_fkey" FOREIGN KEY (created_by_user_id) REFERENCES users(id) ON DELETE SET NULL TABLE "webhooks" CONSTRAINT "webhooks_updated_by_user_id_fkey" FOREIGN KEY (updated_by_user_id) REFERENCES users(id) ON DELETE SET NULL Triggers: + trig_delete_user_repo_permissions_on_user_soft_delete AFTER UPDATE ON users FOR EACH ROW EXECUTE FUNCTION delete_user_repo_permissions_on_user_soft_delete() trig_invalidate_session_on_password_change BEFORE UPDATE OF passwd ON users FOR EACH ROW EXECUTE FUNCTION invalidate_session_for_userid_on_password_change() trig_soft_delete_user_reference_on_external_service AFTER UPDATE OF deleted_at ON users FOR EACH ROW EXECUTE FUNCTION soft_delete_user_reference_on_external_service() diff --git a/migrations/frontend/squashed.sql b/migrations/frontend/squashed.sql index b42a9a24f672..b445108df55f 100755 --- a/migrations/frontend/squashed.sql +++ b/migrations/frontend/squashed.sql @@ -164,6 +164,36 @@ CREATE FUNCTION delete_repo_ref_on_external_service_repos() RETURNS trigger END; $$; +CREATE FUNCTION delete_user_repo_permissions_on_external_account_soft_delete() RETURNS trigger + LANGUAGE plpgsql + AS $$ BEGIN + IF NEW.deleted_at IS NOT NULL AND OLD.deleted_at IS NULL THEN + DELETE FROM user_repo_permissions WHERE user_id = OLD.user_id AND user_external_account_id = OLD.id; + END IF; + RETURN NULL; + END +$$; + +CREATE FUNCTION delete_user_repo_permissions_on_repo_soft_delete() RETURNS trigger + LANGUAGE plpgsql + AS $$ BEGIN + IF NEW.deleted_at IS NOT NULL AND OLD.deleted_at IS NULL THEN + DELETE FROM user_repo_permissions WHERE repo_id = NEW.id; + END IF; + RETURN NULL; + END +$$; + +CREATE FUNCTION delete_user_repo_permissions_on_user_soft_delete() RETURNS trigger + LANGUAGE plpgsql + AS $$ BEGIN + IF NEW.deleted_at IS NOT NULL AND OLD.deleted_at IS NULL THEN + DELETE FROM user_repo_permissions WHERE user_id = OLD.id; + END IF; + RETURN NULL; + END +$$; + CREATE FUNCTION func_configuration_policies_delete() RETURNS trigger LANGUAGE plpgsql AS $$ @@ -5239,6 +5269,12 @@ CREATE TRIGGER trig_delete_batch_change_reference_on_changesets AFTER DELETE ON CREATE TRIGGER trig_delete_repo_ref_on_external_service_repos AFTER UPDATE OF deleted_at ON repo FOR EACH ROW EXECUTE FUNCTION delete_repo_ref_on_external_service_repos(); +CREATE TRIGGER trig_delete_user_repo_permissions_on_external_account_soft_dele AFTER UPDATE ON user_external_accounts FOR EACH ROW EXECUTE FUNCTION delete_user_repo_permissions_on_external_account_soft_delete(); + +CREATE TRIGGER trig_delete_user_repo_permissions_on_repo_soft_delete AFTER UPDATE ON repo FOR EACH ROW EXECUTE FUNCTION delete_user_repo_permissions_on_repo_soft_delete(); + +CREATE TRIGGER trig_delete_user_repo_permissions_on_user_soft_delete AFTER UPDATE ON users FOR EACH ROW EXECUTE FUNCTION delete_user_repo_permissions_on_user_soft_delete(); + CREATE TRIGGER trig_invalidate_session_on_password_change BEFORE UPDATE OF passwd ON users FOR EACH ROW EXECUTE FUNCTION invalidate_session_for_userid_on_password_change(); CREATE TRIGGER trig_recalc_gitserver_repos_statistics_on_delete AFTER DELETE ON gitserver_repos REFERENCING OLD TABLE AS oldtab FOR EACH STATEMENT EXECUTE FUNCTION recalc_gitserver_repos_statistics_on_delete();