diff --git a/app/Http/Controllers/Api/UsersController.php b/app/Http/Controllers/Api/UsersController.php index 9804852b1b..0b3720a2bc 100644 --- a/app/Http/Controllers/Api/UsersController.php +++ b/app/Http/Controllers/Api/UsersController.php @@ -573,6 +573,41 @@ class UsersController extends Controller $user->activated = $request->input('activated'); } + // We need to use has() instead of filled() + // here because we need to overwrite permissions + // if someone needs to null them out + + if ($request->has('permissions')) { + + $permissions_array = $request->input('permissions'); + $orig_permissions_array = $user->decodePermissions(); + + + // Strip out the individual superuser permission if the API user isn't a superadmin + if (!auth()->user()->isSuperUser()) { + + if (is_array($orig_permissions_array)) { + if (array_key_exists('superuser', $orig_permissions_array)) { + $permissions_array['superuser'] = $orig_permissions_array['superuser']; + } + } + + } + + // Strip out the individual admin permission if the API user isn't an admin + if ((!auth()->user()->isAdmin()) && (!auth()->user()->isSuperUser())) { + + if (is_array($orig_permissions_array)) { + if (array_key_exists('admin', $orig_permissions_array)) { + $permissions_array['admin'] = $orig_permissions_array['admin']; + } + } + } + + // This is going to update the whole thing, not just what was passed + $user->permissions = $permissions_array; + } + } @@ -589,34 +624,7 @@ class UsersController extends Controller } - // We need to use has() instead of filled() - // here because we need to overwrite permissions - // if someone needs to null them out - if ($request->has('permissions')) { - - - $permissions_array = $request->input('permissions'); - - // Strip out the individual superuser permission if the API user isn't a superadmin - if (!auth()->user()->isSuperUser()) { - - if ((is_array($permissions_array)) && (array_key_exists('superuser', $permissions_array))) { - unset($permissions_array['superuser']); - } - } - - // Strip out the individual admin permission if the API user isn't an admin - if (!auth()->user()->isAdmin()) { - - if ((is_array($permissions_array)) && (array_key_exists('admin', $permissions_array))) { - unset($permissions_array['admin']); - } - } - - - $user->permissions = $permissions_array; - } if ($request->has('location_id')) { // Update the location of any assets checked out to this user diff --git a/tests/Feature/Users/Api/UpdateUserTest.php b/tests/Feature/Users/Api/UpdateUserTest.php index e08706a091..558a443136 100644 --- a/tests/Feature/Users/Api/UpdateUserTest.php +++ b/tests/Feature/Users/Api/UpdateUserTest.php @@ -253,7 +253,8 @@ class UpdateUserTest extends TestCase } - public function testAdminsCannotEditEscalationFieldsForSuperadmins() + + public function testAdminsCannotDeescalateSuperadmins() { $hashed_original = Hash::make('my-awesome-password!!!!!12345'); $hashed_new = Hash::make('!ABCDEFGIJKL123!!!'); @@ -277,7 +278,7 @@ class UpdateUserTest extends TestCase 'username' => 'testnewusername', 'email' => 'testnewemail@example.org', 'activated' => 0, - 'permissions' => "{'superadmin':1}", + 'permissions' => '{"admin":"1"}', 'password' => $hashed_new, ]);