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

Rate limited resource handlers #182

Merged

Conversation

kamathmanu
Copy link
Contributor

Closes #23 (for the most part, refer to the exceptions section below). Since the actual logic for rate-limiting is not in place (yet), this simply builds but doesn't actually do anything right now.

Exceptions

  • The following endpoints belong to groups in Matrix, which are not documented very well; based on a discussion on the Matrix spec room, it seems that Matrix will get rid of group endpoints in the future anyways so it's best to ignore these endpoints and their handlers for now.

    • POST _matrix/client/r0/create_group
    • GET /_matrix/client/r0/joined_groups
    • GET /_matrix/client/r0/publicised_groups & POST /_matrix/client/r0/publicised_groups
    • Any miscellaneous group-related endpoint not mentioned here.
  • The following family of endpoints are related to rooms and are defined in modules/client/rooms/rooms.h. These endpoints share common GET/PUT/POST handlers. For the purpose of satisfying the general motivation behind rate-limiting (prevent spam/DDoS attacks against the server), it is acceptable to ignore rate-limiting for GETs for rooms, and impose a fairly relaxed rate for PUTs & POSTs.

    • GET /_matrix/client/r0/rooms/{roomId}/aliases
    • POST /_matrix/client/r0/rooms/{roomId}/invite
    • POST /_matrix/client/r0/rooms/{roomId}/join
    • POST /_matrix/client/r0/rooms/{roomId}/leave
    • POST /_matrix/client/r0/rooms/{roomId}/forget
  • Handlers for PUT /_matrix/client/r0/pushrules/* have been ignored. This is because the specs for rate-limiting vary based on the parameters (path/JSON) passed in. This also complicates the design decision for rate-limiting, since it warrants thought whether to rate-limit such endpoints at the abstract HTTP level, or at the application-logic level.

  • Handlers for Endpoints related to 3pid are ignored since they aren't really implemented right now. This includes the /_synapse/admin/v1/deactivate/ endpoint defined in the Admin API since it is related to 3pid.

  • The following endpoint: GET /_matrix/federation/unstable/rooms/ is said to be undocumented in the comments, and as such has been ignored.

@@ -64,6 +64,7 @@ post_3pid
{
account_3pid, "POST", post__3pid,
{
post_3pid.REQUIRES_AUTH
post_3pid.REQUIRES_AUTH |
post_3pid.RATE_LIMITED // revisit this? some of these require rate limiting, some don't
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably best to remove this since I documented in my comment that 3pid is ignored

@jevolk jevolk merged commit a3dadaf into matrix-construct:master Jul 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Many resource handlers are missing the RATE_LIMITED flag as per the spec.
2 participants