From 96227eb3418121d176d6ffb52eb6236ff91b4952 Mon Sep 17 00:00:00 2001 From: Roberts Date: Thu, 1 Feb 2024 18:34:21 +0200 Subject: [PATCH] A0-3946: Fix password saving function not working properly --- .../src/background/handlers/Extension.spec.ts | 5 ++ .../src/background/handlers/Extension.ts | 48 +++++++++++-------- .../src/background/handlers/chromeStorage.ts | 32 +++++++++++++ .../extension-base/src/background/types.ts | 1 - .../src/Popup/Signing/Request/SignArea.tsx | 22 ++++----- 5 files changed, 73 insertions(+), 35 deletions(-) create mode 100644 packages/extension-base/src/background/handlers/chromeStorage.ts diff --git a/packages/extension-base/src/background/handlers/Extension.spec.ts b/packages/extension-base/src/background/handlers/Extension.spec.ts index 5d0f36ea078..05117e9323e 100644 --- a/packages/extension-base/src/background/handlers/Extension.spec.ts +++ b/packages/extension-base/src/background/handlers/Extension.spec.ts @@ -33,6 +33,11 @@ const authUrls = { chromeStub.windows.getAll.resolves([]); +// @ts-expect-error the "sinon-chrome" mocking library does not provide stubs for session storage, so we have to append them ourselves +chromeStub.storage.session = { + get: () => Promise.resolve({}) +}; + const stubChromeStorage = (data: Record = {}) => chromeStub.storage.local.get.resolves({ authUrls, ...data diff --git a/packages/extension-base/src/background/handlers/Extension.ts b/packages/extension-base/src/background/handlers/Extension.ts index 0770377d4dd..1731c1333f4 100644 --- a/packages/extension-base/src/background/handlers/Extension.ts +++ b/packages/extension-base/src/background/handlers/Extension.ts @@ -18,13 +18,12 @@ import { accounts as accountsObservable } from '@polkadot/ui-keyring/observable/ import { assert, isHex, u8aToHex } from '@polkadot/util'; import { keyExtractSuri, mnemonicGenerate, mnemonicValidate } from '@polkadot/util-crypto'; +import chromeStorage from './chromeStorage'; import { POPUP_CREATE_WINDOW_DATA } from './consts'; import { openCenteredWindow } from './helpers'; import State from './State'; import { createSubscription, unsubscribe } from './subscriptions'; -type CachedUnlocks = Record; - type GetContentPort = (tabId: number) => chrome.runtime.Port const SEED_DEFAULT_LENGTH = 12; @@ -40,12 +39,9 @@ function isJsonPayload (value: SignerPayloadJSON | SignerPayloadRaw): value is S } export default class Extension { - readonly #cachedUnlocks: CachedUnlocks; - readonly #state: State; constructor (state: State) { - this.#cachedUnlocks = {}; this.#state = state; } @@ -137,20 +133,26 @@ export default class Extension { return true; } - private refreshAccountPasswordCache (pair: KeyringPair): number { + private async refreshAccountPasswordCache (pair: KeyringPair, options?: {clearCache?: boolean}): Promise<{remainingTime: number, cachedPassword?: string}> { const { address } = pair; - const savedExpiry = this.#cachedUnlocks[address] || 0; - const remainingTime = savedExpiry - Date.now(); + const savedPasswords = await chromeStorage.getItem('password-cache') ?? {}; + const expiresAt = savedPasswords?.[address]?.expiresAt ?? 0; + const remainingTime = expiresAt - Date.now(); + const cachedPassword = savedPasswords?.[address]?.password; - if (remainingTime < 0) { - this.#cachedUnlocks[address] = 0; - pair.lock(); + if (remainingTime > 0 && !options?.clearCache) { + return { remainingTime, cachedPassword }; + } - return 0; + if (Object.keys(savedPasswords).find((key) => key === address)) { + delete savedPasswords[address]; + await chromeStorage.setItem('password-cache', () => (savedPasswords)); } - return remainingTime; + pair.lock(); + + return { remainingTime: 0 }; } private accountsShow ({ address, isShowing }: RequestAccountShow): boolean { @@ -334,7 +336,7 @@ export default class Extension { }; } - private async signingApprovePassword ({ id, password, savePass }: RequestSigningApprovePassword, getContentPort: GetContentPort): Promise { + private async signingApprovePassword ({ id, password: inputPassword, savePass }: RequestSigningApprovePassword, getContentPort: GetContentPort): Promise { const queued = await this.#state.getSignRequest(id); assert(queued, 'Unable to find request'); @@ -342,6 +344,10 @@ export default class Extension { const { payload } = queued; const pair = keyring.getPair(queued.account.address); + const { cachedPassword } = await this.refreshAccountPasswordCache(pair); + + const password = cachedPassword ?? inputPassword; + if (!pair) { const error = new Error('Unable to find pair'); @@ -351,10 +357,10 @@ export default class Extension { throw error; } - this.refreshAccountPasswordCache(pair); + await this.refreshAccountPasswordCache(pair); // if the keyring pair is locked, the password is needed - if (pair.isLocked && !password) { + if (!password) { const error = new Error('Password needed to unlock the account'); await this.#state.removeSignRequest(id); @@ -408,13 +414,14 @@ export default class Extension { .createType('ExtrinsicPayload', payload, { version: payload.version }) .sign(pair); - if (savePass) { + if (savePass && password) { // unlike queued.account.address the following // address is encoded with the default prefix // which what is used for password caching mapping - this.#cachedUnlocks[pair.address] = Date.now() + PASSWORD_EXPIRY_MS; + + await chromeStorage.setItem('password-cache', (savedPasswords) => ({ ...savedPasswords, [pair.address]: { password, expiresAt: Date.now() + PASSWORD_EXPIRY_MS } })); } else { - pair.lock(); + await this.refreshAccountPasswordCache(pair, { clearCache: true }); } await this.#state.removeSignRequest(id); @@ -449,10 +456,9 @@ export default class Extension { assert(pair, 'Unable to find pair'); - const remainingTime = this.refreshAccountPasswordCache(pair); + const { remainingTime } = await this.refreshAccountPasswordCache(pair); return { - isLocked: pair.isLocked, remainingTime }; } diff --git a/packages/extension-base/src/background/handlers/chromeStorage.ts b/packages/extension-base/src/background/handlers/chromeStorage.ts new file mode 100644 index 00000000000..fe650bca8da --- /dev/null +++ b/packages/extension-base/src/background/handlers/chromeStorage.ts @@ -0,0 +1,32 @@ +import { z } from 'zod'; + +// use namespaces to add typization for your storage data +const namespaces = { 'password-cache': z.record(z.string(), z.object({ password: z.string(), expiresAt: z.number() })) } as const satisfies Record; + +export const setItem = async (namespace: N, updater: (currData?: z.TypeOf) => z.TypeOf) => { + const currentData = await getItem(namespace); + + await chrome.storage.session.set({ [namespace]: updater(currentData) }); +}; + +export const getItem = async (namespace: N): Promise | undefined> => { + const { [namespace]: cachedData } = await chrome.storage.session.get(namespace); + + try { + return namespaces[namespace].parse(cachedData); + } catch { + return undefined; + } +}; + +export const removeItem = async (namespace: N) => { + await chrome.storage.session.remove(namespace); +}; + +const chromeStorage = { + getItem, + setItem, + removeItem +}; + +export default chromeStorage; diff --git a/packages/extension-base/src/background/types.ts b/packages/extension-base/src/background/types.ts index 8d7aedfd177..2a026ddc7df 100644 --- a/packages/extension-base/src/background/types.ts +++ b/packages/extension-base/src/background/types.ts @@ -306,7 +306,6 @@ export interface RequestSigningIsLocked { } export interface ResponseSigningIsLocked { - isLocked: boolean; remainingTime: number; } diff --git a/packages/extension-ui/src/Popup/Signing/Request/SignArea.tsx b/packages/extension-ui/src/Popup/Signing/Request/SignArea.tsx index 043e1d4ce7a..594d09d21fd 100644 --- a/packages/extension-ui/src/Popup/Signing/Request/SignArea.tsx +++ b/packages/extension-ui/src/Popup/Signing/Request/SignArea.tsx @@ -37,15 +37,17 @@ function SignArea({ buttonText, className, error, isExternal, isFirst, isLast, s !isExternal && isSignLocked(signId) - .then(({ isLocked, remainingTime }) => { - setIsLocked(isLocked); + .then(({ remainingTime }) => { + setIsLocked(remainingTime <= 0); timeout = setTimeout(() => { setIsLocked(true); }, remainingTime); // if the account was unlocked check the remember me // automatically to prolong the unlock period - !isLocked && setSavePass(true); + if(remainingTime > 0) { + setSavePass(true); + } }) .catch((error: Error) => console.error(error)); @@ -92,20 +94,14 @@ function SignArea({ buttonText, className, error, isExternal, isFirst, isLast, s const StyledCheckbox = styled(Checkbox)` margin-left: 8px; -`; + `; const RememberPasswordCheckbox = () => ( ('Remember password for {{expiration}} minutes', { - replace: { expiration: PASSWORD_EXPIRY_MIN } - }) - : t('Extend the period without password by {{expiration}} minutes', { - replace: { expiration: PASSWORD_EXPIRY_MIN } - }) - } + label={t('Remember password for {{expiration}} minutes', { + replace: { expiration: PASSWORD_EXPIRY_MIN } + })} onChange={setSavePass} /> );