Skip to content
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

Redacting a kick/ban shouldn't redact the reason #1996

Closed
turt2live opened this issue May 13, 2019 · 6 comments
Closed

Redacting a kick/ban shouldn't redact the reason #1996

turt2live opened this issue May 13, 2019 · 6 comments
Labels
client-server Client-Server API improvement A suggestion for a relatively simple improvement to the protocol requires-room-version An idea which will require a bump in room version won't fix

Comments

@turt2live
Copy link
Member

Banning someone for a giant display name can go wrong because clients will be inclined to say "TravisR banned <6000 characters> for: spam", which the moderator probably wants to redact because spam. The reason is currently lost in that redaction, so the record forever shows that the moderator banned someone for no reason.

@turt2live turt2live added improvement A suggestion for a relatively simple improvement to the protocol requires-room-version An idea which will require a bump in room version client-server Client-Server API labels May 13, 2019
@richvdh
Copy link
Member

richvdh commented Jul 14, 2019

I'm not convinced this is the right approach.

Things that are protected from the redaction algorithm should be limited to those which are key to the structure of the room. I worry that if we start opening it up in this way, we'll end up with a huge and unwieldy list.

Also: what a rogue moderator bans somebody with an offensive ban reason? Wouldn't you like to be able to redact the reason?

An alternative approach might be for clients to make sure that, when a join event is redacted (so the displayname is redacted), any places where that displayname is displayed should be updated.

In other words: can this problem be solved at the client level rather than the protocol level?

@turt2live
Copy link
Member Author

Rogue moderators are not a protocol problem to solve: our threat model already acknowledges that moderators can do things to rooms which can be considered undesirable or dangerous.

I'm really not convinced that this can be solved at the client level, as they'd be sending a /redact on the state event, which the server then dutifully applies the algorithm. Although the redaction algorithm is not for convenience features, I would argue that the reason disappearing is an undesirable side effect of the redaction. If we have to maintain two algorithms (redact because protocol and redact because I want to remove irrelevant information), then so be it.

@richvdh
Copy link
Member

richvdh commented Jul 15, 2019

I'm really not convinced that this can be solved at the client level, as they'd be sending a /redact on the state event,

My question is: why are you redacting the ban in the first place? I contend it is a workaround for a client bug.

@richvdh
Copy link
Member

richvdh commented Jul 15, 2019

Rogue moderators are not a protocol problem to solve:

(This is fair. I use the example more as a thought experiment in the things that redacts should or not redact in the theoretical sense than as an actual problem I want to solve.)

@KitsuneRal
Copy link
Member

After re-reading what Travis gave as the original case, I also become unconvinced that redacting the ban/kick itself should be a means to an end. It's not even about rogue moderators or something but the very action of redacting the ban looks a bit counter-intuitive. In particular, I don't think all moderators would readily understand that redacting a ban doesn't lift it (judging by my own experience - it took me quite many months to get to that idea) - so likely to just curse the stupid client (or even the Matrix protocol) that it doesn't "just work".

Maybe let's just clarify the proper behaviour for client authors? This is not quite straightforward either, in fact - what if not the displayname but the user id is offensive? Should it be something like "MrModerator banned , reason: offensive user id"?

@turt2live
Copy link
Member Author

My question is: why are you redacting the ban in the first place? I contend it is a workaround for a client bug.

Because as a moderator it's easier to ban first (stop the spam) then deal with the user's profile later. Yes, clients should be adding all the appropriate flags for overflow handling, however that doesn't feel like a good enough justification to not be helpful at the protocol level.

It sounds like I'm the only one arguing for this though, even external to this issue, so closing as won't fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client-server Client-Server API improvement A suggestion for a relatively simple improvement to the protocol requires-room-version An idea which will require a bump in room version won't fix
Projects
None yet
Development

No branches or pull requests

3 participants