Phase 2: per-user backup and restore via user_migration
CI / unit (8.2) (push) Has been cancelled
CI / unit (8.3) (push) Has been cancelled
CI / unit (8.4) (push) Has been cancelled
CI / psalm (push) Has been cancelled
CI / integration (mariadb, 32) (push) Has been cancelled
CI / integration (mariadb, 33) (push) Has been cancelled
CI / integration (mariadb, 34) (push) Has been cancelled
CI / integration (mariadb, 35) (push) Has been cancelled
CI / integration (pgsql, 32) (push) Has been cancelled
CI / integration (pgsql, 33) (push) Has been cancelled
CI / integration (pgsql, 34) (push) Has been cancelled
CI / integration (pgsql, 35) (push) Has been cancelled
CI / integration (sqlite, 32) (push) Has been cancelled
CI / integration (sqlite, 33) (push) Has been cancelled
CI / integration (sqlite, 34) (push) Has been cancelled
CI / integration (sqlite, 35) (push) Has been cancelled
CI / integration-targets (push) Has been cancelled
CI / integration-user (push) Has been cancelled
CI / unit (8.2) (push) Has been cancelled
CI / unit (8.3) (push) Has been cancelled
CI / unit (8.4) (push) Has been cancelled
CI / psalm (push) Has been cancelled
CI / integration (mariadb, 32) (push) Has been cancelled
CI / integration (mariadb, 33) (push) Has been cancelled
CI / integration (mariadb, 34) (push) Has been cancelled
CI / integration (mariadb, 35) (push) Has been cancelled
CI / integration (pgsql, 32) (push) Has been cancelled
CI / integration (pgsql, 33) (push) Has been cancelled
CI / integration (pgsql, 34) (push) Has been cancelled
CI / integration (pgsql, 35) (push) Has been cancelled
CI / integration (sqlite, 32) (push) Has been cancelled
CI / integration (sqlite, 33) (push) Has been cancelled
CI / integration (sqlite, 34) (push) Has been cancelled
CI / integration (sqlite, 35) (push) Has been cancelled
CI / integration-targets (push) Has been cancelled
CI / integration-user (push) Has been cancelled
Wires the existing RepositoryExportDestination/RepositoryImportSource (see lib/UserMigration/) into user_migration's own UserMigrationService, resolved on demand through a new UserMigrationServiceLocator since the app is an optional dependency (PLAN.md 3.8) -- without it, only this feature is unavailable, the full backup still works. New: UserBackupService::backupUser() (occ backup:backup:user) streams a user_migration export straight into the repository, under the same shared location lock as a regular backup. UserRestoreService::restoreUser() (occ backup:restore:user) never imports over a live account: --mode=replace safety-exports the current state first, then deletes and recreates the account from the backup under the same uid; --mode=as-backup restores into a new "<uid>-bak" account instead, since Nextcloud cannot rename a uid. The catalog now also tracks user export manifests (Repository::recordUserExport/ listUserExports/forgetUserExport/isRecordedUserExport), rollback-protected like a snapshot: listUserExports() verifies each one is still present on the location, and restore only ever reads a manifest the verified catalog actually recorded, never an arbitrary caller-supplied path. prune()'s existing raw users/ scan for blob retention is unchanged. A new short exclusive "catalog" lock (PruneService::catalogLockKey()) serializes the catalog write itself, so two concurrent per-user backups (or one alongside a full backup) can no longer race to write the same next generation and silently drop one of them -- the broader per-target shared lock only ever protected against a concurrent cleanup, not against other shared holders. Two real bugs found by testing this against a live Nextcloud instance, not by unit tests (RepositoryExportDestination/RepositoryImportSource implement OCP interfaces that plain PHPUnit can't load, see tests/Unit/Repository/UserExportTest.php's docblock; the new tests/integration/user-roundtrip.sh covers the real round trip instead): - RepositoryImportSource::copyToFolder() double-closed the stream it handed to putContent()/newFile() (both already close it themselves), which throws a TypeError on PHP 8 and broke every restored file with content. - A replace-mode restore failed on NotPermittedException: IRootFolder::getUserFolder() caches a Folder per uid for the process lifetime, so after delete+recreate of the same uid the import still got the deleted account's (now storage-less) folder. Fixed by dispatching FilesystemTornDownEvent, which Root already listens for to clear exactly that cache -- no internal OC\ classes needed. Local review (CodeRabbit + Codex) against this diff found three more real issues, all fixed: the catalog write race above, a failed as-backup import leaving a broken stub account behind (now deleted on failure), and an export stream leak on a read/backend failure in RepositoryExportDestination::copyFolder() (now closed in a finally). Deliberately left out: wiring per-user export into the automatic backup schedule (on-demand via occ only, for now) and retention/pruning of old user exports -- both open design questions, not yet decided. The lease covering a single export/import still has a fixed TTL (3600s) with no mid-call refresh, since UserMigrationService exposes no progress; a resumable, step-based per-user backup (like the main BackupService run) would remove that cap but is a bigger change than this pass.
This commit is contained in:
@@ -0,0 +1,109 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace OCA\NgBackup\Tests\Unit\Repository;
|
||||
|
||||
use OCA\NgBackup\Backend\LocalBackend;
|
||||
use OCA\NgBackup\Crypto\KeyRing;
|
||||
use OCA\NgBackup\Repository\ICatalogAnchor;
|
||||
use OCA\NgBackup\Repository\Repository;
|
||||
use OCA\NgBackup\Repository\RepositoryException;
|
||||
use OCA\NgBackup\Tests\Unit\TempDirTrait;
|
||||
use PHPUnit\Framework\TestCase;
|
||||
|
||||
/**
|
||||
* Repository::recordUserExport/listUserExports/forgetUserExport: the catalog side of a
|
||||
* user_migration export (see RepositoryExportDestination/RepositoryImportSource, which this
|
||||
* does not exercise directly since they implement OCP\UserMigration interfaces not available to
|
||||
* plain PHPUnit; that round trip is covered by tests/integration/user-roundtrip.sh instead).
|
||||
*/
|
||||
class UserExportTest extends TestCase {
|
||||
use TempDirTrait;
|
||||
|
||||
private LocalBackend $backend;
|
||||
private KeyRing $keys;
|
||||
private ICatalogAnchor $anchor;
|
||||
private string $root;
|
||||
|
||||
protected function setUp(): void {
|
||||
$this->root = $this->tempDir();
|
||||
$this->backend = new LocalBackend($this->root);
|
||||
$this->keys = KeyRing::generate();
|
||||
$this->anchor = new class implements ICatalogAnchor {
|
||||
public array $data = [];
|
||||
public function get(string $r): ?array { return $this->data[$r] ?? null; }
|
||||
public function set(string $r, int $g, string $h): void { $this->data[$r] = ['gen' => $g, 'hash' => $h]; }
|
||||
};
|
||||
}
|
||||
|
||||
private function open(): Repository {
|
||||
return Repository::openWithKey($this->backend, $this->keys, null, $this->anchor);
|
||||
}
|
||||
|
||||
/** Stands in for an export destination's close(): a manifest object plus a catalog record. */
|
||||
private function putManifest(Repository $repo, string $path): void {
|
||||
$repo->putObject($path, json_encode(['uid' => 'alice']));
|
||||
$repo->recordUserExport($path);
|
||||
}
|
||||
|
||||
public function testRecordAndListSurviveAReopen(): void {
|
||||
$repo = Repository::initWithKey($this->backend, $this->keys, $this->keys->wrap('pw'), $this->anchor);
|
||||
$this->putManifest($repo, 'users/alice/20261005T120000Z-aaaa0001');
|
||||
|
||||
$again = $this->open();
|
||||
$this->assertSame(['users/alice/20261005T120000Z-aaaa0001'], $again->listUserExports());
|
||||
$this->assertSame(['users/alice/20261005T120000Z-aaaa0001'], $again->listUserExports('alice'));
|
||||
$this->assertSame([], $again->listUserExports('bob'));
|
||||
}
|
||||
|
||||
public function testListIsNewestFirst(): 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->assertSame(
|
||||
['users/alice/20261006T120000Z-aaaa0002', 'users/alice/20261005T120000Z-aaaa0001'],
|
||||
$repo->listUserExports('alice'),
|
||||
);
|
||||
}
|
||||
|
||||
public function testForgetRemovesItFromTheCatalogAndLocation(): void {
|
||||
$repo = Repository::initWithKey($this->backend, $this->keys, $this->keys->wrap('pw'), $this->anchor);
|
||||
$path = 'users/alice/20261005T120000Z-aaaa0001';
|
||||
$this->putManifest($repo, $path);
|
||||
|
||||
$repo->forgetUserExport($path);
|
||||
|
||||
$this->assertSame([], $repo->listUserExports());
|
||||
$this->assertFalse($this->backend->exists($path));
|
||||
}
|
||||
|
||||
public function testForgetRejectsAPathNotInTheCatalog(): void {
|
||||
$repo = Repository::initWithKey($this->backend, $this->keys, $this->keys->wrap('pw'), $this->anchor);
|
||||
$this->expectException(RepositoryException::class);
|
||||
$repo->forgetUserExport('users/nobody/not-a-real-export');
|
||||
}
|
||||
|
||||
public function testIsRecordedUserExport(): void {
|
||||
$repo = Repository::initWithKey($this->backend, $this->keys, $this->keys->wrap('pw'), $this->anchor);
|
||||
$path = 'users/alice/20261005T120000Z-aaaa0001';
|
||||
$this->putManifest($repo, $path);
|
||||
|
||||
$this->assertTrue($repo->isRecordedUserExport($path));
|
||||
$this->assertFalse($repo->isRecordedUserExport('users/alice/not-the-real-one'));
|
||||
}
|
||||
|
||||
public function testListingDetectsAManifestRemovedBehindItsBack(): void {
|
||||
// A catalog-listed export whose object disappeared from the location (deleted or
|
||||
// hidden outside NG Backup) must be detected, the same as a missing snapshot.
|
||||
$repo = Repository::initWithKey($this->backend, $this->keys, $this->keys->wrap('pw'), $this->anchor);
|
||||
$path = 'users/alice/20261005T120000Z-aaaa0001';
|
||||
$this->putManifest($repo, $path);
|
||||
$this->backend->delete($path);
|
||||
|
||||
$this->expectException(RepositoryException::class);
|
||||
$this->expectExceptionMessage('missing on the location');
|
||||
$repo->listUserExports();
|
||||
}
|
||||
}
|
||||
Executable
+59
@@ -0,0 +1,59 @@
|
||||
#!/bin/bash
|
||||
# Per-user backup/restore round trip via user_migration (optional dependency, see PLAN.md 3.8):
|
||||
# occ backup:backup:user / occ backup:restore:user --mode=as-backup|replace.
|
||||
# Usage: user-roundtrip.sh "<command prefix to run occ/php as the web user>" <datadir> [target-args]
|
||||
# e.g. user-roundtrip.sh "docker exec -u www-data ngb-nc35-pgsql" /var/www/html/data
|
||||
set -euo pipefail
|
||||
X="$1"; DATA="$2"
|
||||
TARGET_ARGS="${3:-local -a null::null -o datadir=/tmp/ngb-urt-target}"
|
||||
occ() { $X php occ "$@"; }
|
||||
fail() { echo "FAIL: $*"; exit 1; }
|
||||
ok() { echo "ok $*"; }
|
||||
U=ngbutest-$RANDOM
|
||||
|
||||
occ app:enable ng_backup >/dev/null
|
||||
if ! occ app:install user_migration >/dev/null 2>&1 && ! occ app:enable user_migration >/dev/null 2>&1; then
|
||||
echo "SKIP: user_migration is not available (no network, or the app store is unreachable) -- per-user backup/restore not tested"
|
||||
exit 0
|
||||
fi
|
||||
sleep 8 # APCu app config cache
|
||||
|
||||
$X sh -c "printf 'user roundtrip passphrase 2026\n' > /tmp/ngb-urt-pass && mkdir -p /tmp/ngb-urt-target"
|
||||
occ backup:key:init --passphrase-file=/tmp/ngb-urt-pass --label=CI --delete-delay-days=7 >/dev/null || fail "key init"
|
||||
CODE=$(occ backup:key:kit | php -r 'echo json_decode(stream_get_contents(STDIN), true)["confirmation_code"];')
|
||||
occ backup:key:confirm -u admin --code="$CODE" --confirm="Yes, I confirm" >/dev/null || fail "confirm"
|
||||
ok "key, kit code and confirmation"
|
||||
|
||||
occ backup:target:add ci $TARGET_ARGS --path=repo >/dev/null || fail "target add"
|
||||
ok "location added"
|
||||
|
||||
$X sh -c "OC_PASS='User-roundtrip-pw-2026' php occ user:add --password-from-env $U >/dev/null"
|
||||
$X sh -c "mkdir -p $DATA/$U/files/Docs && for i in 1 2 3; do head -c 100000 /dev/urandom > $DATA/$U/files/Docs/f\$i.bin; done"
|
||||
occ files:scan "$U" >/dev/null
|
||||
SUM_BEFORE=$($X sh -c "cd $DATA/$U/files && find . -type f -print0 | sort -z | xargs -0 md5sum | md5sum")
|
||||
|
||||
MANIFEST=$(occ backup:backup:user ci "$U" | sed -n 's/.* to \(.*\)\.$/\1/p')
|
||||
[ -n "$MANIFEST" ] || fail "backup:backup:user printed no manifest path"
|
||||
ok "backed up $U to $MANIFEST"
|
||||
|
||||
# as-backup: restore into <uid>-bak; the original account must stay untouched.
|
||||
occ backup:restore:user ci "$MANIFEST" --mode=as-backup >/dev/null || fail "restore as-backup"
|
||||
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 <uid>-bak account's files differ from the original"
|
||||
occ user:delete "$U-bak" >/dev/null
|
||||
$X sh -c "[ -d '$DATA/$U/files/Docs' ]" || fail "the original account should be untouched by an as-backup restore"
|
||||
ok "as-backup restores into <uid>-bak, byte-identical, original account untouched"
|
||||
|
||||
# replace: damage the account, then restore it in place under the same uid.
|
||||
$X sh -c "rm '$DATA/$U/files/Docs/f1.bin' && echo damaged > '$DATA/$U/files/Docs/f1.bin'"
|
||||
occ files:scan "$U" >/dev/null
|
||||
occ backup:restore:user ci "$MANIFEST" --mode=replace >/dev/null || fail "restore replace"
|
||||
occ files:scan "$U" >/dev/null
|
||||
SUM_AFTER=$($X sh -c "cd $DATA/$U/files && find . -type f -print0 | sort -z | xargs -0 md5sum | md5sum")
|
||||
[ "$SUM_BEFORE" = "$SUM_AFTER" ] || fail "files differ after a replace restore"
|
||||
ok "replace restores the original content under the same uid, byte-identical"
|
||||
|
||||
occ user:delete "$U" >/dev/null
|
||||
$X sh -c "rm -rf /tmp/ngb-urt-target /tmp/ngb-urt-pass"
|
||||
echo "USER ROUNDTRIP PASS"
|
||||
Reference in New Issue
Block a user