From 13130e377a6384374407b2805f661d9f88fd8b58 Mon Sep 17 00:00:00 2001 From: Mpendulo Ndlovu Date: Tue, 17 Oct 2023 19:33:14 +0200 Subject: [PATCH] fix: use hostname to fetch approvedhosts (#7489) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR fixes an Android SDK bug whereby: - The wallet was always returning the first permitted account instead of the selected account when connecting the SDK to MetaMask - Changing the selected account on MetaMask did not fire a `metamask_accountsChanged` event so the SDK did not get selected account updated. This change implements the `getApprovedHosts` to fix this issue. ## **Manual testing steps** _1. Have more than one address in your metamask account and select any address as active address _2. Connect from Android Metamask SDK and ensure the active account is the selected address on the SDK _3. Switch to another account on the wallet, and ensure that the SDK updates to the new address ## **Related issues** _Fixes #???_ ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've clearly explained: - [x] What problem this PR is solving. - [x] How this problem was solved. - [x] How reviewers can test my changes. - [ ] I’ve indicated what issue this PR is linked to: Fixes #??? - [ ] I’ve included tests if applicable. - [x] I’ve documented any added code. - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). - [x] I’ve properly set the pull request status: - [ ] In case it's not yet "ready for review", I've set it to "draft". - [ ] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- app/core/BackgroundBridge/BackgroundBridge.js | 7 ++++++- app/core/RPCMethods/RPCMethodMiddleware.ts | 4 ++-- app/core/SDKConnect/AndroidSDK/AndroidService.ts | 16 ++++++++++------ 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/app/core/BackgroundBridge/BackgroundBridge.js b/app/core/BackgroundBridge/BackgroundBridge.js index 2d06aaa46b8..fad72e2d4f7 100644 --- a/app/core/BackgroundBridge/BackgroundBridge.js +++ b/app/core/BackgroundBridge/BackgroundBridge.js @@ -195,7 +195,12 @@ export class BackgroundBridge extends EventEmitter { notifySelectedAddressChanged(selectedAddress) { if (this.isRemoteConn) { - if (!this.getApprovedHosts?.()?.[this.remoteConnHost]) return; + // Pass the remoteConnHost to getApprovedHosts as AndroidSDK requires it + if ( + !this.getApprovedHosts?.(this.remoteConnHost)?.[this.remoteConnHost] + ) { + return; + } } this.sendNotification({ method: NOTIFICATION_NAMES.accountsChanged, diff --git a/app/core/RPCMethods/RPCMethodMiddleware.ts b/app/core/RPCMethods/RPCMethodMiddleware.ts index a08767c6e5d..6e52dc91d06 100644 --- a/app/core/RPCMethods/RPCMethodMiddleware.ts +++ b/app/core/RPCMethods/RPCMethodMiddleware.ts @@ -244,7 +244,8 @@ export const getRpcMethodMiddleware = ({ const getAccounts = (): string[] => { const selectedAddress = Engine.context.PreferencesController.state.selectedAddress?.toLowerCase(); - const isEnabled = isWalletConnect || getApprovedHosts()[hostname]; + const approvedHosts = getApprovedHosts(hostname) || {}; + const isEnabled = isWalletConnect || approvedHosts[hostname]; return isEnabled && selectedAddress ? [selectedAddress] : []; }; @@ -907,5 +908,4 @@ export const getRpcMethodMiddleware = ({ } await rpcMethods[req.method](); }); - export default getRpcMethodMiddleware; diff --git a/app/core/SDKConnect/AndroidSDK/AndroidService.ts b/app/core/SDKConnect/AndroidSDK/AndroidService.ts index f69d38efd31..a3e4c8ca6a3 100644 --- a/app/core/SDKConnect/AndroidSDK/AndroidService.ts +++ b/app/core/SDKConnect/AndroidSDK/AndroidService.ts @@ -125,11 +125,11 @@ export default class AndroidService extends EventEmitter2 { try { if (!this.connectedClients?.[clientInfo.clientId]) { - await this.requestApproval(clientInfo); this.setupBridge(clientInfo); // Save session to SDKConnect SDKConnect.getInstance().addAndroidConnection({ id: clientInfo.clientId, + lastAuthorized: Date.now(), origin: AppConstants.MM_SDK.ANDROID_SDK, originatorInfo: clientInfo.originatorInfo, otherPublicKey: '', @@ -331,11 +331,13 @@ export default class AndroidService extends EventEmitter2 { const bridge = new BackgroundBridge({ webview: null, - isMMSDK: false, + isMMSDK: true, url: PROTOCOLS.METAMASK + '://' + AppConstants.MM_SDK.SDK_REMOTE_ORIGIN, isRemoteConn: true, sendMessage: this.sendMessage.bind(this), - getApprovedHosts: () => false, + getApprovedHosts: (host: string) => ({ + [host]: true, + }), remoteConnHost: clientInfo.originatorInfo.url ?? clientInfo.originatorInfo.title, getRpcMethodMiddleware: ({ @@ -348,10 +350,12 @@ export default class AndroidService extends EventEmitter2 { hostname: clientInfo.originatorInfo.url ?? clientInfo.originatorInfo.title, getProviderState, - isMMSDK: false, + isMMSDK: true, navigation: null, //props.navigation, - getApprovedHosts: () => ({}), - setApprovedHosts: () => ({}), + getApprovedHosts: (host: string) => ({ + [host]: true, + }), + setApprovedHosts: () => true, approveHost: () => ({}), // Website info url: {