feat: multi session fix - #2213
Conversation
Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
Signed-off-by: Alex Matson <alex.matson@digitalasset.com>
This reverts commit 436a3d3. Signed-off-by: Marc Juchli <marc.juchli@digitalasset.com>
Signed-off-by: Marc Juchli <marc.juchli@digitalasset.com>
Signed-off-by: Marc Juchli <marc.juchli@digitalasset.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the wallet gateway to support multiple concurrent sessions by scoping frontend auth state to the initiating dApp origin, propagating that origin into popup contexts, and keying backend session storage/notifiers by session/access-token rather than user ID.
Changes:
- Frontend: introduce origin detection/broadcast and store auth state (access token, network, intended route, session id) per dApp origin.
- Backend: key sessions by access token, persist session origin, and route notifications via session-scoped notifier IDs.
- Contracts/tests: extend OpenRPC/types with
origin, add DB migration + new multisession e2e test, and update unit tests accordingly.
Reviewed changes
Copilot reviewed 72 out of 73 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Updates workspace dependency graph (adds @canton-network/core-types to affected workspaces). |
| wallet-gateway/remote/src/web/frontend/state-manager.ts | Scopes frontend storage keys by origin and adds per-origin session fields. |
| wallet-gateway/remote/src/web/frontend/state-manager.test.ts | Updates and extends StateManager tests for origin-scoped keys and currentOrigin persistence. |
| wallet-gateway/remote/src/web/frontend/sign-message/index.ts | Uses detected origin when resolving the access token. |
| wallet-gateway/remote/src/web/frontend/sign-message/index.test.ts | Extends stateManager mock to include currentOrigin. |
| wallet-gateway/remote/src/web/frontend/settings/index.ts | Uses detected origin when creating RPC clients and fetching data. |
| wallet-gateway/remote/src/web/frontend/settings/index.test.ts | Extends stateManager mock to include currentOrigin. |
| wallet-gateway/remote/src/web/frontend/rpc-client.ts | Uses detected origin during auto-logout to clear the correct origin-scoped state. |
| wallet-gateway/remote/src/web/frontend/rpc-client.test.ts | Extends stateManager mock to include currentOrigin. |
| wallet-gateway/remote/src/web/frontend/parties/index.ts | Uses detected origin for access token and origin-scoped network id reads. |
| wallet-gateway/remote/src/web/frontend/parties/index.test.ts | Extends stateManager mock to include currentOrigin. |
| wallet-gateway/remote/src/web/frontend/parties/add/index.ts | Uses detected origin for access token and origin-scoped network id reads. |
| wallet-gateway/remote/src/web/frontend/parties/add/index.test.ts | Extends stateManager mock to include currentOrigin. |
| wallet-gateway/remote/src/web/frontend/networks/review/index.ts | Uses detected origin when creating RPC clients. |
| wallet-gateway/remote/src/web/frontend/networks/review/index.test.ts | Extends stateManager mock to include currentOrigin. |
| wallet-gateway/remote/src/web/frontend/networks/index.ts | Uses detected origin when creating RPC clients. |
| wallet-gateway/remote/src/web/frontend/networks/index.test.ts | Extends stateManager mock to include currentOrigin. |
| wallet-gateway/remote/src/web/frontend/networks/add/index.ts | Uses detected origin when creating RPC clients. |
| wallet-gateway/remote/src/web/frontend/networks/add/index.test.ts | Extends stateManager mock to include currentOrigin. |
| wallet-gateway/remote/src/web/frontend/login/login.ts | Uses detected origin for token/network persistence and awaits redirect after self-sign. |
| wallet-gateway/remote/src/web/frontend/login/login.test.ts | Updates expectations for origin-scoped state manager calls. |
| wallet-gateway/remote/src/web/frontend/listeners.ts | Adds origin broadcast listener and detectCurrentOrigin() helper. |
| wallet-gateway/remote/src/web/frontend/listeners.test.ts | Adds unit tests for origin detection behavior and timeout. |
| wallet-gateway/remote/src/web/frontend/index.ts | Makes redirects origin-aware, caches session id per origin, and includes origin in addSession params. |
| wallet-gateway/remote/src/web/frontend/index.test.ts | Updates tests for async redirect, session id caching, and no-session behavior. |
| wallet-gateway/remote/src/web/frontend/identity-providers/review/index.ts | Uses detected origin when creating RPC clients. |
| wallet-gateway/remote/src/web/frontend/identity-providers/review/index.test.ts | Extends stateManager mock to include currentOrigin. |
| wallet-gateway/remote/src/web/frontend/identity-providers/index.ts | Uses detected origin when creating RPC clients. |
| wallet-gateway/remote/src/web/frontend/identity-providers/index.test.ts | Extends stateManager mock to include currentOrigin. |
| wallet-gateway/remote/src/web/frontend/identity-providers/add/index.ts | Uses detected origin when creating RPC clients. |
| wallet-gateway/remote/src/web/frontend/identity-providers/add/index.test.ts | Extends stateManager mock to include currentOrigin. |
| wallet-gateway/remote/src/web/frontend/callback/index.ts | Stores token/expiry under detected origin and uses origin-scoped network id. |
| wallet-gateway/remote/src/web/frontend/callback/index.test.ts | Updates expectations for origin-scoped accessToken.set calls. |
| wallet-gateway/remote/src/web/frontend/approve/index.ts | Uses detected origin when creating RPC clients. |
| wallet-gateway/remote/src/web/frontend/approve/index.test.ts | Extends stateManager mock to include currentOrigin. |
| wallet-gateway/remote/src/web/frontend/api-keys/index.ts | Uses detected origin when creating RPC clients. |
| wallet-gateway/remote/src/web/frontend/api-keys/index.test.ts | Extends stateManager mock to include currentOrigin. |
| wallet-gateway/remote/src/web/frontend/api-keys/add/index.ts | Uses detected origin when creating RPC clients. |
| wallet-gateway/remote/src/web/frontend/api-keys/add/index.test.ts | Extends stateManager mock to include currentOrigin. |
| wallet-gateway/remote/src/web/frontend/activities/index.ts | Uses detected origin when creating RPC clients. |
| wallet-gateway/remote/src/web/frontend/activities/index.test.ts | Extends stateManager mock to include currentOrigin. |
| wallet-gateway/remote/src/user-api/rpc-gen/typings.ts | Adds Origin type and session/addSession origin fields in generated typings. |
| wallet-gateway/remote/src/user-api/controller.ts | Switches notifier scoping to session id and adds origin persistence on addSession. |
| wallet-gateway/remote/src/user-api/controller.test.ts | Updates tests for session-id-scoped notifiers and origin-aware addSession/removeSession behavior. |
| wallet-gateway/remote/src/signing/signing-worker.ts | Uses session id notifier scoping (and logs missing session). |
| wallet-gateway/remote/src/signing/signing-worker.test.ts | Adjusts store mocks to support getSession during worker ticks. |
| wallet-gateway/remote/src/middleware/sessionHandler.ts | Updates session lookup to be access-token-keyed. |
| wallet-gateway/remote/src/middleware/apiKeyAuth.ts | Introduces API-key-backed session creation with origin and access-token-keyed sessions. |
| wallet-gateway/remote/src/ledger/wallet-sync-service.test.ts | Updates session fixtures with origin + access token values. |
| wallet-gateway/remote/src/ledger/transaction-service.ts | Ensures executed transaction status is set as a literal 'signed'. |
| wallet-gateway/remote/src/dapp-api/server.ts | Scopes SSE notifier to session id rather than user id. |
| wallet-gateway/remote/src/dapp-api/controller.ts | Updates connect/disconnect/isConnected flows to use access-token-keyed session retrieval and session-id notifiers. |
| wallet-gateway/remote/src/dapp-api/controller.test.ts | Updates tests to reflect session-id-scoped notifiers and access-token-keyed session removal. |
| examples/portfolio/src/routeTree.gen.ts | Regenerated route tree output (ordering changes). |
| examples/ping/tests/multisession.spec.ts | Adds Playwright e2e regression test covering independent sessions across popup vs direct tab. |
| core/wallet-user-rpc-client/src/openrpc.json | Adds required origin to addSession params and includes origin on session objects. |
| core/wallet-user-rpc-client/src/index.ts | Aligns exported client typings with new Origin/session origin fields. |
| core/wallet-ui-components/src/windows/popup.ts | Broadcasts parent origin to popup via postMessage with ACK-based retry. |
| core/wallet-test-utils/src/wallet-gateway.ts | Refactors test utilities to separate page-vs-popup concerns and adds login/logout helpers. |
| core/wallet-test-utils/src/index.ts | Exports all wallet-gateway utilities (incl. new WalletGatewayPage). |
| core/wallet-store/src/Store.ts | Changes session API to be access-token-keyed and adds listSessions + session origin. |
| core/wallet-store-sql/src/store-sql.ts | Implements access-token-keyed sessions, listSessions, and origin enforcement. |
| core/wallet-store-sql/src/store-sql.test.ts | Updates tests for access-token-keyed sessions and origin requirements. |
| core/wallet-store-sql/src/schema.ts | Adds origin to session table mapping. |
| core/wallet-store-sql/src/migrations/015-add-origin-field-session.ts | Adds origin to sessions, adds PK/indexes, and clears legacy session rows. |
| core/wallet-store-sql/src/migrations-test/data/015-add-origin-field-session.test.ts | Adds migration coverage for origin column + indexes. |
| core/wallet-store-sql/package.json | Adds @canton-network/core-types dependency. |
| core/wallet-store-sql/migrations.lock.json | Registers migration 015. |
| core/wallet-store-inmemory/src/store-internal.ts | Switches in-memory session storage to a map keyed by access token; adds listSessions. |
| core/wallet-store-inmemory/src/store-internal.test.ts | Updates tests for access-token-keyed sessions and origin requirements. |
| core/wallet-store-inmemory/package.json | Adds @canton-network/core-types dependency. |
| core/types/src/index.ts | Adds origin-broadcast WalletEvents and extends SpliceMessage schema; introduces AccessToken type alias. |
| api-specs/openrpc-user-api.json | Mirrors OpenRPC updates for addSession origin and session origin. |
Suppressed comments (2)
wallet-gateway/remote/src/web/frontend/state-manager.ts:55
setWithStorage/clearWithStoragealso write/delete the in-memory cache under the barekey, so setting/clearing for one origin will overwrite/clear another origin's cached value. Use an origin-qualified cache key here too.
wallet-gateway/remote/src/web/frontend/callback/index.ts:90redirectToIntendedOrDefaultis now async, but this call doesn't return/await it. IfdetectCurrentOrigin()rejects insideredirectToIntendedOrDefault, that rejection won't be caught by the existing.catch(handleErrorToast)and may become an unhandled rejection.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // delete any old storage entries not corresponding to the current version | ||
| for (const key in localStorage) { | ||
| if (!key.startsWith(VERSION_PREFIX)) { | ||
| localStorage.removeItem(key) | ||
| } | ||
| } |
| if (this.state.has(key)) { | ||
| return this.state.get(key) | ||
| } | ||
|
|
||
| const value = localStorage.getItem(StateManager.localStorageKey(key)) | ||
| const value = storage.getItem(StateManager.localStorageKey(key, origin)) |
| z.object({ | ||
| type: z.literal(WalletEvent.SPLICE_WALLET_EXT_OPEN), | ||
| url: z.string().url(), | ||
| url: z.url(), | ||
| target: SpliceTarget.optional(), |
| // due to the asynchronicity when sending the postMessage immediately after redirecting, | ||
| // there is a chance that the child window has not yet loaded, | ||
| // and does not an event listener established yet. Therefore, | ||
| // we repeatedly poll until the child window sends back an acknowledgment message. | ||
| const handleMessage = (event: MessageEvent) => { | ||
| if (!isSpliceMessageEvent(event)) return | ||
| if ( | ||
| event.data.type !== | ||
| WalletEvent.SPLICE_WALLET_BROADCAST_ORIGIN_ACK | ||
| ) | ||
| return | ||
| if (childOrigin !== event.origin) return | ||
|
|
||
| clearInterval(originPoller) | ||
| window.removeEventListener('message', handleMessage) | ||
| } | ||
|
|
||
| window.addEventListener('message', handleMessage) | ||
|
|
||
| const originPoller = setInterval(() => { | ||
| win.postMessage(message, childOrigin) | ||
| }, 500) |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 72 out of 73 changed files in this pull request and generated no new comments.
Suppressed comments (8)
wallet-gateway/remote/src/web/frontend/state-manager.ts:22
- The constructor removes every localStorage entry that does not start with the wallet prefix. That can delete unrelated application data stored on the same origin (and the
for..initeration overlocalStorageis not a reliable way to enumerate keys while mutating). Limit deletion to the wallet’s own keys and iterate vialocalStorage.key(i)with a two-pass remove list.
wallet-gateway/remote/src/web/frontend/state-manager.ts:38 - In-memory caching in
getWithStorageuses onlykeyas the cache key. With the new origin-scoped storage keys, this will return the wrong value when the samekey(e.g. networkId) is accessed for different origins.
wallet-gateway/remote/src/web/frontend/state-manager.ts:56 setWithStorageupdates the in-memory cache using onlykey, which will overwrite cached values across different origins and break multi-session behavior.
wallet-gateway/remote/src/web/frontend/state-manager.ts:65clearWithStoragedeletes the in-memory cache entry bykeyonly. With origin-scoped state, this can clear the cached value for other origins as well.
core/types/src/index.ts:138z.url()is not a standard Zod API (the usual form isz.string().url()). As written this will fail at runtime/compile-time unless the project has extended Zod, but this file importszdirectly from 'zod'.
z.object({
type: z.literal(WalletEvent.SPLICE_WALLET_EXT_OPEN),
url: z.url(),
target: SpliceTarget.optional(),
}),
core/types/src/index.ts:147
- Same issue here:
origin: z.url()is not a standard Zod API. This should likely bez.string().url()to validate the origin string.
z.object({
type: z.literal(WalletEvent.SPLICE_WALLET_BROADCAST_ORIGIN),
origin: z.url(),
}),
core/wallet-ui-components/src/windows/popup.ts:79
originPollerrepeats indefinitely until an ACK arrives. If the popup is closed early or the wallet never loads / never acks, the interval and message listener will leak and keep firing.
// due to the asynchronicity when sending the postMessage immediately after redirecting,
// there is a chance that the child window has not yet loaded,
// and does not an event listener established yet. Therefore,
// we repeatedly poll until the child window sends back an acknowledgment message.
const handleMessage = (event: MessageEvent) => {
core/wallet-store-inmemory/src/store-internal.ts:256
StoreInternal.setSessionstores sessions keyed only byaccessTokenand does not evict any existing session for the sameorigin. If a session’s access token changes for the same origin (e.g. apiKeyAuth overwrites the session after exchanging tokens), the old entry remains andlistSessions()/memory state diverges from the SQL store behavior.
async setSession(session: Session): Promise<void> {
const storage = this.getStorage()
storage.sessions.set(session.accessToken, session)
this.updateStorage(storage)
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 72 out of 73 changed files in this pull request and generated no new comments.
Suppressed comments (6)
wallet-gateway/remote/src/web/frontend/state-manager.ts:24
- The constructor currently deletes every localStorage entry that doesn't start with the new VERSION_PREFIX. This can wipe unrelated localStorage keys for the same origin (other apps, feature flags, etc.). It should only remove legacy wallet keys (e.g. the old "com.splice.wallet." namespace) that are not on the current version prefix.
wallet-gateway/remote/src/middleware/sessionHandler.ts:47 - If
req.authContextis missing (or missingaccessToken), callingstore.withAuthContext(context).getSession('')will throw viaassertConnected, returning 500 instead of 401. This middleware should short-circuit to 401 before touching the store when auth context/token is absent.
logger.debug('Checking for active session for ' + context?.userId)
const session = await store
.withAuthContext(context)
.getSession(context?.accessToken || '')
if (!session) {
logger.debug('No active session found for ' + context?.userId)
res.status(401).json({ error: 'No active session found' })
} else {
core/types/src/index.ts:138
z.url()is not a Zod API (Zod validates URLs viaz.string().url()). As written this will throw at runtime / fail typechecking, breaking SpliceMessage validation.
z.object({
type: z.literal(WalletEvent.SPLICE_WALLET_EXT_OPEN),
url: z.url(),
target: SpliceTarget.optional(),
}),
core/types/src/index.ts:147
- Same issue here:
z.url()is not a Zod API; this schema should validate a URL string viaz.string().url().
z.object({
type: z.literal(WalletEvent.SPLICE_WALLET_BROADCAST_ORIGIN),
origin: z.url(),
}),
core/wallet-ui-components/src/windows/popup.ts:96
- The origin broadcast poller is never cleaned up if the child window never ACKs (or is closed), so this interval (and message listener) can run forever. Add a closed-window guard (and ideally a timeout) to stop polling and remove the listener.
const originPoller = setInterval(() => {
win.postMessage(message, childOrigin)
}, 500)
wallet-gateway/remote/src/dapp-api/controller.ts:283
store.getSession(context.accessToken)can return undefined (e.g. if the session was removed/expired), but the code usessession!.id, which will crash and turn this into a 500 instead of a controlled "not connected" / auth error. Handle the missing-session case explicitly before using the notifier.
// TODO: consider service accounts / api keys
const session = await store.getSession(context.accessToken)
const sessionId = session!.id
const notifier = notificationService.getNotifier(sessionId)
No description provided.