Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implement Biometrics Authentication #125

Merged
merged 24 commits into from
Apr 13, 2022
Merged

Implement Biometrics Authentication #125

merged 24 commits into from
Apr 13, 2022

Conversation

nicholemnl
Copy link
Collaborator

Fixes /issues/27

@nicholemnl nicholemnl self-assigned this Mar 22, 2022
@nicholemnl nicholemnl marked this pull request as ready for review March 22, 2022 06:13
@nicholemnl
Copy link
Collaborator Author

nicholemnl commented Mar 22, 2022 •
edited
Loading

@danicaerediano found issues:

  • Had issue with logout/login
  • Many attempts of biometrics will ask for device pin instead (check if we can handle this)

Will change this to draft for the mean time

@nicholemnl nicholemnl marked this pull request as draft March 22, 2022 11:02
@nicholemnl nicholemnl marked this pull request as ready for review March 23, 2022 06:45
danicaerediano
danicaerediano approved these changes Mar 23, 2022
View reviewed changes
Copy link
Collaborator

@danicaerediano danicaerediano left a comment

Choose a reason for hiding this comment

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

LGTM

pmigueld
pmigueld requested changes Mar 24, 2022
View reviewed changes
machines/biometrics.ts Outdated Show resolved Hide resolved
machines/biometrics.ts Outdated Show resolved Hide resolved
machines/biometrics.ts Show resolved Hide resolved
pmigueld
pmigueld reviewed Mar 24, 2022
View reviewed changes
machines/biometrics.ts Outdated Show resolved Hide resolved
pmigueld
pmigueld reviewed Mar 24, 2022
View reviewed changes
machines/biometrics.ts Outdated Show resolved Hide resolved
@nicholemnl nicholemnl force-pushed the feature-27-capture-biometrics branch from 5b1b8fe to d81b6d0 Compare March 24, 2022 10:34
@nicholemnl nicholemnl requested a review from pmigueld March 25, 2022 03:29
@nicholemnl nicholemnl force-pushed the feature-27-capture-biometrics branch from 7f880fb to c9fa1fe Compare March 28, 2022 10:39
@kneckinator
Copy link
Collaborator

@pmigueld are there are outstanding issues with this PR?
@nicholemnl could you please resolve the conflict with package-lock.json? Thanks.

@pmigueld
Copy link
Collaborator

pmigueld commented Apr 6, 2022

I was just waiting if @nicholemnl is done with his changes. Is it ready for review again?

@nicholemnl
Copy link
Collaborator Author

nicholemnl commented Apr 6, 2022 •
edited
Loading

I was just waiting if @nicholemnl is done with his changes. Is it ready for review again?

yes please. Thanks @pmigueld

@nicholemnl nicholemnl force-pushed the feature-27-capture-biometrics branch from c9fa1fe to 3e411fa Compare April 6, 2022 02:24
@nicholemnl

This comment was marked as duplicate.

Sign in to view
pmigueld
pmigueld requested changes Apr 7, 2022
View reviewed changes
Copy link
Collaborator

@pmigueld pmigueld left a comment

Choose a reason for hiding this comment

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

The toggle in the profile settings is not working:

image

@nicholemnl nicholemnl marked this pull request as draft April 7, 2022 08:15
@nicholemnl nicholemnl force-pushed the feature-27-capture-biometrics branch from 3e411fa to d96e907 Compare April 7, 2022 09:41
@nicholemnl nicholemnl requested a review from pmigueld April 7, 2022 12:00
@nicholemnl nicholemnl marked this pull request as ready for review April 7, 2022 12:00
danicaerediano
danicaerediano approved these changes Apr 12, 2022
View reviewed changes
Copy link
Collaborator

@danicaerediano danicaerediano left a comment

Choose a reason for hiding this comment

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

Biometric unlock now works as expected, LGTM

screens/Profile/ProfileScreenController.ts Outdated Show resolved Hide resolved
@pmigueld
Copy link
Collaborator

The biometrics toggle in the profile screen still shows it's enabled after disabling fingerprint unlock from the phone settings (it correctly falls back to the PIN input when logging in)

pmigueld
pmigueld approved these changes Apr 13, 2022
View reviewed changes
Copy link
Collaborator

@pmigueld pmigueld left a comment

Choose a reason for hiding this comment

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

Biometric unlock now works as expected 👍

Tested on: Redmi 9C, Android 10

@pmigueld pmigueld merged commit 0521fa6 into develop Apr 13, 2022
@pmigueld pmigueld deleted the feature-27-capture-biometrics branch April 13, 2022 05:44
@pmigueld pmigueld mentioned this pull request May 20, 2022
Merged
pmigueld pushed a commit that referenced this pull request Nov 9, 2022
@gsasikumar
Merge pull request #125 from ckm007/develop
2b4cf11 
[DSD-1451] added gpg keys for build fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pmigueld pmigueld pmigueld approved these changes

@danicaerediano danicaerediano danicaerediano approved these changes

Assignees

@nicholemnl nicholemnl

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Capture biometrics
6 participants
@nicholemnl @kneckinator @pmigueld @danicaerediano @danicaMat @rjmangubat23