Fix server access for admins without subuser (#919)

* fix server access for admins without subuser

* add permission checks to power buttons

* add permission check for console command sending

* fix tests

* fix websocket token permissions

* fix sftp access

* fix server api + small cleanup

* it's "update", not "edit"...

* fix tests

* fix permission const for "activity read"

* fix activity subuser permission
This commit is contained in:
Boy132 2025-01-17 23:04:22 +01:00 committed by GitHub
parent 61bdf0dcd7
commit 03eaddb126
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 103 additions and 45 deletions

View File

@ -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)

View File

@ -10,6 +10,7 @@ use App\Filament\Server\Widgets\ServerMemoryChart;
// 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;
@ -94,16 +95,19 @@ class Console extends Page
->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')
@ -114,6 +118,7 @@ class Console extends Page
->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()),
];
}

View File

@ -115,7 +115,9 @@ class ListUsers extends ListRecords
'settings' => [
'rename',
'reinstall',
'activity',
],
'activity' => [
'read',
],
];
@ -357,6 +359,24 @@ class ListUsers extends ListRecords
]),
]),
]),
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'),
]),
]),
]),
]),
]),

View File

@ -67,9 +67,14 @@ class ServerConsole extends Widget
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

View File

@ -53,12 +53,12 @@ class ClientController extends ClientApiController
} 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());

View File

@ -138,7 +138,7 @@ class SftpAuthenticationController extends Controller
*/
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)) {

View File

@ -35,9 +35,9 @@ class AuthenticateServerAccess
}
// 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'));
@ -53,7 +53,7 @@ class AuthenticateServerAccess
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;
}
}

View File

@ -55,8 +55,8 @@ abstract class SubuserRequest extends ClientApiRequest
/** @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;
}

View File

@ -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.

View File

@ -289,10 +289,23 @@ class User extends Model implements AuthenticatableContract, AuthorizableContrac
}
/**
* 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.*')
@ -315,7 +328,12 @@ class User extends Model implements AuthenticatableContract, AuthorizableContrac
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;
}
@ -361,6 +379,11 @@ class User extends Model implements AuthenticatableContract, AuthorizableContrac
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()) {
@ -368,7 +391,7 @@ class User extends Model implements AuthenticatableContract, AuthorizableContrac
}
if ($panel->getId() === 'admin') {
return $this->roles()->count() >= 1 && $this->getAllPermissions()->count() >= 1;
return $this->isAdmin();
}
return true;
@ -401,7 +424,7 @@ class User extends Model implements AuthenticatableContract, AuthorizableContrac
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;
}

View File

@ -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();

View File

@ -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.',
@ -16,7 +17,7 @@ return [
'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.',

View File

@ -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
@script
<script>