ensdomains / ens-app-v3

The official ENS manager app. Register and manage your ENS names here.
https://app.ens.domains
MIT License
135 stars 114 forks source link

Connect Wallet (Metamask) not working in Firefox #504

Closed cairoeth closed 1 year ago

cairoeth commented 1 year ago

When trying to connect a wallet with Firefox, the app doesn't detect the Metamask extension installed in the browser. Therefore, it redirects the user to scan the QR code. Works as expected with Chrome.

image

On a similar note, the button to "Add to Firefox" in the Get Metamask modal redirects the user to the Chrome store instead of the Firefox addons page.

d00de62688bb92a392eb5834d0a08811

hashfriend commented 1 year ago

worth noting this works flawlessly with rainbowkit out of the box so not clear why such a convoluted soution is needed in #501 for this app

TateB commented 1 year ago

worth noting this works flawlessly with rainbowkit out of the box so not clear why such a convoluted soution is needed in #501 for this app

we have CSP headers enabled on the app site, which breaks the injection mechanism used by metamask (and other wallets) in firefox/ios

chaserene commented 1 year ago

how could you make the new version the default without first testing it on major browsers? you should at least redirect Firefox users to v2...

LeonmanRolls commented 1 year ago

You should ask firefox why they haven't fixed a years old security issue

LeonmanRolls commented 1 year ago

See https://bugzilla.mozilla.org/show_bug.cgi?id=1267027

chaserene commented 1 year ago

@LeonmanRolls so now it's the user's job to resolve a contentious Firefox bug because you didn't perform basic testing and/or aren't willing to implement a sensible UX flow.

for now, all you have to do is redirect users of Firefox and its derivatives who click on Metamask to the previous version. you can change that when you found a workaround. that CSP bug won't be fixed anytime soon.

your next best option is to leave every user of Firefox and Tor Browser user clueless as to why they can't access ENS.

LeonmanRolls commented 1 year ago

It's not that we didn't perform testing, it's more a question of wanting to protect users.

  1. We could have not implemented a CSP for everyone until we figured out this issue, giving everyone a less secure experience
  2. We could have not implemented a CSP for firefox/metmask users only, giving them a less secure experience.

So the decision was to taken to not allow firefox/metamask use on the app as it is less secure only while we look for a workaround, point taken we could have done something in the UI about that in the meantime. As you can imagine implementing things in the UI for specific wallet/browser combos is a bit of a rabbit hole.

TateB commented 1 year ago

fix is now merged and in place

chaserene commented 1 year ago

@TateB nice, thank you!

5ajaki commented 10 months ago

@TateB or @LeonmanRolls - Is this CSP implementation what's preventing the injected provider option ("Browser Wallet") from showing in the Rainbowkit Modal when using Firefox?

RainbowKit
TateB commented 10 months ago

iirc "Browser Wallet" isn't an option on the rainbowkit version the main build is on, unrelated to CSP.

5ajaki commented 10 months ago

The current site seems to disagree. Screen shot from now.

image

I see you're calling the Injected Wallet option in the config of the current build, perhaps that's where this is coming from?

TateB commented 10 months ago

just to clarify, what wallet are you trying to use with firefox?

5ajaki commented 10 months ago

Frame.sh, and it's their companion browser extension that's being picked up in that screen shot above using Brave.

5ajaki commented 10 months ago

@TateB - any thoughts? Are we back to first question of “is it our CSP implementation that’s preventing the “Browser Wallet” option from showing in Firefox while in Brave it does?

TateB commented 10 months ago

sorry, yep this would be the CSP on the site. we need to manually inject a metamask provider loader for firefox user agents. we need to specify and add every injected provider that we want to load in firefox unfortunately.

afaik there's no good solution for this issue (other than firefox fixing it), but i'm open to ideas.