[lib][native][web] Introduce usePasswordLogIn hook
ClosedPublic
Actions

Authored by ashoat on Wed, Apr 24, 12:42 PM.

Details

Reviewers

tomek
inka

Commits

rCOMMb1dff3d63e23: [lib][native][web] Introduce usePasswordLogIn hook

Summary

This addresses ENG-7681, ENG-7671, and ENG-7673.

This furthers the goal of the parent task, which is to move authoritative keyserver auth from KeyserverConnectionHandler to the individual components responsible for login.

Depends on D11763

Test Plan

This whole stack was tested with the following steps:

On native, attempting to log in with an ETH account that hasn’t been registered yet
On native, attemping to register with an ETH account that has already been registered
On native, log in with an ETH account that has already been registered
On native, register with an ETH account that hasn’t already been registered
On native, register with a password account
On native, log in with a password account
On web, attempting to log in with an ETH account that hasn’t been registered yet
On web, log in with an ETH account that has already been registered
On web, log in with a password account

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ashoat created this revision.Wed, Apr 24, 12:42 PM

ashoat added a child revision: D11766: [lib][native] Defer saving user credentials until successful auth.

ashoat added inline comments.Wed, Apr 24, 12:49 PM

lib/hooks/login-hooks.js
31–33 ↗	(On Diff #39451)	In the next diff (D11766) I end up removing this in favor of just calling `saveCredentials` at the end (after keyserver auth), but I decided to leave it in here in case that diff needs to get reverted for some reason

Harbormaster completed remote builds in B28441: Diff 39451.Wed, Apr 24, 1:06 PM

ashoat requested review of this revision.Wed, Apr 24, 1:06 PM

inka added inline comments.Thu, Apr 25, 3:47 AM

lib/hooks/login-hooks.js
57–59 ↗	(On Diff #39451)	In D11763 we check if step is `inactive`. Do we need to do that here?
63 ↗	(On Diff #39451)	This probably shouldn't be named "registration". And, same as in D11763, I think this shouldn't be named "waiting for", when later it represents the state after registration has succeeded. Also because we don't have a third step to represent the keyserver auth, I think we actually care about two states - "no login in progress" and "identity login concluded, keyserver login in progress"

ashoat added inline comments.Thu, Apr 25, 5:55 AM

lib/hooks/login-hooks.js
57–59 ↗	(On Diff #39451)	Good call! Yes, I'll make that change
63 ↗	(On Diff #39451)	This probably shouldn't be named "registration" Good point, I'll make this change And, same as in D11763, I think this shouldn't be named "waiting for", when later it represents the state after registration has succeeded. As described there, we're really waiting for data to appear in Redux, but I thought that name was a bit of a mouthful. Open to other suggestions, but opposed to `identity_registration` here because we're really doing keyserver auth during this step.

waiting_for_identity_registration -> waiting_for_identity_login_in_redux
Add a check that step is inactive in order to avoid triggering auth multiple times

Harbormaster completed remote builds in B28471: Diff 39499.Thu, Apr 25, 10:15 AM

inka accepted this revision.Fri, Apr 26, 5:57 AM

This revision is now accepted and ready to land.Fri, Apr 26, 5:57 AM

waiting_for_identity_login_in_redux -> identity_login_dispatched

This revision was landed with ongoing or failed builds.Fri, Apr 26, 8:18 AM

Closed by commit rCOMMb1dff3d63e23: [lib][native][web] Introduce usePasswordLogIn hook (authored by ashoat). · Explain Why

This revision was automatically updated to reflect the committed changes.

ashoat added a commit: rCOMMb1dff3d63e23: [lib][native][web] Introduce usePasswordLogIn hook.

Harbormaster completed remote builds in B28514: Diff 39550.Fri, Apr 26, 8:26 AM