This repository has been archived by the owner on Sep 11, 2024. It is now read-only.
Fix relationship between guests, .well-known, and auth #3001
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This also causes the components to produce a ValidatedServerConfig for use by other components.
The general idea is that we throw the object around between components so they can pull off the details they care about.
Like registration, the idea is that the object is passed around between components so they can take details they need.
Very similar to password resets and registration, the components pass around a server config for usage by other components. Login is a bit more complicated and needs a few more changes to pull the logic out to a more generic layer.
This way the server config is consistent across login, password reset, and registration. This also brings the code into a more generic place for all 3 duplicated efforts.
Use validated server config for login, registration, and password reset
TODO still remains about making ModularServerConfig extend ServerConfig instead of duplicating everything. See element-hq/element-web#9290
The app is expected to flag a particular config themselves as default. This is primarily intended so that other parts of the app can determine what to do based on whether or not the config is a default config. See element-hq/element-web#9290
Render underlines and tooltips on custom server names in auth pages
Refactor "Next" button into ServerConfig components
Only expose the fallback_hs_url if the homeserver is the default homeserver
Restore use of full mxid login
If you were in the username field and simply tabbed out without entering anything, the form would become "busy" and not let you submit. We should only be doing this if we have work to do, like .well-known discovery of the homeserver.
This is often null while the component is on its first render, and is called during that render. It is eventually populated by React, and the function re-called - we just have to be patient.
12 tasks
jryans
approved these changes
May 21, 2019
| @@ -181,16 +181,8 @@ export default React.createClass({ | |||
| // Parameters used in the registration dance with the IS | |||
There was a problem hiding this comment.
Have you tested the email validation flow with these changes in place? (That's one area we've tended to break...)
There was a problem hiding this comment.
just to follow up: I re-tested the two common scenarios just to be absolutely sure it works.
- Creating a new account and verifying that email works
- Receiving an email invite does not work (see Email invites don't work element-hq/element-web#9816 - this is not an issue unique to this PR)
|
|
||
| const MODULAR_URL = 'https://modular.im/?utm_source=riot-web&utm_medium=web&utm_campaign=riot-web-authentication'; | ||
|
|
||
| // TODO: TravisR - Can this extend ServerConfig for most things? |
There was a problem hiding this comment.
Probably so... At the time, it seemed as if it'd be more distinct.
There was a problem hiding this comment.
I also forgot about this comment and have stuck it on my cleanup list. That list is getting long though, so at some point I'll just take a break from feature work to fix it.
|
RC cut, let's land this thing 🎉 |
This was referenced May 23, 2019
Closed
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reviewer: You may be wondering what part of this giant PR needs your attention, and luckily the answer isn't as scary as the diff. The bulk of this PR has already been reviewed (see the "includes" list below), however there are two commits which do need your seal of approval (see "new commits" list below). The remainder of the PR is really just getting your approval to throw this thing on develop and see how it works in the real world, now that the entire thing has been tested by hand. This PR also serves as a changelog entry for when we release.
Please review with element-hq/element-web#9779. The js-sdk parts of this have already landed on develop.
The primary change of this PR is to require that people configure their Riot with a homeserver it can use to run the app. With that homeserver, the app is expected to use that whenever possible for guest accounts and as a default option for users. Users can change which server they ultimately use, however there's plenty of guards in place to make sure the app has a good chance of actually running once they pass the login screen.
Or written in the form of a TLDR: This is a bunch of maintenance to fix a bunch of bugs and introduce a standardized behaviour for the app's homeserver handling.
New commits (need review):
Includes:
Fixes: