fix: close connection when pairing key does not match#270
fix: close connection when pairing key does not match#270vinistevam merged 5 commits intomain-desktopfrom
Conversation
4ee838b to
86c215f
Compare
There was a problem hiding this comment.
Are there 2 versions of the state? Which one is considered as the source of truth?
There was a problem hiding this comment.
I also couldn't find the case for this extra check. Is it possible to have a discrepancy between the two states?
There was a problem hiding this comment.
Last week this.desktopState.desktopEnabled was returning false while establishing a new connection, but after rebasing today I removed this change and the fix still works.
498a09e to
5a32d49
Compare
packages/common/src/types/desktop.ts
Outdated
There was a problem hiding this comment.
Why do we need the alternate states here? Would no key and the wrong key not both result in the same "app not recognised" error?
There was a problem hiding this comment.
There is another scenario where "Desktop has no pairing key" here
There was a problem hiding this comment.
We really shouldn't be relying on implicit behaviour of enums where the values are assigned to numbers by default which in turn evaluate to truthy. We could change the order of the values, or assign them to strings and this would break.
I'd recommend assigning to a variable then doing a [PairingKeyStatus.NO_MATCH, PairingKeyStatus.MISSING].includes(pairingKeyStatus) to be explicit.
There was a problem hiding this comment.
Good catch! It shouldn't be like that.
I was doing negatively compared with PairingKeyStatus.MATCH but the way you recommended it's more explicit.
Pushing the changes.
packages/common/src/types/desktop.ts
Outdated
There was a problem hiding this comment.
If we need this enum, then pairingKey is implied and we've been using capitals for enums so maybe MATCH, NO_MATCH and MISSING?
5a32d49 to
09b5020
Compare
09b5020 to
a1fd484
Compare
Overview
Bug 1: whenever disabled the pairingKeyHash was not been clean
Changes
PairingKeyStatusto facilitate error screen ticket: 135Issues
Resolves #212