Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix server access for admins without subuser #919

Merged
merged 12 commits into from
Jan 17, 2025
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class ListServers extends ListRecords

public function table(Table $table): Table
{
$baseQuery = auth()->user()->can('viewList server') ? Server::query() : auth()->user()->accessibleServers();
$baseQuery = auth()->user()->accessibleServers();

return $table
->paginated(false)
Expand Down
5 changes: 5 additions & 0 deletions app/Filament/Server/Pages/Console.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
// use App\Filament\Server\Widgets\ServerNetworkChart;
use App\Filament\Server\Widgets\ServerOverview;
use App\Livewire\AlertBanner;
use App\Models\Permission;
use App\Models\Server;
use Filament\Actions\Action;
use Filament\Facades\Filament;
Expand Down Expand Up @@ -94,16 +95,19 @@ protected function getHeaderActions(): array
->color('primary')
->size(ActionSize::ExtraLarge)
->action(fn () => $this->dispatch('setServerState', state: 'start', uuid: $server->uuid))
->authorize(fn () => auth()->user()->can(Permission::ACTION_CONTROL_START, $server))
->disabled(fn () => $server->isInConflictState() || !$this->status->isStartable()),
Action::make('restart')
->color('gray')
->size(ActionSize::ExtraLarge)
->action(fn () => $this->dispatch('setServerState', state: 'restart', uuid: $server->uuid))
->authorize(fn () => auth()->user()->can(Permission::ACTION_CONTROL_RESTART, $server))
->disabled(fn () => $server->isInConflictState() || !$this->status->isRestartable()),
Action::make('stop')
->color('danger')
->size(ActionSize::ExtraLarge)
->action(fn () => $this->dispatch('setServerState', state: 'stop', uuid: $server->uuid))
->authorize(fn () => auth()->user()->can(Permission::ACTION_CONTROL_STOP, $server))
->hidden(fn () => $this->status->isStartingOrStopping() || $this->status->isKillable())
->disabled(fn () => $server->isInConflictState() || !$this->status->isStoppable()),
Action::make('kill')
Expand All @@ -114,6 +118,7 @@ protected function getHeaderActions(): array
->modalSubmitActionLabel('Kill Server')
->size(ActionSize::ExtraLarge)
->action(fn () => $this->dispatch('setServerState', state: 'kill', uuid: $server->uuid))
->authorize(fn () => auth()->user()->can(Permission::ACTION_CONTROL_STOP, $server))
->hidden(fn () => $server->isInConflictState() || !$this->status->isKillable()),
];
}
Expand Down
22 changes: 21 additions & 1 deletion app/Filament/Server/Resources/UserResource/Pages/ListUsers.php
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,9 @@ protected function getHeaderActions(): array
'settings' => [
'rename',
'reinstall',
'activity',
],
'activity' => [
'read',
],
];

Expand Down Expand Up @@ -357,6 +359,24 @@ protected function getHeaderActions(): array
]),
]),
]),
Tabs\Tab::make('Activity')
->schema([
Section::make()
->description(trans('server/users.permissions.activity_desc'))
->icon('tabler-stack')
->schema([
CheckboxList::make('activity')
->bulkToggleable()
->label('')
->columns(2)
->options([
'read' => 'Read',
])
->descriptions([
'read' => trans('server/users.permissions.activity_read'),
]),
]),
]),
]),

]),
Expand Down
7 changes: 6 additions & 1 deletion app/Filament/Server/Widgets/ServerConsole.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,14 @@ protected function getSocket(): string
return $socket;
}

protected function authorizeSendCommand(): bool
{
return $this->user->can(Permission::ACTION_CONTROL_CONSOLE, $this->server);
}

protected function canSendCommand(): bool
{
return !$this->server->isInConflictState() && $this->server->retrieveStatus() === 'running';
return $this->authorizeSendCommand() && !$this->server->isInConflictState() && $this->server->retrieveStatus() === 'running';
}

public function up(): void
Expand Down
4 changes: 2 additions & 2 deletions app/Http/Controllers/Api/Client/ClientController.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,12 @@ public function index(GetServersRequest $request): array
} else {
$builder = $type === 'admin-all'
? $builder
: $builder->whereNotIn('servers.id', $user->accessibleServers()->pluck('id')->all());
: $builder->whereNotIn('servers.id', $user->directAccessibleServers()->pluck('id')->all());
}
} elseif ($type === 'owner') {
$builder = $builder->where('servers.owner_id', $user->id);
} else {
$builder = $builder->whereIn('servers.id', $user->accessibleServers()->pluck('id')->all());
$builder = $builder->whereIn('servers.id', $user->directAccessibleServers()->pluck('id')->all());
}

$servers = $builder->paginate(min($request->query('per_page', '50'), 100))->appends($request->query());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ protected function reject(Request $request, bool $increment = true): void
*/
protected function validateSftpAccess(User $user, Server $server): void
{
if (!$user->isRootAdmin() && $server->owner_id !== $user->id) {
if ($user->cannot('update server', $server) && $server->owner_id !== $user->id) {
$permissions = $this->permissions->handle($server, $user);

if (!in_array(Permission::ACTION_FILE_SFTP, $permissions)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ public function handle(Request $request, \Closure $next): mixed
}

// At the very least, ensure that the user trying to make this request is the
// server owner, a subuser, or a root admin. We'll leave it up to the controllers
// server owner, a subuser, or an admin. We'll leave it up to the controllers
// to authenticate more detailed permissions if needed.
if ($user->id !== $server->owner_id && !$user->isRootAdmin()) {
if ($user->id !== $server->owner_id && $user->cannot('update server', $server)) {
// Check for subuser status.
if (!$server->subusers->contains('user_id', $user->id)) {
throw new NotFoundHttpException(trans('exceptions.api.resource_not_found'));
Expand All @@ -53,7 +53,7 @@ public function handle(Request $request, \Closure $next): mixed
if (($server->isSuspended() || $server->node->isUnderMaintenance()) && !$request->routeIs('api:client:server.resources')) {
throw $exception;
}
if (!$user->isRootAdmin() || !$request->routeIs($this->except)) {
if ($user->cannot('update server', $server) || !$request->routeIs($this->except)) {
throw $exception;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ protected function validatePermissionsCanBeAssigned(array $permissions): void
/** @var \App\Models\Server $server */
$server = $this->route()->parameter('server');

// If we are a root admin or the server owner, no need to perform these checks.
if ($user->isRootAdmin() || $user->id === $server->owner_id) {
// 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) {
return;
}

Expand Down
2 changes: 1 addition & 1 deletion app/Models/Permission.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ class Permission extends Model

public const ACTION_SETTINGS_REINSTALL = 'settings.reinstall';

public const ACTION_ACTIVITY_READ = 'settings.activity';
public const ACTION_ACTIVITY_READ = 'activity.read';

/**
* Should timestamps be used on this model.
Expand Down
33 changes: 28 additions & 5 deletions app/Models/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -289,10 +289,23 @@ public function activity(): MorphToMany
}

/**
* Returns all the servers that a user can access by way of being the owner of the
* server, or because they are assigned as a subuser for that server.
* Returns all the servers that a user can access.
* Either because they are an admin or because they are the owner/ a subuser of the server.
*/
public function accessibleServers(): Builder
{
if ($this->canned('viewList server')) {
return Server::query();
}

return $this->directAccessibleServers();
}

/**
* Returns all the servers that a user can access "directly".
* This means either because they are the owner or a subuser of the server.
*/
public function directAccessibleServers(): Builder
{
return Server::query()
->select('servers.*')
Expand All @@ -315,7 +328,12 @@ public function subServers(): BelongsToMany

protected function checkPermission(Server $server, string $permission = ''): bool
{
if ($this->isRootAdmin() || $server->owner_id === $this->id) {
if ($this->canned('update server', $server) || $server->owner_id === $this->id) {
return true;
}

// If the user only has "view" permissions allow viewing the console
if ($permission === Permission::ACTION_WEBSOCKET_CONNECT && $this->canned('view server', $server)) {
return true;
}

Expand Down Expand Up @@ -361,14 +379,19 @@ public function isRootAdmin(): bool
return $this->hasRole(Role::ROOT_ADMIN);
}

public function isAdmin(): bool
{
return $this->isRootAdmin() || ($this->roles()->count() >= 1 && $this->getAllPermissions()->count() >= 1);
}

public function canAccessPanel(Panel $panel): bool
{
if ($this->isRootAdmin()) {
return true;
}

if ($panel->getId() === 'admin') {
return $this->roles()->count() >= 1 && $this->getAllPermissions()->count() >= 1;
return $this->isAdmin();
}

return true;
Expand Down Expand Up @@ -401,7 +424,7 @@ public function getTenants(Panel $panel): array|Collection
public function canAccessTenant(IlluminateModel $tenant): bool
{
if ($tenant instanceof Server) {
if ($this->isRootAdmin() || $tenant->owner_id === $this->id) {
if ($this->canned('view server', $tenant) || $tenant->owner_id === $this->id) {
return true;
}

Expand Down
20 changes: 11 additions & 9 deletions app/Services/Servers/GetUserPermissionsService.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,25 @@ class GetUserPermissionsService
{
/**
* Returns the server specific permissions that a user has. This checks
* if they are an admin or a subuser for the server. If no permissions are
* found, an empty array is returned.
* if they are an admin, the owner or a subuser for the server. If no
* permissions are found, an empty array is returned.
*/
public function handle(Server $server, User $user): array
{
if ($user->isRootAdmin() || $user->id === $server->owner_id) {
$permissions = ['*'];
if ($user->isAdmin() && ($user->can('view server', $server) || $user->can('update server', $server))) {
$permissions = $user->can('update server', $server) ? ['*'] : ['websocket.connect', 'backup.read'];

if ($user->isRootAdmin()) {
$permissions[] = 'admin.websocket.errors';
$permissions[] = 'admin.websocket.install';
$permissions[] = 'admin.websocket.transfer';
}
$permissions[] = 'admin.websocket.errors';
$permissions[] = 'admin.websocket.install';
$permissions[] = 'admin.websocket.transfer';

return $permissions;
}

if ($user->id === $server->owner_id) {
return ['*'];
}

/** @var \App\Models\Subuser|null $subuserPermissions */
$subuserPermissions = $server->subusers()->where('user_id', $user->id)->first();

Expand Down
5 changes: 3 additions & 2 deletions lang/en/server/users.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@

return [
'permissions' => [
'activity_desc' => 'Permissions that control a user\'s access to the server activity logs.',
'startup_desc' => 'Permissions that control a user\'s ability to view this server\'s startup parameters.',
'settings_desc' => 'Permissions that control a user\'s access to the schedule management for this server.',
'settings_desc' => 'Permissions that control a user\'s ability to modify this server\'s settings.',
'control_desc' => 'Permissions that control a user\'s ability to control the power state of a server, or send commands.',
'user_desc' => 'Permissions that allow a user to manage other subusers on a server. They will never be able to edit their own account, or assign permissions they do not have themselves.',
'file_desc' => 'Permissions that control a user\'s ability to modify the filesystem for this server.',
Expand All @@ -16,7 +17,7 @@
'startup_docker_image' => 'Allows a user to modify the Docker image used when running the server.',
'setting_reinstall' => 'Allows a user to trigger a reinstall of this server.',
'setting_rename' => 'Allows a user to rename this server and change the description of it.',
'setting_activity' => 'Allows a user to view the activity logs for the server.',
'activity_read' => 'Allows a user to view the activity logs for the server.',
'websocket_*' => 'Allows a user access to the websocket for this server.',
'control_console' => 'Allows a user to send data to the server console.',
'control_start' => 'Allows a user to start the server instance.',
Expand Down
36 changes: 19 additions & 17 deletions resources/views/filament/components/server-console.blade.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,25 @@

<div id="terminal" wire:ignore></div>

<div class="flex items-center w-full border-top overflow-hidden dark:bg-gray-900"
style="border-bottom-right-radius: 10px; border-bottom-left-radius: 10px;">
<x-filament::icon
icon="tabler-chevrons-right"
/>
<input
class="w-full focus:outline-none focus:ring-0 border-none dark:bg-gray-900"
type="text"
:readonly="{{ $this->canSendCommand() ? 'false' : 'true' }}"
title="{{ $this->canSendCommand() ? '' : 'Can\'t send command when the server is Offline' }}"
placeholder="{{ $this->canSendCommand() ? 'Type a command...' : 'Server Offline...' }}"
wire:model="input"
wire:keydown.enter="enter"
wire:keydown.up.prevent="up"
wire:keydown.down="down"
>
</div>
@if ($this->authorizeSendCommand())
<div class="flex items-center w-full border-top overflow-hidden dark:bg-gray-900"
style="border-bottom-right-radius: 10px; border-bottom-left-radius: 10px;">
<x-filament::icon
icon="tabler-chevrons-right"
/>
<input
class="w-full focus:outline-none focus:ring-0 border-none dark:bg-gray-900"
type="text"
:readonly="{{ $this->canSendCommand() ? 'false' : 'true' }}"
title="{{ $this->canSendCommand() ? '' : 'Can\'t send command when the server is Offline' }}"
placeholder="{{ $this->canSendCommand() ? 'Type a command...' : 'Server Offline...' }}"
wire:model="input"
wire:keydown.enter="enter"
wire:keydown.up.prevent="up"
wire:keydown.down="down"
>
</div>
@endif
RMartinOscar marked this conversation as resolved.
Show resolved Hide resolved

@script
<script>
Expand Down
Loading