mirror of
https://github.com/Part-DB/Part-DB-server.git
synced 2026-03-15 03:38:50 +01:00
Add manual backup creation and delete buttons to Update Manager (#1255)
* Add manual backup creation and delete buttons to Update Manager - Add "Create Backup" button in the backups tab for on-demand backups - Add delete buttons (trash icons) for update logs and backups - New controller routes with CSRF protection and permission checks - Use data-turbo-confirm for CSP-safe confirmation dialogs - Add deleteLog() method to UpdateExecutor with filename validation * Add Docker backup support: download button, SQLite restore fix, decouple from auto-update - Decouple backup creation/restore UI from can_auto_update so Docker and other non-git installations can use backup features - Add backup download endpoint for saving backups externally - Fix SQLite restore to use configured DATABASE_URL path instead of hardcoded var/app.db (affects Docker and custom SQLite paths) - Show Docker-specific warning about var/backups/ not being persisted - Pass is_docker flag to template via InstallationTypeDetector * Add tests for backup/update manager improvements - Controller tests: auth, CSRF validation, 404 for missing backups, restore disabled check - UpdateExecutor: deleteLog validation, non-existent file, successful deletion - BackupManager: deleteBackup validation for missing/non-zip files * Fix test failures: add locale prefix to URLs, correct log directory path * Fix auth test: expect 401 instead of redirect for HTTP Basic auth * Improve test coverage for update manager controller Add happy-path tests for backup creation, deletion, download, and log deletion with valid CSRF tokens. Also test the locked state blocking backup creation. * Fix CSRF tests: initialize session before getting tokens * Fix CSRF tests: extract tokens from rendered page HTML * Harden backup security: password confirmation, CSRF, env toggle Address security review feedback from jbtronics: - Add IS_AUTHENTICATED_FULLY to all sensitive endpoints (create/delete backup, delete log, download backup, start update, restore) - Change backup download from GET to POST with CSRF token - Require password confirmation before downloading backups (backups contain sensitive data like password hashes and secrets) - Add DISABLE_BACKUP_DOWNLOAD env var (default: disabled) to control whether backup downloads are allowed - Add password confirmation modal with security warning in template - Add comprehensive tests: auth checks, env var blocking, POST-only enforcement, status/progress endpoint auth * Fix download modal: use per-backup modals for CSP/Turbo compatibility - Replace shared modal + inline JS with per-backup modals that have filename pre-set in hidden fields (no JavaScript needed) - Add data-turbo="false" to download forms for native browser handling - Add data-bs-dismiss="modal" to submit button to auto-close modal - Add hidden username field for Chrome accessibility best practice - Fix test: GET on POST-only route returns 404 not 405 * Fixed translation keys * Fixed text justification in download modal * Hardenened security of deleteLogEndpoint * Show whether backup, restores and updates are allowed or disabled by sysadmin on update manager * Added documentation for update manager related env variables --------- Co-authored-by: Jan Böhmer <mail@jan-boehmer.de>
This commit is contained in:
committed by
GitHub
parent
db8881621c
commit
0d58262e19
@@ -23,16 +23,21 @@ declare(strict_types=1);
|
||||
|
||||
namespace App\Controller;
|
||||
|
||||
use App\Entity\UserSystem\User;
|
||||
use App\Services\System\BackupManager;
|
||||
use App\Services\System\InstallationTypeDetector;
|
||||
use App\Services\System\UpdateChecker;
|
||||
use App\Services\System\UpdateExecutor;
|
||||
use Shivas\VersioningBundle\Service\VersionManagerInterface;
|
||||
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
|
||||
use Symfony\Component\DependencyInjection\Attribute\Autowire;
|
||||
use Symfony\Component\HttpFoundation\BinaryFileResponse;
|
||||
use Symfony\Component\HttpFoundation\JsonResponse;
|
||||
use Symfony\Component\HttpFoundation\Request;
|
||||
use Symfony\Component\HttpFoundation\Response;
|
||||
use Symfony\Component\HttpFoundation\ResponseHeaderBag;
|
||||
use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
|
||||
use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface;
|
||||
use Symfony\Component\Routing\Attribute\Route;
|
||||
|
||||
/**
|
||||
@@ -49,10 +54,14 @@ class UpdateManagerController extends AbstractController
|
||||
private readonly UpdateExecutor $updateExecutor,
|
||||
private readonly VersionManagerInterface $versionManager,
|
||||
private readonly BackupManager $backupManager,
|
||||
private readonly InstallationTypeDetector $installationTypeDetector,
|
||||
private readonly UserPasswordHasherInterface $passwordHasher,
|
||||
#[Autowire(env: 'bool:DISABLE_WEB_UPDATES')]
|
||||
private readonly bool $webUpdatesDisabled = false,
|
||||
#[Autowire(env: 'bool:DISABLE_BACKUP_RESTORE')]
|
||||
private readonly bool $backupRestoreDisabled = false,
|
||||
#[Autowire(env: 'bool:DISABLE_BACKUP_DOWNLOAD')]
|
||||
private readonly bool $backupDownloadDisabled = false,
|
||||
) {
|
||||
}
|
||||
|
||||
@@ -76,6 +85,16 @@ class UpdateManagerController extends AbstractController
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if backup download is disabled and throw exception if so.
|
||||
*/
|
||||
private function denyIfBackupDownloadDisabled(): void
|
||||
{
|
||||
if ($this->backupDownloadDisabled) {
|
||||
throw new AccessDeniedHttpException('Backup download is disabled by server configuration.');
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Main update manager page.
|
||||
*/
|
||||
@@ -101,6 +120,8 @@ class UpdateManagerController extends AbstractController
|
||||
'backups' => $this->backupManager->getBackups(),
|
||||
'web_updates_disabled' => $this->webUpdatesDisabled,
|
||||
'backup_restore_disabled' => $this->backupRestoreDisabled,
|
||||
'backup_download_disabled' => $this->backupDownloadDisabled,
|
||||
'is_docker' => $this->installationTypeDetector->isDocker(),
|
||||
]);
|
||||
}
|
||||
|
||||
@@ -206,6 +227,7 @@ class UpdateManagerController extends AbstractController
|
||||
#[Route('/start', name: 'admin_update_manager_start', methods: ['POST'])]
|
||||
public function startUpdate(Request $request): Response
|
||||
{
|
||||
$this->denyAccessUnlessGranted('IS_AUTHENTICATED_FULLY');
|
||||
$this->denyAccessUnlessGranted('@system.manage_updates');
|
||||
$this->denyIfWebUpdatesDisabled();
|
||||
|
||||
@@ -314,12 +336,126 @@ class UpdateManagerController extends AbstractController
|
||||
return $this->json($details);
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a manual backup.
|
||||
*/
|
||||
#[Route('/backup', name: 'admin_update_manager_backup', methods: ['POST'])]
|
||||
public function createBackup(Request $request): Response
|
||||
{
|
||||
$this->denyAccessUnlessGranted('IS_AUTHENTICATED_FULLY');
|
||||
$this->denyAccessUnlessGranted('@system.manage_updates');
|
||||
|
||||
if (!$this->isCsrfTokenValid('update_manager_backup', $request->request->get('_token'))) {
|
||||
$this->addFlash('error', 'Invalid CSRF token.');
|
||||
return $this->redirectToRoute('admin_update_manager');
|
||||
}
|
||||
|
||||
if ($this->updateExecutor->isLocked()) {
|
||||
$this->addFlash('error', 'Cannot create backup while an update is in progress.');
|
||||
return $this->redirectToRoute('admin_update_manager');
|
||||
}
|
||||
|
||||
try {
|
||||
$this->backupManager->createBackup(null, 'manual');
|
||||
$this->addFlash('success', 'update_manager.backup.created');
|
||||
} catch (\Exception $e) {
|
||||
$this->addFlash('error', 'Backup failed: ' . $e->getMessage());
|
||||
}
|
||||
|
||||
return $this->redirectToRoute('admin_update_manager');
|
||||
}
|
||||
|
||||
/**
|
||||
* Delete a backup file.
|
||||
*/
|
||||
#[Route('/backup/delete', name: 'admin_update_manager_backup_delete', methods: ['POST'])]
|
||||
public function deleteBackup(Request $request): Response
|
||||
{
|
||||
$this->denyAccessUnlessGranted('IS_AUTHENTICATED_FULLY');
|
||||
$this->denyAccessUnlessGranted('@system.manage_updates');
|
||||
|
||||
if (!$this->isCsrfTokenValid('update_manager_delete', $request->request->get('_token'))) {
|
||||
$this->addFlash('error', 'Invalid CSRF token.');
|
||||
return $this->redirectToRoute('admin_update_manager');
|
||||
}
|
||||
|
||||
$filename = $request->request->get('filename');
|
||||
if ($filename && $this->backupManager->deleteBackup($filename)) {
|
||||
$this->addFlash('success', 'update_manager.backup.deleted');
|
||||
} else {
|
||||
$this->addFlash('error', 'update_manager.backup.delete_error');
|
||||
}
|
||||
|
||||
return $this->redirectToRoute('admin_update_manager');
|
||||
}
|
||||
|
||||
/**
|
||||
* Delete an update log file.
|
||||
*/
|
||||
#[Route('/log/delete', name: 'admin_update_manager_log_delete', methods: ['POST'])]
|
||||
public function deleteLog(Request $request): Response
|
||||
{
|
||||
$this->denyAccessUnlessGranted('IS_AUTHENTICATED_FULLY');
|
||||
$this->denyAccessUnlessGranted('@system.manage_updates');
|
||||
|
||||
if (!$this->isCsrfTokenValid('update_manager_delete', $request->request->get('_token'))) {
|
||||
$this->addFlash('error', 'Invalid CSRF token.');
|
||||
return $this->redirectToRoute('admin_update_manager');
|
||||
}
|
||||
|
||||
$filename = $request->request->get('filename');
|
||||
if ($filename && $this->updateExecutor->deleteLog($filename)) {
|
||||
$this->addFlash('success', 'update_manager.log.deleted');
|
||||
} else {
|
||||
$this->addFlash('error', 'update_manager.log.delete_error');
|
||||
}
|
||||
|
||||
return $this->redirectToRoute('admin_update_manager');
|
||||
}
|
||||
|
||||
/**
|
||||
* Download a backup file.
|
||||
* Requires password confirmation as backups contain sensitive data (password hashes, secrets, etc.).
|
||||
*/
|
||||
#[Route('/backup/download', name: 'admin_update_manager_backup_download', methods: ['POST'])]
|
||||
public function downloadBackup(Request $request): Response
|
||||
{
|
||||
$this->denyAccessUnlessGranted('IS_AUTHENTICATED_FULLY');
|
||||
$this->denyAccessUnlessGranted('@system.manage_updates');
|
||||
$this->denyIfBackupDownloadDisabled();
|
||||
|
||||
if (!$this->isCsrfTokenValid('update_manager_download', $request->request->get('_token'))) {
|
||||
$this->addFlash('error', 'Invalid CSRF token.');
|
||||
return $this->redirectToRoute('admin_update_manager');
|
||||
}
|
||||
|
||||
// Verify password
|
||||
$password = $request->request->get('password', '');
|
||||
$user = $this->getUser();
|
||||
if (!$user instanceof User || !$this->passwordHasher->isPasswordValid($user, $password)) {
|
||||
$this->addFlash('error', 'update_manager.backup.download.invalid_password');
|
||||
return $this->redirectToRoute('admin_update_manager');
|
||||
}
|
||||
|
||||
$filename = $request->request->get('filename', '');
|
||||
$details = $this->backupManager->getBackupDetails($filename);
|
||||
if (!$details) {
|
||||
throw $this->createNotFoundException('Backup not found');
|
||||
}
|
||||
|
||||
$response = new BinaryFileResponse($details['path']);
|
||||
$response->setContentDisposition(ResponseHeaderBag::DISPOSITION_ATTACHMENT, $details['file']);
|
||||
|
||||
return $response;
|
||||
}
|
||||
|
||||
/**
|
||||
* Restore from a backup.
|
||||
*/
|
||||
#[Route('/restore', name: 'admin_update_manager_restore', methods: ['POST'])]
|
||||
public function restore(Request $request): Response
|
||||
{
|
||||
$this->denyAccessUnlessGranted('IS_AUTHENTICATED_FULLY');
|
||||
$this->denyAccessUnlessGranted('@system.manage_updates');
|
||||
$this->denyIfBackupRestoreDisabled();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user