feat(wallet): Use Wallet WebUI for Send & Swap on iOS #24328

StephenHeaps · 2024-06-21T21:58:11Z

Adds support for using desktop/android wallet WebUI for Send & Swap in native iOS wallet. The send & swap UI will replace the existing native iOS UI when user taps Send / Swap in Portfolio or via tray in Wallet Panel.
NFTs are displayed the same as in desktop/android using an iframe to isolate them with chrome-untrusted://nft-display.
Creating transactions will continue to dismiss the send & swap modal presented over the native iOS wallet and present the native iOS transaction confirmation panel.

Resolves brave/brave-browser#36968

Submitter Checklist:

I confirm that no security/privacy review is needed and no other type of reviews are needed, or that I have requested them
There is a ticket for my issue
Used Github auto-closing keywords in the PR description above
Wrote a good PR/commit description
Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
Checked the PR locally:
- npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
- npm run presubmit wiki, npm run gn_check, npm run tslint
Ran git rebase master (if needed)

Reviewer Checklist:

A security review is not needed, or a link to one is included in the PR description
New files have MPL-2.0 license header
Adequate test coverage exists to prevent regressions
Major classes, functions and non-trivial code blocks are well-commented
Changes in component dependencies are properly reflected in gn
Code follows the style guide
Test plan is specified in PR before merging

After-merge Checklist:

The associated issue milestone is set to the smallest version that the
changes has landed on
All relevant documentation has been updated, for instance:

Test Plan:

Open native Brave Wallet and verify Send & Swap both still use the existing native views when feature flag is disabled (default).
Open brave://flags and find Enable WebUI for Brave Wallet iOS feature flag. Enable it & restart the app.
Open native Brave Wallet and verify Send & Swap both use the WebUI seen on desktop (panel) and android.
Test using WebUI on iOS.
- Creating Send transactions should work the same as on desktop/android. The exact same accounts & networks shown in native iOS wallet should be available for creating transactions.
  - Some assets (including NFTs) marked hidden may be shown in WebUI send if user has balance until [Wallet/iOS] Migrate user assets back to WalletService brave-browser#39098
- Creating Swap transactions should work the same as on desktop/android. The exact same accounts & networks shown in native iOS wallet should be available in swap

Send.WebUI.Demo.MP4

Swap.WebUI.Demo.MP4

brave-builds · 2024-06-21T22:52:44Z

A Storybook has been deployed to preview UI for the latest push

brave-builds · 2024-06-25T20:33:42Z

A Storybook has been deployed to preview UI for the latest push

brave-builds · 2024-06-26T16:07:12Z

A Storybook has been deployed to preview UI for the latest push

brave-builds · 2024-06-26T19:07:55Z

A Storybook has been deployed to preview UI for the latest push

josheleonard

Approved with nits

josheleonard · 2024-06-27T13:58:35Z

components/brave_wallet_ui/utils/amount.ts

@@ -387,4 +394,4 @@ export default class Amount {

    return formatter.format(value) + abbreviation
  }
-}
+}


nit: missing EOF new-line

Fixed in 603c091

josheleonard · 2024-06-27T13:59:49Z

components/brave_wallet_ui/utils/style.utils.ts

-
-  return `rgba(${r},${g},${b},0.2)`
-}
+}


nit: missing EOF new line

Fixed in 603c091

mkarolin

grd++

brave-builds · 2024-06-28T17:48:39Z

A Storybook has been deployed to preview UI for the latest push

brave-builds · 2024-07-02T17:56:45Z

A Storybook has been deployed to preview UI for the latest push

nuo-xu

it was very difficult to save Slippage tolerance (or im not sure its saved or not)
sending and swapping work pretty well.
I'm not sure it's my device. it once became pretty hot after I was trying to make tx on webui.

chromium_src/ios/web/public/webui/url_data_source_ios.h

ios/brave-ios/Sources/BraveWallet/Crypto/CryptoView.swift

ios/brave-ios/Sources/BraveWallet/WebUI/WebUISendView.swift

ios/browser/ui/webui/brave_wallet/brave_wallet_page_ui.mm

brave-builds · 2024-07-09T03:30:26Z

A Storybook has been deployed to preview UI for the latest push

…heme. Build strings using constants.

…s in a shared function using constants for page, panel, android and iOS.

…kChromeUIUntrustedScheme constant, add nullable flags to chrome web view controller ui delegate.

chromium_src/ios/web/webui/DEPS

yrliou

wallet core LGTM (non-iOS code)

components/brave_wallet/browser/brave_wallet_constants.h

components/brave_wallet/browser/brave_wallet_constants.cc

brave-builds · 2024-08-08T17:35:42Z

A Storybook has been deployed to preview UI for the latest push

…namespace closing bracket.

brave-builds · 2024-08-08T20:34:22Z

A Storybook has been deployed to preview UI for the latest push

supermassive · 2024-08-09T04:36:55Z

components/brave_wallet/browser/brave_wallet_constants.cc

+// When isPanel is true, will return:
+//   "img-src 'self' data: chrome://resources chrome://erc-token-images
+//   chrome://image chrome://favicon https://assets.cgproxy.brave.com;"
+const std::string GetWalletImgSrcCSP(bool isPanel) {


nit: is_panel

Also no const in return type, just std::string. Same for GetWalletFrameSrcCSP

supermassive · 2024-08-09T04:37:40Z

components/brave_wallet/browser/brave_wallet_constants.cc

+  frameSrcCSP.append(std::string(" ") +
+                     brave_wallet::mojom::kUntrustedTrezorBridgeURL);
+  frameSrcCSP.append(std::string(" ") +
+                     brave_wallet::mojom::kUntrustedLedgerBridgeURL);
+#endif


nit: ~~brave_wallet::~~

github-actions · 2024-08-09T13:57:57Z

[puLL-Merge] - brave/brave-core@24328

Description

This PR implements WebUI support for the Brave Wallet on iOS, specifically for the Send and Swap functionality. It adds new files, modifies existing ones, and integrates the Brave Wallet UI into the iOS app architecture.

Changes

Changes

browser/ui/webui/brave_wallet/:
- Added new files for handling Brave Wallet WebUI on iOS, including brave_wallet_page_ui.h/mm, nft_ui.h/mm, wallet_common_ui.h/mm, wallet_handler.h/mm, and wallet_page_handler.h/mm.
- These files set up the necessary infrastructure for the Brave Wallet WebUI on iOS.
browser/ui/webui/:
- Modified brave_web_ui_controller_factory.mm to include the new Brave Wallet WebUI controllers.
- Added brave_webui_data_source.h/mm to handle WebUI data sources for Brave on iOS.
components/brave_wallet/browser/brave_wallet_constants.cc/h:
- Added new functions GetWalletFrameSrcCSP() and GetWalletImgSrcCSP() to handle Content Security Policy for the Wallet UI.
components/brave_wallet_ui/:
- Modified several files to support iOS-specific functionality and adjust imports.
ios/brave-ios/Sources/BraveWallet/:
- Added new Swift files for the WebUI implementation of Send and Swap functionality.
ios/browser/:
- Added and modified files to support Brave Wallet features on iOS, including new feature flags.
chromium_src/:
- Added and modified several files to integrate Brave Wallet functionality into the Chromium base on iOS.

Possible Issues

The implementation adds new untrusted schemes and modifies Content Security Policies, which could potentially introduce security risks if not properly configured.
The changes to the build system (GN files) might affect the build process for other platforms or configurations.

Security Hotspots

The addition of chrome-untrusted scheme in ios/chrome/browser/shared/model/browser_state/chrome_browser_state.mm could potentially be a security risk if not properly restricted.
Modifications to Content Security Policies in various files (e.g., brave_wallet_constants.cc, brave_webui_data_source.mm) should be carefully reviewed to ensure they don't introduce vulnerabilities.

…fix in namespace.

brave-builds · 2024-08-09T15:19:34Z

A Storybook has been deployed to preview UI for the latest push

bridiver · 2024-08-14T19:28:25Z

chromium_src/ios/web/webui/url_data_source_ios.mm

+  return "frame-src 'none';";
+}
+
+std::string URLDataSourceIOS::GetContentSecurityPolicy(


it looks like a lot of this stuff is copied from URLDataSource which we should not be doing because it's very fragile.

bridiver · 2024-08-14T19:35:14Z

chromium_src/ios/web/webui/url_data_source_ios.mm

+}  // namespace
+
+namespace web {
+bool URLDataSourceIOS::ShouldAddContentSecurityPolicy() const {


why are we creating our own implementation in the base class instead of URLDataSourceIOSImpl?

bridiver

I have serious concerns with this approach of copying code from content WebUIDataSource to ios. We can discuss options in slack.

bridiver · 2024-08-14T20:03:32Z

components/brave_wallet/browser/brave_wallet_constants.cc

+// On mobile, will return:
+//   "frame-src chrome-untrusted://nft-display/
+//   chrome-untrusted://line-chart-display/ chrome-untrusted://market-display/;"
+std::string GetWalletFrameSrcCSP() {


why are these in constants? There are some other methods already in here that don't make sense in a constants.h file, but I would put these in some kind of helper specific to the relevant webui.

bridiver · 2024-08-14T20:05:04Z

components/brave_wallet/browser/brave_wallet_constants.h

@@ -1635,6 +1635,20 @@ const std::string GetAssetRatioBaseURL();
 const base::flat_map<std::string, std::string>& GetAnkrBlockchains();
 // https://docs.rs/solana-program/1.18.10/src/solana_program/clock.rs.html#129-131
 inline constexpr int kSolanaValidBlockHeightThreshold = 150;
+
+inline constexpr char kCSPFrameSrcName[] = "frame-src";


why are we creating constants in brave wallet for generic CSP directives?

bridiver · 2024-08-14T20:05:39Z

components/brave_wallet/browser/brave_wallet_constants.h

+inline constexpr char kCSPImageSrcName[] = "img-src";
+inline constexpr char kCSPSelf[] = "'self'";
+inline constexpr char kCSPData[] = "data:";
+inline constexpr char kCSPChromeResources[] = "chrome://resources";


same as above, a constant for chrome://resources does not belong in brave_wallet_constants

bridiver · 2024-08-14T21:20:15Z

ios/browser/brave_web_client.mm

 }

 bool BraveWebClient::IsAppSpecificURL(const GURL& url) const {
-  return ChromeWebClient::IsAppSpecificURL(url) || url.SchemeIs(kBraveUIScheme);
+  return ChromeWebClient::IsAppSpecificURL(url) ||
+         url.SchemeIs(kBraveUIScheme) || url.SchemeIs(kChromeUIUntrustedScheme);


doesn't this enable mojo bindings? https://source.chromium.org/chromium/chromium/src/+/main:ios/web/webui/web_ui_messaging_java_script_feature.mm;l=47

StephenHeaps added the needs-security-review label Jun 21, 2024

StephenHeaps self-assigned this Jun 21, 2024

github-actions bot added CI/storybook-url Deploy storybook and provide a unique URL for each build feature/web3/wallet labels Jun 21, 2024

StephenHeaps force-pushed the wallet/ios-swap-send-webui branch from 57c3b89 to ac8e3e5 Compare June 25, 2024 19:36

StephenHeaps force-pushed the wallet/ios-swap-send-webui branch from ac8e3e5 to e797477 Compare June 26, 2024 15:15

StephenHeaps force-pushed the wallet/ios-swap-send-webui branch from e797477 to 863a9f5 Compare June 26, 2024 18:31

StephenHeaps marked this pull request as ready for review June 26, 2024 19:57

StephenHeaps requested review from a team as code owners June 26, 2024 19:57

kylehickinson requested a review from a team June 26, 2024 20:00

josheleonard approved these changes Jun 27, 2024

View reviewed changes

mkarolin approved these changes Jun 28, 2024

View reviewed changes

github-actions bot added the puLL-Merge label Jun 28, 2024

StephenHeaps force-pushed the wallet/ios-swap-send-webui branch from 603c091 to a4a6bc2 Compare July 2, 2024 16:59

StephenHeaps requested a review from nuo-xu July 2, 2024 17:08

nuo-xu reviewed Jul 5, 2024

View reviewed changes

StephenHeaps requested a review from a team as a code owner July 9, 2024 02:49

github-actions bot added the feature/web3/wallet/core label Jul 9, 2024

StephenHeaps requested a review from nuo-xu July 10, 2024 16:44

nuo-xu approved these changes Jul 10, 2024

View reviewed changes

StephenHeaps force-pushed the wallet/ios-swap-send-webui branch from 72dc3fc to fd4114f Compare July 11, 2024 14:37

StephenHeaps added 9 commits August 8, 2024 11:55

review(josheleonard): restore EOF new-lines.

ef92093

review(nuoxu): Rename send/swap feature flag views.

ebd356d

Further remove reliance on isAndroid for iOS WebUI.

1c368f0

review(supermassive): remove unused commented out code.

335a5a7

Add frame-src and img-src content security policy overrides.

b406aa4

review(supermassive): Move ledger/trezor bridge URL to mojom constant.

f4e28b1

review(kdenhartog): Add chromium src override for chrome-untrusted sc…

6c21933

…heme. Build strings using constants.

review(kdenhartog): Use constants for CSP values. Define CSP override…

646331f

…s in a shared function using constants for page, panel, android and iOS.

review(kylehickinson): Unused import, add chromium_src BUILD.gn, use …

ab0bb0b

…kChromeUIUntrustedScheme constant, add nullable flags to chrome web view controller ui delegate.

kylehickinson approved these changes Aug 8, 2024

View reviewed changes

StephenHeaps force-pushed the wallet/ios-swap-send-webui branch from 39a3001 to ab0bb0b Compare August 8, 2024 16:11

kylehickinson reviewed Aug 8, 2024

View reviewed changes

chromium_src/ios/web/webui/DEPS Outdated Show resolved Hide resolved

review(kylehickinson): Update DEPS to remove unneeded dependency.

fb21feb

yrliou approved these changes Aug 8, 2024

View reviewed changes

components/brave_wallet/browser/brave_wallet_constants.h Outdated Show resolved Hide resolved

components/brave_wallet/browser/brave_wallet_constants.cc Show resolved Hide resolved

components/brave_wallet/browser/brave_wallet_constants.cc Outdated Show resolved Hide resolved

review(yrliou): Add comments to wallet CSP functions. Newline before …

8b7b30d

…namespace closing bracket.

supermassive reviewed Aug 9, 2024

View reviewed changes

review(supermassive): is_panel, remove const, remove brave_wallet pre…

bc05824

…fix in namespace.

StephenHeaps force-pushed the wallet/ios-swap-send-webui branch from ca33759 to bc05824 Compare August 9, 2024 14:21

bridiver reviewed Aug 14, 2024

View reviewed changes

bridiver requested changes Aug 14, 2024

View reviewed changes

bridiver reviewed Aug 14, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(wallet): Use Wallet WebUI for Send & Swap on iOS #24328

feat(wallet): Use Wallet WebUI for Send & Swap on iOS #24328

StephenHeaps commented Jun 21, 2024 •

edited

Loading

brave-builds commented Jun 21, 2024

brave-builds commented Jun 25, 2024

brave-builds commented Jun 26, 2024

brave-builds commented Jun 26, 2024

josheleonard left a comment

josheleonard Jun 27, 2024

StephenHeaps Jun 28, 2024

josheleonard Jun 27, 2024

StephenHeaps Jun 28, 2024

mkarolin left a comment

brave-builds commented Jun 28, 2024

brave-builds commented Jul 2, 2024

nuo-xu left a comment

brave-builds commented Jul 9, 2024

yrliou left a comment

brave-builds commented Aug 8, 2024

brave-builds commented Aug 8, 2024

supermassive Aug 9, 2024

supermassive Aug 9, 2024 •

edited

Loading

supermassive Aug 9, 2024

github-actions bot commented Aug 9, 2024

Changes

brave-builds commented Aug 9, 2024

bridiver Aug 14, 2024

bridiver Aug 14, 2024

bridiver left a comment

bridiver Aug 14, 2024

bridiver Aug 14, 2024

bridiver Aug 14, 2024

bridiver Aug 14, 2024

feat(wallet): Use Wallet WebUI for Send & Swap on iOS #24328

Are you sure you want to change the base?

feat(wallet): Use Wallet WebUI for Send & Swap on iOS #24328

Conversation

StephenHeaps commented Jun 21, 2024 • edited Loading

Submitter Checklist:

Reviewer Checklist:

After-merge Checklist:

Test Plan:

brave-builds commented Jun 21, 2024

brave-builds commented Jun 25, 2024

brave-builds commented Jun 26, 2024

brave-builds commented Jun 26, 2024

josheleonard left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkarolin left a comment

Choose a reason for hiding this comment

brave-builds commented Jun 28, 2024

brave-builds commented Jul 2, 2024

nuo-xu left a comment

Choose a reason for hiding this comment

brave-builds commented Jul 9, 2024

yrliou left a comment

Choose a reason for hiding this comment

brave-builds commented Aug 8, 2024

brave-builds commented Aug 8, 2024

Choose a reason for hiding this comment

supermassive Aug 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Aug 9, 2024

Description

Changes

Possible Issues

Security Hotspots

brave-builds commented Aug 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bridiver left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StephenHeaps commented Jun 21, 2024 •

edited

Loading

supermassive Aug 9, 2024 •

edited

Loading