diff --git a/packages/kyc-controller/CHANGELOG.md b/packages/kyc-controller/CHANGELOG.md index 69f5a83095a..0bd2648aaa0 100644 --- a/packages/kyc-controller/CHANGELOG.md +++ b/packages/kyc-controller/CHANGELOG.md @@ -45,6 +45,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Stop routing `KycService` write endpoints through the query cache. Every `POST` (`createSession`, `checkKycRequired`, `createVendorCustomer`, `submitVendorDisclaimers`, `submitSessionDisclaimers`, `createUkycSession`, `setAuthorizations`, `createJourney`) now issues its request directly instead of via `fetchQuery`. Previously these were modelled as queries, so two overlapping calls sharing a `queryKey` were deduplicated into a single request — a second `createVendorCustomer` while the first was in flight never reached the API — and their responses were retained in the cache and published on the messenger as `cacheUpdated` payloads, which for these endpoints include session tokens and applicant access tokens. Writes are also no longer retried by the service policy, so a failed non-idempotent request cannot create duplicate records server-side. ([#10007](https://github.com/MetaMask/core/pull/10007)) - Stop incorrectly prefixing the session client public key onto wrapped `encryptionDataKey` and `ukycCapabilityToken` values from `wrapEncryptionKey`. `data` is now ciphertext+tag only; the server already has the client public key from session creation. ([#10036](https://github.com/MetaMask/core/pull/10036)) - Clear leftover MoonPay `sessionToken`, `accessToken`, and Check/Auth frame credentials when `initialize` or `createVendorCustomer` switches to another vendor, so `buildCheckFrameUrl` cannot return a MoonPay URL for a consents-path session. ([#9908](https://github.com/MetaMask/core/pull/9908)) - Rewind the consents path when SumSub fails before completion (thrown step or SDK close without `Completed`), instead of refreshing user status and forcing `phase` to `done`. A terminal UKYC rejection after the SDK completed still finishes as `done` so the decision can be reflected in user status. ([#9908](https://github.com/MetaMask/core/pull/9908)) diff --git a/packages/kyc-controller/src/KycService.test.ts b/packages/kyc-controller/src/KycService.test.ts index d7dee565e11..03d3afefae8 100644 --- a/packages/kyc-controller/src/KycService.test.ts +++ b/packages/kyc-controller/src/KycService.test.ts @@ -607,6 +607,25 @@ describe('KycService', () => { /Malformed response received from vendor customers API/u, ); }); + + it('sends one request per call when two calls overlap', async () => { + const scope = nock(MOCK_API_URL) + .post('/vendors/iron/customers', { email: 'a@b.co' }) + .times(2) + .reply(200, { + id: 'iron-1', + email: 'a@b.co', + status: 'SigningsRequired', + }); + const { service } = getService(); + + await Promise.all([ + service.createVendorCustomer({ vendor: 'iron', email: 'a@b.co' }), + service.createVendorCustomer({ vendor: 'iron', email: 'a@b.co' }), + ]); + + expect(scope.isDone()).toBe(true); + }); }); describe('submitVendorDisclaimers', () => { diff --git a/packages/kyc-controller/src/KycService.ts b/packages/kyc-controller/src/KycService.ts index 919a41d8bc3..484e59a8077 100644 --- a/packages/kyc-controller/src/KycService.ts +++ b/packages/kyc-controller/src/KycService.ts @@ -393,13 +393,21 @@ export type GetSessionStatusParams = { * `fetch` when provided), and the auth bearer token and geolocation come from * other controllers via the messenger. * - * It extends {@link BaseDataService}, so every request is routed through - * `fetchQuery`: it is wrapped in the shared service policy (retries, circuit - * breaker) and its result is exposed via the service's `QueryClient`. Read-only - * endpoints (`fetchVendorDisclaimers`, `fetchIdosEnclaveJwks`, `fetchIdosRelayJwks`) are cached - * with a `staleTime`; vendor-disclaimer, session-scoped disclaimer, - * session-creating, and status-polling endpoints opt out of caching + * It extends {@link BaseDataService}, so read-only endpoints are routed through + * `fetchQuery`: they are wrapped in the shared service policy (retries, circuit + * breaker) and their results are exposed via the service's `QueryClient`. + * `fetchDisclaimers` and `fetchJwks` are cached with a `staleTime`; + * session-scoped disclaimer and status-polling reads opt out of caching * (`staleTime`/`gcTime` of `0`) so they never serve a stale result. + * + * Write endpoints (every `POST`) deliberately bypass `fetchQuery`. The query + * cache is built for idempotent reads: it deduplicates concurrent requests + * sharing a `queryKey`, retains responses for replay, and publishes them on the + * messenger via `cacheUpdated`. None of that is safe for calls that create + * sessions, customers, or consents — two overlapping `createVendorCustomer` + * calls would collapse into a single `POST`, and session tokens would be + * broadcast as cache payloads. Writes therefore call `#requestJson` directly, + * which also means they are not retried by the service policy. */ export class KycService extends BaseDataService< typeof serviceName, @@ -543,21 +551,9 @@ export class KycService extends BaseDataService< params: CreateSessionParams, ): Promise> { const url = new URL('/vendors/moonpay/sessions', this.#baseUrl); - const data = await this.fetchQuery({ - queryKey: [ - `${this.name}:createSession`, - params.email, - params.termsAcceptedAt, - params.disclaimerIds, - ], - queryFn: async () => - this.#requestJson(url, { - method: 'POST', - body: JSON.stringify(params), - }), - // A session-creating mutation must never serve a stale/cached result. - staleTime: 0, - gcTime: 0, + const data = await this.#requestJson(url, { + method: 'POST', + body: JSON.stringify(params), }); return this.#validateResponse( data, @@ -602,22 +598,9 @@ export class KycService extends BaseDataService< } } - const data = await this.fetchQuery({ - queryKey: [ - `${this.name}:checkKycRequired`, - vendor, - params.accessToken ?? null, - params.country ?? null, - capabilities, - ], - queryFn: async () => - this.#requestJson(url, { - method: 'POST', - body: JSON.stringify(body), - }), - // The requirement can change server-side, so always re-check. - staleTime: 0, - gcTime: 0, + const data = await this.#requestJson(url, { + method: 'POST', + body: JSON.stringify(body), }); const { required } = this.#validateResponse( data, @@ -642,20 +625,9 @@ export class KycService extends BaseDataService< params: CreateVendorCustomerParams, ): Promise { const url = new URL(`/vendors/${params.vendor}/customers`, this.#baseUrl); - const data = await this.fetchQuery({ - queryKey: [ - `${this.name}:createVendorCustomer`, - params.vendor, - params.email, - ], - queryFn: async () => - this.#requestJson(url, { - method: 'POST', - body: JSON.stringify({ email: params.email }), - }), - // Customer creation/resume must never serve a stale/cached result. - staleTime: 0, - gcTime: 0, + const data = await this.#requestJson(url, { + method: 'POST', + body: JSON.stringify({ email: params.email }), }); return this.#validateResponse( data, @@ -683,19 +655,9 @@ export class KycService extends BaseDataService< `/vendors/${encodeURIComponent(params.vendor)}/disclaimers`, this.#baseUrl, ); - const data = await this.fetchQuery({ - queryKey: [ - `${this.name}:submitVendorDisclaimers`, - params.vendor, - params.disclaimerIds, - ], - queryFn: async () => - this.#requestJson(url, { - method: 'POST', - body: JSON.stringify({ disclaimerIds: params.disclaimerIds }), - }), - staleTime: 0, - gcTime: 0, + const data = await this.#requestJson(url, { + method: 'POST', + body: JSON.stringify({ disclaimerIds: params.disclaimerIds }), }); return this.#validateResponse( data, @@ -787,26 +749,14 @@ export class KycService extends BaseDataService< `/sessions/${encodeURIComponent(params.sessionId)}/disclaimers`, this.#baseUrl, ); - const data = await this.fetchQuery({ - queryKey: [ - `${this.name}:submitSessionDisclaimers`, - params.sessionId, - params.idOS, - params.kycProvider, - params.credentialReusabilityConsentGiven, - ], - queryFn: async () => - this.#requestJson(url, { - method: 'POST', - body: JSON.stringify({ - idOS: params.idOS, - kycProvider: params.kycProvider, - credentialReusabilityConsentGiven: - params.credentialReusabilityConsentGiven, - }), - }), - staleTime: 0, - gcTime: 0, + const data = await this.#requestJson(url, { + method: 'POST', + body: JSON.stringify({ + idOS: params.idOS, + kycProvider: params.kycProvider, + credentialReusabilityConsentGiven: + params.credentialReusabilityConsentGiven, + }), }); return this.#validateResponse( data, @@ -920,23 +870,16 @@ export class KycService extends BaseDataService< params: CreateUkycSessionParams, ): Promise { const url = new URL('/sessions', this.#baseUrl); - const data = await this.fetchQuery({ - queryKey: [`${this.name}:createUkycSession`, params.jwtToken], - queryFn: async () => - this.#requestJson(url, { - method: 'POST', - body: JSON.stringify({ - vendorId: params.vendor ?? 'moonpay', - vendorUserId: 'mockedId', - jwtToken: params.jwtToken, - sessionClientPublicKey: params.sessionClientPublicKey, - residenceCountry: params.residenceCountry, - vendorMetadata: params.vendorMetadata ?? {}, - }), - }), - // A session-creating mutation must never serve a stale/cached result. - staleTime: 0, - gcTime: 0, + const data = await this.#requestJson(url, { + method: 'POST', + body: JSON.stringify({ + vendorId: params.vendor ?? 'moonpay', + vendorUserId: 'mockedId', + jwtToken: params.jwtToken, + sessionClientPublicKey: params.sessionClientPublicKey, + residenceCountry: params.residenceCountry, + vendorMetadata: params.vendorMetadata ?? {}, + }), }); return this.#validateResponse( data, @@ -961,18 +904,12 @@ export class KycService extends BaseDataService< `/sessions/${encodeURIComponent(params.sessionId)}/authorizations`, this.#baseUrl, ); - const data = await this.fetchQuery({ - queryKey: [`${this.name}:setAuthorizations`, params.sessionId], - queryFn: async () => - this.#requestJson(url, { - method: 'POST', - body: JSON.stringify({ - wrappedEncryptionDataKey: params.wrappedEncryptionDataKey, - wrappedUkycCapabilityToken: params.wrappedUkycCapabilityToken, - }), - }), - staleTime: 0, - gcTime: 0, + const data = await this.#requestJson(url, { + method: 'POST', + body: JSON.stringify({ + wrappedEncryptionDataKey: params.wrappedEncryptionDataKey, + wrappedUkycCapabilityToken: params.wrappedUkycCapabilityToken, + }), }); return this.#validateResponse( data, @@ -995,13 +932,7 @@ export class KycService extends BaseDataService< `/sessions/${encodeURIComponent(sessionId)}/journey`, this.#baseUrl, ); - const data = await this.fetchQuery({ - queryKey: [`${this.name}:createJourney`, sessionId], - queryFn: async () => this.#requestJson(url, { method: 'POST' }), - // Journeys are (re)created on demand; do not reuse a cached token. - staleTime: 0, - gcTime: 0, - }); + const data = await this.#requestJson(url, { method: 'POST' }); return this.#validateResponse( data, ApplicantAccessTokenResponseStruct, @@ -1077,8 +1008,9 @@ export class KycService extends BaseDataService< /** * Performs a single JSON request. * - * This is meant to be used as the `queryFn` for {@link fetchQuery}, which - * wraps it in the shared service policy (retries, circuit breaker). Requests + * Read endpoints pass this as the `queryFn` to {@link fetchQuery}, which + * wraps it in the shared service policy (retries, circuit breaker). Write + * endpoints call it directly, so they are executed exactly once. Requests * are authenticated with the wallet bearer token by default; pass * `{ authenticated: false }` for calls to services that do not expect it * (e.g. the idOS enclave or idOS relay JWKS endpoints).