-
Notifications
You must be signed in to change notification settings - Fork 270
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
crypto: Use the server name in the QR code login data #3537
Conversation
Using the resolved homeserver URL causes problems if we need to inspect the well-known configuration of the homeserver, for example, if the server name is matrix.org, but the homeserver URL is server.matrix.org, the well-known might be only available for the former. This is why we also need to receive the former, i.e. the server name in the QR code data.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3537 +/- ##
=======================================
Coverage 83.79% 83.79%
=======================================
Files 254 254
Lines 25734 25734
=======================================
Hits 21563 21563
Misses 4171 4171 ☔ View full report in Codecov by Sentry. |
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.
Mostly nits, so approving. The code makes sense, I'm not too sure about the why of the change, but trust you here. (Alternatively, one could ask review from somebody who's more knowledgeable about the QR code login process.)
The MSC got changed: matrix-org/matrix-spec-proposals@87f8317. The commit attempts to clarify why as well, you need to be able to do the well-known discovery, which is guaranteed to only work if you get the server name. |
Using the resolved homeserver URL causes problems if we need to inspect the well-known configuration of the homeserver, for example, if the server name is matrix.org, but the homeserver URL is server.matrix.org, the well-known might be only available for the former.
This is why we also need to receive the former, i.e. the server name in the QR code data.