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

Do not rate limit invites if an appservice has rate_limited: False set #9663

Closed
Half-Shot opened this issue Mar 22, 2021 · 5 comments · Fixed by #9711
Closed

Do not rate limit invites if an appservice has rate_limited: False set #9663

Half-Shot opened this issue Mar 22, 2021 · 5 comments · Fixed by #9711
Assignees
Labels
A-Application-Service Related to AS support A-Invite Inviting users to rooms and accepting invites S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. X-Regression Something broke which worked on a previous release

Comments

@Half-Shot
Copy link
Collaborator

At the moment we are having issues with an EMS script that creates rooms en masse for customers looking to migrate. However calls made to /createRoom will fail due to a recent change to rate limit the number of invites allowed at room creation time. It would be good if the rate_limited: False flag could be respected here, and the limiting be disabled for users attempting to create rooms inside the AS namespace.

@Half-Shot
Copy link
Collaborator Author

(Goes without saying that this used to work before the invites were rate limited, so this is a recent breakage)

@rxl881
Copy link

rxl881 commented Mar 23, 2021

@Half-Shot -- Thanks for flagging this.

@callahad is it intentional that the rate limiting flag isn't being respected in this case? If not, is it possible to get a fix scheduled relatively quickly? It is currently breaking the EMS migration tools :(

@clokep clokep added S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. X-Regression Something broke which worked on a previous release labels Mar 23, 2021
@Half-Shot
Copy link
Collaborator Author

Half-Shot commented Mar 24, 2021

This will also be impacting any bridges that need to invite users to "admin rooms" to a degree, although there is an argument that the bridge should retry on 429 when inviting.

I think that users are allowed to do a new invite every 5 minutes (by default), which is going to be painfully slow for something like the Freenode bridge when it's handling a invite-only room (as invite only rooms require that the bridge bot invite ghost users to a room when they join the IRC side)

This will also break the use case where the bridge will try to create a channel with a prefilled invite list of users of more than 5 users (by default), which we do when doing a migration or as part of the Slack bridge's group DM system. I'm sure there are other bridges out there that run a createRoom with an invite list of >5 too.

@callahad
Copy link
Contributor

We think we can fix the immediate issue in short order (this time next week), but also expect similar one-off issues to continue cropping up with the current design of our rate limiting.

Until we manage to more deeply refactor our implementation, just let us know when you hit another one of these and we'll knock that down, too. :)

@Half-Shot
Copy link
Collaborator Author

Happy to holler when things break :)

@erikjohnston erikjohnston self-assigned this Mar 25, 2021
erikjohnston added a commit that referenced this issue Mar 30, 2021
This should fix a class of bug where we forget to check if e.g. the appservice shouldn't be ratelimited.

We also check the `ratelimit_override` table to check if the user has ratelimiting disabled. That table is really only meant to override the event sender ratelimiting, so we don't use any values from it (as they might not make sense for different rate limits), but we do infer that if ratelimiting is disabled for the user we should disabled all ratelimits.

Fixes #9663
@MadLittleMods MadLittleMods added A-Application-Service Related to AS support A-Invite Inviting users to rooms and accepting invites labels Dec 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Application-Service Related to AS support A-Invite Inviting users to rooms and accepting invites S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. X-Regression Something broke which worked on a previous release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants