Prevent external domains from submitting more than one perm request at once

[rekmarks]

Summary

• Return an error if an external domain requests permissions while they already have a pending permissions request

Details

I haven't fully decided whether we should do this. Are there legitimate uses for multiple permission requests submitted at once?

Related: #7969 (whichever is merged last will have to be rebased and updated with new tests)

[metamaskbot]

Builds ready [8484c1a]

• builds: chrome, firefox, opera
• bundle viz: background, ui, inpage, contentscript, ui-libs, bg-libs, phishing-detect
• dep viz: background
• all artifacts

Page Load Metrics (661 ± 60 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Notification	firstPaint	37	92	50	17	8
		domContentLoaded	349	825	659	125	60
		load	354	827	661	124	60
		domInteractive	348	825	659	125	60

[Gudahtt]

Are there legitimate uses for multiple permission requests submitted at once?

Huh, interesting point 🤔. Maybe? It's conceivable that once we start adding more permissions, a dapp might allow the user to request them one-by-one in the dapp, thus triggering separate permission requests.

[rekmarks]

It's conceivable that once we start adding more permissions, a dapp might allow the user to request them one-by-one in the dapp, thus triggering separate permission requests.

Good point. We probably shouldn't restrict that without good reason. If we merge this, we should add an issue making sure that only e.g. duplicate eth_requestAccounts requests are rejected (that being the one that's probably most likely to get spammed).

[danfinlay]

I kinda like it just because it would fundamentally simplify the process of keeping the approval prompt safe. Devs will build interfaces that work well with our approval system, so we can simply avoid the scenario Mark suggested by merging this.

[rekmarks]

@danfinlay I think the case Mark described warrants an issue and further discussion (we already talked about it some), but I do agree that this should be merged for now regardless!

[Gudahtt]

I think I've come around to this idea - it would definitely be simpler. And if we impose this restriction from the start, dapps will be forced to accommodate it.

If we do allow multiple requests to be pending at once, then that raises the question of how to deal with "corrections", or requests that supersede previous ones. That would be a complex flow to handle; if we do nothing, then the user might customize permissions to their liking on the first request, then not remember what the chose the second time they're asked (i.e. we'd want to go the extra mile to show what's changing in order to help the user understand what's going on). If we try to merge the requests, then we risk things updating while the user is reviewing, which is dangerous. We could issue an alert, but that would be clunky. And on top of all of that, we'd still want to have some sort of rate-limiting in place to prevent abuse.

Far simpler to disallow it altogether.

[Gudahtt]

LGTM!