Fix user create & changeRole permissions/options handling in Panel UI

config/api/routes/roles.php

@@ -8,13 +8,9 @@
 		'pattern' => 'roles',
 		'method'  => 'GET',
 		'action'  => function () {
-			$kirby = $this->kirby();
-
-			return match ($kirby->request()->get('canBe')) {
-				'changed' => $kirby->roles()->canBeChanged(),
-				'created' => $kirby->roles()->canBeCreated(),
-				default   => $kirby->roles()
-			};
+			$kirby   = $this->kirby();
+			$context = $kirby->request()->get('canBe');
+			return $kirby->roles($context);
 		}
 	],
 	[

config/areas/users/dialogs.php

@@ -20,12 +20,16 @@
 		'pattern' => 'users/create',
 		'load' => function () {
 			$kirby = App::instance();
+			$roles = $kirby->roles('create');
 
 			// get default value for role
 			if ($role = $kirby->request()->get('role')) {
-				$role = $kirby->roles()->find($role)?->id();
+				$role = $roles->find($role)?->id();
 			}
 
+			// get role field definition, incl available role options
+			$roles = Field::role(['required' => true], $roles);
+
 			return [
 				'component' => 'k-form-dialog',
 				'props' => [
@@ -39,17 +43,15 @@
 						'translation'  => Field::translation([
 							'required' => true
 						]),
-						'role' => Field::role([
-							'required' => true
-						])
+						'role' => $roles
 					],
 					'submitButton' => I18n::translate('create'),
 					'value' => [
 						'name'        => '',
 						'email'       => '',
 						'password'    => '',
 						'translation' => $kirby->panelLanguage(),
-						'role'        => $role ?? $kirby->user()->role()->name()
+						'role'        => $role ?? $roles['options'][0]['value'] ?? null
 					]
 				]
 			];
@@ -231,7 +233,7 @@
 						'role' => Field::role([
 							'label'    => I18n::translate('user.changeRole.select'),
 							'required' => true,
-						])
+						], $user->roles('change'))
 					],
 					'submitButton' => I18n::translate('user.changeRole'),
 					'value' => [

src/Cms/App.php

@@ -1302,9 +1302,19 @@ public function response(): Responder
 	/**
 	 * Returns all user roles
 	 */
-	public function roles(): Roles
+	public function roles(string|null $context = null): Roles
 	{
-		return $this->roles ??= Roles::load($this->root('roles'));
+		$roles = $this->roles ??= Roles::load($this->root('roles'));
+
+		// filter roles based on the context
+		// as user options can restrict these further
+		$roles = match ($context) {
+			'create' => $roles->canBeCreated(),
+			'change' => $roles->canBeChanged(),
+			default  => $roles
+		};
+
+		return $roles;
 	}
 
 	/**

src/Cms/Roles.php

@@ -33,7 +33,7 @@ class Roles extends Collection
 	 */
 	public function canBeChanged(): static
 	{
-		if (App::instance()->user()) {
+		if (App::instance()->user()?->isAdmin() !== true) {
 			return $this->filter(function ($role) {
 				$newUser = new User([
 					'email' => 'test@getkirby.com',
@@ -57,7 +57,7 @@ public function canBeChanged(): static
 	 */
 	public function canBeCreated(): static
 	{
-		if (App::instance()->user()) {
+		if (App::instance()->user()?->isAdmin() !== true) {
 			return $this->filter(function ($role) {
 				$newUser = new User([
 					'email' => 'test@getkirby.com',

src/Cms/User.php

@@ -574,33 +574,27 @@ 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
+	public function roles(string|null $context = null): 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;
-			}
+		$roles = $kirby->roles($context);
+
+		// for the last admin, only their current role (admin) is available
+		if ($this->isLastAdmin() === true) {
+			// a collection with just the one role of the user
+			return $roles->filter('id', $this->role()->id());
+		}
 
-			// 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;
 	}
 
 	/**

src/Cms/UserPermissions.php

@@ -26,7 +26,20 @@ public function __construct(User $model)
 
 	protected function canChangeRole(): bool
 	{
-		return $this->model->roles()->count() > 1;
+		// 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 true;
 	}
 
 	protected function canCreate(): bool

src/Cms/UserRules.php

@@ -101,43 +101,39 @@ public static function changePassword(
 	 */
 	public static function changeRole(User $user, string $role): bool
 	{
-		// protect admin from role changes by non-admin
-		if (
-			$user->kirby()->user()->isAdmin() === false &&
-			$user->isAdmin() === true
-		) {
-			throw new PermissionException([
-				'key'  => 'user.changeRole.permission',
+		// prevent demoting the last admin
+		if ($role !== 'admin' && $user->isLastAdmin() === true) {
+			throw new LogicException([
+				'key'  => 'user.changeRole.lastAdmin',
 				'data' => ['name' => $user->username()]
 			]);
 		}
 
-		// 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'
 		) {
 			throw new PermissionException([
-				'key'  => 'user.changeRole.toAdmin'
-			]);
-		}
-
-		static::validRole($user, $role);
-
-		if ($role !== 'admin' && $user->isLastAdmin() === true) {
-			throw new LogicException([
-				'key'  => 'user.changeRole.lastAdmin',
-				'data' => ['name' => $user->username()]
+				'key' => 'user.changeRole.toAdmin'
 			]);
 		}
 
+		// check permissions
 		if ($user->permissions()->changeRole() !== true) {
 			throw new PermissionException([
 				'key'  => 'user.changeRole.permission',
 				'data' => ['name' => $user->username()]
 			]);
 		}
 
+		// prevent changing to role that is not available for user
+		if ($user->roles('change')->find($role) instanceof Role === false) {
+			throw new InvalidArgumentException([
+				'key' => 'user.role.invalid',
+			]);
+		}
+
 		return true;
 	}
 
@@ -199,15 +195,6 @@ public static function create(User $user, array $props = []): bool
 			return true;
 		}
 
-		// only admins are allowed to add admins
-		$role = $props['role'] ?? null;
-
-		if ($role === 'admin' && $currentUser?->isAdmin() === false) {
-			throw new PermissionException([
-				'key' => 'user.create.permission'
-			]);
-		}
-
 		// check user permissions (if not on install)
 		if (
 			$user->kirby()->users()->count() > 0 &&
@@ -218,6 +205,18 @@ public static function create(User $user, array $props = []): bool
 			]);
 		}
 
+		$role = $props['role'] ?? null;
+
+		// prevent creating a role that is not available for user
+		if (
+			in_array($role, [null, 'default', 'nobody'], true) === false &&
+			$user->kirby()->roles('create')->find($role) instanceof Role === false
+		) {
+			throw new InvalidArgumentException([
+				'key' => 'user.role.invalid',
+			]);
+		}
+
 		return true;
 	}
 
@@ -365,20 +364,4 @@ public static function validPassword(
 
 		return true;
 	}
-
-	/**
-	 * Validates a user role
-	 *
-	 * @throws \Kirby\Exception\InvalidArgumentException If the user role does not exist
-	 */
-	public static function validRole(User $user, string $role): bool
-	{
-		if ($user->kirby()->roles()->find($role) instanceof Role) {
-			return true;
-		}
-
-		throw new InvalidArgumentException([
-			'key' => 'user.role.invalid',
-		]);
-	}
 }

src/Panel/Field.php

@@ -6,6 +6,8 @@
 use Kirby\Cms\File;
 use Kirby\Cms\ModelWithContent;
 use Kirby\Cms\Page;
+use Kirby\Cms\Roles;
+use Kirby\Cms\User;
 use Kirby\Form\Form;
 use Kirby\Http\Router;
 use Kirby\Toolkit\I18n;
@@ -191,30 +193,32 @@ public static function password(array $props = []): array
 	/**
 	 * User role radio buttons
 	 */
-	public static function role(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[] = [
-				'text'  => $role->title(),
-				'info'  => $role->description() ?? I18n::translate('role.description.placeholder'),
-				'value' => $role->name()
-			];
-		}
+	public static function role(
+		array $props = [],
+		Roles|null $roles = null
+	): array {
+		$kirby = App::instance();
+
+		// if this role field is not for any specific user (but e.g. a new one),
+		// get all roles but filter out the admin role
+		// if the current user is no admin
+		$roles ??= $kirby->roles()->filter(
+			fn ($role) =>
+				$role->name() !== 'admin' ||
+				$kirby->user()?->isAdmin() === true
+		);
+
+		$roles = $roles->values(fn ($role) => [
+			'text'  => $role->title(),
+			'info'  => $role->description() ?? I18n::translate('role.description.placeholder'),
+			'value' => $role->name()
+		]);
 
 		return array_merge([
-			'label'    => I18n::translate('role'),
-			'type'     => count($roles) <= 1 ? 'hidden' : 'radio',
-			'options'  => $roles
+			'label'   => I18n::translate('role'),
+			'type'    => 'radio',
+			'value'   => count($roles) === 1 ? $roles[0]['value'] : null,
+			'options' => $roles
 		], $props);
 	}
 

src/Panel/User.php

@@ -70,7 +70,7 @@ public function dropdown(array $options = []): array
 			'dialog'   => $url . '/changeRole',
 			'icon'     => 'bolt',
 			'text'     => I18n::translate('user.changeRole'),
-			'disabled' => $this->isDisabledDropdownOption('changeRole', $options, $permissions)
+			'disabled' => $this->isDisabledDropdownOption('changeRole', $options, $permissions) || $this->model->roles('change')->count() < 2
 		];
 
 		$result[] = [