Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not including wallet providers for 3p iframes breaks ImmutableX #23142

Closed
bbondy opened this issue May 30, 2022 · 2 comments
Closed

Not including wallet providers for 3p iframes breaks ImmutableX #23142

bbondy opened this issue May 30, 2022 · 2 comments
Labels

Comments

@bbondy
Copy link
Member

bbondy commented May 30, 2022

Reverting brave/brave-core#13268 locally fixes the problem.
Introduced by issue #22686

Steps to reproduce:

  1. Go to https://market.immutable.com/
  2. Try to connect your wallet

Actual results:
No wallet detected error shows up

Expected results:
It pops up Brave Wallet

I did some logging:

diff --git a/renderer/brave_wallet/brave_wallet_render_frame_observer.cc b/renderer/brave_wallet/brave_wallet_render_frame_observer.cc
index 9e219ace5a..ecb06b78bb 100644
--- a/renderer/brave_wallet/brave_wallet_render_frame_observer.cc
+++ b/renderer/brave_wallet/brave_wallet_render_frame_observer.cc
@@ -45,6 +45,11 @@ void BraveWalletRenderFrameObserver::DidCreateScriptContext(
     js_ethereum_provider_.reset();
     return;
   }
+
+  LOG(ERROR) << "====top security origin: " << url::Origin(render_frame()->GetWebFrame()->Top()->GetSecurityOrigin()).GetURL();
+  LOG(ERROR) << "====security origin: " << url::Origin(render_frame()->GetWebFrame()->GetSecurityOrigin()).GetURL();
+

And get:

[70565:259:0530/105109.057027:ERROR:brave_wallet_render_frame_observer.cc(50)] ====top security origin: https://market.immutable.com/
[70565:259:0530/105109.070682:ERROR:brave_wallet_render_frame_observer.cc(51)] ====security origin: https://akuma.immutable.com/

@bbondy bbondy added OS/Android Fixes related to Android browser functionality OS/Desktop labels May 30, 2022
@bbondy bbondy added this to Web3 May 30, 2022
@bbondy bbondy moved this to In Progress in Web3 May 30, 2022
@bbondy
Copy link
Member Author

bbondy commented May 31, 2022

Connect https://link.x.immutable.com
Sign: https://link.x.immutable.com

Modify hosts file to have:
127.0.0.1 z.com
127.0.0.1 y.z.com
127.0.0.1 y.com

Note: Granting permission on z.com does not give you permission on y.z.com and vice versa. They are treated differently. The below is only for what is allowed to prompt for permission.

I break down how it works into 3 sections:

  1. What we used to do (which also matches what MetaMask currently does).
  2. What we changed to in 1.39.x
  3. What I want it to do for webcompat

Original Brave (before 1.39.x) and Current MetaMask:

Test case 1)
Top level load: z.com
iframes of:

  1. Same context:
 Can request permission
  2. z.com 
Can request permission
  3. y.z.com 
Can request permission
  4. y.com 
Can request permission

Test case 2)
Top level load: y.z.com
iframes of:

  1. Same context
: Can request permission
  2. z.com
 Can request permission
  3. y.z.com 
Can request permission
  4. y.com
 Can request permission

1.39.x (current release)

Test case 1)
Top level load: z.com
iframes of:

  1. Same context:
 Can request permission
  2. z.com
 Can request permission
  3. y.z.com
 Cannot request permission (no window.ethereum exists)
  4. y.com 
Cannot request permission (no window.ethereum exists)

Test case 2)
Top level load: y.z.com
iframes of:

  1. Same context
: Can request permission
  2. z.com
 Cannot request permission (no window.ethereum exists)
  3. y.z.com
 Can request permission
  4. y.com
 Cannot request permission (no window.ethereum exists)

With implemented changes

Test case 1)
Top level load: z.com
iframes of:

  1. Same context:
 Can request permission
  2. z.com 
Can request permission
  3. y.z.com 
Can request permission
  4. y.com 
Cannot request permission (no window.ethereum exists)

Test case 2)
Top level load: y.z.com
iframes of:

  1. Same context: 
Can request permission
  2. z.com 
Can request permission
  3. y.z.com 
Can request permission
  4. y.com 
Cannot request permission (no window.ethereum exists)

@bbondy
Copy link
Member Author

bbondy commented Jun 28, 2022

No longer valid with brave/brave-core#13783

@bbondy bbondy closed this as completed Jun 28, 2022
Repository owner moved this from In Progress to Done in Web3 Jun 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant