From 1f4768071b9a8ee1d003984252bbd59c8f292716 Mon Sep 17 00:00:00 2001 From: Gancho Radkov Date: Mon, 8 Apr 2024 13:18:01 +0300 Subject: [PATCH 1/2] feat: adds `session_authenticate` to expirer --- packages/core/src/controllers/pairing.ts | 6 +- .../sign-client/src/controllers/engine.ts | 86 +++++++++++++----- packages/sign-client/test/sdk/auth.spec.ts | 90 ++++++++++++++++++- packages/types/src/sign-client/engine.ts | 21 ++++- 4 files changed, 173 insertions(+), 30 deletions(-) diff --git a/packages/core/src/controllers/pairing.ts b/packages/core/src/controllers/pairing.ts index 9969d2cda..71e9921cd 100644 --- a/packages/core/src/controllers/pairing.ts +++ b/packages/core/src/controllers/pairing.ts @@ -102,9 +102,9 @@ export class Pairing implements IPairing { expiryTimestamp: expiry, methods: params?.methods, }); + this.core.expirer.set(topic, expiry); await this.pairings.set(topic, pairing); await this.core.relayer.subscribe(topic); - this.core.expirer.set(topic, expiry); return { topic, uri }; }; @@ -125,8 +125,8 @@ export class Pairing implements IPairing { const expiry = expiryTimestamp || calcExpiry(FIVE_MINUTES); const pairing = { topic, relay, expiry, active: false, methods }; - await this.pairings.set(topic, pairing); this.core.expirer.set(topic, expiry); + await this.pairings.set(topic, pairing); if (params.activatePairing) { await this.activate({ topic }); @@ -145,8 +145,8 @@ export class Pairing implements IPairing { public activate: IPairing["activate"] = async ({ topic }) => { this.isInitialized(); const expiry = calcExpiry(THIRTY_DAYS); - await this.pairings.update(topic, { active: true, expiry }); this.core.expirer.set(topic, expiry); + await this.pairings.update(topic, { active: true, expiry }); }; public ping: IPairing["ping"] = async (params) => { diff --git a/packages/sign-client/src/controllers/engine.ts b/packages/sign-client/src/controllers/engine.ts index 541363b1d..99ffecab6 100644 --- a/packages/sign-client/src/controllers/engine.ts +++ b/packages/sign-client/src/controllers/engine.ts @@ -604,7 +604,18 @@ export class Engine extends IEngine { this.isInitialized(); this.isValidAuthenticate(params); - const { chains, statement = "", uri, domain, nonce, type, exp, nbf, methods = [] } = params; + const { + chains, + statement = "", + uri, + domain, + nonce, + type, + exp, + nbf, + methods = [], + expiry, + } = params; // reassign resources to remove reference as the array is modified and might cause side effects const resources = [...(params.resources || [])]; @@ -642,7 +653,12 @@ export class Engine extends IEngine { resources.push(recap); } - const expiryTimestamp = calcExpiry(ENGINE_RPC_OPTS.wc_sessionPropose.req.ttl); + // ensure the expiry is at least the minimum required for the request - currently 1h + const authRequestExpiry = + expiry && expiry > ENGINE_RPC_OPTS.wc_sessionAuthenticate.req.ttl + ? expiry + : ENGINE_RPC_OPTS.wc_sessionAuthenticate.req.ttl; + const request = { authPayload: { type: type ?? "caip122", @@ -658,7 +674,7 @@ export class Engine extends IEngine { resources, }, requester: { publicKey, metadata: this.client.metadata }, - expiryTimestamp, + expiryTimestamp: calcExpiry(authRequestExpiry), }; // ----- build namespaces for fallback session proposal ----- // @@ -679,13 +695,10 @@ export class Engine extends IEngine { publicKey, metadata: this.client.metadata, }, - expiryTimestamp, + expiryTimestamp: calcExpiry(ENGINE_RPC_OPTS.wc_sessionPropose.req.ttl), }; - const { done, resolve, reject } = createDelayedPromise( - ENGINE_RPC_OPTS.wc_sessionAuthenticate.req.ttl, - "Request expired", - ); + const { done, resolve, reject } = createDelayedPromise(authRequestExpiry, "Request expired"); // handle fallback session proposal response const onSessionConnect = async ({ error, session }: any) => { @@ -703,6 +716,7 @@ export class Engine extends IEngine { }); } const sessionObject = this.client.session.get(session.topic); + await this.deleteProposal(fallbackId); resolve({ session: sessionObject, }); @@ -710,6 +724,10 @@ export class Engine extends IEngine { }; // handle session authenticate response const onAuthenticate = async (payload: any) => { + // delete this auth request on response + // we're using payload from the wallet to establish the session so we don't need to keep this around + await this.deletePendingAuthRequest(id, { message: "fulfilled", code: 0 }); + if (payload.error) { // wallets that do not support wc_sessionAuthenticate will return an error // we should not reject the promise in this case as the fallback session proposal will be used @@ -720,7 +738,8 @@ export class Engine extends IEngine { this.events.off(engineEvent("session_connect"), onSessionConnect); return reject(payload.error.message); } - + // delete fallback proposal on successful authenticate as the proposal will not be responded to + await this.deleteProposal(fallbackId); // cleanup listener for fallback response this.events.off(engineEvent("session_connect"), onSessionConnect); @@ -834,14 +853,9 @@ export class Engine extends IEngine { } await this.setProposal(fallbackId, { id: fallbackId, ...proposal }); - - await this.client.auth.requests.set(id, { - authPayload: request.authPayload, - requester: request.requester, - expiryTimestamp, - id, + await this.setAuthRequest(id, { + request: { ...request, verifyContext: {} as any }, pairingTopic, - verifyContext: {} as any, }); return { @@ -1082,16 +1096,39 @@ export class Engine extends IEngine { } }; + private deletePendingAuthRequest: EnginePrivate["deletePendingAuthRequest"] = async ( + id, + reason, + expirerHasDeleted = false, + ) => { + await Promise.all([ + this.client.auth.requests.delete(id, reason), + expirerHasDeleted ? Promise.resolve() : this.client.core.expirer.del(id), + ]); + }; + private setExpiry: EnginePrivate["setExpiry"] = async (topic, expiry) => { - if (this.client.session.keys.includes(topic)) { - await this.client.session.update(topic, { expiry }); - } + if (!this.client.session.keys.includes(topic)) return; this.client.core.expirer.set(topic, expiry); + await this.client.session.update(topic, { expiry }); }; private setProposal: EnginePrivate["setProposal"] = async (id, proposal) => { - await this.client.proposal.set(id, proposal); this.client.core.expirer.set(id, calcExpiry(ENGINE_RPC_OPTS.wc_sessionPropose.req.ttl)); + await this.client.proposal.set(id, proposal); + }; + + private setAuthRequest: EnginePrivate["setAuthRequest"] = async (id, params) => { + const { request, pairingTopic } = params; + this.client.core.expirer.set(id, request.expiryTimestamp); + await this.client.auth.requests.set(id, { + authPayload: request.authPayload, + requester: request.requester, + expiryTimestamp: request.expiryTimestamp, + id, + pairingTopic, + verifyContext: request.verifyContext, + }); }; private setPendingSessionRequest: EnginePrivate["setPendingSessionRequest"] = async ( @@ -1100,13 +1137,13 @@ export class Engine extends IEngine { const { id, topic, params, verifyContext } = pendingRequest; const expiry = params.request.expiryTimestamp || calcExpiry(ENGINE_RPC_OPTS.wc_sessionRequest.req.ttl); + this.client.core.expirer.set(id, expiry); await this.client.pendingRequest.set(id, { id, topic, params, verifyContext, }); - if (expiry) this.client.core.expirer.set(id, expiry); }; private sendRequest: EnginePrivate["sendRequest"] = async (args) => { @@ -1799,9 +1836,7 @@ export class Engine extends IEngine { verifyContext, expiryTimestamp, }; - - await this.client.auth.requests.set(payload.id, pendingRequest); - + await this.setAuthRequest(payload.id, { request: pendingRequest, pairingTopic: topic }); this.client.events.emit("session_authenticate", { topic, params: payload.params, @@ -1878,6 +1913,9 @@ export class Engine extends IEngine { if (id && this.client.pendingRequest.keys.includes(id)) { return await this.deletePendingSessionRequest(id, getInternalError("EXPIRED"), true); } + if (id && this.client.auth.requests.keys.includes(id)) { + return await this.deletePendingAuthRequest(id, getInternalError("EXPIRED"), true); + } if (topic) { if (this.client.session.keys.includes(topic)) { diff --git a/packages/sign-client/test/sdk/auth.spec.ts b/packages/sign-client/test/sdk/auth.spec.ts index 6822b02a1..de2fa6741 100644 --- a/packages/sign-client/test/sdk/auth.spec.ts +++ b/packages/sign-client/test/sdk/auth.spec.ts @@ -5,7 +5,6 @@ import { TEST_APP_METADATA_B, TEST_SIGN_CLIENT_OPTIONS, throttle } from "../shar import { buildApprovedNamespaces, buildAuthObject, - calcExpiry, populateAuthPayload, } from "@walletconnect/utils"; import { AuthTypes } from "@walletconnect/types"; @@ -48,6 +47,27 @@ describe("Authenticated Sessions", () => { Promise.race([ new Promise((resolve) => { wallet.on("session_authenticate", async (payload) => { + // validate that the dapp has both `session_authenticate` & `session_proposal` stored + // and expirer configured + const pendingProposals = dapp.proposal.getAll(); + expect(pendingProposals.length).to.eq(1); + expect(dapp.core.expirer.keys).to.include(`id:${pendingProposals[0].id}`); + expect(dapp.core.expirer.get(pendingProposals[0].id)).to.exist; + expect(dapp.core.expirer.get(pendingProposals[0].id)?.expiry).to.exist; + expect(dapp.core.expirer.get(pendingProposals[0].id)?.expiry).to.be.greaterThan(0); + + const pendingAuthRequests = dapp.auth.requests.getAll(); + expect(pendingAuthRequests.length).to.eq(1); + expect(dapp.core.expirer.keys).to.include(`id:${pendingAuthRequests[0].id}`); + expect(dapp.core.expirer.get(pendingAuthRequests[0].id)).to.exist; + expect(dapp.core.expirer.get(pendingAuthRequests[0].id)?.expiry).to.exist; + expect(dapp.core.expirer.get(pendingAuthRequests[0].id)?.expiry).to.be.greaterThan(0); + expect(pendingAuthRequests[0].id).to.eq(payload.id); + + // validate that the wallet doesn't have any pending proposals + const pendingProposalsWallet = wallet.proposal.getAll(); + expect(pendingProposalsWallet.length).to.eq(0); + const authPayload = populateAuthPayload({ authPayload: payload.params.authPayload, chains: requestedChains, @@ -115,6 +135,12 @@ describe("Authenticated Sessions", () => { resolve(); }), ]); + + // confirm that all pending proposals and auth requests have been cleared + expect(wallet.proposal.getAll().length).to.eq(0); + expect(wallet.auth.requests.getAll().length).to.eq(0); + expect(dapp.proposal.getAll().length).to.eq(0); + expect(dapp.auth.requests.getAll().length).to.eq(0); }); // this test simulates the scenario where the wallet supports subset of the requested chains and all methods // and replies with a single signature @@ -146,6 +172,27 @@ describe("Authenticated Sessions", () => { await Promise.all([ new Promise((resolve) => { wallet.on("session_authenticate", async (payload) => { + // validate that the dapp has both `session_authenticate` & `session_proposal` stored + // and expirer configured + const pendingProposals = dapp.proposal.getAll(); + expect(pendingProposals.length).to.eq(1); + expect(dapp.core.expirer.keys).to.include(`id:${pendingProposals[0].id}`); + expect(dapp.core.expirer.get(pendingProposals[0].id)).to.exist; + expect(dapp.core.expirer.get(pendingProposals[0].id)?.expiry).to.exist; + expect(dapp.core.expirer.get(pendingProposals[0].id)?.expiry).to.be.greaterThan(0); + + const pendingAuthRequests = dapp.auth.requests.getAll(); + expect(pendingAuthRequests.length).to.eq(1); + expect(dapp.core.expirer.keys).to.include(`id:${pendingAuthRequests[0].id}`); + expect(dapp.core.expirer.get(pendingAuthRequests[0].id)).to.exist; + expect(dapp.core.expirer.get(pendingAuthRequests[0].id)?.expiry).to.exist; + expect(dapp.core.expirer.get(pendingAuthRequests[0].id)?.expiry).to.be.greaterThan(0); + expect(pendingAuthRequests[0].id).to.eq(payload.id); + + // validate that the wallet doesn't have any pending proposals + const pendingProposalsWallet = wallet.proposal.getAll(); + expect(pendingProposalsWallet.length).to.eq(0); + const authPayload = populateAuthPayload({ authPayload: payload.params.authPayload, chains: supportedChains, @@ -207,6 +254,11 @@ describe("Authenticated Sessions", () => { resolve(); }), ]); + // confirm that all pending proposals and auth requests have been cleared + expect(wallet.proposal.getAll().length).to.eq(0); + expect(wallet.auth.requests.getAll().length).to.eq(0); + expect(dapp.proposal.getAll().length).to.eq(0); + expect(dapp.auth.requests.getAll().length).to.eq(0); }); // this test simulates the scenario where the wallet supports subset of the requested chains and methods // and replies with a single signature @@ -239,6 +291,27 @@ describe("Authenticated Sessions", () => { await Promise.all([ new Promise((resolve) => { wallet.on("session_authenticate", async (payload) => { + // validate that the dapp has both `session_authenticate` & `session_proposal` stored + // and expirer configured + const pendingProposals = dapp.proposal.getAll(); + expect(pendingProposals.length).to.eq(1); + expect(dapp.core.expirer.keys).to.include(`id:${pendingProposals[0].id}`); + expect(dapp.core.expirer.get(pendingProposals[0].id)).to.exist; + expect(dapp.core.expirer.get(pendingProposals[0].id)?.expiry).to.exist; + expect(dapp.core.expirer.get(pendingProposals[0].id)?.expiry).to.be.greaterThan(0); + + const pendingAuthRequests = dapp.auth.requests.getAll(); + expect(pendingAuthRequests.length).to.eq(1); + expect(dapp.core.expirer.keys).to.include(`id:${pendingAuthRequests[0].id}`); + expect(dapp.core.expirer.get(pendingAuthRequests[0].id)).to.exist; + expect(dapp.core.expirer.get(pendingAuthRequests[0].id)?.expiry).to.exist; + expect(dapp.core.expirer.get(pendingAuthRequests[0].id)?.expiry).to.be.greaterThan(0); + expect(pendingAuthRequests[0].id).to.eq(payload.id); + + // validate that the wallet doesn't have any pending proposals + const pendingProposalsWallet = wallet.proposal.getAll(); + expect(pendingProposalsWallet.length).to.eq(0); + const authPayload = populateAuthPayload({ authPayload: payload.params.authPayload, chains: supportedChains, @@ -1132,10 +1205,19 @@ describe("Authenticated Sessions", () => { //@ts-expect-error wallet.core.pairing.registeredMethods = []; wallet.core.pairing.register({ methods: toRegisterMethods }); - await Promise.all([ new Promise((resolve) => { wallet.on("session_proposal", async (payload) => { + // validate that the dapp has both `session_authenticate` & `session_proposal` stored + // and expirer configured + const pendingProposals = dapp.proposal.getAll(); + expect(pendingProposals.length).to.eq(1); + expect(dapp.core.expirer.keys).to.include(`id:${pendingProposals[0].id}`); + expect(dapp.core.expirer.get(pendingProposals[0].id)).to.exist; + expect(dapp.core.expirer.get(pendingProposals[0].id)?.expiry).to.exist; + expect(dapp.core.expirer.get(pendingProposals[0].id)?.expiry).to.be.greaterThan(0); + expect(pendingProposals[0].id).to.eq(payload.id); + try { const approved = buildApprovedNamespaces({ supportedNamespaces: { @@ -1197,5 +1279,9 @@ describe("Authenticated Sessions", () => { resolve(); }), ]); + // confirm that all pending proposals and auth requests have been cleared + expect(wallet.proposal.getAll().length).to.eq(0); + expect(dapp.proposal.getAll().length).to.eq(0); + expect(dapp.auth.requests.getAll().length).to.eq(0); }); }); diff --git a/packages/types/src/sign-client/engine.ts b/packages/types/src/sign-client/engine.ts index 2d1038200..bb96dcf04 100644 --- a/packages/types/src/sign-client/engine.ts +++ b/packages/types/src/sign-client/engine.ts @@ -14,7 +14,7 @@ import { JsonRpcTypes } from "./jsonrpc"; import { EventEmitter } from "events"; import { PendingRequestTypes } from "./pendingRequest"; import { AuthTypes } from "./auth"; -import { CryptoTypes } from "../core"; +import { CryptoTypes, Verify } from "../core"; export declare namespace EngineTypes { type Event = @@ -214,6 +214,19 @@ export interface EnginePrivate { setProposal(id: number, proposal: ProposalTypes.Struct): Promise; + setAuthRequest( + id: number, + params: { + request: { + requester: AuthTypes.SessionAuthenticateRequestParams["requester"]; + authPayload: AuthTypes.SessionAuthenticateRequestParams["authPayload"]; + expiryTimestamp: AuthTypes.SessionAuthenticateRequestParams["expiryTimestamp"]; + verifyContext: Verify.Context; + }; + pairingTopic: string; + }, + ): Promise; + setPendingSessionRequest(pendingRequest: PendingRequestTypes.Struct): Promise; deletePendingSessionRequest( @@ -222,6 +235,12 @@ export interface EnginePrivate { expirerHasDeleted?: boolean, ): Promise; + deletePendingAuthRequest( + id: number, + reason: ErrorResponse, + expirerHasDeleted?: boolean, + ): Promise; + cleanupDuplicatePairings(session: SessionTypes.Struct): Promise; cleanup(): Promise; From 872a5bf9a7d06279ae6d4c3f01bc33490b214d12 Mon Sep 17 00:00:00 2001 From: Gancho Radkov Date: Fri, 19 Apr 2024 10:02:50 +0300 Subject: [PATCH 2/2] refactor: updates types & comment --- packages/sign-client/src/controllers/engine.ts | 2 +- packages/types/src/sign-client/auth.ts | 4 ++++ packages/types/src/sign-client/engine.ts | 9 ++------- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/packages/sign-client/src/controllers/engine.ts b/packages/sign-client/src/controllers/engine.ts index 99ffecab6..881eaf9af 100644 --- a/packages/sign-client/src/controllers/engine.ts +++ b/packages/sign-client/src/controllers/engine.ts @@ -653,7 +653,7 @@ export class Engine extends IEngine { resources.push(recap); } - // ensure the expiry is at least the minimum required for the request - currently 1h + // Ensure the expiry is greater than the minimum required for the request - currently 1h const authRequestExpiry = expiry && expiry > ENGINE_RPC_OPTS.wc_sessionAuthenticate.req.ttl ? expiry diff --git a/packages/types/src/sign-client/auth.ts b/packages/types/src/sign-client/auth.ts index 1d4dcf99f..df1c51877 100644 --- a/packages/types/src/sign-client/auth.ts +++ b/packages/types/src/sign-client/auth.ts @@ -161,6 +161,10 @@ export declare namespace AuthTypes { expiryTimestamp: number; } + interface SessionAuthenticateRequest extends SessionAuthenticateRequestParams { + verifyContext: Verify.Context; + } + type AuthenticateResponseResult = { auths?: AuthTypes.AuthResponse; session: SessionTypes.Struct; diff --git a/packages/types/src/sign-client/engine.ts b/packages/types/src/sign-client/engine.ts index bb96dcf04..81ad4b394 100644 --- a/packages/types/src/sign-client/engine.ts +++ b/packages/types/src/sign-client/engine.ts @@ -14,7 +14,7 @@ import { JsonRpcTypes } from "./jsonrpc"; import { EventEmitter } from "events"; import { PendingRequestTypes } from "./pendingRequest"; import { AuthTypes } from "./auth"; -import { CryptoTypes, Verify } from "../core"; +import { CryptoTypes } from "../core"; export declare namespace EngineTypes { type Event = @@ -217,12 +217,7 @@ export interface EnginePrivate { setAuthRequest( id: number, params: { - request: { - requester: AuthTypes.SessionAuthenticateRequestParams["requester"]; - authPayload: AuthTypes.SessionAuthenticateRequestParams["authPayload"]; - expiryTimestamp: AuthTypes.SessionAuthenticateRequestParams["expiryTimestamp"]; - verifyContext: Verify.Context; - }; + request: AuthTypes.SessionAuthenticateRequest; pairingTopic: string; }, ): Promise;