fix(database restores): don't assume onboarding has completed (#27052)

e2e/src/specs/maintenance/server/database-backups.e2e-spec.ts

@@ -10,7 +10,9 @@ describe('/admin/database-backups', () => {
 
   beforeAll(async () => {
     await utils.resetDatabase();
-    admin = await utils.adminSetup();
+    admin = await utils.adminSetup({
+      onboarding: false,
+    });
     await utils.resetBackups(admin.accessToken);
   });
 
@@ -94,7 +96,9 @@ describe('/admin/database-backups', () => {
         ({ status, body }) => status === 200 && !body.maintenanceMode,
       );
 
-      admin = await utils.adminSetup();
+      admin = await utils.adminSetup({
+        onboarding: false,
+      });
     });
 
     it.sequential('should not work when the server is configured', async () => {

server/src/app.common.ts

@@ -4,7 +4,7 @@ import compression from 'compression';
 import cookieParser from 'cookie-parser';
 import { existsSync } from 'node:fs';
 import sirv from 'sirv';
-import { excludePaths, serverVersion } from 'src/constants';
+import { IMMICH_SERVER_START, excludePaths, serverVersion } from 'src/constants';
 import { MaintenanceWorkerService } from 'src/maintenance/maintenance-worker.service';
 import { WebSocketAdapter } from 'src/middleware/websocket.adapter';
 import { ConfigRepository } from 'src/repositories/config.repository';
@@ -83,5 +83,5 @@ export async function configureExpress(
   const server = await (host ? app.listen(port, host) : app.listen(port));
   server.requestTimeout = 24 * 60 * 60 * 1000;
 
-  logger.log(`Immich Server is listening on ${await app.getUrl()} [v${serverVersion}] [${environment}] `);
+  logger.log(`${IMMICH_SERVER_START} on ${await app.getUrl()} [v${serverVersion}] [${environment}] `);
 }

server/src/app.module.ts

@@ -29,6 +29,7 @@ import { ProcessRepository } from 'src/repositories/process.repository';
 import { StorageRepository } from 'src/repositories/storage.repository';
 import { SystemMetadataRepository } from 'src/repositories/system-metadata.repository';
 import { teardownTelemetry, TelemetryRepository } from 'src/repositories/telemetry.repository';
+import { UserRepository } from 'src/repositories/user.repository';
 import { WebsocketRepository } from 'src/repositories/websocket.repository';
 import { services } from 'src/services';
 import { AuthService } from 'src/services/auth.service';
@@ -111,6 +112,7 @@ export class ApiModule extends BaseModule {}
     StorageRepository,
     ProcessRepository,
     DatabaseRepository,
+    UserRepository,
     SystemMetadataRepository,
     AppRepository,
     MaintenanceHealthRepository,

server/src/constants.ts

@@ -4,6 +4,8 @@ import { dirname, join } from 'node:path';
 import { SemVer } from 'semver';
 import { ApiTag, AudioCodec, DatabaseExtension, ExifOrientation, VectorIndex } from 'src/enum';
 
+export const IMMICH_SERVER_START = 'Immich Server is listening';
+
 export const ErrorMessages = {
   InconsistentMediaLocation:
     'Detected an inconsistent media location. For more information, see https://docs.immich.app/errors#inconsistent-media-location',

server/src/maintenance/maintenance-health.repository.ts

@@ -1,6 +1,7 @@
 import { Injectable } from '@nestjs/common';
 import { fork } from 'node:child_process';
 import { dirname, join } from 'node:path';
+import { IMMICH_SERVER_START } from 'src/constants';
 
 @Injectable()
 export class MaintenanceHealthRepository {
@@ -20,45 +21,27 @@ export class MaintenanceHealthRepository {
         stdio: ['ignore', 'pipe', 'ignore', 'ipc'],
       });
 
-      async function checkHealth() {
+      let output = '';
-        try {
-          const response = await fetch('http://127.0.0.1:33001/api/server/config');
-          const { isOnboarded } = await response.json();
-          if (isOnboarded) {
-            resolve();
-          } else {
-            reject(new Error('Server health check failed, no admin exists.'));
-          }
-        } catch (error) {
-          reject(error);
-        } finally {
-          if (worker.exitCode === null) {
-            worker.kill('SIGTERM');
-          }
-        }
-      }
-
-      let output = '',
-        alive = false;
 
       worker.stdout?.on('data', (data) => {
-        if (alive) {
+        if (worker.exitCode !== null) {
           return;
         }
 
         output += data;
 
-        if (output.includes('Immich Server is listening')) {
+        if (output.includes(IMMICH_SERVER_START)) {
-          alive = true;
+          resolve();
-          void checkHealth();
+          worker.kill('SIGTERM');
         }
       });
 
-      worker.on('exit', reject);
+      worker.on('exit', (code, signal) => reject(`Server health check failed, server exited with ${signal ?? code}`));
-      worker.on('error', reject);
+      worker.on('error', (error) => reject(`Server health check failed, process threw: ${error}`));
 
       setTimeout(() => {
         if (worker.exitCode === null) {
+          reject('Server health check failed, took too long to start.');
           worker.kill('SIGTERM');
         }
       }, 20_000);

server/src/services/database-backup.service.spec.ts

@@ -27,6 +27,7 @@ describe(DatabaseBackupService.name, () => {
       mocks.systemMetadata as never,
       mocks.process,
       mocks.database as never,
+      mocks.user as never,
       mocks.cron as never,
       mocks.job as never,
       maintenanceHealthRepositoryMock as never,
@@ -187,6 +188,7 @@ describe(DatabaseBackupService.name, () => {
         mocks.systemMetadata as never,
         mocks.process,
         mocks.database as never,
+        mocks.user as never,
         mocks.cron as never,
         mocks.job as never,
         void 0 as never,
@@ -400,6 +402,7 @@ describe(DatabaseBackupService.name, () => {
           mocks.systemMetadata as never,
           mocks.process,
           mocks.database as never,
+          mocks.user as never,
           mocks.cron as never,
           mocks.job as never,
           void 0 as never,
@@ -474,6 +477,7 @@ describe(DatabaseBackupService.name, () => {
           mocks.systemMetadata as never,
           mocks.process,
           mocks.database as never,
+          mocks.user as never,
           mocks.cron as never,
           mocks.job as never,
           void 0 as never,
@@ -536,6 +540,7 @@ describe(DatabaseBackupService.name, () => {
           mocks.systemMetadata as never,
           mocks.process,
           mocks.database as never,
+          mocks.user as never,
           mocks.cron as never,
           mocks.job as never,
           void 0 as never,
@@ -663,6 +668,7 @@ describe(DatabaseBackupService.name, () => {
         mocks.systemMetadata as never,
         mocks.process,
         mocks.database as never,
+        mocks.user as never,
         mocks.cron as never,
         mocks.job as never,
         maintenanceHealthRepositoryMock,
@@ -678,6 +684,8 @@ describe(DatabaseBackupService.name, () => {
     it('should successfully restore a backup', async () => {
       let writtenToPsql = '';
 
+      mocks.user.hasAdmin.mockResolvedValue(true);
+
       mocks.process.spawnDuplexStream.mockImplementationOnce(() => mockDuplex()('command', 0, 'data', ''));
       mocks.process.spawnDuplexStream.mockImplementationOnce(() => mockDuplex()('command', 0, 'data', ''));
       mocks.process.spawnDuplexStream.mockImplementationOnce(() => {
@@ -740,6 +748,8 @@ describe(DatabaseBackupService.name, () => {
     it('should generate pg_dumpall specific SQL instructions', async () => {
       let writtenToPsql = '';
 
+      mocks.user.hasAdmin.mockResolvedValue(true);
+
       mocks.process.spawnDuplexStream.mockImplementationOnce(() => mockDuplex()('command', 0, 'data', ''));
       mocks.process.spawnDuplexStream.mockImplementationOnce(() => mockDuplex()('command', 0, 'data', ''));
       mocks.process.spawnDuplexStream.mockImplementationOnce(() => {
@@ -834,7 +844,24 @@ describe(DatabaseBackupService.name, () => {
       expect(mocks.process.spawnDuplexStream).toHaveBeenCalledTimes(4);
     });
 
+    it('should rollback if there is no admin user', async () => {
+      mocks.user.hasAdmin.mockResolvedValue(false);
+
+      const progress = vitest.fn();
+      await expect(
+        sut.restoreDatabaseBackup('development-filename.sql', progress),
+      ).rejects.toThrowErrorMatchingInlineSnapshot(`[Error: Server health check failed, no admin exists.]`);
+
+      expect(progress).toHaveBeenCalledWith('backup', 0.05);
+      expect(progress).toHaveBeenCalledWith('migrations', 0.9);
+      expect(progress).toHaveBeenCalledWith('rollback', 0);
+
+      expect(mocks.user.hasAdmin).toHaveBeenCalled();
+      expect(mocks.process.spawnDuplexStream).toHaveBeenCalledTimes(4);
+    });
+
     it('should rollback if API healthcheck fails', async () => {
+      mocks.user.hasAdmin.mockResolvedValue(true);
       maintenanceHealthRepositoryMock.checkApiHealth.mockRejectedValue(new Error('Health Error'));
 
       const progress = vitest.fn();
@@ -846,6 +873,7 @@ describe(DatabaseBackupService.name, () => {
       expect(progress).toHaveBeenCalledWith('migrations', 0.9);
       expect(progress).toHaveBeenCalledWith('rollback', 0);
 
+      expect(mocks.user.hasAdmin).toHaveBeenCalled();
       expect(maintenanceHealthRepositoryMock.checkApiHealth).toHaveBeenCalled();
       expect(mocks.process.spawnDuplexStream).toHaveBeenCalledTimes(4);
     });

server/src/services/database-backup.service.ts

@@ -20,6 +20,7 @@ import { LoggingRepository } from 'src/repositories/logging.repository';
 import { ProcessRepository } from 'src/repositories/process.repository';
 import { StorageRepository } from 'src/repositories/storage.repository';
 import { SystemMetadataRepository } from 'src/repositories/system-metadata.repository';
+import { UserRepository } from 'src/repositories/user.repository';
 import { getConfig } from 'src/utils/config';
 import {
   findDatabaseBackupVersion,
@@ -40,6 +41,7 @@ export class DatabaseBackupService {
     private readonly systemMetadataRepository: SystemMetadataRepository,
     private readonly processRepository: ProcessRepository,
     private readonly databaseRepository: DatabaseRepository,
+    private readonly userRepository: UserRepository,
     @Optional()
     private readonly cronRepository: CronRepository,
     @Optional()
@@ -405,7 +407,14 @@ export class DatabaseBackupService {
 
       try {
         progressCb?.('migrations', 0.9);
+
         await this.databaseRepository.runMigrations();
+
+        const hasAdmin = await this.userRepository.hasAdmin();
+        if (!hasAdmin) {
+          throw new Error('Server health check failed, no admin exists.');
+        }
+
         await this.maintenanceHealthRepository.checkApiHealth();
       } catch (error) {
         progressCb?.('rollback', 0);