Password caching refactoring (#489)

* refactor password caching

* use an update cache function

* rename update -> refresh

* reject when password expired

* refactor the refactoring without messaging

* lint

* Update packages/extension-base/src/defaults.ts

* use timeout in popup

Co-authored-by: Github Actions <action@github.com>
Co-authored-by: Jaco Greeff <jacogr@gmail.com>

packages/extension-base/src/background/handlers/Extension.ts

@@ -3,7 +3,7 @@
 
 import { MetadataDef } from '@polkadot/extension-inject/types';
 import { SubjectInfo } from '@polkadot/ui-keyring/observable/types';
-import { AccountJson, AllowedPath, AuthorizeRequest, MessageTypes, MetadataRequest, RequestAccountCreateExternal, RequestAccountCreateSuri, RequestAccountEdit, RequestAccountExport, RequestAccountShow, RequestAccountTie, RequestAccountValidate, RequestAuthorizeApprove, RequestAuthorizeReject, RequestDeriveCreate, ResponseDeriveValidate, RequestMetadataApprove, RequestMetadataReject, RequestSigningApprovePassword, RequestSigningApproveSignature, RequestSigningCancel, RequestSigningIsLocked, RequestSeedCreate, RequestTypes, ResponseAccountExport, RequestAccountForget, ResponseSeedCreate, RequestSeedValidate, RequestDeriveValidate, ResponseSeedValidate, ResponseType, SigningRequest, RequestJsonRestore, ResponseJsonRestore, RequestAccountChangePassword } from '../types';
+import { AccountJson, AllowedPath, AuthorizeRequest, MessageTypes, MetadataRequest, RequestAccountChangePassword, RequestAccountCreateExternal, RequestAccountCreateSuri, RequestAccountEdit, RequestAccountExport, RequestAccountShow, RequestAccountTie, RequestAccountValidate, RequestAuthorizeApprove, RequestAuthorizeReject, RequestDeriveCreate, ResponseDeriveValidate, RequestMetadataApprove, RequestMetadataReject, RequestSigningApprovePassword, RequestSigningApproveSignature, RequestSigningCancel, RequestSigningIsLocked, RequestSeedCreate, RequestTypes, ResponseAccountExport, RequestAccountForget, ResponseSeedCreate, RequestSeedValidate, RequestDeriveValidate, RequestJsonRestore, ResponseJsonRestore, ResponseSeedValidate, ResponseSigningIsLocked, ResponseType, SigningRequest } from '../types';
 
 import { ALLOWED_PATH, PASSWORD_EXPIRY_MS } from '@polkadot/extension-base/defaults';
 import chrome from '@polkadot/extension-inject/chrome';
@@ -94,6 +94,22 @@ export default class Extension {
     return true;
   }
 
+  private refreshAccountPasswordCache (pair: KeyringPair): number {
+    const { address } = pair;
+
+    const savedExpiry = this.#cachedUnlocks[address] || 0;
+    const remainingTime = savedExpiry - Date.now();
+
+    if (remainingTime < 0) {
+      this.#cachedUnlocks[address] = 0;
+      pair.lock();
+
+      return 0;
+    }
+
+    return remainingTime;
+  }
+
   private accountsShow ({ address, isShowing }: RequestAccountShow): boolean {
     const pair = keyring.getPair(address);
 
@@ -287,13 +303,18 @@ export default class Extension {
     };
   }
 
-  private signingApprovePassword ({ id, isSavedPass, password }: RequestSigningApprovePassword): boolean {
+  private signingApprovePassword ({ id, password, savePass }: RequestSigningApprovePassword): boolean {
     const queued = this.#state.getSignRequest(id);
 
     assert(queued, 'Unable to find request');
 
     const { reject, request, resolve } = queued;
-    const pair = keyring.getPair(request.payload.address);
+    const pair = keyring.getPair(queued.account.address);
+
+    // unlike queued.account.address the following
+    // address is encoded with the default prefix
+    // which what is used for password caching mapping
+    const { address } = pair;
 
     if (!pair) {
       reject(new Error('Unable to find pair'));
@@ -301,19 +322,22 @@ export default class Extension {
       return false;
     }
 
-    const now = Date.now();
+    this.refreshAccountPasswordCache(pair);
 
-    if (pair.isLocked || password) {
+    // if the keyring pair is locked, the password is needed
+    if (pair.isLocked && !password) {
+      reject(new Error('Password needed to unlock the account'));
+    }
+
+    if (pair.isLocked) {
       pair.decodePkcs8(password);
     }
 
     const result = request.sign(registry, pair);
-    const savedExpiry = this.#cachedUnlocks[request.payload.address] || 0;
 
-    if (isSavedPass) {
-      this.#cachedUnlocks[request.payload.address] = now + PASSWORD_EXPIRY_MS;
-    } else if (savedExpiry < now) {
-      this.#cachedUnlocks[request.payload.address] = 0;
+    if (savePass) {
+      this.#cachedUnlocks[address] = Date.now() + PASSWORD_EXPIRY_MS;
+    } else {
       pair.lock();
     }
 
@@ -349,17 +373,22 @@ export default class Extension {
     return true;
   }
 
-  private signingIsLocked ({ id }: RequestSigningIsLocked): boolean {
+  private signingIsLocked ({ id }: RequestSigningIsLocked): ResponseSigningIsLocked {
     const queued = this.#state.getSignRequest(id);
 
     assert(queued, 'Unable to find request');
 
-    const { request } = queued;
-    const pair = keyring.getPair(request.payload.address);
+    const address = queued.request.payload.address;
+    const pair = keyring.getPair(address);
 
     assert(pair, 'Unable to find pair');
 
-    return pair.isLocked || ((this.#cachedUnlocks[request.payload.address] || 0) < Date.now());
+    const remainingTime = this.refreshAccountPasswordCache(pair);
+
+    return {
+      isLocked: pair.isLocked,
+      remainingTime
+    };
   }
 
   // FIXME This looks very much like what we have in authorization

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

@@ -97,7 +97,7 @@ export interface RequestSignatures {
   'pri(signing.approve.password)': [RequestSigningApprovePassword, boolean];
   'pri(signing.approve.signature)': [RequestSigningApproveSignature, boolean];
   'pri(signing.cancel)': [RequestSigningCancel, boolean];
-  'pri(signing.isLocked)': [RequestSigningIsLocked, boolean];
+  'pri(signing.isLocked)': [RequestSigningIsLocked, ResponseSigningIsLocked];
   'pri(signing.requests)': [RequestSigningSubscribe, boolean, SigningRequest[]];
   'pri(window.open)': [AllowedPath, boolean];
   // public/external requests, i.e. from a page
@@ -244,8 +244,8 @@ export interface RequestRpcUnsubscribe {
 
 export interface RequestSigningApprovePassword {
   id: string;
-  isSavedPass: boolean;
-  password: string;
+  password?: string;
+  savePass: boolean;
 }
 
 export interface RequestSigningApproveSignature {
@@ -261,6 +261,11 @@ export interface RequestSigningIsLocked {
   id: string;
 }
 
+export interface ResponseSigningIsLocked {
+  isLocked: boolean;
+  remainingTime: number;
+}
+
 export type RequestSigningSubscribe = null;
 
 export interface RequestSeedCreate {

packages/extension-ui/src/Popup/Signing/Request.tsx

@@ -5,7 +5,7 @@ import { ExtrinsicPayload } from '@polkadot/types/interfaces';
 import { AccountJson, RequestSign } from '@polkadot/extension-base/background/types';
 import { SignerPayloadJSON, SignerPayloadRaw } from '@polkadot/types/types';
 
-import React, { useCallback, useContext, useState, useEffect } from 'react';
+import React, { useCallback, useEffect, useContext, useState } from 'react';
 import { PASSWORD_EXPIRY_MIN } from '@polkadot/extension-base/defaults';
 import { TypeRegistry } from '@polkadot/types';
 
@@ -46,14 +46,22 @@ export default function Request ({ account: { isExternal }, buttonText, isFirst,
   const [error, setError] = useState<string | null>(null);
   const [isBusy, setIsBusy] = useState(false);
   const [isLocked, setIsLocked] = useState<boolean | null>(null);
-  const [isSavedPass, setIsSavedPass] = useState(false);
+  const [savePass, setSavePass] = useState(false);
 
-  useEffect((): void => {
+  useEffect(() => {
     setIsLocked(null);
+    let timeout: number;
 
     !isExternal && isSignLocked(signId)
-      .then((isLocked) => setIsLocked(isLocked))
+      .then(({ isLocked, remainingTime }) => {
+        setIsLocked(isLocked);
+        timeout = setTimeout(() => {
+          setIsLocked(true);
+        }, remainingTime);
+      })
       .catch((error: Error) => console.error(error));
+
+    return () => { !!timeout && clearTimeout(timeout); };
   }, [isExternal, signId]);
 
   useEffect((): void => {
@@ -82,10 +90,10 @@ export default function Request ({ account: { isExternal }, buttonText, isFirst,
   );
 
   const _onSign = useCallback(
-    (password: string): Promise<void> => {
+    (password?: string): Promise<void> => {
       setIsBusy(true);
 
-      return approveSignPassword(signId, password, !!(password && isSavedPass))
+      return approveSignPassword(signId, savePass, password)
         .then((): void => {
           setIsBusy(false);
           onAction();
@@ -96,11 +104,11 @@ export default function Request ({ account: { isExternal }, buttonText, isFirst,
           console.error(error);
         });
     },
-    [onAction, isSavedPass, signId]
+    [onAction, savePass, signId]
   );
 
   const _onSignQuick = useCallback(
-    () => _onSign(''),
+    () => _onSign(),
     [_onSign]
   );
 
@@ -124,12 +132,12 @@ export default function Request ({ account: { isExternal }, buttonText, isFirst,
         onSign={_onSign}
       >
         <Checkbox
-          checked={!!isSavedPass}
+          checked={savePass}
           label={t<string>(
             "Don't ask me again for the next {{expiration}} minutes",
             { replace: { expiration: PASSWORD_EXPIRY_MIN } }
           )}
-          onChange={setIsSavedPass}
+          onChange={setSavePass}
         />
       </Unlock>
     )

packages/extension-ui/src/Popup/Signing/Unlock.tsx

@@ -52,6 +52,7 @@ function Unlock ({ buttonText, children, className, error, isBusy, onSign }: Pro
       {children}
       <Button
         isBusy={isBusy}
+        isDisabled={!password}
         onClick={_onClick}
       >
         {buttonText}

packages/extension-ui/src/messaging.ts

@@ -2,7 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 import { Message } from '@polkadot/extension-base/types';
-import { AccountJson, AllowedPath, AuthorizeRequest, SigningRequest, RequestTypes, MessageTypes, ResponseTypes, SeedLengths, SubscriptionMessageTypes, MetadataRequest, MessageTypesWithNullRequest, MessageTypesWithNoSubscriptions, MessageTypesWithSubscriptions, ResponseDeriveValidate, ResponseJsonRestore } from '@polkadot/extension-base/background/types';
+import { AccountJson, AllowedPath, AuthorizeRequest, SigningRequest, RequestTypes, MessageTypes, ResponseTypes, SeedLengths, SubscriptionMessageTypes, MetadataRequest, MessageTypesWithNullRequest, MessageTypesWithNoSubscriptions, MessageTypesWithSubscriptions, ResponseDeriveValidate, ResponseJsonRestore, ResponseSigningIsLocked } from '@polkadot/extension-base/background/types';
 import { Chain } from '@polkadot/extension-chains/types';
 import { MetadataDef } from '@polkadot/extension-inject/types';
 import { KeypairType } from '@polkadot/util-crypto/types';
@@ -101,12 +101,12 @@ export async function cancelSignRequest (id: string): Promise<boolean> {
   return sendMessage('pri(signing.cancel)', { id });
 }
 
-export async function isSignLocked (id: string): Promise<boolean> {
+export async function isSignLocked (id: string): Promise<ResponseSigningIsLocked> {
   return sendMessage('pri(signing.isLocked)', { id });
 }
 
-export async function approveSignPassword (id: string, password: string, isSavedPass: boolean): Promise<boolean> {
-  return sendMessage('pri(signing.approve.password)', { id, isSavedPass, password });
+export async function approveSignPassword (id: string, savePass: boolean, password?: string): Promise<boolean> {
+  return sendMessage('pri(signing.approve.password)', { id, password, savePass });
 }
 
 export async function approveSignSignature (id: string, signature: string): Promise<boolean> {