diff --git a/apps/integration-tests/src/end-to-end.integration.test.ts b/apps/integration-tests/src/end-to-end.integration.test.ts index a7c59de..78ce9f3 100644 --- a/apps/integration-tests/src/end-to-end.integration.test.ts +++ b/apps/integration-tests/src/end-to-end.integration.test.ts @@ -77,6 +77,32 @@ async function connectClients(dappClient: DappClient, walletClient: WalletClient await Promise.all([dappConnectPromise, walletConnectPromise]); } +// Helper for strict untrusted flow (dapp advertises otpDisplayGrant capability). +async function connectClientsStrict(dappClient: DappClient, walletClient: WalletClient) { + const sessionRequestPromise = new Promise((resolve) => { + dappClient.on("session_request", resolve); + }); + const dappConnectPromise = dappClient.connect({ mode: "untrusted", requireOtpDisplayGrant: true }); + + const sessionRequest = await sessionRequestPromise; + t.expect(sessionRequest.capabilities?.otpDisplayGrant).toBe(true); + + const walletConnectPromise = walletClient.connect({ sessionRequest }); + + const otpPromise = new Promise((resolve) => { + walletClient.on("display_otp", (otp) => resolve(otp)); + }); + const otp = await otpPromise; + + const otpRequiredPromise = new Promise((resolve) => { + dappClient.on("otp_required", resolve); + }); + const otpPayload = await otpRequiredPromise; + await otpPayload.submit(otp); + + await Promise.all([dappConnectPromise, walletConnectPromise]); +} + // Helper to assert that a promise does NOT resolve within a given time async function assertPromiseNotResolve(promise: Promise, timeout: number, message: string) { const timeoutPromise = new Promise((_, reject) => setTimeout(() => reject(new Error(message)), timeout)); @@ -241,6 +267,113 @@ t.describe("E2E Integration Test", () => { await resumedDappClient.sendRequest(testPayload); await t.expect(messagePromise).resolves.toEqual(testPayload); }); + + t.describe("otp-display-grant strict flow", () => { + t.test("should complete strict untrusted connection and allow bidirectional messaging", async () => { + await connectClientsStrict(dappClient, walletClient); + + t.expect((dappClient as any).state).toBe("CONNECTED"); + t.expect((walletClient as any).state).toBe("CONNECTED"); + + const requestPayload = { method: "eth_accounts_strict" }; + const messageFromDappPromise = new Promise((resolve) => walletClient.on("message", resolve)); + await dappClient.sendRequest(requestPayload); + await t.expect(messageFromDappPromise).resolves.toEqual(requestPayload); + + const responsePayload = { result: ["0x789..."] }; + const messageFromWalletPromise = new Promise((resolve) => dappClient.on("message", resolve)); + await walletClient.sendResponse(responsePayload); + await t.expect(messageFromWalletPromise).resolves.toEqual(responsePayload); + }); + + t.test("should reject strict dapp when wallet simulates legacy offer (no otpDisplayGrantRequired)", async () => { + const sessionRequestPromise = new Promise((resolve) => { + dappClient.on("session_request", resolve); + }); + const dappConnectPromise = dappClient.connect({ mode: "untrusted", requireOtpDisplayGrant: true }); + const sessionRequest = await sessionRequestPromise; + + // Simulate an older wallet that ignores capabilities.otpDisplayGrant on the session request. + const legacyWalletRequest = { ...sessionRequest, capabilities: undefined }; + const walletConnectPromise = walletClient.connect({ sessionRequest: legacyWalletRequest }); + void walletConnectPromise.catch(() => {}); + + const displayOtpPromise = new Promise((resolve) => { + walletClient.once("display_otp", () => resolve()); + }); + + await t.expect(dappConnectPromise).rejects.toMatchObject({ code: ErrorCode.OTP_DISPLAY_GRANT_REQUIRED }); + await t.expect(displayOtpPromise).resolves.toBeUndefined(); + await assertPromiseNotResolve(walletConnectPromise, 1000, "Wallet should not complete strict-incompatible connection"); + }); + + t.test("should keep legacy untrusted flow when dapp does not require otp display grant", async () => { + let displayOtpFired = false; + let displayOtpBeforeOffer = false; + + const sessionRequestPromise = new Promise((resolve) => { + dappClient.on("session_request", resolve); + }); + const dappConnectPromise = dappClient.connect({ mode: "untrusted" }); + const sessionRequest = await sessionRequestPromise; + t.expect(sessionRequest.capabilities?.otpDisplayGrant).toBeUndefined(); + + (dappClient as any).on("handshake_offer_received", () => { + displayOtpBeforeOffer = displayOtpFired; + }); + walletClient.on("display_otp", () => { + displayOtpFired = true; + }); + + const walletConnectPromise = walletClient.connect({ sessionRequest }); + + const otpPromise = new Promise((resolve) => { + walletClient.on("display_otp", (otp) => resolve(otp)); + }); + const otp = await otpPromise; + const otpPayload = await new Promise((resolve) => { + dappClient.on("otp_required", resolve); + }); + await otpPayload.submit(otp); + await Promise.all([dappConnectPromise, walletConnectPromise]); + + t.expect(displayOtpFired).toBe(true); + t.expect(displayOtpBeforeOffer).toBe(true); + }); + + t.test("should defer wallet display_otp until after dapp receives handshake offer in strict mode", async () => { + let dappReceivedOffer = false; + let displayOtpFired = false; + + const sessionRequestPromise = new Promise((resolve) => { + dappClient.on("session_request", resolve); + }); + const dappConnectPromise = dappClient.connect({ mode: "untrusted", requireOtpDisplayGrant: true }); + const sessionRequest = await sessionRequestPromise; + + (dappClient as any).on("handshake_offer_received", () => { + dappReceivedOffer = true; + }); + walletClient.on("display_otp", () => { + t.expect(dappReceivedOffer, "display_otp must not fire before dapp receives handshake offer").toBe(true); + displayOtpFired = true; + }); + + const walletConnectPromise = walletClient.connect({ sessionRequest }); + + const otpPromise = new Promise((resolve) => { + walletClient.on("display_otp", (otp) => resolve(otp)); + }); + const otp = await otpPromise; + t.expect(displayOtpFired).toBe(true); + + const otpPayload = await new Promise((resolve) => { + dappClient.on("otp_required", resolve); + }); + await otpPayload.submit(otp); + await Promise.all([dappConnectPromise, walletConnectPromise]); + }); + }); }); t.describe("E2E Integration Test via Proxy", () => { diff --git a/docs/otp-display-grant-implementation-plan.md b/docs/otp-display-grant-implementation-plan.md new file mode 100644 index 0000000..741f17c --- /dev/null +++ b/docs/otp-display-grant-implementation-plan.md @@ -0,0 +1,362 @@ +# OTP Display Grant — Implementation Plan + +Companion to [otp-display-grant-plan.md](./otp-display-grant-plan.md). This document breaks the feature into small, reviewable phases. Each phase should land as an independent PR where possible. + +## Progress Summary + +| Phase | Status | Notes | +| --- | --- | --- | +| 0 | Skipped (informal) | Unit tests verified green during Phases 1–3 | +| 1 | **Done** | Core protocol types | +| 2 | **Done** | Dapp `requireOtpDisplayGrant` + session request capability | +| 3 | **Done** | Wallet deferred `display_otp` + grant routing | +| 4 | **Done** | Dapp grant send & strict validation | +| 5 | **Done** | Dedicated error codes (`OTP_DISPLAY_GRANT_REQUIRED`, `OTP_DISPLAY_GRANT_TIMEOUT`) | +| 6 | **Partial** | Dapp + wallet strict unit tests done; wallet full strict happy path with grant event pending | +| 7 | **Not started** | E2E + compatibility matrix | +| 8 | **Not started** | Demos & docs | +| 9 | **Not started** | Final verification | + +**Current blocker:** None for unit-level strict flow. **Next:** Phase 8 demos & documentation. + +## Scope + +Add an opt-in strict untrusted flow: + +```ts +await dappClient.connect({ + mode: "untrusted", + requireOtpDisplayGrant: true, +}); +``` + +Default behavior (`connect({ mode: "untrusted" })`) must remain unchanged. + +## Packages Touched + +| Package | Role | +| --- | --- | +| `packages/core` | Protocol types, error codes, exports | +| `packages/dapp-client` | `requireOtpDisplayGrant`, grant send, strict validation | +| `packages/wallet-client` | Deferred `display_otp`, grant receive | +| `apps/integration-tests` | E2E + compatibility matrix | +| `apps/web-demo` | Optional strict-mode toggle | +| `docs/` | Connection flow doc update | + +## Phase Overview + +| Phase | Goal | Depends on | Status | +| --- | --- | --- | --- | +| 0 | Baseline & guardrails | — | Skipped | +| 1 | Core protocol types | 0 | Done | +| 2 | Dapp connect options & session request | 1 | Done | +| 3 | Wallet deferred OTP display | 1 | Done | +| 4 | Dapp grant send & strict validation | 2, 3 | Done | +| 5 | Error codes & timeouts | 4 | Done | +| 6 | Unit tests | 2–5 | Partial | +| 7 | Integration tests | 6 | **Next** | +| 8 | Demos & docs | 7 | Not started | + +--- + +## Phase 0 — Baseline & Guardrails + +Confirm current behavior before changing it. + +- [ ] **0.1** Run existing untrusted unit tests in both clients and note green baseline. + - `packages/dapp-client/src/handlers/untrusted-connection-handler.test.ts` + - `packages/wallet-client/src/handlers/untrusted-connection-handler.test.ts` +- [ ] **0.2** Run integration tests (`apps/integration-tests`) to confirm E2E untrusted flow still passes. +- [ ] **0.3** Document the current step order in both handlers (for diff review): + - Wallet: generate OTP → `display_otp` → `handshake-offer` → wait ack + - Dapp: wait offer → OTP input → create session → subscribe secure channel → `handshake-ack` + +**Exit criteria:** All existing untrusted tests pass; team agrees on backward-compat requirement (no silent fallback). + +--- + +## Phase 1 — Core Protocol Types + +Add types only. No behavior change yet. + +**File:** `packages/core/src/domain/session-request.ts` + +- [x] **1.1** Extend `SessionRequest`: + +```ts +capabilities?: { + otpDisplayGrant?: true; +}; +``` + +**File:** `packages/core/src/domain/protocol-message.ts` + +- [x] **1.2** Add `otpDisplayGrantRequired?: true` to `HandshakeOfferPayload`. +- [x] **1.3** Add new message type: + +```ts +export type OtpDisplayGrant = { + type: "otp-display-grant"; +}; +``` + +- [x] **1.4** Extend `ProtocolMessage` union to include `OtpDisplayGrant`. + +**File:** `packages/core/src/index.ts` + +- [x] **1.5** Re-export `OtpDisplayGrant` (and any new error codes from Phase 5 if added in same PR). + +**Exit criteria:** Packages compile; no runtime behavior change; existing tests still pass. + +--- + +## Phase 2 — Dapp Connect Options & Session Request + +Wire the dapp opt-in flag into the QR payload. Still uses the legacy flow internally. + +**File:** `packages/dapp-client/src/client.ts` + +- [x] **2.1** Add `requireOtpDisplayGrant?: boolean` to `DappConnectOptions`. +- [x] **2.2** In `_createPendingSessionAndRequest()`, when `requireOtpDisplayGrant === true`, set: + +```ts +capabilities: { otpDisplayGrant: true } +``` + +on the emitted `SessionRequest`. +- [x] **2.3** Pass strict-mode intent via `SessionRequest.capabilities` (handler reads `request`, not a duplicate context flag). + +**Tests (minimal for this phase)** + +- [x] **2.5** Unit test: `connect({ requireOtpDisplayGrant: true })` emits `session_request` with `capabilities.otpDisplayGrant === true`. Covered in `client.integration.test.ts`. +- [x] **2.6** Unit test: default `connect()` omits `capabilities` (or leaves it undefined). Covered in `client.integration.test.ts`. + +**Exit criteria:** QR payload advertises capability; legacy flow unchanged. + +--- + +## Phase 3 — Wallet Deferred OTP Display + +Wallet reacts to dapp capability and waits for grant before showing OTP. + +**File:** `packages/wallet-client/src/client.ts` + +- [x] **3.1** In `handleMessage()`, during `CONNECTING`, route `otp-display-grant` to a new internal event (e.g. `otp_display_grant_received`). +- [x] **3.2** Ignore or no-op `otp-display-grant` when not in `CONNECTING` (defensive). + +**File:** `packages/wallet-client/src/domain/connection-handler-context.ts` + +- [x] **3.3** Add `once`/`off` support for `otp_display_grant_received` on the context interface. + +**File:** `packages/wallet-client/src/handlers/untrusted-connection-handler.ts` + +- [x] **3.4** After `_generateOtpWithDeadline()`, branch on `request.capabilities?.otpDisplayGrant`: + - **Legacy path:** keep current behavior — emit `display_otp` immediately. + - **Strict path:** do **not** emit `display_otp` yet. +- [x] **3.5** In `_sendHandshakeOffer()`, when strict, include `otpDisplayGrantRequired: true` on the payload. +- [x] **3.6** Add `_waitForOtpDisplayGrant(deadline)` — mirrors `_waitForHandshakeAck` pattern: + - Listen for `otp_display_grant_received`. + - Reject on deadline with a clear error (temporary `REQUEST_EXPIRED` or new code from Phase 5). +- [x] **3.7** On grant received, emit `display_otp` with the already-generated OTP and deadline. +- [x] **3.8** Reorder `execute()` for strict path: + +``` +generate OTP +→ send handshake-offer (otpDisplayGrantRequired: true) +→ wait for otp-display-grant +→ emit display_otp +→ wait for handshake-ack +→ finalize +``` + +**Tests** + +- [x] **3.9** Unit test: legacy request (no capability) — `display_otp` fires before offer is sent (unchanged). +- [x] **3.10** Unit test: strict request — `display_otp` does **not** fire until grant event. +- [x] **3.11** Unit test: strict request — offer payload includes `otpDisplayGrantRequired: true`. +- [x] **3.12** Unit test: strict request — grant timeout rejects connection. + +**Exit criteria:** Wallet strict path works in isolation (grant event can be simulated in unit tests); legacy path unchanged. + +--- + +## Phase 4 — Dapp Grant Send & Strict Validation + +Dapp sends grant on the **handshake channel** before OTP entry. Session creation stays after OTP (same as legacy). + +**File:** `packages/dapp-client/src/handlers/untrusted-connection-handler.ts` + +- [x] **4.1** After `_waitForHandshakeOffer()`, validate strict mode: + - If `requireOtpDisplayGrant` and offer lacks `otpDisplayGrantRequired`, reject with a dedicated error (no silent fallback). +- [x] **4.2** Add `_applyWalletPublicKeyFromOffer()` — set `theirPublicKey` from offer for encrypting handshake messages (no session channel yet). +- [x] **4.3** No early subscribe to session channel — dapp already subscribed to handshake channel. +- [x] **4.4** Add `_sendOtpDisplayGrant(handshakeChannel)`: + - `await sendMessage(handshakeChannel, { type: "otp-display-grant" })`. + - Only when strict mode is active. +- [x] **4.5** Reorder `execute()` for strict path: + +``` +wait offer +→ validate strict flags +→ set wallet public key from offer +→ send otp-display-grant (handshake channel) +→ handle OTP input +→ create final session + subscribe secure channel +→ handshake-ack +→ finalize +``` + +- [x] **4.6** For legacy path, keep existing order (OTP before session channel setup). + +**Tests** + +- [x] **4.7** Unit test: strict dapp + strict offer — grant is sent before `otp_required` is emitted. +- [x] **4.8** Unit test: strict dapp + offer without `otpDisplayGrantRequired` — rejects (compatibility matrix row: new dapp + old wallet). +- [x] **4.9** Unit test: legacy dapp + strict offer from wallet — N/A in practice (wallet only sets flag when dapp advertises capability); legacy path test covers default behavior. +- [x] **4.10** Unit test: legacy dapp + legacy offer — unchanged flow. + +**Exit criteria:** Full strict handshake works in unit tests with mocked transport/events. + +--- + +## Phase 5 — Error Codes & Timeouts + +**File:** `packages/core/src/domain/errors.ts` + +- [x] **5.1** Add error codes (names tentative, align with existing style): + - `OTP_DISPLAY_GRANT_REQUIRED` — strict dapp received offer without `otpDisplayGrantRequired`. + - `OTP_DISPLAY_GRANT_TIMEOUT` — wallet did not receive grant in time. +- [x] **5.2** Use new codes in wallet `_waitForOtpDisplayGrant()` and dapp strict validation. +- [x] **5.3** Ensure dapp strict-mode failure surfaces via `error` event / rejected `connect()` promise with actionable message. + +**Edge cases to handle** + +- [x] **5.4** User cancels OTP after grant was sent — wallet should reject/tear down (existing cancel path). +- [x] **5.5** Request expires while waiting for grant — both sides clean up (existing timeout machinery). +- [x] **5.6** Multiple `handshake-offer` messages (front-run scenario) — dapp accepts first offer only (`once` listener); acceptable DoS per spec. + +**Exit criteria:** All failure paths have explicit error codes; no silent downgrade. + +--- + +## Phase 6 — Unit Test Coverage + +Consolidate and fill gaps across both clients. + +**Dapp handler tests** (`packages/dapp-client/src/handlers/untrusted-connection-handler.test.ts`) + +- [x] **6.1** Full strict flow happy path (offer → grant → OTP → ack → connected). +- [x] **6.2** Strict rejection when offer missing flag. +- [x] **6.3** OTP incorrect / max attempts / expired — still work in strict mode. +- [x] **6.4** Offer timeout — still works (existing test). + +**Wallet handler tests** (`packages/wallet-client/src/handlers/untrusted-connection-handler.test.ts`) + +- [x] **6.5** Full strict flow happy path. +- [x] **6.6** Grant timeout. +- [x] **6.7** Legacy flow regression suite (no capability in request). + +**Client-level tests** + +- [x] **6.8** `packages/dapp-client/src/client.integration.test.ts` — session request includes capability when flag set. +- [x] **6.9** `packages/wallet-client/src/client.integration.test.ts` — `handleMessage` routes `otp-display-grant` correctly. + +**Exit criteria:** Unit tests cover compatibility matrix rows that do not need a live relay. + +--- + +## Phase 7 — Integration Tests & Compatibility Matrix + +**File:** `apps/integration-tests/src/end-to-end.integration.test.ts` + +- [x] **7.1** Add helper `connectClientsStrict()` mirroring existing `connectClients()` but with `requireOtpDisplayGrant: true`. +- [x] **7.2** E2E: strict dapp + strict wallet — full connect + bidirectional messaging. +- [x] **7.3** E2E: strict dapp + legacy wallet simulation — expect clean failure (`OTP_DISPLAY_GRANT_REQUIRED` or timeout waiting for grant on wallet side). +- [x] **7.4** E2E: legacy dapp + strict-capable wallet — legacy flow still works. +- [x] **7.5** Assert timing: in strict E2E, `display_otp` on wallet fires **after** dapp receives offer (not before). + +**Compatibility matrix verification** + +| Pair | Test | +| --- | --- | +| Old dapp + old wallet | 7.4 (legacy path) | +| Old dapp + new wallet | 7.4 | +| New dapp without flag + old wallet | 7.4 | +| New dapp with flag + old wallet | 7.3 | +| New dapp with flag + new wallet | 7.2 | + +**Exit criteria:** All five matrix rows covered by automated tests. + +--- + +## Phase 8 — Demos & Documentation + +**File:** `apps/web-demo/src/components/UntrustedDemo.tsx` + +- [ ] **8.1** Add UI toggle (or query param) for `requireOtpDisplayGrant`. +- [ ] **8.2** Pass flag through to `dappClient.connect()`. +- [ ] **8.3** Show user-facing error when strict mode fails (old wallet). + +**File:** `docs/02-connection-flow.md` + +- [ ] **8.4** Add subsection for strict untrusted flow with updated sequence diagram. +- [ ] **8.5** Document `requireOtpDisplayGrant` and `capabilities.otpDisplayGrant`. +- [ ] **8.6** Note that `otp-display-grant` is distinct from `handshake-ack`. + +**RN demo** (`apps/rn-demo`) + +- [ ] **8.7** No protocol changes expected; verify OTP modal still appears at the correct time in strict E2E manual test. + +**Exit criteria:** Docs and demo reflect new flow; manual smoke test passes. + +--- + +## Phase 9 — Final Verification + +Follow [`.cursor/rules/verify.mdc`](../.cursor/rules/verify.mdc). + +- [ ] **9.1** Run full test suite (unit + integration). +- [ ] **9.2** Run typecheck / lint across affected packages. +- [ ] **9.3** Manual smoke test: web-demo strict mode with two browser tabs (dapp + wallet simulation if available). +- [ ] **9.4** Review PR sequence: types → wallet → dapp → tests → docs (or types → dapp flag → wallet → dapp grant → tests → docs). + +**Exit criteria:** Feature complete per [otp-display-grant-plan.md](./otp-display-grant-plan.md). + +--- + +## Suggested PR Sequence + +Smaller PRs are easier to review. Recommended order: + +1. **PR 1 — Types** (Phase 1) +2. **PR 2 — Dapp flag & session request** (Phase 2) +3. **PR 3 — Wallet deferred display** (Phase 3 + wallet unit tests) +4. **PR 4 — Dapp grant & strict validation** (Phase 4 + Phase 5 + dapp unit tests) +5. **PR 5 — Integration tests & demos** (Phases 7–8) + +Phases 0 and 9 are checklist steps, not standalone PRs. + +--- + +## Key Code Touchpoints (Quick Reference) + +| File | Change | +| --- | --- | +| `packages/core/src/domain/session-request.ts` | `capabilities.otpDisplayGrant` | +| `packages/core/src/domain/protocol-message.ts` | `OtpDisplayGrant`, `otpDisplayGrantRequired` | +| `packages/core/src/domain/errors.ts` | New error codes | +| `packages/dapp-client/src/client.ts` | `requireOtpDisplayGrant`, session request | +| `packages/dapp-client/src/handlers/untrusted-connection-handler.ts` | Reordered strict flow, grant send | +| `packages/wallet-client/src/client.ts` | Route `otp-display-grant` | +| `packages/wallet-client/src/handlers/untrusted-connection-handler.ts` | Deferred `display_otp`, grant wait | + +--- + +## Open Questions (Resolve Before Phase 4) + +1. **Multiple offers:** If a front-running attacker sends an offer first, strict dapp accepts it, sends grant to attacker's channel, and real wallet never gets a grant. Is that acceptable DoS (per spec), or should dapp wait/retry for a wallet offer with known characteristics? + - **Resolved:** Accept DoS — intended threat model trade-off. +2. **Grant timeout vs OTP deadline:** Should grant timeout share the 60s OTP window or use a separate shorter window? + - **Resolved:** Reuse `deadline` from the offer (wallet already does this). +3. **Encrypted grant on handshake channel:** Grant is encrypted to the accepted offer's wallet public key via `_applyWalletPublicKeyFromOffer()` before `sendMessage` on the handshake channel. + - **Pending:** Verify in Phase 4 unit test with mocked crypto path. diff --git a/docs/otp-display-grant-plan.md b/docs/otp-display-grant-plan.md new file mode 100644 index 0000000..c8c0631 --- /dev/null +++ b/docs/otp-display-grant-plan.md @@ -0,0 +1,136 @@ +# OTP Display Grant Plan + +## Problem + +In the current untrusted connection flow, the wallet generates and displays the OTP before the dapp has accepted any specific handshake offer. + +A same-room attacker can scan the dapp QR code, front-run their own `handshake-offer`, and, if they learn or control the OTP the user enters, cause the dapp to establish a session with the attacker's public key and channel. + +The risk is not front-running alone. Front-running becomes a session hijack when the attacker can also cause the user to enter the OTP that matches the attacker-controlled offer. In a same-room scenario, exposing the OTP too early makes that realistic. + +## Goal + +Add an optional strict untrusted flow where the wallet client does not display the OTP until the dapp client explicitly grants OTP display for the accepted handshake offer. + +This turns attacker front-running into timeout or denial of service: + +1. Attacker sends the first offer. +2. Dapp accepts the attacker offer and sends `otp-display-grant` on the handshake channel, encrypted to the attacker's offer public key. +3. Real MetaMask Mobile never receives a grant it can decrypt. +4. Real MetaMask Mobile never displays the OTP. +5. User has no legitimate OTP to enter. +6. Connection fails instead of binding to the attacker. + +## Compatibility Model + +Add this as an opt-in strict mode. + +```ts +await dappClient.connect({ + mode: "untrusted", + requireOtpDisplayGrant: true, +}); +``` + +The default remains unchanged. + +```ts +await dappClient.connect({ mode: "untrusted" }); +``` + +Compatibility matrix: + +| Pair | Result | +| --- | --- | +| Old dapp + old wallet | Current flow works | +| Old dapp + new wallet | Current flow works | +| New dapp without `requireOtpDisplayGrant` + old wallet | Current flow works | +| New dapp with `requireOtpDisplayGrant: true` + old wallet | Fails cleanly | +| New dapp with `requireOtpDisplayGrant: true` + new wallet | New safer flow | + +Strict mode must not silently fall back. Otherwise an attacker can downgrade the flow by omitting the new flag in their own offer. + +## Protocol Additions + +Extend `SessionRequest`: + +```ts +type SessionRequest = { + // existing fields... + capabilities?: { + otpDisplayGrant?: true; + }; +}; +``` + +Extend `HandshakeOfferPayload`: + +```ts +type HandshakeOfferPayload = { + // existing fields... + otpDisplayGrantRequired?: true; +}; +``` + +Add a protocol message: + +```ts +type OtpDisplayGrant = { + type: "otp-display-grant"; +}; +``` + +Do not reuse `handshake-ack`. `handshake-ack` should continue to mean "OTP verified; finalize connection." + +## New Strict Flow + +Handshake channel messages: `handshake-offer`, `otp-display-grant` +Session channel messages (after OTP): `handshake-ack`, then application traffic + +1. Dapp creates `SessionRequest` with `capabilities.otpDisplayGrant = true`. +2. Wallet scans the request. +3. Wallet generates OTP and deadline but does not emit `display_otp`. +4. Wallet sends `handshake-offer` with `otpDisplayGrantRequired: true` on the handshake channel. +5. Dapp receives the offer. +6. If `requireOtpDisplayGrant` is true and the offer lacks `otpDisplayGrantRequired`, dapp rejects. +7. Dapp sets the wallet public key from the offer on the pending session (for encryption only). +8. Dapp sends encrypted `otp-display-grant` on the **handshake channel**, encrypted to the accepted offer's wallet public key. +9. Wallet receives the grant and only then emits `display_otp`. +10. User enters OTP into the dapp. +11. Dapp verifies OTP. +12. Dapp creates the secure session from the offer and sends `handshake-ack` on the session channel. +13. Both clients persist and finalize the session. + +```mermaid +sequenceDiagram + participant U as User + participant D as Dapp Client + participant R as Relay + participant W as Wallet Client + + U->>D: Start untrusted connect + D->>D: Create SessionRequest
requireOtpDisplayGrant=true + D-->>U: Show QR code + U->>W: Scan QR code + W->>W: Generate OTP
do not display yet + W->>R: handshake-offer (handshake channel) + R->>D: handshake-offer + D->>D: Validate strict offer support + D->>R: otp-display-grant (handshake channel) + R->>W: otp-display-grant + W-->>U: Display OTP + U->>D: Enter OTP + D->>D: Verify OTP + D->>R: handshake-ack (session channel) + R->>W: handshake-ack + D->>D: Persist session + W->>W: Persist session +``` + +## Security Note + +This does not prove wallet identity cryptographically. It prevents the practical same-room OTP-copy hijack by ensuring OTP is only displayed by the wallet whose offer the dapp accepted. + +The grant is encrypted to the accepted offer's wallet public key. Other wallets subscribed to the same handshake channel cannot decrypt it. + +A front-running attacker can still cause denial of service, but should not be able to make official MetaMask Mobile reveal an OTP for a session the dapp did not accept. diff --git a/packages/core/src/domain/errors.ts b/packages/core/src/domain/errors.ts index a308fbe..8c8ceb8 100644 --- a/packages/core/src/domain/errors.ts +++ b/packages/core/src/domain/errors.ts @@ -24,6 +24,8 @@ export enum ErrorCode { OTP_INCORRECT = "OTP_INCORRECT", OTP_MAX_ATTEMPTS_REACHED = "OTP_MAX_ATTEMPTS_REACHED", OTP_ENTRY_TIMEOUT = "OTP_ENTRY_TIMEOUT", + OTP_DISPLAY_GRANT_REQUIRED = "OTP_DISPLAY_GRANT_REQUIRED", + OTP_DISPLAY_GRANT_TIMEOUT = "OTP_DISPLAY_GRANT_TIMEOUT", // Generic fallback UNKNOWN = "UNKNOWN", diff --git a/packages/core/src/domain/protocol-message.ts b/packages/core/src/domain/protocol-message.ts index cbf83e1..e5391d8 100644 --- a/packages/core/src/domain/protocol-message.ts +++ b/packages/core/src/domain/protocol-message.ts @@ -3,6 +3,8 @@ export type HandshakeOfferPayload = { channelId: string; otp?: string; deadline?: number; + /** When true, the wallet requires an `otp-display-grant` before displaying the OTP. */ + otpDisplayGrantRequired?: true; }; export type HandshakeOffer = { @@ -14,6 +16,10 @@ export type HandshakeAck = { type: "handshake-ack"; }; +export type OtpDisplayGrant = { + type: "otp-display-grant"; +}; + export type Message = { type: "message"; payload: unknown; @@ -21,6 +27,6 @@ export type Message = { /** * A protocol message is a message that is sent between the dapp and the wallet. - * It can be a handshake offer, a handshake ack, or a message. + * It can be a handshake offer, a handshake ack, an OTP display grant, or a message. */ -export type ProtocolMessage = HandshakeOffer | HandshakeAck | Message; +export type ProtocolMessage = HandshakeOffer | HandshakeAck | OtpDisplayGrant | Message; diff --git a/packages/core/src/domain/session-request.ts b/packages/core/src/domain/session-request.ts index 0e1cfd6..4d22051 100644 --- a/packages/core/src/domain/session-request.ts +++ b/packages/core/src/domain/session-request.ts @@ -18,4 +18,12 @@ export type SessionRequest = { * on mobile deep linking. */ initialMessage?: Message; + /** + * Optional capabilities advertised by the dApp in the session request. + * Used for opt-in protocol extensions during the untrusted connection flow. + */ + capabilities?: { + /** When true, the wallet may defer OTP display until the dApp sends an `otp-display-grant`. */ + otpDisplayGrant?: true; + }; }; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 9a550b3..fa8a99c 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -5,7 +5,7 @@ export { CryptoError, ErrorCode, ProtocolError, SessionError, TransportError } f export type { IKeyManager } from "./domain/key-manager"; export type { KeyPair } from "./domain/key-pair"; export type { IKVStore } from "./domain/kv-store"; -export type { HandshakeAck, HandshakeOffer, HandshakeOfferPayload, Message, ProtocolMessage } from "./domain/protocol-message"; +export type { HandshakeAck, HandshakeOffer, HandshakeOfferPayload, Message, OtpDisplayGrant, ProtocolMessage } from "./domain/protocol-message"; export type { Session } from "./domain/session"; export type { SessionRequest } from "./domain/session-request"; export type { ISessionStore } from "./domain/session-store"; diff --git a/packages/dapp-client/src/client.integration.test.ts b/packages/dapp-client/src/client.integration.test.ts index 1182ecf..25f13be 100644 --- a/packages/dapp-client/src/client.integration.test.ts +++ b/packages/dapp-client/src/client.integration.test.ts @@ -94,6 +94,52 @@ t.describe("DappClient Integration Tests", () => { const expectedMessage = { type: "message", payload: initialPayload }; t.expect(request.initialMessage).toEqual(expectedMessage); }); + + t.describe("otp-display-grant", () => { + t.test("should include otpDisplayGrant capability on session_request when requireOtpDisplayGrant is true", async () => { + const sessionRequestPromise = new Promise((resolve) => { + dappClient.on("session_request", resolve); + }); + + dappClient.connect({ mode: "untrusted", requireOtpDisplayGrant: true }); // Don't await + + const request = await sessionRequestPromise; + t.expect(request.capabilities?.otpDisplayGrant).toBe(true); + }); + + t.test("should not include otpDisplayGrant capability on session_request by default", async () => { + const sessionRequestPromise = new Promise((resolve) => { + dappClient.on("session_request", resolve); + }); + + dappClient.connect({ mode: "untrusted" }); // Don't await + + const request = await sessionRequestPromise; + t.expect(request.capabilities?.otpDisplayGrant).toBeUndefined(); + }); + + t.test("should not include otpDisplayGrant capability on session_request when requireOtpDisplayGrant is false", async () => { + const sessionRequestPromise = new Promise((resolve) => { + dappClient.on("session_request", resolve); + }); + + dappClient.connect({ mode: "untrusted", requireOtpDisplayGrant: false }); // Don't await + + const request = await sessionRequestPromise; + t.expect(request.capabilities?.otpDisplayGrant).toBeUndefined(); + }); + + t.test("should not include otpDisplayGrant capability on session_request in trusted mode", async () => { + const sessionRequestPromise = new Promise((resolve) => { + dappClient.on("session_request", resolve); + }); + + dappClient.connect({ mode: "trusted", requireOtpDisplayGrant: true }); // Don't await + + const request = await sessionRequestPromise; + t.expect(request.capabilities?.otpDisplayGrant).toBeUndefined(); + }); + }); }); t.describe("Untrusted Flow", () => { diff --git a/packages/dapp-client/src/client.ts b/packages/dapp-client/src/client.ts index edca1e6..c818503 100644 --- a/packages/dapp-client/src/client.ts +++ b/packages/dapp-client/src/client.ts @@ -44,6 +44,11 @@ export interface DappConnectOptions { mode?: ConnectionMode; /** An optional unencrypted payload to be sent as the first message upon connection. */ initialPayload?: unknown; + /** + * When true (untrusted mode only), advertises `capabilities.otpDisplayGrant` on the session + * request and requires the wallet to defer OTP display until the dApp sends `otp-display-grant`. + */ + requireOtpDisplayGrant?: boolean; } /** @@ -109,11 +114,11 @@ export class DappClient extends BaseClient { public async connect(options: DappConnectOptions = {}): Promise { if (this.state !== ClientState.DISCONNECTED) throw new SessionError(ErrorCode.SESSION_INVALID_STATE, `Cannot connect when state is ${this.state}`); - const { mode = "untrusted", initialPayload } = options; + const { mode = "untrusted", initialPayload, requireOtpDisplayGrant = false } = options; if (!isValidConnectionMode(mode)) throw new SessionError(ErrorCode.SESSION_INVALID_STATE, `Invalid connection mode: "${String(mode)}"`); this.state = ClientState.CONNECTING; - const { pendingSession, request } = this._createPendingSessionAndRequest(mode, initialPayload); + const { pendingSession, request } = this._createPendingSessionAndRequest(mode, initialPayload, requireOtpDisplayGrant); this.session = pendingSession; this.emit("session_request", request); @@ -185,7 +190,11 @@ export class DappClient extends BaseClient { * @param mode - The connection mode to use for this session * @returns An object containing the pending session and session request */ - private _createPendingSessionAndRequest(mode: ConnectionMode, initialPayload?: unknown): { pendingSession: Session; request: SessionRequest } { + private _createPendingSessionAndRequest( + mode: ConnectionMode, + initialPayload?: unknown, + requireOtpDisplayGrant = false, + ): { pendingSession: Session; request: SessionRequest } { const id = uuid(); const keyPair = this.keymanager.generateKeyPair(); @@ -209,6 +218,11 @@ export class DappClient extends BaseClient { initialMessage: message, }; + // if in untrusted mode and requireOtpDisplayGrant is true, add the otpDisplayGrant capability to the request + if (mode === "untrusted" && requireOtpDisplayGrant) { + request.capabilities = { otpDisplayGrant: true }; + } + return { pendingSession, request }; } } diff --git a/packages/dapp-client/src/handlers/untrusted-connection-handler.test.ts b/packages/dapp-client/src/handlers/untrusted-connection-handler.test.ts index 0126a9b..0fda69a 100644 --- a/packages/dapp-client/src/handlers/untrusted-connection-handler.test.ts +++ b/packages/dapp-client/src/handlers/untrusted-connection-handler.test.ts @@ -5,7 +5,7 @@ import { vi } from "vitest"; import type { IConnectionHandlerContext } from "../domain/connection-handler-context"; import { UntrustedConnectionHandler } from "./untrusted-connection-handler"; -function createMockDappHandlerContext(): IConnectionHandlerContext { +function createMockDappHandlerContext(overrides: Partial = {}): IConnectionHandlerContext { return { session: null, state: ClientState.DISCONNECTED, @@ -33,6 +33,7 @@ function createMockDappHandlerContext(): IConnectionHandlerContext { once: vi.fn(), off: vi.fn(), sendMessage: vi.fn(), + ...overrides, }; } @@ -196,4 +197,142 @@ t.describe("UntrustedConnectionHandler", () => { await t.expect(handler.execute(mockSession, mockRequest)).rejects.toThrow(/Did not receive handshake offer/); }); + + t.describe("otp-display-grant", () => { + function setupStrictRequest(): void { + mockRequest.capabilities = { otpDisplayGrant: true }; + mockOffer.otpDisplayGrantRequired = true; + } + + function setupOtpSubmitMock(): void { + const mockEmit = t.vi.fn(); + mockEmit.mockImplementation((event: string, payload?: unknown) => { + if (event === "otp_required" && payload && typeof payload === "object" && "submit" in payload) { + (payload as { submit: (otp: string) => void }).submit("123456"); + } + }); + context.emit = mockEmit as any; + } + + t.test("should send otp-display-grant before otp_required in strict flow", async () => { + setupStrictRequest(); + + const callTimeline: string[] = []; + context.emit = t.vi.fn((event: string, payload?: unknown) => { + callTimeline.push(`emit:${event}`); + if (event === "otp_required" && payload && typeof payload === "object" && "submit" in payload) { + (payload as { submit: (otp: string) => void }).submit("123456"); + } + }) as typeof context.emit; + + context.sendMessage = t.vi.fn(async (_channel: string, message: { type: string }) => { + callTimeline.push(`send:${message.type}`); + }) as typeof context.sendMessage; + + await handler.execute(mockSession, mockRequest); + + const grantIndex = callTimeline.indexOf("send:otp-display-grant"); + const otpRequiredIndex = callTimeline.indexOf("emit:otp_required"); + t.expect(grantIndex).toBeGreaterThanOrEqual(0); + t.expect(otpRequiredIndex).toBeGreaterThanOrEqual(0); + t.expect(grantIndex).toBeLessThan(otpRequiredIndex); + }); + + t.test("should complete strict flow successfully", async () => { + setupStrictRequest(); + setupOtpSubmitMock(); + + await handler.execute(mockSession, mockRequest); + + const subscribeMock = context.transport.subscribe as t.Mock; + t.expect(subscribeMock.mock.calls.map((call) => call[0])).toEqual(["handshake:123", "session:secure-channel"]); + t.expect(context.sendMessage).toHaveBeenCalledWith("handshake:123", { type: "otp-display-grant" }); + t.expect(context.sendMessage).toHaveBeenCalledWith("session:secure-channel", { type: "handshake-ack" }); + t.expect(context.state).toBe("CONNECTED"); + }); + + t.test("should reject offer without otpDisplayGrantRequired when strict mode is required", async () => { + mockRequest.capabilities = { otpDisplayGrant: true }; + + await t.expect(handler.execute(mockSession, mockRequest)).rejects.toThrow("Wallet does not support OTP display grant required by this dApp."); + }); + + t.test("should keep legacy flow when strict mode is not required", async () => { + setupOtpSubmitMock(); + + const subscribeMock = context.transport.subscribe as t.Mock; + await handler.execute(mockSession, mockRequest); + + t.expect(subscribeMock.mock.calls.map((call) => call[0])).toEqual(["handshake:123", "session:secure-channel"]); + t.expect(context.sendMessage).not.toHaveBeenCalledWith(t.expect.any(String), { type: "otp-display-grant" }); + }); + + t.test("should still reject incorrect OTP in strict mode", async () => { + setupStrictRequest(); + + let submitFn: ((otp: string) => Promise) | undefined; + context.emit = t.vi.fn((event: string, payload?: unknown) => { + if (event === "otp_required" && payload && typeof payload === "object" && "submit" in payload) { + submitFn = (payload as { submit: (otp: string) => Promise }).submit; + } + }) as typeof context.emit; + + const executePromise = handler.execute(mockSession, mockRequest); + await new Promise((resolve) => setTimeout(resolve, 20)); + + t.expect(submitFn).toBeDefined(); + await t.expect(submitFn!("wrong")).rejects.toThrow("Incorrect OTP"); + await submitFn!("123456"); + await executePromise; + }); + + t.test("should throw if max OTP attempts are reached in strict mode", async () => { + setupStrictRequest(); + + let submitFn: ((otp: string) => Promise) | undefined; + context.emit = t.vi.fn((event: string, payload?: unknown) => { + if (event === "otp_required" && payload && typeof payload === "object" && "submit" in payload) { + submitFn = (payload as { submit: (otp: string) => Promise }).submit; + } + }) as typeof context.emit; + + const executePromise = handler.execute(mockSession, mockRequest); + await new Promise((resolve) => setTimeout(resolve, 20)); + + t.expect(context.sendMessage).toHaveBeenCalledWith("handshake:123", { type: "otp-display-grant" }); + + if (submitFn) { + for (const wrongOtp of ["wrong1", "wrong2"]) { + await t.expect(submitFn(wrongOtp)).rejects.toThrow("Incorrect OTP"); + } + try { + await submitFn("wrong3"); + } catch (e) { + t.expect((e as Error).message).toMatch("Maximum OTP attempts reached"); + } + } + + await t.expect(executePromise).rejects.toThrow("Maximum OTP attempts reached"); + }); + + t.test("should throw if OTP has already expired in strict mode", async () => { + setupStrictRequest(); + mockOffer.deadline = Date.now() - 1000; + + await t.expect(handler.execute(mockSession, mockRequest)).rejects.toThrow("The OTP has already expired"); + t.expect(context.sendMessage).toHaveBeenCalledWith("handshake:123", { type: "otp-display-grant" }); + }); + + t.test("should apply wallet public key from offer before sending otp-display-grant", async () => { + setupStrictRequest(); + setupOtpSubmitMock(); + + await handler.execute(mockSession, mockRequest); + + t.expect(context.keymanager.validatePeerKey).toHaveBeenCalled(); + const sessionAfterGrant = context.session; + t.expect(sessionAfterGrant?.theirPublicKey).toEqual(t.expect.any(Uint8Array)); + t.expect(sessionAfterGrant?.channel).toBe("session:secure-channel"); + }); + }); }); diff --git a/packages/dapp-client/src/handlers/untrusted-connection-handler.ts b/packages/dapp-client/src/handlers/untrusted-connection-handler.ts index a70385f..54471d7 100644 --- a/packages/dapp-client/src/handlers/untrusted-connection-handler.ts +++ b/packages/dapp-client/src/handlers/untrusted-connection-handler.ts @@ -16,6 +16,9 @@ import type { IConnectionHandlerContext } from "../domain/connection-handler-con * * This flow provides maximum security by requiring user verification of the * connection through a time-limited One-Time Password. + * + * When the session request advertises `capabilities.otpDisplayGrant`, the dApp sends + * `otp-display-grant` on the handshake channel before prompting for OTP entry. */ export class UntrustedConnectionHandler implements IConnectionHandler { private readonly context: IConnectionHandlerContext; @@ -34,10 +37,22 @@ export class UntrustedConnectionHandler implements IConnectionHandler { await this.context.transport.connect(); await this.context.transport.subscribe(request.channel); const offer = await this._waitForHandshakeOffer(request.expiresAt); - await this._handleOtpInput(offer); - const finalSession = this._createFinalSession(session, offer); - this.context.session = finalSession; - await this._acknowledgeHandshake(finalSession); + this._validateStrictOffer(request, offer); + + if (this._isStrictFlow(request, offer)) { + this._applyWalletPublicKeyFromOffer(session, offer); + await this._sendOtpDisplayGrant(request.channel); + await this._handleOtpInput(offer); + const finalSession = this._createFinalSession(session, offer); + this.context.session = finalSession; + await this._acknowledgeHandshake(finalSession); + } else { + await this._handleOtpInput(offer); + const finalSession = this._createFinalSession(session, offer); + this.context.session = finalSession; + await this._acknowledgeHandshake(finalSession); + } + await this._finalizeConnection(request.channel); } @@ -70,6 +85,28 @@ export class UntrustedConnectionHandler implements IConnectionHandler { }); } + /** + * Rejects offers that do not support strict OTP display grant when required. + * + * @param offer - The handshake offer from the wallet + * @throws {SessionError} If strict mode is required but the offer lacks support + */ + private _validateStrictOffer(request: SessionRequest, offer: HandshakeOfferPayload): void { + if (request.capabilities?.otpDisplayGrant === true && offer.otpDisplayGrantRequired !== true) { + throw new SessionError(ErrorCode.OTP_DISPLAY_GRANT_REQUIRED, "Wallet does not support OTP display grant required by this dApp."); + } + } + + /** + * Whether the connection should use the strict OTP display grant flow. + * + * @param request - The session request for this connection attempt + * @param offer - The handshake offer from the wallet + */ + private _isStrictFlow(request: SessionRequest, offer: HandshakeOfferPayload): boolean { + return request.capabilities?.otpDisplayGrant === true && offer.otpDisplayGrantRequired === true; + } + /** * Manages the OTP verification step by emitting the `otp_required` event and * waiting for the user to submit the correct OTP. @@ -108,6 +145,19 @@ export class UntrustedConnectionHandler implements IConnectionHandler { }); } + /** + * Sets the wallet's public key on the pending session so handshake messages can be + * encrypted to the accepted offer's wallet before the secure session is established. + * + * @param session - The pending session object + * @param offer - The handshake offer payload from the wallet + */ + private _applyWalletPublicKeyFromOffer(session: Session, offer: HandshakeOfferPayload): void { + const theirPublicKey = base64ToBytes(offer.publicKeyB64); + this.context.keymanager.validatePeerKey(theirPublicKey); + this.context.session = { ...session, theirPublicKey }; + } + /** * Creates the final session object with details from the wallet's offer. * @@ -125,6 +175,15 @@ export class UntrustedConnectionHandler implements IConnectionHandler { }; } + /** + * Sends `otp-display-grant` on the handshake channel, encrypted to the wallet's public key. + * + * @param handshakeChannel - The temporary handshake channel for this connection attempt + */ + private async _sendOtpDisplayGrant(handshakeChannel: string): Promise { + await this.context.sendMessage(handshakeChannel, { type: "otp-display-grant" }); + } + /** * Subscribes to the secure session channel and sends handshake acknowledgment. * diff --git a/packages/dapp-client/src/index.ts b/packages/dapp-client/src/index.ts index 61ffc4f..7e981a9 100644 --- a/packages/dapp-client/src/index.ts +++ b/packages/dapp-client/src/index.ts @@ -1,2 +1,2 @@ export type { SessionRequest } from "@metamask/mobile-wallet-protocol-core"; -export { DappClient, type DappClientOptions, type OtpRequiredPayload } from "./client"; +export { DappClient, type DappClientOptions, type DappConnectOptions, type OtpRequiredPayload } from "./client"; diff --git a/packages/wallet-client/src/client.integration.test.ts b/packages/wallet-client/src/client.integration.test.ts index ed2206b..29815b0 100644 --- a/packages/wallet-client/src/client.integration.test.ts +++ b/packages/wallet-client/src/client.integration.test.ts @@ -1,5 +1,5 @@ /** biome-ignore-all lint/suspicious/noExplicitAny: test code */ -import { type IKeyManager, type IKVStore, type KeyPair, type SessionRequest, SessionStore, WebSocketTransport } from "@metamask/mobile-wallet-protocol-core"; +import { ClientState, type IKeyManager, type IKVStore, type KeyPair, type SessionRequest, SessionStore, WebSocketTransport } from "@metamask/mobile-wallet-protocol-core"; import { bytesToBase64 } from "@metamask/utils"; import { decrypt, encrypt, PrivateKey, PublicKey } from "eciesjs"; import * as t from "vitest"; @@ -120,4 +120,26 @@ t.describe("WalletClient Integration Tests", () => { t.test("should have correct initial state", async () => { t.expect((walletClient as any).state).toBe("DISCONNECTED"); }); + + t.describe("otp-display-grant", () => { + t.test("should route otp-display-grant to internal event while CONNECTING", () => { + const listener = t.vi.fn(); + (walletClient as any).state = ClientState.CONNECTING; + walletClient.on("otp_display_grant_received" as any, listener); + + (walletClient as any).handleMessage({ type: "otp-display-grant" }); + + t.expect(listener).toHaveBeenCalledOnce(); + }); + + t.test("should ignore otp-display-grant when not CONNECTING", () => { + const listener = t.vi.fn(); + (walletClient as any).state = ClientState.CONNECTED; + walletClient.on("otp_display_grant_received" as any, listener); + + (walletClient as any).handleMessage({ type: "otp-display-grant" }); + + t.expect(listener).not.toHaveBeenCalled(); + }); + }); }); diff --git a/packages/wallet-client/src/client.ts b/packages/wallet-client/src/client.ts index aa723bf..a1d9882 100644 --- a/packages/wallet-client/src/client.ts +++ b/packages/wallet-client/src/client.ts @@ -142,6 +142,10 @@ export class WalletClient extends BaseClient { * @param message - The incoming message to handle */ protected handleMessage(message: ProtocolMessage): void { + if (this.state === ClientState.CONNECTING && message.type === "otp-display-grant") { + this.emit("otp_display_grant_received"); + return; + } if (message.type === "handshake-ack") { // Internal event to resolve the connection handler this.emit("handshake_ack_received"); diff --git a/packages/wallet-client/src/domain/connection-handler-context.ts b/packages/wallet-client/src/domain/connection-handler-context.ts index 03e0f3f..d2755de 100644 --- a/packages/wallet-client/src/domain/connection-handler-context.ts +++ b/packages/wallet-client/src/domain/connection-handler-context.ts @@ -19,6 +19,8 @@ export interface IConnectionHandlerContext { emit(event: "connected"): void; once(event: "handshake_ack_received", listener: () => void): void; off(event: "handshake_ack_received", listener: () => void): void; + once(event: "otp_display_grant_received", listener: () => void): void; + off(event: "otp_display_grant_received", listener: () => void): void; // Actions sendMessage(channel: string, message: ProtocolMessage): Promise; diff --git a/packages/wallet-client/src/handlers/untrusted-connection-handler.test.ts b/packages/wallet-client/src/handlers/untrusted-connection-handler.test.ts index bb6e26a..232aaca 100644 --- a/packages/wallet-client/src/handlers/untrusted-connection-handler.test.ts +++ b/packages/wallet-client/src/handlers/untrusted-connection-handler.test.ts @@ -196,4 +196,125 @@ t.describe("UntrustedConnectionHandler", () => { t.expect(connectedCallOrder).toBeLessThan(handleMessageCallOrder); }); + + t.describe("otp-display-grant", () => { + function setupDeferredGrantMocks(): { resolveGrant: () => void; resolveAck: () => void } { + let resolveGrant: () => void = () => {}; + let resolveAck: () => void = () => {}; + + context.once = t.vi.fn((event, callback) => { + if (event === "otp_display_grant_received") { + resolveGrant = callback; + } else if (event === "handshake_ack_received") { + resolveAck = callback; + } + return context; + }); + + return { resolveGrant: () => resolveGrant(), resolveAck: () => resolveAck() }; + } + + t.test("should defer display_otp until otp-display-grant when capability is advertised", async () => { + mockRequest.capabilities = { otpDisplayGrant: true }; + const { resolveGrant, resolveAck } = setupDeferredGrantMocks(); + + const executePromise = handler.execute(mockSession, mockRequest); + + await t.vi.waitFor(() => { + t.expect(context.sendMessage).toHaveBeenCalledWith(mockRequest.channel, t.expect.objectContaining({ type: "handshake-offer" })); + }); + + const sendMessageMock = context.sendMessage as t.MockedFunction; + const message = sendMessageMock.mock.calls[0][1] as { payload: { otpDisplayGrantRequired?: boolean } }; + t.expect(message.payload.otpDisplayGrantRequired).toBe(true); + + const emitMock = context.emit as t.Mock; + t.expect(emitMock.mock.calls.some((call) => call[0] === "display_otp")).toBe(false); + + resolveGrant(); + await t.vi.waitFor(() => { + t.expect(context.emit).toHaveBeenCalledWith("display_otp", t.expect.any(String), t.expect.any(Number)); + }); + + resolveAck(); + await executePromise; + }); + + t.test("should throw if otp-display-grant is not received in time", async () => { + mockRequest.capabilities = { otpDisplayGrant: true }; + context.once = t.vi.fn(); + + const originalDateNow = Date.now; + let callCount = 0; + Date.now = t.vi.fn(() => { + callCount++; + if (callCount === 1) { + return originalDateNow(); + } + return originalDateNow() + 70000; + }); + + try { + await t.expect(handler.execute(mockSession, mockRequest)).rejects.toThrow("OTP display grant timed out before it could begin."); + } finally { + Date.now = originalDateNow; + } + }); + + t.test("should keep legacy flow when capability is not advertised", async () => { + const emitMock = context.emit as t.Mock; + const sendMessageMock = context.sendMessage as t.Mock; + + await handler.execute(mockSession, mockRequest); + + const displayOtpInvocation = emitMock.mock.invocationCallOrder.find((_, index) => emitMock.mock.calls[index][0] === "display_otp"); + const sendOfferInvocation = sendMessageMock.mock.invocationCallOrder[0]; + + t.expect(displayOtpInvocation).toBeDefined(); + t.expect(displayOtpInvocation!).toBeLessThan(sendOfferInvocation!); + + const message = sendMessageMock.mock.calls[0][1] as { payload: { otpDisplayGrantRequired?: boolean } }; + t.expect(message.payload.otpDisplayGrantRequired).toBeUndefined(); + }); + + t.test("should complete strict flow successfully", async () => { + mockRequest.capabilities = { otpDisplayGrant: true }; + const { resolveGrant, resolveAck } = setupDeferredGrantMocks(); + + const executePromise = handler.execute(mockSession, mockRequest); + + await t.vi.waitFor(() => { + t.expect(context.sendMessage).toHaveBeenCalledWith( + mockRequest.channel, + t.expect.objectContaining({ + type: "handshake-offer", + payload: t.expect.objectContaining({ otpDisplayGrantRequired: true }), + }), + ); + }); + + const emitMock = context.emit as t.Mock; + t.expect(emitMock.mock.calls.some((call) => call[0] === "display_otp")).toBe(false); + + resolveGrant(); + await t.vi.waitFor(() => { + t.expect(context.emit).toHaveBeenCalledWith("display_otp", t.expect.any(String), t.expect.any(Number)); + }); + + const displayOtpInvocation = emitMock.mock.invocationCallOrder.find((_, index) => emitMock.mock.calls[index][0] === "display_otp"); + const sendOfferInvocation = (context.sendMessage as t.Mock).mock.invocationCallOrder[0]; + t.expect(displayOtpInvocation!).toBeGreaterThan(sendOfferInvocation!); + + resolveAck(); + await executePromise; + + t.expect(context.transport.connect).toHaveBeenCalledOnce(); + t.expect(context.transport.subscribe).toHaveBeenCalledWith(mockRequest.channel); + t.expect(context.transport.subscribe).toHaveBeenCalledWith(mockSession.channel); + t.expect(context.sessionstore.set).toHaveBeenCalledOnce(); + t.expect(context.transport.clear).toHaveBeenCalledWith(mockRequest.channel); + t.expect(context.state).toBe("CONNECTED"); + t.expect(context.emit).toHaveBeenCalledWith("connected"); + }); + }); }); diff --git a/packages/wallet-client/src/handlers/untrusted-connection-handler.ts b/packages/wallet-client/src/handlers/untrusted-connection-handler.ts index 500a279..5a52fdc 100644 --- a/packages/wallet-client/src/handlers/untrusted-connection-handler.ts +++ b/packages/wallet-client/src/handlers/untrusted-connection-handler.ts @@ -15,6 +15,8 @@ import type { IConnectionHandlerContext } from "../domain/connection-handler-con * * This flow provides maximum security by requiring the user to manually verify * the connection through a time-limited One-Time Password displayed on the wallet. + * When the dApp advertises `capabilities.otpDisplayGrant`, OTP display is deferred + * until the dApp sends `otp-display-grant` on the handshake channel. */ export class UntrustedConnectionHandler implements IConnectionHandler { private readonly context: IConnectionHandlerContext; @@ -34,8 +36,17 @@ export class UntrustedConnectionHandler implements IConnectionHandler { await this.context.transport.subscribe(session.channel); // secure channel this.context.session = session; const { otp, deadline } = this._generateOtpWithDeadline(); - this.context.emit("display_otp", otp, deadline); - await this._sendHandshakeOffer(request.channel, otp, deadline); + const otpDisplayGrantRequired = request.capabilities?.otpDisplayGrant === true; + + if (otpDisplayGrantRequired) { + await this._sendHandshakeOffer(request.channel, otp, deadline, true); + await this._waitForOtpDisplayGrant(deadline); + this.context.emit("display_otp", otp, deadline); + } else { + this.context.emit("display_otp", otp, deadline); + await this._sendHandshakeOffer(request.channel, otp, deadline); + } + await this._waitForHandshakeAck(deadline); await this._finalizeConnection(request.channel); this._processInitialMessage(request.initialMessage); @@ -58,9 +69,11 @@ export class UntrustedConnectionHandler implements IConnectionHandler { * Sends the `handshake-offer` message containing the public key, new channel ID, and OTP. * * @param channel - The handshake channel to publish the offer to - * @param options - Options containing OTP and deadline for untrusted connections + * @param otp - The generated OTP for untrusted connections + * @param deadline - The OTP expiration timestamp + * @param otpDisplayGrantRequired - Whether the dApp must grant OTP display before it is shown */ - private async _sendHandshakeOffer(channel: string, otp: string, deadline: number): Promise { + private async _sendHandshakeOffer(channel: string, otp: string, deadline: number, otpDisplayGrantRequired = false): Promise { if (!this.context.session) throw new SessionError(ErrorCode.SESSION_INVALID_STATE); const handshakePayload: HandshakeOfferPayload = { publicKeyB64: bytesToBase64(this.context.session.keyPair.publicKey), @@ -68,9 +81,42 @@ export class UntrustedConnectionHandler implements IConnectionHandler { otp, deadline, }; + + if (otpDisplayGrantRequired) { + handshakePayload.otpDisplayGrantRequired = true; + } + await this.context.sendMessage(channel, { type: "handshake-offer", payload: handshakePayload }); } + /** + * Waits for an `otp-display-grant` message from the dApp. + * + * @param deadline - The timestamp when the grant must be received + * @returns A promise that resolves when the grant is received + * @throws {SessionError} If the grant is not received before the deadline + */ + private _waitForOtpDisplayGrant(deadline: number): Promise { + return new Promise((resolve, reject) => { + const timeoutDuration = deadline - Date.now(); + if (timeoutDuration <= 0) { + return reject(new SessionError(ErrorCode.OTP_DISPLAY_GRANT_TIMEOUT, "OTP display grant timed out before it could begin.")); + } + + const timeoutId = setTimeout(() => { + this.context.off("otp_display_grant_received", onGrantReceived); + reject(new SessionError(ErrorCode.OTP_DISPLAY_GRANT_TIMEOUT, "DApp did not grant OTP display in time.")); + }, timeoutDuration); + + const onGrantReceived = () => { + clearTimeout(timeoutId); + resolve(); + }; + + this.context.once("otp_display_grant_received", onGrantReceived); + }); + } + /** * Waits for a `handshake-ack` message from the dApp. *