detect-provider icon indicating copy to clipboard operation
detect-provider copied to clipboard

Adds support for delayed injection.

Open MicahZoltu opened this issue 3 years ago • 6 comments

A privacy respecting wallet will not inject into the page until after the user tells it to so websites cannot fingerprint the user. Unfortunately, this script as currently written doesn't support this pattern because it only calls handleEthereum once and if the timeout is hit before ethereum is injected this will unsubscribe from the event listener and refuse any further attempts at connecting things up.

By checking for ethereum in window first in handleEthereum, if the timeout is hit but nothing is hooked up yet then the system will continue listening for the ethereum#initialized event indefinitely.

MicahZoltu avatar Jul 24 '22 16:07 MicahZoltu

Hi @MicahZoltu, sorry for taking so long to reply to this. Can you give us an example of which wallet this pattern would support? And what do you mean by "A privacy respecting wallet will not inject into the page until after the user tells it"? How would the user tell it? Really, I'm looking for a way I can replicate this issue so that we can confirm that it's fixed by your change.

mcmire avatar Oct 03 '22 16:10 mcmire

This package assumes that the provider is injected right away (possibly asynchronously, but still initiated without delay), but some wallets might not do that. If a dapp uses this package to detect the provider, but the user chose to inject the provider five minutes after loading the page, the timeout would have already expired. The result would be a console error about there being no provider, and the dapp would have been handed null in place of a provider. The dapp also wouldn't be notified of the late injection.

This change ensures that the listener is never removed, so late injections are correctly detected and we don't have that console error.

I'm unsure about this change because it (seemingly) would make the call to detectProvider hang indefinitely if the user truly had no wallet. I would like this library to be useful in detecting late-injected providers somehow though.

Gudahtt avatar Oct 03 '22 16:10 Gudahtt

Hi @MicahZoltu, sorry for taking so long to reply to this. Can you give us an example of which wallet this pattern would support? And what do you mean by "A privacy respecting wallet will not inject into the page until after the user tells it"? How would the user tell it? Really, I'm looking for a way I can replicate this issue so that we can confirm that it's fixed by your change.

Unfortunately, there aren't any wallets that do this because most dapps use this package, and the dapp breaks when a wallet doesn't auto-inject if the dapp is using this package. I am trying to build a wallet and ran into this issue, which forced me to "do the bad thing" and auto-inject because without it, my wallet doesn't work with any dapps out there.

Extensions can be configured such that they only inject into the page when they are activated (their icon is clicked). This not only allows the user to choose which pages get injected into, but it also makes it so the extension does not need the "read and write data on all pages" permission which is insanely permissive/risky.

To Replicate

If you happen to have the source code for a wallet that you can build, just change the permissions to activeTab (https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/permissions#activetab_permission) and remove the tabs permission. Then load a page, wait a bit, then click the extension icon (which will trigger the injection script to run).

MicahZoltu avatar Oct 04 '22 12:10 MicahZoltu

I'm unsure about this change because it (seemingly) would make the call to detectProvider hang indefinitely if the user truly had no wallet. I would like this library to be useful in detecting late-injected providers somehow though.

The promise will not resolve, but it is easy enough to set a timeout and have your application present something to the user after a while. That being said, I think an infinite spinner would be fine here, with some text like "Waiting for browser wallet to inject... You may need to click the browser extension to activate your browser wallet for this page."

MicahZoltu avatar Oct 04 '22 12:10 MicahZoltu

To reproduce, you can try just running this in the browser console some time after the dapp is loaded:

var request = async ({method, params}) => { if (method === 'eth_requestAccounts') return ['0xd8da6bf26964af9d7eed9e03e53415d37aa96045']; else if (method === 'eth_chainId') return '0x1'; else return []; }
var send = async (method, params) => { if (typeof method === 'object') { return await request({method: method.method, params: method.params}) } else { return await request({ method, params }) } }
var sendAsync = async (payload, callback) => { request(payload).then(result => callback(null, {jsonrpc: '2.0', id: payload.id, result })).catch(error => callback({ jsonrpc: '2.0', id: payload.id, error: { code: error.code, message: error.message, data: { ...error.data, stack: error.stack } } }, null)) }
var ethereumOn = async (kind, callback) => console.log(`window.ethereum.on called for ${kind}`)
window.ethereum = { request, send, sendAsync, on: ethereumOn }

It is just a dummy provider, but it should be enough to trigger the behavior and verify a fix.

MicahZoltu avatar Oct 04 '22 13:10 MicahZoltu

When I first read this issue, I was hoping we could have some sort of feature-detection, to try to discover wallets that have delayed injection. But for extensions that only inject into the page on-click, they don't even have a contentscript process until after being clicked. So there's no way for the website to communicate with the extension, unless the extension has externally_connectable set to * (which Firefox seemingly has no plans to support anytime soon).

Reliable feature detection would require promoting a standard. That would be a tough sell if it relies upon a specific Chrome-only permission being set. Oh well.

The promise will not resolve, but it is easy enough to set a timeout and have your application present something to the user after a while. That being said, I think an infinite spinner would be fine here, with some text like "Waiting for browser wallet to inject... You may need to click the browser extension to activate your browser wallet for this page."

I see, thanks for the suggestion. That could work. It would make this library more difficult to use, but, maybe we can make up for that in other ways, like via documentation or by wrapping this in an easier-to-use library that handles more of this for devs.

Gudahtt avatar Oct 05 '22 16:10 Gudahtt