fix(members): block self role-change/removal and lock role rules with tests
The remove/role-change guards only protected the account owner, so a non-owner admin could change their own role or remove themselves via a crafted request (the UI hides it, but the backend didn't). Add an explicit self-guard to both updateRole and removeMember. Lock the whole role system with tests: accept assigns the exact invited role (viewer/admin/member), invite requires and persists a role, updateRole supports viewer and blocks self/owner/invalid, removeMember blocks self/owner, and a viewer is read-only (view yes; create post / manage team / invite no).
This commit is contained in:
parent
6c47bac1b8
commit
f572341e36
3 changed files with 73 additions and 0 deletions
|
|
@ -120,6 +120,11 @@ public function removeMember(Request $request, string $userId): RedirectResponse
|
|||
|
||||
$this->authorize('manageTeam', $workspace);
|
||||
|
||||
// You cannot remove yourself
|
||||
if ($userId === $request->user()->id) {
|
||||
return back()->withErrors(['member' => 'You cannot remove yourself.']);
|
||||
}
|
||||
|
||||
// Account owner cannot be removed
|
||||
if ($userId === $workspace->account?->owner_id) {
|
||||
return back()->withErrors(['member' => 'Cannot remove the account owner.']);
|
||||
|
|
@ -143,6 +148,11 @@ public function updateRole(Request $request, string $userId): RedirectResponse
|
|||
|
||||
$this->authorize('manageTeam', $workspace);
|
||||
|
||||
// You cannot change your own role
|
||||
if ($userId === $request->user()->id) {
|
||||
return back()->withErrors(['role' => 'You cannot change your own role.']);
|
||||
}
|
||||
|
||||
// Account owner's role cannot be changed
|
||||
if ($userId === $workspace->account?->owner_id) {
|
||||
return back()->withErrors(['role' => 'Cannot change the account owner role.']);
|
||||
|
|
|
|||
|
|
@ -227,6 +227,48 @@
|
|||
expect($this->workspace->members()->where('user_id', $member->id)->first()->pivot->role)->toBe(WorkspaceRole::Admin->value);
|
||||
});
|
||||
|
||||
test('update role changes member to viewer', function () {
|
||||
$member = User::factory()->create([
|
||||
'account_id' => $this->account->id,
|
||||
]);
|
||||
$this->workspace->members()->attach($member->id, ['role' => WorkspaceRole::Member->value]);
|
||||
|
||||
$response = $this->actingAs($this->user)->put(route('app.members.update-role', $member), [
|
||||
'role' => WorkspaceRole::Viewer->value,
|
||||
]);
|
||||
|
||||
$response->assertRedirect();
|
||||
expect($this->workspace->members()->where('user_id', $member->id)->first()->pivot->role)->toBe(WorkspaceRole::Viewer->value);
|
||||
});
|
||||
|
||||
test('an admin cannot change their own role', function () {
|
||||
$admin = User::factory()->create([
|
||||
'account_id' => $this->account->id,
|
||||
]);
|
||||
$this->workspace->members()->attach($admin->id, ['role' => WorkspaceRole::Admin->value]);
|
||||
$admin->update(['current_workspace_id' => $this->workspace->id]);
|
||||
|
||||
$response = $this->actingAs($admin)->put(route('app.members.update-role', $admin), [
|
||||
'role' => WorkspaceRole::Member->value,
|
||||
]);
|
||||
|
||||
$response->assertSessionHasErrors('role');
|
||||
expect($this->workspace->members()->where('user_id', $admin->id)->first()->pivot->role)->toBe(WorkspaceRole::Admin->value);
|
||||
});
|
||||
|
||||
test('an admin cannot remove themselves', function () {
|
||||
$admin = User::factory()->create([
|
||||
'account_id' => $this->account->id,
|
||||
]);
|
||||
$this->workspace->members()->attach($admin->id, ['role' => WorkspaceRole::Admin->value]);
|
||||
$admin->update(['current_workspace_id' => $this->workspace->id]);
|
||||
|
||||
$response = $this->actingAs($admin)->delete(route('app.members.remove', $admin));
|
||||
|
||||
$response->assertSessionHasErrors('member');
|
||||
expect($this->workspace->members()->where('user_id', $admin->id)->exists())->toBeTrue();
|
||||
});
|
||||
|
||||
test('update role changes admin to member', function () {
|
||||
$member = User::factory()->create([
|
||||
'account_id' => $this->account->id,
|
||||
|
|
|
|||
|
|
@ -256,6 +256,27 @@
|
|||
expect($this->policy->createPost($otherUser, $workspace))->toBeFalse();
|
||||
});
|
||||
|
||||
test('a viewer can view but cannot create posts or manage the team', function () {
|
||||
$account = Account::factory()->create();
|
||||
$owner = User::factory()->create([
|
||||
'account_id' => $account->id,
|
||||
]);
|
||||
$account->update(['owner_id' => $owner->id]);
|
||||
$viewer = User::factory()->create([
|
||||
'account_id' => $account->id,
|
||||
]);
|
||||
$workspace = Workspace::factory()->create([
|
||||
'account_id' => $account->id,
|
||||
'user_id' => $owner->id,
|
||||
]);
|
||||
$workspace->members()->attach($viewer->id, ['role' => Role::Viewer->value]);
|
||||
|
||||
expect($this->policy->view($viewer, $workspace))->toBeTrue();
|
||||
expect($this->policy->createPost($viewer, $workspace))->toBeFalse();
|
||||
expect($this->policy->manageTeam($viewer, $workspace))->toBeFalse();
|
||||
expect($this->policy->inviteMember($viewer, $workspace))->toBeFalse();
|
||||
});
|
||||
|
||||
test('only account owner can manage billing', function () {
|
||||
$account = Account::factory()->create();
|
||||
$owner = User::factory()->create([
|
||||
|
|
|
|||
Loading…
Reference in a new issue