From 76bf462bbf43d903879a6e5be720e6add1a121c7 Mon Sep 17 00:00:00 2001 From: Garrett Delfosse Date: Thu, 4 Jun 2026 14:36:25 -0400 Subject: [PATCH] fix(coderd): prevent user-admin from resetting owner password (#25709) `PUT /api/v2/users/{user}/password` was protected only by `ActionUpdatePersonal`, which the built-in `user-admin` role holds site-wide. No guard prevented targeting an owner. The old-password check is skipped for non-self resets, so a user-admin could reset any owner's password and authenticate as them, gaining full deployment control. Add an owner-role guard to `putUserPassword` that refuses password-reset requests when the target holds the owner role unless the caller is also an owner. This is modeled on the guard in `putUserStatus`, but differs in that it conditionally allows owner-to-owner resets (whereas `putUserStatus` blocks all suspension of owners regardless of caller). Fixes https://linear.app/codercom/issue/PLAT-227
Implementation details - Guard inserted after the `Authorize` check, before `httpapi.Read` - `apiKey.UserID != user.ID` gates the check so self-password-change is unaffected - Acting user's roles fetched from DB to verify owner status (same pattern as `putUserStatus`) - Returns HTTP 400 consistent with sibling handler error style - Two new test cases: `UserAdminCannotResetOwnerPassword`, `OwnerCanResetOwnerPassword`
> Generated with [Coder Agents](https://coder.com) by @f0ssel --- coderd/users.go | 18 ++++++++++++++++ coderd/users_test.go | 51 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/coderd/users.go b/coderd/users.go index 3003cd05f5..8815b6edb0 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -1604,6 +1604,24 @@ func (api *API) putUserPassword(rw http.ResponseWriter, r *http.Request) { return } + // Only owners can change the password of another owner. + if apiKey.UserID != user.ID && slices.Contains(user.RBACRoles, rbac.RoleOwner().String()) { + actingUser, err := api.Database.GetUserByID(ctx, apiKey.UserID) + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error fetching acting user.", + Detail: err.Error(), + }) + return + } + if !slices.Contains(actingUser.RBACRoles, rbac.RoleOwner().String()) { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Only owners can change the password of an owner.", + }) + return + } + } + if !httpapi.Read(ctx, rw, r, ¶ms) { return } diff --git a/coderd/users_test.go b/coderd/users_test.go index 29da1887a4..0f86e6074a 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -1517,6 +1517,57 @@ func TestUpdateUserPassword(t *testing.T) { require.Equal(t, http.StatusNotFound, cerr.StatusCode()) }) + t.Run("UserAdminCannotResetOwnerPassword", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + owner := coderdtest.CreateFirstUser(t, client) + userAdmin, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.RoleUserAdmin()) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + err := userAdmin.UpdateUserPassword(ctx, owner.UserID.String(), codersdk.UpdateUserPasswordRequest{ + Password: "SomeNewStrongPassword!", + }) + require.Error(t, err, "user-admin should not be able to reset owner password") + var apiErr *codersdk.Error + require.ErrorAs(t, err, &apiErr) + require.Equal(t, http.StatusBadRequest, apiErr.StatusCode()) + require.Contains(t, apiErr.Message, "Only owners can change the password of an owner") + }) + + t.Run("OwnerCanResetOwnerPassword", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + owner := coderdtest.CreateFirstUser(t, client) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + anotherOwner, err := client.CreateUserWithOrgs(ctx, codersdk.CreateUserRequestWithOrgs{ + Email: "another-owner@coder.com", + Username: "another-owner", + Password: "SomeStrongPassword!", + OrganizationIDs: []uuid.UUID{owner.OrganizationID}, + }) + require.NoError(t, err) + _, err = client.UpdateUserRoles(ctx, anotherOwner.ID.String(), codersdk.UpdateRoles{ + Roles: []string{rbac.RoleOwner().String()}, + }) + require.NoError(t, err) + + err = client.UpdateUserPassword(ctx, anotherOwner.ID.String(), codersdk.UpdateUserPasswordRequest{ + Password: "SomeNewStrongPassword!", + }) + require.NoError(t, err, "owner should be able to reset another owner's password") + + _, err = client.LoginWithPassword(ctx, codersdk.LoginWithPasswordRequest{ + Email: "another-owner@coder.com", + Password: "SomeNewStrongPassword!", + }) + require.NoError(t, err, "other owner should login with the new password") + }) + t.Run("PasswordsMustDiffer", func(t *testing.T) { t.Parallel()