mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-07-18 00:46:45 +00:00
fix(sync): prevent ImmediateUploadService from triggering during encryption ops
forceUpload() blocks sync() via _isEncryptionOperationInProgress$, but ImmediateUploadService only checked isSyncInProgress. This allowed it to trigger during password changes and run server migration checks that download/decrypt old data - causing DecryptError after password change. - Expose isEncryptionOperationInProgress getter in SyncWrapperService - Check the flag in ImmediateUploadService._canUpload() - Add unit test for the new guard
This commit is contained in:
parent
ba449487a7
commit
4acc7d6ca8
3 changed files with 68 additions and 19 deletions
|
|
@ -134,6 +134,14 @@ export class SyncWrapperService {
|
|||
return this._isSyncInProgress$.getValue();
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns true if an encryption operation (password change, enable/disable) is in progress.
|
||||
* Used by ImmediateUploadService to avoid uploading during critical encryption operations.
|
||||
*/
|
||||
get isEncryptionOperationInProgress(): boolean {
|
||||
return this._isEncryptionOperationInProgress$.getValue();
|
||||
}
|
||||
|
||||
// Expose shared user input wait state for other services (e.g., SyncTriggerService)
|
||||
isWaitingForUserInput$ = this._userInputWaitState.isWaitingForUserInput$;
|
||||
|
||||
|
|
@ -468,27 +476,34 @@ export class SyncWrapperService {
|
|||
|
||||
SyncLog.log('SyncWrapperService: forceUpload called - uploading local state');
|
||||
|
||||
try {
|
||||
const rawProvider = this._providerManager.getActiveProvider();
|
||||
const syncCapableProvider =
|
||||
await this._wrappedProvider.getOperationSyncCapable(rawProvider);
|
||||
// Block parallel syncs during force upload to prevent them from trying to
|
||||
// download/decrypt old data with a potentially different encryption key.
|
||||
// This is critical when forceUpload is triggered after password change.
|
||||
await this.runWithSyncBlocked(async () => {
|
||||
try {
|
||||
const rawProvider = this._providerManager.getActiveProvider();
|
||||
const syncCapableProvider =
|
||||
await this._wrappedProvider.getOperationSyncCapable(rawProvider);
|
||||
|
||||
if (!syncCapableProvider) {
|
||||
SyncLog.warn('SyncWrapperService: Cannot force upload - provider not available');
|
||||
return;
|
||||
if (!syncCapableProvider) {
|
||||
SyncLog.warn(
|
||||
'SyncWrapperService: Cannot force upload - provider not available',
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
await this._opLogSyncService.forceUploadLocalState(syncCapableProvider);
|
||||
this._providerManager.setSyncStatus('IN_SYNC');
|
||||
SyncLog.log('SyncWrapperService: Force upload complete');
|
||||
} catch (error) {
|
||||
SyncLog.err('SyncWrapperService: Force upload failed:', error);
|
||||
const errStr = getSyncErrorStr(error);
|
||||
this._snackService.open({
|
||||
msg: errStr,
|
||||
type: 'ERROR',
|
||||
});
|
||||
}
|
||||
|
||||
await this._opLogSyncService.forceUploadLocalState(syncCapableProvider);
|
||||
this._providerManager.setSyncStatus('IN_SYNC');
|
||||
SyncLog.log('SyncWrapperService: Force upload complete');
|
||||
} catch (error) {
|
||||
SyncLog.err('SyncWrapperService: Force upload failed:', error);
|
||||
const errStr = getSyncErrorStr(error);
|
||||
this._snackService.open({
|
||||
msg: errStr,
|
||||
type: 'ERROR',
|
||||
});
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
async configuredAuthForSyncProviderIfNecessary(
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import { OperationLogSyncService } from './operation-log-sync.service';
|
|||
import { ActionType, Operation, OpType } from '../core/operation.types';
|
||||
import { SyncProviderId } from '../sync-providers/provider.const';
|
||||
import { DataInitStateService } from '../../core/data-init/data-init-state.service';
|
||||
import { SyncWrapperService } from '../../imex/sync/sync-wrapper.service';
|
||||
import { BehaviorSubject } from 'rxjs';
|
||||
|
||||
describe('ImmediateUploadService', () => {
|
||||
|
|
@ -12,6 +13,7 @@ describe('ImmediateUploadService', () => {
|
|||
let mockProviderManager: jasmine.SpyObj<SyncProviderManager>;
|
||||
let mockSyncService: jasmine.SpyObj<OperationLogSyncService>;
|
||||
let mockDataInitStateService: { isAllDataLoadedInitially$: BehaviorSubject<boolean> };
|
||||
let mockSyncWrapperService: { isEncryptionOperationInProgress: boolean };
|
||||
let mockProvider: any;
|
||||
|
||||
const createMockOp = (id: string): Operation => ({
|
||||
|
|
@ -59,12 +61,18 @@ describe('ImmediateUploadService', () => {
|
|||
isAllDataLoadedInitially$: new BehaviorSubject<boolean>(false),
|
||||
};
|
||||
|
||||
// Mock SyncWrapperService - default to no encryption operation in progress
|
||||
mockSyncWrapperService = {
|
||||
isEncryptionOperationInProgress: false,
|
||||
};
|
||||
|
||||
TestBed.configureTestingModule({
|
||||
providers: [
|
||||
ImmediateUploadService,
|
||||
{ provide: SyncProviderManager, useValue: mockProviderManager },
|
||||
{ provide: OperationLogSyncService, useValue: mockSyncService },
|
||||
{ provide: DataInitStateService, useValue: mockDataInitStateService },
|
||||
{ provide: SyncWrapperService, useValue: mockSyncWrapperService },
|
||||
],
|
||||
});
|
||||
|
||||
|
|
@ -189,6 +197,25 @@ describe('ImmediateUploadService', () => {
|
|||
expect(mockSyncService.uploadPendingOps).not.toHaveBeenCalled();
|
||||
}));
|
||||
|
||||
it('should skip upload when encryption operation is in progress', fakeAsync(() => {
|
||||
// Set encryption operation in progress (e.g., password change)
|
||||
mockSyncWrapperService.isEncryptionOperationInProgress = true;
|
||||
mockSyncService.uploadPendingOps.and.returnValue(
|
||||
Promise.resolve({
|
||||
uploadedCount: 1,
|
||||
rejectedCount: 0,
|
||||
piggybackedOps: [],
|
||||
rejectedOps: [],
|
||||
}),
|
||||
);
|
||||
|
||||
service.initialize();
|
||||
service.trigger();
|
||||
tick(2100);
|
||||
|
||||
expect(mockSyncService.uploadPendingOps).not.toHaveBeenCalled();
|
||||
}));
|
||||
|
||||
it('should handle fresh client (syncService returns null)', fakeAsync(() => {
|
||||
// Fresh client handling is now done inside syncService.uploadPendingOps()
|
||||
// which returns null for fresh clients
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ import { isFileBasedProvider, isOperationSyncCapable } from './operation-sync.ut
|
|||
import { OpLog } from '../../core/log';
|
||||
import { DataInitStateService } from '../../core/data-init/data-init-state.service';
|
||||
import { handleStorageQuotaError } from './sync-error-utils';
|
||||
import { SyncWrapperService } from '../../imex/sync/sync-wrapper.service';
|
||||
|
||||
const IMMEDIATE_UPLOAD_DEBOUNCE_MS = 2000;
|
||||
|
||||
|
|
@ -49,6 +50,7 @@ export class ImmediateUploadService implements OnDestroy {
|
|||
private _providerManager = inject(SyncProviderManager);
|
||||
private _syncService = inject(OperationLogSyncService);
|
||||
private _dataInitStateService = inject(DataInitStateService);
|
||||
private _syncWrapper = inject(SyncWrapperService);
|
||||
|
||||
private _uploadTrigger$ = new Subject<void>();
|
||||
private _subscription: Subscription | null = null;
|
||||
|
|
@ -123,6 +125,11 @@ export class ImmediateUploadService implements OnDestroy {
|
|||
return false;
|
||||
}
|
||||
|
||||
// Don't overlap with encryption operations (password change, enable/disable)
|
||||
if (this._syncWrapper.isEncryptionOperationInProgress) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Must have an active provider
|
||||
const provider = this._providerManager.getActiveProvider();
|
||||
if (!provider) {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue