This repository has been archived by the owner on Sep 11, 2024. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 833
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
Previously, the user would be soft logged out because the tabs would fight each other. Now with timing (and approach) differences, the tabs are less likely to conflict with each other.
This was referenced Feb 15, 2022
turt2live
added
the
T-Enhancement
New features, changes in functionality, performance boosts, user-facing improvements
label
Feb 15, 2022
argh, even after trying to fix the branch name on the js-sdk side I still screwed it up. Will leave it alone and fix the CI once reviewed. |
t3chguy
approved these changes
Feb 15, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks sane otherwise
turt2live
added a commit
that referenced
this pull request
Feb 16, 2022
This reverts commit 8395934.
turt2live
added
T-Task
Refactoring, enabling or disabling functionality, other engineering tasks
and removed
T-Enhancement
New features, changes in functionality, performance boosts, user-facing improvements
labels
Feb 16, 2022
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.
This was reverted
#7821
MSC: matrix-org/matrix-spec-proposals#2918
Fixes element-hq/element-web#18698
Fixes element-hq/element-web#20648
Requires matrix-org/matrix-js-sdk#2178
Reviewable commit-by-commit
Note: There's a lot of logging in this PR. That is intentional to ensure that if/when something goes wrong we can chase the exact code path. It does not log any tokens - just where the code is going. Overall, it should be fairly low volume spam (and can be relaxed at a later date).
This approach uses indexeddb (through a mutex library) to manage which tab actually triggers the refresh, preventing issues where multiple tabs try to update the token. If multiple tabs update the token then the server might consider the account hacked and hard logout all the tokens.
If for some reason the timer code gets it wrong, or the user has been offline for too long and the token can't be refreshed, they should be sent to a soft logout screen by the server. This will retain the user's encryption state - they simply need to reauthenticate to get an active access token again.
This additionally contains a change to fix soft logout not working, per the issue links above.
Of interest may be the IPC approach which was ultimately declined in favour of this change instead: #7803
This change is marked as an internal change (Task), so will not be included in the changelog.
Preview: https://pr7802--matrix-react-sdk.netlify.app
⚠️ Do you trust the author of this PR? Maybe this build will steal your keys or give you malware. Exercise caution. Use test accounts.