From 3509f05d89e322830b91868cd675b30d455442de Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Fri, 26 Aug 2022 08:57:40 +0000 Subject: [PATCH 01/19] Removing keys if absent on EKM --- extension/chrome/settings/setup.ts | 5 +++ .../chrome/settings/setup/setup-create-key.ts | 2 +- .../chrome/settings/setup/setup-import-key.ts | 4 +- .../setup/setup-key-manager-autogen.ts | 4 +- .../settings/setup/setup-recover-key.ts | 2 +- extension/js/common/browser/browser-msg.ts | 3 +- extension/js/common/helpers.ts | 32 +++++++++------ .../js/common/platform/store/key-store.ts | 2 +- .../webmail/setup-webmail-content-script.ts | 3 +- test/source/tests/setup.ts | 39 +++++++------------ 10 files changed, 51 insertions(+), 45 deletions(-) diff --git a/extension/chrome/settings/setup.ts b/extension/chrome/settings/setup.ts index d9e5d6989a1..8e8495583f9 100644 --- a/extension/chrome/settings/setup.ts +++ b/extension/chrome/settings/setup.ts @@ -38,6 +38,11 @@ export interface PassphraseOptions { passphrase_save: boolean; } +export interface SaveKeysOptions { + keys_replace?: boolean; // replace existing keys in the storage + ppOptions?: PassphraseOptions; +} + export interface SetupOptions extends PassphraseOptions { submit_main: boolean; submit_all: boolean; diff --git a/extension/chrome/settings/setup/setup-create-key.ts b/extension/chrome/settings/setup/setup-create-key.ts index 60df2cb991d..9cfc4bfb7c3 100644 --- a/extension/chrome/settings/setup/setup-create-key.ts +++ b/extension/chrome/settings/setup/setup-create-key.ts @@ -82,7 +82,7 @@ export class SetupCreateKeyModule { const expireMonths = this.view.clientConfiguration.getEnforcedKeygenExpirationMonths(); const key = await OpenPGPKey.create(pgpUids, keyAlgo, options.passphrase, expireMonths); const prv = await KeyUtil.parse(key.private); - await saveKeysAndPassPhrase(this.view.acctEmail, [prv], options); + await saveKeysAndPassPhrase(this.view.acctEmail, [prv], { ppOptions: options }); return { id: prv.id, family: prv.family }; }; } diff --git a/extension/chrome/settings/setup/setup-import-key.ts b/extension/chrome/settings/setup/setup-import-key.ts index 0681c20ef8c..ba8cd647a24 100644 --- a/extension/chrome/settings/setup/setup-import-key.ts +++ b/extension/chrome/settings/setup/setup-import-key.ts @@ -40,7 +40,7 @@ export class SetupImportKeyModule { } } Xss.sanitizeRender('#step_2b_manual_enter .action_add_private_key', Ui.spinner('white')); - await saveKeysAndPassPhrase(this.view.acctEmail, [checked.encrypted], options); + await saveKeysAndPassPhrase(this.view.acctEmail, [checked.encrypted], { ppOptions: options }); await this.view.submitPublicKeys(options); await this.view.finalizeSetup(); await this.view.setupRender.renderSetupDone(); @@ -71,7 +71,7 @@ export class SetupImportKeyModule { this.view.setupRender.displayBlock('step_2b_manual_enter'); return; } - await saveKeysAndPassPhrase(this.view.acctEmail, [fixedPrv], options); + await saveKeysAndPassPhrase(this.view.acctEmail, [fixedPrv], { ppOptions: options }); await this.view.submitPublicKeys(options); await this.view.finalizeSetup(); await this.view.setupRender.renderSetupDone(); diff --git a/extension/chrome/settings/setup/setup-key-manager-autogen.ts b/extension/chrome/settings/setup/setup-key-manager-autogen.ts index 16cc72c5857..2940c9df99c 100644 --- a/extension/chrome/settings/setup/setup-key-manager-autogen.ts +++ b/extension/chrome/settings/setup/setup-key-manager-autogen.ts @@ -59,7 +59,7 @@ export class SetupWithEmailKeyManagerModule { await processAndStoreKeysFromEkmLocally({ acctEmail: this.view.acctEmail, decryptedPrivateKeys: privateKeys.map(entry => entry.decryptedPrivateKey), - ppOptions: setupOptions + saveKeysOptions: { ppOptions: setupOptions } }); } catch (e) { throw new Error(`Could not store keys from EKM due to error: ${e instanceof Error ? e.message : String(e)}`); @@ -102,7 +102,7 @@ export class SetupWithEmailKeyManagerModule { } const storePrvOnKm = async () => this.view.keyManager!.storePrivateKey(this.view.idToken!, KeyUtil.armor(decryptablePrv)); await Settings.retryUntilSuccessful(storePrvOnKm, 'Failed to store newly generated key on FlowCrypt Email Key Manager', Lang.general.contactIfNeedAssistance(this.view.isFesUsed())); - await saveKeysAndPassPhrase(this.view.acctEmail, [await KeyUtil.parse(generated.private)], setupOptions); // store encrypted key + pass phrase locally + await saveKeysAndPassPhrase(this.view.acctEmail, [await KeyUtil.parse(generated.private)], { ppOptions: setupOptions }); // store encrypted key + pass phrase locally }; } diff --git a/extension/chrome/settings/setup/setup-recover-key.ts b/extension/chrome/settings/setup/setup-recover-key.ts index 0480cd550ae..8ff0b51cffc 100644 --- a/extension/chrome/settings/setup/setup-recover-key.ts +++ b/extension/chrome/settings/setup/setup-recover-key.ts @@ -64,7 +64,7 @@ export class SetupRecoverKeyModule { passphrase_save: true, // todo - reevaluate saving passphrase when recovering recovered: true, }; - await saveKeysAndPassPhrase(this.view.acctEmail, newlyMatchingKeys, options); + await saveKeysAndPassPhrase(this.view.acctEmail, newlyMatchingKeys, { ppOptions: options }); const { setup_done } = await AcctStore.get(this.view.acctEmail, ['setup_done']); if (!setup_done) { // normal situation - fresh setup await this.view.submitPublicKeys(options); diff --git a/extension/js/common/browser/browser-msg.ts b/extension/js/common/browser/browser-msg.ts index dd8c9c67e65..35e5e10143b 100644 --- a/extension/js/common/browser/browser-msg.ts +++ b/extension/js/common/browser/browser-msg.ts @@ -19,6 +19,7 @@ import { GlobalStoreDict, GlobalIndex } from '../platform/store/global-store.js' import { AcctStoreDict, AccountIndex } from '../platform/store/acct-store.js'; import { saveFetchedPubkeysIfNewerThanInStorage } from '../shared.js'; import { ArmoredKeyIdentityWithEmails, KeyUtil } from '../core/crypto/key.js'; +import { SaveKeysOptions } from '../../../chrome/settings/setup.js'; export type GoogleAuthWindowResult$result = 'Success' | 'Denied' | 'Error' | 'Closed'; @@ -67,7 +68,7 @@ export namespace Bm { export type ShowAttachmentPreview = { iframeUrl: string }; export type ReRenderRecipient = { email: string }; export type SaveFetchedPubkeys = { email: string, pubkeys: string[] }; - export type ProcessAndStoreKeysFromEkmLocally = { acctEmail: string, decryptedPrivateKeys: string[] }; + export type ProcessAndStoreKeysFromEkmLocally = { acctEmail: string, decryptedPrivateKeys: string[], saveKeysOptions: SaveKeysOptions }; export namespace Res { export type GetActiveTabInfo = { provider: 'gmail' | undefined, acctEmail: string | undefined, sameWorld: boolean | undefined }; diff --git a/extension/js/common/helpers.ts b/extension/js/common/helpers.ts index b67fc651d50..47649f4d79d 100644 --- a/extension/js/common/helpers.ts +++ b/extension/js/common/helpers.ts @@ -3,7 +3,7 @@ 'use strict'; import { AcctStore } from './platform/store/acct-store.js'; -import { PassphraseOptions } from '../../chrome/settings/setup.js'; +import { SaveKeysOptions } from '../../chrome/settings/setup.js'; import { Buf } from './core/buf.js'; import { Key, KeyInfoWithIdentity, KeyUtil } from './core/crypto/key.js'; import { ClientConfiguration } from './client-configuration.js'; @@ -16,11 +16,18 @@ export const isFesUsed = async (acctEmail: string) => { return Boolean(fesUrl); }; -export const saveKeysAndPassPhrase = async (acctEmail: string, prvs: Key[], ppOptions?: PassphraseOptions) => { +export const saveKeysAndPassPhrase = async (acctEmail: string, prvs: Key[], { keys_replace, ppOptions }: SaveKeysOptions) => { const clientConfiguration = await ClientConfiguration.newInstance(acctEmail); + if (keys_replace) { + await KeyStore.set(acctEmail, await Promise.all(prvs.map(KeyUtil.keyInfoObj))); // todo: duplicate identities + // todo: should we delete passphrases matching the deleted keys? + } for (const prv of prvs) { - await KeyStore.add(acctEmail, prv); + if (!keys_replace) { + await KeyStore.add(acctEmail, prv); + } if (ppOptions !== undefined) { + // todo: perhaps it's easier just to store a set of passphrases without specifying longids? await PassphraseStore.set((ppOptions.passphrase_save && !clientConfiguration.forbidStoringPassPhrase()) ? 'local' : 'session', acctEmail, { longid: KeyUtil.getPrimaryLongid(prv) }, ppOptions.passphrase); } @@ -63,38 +70,41 @@ const parseAndCheckPrivateKeys = async (decryptedPrivateKeys: string[]) => { }; const filterKeysToSave = async (candidateKeys: Key[], existingKeys: KeyInfoWithIdentity[]) => { + // todo: check for uniqueness of candidateKeys identities here? if (!existingKeys.length) { - return candidateKeys; + return { keysToRetain: [], unencryptedKeysToSave: candidateKeys }; } - const result: Key[] = []; + const keysToRetain: Key[] = []; + const unencryptedKeysToSave: Key[] = []; for (const candidate of candidateKeys) { const longid = KeyUtil.getPrimaryLongid(candidate); const keyToUpdate = existingKeys.filter(ki => ki.longid === longid && ki.family === candidate.family); if (keyToUpdate.length === 1) { const oldKey = await KeyUtil.parse(keyToUpdate[0].private); if (!candidate.lastModified || (oldKey.lastModified && oldKey.lastModified >= candidate.lastModified)) { + keysToRetain.push(oldKey); continue; } } else if (keyToUpdate.length > 1) { throw new Error(`Unexpected error: key search by longid=${longid} yielded ${keyToUpdate.length} results`); } - result.push(candidate); + unencryptedKeysToSave.push(candidate); } - return result; + return { keysToRetain, unencryptedKeysToSave }; }; export const processAndStoreKeysFromEkmLocally = async ( - { acctEmail, decryptedPrivateKeys, ppOptions }: { acctEmail: string, decryptedPrivateKeys: string[], ppOptions?: PassphraseOptions } + { acctEmail, decryptedPrivateKeys, saveKeysOptions }: { acctEmail: string, decryptedPrivateKeys: string[], saveKeysOptions: SaveKeysOptions } ): Promise => { const { unencryptedPrvs } = await parseAndCheckPrivateKeys(decryptedPrivateKeys); const existingKeys = await KeyStore.get(acctEmail); - let passphrase = ppOptions?.passphrase; + let passphrase = saveKeysOptions.ppOptions?.passphrase; if (passphrase === undefined && !existingKeys.length) { return { needPassphrase: false, updateCount: 0 }; // return success as we can't possibly validate a passphrase // this can only happen on misconfiguration // todo: or should we throw? } - let unencryptedKeysToSave = await filterKeysToSave(unencryptedPrvs, existingKeys); + let { keysToRetain, unencryptedKeysToSave } = await filterKeysToSave(unencryptedPrvs, existingKeys); let encryptedKeys: Key[] = []; if (unencryptedKeysToSave.length) { if (passphrase === undefined) { @@ -112,7 +122,7 @@ export const processAndStoreKeysFromEkmLocally = async ( } if (encryptedKeys.length) { // also updates `name`, todo: refactor in #4545 - await saveKeysAndPassPhrase(acctEmail, encryptedKeys, ppOptions); + await saveKeysAndPassPhrase(acctEmail, keysToRetain.concat(encryptedKeys), saveKeysOptions); return { needPassphrase: false, updateCount: encryptedKeys.length }; } else { return { needPassphrase: unencryptedKeysToSave.length > 0, updateCount: 0 }; diff --git a/extension/js/common/platform/store/key-store.ts b/extension/js/common/platform/store/key-store.ts index 0ab66a954aa..4527f8d5a14 100644 --- a/extension/js/common/platform/store/key-store.ts +++ b/extension/js/common/platform/store/key-store.ts @@ -41,7 +41,7 @@ export class KeyStore extends AbstractStore { throw new Error('Cannot import plain, unprotected key.'); } for (const i in keyinfos) { - if (prv.id === keyinfos[i].fingerprints[0]) { // replacing a key + if (KeyUtil.identityEquals(prv, keyinfos[i])) { // replacing a key keyinfos[i] = await KeyUtil.keyInfoObj(prv); updated = true; } diff --git a/extension/js/content_scripts/webmail/setup-webmail-content-script.ts b/extension/js/content_scripts/webmail/setup-webmail-content-script.ts index 962331a1b68..a568bf6f1bb 100644 --- a/extension/js/content_scripts/webmail/setup-webmail-content-script.ts +++ b/extension/js/content_scripts/webmail/setup-webmail-content-script.ts @@ -246,7 +246,8 @@ export const contentScriptSetupIfVacant = async (webmailSpecific: WebmailSpecifi const processKeysFromEkm = async (acctEmail: string, decryptedPrivateKeys: string[], factory: XssSafeFactory, ppEvent: { entered?: boolean }) => { try { - const { needPassphrase, updateCount } = await BrowserMsg.send.bg.await.processAndStoreKeysFromEkmLocally({ acctEmail, decryptedPrivateKeys }); + const { needPassphrase, updateCount } = + await BrowserMsg.send.bg.await.processAndStoreKeysFromEkmLocally({ acctEmail, decryptedPrivateKeys, saveKeysOptions: { keys_replace: true } }); if (needPassphrase) { ppEvent.entered = undefined; await showPassphraseDialog(factory, { longids: [], type: 'update_key' }); diff --git a/test/source/tests/setup.ts b/test/source/tests/setup.ts index 2b1a79d5a60..6ba909a4b59 100644 --- a/test/source/tests/setup.ts +++ b/test/source/tests/setup.ts @@ -661,10 +661,9 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await PageRecipe.waitForToastToAppearAndDisappear(gmailPage, 'Account keys updated'); await gmailPage.notPresent('@dialog-passphrase'); - const set10 = await retrieveAndCheckKeys(2); + const set10 = await retrieveAndCheckKeys(1); const mainKey10 = KeyUtil.filterKeysByIdentity(set10, [{ family: 'openpgp', id: '392FB1E9FF4184659AB6A246835C0141B9ECF536' }]); expect(mainKey10.length).to.equal(1); - expect(KeyUtil.filterKeysByIdentity(set10, [{ family: 'openpgp', id: 'FAFB7D675AC74E87F84D169F00B0115807969D75' }]).length).to.equal(1); expect(mainKey10[0].lastModified!).to.be.greaterThan(mainKey9[0].lastModified!); // updated this key // 10. Forget the passphrase, EKM returns a third key, we enter a passphrase that doesn't match any of the existing keys, no update await InboxPageRecipe.finishSessionOnInboxPage(gmailPage); @@ -682,8 +681,8 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== } await ComposePageRecipe.cancelPassphraseDialog(gmailPage, 'keyboard'); await PageRecipe.noToastAppears(gmailPage); - const set11 = await retrieveAndCheckKeys(2); - expect(set11.map(entry => entry.id)).to.eql(['392FB1E9FF4184659AB6A246835C0141B9ECF536', 'FAFB7D675AC74E87F84D169F00B0115807969D75']); + const set11 = await retrieveAndCheckKeys(1); + expect(set11.map(entry => entry.id)).to.eql(['392FB1E9FF4184659AB6A246835C0141B9ECF536']); await gmailPage.close(); // 11. EKM returns a new third key, we enter a passphrase matching an existing key, update happens gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); @@ -696,12 +695,10 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== } await gmailPage.waitTillGone('@dialog-passphrase'); await PageRecipe.waitForToastToAppearAndDisappear(gmailPage, 'Account keys updated'); - const set12 = await retrieveAndCheckKeys(3); - expect(set12.map(entry => entry.id)).to.eql([ - '392FB1E9FF4184659AB6A246835C0141B9ECF536', - 'FAFB7D675AC74E87F84D169F00B0115807969D75', - '277D1ADA213881F4ABE0415395E783DC0289E2E2' - ]); + const set12 = await retrieveAndCheckKeys(1); + expect(set12.map(entry => entry.id)).to.eql(['277D1ADA213881F4ABE0415395E783DC0289E2E2']); + const mainKey12 = KeyUtil.filterKeysByIdentity(set12, [{ family: 'openpgp', id: '277D1ADA213881F4ABE0415395E783DC0289E2E2' }]); + expect(mainKey12.length).to.equal(1); // 12. Forget the passphrase, EKM sends a broken key, no passphrase dialog, no updates await InboxPageRecipe.finishSessionOnInboxPage(gmailPage); await gmailPage.close(); @@ -714,28 +711,20 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== await PageRecipe.waitForToastToAppearAndDisappear(gmailPage, 'Could not update keys from EKM due to error: BrowserMsg(processAndStoreKeysFromEkmLocally) sendRawResponse::Error: Some keys could not be parsed'); await gmailPage.notPresent('@dialog-passphrase'); - const set13 = await retrieveAndCheckKeys(3); - expect(set13.map(entry => entry.id)).to.eql([ - '392FB1E9FF4184659AB6A246835C0141B9ECF536', - 'FAFB7D675AC74E87F84D169F00B0115807969D75', - '277D1ADA213881F4ABE0415395E783DC0289E2E2' - ]); - const mainKey13 = KeyUtil.filterKeysByIdentity(set13, [{ family: 'openpgp', id: '392FB1E9FF4184659AB6A246835C0141B9ECF536' }]); + const set13 = await retrieveAndCheckKeys(1); + expect(set13.map(entry => entry.id)).to.eql(['277D1ADA213881F4ABE0415395E783DC0289E2E2']); + const mainKey13 = KeyUtil.filterKeysByIdentity(set13, [{ family: 'openpgp', id: '277D1ADA213881F4ABE0415395E783DC0289E2E2' }]); expect(mainKey13.length).to.equal(1); - expect(mainKey13[0].lastModified).to.equal(mainKey10[0].lastModified); // no update + expect(mainKey13[0].lastModified).to.equal(mainKey12[0].lastModified); // no update await gmailPage.close(); // 13. EKM down, no toast, no passphrase dialog, no updates MOCK_KM_UPDATING_KEY.badRequestError = 'RequestTimeout'; gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await PageRecipe.noToastAppears(gmailPage); await gmailPage.notPresent('@dialog-passphrase'); - const set14 = await retrieveAndCheckKeys(3); - expect(set14.map(entry => entry.id)).to.eql([ - '392FB1E9FF4184659AB6A246835C0141B9ECF536', - 'FAFB7D675AC74E87F84D169F00B0115807969D75', - '277D1ADA213881F4ABE0415395E783DC0289E2E2' - ]); - const mainKey14 = KeyUtil.filterKeysByIdentity(set14.map(ki => ki), [{ family: 'openpgp', id: '392FB1E9FF4184659AB6A246835C0141B9ECF536' }]); + const set14 = await retrieveAndCheckKeys(1); + expect(set14.map(entry => entry.id)).to.eql(['277D1ADA213881F4ABE0415395E783DC0289E2E2']); + const mainKey14 = KeyUtil.filterKeysByIdentity(set14.map(ki => ki), [{ family: 'openpgp', id: '277D1ADA213881F4ABE0415395E783DC0289E2E2' }]); expect(mainKey14.length).to.equal(1); expect(mainKey14[0].lastModified).to.equal(mainKey13[0].lastModified); // no update await gmailPage.close(); From 9da76b745d2be5775aac23c304d31c167810f64d Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Sun, 28 Aug 2022 13:02:10 +0000 Subject: [PATCH 02/19] redirect to settings page --- extension/chrome/settings/setup.ts | 9 +- .../chrome/settings/setup/setup-create-key.ts | 2 +- .../chrome/settings/setup/setup-import-key.ts | 4 +- .../setup/setup-key-manager-autogen.ts | 6 +- .../settings/setup/setup-recover-key.ts | 2 +- .../chrome/settings/setup/setup-render.ts | 11 +- extension/js/common/browser/browser-msg.ts | 5 +- extension/js/common/helpers.ts | 32 +++--- extension/js/common/lang.ts | 1 + .../webmail/setup-webmail-content-script.ts | 25 +++-- .../mock/key-manager/key-manager-endpoints.ts | 22 +++- test/source/tests/setup.ts | 105 ++++++++++++------ 12 files changed, 140 insertions(+), 84 deletions(-) diff --git a/extension/chrome/settings/setup.ts b/extension/chrome/settings/setup.ts index 8e8495583f9..f7d4ff0bc5d 100644 --- a/extension/chrome/settings/setup.ts +++ b/extension/chrome/settings/setup.ts @@ -38,11 +38,6 @@ export interface PassphraseOptions { passphrase_save: boolean; } -export interface SaveKeysOptions { - keys_replace?: boolean; // replace existing keys in the storage - ppOptions?: PassphraseOptions; -} - export interface SetupOptions extends PassphraseOptions { submit_main: boolean; submit_all: boolean; @@ -53,7 +48,7 @@ export class SetupView extends View { public readonly acctEmail: string; public readonly parentTabId: string | undefined; - public readonly action: 'add_key' | undefined; + public readonly action: 'add_key' | 'update_from_ekm' | undefined; public readonly idToken: string | undefined; // only needed for initial setup, not for add_key public readonly keyImportUi = new KeyImportUi({ checkEncryption: true }); @@ -82,7 +77,7 @@ export class SetupView extends View { super(); const uncheckedUrlParams = Url.parse(['acctEmail', 'action', 'idToken', 'parentTabId']); this.acctEmail = Assert.urlParamRequire.string(uncheckedUrlParams, 'acctEmail'); - this.action = Assert.urlParamRequire.oneof(uncheckedUrlParams, 'action', ['add_key', undefined]) as 'add_key' | undefined; + this.action = Assert.urlParamRequire.oneof(uncheckedUrlParams, 'action', ['add_key', 'update_from_ekm', undefined]) as 'add_key' | 'update_from_ekm' | undefined; if (this.action === 'add_key') { this.parentTabId = Assert.urlParamRequire.string(uncheckedUrlParams, 'parentTabId'); } else { diff --git a/extension/chrome/settings/setup/setup-create-key.ts b/extension/chrome/settings/setup/setup-create-key.ts index 9cfc4bfb7c3..60df2cb991d 100644 --- a/extension/chrome/settings/setup/setup-create-key.ts +++ b/extension/chrome/settings/setup/setup-create-key.ts @@ -82,7 +82,7 @@ export class SetupCreateKeyModule { const expireMonths = this.view.clientConfiguration.getEnforcedKeygenExpirationMonths(); const key = await OpenPGPKey.create(pgpUids, keyAlgo, options.passphrase, expireMonths); const prv = await KeyUtil.parse(key.private); - await saveKeysAndPassPhrase(this.view.acctEmail, [prv], { ppOptions: options }); + await saveKeysAndPassPhrase(this.view.acctEmail, [prv], options); return { id: prv.id, family: prv.family }; }; } diff --git a/extension/chrome/settings/setup/setup-import-key.ts b/extension/chrome/settings/setup/setup-import-key.ts index ba8cd647a24..0681c20ef8c 100644 --- a/extension/chrome/settings/setup/setup-import-key.ts +++ b/extension/chrome/settings/setup/setup-import-key.ts @@ -40,7 +40,7 @@ export class SetupImportKeyModule { } } Xss.sanitizeRender('#step_2b_manual_enter .action_add_private_key', Ui.spinner('white')); - await saveKeysAndPassPhrase(this.view.acctEmail, [checked.encrypted], { ppOptions: options }); + await saveKeysAndPassPhrase(this.view.acctEmail, [checked.encrypted], options); await this.view.submitPublicKeys(options); await this.view.finalizeSetup(); await this.view.setupRender.renderSetupDone(); @@ -71,7 +71,7 @@ export class SetupImportKeyModule { this.view.setupRender.displayBlock('step_2b_manual_enter'); return; } - await saveKeysAndPassPhrase(this.view.acctEmail, [fixedPrv], { ppOptions: options }); + await saveKeysAndPassPhrase(this.view.acctEmail, [fixedPrv], options); await this.view.submitPublicKeys(options); await this.view.finalizeSetup(); await this.view.setupRender.renderSetupDone(); diff --git a/extension/chrome/settings/setup/setup-key-manager-autogen.ts b/extension/chrome/settings/setup/setup-key-manager-autogen.ts index 2940c9df99c..14c6e7c9265 100644 --- a/extension/chrome/settings/setup/setup-key-manager-autogen.ts +++ b/extension/chrome/settings/setup/setup-key-manager-autogen.ts @@ -59,7 +59,7 @@ export class SetupWithEmailKeyManagerModule { await processAndStoreKeysFromEkmLocally({ acctEmail: this.view.acctEmail, decryptedPrivateKeys: privateKeys.map(entry => entry.decryptedPrivateKey), - saveKeysOptions: { ppOptions: setupOptions } + ppOptions: setupOptions }); } catch (e) { throw new Error(`Could not store keys from EKM due to error: ${e instanceof Error ? e.message : String(e)}`); @@ -68,7 +68,7 @@ export class SetupWithEmailKeyManagerModule { // generate keys on client and store them on key manager await this.autoGenerateKeyAndStoreBothLocallyAndToEkm(setupOptions); } else { - await Ui.modal.error(`Keys for your account were not set up yet - please ask your systems administrator.`); + await Ui.modal.error(Lang.setup.noKeys); window.location.href = Url.create('index.htm', { acctEmail: this.view.acctEmail }); return; } @@ -102,7 +102,7 @@ export class SetupWithEmailKeyManagerModule { } const storePrvOnKm = async () => this.view.keyManager!.storePrivateKey(this.view.idToken!, KeyUtil.armor(decryptablePrv)); await Settings.retryUntilSuccessful(storePrvOnKm, 'Failed to store newly generated key on FlowCrypt Email Key Manager', Lang.general.contactIfNeedAssistance(this.view.isFesUsed())); - await saveKeysAndPassPhrase(this.view.acctEmail, [await KeyUtil.parse(generated.private)], { ppOptions: setupOptions }); // store encrypted key + pass phrase locally + await saveKeysAndPassPhrase(this.view.acctEmail, [await KeyUtil.parse(generated.private)], setupOptions, true); // store encrypted key + pass phrase locally }; } diff --git a/extension/chrome/settings/setup/setup-recover-key.ts b/extension/chrome/settings/setup/setup-recover-key.ts index 8ff0b51cffc..0480cd550ae 100644 --- a/extension/chrome/settings/setup/setup-recover-key.ts +++ b/extension/chrome/settings/setup/setup-recover-key.ts @@ -64,7 +64,7 @@ export class SetupRecoverKeyModule { passphrase_save: true, // todo - reevaluate saving passphrase when recovering recovered: true, }; - await saveKeysAndPassPhrase(this.view.acctEmail, newlyMatchingKeys, { ppOptions: options }); + await saveKeysAndPassPhrase(this.view.acctEmail, newlyMatchingKeys, options); const { setup_done } = await AcctStore.get(this.view.acctEmail, ['setup_done']); if (!setup_done) { // normal situation - fresh setup await this.view.submitPublicKeys(options); diff --git a/extension/chrome/settings/setup/setup-render.ts b/extension/chrome/settings/setup/setup-render.ts index 541c2fa3d1d..3ce71440dc7 100644 --- a/extension/chrome/settings/setup/setup-render.ts +++ b/extension/chrome/settings/setup/setup-render.ts @@ -32,7 +32,7 @@ export class SetupRenderModule { return await Settings.promptToRetry(e, Lang.setup.failedToLoadEmailAliases, () => this.renderInitial(), Lang.general.contactIfNeedAssistance(this.view.isFesUsed())); } } - if (this.view.storage!.setup_done) { + if (this.view.storage!.setup_done && this.view.action !== 'update_from_ekm') { if (this.view.action !== 'add_key') { await this.renderSetupDone(); } else if (this.view.clientConfiguration.mustAutoImportOrAutogenPrvWithKeyManager()) { @@ -66,8 +66,13 @@ export class SetupRenderModule { $('.private_key_count').text(storedKeys.length); $('.backups_count').text(this.view.fetchedKeyBackupsUniqueLongids.length); } else { // successful and complete setup - this.displayBlock(this.view.action !== 'add_key' ? 'step_4_done' : 'step_4_close'); - $('h1').text(this.view.action !== 'add_key' ? 'You\'re all set!' : 'Recovered all keys!'); + if (this.view.action === 'add_key') { + this.displayBlock('step_4_close'); + $('h1').text('Recovered all keys!'); + } else { + this.displayBlock('step_4_done'); + $('h1').text('You\'re all set!'); + } $('.email').text(this.view.acctEmail); } }; diff --git a/extension/js/common/browser/browser-msg.ts b/extension/js/common/browser/browser-msg.ts index 35e5e10143b..d0240265d2e 100644 --- a/extension/js/common/browser/browser-msg.ts +++ b/extension/js/common/browser/browser-msg.ts @@ -19,7 +19,6 @@ import { GlobalStoreDict, GlobalIndex } from '../platform/store/global-store.js' import { AcctStoreDict, AccountIndex } from '../platform/store/acct-store.js'; import { saveFetchedPubkeysIfNewerThanInStorage } from '../shared.js'; import { ArmoredKeyIdentityWithEmails, KeyUtil } from '../core/crypto/key.js'; -import { SaveKeysOptions } from '../../../chrome/settings/setup.js'; export type GoogleAuthWindowResult$result = 'Success' | 'Denied' | 'Error' | 'Closed'; @@ -68,7 +67,7 @@ export namespace Bm { export type ShowAttachmentPreview = { iframeUrl: string }; export type ReRenderRecipient = { email: string }; export type SaveFetchedPubkeys = { email: string, pubkeys: string[] }; - export type ProcessAndStoreKeysFromEkmLocally = { acctEmail: string, decryptedPrivateKeys: string[], saveKeysOptions: SaveKeysOptions }; + export type ProcessAndStoreKeysFromEkmLocally = { acctEmail: string, decryptedPrivateKeys: string[] }; export namespace Res { export type GetActiveTabInfo = { provider: 'gmail' | undefined, acctEmail: string | undefined, sameWorld: boolean | undefined }; @@ -88,7 +87,7 @@ export namespace Bm { export type AjaxGmailAttachmentGetChunk = { chunk: Buf }; export type _tab_ = { tabId: string | null | undefined }; export type SaveFetchedPubkeys = boolean; - export type ProcessAndStoreKeysFromEkmLocally = { needPassphrase: boolean, updateCount: number }; + export type ProcessAndStoreKeysFromEkmLocally = { needPassphrase?: boolean, updateCount?: number, needSetup?: boolean }; export type Db = any; // not included in Any below export type Ajax = any; // not included in Any below diff --git a/extension/js/common/helpers.ts b/extension/js/common/helpers.ts index 47649f4d79d..53009eacc8f 100644 --- a/extension/js/common/helpers.ts +++ b/extension/js/common/helpers.ts @@ -3,7 +3,7 @@ 'use strict'; import { AcctStore } from './platform/store/acct-store.js'; -import { SaveKeysOptions } from '../../chrome/settings/setup.js'; +import { PassphraseOptions } from '../../chrome/settings/setup.js'; import { Buf } from './core/buf.js'; import { Key, KeyInfoWithIdentity, KeyUtil } from './core/crypto/key.js'; import { ClientConfiguration } from './client-configuration.js'; @@ -16,14 +16,14 @@ export const isFesUsed = async (acctEmail: string) => { return Boolean(fesUrl); }; -export const saveKeysAndPassPhrase = async (acctEmail: string, prvs: Key[], { keys_replace, ppOptions }: SaveKeysOptions) => { +export const saveKeysAndPassPhrase = async (acctEmail: string, prvs: Key[], ppOptions?: PassphraseOptions, replaceKeys: boolean = false) => { const clientConfiguration = await ClientConfiguration.newInstance(acctEmail); - if (keys_replace) { + if (replaceKeys) { await KeyStore.set(acctEmail, await Promise.all(prvs.map(KeyUtil.keyInfoObj))); // todo: duplicate identities // todo: should we delete passphrases matching the deleted keys? } for (const prv of prvs) { - if (!keys_replace) { + if (!replaceKeys) { await KeyStore.add(acctEmail, prv); } if (ppOptions !== undefined) { @@ -94,17 +94,19 @@ const filterKeysToSave = async (candidateKeys: Key[], existingKeys: KeyInfoWithI }; export const processAndStoreKeysFromEkmLocally = async ( - { acctEmail, decryptedPrivateKeys, saveKeysOptions }: { acctEmail: string, decryptedPrivateKeys: string[], saveKeysOptions: SaveKeysOptions } + { acctEmail, decryptedPrivateKeys, ppOptions }: { acctEmail: string, decryptedPrivateKeys: string[], ppOptions?: PassphraseOptions } ): Promise => { const { unencryptedPrvs } = await parseAndCheckPrivateKeys(decryptedPrivateKeys); const existingKeys = await KeyStore.get(acctEmail); - let passphrase = saveKeysOptions.ppOptions?.passphrase; + let { keysToRetain, unencryptedKeysToSave } = await filterKeysToSave(unencryptedPrvs, existingKeys); + if (!unencryptedKeysToSave.length && keysToRetain.length === existingKeys.length) { + // nothing to update + return { needPassphrase: false, needSetup: !existingKeys.length }; + } + let passphrase = ppOptions?.passphrase; if (passphrase === undefined && !existingKeys.length) { - return { needPassphrase: false, updateCount: 0 }; // return success as we can't possibly validate a passphrase - // this can only happen on misconfiguration - // todo: or should we throw? + return { needPassphrase: true, needSetup: true }; } - let { keysToRetain, unencryptedKeysToSave } = await filterKeysToSave(unencryptedPrvs, existingKeys); let encryptedKeys: Key[] = []; if (unencryptedKeysToSave.length) { if (passphrase === undefined) { @@ -120,11 +122,13 @@ export const processAndStoreKeysFromEkmLocally = async ( unencryptedKeysToSave = []; } } - if (encryptedKeys.length) { + if (encryptedKeys.length || !unencryptedKeysToSave.length) { // also updates `name`, todo: refactor in #4545 - await saveKeysAndPassPhrase(acctEmail, keysToRetain.concat(encryptedKeys), saveKeysOptions); - return { needPassphrase: false, updateCount: encryptedKeys.length }; + const newKeyset = keysToRetain.concat(encryptedKeys); + await saveKeysAndPassPhrase(acctEmail, newKeyset, ppOptions, true); + return { updateCount: encryptedKeys.length + (existingKeys.length - keysToRetain.length), needSetup: !newKeyset.length }; } else { - return { needPassphrase: unencryptedKeysToSave.length > 0, updateCount: 0 }; + // todo: should we delete? + return { needPassphrase: unencryptedKeysToSave.length > 0 }; } }; diff --git a/extension/js/common/lang.ts b/extension/js/common/lang.ts index 4d78d7ca56d..712e69ed3fd 100644 --- a/extension/js/common/lang.ts +++ b/extension/js/common/lang.ts @@ -31,6 +31,7 @@ export const Lang = { // tslint:disable-line:variable-name keyBackupsNotAllowed: 'Key backups are not allowed on this domain.', prvHasFixableCompatIssue: 'This key has minor usability issues that can be fixed. This commonly happens when importing keys from Symantec™ PGP Desktop or other legacy software. It may be missing User IDs, or it may be missing a self-signature. It is also possible that the key is simply expired.', ppMatchAllSet: 'Your pass phrase matches. Good job! You\'re all set.', + noKeys: 'Keys for your account were not set up yet - please ask your systems administrator.', }, account: { googleAcctDisabledOrPolicy: `Your Google Account or Google Email seems to be disabled, or access to this app is disabled by your organisation admin policy. Contact your email administrator.`, diff --git a/extension/js/content_scripts/webmail/setup-webmail-content-script.ts b/extension/js/content_scripts/webmail/setup-webmail-content-script.ts index a568bf6f1bb..75c1066ebc8 100644 --- a/extension/js/content_scripts/webmail/setup-webmail-content-script.ts +++ b/extension/js/content_scripts/webmail/setup-webmail-content-script.ts @@ -20,6 +20,8 @@ import { KeyManager } from '../../common/api/key-server/key-manager.js'; import { InMemoryStore } from '../../common/platform/store/in-memory-store.js'; import { AccountServer } from '../../common/api/account-server.js'; import { ApiErr, BackendAuthErr } from '../../common/api/shared/api-error.js'; +import { Url } from '../../common/core/common.js'; +import { Lang } from '../../common/lang.js'; export type WebmailVariantObject = { newDataLayer: undefined | boolean, newUi: undefined | boolean, email: undefined | string, gmailVariant: WebmailVariantString }; export type IntervalFunction = { interval: number, handler: () => void }; @@ -244,10 +246,19 @@ export const contentScriptSetupIfVacant = async (webmailSpecific: WebmailSpecifi await factory.showPassphraseDialog(longids, type, initiatorFrameId); }; - const processKeysFromEkm = async (acctEmail: string, decryptedPrivateKeys: string[], factory: XssSafeFactory, ppEvent: { entered?: boolean }) => { + const processKeysFromEkm = async (acctEmail: string, decryptedPrivateKeys: string[], clientConfiguration: ClientConfiguration, factory: XssSafeFactory, idToken: string, ppEvent: { entered?: boolean }) => { try { - const { needPassphrase, updateCount } = - await BrowserMsg.send.bg.await.processAndStoreKeysFromEkmLocally({ acctEmail, decryptedPrivateKeys, saveKeysOptions: { keys_replace: true } }); + const { needPassphrase, updateCount, needSetup } = + await BrowserMsg.send.bg.await.processAndStoreKeysFromEkmLocally({ acctEmail, decryptedPrivateKeys }); + if (needSetup) { + if (!needPassphrase && !clientConfiguration.canCreateKeys()) { + await Ui.modal.error(Lang.setup.noKeys); + BrowserMsg.send.bg.settings({ acctEmail, path: 'index.htm' }); + } else { + BrowserMsg.send.bg.settings({ acctEmail, path: Url.create('setup.htm', { idToken, action: 'update_from_ekm' }) }); + } + return; + } if (needPassphrase) { ppEvent.entered = undefined; await showPassphraseDialog(factory, { longids: [], type: 'update_key' }); @@ -255,11 +266,11 @@ export const contentScriptSetupIfVacant = async (webmailSpecific: WebmailSpecifi await Ui.time.sleep(100); } if (ppEvent.entered) { - await processKeysFromEkm(acctEmail, decryptedPrivateKeys, factory, ppEvent); + await processKeysFromEkm(acctEmail, decryptedPrivateKeys, clientConfiguration, factory, idToken, ppEvent); } else { return; } - } else if (updateCount > 0) { + } else if (updateCount && updateCount > 0) { Ui.toast('Account keys updated'); } } catch (e) { @@ -275,9 +286,7 @@ export const contentScriptSetupIfVacant = async (webmailSpecific: WebmailSpecifi const keyManager = new KeyManager(clientConfiguration.getKeyManagerUrlForPrivateKeys()!); Catch.setHandledTimeout(async () => { const { privateKeys } = await keyManager.getPrivateKeys(idToken); - if (privateKeys.length) { - await processKeysFromEkm(acctEmail, privateKeys.map(entry => entry.decryptedPrivateKey), factory, ppEvent); - } + await processKeysFromEkm(acctEmail, privateKeys.map(entry => entry.decryptedPrivateKey), clientConfiguration, factory, idToken, ppEvent); }, 0); } } diff --git a/test/source/mock/key-manager/key-manager-endpoints.ts b/test/source/mock/key-manager/key-manager-endpoints.ts index eccef726c57..c32ee83c9a2 100644 --- a/test/source/mock/key-manager/key-manager-endpoints.ts +++ b/test/source/mock/key-manager/key-manager-endpoints.ts @@ -206,7 +206,7 @@ yeSm0uVPwODhwX7ezB9jW6uVt0R8S8iM3rQdEMsA/jDep5LNn47K6o8VrDt0zYo6 export const MOCK_KM_LAST_INSERTED_KEY: { [acct: string]: { privateKey: string } } = {}; // accessed from test runners -export const MOCK_KM_UPDATING_KEY: { response?: { privateKeys: { decryptedPrivateKey: string }[] }, badRequestError?: string } = {}; +export const MOCK_KM_UPDATING_KEY: { [acct: string]: { response?: { privateKeys: { decryptedPrivateKey: string }[] }, badRequestError?: string } } = {}; export const mockKeyManagerEndpoints: HandlersDefinition = { '/flowcrypt-email-key-manager/v1/keys/private': async ({ body }, req) => { @@ -218,11 +218,12 @@ export const mockKeyManagerEndpoints: HandlersDefinition = { if (acctEmail === 'get.key@key-manager-autogen.flowcrypt.test') { return { privateKeys: [{ decryptedPrivateKey: testConstants.existingPrv }] }; } - if (acctEmail === 'get.updating.key@key-manager-choose-passphrase-forbid-storing.flowcrypt.test') { - if (MOCK_KM_UPDATING_KEY.response !== undefined && MOCK_KM_UPDATING_KEY.badRequestError === undefined) { - return MOCK_KM_UPDATING_KEY.response; + if (acctEmail.includes('updating.key')) { + const { response, badRequestError } = MOCK_KM_UPDATING_KEY[acctEmail]; + if (response !== undefined && badRequestError === undefined) { + return response; } - throw new HttpClientErr(MOCK_KM_UPDATING_KEY.badRequestError || 'Vague error', Status.BAD_REQUEST); + throw new HttpClientErr(badRequestError || 'Vague error', Status.BAD_REQUEST); } if (acctEmail === 'get.key@no-submit-client-configuration.key-manager-autogen.flowcrypt.test') { return { privateKeys: [{ decryptedPrivateKey: prvNoSubmit }] }; @@ -303,6 +304,17 @@ export const mockKeyManagerEndpoints: HandlersDefinition = { MOCK_KM_LAST_INSERTED_KEY[acctEmail] = { privateKey }; return {}; } + if (acctEmail.includes('updating.key')) { + const prv = await KeyUtil.parseMany(privateKey); + expect(prv).to.have.length(1); + expect(prv[0].algo.bits).to.equal(2048); + expect(prv[0].identities).to.have.length(1); + expect(prv[0].isPrivate).to.be.true; + expect(prv[0].fullyDecrypted).to.be.true; + expect(prv[0].expiration).to.not.exist; + MOCK_KM_LAST_INSERTED_KEY[acctEmail] = { privateKey }; + return {}; + } throw new HttpClientErr(`Unexpectedly calling mockKeyManagerEndpoints:/v1/keys/private PUT with acct ${acctEmail}`); } throw new HttpClientErr(`Unknown method: ${req.method}`); diff --git a/test/source/tests/setup.ts b/test/source/tests/setup.ts index 6ba909a4b59..64530ca80af 100644 --- a/test/source/tests/setup.ts +++ b/test/source/tests/setup.ts @@ -8,7 +8,7 @@ import { TestWithBrowser } from './../test'; import { expect } from 'chai'; import { SettingsPageRecipe } from './page-recipe/settings-page-recipe'; import { ComposePageRecipe } from './page-recipe/compose-page-recipe'; -import { Str } from './../core/common'; +import { Str, emailKeyIndex } from './../core/common'; import { MOCK_KM_LAST_INSERTED_KEY, MOCK_KM_UPDATING_KEY } from './../mock/key-manager/key-manager-endpoints'; import { MOCK_ATTESTER_LAST_INSERTED_PUB } from './../mock/attester/attester-endpoints'; import { BrowserRecipe } from './tooling/browser-recipe'; @@ -17,6 +17,7 @@ import { testConstants } from './tooling/consts'; import { InboxPageRecipe } from './page-recipe/inbox-page-recipe'; import { PageRecipe } from './page-recipe/abstract-page-recipe'; import { TestUrls } from '../browser/test-urls'; +import { ControllablePage } from '../browser'; // tslint:disable:no-blank-lines-func // tslint:disable:no-unused-expression @@ -557,9 +558,23 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== await securityFrame.notPresent(['@action-change-passphrase-begin', '@action-test-passphrase-begin', '@action-forget-pp']); })); + const retrieveAndCheckKeys = async (page: ControllablePage, acctEmail: string, passphrase: string, expectedKeyCount: number) => { + const key = `cryptup_${emailKeyIndex(acctEmail, 'keys')}`; + const keyset = (await page.getFromLocalStorage([key]))[key]; + const kis = keyset as KeyInfoWithIdentity[]; + expect(kis.length).to.equal(expectedKeyCount); + return await Promise.all(kis.map(async ki => { + const prv = await KeyUtil.parse(ki.private); + expect(prv.fullyEncrypted).to.be.true; + expect(await KeyUtil.decrypt(prv, passphrase, undefined, undefined)).to.be.true; + expect(prv.lastModified).to.not.be.an.undefined; + return prv; + })); + }; + ava.default('get.updating.key@key-manager-choose-passphrase-forbid-storing.flowcrypt.test - automatic update of key found on key manager', testWithBrowser(undefined, async (t, browser) => { - MOCK_KM_UPDATING_KEY.response = { privateKeys: [{ decryptedPrivateKey: testConstants.updatingPrv }] }; const acct = 'get.updating.key@key-manager-choose-passphrase-forbid-storing.flowcrypt.test'; + MOCK_KM_UPDATING_KEY[acct] = { response: { privateKeys: [{ decryptedPrivateKey: testConstants.updatingPrv }] } }; const settingsPage = await BrowserRecipe.openSettingsLoginApprove(t, browser, acct); const passphrase = 'long enough to suit requirements'; await SetupPageRecipe.autoSetupWithEKM(settingsPage, { @@ -567,43 +582,30 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== }); const accessToken = await BrowserRecipe.getGoogleAccessToken(settingsPage, acct); const extraAuthHeaders = { Authorization: `Bearer ${accessToken}` }; - const retrieveAndCheckKeys = async (expectedKeyCount: number) => { - const { cryptup_getupdatingkeykeymanagerchoosepassphraseforbidstoringflowcrypttest_keys: keyset } - = await settingsPage.getFromLocalStorage(['cryptup_getupdatingkeykeymanagerchoosepassphraseforbidstoringflowcrypttest_keys']); - const kis = keyset as KeyInfoWithIdentity[]; - expect(kis.length).to.equal(expectedKeyCount); - return await Promise.all(kis.map(async ki => { - const prv = await KeyUtil.parse(ki.private); - expect(prv.fullyEncrypted).to.be.true; - expect(await KeyUtil.decrypt(prv, passphrase as string, undefined, undefined)).to.be.true; - expect(prv.lastModified).to.not.be.an.undefined; - return prv; - })); - }; const updateAndArmorKey = async (prv: Key) => { return KeyUtil.armor(await KeyUtil.reformatKey(prv, undefined, [{ name: 'Full Name', email: acct }], 6000)); }; - const set1 = await retrieveAndCheckKeys(1); + const set1 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); // 1. EKM returns the same key, no update, no toast let gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await PageRecipe.noToastAppears(gmailPage); await gmailPage.notPresent('@dialog-passphrase'); - const set2 = await retrieveAndCheckKeys(1); + const set2 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); expect(set2[0].lastModified).to.equal(set1[0].lastModified); // no update await gmailPage.close(); // 2. EKM returns a newer version of the existing key const someOlderVersion = await updateAndArmorKey(set2[0]); - MOCK_KM_UPDATING_KEY.response = { privateKeys: [{ decryptedPrivateKey: someOlderVersion }] }; + MOCK_KM_UPDATING_KEY[acct].response = { privateKeys: [{ decryptedPrivateKey: someOlderVersion }] }; gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await PageRecipe.waitForToastToAppearAndDisappear(gmailPage, 'Account keys updated'); - const set3 = await retrieveAndCheckKeys(1); + const set3 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); expect(set3[0].lastModified!).to.be.greaterThan(set2[0].lastModified!); // an update happened await gmailPage.close(); // 3. EKM returns the same version of the existing key, no toast, no update gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await PageRecipe.noToastAppears(gmailPage); await gmailPage.notPresent('@dialog-passphrase'); - const set4 = await retrieveAndCheckKeys(1); + const set4 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); expect(set4[0].lastModified).to.equal(set3[0].lastModified); // no update // 4. Forget the passphrase, EKM the same version of the existing key, no prompt await InboxPageRecipe.finishSessionOnInboxPage(gmailPage); @@ -611,16 +613,16 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await PageRecipe.noToastAppears(gmailPage); await gmailPage.notPresent('@dialog-passphrase'); - const set5 = await retrieveAndCheckKeys(1); + const set5 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); expect(set5[0].lastModified).to.equal(set4[0].lastModified); // no update await gmailPage.close(); // 5. EKM returns a newer version of the existing key, canceling passphrase prompt, no update - MOCK_KM_UPDATING_KEY.response = { privateKeys: [{ decryptedPrivateKey: await updateAndArmorKey(set5[0]) }] }; + MOCK_KM_UPDATING_KEY[acct].response = { privateKeys: [{ decryptedPrivateKey: await updateAndArmorKey(set5[0]) }] }; gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await gmailPage.waitAll('@dialog-passphrase'); await ComposePageRecipe.cancelPassphraseDialog(gmailPage, 'keyboard'); await PageRecipe.noToastAppears(gmailPage); - const set6 = await retrieveAndCheckKeys(1); + const set6 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); expect(set6[0].lastModified).to.equal(set5[0].lastModified); // no update await gmailPage.close(); // 6. EKM returns a newer version of the existing key, entering the passphrase, update toast @@ -634,41 +636,41 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== } await gmailPage.waitTillGone('@dialog-passphrase'); await PageRecipe.waitForToastToAppearAndDisappear(gmailPage, 'Account keys updated'); - const set7 = await retrieveAndCheckKeys(1); + const set7 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); expect(set7[0].lastModified!).to.be.greaterThan(set6[0].lastModified!); // an update happened await gmailPage.close(); // 7. EKM returns an older version of the existing key, no toast, no update - MOCK_KM_UPDATING_KEY.response = { privateKeys: [{ decryptedPrivateKey: someOlderVersion }] }; + MOCK_KM_UPDATING_KEY[acct].response = { privateKeys: [{ decryptedPrivateKey: someOlderVersion }] }; gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await PageRecipe.noToastAppears(gmailPage); await gmailPage.notPresent('@dialog-passphrase'); - const set8 = await retrieveAndCheckKeys(1); + const set8 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); expect(set8[0].lastModified).to.equal(set7[0].lastModified); // no update await gmailPage.close(); // 8. EKM returns an older version of the existing key, and a new key, toast, new key gets added encrypted with the same passphrase - MOCK_KM_UPDATING_KEY.response = { privateKeys: [{ decryptedPrivateKey: someOlderVersion }, { decryptedPrivateKey: testConstants.existingPrv }] }; + MOCK_KM_UPDATING_KEY[acct].response = { privateKeys: [{ decryptedPrivateKey: someOlderVersion }, { decryptedPrivateKey: testConstants.existingPrv }] }; gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await PageRecipe.waitForToastToAppearAndDisappear(gmailPage, 'Account keys updated'); await gmailPage.notPresent('@dialog-passphrase'); - const set9 = await retrieveAndCheckKeys(2); + const set9 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 2); const mainKey9 = KeyUtil.filterKeysByIdentity(set9, [{ family: 'openpgp', id: '392FB1E9FF4184659AB6A246835C0141B9ECF536' }]); expect(mainKey9.length).to.equal(1); expect(KeyUtil.filterKeysByIdentity(set9, [{ family: 'openpgp', id: 'FAFB7D675AC74E87F84D169F00B0115807969D75' }]).length).to.equal(1); expect(mainKey9[0].lastModified).to.equal(set8[0].lastModified); // no update await gmailPage.close(); // 9. EKM returns a newer version of one key, fully omitting the other one, a toast, and update, no removal - MOCK_KM_UPDATING_KEY.response = { privateKeys: [{ decryptedPrivateKey: await updateAndArmorKey(mainKey9[0]) }] }; + MOCK_KM_UPDATING_KEY[acct].response = { privateKeys: [{ decryptedPrivateKey: await updateAndArmorKey(mainKey9[0]) }] }; gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await PageRecipe.waitForToastToAppearAndDisappear(gmailPage, 'Account keys updated'); await gmailPage.notPresent('@dialog-passphrase'); - const set10 = await retrieveAndCheckKeys(1); + const set10 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); const mainKey10 = KeyUtil.filterKeysByIdentity(set10, [{ family: 'openpgp', id: '392FB1E9FF4184659AB6A246835C0141B9ECF536' }]); expect(mainKey10.length).to.equal(1); expect(mainKey10[0].lastModified!).to.be.greaterThan(mainKey9[0].lastModified!); // updated this key // 10. Forget the passphrase, EKM returns a third key, we enter a passphrase that doesn't match any of the existing keys, no update await InboxPageRecipe.finishSessionOnInboxPage(gmailPage); await gmailPage.close(); - MOCK_KM_UPDATING_KEY.response = { privateKeys: [{ decryptedPrivateKey: testConstants.unprotectedPrvKey }] }; + MOCK_KM_UPDATING_KEY[acct].response = { privateKeys: [{ decryptedPrivateKey: testConstants.unprotectedPrvKey }] }; gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await gmailPage.waitAll('@dialog-passphrase'); { @@ -681,7 +683,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== } await ComposePageRecipe.cancelPassphraseDialog(gmailPage, 'keyboard'); await PageRecipe.noToastAppears(gmailPage); - const set11 = await retrieveAndCheckKeys(1); + const set11 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); expect(set11.map(entry => entry.id)).to.eql(['392FB1E9FF4184659AB6A246835C0141B9ECF536']); await gmailPage.close(); // 11. EKM returns a new third key, we enter a passphrase matching an existing key, update happens @@ -695,14 +697,14 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== } await gmailPage.waitTillGone('@dialog-passphrase'); await PageRecipe.waitForToastToAppearAndDisappear(gmailPage, 'Account keys updated'); - const set12 = await retrieveAndCheckKeys(1); + const set12 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); expect(set12.map(entry => entry.id)).to.eql(['277D1ADA213881F4ABE0415395E783DC0289E2E2']); const mainKey12 = KeyUtil.filterKeysByIdentity(set12, [{ family: 'openpgp', id: '277D1ADA213881F4ABE0415395E783DC0289E2E2' }]); expect(mainKey12.length).to.equal(1); // 12. Forget the passphrase, EKM sends a broken key, no passphrase dialog, no updates await InboxPageRecipe.finishSessionOnInboxPage(gmailPage); await gmailPage.close(); - MOCK_KM_UPDATING_KEY.response.privateKeys = [ + MOCK_KM_UPDATING_KEY[acct].response.privateKeys = [ { decryptedPrivateKey: await updateAndArmorKey(set2[0]) }, // update the main key // only include a half of another armored key { decryptedPrivateKey: testConstants.unprotectedPrvKey.substring(0, testConstants.unprotectedPrvKey.length / 2) } @@ -711,18 +713,18 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== await PageRecipe.waitForToastToAppearAndDisappear(gmailPage, 'Could not update keys from EKM due to error: BrowserMsg(processAndStoreKeysFromEkmLocally) sendRawResponse::Error: Some keys could not be parsed'); await gmailPage.notPresent('@dialog-passphrase'); - const set13 = await retrieveAndCheckKeys(1); + const set13 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); expect(set13.map(entry => entry.id)).to.eql(['277D1ADA213881F4ABE0415395E783DC0289E2E2']); const mainKey13 = KeyUtil.filterKeysByIdentity(set13, [{ family: 'openpgp', id: '277D1ADA213881F4ABE0415395E783DC0289E2E2' }]); expect(mainKey13.length).to.equal(1); expect(mainKey13[0].lastModified).to.equal(mainKey12[0].lastModified); // no update await gmailPage.close(); // 13. EKM down, no toast, no passphrase dialog, no updates - MOCK_KM_UPDATING_KEY.badRequestError = 'RequestTimeout'; + MOCK_KM_UPDATING_KEY[acct] = { badRequestError: 'RequestTimeout' }; gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await PageRecipe.noToastAppears(gmailPage); await gmailPage.notPresent('@dialog-passphrase'); - const set14 = await retrieveAndCheckKeys(1); + const set14 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); expect(set14.map(entry => entry.id)).to.eql(['277D1ADA213881F4ABE0415395E783DC0289E2E2']); const mainKey14 = KeyUtil.filterKeysByIdentity(set14.map(ki => ki), [{ family: 'openpgp', id: '277D1ADA213881F4ABE0415395E783DC0289E2E2' }]); expect(mainKey14.length).to.equal(1); @@ -730,6 +732,35 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== await gmailPage.close(); })); + ava.default('put.updating.key@key-manager-choose-passphrase-forbid-storing.flowcrypt.test - updates of key found on key manager via setup page', testWithBrowser(undefined, async (t, browser) => { + const acct = 'put.updating.key@key-manager-choose-passphrase-forbid-storing.flowcrypt.test'; + MOCK_KM_UPDATING_KEY[acct] = { response: { privateKeys: [{ decryptedPrivateKey: testConstants.updatingPrv }] } }; + const settingsPage = await BrowserRecipe.openSettingsLoginApprove(t, browser, acct); + const passphrase = 'long enough to suit requirements'; + await SetupPageRecipe.autoSetupWithEKM(settingsPage, { + enterPp: { passphrase, checks: { isSavePassphraseChecked: false, isSavePassphraseHidden: true } } + }); + const accessToken = await BrowserRecipe.getGoogleAccessToken(settingsPage, acct); + const extraAuthHeaders = { Authorization: `Bearer ${accessToken}` }; + const set1 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); + MOCK_KM_UPDATING_KEY[acct].response = { privateKeys: [] }; + // 1. EKM returns the empty set, forcing to auto-generate + let gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); + await PageRecipe.noToastAppears(gmailPage); + await gmailPage.notPresent('@dialog-passphrase'); + await gmailPage.close(); // todo: why settingsPage is losing focus? + await retrieveAndCheckKeys(settingsPage, acct, passphrase, 0); // no keys, auto-generation + delete MOCK_KM_LAST_INSERTED_KEY[acct]; + await SetupPageRecipe.autoSetupWithEKM(settingsPage, { + enterPp: { passphrase, checks: { isSavePassphraseChecked: false, isSavePassphraseHidden: true } } + }); + expect(MOCK_KM_LAST_INSERTED_KEY[acct]).to.exist; + const set2 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); + expect(set2[0].id).to.not.equal(set1[0].id); // entirely new key was generated + // wire the new key: const generatedKey = MOCK_KM_LAST_INSERTED_KEY[acct].privateKey; + // MOCK_KM_UPDATING_KEY[acct].response = { privateKeys: [{ decryptedPrivateKey: generatedKey }] }; + })); + ava.default('get.key@key-manager-choose-passphrase.flowcrypt.test - passphrase chosen by user with key found on key manager', testWithBrowser(undefined, async (t, browser) => { const acct = 'get.key@key-manager-choose-passphrase.flowcrypt.test'; const passphrase = 'Long and complicated pass PHRASE'; From d78bcf256b5e96929a4c8bcf68800136b77ab707 Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Sun, 28 Aug 2022 16:13:49 +0000 Subject: [PATCH 03/19] Allow expired key in manualEnter recipe --- test/source/tests/page-recipe/setup-page-recipe.ts | 6 +++++- test/source/tests/setup.ts | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/test/source/tests/page-recipe/setup-page-recipe.ts b/test/source/tests/page-recipe/setup-page-recipe.ts index 789c087304c..de94e0016fe 100644 --- a/test/source/tests/page-recipe/setup-page-recipe.ts +++ b/test/source/tests/page-recipe/setup-page-recipe.ts @@ -21,7 +21,7 @@ type ManualEnterOpts = { fillOnly?: boolean, isInvalidKey?: boolean | undefined, checkEmailAliasIfPresent?: boolean, - key?: { title: string, passphrase: string, armored: string | null, longid: string | null, filePath?: string } + key?: TestKeyInfoWithFilepath }; type CreateKeyOpts = { @@ -191,6 +191,10 @@ export class SetupPageRecipe extends PageRecipe { await settingsPage.page.setOfflineMode(true); // offline mode } await settingsPage.waitAndClick('@input-step2bmanualenter-save', { delay: 1 }); + if (key.expired) { + await settingsPage.waitAndRespondToModal('confirm', 'confirm', 'You are importing a key that is expired.'); + await Util.sleep(1); + } if (fixKey) { await settingsPage.waitAll('@input-compatibility-fix-expire-years', { timeout: 30 }); await settingsPage.selectOption('@input-compatibility-fix-expire-years', '1'); diff --git a/test/source/tests/setup.ts b/test/source/tests/setup.ts index 64530ca80af..94bbe2ebd31 100644 --- a/test/source/tests/setup.ts +++ b/test/source/tests/setup.ts @@ -353,6 +353,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== -----END PGP PRIVATE KEY BLOCK-----`, passphrase: 'correct horse battery staple', longid: '123', + expired: true } }, { isSavePassphraseChecked: false, isSavePassphraseHidden: false }); await SettingsPageRecipe.toggleScreen(settingsPage, 'additional'); From a51eeea786f582ccd9dde51391c37da424553a59 Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Mon, 29 Aug 2022 07:18:20 +0000 Subject: [PATCH 04/19] Fixed test for setup redirection with passphrase --- test/source/tests/setup.ts | 74 +++++++++++++++++++++++--------------- 1 file changed, 46 insertions(+), 28 deletions(-) diff --git a/test/source/tests/setup.ts b/test/source/tests/setup.ts index 94bbe2ebd31..6b57315f889 100644 --- a/test/source/tests/setup.ts +++ b/test/source/tests/setup.ts @@ -733,34 +733,52 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== await gmailPage.close(); })); - ava.default('put.updating.key@key-manager-choose-passphrase-forbid-storing.flowcrypt.test - updates of key found on key manager via setup page', testWithBrowser(undefined, async (t, browser) => { - const acct = 'put.updating.key@key-manager-choose-passphrase-forbid-storing.flowcrypt.test'; - MOCK_KM_UPDATING_KEY[acct] = { response: { privateKeys: [{ decryptedPrivateKey: testConstants.updatingPrv }] } }; - const settingsPage = await BrowserRecipe.openSettingsLoginApprove(t, browser, acct); - const passphrase = 'long enough to suit requirements'; - await SetupPageRecipe.autoSetupWithEKM(settingsPage, { - enterPp: { passphrase, checks: { isSavePassphraseChecked: false, isSavePassphraseHidden: true } } - }); - const accessToken = await BrowserRecipe.getGoogleAccessToken(settingsPage, acct); - const extraAuthHeaders = { Authorization: `Bearer ${accessToken}` }; - const set1 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); - MOCK_KM_UPDATING_KEY[acct].response = { privateKeys: [] }; - // 1. EKM returns the empty set, forcing to auto-generate - let gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); - await PageRecipe.noToastAppears(gmailPage); - await gmailPage.notPresent('@dialog-passphrase'); - await gmailPage.close(); // todo: why settingsPage is losing focus? - await retrieveAndCheckKeys(settingsPage, acct, passphrase, 0); // no keys, auto-generation - delete MOCK_KM_LAST_INSERTED_KEY[acct]; - await SetupPageRecipe.autoSetupWithEKM(settingsPage, { - enterPp: { passphrase, checks: { isSavePassphraseChecked: false, isSavePassphraseHidden: true } } - }); - expect(MOCK_KM_LAST_INSERTED_KEY[acct]).to.exist; - const set2 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); - expect(set2[0].id).to.not.equal(set1[0].id); // entirely new key was generated - // wire the new key: const generatedKey = MOCK_KM_LAST_INSERTED_KEY[acct].privateKey; - // MOCK_KM_UPDATING_KEY[acct].response = { privateKeys: [{ decryptedPrivateKey: generatedKey }] }; - })); + ava.default('put.updating.key@key-manager-choose-passphrase-forbid-storing.flowcrypt.test - updates of key found on key manager via setup page (with passphrase)', + testWithBrowser(undefined, async (t, browser) => { + const acct = 'put.updating.key@key-manager-choose-passphrase-forbid-storing.flowcrypt.test'; + MOCK_KM_UPDATING_KEY[acct] = { response: { privateKeys: [{ decryptedPrivateKey: testConstants.updatingPrv }] } }; + const settingsPage = await BrowserRecipe.openSettingsLoginApprove(t, browser, acct); + const passphrase = 'long enough to suit requirements'; + await SetupPageRecipe.autoSetupWithEKM(settingsPage, { + enterPp: { passphrase, checks: { isSavePassphraseChecked: false, isSavePassphraseHidden: true } } + }); + const accessToken = await BrowserRecipe.getGoogleAccessToken(settingsPage, acct); + const extraAuthHeaders = { Authorization: `Bearer ${accessToken}` }; + const set1 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); + MOCK_KM_UPDATING_KEY[acct].response = { privateKeys: [] }; + // 1. EKM returns the empty set, forcing to auto-generate + let gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); + await PageRecipe.noToastAppears(gmailPage); + await gmailPage.notPresent('@dialog-passphrase'); + await gmailPage.close(); // todo: why settingsPage is losing focus? + await retrieveAndCheckKeys(settingsPage, acct, passphrase, 0); // no keys, auto-generation + delete MOCK_KM_LAST_INSERTED_KEY[acct]; + await SetupPageRecipe.autoSetupWithEKM(settingsPage, { + enterPp: { passphrase, checks: { isSavePassphraseChecked: false, isSavePassphraseHidden: true } } + }); + expect(MOCK_KM_LAST_INSERTED_KEY[acct]).to.exist; + const set2 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); + expect(set2[0].id).to.not.equal(set1[0].id); // entirely new key was generated + // 2. Adding a new key from the key manager when there is none in the storage + // First, erase the keys + gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); + await PageRecipe.noToastAppears(gmailPage); + await gmailPage.notPresent('@dialog-passphrase'); + await gmailPage.close(); + await retrieveAndCheckKeys(settingsPage, acct, passphrase, 0); // no keys, auto-generation + await settingsPage.close(); + MOCK_KM_UPDATING_KEY[acct] = { response: { privateKeys: [{ decryptedPrivateKey: testConstants.updatingPrv }] } }; + gmailPage = await browser.newPage(t, undefined, undefined, extraAuthHeaders); + const newSettingsPage = await browser.newPageTriggeredBy(t, () => gmailPage.goto(TestUrls.mockGmailUrl())); + await SetupPageRecipe.autoSetupWithEKM(newSettingsPage, { + enterPp: { passphrase, checks: { isSavePassphraseChecked: false, isSavePassphraseHidden: true } } + }); + expect(MOCK_KM_LAST_INSERTED_KEY[acct]).to.exist; + const set3 = await retrieveAndCheckKeys(newSettingsPage, acct, passphrase, 1); + expect(set3[0].id).to.equal(set1[0].id); // the key was received from the EKM + await newSettingsPage.close(); + await gmailPage.close(); + })); ava.default('get.key@key-manager-choose-passphrase.flowcrypt.test - passphrase chosen by user with key found on key manager', testWithBrowser(undefined, async (t, browser) => { const acct = 'get.key@key-manager-choose-passphrase.flowcrypt.test'; From a3f5850388c1447d848959732b51182f2a6aea2c Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Mon, 29 Aug 2022 08:45:36 +0000 Subject: [PATCH 05/19] lint fix --- .../webmail/setup-webmail-content-script.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/extension/js/content_scripts/webmail/setup-webmail-content-script.ts b/extension/js/content_scripts/webmail/setup-webmail-content-script.ts index 75c1066ebc8..bf4f6505687 100644 --- a/extension/js/content_scripts/webmail/setup-webmail-content-script.ts +++ b/extension/js/content_scripts/webmail/setup-webmail-content-script.ts @@ -246,7 +246,14 @@ export const contentScriptSetupIfVacant = async (webmailSpecific: WebmailSpecifi await factory.showPassphraseDialog(longids, type, initiatorFrameId); }; - const processKeysFromEkm = async (acctEmail: string, decryptedPrivateKeys: string[], clientConfiguration: ClientConfiguration, factory: XssSafeFactory, idToken: string, ppEvent: { entered?: boolean }) => { + const processKeysFromEkm = async ( + acctEmail: string, + decryptedPrivateKeys: string[], + clientConfiguration: ClientConfiguration, + factory: XssSafeFactory, + idToken: string, + ppEvent: { entered?: boolean } + ) => { try { const { needPassphrase, updateCount, needSetup } = await BrowserMsg.send.bg.await.processAndStoreKeysFromEkmLocally({ acctEmail, decryptedPrivateKeys }); From c887abf6a738caf620d80d913c176d51974d7723 Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Fri, 2 Sep 2022 16:06:19 +0000 Subject: [PATCH 06/19] NO_PRV_CREATE and quiet passphrase generation scenarios --- extension/js/common/browser/browser-msg.ts | 2 +- extension/js/common/helpers.ts | 15 ++- .../webmail/setup-webmail-content-script.ts | 4 +- test/source/tests/setup.ts | 95 ++++++++++++++----- 4 files changed, 85 insertions(+), 31 deletions(-) diff --git a/extension/js/common/browser/browser-msg.ts b/extension/js/common/browser/browser-msg.ts index d0240265d2e..e9c381213a8 100644 --- a/extension/js/common/browser/browser-msg.ts +++ b/extension/js/common/browser/browser-msg.ts @@ -87,7 +87,7 @@ export namespace Bm { export type AjaxGmailAttachmentGetChunk = { chunk: Buf }; export type _tab_ = { tabId: string | null | undefined }; export type SaveFetchedPubkeys = boolean; - export type ProcessAndStoreKeysFromEkmLocally = { needPassphrase?: boolean, updateCount?: number, needSetup?: boolean }; + export type ProcessAndStoreKeysFromEkmLocally = { needPassphrase?: boolean, updateCount?: number, noKeysSetup?: boolean }; export type Db = any; // not included in Any below export type Ajax = any; // not included in Any below diff --git a/extension/js/common/helpers.ts b/extension/js/common/helpers.ts index 53009eacc8f..1ef31f71826 100644 --- a/extension/js/common/helpers.ts +++ b/extension/js/common/helpers.ts @@ -11,6 +11,7 @@ import { ContactStore } from './platform/store/contact-store.js'; import { KeyStore } from './platform/store/key-store.js'; import { PassphraseStore } from './platform/store/passphrase-store.js'; import { Bm } from './browser/browser-msg.js'; +import { PgpPwd } from './core/crypto/pgp/pgp-password.js'; export const isFesUsed = async (acctEmail: string) => { const { fesUrl } = await AcctStore.get(acctEmail, ['fesUrl']); return Boolean(fesUrl); @@ -94,18 +95,24 @@ const filterKeysToSave = async (candidateKeys: Key[], existingKeys: KeyInfoWithI }; export const processAndStoreKeysFromEkmLocally = async ( - { acctEmail, decryptedPrivateKeys, ppOptions }: { acctEmail: string, decryptedPrivateKeys: string[], ppOptions?: PassphraseOptions } + { acctEmail, decryptedPrivateKeys, ppOptions: originalOptions }: { acctEmail: string, decryptedPrivateKeys: string[], ppOptions?: PassphraseOptions } ): Promise => { const { unencryptedPrvs } = await parseAndCheckPrivateKeys(decryptedPrivateKeys); const existingKeys = await KeyStore.get(acctEmail); let { keysToRetain, unencryptedKeysToSave } = await filterKeysToSave(unencryptedPrvs, existingKeys); if (!unencryptedKeysToSave.length && keysToRetain.length === existingKeys.length) { // nothing to update - return { needPassphrase: false, needSetup: !existingKeys.length }; + return { needPassphrase: false, noKeysSetup: !existingKeys.length }; + } + let ppOptions: PassphraseOptions | undefined; // the options to pass to saveKeysAndPassPhrase + if (!originalOptions?.passphrase && (await ClientConfiguration.newInstance(acctEmail)).mustAutogenPassPhraseQuietly()) { + ppOptions = { passphrase: PgpPwd.random(), passphrase_save: true }; + } else { + ppOptions = originalOptions; } let passphrase = ppOptions?.passphrase; if (passphrase === undefined && !existingKeys.length) { - return { needPassphrase: true, needSetup: true }; + return { needPassphrase: true, noKeysSetup: true }; } let encryptedKeys: Key[] = []; if (unencryptedKeysToSave.length) { @@ -126,7 +133,7 @@ export const processAndStoreKeysFromEkmLocally = async ( // also updates `name`, todo: refactor in #4545 const newKeyset = keysToRetain.concat(encryptedKeys); await saveKeysAndPassPhrase(acctEmail, newKeyset, ppOptions, true); - return { updateCount: encryptedKeys.length + (existingKeys.length - keysToRetain.length), needSetup: !newKeyset.length }; + return { updateCount: encryptedKeys.length + (existingKeys.length - keysToRetain.length), noKeysSetup: !newKeyset.length }; } else { // todo: should we delete? return { needPassphrase: unencryptedKeysToSave.length > 0 }; diff --git a/extension/js/content_scripts/webmail/setup-webmail-content-script.ts b/extension/js/content_scripts/webmail/setup-webmail-content-script.ts index bf4f6505687..0594adca92a 100644 --- a/extension/js/content_scripts/webmail/setup-webmail-content-script.ts +++ b/extension/js/content_scripts/webmail/setup-webmail-content-script.ts @@ -255,9 +255,9 @@ export const contentScriptSetupIfVacant = async (webmailSpecific: WebmailSpecifi ppEvent: { entered?: boolean } ) => { try { - const { needPassphrase, updateCount, needSetup } = + const { needPassphrase, updateCount, noKeysSetup } = await BrowserMsg.send.bg.await.processAndStoreKeysFromEkmLocally({ acctEmail, decryptedPrivateKeys }); - if (needSetup) { + if (noKeysSetup) { if (!needPassphrase && !clientConfiguration.canCreateKeys()) { await Ui.modal.error(Lang.setup.noKeys); BrowserMsg.send.bg.settings({ acctEmail, path: 'index.htm' }); diff --git a/test/source/tests/setup.ts b/test/source/tests/setup.ts index 6b57315f889..b0e33a816cf 100644 --- a/test/source/tests/setup.ts +++ b/test/source/tests/setup.ts @@ -559,7 +559,15 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== await securityFrame.notPresent(['@action-change-passphrase-begin', '@action-test-passphrase-begin', '@action-forget-pp']); })); - const retrieveAndCheckKeys = async (page: ControllablePage, acctEmail: string, passphrase: string, expectedKeyCount: number) => { + const getPassphraseFromStorage = async (page: ControllablePage, acctEmail: string, longid: string) => { + const key = `cryptup_${emailKeyIndex(acctEmail, 'passphrase')}_${longid}`; + const passphrase = (await page.getFromLocalStorage([key]))[key]; + expect(passphrase).to.be.a.string; + expect(passphrase).to.be.not.empty; + return passphrase as string; + }; + + const retrieveAndCheckKeys = async (page: ControllablePage, acctEmail: string, expectedKeyCount: number, passphrase?: string) => { const key = `cryptup_${emailKeyIndex(acctEmail, 'keys')}`; const keyset = (await page.getFromLocalStorage([key]))[key]; const kis = keyset as KeyInfoWithIdentity[]; @@ -567,7 +575,8 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== return await Promise.all(kis.map(async ki => { const prv = await KeyUtil.parse(ki.private); expect(prv.fullyEncrypted).to.be.true; - expect(await KeyUtil.decrypt(prv, passphrase, undefined, undefined)).to.be.true; + const passphraseToDecrypt = passphrase || await getPassphraseFromStorage(page, acctEmail, KeyUtil.getPrimaryLongid(prv)); + expect(await KeyUtil.decrypt(prv, passphraseToDecrypt, undefined, undefined)).to.be.true; expect(prv.lastModified).to.not.be.an.undefined; return prv; })); @@ -586,12 +595,12 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== const updateAndArmorKey = async (prv: Key) => { return KeyUtil.armor(await KeyUtil.reformatKey(prv, undefined, [{ name: 'Full Name', email: acct }], 6000)); }; - const set1 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); + const set1 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); // 1. EKM returns the same key, no update, no toast let gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await PageRecipe.noToastAppears(gmailPage); await gmailPage.notPresent('@dialog-passphrase'); - const set2 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); + const set2 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); expect(set2[0].lastModified).to.equal(set1[0].lastModified); // no update await gmailPage.close(); // 2. EKM returns a newer version of the existing key @@ -599,14 +608,14 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== MOCK_KM_UPDATING_KEY[acct].response = { privateKeys: [{ decryptedPrivateKey: someOlderVersion }] }; gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await PageRecipe.waitForToastToAppearAndDisappear(gmailPage, 'Account keys updated'); - const set3 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); + const set3 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); expect(set3[0].lastModified!).to.be.greaterThan(set2[0].lastModified!); // an update happened await gmailPage.close(); // 3. EKM returns the same version of the existing key, no toast, no update gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await PageRecipe.noToastAppears(gmailPage); await gmailPage.notPresent('@dialog-passphrase'); - const set4 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); + const set4 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); expect(set4[0].lastModified).to.equal(set3[0].lastModified); // no update // 4. Forget the passphrase, EKM the same version of the existing key, no prompt await InboxPageRecipe.finishSessionOnInboxPage(gmailPage); @@ -614,7 +623,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await PageRecipe.noToastAppears(gmailPage); await gmailPage.notPresent('@dialog-passphrase'); - const set5 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); + const set5 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); expect(set5[0].lastModified).to.equal(set4[0].lastModified); // no update await gmailPage.close(); // 5. EKM returns a newer version of the existing key, canceling passphrase prompt, no update @@ -623,7 +632,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== await gmailPage.waitAll('@dialog-passphrase'); await ComposePageRecipe.cancelPassphraseDialog(gmailPage, 'keyboard'); await PageRecipe.noToastAppears(gmailPage); - const set6 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); + const set6 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); expect(set6[0].lastModified).to.equal(set5[0].lastModified); // no update await gmailPage.close(); // 6. EKM returns a newer version of the existing key, entering the passphrase, update toast @@ -637,7 +646,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== } await gmailPage.waitTillGone('@dialog-passphrase'); await PageRecipe.waitForToastToAppearAndDisappear(gmailPage, 'Account keys updated'); - const set7 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); + const set7 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); expect(set7[0].lastModified!).to.be.greaterThan(set6[0].lastModified!); // an update happened await gmailPage.close(); // 7. EKM returns an older version of the existing key, no toast, no update @@ -645,7 +654,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await PageRecipe.noToastAppears(gmailPage); await gmailPage.notPresent('@dialog-passphrase'); - const set8 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); + const set8 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); expect(set8[0].lastModified).to.equal(set7[0].lastModified); // no update await gmailPage.close(); // 8. EKM returns an older version of the existing key, and a new key, toast, new key gets added encrypted with the same passphrase @@ -653,18 +662,19 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await PageRecipe.waitForToastToAppearAndDisappear(gmailPage, 'Account keys updated'); await gmailPage.notPresent('@dialog-passphrase'); - const set9 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 2); + const set9 = await retrieveAndCheckKeys(settingsPage, acct, 2, passphrase); const mainKey9 = KeyUtil.filterKeysByIdentity(set9, [{ family: 'openpgp', id: '392FB1E9FF4184659AB6A246835C0141B9ECF536' }]); expect(mainKey9.length).to.equal(1); expect(KeyUtil.filterKeysByIdentity(set9, [{ family: 'openpgp', id: 'FAFB7D675AC74E87F84D169F00B0115807969D75' }]).length).to.equal(1); expect(mainKey9[0].lastModified).to.equal(set8[0].lastModified); // no update await gmailPage.close(); - // 9. EKM returns a newer version of one key, fully omitting the other one, a toast, and update, no removal + // 9. EKM returns a newer version of one key, fully omitting the other one, a toast, an update and removal MOCK_KM_UPDATING_KEY[acct].response = { privateKeys: [{ decryptedPrivateKey: await updateAndArmorKey(mainKey9[0]) }] }; gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await PageRecipe.waitForToastToAppearAndDisappear(gmailPage, 'Account keys updated'); await gmailPage.notPresent('@dialog-passphrase'); - const set10 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); + const set10 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); + // todo: check that the passphrase from the removed key is deleted from session? const mainKey10 = KeyUtil.filterKeysByIdentity(set10, [{ family: 'openpgp', id: '392FB1E9FF4184659AB6A246835C0141B9ECF536' }]); expect(mainKey10.length).to.equal(1); expect(mainKey10[0].lastModified!).to.be.greaterThan(mainKey9[0].lastModified!); // updated this key @@ -684,10 +694,10 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== } await ComposePageRecipe.cancelPassphraseDialog(gmailPage, 'keyboard'); await PageRecipe.noToastAppears(gmailPage); - const set11 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); + const set11 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); expect(set11.map(entry => entry.id)).to.eql(['392FB1E9FF4184659AB6A246835C0141B9ECF536']); await gmailPage.close(); - // 11. EKM returns a new third key, we enter a passphrase matching an existing key, update happens + // 11. EKM returns a new third key, we enter a passphrase matching an existing key, update happens, the old key is removed gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await gmailPage.waitAll('@dialog-passphrase'); { @@ -698,7 +708,8 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== } await gmailPage.waitTillGone('@dialog-passphrase'); await PageRecipe.waitForToastToAppearAndDisappear(gmailPage, 'Account keys updated'); - const set12 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); + const set12 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); + // todo: check that the passphrase from the removed key is deleted from session? expect(set12.map(entry => entry.id)).to.eql(['277D1ADA213881F4ABE0415395E783DC0289E2E2']); const mainKey12 = KeyUtil.filterKeysByIdentity(set12, [{ family: 'openpgp', id: '277D1ADA213881F4ABE0415395E783DC0289E2E2' }]); expect(mainKey12.length).to.equal(1); @@ -714,7 +725,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== await PageRecipe.waitForToastToAppearAndDisappear(gmailPage, 'Could not update keys from EKM due to error: BrowserMsg(processAndStoreKeysFromEkmLocally) sendRawResponse::Error: Some keys could not be parsed'); await gmailPage.notPresent('@dialog-passphrase'); - const set13 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); + const set13 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); expect(set13.map(entry => entry.id)).to.eql(['277D1ADA213881F4ABE0415395E783DC0289E2E2']); const mainKey13 = KeyUtil.filterKeysByIdentity(set13, [{ family: 'openpgp', id: '277D1ADA213881F4ABE0415395E783DC0289E2E2' }]); expect(mainKey13.length).to.equal(1); @@ -725,7 +736,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await PageRecipe.noToastAppears(gmailPage); await gmailPage.notPresent('@dialog-passphrase'); - const set14 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); + const set14 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); expect(set14.map(entry => entry.id)).to.eql(['277D1ADA213881F4ABE0415395E783DC0289E2E2']); const mainKey14 = KeyUtil.filterKeysByIdentity(set14.map(ki => ki), [{ family: 'openpgp', id: '277D1ADA213881F4ABE0415395E783DC0289E2E2' }]); expect(mainKey14.length).to.equal(1); @@ -744,20 +755,24 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== }); const accessToken = await BrowserRecipe.getGoogleAccessToken(settingsPage, acct); const extraAuthHeaders = { Authorization: `Bearer ${accessToken}` }; - const set1 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); + const set1 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); MOCK_KM_UPDATING_KEY[acct].response = { privateKeys: [] }; // 1. EKM returns the empty set, forcing to auto-generate let gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); + // The new settingsPage is loaded in place of the existing settings tab (this is by design) + // However, after a second the newly-activated (old) settings tab loses focus in favour of the gmailPage, why is that? + // Is this only in Puppetteer? await PageRecipe.noToastAppears(gmailPage); await gmailPage.notPresent('@dialog-passphrase'); - await gmailPage.close(); // todo: why settingsPage is losing focus? - await retrieveAndCheckKeys(settingsPage, acct, passphrase, 0); // no keys, auto-generation + await gmailPage.close(); + await retrieveAndCheckKeys(settingsPage, acct, 0, passphrase); // no keys, auto-generation + // todo: check that passphrase(s) are deleted from session? delete MOCK_KM_LAST_INSERTED_KEY[acct]; await SetupPageRecipe.autoSetupWithEKM(settingsPage, { enterPp: { passphrase, checks: { isSavePassphraseChecked: false, isSavePassphraseHidden: true } } }); expect(MOCK_KM_LAST_INSERTED_KEY[acct]).to.exist; - const set2 = await retrieveAndCheckKeys(settingsPage, acct, passphrase, 1); + const set2 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); expect(set2[0].id).to.not.equal(set1[0].id); // entirely new key was generated // 2. Adding a new key from the key manager when there is none in the storage // First, erase the keys @@ -765,7 +780,8 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== await PageRecipe.noToastAppears(gmailPage); await gmailPage.notPresent('@dialog-passphrase'); await gmailPage.close(); - await retrieveAndCheckKeys(settingsPage, acct, passphrase, 0); // no keys, auto-generation + await retrieveAndCheckKeys(settingsPage, acct, 0, passphrase); // no keys, auto-generation + // todo: check that passphrase(s) are deleted from session? await settingsPage.close(); MOCK_KM_UPDATING_KEY[acct] = { response: { privateKeys: [{ decryptedPrivateKey: testConstants.updatingPrv }] } }; gmailPage = await browser.newPage(t, undefined, undefined, extraAuthHeaders); @@ -774,12 +790,43 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== enterPp: { passphrase, checks: { isSavePassphraseChecked: false, isSavePassphraseHidden: true } } }); expect(MOCK_KM_LAST_INSERTED_KEY[acct]).to.exist; - const set3 = await retrieveAndCheckKeys(newSettingsPage, acct, passphrase, 1); + const set3 = await retrieveAndCheckKeys(newSettingsPage, acct, 1, passphrase); expect(set3[0].id).to.equal(set1[0].id); // the key was received from the EKM await newSettingsPage.close(); await gmailPage.close(); })); + ava.default('get.updating.key@key-manager-autoimport-no-prv-create.flowcrypt.test - updates of key found on key manager when NO_PRV_CREATE', + testWithBrowser(undefined, async (t, browser) => { + const acct = 'get.updating.key@key-manager-autoimport-no-prv-create.flowcrypt.test'; + MOCK_KM_UPDATING_KEY[acct] = { response: { privateKeys: [{ decryptedPrivateKey: testConstants.updatingPrv }] } }; + const settingsPage = await BrowserRecipe.openSettingsLoginApprove(t, browser, acct); + await SetupPageRecipe.autoSetupWithEKM(settingsPage); + const accessToken = await BrowserRecipe.getGoogleAccessToken(settingsPage, acct); + const extraAuthHeaders = { Authorization: `Bearer ${accessToken}` }; + const set1 = await retrieveAndCheckKeys(settingsPage, acct, 1); + MOCK_KM_UPDATING_KEY[acct].response = { privateKeys: [] }; + // 1. EKM returns the empty set, auto-generation is not allowed, hence the error modal + let gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); + await gmailPage.waitAndRespondToModal('error', 'confirm', 'Keys for your account were not set up yet - please ask your systems administrator'); await PageRecipe.noToastAppears(gmailPage); + await gmailPage.close(); + await retrieveAndCheckKeys(settingsPage, acct, 0); // no keys + // todo: check that the passphrase is deleted from storage? + await settingsPage.close(); + // 2. Adding a new key from the key manager when there is none in the storage + MOCK_KM_UPDATING_KEY[acct] = { response: { privateKeys: [{ decryptedPrivateKey: testConstants.updatingPrv }] } }; + gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); + await PageRecipe.waitForToastToAppearAndDisappear(gmailPage, 'Account keys updated'); + await gmailPage.close(); + const dbPage = await browser.newPage(t, TestUrls.extension('chrome/dev/ci_unit_test.htm')); + const set2 = await retrieveAndCheckKeys(dbPage, acct, 1); + expect(set2[0].id).to.equal(set1[0].id); // the key was received from the EKM + await dbPage.close(); + })); + + ava.default.todo('DEFAULT_REMEMBER_PASS_PHRASE with auto-generation when all keys are removed by EKM'); + // should we re-use the known passphrase or delete it from the storage in this scenario? + ava.default('get.key@key-manager-choose-passphrase.flowcrypt.test - passphrase chosen by user with key found on key manager', testWithBrowser(undefined, async (t, browser) => { const acct = 'get.key@key-manager-choose-passphrase.flowcrypt.test'; const passphrase = 'Long and complicated pass PHRASE'; From a198b2d1a0c19d8c0c0f293651b8e10848c02cc1 Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Fri, 2 Sep 2022 18:28:50 +0000 Subject: [PATCH 07/19] nicety --- extension/js/common/helpers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extension/js/common/helpers.ts b/extension/js/common/helpers.ts index 1ef31f71826..1fad35ad2cb 100644 --- a/extension/js/common/helpers.ts +++ b/extension/js/common/helpers.ts @@ -95,7 +95,7 @@ const filterKeysToSave = async (candidateKeys: Key[], existingKeys: KeyInfoWithI }; export const processAndStoreKeysFromEkmLocally = async ( - { acctEmail, decryptedPrivateKeys, ppOptions: originalOptions }: { acctEmail: string, decryptedPrivateKeys: string[], ppOptions?: PassphraseOptions } + { acctEmail, decryptedPrivateKeys, ppOptions: originalOptions }: Bm.ProcessAndStoreKeysFromEkmLocally & { ppOptions?: PassphraseOptions } ): Promise => { const { unencryptedPrvs } = await parseAndCheckPrivateKeys(decryptedPrivateKeys); const existingKeys = await KeyStore.get(acctEmail); From e6835ee45ebabc21fa25fe102f7f946444eadaf8 Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Sat, 10 Sep 2022 19:26:00 +0000 Subject: [PATCH 08/19] remove passphrases and check passphrase from stored in session --- extension/js/common/helpers.ts | 7 ++- .../common/platform/store/in-memory-store.ts | 2 +- .../common/platform/store/passphrase-store.ts | 10 +++- .../tests/page-recipe/abstract-page-recipe.ts | 2 +- test/source/tests/setup.ts | 46 +++++++++---------- test/source/tests/tooling/browser-recipe.ts | 9 ++++ 6 files changed, 48 insertions(+), 28 deletions(-) diff --git a/extension/js/common/helpers.ts b/extension/js/common/helpers.ts index 1fad35ad2cb..8369f4e1408 100644 --- a/extension/js/common/helpers.ts +++ b/extension/js/common/helpers.ts @@ -20,8 +20,12 @@ export const isFesUsed = async (acctEmail: string) => { export const saveKeysAndPassPhrase = async (acctEmail: string, prvs: Key[], ppOptions?: PassphraseOptions, replaceKeys: boolean = false) => { const clientConfiguration = await ClientConfiguration.newInstance(acctEmail); if (replaceKeys) { + // track longids to remove related passhprases + const existingKeys = await KeyStore.get(acctEmail); + const deletedKeys = existingKeys.filter(old => !prvs.some(prvIdentity => KeyUtil.identityEquals(prvIdentity, old))); + // set actually replaces the set of keys in storage with the new set await KeyStore.set(acctEmail, await Promise.all(prvs.map(KeyUtil.keyInfoObj))); // todo: duplicate identities - // todo: should we delete passphrases matching the deleted keys? + await PassphraseStore.removeMany(acctEmail, deletedKeys); } for (const prv of prvs) { if (!replaceKeys) { @@ -123,7 +127,6 @@ export const processAndStoreKeysFromEkmLocally = async ( } if (passphrase !== undefined) { const pp = passphrase; - // todo: some more fancy conversion, preserving a passphrase for a particual longid? await Promise.all(unencryptedKeysToSave.map(prv => KeyUtil.encrypt(prv, pp))); encryptedKeys = unencryptedKeysToSave; unencryptedKeysToSave = []; diff --git a/extension/js/common/platform/store/in-memory-store.ts b/extension/js/common/platform/store/in-memory-store.ts index e5c94f73d4a..bfdb7ceed24 100644 --- a/extension/js/common/platform/store/in-memory-store.ts +++ b/extension/js/common/platform/store/in-memory-store.ts @@ -4,7 +4,7 @@ import { AbstractStore } from './abstract-store.js'; import { BrowserMsg } from '../../browser/browser-msg.js'; /** - * Temporrary In-Memory store for sensitive values, expiring after 4 hours + * Temporary In-Memory store for sensitive values, expiring after 4 hours * see background_page.ts for the other end, also ExpirationCache class */ export class InMemoryStore extends AbstractStore { diff --git a/extension/js/common/platform/store/passphrase-store.ts b/extension/js/common/platform/store/passphrase-store.ts index 0dd56386feb..4c22bfc5e3d 100644 --- a/extension/js/common/platform/store/passphrase-store.ts +++ b/extension/js/common/platform/store/passphrase-store.ts @@ -22,6 +22,14 @@ export class PassphraseStore extends AbstractStore { return await PassphraseStore.getByIndexes(acctEmail, storageIndexes, ignoreSession); }; + public static removeMany = async (acctEmail: string, keyInfos: { longid: string }[]) => { + const storageIndexes = keyInfos.map(keyInfo => PassphraseStore.getIndex(keyInfo.longid)); + await Promise.all([ + AcctStore.remove(acctEmail, storageIndexes), // remove from local storage + ...storageIndexes.map(storageIndex => InMemoryStore.set(acctEmail, storageIndex, undefined)) // remove from session + ]); + }; + // if we implement (and migrate) password storage to use KeyIdentity instead of longid, we'll have `keyInfo: KeyIdentity` here public static set = async (storageType: StorageType, acctEmail: string, keyInfo: { longid: string }, passphrase: string | undefined): Promise => { const storageIndex = PassphraseStore.getIndex(keyInfo.longid); @@ -78,7 +86,7 @@ export class PassphraseStore extends AbstractStore { await AcctStore.remove(acctEmail, [storageIndex]); } else { const toSave: AcctStoreDict = {}; - toSave[storageIndex] = passphrase as any; + (toSave as Dict)[storageIndex] = passphrase; await AcctStore.set(acctEmail, toSave); } } diff --git a/test/source/tests/page-recipe/abstract-page-recipe.ts b/test/source/tests/page-recipe/abstract-page-recipe.ts index b94849be86f..22d86e0b88b 100644 --- a/test/source/tests/page-recipe/abstract-page-recipe.ts +++ b/test/source/tests/page-recipe/abstract-page-recipe.ts @@ -47,7 +47,7 @@ export abstract class PageRecipe { } }; - public static sendMessage = async (controllable: Controllable, msg: any) => { + public static sendMessage = async (controllable: Controllable, msg: unknown) => { return await controllable.target.evaluate(async (msg) => await new Promise((resolve) => { chrome.runtime.sendMessage(msg, resolve); }), msg); diff --git a/test/source/tests/setup.ts b/test/source/tests/setup.ts index 651f694b320..374021e4c24 100644 --- a/test/source/tests/setup.ts +++ b/test/source/tests/setup.ts @@ -567,11 +567,10 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== await securityFrame.notPresent(['@action-change-passphrase-begin', '@action-test-passphrase-begin', '@action-forget-pp']); })); - const getPassphraseFromStorage = async (page: ControllablePage, acctEmail: string, longid: string) => { + const getPassphrase = async (page: ControllablePage, acctEmail: string, longid: string) => { const key = `cryptup_${emailKeyIndex(acctEmail, 'passphrase')}_${longid}`; - const passphrase = (await page.getFromLocalStorage([key]))[key]; + const passphrase = (await page.getFromLocalStorage([key]))[key] || await BrowserRecipe.getFromInMemoryStore(page, acctEmail, `passphrase_${longid}`); expect(passphrase).to.be.a.string; - expect(passphrase).to.be.not.empty; return passphrase as string; }; @@ -583,7 +582,8 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== return await Promise.all(kis.map(async ki => { const prv = await KeyUtil.parse(ki.private); expect(prv.fullyEncrypted).to.be.true; - const passphraseToDecrypt = passphrase || await getPassphraseFromStorage(page, acctEmail, KeyUtil.getPrimaryLongid(prv)); + const passphraseToDecrypt = passphrase || await getPassphrase(page, acctEmail, KeyUtil.getPrimaryLongid(prv)); + expect(passphraseToDecrypt).to.be.not.empty; expect(await KeyUtil.decrypt(prv, passphraseToDecrypt, undefined, undefined)).to.be.true; expect(prv.lastModified).to.not.be.an.undefined; return prv; @@ -603,12 +603,12 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== const updateAndArmorKey = async (prv: Key) => { return KeyUtil.armor(await KeyUtil.reformatKey(prv, undefined, [{ name: 'Full Name', email: acct }], 6000)); }; - const set1 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); + const set1 = await retrieveAndCheckKeys(settingsPage, acct, 1); // 1. EKM returns the same key, no update, no toast let gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await PageRecipe.noToastAppears(gmailPage); await gmailPage.notPresent('@dialog-passphrase'); - const set2 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); + const set2 = await retrieveAndCheckKeys(settingsPage, acct, 1); expect(set2[0].lastModified).to.equal(set1[0].lastModified); // no update await gmailPage.close(); // 2. EKM returns a newer version of the existing key @@ -616,7 +616,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== MOCK_KM_UPDATING_KEY[acct].response = { privateKeys: [{ decryptedPrivateKey: someOlderVersion }] }; gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await PageRecipe.waitForToastToAppearAndDisappear(gmailPage, 'Account keys updated'); - const set3 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); + const set3 = await retrieveAndCheckKeys(settingsPage, acct, 1); expect(set3[0].lastModified!).to.be.greaterThan(set2[0].lastModified!); // an update happened await gmailPage.close(); // 3. EKM returns the same version of the existing key, no toast, no update @@ -654,7 +654,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== } await gmailPage.waitTillGone('@dialog-passphrase'); await PageRecipe.waitForToastToAppearAndDisappear(gmailPage, 'Account keys updated'); - const set7 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); + const set7 = await retrieveAndCheckKeys(settingsPage, acct, 1); expect(set7[0].lastModified!).to.be.greaterThan(set6[0].lastModified!); // an update happened await gmailPage.close(); // 7. EKM returns an older version of the existing key, no toast, no update @@ -662,7 +662,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await PageRecipe.noToastAppears(gmailPage); await gmailPage.notPresent('@dialog-passphrase'); - const set8 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); + const set8 = await retrieveAndCheckKeys(settingsPage, acct, 1); expect(set8[0].lastModified).to.equal(set7[0].lastModified); // no update await gmailPage.close(); // 8. EKM returns an older version of the existing key, and a new key, toast, new key gets added encrypted with the same passphrase @@ -670,7 +670,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await PageRecipe.waitForToastToAppearAndDisappear(gmailPage, 'Account keys updated'); await gmailPage.notPresent('@dialog-passphrase'); - const set9 = await retrieveAndCheckKeys(settingsPage, acct, 2, passphrase); + const set9 = await retrieveAndCheckKeys(settingsPage, acct, 2); const mainKey9 = KeyUtil.filterKeysByIdentity(set9, [{ family: 'openpgp', id: '392FB1E9FF4184659AB6A246835C0141B9ECF536' }]); expect(mainKey9.length).to.equal(1); expect(KeyUtil.filterKeysByIdentity(set9, [{ family: 'openpgp', id: 'FAFB7D675AC74E87F84D169F00B0115807969D75' }]).length).to.equal(1); @@ -681,7 +681,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await PageRecipe.waitForToastToAppearAndDisappear(gmailPage, 'Account keys updated'); await gmailPage.notPresent('@dialog-passphrase'); - const set10 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); + const set10 = await retrieveAndCheckKeys(settingsPage, acct, 1); // todo: check that the passphrase from the removed key is deleted from session? const mainKey10 = KeyUtil.filterKeysByIdentity(set10, [{ family: 'openpgp', id: '392FB1E9FF4184659AB6A246835C0141B9ECF536' }]); expect(mainKey10.length).to.equal(1); @@ -702,7 +702,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== } await ComposePageRecipe.cancelPassphraseDialog(gmailPage, 'keyboard'); await PageRecipe.noToastAppears(gmailPage); - const set11 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); + const set11 = await retrieveAndCheckKeys(settingsPage, acct, 1); expect(set11.map(entry => entry.id)).to.eql(['392FB1E9FF4184659AB6A246835C0141B9ECF536']); await gmailPage.close(); // 11. EKM returns a new third key, we enter a passphrase matching an existing key, update happens, the old key is removed @@ -716,7 +716,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== } await gmailPage.waitTillGone('@dialog-passphrase'); await PageRecipe.waitForToastToAppearAndDisappear(gmailPage, 'Account keys updated'); - const set12 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); + const set12 = await retrieveAndCheckKeys(settingsPage, acct, 1); // todo: check that the passphrase from the removed key is deleted from session? expect(set12.map(entry => entry.id)).to.eql(['277D1ADA213881F4ABE0415395E783DC0289E2E2']); const mainKey12 = KeyUtil.filterKeysByIdentity(set12, [{ family: 'openpgp', id: '277D1ADA213881F4ABE0415395E783DC0289E2E2' }]); @@ -733,7 +733,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== await PageRecipe.waitForToastToAppearAndDisappear(gmailPage, 'Could not update keys from EKM due to error: BrowserMsg(processAndStoreKeysFromEkmLocally) sendRawResponse::Error: Some keys could not be parsed'); await gmailPage.notPresent('@dialog-passphrase'); - const set13 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); + const set13 = await retrieveAndCheckKeys(settingsPage, acct, 1); expect(set13.map(entry => entry.id)).to.eql(['277D1ADA213881F4ABE0415395E783DC0289E2E2']); const mainKey13 = KeyUtil.filterKeysByIdentity(set13, [{ family: 'openpgp', id: '277D1ADA213881F4ABE0415395E783DC0289E2E2' }]); expect(mainKey13.length).to.equal(1); @@ -744,7 +744,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await PageRecipe.noToastAppears(gmailPage); await gmailPage.notPresent('@dialog-passphrase'); - const set14 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); + const set14 = await retrieveAndCheckKeys(settingsPage, acct, 1); expect(set14.map(entry => entry.id)).to.eql(['277D1ADA213881F4ABE0415395E783DC0289E2E2']); const mainKey14 = KeyUtil.filterKeysByIdentity(set14.map(ki => ki), [{ family: 'openpgp', id: '277D1ADA213881F4ABE0415395E783DC0289E2E2' }]); expect(mainKey14.length).to.equal(1); @@ -763,24 +763,24 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== }); const accessToken = await BrowserRecipe.getGoogleAccessToken(settingsPage, acct); const extraAuthHeaders = { Authorization: `Bearer ${accessToken}` }; - const set1 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); + const set1 = await retrieveAndCheckKeys(settingsPage, acct, 1); MOCK_KM_UPDATING_KEY[acct].response = { privateKeys: [] }; // 1. EKM returns the empty set, forcing to auto-generate let gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); // The new settingsPage is loaded in place of the existing settings tab (this is by design) // However, after a second the newly-activated (old) settings tab loses focus in favour of the gmailPage, why is that? - // Is this only in Puppetteer? + // Looks like Puppeteer's misbehaviour await PageRecipe.noToastAppears(gmailPage); await gmailPage.notPresent('@dialog-passphrase'); await gmailPage.close(); - await retrieveAndCheckKeys(settingsPage, acct, 0, passphrase); // no keys, auto-generation - // todo: check that passphrase(s) are deleted from session? + await retrieveAndCheckKeys(settingsPage, acct, 0); // no keys, auto-generation + expect(await getPassphrase(settingsPage, acct, KeyUtil.getPrimaryLongid(set1[0]))).to.be.an.undefined; // the passphrase for the old key was deleted delete MOCK_KM_LAST_INSERTED_KEY[acct]; await SetupPageRecipe.autoSetupWithEKM(settingsPage, { enterPp: { passphrase, checks: { isSavePassphraseChecked: false, isSavePassphraseHidden: true } } }); expect(MOCK_KM_LAST_INSERTED_KEY[acct]).to.exist; - const set2 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); + const set2 = await retrieveAndCheckKeys(settingsPage, acct, 1); expect(set2[0].id).to.not.equal(set1[0].id); // entirely new key was generated // 2. Adding a new key from the key manager when there is none in the storage // First, erase the keys @@ -788,8 +788,8 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== await PageRecipe.noToastAppears(gmailPage); await gmailPage.notPresent('@dialog-passphrase'); await gmailPage.close(); - await retrieveAndCheckKeys(settingsPage, acct, 0, passphrase); // no keys, auto-generation - // todo: check that passphrase(s) are deleted from session? + await retrieveAndCheckKeys(settingsPage, acct, 0); // no keys, auto-generation + expect(await getPassphrase(settingsPage, acct, KeyUtil.getPrimaryLongid(set2[0]))).to.be.an.undefined; // the passphrase for the old key was deleted await settingsPage.close(); MOCK_KM_UPDATING_KEY[acct] = { response: { privateKeys: [{ decryptedPrivateKey: testConstants.updatingPrv }] } }; gmailPage = await browser.newPage(t, undefined, undefined, extraAuthHeaders); @@ -798,7 +798,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== enterPp: { passphrase, checks: { isSavePassphraseChecked: false, isSavePassphraseHidden: true } } }); expect(MOCK_KM_LAST_INSERTED_KEY[acct]).to.exist; - const set3 = await retrieveAndCheckKeys(newSettingsPage, acct, 1, passphrase); + const set3 = await retrieveAndCheckKeys(newSettingsPage, acct, 1); expect(set3[0].id).to.equal(set1[0].id); // the key was received from the EKM await newSettingsPage.close(); await gmailPage.close(); diff --git a/test/source/tests/tooling/browser-recipe.ts b/test/source/tests/tooling/browser-recipe.ts index c1666f3be6f..65011e7ba99 100644 --- a/test/source/tests/tooling/browser-recipe.ts +++ b/test/source/tests/tooling/browser-recipe.ts @@ -100,6 +100,15 @@ export class BrowserRecipe { return (result as { result: string }).result; }; + public static getFromInMemoryStore = async (controllable: Controllable, acctEmail: string, key: string): Promise => { + const result = await PageRecipe.sendMessage(controllable, { + name: 'inMemoryStoreGet', + // tslint:disable-next-line:no-null-keyword + data: { bm: { acctEmail, key }, objUrls: {} }, to: null, uid: '2' // todo: random uid? + }); + return (result as { result: string }).result; + }; + public static deleteAllDraftsInGmailAccount = async (settingsPage: ControllablePage): Promise => { const accessToken = await BrowserRecipe.getGoogleAccessToken(settingsPage, 'ci.tests.gmail@flowcrypt.dev'); const gmail = google.gmail({ version: 'v1' }); From 819942934ba0ab4058ea1a6467deb56c22ca380a Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Sun, 11 Sep 2022 10:21:06 +0000 Subject: [PATCH 09/19] renamed `candidate` to `candidateKey` --- extension/js/common/helpers.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/extension/js/common/helpers.ts b/extension/js/common/helpers.ts index 8369f4e1408..1937bbe7532 100644 --- a/extension/js/common/helpers.ts +++ b/extension/js/common/helpers.ts @@ -81,19 +81,19 @@ const filterKeysToSave = async (candidateKeys: Key[], existingKeys: KeyInfoWithI } const keysToRetain: Key[] = []; const unencryptedKeysToSave: Key[] = []; - for (const candidate of candidateKeys) { - const longid = KeyUtil.getPrimaryLongid(candidate); - const keyToUpdate = existingKeys.filter(ki => ki.longid === longid && ki.family === candidate.family); + for (const candidateKey of candidateKeys) { + const longid = KeyUtil.getPrimaryLongid(candidateKey); + const keyToUpdate = existingKeys.filter(ki => ki.longid === longid && ki.family === candidateKey.family); if (keyToUpdate.length === 1) { const oldKey = await KeyUtil.parse(keyToUpdate[0].private); - if (!candidate.lastModified || (oldKey.lastModified && oldKey.lastModified >= candidate.lastModified)) { + if (!candidateKey.lastModified || (oldKey.lastModified && oldKey.lastModified >= candidateKey.lastModified)) { keysToRetain.push(oldKey); continue; } } else if (keyToUpdate.length > 1) { throw new Error(`Unexpected error: key search by longid=${longid} yielded ${keyToUpdate.length} results`); } - unencryptedKeysToSave.push(candidate); + unencryptedKeysToSave.push(candidateKey); } return { keysToRetain, unencryptedKeysToSave }; }; From df5744e6bace2820b5bc0c0baff28bd1602c797b Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Sun, 11 Sep 2022 10:38:19 +0000 Subject: [PATCH 10/19] refactored `filterKeysToSave` according to change request --- extension/js/common/helpers.ts | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/extension/js/common/helpers.ts b/extension/js/common/helpers.ts index 1937bbe7532..32391223031 100644 --- a/extension/js/common/helpers.ts +++ b/extension/js/common/helpers.ts @@ -82,16 +82,13 @@ const filterKeysToSave = async (candidateKeys: Key[], existingKeys: KeyInfoWithI const keysToRetain: Key[] = []; const unencryptedKeysToSave: Key[] = []; for (const candidateKey of candidateKeys) { - const longid = KeyUtil.getPrimaryLongid(candidateKey); - const keyToUpdate = existingKeys.filter(ki => ki.longid === longid && ki.family === candidateKey.family); - if (keyToUpdate.length === 1) { - const oldKey = await KeyUtil.parse(keyToUpdate[0].private); - if (!candidateKey.lastModified || (oldKey.lastModified && oldKey.lastModified >= candidateKey.lastModified)) { - keysToRetain.push(oldKey); + const existingKey = existingKeys.find(ki => KeyUtil.identityEquals(ki, candidateKey)); + if (existingKey) { + const parsedExistingKey = await KeyUtil.parse(existingKey.private); + if (!candidateKey.lastModified || (parsedExistingKey.lastModified && parsedExistingKey.lastModified >= candidateKey.lastModified)) { + keysToRetain.push(parsedExistingKey); continue; } - } else if (keyToUpdate.length > 1) { - throw new Error(`Unexpected error: key search by longid=${longid} yielded ${keyToUpdate.length} results`); } unencryptedKeysToSave.push(candidateKey); } From 13d6d480d4715c8804d70d541afa8e2ad793af23 Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Sun, 11 Sep 2022 10:39:26 +0000 Subject: [PATCH 11/19] test fixes --- test/source/tests/setup.ts | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/test/source/tests/setup.ts b/test/source/tests/setup.ts index 374021e4c24..e0f1f79d64c 100644 --- a/test/source/tests/setup.ts +++ b/test/source/tests/setup.ts @@ -623,7 +623,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await PageRecipe.noToastAppears(gmailPage); await gmailPage.notPresent('@dialog-passphrase'); - const set4 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); + const set4 = await retrieveAndCheckKeys(settingsPage, acct, 1); expect(set4[0].lastModified).to.equal(set3[0].lastModified); // no update // 4. Forget the passphrase, EKM the same version of the existing key, no prompt await InboxPageRecipe.finishSessionOnInboxPage(gmailPage); @@ -670,10 +670,11 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await PageRecipe.waitForToastToAppearAndDisappear(gmailPage, 'Account keys updated'); await gmailPage.notPresent('@dialog-passphrase'); - const set9 = await retrieveAndCheckKeys(settingsPage, acct, 2); + const set9 = await retrieveAndCheckKeys(settingsPage, acct, 2, passphrase); // todo: passphrase should be stored?! const mainKey9 = KeyUtil.filterKeysByIdentity(set9, [{ family: 'openpgp', id: '392FB1E9FF4184659AB6A246835C0141B9ECF536' }]); expect(mainKey9.length).to.equal(1); - expect(KeyUtil.filterKeysByIdentity(set9, [{ family: 'openpgp', id: 'FAFB7D675AC74E87F84D169F00B0115807969D75' }]).length).to.equal(1); + const secondaryKey9 = KeyUtil.filterKeysByIdentity(set9, [{ family: 'openpgp', id: 'FAFB7D675AC74E87F84D169F00B0115807969D75' }]); + expect(secondaryKey9.length).to.equal(1); expect(mainKey9[0].lastModified).to.equal(set8[0].lastModified); // no update await gmailPage.close(); // 9. EKM returns a newer version of one key, fully omitting the other one, a toast, an update and removal @@ -682,8 +683,8 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== await PageRecipe.waitForToastToAppearAndDisappear(gmailPage, 'Account keys updated'); await gmailPage.notPresent('@dialog-passphrase'); const set10 = await retrieveAndCheckKeys(settingsPage, acct, 1); - // todo: check that the passphrase from the removed key is deleted from session? - const mainKey10 = KeyUtil.filterKeysByIdentity(set10, [{ family: 'openpgp', id: '392FB1E9FF4184659AB6A246835C0141B9ECF536' }]); + const mainKey10 = KeyUtil.filterKeysByIdentity(set10, [mainKey9[0]]); + expect(await getPassphrase(settingsPage, acct, KeyUtil.getPrimaryLongid(secondaryKey9[0]))).to.be.an.undefined; // the passphrase for the old key was deleted expect(mainKey10.length).to.equal(1); expect(mainKey10[0].lastModified!).to.be.greaterThan(mainKey9[0].lastModified!); // updated this key // 10. Forget the passphrase, EKM returns a third key, we enter a passphrase that doesn't match any of the existing keys, no update @@ -702,7 +703,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== } await ComposePageRecipe.cancelPassphraseDialog(gmailPage, 'keyboard'); await PageRecipe.noToastAppears(gmailPage); - const set11 = await retrieveAndCheckKeys(settingsPage, acct, 1); + const set11 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); expect(set11.map(entry => entry.id)).to.eql(['392FB1E9FF4184659AB6A246835C0141B9ECF536']); await gmailPage.close(); // 11. EKM returns a new third key, we enter a passphrase matching an existing key, update happens, the old key is removed @@ -716,8 +717,8 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== } await gmailPage.waitTillGone('@dialog-passphrase'); await PageRecipe.waitForToastToAppearAndDisappear(gmailPage, 'Account keys updated'); - const set12 = await retrieveAndCheckKeys(settingsPage, acct, 1); - // todo: check that the passphrase from the removed key is deleted from session? + const set12 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); // todo: passphrase should be stored?! + expect(await getPassphrase(settingsPage, acct, KeyUtil.getPrimaryLongid(set11[0]))).to.be.an.undefined; // the passphrase for the old key was deleted expect(set12.map(entry => entry.id)).to.eql(['277D1ADA213881F4ABE0415395E783DC0289E2E2']); const mainKey12 = KeyUtil.filterKeysByIdentity(set12, [{ family: 'openpgp', id: '277D1ADA213881F4ABE0415395E783DC0289E2E2' }]); expect(mainKey12.length).to.equal(1); @@ -733,7 +734,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== await PageRecipe.waitForToastToAppearAndDisappear(gmailPage, 'Could not update keys from EKM due to error: BrowserMsg(processAndStoreKeysFromEkmLocally) sendRawResponse::Error: Some keys could not be parsed'); await gmailPage.notPresent('@dialog-passphrase'); - const set13 = await retrieveAndCheckKeys(settingsPage, acct, 1); + const set13 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); expect(set13.map(entry => entry.id)).to.eql(['277D1ADA213881F4ABE0415395E783DC0289E2E2']); const mainKey13 = KeyUtil.filterKeysByIdentity(set13, [{ family: 'openpgp', id: '277D1ADA213881F4ABE0415395E783DC0289E2E2' }]); expect(mainKey13.length).to.equal(1); @@ -744,7 +745,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await PageRecipe.noToastAppears(gmailPage); await gmailPage.notPresent('@dialog-passphrase'); - const set14 = await retrieveAndCheckKeys(settingsPage, acct, 1); + const set14 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); expect(set14.map(entry => entry.id)).to.eql(['277D1ADA213881F4ABE0415395E783DC0289E2E2']); const mainKey14 = KeyUtil.filterKeysByIdentity(set14.map(ki => ki), [{ family: 'openpgp', id: '277D1ADA213881F4ABE0415395E783DC0289E2E2' }]); expect(mainKey14.length).to.equal(1); @@ -819,7 +820,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== await gmailPage.waitAndRespondToModal('error', 'confirm', 'Keys for your account were not set up yet - please ask your systems administrator'); await PageRecipe.noToastAppears(gmailPage); await gmailPage.close(); await retrieveAndCheckKeys(settingsPage, acct, 0); // no keys - // todo: check that the passphrase is deleted from storage? + expect(await getPassphrase(settingsPage, acct, KeyUtil.getPrimaryLongid(set1[0]))).to.be.an.undefined; // the passphrase for the old key was deleted await settingsPage.close(); // 2. Adding a new key from the key manager when there is none in the storage MOCK_KM_UPDATING_KEY[acct] = { response: { privateKeys: [{ decryptedPrivateKey: testConstants.updatingPrv }] } }; From c8fb7674d91508e4e8bae3e9d8a7e263fd10d63c Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Sun, 11 Sep 2022 13:40:59 +0000 Subject: [PATCH 12/19] renamed unencryptedKeysToSave to newUnencryptedKeysToSave --- extension/js/common/helpers.ts | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/extension/js/common/helpers.ts b/extension/js/common/helpers.ts index 32391223031..3a5c8be56af 100644 --- a/extension/js/common/helpers.ts +++ b/extension/js/common/helpers.ts @@ -77,10 +77,10 @@ const parseAndCheckPrivateKeys = async (decryptedPrivateKeys: string[]) => { const filterKeysToSave = async (candidateKeys: Key[], existingKeys: KeyInfoWithIdentity[]) => { // todo: check for uniqueness of candidateKeys identities here? if (!existingKeys.length) { - return { keysToRetain: [], unencryptedKeysToSave: candidateKeys }; + return { keysToRetain: [], newUnencryptedKeysToSave: candidateKeys }; } const keysToRetain: Key[] = []; - const unencryptedKeysToSave: Key[] = []; + const newUnencryptedKeysToSave: Key[] = []; for (const candidateKey of candidateKeys) { const existingKey = existingKeys.find(ki => KeyUtil.identityEquals(ki, candidateKey)); if (existingKey) { @@ -90,9 +90,9 @@ const filterKeysToSave = async (candidateKeys: Key[], existingKeys: KeyInfoWithI continue; } } - unencryptedKeysToSave.push(candidateKey); + newUnencryptedKeysToSave.push(candidateKey); } - return { keysToRetain, unencryptedKeysToSave }; + return { keysToRetain, newUnencryptedKeysToSave }; }; export const processAndStoreKeysFromEkmLocally = async ( @@ -100,8 +100,8 @@ export const processAndStoreKeysFromEkmLocally = async ( ): Promise => { const { unencryptedPrvs } = await parseAndCheckPrivateKeys(decryptedPrivateKeys); const existingKeys = await KeyStore.get(acctEmail); - let { keysToRetain, unencryptedKeysToSave } = await filterKeysToSave(unencryptedPrvs, existingKeys); - if (!unencryptedKeysToSave.length && keysToRetain.length === existingKeys.length) { + let { keysToRetain, newUnencryptedKeysToSave } = await filterKeysToSave(unencryptedPrvs, existingKeys); + if (!newUnencryptedKeysToSave.length && keysToRetain.length === existingKeys.length) { // nothing to update return { needPassphrase: false, noKeysSetup: !existingKeys.length }; } @@ -116,7 +116,7 @@ export const processAndStoreKeysFromEkmLocally = async ( return { needPassphrase: true, noKeysSetup: true }; } let encryptedKeys: Key[] = []; - if (unencryptedKeysToSave.length) { + if (newUnencryptedKeysToSave.length) { if (passphrase === undefined) { // trying to find a passphrase that unlocks at least one key const passphrases = await PassphraseStore.getMany(acctEmail, existingKeys); @@ -124,18 +124,18 @@ export const processAndStoreKeysFromEkmLocally = async ( } if (passphrase !== undefined) { const pp = passphrase; - await Promise.all(unencryptedKeysToSave.map(prv => KeyUtil.encrypt(prv, pp))); - encryptedKeys = unencryptedKeysToSave; - unencryptedKeysToSave = []; + await Promise.all(newUnencryptedKeysToSave.map(prv => KeyUtil.encrypt(prv, pp))); + encryptedKeys = newUnencryptedKeysToSave; + newUnencryptedKeysToSave = []; } } - if (encryptedKeys.length || !unencryptedKeysToSave.length) { + if (encryptedKeys.length || !newUnencryptedKeysToSave.length) { // also updates `name`, todo: refactor in #4545 const newKeyset = keysToRetain.concat(encryptedKeys); await saveKeysAndPassPhrase(acctEmail, newKeyset, ppOptions, true); return { updateCount: encryptedKeys.length + (existingKeys.length - keysToRetain.length), noKeysSetup: !newKeyset.length }; } else { // todo: should we delete? - return { needPassphrase: unencryptedKeysToSave.length > 0 }; + return { needPassphrase: newUnencryptedKeysToSave.length > 0 }; } }; From 5c936a73de11460fec8d4a51fb1967fe24018123 Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Sun, 11 Sep 2022 13:59:57 +0000 Subject: [PATCH 13/19] removed unnecessary `replace` option in setup-key-manager-autogen --- extension/chrome/settings/setup/setup-key-manager-autogen.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extension/chrome/settings/setup/setup-key-manager-autogen.ts b/extension/chrome/settings/setup/setup-key-manager-autogen.ts index 14c6e7c9265..83bcef0d70b 100644 --- a/extension/chrome/settings/setup/setup-key-manager-autogen.ts +++ b/extension/chrome/settings/setup/setup-key-manager-autogen.ts @@ -102,7 +102,7 @@ export class SetupWithEmailKeyManagerModule { } const storePrvOnKm = async () => this.view.keyManager!.storePrivateKey(this.view.idToken!, KeyUtil.armor(decryptablePrv)); await Settings.retryUntilSuccessful(storePrvOnKm, 'Failed to store newly generated key on FlowCrypt Email Key Manager', Lang.general.contactIfNeedAssistance(this.view.isFesUsed())); - await saveKeysAndPassPhrase(this.view.acctEmail, [await KeyUtil.parse(generated.private)], setupOptions, true); // store encrypted key + pass phrase locally + await saveKeysAndPassPhrase(this.view.acctEmail, [await KeyUtil.parse(generated.private)], setupOptions); // store encrypted key + pass phrase locally }; } From 1c9a194a18edc50b0fa7c788173542bf8aae4758 Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Sat, 17 Sep 2022 17:59:38 +0000 Subject: [PATCH 14/19] store passphrase for newly-added keys from EKM --- extension/js/common/helpers.ts | 62 ++++++++++++------- .../common/platform/store/passphrase-store.ts | 16 +++-- test/source/tests/setup.ts | 4 +- 3 files changed, 54 insertions(+), 28 deletions(-) diff --git a/extension/js/common/helpers.ts b/extension/js/common/helpers.ts index 3a5c8be56af..c1f0bbddf48 100644 --- a/extension/js/common/helpers.ts +++ b/extension/js/common/helpers.ts @@ -17,8 +17,15 @@ export const isFesUsed = async (acctEmail: string) => { return Boolean(fesUrl); }; -export const saveKeysAndPassPhrase = async (acctEmail: string, prvs: Key[], ppOptions?: PassphraseOptions, replaceKeys: boolean = false) => { +const setPassphraseForPrvs = async (acctEmail: string, prvs: Key[], ppOptions: PassphraseOptions) => { const clientConfiguration = await ClientConfiguration.newInstance(acctEmail); + const storageType = (ppOptions.passphrase_save && !clientConfiguration.forbidStoringPassPhrase()) ? 'local' : 'session'; + for (const prv of prvs) { + await PassphraseStore.set(storageType, acctEmail, { longid: KeyUtil.getPrimaryLongid(prv) }, ppOptions.passphrase); + } +}; + +export const saveKeysAndPassPhrase = async (acctEmail: string, prvs: Key[], ppOptions?: PassphraseOptions, replaceKeys: boolean = false) => { if (replaceKeys) { // track longids to remove related passhprases const existingKeys = await KeyStore.get(acctEmail); @@ -26,16 +33,15 @@ export const saveKeysAndPassPhrase = async (acctEmail: string, prvs: Key[], ppOp // set actually replaces the set of keys in storage with the new set await KeyStore.set(acctEmail, await Promise.all(prvs.map(KeyUtil.keyInfoObj))); // todo: duplicate identities await PassphraseStore.removeMany(acctEmail, deletedKeys); - } - for (const prv of prvs) { - if (!replaceKeys) { + } else { + for (const prv of prvs) { await KeyStore.add(acctEmail, prv); } - if (ppOptions !== undefined) { - // todo: perhaps it's easier just to store a set of passphrases without specifying longids? - await PassphraseStore.set((ppOptions.passphrase_save && !clientConfiguration.forbidStoringPassPhrase()) ? 'local' : 'session', - acctEmail, { longid: KeyUtil.getPrimaryLongid(prv) }, ppOptions.passphrase); - } + } + if (ppOptions !== undefined) { + // todo: it would be good to check that the passphrase isn't present in the other storage type + // though this situation is not possible with current use cases + await setPassphraseForPrvs(acctEmail, prvs, ppOptions); } const { sendAs, full_name: name } = await AcctStore.get(acctEmail, ['sendAs', 'full_name']); const myOwnEmailsAddrs: string[] = [acctEmail].concat(Object.keys(sendAs!)); @@ -112,30 +118,44 @@ export const processAndStoreKeysFromEkmLocally = async ( ppOptions = originalOptions; } let passphrase = ppOptions?.passphrase; + let passphraseInLocalStorage = !!ppOptions?.passphrase_save; if (passphrase === undefined && !existingKeys.length) { return { needPassphrase: true, noKeysSetup: true }; } - let encryptedKeys: Key[] = []; + let encryptedKeys: { passphrase: string, keys: Key[] } | undefined; if (newUnencryptedKeysToSave.length) { if (passphrase === undefined) { // trying to find a passphrase that unlocks at least one key const passphrases = await PassphraseStore.getMany(acctEmail, existingKeys); - passphrase = passphrases.find(pp => pp !== undefined); + const foundPassphrase = passphrases.find(pp => pp !== undefined); + if (foundPassphrase) { + passphrase = foundPassphrase.value; + passphraseInLocalStorage = foundPassphrase.source === 'local'; + } } if (passphrase !== undefined) { - const pp = passphrase; + const pp = passphrase; // explicitly defined constant string for the mapping function await Promise.all(newUnencryptedKeysToSave.map(prv => KeyUtil.encrypt(prv, pp))); - encryptedKeys = newUnencryptedKeysToSave; + encryptedKeys = { keys: newUnencryptedKeysToSave, passphrase }; newUnencryptedKeysToSave = []; } } - if (encryptedKeys.length || !newUnencryptedKeysToSave.length) { - // also updates `name`, todo: refactor in #4545 - const newKeyset = keysToRetain.concat(encryptedKeys); - await saveKeysAndPassPhrase(acctEmail, newKeyset, ppOptions, true); - return { updateCount: encryptedKeys.length + (existingKeys.length - keysToRetain.length), noKeysSetup: !newKeyset.length }; - } else { - // todo: should we delete? - return { needPassphrase: newUnencryptedKeysToSave.length > 0 }; + if (newUnencryptedKeysToSave.length > 0) { + return { needPassphrase: true }; + } + // stage 1. Clear all existingKeys, except for keysToRetain + if (existingKeys.length !== keysToRetain.length) { + await saveKeysAndPassPhrase(acctEmail, keysToRetain, undefined, true); + } + // stage 2. Adding new keys + if (encryptedKeys?.keys.length) { + // new keys are about to be added, they must be accompanied with the passphrase setting + const effectivePpOptions = { passphrase: encryptedKeys.passphrase, passphrase_save: passphraseInLocalStorage }; + // ppOptions have special meaning in saveKeysAndPassPhrase(), they trigger `name` updates, todo: refactor in #4545 + await saveKeysAndPassPhrase(acctEmail, encryptedKeys.keys, ppOptions ? effectivePpOptions : undefined); + if (!ppOptions) { + await setPassphraseForPrvs(acctEmail, encryptedKeys.keys, effectivePpOptions); + } } + return { updateCount: encryptedKeys?.keys.length ?? 0 + (existingKeys.length - keysToRetain.length), noKeysSetup: !(encryptedKeys?.keys.length || keysToRetain.length) }; }; diff --git a/extension/js/common/platform/store/passphrase-store.ts b/extension/js/common/platform/store/passphrase-store.ts index 4c22bfc5e3d..d7b53bf7f71 100644 --- a/extension/js/common/platform/store/passphrase-store.ts +++ b/extension/js/common/platform/store/passphrase-store.ts @@ -13,11 +13,12 @@ export class PassphraseStore extends AbstractStore { // if we implement (and migrate) password storage to use KeyIdentity instead of longid, we'll have `keyInfo: KeyIdentity` here public static get = async (acctEmail: string, keyInfo: { longid: string }, ignoreSession: boolean = false): Promise => { - return (await PassphraseStore.getMany(acctEmail, [keyInfo], ignoreSession))[0]; + return (await PassphraseStore.getMany(acctEmail, [keyInfo], ignoreSession))[0]?.value; }; // if we implement (and migrate) password storage to use KeyIdentity instead of longid, we'll have `keyInfo: KeyIdentity` here - public static getMany = async (acctEmail: string, keyInfos: { longid: string }[], ignoreSession: boolean = false): Promise<(string | undefined)[]> => { + public static getMany = async (acctEmail: string, keyInfos: { longid: string }[], ignoreSession: boolean = false): + Promise<({ value: string, source: StorageType } | undefined)[]> => { const storageIndexes = keyInfos.map(keyInfo => PassphraseStore.getIndex(keyInfo.longid)); return await PassphraseStore.getByIndexes(acctEmail, storageIndexes, ignoreSession); }; @@ -63,17 +64,22 @@ export class PassphraseStore extends AbstractStore { return `passphrase_${longid}` as unknown as AccountIndex; }; - private static getByIndexes = async (acctEmail: string, storageIndexes: AccountIndex[], ignoreSession: boolean = false): Promise<(string | undefined)[]> => { + private static getByIndexes = async (acctEmail: string, storageIndexes: AccountIndex[], ignoreSession: boolean = false): + Promise<({ value: string, source: StorageType } | undefined)[]> => { const storage = await AcctStore.get(acctEmail, storageIndexes); const results = await Promise.all(storageIndexes.map(async storageIndex => { const found = storage[storageIndex]; if (typeof found === 'string') { - return found; + return { value: found, source: 'local' as StorageType }; } if (ignoreSession) { return undefined; } - return await InMemoryStore.get(acctEmail, storageIndex) ?? undefined; + const value = await InMemoryStore.get(acctEmail, storageIndex); + if (typeof value === 'undefined') { + return undefined; + } + return { value, source: 'session' as StorageType }; })); return results; }; diff --git a/test/source/tests/setup.ts b/test/source/tests/setup.ts index e0f1f79d64c..c45d019d551 100644 --- a/test/source/tests/setup.ts +++ b/test/source/tests/setup.ts @@ -670,7 +670,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await PageRecipe.waitForToastToAppearAndDisappear(gmailPage, 'Account keys updated'); await gmailPage.notPresent('@dialog-passphrase'); - const set9 = await retrieveAndCheckKeys(settingsPage, acct, 2, passphrase); // todo: passphrase should be stored?! + const set9 = await retrieveAndCheckKeys(settingsPage, acct, 2); const mainKey9 = KeyUtil.filterKeysByIdentity(set9, [{ family: 'openpgp', id: '392FB1E9FF4184659AB6A246835C0141B9ECF536' }]); expect(mainKey9.length).to.equal(1); const secondaryKey9 = KeyUtil.filterKeysByIdentity(set9, [{ family: 'openpgp', id: 'FAFB7D675AC74E87F84D169F00B0115807969D75' }]); @@ -717,7 +717,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== } await gmailPage.waitTillGone('@dialog-passphrase'); await PageRecipe.waitForToastToAppearAndDisappear(gmailPage, 'Account keys updated'); - const set12 = await retrieveAndCheckKeys(settingsPage, acct, 1, passphrase); // todo: passphrase should be stored?! + const set12 = await retrieveAndCheckKeys(settingsPage, acct, 1); expect(await getPassphrase(settingsPage, acct, KeyUtil.getPrimaryLongid(set11[0]))).to.be.an.undefined; // the passphrase for the old key was deleted expect(set12.map(entry => entry.id)).to.eql(['277D1ADA213881F4ABE0415395E783DC0289E2E2']); const mainKey12 = KeyUtil.filterKeysByIdentity(set12, [{ family: 'openpgp', id: '277D1ADA213881F4ABE0415395E783DC0289E2E2' }]); From d479866d8fa30a7ffbfdc31db183b920c01a3cad Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Sun, 18 Sep 2022 07:14:35 +0000 Subject: [PATCH 15/19] refactored to use saveKeysAndPassPhrase by manual add_key too --- extension/chrome/settings/modules/add_key.ts | 16 +++++++++------- extension/js/common/helpers.ts | 18 +++++++++++------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/extension/chrome/settings/modules/add_key.ts b/extension/chrome/settings/modules/add_key.ts index d88ea3f300a..b97c01d0eab 100644 --- a/extension/chrome/settings/modules/add_key.ts +++ b/extension/chrome/settings/modules/add_key.ts @@ -13,13 +13,11 @@ import { Ui } from '../../../js/common/browser/ui.js'; import { View } from '../../../js/common/view.js'; import { Xss } from '../../../js/common/platform/xss.js'; import { initPassphraseToggle } from '../../../js/common/ui/passphrase-ui.js'; -import { PassphraseStore } from '../../../js/common/platform/store/passphrase-store.js'; -import { KeyStore } from '../../../js/common/platform/store/key-store.js'; -import { KeyUtil, UnexpectedKeyTypeError } from '../../../js/common/core/crypto/key.js'; +import { UnexpectedKeyTypeError } from '../../../js/common/core/crypto/key.js'; import { ClientConfiguration } from '../../../js/common/client-configuration.js'; -import { StorageType } from '../../../js/common/platform/store/abstract-store.js'; import { Lang } from '../../../js/common/lang.js'; import { AcctStore } from '../../../js/common/platform/store/acct-store.js'; +import { saveKeysAndPassPhrase, setPassphraseForPrvs } from '../../../js/common/helpers.js'; View.run(class AddKeyView extends View { @@ -100,9 +98,13 @@ View.run(class AddKeyView extends View { try { const checked = await this.keyImportUi.checkPrv(this.acctEmail, String($('.input_private_key').val()), String($('.input_passphrase').val())); if (checked) { - await KeyStore.add(this.acctEmail, checked.encrypted); // resulting new_key checked above - const storageType: StorageType = ($('.input_passphrase_save').prop('checked') && !this.clientConfiguration.forbidStoringPassPhrase()) ? 'local' : 'session'; - await PassphraseStore.set(storageType, this.acctEmail, { longid: KeyUtil.getPrimaryLongid(checked.encrypted) }, checked.passphrase); + await saveKeysAndPassPhrase(this.acctEmail, [checked.encrypted]); // resulting new_key checked above + await setPassphraseForPrvs( + this.clientConfiguration, + this.acctEmail, + [checked.encrypted], + { passphrase: checked.passphrase, passphrase_save: $('.input_passphrase_save').prop('checked') } + ); BrowserMsg.send.reload(this.parentTabId, { advanced: true }); } } catch (e) { diff --git a/extension/js/common/helpers.ts b/extension/js/common/helpers.ts index 52355bbd5c5..fcc6421de08 100644 --- a/extension/js/common/helpers.ts +++ b/extension/js/common/helpers.ts @@ -18,21 +18,23 @@ export const isFesUsed = async (acctEmail: string) => { return Boolean(fesUrl); }; -const setPassphraseForPrvs = async (acctEmail: string, prvs: Key[], ppOptions: PassphraseOptions) => { - const clientConfiguration = await ClientConfiguration.newInstance(acctEmail); +export const setPassphraseForPrvs = async (clientConfiguration: ClientConfiguration, acctEmail: string, prvs: Key[], ppOptions: PassphraseOptions) => { const storageType = (ppOptions.passphrase_save && !clientConfiguration.forbidStoringPassPhrase()) ? 'local' : 'session'; for (const prv of prvs) { await PassphraseStore.set(storageType, acctEmail, { longid: KeyUtil.getPrimaryLongid(prv) }, ppOptions.passphrase); } }; -export const saveKeysAndPassPhrase = async (acctEmail: string, prvs: Key[], ppOptions?: PassphraseOptions, replaceKeys: boolean = false) => { +// note: for `replaceKeys = true` need to make sure that `prvs` don't have duplicate identities, +// they is currently guaranteed by filterKeysToSave() +// todo: perhaps split into two different functions for add or replace as part of #4545? +const addOrReplaceKeysAndPassPhrase = async (acctEmail: string, prvs: Key[], ppOptions?: PassphraseOptions, replaceKeys: boolean = false) => { if (replaceKeys) { // track longids to remove related passhprases const existingKeys = await KeyStore.get(acctEmail); const deletedKeys = existingKeys.filter(old => !prvs.some(prvIdentity => KeyUtil.identityEquals(prvIdentity, old))); // set actually replaces the set of keys in storage with the new set - await KeyStore.set(acctEmail, await Promise.all(prvs.map(KeyUtil.keyInfoObj))); // todo: duplicate identities + await KeyStore.set(acctEmail, await Promise.all(prvs.map(KeyUtil.keyInfoObj))); await PassphraseStore.removeMany(acctEmail, deletedKeys); } else { for (const prv of prvs) { @@ -42,7 +44,7 @@ export const saveKeysAndPassPhrase = async (acctEmail: string, prvs: Key[], ppOp if (ppOptions !== undefined) { // todo: it would be good to check that the passphrase isn't present in the other storage type // though this situation is not possible with current use cases - await setPassphraseForPrvs(acctEmail, prvs, ppOptions); + await setPassphraseForPrvs(await ClientConfiguration.newInstance(acctEmail), acctEmail, prvs, ppOptions); } const { sendAs, full_name: name } = await AcctStore.get(acctEmail, ['sendAs', 'full_name']); const myOwnEmailsAddrs: string[] = [acctEmail].concat(Object.keys(sendAs!)); @@ -57,6 +59,8 @@ export const saveKeysAndPassPhrase = async (acctEmail: string, prvs: Key[], ppOp } }; +export const saveKeysAndPassPhrase: (acctEmail: string, prvs: Key[], ppOptions?: PassphraseOptions) => Promise = addOrReplaceKeysAndPassPhrase; + const parseAndCheckPrivateKeys = async (decryptedPrivateKeys: string[]) => { const unencryptedPrvs: Key[] = []; // parse and check that all the keys are valid @@ -146,7 +150,7 @@ export const processAndStoreKeysFromEkmLocally = async ( } // stage 1. Clear all existingKeys, except for keysToRetain if (existingKeys.length !== keysToRetain.length) { - await saveKeysAndPassPhrase(acctEmail, keysToRetain, undefined, true); + await addOrReplaceKeysAndPassPhrase(acctEmail, keysToRetain, undefined, true); } // stage 2. Adding new keys if (encryptedKeys?.keys.length) { @@ -155,7 +159,7 @@ export const processAndStoreKeysFromEkmLocally = async ( // ppOptions have special meaning in saveKeysAndPassPhrase(), they trigger `name` updates, todo: refactor in #4545 await saveKeysAndPassPhrase(acctEmail, encryptedKeys.keys, ppOptions ? effectivePpOptions : undefined); if (!ppOptions) { - await setPassphraseForPrvs(acctEmail, encryptedKeys.keys, effectivePpOptions); + await setPassphraseForPrvs(await ClientConfiguration.newInstance(acctEmail), acctEmail, encryptedKeys.keys, effectivePpOptions); } } return { updateCount: encryptedKeys?.keys.length ?? 0 + (existingKeys.length - keysToRetain.length), noKeysSetup: !(encryptedKeys?.keys.length || keysToRetain.length) }; From 8d7e5b09625cf87f49627803cd34b8fd29f99a55 Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Sun, 18 Sep 2022 07:42:34 +0000 Subject: [PATCH 16/19] tslint_fix --- extension/chrome/settings/modules/add_key.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extension/chrome/settings/modules/add_key.ts b/extension/chrome/settings/modules/add_key.ts index b97c01d0eab..a0e1fcb0fc5 100644 --- a/extension/chrome/settings/modules/add_key.ts +++ b/extension/chrome/settings/modules/add_key.ts @@ -103,7 +103,7 @@ View.run(class AddKeyView extends View { this.clientConfiguration, this.acctEmail, [checked.encrypted], - { passphrase: checked.passphrase, passphrase_save: $('.input_passphrase_save').prop('checked') } + { passphrase: checked.passphrase, passphrase_save: !!$('.input_passphrase_save').prop('checked') } ); BrowserMsg.send.reload(this.parentTabId, { advanced: true }); } From a49456523541f9c4800576e5a10bb56396979bab Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Mon, 19 Sep 2022 13:17:44 +0000 Subject: [PATCH 17/19] small refactoring --- test/source/tests/setup.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/source/tests/setup.ts b/test/source/tests/setup.ts index 6984f30f63a..3b9188b832e 100644 --- a/test/source/tests/setup.ts +++ b/test/source/tests/setup.ts @@ -584,7 +584,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== const getPassphrase = async (page: ControllablePage, acctEmail: string, longid: string) => { const key = `cryptup_${emailKeyIndex(acctEmail, 'passphrase')}_${longid}`; - const passphrase = (await page.getFromLocalStorage([key]))[key] || await BrowserRecipe.getFromInMemoryStore(page, acctEmail, `passphrase_${longid}`); + const passphrase = (await page.getFromLocalStorage([key]))[key] || await BrowserRecipe.getPassphraseFromInMemoryStore(page, acctEmail, longid); expect(passphrase).to.be.a.string; return passphrase as string; }; From 4e02b6e67d2cd9376b42ab2674f5a3448d478f0d Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Mon, 19 Sep 2022 13:30:43 +0000 Subject: [PATCH 18/19] removed merge artifact --- extension/js/common/platform/store/in-memory-store.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/extension/js/common/platform/store/in-memory-store.ts b/extension/js/common/platform/store/in-memory-store.ts index bfd1e230c59..b75c41d1868 100644 --- a/extension/js/common/platform/store/in-memory-store.ts +++ b/extension/js/common/platform/store/in-memory-store.ts @@ -4,7 +4,6 @@ import { AbstractStore } from './abstract-store.js'; import { BrowserMsg } from '../../browser/browser-msg.js'; /** -<<<<<<< HEAD * Temporary In-Memory store for sensitive values, expiring after clientConfiguration.in_memory_pass_phrase_session_length (or default 4 hours) * see background_page.ts for the other end, also ExpirationCache class */ From 2d5b7f5b74af5c611f5096cdefd3af0c8bc97151 Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Mon, 19 Sep 2022 17:54:10 +0000 Subject: [PATCH 19/19] more obvious test --- test/source/tests/setup.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/source/tests/setup.ts b/test/source/tests/setup.ts index 3b9188b832e..ea5b2d229aa 100644 --- a/test/source/tests/setup.ts +++ b/test/source/tests/setup.ts @@ -780,8 +780,8 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== const accessToken = await BrowserRecipe.getGoogleAccessToken(settingsPage, acct); const extraAuthHeaders = { Authorization: `Bearer ${accessToken}` }; const set1 = await retrieveAndCheckKeys(settingsPage, acct, 1); - MOCK_KM_UPDATING_KEY[acct].response = { privateKeys: [] }; // 1. EKM returns the empty set, forcing to auto-generate + MOCK_KM_UPDATING_KEY[acct].response = { privateKeys: [] }; let gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); // The new settingsPage is loaded in place of the existing settings tab (this is by design) // However, after a second the newly-activated (old) settings tab loses focus in favour of the gmailPage, why is that? @@ -799,7 +799,8 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== const set2 = await retrieveAndCheckKeys(settingsPage, acct, 1); expect(set2[0].id).to.not.equal(set1[0].id); // entirely new key was generated // 2. Adding a new key from the key manager when there is none in the storage - // First, erase the keys + // First, erase the keys by supplying an empty set from mock EKM + MOCK_KM_UPDATING_KEY[acct].response = { privateKeys: [] }; gmailPage = await browser.newPage(t, TestUrls.mockGmailUrl(), undefined, extraAuthHeaders); await PageRecipe.noToastAppears(gmailPage); await gmailPage.notPresent('@dialog-passphrase'); @@ -807,6 +808,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg== await retrieveAndCheckKeys(settingsPage, acct, 0); // no keys, auto-generation expect(await getPassphrase(settingsPage, acct, KeyUtil.getPrimaryLongid(set2[0]))).to.be.an.undefined; // the passphrase for the old key was deleted await settingsPage.close(); + // Secondly, configure mock EKM to return a key and re-load the gmail page MOCK_KM_UPDATING_KEY[acct] = { response: { privateKeys: [{ decryptedPrivateKey: testConstants.updatingPrv }] } }; gmailPage = await browser.newPage(t, undefined, undefined, extraAuthHeaders); const newSettingsPage = await browser.newPageTriggeredBy(t, () => gmailPage.goto(TestUrls.mockGmailUrl()));