fix: Prevent suspending owners (coder#3757)

Emyrk · web-flow · commit aa9a1c3f56de · 2022-08-31T15:26:36.000Z
diff --git a/coderd/users.go b/coderd/users.go
@@ -21,14 +21,14 @@ 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"
 	"github.com/coder/coder/coderd/httpmw"
 	"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 {
-			httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{
-				Message: "You cannot suspend yourself.",
-			})
-			return
+		if status == database.UserStatusSuspended {
+			// There are some manual protections when suspending a user to
+			// prevent certain situations.
+			switch {
+			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{
diff --git a/coderd/users_test.go b/coderd/users_test.go
@@ -737,21 +737,28 @@ func TestInitialRoles(t *testing.T) {
 func TestPutUserSuspend(t *testing.T) {
 	t.Parallel()
 
+	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")
