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

Inaccurate error message: "You don't have permission to ban" when banning or unbanning someone with same or higher PL. #16202

Closed
Gnuxie opened this issue Aug 28, 2023 · 3 comments · Fixed by #16205
Labels
good first issue Good for newcomers O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@Gnuxie
Copy link

Gnuxie commented Aug 28, 2023

Description

POST /_matrix/client/v3/rooms/!NmyHBEdVXAZOzxAdBa%3Alocalhost%3A9999/ban: 403 Forbidden -- {"errcode":"M_FORBIDDEN","error":"You don't have permission to ban"}

When possessing the power level to ban, this error message is inaccurate and confusing to users with clients that pass through some error messages from Synapse to the user (e.g. Mjolnir/Draupnir and Nheko).

Steps to reproduce

  • have a room with two moderators
  • try to ban the other moderator by calling /_matrix/client/v3/rooms//ban
  • you will receive a standard matrix error response with this text in the body.

Instead it should say something like:

"You don't have permission to ban this user" or "You don't have a high enough power level to ban this user".

Homeserver

localhost

Synapse Version

{"server_version":"1.90.0","python_version":"3.11.4"}

Installation Method

Docker (matrixdotorg/synapse)

Database

PostgreSQL

Workers

Single process

Platform

NA

Configuration

No response

Relevant log output

NA

Anything else that would be useful to know?

No response

@clokep
Copy link
Member

clokep commented Aug 28, 2023

The error comes from:

if user_level < ban_level or user_level <= target_level:
raise UnstableSpecAuthError(
403,
"You don't have permission to ban",
errcode=Codes.INSUFFICIENT_POWER,
)

Looks like it would be easy enough to split those conditionals if needed (or add the "this user" as you suggest).

@clokep clokep added good first issue Good for newcomers S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. O-Occasional Affects or can be seen by some users regularly or most users rarely labels Aug 28, 2023
@leviosacz
Copy link
Contributor

Hey @clokep
Here is the PR: #16205
Could you please take a look and see if I missed something?
Thanks!

@DMRobertson
Copy link
Contributor

(More generally, it seems like a spec deficiency if you can't distinguish between these error cases using the errcode alone.)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue Good for newcomers O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
4 participants