Permission check fixes (#1406)

This commit is contained in:
Boy132 2025-05-27 19:30:30 +02:00 committed by GitHub
parent 17555a1d09
commit 35ce1d34ab
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 36 additions and 32 deletions

View File

@ -79,7 +79,7 @@ class ApiKeyResource extends Resource
TextColumn::make('user.username') TextColumn::make('user.username')
->label(trans('admin/apikey.table.created_by')) ->label(trans('admin/apikey.table.created_by'))
->icon('tabler-user') ->icon('tabler-user')
->url(fn (ApiKey $apiKey) => auth()->user()->can('update user', $apiKey->user) ? EditUser::getUrl(['record' => $apiKey->user]) : null), ->url(fn (ApiKey $apiKey) => auth()->user()->can('update', $apiKey->user) ? EditUser::getUrl(['record' => $apiKey->user]) : null),
]) ])
->actions([ ->actions([
DeleteAction::make(), DeleteAction::make(),

View File

@ -71,10 +71,10 @@ class DatabasesRelationManager extends RelationManager
]) ])
->actions([ ->actions([
DeleteAction::make() DeleteAction::make()
->authorize(fn (Database $database) => auth()->user()->can('delete database', $database)), ->authorize(fn (Database $database) => auth()->user()->can('delete', $database)),
ViewAction::make() ViewAction::make()
->color('primary') ->color('primary')
->hidden(fn () => !auth()->user()->can('viewList database')), ->hidden(fn () => !auth()->user()->can('viewAny', Database::class)),
]); ]);
} }
} }

View File

@ -97,7 +97,7 @@ class AllocationsRelationManager extends RelationManager
]) ])
->groupedBulkActions([ ->groupedBulkActions([
DeleteBulkAction::make() DeleteBulkAction::make()
->authorize(fn () => auth()->user()->can('update node')), ->authorize(fn () => auth()->user()->can('update', $this->getOwnerRecord())),
]); ]);
} }
} }

View File

@ -79,8 +79,6 @@ class ServerResource extends Resource
{ {
$query = parent::getEloquentQuery(); $query = parent::getEloquentQuery();
return $query->whereHas('node', function (Builder $query) { return $query->whereIn('node_id', auth()->user()->accessibleNodes()->pluck('id'));
$query->whereIn('id', auth()->user()->accessibleNodes()->pluck('id'));
});
} }
} }

View File

@ -144,6 +144,7 @@ class CreateServer extends CreateRecord
->relationship('user', 'username') ->relationship('user', 'username')
->searchable(['username', 'email']) ->searchable(['username', 'email'])
->getOptionLabelFromRecordUsing(fn (User $user) => "$user->username ($user->email)") ->getOptionLabelFromRecordUsing(fn (User $user) => "$user->username ($user->email)")
->createOptionAction(fn (Action $action) => $action->authorize(fn () => auth()->user()->can('create', User::class)))
->createOptionForm([ ->createOptionForm([
TextInput::make('username') TextInput::make('username')
->label(trans('admin/user.username')) ->label(trans('admin/user.username'))
@ -205,6 +206,7 @@ class CreateServer extends CreateRecord
->where('node_id', $get('node_id')) ->where('node_id', $get('node_id'))
->whereNull('server_id'), ->whereNull('server_id'),
) )
->createOptionAction(fn (Action $action) => $action->authorize(fn (Get $get) => auth()->user()->can('create', Node::find($get('node_id')))))
->createOptionForm(function (Get $get) { ->createOptionForm(function (Get $get) {
$getPage = $get; $getPage = $get;

View File

@ -686,7 +686,7 @@ class EditServer extends EditRecord
ServerResource::getMountCheckboxList($get), ServerResource::getMountCheckboxList($get),
]), ]),
Tab::make(trans('admin/server.databases')) Tab::make(trans('admin/server.databases'))
->hidden(fn () => !auth()->user()->can('viewList database')) ->hidden(fn () => !auth()->user()->can('viewAny', Database::class))
->icon('tabler-database') ->icon('tabler-database')
->columns(4) ->columns(4)
->schema([ ->schema([
@ -710,7 +710,7 @@ class EditServer extends EditRecord
->hintAction( ->hintAction(
Action::make('Delete') Action::make('Delete')
->label(trans('filament-actions::delete.single.modal.actions.delete.label')) ->label(trans('filament-actions::delete.single.modal.actions.delete.label'))
->authorize(fn (Database $database) => auth()->user()->can('delete database', $database)) ->authorize(fn (Database $database) => auth()->user()->can('delete', $database))
->color('danger') ->color('danger')
->icon('tabler-trash') ->icon('tabler-trash')
->requiresConfirmation() ->requiresConfirmation()
@ -763,7 +763,7 @@ class EditServer extends EditRecord
->columnSpan(4), ->columnSpan(4),
FormActions::make([ FormActions::make([
Action::make('createDatabase') Action::make('createDatabase')
->authorize(fn () => auth()->user()->can('create database')) ->authorize(fn () => auth()->user()->can('create', Database::class))
->disabled(fn () => DatabaseHost::query()->count() < 1) ->disabled(fn () => DatabaseHost::query()->count() < 1)
->label(fn () => DatabaseHost::query()->count() < 1 ? trans('admin/server.no_db_hosts') : trans('admin/server.create_database')) ->label(fn () => DatabaseHost::query()->count() < 1 ? trans('admin/server.no_db_hosts') : trans('admin/server.create_database'))
->color(fn () => DatabaseHost::query()->count() < 1 ? 'danger' : 'primary') ->color(fn () => DatabaseHost::query()->count() < 1 ? 'danger' : 'primary')
@ -1065,7 +1065,7 @@ class EditServer extends EditRecord
} }
}) })
->hidden(fn () => $canForceDelete) ->hidden(fn () => $canForceDelete)
->authorize(fn (Server $server) => auth()->user()->can('delete server', $server)), ->authorize(fn (Server $server) => auth()->user()->can('delete', $server)),
Actions\Action::make('ForceDelete') Actions\Action::make('ForceDelete')
->color('danger') ->color('danger')
->label(trans('filament-actions::force-delete.single.label')) ->label(trans('filament-actions::force-delete.single.label'))
@ -1082,7 +1082,7 @@ class EditServer extends EditRecord
} }
}) })
->visible(fn () => $canForceDelete) ->visible(fn () => $canForceDelete)
->authorize(fn (Server $server) => auth()->user()->can('delete server', $server)), ->authorize(fn (Server $server) => auth()->user()->can('delete', $server)),
Actions\Action::make('console') Actions\Action::make('console')
->label(trans('admin/server.console')) ->label(trans('admin/server.console'))
->icon('tabler-terminal') ->icon('tabler-terminal')

View File

@ -68,13 +68,13 @@ class ListServers extends ListRecords
->searchable(), ->searchable(),
SelectColumn::make('allocation_id') SelectColumn::make('allocation_id')
->label(trans('admin/server.primary_allocation')) ->label(trans('admin/server.primary_allocation'))
->hidden(!auth()->user()->can('update server')) ->hidden(!auth()->user()->can('update server')) // TODO: update to policy check (fn (Server $server) --> $server is empty)
->options(fn (Server $server) => $server->allocations->mapWithKeys(fn ($allocation) => [$allocation->id => $allocation->address])) ->options(fn (Server $server) => $server->allocations->mapWithKeys(fn ($allocation) => [$allocation->id => $allocation->address]))
->selectablePlaceholder(false) ->selectablePlaceholder(false)
->sortable(), ->sortable(),
TextColumn::make('allocation_id_readonly') TextColumn::make('allocation_id_readonly')
->label(trans('admin/server.primary_allocation')) ->label(trans('admin/server.primary_allocation'))
->hidden(auth()->user()->can('update server')) ->hidden(auth()->user()->can('update server')) // TODO: update to policy check (fn (Server $server) --> $server is empty)
->state(fn (Server $server) => $server->allocation->address), ->state(fn (Server $server) => $server->allocation->address),
TextColumn::make('image')->hidden(), TextColumn::make('image')->hidden(),
TextColumn::make('backups_count') TextColumn::make('backups_count')

View File

@ -26,7 +26,7 @@ class RotateDatabasePasswordAction extends Action
$this->icon('tabler-refresh'); $this->icon('tabler-refresh');
$this->authorize(fn (Database $database) => auth()->user()->can('update database', $database)); $this->authorize(fn (Database $database) => auth()->user()->can('update', $database));
$this->modalHeading(trans('admin/databasehost.rotate_password')); $this->modalHeading(trans('admin/databasehost.rotate_password'));

View File

@ -68,7 +68,7 @@ class ActivityResource extends Resource
return $user; return $user;
}) })
->tooltip(fn (ActivityLog $activityLog) => auth()->user()->can('seeIps activityLog') ? $activityLog->ip : '') ->tooltip(fn (ActivityLog $activityLog) => auth()->user()->can('seeIps activityLog') ? $activityLog->ip : '')
->url(fn (ActivityLog $activityLog) => $activityLog->actor instanceof User && auth()->user()->can('update user') ? EditUser::getUrl(['record' => $activityLog->actor], panel: 'admin') : '') ->url(fn (ActivityLog $activityLog) => $activityLog->actor instanceof User && auth()->user()->can('update', $activityLog->actor) ? EditUser::getUrl(['record' => $activityLog->actor], panel: 'admin') : '')
->grow(false), ->grow(false),
DateTimeColumn::make('timestamp') DateTimeColumn::make('timestamp')
->since() ->since()
@ -105,7 +105,7 @@ class ActivityResource extends Resource
Action::make('edit') Action::make('edit')
->label(trans('filament-actions::edit.single.label')) ->label(trans('filament-actions::edit.single.label'))
->icon('tabler-edit') ->icon('tabler-edit')
->visible(fn (ActivityLog $activityLog) => $activityLog->actor instanceof User && auth()->user()->can('update user')) ->visible(fn (ActivityLog $activityLog) => $activityLog->actor instanceof User && auth()->user()->can('update', $activityLog->actor))
->url(fn (ActivityLog $activityLog) => EditUser::getUrl(['record' => $activityLog->actor], panel: 'admin')) ->url(fn (ActivityLog $activityLog) => EditUser::getUrl(['record' => $activityLog->actor], panel: 'admin'))
), ),
DateTimePicker::make('timestamp'), DateTimePicker::make('timestamp'),

View File

@ -58,7 +58,7 @@ abstract class SubuserRequest extends ClientApiRequest
$server = $this->route()->parameter('server'); $server = $this->route()->parameter('server');
// If we are an admin or the server owner, no need to perform these checks. // If we are an admin or the server owner, no need to perform these checks.
if ($user->can('update server', $server) || $user->id === $server->owner_id) { if ($user->can('update', $server) || $user->id === $server->owner_id) {
return; return;
} }

View File

@ -265,8 +265,13 @@ class User extends Model implements AuthenticatableContract, AuthorizableContrac
*/ */
public function accessibleServers(): Builder public function accessibleServers(): Builder
{ {
if ($this->canned('viewList server')) { if ($this->canned('viewAny', Server::class)) {
return Server::query(); return Server::select('servers.*')
->leftJoin('subusers', 'subusers.server_id', '=', 'servers.id')
->where(function (Builder $builder) {
$builder->where('servers.owner_id', $this->id)->orWhere('subusers.user_id', $this->id)->orWhereIn('servers.node_id', $this->accessibleNodes()->pluck('id'));
})
->distinct('servers.id');
} }
return $this->directAccessibleServers(); return $this->directAccessibleServers();
@ -278,8 +283,7 @@ class User extends Model implements AuthenticatableContract, AuthorizableContrac
*/ */
public function directAccessibleServers(): Builder public function directAccessibleServers(): Builder
{ {
return Server::query() return Server::select('servers.*')
->select('servers.*')
->leftJoin('subusers', 'subusers.server_id', '=', 'servers.id') ->leftJoin('subusers', 'subusers.server_id', '=', 'servers.id')
->where(function (Builder $builder) { ->where(function (Builder $builder) {
$builder->where('servers.owner_id', $this->id)->orWhere('subusers.user_id', $this->id); $builder->where('servers.owner_id', $this->id)->orWhere('subusers.user_id', $this->id);
@ -314,12 +318,12 @@ class User extends Model implements AuthenticatableContract, AuthorizableContrac
protected function checkPermission(Server $server, string $permission = ''): bool protected function checkPermission(Server $server, string $permission = ''): bool
{ {
if ($this->canned('update server', $server) || $server->owner_id === $this->id) { if ($this->canned('update', $server) || $server->owner_id === $this->id) {
return true; return true;
} }
// If the user only has "view" permissions allow viewing the console // If the user only has "view" permissions allow viewing the console
if ($permission === Permission::ACTION_WEBSOCKET_CONNECT && $this->canned('view server', $server)) { if ($permission === Permission::ACTION_WEBSOCKET_CONNECT && $this->canned('view', $server)) {
return true; return true;
} }
@ -434,7 +438,7 @@ class User extends Model implements AuthenticatableContract, AuthorizableContrac
public function canAccessTenant(Model $tenant): bool public function canAccessTenant(Model $tenant): bool
{ {
if ($tenant instanceof Server) { if ($tenant instanceof Server) {
if ($this->canned('view server', $tenant) || $tenant->owner_id === $this->id) { if ($this->canned('view', $tenant) || $tenant->owner_id === $this->id) {
return true; return true;
} }

View File

@ -21,11 +21,6 @@ class ServerPolicy
return null; return null;
} }
// Make sure user can target node of the server
if (!$user->canTarget($server->node)) {
return false;
}
// Owner has full server permissions // Owner has full server permissions
if ($server->owner_id === $user->id) { if ($server->owner_id === $user->id) {
return true; return true;
@ -37,6 +32,11 @@ class ServerPolicy
return true; return true;
} }
// Make sure user can target node of the server
if (!$user->canTarget($server->node)) {
return false;
}
// Return null to let default policies take over // Return null to let default policies take over
return null; return null;
} }

View File

@ -16,8 +16,8 @@ class GetUserPermissionsService
*/ */
public function handle(Server $server, User $user): array public function handle(Server $server, User $user): array
{ {
if ($user->isAdmin() && ($user->can('view server', $server) || $user->can('update server', $server))) { if ($user->isAdmin() && ($user->can('view', $server) || $user->can('update', $server))) {
$permissions = $user->can('update server', $server) ? ['*'] : ['websocket.connect', 'backup.read']; $permissions = $user->can('update', $server) ? ['*'] : ['websocket.connect', 'backup.read'];
$permissions[] = 'admin.websocket.errors'; $permissions[] = 'admin.websocket.errors';
$permissions[] = 'admin.websocket.install'; $permissions[] = 'admin.websocket.install';