Skip to content

Fix/explicit rpid config - #585

Open
Davoski1 wants to merge 2 commits into
Miracle656:mainfrom
Davoski1:fix/explicit-rpid-config
Open

Fix/explicit rpid config#585
Davoski1 wants to merge 2 commits into
Miracle656:mainfrom
Davoski1:fix/explicit-rpid-config

Conversation

@Davoski1

Copy link
Copy Markdown

Summary

Supply the Relying Party ID (rpId) and origin explicitly in the wallet configuration so native builds do not fall back to localhost.

Changes

  • Update frontend/wallet/lib/network.ts
    • Add rpId and origin fields to walletConfig, read from NEXT_PUBLIC_RP_ID and NEXT_PUBLIC_ORIGIN.
  • Update examples/nextjs/src/lib/network.ts
    • Same environment-backed rpId / origin additions for the example app.

Why

Native apps (Expo / React Native) lack window.location.hostname, causing the SDK to default to localhost. That breaks passkey binding on real devices because the relying-party id must match the production domain. Reading rpId and origin from environment variables allows CI and device builds to set the correct production values without changing source.

Notes for maintainers / operators

  • Set NEXT_PUBLIC_RP_ID (e.g. veil.app) and NEXT_PUBLIC_ORIGIN (e.g. https://veil.app) in your environment/CI before building production artifacts.
  • Existing behavior is preserved when env vars are unset: the SDK will continue to resolve rpId/origin from window.location or its previous fallbacks.

Testing / Verification

  1. Build the app with NEXT_PUBLIC_RP_ID=veil.app and NEXT_PUBLIC_ORIGIN=https://veil.app set and verify walletConfig passed to useInvisibleWallet() contains those values.
  2. On a device, run register/authenticate flows and confirm passkeys are associated with the intended domain (not localhost).

Closes #442

@Davoski1
Davoski1 requested a review from Miracle656 as a code owner July 29, 2026 16:49
@vercel

vercel Bot commented Jul 29, 2026

Copy link
Copy Markdown

@rhoggs-bot-test-account is attempting to deploy a commit to the miracle656's projects Team on Vercel.

A member of the Team first needs to authorize it.

@Miracle656

Copy link
Copy Markdown
Owner

Needs a rebase onto main. Two things drifted: the mobile config is now app.config.ts (not app.json/app.config.js), and both frontend/mobile/lib/network.ts and frontend/wallet/lib/network.ts have changed on main. Please rebase and move the explicit rpId config into the current app.config.ts / network files, then I'll take another look.

@Miracle656

Copy link
Copy Markdown
Owner

Apologies — this sat unreviewed for a month while main moved underneath it, and that is on us, not you.

What happened: PR #508 (3a9e337, "register veil:// scheme and universal/app links") landed after you opened this. It did the same job from the other direction:

  • Added associatedDomains to frontend/mobile/app.config.ts (line 56)
  • Added Android intentFilters with the assetlinks reference (lines 70–80)
  • Set bundleIdentifier, package and scheme
  • Deleted frontend/mobile/app.json

And the domain-association files are now hosted for real, not as examples:

frontend/wallet/public/.well-known/assetlinks.json
frontend/wallet/public/.well-known/apple-app-site-association

So the gap you were closing is closed. I have checked main rather than assuming.

One part of this is NOT superseded, and it is worth salvaging.

Your change to frontend/wallet/lib/network.ts:

rpId:   process.env.NEXT_PUBLIC_RP_ID?.trim() || undefined,
origin: process.env.NEXT_PUBLIC_ORIGIN?.trim() || undefined,

main has no equivalent — the only rpId handling is in lib/backup.ts, which falls back to window.location.hostname each time. So the config fields genuinely do not exist yet.

Two thoughts if you want to re-open it as a small focused PR:

  • The stated rationale — letting native builds supply the value — does not quite fit, since frontend/wallet is the web app and frontend/mobile has its own Expo config. But there is a better one: the wallet is served from app.useveilapp.xyz and from Vercel preview URLs. A passkey registered under one hostname will not resolve under the other, because rpId is the hostname. An explicit, pinned rpId is exactly how you stop that.
  • It is also a footgun worth documenting: setting NEXT_PUBLIC_RP_ID to a domain the page is not served from makes WebAuthn fail outright with a SecurityError. Defaulting to undefined, as you did, is the right call — the risk only appears if someone sets it wrong.

So: the three Expo/well-known files here are superseded, but those two lines are not. A PR with just them, plus a note in the env documentation about the preview-URL case, would land.

Not closing this. Sorry for the month.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

14. Global error boundary + Sentry

2 participants