DRCIT/snipe-it
1
0
Fork 0
mirror of https://github.com/snipe/snipe-it.git synced 2024-09-19 15:27:39 -07:00
Code Issues Actions 7 Packages Projects Releases Wiki Activity

Merge pull request #14745 from snipe/fixes/self_api_endpoint

Browse source 
Refactor group syncing on user edit API endpoint
This commit is contained in:
snipe 2024-05-22 12:43:22 +01:00 committed by GitHub
parent f422c2ecad 6fad085a7d
commit c9f7847fb3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 89 additions and 29 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

29
app/Http/Controllers/Api/UsersController.php
View file

@ -475,7 +475,7 @@ class UsersController extends Controller
if ($request->has('permissions')) {
$permissions_array = $request->input('permissions');
// Strip out the superuser permission if the API user isn't a superadmin
// Strip out the individual superuser permission if the API user isn't a superadmin
if (! Auth::user()->isSuperUser()) {
unset($permissions_array['superuser']);
}
@ -493,32 +493,23 @@ class UsersController extends Controller
if ($user->save()) {
// Check if the request has groups passed and has a value
if ($request->filled('groups')) {
// Check if the request has groups passed and has a value, AND that the user us a superuser
if (($request->has('groups')) && (Auth::user()->isSuperUser())) {
$validator = Validator::make($request->all(), [
$validator = Validator::make($request->only('groups'), [
'groups.*' => 'integer|exists:permission_groups,id',
]);
if ($validator->fails()){
return response()->json(Helper::formatStandardApiResponse('error', null, $user->getErrors()));
if ($validator->fails()) {
return response()->json(Helper::formatStandardApiResponse('error', null, $validator->errors()));
}
// Only save groups if the user is a superuser
if (Auth::user()->isSuperUser()) {
$user->groups()->sync($request->input('groups'));
}
// Sync the groups since the user is a superuser and the groups pass validation
$user->groups()->sync($request->input('groups'));
// The groups field has been passed but it is null, so we should blank it out
} elseif ($request->has('groups')) {
// Only save groups if the user is a superuser
if (Auth::user()->isSuperUser()) {
$user->groups()->sync($request->input('groups'));
}
}
return response()->json(Helper::formatStandardApiResponse('success', (new UsersTransformer)->transformUser($user), trans('admin/users/message.success.update')));
}

2
resources/views/groups/view.blade.php
View file

@ -52,7 +52,7 @@
@if (is_array($group->decodePermissions()))
<ul class="list-unstyled">
@foreach ($group->decodePermissions() as $permission_name => $permission)
<li>{!! ($permission == '1') ? '<i class="fas fa-check text-success" aria-hidden="true"></i><span class="sr-only">GRANTED: </span>' : '<i class="fas fa-times text-danger" aria-hidden="true"></i><span class="sr-only">DENIED: </span>' !!} {{ e(str_replace('.', ': ', ucwords($permission_name))) }} </li>
<li>{!! ($permission == '1') ? '<i class="fas fa-check text-success" aria-hidden="true"></i><span class="sr-only">'.trans('general.yes').': </span>' : '<i class="fas fa-times text-danger" aria-hidden="true"></i><span class="sr-only">'.trans('general.no').': </span>' !!} {{ e(str_replace('.', ': ', ucwords($permission_name))) }} </li>
@endforeach
</ul>

87
tests/Feature/Api/Users/UpdateUserApiTest.php
View file

@ -127,27 +127,27 @@ class UpdateUserApiTest extends TestCase
{
$groupToJoin = Group::factory()->create();
$normalUser = User::factory()->editUsers()->create();
$userWhoCanEditUsers = User::factory()->editUsers()->create();
$superUser = User::factory()->superuser()->create();
$oneUserToUpdate = User::factory()->create();
$anotherUserToUpdate = User::factory()->create();
$userToUpdateByUserWhoCanEditUsers = User::factory()->create();
$userToUpdateByToUserBySuperuser = User::factory()->create();
$this->actingAsForApi($normalUser)
->patchJson(route('api.users.update', $oneUserToUpdate), [
$this->actingAsForApi($userWhoCanEditUsers)
->patchJson(route('api.users.update', $userToUpdateByUserWhoCanEditUsers), [
'groups' => [$groupToJoin->id],
]);
$this->actingAsForApi($superUser)
->patchJson(route('api.users.update', $anotherUserToUpdate), [
->patchJson(route('api.users.update', $userToUpdateByToUserBySuperuser), [
'groups' => [$groupToJoin->id],
]);
$this->assertFalse(
$oneUserToUpdate->refresh()->groups->contains($groupToJoin),
$this->assertFalse($userToUpdateByUserWhoCanEditUsers->refresh()->groups->contains($groupToJoin),
'Non-super-user was able to modify user group'
);
$this->assertTrue($anotherUserToUpdate->refresh()->groups->contains($groupToJoin));
$this->assertTrue($userToUpdateByToUserBySuperuser->refresh()->groups->contains($groupToJoin));
}
public function testUserGroupsCanBeClearedBySuperUser()
@ -175,4 +175,73 @@ class UpdateUserApiTest extends TestCase
$this->assertTrue($oneUserToUpdate->refresh()->groups->contains($joinedGroup));
$this->assertFalse($anotherUserToUpdate->refresh()->groups->contains($joinedGroup));
}
public function testNonSuperuserCannotUpdateOwnGroups()
{
$groupToJoin = Group::factory()->create();
$user = User::factory()->editUsers()->create();
$this->actingAsForApi($user)
->patchJson(route('api.users.update', $user), [
'groups' => [$groupToJoin->id],
]);
$this->assertFalse($user->refresh()->groups->contains($groupToJoin),
'Non-super-user was able to modify user group'
);
}
public function testNonSuperuserCannotUpdateGroups()
{
$user = User::factory()->editUsers()->create();
$group = Group::factory()->create();
$user->groups()->sync([$group->id]);
$newGroupToJoin = Group::factory()->create();
$this->actingAsForApi($user)
->patchJson(route('api.users.update', $user), [
'groups' => [$newGroupToJoin->id],
]);
$this->assertFalse($user->refresh()->groups->contains($newGroupToJoin),
'Non-super-user was able to modify user group membership'
);
$this->assertTrue($user->refresh()->groups->contains($group));
}
public function testUsersGroupsAreNotClearedIfNoGroupPassedBySuperUser()
{
$user = User::factory()->create();
$superUser = User::factory()->superuser()->create();
$group = Group::factory()->create();
$user->groups()->sync([$group->id]);
$this->actingAsForApi($superUser)
->patchJson(route('api.users.update', $user), []);
$this->assertTrue($user->refresh()->groups->contains($group));
}
public function testMultipleGroupsUpdateBySuperUser()
{
$user = User::factory()->create();
$superUser = User::factory()->superuser()->create();
$groupA = Group::factory()->create(['name'=>'Group A']);
$groupB = Group::factory()->create(['name'=>'Group B']);
$this->actingAsForApi($superUser)
->patchJson(route('api.users.update', $user), [
'groups' => [$groupA->id, $groupB->id],
])->json();
$this->assertTrue($user->refresh()->groups->contains($groupA));
$this->assertTrue($user->refresh()->groups->contains($groupB));
}
}