Page MenuHomePhabricator

Setting a temporary usergroup (allow expiry of user rights via Special:UserRights form)
Closed, ResolvedPublic

Description

Author: Davide21.Wikipediano

Description:
On italian wikipedia, due a recently found mass copyvioler user, came up the need of some temporary sysops. I think it would be quite useful that bureaucrats have the opportunity to set the duration for the flag, as normally occours with blocks or protections. The majority of adminship duration will be for example "infinite", but for some case it would be useful to have a adminship that expires. Thank you in advance for your consideration,
Davide21.

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

(...) and the interface would be much more complex too. (...)

No, you just need a dropdown for the expiry time, and a field for custom time, like at Special:Block. (If you want to set permanent, you can set expiry=never)

(...) how is this going to be displayed in the log? (...)

Just like the block log. For example:

20:38, 24 November 2015 Luke081515 (talk | contribs | block) changed group membership for Luke from editor to administrator and editor (temporary) with an expiry time of 2 weeks

That's my proposal.

(...) and the interface would be much more complex too. (...)

No, you just need a dropdown for the expiry time, and a field for custom time, like at Special:Block. (If you want to set permanent, you can set expiry=never)

(...) how is this going to be displayed in the log? (...)

Just like the block log. For example:

20:38, 24 November 2015 Luke081515 (talk | contribs | block) changed group membership for Luke from editor to administrator and editor (temporary) with an expiry time of 2 weeks

That's my proposal.

Hmm, not sure. It can be useful, yes, but sometimes people request a userright for a week (for example), but then find out they need it a bit longer. With this 'feature' we would need to extend it; while with the current tool we can just leave the rights. I think it's much better to handle this manually... at least on WMF wikis.

Change 328377 had a related patch set uploaded (by TTO):
[WIP] User group memberships that expire

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

Does every group geht its own expiry time? If not, I think this isn't a really good idea.

How do temporary groups relate to user_former_groups ? I guess the group should be added there regardless of it being removed manually or automatically?

@MGChecker: it does

This would be a useful feature, but to code it would be really complicated in my opinion and the interface would be much more complex too.

Turned out not to be too bad after all :) Although my patch is just a start. In particular, it doesn't do anything about global groups.

If each group has an expiry time, single entries can be really long.

In practice, I expect it's unlikely that a user will have more than one "temporary" group membership at a time. Of course, such assignments are possible, and are handled correctly by the software, but I don't see it as a very common state of affairs.

Does every group geht its own expiry time?

Yes; every group a user is explicitly assigned to (such as "bureaucrat", not implicit groups like "user" or "autoconfirmed") has an expiry for that particular user, which may be NULL (meaning the membership does not expire).

How do temporary groups relate to user_former_groups ? I guess the group should be added there regardless of it being removed manually or automatically?

Expired groups are indeed added to user_former_groups. The patch also fixes a bug where remote removal of local groups via UserRightsProxy (e.g. where a steward at metawiki revokes a user's admin permissions on xxwiki) didn't add the group to user_former_groups.

I should add that I really just dived in and implemented this the way I thought it should be implemented. I'm very much open to any feedback.

Change 328377 had a related patch set uploaded (by TTO):
[WIP] User group memberships that expire

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

Just for reference, since I didn't know or forgot about user_former_groups, this table was added in https://www.mediawiki.org/wiki/Special:Code/MediaWiki/90749 (June 2011).

I think modeling after blocks is smart. I haven't looked at the linked Gerrit changeset much, but I think using a similar expiration interface with a customizable drop-down menu of common expiries and allowing other time for a custom time makes sense. Instead of storing indefinite user group assignments as NULL, they could be stored as infinity like we do for ipblocks.ipb_expiry. Though perhaps this mix of stringy timestamps and stringy durations for a field is more of an anti-pattern.

Change 328491 had a related patch set uploaded (by TTO):
Disregard expired user_group rows in special page and API DB queries

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

TTO added a subscriber: jcrespo.

Instead of storing indefinite user group assignments as NULL, they could be stored as infinity like we do for ipblocks.ipb_expiry.

The code seems to fall out more nicely if NULL is used. Although it does require a little bit of care, the alternative is lots of tricky special-casing and the hard-coded string 'infinity' everywhere. The API still outputs 'infinity' for indefinite memberships.


Adding Schema-change and @jcrespo here. This is going to require schema changes (gerrit 382377), and it will also require modifications to several of the complex queries used in API query modules and special pages (gerrit 328491), which has performance implications for the WMF cluster.

Hello:

  • How the logging of granting indefinite and temporary user groups would look like?
  • Are the expiration of user groups also logged? If so, how would they look like? Under which performer name would it appear?

Thanks.

Not only NULL is preferred over a string because it is used for columns where it means "do not know" or "does not apply", it tends to take less bytes to store it. As usual, depending on how many rows are expected with that value, that may be relevant or not.

I am not very up to date with this functionality, the only things I can see as dangerous is how expiration is handled- you can check at any time membership is expired; make sure cleaning up (if needed) is not done synchronously, so we do not have 1 million rows written/deleted at the same time.

  • How the logging of granting indefinite and temporary user groups would look like?

Currently it is (please excuse the lack of formatting):

14:52, 23 December 2016 TTO (talk | contribs | block) changed group membership for Mary from bot and administrator to bureaucrat (temporary, until 14:52, 23 December 2017), bot and administrator (per Mary's request on IRC)
14:50, 23 December 2016 TTO (talk | contribs | block) changed group membership for Mary from bot to bot and administrator 
22:48, 21 December 2016 TTO (talk | contribs | block) changed group membership for Account from (none) to bot (temporary, until 22:48, 22 December 2016)

although this is flexible to be changed. In particular the word "temporary" could be removed to reduce clutter.

  • Are the expiration of user groups also logged? If so, how would they look like? Under which performer name would it appear?

Expirations are not logged. This matches the current behaviour for blocks and protections, which both expire silently. Although I've always missed the ability to see in a page's history exactly when the protection expired, I think the user rights log for an individual user will be much less noisy than a history page, and it will be easy to see what is going on.

Of course, expirations could be logged:

22:48, 22 December 2016 The Grinch That Stole Your User Rights (talk | contribs | block) changed group membership for Account from bot (temporary, until 22:48, 22 December 2016) to none (Removing permissions that have expired)
22:48, 21 December 2016 TTO (talk | contribs | block) changed group membership for Account from (none) to bot (temporary, until 22:48, 22 December 2016)

But the main problem with that is that the expiration log entries might not be written to the log in a timely fashion. Because we don't like to write things to the database unless someone is actually performing an action, like submitting the Special:UserRights form, the log entry might not be added until as long as several days after the rights expire. (The other problem is that I don't think it is necessary, since no-one seems to have missed the corresponding functionality for blocks and protections - I couldn't find a task in Phabricator older than T148649, which was only filed a couple of months ago.)

Change 328377 merged by jenkins-bot:
User group memberships that expire

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

Change 328491 merged by jenkins-bot:
Disregard expired user_group rows in special page and API DB queries

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

This broke unit tests which created a User object (without storing it in the DB) and added groups to it.

In T12493#2978852, @Tgr wrote:

This broke unit tests which created a User object (without storing it in the DB) and added groups to it.

Sorry about that! That breakage was pointed out in the release notes, but in hindsight. I probably should have put it in the commit message too. Only Wikibase tests had been failing in Jenkins, so I only updated those.

Only a few select extensions are tested for core patches (running all tests would be way too slow).

This feature is enabled on the beta cluster now. Please go and try it out (you'll need some kind of advanced rights on at least one beta cluster site).

In a week or so, if there are no significant implementation issues, I'll mark T155605: Schema changes for expiring user groups as Schema-change-in-production, which will add it to the queue of schema changes to be done in WMF production.

I'm testing it in the beta cluster of Meta-Wiki and it seems it's working fine. Maybe some UI tweaks I'd do, but it looks it's working fine.