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.

Sign up for GitHub

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

MSC4151: Reporting rooms #1938

Merged
merged 10 commits into from
Oct 10, 2024
Merged

MSC4151: Reporting rooms #1938

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
fb71935
MSC4151: Reporting rooms
Johennes Aug 26, 2024
ab88044
Add changelog
Johennes Aug 26, 2024
9d63f31
Update changelogs/client_server/newsfragments/1938.clarification
Johennes Aug 27, 2024
1cf5104
Update changelogs/client_server/newsfragments/1938.new
Johennes Aug 27, 2024
3aad778
Add x-addedInMatrixVersion property
Johennes Sep 9, 2024
414001a
Add added-in shortcode
Johennes Sep 9, 2024
8c8c37d
Update content/client-server-api/modules/report_content.md
Johennes Sep 25, 2024
00d4e6d
Update content/client-server-api/modules/report_content.md
Johennes Sep 25, 2024
883e9e3
Update data/api/client-server/report_content.yaml
Johennes Sep 25, 2024
c2b90d5
Merge branch 'main' into johannes/reporting-rooms
Johennes Sep 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelogs/client_server/newsfragments/1938.clarification
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
The `reason` parameter in `POST /_matrix/client/v3/rooms/{roomId}/report/{eventId}` can be omitted instead of left blank as per [MSC2414](https://github.com/matrix-org/matrix-spec-proposals/pull/2414).
1 change: 1 addition & 0 deletions changelogs/client_server/newsfragments/1938.new
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add `POST /_matrix/client/v3/rooms/{roomId}/report` as per MSC4151.
Johennes marked this conversation as resolved.
Show resolved Hide resolved
19 changes: 13 additions & 6 deletions content/client-server-api/modules/report_content.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@ Users may encounter content which they find inappropriate and should be
able to report it to the server administrators or room moderators for
review. This module defines a way for users to report content.

Content is reported based upon a negative score, where -100 is "most
offensive" and 0 is "inoffensive".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this here because it only applies to event reporting and is repeated in the corresponding endpoint's documentation.

#### Client behaviour

{{% http-api spec="client-server" api="report_content" %}}
Expand All @@ -19,6 +16,16 @@ This may be a dedicated room to alert server administrators to the
reported content or some other mechanism for notifying the appropriate
people.

{{< changed-in v="1.8" >}} The server MUST verify that the user
reporting the event is currently joined to the room the event is
in before accepting a report.
Particularly during waves of a harmful content, users may report whole
Johennes marked this conversation as resolved.
Show resolved Hide resolved
rooms instead of individual events. Server administrators and safety teams
should, therefore, be cautious not to shut down rooms that might otherwise
be legitimate.

{{< changed-in v="1.8" >}} When processing event reports, servers MUST
verify that the reporting user is currently joined to the room the event
is in before accepting a report.

Contrarily, servers MUST NOT restrict room reports based on whether or not the
reporting user is joined to the room. This is because users can be exposed
to harmful content without being joined to a room, for instance, through
room directories.
67 changes: 65 additions & 2 deletions data/api/client-server/report_content.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,69 @@ info:
title: Matrix Client-Server Report Content API
version: 1.0.0
paths:
"/rooms/{roomId}/report":
post:
summary: Report a room as inappropriate.
description: |-
Reports a room as inappropriate to the server, which may then notify
the appropriate people. The caller is not required to be joined to the
room to report it.
Johennes marked this conversation as resolved.
Show resolved Hide resolved
operationId: reportRoom
parameters:
- in: path
name: roomId
description: The room being reported.
required: true
example: "!637q39766251:example.com"
schema:
type: string
requestBody:
content:
application/json:
schema:
type: object
example: {
"reason": "this makes me sad"
}
properties:
reason:
type: string
description: The reason the room is being reported.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The MSC made this required to align with the event reporting endpoint. However, MSC2414 made both body parameters on that endpoint optional. Therefore, I made reaaonoptional here, too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the event reporting endpoint already has the parameters marked as optional, and MSC2414 was trying to match that endpoint, making them optional here too looks like the right thing to me.

required: true
security:
- accessTokenQuery: []
- accessTokenBearer: []
responses:
"200":
description: The room has been reported successfully.
content:
application/json:
schema:
type: object
examples:
response:
value: {}
"404":
description: |-
The room was not found on the homeserver.
content:
application/json:
schema:
$ref: definitions/errors/error.yaml
examples:
response:
value: {
"errcode": "M_NOT_FOUND",
"error": "The room was not found."
}
"429":
description: This request was rate-limited.
content:
application/json:
schema:
$ref: definitions/errors/rate_limited.yaml
Johennes marked this conversation as resolved.
Show resolved Hide resolved
tags:
- Reporting content
"/rooms/{roomId}/report/{eventId}":
post:
summary: Report an event in a joined room as inappropriate.
Expand All @@ -29,7 +92,7 @@ paths:
will require the homeserver to check whether a user is joined to
the room. To combat this, homeserver implementations should add
a random delay when generating a response.
operationId: reportContent
operationId: reportEvent
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure what consequences changing this has but the old ID felt unsuitable now that there are two endpoints in the module.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it's used to generate bindings, and as such we probably shouldn't change it...

matrix-spec/meta/documentation_style.rst

Lines 113 to 114 in 2cbf606

* ``operationId``: a camelCased unique identifier for this endpoint. This will
be used to automatically generate bindings for the endpoint.

@turt2live curious if you happen to know more about this?

Copy link
Contributor Author

@Johennes Johennes Sep 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose if we want to be safe, keeping operationId: reportContent on the existing endpoint and operationId: reportRoom on the new one would not be the end of the world. So maybe reverting back would be best?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, operationId usually ends up as the method or class name in generated APIs, and therefore, may (actually will, in the vast majority of cases) break the generated code. However, given that even more fine-grained changes (e.g. adding a field in a request body) that are API-compatible in JSON may be breaking in the generated code (depending on the target language and/or the shape of the API) authors producing auto-generated code have to be careful with any upgrades of the API definitions. In particular, 1.12 has already introduced breaking changes elsewhere from the Quotient perspective. So I wouldn't be too concerned around it, just be cognisant that not everything back-compatible in JSON remains such in a native API.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We concluded in #matrix-docs:matrix.org that changing the operation ID is not ideal but acceptable in this case as otherwise it'll be even harder to fix in future.

parameters:
- in: path
name: roomId
Expand Down Expand Up @@ -62,7 +125,7 @@ paths:
and 0 is inoffensive.
reason:
type: string
description: The reason the content is being reported. May be blank.
description: The reason the content is being reported.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have removed this as proposed in MSC2414 and thought it aligns well enough with the rest of this pull request to sneak in. 😇

required: true
security:
- accessTokenQuery: []
Expand Down