Skip to content

fix: stop reporting an unrecognized biometric as a final failure - #1098

Merged
pmathew92 merged 7 commits into
v3.21.2from
fix/github-1095
Oct 21, 2026
Merged

pmathew92 merged 7 commits into
v3.21.2from
fix/github-1095

Conversation

@pmathew92

@pmathew92 pmathew92 commented Oct 19, 2026 •

Copy link
Copy Markdown
Member

Changes

SecureCredentialsManager with LocalAuthenticationOptions could deliver two results for a single getCredentials() / getApiCredentials() call when the user presented an unrecognized biometric and then authenticated successfully (for example via the device credential fallback).

LocalAuthenticationManager.onAuthenticationFailed() called resultCallback.onFailure(BIOMETRICS_INVALID_USER). BiometricPrompt invokes onAuthenticationFailed() when a biometric is read but not recognized; it is not a terminal event, and the prompt stays open. The terminal result always arrives later via onAuthenticationSucceeded() or onAuthenticationError().

Impact before this change:

  • Callback API: the caller received onFailure(BIOMETRICS_INVALID_USER) while the prompt was still open, then onSuccess after the user authenticated. Callers that settle on the first result (e.g. react-native-auth0) showed a failure and dropped the success, while the biometric session was still started.
  • Coroutine API (awaitCredentials / awaitApiCredentials): the continuation was resumed twice, throwing IllegalStateException: Already resumed. runCatchingOnExecutor converted that into an UNKNOWN_ERROR failure, which attempted a third resume and threw again from the executor thread.

References

Testing

New unit tests in LocalAuthenticationManagerTest:

  • An unrecognized biometric alone delivers no result.
  • An unrecognized biometric followed by success delivers only onSuccess(true).
  • An unrecognized biometric followed by ERROR_LOCKOUT delivers only BIOMETRIC_ERROR_LOCKOUT.

New coroutine tests in SecureCredentialsManagerTest, using a real LocalAuthenticationManager (spy with only authenticate() stubbed) so the actual onAuthenticationFailed() path is exercised:

  • awaitCredentials() returns credentials when an unrecognized biometric is followed by success.
  • awaitCredentials() throws only BIOMETRIC_ERROR_LOCKOUT when an unrecognized biometric is followed by lockout.
  • awaitApiCredentials() returns API credentials when an unrecognized biometric is followed by success.

With the old onFailure call restored, all six new tests fail, and the three coroutine tests fail with IllegalStateException: Already resumed. With the fix, LocalAuthenticationManagerTest (40) and SecureCredentialsManagerTest (135) pass.

  • This change adds unit test coverage

  • This change adds integration test coverage

  • This change has been tested on the latest version of the platform/language or why not

Checklist

@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Team
  • Run ID: 52e42b52-44d0-459b-97e1-fbc559c4a520

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@pmathew92
pmathew92 marked this pull request as ready for review October 19, 2026 15:38
@pmathew92
pmathew92 requested a review from a team as a code owner October 19, 2026 15:38
@pmathew92 pmathew92 changed the title fix(storage): stop reporting an unrecognized biometric as a final failure [non-main] fix: stop reporting an unrecognized biometric as a final failure Oct 19, 2026

@Test
@ExperimentalCoroutinesApi
public fun shouldThrowOnlyTheFinalErrorOnAwaitCredentialsWhenUnrecognizedBiometricIsFollowedByError(): Unit =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The bug was that a biometric session got started even though the app saw a failure. Could we also check the session here: invalid after lockout, valid after success?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added case for the same

* unrecognized biometric before delivering [finalResult], mirroring how BiometricPrompt
* keeps the prompt open after a failed attempt.
*/
private fun simulatePromptWithUnrecognizedBiometric(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we add one test of the callback API (getCredentials(callback)) using it, checking that onSuccess is called exactly once and onFailure is never called? That's the path react-native-auth0 actually uses, and right now it's only covered at the LocalAuthenticationManager level.

@pmathew92 pmathew92 Oct 21, 2026 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added in ac017cb: shouldDeliverOnlySuccessAndStartBiometricSessionWhenUnrecognizedBiometricIsFollowedBySuccess drives getCredentials(callback) through a real LocalAuthenticationManager (not recognized → success) and verifies onSuccess exactly once and onFailure never. It also runs under BiometricPolicy.Session and asserts the session is valid afterwards .

@pmathew92
pmathew92 merged commit 12c5720 into v3.21.2 Oct 21, 2026
7 checks passed
@pmathew92
pmathew92 deleted the fix/github-1095 branch October 21, 2026 05:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants