Page MenuHomePhabricator
Differential D14151

[native] add callback to remove secondary device during QR auth
ClosedPublic
Actions

Authored by kamil on Thu, Dec 12, 11:26 AM.
Tags
None
Referenced Files
F3516104: D14151.id46552.diff
Sun, Dec 22, 12:15 PM
Unknown Object (File)
Fri, Dec 20, 4:00 PM
Unknown Object (File)
Thu, Dec 19, 8:36 PM
Unknown Object (File)
Wed, Dec 18, 3:49 PM
Unknown Object (File)
Wed, Dec 18, 10:22 AM
Unknown Object (File)
Mon, Dec 16, 5:19 PM
Subscribers
None

Details

Reviewers
tomek
ashoat
Commits
rCOMM7be865dbb8df: [native] add callback to remove secondary device during QR auth
Summary

ENG-9959.

According to this we want to be able to remove the device on a timeout.

Depends on D14150

Test Plan

Call this callback when QR auth flow is finished - devices is removed from device list, secondary device is visually logged out.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

kamil created this revision.Thu, Dec 12, 11:26 AM
kamil held this revision as a draft.
kamil edited the summary of this revision. (Show Details)Thu, Dec 12, 11:27 AM
kamil added inline comments.
native/account/qr-auth/qr-auth-context-provider.js
249

Actually, we should throw an error here

kamil added a child revision: D14152: [native] implement screen when secondary device is not responding.Thu, Dec 12, 11:34 AM
kamil added inline comments.Thu, Dec 12, 11:37 AM
native/account/qr-auth/qr-auth-context-provider.js
60–62

This is just in case to ignore messages when there is no ongoing QR auth, e.g., Secondary responded,d but we're already in the process of removing it and we communicated to the user that it is not responding.

Harbormaster completed remote builds in B33065: Diff 46386.Thu, Dec 12, 11:44 AM
kamil published this revision for review.Thu, Dec 12, 11:57 AM
ashoat resigned from this revision.Thu, Dec 12, 12:22 PM

Will defer to @tomek on this one – I think it's probably a better bet for him to gain the context on QRAuthContextProvider than for me

tomek accepted this revision.Fri, Dec 13, 6:26 AM
tomek added inline comments.
native/account/qr-auth/qr-auth-context-provider.js
97–101

Wondering whether we should add some conditions here. We early return in a couple of scenarios, but are we sure that all the cases are handled? I guess it might be safer to wrap this with an if with appropriate conditions.

This revision is now accepted and ready to land.Fri, Dec 13, 6:26 AM
kamil added inline comments.Tue, Dec 17, 4:52 AM
native/account/qr-auth/qr-auth-context-provider.js
97–101

In if above we early-exist or any message different that SECONDARY_DEVICE_REGISTRATION_SUCCESS so this should be safe

Closed by commit rCOMM7be865dbb8df: [native] add callback to remove secondary device during QR auth (authored by kamil). · Explain WhyTue, Dec 17, 7:06 AM
This revision was automatically updated to reflect the committed changes.
kamil added a commit: rCOMM7be865dbb8df: [native] add callback to remove secondary device during QR auth.

Revision Contents
Changeset List

PathSize
native/
account/
qr-auth/
20 lines
1 line

Diff 46386

View Options

native/account/qr-auth/qr-auth-context-provider.js

Loading...
View Options

native/account/qr-auth/qr-auth-context.js

Loading...