From 285aa6b6d493794b22528a7273339071ed3ba4fe Mon Sep 17 00:00:00 2001 From: tomiir Date: Wed, 31 Aug 2022 17:04:43 +0100 Subject: [PATCH] Fix/verify whitelist (#576) * Update storage whitelist on verify whitelist * Added canister info fetching for batch transactions when not all canisters are whitelisted. Fixed connection and whitelist check flow by serializing whitelist one and reconcilliating previous whitelist correctly with new one. * Removed showList and updateWhitelist conditionals now not needed in new flow * Removed hidden allow agent component * Removed leftover handler * Serialize metadata into handleAllow agent to properly save the connection data and allow the apps tab to display correct ingo * Serialize metadata in handleAllowAgent call and add url to the persisted app in storage * Fixed modal heights * Fixed issue where disconnected entries were being added even if the app was not connected. Removed storage update on app fetch Co-authored-by: rocky-fleek --- package.json | 2 +- source/Modules/Controller/connection.js | 148 +++--- source/Modules/Controller/transaction.js | 12 +- source/Modules/storageManager.js | 3 +- source/hooks/useApps.jsx | 7 +- .../Views/SendFlow/hooks/useSteps.jsx | 1 - .../Popup/components/AllowAgent/index.jsx | 37 +- .../Popup/components/AppConnection/index.jsx | 9 +- yarn.lock | 485 +++++++++--------- 9 files changed, 335 insertions(+), 369 deletions(-) diff --git a/package.json b/package.json index 22a4b214..8fd640f3 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "plug", - "version": "0.5.4", + "version": "0.5.4.1", "description": "Your plug into the Internet Computer", "private": true, "repository": "https://github.com/Psychedelic/plug", diff --git a/source/Modules/Controller/connection.js b/source/Modules/Controller/connection.js index 88366f89..1d2817f2 100644 --- a/source/Modules/Controller/connection.js +++ b/source/Modules/Controller/connection.js @@ -120,48 +120,27 @@ export class ConnectionModule extends ControllerModuleBase { opts.callback(ERRORS.CANISTER_ID_ERROR, null); return; } - const { message, sender } = opts; + const { message, sender, callback } = opts; const { id: callId } = message.data.data; const { id: portId } = sender; - const { url: domainUrl, name, icons } = metadata; + const { url: domainUrl, icons } = metadata; if (isValidWhitelist) { canistersInfo = await fetchCanistersInfo(whitelist); } - const date = new Date().toISOString(); - + // Generate a whitelist object from the canister Info const populatedWhitelist = canistersInfo.reduce( (accum, canisterInfo) => ({ ...accum, [canisterInfo.id]: canisterInfo }), {}, ); - getApps(this.keyring?.currentWalletId.toString(), (apps = {}) => { - const newApps = { - ...apps, - [domainUrl]: { - url: domainUrl, - name, - status: CONNECTION_STATUS.pending, - icon: icons[0] || null, - timeout, - date, - events: [ - ...apps[domainUrl]?.events || [], - ], - whitelist: populatedWhitelist, - host, - }, - }; - setApps(this.keyring?.currentWalletId.toString(), newApps); - }); - - // if we receive a whitelist, we create agent + // If we receive a whitelist, we open the allow agent modal if (isValidWhitelist) { const newMetadata = { ...metadata, requestConnect: true }; - const height = this.keyring?.isUnlocked - ? Math.min(422 + 37 * whitelist.length, 600) + const fixedHeight = this.keyring?.isUnlocked + ? Math.min(422 + 65 * whitelist.length, 550) : SIZES.loginHeight; this.displayPopUp( @@ -169,35 +148,29 @@ export class ConnectionModule extends ControllerModuleBase { callId, portId, argsJson: JSON.stringify({ - whitelist, canistersInfo, timeout, transactionId, + whitelist: populatedWhitelist, canistersInfo, timeout, transactionId, }), metadataJson: JSON.stringify(newMetadata), domainUrl, type: 'allowAgent', screenArgs: { - fixedHeight: height, + fixedHeight, top: 65, left: metadata.pageWidth - SIZES.width, }, }, - opts.callback, + callback, ); } else { - const height = this.keyring?.isUnlocked - ? SIZES.appConnectHeight - : SIZES.loginHeight; - + // Else it's a plain connection request one this.displayPopUp({ callId, portId, icon: icons[0] || null, argsJson: JSON.stringify({ timeout, transactionId }), type: 'connect', - screenArgs: { - fixedHeight: height, - }, domainUrl, - }, opts.callback); + }, callback); } }, }; @@ -208,37 +181,39 @@ export class ConnectionModule extends ControllerModuleBase { methodName: 'handleAllowAgent', handler: async (opts, url, response, callId, portId) => { const { callback } = opts; + const { status = CONNECTION_STATUS.rejected, whitelist = {} } = response || {}; + if (status === CONNECTION_STATUS.accepted) { + // If connection request was accepted + // we update the storage entry then return the public key - getApps(this.keyring?.currentWalletId.toString(), async (apps = {}) => { - const status = response.status === CONNECTION_STATUS.rejectedAgent - ? CONNECTION_STATUS.accepted - : response.status; - const whitelist = response.status === CONNECTION_STATUS.accepted - ? apps[url]?.whitelist - : []; - - const date = new Date().toISOString(); - - const newApps = { - ...apps, - [url]: { - ...apps[url], - status: status || CONNECTION_STATUS.rejected, - date, - whitelist: { ...apps[url]?.whitelist, ...whitelist }, - events: [ - ...apps[url]?.events || [], - { - status: status || CONNECTION_STATUS.rejected, - date, - }, - ], - }, - }; - setApps(this.keyring?.currentWalletId.toString(), newApps); - }); - - if (response?.status === CONNECTION_STATUS.accepted) { + // Update the storage with this new app, keep old whitelist records and add new ones + getApps(this.keyring?.currentWalletId.toString(), async (apps = {}) => { + const date = new Date().toISOString(); + const app = apps[url] || {}; + const appWhitelist = app?.status === CONNECTION_STATUS.accepted ? app.whitelist : {}; + const { name, host, icons } = response?.metadata || {}; + const newApps = { + ...apps, + [url]: { + ...app, + url, + name, + host, + icon: icons?.[0] || '', + status, + date, + whitelist: { ...appWhitelist, ...whitelist }, + events: [ + ...app?.events?.slice(-20) || [], // Keep only last 20 events + { + status, + date, + }, + ], + }, + }; + setApps(this.keyring?.currentWalletId.toString(), newApps); + }); try { const publicKey = await this.keyring?.getPublicKey(); callback(null, publicKey, [{ portId, callId }]); @@ -248,6 +223,9 @@ export class ConnectionModule extends ControllerModuleBase { callback(null, false); } } else { + // If the call was not accepted, then we do nothing to storage + // since previously accepted apps are still there and a disconnect + // entry would be added in `disconnect` method callback(ERRORS.AGENT_REJECTED, null, [{ portId, callId }]); callback(null, true); // Return true to close the modal } @@ -266,16 +244,17 @@ export class ConnectionModule extends ControllerModuleBase { let canistersInfo = []; + // Validate whiltelist and if valid, get canisters Info const isValidWhitelist = Array.isArray(whitelist) && whitelist.length; - if (isValidWhitelist) { canistersInfo = await fetchCanistersInfo(whitelist); } - if (!whitelist.every((canisterId) => validatePrincipalId(canisterId))) { + if (!whitelist?.every((canisterId) => validatePrincipalId(canisterId))) { callback(ERRORS.CANISTER_ID_ERROR, null); return; } + // Get saved app and check if all of the entries received are there getApps(this.keyring?.currentWalletId.toString(), async (apps = {}) => { const app = apps?.[metadata.url] || {}; if (app?.status === CONNECTION_STATUS.accepted) { @@ -283,51 +262,54 @@ export class ConnectionModule extends ControllerModuleBase { whitelist, app?.whitelist ? Object.keys(app?.whitelist) : [], ); - const height = this.keyring?.isUnlocked + const fixedHeight = this.keyring?.isUnlocked ? SIZES.detailHeightSmall : SIZES.loginHeight; - + const populatedWhitelist = canistersInfo.reduce( + (accum, canisterInfo) => ({ ...accum, [canisterInfo.id]: canisterInfo }), + {}, + ); if (allWhitelisted) { - if (!this.keyring.isUnlocked) { + // If keyring is unlocked then we return the public key + if (this.keyring.isUnlocked) { + const publicKey = await this.keyring?.getPublicKey(); + callback(null, publicKey); + } else { + // If locked we need to display the unlock modal this.displayPopUp({ callId, portId, metadataJson: JSON.stringify(metadata), argsJson: JSON.stringify({ - whitelist, + whitelist: populatedWhitelist, canistersInfo, - updateWhitelist: true, - showList: false, timeout: app?.timeout, transactionId, }), domainUrl: metadata.url, type: 'allowAgent', screenArgs: { - fixedHeight: height, + fixedHeight, top: 65, left: metadata.pageWidth - SIZES.width, }, }, callback); } - const publicKey = await this.keyring?.getPublicKey(); - callback(null, publicKey); } else { + // If they are not all whitelisted, we need to show the allow agent modal this.displayPopUp({ callId, portId, metadataJson: JSON.stringify(metadata), domainUrl: metadata.url, argsJson: JSON.stringify({ - whitelist, + whitelist: populatedWhitelist, canistersInfo, - updateWhitelist: true, - showList: true, transactionId, }), type: 'allowAgent', screenArgs: { - fixedHeight: height, + fixedHeight, top: 65, left: metadata.pageWidth - SIZES.width, }, diff --git a/source/Modules/Controller/transaction.js b/source/Modules/Controller/transaction.js index 5a372db2..20118780 100644 --- a/source/Modules/Controller/transaction.js +++ b/source/Modules/Controller/transaction.js @@ -10,6 +10,7 @@ import { bufferToBase64, handleCallRequest, generateRequestInfo, + fetchCanistersInfo, } from '@background/utils'; import { CONNECTION_STATUS } from '@shared/constants/connectionStatus'; import { ICP_CANISTER_ID } from '@shared/constants/canisters'; @@ -370,7 +371,16 @@ export class TransactionModule extends ControllerModuleBase { callback(transactionsError, null); return; } - const canistersInfo = app?.whitelist || {}; + const whitelist = app?.whitelist || {}; + // Check if all the canisters are whitelisted + const transactionCanisterIds = transactions.map(({ canisterId }) => canisterId); + let canistersInfo = whitelist || {}; + if (!transactionCanisterIds.every((id) => Object.keys(whitelist).includes(id))) { + const canistersData = await fetchCanistersInfo(transactionCanisterIds); + canistersInfo = canistersData.reduce( + (acum, canister) => ({ ...acum, [canister.canisterId]: canister }), {}, + ); + } const transactionsWithInfo = transactions.map((tx) => ({ ...tx, canisterInfo: canistersInfo[tx.canisterId], diff --git a/source/Modules/storageManager.js b/source/Modules/storageManager.js index 9630fd3a..dd8f8665 100644 --- a/source/Modules/storageManager.js +++ b/source/Modules/storageManager.js @@ -47,9 +47,8 @@ export const setApps = (currentWalletId, apps, cb = () => { }) => { export const removeApp = (currentWalletId, appUrl, cb = () => { }) => { const defaultValue = false; - getApps(currentWalletId, (apps) => { - if (apps?.[appUrl]) { + if (apps?.[appUrl]?.status === CONNECTION_STATUS.accepted) { const newApps = addDisconnectedEntry({ apps, appUrl }); setApps(currentWalletId, newApps, cb); } else { diff --git a/source/hooks/useApps.jsx b/source/hooks/useApps.jsx index 1328a1aa..f43c1cc0 100644 --- a/source/hooks/useApps.jsx +++ b/source/hooks/useApps.jsx @@ -3,7 +3,6 @@ import { useSelector } from 'react-redux'; import { CONNECTION_STATUS } from '@shared/constants/connectionStatus'; import { addDisconnectedEntry } from '@shared/utils/apps'; import { getApps, setApps as setStorageApps } from '@modules'; -import extensionizer from 'extensionizer'; const useApps = () => { const [apps, setApps] = useState({}); @@ -25,11 +24,7 @@ const useApps = () => { }, [walletNumber]); useEffect(() => { - setStorageApps(walletNumber, apps, () => { - extensionizer.tabs.query({ active: true }, (activeTabs) => { - extensionizer.tabs.sendMessage(activeTabs[0].id, { action: 'updateConnection' }); - }); - }); + setStorageApps(walletNumber, apps); const parsed = Object.values(apps) || []; const allEvents = parsed?.flatMap((app) => app?.events?.map((event) => ({ diff --git a/source/views/Extension/Views/SendFlow/hooks/useSteps.jsx b/source/views/Extension/Views/SendFlow/hooks/useSteps.jsx index cd001fbe..9258fe12 100644 --- a/source/views/Extension/Views/SendFlow/hooks/useSteps.jsx +++ b/source/views/Extension/Views/SendFlow/hooks/useSteps.jsx @@ -215,7 +215,6 @@ const useSteps = () => { } if (!hasAmount && !hasPrice) { - console.log(444) dispatch(setSendTokenSelectedAsset({ icpPrice, value: assets[0] })); } } diff --git a/source/views/Popup/components/AllowAgent/index.jsx b/source/views/Popup/components/AllowAgent/index.jsx index 708e1483..bb24b0ba 100644 --- a/source/views/Popup/components/AllowAgent/index.jsx +++ b/source/views/Popup/components/AllowAgent/index.jsx @@ -61,7 +61,7 @@ const AllowAgent = ({ reviewPendingTransaction(transactionId, () => {}); const success = await portRPC.call('handleAllowAgent', [ url, - { status }, + { status, whitelist: args.whitelist || {}, metadata }, callId, portId, transactionId, @@ -85,20 +85,8 @@ const AllowAgent = ({ dispatch(setAccountInfo(state.wallets[state.currentWalletId])); } }); - - if (!args?.updateWhitelist || args?.showList) { - extension.windows.update(extension.windows.WINDOW_ID_CURRENT, { - height: 355 - + (canistersLength > 2 ? 76 : 30) - + 65 * (canistersLength > 2 ? 2 : canistersLength), - }); - } else { - handleAllowAgent(CONNECTION_STATUS.accepted).then(() => { - setHandled(true); - window?.close?.(); - }); - } - }, []); + }, + []); window.onbeforeunload = () => { if (!handled) { @@ -107,14 +95,7 @@ const AllowAgent = ({ }; const toggleExpand = () => { - let height; - - if (expand) { - height = 355 + 76 + 65 * Math.min(canistersLength, 2); - } else { - height = 355 + 76 + 65 * Math.min(canistersLength, 5); - } - + const height = 430 + 65 * Math.min(canistersLength, expand ? 2 : 5); extension.windows.update(extension.windows.WINDOW_ID_CURRENT, { height, }); @@ -122,7 +103,7 @@ const AllowAgent = ({ setExpand((prevState) => !prevState); }; - return !args?.updateWhitelist || args?.showList ? ( + return ( @@ -171,11 +152,7 @@ const AllowAgent = ({