Feb 20, 2024 · Feb 12, 2024 · Feb 12, 2024 · Feb 12, 2024 · Feb 12, 2024 · Feb 12, 2024
diff --git a/cli/delete_test.go b/cli/delete_test.go

 "github.com/coder/coder/v2/cli/clitest"
 "github.com/coder/coder/v2/coderd/coderdtest"
 "github.com/coder/coder/v2/coderd/database"
 "github.com/coder/coder/v2/coderd/database/dbauthz"
 "github.com/coder/coder/v2/codersdk"
 "github.com/coder/coder/v2/pty/ptytest"
 // this way.
 ctx := testutil.Context(t, testutil.WaitShort)
 // nolint:gocritic // Unit test
 err := api.Database.UpdateUserDeletedByID(dbauthz.AsSystemRestricted(ctx), database.UpdateUserDeletedByIDParams{
 ID:      deleteMeUser.ID,
 Deleted: true,
 })
 err := api.Database.UpdateUserDeletedByID(dbauthz.AsSystemRestricted(ctx), deleteMeUser.ID)
 require.NoError(t, err)

 inv, root := clitest.New(t, "delete", fmt.Sprintf("%s/%s", deleteMeUser.ID, workspace.Name), "-y", "--orphan")
diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go
 return deleteQ(q.log, q.auth, q.db.GetTemplateByID, deleteF)(ctx, id)
 }

 func (q *querier) SoftDeleteUserByID(ctx context.Context, id uuid.UUID) error {
 deleteF := func(ctx context.Context, id uuid.UUID) error {
 return q.db.UpdateUserDeletedByID(ctx, database.UpdateUserDeletedByIDParams{
 ID:      id,
 Deleted: true,
 })
 }
 return deleteQ(q.log, q.auth, q.db.GetUserByID, deleteF)(ctx, id)
 }

 func (q *querier) SoftDeleteWorkspaceByID(ctx context.Context, id uuid.UUID) error {
 return deleteQ(q.log, q.auth, q.db.GetWorkspaceByID, func(ctx context.Context, id uuid.UUID) error {
 return q.db.UpdateWorkspaceDeletedByID(ctx, database.UpdateWorkspaceDeletedByIDParams{
 return q.db.UpdateUserAppearanceSettings(ctx, arg)
 }

 // UpdateUserDeletedByID
 // Deprecated: Delete this function in favor of 'SoftDeleteUserByID'. Deletes are
 // irreversible.
 func (q *querier) UpdateUserDeletedByID(ctx context.Context, arg database.UpdateUserDeletedByIDParams) error {
 fetch := func(ctx context.Context, arg database.UpdateUserDeletedByIDParams) (database.User, error) {
 return q.db.GetUserByID(ctx, arg.ID)
 }
 // This uses the rbac.ActionDelete action always as this function should always delete.
 // We should delete this function in favor of 'SoftDeleteUserByID'.
 return deleteQ(q.log, q.auth, fetch, q.db.UpdateUserDeletedByID)(ctx, arg)
 func (q *querier) UpdateUserDeletedByID(ctx context.Context, id uuid.UUID) error {
 return deleteQ(q.log, q.auth, q.db.GetUserByID, q.db.UpdateUserDeletedByID)(ctx, id)
 }

 func (q *querier) UpdateUserHashedPassword(ctx context.Context, arg database.UpdateUserHashedPasswordParams) error {
diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go
 LoginType: database.LoginTypeOIDC,
 }).Asserts(u, rbac.ActionUpdate)
 }))
 s.Run("SoftDeleteUserByID", s.Subtest(func(db database.Store, check *expects) {
 s.Run("UpdateUserDeletedByID", s.Subtest(func(db database.Store, check *expects) {
 u := dbgen.User(s.T(), db, database.User{})
 check.Args(u.ID).Asserts(u, rbac.ActionDelete).Returns()
 }))
 s.Run("UpdateUserDeletedByID", s.Subtest(func(db database.Store, check *expects) {
 u := dbgen.User(s.T(), db, database.User{Deleted: true})
 check.Args(database.UpdateUserDeletedByIDParams{
 ID:      u.ID,
 Deleted: true,
 }).Asserts(u, rbac.ActionDelete).Returns()
 }))
 s.Run("UpdateUserHashedPassword", s.Subtest(func(db database.Store, check *expects) {
 u := dbgen.User(s.T(), db, database.User{})
 check.Args(database.UpdateUserHashedPasswordParams{
diff --git a/coderd/database/dbgen/dbgen.go b/coderd/database/dbgen/dbgen.go
 }

 if orig.Deleted {
 err = db.UpdateUserDeletedByID(genCtx, database.UpdateUserDeletedByIDParams{
 ID:      user.ID,
 Deleted: orig.Deleted,
 })
 err = db.UpdateUserDeletedByID(genCtx, user.ID)
 require.NoError(t, err, "set user as deleted")
 }
 return user
diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go
 return reflect.ValueOf(v).FieldByName("Valid").Bool()
 }

 // Took the error from the real database.
 var deletedUserLinkError = &pq.Error{
 Severity: "ERROR",
 // "raise_exception" error
 Code:    "P0001",
 Message: "Cannot create user_link for deleted user",
 Where:   "PL/pgSQL function insert_user_links_fail_if_user_deleted() line 7 at RAISE",
 File:    "pl_exec.c",
 Line:    "3864",
 Routine: "exec_stmt_raise",
 }

 func (*FakeQuerier) AcquireLock(_ context.Context, _ int64) error {
 return xerrors.New("AcquireLock must only be called within a transaction")
 }
 q.mutex.Lock()
 defer q.mutex.Unlock()

 if u, err := q.getUserByIDNoLock(args.UserID); err == nil && u.Deleted {
 return database.UserLink{}, deletedUserLinkError
 }

 //nolint:gosimple
 link := database.UserLink{
 UserID:                 args.UserID,
 return database.User{}, sql.ErrNoRows
 }

 func (q *FakeQuerier) UpdateUserDeletedByID(_ context.Context, params database.UpdateUserDeletedByIDParams) error {
 if err := validateDatabaseType(params); err != nil {
 return err
 }

 func (q *FakeQuerier) UpdateUserDeletedByID(_ context.Context, id uuid.UUID) error {
 q.mutex.Lock()
 defer q.mutex.Unlock()

 for i, u := range q.users {
 if u.ID ==params.ID {
 u.Deleted =params.Deleted
 if u.ID ==id {
 u.Deleted =true
 q.users[i] = u
 // NOTE: In the real world, this is done by a trigger.
 i := 0
 for {
 if i >= len(q.apiKeys) {
 break
 }
 k := q.apiKeys[i]
 if k.UserID == u.ID {
 q.apiKeys[i] = q.apiKeys[len(q.apiKeys)-1]
 q.apiKeys = q.apiKeys[:len(q.apiKeys)-1]
 // We removed an element, so decrement
 i--
 }
 i++
 }
 q.apiKeys = slices.DeleteFunc(q.apiKeys, func(u database.APIKey) bool {
 return id == u.UserID
 })

 q.userLinks = slices.DeleteFunc(q.userLinks, func(u database.UserLink) bool {
 return id == u.UserID
 })
 return nil
 }
 }
 q.mutex.Lock()
 defer q.mutex.Unlock()

 if u, err := q.getUserByIDNoLock(params.UserID); err == nil && u.Deleted {
 return database.UserLink{}, deletedUserLinkError
 }

 for i, link := range q.userLinks {
 if link.UserID == params.UserID && link.LoginType == params.LoginType {
 link.OAuthAccessToken = params.OAuthAccessToken
diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go
diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go
diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql
diff --git a/coderd/database/migrations/000194_trigger_delete_user_user_link.down.sql b/coderd/database/migrations/000194_trigger_delete_user_user_link.down.sql
 DROP TRIGGER IF EXISTS trigger_update_users ON users;
 DROP FUNCTION IF EXISTS delete_deleted_user_resources;

 DROP TRIGGER IF EXISTS trigger_upsert_user_links ON user_links;
 DROP FUNCTION IF EXISTS insert_user_links_fail_if_user_deleted;

 -- Restore the previous trigger
 CREATE FUNCTION delete_deleted_user_api_keys() RETURNS trigger
 LANGUAGE plpgsql
 AS $$
 DECLARE
 BEGIN
 IF (NEW.deleted) THEN
 DELETE FROM api_keys
 WHERE user_id = OLD.id;
 END IF;
 RETURN NEW;
 END;
 $$;


 CREATE TRIGGER trigger_update_users
 AFTER INSERT OR UPDATE ON users
 FOR EACH ROW
 WHEN (NEW.deleted = true)
 EXECUTE PROCEDURE delete_deleted_user_api_keys();
diff --git a/coderd/database/migrations/000194_trigger_delete_user_user_link.up.sql b/coderd/database/migrations/000194_trigger_delete_user_user_link.up.sql
 -- We need to delete all existing user_links for soft-deleted users
 DELETE FROM
 user_links
 WHERE
 user_id
 IN (
 SELECT id FROM users WHERE deleted
 );

 -- Drop the old trigger
 DROP TRIGGER trigger_update_users ON users;
 -- Drop the old function
 DROP FUNCTION delete_deleted_user_api_keys;

 -- When we soft-delete a user, we also want to delete their API key.
 -- The previous function deleted all api keys. This extends that with user_links.
 CREATE FUNCTION delete_deleted_user_resources() RETURNS trigger
 LANGUAGE plpgsql
 AS $$
 DECLARE
 BEGIN
 IF (NEW.deleted) THEN
 -- Remove their api_keys
 DELETE FROM api_keys
 WHERE user_id = OLD.id;

 -- Remove their user_links
 -- Their login_type is preserved in the users table.
 -- Matching this user back to the link can still be done by their
 -- email if the account is undeleted. Although that is not a guarantee.
 DELETE FROM user_links
 WHERE user_id = OLD.id;
 END IF;
 RETURN NEW;
 END;
 $$;


 -- Update it to the new trigger
 CREATE TRIGGER trigger_update_users
 AFTER INSERT OR UPDATE ON users
 FOR EACH ROW
 WHEN (NEW.deleted = true)
 EXECUTE PROCEDURE delete_deleted_user_resources();


 -- Prevent adding new user_links for soft-deleted users
 CREATE FUNCTION insert_user_links_fail_if_user_deleted() RETURNS trigger
 LANGUAGE plpgsql
 AS $$

 DECLARE
 BEGIN
 IF (NEW.user_id IS NOT NULL) THEN
 IF (SELECT deleted FROM users WHERE id = NEW.user_id LIMIT 1) THEN
 RAISE EXCEPTION 'Cannot create user_link for deleted user';
 END IF;
 END IF;
 RETURN NEW;
 END;
 $$;

 CREATE TRIGGER trigger_upsert_user_links
 BEFORE INSERT OR UPDATE ON user_links
 FOR EACH ROW
 EXECUTE PROCEDURE insert_user_links_fail_if_user_deleted();
diff --git a/coderd/database/querier.go b/coderd/database/querier.go
diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go
diff --git a/coderd/database/queries/users.sql b/coderd/database/queries/users.sql
 UPDATE
 users
 SET
 deleted =$2
 deleted =true
 WHERE
 id = $1;
Original file line number	Diff line number	Diff line change
Expand Up		@@ -11,7 +11,6 @@ import (

		"github.com/coder/coder/v2/cli/clitest"
		"github.com/coder/coder/v2/coderd/coderdtest"
		"github.com/coder/coder/v2/coderd/database"
		"github.com/coder/coder/v2/coderd/database/dbauthz"
		"github.com/coder/coder/v2/codersdk"
		"github.com/coder/coder/v2/pty/ptytest"
Expand DownExpand Up		@@ -95,10 +94,7 @@ func TestDelete(t *testing.T) {
		// this way.
		ctx := testutil.Context(t, testutil.WaitShort)
		// nolint:gocritic // Unit test
		err := api.Database.UpdateUserDeletedByID(dbauthz.AsSystemRestricted(ctx), database.UpdateUserDeletedByIDParams{
		ID: deleteMeUser.ID,
		Deleted: true,
		})
		err := api.Database.UpdateUserDeletedByID(dbauthz.AsSystemRestricted(ctx), deleteMeUser.ID)
		require.NoError(t, err)

		inv, root := clitest.New(t, "delete", fmt.Sprintf("%s/%s", deleteMeUser.ID, workspace.Name), "-y", "--orphan")
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -607,16 +607,6 @@ func (q *querier) SoftDeleteTemplateByID(ctx context.Context, id uuid.UUID) erro
		return deleteQ(q.log, q.auth, q.db.GetTemplateByID, deleteF)(ctx, id)
		}

		func (q *querier) SoftDeleteUserByID(ctx context.Context, id uuid.UUID) error {
		deleteF := func(ctx context.Context, id uuid.UUID) error {
		return q.db.UpdateUserDeletedByID(ctx, database.UpdateUserDeletedByIDParams{
		ID: id,
		Deleted: true,
		})
		}
		return deleteQ(q.log, q.auth, q.db.GetUserByID, deleteF)(ctx, id)
		}

		func (q *querier) SoftDeleteWorkspaceByID(ctx context.Context, id uuid.UUID) error {
		return deleteQ(q.log, q.auth, q.db.GetWorkspaceByID, func(ctx context.Context, id uuid.UUID) error {
		return q.db.UpdateWorkspaceDeletedByID(ctx, database.UpdateWorkspaceDeletedByIDParams{
Expand DownExpand Up		@@ -2881,16 +2871,8 @@ func (q *querier) UpdateUserAppearanceSettings(ctx context.Context, arg database
		return q.db.UpdateUserAppearanceSettings(ctx, arg)
		}

		// UpdateUserDeletedByID
		// Deprecated: Delete this function in favor of 'SoftDeleteUserByID'. Deletes are
		// irreversible.
		func (q *querier) UpdateUserDeletedByID(ctx context.Context, arg database.UpdateUserDeletedByIDParams) error {
		fetch := func(ctx context.Context, arg database.UpdateUserDeletedByIDParams) (database.User, error) {
		return q.db.GetUserByID(ctx, arg.ID)
		}
		// This uses the rbac.ActionDelete action always as this function should always delete.
		// We should delete this function in favor of 'SoftDeleteUserByID'.
		return deleteQ(q.log, q.auth, fetch, q.db.UpdateUserDeletedByID)(ctx, arg)
		func (q *querier) UpdateUserDeletedByID(ctx context.Context, id uuid.UUID) error {
		return deleteQ(q.log, q.auth, q.db.GetUserByID, q.db.UpdateUserDeletedByID)(ctx, id)
		}

		func (q *querier) UpdateUserHashedPassword(ctx context.Context, arg database.UpdateUserHashedPasswordParams) error {
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -1015,17 +1015,10 @@ func (s *MethodTestSuite) TestUser() {
		LoginType: database.LoginTypeOIDC,
		}).Asserts(u, rbac.ActionUpdate)
		}))
		s.Run("SoftDeleteUserByID", s.Subtest(func(db database.Store, check *expects) {
		s.Run("UpdateUserDeletedByID", s.Subtest(func(db database.Store, check *expects) {
		u := dbgen.User(s.T(), db, database.User{})
		check.Args(u.ID).Asserts(u, rbac.ActionDelete).Returns()
		}))
		s.Run("UpdateUserDeletedByID", s.Subtest(func(db database.Store, check *expects) {
		u := dbgen.User(s.T(), db, database.User{Deleted: true})
		check.Args(database.UpdateUserDeletedByIDParams{
		ID: u.ID,
		Deleted: true,
		}).Asserts(u, rbac.ActionDelete).Returns()
		}))
		s.Run("UpdateUserHashedPassword", s.Subtest(func(db database.Store, check *expects) {
		u := dbgen.User(s.T(), db, database.User{})
		check.Args(database.UpdateUserHashedPasswordParams{
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -311,10 +311,7 @@ func User(t testing.TB, db database.Store, orig database.User) database.User {
		}

		if orig.Deleted {
		err = db.UpdateUserDeletedByID(genCtx, database.UpdateUserDeletedByIDParams{
		ID: user.ID,
		Deleted: orig.Deleted,
		})
		err = db.UpdateUserDeletedByID(genCtx, user.ID)
		require.NoError(t, err, "set user as deleted")
		}
		return user
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -733,6 +733,18 @@ func isNotNull(v interface{}) bool {
		return reflect.ValueOf(v).FieldByName("Valid").Bool()
		}

		// Took the error from the real database.
		var deletedUserLinkError = &pq.Error{
		Severity: "ERROR",
		// "raise_exception" error
		Code: "P0001",
		Message: "Cannot create user_link for deleted user",
		Where: "PL/pgSQL function insert_user_links_fail_if_user_deleted() line 7 at RAISE",
		File: "pl_exec.c",
		Line: "3864",
		Routine: "exec_stmt_raise",
		}

		func (*FakeQuerier) AcquireLock(_ context.Context, _ int64) error {
		return xerrors.New("AcquireLock must only be called within a transaction")
		}
Expand DownExpand Up		@@ -5560,6 +5572,10 @@ func (q *FakeQuerier) InsertUserLink(_ context.Context, args database.InsertUser
		q.mutex.Lock()
		defer q.mutex.Unlock()

		if u, err := q.getUserByIDNoLock(args.UserID); err == nil && u.Deleted {
		return database.UserLink{}, deletedUserLinkError
		}

		//nolint:gosimple
		link := database.UserLink{
		UserID: args.UserID,
Expand DownExpand Up		@@ -6724,33 +6740,22 @@ func (q *FakeQuerier) UpdateUserAppearanceSettings(_ context.Context, arg databa
		return database.User{}, sql.ErrNoRows
		}

		func (q *FakeQuerier) UpdateUserDeletedByID(_ context.Context, params database.UpdateUserDeletedByIDParams) error {
		if err := validateDatabaseType(params); err != nil {
		return err
		}

		func (q *FakeQuerier) UpdateUserDeletedByID(_ context.Context, id uuid.UUID) error {
		q.mutex.Lock()
		defer q.mutex.Unlock()

		for i, u := range q.users {
		if u.ID ==params.ID {
		u.Deleted =params.Deleted
		if u.ID ==id {
		u.Deleted =true
		q.users[i] = u
		// NOTE: In the real world, this is done by a trigger.
		i := 0
		for {
		if i >= len(q.apiKeys) {
		break
		}
		k := q.apiKeys[i]
		if k.UserID == u.ID {
		q.apiKeys[i] = q.apiKeys[len(q.apiKeys)-1]
		q.apiKeys = q.apiKeys[:len(q.apiKeys)-1]
		// We removed an element, so decrement
		i--
		}
		i++
		}
		q.apiKeys = slices.DeleteFunc(q.apiKeys, func(u database.APIKey) bool {
		return id == u.UserID
		})

		q.userLinks = slices.DeleteFunc(q.userLinks, func(u database.UserLink) bool {
		return id == u.UserID
		})
		return nil
		}
		}
Expand DownExpand Up		@@ -6804,6 +6809,10 @@ func (q *FakeQuerier) UpdateUserLink(_ context.Context, params database.UpdateUs
		q.mutex.Lock()
		defer q.mutex.Unlock()

		if u, err := q.getUserByIDNoLock(params.UserID); err == nil && u.Deleted {
		return database.UserLink{}, deletedUserLinkError
		}

		for i, link := range q.userLinks {
		if link.UserID == params.UserID && link.LoginType == params.LoginType {
		link.OAuthAccessToken = params.OAuthAccessToken
Expand Down
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,26 @@
		DROP TRIGGER IF EXISTS trigger_update_users ON users;
		DROP FUNCTION IF EXISTS delete_deleted_user_resources;

		DROP TRIGGER IF EXISTS trigger_upsert_user_links ON user_links;
		DROP FUNCTION IF EXISTS insert_user_links_fail_if_user_deleted;

		-- Restore the previous trigger
		CREATE FUNCTION delete_deleted_user_api_keys() RETURNS trigger
		LANGUAGE plpgsql
		AS $$
		DECLARE
		BEGIN
		IF (NEW.deleted) THEN
		DELETE FROM api_keys
		WHERE user_id = OLD.id;
		END IF;
		RETURN NEW;
		END;
		$$;


		CREATE TRIGGER trigger_update_users
		AFTER INSERT OR UPDATE ON users
		FOR EACH ROW
		WHEN (NEW.deleted = true)
		EXECUTE PROCEDURE delete_deleted_user_api_keys();
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,66 @@
		-- We need to delete all existing user_links for soft-deleted users
		DELETE FROM
		user_links
		WHERE
		user_id
		IN (
		SELECT id FROM users WHERE deleted
		);

		-- Drop the old trigger
		DROP TRIGGER trigger_update_users ON users;
		-- Drop the old function
		DROP FUNCTION delete_deleted_user_api_keys;

		-- When we soft-delete a user, we also want to delete their API key.
		-- The previous function deleted all api keys. This extends that with user_links.
		CREATE FUNCTION delete_deleted_user_resources() RETURNS trigger
		LANGUAGE plpgsql
		AS $$
		DECLARE
		BEGIN
		IF (NEW.deleted) THEN
		-- Remove their api_keys
		DELETE FROM api_keys
		WHERE user_id = OLD.id;

		-- Remove their user_links
		-- Their login_type is preserved in the users table.
		-- Matching this user back to the link can still be done by their
		-- email if the account is undeleted. Although that is not a guarantee.
		DELETE FROM user_links
		WHERE user_id = OLD.id;
		END IF;
		RETURN NEW;
		END;
		$$;


		-- Update it to the new trigger
		CREATE TRIGGER trigger_update_users
		AFTER INSERT OR UPDATE ON users
		FOR EACH ROW
		WHEN (NEW.deleted = true)
		EXECUTE PROCEDURE delete_deleted_user_resources();


		-- Prevent adding new user_links for soft-deleted users
		CREATE FUNCTION insert_user_links_fail_if_user_deleted() RETURNS trigger
		LANGUAGE plpgsql
		AS $$

		DECLARE
		BEGIN
		IF (NEW.user_id IS NOT NULL) THEN
		IF (SELECT deleted FROM users WHERE id = NEW.user_id LIMIT 1) THEN
		RAISE EXCEPTION 'Cannot create user_link for deleted user';
		END IF;
		END IF;
		RETURN NEW;
		END;
		$$;

		CREATE TRIGGER trigger_upsert_user_links
		BEFORE INSERT OR UPDATE ON user_links
		FOR EACH ROW
		EXECUTE PROCEDURE insert_user_links_fail_if_user_deleted();
Original file line number	Diff line number	Diff line change
Expand Up		@@ -116,7 +116,7 @@ WHERE
		UPDATE
		users
		SET
		deleted =$2
		deleted =true
		WHERE
		id = $1;

Expand Down