diff --git a/coderd/users.go b/coderd/users.go index 08175455b3..95f2da3b65 100644 --- a/coderd/users.go +++ b/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 { - 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 index e2752987f6..6836c25729 100644 --- 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("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")