Page MenuHomePhabricator

Add rate limiting to Authority
Closed, ResolvedPublic

Description

Per the original proposal (T231930), Authority should be responsible for applying rate limits to actions they authorize. The idea is that callers that ask for an action to be authorized should not know or care what factors into that decision - be it group permissions, user blocks, rate limits, etc.

Rate limits would be tested (but not bumped) by "thorough" checks as performed by definitelyCan() (and, in the future, isDefinitelyAllowed). Methods that "authorize" an action would enforce and bump the limit, this would be the authorizwRead() and authorizeWrite() methods (as well as, in t he future, authorizeDo()).

Performing rate limit checks implicitly for all permission checks allows all code to be removed that currently enforces rate limits explicitly, by calling User::pingLimiter or RateLimiter::limit. It also allows error handling for rate limits to be generalized in the API, so that clients are informed avout limit violations in a uniform way.

Event Timeline

daniel triaged this task as Medium priority.

Change 809295 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[mediawiki/core@master] Experiment: Implement rate limiting in Authority.

https://gerrit.wikimedia.org/r/809295

Change 879862 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[mediawiki/core@master] Rename rate limits to match permission names

https://gerrit.wikimedia.org/r/879862

Change 879862 merged by jenkins-bot:

[mediawiki/core@master] Rename rate limits to match permission names

https://gerrit.wikimedia.org/r/879862

Change 930934 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[mediawiki/core@master] RateLimiter: collect statistics

https://gerrit.wikimedia.org/r/930934

Change 930934 merged by jenkins-bot:

[mediawiki/core@master] RateLimiter: collect statistics

https://gerrit.wikimedia.org/r/930934

See also T134953: Merge Throttler and ping limiter - Throttler handles login and signup, which is not handled by Authority (signup somewhat is, via the createaccount user right, but most permission checks happen elsewhere) but it would still be nice to have a single component for rate limiting.

Change 814359 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[mediawiki/core@master] Introduce authorizeDo() into Authority.

https://gerrit.wikimedia.org/r/814359

Change 809295 merged by jenkins-bot:

[mediawiki/core@master] Implement rate limiting in Authority.

https://gerrit.wikimedia.org/r/809295

Change 814359 merged by jenkins-bot:

[mediawiki/core@master] Introduce authorizeAction() into Authority.

https://gerrit.wikimedia.org/r/814359