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.

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

Lock KeyringController on logout #1729

Merged
merged 7 commits into from
Aug 12, 2020
Merged

Conversation

rickycodes
Copy link
Member

@rickycodes rickycodes commented Jul 28, 2020

based on feedback from @Gudahtt we now lock the KeyringController when we logout and when we lockApp

@rickycodes rickycodes requested a review from a team as a code owner July 28, 2020 16:34
@rickycodes rickycodes added the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Jul 29, 2020
Copy link
Member

@andrepimenta andrepimenta left a comment

Choose a reason for hiding this comment

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

LGTM!

@andrepimenta andrepimenta added the needs-qa Any New Features that needs a full manual QA prior to being added to a release. label Jul 29, 2020
@omnat omnat removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Jul 29, 2020
@rickycodes rickycodes force-pushed the feature/lock-KeyringController branch 4 times, most recently from 40bde65 to 757b802 Compare July 31, 2020 13:36
Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

LGTM!

@rickycodes rickycodes force-pushed the feature/lock-KeyringController branch from 757b802 to 891e465 Compare July 31, 2020 15:13
Copy link
Contributor

@ibrahimtaveras00 ibrahimtaveras00 left a comment

Choose a reason for hiding this comment

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

The only issue I've found here is on iOS

  • I came from the testflight build 0.2.19 and then synced with extension with Face ID enabled
  • I then built this branch and built to the real device
  • Once I was prompted for Face ID I saw the check mark, however, it didn't unlock the app

seen here = https://recordit.co/IbI0JRu9jN

I couldn't reproduce this on Android with an imported account nor fingerprint with a wallet synced with extension = https://recordit.co/sSRW7PpGgr

@ibrahimtaveras00 ibrahimtaveras00 added QA'd - Issues Found QA has been complete, however issues have been discovered that need to be addressed and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Aug 11, 2020
Copy link
Contributor

@ibrahimtaveras00 ibrahimtaveras00 left a comment

Choose a reason for hiding this comment

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

QA Passed

@ibrahimtaveras00 ibrahimtaveras00 added QA Passed A successful QA run through has been done and removed QA'd - Issues Found QA has been complete, however issues have been discovered that need to be addressed labels Aug 12, 2020
@andrepimenta andrepimenta merged commit 4ff28c6 into develop Aug 12, 2020
@andrepimenta andrepimenta deleted the feature/lock-KeyringController branch August 12, 2020 17:06
rickycodes added a commit that referenced this pull request Jan 31, 2022
* Lock KeyringController on logout

* Also call .setLocked() on KeyringController for LockScreen

* Lock KeyringController on lockApp() instead

* Undo async

* Add setLockedErrora and gotoLockScreen methods

Co-authored-by: Andre Pimenta <andrepimenta7@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next release QA Passed A successful QA run through has been done
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants