diff --git a/package.json b/package.json index 9a18ac7c6..82ac06ba2 100644 --- a/package.json +++ b/package.json @@ -186,7 +186,7 @@ "@babel/preset-typescript": "7.16.0", "@chanzuckerberg/axe-storybook-testing": "3.0.2", "@electron/fuses": "1.5.0", - "@signalapp/mock-server": "1.0.1", + "@signalapp/mock-server": "1.1.0", "@storybook/addon-actions": "5.1.11", "@storybook/addon-knobs": "5.1.11", "@storybook/addons": "5.1.11", diff --git a/ts/services/storage.ts b/ts/services/storage.ts index 8e9fb5d25..ceb91b213 100644 --- a/ts/services/storage.ts +++ b/ts/services/storage.ts @@ -12,6 +12,7 @@ import { deriveStorageManifestKey, encryptProfile, decryptProfile, + deriveMasterKeyFromGroupV1, } from '../Crypto'; import { mergeAccountRecord, @@ -1046,26 +1047,86 @@ async function processRemoteRecords( `count=${missingKeys.size}` ); + const ITEM_TYPE = Proto.ManifestRecord.Identifier.Type; const droppedKeys = new Set(); - // Merge Account records last since it contains the pinned conversations - // and we need all other records merged first before we can find the pinned - // records in our db - const ITEM_TYPE = Proto.ManifestRecord.Identifier.Type; - const sortedStorageItems = decryptedStorageItems.sort((_, b) => - b.itemType === ITEM_TYPE.ACCOUNT ? -1 : 1 - ); + // Drop all GV1 records for which we have GV2 record in the same manifest + const masterKeys = new Map(); + for (const { itemType, storageID, storageRecord } of decryptedStorageItems) { + if (itemType === ITEM_TYPE.GROUPV2 && storageRecord.groupV2?.masterKey) { + masterKeys.set( + Bytes.toBase64(storageRecord.groupV2.masterKey), + storageID + ); + } + } + + let accountItem: MergeableItemType | undefined; + + const prunedStorageItems = decryptedStorageItems.filter(item => { + const { itemType, storageID, storageRecord } = item; + if (itemType === ITEM_TYPE.ACCOUNT) { + if (accountItem !== undefined) { + log.warn( + `storageService.process(${storageVersion}): duplicate account ` + + `record=${redactStorageID(storageID, storageVersion)} ` + + `previous=${redactStorageID(accountItem.storageID, storageVersion)}` + ); + droppedKeys.add(accountItem.storageID); + } + + accountItem = item; + return false; + } + + if (itemType !== ITEM_TYPE.GROUPV1 || !storageRecord.groupV1?.id) { + return true; + } + + const masterKey = deriveMasterKeyFromGroupV1(storageRecord.groupV1.id); + const gv2StorageID = masterKeys.get(Bytes.toBase64(masterKey)); + if (!gv2StorageID) { + return true; + } + + log.warn( + `storageService.process(${storageVersion}): dropping ` + + `GV1 record=${redactStorageID(storageID, storageVersion)} ` + + `GV2 record=${redactStorageID(gv2StorageID, storageVersion)} ` + + 'is in the same manifest' + ); + droppedKeys.add(storageID); + + return false; + }); try { log.info( `storageService.process(${storageVersion}): ` + - `attempting to merge records=${sortedStorageItems.length}` - ); - const mergedRecords = await pMap( - sortedStorageItems, - (item: MergeableItemType) => mergeRecord(storageVersion, item), - { concurrency: 5 } + `attempting to merge records=${prunedStorageItems.length}` ); + if (accountItem === undefined) { + log.warn(`storageService.process(${storageVersion}): no account record`); + } else { + log.info( + `storageService.process(${storageVersion}): account ` + + `record=${redactStorageID(accountItem.storageID, storageVersion)}` + ); + } + + const mergedRecords = [ + ...(await pMap( + prunedStorageItems, + (item: MergeableItemType) => mergeRecord(storageVersion, item), + { concurrency: 5 } + )), + + // Merge Account records last since it contains the pinned conversations + // and we need all other records merged first before we can find the pinned + // records in our db + ...(accountItem ? [await mergeRecord(storageVersion, accountItem)] : []), + ]; + log.info( `storageService.process(${storageVersion}): ` + `processed records=${mergedRecords.length}` @@ -1190,7 +1251,9 @@ async function sync( throw new Error('storageService.sync: Cannot start; no storage key!'); } - log.info('storageService.sync: starting...'); + log.info( + `storageService.sync: starting... ignoreConflicts=${ignoreConflicts}` + ); let manifest: Proto.ManifestRecord | undefined; try { @@ -1329,6 +1392,9 @@ async function upload(fromSync = false): Promise { false ); await uploadManifest(version, generatedManifest); + + // Clear pending delete keys after successful upload + await window.storage.put('storage-service-pending-deletes', []); } catch (err) { if (err.code === 409) { await sleep(conflictBackOff.getAndIncrement()); diff --git a/ts/test-mock/storage/drop_test.ts b/ts/test-mock/storage/drop_test.ts new file mode 100644 index 000000000..e1e6ff3ce --- /dev/null +++ b/ts/test-mock/storage/drop_test.ts @@ -0,0 +1,140 @@ +// Copyright 2022 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import { assert } from 'chai'; +import { Proto } from '@signalapp/mock-server'; + +import * as durations from '../../util/durations'; +import type { App, Bootstrap } from './fixtures'; +import { initStorage, debug } from './fixtures'; + +const IdentifierType = Proto.ManifestRecord.Identifier.Type; + +describe('storage service', function needsName() { + this.timeout(durations.MINUTE); + + let bootstrap: Bootstrap; + let app: App; + + beforeEach(async () => { + ({ bootstrap, app } = await initStorage()); + }); + + afterEach(async () => { + await app.close(); + await bootstrap.teardown(); + }); + + it('should drop gv1 record if there is a matching gv2 record', async () => { + const { phone } = bootstrap; + + debug('adding both records'); + { + const state = await phone.expectStorageState('consistency check'); + + const groupV1Id = Buffer.from('Wi9258rCEp7AdSdp+jCMlQ==', 'base64'); + const masterKey = Buffer.from( + '2+rdvzFGCOJI8POHcPNZHrYQWS/JXmT63R5OXKxhrPk=', + 'base64' + ); + + const updatedState = await phone.setStorageState( + state + .addRecord({ + type: IdentifierType.GROUPV1, + record: { + groupV1: { + id: groupV1Id, + }, + }, + }) + .addRecord({ + type: IdentifierType.GROUPV2, + record: { + groupV2: { + masterKey, + }, + }, + }) + ); + + debug('sending fetch storage'); + await phone.sendFetchStorage({ + timestamp: bootstrap.getTimestamp(), + }); + + debug('waiting for next storage state'); + const nextState = await phone.waitForStorageState({ + after: updatedState, + }); + + assert.isFalse( + nextState.hasRecord(({ type }) => { + return type === IdentifierType.GROUPV1; + }), + 'should not have gv1 record' + ); + + assert.isTrue( + nextState.hasRecord(({ type, record }) => { + if (type !== IdentifierType.GROUPV2) { + return false; + } + + if (!record.groupV2?.masterKey) { + return false; + } + return Buffer.from(masterKey).equals(record.groupV2.masterKey); + }), + 'should have gv2 record' + ); + } + }); + + it('should drop duplicate account record', async () => { + const { phone } = bootstrap; + + debug('duplicating account record'); + { + const state = await phone.expectStorageState('consistency check'); + + const oldAccount = state.findRecord(({ type }) => { + return type === IdentifierType.ACCOUNT; + }); + if (oldAccount === undefined) { + throw new Error('should have initial account record'); + } + + const updatedState = await phone.setStorageState( + state.addRecord({ + type: IdentifierType.ACCOUNT, + record: oldAccount.record, + }) + ); + + debug('sending fetch storage'); + await phone.sendFetchStorage({ + timestamp: bootstrap.getTimestamp(), + }); + + debug('waiting for next storage state'); + const nextState = await phone.waitForStorageState({ + after: updatedState, + }); + + assert.isFalse( + nextState.hasRecord(({ type, key }) => { + return type === IdentifierType.ACCOUNT && key.equals(oldAccount.key); + }), + 'should not have old account record' + ); + + assert.isTrue( + nextState.hasRecord(({ type }) => { + return type === IdentifierType.ACCOUNT; + }), + 'should have new account record' + ); + } + }); +}); diff --git a/yarn.lock b/yarn.lock index 6ab4b0ba7..cb1c3eadb 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1342,10 +1342,10 @@ "@react-spring/shared" "~9.4.0" "@react-spring/types" "~9.4.0" -"@signalapp/mock-server@1.0.1": - version "1.0.1" - resolved "https://registry.yarnpkg.com/@signalapp/mock-server/-/mock-server-1.0.1.tgz#ae461528ca18218cf34366d5afa1c672b0ddabe0" - integrity sha512-9XYIFZwwGnFEg/WSffn3KWOHHe/ooL44+UQ3cFX68jEtgOk575EeRZaTFge+XNxzciAbDuCtkWivYCODPBJISA== +"@signalapp/mock-server@1.1.0": + version "1.1.0" + resolved "https://registry.yarnpkg.com/@signalapp/mock-server/-/mock-server-1.1.0.tgz#1b6ca0f4c89fb2f0b803af06bb6571780edebf10" + integrity sha512-dsrxPhvbNM5kdaGzQSwLc2jgDNkNi93DvccWwSl8P28Wfir/1IVt+8/9alaTen0xr0UBbscfreZ2sJ8junNLjA== dependencies: "@signalapp/signal-client" "0.12.1" debug "^4.3.2"