This repository has been archived by the owner on Sep 11, 2024. It is now read-only.
Use validated server config for login, registration, and password reset #2941
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 was referenced May 3, 2019
12 tasks
turt2live
force-pushed
the
travis/guests/block-ui
branch
from
d2accbc to
d716745
Compare
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.
turt2live
force-pushed
the
travis/guests/block-ui
branch
from
d716745 to
4ada66d
Compare
|
Changing the base to |
dbkr
added a commit
that referenced
this pull request
Jun 12, 2019
We were ignoring the hs/is from the query parameters so after clicking the link, the new client tried to use the wrong server. Broken by #2941 Fixes element-hq/element-web#9659
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.
Requires element-hq/element-web#9496 - please review together.
Reviewer: Sorry about the size of this, however to keep the app functional the whole changeset needed to be one giant PR. Note that there's several TODO comments assigned to me - these are planned to be followed up by smaller (more manageable) PRs, however core app functionality is still maintained.
See element-hq/element-web#9290
Fixes element-hq/element-web#8628 (see element-hq/element-web#8628 (comment))
Fixes element-hq/element-web#3629
Fixes element-hq/element-web#9224
The overarching principle of this change is that all 3 pages shouldn't have to handle server configuration in any special way and instead just throw around a configuration object for that. This also makes it possible for the riot-web startup to validate and sanitize the config so that each page doesn't have to do it individually.
The design (look and feel) of this is not yet final, as per the TODO comments. Users should not normally hit the bad UX on develop unless they compile the branch themselves and make use of the new
default_server_configconfig.json option.