Manage guest accounts better on the welcome page for auth #2819
Conversation
This is ultimately the fix for element-hq/element-web#9224 however it is not really great. By not logging out of the guest account, getCurrentHsUrl() returns the wrong homeserver URL (matrix.org) instead of the discovered one.
In 5b2328b we log out of the guest account before moving on, however that doesn't solve the case of the guest account being wrong to begin with. We should still keep 5b2328b in the event this races, however this commit's goal is to use the right homeserver for the guest account once it has been discovered.
There was a problem hiding this comment.
Hmm... I'm not sure how I feel about this. 😕
It seems a bit confusing to have a potential flow of:
- Create guest account on config default HS
- Log it out and use discovered default HS
- Create second guest account on discovered default HS
(Am I correct that that's possible here?)
We have another issue in play that has caused @lampholder to start a flow chart of how all these defaults are meant to work. Maybe defining guests as part of that would help here?
What about blocking guest account creation until server discovery returns? That sounds like it might be simpler to follow.
| // Change over the guest account, if one exists | ||
| if (MatrixClientPeg.get() && MatrixClientPeg.get().isGuest()) { | ||
| console.log("Discovered homeserver URLs - logging out current guest to use the homeserver"); | ||
| Lifecycle.logout().then(() => this._loadSession()); |
There was a problem hiding this comment.
Huh, so this will remake a new guest account once we discover the HS?
There was a problem hiding this comment.
yup, although it could probably do with a comment (or even better: not doing this and actually implementing the plan we talked about)
| onLoggedOut(); | ||
| }, 0); | ||
| return; | ||
| return new Promise((resolve, _reject) => { |
There was a problem hiding this comment.
Nit: I think it's more typical to not include reject at all if it's unused, at least for code I've seen here so far...
|
as per the issue, this is blocked on element-hq/element-web#9290 |
|
Closing - the solution for element-hq/element-web#9290 is more involved than what this covers, and so this needs to be reworked. |
Fixes element-hq/element-web#9224
Please review with element-hq/element-web#9275
Reviewer: The commits are probably of more interest than the words I put here.