[PM-30394] PM-29960: Skip biometric prompt on Xiaomi HyperOS #6316
Conversation
Gavin-Guiii
requested review from
a team,
brian-livefront and
david-livefront
as code owners
|
|
|
|
|
Thank you for your contribution! We've added this to our internal tracking system for review. Details on our contribution process can be found here: https://contributing.bitwarden.com/contributing/pull-requests/community-pr-process. |
bitwarden-bot
changed the title
SaintPatrck
left a comment
SaintPatrck
left a comment
There was a problem hiding this comment.
Confirmed with our product team that blocking HyperOS is an acceptable approach.
Changes look good, overall. One minor nitpick about the newline.
| } catch (_: Throwable) { | ||
| null | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
⛏️ All files must end in a newline.
There was a problem hiding this comment.
⛏️ All files must end in a newline.
Thank you! Just updated the file.
|
Great job! No new security vulnerabilities introduced in this pull request |
| cipher: Cipher?, | ||
| ): PasswordCredentialEntry.Builder = | ||
| if (isBuildVersionAtLeast(Build.VERSION_CODES.VANILLA_ICE_CREAM) && cipher != null) { | ||
| if (isBuildVersionAtLeast(Build.VERSION_CODES.VANILLA_ICE_CREAM) && !isHyperOS() && cipher != null) { |
There was a problem hiding this comment.
The linter is mad about the line length here, you can probably break this out into a private method that can be used in both places.
private fun shouldSetBiometricPromptData(
cipher: Cipher?,
): Boolean {
return isBuildVersionAtLeast(Build.VERSION_CODES.VANILLA_ICE_CREAM) &&
!isHyperOS() &&
cipher != null
}There was a problem hiding this comment.
Thanks. I broke it out into this and kept the cipher null check in the original functions, so the compiler knows cipher isn't null. Hope that's okay.
private fun isBiometricPromptDataSupported(): Boolean {
return isBuildVersionAtLeast(Build.VERSION_CODES.VANILLA_ICE_CREAM) &&
!isHyperOS()
}| null | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Another minor lint thing, it does not like consecutive blank lines. There should just be one here.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6316 +/- ##
==========================================
- Coverage 84.93% 84.87% -0.07%
==========================================
Files 876 943 +67
Lines 56942 57773 +831
Branches 7900 7979 +79
==========================================
+ Hits 48366 49034 +668
- Misses 5833 5967 +134
- Partials 2743 2772 +29 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
5 participants
🎟️ Tracking
#6284
📔 Objective
Xiaomi HyperOS changed AOSP's credential manager and broke BiometricPrompt. One way to fix it is to skip biometric prompt handling if Bitwarden is running on Hyper OS.
See more details here: #6284 (comment)
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes