QT/InvoiceShelf
1
0
Fork 0
mirror of https://github.com/InvoiceShelf/InvoiceShelf.git synced 2026-07-16 13:55:21 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(members): scope member view & update to the acting company

Browse Source 
Member view/update bound the target user by global id and authorized only that the requester owns their active company, not that the target belonged to it. Bind the route model under the members param and require shared company membership in UserPolicy so an owner of one company can no longer read or modify users of another.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Darko Gjorgjijoski
2026-06-14 23:42:11 +02:00
parent 8d929ec09d
commit d3202b8b2a
4 changed files with 50 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
app/Http/Controllers/Company/Members/MembersController.php
View File

@@ -61,11 +61,11 @@ class MembersController extends Controller
*
* @return JsonResponse
*/
public function show(User $user)
public function show(User $member)
{
$this->authorize('view', $user);
$this->authorize('view', $member);
return new UserResource($user);
return new UserResource($member);
}
/**
@@ -73,13 +73,13 @@ class MembersController extends Controller
*
* @return JsonResponse
*/
public function update(MemberRequest $request, User $user)
public function update(MemberRequest $request, User $member)
{
$this->authorize('update', $user);
$this->authorize('update', $member);
$this->memberService->update($user, $request);
$this->memberService->update($member, $request);
return new UserResource($user);
return new UserResource($member);
}
/**

2
app/Http/Requests/MemberRequest.php
View File

@@ -52,7 +52,7 @@ class MemberRequest extends FormRequest
$rules['email'] = [
'required',
new IdnEmail,
Rule::unique('users')->ignore($this->user),
Rule::unique('users')->ignore($this->member),
];
$rules['password'] = [
'nullable',

32
app/Policies/UserPolicy.php
View File

@@ -30,11 +30,7 @@ class UserPolicy
*/
public function view(User $user, User $model): bool
{
if ($user->isOwner()) {
return true;
}
return false;
return $user->isOwner() && $this->sharesActiveCompany($model);
}
/**
@@ -58,11 +54,7 @@ class UserPolicy
*/
public function update(User $user, User $model): bool
{
if ($user->isOwner()) {
return true;
}
return false;
return $user->isOwner() && $this->sharesActiveCompany($model);
}
/**
@@ -72,11 +64,7 @@ class UserPolicy
*/
public function delete(User $user, User $model): bool
{
if ($user->isOwner()) {
return true;
}
return false;
return $user->isOwner() && $this->sharesActiveCompany($model);
}
/**
@@ -134,4 +122,18 @@ class UserPolicy
return false;
}
/**
* A company owner may only act on members of the company set in the
* `company` request header. Without this, view/update/delete resolve the
* target user by global id, which would let an owner of one company read
* or overwrite users belonging to another company (cross-tenant IDOR).
*/
private function sharesActiveCompany(User $model): bool
{
$companyId = request()->header('company');
return $companyId
&& $model->companies()->wherePivot('company_id', $companyId)->exists();
}
}

26
tests/Feature/Admin/MemberTest.php
View File

@@ -8,6 +8,7 @@ use Laravel\Sanctum\Sanctum;
use function Pest\Laravel\getJson;
use function Pest\Laravel\postJson;
use function Pest\Laravel\putJson;
beforeEach(function () {
Artisan::call('db:seed', ['--class' => 'DatabaseSeeder', '--force' => true]);
@@ -37,12 +38,35 @@ test('store member using a form request', function () {
);
});
test('get member', function () {
test('get member belonging to the current company', function () {
$companyId = User::where('role', 'super admin')->first()->companies()->first()->id;
$user = User::factory()->create();
$user->companies()->attach($companyId);
getJson("/api/v1/members/{$user->id}")->assertOk();
});
test('cannot view a member belonging to another company', function () {
$user = User::factory()->create();
$user->companies()->attach(Company::factory()->create()->id);
getJson("/api/v1/members/{$user->id}")->assertForbidden();
});
test('cannot update a member belonging to another company', function () {
$companyId = User::where('role', 'super admin')->first()->companies()->first()->id;
$user = User::factory()->create();
$user->companies()->attach(Company::factory()->create()->id);
putJson("/api/v1/members/{$user->id}", [
'name' => 'Hacked',
'email' => 'pwned@attacker.test',
'companies' => [['id' => $companyId, 'role' => 'super admin']],
])->assertForbidden();
$this->assertDatabaseMissing('users', ['email' => 'pwned@attacker.test']);
});
test('update member using a form request', function () {
$this->assertActionUsesFormRequest(
MembersController::class,