diff --git a/app/Events/User/Deleting.php b/app/Events/User/Deleting.php new file mode 100644 index 0000000000..d8ab610c03 --- /dev/null +++ b/app/Events/User/Deleting.php @@ -0,0 +1,17 @@ +user()->email; - $this->updateService->handle($request->user(), $request->validated()); + $user = $request->user(); + + // Only allow a user to change their email three times in the span + // of 24 hours. This prevents malicious users from trying to find + // existing accounts in the system by constantly changing their email. + if (RateLimiter::tooManyAttempts($key = "user:update-email:{$user->uuid}", 3)) { + throw new TooManyRequestsHttpException(message: 'Your email address has been changed too many times today. Please try again later.'); + } + + $original = $user->email; + + if (mb_strtolower($original) !== mb_strtolower($request->validated('email'))) { + RateLimiter::hit($key, self::EMAIL_UPDATE_THROTTLE); + + $this->updateService->handle($user, $request->validated()); - if ($original !== $request->input('email')) { Activity::event('user:account.email-changed') ->property(['old' => $original, 'new' => $request->input('email')]) ->log(); @@ -85,7 +104,9 @@ public function updateEmail(UpdateEmailRequest $request): JsonResponse */ public function updatePassword(UpdatePasswordRequest $request): JsonResponse { - $user = $this->updateService->handle($request->user(), $request->validated()); + $user = Activity::event('user:account.password-changed')->transaction(function () use ($request) { + return $this->updateService->handle($request->user(), $request->validated()); + }); $guard = $this->manager->guard(); // If you do not update the user in the session you'll end up working with a @@ -98,8 +119,6 @@ public function updatePassword(UpdatePasswordRequest $request): JsonResponse $guard->logoutOtherDevices($request->input('password')); } - Activity::event('user:account.password-changed')->log(); - return new JsonResponse([], Response::HTTP_NO_CONTENT); } } diff --git a/app/Http/Controllers/Api/Client/Servers/BackupController.php b/app/Http/Controllers/Api/Client/Servers/BackupController.php index d5492cbcd8..5c63953afa 100644 --- a/app/Http/Controllers/Api/Client/Servers/BackupController.php +++ b/app/Http/Controllers/Api/Client/Servers/BackupController.php @@ -87,7 +87,7 @@ public function store(StoreBackupRequest $request, Server $server): array } $backup = Activity::event('server:backup.start')->transaction(function ($log) use ($action, $server, $request) { - $server->backups()->lockForUpdate(); + $server->backups()->lockForUpdate()->count(); $backup = $action->handle($server, $request->input('name')); diff --git a/app/Http/Controllers/Api/Client/Servers/DatabaseController.php b/app/Http/Controllers/Api/Client/Servers/DatabaseController.php index 445c8b2bcc..c741544410 100644 --- a/app/Http/Controllers/Api/Client/Servers/DatabaseController.php +++ b/app/Http/Controllers/Api/Client/Servers/DatabaseController.php @@ -60,7 +60,7 @@ public function index(GetDatabasesRequest $request, Server $server): array public function store(StoreDatabaseRequest $request, Server $server): array { $database = Activity::event('server:database.create')->transaction(function ($log) use ($request, $server) { - $server->databases()->lockForUpdate(); + $server->databases()->lockForUpdate()->count(); $database = $this->deployDatabaseService->handle($server, $request->validated()); @@ -87,15 +87,12 @@ public function store(StoreDatabaseRequest $request, Server $server): array */ public function rotatePassword(RotatePasswordRequest $request, Server $server, Database $database): array { - $this->managementService->rotatePassword($database); - $database->refresh(); - Activity::event('server:database.rotate-password') ->subject($database) ->property('name', $database->database) - ->log(); + ->transaction(fn () => $this->managementService->rotatePassword($database)); - return $this->fractal->item($database) + return $this->fractal->item($database->refresh()) ->parseIncludes(['password']) ->transformWith($this->getTransformer(DatabaseTransformer::class)) ->toArray(); diff --git a/app/Http/Controllers/Api/Client/Servers/StartupController.php b/app/Http/Controllers/Api/Client/Servers/StartupController.php index 36f485b746..8ee9a9d245 100644 --- a/app/Http/Controllers/Api/Client/Servers/StartupController.php +++ b/app/Http/Controllers/Api/Client/Servers/StartupController.php @@ -87,13 +87,13 @@ public function update(UpdateStartupVariableRequest $request, Server $server): a $startup = $this->startupCommandService->handle($server); - if ($variable->env_variable !== $request->input('value')) { + if ($original !== $request->input('value')) { Activity::event('server:startup.edit') ->subject($variable) ->property([ 'variable' => $variable->env_variable, 'old' => $original, - 'new' => $request->input('value'), + 'new' => $request->input('value') ?? '', ]) ->log(); } diff --git a/app/Http/Controllers/Api/Remote/Backups/BackupRemoteUploadController.php b/app/Http/Controllers/Api/Remote/Backups/BackupRemoteUploadController.php index fa149c31e6..4f413fe128 100644 --- a/app/Http/Controllers/Api/Remote/Backups/BackupRemoteUploadController.php +++ b/app/Http/Controllers/Api/Remote/Backups/BackupRemoteUploadController.php @@ -56,7 +56,7 @@ public function __invoke(Request $request, string $backup): JsonResponse /** @var Server $server */ $server = $model->server; if ($server->node_id !== $node->id) { - throw new HttpForbiddenException('You do not have permission to access that backup.'); + throw new HttpForbiddenException('Requesting node does not have permission to access this server.'); } // Prevent backups that have already been completed from trying to diff --git a/app/Http/Controllers/Api/Remote/Backups/BackupStatusController.php b/app/Http/Controllers/Api/Remote/Backups/BackupStatusController.php index ba69fd0047..824ea9e3f7 100644 --- a/app/Http/Controllers/Api/Remote/Backups/BackupStatusController.php +++ b/app/Http/Controllers/Api/Remote/Backups/BackupStatusController.php @@ -47,7 +47,7 @@ public function index(ReportBackupCompleteRequest $request, string $backup): Jso /** @var Server $server */ $server = $model->server; if ($server->node_id !== $node->id) { - throw new HttpForbiddenException('You do not have permission to access that backup.'); + throw new HttpForbiddenException('Requesting node does not have permission to access this server.'); } if ($model->is_successful) { @@ -97,6 +97,11 @@ public function restore(Request $request, string $backup): JsonResponse /** @var Backup $model */ $model = Backup::query()->where('uuid', $backup)->firstOrFail(); + $node = $request->attributes->get('node'); + if (!$model->server->node->is($node)) { + throw new HttpForbiddenException('Requesting node does not have permission to access this server.'); + } + $model->server->update(['status' => null]); Activity::event($request->boolean('successful') ? 'server:backup.restore-complete' : 'server.backup.restore-failed') diff --git a/app/Http/Controllers/Api/Remote/Servers/ServerContainersController.php b/app/Http/Controllers/Api/Remote/Servers/ServerContainersController.php index ee24ee7dc0..424733240c 100644 --- a/app/Http/Controllers/Api/Remote/Servers/ServerContainersController.php +++ b/app/Http/Controllers/Api/Remote/Servers/ServerContainersController.php @@ -3,18 +3,23 @@ namespace App\Http\Controllers\Api\Remote\Servers; use App\Enums\ContainerStatus; +use App\Exceptions\Http\HttpForbiddenException; use App\Http\Controllers\Controller; -use App\Http\Requests\Api\Remote\ServerRequest; use App\Models\Server; use Illuminate\Http\JsonResponse; +use Illuminate\Http\Request; class ServerContainersController extends Controller { /** * Updates the server container's status on the Panel */ - public function status(ServerRequest $request, Server $server): JsonResponse + public function status(Request $request, Server $server): JsonResponse { + if (!$server->node->is($request->attributes->get('node'))) { + throw new HttpForbiddenException('Requesting node does not have permission to access this server.'); + } + $status = ContainerStatus::tryFrom($request->json('data.new_state')) ?? ContainerStatus::Missing; cache()->put("servers.$server->uuid.status", $status, now()->addHour()); diff --git a/app/Http/Controllers/Api/Remote/Servers/ServerDetailsController.php b/app/Http/Controllers/Api/Remote/Servers/ServerDetailsController.php index 0d1a18a16b..7c53af6ee1 100644 --- a/app/Http/Controllers/Api/Remote/Servers/ServerDetailsController.php +++ b/app/Http/Controllers/Api/Remote/Servers/ServerDetailsController.php @@ -3,9 +3,9 @@ namespace App\Http\Controllers\Api\Remote\Servers; use App\Enums\ServerState; +use App\Exceptions\Http\HttpForbiddenException; use App\Facades\Activity; use App\Http\Controllers\Controller; -use App\Http\Requests\Api\Remote\ServerRequest; use App\Http\Resources\Daemon\ServerConfigurationCollection; use App\Models\ActivityLog; use App\Models\Backup; @@ -17,6 +17,7 @@ use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; use Throwable; +use Webmozart\Assert\Assert; class ServerDetailsController extends Controller { @@ -33,8 +34,21 @@ public function __construct( * Returns details about the server that allows daemon to self-recover and ensure * that the state of the server matches the Panel at all times. */ - public function __invoke(ServerRequest $request, Server $server): JsonResponse + public function __invoke(Request $request, Server $server): JsonResponse { + Assert::isInstanceOf($node = $request->attributes->get('node'), Node::class); + + $transfer = $server->transfer; + + // If the server is being transferred allow either node to request information about + // the server. If the server is not being transferred only the target node is allowed + // to fetch these details. + $valid = $transfer ? $node->id === $transfer->old_node || $node->id === $transfer->new_node : $node->id === $server->node_id; + + if (!$valid) { + throw new HttpForbiddenException('Requesting node does not have permission to access this server.'); + } + return new JsonResponse([ 'settings' => $this->configurationStructureService->handle($server), 'process_configuration' => $this->eggConfigurationService->handle($server), diff --git a/app/Http/Controllers/Api/Remote/Servers/ServerInstallController.php b/app/Http/Controllers/Api/Remote/Servers/ServerInstallController.php index a6b8cf1ca8..bedf6adc6c 100644 --- a/app/Http/Controllers/Api/Remote/Servers/ServerInstallController.php +++ b/app/Http/Controllers/Api/Remote/Servers/ServerInstallController.php @@ -4,12 +4,13 @@ use App\Enums\ServerState; use App\Events\Server\Installed as ServerInstalled; +use App\Exceptions\Http\HttpForbiddenException; use App\Exceptions\Model\DataValidationException; use App\Http\Controllers\Controller; use App\Http\Requests\Api\Remote\InstallationDataRequest; -use App\Http\Requests\Api\Remote\ServerRequest; use App\Models\Server; use Illuminate\Http\JsonResponse; +use Illuminate\Http\Request; use Illuminate\Http\Response; class ServerInstallController extends Controller @@ -17,12 +18,18 @@ class ServerInstallController extends Controller /** * Returns installation information for a server. */ - public function index(ServerRequest $request, Server $server): JsonResponse + public function index(Request $request, Server $server): JsonResponse { + if (!$server->node->is($request->attributes->get('node'))) { + throw new HttpForbiddenException('Requesting node does not have permission to access this server.'); + } + + $egg = $server->egg; + return new JsonResponse([ - 'container_image' => $server->egg->copy_script_container, - 'entrypoint' => $server->egg->copy_script_entry, - 'script' => $server->egg->copy_script_install, + 'container_image' => $egg->copy_script_container, + 'entrypoint' => $egg->copy_script_entry, + 'script' => $egg->copy_script_install, ]); } @@ -35,6 +42,10 @@ public function store(InstallationDataRequest $request, Server $server): JsonRes { $status = null; + if (!$server->node->is($request->attributes->get('node'))) { + throw new HttpForbiddenException('Requesting node does not have permission to access this server.'); + } + $successful = $request->boolean('successful'); // Make sure the type of failure is accurate diff --git a/app/Http/Controllers/Api/Remote/Servers/ServerTransferController.php b/app/Http/Controllers/Api/Remote/Servers/ServerTransferController.php index 6423d532a1..f3db17c502 100644 --- a/app/Http/Controllers/Api/Remote/Servers/ServerTransferController.php +++ b/app/Http/Controllers/Api/Remote/Servers/ServerTransferController.php @@ -2,17 +2,20 @@ namespace App\Http\Controllers\Api\Remote\Servers; +use App\Exceptions\Http\HttpForbiddenException; use App\Http\Controllers\Controller; -use App\Http\Requests\Api\Remote\ServerRequest; use App\Models\Allocation; +use App\Models\Node; use App\Models\Server; use App\Repositories\Daemon\DaemonServerRepository; use Illuminate\Database\ConnectionInterface; use Illuminate\Http\Client\ConnectionException; use Illuminate\Http\JsonResponse; +use Illuminate\Http\Request; use Illuminate\Http\Response; use Symfony\Component\HttpKernel\Exception\ConflictHttpException; use Throwable; +use Webmozart\Assert\Assert; class ServerTransferController extends Controller { @@ -29,13 +32,22 @@ public function __construct( * * @throws Throwable */ - public function failure(ServerRequest $request, Server $server): JsonResponse + public function failure(Request $request, Server $server): JsonResponse { $transfer = $server->transfer; if (is_null($transfer)) { throw new ConflictHttpException('Server is not being transferred.'); } + /* @var Node $node */ + Assert::isInstanceOf($node = $request->attributes->get('node'), Node::class); + + // Either node can tell the panel that the transfer has failed. Only the new node + // can tell the panel that it was successful. + if (!$node->is($transfer->newNode) && !$node->is($transfer->oldNode)) { + throw new HttpForbiddenException('Requesting node does not have permission to access this server.'); + } + $this->connection->transaction(function () use ($transfer) { $transfer->forceFill(['successful' => false])->saveOrFail(); @@ -53,13 +65,22 @@ public function failure(ServerRequest $request, Server $server): JsonResponse * * @throws Throwable */ - public function success(ServerRequest $request, Server $server): JsonResponse + public function success(Request $request, Server $server): JsonResponse { $transfer = $server->transfer; if (is_null($transfer)) { throw new ConflictHttpException('Server is not being transferred.'); } + /* @var Node $node */ + Assert::isInstanceOf($node = $request->attributes->get('node'), Node::class); + + // Only the new node communicates a successful state to the panel, so we should + // not allow the old node to hit this endpoint. + if (!$node->is($transfer->newNode)) { + throw new HttpForbiddenException('Requesting node does not have permission to access this server.'); + } + /** @var Server $server */ $server = $this->connection->transaction(function () use ($server, $transfer) { $data = []; diff --git a/app/Http/Middleware/SetSecurityHeaders.php b/app/Http/Middleware/SetSecurityHeaders.php new file mode 100644 index 0000000000..89dac3721f --- /dev/null +++ b/app/Http/Middleware/SetSecurityHeaders.php @@ -0,0 +1,46 @@ + + */ + protected static array $headers = [ + 'X-Frame-Options' => 'DENY', + 'X-Content-Type-Options' => 'nosniff', + 'X-XSS-Protection' => '1; mode=block', + 'Referrer-Policy' => 'no-referrer-when-downgrade', + ]; + + /** + * Enforces some basic security headers on all responses returned by the software. + * If a header has already been set in another location within the code it will be + * skipped over here. + * + * @param (\Closure(mixed): Response) $next + */ + public function handle(Request $request, \Closure $next): mixed + { + $response = $next($request); + + foreach (static::$headers as $key => $value) { + if (!$response->headers->has($key)) { + $response->headers->set($key, $value); + } + } + + return $response; + } +} diff --git a/app/Http/Requests/Api/Remote/InstallationDataRequest.php b/app/Http/Requests/Api/Remote/InstallationDataRequest.php index 59d52c9780..99adcef5fc 100644 --- a/app/Http/Requests/Api/Remote/InstallationDataRequest.php +++ b/app/Http/Requests/Api/Remote/InstallationDataRequest.php @@ -2,8 +2,15 @@ namespace App\Http\Requests\Api\Remote; -class InstallationDataRequest extends ServerRequest +use Illuminate\Foundation\Http\FormRequest; + +class InstallationDataRequest extends FormRequest { + public function authorize(): bool + { + return true; + } + /** * @return array */ diff --git a/app/Http/Requests/Api/Remote/ServerRequest.php b/app/Http/Requests/Api/Remote/ServerRequest.php deleted file mode 100644 index 12da385f1e..0000000000 --- a/app/Http/Requests/Api/Remote/ServerRequest.php +++ /dev/null @@ -1,29 +0,0 @@ -attributes->get('node'); - - /** @var ?Server $server */ - $server = $this->route()->parameter('server'); - - if ($server) { - if ($server->transfer) { - return $server->transfer->old_node === $node->id || $server->transfer->new_node === $node->id; - } - - return $server->node_id === $node->id; - } - - return false; - } -} diff --git a/app/Jobs/Job.php b/app/Jobs/Job.php deleted file mode 100644 index 55ece29acb..0000000000 --- a/app/Jobs/Job.php +++ /dev/null @@ -1,21 +0,0 @@ -target instanceof Node ? "node:{$this->target->uuid}" : "server:{$this->target->uuid}"; + + return "revoke-sftp:{$this->user}:{$target}"; + } + + public function handle(DaemonServerRepository $repository): void + { + try { + if ($this->target instanceof Server) { + $repository->setServer($this->target)->deauthorize($this->user); + } else { + $repository->setNode($this->target)->deauthorize($this->user); + } + } catch (ConnectionException) { + // Keep retrying this job with a longer and longer backoff until we hit three + // attempts at which point we stop and will assume the node is fully offline + // and we are just wasting time. + $this->release($this->attempts() * 10); + } + } +} diff --git a/app/Jobs/Schedule/RunTaskJob.php b/app/Jobs/Schedule/RunTaskJob.php index 476222a5f9..56d1a47563 100644 --- a/app/Jobs/Schedule/RunTaskJob.php +++ b/app/Jobs/Schedule/RunTaskJob.php @@ -7,6 +7,7 @@ use App\Models\Task; use Carbon\CarbonImmutable; use Exception; +use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Http\Client\ConnectionException; use Illuminate\Queue\InteractsWithQueue; @@ -14,9 +15,10 @@ use InvalidArgumentException; use Throwable; -class RunTaskJob extends Job implements ShouldQueue +class RunTaskJob implements ShouldQueue { use InteractsWithQueue; + use Queueable; use SerializesModels; /** diff --git a/app/Listeners/RevocationListener.php b/app/Listeners/RevocationListener.php new file mode 100644 index 0000000000..d01a711878 --- /dev/null +++ b/app/Listeners/RevocationListener.php @@ -0,0 +1,25 @@ +user; + + // Look at all of the nodes that a user is associated with and trigger a job + // that disconnects them from websockets and SFTP. + Node::query() + ->whereIn('nodes.id', $user->directAccessibleServers()->select('servers.node_id')->distinct()) + ->chunk(50, function (Collection $nodes) use ($user) { + $nodes->each(fn (Node $node) => RevokeSftpAccessJob::dispatch($user->uuid, $node)); + }); + } +} diff --git a/app/Models/ServerTransfer.php b/app/Models/ServerTransfer.php index d1c0c06640..46e5f02e28 100644 --- a/app/Models/ServerTransfer.php +++ b/app/Models/ServerTransfer.php @@ -4,6 +4,7 @@ use App\Contracts\Validatable; use App\Traits\HasValidation; +use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\BelongsTo; use Illuminate\Database\Eloquent\Relations\HasOne; @@ -44,6 +45,7 @@ */ class ServerTransfer extends Model implements Validatable { + use HasFactory; use HasValidation; /** diff --git a/app/Models/User.php b/app/Models/User.php index 0ca6279c6b..ca94f77014 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -5,6 +5,7 @@ use App\Contracts\Validatable; use App\Enums\CustomizationKey; use App\Enums\SubuserPermission; +use App\Events\User\Deleting; use App\Exceptions\DisplayException; use App\Extensions\Avatar\AvatarService; use App\Models\Traits\HasAccessTokens; @@ -225,6 +226,8 @@ protected static function booted(): void throw_if($user->servers()->count() > 0, new DisplayException(trans('exceptions.users.has_servers'))); throw_if(request()->user()?->id === $user->id, new DisplayException(trans('exceptions.users.is_self'))); + + event(new Deleting($user)); }); } diff --git a/app/Providers/Filament/PanelProvider.php b/app/Providers/Filament/PanelProvider.php index 76968af759..524a55c196 100644 --- a/app/Providers/Filament/PanelProvider.php +++ b/app/Providers/Filament/PanelProvider.php @@ -7,6 +7,7 @@ use App\Filament\Pages\Auth\Login; use App\Http\Middleware\LanguageMiddleware; use App\Http\Middleware\RequireTwoFactorAuthentication; +use App\Http\Middleware\SetSecurityHeaders; use Filament\Actions\Action; use Filament\Auth\MultiFactor\App\AppAuthentication; use Filament\Auth\MultiFactor\Email\EmailAuthentication; @@ -70,6 +71,7 @@ public function panel(Panel $panel): Panel DisableBladeIconComponents::class, DispatchServingFilamentEvent::class, LanguageMiddleware::class, + SetSecurityHeaders::class, ]) ->authMiddleware([ Authenticate::class, diff --git a/app/Repositories/Daemon/DaemonServerRepository.php b/app/Repositories/Daemon/DaemonServerRepository.php index 6088acf130..0109ba6c48 100644 --- a/app/Repositories/Daemon/DaemonServerRepository.php +++ b/app/Repositories/Daemon/DaemonServerRepository.php @@ -147,7 +147,7 @@ public function revokeUserJTI(int $id): void } /** - * Deauthorizes a user (disconnects websockets and SFTP) on the Wings instance for the server. + * Deauthorizes a user (disconnects websockets and SFTP) on the Wings instance for the server (or all servers of a node). * * @throws ConnectionException */ @@ -156,7 +156,7 @@ public function deauthorize(string $user): void $this->getHttpClient()->post('/api/deauthorize-user', [ 'json' => [ 'user' => $user, - 'servers' => [$this->server->uuid], + 'servers' => $this->server ? [$this->server->uuid] : [], ], ]); } diff --git a/app/Services/Allocations/FindAssignableAllocationService.php b/app/Services/Allocations/FindAssignableAllocationService.php index 69aa8470d1..7000dabdf0 100644 --- a/app/Services/Allocations/FindAssignableAllocationService.php +++ b/app/Services/Allocations/FindAssignableAllocationService.php @@ -56,6 +56,7 @@ public function handle(Server $server): Allocation // those belonging to the current server (making it impossible to find unassigned ones) /** @var Allocation|null $allocation */ $allocation = Allocation::withoutGlobalScopes() + ->lockForUpdate() ->where('node_id', $server->node_id) ->when($server->allocation, function ($query) use ($server) { $query->where('ip', $server->allocation->ip); @@ -125,6 +126,7 @@ protected function createNewAllocation(Server $server, ?int $start, ?int $end): /** @var Allocation $allocation */ $allocation = Allocation::withoutGlobalScopes() + ->lockForUpdate() ->where('node_id', $server->node_id) ->where('ip', $server->allocation->ip) ->where('port', $port) diff --git a/app/Services/Servers/DetailsModificationService.php b/app/Services/Servers/DetailsModificationService.php index e8c40e3c56..988ab70dc1 100644 --- a/app/Services/Servers/DetailsModificationService.php +++ b/app/Services/Servers/DetailsModificationService.php @@ -2,11 +2,10 @@ namespace App\Services\Servers; +use App\Jobs\RevokeSftpAccessJob; use App\Models\Server; -use App\Repositories\Daemon\DaemonServerRepository; use App\Traits\Services\ReturnsUpdatedModels; use Illuminate\Database\ConnectionInterface; -use Illuminate\Http\Client\ConnectionException; use Illuminate\Support\Arr; use Throwable; @@ -17,7 +16,7 @@ class DetailsModificationService /** * DetailsModificationService constructor. */ - public function __construct(private ConnectionInterface $connection, private DaemonServerRepository $serverRepository) {} + public function __construct(private ConnectionInterface $connection) {} /** * Update the details for a single server instance. @@ -34,7 +33,7 @@ public function __construct(private ConnectionInterface $connection, private Dae public function handle(Server $server, array $data): Server { return $this->connection->transaction(function () use ($data, $server) { - $owner = $server->owner_id; + $oldOwner = $server->user; $server->forceFill([ 'external_id' => Arr::get($data, 'external_id'), @@ -46,14 +45,8 @@ public function handle(Server $server, array $data): Server // If the owner_id value is changed we need to revoke any tokens that exist for the server // on the daemon instance so that the old owner no longer has any permission to access the // websockets. - if ($server->owner_id !== $owner) { - try { - $this->serverRepository->setServer($server)->deauthorize($server->user->uuid); - } catch (ConnectionException) { - // Do nothing. A failure here is not ideal, but it is likely to be caused by daemon - // being offline, or in an entirely broken state. Remember, these tokens reset every - // few minutes by default, we're just trying to help it along a little quicker. - } + if ($server->owner_id !== $oldOwner->id) { + RevokeSftpAccessJob::dispatch($oldOwner->uuid, $server); } return $server; diff --git a/app/Services/Subusers/SubuserDeletionService.php b/app/Services/Subusers/SubuserDeletionService.php index bbf232995d..f34a3a2c11 100644 --- a/app/Services/Subusers/SubuserDeletionService.php +++ b/app/Services/Subusers/SubuserDeletionService.php @@ -4,17 +4,12 @@ use App\Events\Server\SubUserRemoved; use App\Facades\Activity; +use App\Jobs\RevokeSftpAccessJob; use App\Models\Server; use App\Models\Subuser; -use App\Repositories\Daemon\DaemonServerRepository; -use Illuminate\Http\Client\ConnectionException; class SubuserDeletionService { - public function __construct( - private DaemonServerRepository $serverRepository, - ) {} - public function handle(Subuser $subuser, Server $server): void { $log = Activity::event('server:subuser.delete') @@ -27,14 +22,7 @@ public function handle(Subuser $subuser, Server $server): void event(new SubUserRemoved($subuser->server, $subuser->user)); - try { - $this->serverRepository->setServer($server)->deauthorize($subuser->user->uuid); - } catch (ConnectionException $exception) { - // Don't block this request if we can't connect to the daemon instance. - logger()->warning($exception, ['user_id' => $subuser->user_id, 'server_id' => $server->id]); - - $instance->property('revoked', false); - } + RevokeSftpAccessJob::dispatch($subuser->user->uuid, $server); }); } } diff --git a/app/Services/Subusers/SubuserUpdateService.php b/app/Services/Subusers/SubuserUpdateService.php index 1ce9c0ac53..c2d693c95d 100644 --- a/app/Services/Subusers/SubuserUpdateService.php +++ b/app/Services/Subusers/SubuserUpdateService.php @@ -4,17 +4,12 @@ use App\Enums\SubuserPermission; use App\Facades\Activity; +use App\Jobs\RevokeSftpAccessJob; use App\Models\Server; use App\Models\Subuser; -use App\Repositories\Daemon\DaemonServerRepository; -use Illuminate\Http\Client\ConnectionException; class SubuserUpdateService { - public function __construct( - private DaemonServerRepository $serverRepository, - ) {} - /** * @param string[] $permissions */ @@ -42,18 +37,10 @@ public function handle(Subuser $subuser, Server $server, array $permissions): vo // Only update the database and hit up the daemon instance to invalidate JTI's if the permissions // have actually changed for the user. if ($cleanedPermissions !== $current) { - $log->transaction(function ($instance) use ($subuser, $cleanedPermissions, $server) { + $log->transaction(function () use ($subuser, $cleanedPermissions, $server) { $subuser->update(['permissions' => $cleanedPermissions]); - try { - $this->serverRepository->setServer($server)->deauthorize($subuser->user->uuid); - } catch (ConnectionException $exception) { - // Don't block this request if we can't connect to the daemon instance. Chances are it is - // offline and the token will be invalid once daemon boots back. - logger()->warning($exception, ['user_id' => $subuser->user_id, 'server_id' => $server->id]); - - $instance->property('revoked', false); - } + RevokeSftpAccessJob::dispatch($subuser->user->uuid, $server); }); } diff --git a/app/Services/Users/UserUpdateService.php b/app/Services/Users/UserUpdateService.php index aacb0cb479..e5aaa3ad1f 100644 --- a/app/Services/Users/UserUpdateService.php +++ b/app/Services/Users/UserUpdateService.php @@ -2,6 +2,7 @@ namespace App\Services\Users; +use App\Events\User\PasswordChanged; use App\Models\User; use App\Traits\Services\HasUserLevels; use Illuminate\Contracts\Hashing\Hasher; @@ -30,6 +31,10 @@ public function handle(User $user, array $data): User $user->forceFill($data)->saveOrFail(); + if (isset($data['password'])) { + PasswordChanged::dispatch($user); + } + return $user->refresh(); } } diff --git a/bootstrap/app.php b/bootstrap/app.php index d0862ec3ea..a64834b6f0 100644 --- a/bootstrap/app.php +++ b/bootstrap/app.php @@ -12,6 +12,7 @@ use App\Http\Middleware\LanguageMiddleware; use App\Http\Middleware\MaintenanceMiddleware; use App\Http\Middleware\RedirectIfAuthenticated; +use App\Http\Middleware\SetSecurityHeaders; use App\Http\Middleware\VerifyCsrfToken; use Illuminate\Contracts\Debug\ExceptionHandler; use Illuminate\Foundation\Application; @@ -28,7 +29,10 @@ ->withMiddleware(function (Middleware $middleware) { $middleware->redirectGuestsTo(fn () => route('filament.app.auth.login')); - $middleware->web(LanguageMiddleware::class); + $middleware->web([ + LanguageMiddleware::class, + SetSecurityHeaders::class, + ]); $middleware->api([ EnsureStatefulRequests::class, diff --git a/config/http.php b/config/http.php index 69bf4b5305..e76ec923da 100644 --- a/config/http.php +++ b/config/http.php @@ -13,9 +13,9 @@ */ 'rate_limit' => [ 'client_period' => 1, - 'client' => env('APP_API_CLIENT_RATELIMIT', 120), + 'client' => env('APP_API_CLIENT_RATELIMIT', 256), 'application_period' => 1, - 'application' => env('APP_API_APPLICATION_RATELIMIT', 240), + 'application' => env('APP_API_APPLICATION_RATELIMIT', 256), ], ]; diff --git a/database/Factories/ServerTransferFactory.php b/database/Factories/ServerTransferFactory.php new file mode 100644 index 0000000000..420c3c5135 --- /dev/null +++ b/database/Factories/ServerTransferFactory.php @@ -0,0 +1,29 @@ + [], + 'new_additional_allocations' => [], + 'successful' => null, + 'archived' => false, + ]; + } +} diff --git a/routes/api-client.php b/routes/api-client.php index 4365cfeb22..16f8f678e9 100644 --- a/routes/api-client.php +++ b/routes/api-client.php @@ -23,7 +23,9 @@ Route::get('/', [Client\AccountController::class, 'index'])->name('api:client.account'); Route::put('/username', [Client\AccountController::class, 'updateUsername'])->name('api:client.account.update-username'); - Route::put('/email', [Client\AccountController::class, 'updateEmail'])->name('api:client.account.update-email'); + Route::put('/email', [Client\AccountController::class, 'updateEmail']) + ->middleware('throttle') + ->name('api:client.account.update-email'); Route::put('/password', [Client\AccountController::class, 'updatePassword'])->name('api:client.account.update-password'); Route::get('/activity', Client\ActivityLogController::class)->name('api:client.account.activity'); diff --git a/routes/api-remote.php b/routes/api-remote.php index 10b0c699bb..52d7e37c32 100644 --- a/routes/api-remote.php +++ b/routes/api-remote.php @@ -15,8 +15,6 @@ Route::get('/install', [Remote\Servers\ServerInstallController::class, 'index']); Route::post('/install', [Remote\Servers\ServerInstallController::class, 'store']); - Route::get('/transfer/failure', [Remote\Servers\ServerTransferController::class, 'failure']); - Route::get('/transfer/success', [Remote\Servers\ServerTransferController::class, 'success']); Route::post('/transfer/failure', [Remote\Servers\ServerTransferController::class, 'failure']); Route::post('/transfer/success', [Remote\Servers\ServerTransferController::class, 'success']); diff --git a/tests/Integration/Api/Client/AccountControllerTest.php b/tests/Integration/Api/Client/AccountControllerTest.php index b6ad33ed35..d052e374f5 100644 --- a/tests/Integration/Api/Client/AccountControllerTest.php +++ b/tests/Integration/Api/Client/AccountControllerTest.php @@ -2,8 +2,12 @@ namespace App\Tests\Integration\Api\Client; +use App\Jobs\RevokeSftpAccessJob; +use App\Models\Subuser; use App\Models\User; +use Illuminate\Database\Eloquent\Collection; use Illuminate\Http\Response; +use Illuminate\Support\Facades\Bus; use Illuminate\Support\Facades\Hash; use Illuminate\Support\Str; @@ -44,16 +48,38 @@ public function test_email_is_updated(): void /** @var User $user */ $user = User::factory()->create(); - $response = $this->actingAs($user)->putJson('/api/client/account/email', [ - 'email' => $email = mb_strtolower(Str::random() . '@example.com'), - 'password' => 'password', - ]); - - $response->assertStatus(Response::HTTP_NO_CONTENT); + $this->actingAs($user) + ->putJson('/api/client/account/email', [ + 'email' => $email = mb_strtolower(Str::random() . '@example.com'), + 'password' => 'password', + ]) + ->assertStatus(Response::HTTP_NO_CONTENT); + $this->assertActivityFor('user:account.email-changed', $user, $user); $this->assertDatabaseHas('users', ['id' => $user->id, 'email' => $email]); } + public function test_email_change_is_throttled(): void + { + /** @var Collection $users */ + $users = User::factory()->count(2)->create(); + + for ($i = 0; $i < 3; $i++) { + $this->actingAs($users[0]) + ->putJson('/api/client/account/email', ['email' => "foo+{$i}@example.com", 'password' => 'password']) + ->assertNoContent(); + } + + $this->putJson('/api/client/account/email', ['email' => 'bar@example.com', 'password' => 'password']) + ->assertTooManyRequests(); + + // The other user should still be able to update their email because the throttle + // is tied to the account, not to the IP address. + $this->actingAs($users[1]) + ->putJson('/api/client/account/email', ['email' => 'bar+1@example.com', 'password' => 'password']) + ->assertNoContent(); + } + /** * Tests that an email is not updated if the password provided in the request is not * valid for the account. @@ -109,13 +135,24 @@ public function test_password_is_updated(): void /** @var User $user */ $user = User::factory()->create(); + // Assign the user to two servers, one as the owner the other as a subuser, both + // on different nodes to ensure our logic fires off correctly and the user has their + // credentials revoked on both nodes. + $server = $this->createServerModel(['owner_id' => $user->id]); + $server2 = $this->createServerModel(); + Subuser::factory()->for($server2)->for($user)->create(); + $initialHash = $user->password; - $response = $this->actingAs($user)->putJson('/api/client/account/password', [ - 'current_password' => 'password', - 'password' => 'New_Password1', - 'password_confirmation' => 'New_Password1', - ]); + Bus::fake([RevokeSftpAccessJob::class]); + + $this->actingAs($user) + ->putJson('/api/client/account/password', [ + 'current_password' => 'password', + 'password' => 'New_Password1', + 'password_confirmation' => 'New_Password1', + ]) + ->assertNoContent(); $user = $user->refresh(); @@ -123,7 +160,12 @@ public function test_password_is_updated(): void $this->assertTrue(Hash::check('New_Password1', $user->password)); $this->assertFalse(Hash::check('password', $user->password)); - $response->assertStatus(Response::HTTP_NO_CONTENT); + $this->assertActivityFor('user:account.password-changed', $user, $user); + $this->assertNotEquals($server->node_id, $server2->node_id); + + Bus::assertDispatchedTimes(RevokeSftpAccessJob::class, 2); + Bus::assertDispatched(fn (RevokeSftpAccessJob $job) => $job->user === $user->uuid && $job->target->is($server->node)); + Bus::assertDispatched(fn (RevokeSftpAccessJob $job) => $job->user === $user->uuid && $job->target->is($server2->node)); } /** diff --git a/tests/Integration/Api/Client/Server/Subuser/DeleteSubuserTest.php b/tests/Integration/Api/Client/Server/Subuser/DeleteSubuserTest.php index 49d6a9166a..998fa51e85 100644 --- a/tests/Integration/Api/Client/Server/Subuser/DeleteSubuserTest.php +++ b/tests/Integration/Api/Client/Server/Subuser/DeleteSubuserTest.php @@ -3,10 +3,11 @@ namespace App\Tests\Integration\Api\Client\Server\Subuser; use App\Enums\SubuserPermission; +use App\Jobs\RevokeSftpAccessJob; use App\Models\Subuser; use App\Models\User; -use App\Repositories\Daemon\DaemonServerRepository; use App\Tests\Integration\Api\Client\ClientApiIntegrationTestCase; +use Illuminate\Support\Facades\Bus; use Ramsey\Uuid\Uuid; class DeleteSubuserTest extends ClientApiIntegrationTestCase @@ -22,7 +23,7 @@ class DeleteSubuserTest extends ClientApiIntegrationTestCase */ public function test_correct_subuser_is_deleted_from_server(): void { - $this->swap(DaemonServerRepository::class, $mock = \Mockery::mock(DaemonServerRepository::class)); + Bus::fake([RevokeSftpAccessJob::class]); [$user, $server] = $this->generateTestAccount(); @@ -42,10 +43,12 @@ public function test_correct_subuser_is_deleted_from_server(): void 'permissions' => [SubuserPermission::WebsocketConnect], ]); - $mock->expects('setServer->deauthorize')->with($subuser->uuid)->andReturnUndefined(); - $this->actingAs($user)->deleteJson($this->link($server) . "/users/$subuser->uuid")->assertNoContent(); + Bus::assertDispatched(function (RevokeSftpAccessJob $job) use ($subuser, $server) { + return $job->user === $subuser->uuid && $job->target->is($server); + }); + // Try the same test, but this time with a UUID that if cast to an int (shouldn't) line up with // anything in the database. $uuid = '18180000' . substr(Uuid::uuid4()->toString(), 8); @@ -58,8 +61,10 @@ public function test_correct_subuser_is_deleted_from_server(): void 'permissions' => [SubuserPermission::WebsocketConnect], ]); - $mock->expects('setServer->deauthorize')->with($subuser->uuid)->andReturnUndefined(); - $this->actingAs($user)->deleteJson($this->link($server) . "/users/$subuser->uuid")->assertNoContent(); + + Bus::assertDispatched(function (RevokeSftpAccessJob $job) use ($subuser, $server) { + return $job->user === $subuser->uuid && $job->target->is($server); + }); } } diff --git a/tests/Integration/Api/Client/Server/Subuser/SubuserAuthorizationTest.php b/tests/Integration/Api/Client/Server/Subuser/SubuserAuthorizationTest.php index 9d9c85d4f9..9a8a0c2d97 100644 --- a/tests/Integration/Api/Client/Server/Subuser/SubuserAuthorizationTest.php +++ b/tests/Integration/Api/Client/Server/Subuser/SubuserAuthorizationTest.php @@ -2,10 +2,11 @@ namespace App\Tests\Integration\Api\Client\Server\Subuser; +use App\Jobs\RevokeSftpAccessJob; use App\Models\Subuser; use App\Models\User; -use App\Repositories\Daemon\DaemonServerRepository; use App\Tests\Integration\Api\Client\ClientApiIntegrationTestCase; +use Illuminate\Support\Facades\Bus; use PHPUnit\Framework\Attributes\DataProvider; class SubuserAuthorizationTest extends ClientApiIntegrationTestCase @@ -16,6 +17,8 @@ class SubuserAuthorizationTest extends ClientApiIntegrationTestCase #[DataProvider('methodDataProvider')] public function test_user_cannot_access_resource_belonging_to_other_servers(string $method): void { + Bus::fake([RevokeSftpAccessJob::class]); + // Generic subuser, the specific resource we're trying to access. /** @var User $internal */ $internal = User::factory()->create(); @@ -35,11 +38,6 @@ public function test_user_cannot_access_resource_belonging_to_other_servers(stri Subuser::factory()->create(['server_id' => $server2->id, 'user_id' => $internal->id]); Subuser::factory()->create(['server_id' => $server3->id, 'user_id' => $internal->id]); - $this->instance(DaemonServerRepository::class, $mock = \Mockery::mock(DaemonServerRepository::class)); - if ($method === 'DELETE') { - $mock->expects('setServer->deauthorize')->with($internal->uuid)->andReturnUndefined(); - } - // This route is acceptable since they're accessing a subuser on their own server. $this->actingAs($user)->json($method, $this->link($server1, '/users/' . $internal->uuid))->assertStatus($method === 'POST' ? 422 : ($method === 'DELETE' ? 204 : 200)); @@ -47,6 +45,14 @@ public function test_user_cannot_access_resource_belonging_to_other_servers(stri // errors out with a 403 since $user does not have the right permissions for this. $this->actingAs($user)->json($method, $this->link($server2, '/users/' . $internal->uuid))->assertForbidden(); $this->actingAs($user)->json($method, $this->link($server3, '/users/' . $internal->uuid))->assertNotFound(); + + if ($method === 'DELETE') { + Bus::assertDispatchedTimes(function (RevokeSftpAccessJob $job) use ($server1, $internal) { + return $job->user === $internal->uuid && $job->target->is($server1); + }); + } else { + Bus::assertNotDispatched(RevokeSftpAccessJob::class); + } } public static function methodDataProvider(): array diff --git a/tests/Integration/Api/Client/Server/Subuser/UpdateSubuserTest.php b/tests/Integration/Api/Client/Server/Subuser/UpdateSubuserTest.php index 56c3b6123d..48e71d52b1 100644 --- a/tests/Integration/Api/Client/Server/Subuser/UpdateSubuserTest.php +++ b/tests/Integration/Api/Client/Server/Subuser/UpdateSubuserTest.php @@ -3,9 +3,11 @@ namespace App\Tests\Integration\Api\Client\Server\Subuser; use App\Enums\SubuserPermission; +use App\Jobs\RevokeSftpAccessJob; use App\Models\Subuser; use App\Models\User; use App\Tests\Integration\Api\Client\ClientApiIntegrationTestCase; +use Illuminate\Support\Facades\Bus; use Illuminate\Support\Facades\Context; use Illuminate\Support\Facades\Http; @@ -17,6 +19,8 @@ class UpdateSubuserTest extends ClientApiIntegrationTestCase */ public function test_correct_permissions_are_required_for_updating(): void { + Bus::fake([RevokeSftpAccessJob::class]); + [$user, $server] = $this->generateTestAccount(['user.read']); Http::fake(); @@ -54,6 +58,10 @@ public function test_correct_permissions_are_required_for_updating(): void ]); $this->postJson($endpoint, $data)->assertOk(); + + Bus::assertDispatched(function (RevokeSftpAccessJob $job) use ($server, $subuser) { + return $job->user === $subuser->user->uuid && $job->target->is($server); + }); } /** @@ -62,6 +70,8 @@ public function test_correct_permissions_are_required_for_updating(): void */ public function test_permissions_are_saved_to_account(): void { + Bus::fake([RevokeSftpAccessJob::class]); + [$user, $server] = $this->generateTestAccount(); /** @var Subuser $subuser */ @@ -91,6 +101,10 @@ public function test_permissions_are_saved_to_account(): void ['control.start', 'control.stop', 'websocket.connect'], $subuser->permissions ); + + Bus::assertDispatched(function (RevokeSftpAccessJob $job) use ($server, $subuser) { + return $job->user === $subuser->user->uuid && $job->target->is($server); + }); } /** @@ -99,6 +113,8 @@ public function test_permissions_are_saved_to_account(): void */ public function test_user_cannot_assign_permissions_they_do_not_have(): void { + Bus::fake([RevokeSftpAccessJob::class]); + [$user, $server] = $this->generateTestAccount([SubuserPermission::UserRead, SubuserPermission::UserUpdate]); $subuser = Subuser::factory() @@ -113,6 +129,8 @@ public function test_user_cannot_assign_permissions_they_do_not_have(): void ->assertForbidden(); $this->assertEqualsCanonicalizing(['foo.bar'], $subuser->refresh()->permissions); + + Bus::assertNothingDispatched(); } /** diff --git a/tests/Integration/Api/Remote/ServerTransferControllerTest.php b/tests/Integration/Api/Remote/ServerTransferControllerTest.php new file mode 100644 index 0000000000..5d9c812293 --- /dev/null +++ b/tests/Integration/Api/Remote/ServerTransferControllerTest.php @@ -0,0 +1,108 @@ +createServerModel(); + + $new = Node::factory() + ->has(Allocation::factory()) + ->create(); + + $this->transfer = ServerTransfer::factory()->for($server)->create([ + 'old_allocation' => $server->allocation_id, + 'new_allocation' => $new->allocations->first()->id, + 'new_node' => $new->id, + 'old_node' => $server->node_id, + ]); + } + + public function test_success_status_update_can_be_sent_from_new_node(): void + { + $server = $this->transfer->server; + $newNode = $this->transfer->newNode; + + $this->withHeader('Authorization', "Bearer $newNode->daemon_token_id." . $newNode->daemon_token) + ->postJson("/api/remote/servers/{$server->uuid}/transfer/success") + ->assertNoContent(); + + $this->assertTrue($this->transfer->refresh()->successful); + } + + public function test_failure_status_update_can_be_sent_from_old_node(): void + { + $server = $this->transfer->server; + $oldNode = $this->transfer->oldNode; + + $this->withHeader('Authorization', "Bearer $oldNode->daemon_token_id." . $oldNode->daemon_token) + ->postJson("/api/remote/servers/{$server->uuid}/transfer/failure") + ->assertNoContent(); + + $this->assertFalse($this->transfer->refresh()->successful); + } + + public function test_failure_status_update_can_be_sent_from_new_node(): void + { + $server = $this->transfer->server; + $newNode = $this->transfer->newNode; + + $this->withHeader('Authorization', "Bearer $newNode->daemon_token_id." . $newNode->daemon_token) + ->postJson("/api/remote/servers/{$server->uuid}/transfer/failure") + ->assertNoContent(); + + $this->assertFalse($this->transfer->refresh()->successful); + } + + public function test_success_status_update_cannot_be_sent_from_old_node(): void + { + $server = $this->transfer->server; + $oldNode = $this->transfer->oldNode; + + $this->withHeader('Authorization', "Bearer $oldNode->daemon_token_id." . $oldNode->daemon_token) + ->postJson("/api/remote/servers/{$server->uuid}/transfer/success") + ->assertForbidden() + ->assertJsonPath('errors.0.code', 'HttpForbiddenException') + ->assertJsonPath('errors.0.detail', 'Requesting node does not have permission to access this server.'); + + $this->assertNull($this->transfer->refresh()->successful); + } + + public function test_success_status_update_cannot_be_sent_from_unauthorized_node(): void + { + $server = $this->transfer->server; + $node = Node::factory()->create(); + + $this->withHeader('Authorization', "Bearer $node->daemon_token_id." . $node->daemon_token) + ->postJson("/api/remote/servers/$server->uuid/transfer/success") + ->assertForbidden() + ->assertJsonPath('errors.0.code', 'HttpForbiddenException') + ->assertJsonPath('errors.0.detail', 'Requesting node does not have permission to access this server.'); + + $this->assertNull($this->transfer->refresh()->successful); + } + + public function test_failure_status_update_cannot_be_sent_from_unauthorized_node(): void + { + $server = $this->transfer->server; + $node = Node::factory()->create(); + + $this->withHeader('Authorization', "Bearer $node->daemon_token_id." . $node->daemon_token) + ->postJson("/api/remote/servers/$server->uuid/transfer/failure")->assertForbidden() + ->assertJsonPath('errors.0.code', 'HttpForbiddenException') + ->assertJsonPath('errors.0.detail', 'Requesting node does not have permission to access this server.'); + + $this->assertNull($this->transfer->refresh()->successful); + } +} diff --git a/tests/Integration/Jobs/RevokeSftpAccessJobTest.php b/tests/Integration/Jobs/RevokeSftpAccessJobTest.php new file mode 100644 index 0000000000..716ad084e8 --- /dev/null +++ b/tests/Integration/Jobs/RevokeSftpAccessJobTest.php @@ -0,0 +1,68 @@ +make(['uuid' => 'uuid-1234']); + + $job = new RevokeSftpAccessJob('user-1', $model); + + $this->assertEquals( + "revoke-sftp:user-1:{$key}:uuid-1234", + $job->uniqueId() + ); + } + + public function test_job_releases_back_to_queue_on_failure(): void + { + $node = Node::factory()->make(['uuid' => 'uuid-1234']); + + $mock = $this->mock(DaemonServerRepository::class, function ($mock) { + $mock->expects('setNode')->andReturnSelf(); + $mock->expects('deauthorize')->andThrows(new ConnectionException()); + }); + + $job = \Mockery::mock(RevokeSftpAccessJob::class, ['user-1', $node])->makePartial(); + $job->expects('release')->with(10); + + $job->handle($mock); + } + + public function test_job_dispatches_for_node(): void + { + $node = Node::factory()->make(['uuid' => 'uuid-1234']); + + $mock = $this->mock(DaemonServerRepository::class, function ($mock) { + $mock->expects('setNode')->andReturnSelf(); + $mock->expects('deauthorize')->with('user-1')->andReturnUndefined(); + }); + + (new RevokeSftpAccessJob('user-1', $node))->handle($mock); + } + + public function test_job_dispatches_for_individual_server(): void + { + $node = Node::factory()->make(['uuid' => 'node-1234']); + $server = Server::factory()->make(['uuid' => 'server-1234'])->setRelation('node', $node); + + $mock = $this->mock(DaemonServerRepository::class, function ($mock) { + $mock->expects('setServer')->with(\Mockery::on(fn (Server $server) => $server->uuid === 'server-1234'))->andReturnSelf(); + $mock->expects('deauthorize')->with('user-1')->andReturnUndefined(); + }); + + (new RevokeSftpAccessJob('user-1', $server))->handle($mock); + } +} diff --git a/tests/Integration/Services/Users/UserDeletionServiceTest.php b/tests/Integration/Services/Users/UserDeletionServiceTest.php new file mode 100644 index 0000000000..aa31d3ab98 --- /dev/null +++ b/tests/Integration/Services/Users/UserDeletionServiceTest.php @@ -0,0 +1,63 @@ +createServerModel(); + + $this->expectException(DisplayException::class); + $this->expectExceptionMessage(trans('exceptions.users.has_servers')); + + $server->user->delete(); + + $this->assertModelExists($server->user); + + Bus::assertNotDispatched(RevokeSftpAccessJob::class); + } + + public function test_user_is_deleted(): void + { + $user = User::factory()->create(); + + $user->delete(); + + $this->assertModelMissing($user); + + Bus::assertNotDispatched(RevokeSftpAccessJob::class); + } + + public function test_user_is_deleted_and_access_revoked(): void + { + $user = User::factory()->create(); + + $server1 = $this->createServerModel(); + $server2 = $this->createServerModel(['node_id' => $server1->node_id]); + + Subuser::factory()->for($server1)->for($user)->create(); + Subuser::factory()->for($server2)->for($user)->create(); + + $user->delete(); + + $this->assertModelMissing($user); + + Bus::assertDispatchedTimes(RevokeSftpAccessJob::class); + Bus::assertDispatched(fn (RevokeSftpAccessJob $job) => $job->user === $user->uuid && $job->target->is($server1->node)); + } +}