fix: filter out deleted users when attempting to delete an organization (#17621)

Closes [coder/internal#601](https://github.com/coder/internal/issues/601)
2025-07-15 22:20:27 +00:00 · 2025-05-01 12:26:01 -04:00
parent cae4fa8b45
commit b7e08ba7c9
6 changed files with 319 additions and 11 deletions
--- a/coderd/database/dump.sql
+++ b/coderd/database/dump.sql
@ -482,9 +482,14 @@ BEGIN
    );

    member_count := (
-        SELECT count(*) as count FROM organization_members
+        SELECT
+            count(*) AS count
+        FROM
+            organization_members
+        LEFT JOIN users ON users.id = organization_members.user_id
        WHERE
            organization_members.organization_id = OLD.id
+            AND users.deleted = FALSE
    );

    provisioner_keys_count := (
--- a/coderd/database/migrations/000318_update_protect_deleting_orgs_to_filter_deleted_users.down.sql
+++ b/coderd/database/migrations/000318_update_protect_deleting_orgs_to_filter_deleted_users.down.sql
@ -0,0 +1,96 @@
+DROP TRIGGER IF EXISTS protect_deleting_organizations ON organizations;
+
+-- Replace the function with the new implementation
+CREATE OR REPLACE FUNCTION protect_deleting_organizations()
+    RETURNS TRIGGER AS
+$$
+DECLARE
+    workspace_count int;
+    template_count int;
+    group_count int;
+    member_count int;
+    provisioner_keys_count int;
+BEGIN
+    workspace_count := (
+        SELECT count(*) as count FROM workspaces
+        WHERE
+            workspaces.organization_id = OLD.id
+            AND workspaces.deleted = false
+    );
+
+    template_count := (
+        SELECT count(*) as count FROM templates
+        WHERE
+            templates.organization_id = OLD.id
+            AND templates.deleted = false
+    );
+
+    group_count := (
+        SELECT count(*) as count FROM groups
+        WHERE
+            groups.organization_id = OLD.id
+    );
+
+    member_count := (
+        SELECT count(*) as count FROM organization_members
+        WHERE
+            organization_members.organization_id = OLD.id
+    );
+
+    provisioner_keys_count := (
+        Select count(*) as count FROM provisioner_keys
+        WHERE
+            provisioner_keys.organization_id = OLD.id
+    );
+
+    -- Fail the deletion if one of the following:
+    -- * the organization has 1 or more workspaces
+    -- * the organization has 1 or more templates
+    -- * the organization has 1 or more groups other than "Everyone" group
+    -- * the organization has 1 or more members other than the organization owner
+    -- * the organization has 1 or more provisioner keys
+
+    -- Only create error message for resources that actually exist
+    IF (workspace_count + template_count + provisioner_keys_count) > 0 THEN
+        DECLARE
+            error_message text := 'cannot delete organization: organization has ';
+            error_parts text[] := '{}';
+        BEGIN
+            IF workspace_count > 0 THEN
+                error_parts := array_append(error_parts, workspace_count || ' workspaces');
+            END IF;
+            
+            IF template_count > 0 THEN
+                error_parts := array_append(error_parts, template_count || ' templates');
+            END IF;
+            
+            IF provisioner_keys_count > 0 THEN
+                error_parts := array_append(error_parts, provisioner_keys_count || ' provisioner keys');
+            END IF;
+            
+            error_message := error_message || array_to_string(error_parts, ', ') || ' that must be deleted first';
+            RAISE EXCEPTION '%', error_message;
+        END;
+    END IF;
+
+    IF (group_count) > 1 THEN
+            RAISE EXCEPTION 'cannot delete organization: organization has % groups that must be deleted first', group_count - 1;
+    END IF;
+
+    -- Allow 1 member to exist, because you cannot remove yourself. You can
+    -- remove everyone else. Ideally, we only omit the member that matches
+    -- the user_id of the caller, however in a trigger, the caller is unknown.
+    IF (member_count) > 1 THEN
+            RAISE EXCEPTION 'cannot delete organization: organization has % members that must be deleted first', member_count - 1;
+    END IF;
+
+    RETURN NEW;
+END;
+$$ LANGUAGE plpgsql;
+
+-- Trigger to protect organizations from being soft deleted with existing resources
+CREATE TRIGGER protect_deleting_organizations
+    BEFORE UPDATE ON organizations
+    FOR EACH ROW
+    WHEN (NEW.deleted = true AND OLD.deleted = false)
+    EXECUTE FUNCTION protect_deleting_organizations();
--- a/coderd/database/migrations/000318_update_protect_deleting_orgs_to_filter_deleted_users.up.sql
+++ b/coderd/database/migrations/000318_update_protect_deleting_orgs_to_filter_deleted_users.up.sql
@ -0,0 +1,101 @@
+DROP TRIGGER IF EXISTS protect_deleting_organizations ON organizations;
+
+-- Replace the function with the new implementation
+CREATE OR REPLACE FUNCTION protect_deleting_organizations()
+    RETURNS TRIGGER AS
+$$
+DECLARE
+    workspace_count int;
+    template_count int;
+    group_count int;
+    member_count int;
+    provisioner_keys_count int;
+BEGIN
+    workspace_count := (
+        SELECT count(*) as count FROM workspaces
+        WHERE
+            workspaces.organization_id = OLD.id
+            AND workspaces.deleted = false
+    );
+
+    template_count := (
+        SELECT count(*) as count FROM templates
+        WHERE
+            templates.organization_id = OLD.id
+            AND templates.deleted = false
+    );
+
+    group_count := (
+        SELECT count(*) as count FROM groups
+        WHERE
+            groups.organization_id = OLD.id
+    );
+
+    member_count := (
+        SELECT
+            count(*) AS count
+        FROM
+            organization_members
+        LEFT JOIN users ON users.id = organization_members.user_id
+        WHERE
+            organization_members.organization_id = OLD.id
+            AND users.deleted = FALSE
+    );
+
+    provisioner_keys_count := (
+        Select count(*) as count FROM provisioner_keys
+        WHERE
+            provisioner_keys.organization_id = OLD.id
+    );
+
+    -- Fail the deletion if one of the following:
+    -- * the organization has 1 or more workspaces
+    -- * the organization has 1 or more templates
+    -- * the organization has 1 or more groups other than "Everyone" group
+    -- * the organization has 1 or more members other than the organization owner
+    -- * the organization has 1 or more provisioner keys
+
+    -- Only create error message for resources that actually exist
+    IF (workspace_count + template_count + provisioner_keys_count) > 0 THEN
+        DECLARE
+            error_message text := 'cannot delete organization: organization has ';
+            error_parts text[] := '{}';
+        BEGIN
+            IF workspace_count > 0 THEN
+                error_parts := array_append(error_parts, workspace_count || ' workspaces');
+            END IF;
+            
+            IF template_count > 0 THEN
+                error_parts := array_append(error_parts, template_count || ' templates');
+            END IF;
+            
+            IF provisioner_keys_count > 0 THEN
+                error_parts := array_append(error_parts, provisioner_keys_count || ' provisioner keys');
+            END IF;
+            
+            error_message := error_message || array_to_string(error_parts, ', ') || ' that must be deleted first';
+            RAISE EXCEPTION '%', error_message;
+        END;
+    END IF;
+
+    IF (group_count) > 1 THEN
+            RAISE EXCEPTION 'cannot delete organization: organization has % groups that must be deleted first', group_count - 1;
+    END IF;
+
+    -- Allow 1 member to exist, because you cannot remove yourself. You can
+    -- remove everyone else. Ideally, we only omit the member that matches
+    -- the user_id of the caller, however in a trigger, the caller is unknown.
+    IF (member_count) > 1 THEN
+            RAISE EXCEPTION 'cannot delete organization: organization has % members that must be deleted first', member_count - 1;
+    END IF;
+
+    RETURN NEW;
+END;
+$$ LANGUAGE plpgsql;
+
+-- Trigger to protect organizations from being soft deleted with existing resources
+CREATE TRIGGER protect_deleting_organizations
+    BEFORE UPDATE ON organizations
+    FOR EACH ROW
+    WHEN (NEW.deleted = true AND OLD.deleted = false)
+    EXECUTE FUNCTION protect_deleting_organizations();
--- a/coderd/database/querier_test.go
+++ b/coderd/database/querier_test.go
@ -3586,6 +3586,43 @@ func TestOrganizationDeleteTrigger(t *testing.T) {
 		require.ErrorContains(t, err, "cannot delete organization")
 		require.ErrorContains(t, err, "has 1 members")
 	})
+
+	t.Run("UserDeletedButNotRemovedFromOrg", func(t *testing.T) {
+		t.Parallel()
+		db, _ := dbtestutil.NewDB(t)
+
+		orgA := dbfake.Organization(t, db).Do()
+
+		userA := dbgen.User(t, db, database.User{})
+		userB := dbgen.User(t, db, database.User{})
+		userC := dbgen.User(t, db, database.User{})
+
+		dbgen.OrganizationMember(t, db, database.OrganizationMember{
+			OrganizationID: orgA.Org.ID,
+			UserID:         userA.ID,
+		})
+		dbgen.OrganizationMember(t, db, database.OrganizationMember{
+			OrganizationID: orgA.Org.ID,
+			UserID:         userB.ID,
+		})
+		dbgen.OrganizationMember(t, db, database.OrganizationMember{
+			OrganizationID: orgA.Org.ID,
+			UserID:         userC.ID,
+		})
+
+		// Delete one of the users but don't remove them from the org
+		ctx := testutil.Context(t, testutil.WaitShort)
+		db.UpdateUserDeletedByID(ctx, userB.ID)
+
+		err := db.UpdateOrganizationDeletedByID(ctx, database.UpdateOrganizationDeletedByIDParams{
+			UpdatedAt: dbtime.Now(),
+			ID:        orgA.Org.ID,
+		})
+		require.Error(t, err)
+		// cannot delete organization: organization has 1 members that must be deleted first
+		require.ErrorContains(t, err, "cannot delete organization")
+		require.ErrorContains(t, err, "has 1 members")
+	})
 }

 type templateVersionWithPreset struct {
--- a/coderd/database/queries.sql.go
+++ b/coderd/database/queries.sql.go
@ -5586,11 +5586,45 @@ func (q *sqlQuerier) GetOrganizationByName(ctx context.Context, arg GetOrganizat

 const getOrganizationResourceCountByID = `-- name: GetOrganizationResourceCountByID :one
 SELECT
-    (SELECT COUNT(*) FROM workspaces WHERE workspaces.organization_id = $1 AND workspaces.deleted = false) AS workspace_count,
-    (SELECT COUNT(*) FROM groups WHERE groups.organization_id = $1) AS group_count,
-    (SELECT COUNT(*) FROM templates WHERE templates.organization_id = $1 AND templates.deleted = false) AS template_count,
-    (SELECT COUNT(*) FROM organization_members WHERE organization_members.organization_id = $1) AS member_count,
-    (SELECT COUNT(*) FROM provisioner_keys WHERE provisioner_keys.organization_id = $1) AS provisioner_key_count
+	(
+		SELECT
+			count(*)
+		FROM
+			workspaces
+		WHERE
+			workspaces.organization_id = $1
+			AND workspaces.deleted = FALSE) AS workspace_count,
+	(
+		SELECT
+			count(*)
+		FROM
+			GROUPS
+		WHERE
+			groups.organization_id = $1) AS group_count,
+	(
+		SELECT
+			count(*)
+		FROM
+			templates
+		WHERE
+			templates.organization_id = $1
+			AND templates.deleted = FALSE) AS template_count,
+	(
+		SELECT
+			count(*)
+		FROM
+			organization_members
+		LEFT JOIN users ON organization_members.user_id = users.id
+	WHERE
+		organization_members.organization_id = $1
+		AND users.deleted = FALSE) AS member_count,
+(
+	SELECT
+		count(*)
+	FROM
+		provisioner_keys
+	WHERE
+		provisioner_keys.organization_id = $1) AS provisioner_key_count
 `

 type GetOrganizationResourceCountByIDRow struct {
--- a/coderd/database/queries/organizations.sql
+++ b/coderd/database/queries/organizations.sql
@ -73,11 +73,46 @@ WHERE

 -- name: GetOrganizationResourceCountByID :one
 SELECT
-    (SELECT COUNT(*) FROM workspaces WHERE workspaces.organization_id = $1 AND workspaces.deleted = false) AS workspace_count,
-    (SELECT COUNT(*) FROM groups WHERE groups.organization_id = $1) AS group_count,
-    (SELECT COUNT(*) FROM templates WHERE templates.organization_id = $1 AND templates.deleted = false) AS template_count,
-    (SELECT COUNT(*) FROM organization_members WHERE organization_members.organization_id = $1) AS member_count,
-    (SELECT COUNT(*) FROM provisioner_keys WHERE provisioner_keys.organization_id = $1) AS provisioner_key_count;
+	(
+		SELECT
+			count(*)
+		FROM
+			workspaces
+		WHERE
+			workspaces.organization_id = $1
+			AND workspaces.deleted = FALSE) AS workspace_count,
+	(
+		SELECT
+			count(*)
+		FROM
+			GROUPS
+		WHERE
+			groups.organization_id = $1) AS group_count,
+	(
+		SELECT
+			count(*)
+		FROM
+			templates
+		WHERE
+			templates.organization_id = $1
+			AND templates.deleted = FALSE) AS template_count,
+	(
+		SELECT
+			count(*)
+		FROM
+			organization_members
+		LEFT JOIN users ON organization_members.user_id = users.id
+	WHERE
+		organization_members.organization_id = $1
+		AND users.deleted = FALSE) AS member_count,
+(
+	SELECT
+		count(*)
+	FROM
+		provisioner_keys
+	WHERE
+		provisioner_keys.organization_id = $1) AS provisioner_key_count;
+

 -- name: InsertOrganization :one
 INSERT INTO