fix: Prevent suspending owners (#3757)

coderd/users.go

@@ -21,7 +21,6 @@ import (
 	"golang.org/x/xerrors"
 
 	"cdr.dev/slog"
-
 	"github.com/coder/coder/coderd/database"
 	"github.com/coder/coder/coderd/gitsshkey"
 	"github.com/coder/coder/coderd/httpapi"
@@ -29,6 +28,7 @@ import (
 	"github.com/coder/coder/coderd/rbac"
 	"github.com/coder/coder/coderd/telemetry"
 	"github.com/coder/coder/coderd/userpassword"
+	"github.com/coder/coder/coderd/util/slice"
 	"github.com/coder/coder/codersdk"
 	"github.com/coder/coder/cryptorand"
 	"github.com/coder/coder/examples"
@@ -425,11 +425,24 @@ func (api *API) putUserStatus(status database.UserStatus) func(rw http.ResponseW
 			return
 		}
 
-		if status == database.UserStatusSuspended && user.ID == apiKey.UserID {
+		if status == database.UserStatusSuspended {
-			httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{
+			// There are some manual protections when suspending a user to
-				Message: "You cannot suspend yourself.",
+			// prevent certain situations.
-			})
+			switch {
-			return
+			case user.ID == apiKey.UserID:
+				// Suspending yourself is not allowed, as you can lock yourself
+				// out of the system.
+				httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{
+					Message: "You cannot suspend yourself.",
+				})
+				return
+			case slice.Contains(user.RBACRoles, rbac.RoleOwner()):
+				// You may not suspend an owner
+				httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{
+					Message: fmt.Sprintf("You cannot suspend a user with the %q role. You must remove the role first.", rbac.RoleOwner()),
+				})
+				return
+			}
 		}
 
 		suspendedUser, err := api.Database.UpdateUserStatus(r.Context(), database.UpdateUserStatusParams{

coderd/users_test.go

@@ -737,21 +737,28 @@ func TestInitialRoles(t *testing.T) {
 func TestPutUserSuspend(t *testing.T) {
 	t.Parallel()
 
-	t.Run("SuspendAnotherUser", func(t *testing.T) {
+	t.Run("SuspendAnOwner", func(t *testing.T) {
 		t.Parallel()
 		client := coderdtest.New(t, nil)
 		me := coderdtest.CreateFirstUser(t, client)
+		_, user := coderdtest.CreateAnotherUserWithUser(t, client, me.OrganizationID, rbac.RoleOwner())
+
+		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
+		defer cancel()
+
+		_, err := client.UpdateUserStatus(ctx, user.Username, codersdk.UserStatusSuspended)
+		require.Error(t, err, "cannot suspend owners")
+	})
+
+	t.Run("SuspendAnotherUser", func(t *testing.T) {
+		t.Parallel()
+		client := coderdtest.New(t, nil)
+		me := coderdtest.CreateFirstUser(t, client)
+		_, user := coderdtest.CreateAnotherUserWithUser(t, client, me.OrganizationID)
 
 		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
 		defer cancel()
 
-		client.User(ctx, codersdk.Me)
-		user, _ := client.CreateUser(ctx, codersdk.CreateUserRequest{
-			Email:          "bruno@coder.com",
-			Username:       "bruno",
-			Password:       "password",
-			OrganizationID: me.OrganizationID,
-		})
 		user, err := client.UpdateUserStatus(ctx, user.Username, codersdk.UserStatusSuspended)
 		require.NoError(t, err)
 		require.Equal(t, user.Status, codersdk.UserStatusSuspended)
@@ -841,7 +848,7 @@ func TestUsersFilter(t *testing.T) {
 	for i := 0; i < 15; i++ {
 		roles := []string{}
 		if i%2 == 0 {
-			roles = append(roles, rbac.RoleOwner())
+			roles = append(roles, rbac.RoleTemplateAdmin(), rbac.RoleUserAdmin())
 		}
 		if i%3 == 0 {
 			roles = append(roles, "auditor")