wallet-sdk: drop pre-emptive popup for unauthenticated wallet_connect#37
Merged
Merged
Conversation
The host now renders <LoginView> inline in the iframe for unauthenticated
sessions (email OTP, SMS, passkey) — Privy's auth `allowed_domains`
already covers the wallet host origin, so these flows don't need a
popup. External OAuth providers (Google, Apple, …) still demand a
top-level surface; the Privy SDK handles those by opening its own
provider popup directly from the iframe.
This removes the `TOP_LEVEL_AUTH_METHODS` set + `requiresTopLevelAuth`
gate from `pickInitialMode`. The iframe is now the default surface for
every request unless the caller forces popup mode or the iframe
explicitly escalates via `__internal { type: 'switch', mode: 'popup' }`
(IO-v2-unsupported parents not on the trusted-host list, Safari-only
WebAuthn enrollment).
Updates Wallet.test to reflect the new behaviour: `wallet_connect`
under auto mode no longer pre-opens a popup; explicit `dialog: 'popup'`
override still does.
https://claude.ai/code/session_018kuvZqC6ReXAqjGbJt1yNz
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
pickInitialModecurrently escalates to popup mode whenever the user invokes one ofwallet_connect/eth_requestAccounts/wallet_requestPermissionsagainst a wallet host that reportsauthenticated: false. The original rationale (preserved in the file's header docstring) was that external OAuth providers reject embedded user agents.That gate is too coarse. Email OTP, SMS, and
navigator.credentials.get()-based passkey sign-in all work fine inside the iframe; only OAuth providers genuinely require a top-level surface, and the Privy SDK already pops its own provider window from inside the iframe when the user clicks one of those buttons. Forcing a popup for every unauth auth method just to handle the OAuth subset is a UX tax we don't need to pay.This PR makes the iframe the default surface for every request unless:
dialog: 'popup', or__internal { type: 'switch', mode: 'popup' }(the existing channel — used today for IO-v2-unsupported parents not on the trusted-host list, and reserved for cases like Safari-only WebAuthncreate()that are genuinely top-level-only).The
secure()iframe-eligibility check (HTTPS, IO-v2 supported OR parent on trusted-hosts) still runs before the first request and still transparently downgrades to popup when those gates fail. So the existing safety properties for clickjacking-vulnerable browsers stay intact.Changes
packages/wallet-sdk/src/core/Wallet.tsTOP_LEVEL_AUTH_METHODSset +requiresTopLevelAuthhelperpickInitialModesimplified to: if the caller forced popup, return popup; otherwise iframepackages/wallet-sdk/test/src/Wallet.test.tswindow.openis not calleddialog: 'popup'override still pops a window foreth_requestAccountsTest plan
pnpm test(17/17 pass)pnpm typecheckpnpm buildwallet_connectfrom the SDK against a host serving an inline<LoginView>— auth happens inline; popup only opens when the user picks an OAuth providerdialog: 'popup'override still opens a top-level popup for bothwallet_connectandeth_requestAccountssecure()Notes for reviewers
__internal { type: 'switch' }escalation contract, the popup factory, the messenger, or the security model.secure()continues to be the authoritative iframe-eligibility check for clickjacking defence; this PR doesn't touch it.wallet_connectunauthenticated) will need to render their own login surface in the iframe — or passdialog: 'popup'throughcreateWallet.https://claude.ai/code/session_018kuvZqC6ReXAqjGbJt1yNz
Generated by Claude Code