Skip to content

fix keeping the wallet unlocked after service worker restarted more than once#17950

Merged
pedronfigueiredo merged 1 commit intodevelopfrom
fix/keep-user-login
Mar 1, 2023
Merged

fix keeping the wallet unlocked after service worker restarted more than once#17950
pedronfigueiredo merged 1 commit intodevelopfrom
fix/keep-user-login

Conversation

@pedronfigueiredo
Copy link
Copy Markdown
Contributor

@pedronfigueiredo pedronfigueiredo commented Mar 1, 2023

Explanation

We want to keep the wallet unlocked when a service worker restarts.

In submitEncryptionKey on the metamask-controller.js, the vault is a string, not an object. So to correctly compare the salt, we need to parse that string into JSON.

Note that this PR does not fix the first time the service worker is restarted after a fresh wallet is loaded and the onboarding flow is complete. This will be fixed in a separate PR. However, in any number of subsequent restarts, the unlock state is now persisted.

For context, this was previously addressed, but the wallet was still locked after a long enough waiting period after the service worker restarted. Another PR merged immediately after made the wallet lock more immediate.

Manual Testing Steps

  1. Build the wallet from develop

git checkout develop && git pull && yarn && yarn:start:mv3

  1. Load the wallet in the browser and complete the onboarding flow

  2. Terminate the service worker using chrome://inspect/#service-workers.

  3. The wallet should lock after a couple of seconds.

  4. Input the password and unlock the wallet

  5. Terminate the service worker again (repeat step 3)

  6. The wallet should lock after a couple of seconds.

  7. Build the wallet from this branch

git checkout fix/keep-user-login && git pull && yarn && yarn:start:mv3

  1. Load the wallet in the browser and complete the onboarding flow

  2. Terminate the service worker using chrome://inspect/#service-workers.

  3. The wallet should lock after a couple of seconds.

  4. Input the password and unlock the wallet

  5. Terminate the service worker again (repeat step 11)

  6. The wallet should remain unlocked.

Pre-merge author checklist

  • I've clearly explained:
    • What problem this PR is solving
    • How this problem was solved
    • How reviewers can test my changes
  • Sufficient automated test coverage has been added

Pre-merge reviewer checklist

  • Manual testing (e.g. pull and build branch, run in browser, test code being changed)
  • PR is linked to the appropriate GitHub issue
  • IF this PR fixes a bug in the release milestone, add this PR to the release milestone

If further QA is required (e.g. new feature, complex testing steps, large refactor), add the Extension QA Board label.

In this case, a QA Engineer approval will be be required.

@pedronfigueiredo pedronfigueiredo added type-bug Something isn't working team-extension-client team-extension-platform Extension Platform team labels Mar 1, 2023
@pedronfigueiredo pedronfigueiredo requested a review from a team as a code owner March 1, 2023 18:02
@pedronfigueiredo pedronfigueiredo self-assigned this Mar 1, 2023
@pedronfigueiredo pedronfigueiredo requested a review from jpuri March 1, 2023 18:02
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 1, 2023

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@pedronfigueiredo pedronfigueiredo changed the title fix keeping the user login after the first login fix keeping the wallet unlocked after service worker restarted more than once Mar 1, 2023
@pedronfigueiredo pedronfigueiredo merged commit eb2efc1 into develop Mar 1, 2023
@pedronfigueiredo pedronfigueiredo deleted the fix/keep-user-login branch March 1, 2023 18:20
@github-actions github-actions bot locked and limited conversation to collaborators Mar 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

team-extension-platform Extension Platform team type-bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants