Skip to content

Commit

Permalink
Merge pull request #4420 from WalletConnect/fix/set-item-in-expirer
Browse files Browse the repository at this point in the history 
fix: adds `session_authenticate` to expirer
  • Loading branch information
@ganchoradkov
ganchoradkov committed Apr 19, 2024
2 parents f17f5ad + 872a5bf commit 6b68620
Show file tree
Hide file tree
Showing 5 changed files with 171 additions and 29 deletions.
6 changes: 3 additions & 3 deletions packages/core/src/controllers/pairing.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
};
Expand All @@ -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 });
Expand All @@ -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) => {
Expand Down
86 changes: 62 additions & 24 deletions packages/sign-client/src/controllers/engine.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 || [])];

Expand Down Expand Up @@ -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 greater than 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",
Expand All @@ -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 ----- //
Expand All @@ -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) => {
Expand All @@ -703,13 +716,18 @@ export class Engine extends IEngine {
});
}
const sessionObject = this.client.session.get(session.topic);
await this.deleteProposal(fallbackId);
resolve({
session: sessionObject,
});
}
};
// 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
Expand All @@ -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);

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 (
Expand All @@ -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) => {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)) {
Expand Down
90 changes: 88 additions & 2 deletions packages/sign-client/test/sdk/auth.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -48,6 +47,27 @@ describe("Authenticated Sessions", () => {
Promise.race<void>([
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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -146,6 +172,27 @@ describe("Authenticated Sessions", () => {
await Promise.all([
new Promise<void>((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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -239,6 +291,27 @@ describe("Authenticated Sessions", () => {
await Promise.all([
new Promise<void>((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,
Expand Down Expand Up @@ -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<void>((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: {
Expand Down Expand Up @@ -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);
});
});
4 changes: 4 additions & 0 deletions packages/types/src/sign-client/auth.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading

0 comments on commit 6b68620

Please sign in to comment.