From c104a2cabb16f97b7ad8de9752a11680c1215fb7 Mon Sep 17 00:00:00 2001 From: Vladimir Y Date: Sat, 14 Jul 2018 21:04:56 +0300 Subject: [PATCH] finalize "readSettingsAuto" api method removing --- src/e2e/index.spec.ts | 57 +++++++++++++---------------- src/e2e/workflow.ts | 19 +++++----- src/electron-main/api/index.spec.ts | 41 ++++++++++++--------- src/electron-main/api/index.ts | 12 ++++-- 4 files changed, 66 insertions(+), 63 deletions(-) diff --git a/src/e2e/index.spec.ts b/src/e2e/index.spec.ts index 6154866c5..fd32b89e6 100644 --- a/src/e2e/index.spec.ts +++ b/src/e2e/index.spec.ts @@ -16,30 +16,16 @@ import { RUNTIME_ENV_E2E_TUTANOTA_PASSWORD, RUNTIME_ENV_E2E_TUTANOTA_UNREAD_MIN, } from "src/shared/constants"; -import {catchError, CONF, ENV, initApp, test, workflow} from "./workflow"; +import {ENV, initApp, test, workflow} from "./workflow"; import {AccountType} from "src/shared/model/account"; -test.beforeEach(async (t) => { - try { - await initApp(t, {initial: true}); - } catch (error) { - await catchError(t, error); - } -}); +test("starting app / master password setup / add accounts / logout / auto login", async (t) => { + await initApp(t, {initial: true}); -test.afterEach(async (t) => { - try { - await workflow.destroyApp(t); - } catch (error) { - await catchError(t, error); - } -}); + let accountsCount = 0; -test("login, add account, logout, auto login", async (t) => { + // initial setup and logout await (async () => { - const client = t.context.app.client; - let accountsCount = 0; - await workflow.login(t, {setup: true, savePassword: false}); await workflow.addAccount(t, {type: "protonmail"}); @@ -74,14 +60,14 @@ test("login, add account, logout, auto login", async (t) => { await workflow.selectAccount(t, accountsCount - 1); if (unread && !isNaN(unread)) { - await client.pause(ONE_SECOND_MS * 15); + await t.context.app.client.pause(ONE_SECOND_MS * 15); const verify = async (requiredAssertion = false) => { let actualUnreadText: string; try { // tslint:disable-next-line:max-line-length - actualUnreadText = String(await client.getText(`.list-group.accounts-list > .list-group-item:nth-child(${accountsCount}) email-securely-app-account-title > .account-value-sync-unread > .badge`)); + actualUnreadText = String(await t.context.app.client.getText(`.list-group.accounts-list > .list-group-item:nth-child(${accountsCount}) email-securely-app-account-title > .account-value-sync-unread > .badge`)); } catch (e) { if (!requiredAssertion) { return false; @@ -95,30 +81,37 @@ test("login, add account, logout, auto login", async (t) => { }; if (!(await verify())) { - await client.pause(ONE_SECOND_MS * (type === "tutanota" ? 70 : 10)); + await t.context.app.client.pause(ONE_SECOND_MS * (type === "tutanota" ? 70 : 10)); await verify(true); } } } await workflow.logout(t); - await workflow.login(t, {setup: false, savePassword: true}); + })(); + + const afterLoginTest = async (workflowPrefix: string) => { t.is( - (await client.getUrl()).split("#").pop(), "/(accounts-outlet:accounts)", - `login: "accounts" page url`, + (await t.context.app.client.getUrl()).split("#").pop(), + "/(accounts-outlet:accounts)", `workflow.${workflowPrefix}: "accounts" page url`, ); t.is(await workflow.accountsCount(t), accountsCount); + }; - await workflow.destroyApp(t); - })(); + // login with password saving + await workflow.login(t, {setup: false, savePassword: true}); + await afterLoginTest("explicit-login-passwordSave"); + await workflow.destroyApp(t); + // auto login 1 await initApp(t, {initial: false}); - await t.context.app.client.pause(CONF.timeouts.transition + CONF.timeouts.encryption); + await afterLoginTest("auto-login-1"); + await workflow.destroyApp(t); - t.is( - (await t.context.app.client.getUrl()).split("#").pop(), "/(accounts-outlet:accounts)", - `root: "accounts" page url`, - ); + // auto login 2, making sure previous auto login step didn't remove saved password + await initApp(t, {initial: false}); + await afterLoginTest("auto-login-2"); + await workflow.destroyApp(t); // final app instance is being destroyed by "afterEach" call // making sure log file has not been created (no errors happened) t.false(fs.existsSync(t.context.logFilePath), `"${t.context.logFilePath}" file should not exist`); diff --git a/src/e2e/workflow.ts b/src/e2e/workflow.ts index f423bea6d..caf1b1df7 100644 --- a/src/e2e/workflow.ts +++ b/src/e2e/workflow.ts @@ -13,8 +13,7 @@ import {Application} from "spectron"; import {promisify} from "util"; import {AccountType} from "src/shared/model/account"; -import {RUNTIME_ENV_E2E, RUNTIME_ENV_USER_DATA_DIR} from "src/shared/constants"; -import {ACCOUNTS_CONFIG} from "src/shared/constants"; +import {ACCOUNTS_CONFIG, RUNTIME_ENV_E2E, RUNTIME_ENV_USER_DATA_DIR} from "src/shared/constants"; export interface TestContext { app: Application; @@ -40,7 +39,7 @@ export const CONF = { element: 700, elementTouched: 300, encryption: CI ? 5000 : 1000, - transition: CI ? 2000 : 500, + transition: CI ? 2500 : 500, }, }; @@ -56,13 +55,13 @@ export async function initApp(t: ExecutionContext, options: { initi || path.join(rootDirPath, "./output/e2e", String(Number(new Date()))); const userDataDirPath = path.join(outputDirPath, "./app-data"); const logFilePath = path.join(userDataDirPath, "log.log"); - // const webdriverLogDirPath = path.join(outputDirPath, "webdriver-driver-log"); - // const chromeDriverLogFilePath = path.join(outputDirPath, "chrome-driver.log"); + const webdriverLogDirPath = path.join(outputDirPath, "webdriver-driver-log"); + const chromeDriverLogFilePath = path.join(outputDirPath, "chrome-driver.log"); await mkOutputDirs([ outputDirPath, userDataDirPath, - // webdriverLogDirPath, + webdriverLogDirPath, ]); t.context.appDirPath = appDirPath; @@ -89,8 +88,8 @@ export async function initApp(t: ExecutionContext, options: { initi // TODO chromedriver throws 'Failed to redirect stderr to log file.' and 'Unable to initialize logging. Exiting...' // errors if log paths are specified, only on first run - // webdriverLogPath: webdriverLogDirPath, - // chromeDriverLogPath: chromeDriverLogFilePath, + webdriverLogPath: webdriverLogDirPath, + chromeDriverLogPath: chromeDriverLogFilePath, // ...(CI ? { // startTimeout: 30000, @@ -130,6 +129,8 @@ export async function initApp(t: ExecutionContext, options: { initi // await awaitAngular(t.context.app.client); // seems to be not needed // await t.context.app.client.pause(2000); + + await t.context.app.client.pause(CONF.timeouts.encryption); } catch (error) { if (error.message.indexOf("The inAppPurchase module can only be used on macOS") !== -1) { // tslint:disable:no-console @@ -277,7 +278,7 @@ export const workflow = { `addAccount: "accounts?login=${login}" page url`, ); await this.closeSettingsModal(t); - await client.pause(CONF.timeouts.transition); + await client.pause(CONF.timeouts.encryption); }, async selectAccount(t: ExecutionContext, index = 0) { diff --git a/src/electron-main/api/index.spec.ts b/src/electron-main/api/index.spec.ts index 3d750d02c..378c02b4c 100644 --- a/src/electron-main/api/index.spec.ts +++ b/src/electron-main/api/index.spec.ts @@ -50,7 +50,7 @@ const tests: Record) => Imple credentialsKeePass: {}, }; - const settings = await initConfigAndSettings(endpoints, {password: OPTIONS.masterPassword}); + const settings = await readConfigAndSettings(endpoints, {password: OPTIONS.masterPassword}); t.is(settings.accounts.length, 0, `accounts list is empty`); @@ -116,7 +116,7 @@ const tests: Record) => Imple const emptyPasswordPayload = {password: "", newPassword: "new password 2"}; const wrongPasswordPayload = {password: "wrong password", newPassword: "new password 3"}; - const settings = await initConfigAndSettings(endpoints, {password: payload.password}); + const settings = await readConfigAndSettings(endpoints, {password: payload.password}); await t.throws(endpoint(emptyPasswordPayload).toPromise(), /Decryption\sfailed/gi); await t.throws(endpoint(wrongPasswordPayload).toPromise(), /Decryption\sfailed/gi); @@ -161,17 +161,20 @@ const tests: Record) => Imple logout: async (t) => { const deletePasswordSpy = t.context.mocks.keytar.deletePassword; const endpoints = t.context.endpoints; - const action = endpoints.logout; - await action().toPromise(); + await endpoints.logout().toPromise(); t.falsy(t.context.ctx.settingsStore.adapter); t.is(deletePasswordSpy.callCount, 1); - await initConfigAndSettings(endpoints, {password: OPTIONS.masterPassword}); + await readConfigAndSettings(endpoints, {password: OPTIONS.masterPassword}); + t.truthy(t.context.ctx.settingsStore.adapter); + t.is(deletePasswordSpy.callCount, 1); + + await readConfigAndSettings(endpoints, {password: OPTIONS.masterPassword, savePassword: false}); t.truthy(t.context.ctx.settingsStore.adapter); t.is(deletePasswordSpy.callCount, 2); - await action().toPromise(); + await endpoints.logout().toPromise(); t.falsy(t.context.ctx.settingsStore.adapter); t.is(deletePasswordSpy.callCount, 3); @@ -241,7 +244,7 @@ const tests: Record) => Imple }, ]; - await initConfig(endpoints); + await readConfig(endpoints); for (const patch of patches) { const initialConfig = await t.context.ctx.configStore.readExisting(); @@ -261,7 +264,7 @@ const tests: Record) => Imple readConfig: async (t) => { t.false(await t.context.ctx.configStore.readable(), "config file does not exist"); - const initial = await initConfig(t.context.endpoints); + const initial = await readConfig(t.context.endpoints); const initialExpected = {...t.context.ctx.initialStores.config, ...{_rev: 0}}; t.deepEqual(initial, initialExpected, "checking initial config file"); t.true(await t.context.ctx.configStore.readable(), "config file exists"); @@ -274,7 +277,7 @@ const tests: Record) => Imple t.false(await t.context.ctx.settingsStore.readable(), "settings file does not exist"); t.falsy(t.context.ctx.settingsStore.adapter, "adapter is not set"); - const initial = await initConfigAndSettings(endpoints, {password: OPTIONS.masterPassword}); + const initial = await readConfigAndSettings(endpoints, {password: OPTIONS.masterPassword, savePassword: false}); t.is(t.context.ctx.settingsStore.adapter && t.context.ctx.settingsStore.adapter.constructor.name, EncryptionAdapter.name, "adapter is an EncryptionAdapter", @@ -291,8 +294,9 @@ const tests: Record) => Imple t.deepEqual(initialUpdated, initialUpdatedExpected, "saved initial settings file"); t.deepEqual(await t.context.ctx.settingsStore.read(), initialUpdatedExpected, "loaded initial settings file"); - await initConfigAndSettings(endpoints, {password: OPTIONS.masterPassword, savePassword: true}); + await readConfigAndSettings(endpoints, {password: OPTIONS.masterPassword, savePassword: true}); t.is(setPasswordSpy.callCount, 1); + t.is(deletePasswordSpy.callCount, 1); setPasswordSpy.calledWithExactly(KEYTAR_SERVICE_NAME, KEYTAR_MASTER_PASSWORD_ACCOUNT, OPTIONS.masterPassword); }, @@ -329,7 +333,7 @@ const tests: Record) => Imple const removePayload = {login: addProtonPayload.login}; const removePayload404 = {login: "404 login"}; - await initConfigAndSettings(endpoints, {password: OPTIONS.masterPassword}); + await readConfigAndSettings(endpoints, {password: OPTIONS.masterPassword}); try { await removeHandler(removePayload404).toPromise(); @@ -363,7 +367,7 @@ const tests: Record) => Imple settingsExists: async (t) => { t.false(await t.context.ctx.settingsStore.readable(), "store: settings file does not exist"); - await initConfigAndSettings(t.context.endpoints, {password: OPTIONS.masterPassword}); + await readConfigAndSettings(t.context.endpoints, {password: OPTIONS.masterPassword}); t.true(await t.context.ctx.settingsStore.readable(), "store: settings file exists"); }, @@ -385,7 +389,7 @@ const tests: Record) => Imple const endpoints = t.context.endpoints; const action = endpoints.toggleCompactLayout; - const initial = await initConfig(endpoints); + const initial = await readConfig(endpoints); t.true(!initial.compactLayout); let updated = await action().toPromise(); @@ -402,7 +406,7 @@ const tests: Record) => Imple const updateHandler = endpoints.updateAccount; const updatePatch: AccountConfigPatch = {login: "login345", credentials: {password: "ps-1", twoFactorCode: "2fa-1"}}; - await initConfigAndSettings(endpoints, {password: OPTIONS.masterPassword}); + await readConfigAndSettings(endpoints, {password: OPTIONS.masterPassword}); try { await updateHandler({login: "login123", credentials: {password: "password1"}}).toPromise(); @@ -443,13 +447,14 @@ Object.entries(tests).forEach(([apiMethodName, method]) => { test.serial(apiMethodName, method); }); -async function initConfig(endpoints: Endpoints): Promise { +async function readConfig(endpoints: Endpoints): Promise { return await endpoints.readConfig().toPromise(); } -// tslint:disable-next-line:max-line-length -async function initConfigAndSettings(endpoints: Endpoints, payload: PasswordFieldContainer & { savePassword?: boolean; supressErrors?: boolean }): Promise { - await initConfig(endpoints); +async function readConfigAndSettings( + endpoints: Endpoints, payload: PasswordFieldContainer & { savePassword?: boolean; supressErrors?: boolean }, +): Promise { + await readConfig(endpoints); return await endpoints.readSettings(payload).toPromise(); } diff --git a/src/electron-main/api/index.ts b/src/electron-main/api/index.ts index 0b404e752..f68e122fc 100644 --- a/src/electron-main/api/index.ts +++ b/src/electron-main/api/index.ts @@ -87,6 +87,7 @@ export const initApi = async (ctx: Context): Promise => { // TODO update "readSettings" api method test, "upgradeSettings" and "no password provided" cases readSettings: ({password, savePassword}) => from((async () => { + // trying to auto login if (!password) { const storedPassword = await keytar.getPassword(KEYTAR_SERVICE_NAME, KEYTAR_MASTER_PASSWORD_ACCOUNT); @@ -112,10 +113,13 @@ export const initApi = async (ctx: Context): Promise => { return store.write(ctx.initialStores.settings); })(); - if (savePassword) { - await keytar.setPassword(KEYTAR_SERVICE_NAME, KEYTAR_MASTER_PASSWORD_ACCOUNT, password); - } else { - await keytar.deletePassword(KEYTAR_SERVICE_NAME, KEYTAR_MASTER_PASSWORD_ACCOUNT); + // "savePassword" is unset in auto login case + if (typeof savePassword !== "undefined") { + if (savePassword) { + await keytar.setPassword(KEYTAR_SERVICE_NAME, KEYTAR_MASTER_PASSWORD_ACCOUNT, password); + } else { + await keytar.deletePassword(KEYTAR_SERVICE_NAME, KEYTAR_MASTER_PASSWORD_ACCOUNT); + } } ctx.settingsStore = store;