svc-admin01/snipe-it
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Disallow ldap_import and activated in bulk editing users if user doesn’t have permission

Browse Source
This commit is contained in:
snipe
2026-05-21 14:45:04 +01:00
parent 480d252173
commit 403f9c848b
5 changed files with 255 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/Http/Controllers/Users/BulkUsersController.php
+20 -8
View File
@@ -171,8 +171,6 @@ class BulkUsersController extends Controller
->conditionallyAddItem('company_id')
->conditionallyAddItem('locale')
->conditionallyAddItem('remote')
->conditionallyAddItem('ldap_import')
->conditionallyAddItem('activated')
->conditionallyAddItem('display_name')
->conditionallyAddItem('start_date')
->conditionallyAddItem('end_date')
@@ -235,11 +233,21 @@ class BulkUsersController extends Controller
->update(['location_id' => $this->update_array['location_id']]);
}
// Only sync groups if groups were selected
if ($request->filled('groups')) {
foreach ($users as $user) {
if (auth()->user()->can('canEditAuthFields', $user) && auth()->user()->can('editableOnDemo')) {
// Fields that require canEditAuthFields (non-admins cannot touch admins/superusers,
// admins cannot touch superusers) must be applied per-user, not via mass update.
foreach ($users as $user) {
if (auth()->user()->can('canEditAuthFields', $user) && auth()->user()->can('editableOnDemo')) {
$authFieldUpdate = [];
if ($request->filled('activated')) {
$authFieldUpdate['activated'] = $request->input('activated');
}
if ($request->filled('ldap_import')) {
$authFieldUpdate['ldap_import'] = $request->input('ldap_import');
}
if (! empty($authFieldUpdate)) {
$user->update($authFieldUpdate);
}
if ($request->filled('groups')) {
$user->groups()->sync($request->input('groups'));
}
}
@@ -398,7 +406,7 @@ class BulkUsersController extends Controller
*/
public function merge(Request $request)
{
$this->authorize('update', User::class);
$this->authorize('delete', User::class);
if (config('app.lock_passwords')) {
return redirect()->route('users.index')->with('error', trans('general.feature_disabled'));
@@ -419,6 +427,10 @@ class BulkUsersController extends Controller
// Walk users
foreach ($users_to_merge as $user_to_merge) {
if (! auth()->user()->can('canEditAuthFields', $user_to_merge) || ! auth()->user()->can('editableOnDemo')) {
return redirect()->route('users.index')->with('error', trans('general.insufficient_permissions'));
}
foreach ($user_to_merge->assets as $asset) {
Log::debug('Updating asset: '.$asset->asset_tag.' to '.$merge_into_user->id);
$asset->assigned_to = $request->input('merge_into_id');
resources/lang/en-US/general.php
+1
View File
@@ -679,6 +679,7 @@ return [
'user_managed_passwords' => 'Password Management',
'user_managed_passwords_disallow' => 'Disallow users from managing their own passwords',
'user_managed_passwords_allow' => 'Allow users to manage their own passwords',
'user_managed_passwords_bulk_help' => 'This setting will only be applied to users you are abler to edit authentication settings on.',
'from' => 'From',
'by' => 'By',
'by_user' => 'By',
resources/views/users/bulk-edit.blade.php
+1
View File
@@ -159,6 +159,7 @@
<input type="radio" name="ldap_import" id="ldap_import" value="1" aria-label="ldap_import">
{{ trans('general.user_managed_passwords_disallow') }}
</label>
<p class="help-block">{{ trans('general.user_managed_passwords_bulk_help') }}</p>
</div>
</div> <!--/form-group-->
tests/Feature/Users/Ui/BulkActions/BulkEditUsersTest.php
+124
View File
@@ -0,0 +1,124 @@
<?php
namespace Tests\Feature\Users\Ui\BulkActions;
use App\Models\Asset;
use App\Models\User;
use Tests\TestCase;
class BulkEditUsersTest extends TestCase
{
public function test_requires_correct_permission()
{
$this->actingAs(User::factory()->create())
->post(route('users/bulkeditsave'), [
'ids' => [User::factory()->create()->id],
])
->assertForbidden();
}
public function test_non_admin_cannot_deactivate_admin_via_bulk_edit()
{
$actor = User::factory()->editUsers()->create();
$admin = User::factory()->admin()->create(['activated' => 1]);
$this->actingAs($actor)
->post(route('users/bulkeditsave'), [
'ids' => [$admin->id],
'activated' => '0',
])
->assertRedirect(route('users.index'));
$this->assertEquals(1, $admin->fresh()->activated);
}
public function test_non_admin_cannot_deactivate_superuser_via_bulk_edit()
{
$actor = User::factory()->editUsers()->create();
$superuser = User::factory()->superuser()->create(['activated' => 1]);
$this->actingAs($actor)
->post(route('users/bulkeditsave'), [
'ids' => [$superuser->id],
'activated' => '0',
])
->assertRedirect(route('users.index'));
$this->assertEquals(1, $superuser->fresh()->activated);
}
public function test_admin_cannot_deactivate_superuser_via_bulk_edit()
{
$admin = User::factory()->admin()->create();
$superuser = User::factory()->superuser()->create(['activated' => 1]);
$this->actingAs($admin)
->post(route('users/bulkeditsave'), [
'ids' => [$superuser->id],
'activated' => '0',
])
->assertRedirect(route('users.index'));
$this->assertEquals(1, $superuser->fresh()->activated);
}
public function test_non_admin_can_deactivate_regular_user_via_bulk_edit()
{
$actor = User::factory()->editUsers()->create();
$target = User::factory()->create(['activated' => 1]);
$this->actingAs($actor)
->post(route('users/bulkeditsave'), [
'ids' => [$target->id],
'activated' => '0',
])
->assertRedirect(route('users.index'));
$this->assertEquals(0, $target->fresh()->activated);
}
public function test_admin_can_deactivate_regular_user_via_bulk_edit()
{
$admin = User::factory()->admin()->create();
$target = User::factory()->create(['activated' => 1]);
$this->actingAs($admin)
->post(route('users/bulkeditsave'), [
'ids' => [$target->id],
'activated' => '0',
])
->assertRedirect(route('users.index'));
$this->assertEquals(0, $target->fresh()->activated);
}
public function test_non_admin_cannot_set_ldap_import_on_admin_via_bulk_edit()
{
$actor = User::factory()->editUsers()->create();
$admin = User::factory()->admin()->create(['ldap_import' => 0]);
$this->actingAs($actor)
->post(route('users/bulkeditsave'), [
'ids' => [$admin->id],
'ldap_import' => '1',
])
->assertRedirect(route('users.index'));
$this->assertEquals(0, $admin->fresh()->ldap_import);
}
public function test_non_auth_fields_are_still_updated_for_admin_targets()
{
$actor = User::factory()->editUsers()->create();
$admin = User::factory()->admin()->create(['city' => 'Springfield']);
$this->actingAs($actor)
->post(route('users/bulkeditsave'), [
'ids' => [$admin->id],
'city' => 'Shelbyville',
])
->assertRedirect(route('users.index'));
$this->assertEquals('Shelbyville', $admin->fresh()->city);
}
}
tests/Feature/Users/Ui/BulkActions/BulkMergeUsersTest.php
+109
View File
@@ -0,0 +1,109 @@
<?php
namespace Tests\Feature\Users\Ui\BulkActions;
use App\Models\Asset;
use App\Models\User;
use Tests\TestCase;
class BulkMergeUsersTest extends TestCase
{
public function test_requires_delete_permission()
{
$target = User::factory()->create();
$to_merge = User::factory()->create();
$this->actingAs(User::factory()->editUsers()->create())
->post(route('users.merge.save'), [
'ids_to_merge' => [$to_merge->id],
'merge_into_id' => $target->id,
])
->assertForbidden();
$this->assertNotSoftDeleted($to_merge);
}
public function test_non_admin_cannot_merge_admin_into_self()
{
$actor = User::factory()->deleteUsers()->create();
$admin = User::factory()->admin()->create();
$this->actingAs($actor)
->post(route('users.merge.save'), [
'ids_to_merge' => [$admin->id],
'merge_into_id' => $actor->id,
])
->assertRedirect(route('users.index'))
->assertSessionHas('error');
$this->assertNotSoftDeleted($admin);
}
public function test_non_admin_cannot_merge_superuser_into_self()
{
$actor = User::factory()->deleteUsers()->create();
$superuser = User::factory()->superuser()->create();
$this->actingAs($actor)
->post(route('users.merge.save'), [
'ids_to_merge' => [$superuser->id],
'merge_into_id' => $actor->id,
])
->assertRedirect(route('users.index'))
->assertSessionHas('error');
$this->assertNotSoftDeleted($superuser);
}
public function test_admin_cannot_merge_superuser_into_self()
{
$admin = User::factory()->admin()->create();
$superuser = User::factory()->superuser()->create();
$this->actingAs($admin)
->post(route('users.merge.save'), [
'ids_to_merge' => [$superuser->id],
'merge_into_id' => $admin->id,
])
->assertRedirect(route('users.index'))
->assertSessionHas('error');
$this->assertNotSoftDeleted($superuser);
}
public function test_assets_are_transferred_and_source_user_is_deleted_on_merge()
{
$admin = User::factory()->admin()->create();
$source = User::factory()->create();
$target = User::factory()->create();
$asset = Asset::factory()->assignedToUser($source)->create();
$this->actingAs($admin)
->post(route('users.merge.save'), [
'ids_to_merge' => [$source->id],
'merge_into_id' => $target->id,
])
->assertRedirect(route('users.index'))
->assertSessionHas('success');
$this->assertSoftDeleted($source);
$this->assertEquals($target->id, $asset->fresh()->assigned_to);
}
public function test_merge_does_not_transfer_assets_when_source_is_protected()
{
$actor = User::factory()->deleteUsers()->create();
$admin = User::factory()->admin()->create();
$asset = Asset::factory()->assignedToUser($admin)->create();
$this->actingAs($actor)
->post(route('users.merge.save'), [
'ids_to_merge' => [$admin->id],
'merge_into_id' => $actor->id,
])
->assertRedirect(route('users.index'))
->assertSessionHas('error');
$this->assertEquals($admin->id, $asset->fresh()->assigned_to);
}
}