Permissions: Updated guest user handling so additional roles apply

Previously additional roles would only partially apply (system or "all"
permissions). This aligns the query-handling of permissions so that
additional roles will be used for permission queries.

Adds migration to detach existing roles as a safety precaution since
this is likely to widen permissions in scenarios that the public user
has other roles assigned already.

For #1229
This commit is contained in:
Dan Brown 2023-06-10 11:37:01 +01:00
parent 1e220c473f
commit 777027bc48
No known key found for this signature in database
GPG key ID: 46D9F943C24A2EF9
3 changed files with 60 additions and 4 deletions

View file

@ -183,10 +183,6 @@ class PermissionApplicator
*/
protected function getCurrentUserRoleIds(): array
{
if (auth()->guest()) {
return [Role::getSystemRole('public')->id];
}
return $this->currentUser()->roles->pluck('id')->values()->all();
}

View file

@ -0,0 +1,46 @@
<?php
use Illuminate\Database\Migrations\Migration;
use Illuminate\Support\Facades\DB;
return new class extends Migration
{
/**
* Run the migrations.
*
* @return void
*/
public function up()
{
$guestUserId = DB::table('users')
->where('system_name', '=', 'public')
->first(['id'])->id;
$publicRoleId = DB::table('roles')
->where('system_name', '=', 'public')
->first(['id'])->id;
// This migration deletes secondary "Guest" user role assignments
// as a safety precaution upon upgrade since the logic is changing
// within the release this is introduced in, which could result in wider
// permissions being provided upon upgrade without this intervention.
// Previously, added roles would only partially apply their permissions
// since some permission checks would only consider the originally assigned
// public role, and not added roles. Within this release, additional roles
// will fully apply.
DB::table('role_user')
->where('user_id', '=', $guestUserId)
->where('role_id', '!=', $publicRoleId)
->delete();
}
/**
* Reverse the migrations.
*
* @return void
*/
public function down()
{
// No structural changes to make, and we cannot know the role ids to re-assign.
}
};

View file

@ -193,4 +193,18 @@ class PublicActionTest extends TestCase
$resp->assertRedirect($book->getUrl());
$this->followRedirects($resp)->assertSee($book->name);
}
public function test_public_view_can_take_on_other_roles()
{
$this->setSettings(['app-public' => 'true']);
$newRole = $this->users->attachNewRole(User::getDefault(), []);
$page = $this->entities->page();
$this->permissions->disableEntityInheritedPermissions($page);
$this->permissions->addEntityPermission($page, ['view', 'update'], $newRole);
$resp = $this->get($page->getUrl());
$resp->assertOk();
$this->withHtml($resp)->assertLinkExists($page->getUrl('/edit'));
}
}