From 4ef4d3f88092c98af1f3dd66d5b0d3848e978fa1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Harrison=20Mendon=C3=A7a?= Date: Wed, 11 Sep 2024 12:34:20 +0200 Subject: [PATCH 01/14] feat: rename SwitchAccountPanel --- apps/namadillo/src/App/AppRoutes.tsx | 4 ++-- .../{SwitchAccountModal.tsx => SwitchAccountPanel.tsx} | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) rename apps/namadillo/src/App/SwitchAccount/{SwitchAccountModal.tsx => SwitchAccountPanel.tsx} (97%) diff --git a/apps/namadillo/src/App/AppRoutes.tsx b/apps/namadillo/src/App/AppRoutes.tsx index aea729966..6facd7bf2 100644 --- a/apps/namadillo/src/App/AppRoutes.tsx +++ b/apps/namadillo/src/App/AppRoutes.tsx @@ -13,7 +13,7 @@ import { RouteErrorBoundary } from "./Common/RouteErrorBoundary"; import { Governance } from "./Governance"; import { SettingsPanel } from "./Settings/SettingsPanel"; import { Staking } from "./Staking"; -import { SwitchAccountModal } from "./SwitchAccount/SwitchAccountModal"; +import { SwitchAccountPanel } from "./SwitchAccount/SwitchAccountPanel"; import GovernanceRoutes from "./Governance/routes"; import SettingsRoutes from "./Settings/routes"; @@ -52,7 +52,7 @@ export const MainRoutes = (): JSX.Element => { /> } + element={} errorElement={} /> { +export const SwitchAccountPanel = (): JSX.Element => { const { onCloseModal } = useModalCloseEvent(); const { data: defaultAccount } = useAtomValue(defaultAccountAtom); const { data } = useAtomValue(accountsAtom); From eea10c9cd6be6885fc25f7813cd72b2edc3cc3dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Harrison=20Mendon=C3=A7a?= Date: Wed, 11 Sep 2024 12:42:42 +0200 Subject: [PATCH 02/14] feat: anchor panel top right --- .../src/App/SwitchAccount/SwitchAccountPanel.tsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/apps/namadillo/src/App/SwitchAccount/SwitchAccountPanel.tsx b/apps/namadillo/src/App/SwitchAccount/SwitchAccountPanel.tsx index cf35c719c..9e230817a 100644 --- a/apps/namadillo/src/App/SwitchAccount/SwitchAccountPanel.tsx +++ b/apps/namadillo/src/App/SwitchAccount/SwitchAccountPanel.tsx @@ -18,7 +18,13 @@ export const SwitchAccountPanel = (): JSX.Element => { const { mutateAsync: updateAccount } = useAtomValue(updateDefaultAccountAtom); return ( - +
Date: Wed, 11 Sep 2024 14:17:12 +0200 Subject: [PATCH 03/14] feat: add approval step to disconnect --- apps/extension/src/Approvals/Approvals.tsx | 5 +++ .../src/Approvals/ApproveDisconnect.tsx | 45 +++++++++++++++++++ apps/extension/src/Approvals/types.ts | 1 + .../src/background/approvals/handler.ts | 14 ++++++ .../src/background/approvals/init.ts | 2 + .../src/background/approvals/service.ts | 25 +++++++++++ apps/extension/src/provider/Namada.ts | 4 +- apps/extension/src/provider/messages.ts | 23 ++++++++++ 8 files changed, 117 insertions(+), 2 deletions(-) create mode 100644 apps/extension/src/Approvals/ApproveDisconnect.tsx diff --git a/apps/extension/src/Approvals/Approvals.tsx b/apps/extension/src/Approvals/Approvals.tsx index 44dcaa866..c418ebabb 100644 --- a/apps/extension/src/Approvals/Approvals.tsx +++ b/apps/extension/src/Approvals/Approvals.tsx @@ -7,6 +7,7 @@ import { AccountType, TxDetails } from "@namada/types"; import { AppHeader } from "App/Common/AppHeader"; import { TopLevelRoute } from "Approvals/types"; import { ApproveConnection } from "./ApproveConnection"; +import { ApproveDisconnect } from "./ApproveDisconnect"; import { ApproveSignArbitrary } from "./ApproveSignArbitrary"; import { ApproveSignTx } from "./ApproveSignTx"; import { ConfirmSignature } from "./ConfirmSignArbitrary"; @@ -65,6 +66,10 @@ export const Approvals: React.FC = () => { path={TopLevelRoute.ApproveConnection} element={} /> + } + /> { + const requester = useRequester(); + const params = useQuery(); + const interfaceOrigin = params.get("interfaceOrigin"); + + const handleResponse = async (revokeConnection: boolean): Promise => { + if (revokeConnection && interfaceOrigin) { + await requester.sendMessage( + Ports.Background, + new RevokeConnectionMsg(interfaceOrigin) + ); + } + await closeCurrentTab(); + }; + + return ( + + + + + Approve disconnect for {interfaceOrigin}? + + + handleResponse(true)}> + Approve + + handleResponse(false)} + > + Reject + + + + + ); +}; diff --git a/apps/extension/src/Approvals/types.ts b/apps/extension/src/Approvals/types.ts index 8578a1a28..b12efb18b 100644 --- a/apps/extension/src/Approvals/types.ts +++ b/apps/extension/src/Approvals/types.ts @@ -5,6 +5,7 @@ export enum TopLevelRoute { // Connection approval ApproveConnection = "/approve-connection", + ApproveDisconnect = "/approve-disconnect", // Sign Tx approval ApproveSignTx = "/approve-sign-tx", diff --git a/apps/extension/src/background/approvals/handler.ts b/apps/extension/src/background/approvals/handler.ts index 02153b0cd..c4a5b4b0b 100644 --- a/apps/extension/src/background/approvals/handler.ts +++ b/apps/extension/src/background/approvals/handler.ts @@ -1,5 +1,6 @@ import { ApproveConnectInterfaceMsg, + ApproveDisconnectInterfaceMsg, ApproveSignArbitraryMsg, ApproveSignTxMsg, IsConnectionApprovedMsg, @@ -37,6 +38,11 @@ export const getHandler: (service: ApprovalsService) => Handler = (service) => { env, msg as ConnectInterfaceResponseMsg ); + case ApproveDisconnectInterfaceMsg: + return handleApproveDisconnectInterfaceMsg(service)( + env, + msg as ApproveDisconnectInterfaceMsg + ); case RevokeConnectionMsg: return handleRevokeConnectionMsg(service)( env, @@ -122,6 +128,14 @@ const handleConnectInterfaceResponseMsg: ( }; }; +const handleApproveDisconnectInterfaceMsg: ( + service: ApprovalsService +) => InternalHandler = (service) => { + return async (_, { origin }) => { + return await service.approveDisconnect(origin); + }; +}; + const handleRevokeConnectionMsg: ( service: ApprovalsService ) => InternalHandler = (service) => { diff --git a/apps/extension/src/background/approvals/init.ts b/apps/extension/src/background/approvals/init.ts index dcf3cc9be..dc190c805 100644 --- a/apps/extension/src/background/approvals/init.ts +++ b/apps/extension/src/background/approvals/init.ts @@ -1,5 +1,6 @@ import { ApproveConnectInterfaceMsg, + ApproveDisconnectInterfaceMsg, ApproveSignArbitraryMsg, ApproveSignTxMsg, IsConnectionApprovedMsg, @@ -32,6 +33,7 @@ export function init(router: Router, service: ApprovalsService): void { router.registerMessage(SubmitApprovedSignLedgerTxMsg); router.registerMessage(IsConnectionApprovedMsg); router.registerMessage(ApproveConnectInterfaceMsg); + router.registerMessage(ApproveDisconnectInterfaceMsg); router.registerMessage(ConnectInterfaceResponseMsg); router.registerMessage(RevokeConnectionMsg); router.registerMessage(QueryTxDetailsMsg); diff --git a/apps/extension/src/background/approvals/service.ts b/apps/extension/src/background/approvals/service.ts index b255be9f6..4cfe3be7f 100644 --- a/apps/extension/src/background/approvals/service.ts +++ b/apps/extension/src/background/approvals/service.ts @@ -7,6 +7,7 @@ import { SignArbitraryResponse, TxDetails } from "@namada/types"; import { paramsToUrl } from "@namada/utils"; import { ResponseSign } from "@zondax/ledger-namada"; +import { TopLevelRoute } from "Approvals/types"; import { ChainsService } from "background/chains"; import { KeyRingService } from "background/keyring"; import { SdkService } from "background/sdk"; @@ -281,6 +282,30 @@ export class ApprovalsService { } } + async approveDisconnect(interfaceOrigin: string): Promise { + const baseUrl = `${browser.runtime.getURL( + "approvals.html" + )}#${TopLevelRoute.ApproveDisconnect}`; + + const url = paramsToUrl(baseUrl, { + interfaceOrigin, + }); + + const popupTabId = await this.getPopupTabId(url); + + if (!popupTabId) { + throw new Error("no popup tab ID"); + } + + if (popupTabId in this.resolverMap) { + throw new Error(`tab ID ${popupTabId} already exists in promise map`); + } + + return new Promise((resolve, reject) => { + this.resolverMap[popupTabId] = { resolve, reject }; + }); + } + async revokeConnection(originToRevoke: string): Promise { await this.localStorage.removeApprovedOrigin(originToRevoke); await this.broadcaster.revokeConnection(); diff --git a/apps/extension/src/provider/Namada.ts b/apps/extension/src/provider/Namada.ts index 498414ad1..6735fa520 100644 --- a/apps/extension/src/provider/Namada.ts +++ b/apps/extension/src/provider/Namada.ts @@ -9,10 +9,10 @@ import { } from "@namada/types"; import { MessageRequester, Ports } from "router"; -import { RevokeConnectionMsg } from "background/approvals"; import { toEncodedTx } from "utils"; import { ApproveConnectInterfaceMsg, + ApproveDisconnectInterfaceMsg, ApproveSignArbitraryMsg, ApproveSignTxMsg, CheckDurabilityMsg, @@ -40,7 +40,7 @@ export class Namada implements INamada { public async disconnect(): Promise { return await this.requester?.sendMessage( Ports.Background, - new RevokeConnectionMsg(location.origin) + new ApproveDisconnectInterfaceMsg(location.origin) ); } diff --git a/apps/extension/src/provider/messages.ts b/apps/extension/src/provider/messages.ts index 773cf0f9b..d53f3f7ee 100644 --- a/apps/extension/src/provider/messages.ts +++ b/apps/extension/src/provider/messages.ts @@ -20,6 +20,7 @@ enum MessageType { ApproveSignArbitrary = "approve-sign-arbitrary", IsConnectionApproved = "is-connection-approved", ApproveConnectInterface = "approve-connect-interface", + ApproveDisconnectInterface = "approve-disconnect-interface", QueryAccounts = "query-accounts", QueryDefaultAccount = "query-default-account", UpdateDefaultAccount = "update-default-account", @@ -134,6 +135,28 @@ export class ApproveConnectInterfaceMsg extends Message { } } +export class ApproveDisconnectInterfaceMsg extends Message { + public static type(): MessageType { + return MessageType.ApproveDisconnectInterface; + } + + constructor(public readonly originToRevoke: string) { + super(); + } + + validate(): void { + validateProps(this, ["originToRevoke"]); + } + + route(): string { + return Route.Approvals; + } + + type(): string { + return ApproveDisconnectInterfaceMsg.type(); + } +} + export class GetChainMsg extends Message { public static type(): MessageType { return MessageType.GetChain; From 4754eb2ae162ab08781d7ecd86fc83a33ced9a51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Harrison=20Mendon=C3=A7a?= Date: Thu, 12 Sep 2024 12:05:32 +0200 Subject: [PATCH 04/14] feat: add new message when disconnect is approved --- .../src/Approvals/ApproveDisconnect.tsx | 13 +++-- .../src/background/approvals/handler.ts | 26 +++++++++- .../src/background/approvals/init.ts | 2 + .../src/background/approvals/messages.ts | 31 +++++++++++ .../src/background/approvals/service.ts | 52 +++++++++++++++---- 5 files changed, 109 insertions(+), 15 deletions(-) diff --git a/apps/extension/src/Approvals/ApproveDisconnect.tsx b/apps/extension/src/Approvals/ApproveDisconnect.tsx index b93fd0ba8..01d6bcc9b 100644 --- a/apps/extension/src/Approvals/ApproveDisconnect.tsx +++ b/apps/extension/src/Approvals/ApproveDisconnect.tsx @@ -1,6 +1,6 @@ import { ActionButton, Alert, GapPatterns, Stack } from "@namada/components"; import { PageHeader } from "App/Common"; -import { RevokeConnectionMsg } from "background/approvals"; +import { DisconnectInterfaceResponseMsg } from "background/approvals"; import { useQuery } from "hooks"; import { useRequester } from "hooks/useRequester"; import { Ports } from "router"; @@ -10,15 +10,20 @@ export const ApproveDisconnect: React.FC = () => { const requester = useRequester(); const params = useQuery(); const interfaceOrigin = params.get("interfaceOrigin"); + const interfaceTabId = params.get("interfaceTabId"); const handleResponse = async (revokeConnection: boolean): Promise => { - if (revokeConnection && interfaceOrigin) { + if (interfaceTabId && interfaceOrigin) { await requester.sendMessage( Ports.Background, - new RevokeConnectionMsg(interfaceOrigin) + new DisconnectInterfaceResponseMsg( + parseInt(interfaceTabId), + interfaceOrigin, + revokeConnection + ) ); + await closeCurrentTab(); } - await closeCurrentTab(); }; return ( diff --git a/apps/extension/src/background/approvals/handler.ts b/apps/extension/src/background/approvals/handler.ts index c4a5b4b0b..07bba7bfe 100644 --- a/apps/extension/src/background/approvals/handler.ts +++ b/apps/extension/src/background/approvals/handler.ts @@ -8,6 +8,7 @@ import { import { Env, Handler, InternalHandler, Message } from "router"; import { ConnectInterfaceResponseMsg, + DisconnectInterfaceResponseMsg, QueryPendingTxBytesMsg, QuerySignArbitraryDataMsg, QueryTxDetailsMsg, @@ -43,6 +44,11 @@ export const getHandler: (service: ApprovalsService) => Handler = (service) => { env, msg as ApproveDisconnectInterfaceMsg ); + case DisconnectInterfaceResponseMsg: + return handleDisconnectInterfaceResponseMsg(service)( + env, + msg as DisconnectInterfaceResponseMsg + ); case RevokeConnectionMsg: return handleRevokeConnectionMsg(service)( env, @@ -131,8 +137,24 @@ const handleConnectInterfaceResponseMsg: ( const handleApproveDisconnectInterfaceMsg: ( service: ApprovalsService ) => InternalHandler = (service) => { - return async (_, { origin }) => { - return await service.approveDisconnect(origin); + return async ({ senderTabId: interfaceTabId }, { origin }) => { + return await service.approveDisconnect(interfaceTabId, origin); + }; +}; + +const handleDisconnectInterfaceResponseMsg: ( + service: ApprovalsService +) => InternalHandler = (service) => { + return async ( + { senderTabId: popupTabId }, + { interfaceTabId, interfaceOrigin, revokeConnection } + ) => { + return await service.approveDisconnectionResponse( + interfaceTabId, + interfaceOrigin, + revokeConnection, + popupTabId + ); }; }; diff --git a/apps/extension/src/background/approvals/init.ts b/apps/extension/src/background/approvals/init.ts index dc190c805..7c32746a2 100644 --- a/apps/extension/src/background/approvals/init.ts +++ b/apps/extension/src/background/approvals/init.ts @@ -8,6 +8,7 @@ import { import { Router } from "router"; import { ConnectInterfaceResponseMsg, + DisconnectInterfaceResponseMsg, QueryPendingTxBytesMsg, QuerySignArbitraryDataMsg, QueryTxDetailsMsg, @@ -35,6 +36,7 @@ export function init(router: Router, service: ApprovalsService): void { router.registerMessage(ApproveConnectInterfaceMsg); router.registerMessage(ApproveDisconnectInterfaceMsg); router.registerMessage(ConnectInterfaceResponseMsg); + router.registerMessage(DisconnectInterfaceResponseMsg); router.registerMessage(RevokeConnectionMsg); router.registerMessage(QueryTxDetailsMsg); router.registerMessage(QuerySignArbitraryDataMsg); diff --git a/apps/extension/src/background/approvals/messages.ts b/apps/extension/src/background/approvals/messages.ts index 2ee94399c..8062a588f 100644 --- a/apps/extension/src/background/approvals/messages.ts +++ b/apps/extension/src/background/approvals/messages.ts @@ -12,6 +12,7 @@ export enum MessageType { SubmitApprovedSignLedgerTx = "submit-approved-sign-ledger-tx", RejectSignArbitrary = "reject-sign-arbitrary", ConnectInterfaceResponse = "connect-interface-response", + DisconnectInterfaceResponse = "disconnect-interface-response", RevokeConnection = "revoke-connection", QueryTxDetails = "query-tx-details", QuerySignArbitraryData = "query-sign-arbitrary-data", @@ -168,6 +169,36 @@ export class ConnectInterfaceResponseMsg extends Message { } } +export class DisconnectInterfaceResponseMsg extends Message { + public static type(): MessageType { + return MessageType.DisconnectInterfaceResponse; + } + + constructor( + public readonly interfaceTabId: number, + public readonly interfaceOrigin: string, + public readonly revokeConnection: boolean + ) { + super(); + } + + validate(): void { + validateProps(this, [ + "interfaceTabId", + "interfaceOrigin", + "revokeConnection", + ]); + } + + route(): string { + return ROUTE; + } + + type(): string { + return DisconnectInterfaceResponseMsg.type(); + } +} + export class RevokeConnectionMsg extends Message { public static type(): MessageType { return MessageType.RevokeConnection; diff --git a/apps/extension/src/background/approvals/service.ts b/apps/extension/src/background/approvals/service.ts index 4cfe3be7f..934273bff 100644 --- a/apps/extension/src/background/approvals/service.ts +++ b/apps/extension/src/background/approvals/service.ts @@ -282,28 +282,62 @@ export class ApprovalsService { } } - async approveDisconnect(interfaceOrigin: string): Promise { + async approveDisconnect( + interfaceTabId: number, + interfaceOrigin: string + ): Promise { const baseUrl = `${browser.runtime.getURL( "approvals.html" )}#${TopLevelRoute.ApproveDisconnect}`; const url = paramsToUrl(baseUrl, { + interfaceTabId: interfaceTabId.toString(), interfaceOrigin, }); - const popupTabId = await this.getPopupTabId(url); + const isConnected = await this.isConnectionApproved(interfaceOrigin); - if (!popupTabId) { - throw new Error("no popup tab ID"); + if (isConnected) { + const popupTabId = await this.getPopupTabId(url); + + if (!popupTabId) { + throw new Error("no popup tab ID"); + } + + if (popupTabId in this.resolverMap) { + throw new Error(`tab ID ${popupTabId} already exists in promise map`); + } + + return new Promise((resolve, reject) => { + this.resolverMap[popupTabId] = { resolve, reject }; + }); } - if (popupTabId in this.resolverMap) { - throw new Error(`tab ID ${popupTabId} already exists in promise map`); + // A resolved promise is implicitly returned here if the origin had + // previously been disconnected. + } + + async approveDisconnectionResponse( + interfaceTabId: number, + interfaceOrigin: string, + revokeConnection: boolean, + popupTabId: number + ): Promise { + const resolvers = this.resolverMap[popupTabId]; + if (!resolvers) { + throw new Error(`no resolvers found for tab ID ${interfaceTabId}`); } - return new Promise((resolve, reject) => { - this.resolverMap[popupTabId] = { resolve, reject }; - }); + if (revokeConnection) { + try { + await this.revokeConnection(interfaceOrigin); + } catch (e) { + resolvers.reject(e); + } + resolvers.resolve(); + } else { + resolvers.reject(); + } } async revokeConnection(originToRevoke: string): Promise { From 5a86087b24b5615c44f50336015f3badad9f7d3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Harrison=20Mendon=C3=A7a?= Date: Thu, 12 Sep 2024 12:29:27 +0200 Subject: [PATCH 05/14] feat: create common function openPopup --- .../src/background/approvals/service.ts | 79 ++++++------------- 1 file changed, 25 insertions(+), 54 deletions(-) diff --git a/apps/extension/src/background/approvals/service.ts b/apps/extension/src/background/approvals/service.ts index 934273bff..a41885403 100644 --- a/apps/extension/src/background/approvals/service.ts +++ b/apps/extension/src/background/approvals/service.ts @@ -61,19 +61,7 @@ export class ApprovalsService { "approvals.html" )}#/approve-sign-tx/${msgId}/${details.type}/${signer}`; - const popupTabId = await this.getPopupTabId(url); - - if (!popupTabId) { - throw new Error("no popup tab ID"); - } - - if (popupTabId in this.resolverMap) { - throw new Error(`tab ID ${popupTabId} already exists in promise map`); - } - - return await new Promise((resolve, reject) => { - this.resolverMap[popupTabId] = { resolve, reject }; - }); + return this.openPopup(url); } async approveSignArbitrary( @@ -90,19 +78,7 @@ export class ApprovalsService { const url = paramsToUrl(baseUrl, { msgId, }); - const popupTabId = await this.getPopupTabId(url); - - if (!popupTabId) { - throw new Error("no popup tab ID"); - } - - if (popupTabId in this.resolverMap) { - throw new Error(`tab ID ${popupTabId} already exists in promise map`); - } - - return await new Promise((resolve, reject) => { - this.resolverMap[popupTabId] = { resolve, reject }; - }); + return this.openPopup(url); } async submitSignTx( @@ -240,19 +216,7 @@ export class ApprovalsService { const alreadyApproved = await this.isConnectionApproved(interfaceOrigin); if (!alreadyApproved) { - const popupTabId = await this.getPopupTabId(url); - - if (!popupTabId) { - throw new Error("no popup tab ID"); - } - - if (popupTabId in this.resolverMap) { - throw new Error(`tab ID ${popupTabId} already exists in promise map`); - } - - return new Promise((resolve, reject) => { - this.resolverMap[popupTabId] = { resolve, reject }; - }); + return this.openPopup(url); } // A resolved promise is implicitly returned here if the origin had @@ -298,19 +262,7 @@ export class ApprovalsService { const isConnected = await this.isConnectionApproved(interfaceOrigin); if (isConnected) { - const popupTabId = await this.getPopupTabId(url); - - if (!popupTabId) { - throw new Error("no popup tab ID"); - } - - if (popupTabId in this.resolverMap) { - throw new Error(`tab ID ${popupTabId} already exists in promise map`); - } - - return new Promise((resolve, reject) => { - this.resolverMap[popupTabId] = { resolve, reject }; - }); + return this.openPopup(url); } // A resolved promise is implicitly returned here if the origin had @@ -397,11 +349,30 @@ export class ApprovalsService { }); }; - private getPopupTabId = async (url: string): Promise => { + private openPopup = async (url: string): Promise => { const window = await this._launchApprovalWindow(url); const firstTab = window.tabs?.[0]; const popupTabId = firstTab?.id; - return popupTabId; + if (!popupTabId) { + throw new Error("no popup tab ID"); + } + + if (popupTabId in this.resolverMap) { + throw new Error(`tab ID ${popupTabId} already exists in promise map`); + } + + return await new Promise((resolve, reject) => { + this.resolverMap[popupTabId] = { + resolve: (args: T) => { + delete this.resolverMap[popupTabId]; + return resolve(args); + }, + reject: (args: unknown) => { + delete this.resolverMap[popupTabId]; + return reject(args); + }, + }; + }); }; } From 6aee9ef74e530afc8217a097fd4fe72761beeb36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Harrison=20Mendon=C3=A7a?= Date: Thu, 12 Sep 2024 12:57:12 +0200 Subject: [PATCH 06/14] feat: create common function getResolver --- .../src/Approvals/ApproveConnection.tsx | 9 +- .../src/Approvals/ApproveDisconnect.tsx | 9 +- .../src/background/approvals/handler.test.ts | 1 - .../src/background/approvals/handler.ts | 22 +++-- .../src/background/approvals/messages.test.ts | 16 ++-- .../src/background/approvals/messages.ts | 14 +--- .../src/background/approvals/service.test.ts | 36 ++------ .../src/background/approvals/service.ts | 83 +++++++------------ 8 files changed, 59 insertions(+), 131 deletions(-) diff --git a/apps/extension/src/Approvals/ApproveConnection.tsx b/apps/extension/src/Approvals/ApproveConnection.tsx index 84ef3f096..9d396f60f 100644 --- a/apps/extension/src/Approvals/ApproveConnection.tsx +++ b/apps/extension/src/Approvals/ApproveConnection.tsx @@ -10,17 +10,12 @@ export const ApproveConnection: React.FC = () => { const requester = useRequester(); const params = useQuery(); const interfaceOrigin = params.get("interfaceOrigin"); - const interfaceTabId = params.get("interfaceTabId"); const handleResponse = async (allowConnection: boolean): Promise => { - if (interfaceTabId && interfaceOrigin) { + if (interfaceOrigin) { await requester.sendMessage( Ports.Background, - new ConnectInterfaceResponseMsg( - parseInt(interfaceTabId), - interfaceOrigin, - allowConnection - ) + new ConnectInterfaceResponseMsg(interfaceOrigin, allowConnection) ); await closeCurrentTab(); } diff --git a/apps/extension/src/Approvals/ApproveDisconnect.tsx b/apps/extension/src/Approvals/ApproveDisconnect.tsx index 01d6bcc9b..7f05a6506 100644 --- a/apps/extension/src/Approvals/ApproveDisconnect.tsx +++ b/apps/extension/src/Approvals/ApproveDisconnect.tsx @@ -10,17 +10,12 @@ export const ApproveDisconnect: React.FC = () => { const requester = useRequester(); const params = useQuery(); const interfaceOrigin = params.get("interfaceOrigin"); - const interfaceTabId = params.get("interfaceTabId"); const handleResponse = async (revokeConnection: boolean): Promise => { - if (interfaceTabId && interfaceOrigin) { + if (interfaceOrigin) { await requester.sendMessage( Ports.Background, - new DisconnectInterfaceResponseMsg( - parseInt(interfaceTabId), - interfaceOrigin, - revokeConnection - ) + new DisconnectInterfaceResponseMsg(interfaceOrigin, revokeConnection) ); await closeCurrentTab(); } diff --git a/apps/extension/src/background/approvals/handler.test.ts b/apps/extension/src/background/approvals/handler.test.ts index 408b65297..0f0acf72c 100644 --- a/apps/extension/src/background/approvals/handler.test.ts +++ b/apps/extension/src/background/approvals/handler.test.ts @@ -80,7 +80,6 @@ describe("approvals handler", () => { expect(service.approveConnection).toBeCalled(); const connectInterfaceResponseMsg = new ConnectInterfaceResponseMsg( - 0, "", true ); diff --git a/apps/extension/src/background/approvals/handler.ts b/apps/extension/src/background/approvals/handler.ts index 07bba7bfe..8a37c19e1 100644 --- a/apps/extension/src/background/approvals/handler.ts +++ b/apps/extension/src/background/approvals/handler.ts @@ -113,8 +113,8 @@ const handleIsConnectionApprovedMsg: ( const handleApproveConnectInterfaceMsg: ( service: ApprovalsService ) => InternalHandler = (service) => { - return async ({ senderTabId: interfaceTabId }, { origin }) => { - return await service.approveConnection(interfaceTabId, origin); + return async (_, { origin }) => { + return await service.approveConnection(origin); }; }; @@ -123,13 +123,12 @@ const handleConnectInterfaceResponseMsg: ( ) => InternalHandler = (service) => { return async ( { senderTabId: popupTabId }, - { interfaceTabId, interfaceOrigin, allowConnection } + { interfaceOrigin, allowConnection } ) => { return await service.approveConnectionResponse( - interfaceTabId, + popupTabId, interfaceOrigin, - allowConnection, - popupTabId + allowConnection ); }; }; @@ -137,8 +136,8 @@ const handleConnectInterfaceResponseMsg: ( const handleApproveDisconnectInterfaceMsg: ( service: ApprovalsService ) => InternalHandler = (service) => { - return async ({ senderTabId: interfaceTabId }, { origin }) => { - return await service.approveDisconnect(interfaceTabId, origin); + return async (_, { origin }) => { + return await service.approveDisconnect(origin); }; }; @@ -147,13 +146,12 @@ const handleDisconnectInterfaceResponseMsg: ( ) => InternalHandler = (service) => { return async ( { senderTabId: popupTabId }, - { interfaceTabId, interfaceOrigin, revokeConnection } + { interfaceOrigin, revokeConnection } ) => { return await service.approveDisconnectionResponse( - interfaceTabId, + popupTabId, interfaceOrigin, - revokeConnection, - popupTabId + revokeConnection ); }; }; diff --git a/apps/extension/src/background/approvals/messages.test.ts b/apps/extension/src/background/approvals/messages.test.ts index ab6cbe770..bf35570f1 100644 --- a/apps/extension/src/background/approvals/messages.test.ts +++ b/apps/extension/src/background/approvals/messages.test.ts @@ -63,7 +63,7 @@ describe("approvals messages", () => { }); test("valid ConnectInterfaceResponseMsg", () => { - const msg = new ConnectInterfaceResponseMsg(0, "interface", true); + const msg = new ConnectInterfaceResponseMsg("interface", true); expect(msg.type()).toBe(MessageType.ConnectInterfaceResponse); expect(msg.route()).toBe(ROUTE); @@ -71,21 +71,15 @@ describe("approvals messages", () => { }); test("invalid ConnectInterfaceResponseMsg", () => { - const msg = new ConnectInterfaceResponseMsg(0, "interface", true); - - (msg as any).interfaceTabId = undefined; + const msg = new ConnectInterfaceResponseMsg("interface", true); + (msg as any).interfaceOrigin = undefined; expect(() => msg.validate()).toThrow(); - const msg2 = new ConnectInterfaceResponseMsg(0, "interface", true); - (msg2 as any).interfaceOrigin = undefined; + const msg2 = new ConnectInterfaceResponseMsg("interface", true); + (msg2 as any).allowConnection = undefined; expect(() => msg2.validate()).toThrow(); - - const msg3 = new ConnectInterfaceResponseMsg(0, "interface", true); - (msg3 as any).allowConnection = undefined; - - expect(() => msg3.validate()).toThrow(); }); test("valid RevokeConnectionMsg", () => { diff --git a/apps/extension/src/background/approvals/messages.ts b/apps/extension/src/background/approvals/messages.ts index 8062a588f..d4b8057e3 100644 --- a/apps/extension/src/background/approvals/messages.ts +++ b/apps/extension/src/background/approvals/messages.ts @@ -145,7 +145,6 @@ export class ConnectInterfaceResponseMsg extends Message { } constructor( - public readonly interfaceTabId: number, public readonly interfaceOrigin: string, public readonly allowConnection: boolean ) { @@ -153,11 +152,7 @@ export class ConnectInterfaceResponseMsg extends Message { } validate(): void { - validateProps(this, [ - "interfaceTabId", - "interfaceOrigin", - "allowConnection", - ]); + validateProps(this, ["interfaceOrigin", "allowConnection"]); } route(): string { @@ -175,7 +170,6 @@ export class DisconnectInterfaceResponseMsg extends Message { } constructor( - public readonly interfaceTabId: number, public readonly interfaceOrigin: string, public readonly revokeConnection: boolean ) { @@ -183,11 +177,7 @@ export class DisconnectInterfaceResponseMsg extends Message { } validate(): void { - validateProps(this, [ - "interfaceTabId", - "interfaceOrigin", - "revokeConnection", - ]); + validateProps(this, ["interfaceOrigin", "revokeConnection"]); } route(): string { diff --git a/apps/extension/src/background/approvals/service.test.ts b/apps/extension/src/background/approvals/service.test.ts index 783eb71cf..804c975b0 100644 --- a/apps/extension/src/background/approvals/service.test.ts +++ b/apps/extension/src/background/approvals/service.test.ts @@ -261,7 +261,6 @@ describe("approvals service", () => { describe("approveConnection", () => { it("should approve connection if it's not already approved", async () => { const url = "url-with-params"; - const interfaceTabId = 999; const interfaceOrigin = "origin"; const tabId = 1; @@ -272,10 +271,7 @@ describe("approvals service", () => { }); service["resolverMap"] = {}; - const promise = service.approveConnection( - interfaceTabId, - interfaceOrigin - ); + const promise = service.approveConnection(interfaceOrigin); await new Promise((r) => setTimeout(() => { r(); @@ -284,7 +280,6 @@ describe("approvals service", () => { service["resolverMap"][tabId]?.resolve(true); expect(paramsToUrl).toHaveBeenCalledWith("url#/approve-connection", { - interfaceTabId: interfaceTabId.toString(), interfaceOrigin, }); expect(service.isConnectionApproved).toHaveBeenCalledWith( @@ -296,19 +291,17 @@ describe("approvals service", () => { it("should not approve connection if it was already approved", async () => { const url = "url-with-params"; - const interfaceTabId = 999; const interfaceOrigin = "origin"; (paramsToUrl as any).mockImplementation(() => url); jest.spyOn(service, "isConnectionApproved").mockResolvedValue(true); await expect( - service.approveConnection(interfaceTabId, interfaceOrigin) + service.approveConnection(interfaceOrigin) ).resolves.toBeUndefined(); }); it("should throw an error when popupTabId is not found", async () => { const url = "url-with-params"; - const interfaceTabId = 999; const interfaceOrigin = "origin"; (paramsToUrl as any).mockImplementation(() => url); @@ -318,13 +311,12 @@ describe("approvals service", () => { }); await expect( - service.approveConnection(interfaceTabId, interfaceOrigin) + service.approveConnection(interfaceOrigin) ).rejects.toBeDefined(); }); it("should throw an error when popupTabId is found in resolverMap", async () => { const url = "url-with-params"; - const interfaceTabId = 999; const interfaceOrigin = "origin"; const approvedOrigins = ["other-origin"]; const tabId = 1; @@ -345,14 +337,13 @@ describe("approvals service", () => { }); await expect( - service.approveConnection(interfaceTabId, interfaceOrigin) + service.approveConnection(interfaceOrigin) ).rejects.toBeDefined(); }); }); describe("approveConnectionResponse", () => { it("should approve connection response", async () => { - const interfaceTabId = 999; const interfaceOrigin = "origin"; const popupTabId = 1; service["resolverMap"] = { @@ -364,10 +355,9 @@ describe("approvals service", () => { jest.spyOn(localStorage, "addApprovedOrigin").mockResolvedValue(); await service.approveConnectionResponse( - interfaceTabId, + popupTabId, interfaceOrigin, - true, - popupTabId + true ); expect(service["resolverMap"][popupTabId].resolve).toHaveBeenCalled(); @@ -377,22 +367,15 @@ describe("approvals service", () => { }); it("should throw an error if resolvers are not found", async () => { - const interfaceTabId = 999; const interfaceOrigin = "origin"; const popupTabId = 1; await expect( - service.approveConnectionResponse( - interfaceTabId, - interfaceOrigin, - true, - popupTabId - ) + service.approveConnectionResponse(popupTabId, interfaceOrigin, true) ).rejects.toBeDefined(); }); it("should reject the connection if allowConnection is set to false", async () => { - const interfaceTabId = 999; const interfaceOrigin = "origin"; const popupTabId = 1; service["resolverMap"] = { @@ -403,10 +386,9 @@ describe("approvals service", () => { }; await service.approveConnectionResponse( - interfaceTabId, + popupTabId, interfaceOrigin, - false, - popupTabId + false ); expect(service["resolverMap"][popupTabId].reject).toHaveBeenCalled(); diff --git a/apps/extension/src/background/approvals/service.ts b/apps/extension/src/background/approvals/service.ts index a41885403..441c31be4 100644 --- a/apps/extension/src/background/approvals/service.ts +++ b/apps/extension/src/background/approvals/service.ts @@ -17,14 +17,16 @@ import { LocalStorage } from "storage"; import { fromEncodedTx } from "utils"; import { EncodedTxData, PendingTx } from "./types"; +type Resolver = { + // TODO: there should be better typing for this + // eslint-disable-next-line + resolve: (result?: any) => void; + reject: (error?: unknown) => void; +}; + export class ApprovalsService { // holds promises which can be resolved with a message from a pop-up window - protected resolverMap: Record< - number, - // TODO: there should be better typing for this - // eslint-disable-next-line - { resolve: (result?: any) => void; reject: (error?: any) => void } - > = {}; + protected resolverMap: Record = {}; constructor( protected readonly txStore: KVStore, @@ -87,11 +89,7 @@ export class ApprovalsService { signer: string ): Promise { const pendingTx = (await this.txStore.get(msgId)) as PendingTx; - const resolvers = this.resolverMap[popupTabId]; - - if (!resolvers) { - throw new Error(`no resolvers found for tab ID ${popupTabId}`); - } + const resolvers = this.getResolver(popupTabId); if (!pendingTx) { throw new Error(`Signing data for ${msgId} not found!`); @@ -116,11 +114,7 @@ export class ApprovalsService { responseSign: ResponseSign[] ): Promise { const pendingTx = await this.txStore.get(msgId); - const resolvers = this.resolverMap[popupTabId]; - - if (!resolvers) { - throw new Error(`no resolvers found for tab ID ${popupTabId}`); - } + const resolvers = this.getResolver(popupTabId); if (!pendingTx || !pendingTx.txs) { throw new Error(`Transaction data for ${msgId} not found!`); @@ -150,11 +144,7 @@ export class ApprovalsService { signer: string ): Promise { const data = await this.dataStore.get(msgId); - const resolvers = this.resolverMap[popupTabId]; - - if (!resolvers) { - throw new Error(`no resolvers found for tab ID ${popupTabId}`); - } + const resolvers = this.getResolver(popupTabId); if (!data) { throw new Error(`Signing data for ${msgId} not found!`); @@ -172,11 +162,7 @@ export class ApprovalsService { } async rejectSignArbitrary(popupTabId: number, msgId: string): Promise { - const resolvers = this.resolverMap[popupTabId]; - - if (!resolvers) { - throw new Error(`no resolvers found for tab ID ${popupTabId}`); - } + const resolvers = this.getResolver(popupTabId); await this._clearPendingSignature(msgId); resolvers.reject(new Error("Sign arbitrary rejected")); @@ -184,10 +170,7 @@ export class ApprovalsService { // Remove pending transaction from storage async rejectSignTx(popupTabId: number, msgId: string): Promise { - const resolvers = this.resolverMap[popupTabId]; - if (!resolvers) { - throw new Error(`no resolvers found for tab ID ${popupTabId}`); - } + const resolvers = this.getResolver(popupTabId); await this._clearPendingTx(msgId); resolvers.reject(new Error("Sign Tx rejected")); @@ -200,16 +183,12 @@ export class ApprovalsService { return approvedOrigins.includes(interfaceOrigin); } - async approveConnection( - interfaceTabId: number, - interfaceOrigin: string - ): Promise { + async approveConnection(interfaceOrigin: string): Promise { const baseUrl = `${browser.runtime.getURL( "approvals.html" )}#/approve-connection`; const url = paramsToUrl(baseUrl, { - interfaceTabId: interfaceTabId.toString(), interfaceOrigin, }); @@ -224,15 +203,11 @@ export class ApprovalsService { } async approveConnectionResponse( - interfaceTabId: number, + popupTabId: number, interfaceOrigin: string, - allowConnection: boolean, - popupTabId: number + allowConnection: boolean ): Promise { - const resolvers = this.resolverMap[popupTabId]; - if (!resolvers) { - throw new Error(`no resolvers found for tab ID ${interfaceTabId}`); - } + const resolvers = this.getResolver(popupTabId); if (allowConnection) { try { @@ -246,16 +221,12 @@ export class ApprovalsService { } } - async approveDisconnect( - interfaceTabId: number, - interfaceOrigin: string - ): Promise { + async approveDisconnect(interfaceOrigin: string): Promise { const baseUrl = `${browser.runtime.getURL( "approvals.html" )}#${TopLevelRoute.ApproveDisconnect}`; const url = paramsToUrl(baseUrl, { - interfaceTabId: interfaceTabId.toString(), interfaceOrigin, }); @@ -270,15 +241,11 @@ export class ApprovalsService { } async approveDisconnectionResponse( - interfaceTabId: number, + popupTabId: number, interfaceOrigin: string, - revokeConnection: boolean, - popupTabId: number + revokeConnection: boolean ): Promise { - const resolvers = this.resolverMap[popupTabId]; - if (!resolvers) { - throw new Error(`no resolvers found for tab ID ${interfaceTabId}`); - } + const resolvers = this.getResolver(popupTabId); if (revokeConnection) { try { @@ -375,4 +342,12 @@ export class ApprovalsService { }; }); }; + + private getResolver = (popupTabId: number): Resolver => { + const resolvers = this.resolverMap[popupTabId]; + if (!resolvers) { + throw new Error(`no resolvers found for tab ID ${popupTabId}`); + } + return resolvers; + }; } From a331d4bc1dc068098598613adfaa60e323e56c89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Harrison=20Mendon=C3=A7a?= Date: Thu, 12 Sep 2024 13:26:39 +0200 Subject: [PATCH 07/14] feat: remove resolvers from closed tabs --- .../src/background/approvals/service.ts | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/apps/extension/src/background/approvals/service.ts b/apps/extension/src/background/approvals/service.ts index 441c31be4..c04d00d11 100644 --- a/apps/extension/src/background/approvals/service.ts +++ b/apps/extension/src/background/approvals/service.ts @@ -37,7 +37,11 @@ export class ApprovalsService { protected readonly vaultService: VaultService, protected readonly chainService: ChainsService, protected readonly broadcaster: ExtensionBroadcaster - ) {} + ) { + browser.tabs.onRemoved.addListener((tabId) => { + delete this.resolverMap[tabId]; + }); + } async approveSignTx( signer: string, @@ -63,7 +67,7 @@ export class ApprovalsService { "approvals.html" )}#/approve-sign-tx/${msgId}/${details.type}/${signer}`; - return this.openPopup(url); + return this.launchApprovalWindow(url); } async approveSignArbitrary( @@ -80,7 +84,7 @@ export class ApprovalsService { const url = paramsToUrl(baseUrl, { msgId, }); - return this.openPopup(url); + return this.launchApprovalWindow(url); } async submitSignTx( @@ -195,7 +199,7 @@ export class ApprovalsService { const alreadyApproved = await this.isConnectionApproved(interfaceOrigin); if (!alreadyApproved) { - return this.openPopup(url); + return this.launchApprovalWindow(url); } // A resolved promise is implicitly returned here if the origin had @@ -233,7 +237,7 @@ export class ApprovalsService { const isConnected = await this.isConnectionApproved(interfaceOrigin); if (isConnected) { - return this.openPopup(url); + return this.launchApprovalWindow(url); } // A resolved promise is implicitly returned here if the origin had @@ -307,7 +311,7 @@ export class ApprovalsService { return await this.dataStore.set(msgId, null); } - private _launchApprovalWindow = (url: string): Promise => { + private openPopup = (url: string): Promise => { return browser.windows.create({ url, width: 396, @@ -316,8 +320,7 @@ export class ApprovalsService { }); }; - private openPopup = async (url: string): Promise => { - const window = await this._launchApprovalWindow(url); + private getPopupTabId = (window: browser.Windows.Window): number => { const firstTab = window.tabs?.[0]; const popupTabId = firstTab?.id; @@ -329,6 +332,13 @@ export class ApprovalsService { throw new Error(`tab ID ${popupTabId} already exists in promise map`); } + return popupTabId; + }; + + private launchApprovalWindow = async (url: string): Promise => { + const window = await this.openPopup(url); + const popupTabId = this.getPopupTabId(window); + return await new Promise((resolve, reject) => { this.resolverMap[popupTabId] = { resolve: (args: T) => { From 0c8c2e29c5d21bc0ea31ed52bf65ccc5983b7f54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Harrison=20Mendon=C3=A7a?= Date: Thu, 12 Sep 2024 17:45:40 +0200 Subject: [PATCH 08/14] feat: improve tests --- apps/extension/src/Approvals/Approvals.tsx | 6 +- ...isconnect.tsx => ApproveDisconnection.tsx} | 2 +- apps/extension/src/Approvals/types.ts | 2 +- .../src/background/approvals/handler.test.ts | 8 + .../src/background/approvals/handler.ts | 2 +- .../src/background/approvals/service.test.ts | 171 ++++++++++++++++-- .../src/background/approvals/service.ts | 6 +- 7 files changed, 168 insertions(+), 29 deletions(-) rename apps/extension/src/Approvals/{ApproveDisconnect.tsx => ApproveDisconnection.tsx} (96%) diff --git a/apps/extension/src/Approvals/Approvals.tsx b/apps/extension/src/Approvals/Approvals.tsx index c418ebabb..9a839dd2b 100644 --- a/apps/extension/src/Approvals/Approvals.tsx +++ b/apps/extension/src/Approvals/Approvals.tsx @@ -7,7 +7,7 @@ import { AccountType, TxDetails } from "@namada/types"; import { AppHeader } from "App/Common/AppHeader"; import { TopLevelRoute } from "Approvals/types"; import { ApproveConnection } from "./ApproveConnection"; -import { ApproveDisconnect } from "./ApproveDisconnect"; +import { ApproveDisconnection } from "./ApproveDisconnection"; import { ApproveSignArbitrary } from "./ApproveSignArbitrary"; import { ApproveSignTx } from "./ApproveSignTx"; import { ConfirmSignature } from "./ConfirmSignArbitrary"; @@ -67,8 +67,8 @@ export const Approvals: React.FC = () => { element={} /> } + path={TopLevelRoute.ApproveDisconnection} + element={} /> { +export const ApproveDisconnection: React.FC = () => { const requester = useRequester(); const params = useQuery(); const interfaceOrigin = params.get("interfaceOrigin"); diff --git a/apps/extension/src/Approvals/types.ts b/apps/extension/src/Approvals/types.ts index b12efb18b..5b4380cfb 100644 --- a/apps/extension/src/Approvals/types.ts +++ b/apps/extension/src/Approvals/types.ts @@ -5,7 +5,7 @@ export enum TopLevelRoute { // Connection approval ApproveConnection = "/approve-connection", - ApproveDisconnect = "/approve-disconnect", + ApproveDisconnection = "/approve-disconnection", // Sign Tx approval ApproveSignTx = "/approve-sign-tx", diff --git a/apps/extension/src/background/approvals/handler.test.ts b/apps/extension/src/background/approvals/handler.test.ts index 0f0acf72c..1b4903efa 100644 --- a/apps/extension/src/background/approvals/handler.test.ts +++ b/apps/extension/src/background/approvals/handler.test.ts @@ -12,6 +12,7 @@ import { Message } from "router"; import { getHandler } from "./handler"; import { ConnectInterfaceResponseMsg, + DisconnectInterfaceResponseMsg, RejectSignArbitraryMsg, RejectSignTxMsg, RevokeConnectionMsg, @@ -86,6 +87,13 @@ describe("approvals handler", () => { handler(env, connectInterfaceResponseMsg); expect(service.approveConnectionResponse).toBeCalled(); + const disconnectInterfaceResponseMsg = new DisconnectInterfaceResponseMsg( + "", + true + ); + handler(env, disconnectInterfaceResponseMsg); + expect(service.approveDisconnectionResponse).toBeCalled(); + const revokeConnectionMsg = new RevokeConnectionMsg(""); handler(env, revokeConnectionMsg); expect(service.revokeConnection).toBeCalled(); diff --git a/apps/extension/src/background/approvals/handler.ts b/apps/extension/src/background/approvals/handler.ts index 8a37c19e1..f81f8b27b 100644 --- a/apps/extension/src/background/approvals/handler.ts +++ b/apps/extension/src/background/approvals/handler.ts @@ -137,7 +137,7 @@ const handleApproveDisconnectInterfaceMsg: ( service: ApprovalsService ) => InternalHandler = (service) => { return async (_, { origin }) => { - return await service.approveDisconnect(origin); + return await service.approveDisconnection(origin); }; }; diff --git a/apps/extension/src/background/approvals/service.test.ts b/apps/extension/src/background/approvals/service.test.ts index 804c975b0..299083bc0 100644 --- a/apps/extension/src/background/approvals/service.test.ts +++ b/apps/extension/src/background/approvals/service.test.ts @@ -21,6 +21,11 @@ jest.mock("webextension-polyfill", () => ({ windows: { create: jest.fn().mockResolvedValue({ tabs: [{ id: 1 }] }), }, + tabs: { + onRemoved: { + addListener: jest.fn(), + }, + }, })); jest.mock("@namada/utils", () => { @@ -266,7 +271,7 @@ describe("approvals service", () => { (paramsToUrl as any).mockImplementation(() => url); jest.spyOn(service, "isConnectionApproved").mockResolvedValue(false); - jest.spyOn(service as any, "_launchApprovalWindow").mockResolvedValue({ + jest.spyOn(service as any, "openPopup").mockResolvedValue({ tabs: [{ id: tabId }], }); service["resolverMap"] = {}; @@ -285,7 +290,7 @@ describe("approvals service", () => { expect(service.isConnectionApproved).toHaveBeenCalledWith( interfaceOrigin ); - expect(service["_launchApprovalWindow"]).toHaveBeenCalledWith(url); + expect(service["openPopup"]).toHaveBeenCalledWith(url); await expect(promise).resolves.toBeDefined(); }); @@ -306,7 +311,7 @@ describe("approvals service", () => { (paramsToUrl as any).mockImplementation(() => url); jest.spyOn(service, "isConnectionApproved").mockResolvedValue(false); - jest.spyOn(service as any, "_launchApprovalWindow").mockResolvedValue({ + jest.spyOn(service as any, "openPopup").mockResolvedValue({ tabs: [], }); @@ -332,7 +337,7 @@ describe("approvals service", () => { jest .spyOn(localStorage, "getApprovedOrigins") .mockResolvedValue(approvedOrigins); - jest.spyOn(service as any, "_launchApprovalWindow").mockResolvedValue({ + jest.spyOn(service as any, "openPopup").mockResolvedValue({ tabs: [{ id: tabId }], }); @@ -395,6 +400,91 @@ describe("approvals service", () => { }); }); + describe("approveDisconnection", () => { + it("should approve disconnection if there is a connection already approved", async () => { + const url = "url-with-params"; + const interfaceOrigin = "origin"; + const tabId = 1; + + (paramsToUrl as any).mockImplementation(() => url); + jest.spyOn(service, "isConnectionApproved").mockResolvedValue(true); + jest.spyOn(service as any, "openPopup").mockResolvedValue({ + tabs: [{ id: tabId }], + }); + service["resolverMap"] = {}; + + const promise = service.approveDisconnection(interfaceOrigin); + await new Promise((r) => + setTimeout(() => { + r(); + }) + ); + service["resolverMap"][tabId]?.resolve(true); + + expect(paramsToUrl).toHaveBeenCalledWith("url#/approve-disconnection", { + interfaceOrigin, + }); + expect(service.isConnectionApproved).toHaveBeenCalledWith( + interfaceOrigin + ); + expect(service["openPopup"]).toHaveBeenCalledWith(url); + await expect(promise).resolves.toBeDefined(); + }); + + it("should not approve disconnection if it is NOT already approved", async () => { + const url = "url-with-params"; + const interfaceOrigin = "origin"; + (paramsToUrl as any).mockImplementation(() => url); + jest.spyOn(service, "isConnectionApproved").mockResolvedValue(false); + + await expect( + service.approveDisconnection(interfaceOrigin) + ).resolves.toBeUndefined(); + }); + }); + + describe("approveDisconnectionResponse", () => { + it("should approve disconnection response", async () => { + const interfaceOrigin = "origin"; + const popupTabId = 1; + service["resolverMap"] = { + [popupTabId]: { + resolve: jest.fn(), + reject: jest.fn(), + }, + }; + jest.spyOn(service, "revokeConnection").mockResolvedValue(); + + await service.approveDisconnectionResponse( + popupTabId, + interfaceOrigin, + true + ); + + expect(service["resolverMap"][popupTabId].resolve).toHaveBeenCalled(); + expect(service.revokeConnection).toHaveBeenCalledWith(interfaceOrigin); + }); + + it("should reject the connection if revokeConnection is set to false", async () => { + const interfaceOrigin = "origin"; + const popupTabId = 1; + service["resolverMap"] = { + [popupTabId]: { + resolve: jest.fn(), + reject: jest.fn(), + }, + }; + + await service.approveConnectionResponse( + popupTabId, + interfaceOrigin, + false + ); + + expect(service["resolverMap"][popupTabId].reject).toHaveBeenCalled(); + }); + }); + describe("revokeConnection", () => { it("should reject connection response", async () => { const originToRevoke = "origin"; @@ -408,31 +498,72 @@ describe("approvals service", () => { }); }); + describe("openPopup", () => { + it("should create and return a new window", async () => { + const url = "url"; + const window = { tabs: [{ id: 1 }] }; + (webextensionPolyfill.windows.create as any).mockResolvedValue(window); + + await expect((service as any).openPopup(url)).resolves.toBe(window); + expect(webextensionPolyfill.windows.create).toHaveBeenCalledWith( + expect.objectContaining({ url }) + ); + }); + }); + describe("getPopupTabId", () => { it("should return tab id", async () => { - (webextensionPolyfill.windows.create as any).mockResolvedValue({ - tabs: [{ id: 1 }], - }); + const window = { tabs: [{ id: 1 }] }; - await expect((service as any).getPopupTabId("url")).resolves.toBe(1); + expect((service as any).getPopupTabId(window)).toBe(1); }); - it("should return undefined if tabs are undefined", async () => { - (webextensionPolyfill.windows.create as any).mockResolvedValue({}); + it("should throw an error if tabs are undefined", async () => { + const window = { tabs: undefined }; + expect(() => (service as any).getPopupTabId(window)).toThrow(); + }); - await expect( - (service as any).getPopupTabId("url") - ).resolves.toBeUndefined(); + it("should throw an error if tabs are empty", async () => { + const window = { tabs: [] }; + + expect(() => (service as any).getPopupTabId(window)).toThrow(); }); + }); - it("should return undefined if tabs are empty", async () => { - (webextensionPolyfill.windows.create as any).mockResolvedValue({ - tabs: [], - }); + describe("launchApprovalWindow", () => { + it("should create a window and save the resolver on the resolverMap", async () => { + const url = "url"; + const popupTabId = 1; + const window = { tabs: [{ id: popupTabId }] }; + jest + .spyOn(service, "openPopup") + .mockImplementationOnce(() => window); + (service as any).launchApprovalWindow(url); - await expect( - (service as any).getPopupTabId("url") - ).resolves.toBeUndefined(); + expect(await (service as any).openPopup).toHaveBeenCalledWith(url); + expect((service as any).resolverMap[popupTabId]).toBeDefined(); + }); + }); + + describe("getResolver", () => { + it("should get the related tab id resolver from resolverMap", async () => { + const popupTabId = 1; + const resolver = new Promise(() => {}); + (service as any).resolverMap = { + [popupTabId]: resolver, + }; + + expect((service as any).getResolver(popupTabId)).toBe(resolver); + }); + + it("should throw an error if there is no resolver for the tab id", async () => { + const myPopupTabId = 1; + const myResolver = new Promise(() => {}); + (service as any).resolverMap = { + [myPopupTabId]: myResolver, + }; + + expect(() => (service as any).getResolver("999")).toThrow(); }); }); diff --git a/apps/extension/src/background/approvals/service.ts b/apps/extension/src/background/approvals/service.ts index c04d00d11..8766abe13 100644 --- a/apps/extension/src/background/approvals/service.ts +++ b/apps/extension/src/background/approvals/service.ts @@ -225,10 +225,10 @@ export class ApprovalsService { } } - async approveDisconnect(interfaceOrigin: string): Promise { + async approveDisconnection(interfaceOrigin: string): Promise { const baseUrl = `${browser.runtime.getURL( "approvals.html" - )}#${TopLevelRoute.ApproveDisconnect}`; + )}#${TopLevelRoute.ApproveDisconnection}`; const url = paramsToUrl(baseUrl, { interfaceOrigin, @@ -339,7 +339,7 @@ export class ApprovalsService { const window = await this.openPopup(url); const popupTabId = this.getPopupTabId(window); - return await new Promise((resolve, reject) => { + return new Promise((resolve, reject) => { this.resolverMap[popupTabId] = { resolve: (args: T) => { delete this.resolverMap[popupTabId]; From 6951451037cd856f56e041b5e268beee5ea972ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Harrison=20Mendon=C3=A7a?= Date: Thu, 12 Sep 2024 18:23:05 +0200 Subject: [PATCH 09/14] feat: use route to launch popup --- .../src/background/approvals/service.test.ts | 88 ++++--------------- .../src/background/approvals/service.ts | 51 +++++------ 2 files changed, 39 insertions(+), 100 deletions(-) diff --git a/apps/extension/src/background/approvals/service.test.ts b/apps/extension/src/background/approvals/service.test.ts index 299083bc0..4848964b5 100644 --- a/apps/extension/src/background/approvals/service.test.ts +++ b/apps/extension/src/background/approvals/service.test.ts @@ -1,6 +1,5 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import { WrapperTxMsgValue } from "@namada/types"; -import { paramsToUrl } from "@namada/utils"; import { ChainsService } from "background/chains"; import { KeyRingService } from "background/keyring"; import { SdkService } from "background/sdk"; @@ -31,7 +30,6 @@ jest.mock("webextension-polyfill", () => ({ jest.mock("@namada/utils", () => { return { ...jest.requireActual("@namada/utils"), - paramsToUrl: jest.fn(), __esModule: true, }; }); @@ -265,15 +263,11 @@ describe("approvals service", () => { describe("approveConnection", () => { it("should approve connection if it's not already approved", async () => { - const url = "url-with-params"; const interfaceOrigin = "origin"; const tabId = 1; - (paramsToUrl as any).mockImplementation(() => url); jest.spyOn(service, "isConnectionApproved").mockResolvedValue(false); - jest.spyOn(service as any, "openPopup").mockResolvedValue({ - tabs: [{ id: tabId }], - }); + jest.spyOn(service as any, "launchApprovalWindow"); service["resolverMap"] = {}; const promise = service.approveConnection(interfaceOrigin); @@ -284,67 +278,24 @@ describe("approvals service", () => { ); service["resolverMap"][tabId]?.resolve(true); - expect(paramsToUrl).toHaveBeenCalledWith("url#/approve-connection", { - interfaceOrigin, - }); + expect((service as any).launchApprovalWindow).toHaveBeenCalledWith( + "/approve-connection", + { interfaceOrigin } + ); expect(service.isConnectionApproved).toHaveBeenCalledWith( interfaceOrigin ); - expect(service["openPopup"]).toHaveBeenCalledWith(url); await expect(promise).resolves.toBeDefined(); }); it("should not approve connection if it was already approved", async () => { - const url = "url-with-params"; const interfaceOrigin = "origin"; - (paramsToUrl as any).mockImplementation(() => url); jest.spyOn(service, "isConnectionApproved").mockResolvedValue(true); await expect( service.approveConnection(interfaceOrigin) ).resolves.toBeUndefined(); }); - - it("should throw an error when popupTabId is not found", async () => { - const url = "url-with-params"; - const interfaceOrigin = "origin"; - - (paramsToUrl as any).mockImplementation(() => url); - jest.spyOn(service, "isConnectionApproved").mockResolvedValue(false); - jest.spyOn(service as any, "openPopup").mockResolvedValue({ - tabs: [], - }); - - await expect( - service.approveConnection(interfaceOrigin) - ).rejects.toBeDefined(); - }); - - it("should throw an error when popupTabId is found in resolverMap", async () => { - const url = "url-with-params"; - const interfaceOrigin = "origin"; - const approvedOrigins = ["other-origin"]; - const tabId = 1; - - service["resolverMap"] = { - [tabId]: { - resolve: jest.fn(), - reject: jest.fn(), - }, - }; - - (paramsToUrl as any).mockImplementation(() => url); - jest - .spyOn(localStorage, "getApprovedOrigins") - .mockResolvedValue(approvedOrigins); - jest.spyOn(service as any, "openPopup").mockResolvedValue({ - tabs: [{ id: tabId }], - }); - - await expect( - service.approveConnection(interfaceOrigin) - ).rejects.toBeDefined(); - }); }); describe("approveConnectionResponse", () => { @@ -402,15 +353,11 @@ describe("approvals service", () => { describe("approveDisconnection", () => { it("should approve disconnection if there is a connection already approved", async () => { - const url = "url-with-params"; const interfaceOrigin = "origin"; const tabId = 1; - (paramsToUrl as any).mockImplementation(() => url); jest.spyOn(service, "isConnectionApproved").mockResolvedValue(true); - jest.spyOn(service as any, "openPopup").mockResolvedValue({ - tabs: [{ id: tabId }], - }); + jest.spyOn(service as any, "launchApprovalWindow"); service["resolverMap"] = {}; const promise = service.approveDisconnection(interfaceOrigin); @@ -421,20 +368,18 @@ describe("approvals service", () => { ); service["resolverMap"][tabId]?.resolve(true); - expect(paramsToUrl).toHaveBeenCalledWith("url#/approve-disconnection", { - interfaceOrigin, - }); + expect((service as any).launchApprovalWindow).toHaveBeenCalledWith( + "/approve-disconnection", + { interfaceOrigin } + ); expect(service.isConnectionApproved).toHaveBeenCalledWith( interfaceOrigin ); - expect(service["openPopup"]).toHaveBeenCalledWith(url); await expect(promise).resolves.toBeDefined(); }); it("should not approve disconnection if it is NOT already approved", async () => { - const url = "url-with-params"; const interfaceOrigin = "origin"; - (paramsToUrl as any).mockImplementation(() => url); jest.spyOn(service, "isConnectionApproved").mockResolvedValue(false); await expect( @@ -531,16 +476,21 @@ describe("approvals service", () => { }); describe("launchApprovalWindow", () => { - it("should create a window and save the resolver on the resolverMap", async () => { - const url = "url"; + it("should create a window with the given route and params saving the resolver on the resolverMap", async () => { + const route = "route"; + const params = { foo: "bar" }; const popupTabId = 1; const window = { tabs: [{ id: popupTabId }] }; + jest .spyOn(service, "openPopup") .mockImplementationOnce(() => window); - (service as any).launchApprovalWindow(url); - expect(await (service as any).openPopup).toHaveBeenCalledWith(url); + (service as any).launchApprovalWindow(route, params); + + expect(await (service as any).openPopup).toHaveBeenCalledWith( + `url#${route}?foo=bar` + ); expect((service as any).resolverMap[popupTabId]).toBeDefined(); }); }); diff --git a/apps/extension/src/background/approvals/service.ts b/apps/extension/src/background/approvals/service.ts index 8766abe13..b349b8d1c 100644 --- a/apps/extension/src/background/approvals/service.ts +++ b/apps/extension/src/background/approvals/service.ts @@ -63,11 +63,9 @@ export class ApprovalsService { await this.txStore.set(msgId, pendingTx); - const url = `${browser.runtime.getURL( - "approvals.html" - )}#/approve-sign-tx/${msgId}/${details.type}/${signer}`; - - return this.launchApprovalWindow(url); + return this.launchApprovalWindow( + `${TopLevelRoute.ApproveSignTx}/${msgId}/${details.type}/${signer}` + ); } async approveSignArbitrary( @@ -77,14 +75,11 @@ export class ApprovalsService { const msgId = uuid(); await this.dataStore.set(msgId, data); - const baseUrl = `${browser.runtime.getURL( - "approvals.html" - )}#/approve-sign-arbitrary/${signer}`; - const url = paramsToUrl(baseUrl, { - msgId, - }); - return this.launchApprovalWindow(url); + return this.launchApprovalWindow( + `${TopLevelRoute.ApproveSignArbitrary}/${signer}`, + { msgId } + ); } async submitSignTx( @@ -188,18 +183,12 @@ export class ApprovalsService { } async approveConnection(interfaceOrigin: string): Promise { - const baseUrl = `${browser.runtime.getURL( - "approvals.html" - )}#/approve-connection`; - - const url = paramsToUrl(baseUrl, { - interfaceOrigin, - }); - const alreadyApproved = await this.isConnectionApproved(interfaceOrigin); if (!alreadyApproved) { - return this.launchApprovalWindow(url); + return this.launchApprovalWindow(TopLevelRoute.ApproveConnection, { + interfaceOrigin, + }); } // A resolved promise is implicitly returned here if the origin had @@ -226,18 +215,12 @@ export class ApprovalsService { } async approveDisconnection(interfaceOrigin: string): Promise { - const baseUrl = `${browser.runtime.getURL( - "approvals.html" - )}#${TopLevelRoute.ApproveDisconnection}`; - - const url = paramsToUrl(baseUrl, { - interfaceOrigin, - }); - const isConnected = await this.isConnectionApproved(interfaceOrigin); if (isConnected) { - return this.launchApprovalWindow(url); + return this.launchApprovalWindow(TopLevelRoute.ApproveDisconnection, { + interfaceOrigin, + }); } // A resolved promise is implicitly returned here if the origin had @@ -335,7 +318,13 @@ export class ApprovalsService { return popupTabId; }; - private launchApprovalWindow = async (url: string): Promise => { + private launchApprovalWindow = async ( + route: string, + params?: Record + ): Promise => { + const baseUrl = `${browser.runtime.getURL("approvals.html")}#${route}`; + const url = params ? paramsToUrl(baseUrl, params) : baseUrl; + const window = await this.openPopup(url); const popupTabId = this.getPopupTabId(window); From 5a9e1758a11f6f86da1dbab367e6a119daea9301 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Harrison=20Mendon=C3=A7a?= Date: Thu, 12 Sep 2024 18:25:14 +0200 Subject: [PATCH 10/14] feat: rename popup --- .../src/background/approvals/service.test.ts | 18 +++++++++--------- .../src/background/approvals/service.ts | 14 +++++++------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/apps/extension/src/background/approvals/service.test.ts b/apps/extension/src/background/approvals/service.test.ts index 4848964b5..4cdae5764 100644 --- a/apps/extension/src/background/approvals/service.test.ts +++ b/apps/extension/src/background/approvals/service.test.ts @@ -267,7 +267,7 @@ describe("approvals service", () => { const tabId = 1; jest.spyOn(service, "isConnectionApproved").mockResolvedValue(false); - jest.spyOn(service as any, "launchApprovalWindow"); + jest.spyOn(service as any, "launchApprovalPopup"); service["resolverMap"] = {}; const promise = service.approveConnection(interfaceOrigin); @@ -278,7 +278,7 @@ describe("approvals service", () => { ); service["resolverMap"][tabId]?.resolve(true); - expect((service as any).launchApprovalWindow).toHaveBeenCalledWith( + expect((service as any).launchApprovalPopup).toHaveBeenCalledWith( "/approve-connection", { interfaceOrigin } ); @@ -357,7 +357,7 @@ describe("approvals service", () => { const tabId = 1; jest.spyOn(service, "isConnectionApproved").mockResolvedValue(true); - jest.spyOn(service as any, "launchApprovalWindow"); + jest.spyOn(service as any, "launchApprovalPopup"); service["resolverMap"] = {}; const promise = service.approveDisconnection(interfaceOrigin); @@ -368,7 +368,7 @@ describe("approvals service", () => { ); service["resolverMap"][tabId]?.resolve(true); - expect((service as any).launchApprovalWindow).toHaveBeenCalledWith( + expect((service as any).launchApprovalPopup).toHaveBeenCalledWith( "/approve-disconnection", { interfaceOrigin } ); @@ -443,13 +443,13 @@ describe("approvals service", () => { }); }); - describe("openPopup", () => { + describe("createPopup", () => { it("should create and return a new window", async () => { const url = "url"; const window = { tabs: [{ id: 1 }] }; (webextensionPolyfill.windows.create as any).mockResolvedValue(window); - await expect((service as any).openPopup(url)).resolves.toBe(window); + await expect((service as any).createPopup(url)).resolves.toBe(window); expect(webextensionPolyfill.windows.create).toHaveBeenCalledWith( expect.objectContaining({ url }) ); @@ -475,7 +475,7 @@ describe("approvals service", () => { }); }); - describe("launchApprovalWindow", () => { + describe("launchApprovalPopup", () => { it("should create a window with the given route and params saving the resolver on the resolverMap", async () => { const route = "route"; const params = { foo: "bar" }; @@ -483,10 +483,10 @@ describe("approvals service", () => { const window = { tabs: [{ id: popupTabId }] }; jest - .spyOn(service, "openPopup") + .spyOn(service, "createPopup") .mockImplementationOnce(() => window); - (service as any).launchApprovalWindow(route, params); + (service as any).launchApprovalPopup(route, params); expect(await (service as any).openPopup).toHaveBeenCalledWith( `url#${route}?foo=bar` diff --git a/apps/extension/src/background/approvals/service.ts b/apps/extension/src/background/approvals/service.ts index b349b8d1c..ed616e494 100644 --- a/apps/extension/src/background/approvals/service.ts +++ b/apps/extension/src/background/approvals/service.ts @@ -63,7 +63,7 @@ export class ApprovalsService { await this.txStore.set(msgId, pendingTx); - return this.launchApprovalWindow( + return this.launchApprovalPopup( `${TopLevelRoute.ApproveSignTx}/${msgId}/${details.type}/${signer}` ); } @@ -76,7 +76,7 @@ export class ApprovalsService { await this.dataStore.set(msgId, data); - return this.launchApprovalWindow( + return this.launchApprovalPopup( `${TopLevelRoute.ApproveSignArbitrary}/${signer}`, { msgId } ); @@ -186,7 +186,7 @@ export class ApprovalsService { const alreadyApproved = await this.isConnectionApproved(interfaceOrigin); if (!alreadyApproved) { - return this.launchApprovalWindow(TopLevelRoute.ApproveConnection, { + return this.launchApprovalPopup(TopLevelRoute.ApproveConnection, { interfaceOrigin, }); } @@ -218,7 +218,7 @@ export class ApprovalsService { const isConnected = await this.isConnectionApproved(interfaceOrigin); if (isConnected) { - return this.launchApprovalWindow(TopLevelRoute.ApproveDisconnection, { + return this.launchApprovalPopup(TopLevelRoute.ApproveDisconnection, { interfaceOrigin, }); } @@ -294,7 +294,7 @@ export class ApprovalsService { return await this.dataStore.set(msgId, null); } - private openPopup = (url: string): Promise => { + private createPopup = (url: string): Promise => { return browser.windows.create({ url, width: 396, @@ -318,14 +318,14 @@ export class ApprovalsService { return popupTabId; }; - private launchApprovalWindow = async ( + private launchApprovalPopup = async ( route: string, params?: Record ): Promise => { const baseUrl = `${browser.runtime.getURL("approvals.html")}#${route}`; const url = params ? paramsToUrl(baseUrl, params) : baseUrl; - const window = await this.openPopup(url); + const window = await this.createPopup(url); const popupTabId = this.getPopupTabId(window); return new Promise((resolve, reject) => { From 66d5903bb512a9ab7b3f1cdfd3fe57fe635ac05a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Harrison=20Mendon=C3=A7a?= Date: Thu, 12 Sep 2024 18:27:22 +0200 Subject: [PATCH 11/14] feat: add more test --- .../src/background/approvals/service.test.ts | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/apps/extension/src/background/approvals/service.test.ts b/apps/extension/src/background/approvals/service.test.ts index 4cdae5764..e0283a5dc 100644 --- a/apps/extension/src/background/approvals/service.test.ts +++ b/apps/extension/src/background/approvals/service.test.ts @@ -473,6 +473,19 @@ describe("approvals service", () => { expect(() => (service as any).getPopupTabId(window)).toThrow(); }); + + it("should throw an error if the tab already exists on the resolverMap", async () => { + const popupTabId = 1; + const window = { tabs: [{ id: popupTabId }] }; + service["resolverMap"] = { + [popupTabId]: { + resolve: jest.fn(), + reject: jest.fn(), + }, + }; + + expect(() => (service as any).getPopupTabId(window)).toThrow(); + }); }); describe("launchApprovalPopup", () => { @@ -488,7 +501,7 @@ describe("approvals service", () => { (service as any).launchApprovalPopup(route, params); - expect(await (service as any).openPopup).toHaveBeenCalledWith( + expect(await (service as any).createPopup).toHaveBeenCalledWith( `url#${route}?foo=bar` ); expect((service as any).resolverMap[popupTabId]).toBeDefined(); From b3d34be3f5869fc24b240e5bf296f889b1887b4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Harrison=20Mendon=C3=A7a?= Date: Thu, 12 Sep 2024 18:51:41 +0200 Subject: [PATCH 12/14] feat: add removeResolver --- .../src/background/approvals/service.test.ts | 54 +++++++++++-------- .../src/background/approvals/service.ts | 10 ++-- 2 files changed, 39 insertions(+), 25 deletions(-) diff --git a/apps/extension/src/background/approvals/service.test.ts b/apps/extension/src/background/approvals/service.test.ts index e0283a5dc..fb40739f0 100644 --- a/apps/extension/src/background/approvals/service.test.ts +++ b/apps/extension/src/background/approvals/service.test.ts @@ -449,7 +449,7 @@ describe("approvals service", () => { const window = { tabs: [{ id: 1 }] }; (webextensionPolyfill.windows.create as any).mockResolvedValue(window); - await expect((service as any).createPopup(url)).resolves.toBe(window); + await expect(service["createPopup"](url)).resolves.toBe(window); expect(webextensionPolyfill.windows.create).toHaveBeenCalledWith( expect.objectContaining({ url }) ); @@ -458,25 +458,23 @@ describe("approvals service", () => { describe("getPopupTabId", () => { it("should return tab id", async () => { - const window = { tabs: [{ id: 1 }] }; - - expect((service as any).getPopupTabId(window)).toBe(1); + const window = { tabs: [{ id: 1 }] } as any; + expect(service["getPopupTabId"](window)).toBe(1); }); it("should throw an error if tabs are undefined", async () => { - const window = { tabs: undefined }; - expect(() => (service as any).getPopupTabId(window)).toThrow(); + const window = { tabs: undefined } as any; + expect(() => service["getPopupTabId"](window)).toThrow(); }); it("should throw an error if tabs are empty", async () => { - const window = { tabs: [] }; - - expect(() => (service as any).getPopupTabId(window)).toThrow(); + const window = { tabs: [] } as any; + expect(() => service["getPopupTabId"](window)).toThrow(); }); it("should throw an error if the tab already exists on the resolverMap", async () => { const popupTabId = 1; - const window = { tabs: [{ id: popupTabId }] }; + const window = { tabs: [{ id: popupTabId }] } as any; service["resolverMap"] = { [popupTabId]: { resolve: jest.fn(), @@ -484,7 +482,7 @@ describe("approvals service", () => { }, }; - expect(() => (service as any).getPopupTabId(window)).toThrow(); + expect(() => service["getPopupTabId"](window)).toThrow(); }); }); @@ -499,34 +497,46 @@ describe("approvals service", () => { .spyOn(service, "createPopup") .mockImplementationOnce(() => window); - (service as any).launchApprovalPopup(route, params); + void service["launchApprovalPopup"](route, params); - expect(await (service as any).createPopup).toHaveBeenCalledWith( + expect(service["createPopup"]).toHaveBeenCalledWith( `url#${route}?foo=bar` ); - expect((service as any).resolverMap[popupTabId]).toBeDefined(); + await new Promise((r) => r()); + expect(service["resolverMap"][popupTabId]).toBeDefined(); }); }); describe("getResolver", () => { it("should get the related tab id resolver from resolverMap", async () => { const popupTabId = 1; - const resolver = new Promise(() => {}); - (service as any).resolverMap = { + const resolver = { resolve: () => {}, reject: () => {} }; + service["resolverMap"] = { [popupTabId]: resolver, }; - expect((service as any).getResolver(popupTabId)).toBe(resolver); + expect(service["getResolver"](popupTabId)).toBe(resolver); }); it("should throw an error if there is no resolver for the tab id", async () => { - const myPopupTabId = 1; - const myResolver = new Promise(() => {}); - (service as any).resolverMap = { - [myPopupTabId]: myResolver, + const popupTabId = 1; + service["resolverMap"] = { + [popupTabId]: { resolve: () => {}, reject: () => {} }, + }; + + expect(() => service["getResolver"](999)).toThrow(); + }); + }); + + describe("removeResolver", () => { + it("should remove related tab id resolver from resolverMap", async () => { + const popupTabId = 1; + service["resolverMap"] = { + [popupTabId]: { resolve: () => {}, reject: () => {} }, }; + service["removeResolver"](popupTabId); - expect(() => (service as any).getResolver("999")).toThrow(); + expect(service["resolverMap"][popupTabId]).toBeUndefined(); }); }); diff --git a/apps/extension/src/background/approvals/service.ts b/apps/extension/src/background/approvals/service.ts index ed616e494..9c77337ab 100644 --- a/apps/extension/src/background/approvals/service.ts +++ b/apps/extension/src/background/approvals/service.ts @@ -39,7 +39,7 @@ export class ApprovalsService { protected readonly broadcaster: ExtensionBroadcaster ) { browser.tabs.onRemoved.addListener((tabId) => { - delete this.resolverMap[tabId]; + this.removeResolver(tabId); }); } @@ -331,11 +331,11 @@ export class ApprovalsService { return new Promise((resolve, reject) => { this.resolverMap[popupTabId] = { resolve: (args: T) => { - delete this.resolverMap[popupTabId]; + this.removeResolver(popupTabId); return resolve(args); }, reject: (args: unknown) => { - delete this.resolverMap[popupTabId]; + this.removeResolver(popupTabId); return reject(args); }, }; @@ -349,4 +349,8 @@ export class ApprovalsService { } return resolvers; }; + + private removeResolver = (popupTabId: number): void => { + delete this.resolverMap[popupTabId]; + }; } From 8cc08e8655f459a1cd4936291165a5462db59027 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Harrison=20Mendon=C3=A7a?= Date: Thu, 12 Sep 2024 19:35:32 +0200 Subject: [PATCH 13/14] feat: fix test --- apps/extension/src/provider/Namada.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/apps/extension/src/provider/Namada.test.ts b/apps/extension/src/provider/Namada.test.ts index bd151b59d..9cc91aa25 100644 --- a/apps/extension/src/provider/Namada.test.ts +++ b/apps/extension/src/provider/Namada.test.ts @@ -21,6 +21,14 @@ jest.mock( ) ); +jest.mock("webextension-polyfill", () => ({ + tabs: { + onRemoved: { + addListener: jest.fn(), + }, + }, +})); + describe("Namada", () => { let namada: Namada; let vaultStorage: VaultStorage; From 32954081697c0a2984d95f49a1c9cf3f37c7cbdb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Harrison=20Mendon=C3=A7a?= Date: Thu, 12 Sep 2024 19:54:10 +0200 Subject: [PATCH 14/14] feat: clean --- apps/extension/src/background/approvals/init.ts | 2 +- apps/extension/src/background/approvals/service.test.ts | 2 +- apps/extension/src/background/approvals/service.ts | 6 +++++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/apps/extension/src/background/approvals/init.ts b/apps/extension/src/background/approvals/init.ts index 7c32746a2..f9065dca4 100644 --- a/apps/extension/src/background/approvals/init.ts +++ b/apps/extension/src/background/approvals/init.ts @@ -34,8 +34,8 @@ export function init(router: Router, service: ApprovalsService): void { router.registerMessage(SubmitApprovedSignLedgerTxMsg); router.registerMessage(IsConnectionApprovedMsg); router.registerMessage(ApproveConnectInterfaceMsg); - router.registerMessage(ApproveDisconnectInterfaceMsg); router.registerMessage(ConnectInterfaceResponseMsg); + router.registerMessage(ApproveDisconnectInterfaceMsg); router.registerMessage(DisconnectInterfaceResponseMsg); router.registerMessage(RevokeConnectionMsg); router.registerMessage(QueryTxDetailsMsg); diff --git a/apps/extension/src/background/approvals/service.test.ts b/apps/extension/src/background/approvals/service.test.ts index fb40739f0..52247a1bf 100644 --- a/apps/extension/src/background/approvals/service.test.ts +++ b/apps/extension/src/background/approvals/service.test.ts @@ -278,7 +278,7 @@ describe("approvals service", () => { ); service["resolverMap"][tabId]?.resolve(true); - expect((service as any).launchApprovalPopup).toHaveBeenCalledWith( + expect(service["launchApprovalPopup"]).toHaveBeenCalledWith( "/approve-connection", { interfaceOrigin } ); diff --git a/apps/extension/src/background/approvals/service.ts b/apps/extension/src/background/approvals/service.ts index 9c77337ab..ca167473d 100644 --- a/apps/extension/src/background/approvals/service.ts +++ b/apps/extension/src/background/approvals/service.ts @@ -39,7 +39,11 @@ export class ApprovalsService { protected readonly broadcaster: ExtensionBroadcaster ) { browser.tabs.onRemoved.addListener((tabId) => { - this.removeResolver(tabId); + const resolver = this.getResolver(tabId); + if (resolver) { + resolver.reject(new Error("Window closed")); + this.removeResolver(tabId); + } }); }