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

Display origin property in SIWE more effectively #33227

Open
kdenhartog opened this issue Sep 25, 2023 · 5 comments
Open

Display origin property in SIWE more effectively #33227

kdenhartog opened this issue Sep 25, 2023 · 5 comments
Labels
feature/web3/wallet/core feature/web3/wallet Integrating Ethereum+ wallet support OS/Android Fixes related to Android browser functionality OS/Desktop QA/Yes release-notes/exclude

Comments

@kdenhartog
Copy link
Member

Currently the Origin property in the SIWE panel will display JSON. This will likely lead to in-effective wrapping of the host such that it's not clear who's making the request or the request could be obfuscated in a way to trick the user.

Screenshot 2023-09-25 at 6 59 55 PM

To address this we should display this like a normal URL using bold text for the eTLD+1 similar to what we do in many other wallet panels.
Screenshot 2023-09-25 at 7 44 17 PM

To address this we'll likely need an update to the property being returned from core and then we'll need subsequent UI changes to display it properly.

cc @brave/ios to make sure you get a subsequent issue in for this as well.

@nuo-xu
Copy link

nuo-xu commented Sep 25, 2023

This is the current request panel for SIWE. We are displaying the correct eTLD+1
https://github.com/brave/brave-browser/assets/1187676/3c6b6d3e-168a-4d85-bd0b-52fbc8b1719c
For SIWE new UI, we will address in brave/brave-ios#7827

@kdenhartog
Copy link
Member Author

Thanks, I just started following that issue. This is likely going to require core changes first, but I suspect it will be done around similar time to the iOS changes.

@StephenHeaps
Copy link

StephenHeaps commented Sep 26, 2023

@kdenhartog I have the initial work for iOS to allow compilation after core changes (so iOS team can code review core PRs on master) available in our brave-core-1.60.x working branch. The initial work only implements the SIWE error view, but uses existing signature request view for SIWE (will be updated in brave/brave-ios#7827).

SIWE - Light mode Domain Mismatch - Light mode

@kdenhartog
Copy link
Member Author

Are you displaying the eTLD+1 as the origin here? Android was looking at doing that but ports/schemes were getting dropped which could become an issue in very rare circumstances (e.g. wss:// hosts different content or different port hosts different content on a shared site domain like github.io).

This is mainly about displaying the additional panels on the left side. What you've got on the right wouldn't be affected by this issue and LGTM.

@kdenhartog
Copy link
Member Author

After discussing this further in iOS it appears that if we use the origin_spec which will return the port if it's non standard here. https://bravesoftware.slack.com/archives/C04JJNFRPK7/p1698269920543509

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/web3/wallet/core feature/web3/wallet Integrating Ethereum+ wallet support OS/Android Fixes related to Android browser functionality OS/Desktop QA/Yes release-notes/exclude
Projects
None yet
Development

No branches or pull requests

3 participants