Skip to content

Commit 550345f

Browse files
dariusz-bieladariusz-biela
committed
refactor(mfa): decide registration from the credentials snapshot captured at flow start
The registration decision needed the same credentials check the Provider already runs for start telemetry, and the machine actor duplicated the hook logic in the operations modules to get it, costing a second native keystore read per flow start. INIT now carries the captured localCredentialsKnownToServer flag, the decision becomes an eventless transition on it, and the operations copies and their suites go away, leaving the hooks as the single implementation. A keystore read failure now routes to registration instead of a fatal outcome, because the hooks resolve the check to false instead of rejecting; re-registration recovers such an account anyway. The UI walk grows stronger: the INIT executor seeds the biometrics hook mock, so the flag flows through the real Provider wiring, and both decision branches traverse via INIT fixture variants.
1 parent 115fbb3 commit 550345f

15 files changed

Lines changed: 70 additions & 263 deletions

File tree

‎src/components/MultifactorAuthentication/Context/MultifactorAuthenticationMainContext.tsx‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,14 @@ function MultifactorAuthenticationContextProvider({children}: MultifactorAuthent
8181

8282
const scenario = getScenarioConfig(scenarioName);
8383

84-
send({type: 'INIT', accountID, scenarioName, scenario, payload: params && Object.keys(params).length > 0 ? params : undefined});
84+
send({
85+
type: 'INIT',
86+
accountID,
87+
scenarioName,
88+
scenario,
89+
payload: params && Object.keys(params).length > 0 ? params : undefined,
90+
localCredentialsKnownToServer: startCredentialsState.hasLocalCredentials,
91+
});
8592
};
8693

8794
const closeModal = () => send({type: 'CLOSE_MODAL'});

‎src/components/MultifactorAuthentication/biometrics/operations/index.native.ts‎

Lines changed: 4 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,10 @@
1-
import addMFABreadcrumb from '@components/MultifactorAuthentication/observability/breadcrumbs';
2-
3-
import {decodeLibraryError, getKeyAlias} from '@libs/MultifactorAuthentication/NativeBiometricsHSM/helpers';
4-
import readOnyxValueOnce from '@libs/MultifactorAuthentication/shared/readOnyxValueOnce';
5-
61
import CONST from '@src/CONST';
7-
import ONYXKEYS from '@src/ONYXKEYS';
8-
import Base64URL from '@src/utils/Base64URL';
92

10-
import {getAllKeys, isSensorAvailable} from '@sbaiahmed1/react-native-biometrics';
11-
import {mfaCredentialIDsSelector} from '@selectors/Account';
3+
import {isSensorAvailable} from '@sbaiahmed1/react-native-biometrics';
124

135
/**
14-
* Platform-resolved biometric operations for the MFA machine's pre-screen checks. These functions
15-
* read no React state, so the machine actors and other non-React callers can import them directly.
6+
* Platform-resolved biometric operations for the device check. These functions read no Onyx and no
7+
* React state, so the MFA machine actors and other non-React callers can import them directly.
168
*/
179

1810
/** The authentication method this platform verifies with. Native verifies with HSM-backed biometrics. */
@@ -27,29 +19,4 @@ async function doesDeviceSupportAuthenticationMethod(): Promise<boolean> {
2719
return sensorResult.isDeviceSecure;
2820
}
2921

30-
/** Resolves to the account's HSM-backed credential ID, or undefined when no key exists or the keystore read fails. */
31-
async function getLocalCredentialID(accountID: number): Promise<string | undefined> {
32-
try {
33-
const {keys} = await getAllKeys(getKeyAlias(accountID));
34-
const entry = keys.at(0);
35-
if (!entry) {
36-
return undefined;
37-
}
38-
return Base64URL.base64ToBase64url(entry.publicKey);
39-
} catch (error) {
40-
addMFABreadcrumb('Failed to get local credential ID', decodeLibraryError(error), 'error');
41-
return undefined;
42-
}
43-
}
44-
45-
/** Resolves to whether the account has a local HSM key the server also knows, meaning it can skip registration. */
46-
async function areLocalCredentialsKnownToServer(accountID: number): Promise<boolean> {
47-
const localCredentialID = await getLocalCredentialID(accountID);
48-
if (!localCredentialID) {
49-
return false;
50-
}
51-
const account = await readOnyxValueOnce(ONYXKEYS.ACCOUNT);
52-
return (mfaCredentialIDsSelector(account) ?? []).includes(localCredentialID);
53-
}
54-
55-
export {areLocalCredentialsKnownToServer, deviceVerificationType, deviceCheckFailureReason, doesDeviceSupportAuthenticationMethod};
22+
export {deviceVerificationType, deviceCheckFailureReason, doesDeviceSupportAuthenticationMethod};

‎src/components/MultifactorAuthentication/biometrics/operations/index.ts‎

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,10 @@
11
import {isWebAuthnSupported} from '@libs/MultifactorAuthentication/Passkeys/WebAuthn';
2-
import readOnyxValueOnce from '@libs/MultifactorAuthentication/shared/readOnyxValueOnce';
3-
4-
import {getPasskeyOnyxKey} from '@userActions/Passkey';
52

63
import CONST from '@src/CONST';
7-
import ONYXKEYS from '@src/ONYXKEYS';
8-
9-
import {mfaCredentialIDsSelector} from '@selectors/Account';
104

115
/**
12-
* Platform-resolved biometric operations for the MFA machine's pre-screen checks. These functions
13-
* read no React state, so the machine actors and other non-React callers can import them directly.
6+
* Platform-resolved biometric operations for the device check. These functions read no Onyx and no
7+
* React state, so the MFA machine actors and other non-React callers can import them directly.
148
*/
159

1610
/** The authentication method this platform verifies with. Web verifies with passkeys. */
@@ -24,11 +18,4 @@ async function doesDeviceSupportAuthenticationMethod(): Promise<boolean> {
2418
return isWebAuthnSupported();
2519
}
2620

27-
/** Resolves to whether the account has a local passkey the server also knows, meaning it can skip registration. */
28-
async function areLocalCredentialsKnownToServer(accountID: number): Promise<boolean> {
29-
const [account, localPasskeyCredentials] = await Promise.all([readOnyxValueOnce(ONYXKEYS.ACCOUNT), readOnyxValueOnce(getPasskeyOnyxKey(String(accountID)))]);
30-
const serverKnownCredentialIDs = new Set(mfaCredentialIDsSelector(account) ?? []);
31-
return (localPasskeyCredentials ?? []).some((credential) => serverKnownCredentialIDs.has(credential.id));
32-
}
33-
34-
export {areLocalCredentialsKnownToServer, deviceVerificationType, deviceCheckFailureReason, doesDeviceSupportAuthenticationMethod};
21+
export {deviceVerificationType, deviceCheckFailureReason, doesDeviceSupportAuthenticationMethod};

‎src/components/MultifactorAuthentication/machine/mfaActors.ts‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import checkDeviceEligibility from '@components/MultifactorAuthentication/biometrics/checkDeviceEligibility';
2-
import {areLocalCredentialsKnownToServer} from '@components/MultifactorAuthentication/biometrics/operations';
32

43
import {isHttpSuccess} from '@libs/MultifactorAuthentication/shared/helpers';
54
import type {MFAResult} from '@libs/MultifactorAuthentication/shared/MFAResult';
@@ -10,7 +9,7 @@ import {getDeviceBiometricsOnyxKey, requestRegistrationChallenge} from '@userAct
109

1110
import {fromPromise} from 'xstate';
1211

13-
import type {CheckLocalCredentialsInput, ReadHasAcceptedSoftPromptInput, RequestRegistrationChallengeInput, RequestRegistrationChallengeOutput, ValidateDeviceInput} from './types';
12+
import type {ReadHasAcceptedSoftPromptInput, RequestRegistrationChallengeInput, RequestRegistrationChallengeOutput, ValidateDeviceInput} from './types';
1413

1514
/**
1615
* A refused device resolves as a failed MFAResult, so the machine's onError transition for this
@@ -27,12 +26,6 @@ const readHasAcceptedSoftPrompt = fromPromise<boolean, ReadHasAcceptedSoftPrompt
2726
return deviceBiometrics?.hasAcceptedSoftPrompt ?? false;
2827
});
2928

30-
/**
31-
* Resolves to whether the account's local credentials are known to the server. A returning user
32-
* (true) skips the registration path entirely.
33-
*/
34-
const checkLocalCredentials = fromPromise<boolean, CheckLocalCredentialsInput>(({input}) => areLocalCredentialsKnownToServer(input.accountID));
35-
3629
/**
3730
* Exchanges the submitted magic code for a validated registration challenge. The action normalizes
3831
* backend failures into a reason; the actor exposes them as failed MFA results for machine routing.
@@ -50,7 +43,7 @@ const requestRegistrationChallengeActor = fromPromise<RequestRegistrationChallen
5043
* these working implementations, so no caller needs to provide stubs or overrides.
5144
*/
5245
function createActors() {
53-
return {validateDevice, readHasAcceptedSoftPrompt, checkLocalCredentials, requestRegistrationChallenge: requestRegistrationChallengeActor};
46+
return {validateDevice, readHasAcceptedSoftPrompt, requestRegistrationChallenge: requestRegistrationChallengeActor};
5447
}
5548

5649
export default createActors;

‎src/components/MultifactorAuthentication/machine/mfaMachine.ts‎

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ const DEFAULT_CONTEXT: MfaContext = {
3535
scenarioName: undefined,
3636
scenario: undefined,
3737
payload: undefined,
38+
localCredentialsKnownToServer: false,
3839
validateCode: undefined,
3940
continuableError: undefined,
4041
registrationChallenge: undefined,
@@ -76,6 +77,7 @@ const MFAMachine = setup({
7677
scenarioName: event.scenarioName,
7778
scenario: event.scenario,
7879
payload: event.payload,
80+
localCredentialsKnownToServer: event.localCredentialsKnownToServer,
7981
};
8082
}),
8183
// Deferring the outcome push until the modal-open transition settles lets the screen slide in
@@ -176,25 +178,12 @@ const MFAMachine = setup({
176178
},
177179
},
178180
[MFA_STATE.DECIDING_REGISTRATION]: {
179-
invoke: {
180-
id: 'checkLocalCredentials',
181-
src: 'checkLocalCredentials',
182-
input: ({context}) => {
183-
if (context.accountID === undefined) {
184-
throw new Error('MFA account must be initialized before the registration decision');
185-
}
186-
return {accountID: context.accountID};
187-
},
188-
// A returning user's credentials are already registered, so only a fresh registration asks for a code.
189-
onDone: [
190-
{guard: ({event}) => event.output, target: SOFT_PROMPT_CHECK_TARGET},
191-
{target: MAGIC_CODE_TARGET, actions: ['requestValidateCode', 'navigateToMagicCode']},
192-
],
193-
onError: {
194-
target: OUTCOME_TARGET,
195-
actions: assign({error: ({event}) => createUnhandledExceptionMFAError('Local credentials check', event.error)}),
196-
},
197-
},
181+
// The Provider captures this value once for start telemetry and INIT. Reusing that
182+
// snapshot here avoids a second native keystore read before the first screen appears.
183+
always: [
184+
{guard: ({context}) => context.localCredentialsKnownToServer, target: SOFT_PROMPT_CHECK_TARGET},
185+
{target: MAGIC_CODE_TARGET, actions: ['requestValidateCode', 'navigateToMagicCode']},
186+
],
198187
},
199188
[MFA_STATE.CHECKING_SOFT_PROMPT_ACCEPTANCE]: {
200189
id: MFA_STATE.CHECKING_SOFT_PROMPT_ACCEPTANCE,

‎src/components/MultifactorAuthentication/machine/types.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ type MfaContext = {
3333
/** Additional parameters for the current scenario */
3434
payload: MultifactorAuthenticationScenarioAdditionalParams<MultifactorAuthenticationScenario> | undefined;
3535

36+
/** Whether the local credential captured at flow start is among the server-known credential IDs */
37+
localCredentialsKnownToServer: boolean;
38+
3639
/** Magic code the user entered on this flow's validate-code screen */
3740
validateCode: string | undefined;
3841

@@ -65,6 +68,7 @@ type MultifactorAuthenticationInitEvent<T extends MultifactorAuthenticationScena
6568
scenarioName: T;
6669
scenario: MultifactorAuthenticationScenarioConfigFor<T>;
6770
payload: MultifactorAuthenticationScenarioParams<T> | undefined;
71+
localCredentialsKnownToServer: boolean;
6872
};
6973

7074
/** Events handled by the MFA state machine. */
@@ -82,17 +86,13 @@ type ValidateDeviceInput = {allowedAuthenticationMethods: AllowedAuthenticationM
8286
/** Identifies the per-account Onyx member read by the soft-prompt actor. */
8387
type ReadHasAcceptedSoftPromptInput = {accountID: number};
8488

85-
/** Identifies the account whose local credentials the registration-decision actor checks. */
86-
type CheckLocalCredentialsInput = {accountID: number};
87-
8889
/** Magic code sent to the backend to obtain a registration challenge. */
8990
type RequestRegistrationChallengeInput = {validateCode: string};
9091

9192
/** A successful response must carry the validated registration challenge. */
9293
type RequestRegistrationChallengeOutput = MFAResult<{challenge: RegistrationChallenge}>;
9394

9495
export type {
95-
CheckLocalCredentialsInput,
9696
MfaContext,
9797
MfaEvent,
9898
MfaModalState,

‎tests/unit/components/MultifactorAuthentication/biometricsOperations.test.ts‎

Lines changed: 1 addition & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,18 @@
11
// jest-expo defaults to the ios platform, so this import resolves the native operations module
22
// (operations/index.native.ts), which checks the HSM biometric sensor.
3-
import {
4-
areLocalCredentialsKnownToServer,
5-
deviceCheckFailureReason,
6-
deviceVerificationType,
7-
doesDeviceSupportAuthenticationMethod,
8-
} from '@components/MultifactorAuthentication/biometrics/operations';
3+
import {deviceCheckFailureReason, deviceVerificationType, doesDeviceSupportAuthenticationMethod} from '@components/MultifactorAuthentication/biometrics/operations';
94

105
import VALUES from '@libs/MultifactorAuthentication/VALUES';
116

127
import CONST from '@src/CONST';
13-
import ONYXKEYS from '@src/ONYXKEYS';
14-
15-
import Onyx from 'react-native-onyx';
16-
import waitForBatchedUpdates from 'tests/utils/waitForBatchedUpdates';
178

189
const mockIsSensorAvailable = jest.fn();
19-
const mockGetAllKeys = jest.fn();
2010

2111
jest.mock('@sbaiahmed1/react-native-biometrics', () => ({
2212
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
2313
isSensorAvailable: (...args: unknown[]) => mockIsSensorAvailable(...args),
24-
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
25-
getAllKeys: (...args: unknown[]) => mockGetAllKeys(...args),
2614
}));
2715

28-
const ACCOUNT_ID = 12345;
29-
// The keystore returns the public key as plain base64 while the server stores base64url IDs, so the
30-
// characters below only match after the module's base64url conversion.
31-
const LOCAL_PUBLIC_KEY_BASE64 = 'Ab+/cd==';
32-
const LOCAL_CREDENTIAL_ID = 'Ab-_cd';
33-
3416
describe('biometrics operations (native)', () => {
3517
beforeEach(() => {
3618
jest.clearAllMocks();
@@ -62,39 +44,4 @@ describe('biometrics operations (native)', () => {
6244
await expect(doesDeviceSupportAuthenticationMethod()).resolves.toBe(false);
6345
});
6446
});
65-
66-
describe('areLocalCredentialsKnownToServer', () => {
67-
afterEach(async () => {
68-
await Onyx.clear();
69-
await waitForBatchedUpdates();
70-
});
71-
72-
it('should return true when the local HSM key is among the server-known credential IDs', async () => {
73-
mockGetAllKeys.mockResolvedValue({keys: [{publicKey: LOCAL_PUBLIC_KEY_BASE64}]});
74-
await Onyx.merge(ONYXKEYS.ACCOUNT, {multifactorAuthenticationPublicKeyIDs: ['other-credential-id', LOCAL_CREDENTIAL_ID]});
75-
76-
await expect(areLocalCredentialsKnownToServer(ACCOUNT_ID)).resolves.toBe(true);
77-
});
78-
79-
it('should return false when the server does not know the local HSM key', async () => {
80-
mockGetAllKeys.mockResolvedValue({keys: [{publicKey: LOCAL_PUBLIC_KEY_BASE64}]});
81-
await Onyx.merge(ONYXKEYS.ACCOUNT, {multifactorAuthenticationPublicKeyIDs: ['other-credential-id']});
82-
83-
await expect(areLocalCredentialsKnownToServer(ACCOUNT_ID)).resolves.toBe(false);
84-
});
85-
86-
it('should return false when the device holds no key for the account', async () => {
87-
mockGetAllKeys.mockResolvedValue({keys: []});
88-
await Onyx.merge(ONYXKEYS.ACCOUNT, {multifactorAuthenticationPublicKeyIDs: [LOCAL_CREDENTIAL_ID]});
89-
90-
await expect(areLocalCredentialsKnownToServer(ACCOUNT_ID)).resolves.toBe(false);
91-
});
92-
93-
it('should return false when the keystore read throws', async () => {
94-
mockGetAllKeys.mockRejectedValue(new Error('Keystore unavailable'));
95-
await Onyx.merge(ONYXKEYS.ACCOUNT, {multifactorAuthenticationPublicKeyIDs: [LOCAL_CREDENTIAL_ID]});
96-
97-
await expect(areLocalCredentialsKnownToServer(ACCOUNT_ID)).resolves.toBe(false);
98-
});
99-
});
10047
});

‎tests/unit/components/MultifactorAuthentication/biometricsOperationsWeb.test.ts‎

Lines changed: 1 addition & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -6,22 +6,13 @@
66
*/
77
import type * as WebBiometricsOperations from '@components/MultifactorAuthentication/biometrics/operations/index';
88

9-
import {getPasskeyOnyxKey} from '@userActions/Passkey';
10-
119
import CONST from '@src/CONST';
12-
import ONYXKEYS from '@src/ONYXKEYS';
13-
14-
import Onyx from 'react-native-onyx';
15-
import waitForBatchedUpdates from 'tests/utils/waitForBatchedUpdates';
1610

1711
// jest-expo resolves the native variant by default, so load the web entry point explicitly.
18-
const {areLocalCredentialsKnownToServer, deviceCheckFailureReason, deviceVerificationType, doesDeviceSupportAuthenticationMethod} = jest.requireActual<typeof WebBiometricsOperations>(
12+
const {deviceCheckFailureReason, deviceVerificationType, doesDeviceSupportAuthenticationMethod} = jest.requireActual<typeof WebBiometricsOperations>(
1913
'@components/MultifactorAuthentication/biometrics/operations/index.ts',
2014
);
2115

22-
const ACCOUNT_ID = 12345;
23-
const LOCAL_PASSKEY_ID = 'local-passkey-credential-id';
24-
2516
const originalPublicKeyCredentialDescriptor = Object.getOwnPropertyDescriptor(window, 'PublicKeyCredential');
2617

2718
function setWebAuthnSupport(isSupported: boolean) {
@@ -57,31 +48,4 @@ describe('biometrics operations (web)', () => {
5748

5849
await expect(doesDeviceSupportAuthenticationMethod()).resolves.toBe(expected);
5950
});
60-
61-
describe('areLocalCredentialsKnownToServer', () => {
62-
afterEach(async () => {
63-
await Onyx.clear();
64-
await waitForBatchedUpdates();
65-
});
66-
67-
it('returns true when a local passkey is among the server-known credential IDs', async () => {
68-
await Onyx.merge(ONYXKEYS.ACCOUNT, {multifactorAuthenticationPublicKeyIDs: ['other-credential-id', LOCAL_PASSKEY_ID]});
69-
await Onyx.set(getPasskeyOnyxKey(String(ACCOUNT_ID)), [{id: LOCAL_PASSKEY_ID, type: CONST.PASSKEY_CREDENTIAL_TYPE}]);
70-
71-
await expect(areLocalCredentialsKnownToServer(ACCOUNT_ID)).resolves.toBe(true);
72-
});
73-
74-
it('returns false when the server does not know the local passkey', async () => {
75-
await Onyx.merge(ONYXKEYS.ACCOUNT, {multifactorAuthenticationPublicKeyIDs: ['other-credential-id']});
76-
await Onyx.set(getPasskeyOnyxKey(String(ACCOUNT_ID)), [{id: LOCAL_PASSKEY_ID, type: CONST.PASSKEY_CREDENTIAL_TYPE}]);
77-
78-
await expect(areLocalCredentialsKnownToServer(ACCOUNT_ID)).resolves.toBe(false);
79-
});
80-
81-
it('returns false when the account has no local passkeys', async () => {
82-
await Onyx.merge(ONYXKEYS.ACCOUNT, {multifactorAuthenticationPublicKeyIDs: [LOCAL_PASSKEY_ID]});
83-
84-
await expect(areLocalCredentialsKnownToServer(ACCOUNT_ID)).resolves.toBe(false);
85-
});
86-
});
8751
});

0 commit comments

Comments
 (0)