mirror of
https://github.com/netbirdio/netbird.git
synced 2026-10-23 05:49:07 +02:00
[management] Fix user update permission validation (#5441)
This commit is contained in:
@@ -742,6 +742,11 @@ func (am *DefaultAccountManager) processUserUpdate(ctx context.Context, transact
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
return false, nil, nil, nil, fmt.Errorf("failed to re-read initiator user in transaction: %w", err)
|
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
|
initiatorUser = freshInitiator
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -872,10 +877,6 @@ func validateUserUpdate(groupsMap map[string]*types.Group, initiatorUser, oldUse
|
|||||||
return nil
|
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 {
|
if initiatorUser.HasAdminPower() && initiatorUser.Id == update.Id && oldUser.Blocked != update.Blocked {
|
||||||
return status.Errorf(status.PermissionDenied, "admins can't block or unblock themselves")
|
return status.Errorf(status.PermissionDenied, "admins can't block or unblock themselves")
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -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) {
|
func TestProcessUserUpdate_RejectsStaleInitiatorRole(t *testing.T) {
|
||||||
s, cleanup, err := store.NewTestStoreFromSQL(context.Background(), "", t.TempDir())
|
s, cleanup, err := store.NewTestStoreFromSQL(context.Background(), "", t.TempDir())
|
||||||
require.NoError(t, err)
|
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")
|
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)
|
targetUser, err := s.GetUserByUserID(context.Background(), store.LockingStrengthNone, targetID)
|
||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
|
|||||||
Reference in New Issue
Block a user