diff --git a/CHANGELOG.md b/CHANGELOG.md index 32be50d..c424573 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,20 @@ the app store accepts a `-beta` suffix, but Nextcloud servers then only offer th beta/daily update channel, which would hide the app from most admins. The stability stage is instead carried by the app description, the git tag, the release, and this file. +## [Unreleased] + +### Added +- `occ backup:user:list [location] [user]` shows the user exports on a location, with the manifest + path `backup:user:restore` expects. Before, that path was only printed once by + `backup:user:backup` (#22). + +### Changed +- `occ backup:user:restore` also takes a user id instead of the manifest path, when that user has + exactly one export on the location. With more, and for a manifest path that is not on the + location, the error lists that user's exports; for a user without exports it names the + expected form (#22). Before, `backup:user:restore ` only said "is not a + recorded user export for this location". + ## [1.0.0-rc.6] - 2026-10-24 Sixth release candidate for 1.0.0; the app version is 0.9.12. Fixes from two disaster recovery diff --git a/PLAN.md b/PLAN.md index f21b62b..f3d0abb 100644 --- a/PLAN.md +++ b/PLAN.md @@ -443,6 +443,11 @@ in README "Known limitations": 9. **Tijdzone in `occ backup:status`**: de tabel toont de servertijd (UTC), de webinterface de tijd van de beheerder; dat leest als een verkeerde starttijd. Zet de zone in de kolomkop ("started (UTC)") of toon de tijden in de zone van het schema. + 10. **User-restore op gebruikersnaam** (issue #22, 2026-10-27): `backup:user:restore + New_User` gaf alleen "is not a recorded user export"; het argument moet het manifestpad + zijn, dat alleen `backup:user:backup` eenmalig toonde. Nu: `backup:user:list` toont de + exports, een kale uid werkt bij precies één export, anders een lijst om uit te kiezen — + **klaar** (2026-10-27). - **DR-tests over databasegrenzen** (besluit 2026-10-20; productie nc01 draait PostgreSQL): na de MariaDB→PostgreSQL-fout (tellers, rc5) elke overstap in de integratietest (`to-pgsql`, `to-mysql`, `to-sqlite`, ook in CI). Volle DR-tests met echte data: vóór 1.0.0 de S3-simulatie diff --git a/README.md b/README.md index a01bdfa..8a578db 100644 --- a/README.md +++ b/README.md @@ -334,4 +334,4 @@ On the backlog; none of these block the beta or 1.0.0, but know them before you | `backup:verify` | checksum audit of a restore point (`--deep` downloads and decrypts everything) | | `backup:restore:full` | whole-server restore onto a fresh installation (after `backup:key:import`) | | `backup:key:import` | import a recovery kit on a fresh installation | -| `backup:user:backup`, `backup:user:restore` | one user via user_migration (asks before installing it) | +| `backup:user:backup`, `backup:user:list`, `backup:user:restore` | one user via user_migration (asks before installing it); list the exports | diff --git a/appinfo/info.xml b/appinfo/info.xml index 79b83ce..d89cd53 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -92,6 +92,7 @@ you need help with restoring a backup after a crash.]]> OCA\NgBackup\Command\KeySlotRemove OCA\NgBackup\Command\BackupUser OCA\NgBackup\Command\RestoreUser + OCA\NgBackup\Command\ListUserExports OCA\NgBackup\Command\KeyImport OCA\NgBackup\Command\RestoreFull OCA\NgBackup\Command\Verify diff --git a/docs/HOWTO.md b/docs/HOWTO.md index afccaec..3ff6136 100644 --- a/docs/HOWTO.md +++ b/docs/HOWTO.md @@ -114,10 +114,16 @@ Needs the `user_migration` app. NG Backup asks before installing it: ``` occ backup:user:backup offsite +occ backup:user:list offsite [] # the exports and their manifest occ backup:user:restore offsite users// # replace (default) occ backup:user:restore offsite users// --mode=as-backup ``` +- `backup:user:restore` needs the manifest path that `backup:user:backup` prints at the end + (`users//`); `backup:user:list` shows it again. A user id alone also works when + that user has exactly one export on the location. With more (a `replace` restore adds a safety + export), the command lists them and you pass one. + - If `user_migration` is missing, the command asks to install and enable it. For non-interactive use, add `--install-user-migration` to approve that explicitly. The command stops after the installation; run it once more. diff --git a/lib/Command/ListUserExports.php b/lib/Command/ListUserExports.php new file mode 100644 index 0000000..4f62270 --- /dev/null +++ b/lib/Command/ListUserExports.php @@ -0,0 +1,64 @@ +setName('backup:user:list') + ->setDescription('List the user exports (backup:user:backup) on a location') + ->addArgument('target', InputArgument::OPTIONAL, 'Location name or id (default: all)') + ->addArgument('user', InputArgument::OPTIONAL, 'Only the exports of this user id') + ->setHelp(<<<'HELP' +The manifest column is what occ backup:user:restore expects. A safety export made by a restore +with --mode=replace is listed here as well. + +Example: + occ backup:user:list offsite alice +HELP); + } + + protected function execute(InputInterface $input, OutputInterface $output): int { + $name = $input->getArgument('target'); + $uid = $input->getArgument('user'); + $rows = []; + try { + $targets = $name === null ? $this->targets->list() : [$this->targets->get($name)]; + foreach ($targets as $t) { + foreach ($this->targets->repository($t)->listUserExports($uid) as $path) { + $parts = explode('/', $path, 3); + $user = $parts[1] ?? ''; + $exportId = $parts[2] ?? ''; + $time = \DateTimeImmutable::createFromFormat('Ymd\THis\Z', substr($exportId, 0, 16), new \DateTimeZone('UTC')); + $rows[] = [$t->getName(), $user, $time !== false ? $time->format('Y-m-d H:i') : '', $path]; + } + } + } catch (\Throwable $e) { + $output->writeln('' . $e->getMessage() . ''); + return 1; + } + if ($rows === []) { + $output->writeln('No user exports yet (occ backup:user:backup).'); + return 0; + } + (new Table($output))->setHeaders(['location', 'user', 'date (UTC)', 'manifest'])->setRows($rows)->render(); + return 0; + } +} diff --git a/lib/Command/RestoreUser.php b/lib/Command/RestoreUser.php index 6fa415e..98eb843 100644 --- a/lib/Command/RestoreUser.php +++ b/lib/Command/RestoreUser.php @@ -33,7 +33,7 @@ final class RestoreUser extends Command { $this->setName('backup:user:restore') ->setDescription('Restore a user from a user_migration export (see backup:user:backup)') ->addArgument('target', InputArgument::REQUIRED, 'Location name or id') - ->addArgument('manifest', InputArgument::REQUIRED, 'Manifest path printed by backup:user:backup (users//)') + ->addArgument('manifest', InputArgument::REQUIRED, 'Manifest path printed by backup:user:backup (users//), or a user id with exactly one export') ->addOption('mode', 'm', InputOption::VALUE_REQUIRED, 'replace or as-backup', UserRestoreService::MODE_REPLACE) ->addOption('force', null, InputOption::VALUE_NONE, 'Replace an existing account without asking (required when not interactive)') ->addOption('install-user-migration', null, InputOption::VALUE_NONE, 'Install and enable user_migration if it is missing (non-interactive consent)') @@ -45,16 +45,31 @@ Never imports over a live account. Modes: compare and transfer by hand; Nextcloud cannot rename a user id, so the backup gets the new name, never the existing account +occ backup:user:list shows the exports on a location. A user id alone works when that user +has exactly one export there; with more, the command lists them and you pass one. + Example: occ backup:user:restore offsite users/alice/20261005T120000Z-ab12cd34 + occ backup:user:restore offsite alice occ backup:user:restore offsite users/alice/20261005T120000Z-ab12cd34 --mode=as-backup HELP); } protected function execute(InputInterface $input, OutputInterface $output): int { $mode = (string)$input->getOption('mode'); - $uid = explode('/', (string)$input->getArgument('manifest'))[1] ?? ''; - if ($mode === UserRestoreService::MODE_REPLACE && $uid !== '' && $this->userManager->userExists($uid) && !$input->getOption('force')) { + try { + $target = $this->targets->get($input->getArgument('target')); + $ref = (string)$input->getArgument('manifest'); + $manifest = $this->targets->repository($target)->resolveUserExport($ref); + } catch (\Throwable $e) { + $output->writeln('' . $e->getMessage() . ''); + return 1; + } + if ($manifest !== $ref) { + $output->writeln("Using $manifest."); + } + $uid = explode('/', $manifest)[1]; + if ($mode === UserRestoreService::MODE_REPLACE && $this->userManager->userExists($uid) && !$input->getOption('force')) { // Replace deletes the live account (after a safety export). Shares to others and data of // apps without a user_migration migrator do not come back, so never do it unasked. if (!$input->isInteractive()) { @@ -69,9 +84,8 @@ HELP); } try { $output->writeln('Note: on restore, user_migration only imports the app settings on its own allowlist (e.g. calendar view and reminder settings); other app settings of this user are skipped. Reason: NG Backup uses user_migration\'s importer; widening the list would mean building a settings importer of our own.'); - $target = $this->targets->get($input->getArgument('target')); $this->installer->ensureEnabled($input, $output); - $user = $this->userRestore->restoreUser($target, (string)$input->getArgument('manifest'), $mode); + $user = $this->userRestore->restoreUser($target, $manifest, $mode); } catch (\Throwable $e) { $output->writeln('' . $e->getMessage() . ''); return 1; diff --git a/lib/Repository/Repository.php b/lib/Repository/Repository.php index 56a837f..6d547ca 100644 --- a/lib/Repository/Repository.php +++ b/lib/Repository/Repository.php @@ -561,6 +561,31 @@ final class Repository { return isset($this->catalog->userExports[$manifestPath]); } + /** + * The recorded export $ref names: a manifest path (users//) as is, or a bare + * user id (also "users/") when that user has exactly one export here. Anything else + * throws, with the exports to choose from in the message. + */ + public function resolveUserExport(string $ref): string { + if ($this->isRecordedUserExport($ref)) { + return $ref; + } + $rest = trim($ref, '/'); + if (str_starts_with($rest, 'users/')) { + $rest = substr($rest, strlen('users/')); + } + $uid = explode('/', $rest)[0]; + $exports = $uid !== '' ? $this->listUserExports($uid) : []; + if (!str_contains($rest, '/') && count($exports) === 1) { + return $exports[0]; + } + if ($exports === []) { + throw new RepositoryException("$ref is not a recorded user export for this location, and $uid has no exports here. Expected users//; occ backup:user:list shows the exports"); + } + $which = str_contains($rest, '/') ? "$ref is not a recorded user export for this location" : "$uid has " . count($exports) . ' exports here'; + throw new RepositoryException("$which; pass one of these (newest first):\n " . implode("\n ", $exports)); + } + /** Permanently delete a user export. Its blobs are freed by the next prune() like any other. */ public function forgetUserExport(string $manifestPath): void { if (!isset($this->catalog->userExports[$manifestPath])) { diff --git a/tests/Unit/Repository/UserExportTest.php b/tests/Unit/Repository/UserExportTest.php index 8750dc6..18e6970 100644 --- a/tests/Unit/Repository/UserExportTest.php +++ b/tests/Unit/Repository/UserExportTest.php @@ -106,4 +106,51 @@ class UserExportTest extends TestCase { $this->expectExceptionMessage('missing on the location'); $repo->listUserExports(); } + + public function testResolveTakesARecordedPathAsIs(): void { + $repo = Repository::initWithKey($this->backend, $this->keys, $this->keys->wrap('pw'), $this->anchor); + $path = 'users/alice/20261005T120000Z-aaaa0001'; + $this->putManifest($repo, $path); + + $this->assertSame($path, $repo->resolveUserExport($path)); + } + + public function testResolveTakesAUserIdWithOneExport(): void { + // Issue #22: "backup:user:restore New_User" was rejected as not a recorded export. + $repo = Repository::initWithKey($this->backend, $this->keys, $this->keys->wrap('pw'), $this->anchor); + $path = 'users/alice/20261005T120000Z-aaaa0001'; + $this->putManifest($repo, $path); + $this->putManifest($repo, 'users/alice2/20261005T120000Z-aaaa0002'); + + $this->assertSame($path, $repo->resolveUserExport('alice')); + $this->assertSame($path, $repo->resolveUserExport('users/alice/')); + } + + public function testResolveListsTheExportsOfAUserWithMore(): void { + $repo = Repository::initWithKey($this->backend, $this->keys, $this->keys->wrap('pw'), $this->anchor); + $this->putManifest($repo, 'users/alice/20261005T120000Z-aaaa0001'); + $this->putManifest($repo, 'users/alice/20261006T120000Z-aaaa0002'); + + $this->expectException(RepositoryException::class); + $this->expectExceptionMessage("alice has 2 exports here; pass one of these (newest first):\n users/alice/20261006T120000Z-aaaa0002\n users/alice/20261005T120000Z-aaaa0001"); + $repo->resolveUserExport('alice'); + } + + public function testResolveListsTheExportsOfTheUserOfAWrongPath(): void { + $repo = Repository::initWithKey($this->backend, $this->keys, $this->keys->wrap('pw'), $this->anchor); + $this->putManifest($repo, 'users/alice/20261005T120000Z-aaaa0001'); + + $this->expectException(RepositoryException::class); + $this->expectExceptionMessage("users/alice/typo is not a recorded user export for this location; pass one of these (newest first):\n users/alice/20261005T120000Z-aaaa0001"); + $repo->resolveUserExport('users/alice/typo'); + } + + public function testResolveNamesTheExpectedFormForAUserWithoutExports(): void { + $repo = Repository::initWithKey($this->backend, $this->keys, $this->keys->wrap('pw'), $this->anchor); + $this->putManifest($repo, 'users/alice/20261005T120000Z-aaaa0001'); + + $this->expectException(RepositoryException::class); + $this->expectExceptionMessage('bob has no exports here. Expected users//; occ backup:user:list shows the exports'); + $repo->resolveUserExport('bob'); + } } diff --git a/tests/integration/user-roundtrip.sh b/tests/integration/user-roundtrip.sh index 175b4b1..67bc48c 100755 --- a/tests/integration/user-roundtrip.sh +++ b/tests/integration/user-roundtrip.sh @@ -36,8 +36,13 @@ MANIFEST=$(occ backup:user:backup ci "$U" | sed -n 's/.* to \(.*\)\.$/\1/p') [ -n "$MANIFEST" ] || fail "backup:user:backup printed no manifest path" ok "backed up $U to $MANIFEST" -# as-backup: restore into -bak; the original account must stay untouched. -occ backup:user:restore ci "$MANIFEST" --mode=as-backup >/dev/null || fail "restore as-backup" +occ backup:user:list ci "$U" | grep -qF "$MANIFEST" || fail "backup:user:list does not show $MANIFEST" +ok "backup:user:list shows the export" + +# as-backup: restore into -bak; the original account must stay untouched. $U has one export, +# so the bare user id names it (#22). +OUT=$(occ backup:user:restore ci "$U" --mode=as-backup) || fail "restore as-backup by user id" +echo "$OUT" | grep -qF "Using $MANIFEST." || fail "restore by user id did not say which export it used" occ files:scan "$U-bak" >/dev/null SUM_BAK=$($X sh -c "cd $DATA/$U-bak/files && find . -type f -print0 | sort -z | xargs -0 md5sum | md5sum") [ "$SUM_BEFORE" = "$SUM_BAK" ] || fail "the -bak account's files differ from the original" @@ -55,6 +60,11 @@ SUM_AFTER=$($X sh -c "cd $DATA/$U/files && find . -type f -print0 | sort -z | xa [ "$SUM_BEFORE" = "$SUM_AFTER" ] || fail "files differ after a replace restore" ok "replace restores the original content under the same uid, byte-identical" +# The replace above added a safety export: a bare user id is now ambiguous and must list both. +if OUT=$(occ backup:user:restore ci "$U" --mode=as-backup 2>&1); then fail "restore by an ambiguous user id ran"; fi +echo "$OUT" | grep -qF "$U has 2 exports here" && echo "$OUT" | grep -qF "$MANIFEST" || fail "an ambiguous user id did not list the exports" +ok "an ambiguous user id lists the exports instead of picking one" + # Retention: $U now has the manual export above plus the automatic safety-export the replace # restore made of it just before deleting it (4 total after these two more), then --user-last=2 # must forget exactly the 2 oldest.