Metamask-extension: Small bug with .enable() with privacy mode off.

Created on 5 Nov 2018  Â·  11Comments  Â·  Source: MetaMask/metamask-extension

When privacy mode is disabled, and .enable() is called, the user is shown up to two screens:

  1. Enter your password to unlock.
  2. The current site would like to view your account.

The site receives its callback with the current account after step 1, emphasizing that step 2 is just a formality. This is probably fine, since privacy mode is disabled, but it does separate the prompt from its result.

All 11 comments

Ideally, the site would receive its callback after the user approves. This is probably not actually an option without privacy mode, so feel free to close/ignore.

Thanks for reporting this. The user should not see any popup if privacy mode is disabled, the Promise should resolve immediately. Investigating...

If the user's MetaMask is locked, showing the popup is a great user experience, so I personally prefer this to an immediate rejection, it's just that the expectations of the two screens don't line up with when the callback is called.

@danfinlay If I read your issue correctly, this happens when privacy mode is _off_. This means legacy behavior should be maintained, so calling enable should just resolve the promise immediately with whatever web3.eth.accounts would return. There shouldn't be any popup at all when privacy mode is off and a user is browsing in legacy mode.

Immediate rejection is the old behavior to the letter, but I think prompting the user to unlock on enable() may actually be completely backwards compatible, although I'll need to step through all the possibilities to be sure, I'm pretty sure the one breaking edge case is that an app receives accounts when it otherwise had UI for instructing a user how to unlock manually. This is a usability gain, and shouldn't be considered a breaking change.

Meanwhile, the dev experience created by not prompting on enable() with privacy mode off would prompt this concern:

As a developer who wants to support MetaMask users who may or may not have privacy-mode enabled, I would like a way to distinguish between:

  • Users who have MetaMask locked (non-privacy mode) => I should suggest a manual unlock.
  • Users who have privacy mode enabled => I should call .enable().

This would allow developers to provide some conditional UI. Maybe it could be a property on the provider:

  • provider.hasPrivacyMode === true

Without a data source like that, I am forced to have some naĂŻve conditional UI:

  1. If the user is not signed in, I can show a “sign in” button.
  2. I then must take multiple actions at once:

    1. I call ethereum.enable()

    2. I show instructions on how to unlock MetaMask.

    3. I poll for accounts

    4. I wait for response to enable()

Ah, I think the proof is simple that this is non-breaking:

If a dapp is calling .enable(), they are already poised to receive accounts, by virtue of being EIP-1102 compatible, and so we don't need to protect "old" behavior for this new method.

Old behavior (non-updated dapps) simply don't call enable, and so they don't run into this edge case.

Thanks very much for filing this @danfinlay.

Per discussion, calling ethereum.enable() when MetaMask is locked, even if privacy mode is disabled, will show an approval popup that allows the user to sign in. This means dapps will never receive an empty array of accounts and will never have to account for it when calling ethereum.enable(). This functionality currently exists on metamask/metamask-extension#4703.

cc @derrickpelletier.

Hmm, so this sounds to me like you're not making any change based on the discussion? I might have miscommunicated the original issue I had, so i'll elaborate just in case:

I'll share a gif of what I was referring to. In this gif:

  • MetaMask is locked
  • privacy mode is OFF
  • I call window.ethereum.enable()

I agree that the password prompt should appear to sign into metamask, but why the subsequent screen asking for connection approval?

... Even if the user clicks cancel at this point (which is normally rejecting approval), my dapp still has access to the addresses—which is at the very least misleading.

locked-approval-req

Thanks for the added context @derrickpelletier. I agree that the most optimal flow would be to only show the screen allowing users to log in. We're investigating the effort required to implement this now.

... Even if the user clicks cancel at this point (which is normally rejecting approval), my dapp still has access to the addresses—which is at the very least misleading.

Was going to submit an issue for that

This happen in non-privacy mode and will be very confusing to the user. The dapp can detect rejection but after a reload, the dapp can't know and will thus let the user access, even if the user rejected.

This happen in non-privacy mode and will be very confusing to the user. The dapp can detect rejection but after a reload, the dapp can't know and will thus let the user access, even if the user rejected.

We are currently refactoring the log-in code, and the rework fixes this issue.

Was this page helpful?
0 / 5 - 0 ratings