From f0f04fd86a79ff067048b51f234694ed2619e603 Mon Sep 17 00:00:00 2001 From: Boy132 Date: Fri, 21 Feb 2025 11:02:08 +0100 Subject: [PATCH] Add backend validation to subuser permissions (#1014) * add backend validation to subuser permissions * always allow websocket.connect * use collection to clean permissions --- .../Subusers/SubuserCreationService.php | 10 +++++++++- app/Services/Subusers/SubuserUpdateService.php | 18 ++++++++++++------ 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/app/Services/Subusers/SubuserCreationService.php b/app/Services/Subusers/SubuserCreationService.php index e4c66d61a..83c268330 100644 --- a/app/Services/Subusers/SubuserCreationService.php +++ b/app/Services/Subusers/SubuserCreationService.php @@ -11,6 +11,7 @@ use Illuminate\Database\ConnectionInterface; use App\Services\Users\UserCreationService; use App\Exceptions\Service\Subuser\UserIsServerOwnerException; use App\Exceptions\Service\Subuser\ServerSubuserExistsException; +use App\Models\Permission; class SubuserCreationService { @@ -57,10 +58,17 @@ class SubuserCreationService throw new ServerSubuserExistsException(trans('exceptions.subusers.subuser_exists')); } + $cleanedPermissions = collect($permissions) + ->unique() + ->filter(fn ($permission) => $permission === Permission::ACTION_WEBSOCKET_CONNECT || auth()->user()->can($permission, $server)) + ->sort() + ->values() + ->all(); + $subuser = Subuser::query()->create([ 'user_id' => $user->id, 'server_id' => $server->id, - 'permissions' => array_unique($permissions), + 'permissions' => $cleanedPermissions, ]); event(new SubUserAdded($subuser)); diff --git a/app/Services/Subusers/SubuserUpdateService.php b/app/Services/Subusers/SubuserUpdateService.php index 0b349b864..5f2c61d78 100644 --- a/app/Services/Subusers/SubuserUpdateService.php +++ b/app/Services/Subusers/SubuserUpdateService.php @@ -3,6 +3,7 @@ namespace App\Services\Subusers; use App\Facades\Activity; +use App\Models\Permission; use App\Models\Server; use App\Models\Subuser; use App\Repositories\Daemon\DaemonServerRepository; @@ -16,9 +17,14 @@ class SubuserUpdateService public function handle(Subuser $subuser, Server $server, array $permissions): void { - $current = $subuser->permissions; + $cleanedPermissions = collect($permissions) + ->unique() + ->filter(fn ($permission) => $permission === Permission::ACTION_WEBSOCKET_CONNECT || auth()->user()->can($permission, $server)) + ->sort() + ->values() + ->all(); - sort($permissions); + $current = $subuser->permissions; sort($current); $log = Activity::event('server:subuser.update') @@ -26,15 +32,15 @@ class SubuserUpdateService ->property([ 'email' => $subuser->user->email, 'old' => $current, - 'new' => $permissions, + 'new' => $cleanedPermissions, 'revoked' => true, ]); // Only update the database and hit up the daemon instance to invalidate JTI's if the permissions // have actually changed for the user. - if ($permissions !== $current) { - $log->transaction(function ($instance) use ($subuser, $permissions, $server) { - $subuser->update(['permissions' => $permissions]); + if ($cleanedPermissions !== $current) { + $log->transaction(function ($instance) use ($subuser, $cleanedPermissions, $server) { + $subuser->update(['permissions' => $cleanedPermissions]); try { $this->serverRepository->setServer($server)->revokeUserJTI($subuser->user_id);