From 996db72f96f405d5556fa12f64ba14723e08204d Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Wed, 28 Aug 2024 14:08:20 +1000 Subject: [PATCH] chore: address PR comments --- preload.js | 4 ++-- .../settings/section/CategoryRecoveryPassword.tsx | 3 +-- ts/session/constants.ts | 11 +++++++++++ .../utils/job_runners/jobs/ConfigurationSyncJob.ts | 4 ++-- ts/test/session/unit/onion/GuardNodes_test.ts | 3 ++- ts/test/session/unit/onion/SnodePoolUpdate_test.ts | 2 +- .../session/unit/utils/job_runner/JobRunner_test.ts | 2 +- ts/util/passwordUtils.ts | 4 ++-- 8 files changed, 22 insertions(+), 11 deletions(-) diff --git a/preload.js b/preload.js index eb1e0afd9..801364155 100644 --- a/preload.js +++ b/preload.js @@ -72,9 +72,9 @@ window.setPassword = async (passPhrase, oldPhrase) => }); // called to verify that the password is correct when showing the recovery from seed modal -window.onTryPassword = passPhrase => +window.onTryPassword = async passPhrase => new Promise((resolve, reject) => { - ipcRenderer.once('password-recovery-phrase-response', (event, error) => { + ipcRenderer.once('password-recovery-phrase-response', (_event, error) => { if (error) { return reject(error); } diff --git a/ts/components/settings/section/CategoryRecoveryPassword.tsx b/ts/components/settings/section/CategoryRecoveryPassword.tsx index 06dd9120f..a56154db6 100644 --- a/ts/components/settings/section/CategoryRecoveryPassword.tsx +++ b/ts/components/settings/section/CategoryRecoveryPassword.tsx @@ -1,4 +1,3 @@ -import { isEmpty } from 'lodash'; import { useState } from 'react'; import { useDispatch, useSelector } from 'react-redux'; import styled from 'styled-components'; @@ -68,7 +67,7 @@ const qrLogoProps: QRCodeLogoProps = { export const SettingsCategoryRecoveryPassword = () => { const recoveryPhrase = getCurrentRecoveryPhrase(); - if (!recoveryPhrase || isEmpty(recoveryPhrase)) { + if (!recoveryPhrase) { throw new Error('SettingsCategoryRecoveryPassword recovery seed is empty'); } const hexEncodedSeed = mnDecode(recoveryPhrase, 'english'); diff --git a/ts/session/constants.ts b/ts/session/constants.ts index fa3782faf..d1e5eb7c6 100644 --- a/ts/session/constants.ts +++ b/ts/session/constants.ts @@ -92,3 +92,14 @@ export const ONBOARDING_TIMES = { /** 0.2 seconds */ RECOVERY_FINISHED: 0.2 * DURATION.SECONDS, }; + +export const PASSWORD_LENGTH = { + /** + * 6 chars + */ + MIN_PASSWORD_LEN: 6, + /** + * 64 chars + */ + MAX_PASSWORD_LEN: 64, +}; diff --git a/ts/session/utils/job_runners/jobs/ConfigurationSyncJob.ts b/ts/session/utils/job_runners/jobs/ConfigurationSyncJob.ts index 0fe1a7fbd..3350d4070 100644 --- a/ts/session/utils/job_runners/jobs/ConfigurationSyncJob.ts +++ b/ts/session/utils/job_runners/jobs/ConfigurationSyncJob.ts @@ -23,7 +23,7 @@ import { } from '../PersistedJob'; import { DURATION } from '../../../constants'; -const defaultMsBetweenRetries = 5000; // a long time between retries, to avoid running multiple jobs at the same time, when one was postponed at the same time as one already planned (5s) +const defaultMsBetweenRetries = 5 * DURATION.SECONDS; // a long time between retries, to avoid running multiple jobs at the same time, when one was postponed at the same time as one already planned (5s) const defaultMaxAttempts = 4; /** @@ -320,7 +320,7 @@ async function queueNewJobIfNeeded() { ) { // Note: we postpone by 3s for two reasons: // - to make sure whoever is adding this job is done with what is needs to do first - // - to allow a just created device to process incoming config messages before pushing a new one + // - to allow a recently created device to process incoming config messages before pushing a new one // this call will make sure that there is only one configuration sync job at all times await runners.configurationSyncRunner.addJob( new ConfigurationSyncJob({ nextAttemptTimestamp: Date.now() + 3 * DURATION.SECONDS }) diff --git a/ts/test/session/unit/onion/GuardNodes_test.ts b/ts/test/session/unit/onion/GuardNodes_test.ts index 1295e8a69..dee95c772 100644 --- a/ts/test/session/unit/onion/GuardNodes_test.ts +++ b/ts/test/session/unit/onion/GuardNodes_test.ts @@ -52,7 +52,7 @@ describe('GuardNodes', () => { Sinon.restore(); }); - it('does not fetch from seed if we got 8 or more snodes in the db', async () => { + it('does not fetch from seed if we have 8 or more snodes in the db', async () => { stubData('getSnodePoolFromDb').resolves(fakeSnodePool); getSnodePoolFromDBOrFetchFromSeed = Sinon.stub( @@ -166,6 +166,7 @@ describe('GuardNodes', () => { // run the command const guardNodes = await OnionPaths.selectGuardNodes(); + // 2 because our desiredGuardCount is 2 (not putting the variable to make the test fails if we ever change it) expect(guardNodes.length).to.be.equal(2); expect(testGuardNode.callCount).to.be.equal(2); }); diff --git a/ts/test/session/unit/onion/SnodePoolUpdate_test.ts b/ts/test/session/unit/onion/SnodePoolUpdate_test.ts index 7f141da27..afce0a793 100644 --- a/ts/test/session/unit/onion/SnodePoolUpdate_test.ts +++ b/ts/test/session/unit/onion/SnodePoolUpdate_test.ts @@ -67,7 +67,7 @@ describe('OnionPaths', () => { expect(fetched).to.deep.equal(fakeSnodePool); }); - it('if the cached snode pool 8 or less snodes, trigger a fetch from the seed nodes', async () => { + it('if the cached snode pool is 8 or less snodes, trigger a fetch from the seed nodes', async () => { const length8 = fakeSnodePool.slice(0, 8); expect(length8.length).to.eq(8); getSnodePoolFromDb = stubData('getSnodePoolFromDb').resolves(length8); diff --git a/ts/test/session/unit/utils/job_runner/JobRunner_test.ts b/ts/test/session/unit/utils/job_runner/JobRunner_test.ts index 090b3d377..1c6d0ef70 100644 --- a/ts/test/session/unit/utils/job_runner/JobRunner_test.ts +++ b/ts/test/session/unit/utils/job_runner/JobRunner_test.ts @@ -304,7 +304,7 @@ describe('JobRunner', () => { await job2.waitForCurrentTry(); await runnerMulti.waitCurrentJob(); - // we need to give sometime to the jobrunner to handle the return of job2 and remove it + // we need to give some time for the jobrunner to handle the return of job2 and remove it await sleepFor(100); expect(runnerMulti.getJobList()).to.deep.eq([]); diff --git a/ts/util/passwordUtils.ts b/ts/util/passwordUtils.ts index 5bb9cbda5..bec0e4415 100644 --- a/ts/util/passwordUtils.ts +++ b/ts/util/passwordUtils.ts @@ -1,4 +1,5 @@ import * as crypto from 'crypto'; +import { PASSWORD_LENGTH } from '../session/constants'; const ERRORS = { TYPE: 'Password must be a string', @@ -12,7 +13,6 @@ const sha512 = (text: string) => { return hash.digest('hex'); }; -export const MAX_PASSWORD_LENGTH = 64; export const generateHash = (phrase: string) => phrase && sha512(phrase); export const matchesHash = (phrase: string | null, hash: string) => @@ -27,7 +27,7 @@ export const validatePassword = (phrase: string) => { return window?.i18n ? window?.i18n('noGivenPassword') : ERRORS.LENGTH; } - if (phrase.length < 6 || phrase.length > MAX_PASSWORD_LENGTH) { + if (phrase.length < PASSWORD_LENGTH.MIN_PASSWORD_LEN || phrase.length > PASSWORD_LENGTH.MAX_PASSWORD_LEN) { return window?.i18n ? window?.i18n('passwordLengthError') : ERRORS.LENGTH; }