Page MenuHomePhabricator

Rate limits for Temporary account should match those for anon users
Closed, ResolvedPublic

Description

Motivation

For IP Masking MVP we want to keep the experience for temporary users as consistent as possible with the current experience of unregistered editors (with the exception of Notifications). This is so that we don't introduce too many major changes all at once. In the post-IP Masking MVP future these changes can be re-evaluated.

Spec

Update: rather than define new rate limits, we confirmed that the existing anon rate limits will apply and clarified this in documentation.

Notes:

Event Timeline

I think the newbie category means that temporary accounts are already being rate limited like IPs:

Further reading

I think the newbie category means that temporary accounts are already being rate limited like IPs:

We should probably update https://www.mediawiki.org/wiki/Manual:Autoconfirmed_users to mention that temp users can't be added to the autoconfirmed group. Similarly, we might want to mention in https://www.mediawiki.org/wiki/Manual:$wgRateLimits that 'newbie' limits would apply to temp users. Finally, https://www.mediawiki.org/wiki/Manual:$wgAutopromote also doesn't mention that temp users are excluded from auto promote config.

  • Users in the newbie category are treated like IPs (config)

Would make sense to duplicate the rate limit config for newbies to a temp category, so that we can control that independently of the rate limits applied to IPs? When we roll this out, we might see in practice that we need to have different rate limits applied for temp users from full user accounts in the newbie category. But, I suppose we could always create that configuration if see that it's needed.

We should probably update https://www.mediawiki.org/wiki/Manual:Autoconfirmed_users to mention that temp users can't be added to the autoconfirmed group. Similarly, we might want to mention in https://www.mediawiki.org/wiki/Manual:$wgRateLimits that 'newbie' limits would apply to temp users. Finally, https://www.mediawiki.org/wiki/Manual:$wgAutopromote also doesn't mention that temp users are excluded from auto promote config.

Thanks - these three pages have been updated.

Would make sense to duplicate the rate limit config for newbies to a temp category, so that we can control that independently of the rate limits applied to IPs? When we roll this out, we might see in practice that we need to have different rate limits applied for temp users from full user accounts in the newbie category. But, I suppose we could always create that configuration if see that it's needed.

I think I'm tempted to err on the side of not adding more complexity to the RateLimiter until we know we need to here.

Newbies are subject to the newbie rate limits as well as the ip and subnet limits (code here), so my gut feeling is that, if those limits are currently working then they would probably work for temp accounts too.

In theory having a more permissive newbie limit could allow you to get round the ip-all and subnet-alllimits (code here), but we don't seem to have config like that set up in production (here's the config again).

Change 981370 had a related patch set uploaded (by Tchanders; author: Tchanders):

[mediawiki/core@master] RateLimiter: Add condition for temporary users

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

We should probably update https://www.mediawiki.org/wiki/Manual:Autoconfirmed_users to mention that temp users can't be added to the autoconfirmed group. Similarly, we might want to mention in https://www.mediawiki.org/wiki/Manual:$wgRateLimits that 'newbie' limits would apply to temp users. Finally, https://www.mediawiki.org/wiki/Manual:$wgAutopromote also doesn't mention that temp users are excluded from auto promote config.

Thanks - these three pages have been updated.

Would make sense to duplicate the rate limit config for newbies to a temp category, so that we can control that independently of the rate limits applied to IPs? When we roll this out, we might see in practice that we need to have different rate limits applied for temp users from full user accounts in the newbie category. But, I suppose we could always create that configuration if see that it's needed.

I think I'm tempted to err on the side of not adding more complexity to the RateLimiter until we know we need to here.

Newbies are subject to the newbie rate limits as well as the ip and subnet limits (code here), so my gut feeling is that, if those limits are currently working then they would probably work for temp accounts too.

In theory having a more permissive newbie limit could allow you to get round the ip-all and subnet-alllimits (code here), but we don't seem to have config like that set up in production (here's the config again).

Copying over my comments from Slack; my thoughts on this are:

  1. if someone (say, SRE) wants to figure out what rate limits apply to temp users, there's nothing in operations/mediawiki-config to guide them on that. So if we don't create a separate rate limit config for temp users, then I think we should update operations/mediawiki-config to clarify "newbie" applies to temp users
  2. I personally find it confusing to config that indirectly maps to other categories, e.g. a config that says "newbie" but also applies to a different category of user (temp account) is not straightforward to model mentally and has discoverability issues
  3. it's possible we'll end up needing different rate limits for temp account "newbie" users vs full account "newbie" users. Though we don't really know what to expect from temp account users, it is likely that the behavior will be different in some ways from IP users (and full account newbie users) and it would be nice to be prepared ahead of time to be able to adjust rate limits.

Anyway, this is not a difficult-to-undo decision either way, so I think if there is a clear majority view for one approach vs the other, I am happy to go with that. If we don't create separate RateLimit config for temp accounts, though, I think we should update the relevant config in operations/mediawiki-config to clarify which rate limits would apply.

We should probably update https://www.mediawiki.org/wiki/Manual:Autoconfirmed_users to mention that temp users can't be added to the autoconfirmed group. Similarly, we might want to mention in https://www.mediawiki.org/wiki/Manual:$wgRateLimits that 'newbie' limits would apply to temp users. Finally, https://www.mediawiki.org/wiki/Manual:$wgAutopromote also doesn't mention that temp users are excluded from auto promote config.

Thanks - these three pages have been updated.

Would make sense to duplicate the rate limit config for newbies to a temp category, so that we can control that independently of the rate limits applied to IPs? When we roll this out, we might see in practice that we need to have different rate limits applied for temp users from full user accounts in the newbie category. But, I suppose we could always create that configuration if see that it's needed.

I think I'm tempted to err on the side of not adding more complexity to the RateLimiter until we know we need to here.

Newbies are subject to the newbie rate limits as well as the ip and subnet limits (code here), so my gut feeling is that, if those limits are currently working then they would probably work for temp accounts too.

In theory having a more permissive newbie limit could allow you to get round the ip-all and subnet-alllimits (code here), but we don't seem to have config like that set up in production (here's the config again).

Copying over my comments from Slack; my thoughts on this are:

  1. if someone (say, SRE) wants to figure out what rate limits apply to temp users, there's nothing in operations/mediawiki-config to guide them on that. So if we don't create a separate rate limit config for temp users, then I think we should update operations/mediawiki-config to clarify "newbie" applies to temp users
  2. I personally find it confusing to config that indirectly maps to other categories, e.g. a config that says "newbie" but also applies to a different category of user (temp account) is not straightforward to model mentally and has discoverability issues
  3. it's possible we'll end up needing different rate limits for temp account "newbie" users vs full account "newbie" users. Though we don't really know what to expect from temp account users, it is likely that the behavior will be different in some ways from IP users (and full account newbie users) and it would be nice to be prepared ahead of time to be able to adjust rate limits.

Anyway, this is not a difficult-to-undo decision either way, so I think if there is a clear majority view for one approach vs the other, I am happy to go with that. If we don't create separate RateLimit config for temp accounts, though, I think we should update the relevant config in operations/mediawiki-config to clarify which rate limits would apply.

@STran @Dreamy_Jazz could you review this and leave your comments on this as well, please?

This comment was removed by Dreamy_Jazz.

I will re-word my thoughts from above to be a bit clearer.

I don't see a need for a separate rate limit category for temporary accounts. This is because:

  1. The IP rate limits will still apply to temporary accounts as they will always be a newbie. As such, we can apply IP limits which will affect all temporary accounts on a given IP (along with other newbie accounts). Therefore, if it is really easy to make a temporary account compared to a named account we can make IP rate limits more strict to combat abuse.
  2. The newbie rate limits will apply to temporary accounts. As such, if it is as hard to get a named account as a temporary account then we should still treat both as equivalent for rate limits.
  3. Adding an extra category may make sense, but this third category makes it harder IMO to understand what rate-limit category actually affects temporary accounts for an action as there are now three categories that apply, and two of which apply to single accounts.

However, I am not strongly opposed to adding a new category for temporary accounts.

Change 989569 had a related patch set uploaded (by Tchanders; author: Tchanders):

[operations/mediawiki-config@master] Add comment to clarify which rate limits apply to temporary users

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

Thanks everyone who has commented here and on gerrit.

Anyway, this is not a difficult-to-undo decision either way, so I think if there is a clear majority view for one approach vs the other, I am happy to go with that. If we don't create separate RateLimit config for temp accounts, though, I think we should update the relevant config in operations/mediawiki-config to clarify which rate limits would apply.

OK it looks like we should hold off on the new rate limit category for now, so instead I've added a comment to the config in https://gerrit.wikimedia.org/r/989569. I'll abandon the other patch for now.

Change 981370 abandoned by Tchanders:

[mediawiki/core@master] RateLimiter: Add condition for temporary users

Reason:

Not needed for now as discussed in T331576#9451400

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

Change 989569 merged by jenkins-bot:

[operations/mediawiki-config@master] Add comment to clarify which rate limits apply to temporary users

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

Mentioned in SAL (#wikimedia-operations) [2024-01-10T21:25:34Z] <dreamyjazz@deploy2002> Started scap: Backport for [[gerrit:989569|Add comment to clarify which rate limits apply to temporary users (T331576)]]

Mentioned in SAL (#wikimedia-operations) [2024-01-10T21:27:15Z] <dreamyjazz@deploy2002> dreamyjazz and tchanders: Backport for [[gerrit:989569|Add comment to clarify which rate limits apply to temporary users (T331576)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Mentioned in SAL (#wikimedia-operations) [2024-01-10T21:33:40Z] <dreamyjazz@deploy2002> Finished scap: Backport for [[gerrit:989569|Add comment to clarify which rate limits apply to temporary users (T331576)]] (duration: 08m 05s)