Make login more robust against faulty cases #858
Merged
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
KitsuneRal
added
bug/fix
For saved but not logged in accounts, using an initial device name that is different from the saved one causes the homeserver to generate a new device, which may be undesirable, especially from the E2EE perspective. This commit makes LoginDialog to load the initial device name and the device id for a specified account if that account is saved but not logged in yet. The device id is now shown in UI (TODO: tuck the device id and the homeserver behind some "Advanced" button); changing the initial device name clears the device id (to reflect the homeserver logic). To make the login experience even better, two more things would be great on top: making the user id a combo box so that the user could browse saved account names; and adding a "Forget" button to completely remove the saved account from the configuration.
Singletons are increasingly seen as an anti-pattern that introduces very strong coupling across the codebase. Quotient::Accounts ended up being a prominent example of such coupling causing difficulties - e.g., interfering with clients making their own registries with different account/connection lifecycle management strategies. This commit introduces a member in MainWindow that points to Accounts; all other calls to Accounts are replaced with calls to this member. All classes that use Accounts for now are actually MainWindow children and only use Accounts at initialisation; so it's quite easy to pass the pointer to the registry as one more argument to the respective constructors. Eventually, MainWindow will have an AccountRegistry instance of its own instead of using the library singleton.
AccountRegistry::invokeLogin() is deprecated as it is too opinionated and tries to do too many things at once (collecting account settings, creating Connection objects, and validating access tokens, to name a few). So it is replaced with MainWindow::invokeLogin() (as it used to be before) that, in turn, delegates the retriying loop to a newly introduced class, ConnectionInitiator (which might be worth putting into libQuotient further down the road).
KitsuneRal
force-pushed
the
kitsune/make-login-actually-work-again
branch
from
11ab26b
to
b07fd1a
Compare
|
There were unrelated commits before the force-push, disregard them. |
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Main part of that is pulling the login code back from
AccountRegistryintoMainWindow.