Sign in with Ethereum: re-enable warning UI for mismatched domains / disable domain binding by digiwand · Pull Request #18200 · MetaMask/metamask-extension

digiwand · 2023-03-16T17:18:28Z

Explanation

Fixes #17707

Hotfix targeting v10.26.2

unblocks mismatched domain support
reverts logic from Sign-in with Ethereum Domain Binding #16616
we may re-add error handling here with proper error handling Handle SIWE domain mismatch error #18184
temp fix SIWE actionable-message styles
- Improved style fix in follow-up PR since it may effect other instances: Replace ActionableMessage components with BannerAlerts in SIWE Sign-in with Ethereum page #18207

Screenshots/Screencaps

Before

SIWE.mov

previous warning UI:

After

Manual Testing Steps

Login to the extension
Navigate to the test dapp
Click Sign In With Ethereum (Bad Account)
Observe warning flow

Pre-merge author checklist

I've clearly explained:
- What problem this PR is solving
- How this problem was solved
- How reviewers can test my changes
Sufficient automated test coverage has been added

Pre-merge reviewer checklist

Manual testing (e.g. pull and build branch, run in browser, test code being changed)
PR is linked to the appropriate GitHub issue
IF this PR fixes a bug in the release milestone, add this PR to the release milestone

If further QA is required (e.g. new feature, complex testing steps, large refactor), add the Extension QA Board label.

In this case, a QA Engineer approval will be be required.

- unblocks mismatched domain support - we may re-add error handling here #18184 - reverts logic from #16616

github-actions · 2023-03-16T17:18:41Z

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

skgbafa · 2023-03-16T17:42:38Z

The code change here looks solid. I think the shared components used in the warning was modified in other PRs, thus the odd views. @digiwand There is no other code to worry about from a functionality perspective

metamaskbot · 2023-03-16T17:46:09Z

Builds ready [d924916]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1591 ± 55 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	96	172	117	17	8
		domContentLoaded	1392	1802	1570	105	50
		load	1412	1849	1591	115	55
		domInteractive	1392	1802	1570	105	50

Bundle size diffs [🚀 Bundle size reduced!]

background: -176 bytes
ui: 0 bytes
common: 0 bytes

codecov · 2023-03-16T17:46:18Z

Codecov Report

Merging #18200 (e11c2fc) into develop (c21c2bd) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop   #18200      +/-   ##
===========================================
- Coverage    63.96%   63.95%   -0.00%     
===========================================
  Files          914      914              
  Lines        35623    35616       -7     
  Branches      9030     9027       -3     
===========================================
- Hits         22784    22778       -6     
+ Misses       12839    12838       -1

Impacted Files	Coverage Δ
app/scripts/lib/personal-message-manager.js	`75.12% <ø> (-0.36%)`	⬇️
...p/signature-request-siwe/signature-request-siwe.js	`2.17% <ø> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

digiwand · 2023-03-16T18:02:21Z

Thanks @skgbafa! for now, I've added code to fix the warning UI. We can follow-up with a proper fix and fine-tune the UI after

metamaskbot · 2023-03-16T19:01:32Z

Builds ready [bdbc997]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1669 ± 135 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	96	317	129	47	22
		domContentLoaded	1329	2334	1640	251	120
		load	1329	2556	1669	281	135
		domInteractive	1329	2334	1640	251	120

Bundle size diffs [🚀 Bundle size reduced!]

background: -176 bytes
ui: -19 bytes
common: 0 bytes

…ning-ui

metamaskbot · 2023-03-16T22:47:32Z

Builds ready [185213d]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1563 ± 41 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	98	235	120	29	14
		domContentLoaded	1465	1832	1552	88	42
		load	1471	1832	1563	86	41
		domInteractive	1465	1832	1552	88	42

Bundle size diffs [🚀 Bundle size reduced!]

background: -176 bytes
ui: -19 bytes
common: 0 bytes

app/scripts/lib/personal-message-manager.test.js

This reverts commit c80a4a2.

metamaskbot · 2023-03-17T00:33:10Z

Builds ready [412a91a]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1488 ± 30 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	91	124	103	10	5
		domContentLoaded	1346	1534	1466	51	25
		load	1346	1591	1488	62	30
		domInteractive	1346	1534	1466	51	25

Bundle size diffs [🚀 Bundle size reduced!]

background: -176 bytes
ui: -19 bytes
common: 0 bytes

danjm

LGTM

…ning-ui

digiwand · 2023-03-17T17:18:04Z

ui/components/app/signature-request-siwe/index.scss

    box-shadow: 0 0 7px 0 rgba(0, 0, 0, 0.08);
  }

+  /** @todo replace ActionableMessage or remove overwritten code. */


this is addressed in this follow-up PR: #18207

metamaskbot · 2023-03-17T17:23:58Z

Builds ready [e11c2fc]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1490 ± 35 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	90	195	113	26	12
		domContentLoaded	1299	1639	1482	73	35
		load	1313	1639	1490	74	35
		domInteractive	1299	1639	1482	73	35

Bundle size diffs [🚀 Bundle size reduced!]

background: -176 bytes
ui: -19 bytes
common: 0 bytes

naugtur · 2023-04-05T11:39:45Z

Follow-up to this
#18188 (comment)

siwe: re-enable warning UI for mismatched domains

d924916

- unblocks mismatched domain support - we may re-add error handling here #18184 - reverts logic from #16616

digiwand requested a review from a team as a code owner March 16, 2023 17:18

digiwand requested a review from NidhiKJha March 16, 2023 17:18

digiwand added 2 commits March 16, 2023 10:50

siwe: fix mismatch domain warning msg UI

9fb40be

lint: rm whitespace EOL

da120ff

digiwand added 2 commits March 16, 2023 11:07

siwe: rm unit test

c80a4a2

lint: fix whitespace

bdbc997

digiwand added needs-qa Label will automate into QA workspace team-confirmations-secure-ux-PR PRs from the confirmations team labels Mar 16, 2023

digiwand linked an issue Mar 16, 2023 that may be closed by this pull request

[Bug]: Sign In with Ethereum throws an unhandled exception for bad domains #17707

Closed

Merge branch 'develop' into fix-siwe-unblock-mismatch-domain-with-war…

185213d

…ning-ui

jpuri previously approved these changes Mar 16, 2023

View reviewed changes

jpuri reviewed Mar 16, 2023

View reviewed changes

app/scripts/lib/personal-message-manager.test.js Show resolved Hide resolved

Revert "siwe: rm unit test"

412a91a

This reverts commit c80a4a2.

digiwand dismissed jpuri’s stale review via 412a91a March 17, 2023 00:09

digiwand requested a review from jpuri March 17, 2023 00:29

danjm approved these changes Mar 17, 2023

View reviewed changes

Merge branch 'develop' into fix-siwe-unblock-mismatch-domain-with-war…

e11c2fc

…ning-ui

digiwand commented Mar 17, 2023

View reviewed changes

jpuri approved these changes Mar 17, 2023

View reviewed changes

digiwand merged commit c4caef5 into develop Mar 17, 2023

digiwand deleted the fix-siwe-unblock-mismatch-domain-with-warning-ui branch March 17, 2023 17:36

github-actions bot locked and limited conversation to collaborators Mar 17, 2023

digiwand added the area-signatures label Mar 24, 2023

PeterYinusa added the rc-cherry-picked label Mar 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sign in with Ethereum: re-enable warning UI for mismatched domains / disable domain binding#18200

Sign in with Ethereum: re-enable warning UI for mismatched domains / disable domain binding#18200
digiwand merged 8 commits intodevelopfrom
fix-siwe-unblock-mismatch-domain-with-warning-ui

digiwand commented Mar 16, 2023 •

edited

Loading

Uh oh!

github-actions bot commented Mar 16, 2023

Uh oh!

skgbafa commented Mar 16, 2023 •

edited

Loading

Uh oh!

metamaskbot commented Mar 16, 2023

Uh oh!

codecov bot commented Mar 16, 2023 •

edited

Loading

Uh oh!

digiwand commented Mar 16, 2023

Uh oh!

metamaskbot commented Mar 16, 2023

Uh oh!

metamaskbot commented Mar 16, 2023

Uh oh!

Uh oh!

metamaskbot commented Mar 17, 2023

Uh oh!

danjm left a comment

Uh oh!

digiwand Mar 17, 2023

Uh oh!

metamaskbot commented Mar 17, 2023

Uh oh!

naugtur commented Apr 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Conversation

digiwand commented Mar 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation

Screenshots/Screencaps

Before

After

Manual Testing Steps

Pre-merge author checklist

Pre-merge reviewer checklist

Uh oh!

github-actions bot commented Mar 16, 2023

Uh oh!

skgbafa commented Mar 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

metamaskbot commented Mar 16, 2023

Uh oh!

codecov bot commented Mar 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

digiwand commented Mar 16, 2023

Uh oh!

metamaskbot commented Mar 16, 2023

Uh oh!

metamaskbot commented Mar 16, 2023

Uh oh!

Uh oh!

metamaskbot commented Mar 17, 2023

Uh oh!

danjm left a comment

Choose a reason for hiding this comment

Uh oh!

digiwand Mar 17, 2023

Choose a reason for hiding this comment

Uh oh!

metamaskbot commented Mar 17, 2023

Uh oh!

naugtur commented Apr 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

digiwand commented Mar 16, 2023 •

edited

Loading

skgbafa commented Mar 16, 2023 •

edited

Loading

codecov bot commented Mar 16, 2023 •

edited

Loading