A0-3946: Fix password saving function not working properly

Author: Roberts

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<string, unknown> = {}) => chromeStub.storage.local.get.resolves({
   authUrls,
   ...data

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<string, number>;
-
 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,14 +336,18 @@ export default class Extension {
     };
   }
 
-  private async signingApprovePassword ({ id, password, savePass }: RequestSigningApprovePassword, getContentPort: GetContentPort): Promise<void> {
+  private async signingApprovePassword ({ id, password: inputPassword, savePass }: RequestSigningApprovePassword, getContentPort: GetContentPort): Promise<void> {
     const queued = await this.#state.getSignRequest(id);
 
     assert(queued, 'Unable to find request');
 
     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
     };
   }

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<string, z.ZodSchema>;
+
+export const setItem = async <N extends keyof typeof namespaces>(namespace: N, updater: (currData?: z.TypeOf<typeof namespaces[N]>) => z.TypeOf<typeof namespaces[N]>) => {
+  const currentData = await getItem(namespace);
+
+  await chrome.storage.session.set({ [namespace]: updater(currentData) });
+};
+
+export const getItem = async <N extends keyof typeof namespaces>(namespace: N): Promise<z.infer<typeof namespaces[N]> | undefined> => {
+  const { [namespace]: cachedData } = await chrome.storage.session.get(namespace);
+
+  try {
+    return namespaces[namespace].parse(cachedData);
+  } catch {
+    return undefined;
+  }
+};
+
+export const removeItem = async <N extends keyof typeof namespaces>(namespace: N) => {
+  await chrome.storage.session.remove(namespace);
+};
+
+const chromeStorage = {
+  getItem,
+  setItem,
+  removeItem
+};
+
+export default chromeStorage;

packages/extension-base/src/background/types.ts

@@ -306,7 +306,6 @@ export interface RequestSigningIsLocked {
 }
 
 export interface ResponseSigningIsLocked {
-  isLocked: boolean;
   remainingTime: number;
 }
 

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 = () => (
     <StyledCheckbox
       checked={savePass}
-      label={
-        isLocked
-          ? t<string>('Remember password for {{expiration}} minutes', {
-              replace: { expiration: PASSWORD_EXPIRY_MIN }
-            })
-          : t<string>('Extend the period without password by {{expiration}} minutes', {
-              replace: { expiration: PASSWORD_EXPIRY_MIN }
-            })
-      }
+      label={t<string>('Remember password for {{expiration}} minutes', {
+        replace: { expiration: PASSWORD_EXPIRY_MIN }
+      })}
       onChange={setSavePass}
     />
   );