Skip to content

[local-authentication][android] fix: negative text must be set and non-empty error - #32076

Merged
behenate merged 1 commit into
expo:mainfrom
dcangulo:dcangulo-patch-1
Oct 17, 2024
Merged

[local-authentication][android] fix: negative text must be set and non-empty error#32076
behenate merged 1 commit into
expo:mainfrom
dcangulo:dcangulo-patch-1

Conversation

@dcangulo

Copy link
Copy Markdown
Contributor

Why

LocalAuthentication.authenticateAsync on Android crashes when disableDeviceFallback is true. This line specifically: https://github.com/expo/expo/blob/main/packages/expo-local-authentication/android/src/main/java/expo/modules/localauthentication/LocalAuthenticationModule.kt#L225

Closes #32075 and #30556

How

The docs (https://docs.expo.dev/versions/latest/sdk/local-authentication/#localauthenticationoptions), says that the cancelLabel has a default value of Cancel.

I just made it explicit so that it will have the default value whether disableDeviceFallback is true or false and it won't cause a crash due to not providing it.

Test Plan

This code must not crash on Android:

LocalAuthentication.authenticateAsync({
  promptMessage: 'Setup your biometric login',
  disableDeviceFallback: true,
})

Checklist

@github-actions

Copy link
Copy Markdown
Contributor

Subscribed to pull request

File Patterns Mentions
packages/expo-local-authentication/** @behenate

Generated by CodeMention

@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label Oct 16, 2024
@behenate
behenate self-requested a review October 16, 2024 10:35
@behenate behenate changed the title fix: negative text must be set and non-empty error [local-authentication][android] fix: negative text must be set and non-empty error Oct 16, 2024

@behenate behenate left a comment

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.

Hi, looks good! Can you just add an entry to to packages/expo-local-authentication/CHANGELOG.md before we merge?

@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Oct 16, 2024
@dcangulo

Copy link
Copy Markdown
Contributor Author

Hi @behenate, I added an entry now. Thanks.

@behenate

Copy link
Copy Markdown
Member

@dcangulo You will also have to run yarn build in the packages/expo-local-authentication folder an commit the changes from the build folder 😅 We should be good to go then, sorry I didn't notice the first time

@dcangulo

Copy link
Copy Markdown
Contributor Author

@behenate Done!

@behenate

Copy link
Copy Markdown
Member

Sorry last thing, there seems to be a lint error:
run yarn lint --fix in packages/expo-local-authentication
and then yarn build in packages/expo-local-authentication
after you commit the changes run
et cp expo-local-authentication to be sure that the check passes this time 😄

@dcangulo

Copy link
Copy Markdown
Contributor Author

@behenate Done!

@MisterCommit

Copy link
Copy Markdown

jus add cancelLabel: "Cancel",

const biometric = await authenticateAsync({
biometricsSecurityLevel: "strong",
disableDeviceFallback: true,
cancelLabel: "Cancel",
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot: passed checks ExpoBot has nothing to complain about

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[expo-local-authentication] Android crashes when disableDeviceFallback is true

4 participants