From f89fe1ebc3e0a1dacee05268c879c63a6729b220 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Thu, 16 Apr 2026 19:10:51 +0200 Subject: [PATCH] fix(sync): prevent Dropbox setup loop when auth is cancelled (#7131) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit save() previously persisted isEnabled:true and closed the dialog before auth completed, then called sync(). When the user cancelled the auth code dialog, sync would fail with MissingCredentialsSPError, showing a snack whose "Configure" action reopened the dialog — the infinite loop reported in #7131. Run auth first, then gate save+close on provider.isReady() when the provider has getAuthHelper. If auth was cancelled or failed, the dialog stays open and config is left untouched. --- .../dialog-sync-initial-cfg.component.spec.ts | 226 ++++++++++++++++++ .../dialog-sync-initial-cfg.component.ts | 11 +- 2 files changed, 236 insertions(+), 1 deletion(-) create mode 100644 src/app/imex/sync/dialog-sync-initial-cfg/dialog-sync-initial-cfg.component.spec.ts diff --git a/src/app/imex/sync/dialog-sync-initial-cfg/dialog-sync-initial-cfg.component.spec.ts b/src/app/imex/sync/dialog-sync-initial-cfg/dialog-sync-initial-cfg.component.spec.ts new file mode 100644 index 0000000000..4fee149487 --- /dev/null +++ b/src/app/imex/sync/dialog-sync-initial-cfg/dialog-sync-initial-cfg.component.spec.ts @@ -0,0 +1,226 @@ +import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { provideNoopAnimations } from '@angular/platform-browser/animations'; +import { MatDialogRef } from '@angular/material/dialog'; +import { TranslateModule } from '@ngx-translate/core'; +import { FormlyModule } from '@ngx-formly/core'; +import { of } from 'rxjs'; +import { DialogSyncInitialCfgComponent } from './dialog-sync-initial-cfg.component'; +import { SyncConfigService } from '../sync-config.service'; +import { SyncWrapperService } from '../sync-wrapper.service'; +import { SyncProviderManager } from '../../../op-log/sync-providers/provider-manager.service'; +import { GlobalConfigService } from '../../../features/config/global-config.service'; +import { SyncProviderId } from '../../../op-log/sync-providers/provider.const'; +import { SyncConfig } from '../../../features/config/global-config.model'; + +describe('DialogSyncInitialCfgComponent', () => { + let component: DialogSyncInitialCfgComponent; + let fixture: ComponentFixture; + let mockDialogRef: jasmine.SpyObj>; + let mockSyncConfigService: jasmine.SpyObj; + let mockSyncWrapperService: jasmine.SpyObj; + let mockProviderManager: jasmine.SpyObj; + let mockGlobalConfigService: jasmine.SpyObj; + + const baseSyncConfig: SyncConfig = { + isEnabled: false, + syncProvider: null, + syncInterval: 300000, + encryptKey: '', + isEncryptionEnabled: false, + localFileSync: {} as any, + webDav: {} as any, + nextcloud: {} as any, + superSync: {} as any, + } as SyncConfig; + + beforeEach(async () => { + mockDialogRef = jasmine.createSpyObj('MatDialogRef', ['close']); + mockSyncConfigService = jasmine.createSpyObj('SyncConfigService', [ + 'updateSettingsFromForm', + ]); + (mockSyncConfigService as any).syncSettingsForm$ = of(baseSyncConfig); + mockSyncConfigService.updateSettingsFromForm.and.resolveTo(); + + mockSyncWrapperService = jasmine.createSpyObj('SyncWrapperService', [ + 'configuredAuthForSyncProviderIfNecessary', + 'sync', + ]); + mockSyncWrapperService.sync.and.resolveTo(); + + mockProviderManager = jasmine.createSpyObj('SyncProviderManager', [ + 'getProviderById', + ]); + + mockGlobalConfigService = jasmine.createSpyObj('GlobalConfigService', [], { + sync$: of(baseSyncConfig), + }); + + TestBed.configureTestingModule({ + imports: [ + DialogSyncInitialCfgComponent, + TranslateModule.forRoot(), + FormlyModule.forRoot(), + ], + providers: [ + provideNoopAnimations(), + { provide: MatDialogRef, useValue: mockDialogRef }, + { provide: SyncConfigService, useValue: mockSyncConfigService }, + { provide: SyncWrapperService, useValue: mockSyncWrapperService }, + { provide: SyncProviderManager, useValue: mockProviderManager }, + { provide: GlobalConfigService, useValue: mockGlobalConfigService }, + ], + }); + // Replace the Formly-based template with a minimal placeholder so we can + // test the save() business logic without registering every Formly field type. + TestBed.overrideComponent(DialogSyncInitialCfgComponent, { + set: { template: '' }, + }); + await TestBed.compileComponents(); + + fixture = TestBed.createComponent(DialogSyncInitialCfgComponent); + component = fixture.componentInstance; + }); + + describe('save() — auth cancelled (reproduces issue #7131)', () => { + it('should NOT close dialog when Dropbox auth is cancelled and provider is not ready', async () => { + const providerNeedingAuth = { + id: SyncProviderId.Dropbox, + getAuthHelper: () => Promise.resolve({} as any), + isReady: jasmine.createSpy('isReady').and.resolveTo(false), + }; + mockProviderManager.getProviderById.and.resolveTo(providerNeedingAuth as any); + mockSyncWrapperService.configuredAuthForSyncProviderIfNecessary.and.resolveTo({ + wasConfigured: false, + }); + + (component as any)._tmpUpdatedCfg = { + ...(component as any)._tmpUpdatedCfg, + syncProvider: SyncProviderId.Dropbox, + isEnabled: true, + }; + + await component.save(); + + expect(mockDialogRef.close).not.toHaveBeenCalled(); + }); + + it('should NOT save config when Dropbox auth is cancelled and provider is not ready', async () => { + const providerNeedingAuth = { + id: SyncProviderId.Dropbox, + getAuthHelper: () => Promise.resolve({} as any), + isReady: jasmine.createSpy('isReady').and.resolveTo(false), + }; + mockProviderManager.getProviderById.and.resolveTo(providerNeedingAuth as any); + mockSyncWrapperService.configuredAuthForSyncProviderIfNecessary.and.resolveTo({ + wasConfigured: false, + }); + + (component as any)._tmpUpdatedCfg = { + ...(component as any)._tmpUpdatedCfg, + syncProvider: SyncProviderId.Dropbox, + isEnabled: true, + }; + + await component.save(); + + expect(mockSyncConfigService.updateSettingsFromForm).not.toHaveBeenCalled(); + }); + + it('should NOT trigger sync when auth is cancelled', async () => { + const providerNeedingAuth = { + id: SyncProviderId.Dropbox, + getAuthHelper: () => Promise.resolve({} as any), + isReady: jasmine.createSpy('isReady').and.resolveTo(false), + }; + mockProviderManager.getProviderById.and.resolveTo(providerNeedingAuth as any); + mockSyncWrapperService.configuredAuthForSyncProviderIfNecessary.and.resolveTo({ + wasConfigured: false, + }); + + (component as any)._tmpUpdatedCfg = { + ...(component as any)._tmpUpdatedCfg, + syncProvider: SyncProviderId.Dropbox, + isEnabled: true, + }; + + await component.save(); + + expect(mockSyncWrapperService.sync).not.toHaveBeenCalled(); + }); + }); + + describe('save() — auth succeeds', () => { + it('should close dialog and save config when Dropbox auth succeeds', async () => { + const configuredProvider = { + id: SyncProviderId.Dropbox, + getAuthHelper: () => Promise.resolve({} as any), + isReady: jasmine.createSpy('isReady').and.resolveTo(true), + }; + mockProviderManager.getProviderById.and.resolveTo(configuredProvider as any); + mockSyncWrapperService.configuredAuthForSyncProviderIfNecessary.and.resolveTo({ + wasConfigured: true, + }); + + (component as any)._tmpUpdatedCfg = { + ...(component as any)._tmpUpdatedCfg, + syncProvider: SyncProviderId.Dropbox, + isEnabled: true, + }; + + await component.save(); + + expect(mockSyncConfigService.updateSettingsFromForm).toHaveBeenCalled(); + expect(mockDialogRef.close).toHaveBeenCalled(); + }); + }); + + describe('save() — provider without auth requirement', () => { + it('should close dialog and save config for WebDAV (no getAuthHelper)', async () => { + const webdavProvider = { + id: SyncProviderId.WebDAV, + // no getAuthHelper + isReady: jasmine.createSpy('isReady').and.resolveTo(true), + }; + mockProviderManager.getProviderById.and.resolveTo(webdavProvider as any); + mockSyncWrapperService.configuredAuthForSyncProviderIfNecessary.and.resolveTo({ + wasConfigured: false, + }); + + (component as any)._tmpUpdatedCfg = { + ...(component as any)._tmpUpdatedCfg, + syncProvider: SyncProviderId.WebDAV, + isEnabled: true, + }; + + await component.save(); + + expect(mockSyncConfigService.updateSettingsFromForm).toHaveBeenCalled(); + expect(mockDialogRef.close).toHaveBeenCalled(); + }); + }); + + describe('save() — already configured provider', () => { + it('should close dialog when Dropbox is already configured (wasConfigured=false, isReady=true)', async () => { + const alreadyConfigured = { + id: SyncProviderId.Dropbox, + getAuthHelper: () => Promise.resolve({} as any), + isReady: jasmine.createSpy('isReady').and.resolveTo(true), + }; + mockProviderManager.getProviderById.and.resolveTo(alreadyConfigured as any); + mockSyncWrapperService.configuredAuthForSyncProviderIfNecessary.and.resolveTo({ + wasConfigured: false, + }); + + (component as any)._tmpUpdatedCfg = { + ...(component as any)._tmpUpdatedCfg, + syncProvider: SyncProviderId.Dropbox, + isEnabled: true, + }; + + await component.save(); + + expect(mockSyncConfigService.updateSettingsFromForm).toHaveBeenCalled(); + expect(mockDialogRef.close).toHaveBeenCalled(); + }); + }); +}); diff --git a/src/app/imex/sync/dialog-sync-initial-cfg/dialog-sync-initial-cfg.component.ts b/src/app/imex/sync/dialog-sync-initial-cfg/dialog-sync-initial-cfg.component.ts index aa9b8c271b..3d974e0971 100644 --- a/src/app/imex/sync/dialog-sync-initial-cfg/dialog-sync-initial-cfg.component.ts +++ b/src/app/imex/sync/dialog-sync-initial-cfg/dialog-sync-initial-cfg.component.ts @@ -219,12 +219,21 @@ export class DialogSyncInitialCfgComponent implements AfterViewInit { isEnabled: this._tmpUpdatedCfg.isEnabled || !this.isWasEnabled(), }; - await this.syncConfigService.updateSettingsFromForm(configToSave as SyncConfig, true); const providerId = toSyncProviderId(this._tmpUpdatedCfg.syncProvider); if (providerId && this._tmpUpdatedCfg.isEnabled) { await this.syncWrapperService.configuredAuthForSyncProviderIfNecessary(providerId); + + // If the provider requires auth (e.g. Dropbox) and is still not ready, + // the auth dialog was cancelled or failed. Keep the dialog open so the + // user can retry, and do not persist isEnabled:true with missing credentials + // (which would trigger the "Sync credentials are missing" snack loop — issue #7131). + const provider = await this._providerManager.getProviderById(providerId); + if (provider?.getAuthHelper && !(await provider.isReady())) { + return; + } } + await this.syncConfigService.updateSettingsFromForm(configToSave as SyncConfig, true); this._matDialogRef.close(); if (isOnline()) {