Well known lookups should be to port 443 only.

[michaelkaye]

Type of change

• Feature
• Bugfix
• Technical
• Other :

Content

Per https://spec.matrix.org/unstable/client-server-api/#well-known-uri, step two:

1. Extract the server name from the user’s Matrix ID by splitting the Matrix ID at the first colon.
2. Extract the hostname from the server name.
3. Make a GET request to https://hostname/.well-known/matrix/client. 

The hostname should not include a port, and should not query https://hostname:port/.well-known/matrix/client.

We achieve this by removing the port from the server_name before making a well-known lookup.

Additionally, I have renamed "getDomain" to "getServerName" as that is closer to the meaning of the second half of the matrix ID (after the first colon)

Motivation and context

There were originally issues with unit tests running against servers on non-standard ports (eg http://localhost:8080). These would then attempt to lookup well-known data by accessing https://localhost:8080/ (NB: https). These would fail because the port was listening for plain http.

This attempts to fix those tests by following the well-known resolution steps fully, and removing the port. This causes lookups against eg https://localhost:443/ instead, which in the test cases, is generally not responding with valid data.

This is also closer to the specification.

Tests

Tests continue to work; login has become faster for hosts over http because the timeout attempting to connect has been removed.

However, we should be aware that this is a change in behaviour and may possibly cause some servers with an interesting configuration to have issues, eg if your server_name is target-domain.com:8443 and you host your well-known at https://target-domain.com:8443/ not https://target-domain.com:443/.

Tested devices

• Physical
• Emulator
• OS version(s):

Checklist

• Changes has been tested on an Android device or Android emulator with API 21
• UI change has been tested on both light and dark themes
• Accessibility has been taken into account. See https://github.com/vector-im/element-android/blob/develop/CONTRIBUTING.md#accessibility
• Pull request is based on the develop branch
• Pull request includes a new file under ./changelog.d. See https://github.com/vector-im/element-android/blob/develop/CONTRIBUTING.md#changelog
• Pull request includes screenshots or videos if containing UI changes
• Pull request includes a sign off
• You've made a self review of your PR
• If you have modified the screen flow, or added new screens to the application, you have updated the test UiAllScreensSanityTest.allScreensTest()

[michaelkaye]

I'm not 100% sure this is the right change, but given the reading of the spec I believe so. However, the other clients also handle well-known the old way.

iOS does this here: https://github.com/vector-im/element-ios/blob/develop/RiotSwiftUI/Modules/Authentication/Common/Service/MatrixSDK/AuthenticationService.swift#L230

(which is to make the request to the homeserver url without removing the port)

There is also a commented out section here, which talks about doing a lookup based on the matrix ID:
https://github.com/vector-im/element-ios/blob/develop/RiotSwiftUI/Modules/Authentication/Common/Service/MatrixSDK/AuthenticationService.swift#L150

(I'm unable to test actual behaviour due to lack of iOS device)

Element Web makes a https request with the specified port:

https://github.com/matrix-org/matrix-js-sdk/blob/develop/src/autodiscovery.ts#L270

eg entering your username as @michaelk:matrix.org:80 will perform a discovery request to matrix.org:80/.well-known and expect the endpoint to talk TLS - you can see a spinner if you try it.

[michaelkaye]

Hydrogen is making a request to eg https://hydrogen.element.io/null/_matrix/client/r0/login when i give my homeserver as eg "matrix.org:8080", so disreguarding that as it doesn't appear to be implemented yet.

[michaelkaye]

If we don't want to go this way, we could perhaps add a much shorter timeout on the well-known lookup

[github-actions]

Unit Test Results

122 files  122 suites   2m 3s ⏱️
205 tests 205 ✔️ 0 💤 0 ❌
690 runs  690 ✔️ 0 💤 0 ❌

Results for commit 9c99d12.

[bmarty]

LGTM, thanks!

[github-actions]

Matrix SDK

Integration Tests Results:

• [org.matrix.android.sdk.session]
  = passed=20 failures=0 errors=0 skipped=3
• [org.matrix.android.sdk.account]
  = passed=3 failures=0 errors=0 skipped=2
• [org.matrix.android.sdk.internal]
  = passed=5 failures=1 errors=0 skipped=0
• [org.matrix.android.sdk.ordering]
  = passed=16 failures=0 errors=0 skipped=0
• [org.matrix.android.sdk.PermalinkParserTest]
  = passed=2 failures=0 errors=0 skipped=0