Support .well-known discovery #2227

turt2live · 2018-10-18T22:53:07Z

Note: this does not implement step 2 of the discovery process. This is because IPv6 addresses are difficult to verify this way. Instead, this will adopt any port number in the user's ID as part of the address it queries for a .well-known config. See matrix-org/matrix-spec-proposals#1700 for more information.

Here's it working for someone trying to log in with a not-matrix.org address (for example):

Fixes element-hq/element-web#7253

Half-Shot · 2018-10-18T23:20:55Z

Can I be a pain and suggest that the discovery parts go in matrix-js-sdk. I can certainly think of projects which would benefit from this code and it would limit duping the same algo all over the place?

turt2live · 2018-10-18T23:21:59Z

It would be nearly impossible to put the .well-known bits in the js-sdk and maintain a helpful UI, sorry.

Edit: unless anyone has bright ideas on how to do so, given all the potential surrounding the redesign.

maxidorius · 2018-10-19T00:39:46Z

this does not implement step 2 of the discovery process.

and

this will adopt any port number in the user's ID as part of the address it queries for a .well-known config.

Then I'm afraid you are:

Not following The Spec.
Doing something that goes against recommended synapse setup.
Goes against the idea of .well-known which is to be independent of the service itself. This is a discovery mechanism.
Might force sysadmins to put a reverse-proxy on federation port.
Produce errors in clients which are not meant to handle self-signed certificates, which is the recommended and natural thing on federation port.
Will create a big(ger) mess with the whole dual port situation that already exists.
Will deviate from what other clients/libraries already implement and is already used in the wild, forcing sysadmins to now support two different systems.
Using Riot near-monopoly over the Matrix ecosystem to force a change down the community's throat, where current behaviour in The Spec was extensively discussed over several months and involved many actors.

There was a rationale why this was made this way, and I would strongly recommend you do not deviate from it in the slightest.

maxidorius · 2018-10-19T03:10:00Z

It would be nearly impossible to put the .well-known bits in the js-sdk and maintain a helpful UI, sorry.

Edit: unless anyone has bright ideas on how to do so, given all the potential surrounding the redesign.

Return an object that would give the outcome of the auto-discovery (ignore, fail prompt, fail error, ok, etc.) so the UI can react accordingly.

Half-Shot · 2018-10-19T11:11:57Z

It would be nearly impossible to put the .well-known bits in the js-sdk and maintain a helpful UI, sorry.
Edit: unless anyone has bright ideas on how to do so, given all the potential surrounding the redesign.

Return an object that would give the outcome of the auto-discovery (ignore, fail prompt, fail error, ok, etc.) so the UI can react accordingly.

Basically this, but @turt2live thinks this might be quite difficult with the UX requirements in the spec so it may not be so simple.

dbkr

Looks good modulo comments here & over on the spec PR.

dbkr · 2018-10-22T15:37:54Z

src/components/structures/login/Login.js

+
+        if (this.state.discoveredHsUrl) {
+            console.log("Rewriting username because the homeserver was discovered");
+            username = username.substring(1).split(":")[0];


Should we not just be sending the full mxid to the server?

this eventually gets passed along to /login as just the localpart. Here seemed like a good enough place to do the conversion is all.

hmm, where's that? I thought it just went through as 'user' in https://github.com/matrix-org/matrix-react-sdk/blob/master/src/Login.js#L130 ?

yea, around there. According to the spec ( https://matrix.org/docs/spec/client_server/r0.4.0.html#identifier-types ) it should work fine with the local part or user ID - I might have stumbled on a synapse bug (or I suck at testing)

My testing was flawed - will fix.

dbkr · 2018-10-22T15:38:42Z

src/components/structures/login/Login.js

@@ -218,8 +233,12 @@ module.exports = React.createClass({
        }).done();
    },

-    onUsernameChanged: function(username) {
+    onUsernameChanged: function(username, endOfInput) {


I would be tempted to just have a separate onUsernameBlur rather than adding a param

dbkr · 2018-10-22T15:40:11Z

src/components/structures/login/Login.js

+                return;
+            }
+
+            console.log("Verifying homeserver URL: " + hsUrl);


Presumably this would go if matrix-org/matrix-spec-proposals#1700 is approved?

dbkr · 2018-10-22T15:41:08Z

src/components/structures/login/Login.js

+                // XXX: We don't verify the identity server URL because sydent doesn't register
+                // the route we need.
+
+                // console.log("Verifying identity server URL: " + isUrl);


I assume also unnecessary with matrix-org/matrix-spec-proposals#1700

dbkr · 2018-10-22T15:45:56Z

src/components/structures/login/Login.js

-                customHsUrl={this.props.customHsUrl}
-                customIsUrl={this.props.customIsUrl}
+                customHsUrl={this.state.discoveredHsUrl || this.props.customHsUrl}
+                customIsUrl={this.state.discoveredIsUrl ||this.props.customIsUrl}


Should the value from .well-known be changing the default rather than setting custom URLs? You may still want to override the HS URL (eg. https://example.com/ -> https://example-test.com/) but having got the urls from .well-known essentially provides us a better default.

There was something that got in the way when I tried to set them explicitly the first time, however I think I might have fixed the bug towards the end of implementing this. I'll see if I can repurpose the custom properties instead of introducing new ones, while also maintaining the ability to override the discovered URL (for power users).

I think it's best to keep this as-is. It forces the UI to switch to "Custom Server", signifying that something happened. It isn't the best UX, but it is something.

dbkr

otherwise looks fine

dbkr · 2018-11-26T09:49:24Z

src/components/structures/login/Login.js

+                const url = new URL("https://" + serverName);
+                this._tryWellKnownDiscovery(url.hostname);
+            } catch (e) {
+                console.error("Problem parsing URL or unhandled error doing .well-known discovery");


I think we've been converging on console.log("message", e) which does a sensible thing

Support .well-known discovery

0030ba7

Fixes element-hq/element-web#7253

turt2live requested a review from a team October 18, 2018 22:53

Linting

71a9717

turt2live mentioned this pull request May 11, 2023

Extend homeserver autodiscovery to 3pids element-hq/element-meta#1582

Open

dbkr reviewed Oct 22, 2018

View reviewed changes

turt2live self-assigned this Oct 25, 2018

turt2live added 3 commits November 21, 2018 13:30

Merge branch 'develop' into travis/well-known

6f0f930

Introduce an onUsernameBlur and fix hostname parsing

4cfefe4

Validate the identity server

5600a9d

turt2live requested a review from a team November 21, 2018 21:15

turt2live removed their assignment Nov 22, 2018

dbkr reviewed Nov 26, 2018

View reviewed changes

Use sensible logging

015af7d

turt2live merged commit e3f2e69 into develop Nov 27, 2018

turt2live mentioned this pull request Nov 27, 2018

.well-known support element-hq/element-web#7253

Closed

turt2live deleted the travis/well-known branch November 27, 2018 20:51

aaronraimist mentioned this pull request Dec 8, 2018

Implement .well-known discovery for server URL element-hq/element-ios#2117

Closed

aaronraimist mentioned this pull request Dec 17, 2018

Implement .well-known support for server discovery neilalexander/seaglass#104

Open

manuroe mentioned this pull request Dec 19, 2018

.well-known support element-hq/riot-meta#243

Closed

manuroe mentioned this pull request Feb 21, 2019

.well-known support element-hq/riot-android#2982

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support .well-known discovery #2227

Support .well-known discovery #2227

turt2live commented Oct 18, 2018 •

edited

Loading

Half-Shot commented Oct 18, 2018

turt2live commented Oct 18, 2018 •

edited

Loading

maxidorius commented Oct 19, 2018 •

edited

Loading

maxidorius commented Oct 19, 2018

Half-Shot commented Oct 19, 2018

dbkr left a comment

dbkr Oct 22, 2018

turt2live Oct 22, 2018

dbkr Oct 22, 2018

turt2live Oct 22, 2018

turt2live Nov 21, 2018

dbkr Oct 22, 2018

dbkr Oct 22, 2018

turt2live Oct 22, 2018

dbkr Oct 22, 2018

turt2live Oct 22, 2018

dbkr Oct 22, 2018

turt2live Oct 22, 2018

turt2live Nov 21, 2018

dbkr left a comment

dbkr Nov 26, 2018

Support .well-known discovery #2227

Support .well-known discovery #2227

Conversation

turt2live commented Oct 18, 2018 • edited Loading

Half-Shot commented Oct 18, 2018

turt2live commented Oct 18, 2018 • edited Loading

maxidorius commented Oct 19, 2018 • edited Loading

maxidorius commented Oct 19, 2018

Half-Shot commented Oct 19, 2018

dbkr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbkr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turt2live commented Oct 18, 2018 •

edited

Loading

turt2live commented Oct 18, 2018 •

edited

Loading

maxidorius commented Oct 19, 2018 •

edited

Loading