-
Notifications
You must be signed in to change notification settings - Fork 399
MSC4293: Redact on ban #4293
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
base: main
Are you sure you want to change the base?
MSC4293: Redact on ban #4293
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,167 @@ | ||||||||||
# MSC4293: Redact on ban | ||||||||||
|
||||||||||
[MSC2244 (accepted)](https://github.com/matrix-org/matrix-spec-proposals/blob/main/proposals/2244-mass-redactions.md)-style | ||||||||||
mass redactions are incredibly helpful for cleaning up large volumes of spam, but still require sending | ||||||||||
a dedicated event to clean up the spam. In a typical case, a user will get kicked/banned from a room | ||||||||||
and the moderators will further redact some or all of their messages. Mass redactions have more use | ||||||||||
cases, but the specific case of "redact everything upon ban" is something which may be easily backported | ||||||||||
to existing room versions. | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think I'm quite getting this proposal. So it's just a temporary measure to do mass redactions in old room versions? Why not just a separate 'mass redact' endpoint? Or, for that matter, have the HS look for mass redact events sent into old room versions and do them manually exactly as it would do here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. MSC4194 is a prior exploration of a mass redact endpoint for this use case. A reason for collecting the responsibilities into a flag on the ban event are because of the following complications:
Footnotes
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As Gnuxie mentions, we can't/shouldn't be sending 1:1 redaction events anymore, especially post-ban (the original sender can't "self-redact", and the moderator who did the ban may have left). This MSC specifically tries to reduce the number of redaction events to zero if it can, or one in the worst case - mass redactions themselves would be the "one" case. The issue with mass redact events is that they aren't backwards compatible to existing room versions, which will cause breakage. We could spend some time trying to fix this (fallback fields, different event types, etc), but those solutions don't look as great (in my opinion) and slow down the rollout of the feature due to broader use cases needing consideration. We (T&S) expect that both redact-on-ban and mass redactions will be used in future room versions concurrently for different use cases, and require a more immediate solution to 1:1 redaction event sending. I'll cover this in the alternatives section when I loop back around for an edit. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm afraid I may be more confused if anything. I was assuming the HS would do something to make this backwards compatible but now I wonder if I've misunderstood that. What makes this backwards compatible given a mass redaction isn't? If this is not a short-term measure, in what circumstances would this be used vs a mass redaction? In any case, hopefully the edit should make things clearer. The info in the responses here probably needs to go into the MSC. |
||||||||||
|
||||||||||
This proposal suggests adding a new flag to membership events to indicate to clients and servers that | ||||||||||
all of that user's events should be redacted in addition to being kicked or banned. The flag isn't | ||||||||||
protected from redaction itself, so may have some consistency issues, but overall should still provide | ||||||||||
relatively high amounts of protection to rooms. | ||||||||||
|
||||||||||
## Proposal | ||||||||||
|
||||||||||
A new flag is added to [`m.room.member`](https://spec.matrix.org/v1.14/client-server-api/#mroommember) | ||||||||||
events where the target user is kicked or banned: `redact_events`. This flag is a boolean and, when | ||||||||||
`true`, causes servers (and clients) to redact all of the user's events as though they received an | ||||||||||
[`m.room.redaction`](https://spec.matrix.org/v1.14/client-server-api/#mroomredaction), including | ||||||||||
adding [`redacted_because`](https://spec.matrix.org/v1.14/client-server-api/#redactions) to `unsigned` | ||||||||||
where applicable. An `m.room.redaction` event is not actually sent, however. | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Later on in the proposal, it's said several times that sending redactions as fallback support is intended, so I'm getting mixed signals here. When this flag is passed, should the server send actual redactions or not? I assume it's "yes, send actual redactions, until greater adoption makes them redundant" from later paragraphs, however it is not clear. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If this is the implication then it shouldn't be. Redactions will always need to be sent in legacy room versions. And if the proposal has been written with otherwise then that's a pretty serious issue. @turt2live could you please clarify? My understanding was that redaction events would have to be sent for all room versions prior to the inclusion of MSC4298. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TLDR: "yes, send actual redactions if you want to, until greater adoption makes them redundant" (added emphasis mine) is accurate. Fallback behaviour is a bit of an awkward case for MSCs. In this case, the intended change is meant to be additive to the spec with no actual requirement to send redaction events (even in existing room versions), but there are several implementations of Matrix clients and servers which already exist today that won't know how to handle this new behaviour, and thus senders are encouraged (not required) to send "fallback redactions". The MSC does give some guidance on what fallback behaviour might look like, but leaves removal of that fallback behaviour up to the individual projects. One project may, for example, decide that pre-MSC4298 rooms will forever get fallback redactions while another decides that 3 months from spec release there is enough client implementations to safely drop the fallback support. Some implementations may also choose somewhere in the middle of that spectrum: instead of chasing events and retrying redactions, it could do a single query and send redactions once, leaving any errors to hopefully be picked up by the redact-on-ban flag. Neither this MSC or MSC4298 make the fallback behaviour required, either for existing room versions or implementations of Matrix. |
||||||||||
|
||||||||||
**Note**: This also means that if the user was kicked/banned with a `reason`, that event is automatically | ||||||||||
compatible with the redaction `reason` field and shows up accordingly. | ||||||||||
|
||||||||||
Similar to regular redactions, if the sender of the membership event can't actually redact the target's | ||||||||||
events, the redaction doesn't apply. This means having a power level higher than or equal to `redacts` | ||||||||||
*and* `events["m.room.redaction"]` (if set). We maintain the `events` check despite not actually sending | ||||||||||
events of that type to keep the same expectations within rooms. If the sender doesn't have permission | ||||||||||
to redact an event normally, no redaction is applied. | ||||||||||
Comment on lines
+27
to
+31
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can a user send this when leaving a room? Not in the case of kick but to self redact all. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, self-leaves are not permitted by this MSC. Rationale being it opens a can of worms I'd rather not get this MSC stuck in. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I think that's unclear that the behavior only applies if the sender and state key don't match then. |
||||||||||
|
||||||||||
If the sender is allowed to redact, the redaction behaviour continues until the membership event itself | ||||||||||
is redacted (thus removing the field), another membership event removes the field, or the flag is set | ||||||||||
Comment on lines
+33
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why redact the event itself instead of just stopping at it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. err, sorry: the membership event is not redacted by the autoredaction stuff. It's just that since we're doing this without a room version, the flag is at risk of being redacted off the event (technically). We otherwise keep redacting after the ban because of eventual consistency and delayed receipt of events. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel like this says the opposite?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can see how that might be interpreted that way, but it is not the intention. It's trying to clarify to server implementations that they won't be able to see the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've read this a handful of times now and I can't interpret it any other way.
Suggested change
|
||||||||||
to `false`. Events already redacted up to that point remain redacted after the flag changes to a falsey | ||||||||||
value. For example, if the user is unbanned, the moderator MAY NOT choose to carry the `redact_events` | ||||||||||
flag to that kick (unban) event. Or, when the user rejoins after a kick, the flag is implicitly dropped. | ||||||||||
|
||||||||||
In essence, the `redact_events` flag applies to all events which topologically come before the falsey | ||||||||||
value. | ||||||||||
|
||||||||||
Events which are delivered after the ban are likely [soft failed](https://spec.matrix.org/v1.14/server-server-api/#soft-failure) | ||||||||||
and are still redacted if the current membership event in the room has a valid `redact_events` | ||||||||||
field. | ||||||||||
|
||||||||||
Other membership states with the flag no-op, such as joins, knocks, and invites. | ||||||||||
|
||||||||||
Moderation bots and similar MAY still wish to issue (mass) redactions upon kick/ban to protect users | ||||||||||
on servers or clients which don't have this feature. | ||||||||||
turt2live marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
|
||||||||||
Example ban: | ||||||||||
|
||||||||||
```jsonc | ||||||||||
{ | ||||||||||
// Irrelevant fields excluded | ||||||||||
"type": "m.room.member", | ||||||||||
"state_key": "@spam:example.org", | ||||||||||
"sender": "@mod:example.org", | ||||||||||
"content": { | ||||||||||
"membership": "ban", | ||||||||||
"reason": "flooding", // this is copied to `redacted_because`, leading to clients showing it | ||||||||||
"redact_events": true | ||||||||||
} | ||||||||||
} | ||||||||||
``` | ||||||||||
|
||||||||||
The new field is proxied through to the event by the [`/kick`](https://spec.matrix.org/v1.14/client-server-api/#post_matrixclientv3roomsroomidkick) | ||||||||||
and [`/ban`](https://spec.matrix.org/v1.14/client-server-api/#post_matrixclientv3roomsroomidban) | ||||||||||
sugar APIs, like `reason` is. | ||||||||||
|
||||||||||
## Fallback behaviour | ||||||||||
|
||||||||||
Servers which don't support this feature may be served redacted events over federation when attempting | ||||||||||
to fill gaps or backfill. This is considered expected behaviour. | ||||||||||
|
||||||||||
Clients which don't support this feature may see events remain unredacted until they clear their local | ||||||||||
cache. Upon clearing or invalidating their cache, they will either receive redacted events if their | ||||||||||
server supports the feature or unredacted events otherwise. | ||||||||||
|
||||||||||
To (primarily) help protect users on unsupported *clients*, implementations SHOULD continue to try | ||||||||||
sending individual redaction events in addition to the redact-on-ban flag. They MAY cease to do so | ||||||||||
once they are comfortable with the level of adoption for this proposal. Servers in particular SHOULD | ||||||||||
assist clients and send individual redaction events on their behalf, meaning clients SHOULD wait a | ||||||||||
little bit before trying to issue redactions themselves. For example, a client may ban a user, wait | ||||||||||
a minute, then start sending redactions if it hasn't seen an `m.room.redaction` event targeting some | ||||||||||
of the banned user's events. Servers MAY deduplicate redactions to lower federation load, as they | ||||||||||
always could. | ||||||||||
turt2live marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
|
||||||||||
**Note**: It is possible due to implementation and real-world constraints that not all individual | ||||||||||
redactions will "make it" over federation to another server. This is why mass redaction approaches | ||||||||||
are preferred, as they are significantly more reliable. | ||||||||||
|
||||||||||
## Potential issues | ||||||||||
turt2live marked this conversation as resolved.
Show resolved
Hide resolved
turt2live marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't this potentially have weird behavior if a server redacts the events, redacts the membership event itself, then backfills events? It is unclear to me if this is covered the text below (or above in fallback behavior?) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it should be covered in the potential issues, but I'll aim to clarify in the text. The short version is: yes, it's weird, but not any more weird than usual. A server which requests events from a server that redacted them will receive redacted copies. This infectious behaviour is a feature, though if a server really wanted to it could ask other servers until it finds an unredacted copy. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm worried about the opposite -- receiving unredacted events and not knowing they should be redacted. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That should already be addressed in the MSC, as a known limitation. |
||||||||||
|
||||||||||
It's a little annoying that the flag is redacted when the membership event is redacted, however it's | ||||||||||
extremely rare for a moderator/admin to redact a kick or ban event. This can be fixed in a future | ||||||||||
room version, like what is proposed by [MSC4298](https://github.com/matrix-org/matrix-spec-proposals/pull/4298). | ||||||||||
|
||||||||||
Though extremely rare, if an existing server in the room didn't apply the redactions *and* a sender's | ||||||||||
ban was redacted, a new server to the room may backfill through that existing server and see unredacted | ||||||||||
events without knowing it's supposed to redact them due to the ban having lost the `redact_events` | ||||||||||
field. This is fixed for future room versions by implementing something like [MSC4298](https://github.com/matrix-org/matrix-spec-proposals/pull/4298). | ||||||||||
|
||||||||||
Clients may miss the membership event if they are using lazy loading, though servers should already | ||||||||||
be tracking which membership events the client has received and needs to render events in the timeline. | ||||||||||
This should mean that those clients will still receive the event. | ||||||||||
|
||||||||||
Servers which miss the event will eventually receive or retrieve it, just like they would with any | ||||||||||
other event. | ||||||||||
|
||||||||||
Moderation bots/clients which attempt to reduce the amount of duplicate work they do may need to | ||||||||||
inspect `redacted_because` instead of checking for its presence to determine which kind of redaction | ||||||||||
was applied to a given event. This is especially true if the moderation bot/client is providing the | ||||||||||
fallback support described above. | ||||||||||
|
||||||||||
If a user is banned using `redact_events: true`, unbanned, rejoins, sends more events, and is banned | ||||||||||
again using `redact_events: true`, the user's events between bans will be subsequently redacted. The | ||||||||||
events redacted by the first ban may also be re-redacted by servers/clients depending on implementation. | ||||||||||
This is considered expected behaviour, and implementations can internally track which events they've | ||||||||||
already auto-redacted to avoid duplicate work. | ||||||||||
|
||||||||||
With respect to the fallback behaviour, a client might not know if a server is applying fallback | ||||||||||
redactions and may not wish to wait an arbitrary amount of time to see if it does. One solution would | ||||||||||
be to have the server expose a [capability](https://spec.matrix.org/v1.15/client-server-api/#capabilities-negotiation), | ||||||||||
however such a flag would be longer lived than the fallback behaviour itself (hopefully). Instead, | ||||||||||
clients which don't implement watchdog functionality SHOULD send redactions anyway, even if it | ||||||||||
duplicates the server's fallback efforts. Further, as already mentioned above, server MAY deduplicate | ||||||||||
redactions to lower their federation load, though this is closer to a SHOULD considering clients are | ||||||||||
already sending their own redaction events (like in the case of Mjolnir). | ||||||||||
|
||||||||||
## Alternatives | ||||||||||
|
||||||||||
Mass redactions are the cited major alternative, where a single event can target approximately 1500 | ||||||||||
other events in the room. New rooms can benefit from that functionality, especially for cases not | ||||||||||
covered by this proposal, while existing rooms can be given an option to protect their users with | ||||||||||
relative ease. | ||||||||||
|
||||||||||
## Future considerations | ||||||||||
|
||||||||||
It may be desirable to place this behaviour on self-leaves too, allowing for faster removal of one's | ||||||||||
own messages/events. This proposal doesn't suggest adding this functionality here to maintain narrow | ||||||||||
scope on T&S functionality. A future proposal may introduce this, or rely on regular mass redactions | ||||||||||
instead. | ||||||||||
|
||||||||||
## Security considerations | ||||||||||
turt2live marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
|
||||||||||
As the room moderator/administrator would already send redactions, and may still for full protection, | ||||||||||
it's not deemed any more risk than today. This may change if self-leaves are permitted to also carry | ||||||||||
the field. | ||||||||||
|
||||||||||
There may also be implementation or reliability bugs which inhibit the "stop redacting now" point | ||||||||||
from working as intended. Server implementations in particular should ensure that an event received | ||||||||||
after a membership event which asks for redaction is *really* affected by that redaction. ie: whether | ||||||||||
it's just a late delivery, or if there's a join waiting for state res to make a determination. | ||||||||||
|
||||||||||
## Unstable prefix | ||||||||||
|
||||||||||
While this proposal is not considered stable, implementations should use `org.matrix.msc4293.redact_events` | ||||||||||
instead of `redact_events`. | ||||||||||
|
||||||||||
## Dependencies | ||||||||||
|
||||||||||
This MSC has no direct dependencies. | ||||||||||
|
||||||||||
## Credits | ||||||||||
|
||||||||||
Credit goes to Erik of the Spec Core Team for the suggestion to look into this. |
Uh oh!
There was an error while loading. Please reload this page.
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.
Implementation requirements:
Other non-qualifying (as of writing) implementations: