diff --git a/management/server/user.go b/management/server/user.go index 924efc1e4..327aec2d0 100644 --- a/management/server/user.go +++ b/management/server/user.go @@ -742,6 +742,11 @@ func (am *DefaultAccountManager) processUserUpdate(ctx context.Context, transact if err != nil { return false, nil, nil, nil, fmt.Errorf("failed to re-read initiator user in transaction: %w", err) } + + // Ensure the initiator still has admin privileges + if initiatorUser.HasAdminPower() && !freshInitiator.HasAdminPower() { + return false, nil, nil, nil, status.Errorf(status.PermissionDenied, "initiator role was changed during request processing") + } initiatorUser = freshInitiator } @@ -872,10 +877,6 @@ func validateUserUpdate(groupsMap map[string]*types.Group, initiatorUser, oldUse return nil } - if !initiatorUser.HasAdminPower() { - return status.Errorf(status.PermissionDenied, "only admins and owners can update users") - } - if initiatorUser.HasAdminPower() && initiatorUser.Id == update.Id && oldUser.Blocked != update.Blocked { return status.Errorf(status.PermissionDenied, "admins can't block or unblock themselves") } diff --git a/management/server/user_test.go b/management/server/user_test.go index 72a19a9a5..800d2406c 100644 --- a/management/server/user_test.go +++ b/management/server/user_test.go @@ -2032,27 +2032,6 @@ func TestUser_Operations_WithEmbeddedIDP(t *testing.T) { }) } -func TestValidateUserUpdate_RejectsNonAdminInitiator(t *testing.T) { - groupsMap := map[string]*types.Group{} - - initiator := &types.User{ - Id: "initiator", - Role: types.UserRoleUser, - } - oldUser := &types.User{ - Id: "target", - Role: types.UserRoleUser, - } - update := &types.User{ - Id: "target", - Role: types.UserRoleOwner, - } - - err := validateUserUpdate(groupsMap, initiator, oldUser, update) - require.Error(t, err, "regular user should not be able to promote to owner") - assert.Contains(t, err.Error(), "only admins and owners can update users") -} - func TestProcessUserUpdate_RejectsStaleInitiatorRole(t *testing.T) { s, cleanup, err := store.NewTestStoreFromSQL(context.Background(), "", t.TempDir()) require.NoError(t, err) @@ -2109,7 +2088,7 @@ func TestProcessUserUpdate_RejectsStaleInitiatorRole(t *testing.T) { }) require.Error(t, err, "processUserUpdate should reject stale initiator whose role was demoted") - assert.Contains(t, err.Error(), "only admins and owners can update users") + assert.Contains(t, err.Error(), "initiator role was changed during request processing") targetUser, err := s.GetUserByUserID(context.Background(), store.LockingStrengthNone, targetID) require.NoError(t, err)