mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
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
<details><summary>Implementation details</summary>
- 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`
</details>
> Generated with [Coder Agents](https://coder.com) by @f0ssel
This commit is contained in:
@@ -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
|
||||
}
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user