Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Add an admin API for shadow-banning users. #9209

Merged
merged 3 commits into from
Jan 25, 2021
Merged

Conversation

clokep
Copy link
Member

@clokep clokep commented Jan 22, 2021

This expands the current shadow-banning feature to be usable via the admin API and adds documentation for it.

A shadow-banned users receives successful responses to their client-server API requests, but the events are not propagated into rooms.

Shadow-banning a user should be used as a tool of last resort and may lead to confusing or broken behaviour for the client.

Most of this PR is just the mechanics of piping through from the REST servlet layer to the datastore, although invalidating the cache is a bit wobbly. I don't think we have a helper for invalidating multiple cache entries at once, but I didn't want to blow away the whole cache.

This builds on the piece of work originally done in #8028 by adding an admin API. (That PR was split up so there's a bunch of other PRs that were actually merged. It is a bit confusing, sorry about that.)

This expands the current shadow-banning feature to be usable via
the admin API and adds documentation for it.

A shadow-banned users receives successful responses to their
client-server API requests, but the events are not propagated into rooms.

Shadow-banning a user should be used as a tool of last resort and may lead
to confusing or broken behaviour for the client.
@clokep clokep force-pushed the clokep/shadow-ban-admin branch from 2109708 to 22fc1bf Compare January 22, 2021 17:07
@clokep clokep requested a review from a team January 22, 2021 17:37
@clokep clokep merged commit 4a55d26 into develop Jan 25, 2021
@clokep clokep deleted the clokep/shadow-ban-admin branch January 25, 2021 19:49
@dkoao
Copy link

dkoao commented Feb 3, 2021

IMHO, shadow banning the user is never a good thing. You should be clear and upfront with the user if you don't want them to participate in your servers.

It's honestly disappointing seeing this behaviour from Matrix.

@ghost
Copy link

ghost commented Feb 3, 2021

Very sad that it has come to this even on Matrix. As the owner of one of the larger homeservers, I think that this will deeply undermine the trust of our users in the service.

@pizzadude
Copy link

pizzadude commented Feb 3, 2021

Shadow banning is evil. Don't be evil. This should be un-merged.

@callahad
Copy link
Contributor

callahad commented Feb 3, 2021

Hi. I made the call for us to merge this.

You're absolutely correct that shadowbanning is aggressively disrespectful, and should not be used for routine moderation. However, I do believe that legitimate scenarios exist where it is one of the only effective mitigations against persistent, intentional, malicious behavior. And thus, it should be optionally available as a tool of last resort to server admins.

The strength of federated systems is that each community is empowered to set its own policies around moderation, including whether or not shadowbanning is ever considered acceptable on their homeserver.

I think that this will deeply undermine the trust of our users in the service.

@gardenBanter Are there ways we could mitigate that diminution in trust?

Perhaps some sort of requirement around intent and disclosure, e.g., making the API disabled by default unless overridden in the config, and somehow publicly exposing whether or not the API is turned on? Or automated transparency reports around its usage on a homeserver?

@MiguelX413
Copy link

I also think it should be unmerged.

@9Morello
Copy link

9Morello commented Feb 3, 2021

Please, reconsider this.. I also believe this is better off unmerged.

Perhaps some sort of requirement around intent and disclosure, e.g., making the API disabled by default unless overridden in the config, and somehow publicly exposing whether or not the API is turned on? Or automated transparency reports around its usage on a homeserver?

@callahad Thanks for taking your time to write a serious reply to concerned users. I'm not the person you asked, but imo both are steps in the right direction.

@ghost
Copy link

ghost commented Feb 3, 2021

Shadowbanning is inherently deceitful. This should be unmerged.

@arthurrasmusson
Copy link

Please remove this functionality.

@krruzic
Copy link

krruzic commented Feb 3, 2021

Please do both of these. I think the other admins concern about trust is a really good point, the idea that an admin can invisibly and permanently silence someone will both lower signups and increase abandonment. The bare minimum is SOME transparency.

Perhaps some sort of requirement around intent and disclosure, e.g., making the API disabled by default unless overridden in the config, and somehow publicly exposing whether or not the API is turned on?

@licentiapoetica
Copy link

This is a big ooof, please unmerge this

@ara4n
Copy link
Member

ara4n commented Feb 3, 2021

Hi all: we added shadowbanning back in August in #8034, and while I completely agree that it should never be used for moderation purposes, it is nevertheless useful as a measure of last resort against spammers. We're not going to back it out, nor are we going to back out this PR which simply exposes an API for the server admin (n.b. not room admins) to use.

Meanwhile, it's worth noting there are much worse things that server admins can do to abuse users than shadowbanning (e.g. steal their access_token and snoop on their account; tarpitting them; randomly dropping traffic), so I really don't think this should be controversial.

Something we could consider doing is to turn it off by default, to discourage casual use, and keep it as an emergency measure.

Thanks all for the feedback on this; I think we have enough datapoints to know how strongly some folks feel about this - I'm going to lock the issue now.

@matrix-org matrix-org locked as too heated and limited conversation to collaborators Feb 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.