Skip to content

Commit

Permalink
Fix changeRole permission handling
Browse files Browse the repository at this point in the history
Fixes #5146
  • Loading branch information
distantnative committed Aug 11, 2024
1 parent ee6ef3d commit 0358f68
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 30 deletions.
2 changes: 1 addition & 1 deletion config/areas/users/dialogs.php
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@
'component' => 'k-form-dialog',
'props' => [
'fields' => [
'role' => Field::role([
'role' => Field::role($user, [
'label' => I18n::translate('user.changeRole.select'),
'required' => true,
])
Expand Down
30 changes: 13 additions & 17 deletions src/Cms/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -574,33 +574,29 @@ public function role(): Role
}

/**
* Returns all available roles
* for this user, that can be selected
* by the authenticated user
* Returns all available roles for this user,
* that can be selected by the authenticated user
*/
public function roles(): Roles
{
$kirby = $this->kirby();
$roles = $kirby->roles();

// a collection with just the one role of the user
$myRole = $roles->filter('id', $this->role()->id());

// if there's an authenticated user …
// admin users can select pretty much any role
if ($kirby->user()?->isAdmin() === true) {
// except if the user is the last admin
if ($this->isLastAdmin() === true) {
// in which case they have to stay admin
return $myRole;
}
$current = $roles->filter('id', $this->role()->id());

// for the last admin, only their current role (admin) is available
if ($this->isLastAdmin() === true) {
return $current;
}

// return all roles for mighty admins
return $roles;
// exclude the admin role, if the user
// is not allowed to change role to admin
if ($kirby->user()?->isAdmin() !== true) {
$roles = $roles->filter(fn ($role) => $role->name() !== 'admin');
}

// any other user can only keep their role
return $myRole;
return $roles;
}

/**
Expand Down
13 changes: 13 additions & 0 deletions src/Cms/UserPermissions.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,19 @@ public function __construct(User $model)

protected function canChangeRole(): bool
{
// protect admin from role changes by non-admin
if (
$this->model->isAdmin() === true &&
$this->user?->isAdmin() !== true
) {
return false;
}

// prevent demoting the last admin
if ($this->model->isLastAdmin() === true) {
return false;
}

return $this->model->roles()->count() > 1;
}

Expand Down
3 changes: 2 additions & 1 deletion src/Cms/UserRules.php
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public static function changeRole(User $user, string $role): bool
]);
}

// prevent non-admins making a user to admin
// prevent non-admins promoting a user to the admin role
if (
$user->kirby()->user()->isAdmin() === false &&
$role === 'admin'
Expand All @@ -124,6 +124,7 @@ public static function changeRole(User $user, string $role): bool

static::validRole($user, $role);

// prevent demoting the last admin
if ($role !== 'admin' && $user->isLastAdmin() === true) {
throw new LogicException([
'key' => 'user.changeRole.lastAdmin',
Expand Down
15 changes: 4 additions & 11 deletions src/Panel/Field.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use Kirby\Cms\File;
use Kirby\Cms\ModelWithContent;
use Kirby\Cms\Page;
use Kirby\Cms\User;
use Kirby\Form\Form;
use Kirby\Http\Router;
use Kirby\Toolkit\I18n;
Expand Down Expand Up @@ -191,19 +192,11 @@ public static function password(array $props = []): array
/**
* User role radio buttons
*/
public static function role(array $props = []): array
public static function role(User $user, array $props = []): array
{
$kirby = App::instance();
$isAdmin = $kirby->user()?->isAdmin() ?? false;
$roles = [];

foreach ($kirby->roles() as $role) {
// exclude the admin role, if the user
// is not allowed to change role to admin
if ($role->name() === 'admin' && $isAdmin === false) {
continue;
}
$roles = [];

foreach ($user->roles() as $role) {
$roles[] = [
'text' => $role->title(),
'info' => $role->description() ?? I18n::translate('role.description.placeholder'),
Expand Down

0 comments on commit 0358f68

Please sign in to comment.