DRCIT/snipe-it
1
0
Fork 0
mirror of https://github.com/snipe/snipe-it.git synced 2025-02-21 03:15:45 -08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

Merge pull request #14801 from snipe/fixes/companyable_trait_on_users

Browse source 
Allow CompanyableTrait trait on users
This commit is contained in:
snipe 2024-06-12 12:42:56 +01:00 committed by GitHub
parent 1ca9bb5aa8 3159fdec9f
commit a8b47a55bc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 242 additions and 81 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -13,7 +13,6 @@ use App\Http\Transformers\SelectlistTransformer;
use App\Http\Transformers\UsersTransformer;
use App\Models\Actionlog;
use App\Models\Asset;
use App\Models\Company;
use App\Models\License;
use App\Models\User;
use App\Notifications\CurrentInventory;
@ -290,10 +289,6 @@ class UsersController extends Controller
// Apply companyable scope
$users = Company::scopeCompanyables($users);
// Make sure the offset and limit are actually integers and do not exceed system limits
$offset = ($request->input('offset') > $users->count()) ? $users->count() : app('api_offset_value');
$limit = app('api_limit_value');
@ -326,8 +321,6 @@ class UsersController extends Controller
]
)->where('show_in_list', '=', '1');
$users = Company::scopeCompanyables($users);
if ($request->filled('search')) {
$users = $users->where(function ($query) use ($request) {
$query->SimpleNameSearch($request->get('search'))
@ -422,9 +415,7 @@ class UsersController extends Controller
{
$this->authorize('view', User::class);
$user = User::withCount('assets as assets_count', 'licenses as licenses_count', 'accessories as accessories_count', 'consumables as consumables_count', 'managesUsers as manages_users_count', 'managedLocations as manages_locations_count');
if ($user = Company::scopeCompanyables($user)->find($id)) {
if ($user = User::withCount('assets as assets_count', 'licenses as licenses_count', 'accessories as accessories_count', 'consumables as consumables_count', 'managesUsers as manages_users_count', 'managedLocations as manages_locations_count')->find($id)) {
$this->authorize('view', $user);
return (new UsersTransformer)->transformUser($user);
}
@ -447,15 +438,12 @@ class UsersController extends Controller
{
$this->authorize('update', User::class);
$user = User::findOrFail($id);
$user = Company::scopeCompanyables($user)->find($id);
$user = User::find($id);
$this->authorize('update', $user);
/**
* This is a janky hack to prevent people from changing admin demo user data on the public demo.
*
* The $ids 1 and 2 are special since they are seeded as superadmins in the demo seeder.
*
* Thanks, jerks. You are why we can't have nice things. - snipe
*
*/
@ -534,8 +522,8 @@ class UsersController extends Controller
public function destroy(DeleteUserRequest $request, $id)
{
$this->authorize('delete', User::class);
$user = User::with('assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc');
$user = Company::scopeCompanyables($user)->find($id);
$user = User::with('assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc')->withTrashed()->find($id);
$this->authorize('delete', $user);
@ -554,9 +542,12 @@ class UsersController extends Controller
return response()->json(Helper::formatStandardApiResponse('success', null, trans('admin/users/message.success.delete')));
}
return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/users/message.error.delete')));
}
return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/users/message.user_not_found', compact('id'))));
}
/**
@ -572,32 +563,35 @@ class UsersController extends Controller
$this->authorize('view', User::class);
$this->authorize('view', Asset::class);
$user = User::with('assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc')->withTrashed();
$user = Company::scopeCompanyables($user)->find($id);
$this->authorize('view', $user);
if ($user = User::with('assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc')->withTrashed()->find($id)) {
$this->authorize('view', $user);
$assets = Asset::where('assigned_to', '=', $id)->where('assigned_type', '=', User::class)->with('model');
$assets = Asset::where('assigned_to', '=', $id)->where('assigned_type', '=', User::class)->with('model');
// Filter on category ID
if ($request->filled('category_id')) {
$assets = $assets->InCategory($request->input('category_id'));
}
// Filter on model ID
if ($request->filled('model_id')) {
$model_ids = $request->input('model_id');
if (!is_array($model_ids)) {
$model_ids = array($model_ids);
// Filter on category ID
if ($request->filled('category_id')) {
$assets = $assets->InCategory($request->input('category_id'));
}
$assets = $assets->InModelList($model_ids);
// Filter on model ID
if ($request->filled('model_id')) {
$model_ids = $request->input('model_id');
if (!is_array($model_ids)) {
$model_ids = array($model_ids);
}
$assets = $assets->InModelList($model_ids);
}
$assets = $assets->get();
return (new AssetsTransformer)->transformAssets($assets, $assets->count(), $request);
}
$assets = $assets->get();
return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/users/message.user_not_found', compact('id'))));
return (new AssetsTransformer)->transformAssets($assets, $assets->count(), $request);
}
/**
@ -612,17 +606,21 @@ class UsersController extends Controller
public function emailAssetList(Request $request, $id)
{
$this->authorize('update', User::class);
$user = User::findOrFail($id);
$user = Company::scopeCompanyables($user)->find($id);
$this->authorize('update', $user);
if (empty($user->email)) {
return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/users/message.inventorynotification.error')));
if ($user = User::find($id)) {
$this->authorize('update', $user);
if (empty($user->email)) {
return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/users/message.inventorynotification.error')));
}
$user->notify((new CurrentInventory($user)));
return response()->json(Helper::formatStandardApiResponse('success', null, trans('admin/users/message.inventorynotification.success')));
}
$user->notify((new CurrentInventory($user)));
return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/users/message.user_not_found', compact('id'))));
return response()->json(Helper::formatStandardApiResponse('success', null, trans('admin/users/message.inventorynotification.success')));
}
/**

37
app/Http/Controllers/Users/UsersController.php
View file

@ -182,8 +182,7 @@ class UsersController extends Controller
{
$this->authorize('update', User::class);
$user = User::with('assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc')->withTrashed();
$user = Company::scopeCompanyables($user)->find($id);
$user = User::with('assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc')->withTrashed()->find($id);
if ($user) {
@ -229,9 +228,7 @@ class UsersController extends Controller
$permissions = $request->input('permissions', []);
app('request')->request->set('permissions', $permissions);
$user = User::with('assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc')->withTrashed();
$user = Company::scopeCompanyables($user)->find($id);
$user = User::with('assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc')->withTrashed()->find($id);
// User is valid - continue...
if ($user) {
@ -338,8 +335,8 @@ class UsersController extends Controller
{
$this->authorize('delete', User::class);
$user = User::with('assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc');
$user = Company::scopeCompanyables($user)->find($id);
if (($user) && ($user->deleted_at = '')) {
// Delete the user
@ -408,8 +405,7 @@ class UsersController extends Controller
// Make sure the user can view users at all
$this->authorize('view', User::class);
$user = User::with('assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc')->withTrashed();
$user = Company::scopeCompanyables($user)->find($userId);
$user = User::with('assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc')->withTrashed()->find($userId);
// Make sure they can view this particular user
$this->authorize('view', $user);
@ -444,9 +440,7 @@ class UsersController extends Controller
app('request')->request->set('permissions', $permissions);
$user_to_clone = User::with('assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc')->withTrashed();
$user_to_clone = Company::scopeCompanyables($user_to_clone)->find($id);
$user_to_clone = User::with('assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc')->withTrashed()->find($id);
// Make sure they can view this particular user
$this->authorize('view', $user_to_clone);
@ -510,10 +504,7 @@ class UsersController extends Controller
'groups',
'userloc',
'company'
)->orderBy('created_at', 'DESC');
// FMCS scoping
Company::scopeCompanyables($users)
)->orderBy('created_at', 'DESC')
->chunk(500, function ($users) use ($handle) {
$headers = [
// strtolower to prevent Excel from trying to open it as a SYLK file
@ -607,20 +598,20 @@ class UsersController extends Controller
public function printInventory($id)
{
$this->authorize('view', User::class);
$show_user = Company::scopeCompanyables(User::where('id', $id)->withTrashed()->first());
$user = User::where('id', $id)->withTrashed()->first();
// Make sure they can view this particular user
$this->authorize('view', $show_user);
$this->authorize('view', $user);
$assets = Asset::where('assigned_to', $id)->where('assigned_type', User::class)->with('model', 'model.category')->get();
$accessories = $show_user->accessories()->get();
$consumables = $show_user->consumables()->get();
$accessories = $user->accessories()->get();
$consumables = $user->consumables()->get();
return view('users/print')->with('assets', $assets)
->with('licenses', $show_user->licenses()->get())
->with('licenses', $user->licenses()->get())
->with('accessories', $accessories)
->with('consumables', $consumables)
->with('show_user', $show_user)
->with('show_user', $user)
->with('settings', Setting::getSettings());
}
@ -636,7 +627,7 @@ class UsersController extends Controller
{
$this->authorize('view', User::class);
$user = Company::scopeCompanyables(User::find($id));
$user = User::find($id);
// Make sure they can view this particular user
$this->authorize('view', $user);
@ -664,7 +655,7 @@ class UsersController extends Controller
*/
public function sendPasswordReset($id)
{
if (($user = Company::scopeCompanyables(User::find($id))) && ($user->activated == '1') && ($user->email != '') && ($user->ldap_import == '0')) {
if (($user = User::find($id)) && ($user->activated == '1') && ($user->email != '') && ($user->ldap_import == '0')) {
$credentials = ['email' => trim($user->email)];
try {

28
app/Models/Company.php
View file

@ -5,11 +5,11 @@ namespace App\Models;
use App\Models\Traits\Searchable;
use App\Presenters\Presentable;
use Illuminate\Support\Facades\Auth;
use Illuminate\Support\Facades\DB;
use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Support\Facades\Gate;
use Watson\Validating\ValidatingTrait;
use Illuminate\Support\Facades\Log;
use Illuminate\Support\Facades\Schema;
/**
* Model for Companies.
*
@ -147,7 +147,7 @@ final class Company extends SnipeModel
// This is primary for the gate:allows-check in location->isDeletable()
// Locations don't have a company_id so without this it isn't possible to delete locations with FullMultipleCompanySupport enabled
// because this function is called by SnipePermissionsPolicy->before()
if (!$companyable instanceof Company && !\Schema::hasColumn($company_table, 'company_id')) {
if (!$companyable instanceof Company && !Schema::hasColumn($company_table, 'company_id')) {
return true;
}
@ -259,7 +259,7 @@ final class Company extends SnipeModel
public static function scopeCompanyables($query, $column = 'company_id', $table_name = null)
{
// If not logged in and hitting this, assume we are on the command line and don't scope?'
if (! static::isFullMultipleCompanySupportEnabled() || (Auth::check() && Auth::user()->isSuperUser()) || (! Auth::check())) {
if (! static::isFullMultipleCompanySupportEnabled() || (Auth::hasUser() && Auth::user()->isSuperUser()) || (! Auth::hasUser())) {
return $query;
} else {
return static::scopeCompanyablesDirectly($query, $column, $table_name);
@ -267,27 +267,31 @@ final class Company extends SnipeModel
}
/**
* Scoping table queries, determining if a logged in user is part of a company, and only allows
* Scoping table queries, determining if a logged-in user is part of a company, and only allows
* that user to see items associated with that company
*
* @see https://github.com/laravel/framework/pull/24518 for info on Auth::hasUser()
*/
private static function scopeCompanyablesDirectly($query, $column = 'company_id', $table_name = null)
{
// Get the company ID of the logged in user, or set it to null if there is no company assicoated with the user
if (Auth::user()) {
// Get the company ID of the logged-in user, or set it to null if there is no company associated with the user
if (Auth::hasUser()) {
$company_id = Auth::user()->company_id;
} else {
$company_id = null;
}
// Dynamically get the table name if it's not passed in, based on the model we're querying against
$table = ($table_name) ? $table_name."." : $query->getModel()->getTable().".";
// If the column exists in the table, use it to scope the query
if (\Schema::hasColumn($query->getModel()->getTable(), $column)) {
if ((($query) && ($query->getModel()) && (Schema::hasColumn($query->getModel()->getTable(), $column)))) {
// Dynamically get the table name if it's not passed in, based on the model we're querying against
$table = ($table_name) ? $table_name."." : $query->getModel()->getTable().".";
return $query->where($table.$column, '=', $company_id);
} else {
return $query->join('users as users_comp', 'users_comp.id', 'user_id')->where('users_comp.company_id', '=', $company_id);
}
}
/**
@ -305,7 +309,7 @@ final class Company extends SnipeModel
if (count($companyable_names) == 0) {
throw new Exception('No Companyable Children to scope');
} elseif (! static::isFullMultipleCompanySupportEnabled() || (Auth::check() && Auth::user()->isSuperUser())) {
} elseif (! static::isFullMultipleCompanySupportEnabled() || (Auth::hasUser() && Auth::user()->isSuperUser())) {
return $query;
} else {
$f = function ($q) {

2
app/Models/User.php
View file

@ -5,7 +5,6 @@ namespace App\Models;
use App\Http\Traits\UniqueUndeletedTrait;
use App\Models\Traits\Searchable;
use App\Presenters\Presentable;
use Illuminate\Support\Facades\DB;
use Illuminate\Auth\Authenticatable;
use Illuminate\Auth\Passwords\CanResetPassword;
use Illuminate\Contracts\Auth\Access\Authorizable as AuthorizableContract;
@ -24,6 +23,7 @@ use Watson\Validating\ValidatingTrait;
class User extends SnipeModel implements AuthenticatableContract, AuthorizableContract, CanResetPasswordContract, HasLocalePreference
{
use HasFactory;
use CompanyableTrait;
protected $presenter = \App\Presenters\UserPresenter::class;
use SoftDeletes, ValidatingTrait;

2
resources/views/users/print.blade.php
View file

@ -470,7 +470,7 @@
});
});
</script>
@endpush
</body>

4
tests/Feature/Users/Api/DeleteUserTest.php
View file

@ -56,7 +56,7 @@ class DeleteUserTest extends TestCase
->json();
}
public function testDisallowUserDeletionIfNoDeletePermissions()
public function testPermissionsForDeletingUsers()
{
$this->actingAsForApi(User::factory()->create())
@ -65,7 +65,7 @@ class DeleteUserTest extends TestCase
->json();
}
public function testDisallowUserDeletionIfNotInSameCompanyAndNotSuperadmin()
public function testPermissionsIfNotInSameCompanyAndNotSuperadmin()
{
$this->settings->enableMultipleFullCompanySupport();
[$companyA, $companyB] = Company::factory()->count(2)->create();

61
tests/Feature/Users/Api/ViewUserTest.php Normal file
View file

@ -0,0 +1,61 @@
<?php
namespace Tests\Feature\Users\Api;
use App\Models\Company;
use App\Models\User;
use Illuminate\Testing\Fluent\AssertableJson;
use Laravel\Passport\Passport;
use Tests\TestCase;
class ViewUserTest extends TestCase
{
public function testCanReturnUser()
{
$user = User::factory()->create();
$this->actingAsForApi(User::factory()->viewUsers()->create())
->getJson(route('api.users.show', $user))
->assertOk();
}
public function testPermissionsWithCompanyableToDeleteUser()
{
$this->settings->enableMultipleFullCompanySupport();
[$companyA, $companyB] = Company::factory()->count(2)->create();
$superuser = User::factory()->superuser()->create();
$userFromA = User::factory()->for($companyA)->create();
$userFromB = User::factory()->for($companyB)->create();
$this->actingAsForApi(User::factory()->deleteUsers()->for($companyA)->create())
->deleteJson(route('api.users.destroy', $userFromA->id))
->assertOk()
->assertStatus(200)
->assertStatusMessageIs('success')
->json();
$this->actingAsForApi(User::factory()->deleteUsers()->for($companyB)->create())
->deleteJson(route('api.users.destroy', $userFromA->id))
->assertStatus(403);
$this->actingAsForApi($superuser)
->deleteJson(route('api.users.destroy', $userFromA->id))
->assertOk()
->assertStatus(200)
->assertStatusMessageIs('success')
->json();
$this->actingAsForApi($superuser)
->deleteJson(route('api.users.destroy', $userFromB->id))
->assertOk()
->assertStatus(200)
->assertStatusMessageIs('success')
->json();
}
}

23
tests/Feature/Users/Ui/DeleteUserTest.php
View file

@ -7,12 +7,35 @@ use App\Models\LicenseSeat;
use App\Models\Location;
use App\Models\Accessory;
use App\Models\User;
use App\Models\Company;
use App\Models\Asset;
class DeleteUserTest extends TestCase
{
public function testPermissionsToDeleteUser()
{
$this->settings->enableMultipleFullCompanySupport();
[$companyA, $companyB] = Company::factory()->count(2)->create();
$superuser = User::factory()->superuser()->create();
$userFromA = User::factory()->for($companyA)->create();
$userFromB = User::factory()->for($companyB)->create();
$this->followingRedirects()->actingAs(User::factory()->deleteUsers()->for($companyA)->create())
->delete(route('users.destroy', ['user' => $userFromB->id]))
->assertStatus(403);
$this->actingAs(User::factory()->deleteUsers()->for($companyA)->create())
->delete(route('users.destroy', ['user' => $userFromA->id]))
->assertStatus(302)
->assertRedirect(route('users.index'));
}
public function testDisallowUserDeletionIfStillManagingPeople()
{

84
tests/Feature/Users/Ui/ViewUserTest.php Normal file
View file

@ -0,0 +1,84 @@
<?php
namespace Feature\Users\Ui;
use App\Models\Company;
use App\Models\User;
use App\Notifications\CurrentInventory;
use Illuminate\Support\Facades\Notification;
use Tests\TestCase;
class ViewUserTest extends TestCase
{
public function testPermissionsForUserDetailPage()
{
$this->settings->enableMultipleFullCompanySupport();
[$companyA, $companyB] = Company::factory()->count(2)->create();
$superuser = User::factory()->superuser()->create();
$user = User::factory()->for($companyB)->create();
$this->actingAs(User::factory()->editUsers()->for($companyA)->create())
->get(route('users.show', ['user' => $user->id]))
->assertStatus(403);
$this->actingAs($superuser)
->get(route('users.show', ['user' => $user->id]))
->assertOk()
->assertStatus(200);
}
public function testPermissionsForPrintAllInventoryPage()
{
$this->settings->enableMultipleFullCompanySupport();
[$companyA, $companyB] = Company::factory()->count(2)->create();
$superuser = User::factory()->superuser()->create();
$user = User::factory()->for($companyB)->create();
$this->actingAs(User::factory()->viewUsers()->for($companyA)->create())
->get(route('users.print', ['userId' => $user->id]))
->assertStatus(403);
$this->actingAs(User::factory()->viewUsers()->for($companyB)->create())
->get(route('users.print', ['userId' => $user->id]))
->assertStatus(200);
$this->actingAs($superuser)
->get(route('users.print', ['userId' => $user->id]))
->assertOk()
->assertStatus(200);
}
public function testUserWithoutCompanyPermissionsCannotSendInventory()
{
Notification::fake();
$this->settings->enableMultipleFullCompanySupport();
[$companyA, $companyB] = Company::factory()->count(2)->create();
$superuser = User::factory()->superuser()->create();
$user = User::factory()->for($companyB)->create();
$this->actingAs(User::factory()->viewUsers()->for($companyA)->create())
->post(route('users.email', ['userId' => $user->id]))
->assertStatus(403);
$this->actingAs(User::factory()->viewUsers()->for($companyB)->create())
->post(route('users.email', ['userId' => $user->id]))
->assertStatus(302);
$this->actingAs($superuser)
->post(route('users.email', ['userId' => $user->id]))
->assertStatus(302);
Notification::assertSentTo(
[$user], CurrentInventory::class
);
}
}