Skip to content

Commit a158233

Browse files
committed
Revert "refactor(mfa): decide registration from the credentials snapshot captured at flow start"
This reverts commit 550345f.
1 parent 550345f commit a158233

15 files changed

Lines changed: 263 additions & 70 deletions

File tree

src/components/MultifactorAuthentication/Context/MultifactorAuthenticationMainContext.tsx

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

8282
const scenario = getScenarioConfig(scenarioName);
8383

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-
});
84+
send({type: 'INIT', accountID, scenarioName, scenario, payload: params && Object.keys(params).length > 0 ? params : undefined});
9285
};
9386

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

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

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,18 @@
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+
16
import CONST from '@src/CONST';
7+
import ONYXKEYS from '@src/ONYXKEYS';
8+
import Base64URL from '@src/utils/Base64URL';
29

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

513
/**
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.
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.
816
*/
917

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

22-
export {deviceVerificationType, deviceCheckFailureReason, doesDeviceSupportAuthenticationMethod};
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};

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

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

36
import CONST from '@src/CONST';
7+
import ONYXKEYS from '@src/ONYXKEYS';
8+
9+
import {mfaCredentialIDsSelector} from '@selectors/Account';
410

511
/**
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.
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.
814
*/
915

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

21-
export {deviceVerificationType, deviceCheckFailureReason, doesDeviceSupportAuthenticationMethod};
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};

src/components/MultifactorAuthentication/machine/mfaActors.ts

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

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

1011
import {fromPromise} from 'xstate';
1112

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

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

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+
2936
/**
3037
* Exchanges the submitted magic code for a validated registration challenge. The action normalizes
3138
* backend failures into a reason; the actor exposes them as failed MFA results for machine routing.
@@ -43,7 +50,7 @@ const requestRegistrationChallengeActor = fromPromise<RequestRegistrationChallen
4350
* these working implementations, so no caller needs to provide stubs or overrides.
4451
*/
4552
function createActors() {
46-
return {validateDevice, readHasAcceptedSoftPrompt, requestRegistrationChallenge: requestRegistrationChallengeActor};
53+
return {validateDevice, readHasAcceptedSoftPrompt, checkLocalCredentials, requestRegistrationChallenge: requestRegistrationChallengeActor};
4754
}
4855

4956
export default createActors;

src/components/MultifactorAuthentication/machine/mfaMachine.ts

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ const DEFAULT_CONTEXT: MfaContext = {
3535
scenarioName: undefined,
3636
scenario: undefined,
3737
payload: undefined,
38-
localCredentialsKnownToServer: false,
3938
validateCode: undefined,
4039
continuableError: undefined,
4140
registrationChallenge: undefined,
@@ -77,7 +76,6 @@ const MFAMachine = setup({
7776
scenarioName: event.scenarioName,
7877
scenario: event.scenario,
7978
payload: event.payload,
80-
localCredentialsKnownToServer: event.localCredentialsKnownToServer,
8179
};
8280
}),
8381
// Deferring the outcome push until the modal-open transition settles lets the screen slide in
@@ -178,12 +176,25 @@ const MFAMachine = setup({
178176
},
179177
},
180178
[MFA_STATE.DECIDING_REGISTRATION]: {
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-
],
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+
},
187198
},
188199
[MFA_STATE.CHECKING_SOFT_PROMPT_ACCEPTANCE]: {
189200
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,9 +33,6 @@ 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-
3936
/** Magic code the user entered on this flow's validate-code screen */
4037
validateCode: string | undefined;
4138

@@ -68,7 +65,6 @@ type MultifactorAuthenticationInitEvent<T extends MultifactorAuthenticationScena
6865
scenarioName: T;
6966
scenario: MultifactorAuthenticationScenarioConfigFor<T>;
7067
payload: MultifactorAuthenticationScenarioParams<T> | undefined;
71-
localCredentialsKnownToServer: boolean;
7268
};
7369

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

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

9291
/** A successful response must carry the validated registration challenge. */
9392
type RequestRegistrationChallengeOutput = MFAResult<{challenge: RegistrationChallenge}>;
9493

9594
export type {
95+
CheckLocalCredentialsInput,
9696
MfaContext,
9797
MfaEvent,
9898
MfaModalState,

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

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,36 @@
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 {deviceCheckFailureReason, deviceVerificationType, doesDeviceSupportAuthenticationMethod} from '@components/MultifactorAuthentication/biometrics/operations';
3+
import {
4+
areLocalCredentialsKnownToServer,
5+
deviceCheckFailureReason,
6+
deviceVerificationType,
7+
doesDeviceSupportAuthenticationMethod,
8+
} from '@components/MultifactorAuthentication/biometrics/operations';
49

510
import VALUES from '@libs/MultifactorAuthentication/VALUES';
611

712
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';
817

918
const mockIsSensorAvailable = jest.fn();
19+
const mockGetAllKeys = jest.fn();
1020

1121
jest.mock('@sbaiahmed1/react-native-biometrics', () => ({
1222
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
1323
isSensorAvailable: (...args: unknown[]) => mockIsSensorAvailable(...args),
24+
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
25+
getAllKeys: (...args: unknown[]) => mockGetAllKeys(...args),
1426
}));
1527

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+
1634
describe('biometrics operations (native)', () => {
1735
beforeEach(() => {
1836
jest.clearAllMocks();
@@ -44,4 +62,39 @@ describe('biometrics operations (native)', () => {
4462
await expect(doesDeviceSupportAuthenticationMethod()).resolves.toBe(false);
4563
});
4664
});
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+
});
47100
});

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

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

9+
import {getPasskeyOnyxKey} from '@userActions/Passkey';
10+
911
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';
1016

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

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

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

4958
await expect(doesDeviceSupportAuthenticationMethod()).resolves.toBe(expected);
5059
});
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+
});
5187
});

0 commit comments

Comments
 (0)