Don't allow users to ignore themselves. #18508
Conversation
| else: | ||
| currently_ignored_users = set() | ||
|
|
||
| if user_id in currently_ignored_users: |
There was a problem hiding this comment.
Noting: This doesn't fix any existing account data, so any users who have put themselves in the ignore list will need to remove it. Given lots of things stop working when you ignore yourself, i'd imagine most users aren't in this state
There was a problem hiding this comment.
The problem here is that the cause of "lots of things stop working" is very non-obvious. Presumably it would be reasonably trivial to do a one-off check (at least on matrix.org) for accounts in this state to investigate the magnitude of the problem? I wouldn't be surprised if a lot of accounts in this state end up being abandoned otherwise.
There was a problem hiding this comment.
self-ignore prevention should've been handled in the client side because certain IoT use cases (those with single-instruction, multiple-data remoting) benefit from self-ignores
There was a problem hiding this comment.
@erkinalp Sounds interesting. Can you expand on your use case and the exact side-effects of self-ignore that you're relying on and the context for why each thing matters?
There was a problem hiding this comment.
In the multi-seat IoT use cases, all tenants sharing the same command stream join the same room, receive the same messages in the room, then they send their performance, science and telemetry data back in the same room. To preserve processing time and memory, a self-ignore is useful in such usecase to hide (possibly large) data of other seats using the same instructions.
There was a problem hiding this comment.
Thanks for the context!
I assume you're using /sync for this "command stream"? If that's the case, just to throw out a possible alternative, you could use a filter with not_senders (ex. /sync?filter={"room":{"timeline":{"not_senders":["@me:matrix.org"]}}}). Same applies to other endpoints like /messages or /context. Or perhaps even better would be to use senders instead as an allow-list so you don't have to manually ignore every other IoT device.
Does that cover the extent of the benefits you get from self-ignore?
For reference, this wouldn't work if you're using Simplified Sliding Sync as proposed in MSC4186 as there isn't a filters for the timeline. Depending on the outcome of how this self-ignore situation ends, you may want to comment about your use there as well.
To be clear, I'm definitely on the side that self-ignore should be caught client-side. In addition, the sanity-check on the server-side also seems good if there is a good proper alternative for your use case. Those are just my current thoughts though and I'm no arbiter on the final call here. Sounds like there may be enough of a case that there should be a MSC to clarify this detail.
There was a problem hiding this comment.
I would also concur with a MSC. The present spec doesn't really state an actual use case for the ignore feature http://spec.matrix.org/v1.14/client-server-api/#ignoring-users so I couldn't say whether or not self ignores are a valid use case, so by default they are. However, the spec does not forbid the server from sanity checking the list and evidently we are seeing a reasonably high rate of users self ignoring.
A MSC would be a great way to clarify the use cases :)
reivilibre
left a comment
reivilibre
left a comment
There was a problem hiding this comment.
sure, hopefully there's no valid reason to rely on this
- Improvements to generate config documentation from JSON Schema file. ([\element-hq#18522](element-hq#18522)) - Add support for [MSC4155](matrix-org/matrix-spec-proposals#4155) Invite Filtering. ([\element-hq#18288](element-hq#18288)) - Add experimental `user_may_send_state_event` module API callback. ([\element-hq#18455](element-hq#18455)) - Add experimental `get_media_config_for_user` and `is_user_allowed_to_upload_media_of_size` module API callbacks that allow overriding of media repository maximum upload size. ([\element-hq#18457](element-hq#18457)) - Add experimental `get_ratelimit_override_for_user` module API callback that allows overriding of per-user ratelimits. ([\element-hq#18458](element-hq#18458)) - Pass `room_config` argument to `user_may_create_room` spam checker module callback. ([\element-hq#18486](element-hq#18486)) - Support configuration of default and extra user types. ([\element-hq#18456](element-hq#18456)) - Successful requests to `/_matrix/app/v1/ping` will now force Synapse to reattempt delivering transactions to appservices. ([\element-hq#18521](element-hq#18521)) - Support the import of the `RatelimitOverride` type from `synapse.module_api` in modules and rename `messages_per_second` to `per_second`. ([\element-hq#18513](element-hq#18513)) - Remove destinations from sending if not whitelisted. ([\element-hq#18484](element-hq#18484)) - Fixed room summary API incorrectly returning that a room is private in the room summary response when the join rule is omitted by the remote server. Contributed by @nexy7574. ([\element-hq#18493](element-hq#18493)) - Prevent users from adding themselves to their own user ignore list. ([\element-hq#18508](element-hq#18508)) - Generate config documentation from JSON Schema file. ([\element-hq#17892](element-hq#17892)) - Mention `CAP_NET_BIND_SERVICE` as an alternative to running Synapse as root in order to bind to a privileged port. ([\element-hq#18408](element-hq#18408)) - Surface hidden Admin API documentation regarding fetching of scheduled tasks. ([\element-hq#18516](element-hq#18516)) - Mark the new module APIs in this release as experimental. ([\element-hq#18536](element-hq#18536)) - Mark dehydrated devices in the [List All User Devices Admin API](https://element-hq.github.io/synapse/latest/admin_api/user_admin_api.html#list-all-devices). ([\element-hq#18252](element-hq#18252)) - Reduce disk wastage by cleaning up `received_transactions` older than 1 day, rather than 30 days. ([\element-hq#18310](element-hq#18310)) - Distinguish all vs local events being persisted in the "Event Send Time Quantiles" graph (Grafana). ([\element-hq#18510](element-hq#18510))
5 participants
Fixes the self-ignore issues we've being seeing of reports of by ignoring bad requests from clients.
Fixes #11963
Fix element-hq/element-web#29969 although this should also be fixed on the client to avoid confusing errors popping up while rejecting invites.
Related to matrix-org/matrix-rust-sdk#5073
Pull Request Checklist
EventStoretoEventWorkerStore.".code blocks.