Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 15 additions & 13 deletions packages/kyc-controller/ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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` /
Expand Down Expand Up @@ -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
Expand All @@ -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):

Expand Down Expand Up @@ -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. |
1 change: 1 addition & 0 deletions packages/kyc-controller/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
49 changes: 46 additions & 3 deletions packages/kyc-controller/src/KycController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
});
Expand All @@ -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'));
Expand Down Expand Up @@ -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();
Expand Down
63 changes: 48 additions & 15 deletions packages/kyc-controller/src/KycController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Comment on lines +76 to +77

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would these statuses not be sign of an immediate error? Or at least FinallyRejected should?

'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
Expand Down Expand Up @@ -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`).
Expand Down Expand Up @@ -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' &&
Expand Down Expand Up @@ -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({
Expand All @@ -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;
Expand Down
Loading