Skip to content

Commit

Permalink
fix: display the DApp URL in connect screen for MetaMask IOS-SDK (#9755)
Browse files Browse the repository at this point in the history
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**
- Display the DApp URL in connect screen for MetaMask IOS-SDK.
- Fixes the dapp icon on the connection screen
(#9834)

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

## **Related issues**

Fixes:

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**


https://github.com/MetaMask/metamask-mobile/assets/61094771/4da53e62-d80f-4b09-957b-a19f2775e824

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] 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)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] 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.

---------

Co-authored-by: Christopher Ferreira <104831203+christopherferreira9@users.noreply.github.com>
  • Loading branch information
omridan159 and christopherferreira9 authored Jun 4, 2024
1 parent 7e931f0 commit 7caff7b
Show file tree
Hide file tree
Showing 8 changed files with 634 additions and 77 deletions.
42 changes: 25 additions & 17 deletions app/components/Views/AccountConnect/AccountConnect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ import {
import AccountConnectMultiSelector from './AccountConnectMultiSelector';
import AccountConnectSingle from './AccountConnectSingle';
import AccountConnectSingleSelector from './AccountConnectSingleSelector';
import { SourceType } from '../../hooks/useMetrics/useMetrics.types';

const createStyles = () =>
StyleSheet.create({
fullScreenModal: {
Expand Down Expand Up @@ -107,24 +109,28 @@ const AccountConnect = (props: AccountConnectProps) => {
: AvatarAccountType.JazzIcon,
);

// on inappBrowser: hostname
// on walletConnect: hostname
// on sdk or walletconnect
// origin is set to the last active tab url in the browser which can conflict with sdk
const inappBrowserOrigin: string = useSelector(getActiveTabUrl, isEqual);
const accountsLength = useSelector(selectAccountsLength);

// TODO: pending transaction controller update, we need to have a parameter that can be extracted from the metadata to know the correct source (inappbrowser, walletconnect, sdk)
// on inappBrowser: hostname from inappBrowserOrigin
// on walletConnect: hostname from hostInfo
// on sdk: channelId
const { origin: channelIdOrHostname } = hostInfo.metadata as {
id: string;
origin: string;
};

const origin: string = useSelector(getActiveTabUrl, isEqual);
const accountsLength = useSelector(selectAccountsLength);

const sdkConnection = SDKConnect.getInstance().getConnection({
channelId: channelIdOrHostname,
});
const hostname =
origin ?? channelIdOrHostname.indexOf('.') !== -1

const hostname = channelIdOrHostname
? channelIdOrHostname.indexOf('.') !== -1
? channelIdOrHostname
: sdkConnection?.originatorInfo?.url ?? '';
: sdkConnection?.originatorInfo?.url ?? ''
: inappBrowserOrigin;

const urlWithProtocol = prefixUrlWithProtocol(hostname);

Expand Down Expand Up @@ -160,7 +166,7 @@ const AccountConnect = (props: AccountConnectProps) => {
}
}, [isAllowedUrl, dappUrl, channelIdOrHostname]);

const faviconSource = useFavicon(origin);
const faviconSource = useFavicon(inappBrowserOrigin);

const actualIcon = useMemo(
() => (dappIconUrl ? { uri: dappIconUrl } : faviconSource),
Expand All @@ -179,13 +185,15 @@ const AccountConnect = (props: AccountConnectProps) => {
// walletconnect channelId format: app.name.org
// sdk channelId format: uuid
// inappbrowser channelId format: app.name.org but origin is set
if (sdkConnection) {
return 'sdk';
} else if (origin) {
return 'in-app browser';
if (channelIdOrHostname) {
if (sdkConnection) {
return SourceType.SDK;
}
return SourceType.WALLET_CONNECT;
}
return 'walletconnect';
}, [sdkConnection, origin]);

return SourceType.IN_APP_BROWSER;
}, [sdkConnection, channelIdOrHostname]);

// Refreshes selected addresses based on the addition and removal of accounts.
useEffect(() => {
Expand Down Expand Up @@ -216,7 +224,7 @@ const AccountConnect = (props: AccountConnectProps) => {

trackEvent(MetaMetricsEvents.CONNECT_REQUEST_CANCELLED, {
number_of_accounts: accountsLength,
source: 'permission system',
source: SourceType.PERMISSION_SYSTEM,
});
},
[
Expand Down
7 changes: 7 additions & 0 deletions app/components/hooks/useMetrics/useMetrics.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,13 @@ import {
IMetaMetricsEvent,
} from '../../../core/Analytics/MetaMetrics.types';

export enum SourceType {
SDK = 'sdk',
WALLET_CONNECT = 'walletconnect',
IN_APP_BROWSER = 'in-app browser',
PERMISSION_SYSTEM = 'permission system',
}

export interface IUseMetricsHook {
isEnabled(): boolean;
enable(enable?: boolean): Promise<void>;
Expand Down
1 change: 1 addition & 0 deletions app/core/DeeplinkManager/ParseManager/extractURLParams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export interface DeeplinkUrlParams {
message?: string;
originatorInfo?: string;
request?: string;
account?: string; // This is the format => "address@chainId"
}

function extractURLParams(url: string) {
Expand Down
133 changes: 133 additions & 0 deletions app/core/DeeplinkManager/ParseManager/handleMetaMaskDeeplink.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,139 @@ describe('handleMetaMaskProtocol', () => {
});
});

describe('when params.comm is "deeplinking"', () => {
beforeEach(() => {
url = `${PREFIXES.METAMASK}${ACTIONS.CONNECT}`;
params.comm = 'deeplinking';
params.channelId = 'test-channel-id';
params.pubkey = 'test-pubkey';
params.originatorInfo = 'test-originator-info';
params.request = 'test-request';
});

it('should throw an error if params.scheme is not defined', () => {
params.scheme = undefined;

expect(() => {
handleMetaMaskDeeplink({
instance,
handled,
params,
url,
origin,
wcURL,
});
}).toThrow('DeepLinkManager failed to connect - Invalid scheme');
});

it('should call handleConnection if params.scheme is defined', () => {
const mockHandleConnection = jest.fn();
mockSDKConnectGetInstance.mockImplementation(() => ({
state: {
deeplinkingService: {
handleConnection: mockHandleConnection,
},
},
}));

params.scheme = 'test-scheme';

handleMetaMaskDeeplink({
instance,
handled,
params,
url,
origin,
wcURL,
});

expect(mockHandleConnection).toHaveBeenCalledWith({
channelId: params.channelId,
url,
scheme: params.scheme,
dappPublicKey: params.pubkey,
originatorInfo: params.originatorInfo,
request: params.request,
});
});
});

describe('when url starts with ${PREFIXES.METAMASK}${ACTIONS.MMSDK}', () => {
beforeEach(() => {
url = `${PREFIXES.METAMASK}${ACTIONS.MMSDK}`;
params.channelId = 'test-channel-id';
params.pubkey = 'test-pubkey';
params.account = 'test-account';
});

it('should throw an error if params.message is not defined', () => {
params.message = undefined;

expect(() => {
handleMetaMaskDeeplink({
instance,
handled,
params,
url,
origin,
wcURL,
});
}).toThrow(
'DeepLinkManager: deeplinkingService failed to handleMessage - Invalid message',
);
});

it('should throw an error if params.scheme is not defined', () => {
params.message = 'test-message';
params.scheme = undefined;

expect(() => {
handleMetaMaskDeeplink({
instance,
handled,
params,
url,
origin,
wcURL,
});
}).toThrow(
'DeepLinkManager: deeplinkingService failed to handleMessage - Invalid scheme',
);
});

it('should call handleMessage if params.message and params.scheme are defined', () => {
const mockHandleMessage = jest.fn();
mockSDKConnectGetInstance.mockImplementation(() => ({
state: {
deeplinkingService: {
handleMessage: mockHandleMessage,
},
},
}));

params.message = 'test-message';
params.scheme = 'test-scheme';

handleMetaMaskDeeplink({
instance,
handled,
params,
url,
origin,
wcURL,
});

expect(mockHandleMessage).toHaveBeenCalledWith({
channelId: params.channelId,
url,
message: params.message,
dappPublicKey: params.pubkey,
scheme: params.scheme,
account: params.account ?? '@',
});
});
});

describe('when url starts with ${PREFIXES.METAMASK}${ACTIONS.CONNECT}', () => {
beforeEach(() => {
url = `${PREFIXES.METAMASK}${ACTIONS.CONNECT}`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,14 +89,13 @@ export function handleMetaMaskDeeplink({
);
}

DevLogger.log('DeepLinkManager:: ===> params from deeplink', params);

SDKConnect.getInstance().state.deeplinkingService?.handleMessage({
channelId: params.channelId,
url,
message: params.message,
dappPublicKey: params.pubkey,
scheme: params.scheme,
account: params.account ?? '@',
});
} else if (
url.startsWith(`${PREFIXES.METAMASK}${ACTIONS.WC}`) ||
Expand Down
12 changes: 11 additions & 1 deletion app/core/SDKConnect/AndroidSDK/AndroidService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ export default class AndroidService extends EventEmitter2 {
}

private setupOnClientsConnectedListener() {
this.eventHandler.onClientsConnected((sClientInfo: string) => {
this.eventHandler.onClientsConnected(async (sClientInfo: string) => {
const clientInfo: DappClient = JSON.parse(sClientInfo);

DevLogger.log(`AndroidService::clients_connected`, clientInfo);
Expand Down Expand Up @@ -155,6 +155,15 @@ export default class AndroidService extends EventEmitter2 {
return;
}

await SDKConnect.getInstance().addDappConnection({
id: clientInfo.clientId,
lastAuthorized: Date.now(),
origin: AppConstants.MM_SDK.ANDROID_SDK,
originatorInfo: clientInfo.originatorInfo,
otherPublicKey: '',
validUntil: Date.now() + DEFAULT_SESSION_TIMEOUT_MS,
});

const handleEventAsync = async () => {
const keyringController = (
Engine.context as { KeyringController: KeyringController }
Expand Down Expand Up @@ -361,6 +370,7 @@ export default class AndroidService extends EventEmitter2 {
const chainId = networkController.state.providerConfig.chainId;

this.currentClientId = sessionId;

// Handle custom rpc method
const processedRpc = await handleCustomRpcCalls({
batchRPCManager: this.batchRPCManager,
Expand Down
Loading

0 comments on commit 7caff7b

Please sign in to comment.