Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion coderd/database/dump.sql

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -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();
Original file line number Diff line number Diff line change
@@ -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();
37 changes: 37 additions & 0 deletions coderd/database/querier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
44 changes: 39 additions & 5 deletions coderd/database/queries.sql.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

45 changes: 40 additions & 5 deletions coderd/database/queries/organizations.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading