-
-
Notifications
You must be signed in to change notification settings - Fork 578
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for warning users when unknown devices show up #335
Merged
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
fb820fa
experimental support for warning users when unknown devices show up i…
ara4n 34a0bd4
oops, unbreak it
ara4n e79926d
fix lint
ara4n 247deac
some incoherent jottings on the warning semantics
ara4n 4ccd649
Address my own review comments
richvdh 5245c7f
Merge remote-tracking branch 'origin/develop' into matthew/warn-unkno…
richvdh 085493d
Fix tests
richvdh dfae72e
Merge branch 'matthew/warn-unknown-devices' of git+ssh://github.com/m…
ara4n 5911c4d
don't automatically mark devices as known; require the app to do it
ara4n 34fde7d
Store device 'known' status in session store
richvdh 5d544c7
Merge branch 'develop' into matthew/warn-unknown-devices
ara4n File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
Random notes from Matthew on the two possible approaches for warning users about unexpected | ||
unverified devices popping up in their rooms.... | ||
|
||
Original idea... | ||
================ | ||
|
||
Warn when an existing user adds an unknown device to a room. | ||
|
||
Warn when a user joins the room with unverified or unknown devices. | ||
|
||
Warn when you initial sync if the room has any unverified devices in it. | ||
^ this is good enough if we're doing local storage. | ||
OR, better: | ||
Warn when you initial sync if the room has any new undefined devices since you were last there. | ||
=> This means persisting the rooms that devices are in, across initial syncs. | ||
|
||
|
||
Updated idea... | ||
=============== | ||
|
||
Warn when the user tries to send a message: | ||
- If the room has unverified devices which the user has not yet been told about in the context of this room | ||
...or in the context of this user? currently all verification is per-user, not per-room. | ||
...this should be good enough. | ||
|
||
- so track whether we have warned the user or not about unverified devices - blocked, unverified, verified, unverified_warned. | ||
throw an error when trying to encrypt if there are pure unverified devices there | ||
app will have to search for the devices which are pure unverified to warn about them - have to do this from MembersList anyway? | ||
- or megolm could warn which devices are causing the problems. | ||
|
||
Why do we wait to establish outbound sessions? It just makes a horrible pause when we first try to send a message... but could otherwise unnecessarily consume resources? | ||
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 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 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 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 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 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
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.
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.
We've discussed this before. Because each megolm session consumes 1K of localstorage on every other device in the room. I don't want to add a megolm session to my storage every time someone in #megolm logs in on a new device, and it feels O(N^2)y to start to do so.
We should be more proactive in fetching the device list (this ties into Erik's work there) - and a lot of the work we do (and consequent pause) is in verifying the content of that device list. See element-hq/element-web#2157 (comment) for further thoughts on reducing the pause.