Skip to content
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

Do not prevent login if .well-known response is invalid #1815

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

p12tic
Copy link
Contributor

@p12tic p12tic commented Sep 24, 2024

The current code results in confusing behavior under the following circumstances:

  • Homeserver is guessed correctly from the Matrix ID

  • The server returns invalid (e.g. HTTP 200 with empty body) response for .well-known data

The user will be presented with "Autodiscovery failed. Received malformed response" message even though all details are entered correctly. Login button will not work. The only workaround is to edit the home server field a few times so that homeserverChanged() and checkHomeserverVersion() would be invoked.

This is not optimal, especially because autodiscovery based on .well-known is optional. The solution is to always invoke checkHomeserverVersion() even when autodiscovery fails due to any reason.

checkHomeserverVersion() will check the home server against required Matrix API endpoints. If this fails, user will be presented with error messages instructing to check home server URL. Accordingly, allowing to continue in case of failed autodiscovery does not reduce clarity of errors presented to the user.

The current code results in confusing behavior under the following
circumstances:

 - Homeserver is guessed correctly from the Matrix ID

- The server returns invalid (e.g. HTTP 200 with empty body) response
for .well-known data

The user will be presented with "Autodiscovery failed. Received
malformed response" message even though all details are entered
correctly. Login button will not work. The only workaround is to edit
the home server field a few times so that homeserverChanged() and
checkHomeserverVersion() would be invoked.

This is not optimal, especially because autodiscovery based on
.well-known is optional. The solution is to always invoke
checkHomeserverVersion() even when autodiscovery fails due to any
reason.

checkHomeserverVersion() will check the home server against required
Matrix API endpoints. If this fails, user will be presented with error
messages instructing to check home server URL. Accordingly, allowing to
continue in case of failed autodiscovery does not reduce clarity of
errors presented to the user.
@deepbluev7
Copy link
Member

The Matrix specification is quite explicit, that the user should be provided with an error message, if .well-known discovery fails in those cases and enter the homeserver url themselves: https://spec.matrix.org/latest/client-server-api/#server-discovery . I'm a bit concerned, that just dropping that error information will lead to even more misconfigured homeservers. In both of these cases the homeserver is misconfigured, since it provides a .well-known with invalid content. If this is very widespread, then working around that in Nheko would make sense, but so far those servers seem to be in the minority?

Do you know why those servers return an empty body with a HTTP 200?

And as you said, the .well-known should be optional, but then you need to manually enter the homeserver address, which should still work.

@p12tic
Copy link
Contributor Author

p12tic commented Sep 24, 2024

Thanks for feedback. I agree. I will think of a way to keep error message in case autodiscovery fails.

@p12tic
Copy link
Contributor Author

p12tic commented Sep 24, 2024

Do you know why those servers return an empty body with a HTTP 200?

I don't know, probably misconfiguration.

And as you said, the .well-known should be optional, but then you need to manually enter the homeserver address, which should still work.

Just one data point, in my particular case homeserver address was guessed correctly. Putting @username:homeserver into matrix ID correctly guessed homeserver as the home server. This is what was the most confusing - if home server was guessed incorrectly, then I would have edited it and allowed to login. Now there was nothing to edit and I managed to log in only after I looked into the source code.

@deepbluev7
Copy link
Member

Just one data point, in my particular case homeserver address was guessed correctly. Putting @username:homeserver into matrix ID correctly guessed homeserver as the home server. This is what was the most confusing - if home server was guessed incorrectly, then I would have edited it and allowed to login. Now there was nothing to edit and I managed to log in only after I looked into the source code.

Okay, that makes a bit more sense to me now. You basically get prompted to enter a value, that was correctly guessed, which is confusing. I guess it does make sense to continue in that case, since the user will just enter the same value anyway, but maybe it should instead be a prompt like "The server is misconfigured, but example.com might be your homeserver. Continue with the login?"?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants