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
[multi-device] Handle incoming pairing authorisation message #430
[multi-device] Handle incoming pairing authorisation message #430
Conversation
d63762a
to
6ab05e2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Immaculate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few minor things
libtextsecure/message_receiver.js
Outdated
return true; | ||
}, | ||
async handlePairingRequest(pairingRequest) { | ||
if (!this.validateAuthorisation(pairingRequest)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this function async? In this case it would always be true and continue even on invalid requests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aye good catch
libtextsecure/message_receiver.js
Outdated
if (!this.validateAuthorisation(pairingRequest)) { | ||
return; | ||
} | ||
window.libloki.storage.savePairingAuthorisation(pairingRequest); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to await this incase it fails somehow? or is it fine as is?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cleaner to wait :)
libtextsecure/message_receiver.js
Outdated
); | ||
}, | ||
async handleAuthorisationForSelf(pairingAuthorisation) { | ||
if (!this.validateAuthorisation(pairingAuthorisation)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixing asap
libtextsecure/message_receiver.js
Outdated
} else { | ||
window.log.warn('Unimplemented pairing authorisation message type'); | ||
} | ||
}, | ||
async handleAuthorisationForContact(pairingAuthorisation) { | ||
if (!this.validateAuthorisation(pairingAuthorisation)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixing asap
Co-Authored-By: Mikunj Varsani <Mikunj@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
The control flow starts at the bottom of the diff and splits in 3.
The authorisation is saved only if valid.
Note:
Whisper.events
are used to propagate events across the application:secondaryDeviceRegistration
event: we (the secondary device) have received the grant authorisation from the primary device. This makes the UI transition from the registration page to the inbox page.devicePairingRequestReceived
event: we (the primary device) have received a pairing authorisation request. This will trigger the UI to prompt the user to accept the pairing if the pairing modal window is open, otherwise it's ignored.