diff --git a/packages/kyc-controller/ARCHITECTURE.md b/packages/kyc-controller/ARCHITECTURE.md index a8035e8777d..ce8a481b2f9 100644 --- a/packages/kyc-controller/ARCHITECTURE.md +++ b/packages/kyc-controller/ARCHITECTURE.md @@ -277,8 +277,8 @@ stateDiagram-v2 > disclaimers, and launches SumSub. MoonPay Check/Auth frames are skipped; > `phase` moves `terms → session → submit → done`. A SumSub failure (thrown step > or SDK close without completion) rewinds to `terms` instead of forcing `done`. -> A terminal UKYC rejection after the SDK reported `Completed` still finishes as -> `done` so `refreshKycStatus` can surface the decision. +> A terminal UKYC rejection after the SDK reported a submitted status still +> finishes as `done` so `refreshKycStatus` can surface the decision. > `acceptTermsAndStartSession` requires `sumsubTncSigned` and `idosTncSigned` > (T&C2) for every vendor; omitted flags fail the flow instead of defaulting to > `true`. Those flags are mapped onto the session catalog's `idOS` / @@ -473,11 +473,11 @@ stateDiagram-v2 creatingSession --> fetchingToken : setAuthorizations() ok creatingSession --> vendorProcessing : setAuthorizations() kycStatus=approved, finalStatus=pending fetchingToken --> launching : createJourney() ok - launching --> inProgress : onStatusChange (non-Completed) - launching --> complete : onStatusChange = Completed - inProgress --> complete : onStatusChange = Completed - launching --> failed : resolves without a Completed status - inProgress --> failed : resolves without a Completed status + launching --> inProgress : onStatusChange (not submitted) + launching --> complete : submitted status (Approved / Pending / Completed / ...) + inProgress --> complete : submitted status + launching --> failed : resolves without a submitted status + inProgress --> failed : resolves without a submitted status creatingSession --> failed : error fetchingToken --> failed : error launching --> failed : launcher unavailable / error @@ -491,11 +491,13 @@ stateDiagram-v2 > applicant is not asked to verify again. > **Completion is status-driven, not resolution-driven.** A resolved `launch` -> is only recorded as `complete` when the SDK reported the `Completed` status -> via `onStatusChange` at least once. If `launch` resolves without ever having -> reported `Completed` (e.g. the applicant abandoned the flow, or a non-success -> outcome), the controller records `failed` — so consumers never mistake an -> unfinished flow for a verified one. +> is only recorded as submitted when the SDK reported a submitted status via +> `onStatusChange` **or** on the `launch()` result `status`. Native SumSub +> closes with `Pending` / `Approved` / `TemporarilyDeclined` / +> `FinallyRejected` (and some launchers still emit `Completed`). If neither +> the callback nor the result carries one of those (e.g. the applicant +> abandoned with `Incomplete`), the controller records `failed` — so +> consumers never mistake an unfinished flow for a verified one. The `KycSumSubLauncher` interface (injected per client): @@ -710,6 +712,6 @@ Reference client (metamask-mobile): | `app/core/Engine/controllers/kyc/kyc-service-init.ts` | Construct service. | | `app/core/Engine/controllers/kyc/reactNativeSumSubLauncher.ts` | Native SumSub adapter. | | `app/core/Engine/messengers/kyc/*.ts` | Messenger delegation. | -| `app/components/Views/MoonpayDemo/useKycFlow.ts` | React ↔ controller binding. | +| `app/components/Views/MoonpayDemo/useKycFlow.ts` | React ↔ controller binding. | | `app/components/Views/MoonpayDemo/useMoonpayFrame.ts` | WebView postMessage bridge. | | `app/selectors/kycController.ts` | Redux selectors. | diff --git a/packages/kyc-controller/CHANGELOG.md b/packages/kyc-controller/CHANGELOG.md index 0bd2648aaa0..cf815241c5a 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 +- Treat native SumSub SDK close statuses (`Pending`, `Approved`, `TemporarilyDeclined`, `FinallyRejected`) as applicant-submitted in `startSumSub`, including the `launch()` result `status`, so a finished SDK session is not recorded as failed. `Incomplete` / `Initial` / `Failed` remain unfinished. - 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)) diff --git a/packages/kyc-controller/src/KycController.test.ts b/packages/kyc-controller/src/KycController.test.ts index 5651378d5ea..c1839992f18 100644 --- a/packages/kyc-controller/src/KycController.test.ts +++ b/packages/kyc-controller/src/KycController.test.ts @@ -1941,11 +1941,11 @@ describe('KycController', () => { }); }); - it('marks failed when launch resolves without a Completed status', async () => { + it('marks failed when launch resolves without a submitted status', async () => { await withController(async ({ controller, launcher }) => { launcher.launch.mockImplementation(async ({ onStatusChange }) => { // The applicant abandons the flow: the SDK reports progress but never - // a Completed status, yet `launch` still resolves. + // a submitted status, yet `launch` still resolves. onStatusChange?.('idle', 'InProgress'); return { ok: false }; }); @@ -1958,6 +1958,49 @@ describe('KycController', () => { }); }); + it('treats a Pending launch result as submitted when onStatusChange never fires', async () => { + await withController(async ({ controller, launcher }) => { + // Native SumSub closes with Pending on the result and may never emit + // Completed via onStatusChange. + launcher.launch.mockResolvedValue({ status: 'Pending' }); + + const result = await controller.startSumSub(); + + expect(result).toStrictEqual({ status: 'Pending' }); + expect(controller.state.sumsub.status).toBe('complete'); + }); + }); + + it.each(['Approved', 'TemporarilyDeclined', 'FinallyRejected'])( + 'treats an %s onStatusChange as submitted', + async (status) => { + await withController(async ({ controller, launcher }) => { + launcher.launch.mockImplementation(async ({ onStatusChange }) => { + onStatusChange?.('Incomplete', status); + return { status }; + }); + + await controller.startSumSub(); + + expect(controller.state.sumsub.status).toBe('complete'); + }); + }, + ); + + it('marks failed when launch resolves with Incomplete', async () => { + await withController(async ({ controller, launcher }) => { + launcher.launch.mockImplementation(async ({ onStatusChange }) => { + onStatusChange?.('Ready', 'Incomplete'); + return { status: 'Incomplete' }; + }); + + const result = await controller.startSumSub(); + + expect(result).toStrictEqual({ status: 'Incomplete' }); + expect(controller.state.sumsub.status).toBe('failed'); + }); + }); + it('marks failed and returns the error when a step throws', async () => { await withController(async ({ controller, handlers }) => { handlers.createUkycSession.mockRejectedValue(new Error('ukyc down')); @@ -2127,7 +2170,7 @@ describe('KycController', () => { it('does not poll when the SDK did not report completion', async () => { await withController(async ({ controller, handlers, launcher }) => { // The applicant abandons the flow: `launch` resolves without ever - // reporting a Completed status. + // reporting a submitted status. launcher.launch.mockResolvedValue({ ok: false }); await controller.startSumSub(); diff --git a/packages/kyc-controller/src/KycController.ts b/packages/kyc-controller/src/KycController.ts index 94a7af58c52..10d551f2868 100644 --- a/packages/kyc-controller/src/KycController.ts +++ b/packages/kyc-controller/src/KycController.ts @@ -65,10 +65,18 @@ const MOCK_JWT_TOKEN = 'mock-jwt-token'; // rather than a fixed short window, so this is a session-scoped window. const UKYC_CAPABILITY_TOKEN_TTL_MS = 4 * 60 * 60 * 1000; -// The SumSub SDK status that signals the applicant finished the flow -// successfully. Any other resolution (abandonment, failure, or a non-success -// outcome) must not be recorded as `complete`. -const SUMSUB_COMPLETED_STATUS = 'Completed'; +// SumSub SDK statuses that mean the applicant submitted verification (or the +// vendor already decided). Native SNSMobileSDKStatus uses Approved / Pending / +// TemporarilyDeclined / FinallyRejected; some launchers and tests still emit +// Completed. Incomplete / Initial / Failed / Ready are unfinished and must +// not be recorded as submitted. +const SUMSUB_SUBMITTED_STATUSES = new Set([ + 'Approved', + 'Pending', + 'TemporarilyDeclined', + 'FinallyRejected', + 'Completed', +]); // Phases that represent an active vendor-session flow (tokens issued and/or // Check/Auth frames in progress). A repeat `initialize` while in one of these @@ -454,6 +462,20 @@ function isSessionAlreadyCompletedError(error: unknown): boolean { return String(error).includes(SESSION_NOT_IN_VALID_STATE); } +/** + * Whether a SumSub SDK status means the applicant finished submitting. + * + * Checked against both `onStatusChange` and the `launch()` result `status`, + * because the native SDK often closes with Pending/Approved on the result + * and never emits `Completed`. + * + * @param status - SDK status string from the callback or launch result. + * @returns True when UKYC session-status polling should start. + */ +function isSumSubSubmittedStatus(status: unknown): boolean { + return typeof status === 'string' && SUMSUB_SUBMITTED_STATUSES.has(status); +} + /** * Whether recording session disclaimers failed because those document * versions were already consented for the session (`409 Conflict`). @@ -1155,10 +1177,10 @@ export class KycController extends BaseController< return; } // `startSumSub` records `sumsub.status = 'failed'` for thrown steps, - // an SDK close without Completed, *and* a terminal UKYC rejection - // after the SDK reported Completed. Only rewind when there is no - // session-status decision yet (abandonment / thrown step). A - // Completed-then-rejected poll writes `sessionStatus` and is a + // an SDK close without a submitted status, *and* a terminal UKYC + // rejection after the SDK reported submitted. Only rewind when there + // is no session-status decision yet (abandonment / thrown step). A + // submitted-then-rejected poll writes `sessionStatus` and is a // finished flow: refresh user status and land on `done`. if ( this.state.sumsub.status === 'failed' && @@ -2073,9 +2095,9 @@ export class KycController extends BaseController< return {}; } - // Track whether the SDK ever reported a successful completion. A resolved + // Track whether the SDK reported that the applicant submitted. A resolved // `launch` alone does not imply success — the applicant may have - // abandoned the flow or the SDK may have reported a non-success outcome. + // abandoned the flow (`Incomplete`) or the SDK may have failed. let reachedCompletion = false; const result = await this.#sumsubLauncher.launch({ @@ -2096,21 +2118,32 @@ export class KycController extends BaseController< return refreshed.applicantAccessToken; }, onStatusChange: (_prev, next) => { - if (next === SUMSUB_COMPLETED_STATUS) { + if (isSumSubSubmittedStatus(next)) { reachedCompletion = true; + this.#updateIfCurrent(generation, (state) => { + state.sumsub.status = 'complete'; + }); + return; } this.#updateIfCurrent(generation, (state) => { - state.sumsub.status = - next === SUMSUB_COMPLETED_STATUS ? 'complete' : 'inProgress'; + state.sumsub.status = 'inProgress'; }); }, locale: params?.locale ?? 'en', debug: params?.debug ?? false, }); + // Native SumSub often closes with Pending/Approved on the result and + // never emits `Completed` via `onStatusChange`. Honor `result.status` + // as well so a finished SDK session is not recorded as failed. + if (isSumSubSubmittedStatus(result.status)) { + reachedCompletion = true; + } + // A resolved `launch` alone is not the final outcome: only a SDK-reported - // completion is worth polling for a verification decision. Anything else - // (abandonment, non-success) is `failed` and must not be polled. + // submitted status is worth polling for a verification decision. + // Anything else (abandonment, non-success) is `failed` and must not be + // polled. const applied = this.#updateIfCurrent(generation, (state) => { state.sumsub.status = reachedCompletion ? 'polling' : 'failed'; state.sumsub.result = result as Json;