Support for MSC3906 v2 #3193
Support for MSC3906 v2 #3193
Conversation
hughns force-pushed the hughns/msc3906-v2 branch from 3639083 to b1d7788 Compare hughns force-pushed the hughns/msc3906-v2 branch from a8cc3a1 to 59ee0a2 Compare hughns force-pushed the hughns/msc3906-v2 branch from 59ee0a2 to 7bb10a1 Compare 
| n.b. the code smells reported by SonarCloud are for deliberate use of deprecated MSC3906 v1 types |
hughns force-pushed the hughns/msc3906-v2 branch from 24a045d to b6699ba Compare 
| for links: previous impl was #2747 |
There was a problem hiding this comment.
Afraid I have to step away for a meeting, but I've left a few initial thoughts/stupid questions.
As a general theme: I would like to see more doc-comments explaining what things do/what they are for, even if it seems obvious. Writing out in words how an API is used/what it does can be really useful in terms of helping people understand, and maintaining a clean API.
src/rendezvous/MSC3906Rendezvous.ts Outdated
| * @param channel - The secure channel used for communication | ||
| * @param client - The Matrix client in used on the device already logged in | ||
| * @param onFailure - Callback for when the rendezvous fails | ||
| * @param flow - The flow to use. Defaults to MSC3906 v1 for backwards compatibility. |
There was a problem hiding this comment.
please could we have some more words explaining what a "flow" means and how I, as a user of this API, should pick one?
| flow, | ||
| intent, |
There was a problem hiding this comment.
| /** | ||
| * | ||
| * @returns the checksum of the secure channel if the rendezvous set up was successful, otherwise undefined | ||
| */ | ||
| public async startAfterShowingCode(): Promise<string | undefined> { |
There was a problem hiding this comment.
Particularly given this is a public function, please could it have a better doc-comment? What does it actually do, beyond returning a success flag?
hughns force-pushed the hughns/msc3906-v2 branch from e4559eb to c4ea55b Compare 
| Please make sure to rebase this PR so it can be compared against the current codebase cleanly |
| @hughns what's the state of play with this? Can it be closed for now? |
| Closing this in preference to moving directly to supporting Sign in with QR with OIDC. |
3 participants
This PR adds support for v2 of MSC3906 which is used for Sign in with QR.
Both v1 (default) and v2 are supported so compatibility is maintained.
Signed-off-by: Hugh Nimmo-Smith hughns@element.io
Checklist
Notes: Updates to protocol used for Sign in with QR code
Here's what your changelog entry will look like:
✨ Features