Locations: edit connection settings without losing the history

A changed password or host name could only be handled by adding a new
location, which starts an empty repository and, with a retention lock at
the provider, keeps the old one billed. Edit now changes a location in
place (web interface and occ backup:target:edit). The settings are only
saved when they lead to the same repository; a failed connection and a
missing repository give separate errors, and the folder is not created
while checking. Stored passwords are never sent to the browser: an empty
password field keeps the stored value.
This commit is contained in:
2026-10-08 12:56:15 +02:00
parent a8da8cb54a
commit 9775cdc475
13 changed files with 300 additions and 43 deletions
+11 -1
View File
@@ -21,11 +21,12 @@ final class StorageBackend implements IBackend {
public function __construct(
private IStorage $storage,
private string $base = '',
bool $createBase = true,
) {
$this->base = trim($base, '/');
self::validate($this->base, true);
$this->atomicPut = $storage->instanceOfStorage(\OCA\Files_External\Lib\Storage\AmazonS3::class);
if ($this->base !== '') {
if ($createBase && $this->base !== '') {
$this->mkdirs($this->base);
}
}
@@ -109,6 +110,15 @@ final class StorageBackend implements IBackend {
}
}
/** Whether the storage accepts the connection settings (files_external's own check). */
public function canConnect(): bool {
try {
return !method_exists($this->storage, 'test') || (bool)$this->storage->test();
} catch (\Throwable) {
return false;
}
}
public function exists(string $path): bool {
return $this->storage->file_exists($this->abs($path));
}
+71
View File
@@ -0,0 +1,71 @@
<?php
declare(strict_types=1);
// SPDX-FileCopyrightText: 2026 Dillard Blom
// SPDX-License-Identifier: AGPL-3.0-or-later
namespace OCA\NgBackup\Command;
use OCA\NgBackup\Service\AlertService;
use OCA\NgBackup\Service\TargetService;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;
final class TargetEdit extends Command {
public function __construct(
private TargetService $targets,
private AlertService $alerts,
) {
parent::__construct();
}
protected function configure(): void {
$this->setName('backup:target:edit')
->setDescription('Change the connection settings or the name of a backup location, keeping its backups')
->addArgument('target', InputArgument::REQUIRED, 'Location name or id')
->addOption('name', null, InputOption::VALUE_REQUIRED, 'New name')
->addOption('backend', 'b', InputOption::VALUE_REQUIRED, 'New files_external backend id (see backup:target:backends)')
->addOption('auth', 'a', InputOption::VALUE_REQUIRED, 'New auth mechanism id')
->addOption('option', 'o', InputOption::VALUE_REQUIRED | InputOption::VALUE_IS_ARRAY, 'Option key=value to change (repeatable); other options keep their value')
->addOption('path', 'p', InputOption::VALUE_REQUIRED, 'New folder on the location')
->setHelp(<<<'HELP'
The new settings must lead to the same repository as before, so the history of the location is
kept. Use it after a password change, a new host name, or after moving the repository.
Example:
occ backup:target:edit nas -o password=new-secret
HELP);
}
protected function execute(InputInterface $input, OutputInterface $output): int {
try {
$t = $this->targets->get($input->getArgument('target'));
$options = $this->targets->options($t);
if ($input->getOption('backend') !== null || $input->getOption('auth') !== null) {
$options = []; // a different connection type has its own options
}
foreach ($input->getOption('option') as $pair) {
if (!str_contains($pair, '=')) {
$output->writeln("<error>Option '$pair' must be key=value</error>");
return 1;
}
$k = (string)strstr($pair, '=', true);
$v = substr($pair, strlen($k) + 1);
$options[$k] = match (strtolower($v)) { 'true' => true, 'false' => false, default => $v };
}
$before = $t->getName();
$t = $this->targets->update($t, $input->getOption('name') ?? $t->getName(), $input->getOption('backend') ?? $t->getBackend(),
$input->getOption('auth') ?? $t->getAuth(), $options, $input->getOption('path') ?? $t->getBasePath());
} catch (\Throwable $e) {
$output->writeln('<error>' . $e->getMessage() . '</error>');
return 1;
}
$this->alerts->securityEvent('target_changed', ['target' => $before]);
$output->writeln(sprintf('Location <info>%s</info> updated; repository %s found with the new settings.', $t->getName(), (string)$t->getRepositoryId()));
return 0;
}
}
+22
View File
@@ -181,6 +181,28 @@ final class AdminApiController extends Controller {
return new JSONResponse(['id' => $r['target']->getId(), 'created' => $r['created']]);
}
/** The stored settings for the edit form; password values are left out (null). */
#[PasswordConfirmationRequired]
public function targetSettings(int $id): JSONResponse {
$t = $this->targets->get((string)$id);
return new JSONResponse(['name' => $t->getName(), 'backend' => $t->getBackend(), 'auth' => $t->getAuth(),
'options' => $this->targets->editableOptions($t), 'path' => $t->getBasePath()]);
}
/** Options with a null value keep their stored value. */
#[PasswordConfirmationRequired]
public function updateTarget(int $id, string $name, string $backend, string $auth, array $options = [], string $path = 'ng_backup'): JSONResponse {
try {
$t = $this->targets->get((string)$id);
$before = $t->getName();
$t = $this->targets->update($t, $name, $backend, $auth, $options, $path);
} catch (\Throwable $e) {
return new JSONResponse(['error' => $e->getMessage()], Http::STATUS_BAD_REQUEST);
}
$this->alerts->securityEvent('target_changed', ['target' => $before]);
return new JSONResponse(['id' => $t->getId()]);
}
/** Size limit in GiB; null or 0 = unlimited. */
#[PasswordConfirmationRequired]
public function setTargetLimit(int $id, ?float $maxGb = null): JSONResponse {
+1
View File
@@ -59,6 +59,7 @@ final class Notifier implements INotifier {
$notification->setParsedSubject(match ($p['event'] ?? '') {
'kit_downloaded' => $l->t('%s downloaded the backup recovery kit', [$who]),
'target_removed' => $l->t('%1$s removed backup location %2$s', [$who, $p['target'] ?? '']),
'target_changed' => $l->t('%1$s changed the connection settings of backup location %2$s', [$who, $p['target'] ?? '']),
'append_only_off' => $l->t('%1$s turned off append-only for backup location %2$s', [$who, $p['target'] ?? '']),
'retention_changed' => $l->t('%1$s changed the backup retention to %2$s', [$who, $p['policy'] ?? '']),
'snapshots_forgotten' => $l->t('%1$s moved %2$s snapshots of %3$s to the trash; they are deleted permanently after %4$s unless restored', [$who, $p['count'] ?? '', $p['target'] ?? '', $p['until'] ?? '']),
+102 -17
View File
@@ -37,7 +37,7 @@ final class TargetService {
) {
}
/** @return list<array{id:string, name:string, auth:list<string>, parameters:array}> */
/** @return list<array{id:string, name:string, auth:list<array{id:string, name:string, parameters:array}>, parameters:array}> */
public function availableBackends(): array {
return $this->factory()->describeBackends();
}
@@ -50,20 +50,8 @@ final class TargetService {
* @return array{target: Target, created: bool}
*/
public function add(string $name, string $backend, string $auth, array $options, string $basePath): array {
if (!preg_match('/^[A-Za-z0-9][A-Za-z0-9 _.-]{0,63}$/', $name)) {
throw new \InvalidArgumentException('Name: letters, digits, space, _ . - (max 64)');
}
// Without the native extension files_external falls back to the smbclient binary, which
// registers fine but cannot seek: the first backup then fails with "Seek failed". Refuse
// up front instead of letting the admin find out from a failed run.
if ($backend === 'smb' && !extension_loaded('smbclient')) {
throw new \InvalidArgumentException('The SMB backend needs the PHP smbclient extension (pecl install smbclient); the smbclient binary alone cannot seek and every backup would fail');
}
try {
$this->mapper->findByName($name);
throw new \InvalidArgumentException("A target named '$name' already exists");
} catch (\OCP\AppFramework\Db\DoesNotExistException) {
}
$this->validateName($name);
$this->requireUsableBackend($backend);
$keys = $this->keys->keyRing();
$storageBackend = $this->buildBackend($backend, $auth, $options, $basePath);
if ($storageBackend->exists('config')) {
@@ -97,6 +85,103 @@ final class TargetService {
return ['target' => $target, 'created' => $created];
}
/**
* Change how an existing location is reached (credentials, host, folder, even the connection
* type) or its name. The new settings must lead to the same repository: the history stays
* and a typo cannot silently start a new, empty repository. A null option value keeps the
* stored one, so the web interface never has to send stored passwords back.
*
* @param array<string, mixed> $options
*/
public function update(Target $target, string $name, string $backend, string $auth, array $options, string $basePath): Target {
$this->validateName($name, $target->getId());
$this->requireUsableBackend($backend);
$stored = $this->options($target);
foreach ($options as $key => $value) {
if ($value === null) {
if (array_key_exists($key, $stored)) {
$options[$key] = $stored[$key];
} else {
unset($options[$key]);
}
}
}
// Do not create the folder: a typo in the path must not leave an empty folder behind.
$storageBackend = $this->buildBackend($backend, $auth, $options, $basePath, false);
if (!$storageBackend->exists('config')) {
if (!$storageBackend->canConnect()) {
throw new \InvalidArgumentException('Cannot connect with these settings. Check the host, user name and password');
}
throw new \InvalidArgumentException("No NG Backup repository found in folder '" . trim($basePath, '/') . "'. To start an empty repository, add a new location instead");
}
$repo = Repository::openWithKey($storageBackend, $this->keys->keyRing(), $this->indexCache, $this->anchor);
if ($target->getRepositoryId() !== null && $repo->id() !== $target->getRepositoryId()) {
throw new \InvalidArgumentException('These settings lead to a different repository than the one of this location');
}
$target->setName($name);
$target->setBackend($backend);
$target->setAuth($auth);
$target->setOptions($this->crypto->encrypt(json_encode($options, JSON_THROW_ON_ERROR)));
$target->setBasePath(trim($basePath, '/'));
return $this->mapper->update($target);
}
/**
* The stored options for an edit form: password-type values are replaced by null, which
* update() reads as "keep the stored value".
*
* @return array<string, mixed>
*/
public function editableOptions(Target $target): array {
$secret = [];
foreach ($this->availableBackends() as $b) {
if ($b['id'] !== $target->getBackend()) {
continue;
}
$params = $b['parameters'];
foreach ($b['auth'] as $a) {
if ($a['id'] === $target->getAuth()) {
$params += $a['parameters'];
}
}
foreach ($params as $key => $p) {
if (($p['type'] ?? 0) === 2) {
$secret[$key] = true;
}
}
}
$options = $this->options($target);
foreach ($options as $key => $value) {
if (isset($secret[$key])) {
$options[$key] = null;
}
}
return $options;
}
private function validateName(string $name, ?int $ownId = null): void {
if (!preg_match('/^[A-Za-z0-9][A-Za-z0-9 _.-]{0,63}$/', $name)) {
throw new \InvalidArgumentException('Name: letters, digits, space, _ . - (max 64)');
}
try {
$other = $this->mapper->findByName($name);
} catch (DoesNotExistException) {
return;
}
if ($other->getId() !== $ownId) {
throw new \InvalidArgumentException("A target named '$name' already exists");
}
}
private function requireUsableBackend(string $backend): void {
// Without the native extension files_external falls back to the smbclient binary, which
// registers fine but cannot seek: the first backup then fails with "Seek failed". Refuse
// up front instead of letting the admin find out from a failed run.
if ($backend === 'smb' && !extension_loaded('smbclient')) {
throw new \InvalidArgumentException('The SMB backend needs the PHP smbclient extension (pecl install smbclient); the smbclient binary alone cannot seek and every backup would fail');
}
}
/**
* A target's options are encrypted with Nextcloud's own instance secret (ICrypto), not
* ng_backup's key, so changing that secret (occ backup:restore:full merging config.php)
@@ -193,8 +278,8 @@ final class TargetService {
return json_decode($this->crypto->decrypt($target->getOptions()), true, 512, JSON_THROW_ON_ERROR);
}
private function buildBackend(string $backend, string $auth, array $options, string $basePath): StorageBackend {
return new StorageBackend($this->factory()->create($backend, $auth, $options), $basePath);
private function buildBackend(string $backend, string $auth, array $options, string $basePath, bool $createBase = true): StorageBackend {
return new StorageBackend($this->factory()->create($backend, $auth, $options), $basePath, $createBase);
}
private ?ExternalStorageFactory $factory = null;