Harden permission management (#1651)

* Protect superuser users

Prevent non-superusers from deleting or editing superusers, prevent non-superusers from assigning the superuser role to other users

* Add permission restrictions

* Show raw permission name in admin UI

* Add roles and permissions page to docs

* Fix cs

* Fix cs

* Refactor create superuser command

* Add test for superuser protection

* Add tests for superuser role assignment

* Add tests for permission restriction

* Fix cs

* Add wildcard support for permission restrictions

* Update changelog

* Update changelog

* Correction of the wording of the documentation to avoid confusion

CHANGELOG.md

@@ -15,12 +15,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Rate limiting for room access code authentication ([#669], [#1617])
 - Logging for room authentication ([#669], [#1617])
 - Command to test email configuration ([#530], [#1618])
+- Permission restrictions to prevent non-superusers from editing and deleting superusers ([#1651])
+- Permission restrictions to prevent non-superusers from assigning the superuser role ([#1651])
+- Environment variable for configuring restricted permissions that cannot be assigned to non-superuser roles ([#1651])
+- Display raw permission names in the admin interface ([#1651])
 
 ### Changed
 
 - The recording import task is now prevented from running until the previous run has finished ([#1484], [#1604])
 - Adjust frontend tests to better check the resetting of form errors ([#1679], [#1702])
 - Error handling in create room dialog ([#1704])
+- Real-time input validation on create superuser command ([#1651])
 
 ### Fixed
 
@@ -311,6 +316,7 @@ You can find the changelog for older versions there [here](https://github.com/TH
 [#1617]: https://github.com/THM-Health/PILOS/pull/1617
 [#1618]: https://github.com/THM-Health/PILOS/pull/1618
 [#1679]: https://github.com/THM-Health/PILOS/issues/1679
+[#1651]: https://github.com/THM-Health/PILOS/issues/1651
 [#1702]: https://github.com/THM-Health/PILOS/pull/1702
 [#1704]: https://github.com/THM-Health/PILOS/pull/1704
 [#1721]: https://github.com/THM-Health/PILOS/issues/1721

app/Console/Commands/CreateSuperuserCommand.php

@@ -2,12 +2,16 @@
 
 namespace App\Console\Commands;
 
-use App\Http\Requests\UserRequest;
 use App\Models\Role;
 use App\Models\User;
+use App\Rules\Password;
 use Illuminate\Console\Command;
 use Illuminate\Support\Facades\Hash;
-use Illuminate\Support\Facades\Validator;
+use Illuminate\Validation\Rule;
+
+use function Laravel\Prompts\password;
+use function Laravel\Prompts\select;
+use function Laravel\Prompts\text;
 
 /**
  * Command class that makes it possible to create an new superuser.
@@ -65,36 +69,19 @@ public function handle()
         $this->info('Creating an new superuser, please notify your inputs.');
 
         $data = [];
-        $data['firstname'] = $this->ask('Firstname');
-        $data['lastname'] = $this->ask('Lastname');
-        $data['email'] = $this->ask('E-Mail');
-        $data['user_locale'] = $this->ask('Locale (possible values: '.implode(',', array_keys(config('app.enabled_locales'))).')');
-        $data['password'] = $this->secret('Password');
-        $data['password_confirmation'] = $this->secret('Password Confirmation');
-        $data['generate_password'] = false;
-        $data['bbb_skip_check_audio'] = false;
-        $data['roles'] = [$superuserRole->id];
-        $data['timezone'] = 'UTC';
-
-        $validator = Validator::make($data, (new UserRequest)->rules());
-
-        if ($validator->fails()) {
-            $this->info('Something went wrong, please see the error messages below for more information.');
-
-            foreach ($validator->errors()->all() as $error) {
-                $this->error($error);
-            }
-
-            return 1;
-        }
+        $firstname = text('Firstname', validate: ['firstname' => 'required|max:255']);
+        $lastname = text('Lastname', validate: ['lastname' => 'required|max:255']);
+        $email = text('E-Mail', validate: ['email' => ['required', 'max:255', 'email', Rule::unique('users', 'email')->where('authenticator', 'local')]]);
+        $locale = select('Locale', array_keys(config('app.enabled_locales')));
+        $password = password('Password', required: true, validate: ['min:8', new Password]);
 
         $user = new User;
 
-        $user->firstname = $data['firstname'];
-        $user->lastname = $data['lastname'];
-        $user->email = $data['email'];
-        $user->locale = $data['user_locale'];
-        $user->password = Hash::make($data['password']);
+        $user->firstname = $firstname;
+        $user->lastname = $lastname;
+        $user->email = $email;
+        $user->locale = $locale;
+        $user->password = Hash::make($password);
         $user->email_verified_at = $user->freshTimestamp();
 
         $user->save();

app/Http/Controllers/api/v1/PermissionController.php

@@ -15,6 +15,10 @@ class PermissionController extends Controller
      */
     public function index()
     {
-        return new PermissionResourceCollection(Permission::all());
+        return (new PermissionResourceCollection(Permission::all()))->additional([
+            'meta' => [
+                'restrictions' => config('permissions.restrictions'),
+            ],
+        ]);
     }
 }

app/Http/Controllers/api/v1/RoleController.php

@@ -72,7 +72,10 @@ public function store(RoleRequest $request)
         $role->superuser = false;
 
         $role->save();
-        $role->permissions()->sync($request->permissions);
+
+        $new_permissions = $role->filterRestrictedPermissions(collect($request->permissions));
+
+        $role->permissions()->sync($new_permissions);
 
         return (new RoleResource($role))->withPermissions();
     }
@@ -98,7 +101,9 @@ public function update(RoleRequest $request, Role $role)
         if (! $role->superuser) {
             $old_role_permissions = $role->permissions()->pluck('permissions.id')->toArray();
 
-            $role->permissions()->sync($request->permissions);
+            $new_permissions = $role->filterRestrictedPermissions(collect($request->permissions));
+
+            $role->permissions()->sync($new_permissions);
 
             $user = Auth::user();
 

app/Http/Requests/NewUserRequest.php

@@ -2,6 +2,7 @@
 
 namespace App\Http\Requests;
 
+use App\Models\Role;
 use App\Rules\Password;
 use Illuminate\Foundation\Http\FormRequest;
 use Illuminate\Validation\Rule;
@@ -15,14 +16,16 @@ class NewUserRequest extends FormRequest
      */
     public function rules()
     {
+        $prohibitedRoles = \Auth::user()->superuser ? [] : Role::where(['superuser' => true])->pluck('id')->toArray();
+
         return [
             'firstname' => ['required', 'string', 'max:255'],
             'lastname' => ['required', 'string', 'max:255'],
             'email' => ['required', 'string', 'email', 'max:255', Rule::unique('users', 'email')->where('authenticator', 'local')],
             'user_locale' => ['required', 'string', Rule::in(array_keys(config('app.enabled_locales')))],
             'timezone' => ['required', 'string', Rule::in(timezone_identifiers_list())],
             'roles' => ['required', 'array'],
-            'roles.*' => ['distinct', 'integer', 'exists:App\Models\Role,id'],
+            'roles.*' => ['distinct', 'integer', 'exists:App\Models\Role,id', Rule::notIn($prohibitedRoles)],
             'generate_password' => ['required', 'boolean'],
             'new_password' => ['required_if:generate_password,false', 'string', 'min:8', 'confirmed', new Password],
         ];

app/Http/Requests/UserRequest.php

@@ -2,6 +2,7 @@
 
 namespace App\Http\Requests;
 
+use App\Models\Role;
 use Illuminate\Foundation\Http\FormRequest;
 use Illuminate\Validation\Rule;
 
@@ -14,12 +15,14 @@ class UserRequest extends FormRequest
      */
     public function rules()
     {
+        $prohibitedRoles = \Auth::user()->superuser ? [] : Role::where(['superuser' => true])->pluck('id')->toArray();
+
         $rules = [
             'user_locale' => ['sometimes', 'required', 'string', Rule::in(array_keys(config('app.enabled_locales')))],
             'bbb_skip_check_audio' => ['sometimes', 'required', 'boolean'],
             'timezone' => ['sometimes', 'required', 'string', Rule::in(timezone_identifiers_list())],
             'roles' => ['sometimes', 'required', 'array'],
-            'roles.*' => ['sometimes', 'distinct', 'integer', 'exists:App\Models\Role,id'],
+            'roles.*' => ['sometimes', 'distinct', 'integer', 'exists:App\Models\Role,id', Rule::notIn($prohibitedRoles)],
             'image' => ['sometimes', 'nullable', 'mimes:jpg', 'dimensions:width=100,height=100'],
         ];
 

app/Http/Resources/RoleCollection.php

@@ -22,6 +22,7 @@ public function toArray($request)
                 'automatic' => $role->whenPivotLoaded('role_user', function () use ($role) {
                     return $role->pivot->automatic;
                 }),
+                'superuser' => $role->superuser,
             ];
         })->toArray();
     }

app/Http/Resources/User.php

@@ -84,6 +84,7 @@ public function toArray($request)
             'bbb_skip_check_audio' => $this->bbb_skip_check_audio,
             'initial_password_set' => $this->initial_password_set,
             'timezone' => $this->timezone,
+            'superuser' => $this->superuser,
         ];
     }
 }

app/Models/Role.php

@@ -7,6 +7,7 @@
 use Illuminate\Database\Eloquent\Factories\HasFactory;
 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Database\Eloquent\Relations\BelongsToMany;
+use Illuminate\Support\Collection;
 
 /**
  * Class Role
@@ -46,6 +47,42 @@ public function permissions()
         return $this->belongsToMany(Permission::class)->using(PermissionRole::class);
     }
 
+    /**
+     * Filters out restricted permissions from the given collection of permission IDs.
+     *
+     * @param  Collection  $permissionIDs  Collection of permission IDs to be filtered.
+     * @return Collection Filtered collection of permission IDs that are not restricted.
+     */
+    public function filterRestrictedPermissions(Collection $permissionIDs): Collection
+    {
+        return $permissionIDs->filter(function ($permissionID) {
+            // Find the permission by its ID
+            $permission = Permission::find($permissionID);
+            if ($permission === null) {
+                return false;
+            }
+
+            // Get the list of restricted permissions from the configuration
+            $restrictions = collect(config('permissions.restrictions'));
+
+            // Check if the permission is not in the list of restricted permissions
+            return $restrictions->doesntContain(function (string $restriction) use ($permission) {
+                // If the restriction matches the permission name, it is restricted
+                if ($restriction === $permission->name) {
+                    return true;
+                }
+
+                // Split the restriction and permission names into groups and permissions
+                $restrictionGroup = explode('.', $restriction, 2)[0];
+                $restrictionPermission = explode('.', $restriction, 2)[1] ?? null;
+                $permissionGroup = explode('.', $permission->name, 2)[0];
+
+                // If the restriction applies to all permissions in the group, it is restricted
+                return $restrictionPermission === '*' && $restrictionGroup === $permissionGroup;
+            });
+        });
+    }
+
     /**
      * Types of rooms that can be used by the user of this role.
      *

app/Models/User.php

@@ -327,4 +327,12 @@ public function sendPasswordResetNotification($token)
 
         $this->notify(new PasswordReset($token, Carbon::parse($reset->created_at)));
     }
+
+    /**
+     * @return bool
+     */
+    public function getSuperuserAttribute()
+    {
+        return $this->roles->where('superuser', true)->isNotEmpty();
+    }
 }

app/Policies/UserPolicy.php

@@ -47,6 +47,11 @@ public function create(User $user)
      */
     public function update(User $user, User $model)
     {
+        // Prevent users of the super-user role to be updated by users without the super-user role
+        if ($model->superuser && ! $user->superuser) {
+            return false;
+        }
+
         return $user->can('users.update') || $model->id === $user->id;
     }
 
@@ -57,6 +62,11 @@ public function update(User $user, User $model)
      */
     public function delete(User $user, User $model)
     {
+        // Prevent users of the super-user role to be deleted by users without the super-user role
+        if ($model->superuser && ! $user->superuser) {
+            return false;
+        }
+
         return $user->can('users.delete') && $model->id !== $user->id;
     }
 
@@ -102,6 +112,11 @@ public function changePassword(User $user, User $model)
      */
     public function resetPassword(User $user, User $model)
     {
+        // Prevent users of the super-user role to be deleted by users without the super-user role
+        if ($model->superuser && ! $user->superuser) {
+            return false;
+        }
+
         return $model->authenticator === 'local'
             && $user->can('update', $model)
             && $model->id !== $user->id

config/permissions.php

@@ -0,0 +1,5 @@
+<?php
+
+return [
+    'restrictions' => explode(',', env('PERMISSION_RESTRICTIONS', '')),
+];

docs/docs/administration/03-configuration.md

@@ -142,7 +142,7 @@ To enable external authentication methods like LDAP, please refer to the [Extern
 ## More config options
 
 - [External Authentication](./08-advanced/01-external-authentication.md)
-- [Recording](./08-advanced/02-recording.md)
-- [Scaling](./08-advanced/03-scaling.md)
-- [Greenlight Configuration](./08-advanced/04-migrate-greenlight.md)
+- [Recording](./08-advanced/03-recording.md)
+- [Scaling](./08-advanced/04-scaling.md)
+- [Greenlight Configuration](./08-advanced/05-migrate-greenlight.md)
 - [Development](../development/03-configuration.md)

docs/docs/administration/04-pilos-cli.md

@@ -34,7 +34,7 @@ Build the BigBlueButton recording player with the release version you want.
 docker compose exec app pilos-cli playback-player:build 5.0.2
 ```
 
-See [Recording](./08-advanced/02-recording.md#update-the-bigbluebutton-recording-player) for more information.
+See [Recording](./08-advanced/03-recording.md#update-the-bigbluebutton-recording-player) for more information.
 
 ### locales\:cache