Migrated remaining relation permission usages

Now all tests are passing.
Some level of manual checks to do.
This commit is contained in:
Dan Brown 2023-01-24 19:04:32 +00:00
parent 2d1f1abce4
commit 1660e72cc5
No known key found for this signature in database
GPG Key ID: 46D9F943C24A2EF9
6 changed files with 29 additions and 46 deletions

View File

@ -150,51 +150,16 @@ class PermissionApplicator
*/
public function restrictPageRelationQuery(Builder $query, string $tableName, string $pageIdColumn): Builder
{
// TODO - Refactor
$fullPageIdColumn = $tableName . '.' . $pageIdColumn;
$morphClass = (new Page())->getMorphClass();
$existsQuery = function ($permissionQuery) use ($fullPageIdColumn, $morphClass) {
/** @var Builder $permissionQuery */
$permissionQuery->select('joint_permissions.role_id')->from('joint_permissions')
->whereColumn('joint_permissions.entity_id', '=', $fullPageIdColumn)
->where('joint_permissions.entity_type', '=', $morphClass)
->whereIn('joint_permissions.role_id', $this->getCurrentUserRoleIds())
->where(function (QueryBuilder $query) {
$this->addJointHasPermissionCheck($query, $this->currentUser()->id);
return $this->restrictEntityQuery($query)
->where(function ($query) use ($fullPageIdColumn) {
/** @var Builder $query */
$query->whereExists(function (QueryBuilder $query) use ($fullPageIdColumn) {
$query->select('id')->from('pages')
->whereColumn('pages.id', '=', $fullPageIdColumn)
->where('pages.draft', '=', false);
});
};
$q = $query->where(function ($query) use ($existsQuery, $fullPageIdColumn) {
$query->whereExists($existsQuery)
->orWhere($fullPageIdColumn, '=', 0);
});
// Prevent visibility of non-owned draft pages
$q->whereExists(function (QueryBuilder $query) use ($fullPageIdColumn) {
$query->select('id')->from('pages')
->whereColumn('pages.id', '=', $fullPageIdColumn)
->where(function (QueryBuilder $query) {
$query->where('pages.draft', '=', false)
->orWhere('pages.owned_by', '=', $this->currentUser()->id);
});
});
return $q;
}
/**
* Add the query for checking the given user id has permission
* within the join_permissions table.
*
* @param QueryBuilder|Builder $query
*/
protected function addJointHasPermissionCheck($query, int $userIdToCheck)
{
$query->where('joint_permissions.has_permission', '=', true)->orWhere(function ($query) use ($userIdToCheck) {
$query->where('joint_permissions.has_permission_own', '=', true)
->where('joint_permissions.owned_by', '=', $userIdToCheck);
});
});
}
/**

View File

@ -200,6 +200,7 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon
public function attachRole(Role $role)
{
$this->roles()->attach($role->id);
$this->unsetRelation('roles');
}
/**

View File

@ -2,6 +2,7 @@
namespace BookStack\Uploads;
use BookStack\Auth\Permissions\JointPermission;
use BookStack\Auth\Permissions\PermissionApplicator;
use BookStack\Auth\User;
use BookStack\Entities\Models\Entity;
@ -10,6 +11,7 @@ use BookStack\Model;
use BookStack\Traits\HasCreatorAndUpdater;
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Database\Eloquent\Relations\BelongsTo;
use Illuminate\Database\Eloquent\Relations\HasMany;
/**
* @property int $id
@ -56,6 +58,12 @@ class Attachment extends Model
return $this->belongsTo(Page::class, 'uploaded_to');
}
public function jointPermissions(): HasMany
{
return $this->hasMany(JointPermission::class, 'entity_id', 'uploaded_to')
->where('joint_permissions.entity_type', '=', 'page');
}
/**
* Get the url of this file.
*/

View File

@ -2,10 +2,12 @@
namespace BookStack\Uploads;
use BookStack\Auth\Permissions\JointPermission;
use BookStack\Entities\Models\Page;
use BookStack\Model;
use BookStack\Traits\HasCreatorAndUpdater;
use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Database\Eloquent\Relations\HasMany;
/**
* @property int $id
@ -25,6 +27,12 @@ class Image extends Model
protected $fillable = ['name'];
protected $hidden = [];
public function jointPermissions(): HasMany
{
return $this->hasMany(JointPermission::class, 'entity_id', 'uploaded_to')
->where('joint_permissions.entity_type', '=', 'page');
}
/**
* Get a thumbnail for this image.
*

View File

@ -29,7 +29,7 @@ class RegeneratePermissionsCommandTest extends TestCase
'entity_id' => $page->id,
'entity_type' => 'page',
'role_id' => $role->id,
'has_permission' => 1,
'status' => 3, // Explicit allow
]);
$page->permissions()->delete();

View File

@ -663,7 +663,7 @@ class EntityPermissionsTest extends TestCase
$chapter = $this->entities->chapter();
$book = $chapter->book;
$this->permissions->setEntityPermissions($book, ['edit'], [$viewerRole], false);
$this->permissions->setEntityPermissions($book, ['update'], [$viewerRole], false);
$this->permissions->setEntityPermissions($chapter, [], [$viewerRole], true);
$this->assertFalse(userCan('chapter-update', $chapter));
@ -678,9 +678,10 @@ class EntityPermissionsTest extends TestCase
$chapter = $this->entities->chapter();
$book = $chapter->book;
$this->permissions->setEntityPermissions($book, ['edit'], [$editorRole], false);
$this->permissions->setEntityPermissions($book, ['update'], [$editorRole], false);
$this->permissions->setEntityPermissions($chapter, [], [$viewerRole], true);
$this->actingAs($user);
$this->assertTrue(userCan('chapter-update', $chapter));
}