[lib] avoid listening to Tunnelbroker messages in `QRAuthProvider` when QR auth is finished
ClosedPublic
Actions

Authored by kamil on Jun 17 2024, 4:07 AM.

Details

Reviewers

bartek
ashoat

Commits

rCOMMe5cb65cf5e8a: [lib] avoid listening to Tunnelbroker messages in `QRAuthProvider` when QR auth…

Summary

ENG-8385.

QR code login is not working right now. As described here (Point 3) and here the issue is occuring because of the fact that useQRAuth is unmounting before it can send SECONDARY_DEVICE_REGISTRATION_SUCCESS. This stack is refactoring the code to move logic to context (see here).

Make sure we no longer track Tunnelbroker messages here and avoid sending SECONDARY_DEVICE_REGISTRATION_SUCCESS multiple times (BACKUP_DATA_KEY_MESSAGE is the last message between devices).

Note that without scanning code listener is not added, so in normal app usage, listener code shouldn't be executed.

Depends on D12453

Test Plan

Test QR login flow on native and web

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

kamil created this revision.Jun 17 2024, 4:07 AM

kamil held this revision as a draft.

Herald added subscribers: tomek, ashoat. · View Herald TranscriptJun 17 2024, 4:07 AM

kamil edited the summary of this revision. (Show Details)Jun 17 2024, 4:08 AM

kamil added a child revision: D12455: [lib] broadcast device list updates after Tunnelbroker socket connection is established.Jun 17 2024, 4:19 AM

kamil published this revision for review.Jun 17 2024, 4:37 AM

Harbormaster completed remote builds in B29717: Diff 41386.Jun 17 2024, 4:39 AM

What happens if the user logs in via QR code, then logs out, and then tries again? It seems like qrAuthFinished is never set to false after being set to true, which makes me suspect that things will be broken in this case.

This revision now requires changes to proceed.Jun 17 2024, 10:22 AM

set qrAuthFinished to false when calling generateQRCode

In D12454#353497, @ashoat wrote:

What happens if the user logs in via QR code, then logs out, and then tries again? It seems like qrAuthFinished is never set to false after being set to true, which makes me suspect that things will be broken in this case.

Good catch, updated code to unset this each time when we generate new QR code and tested case you described.

Harbormaster completed remote builds in B29747: Diff 41454.Jun 18 2024, 8:17 AM

Please remove the unnecessary effect before landing, or re-request review if you think I'm missing something

lib/components/qr-auth-provider.react.js
225–229	This effect should not be necessary. If `qrAuthFinished` flips to true, the above effect will be re-run. Its cleanup function will run first, which will call `removeListener`. Then it will be processed again, but since `qrAuthFinished` it won't do anything