Page MenuHomePhabricator

Support declaring conditions for global groups
Closed, ResolvedPublic

Description

The goal of this task is to support defining conditions for members of global groups, so that policies about granting them can be enforced by software.

Approach

Global groups (unlike local ones) are defined in the database and are editable from an on-wiki interface. Therefore, we have two potential options of how group conditions are defined:

  1. Stewards can define conditions somewhere on wiki, and they will be stored in global_group_restrictions DB table (in a new column).
  2. The conditions are defined in PHP config, in a similar way to $wgRestrictedGroups.

Approach 1 would be more consistent with the general flow around global groups, but at the same time would also require to define a new syntax and editor for conditions. Given that they are currently PHP values (primitives and arrays), there's no straightforward way to define them in the user interface. Instead, a custom validator and parser would need to be written. On the other hand, Approach 2 would reuse as much of the existing services as possible, by having the condition formatted the same as for local groups.

Given that I expect that conditions for a given group will be changed extremely rarely, I think it's acceptable to introduce global group conditions using Approach 2, which is simpler from the developer perspective. If it turns out in future that an on-wiki interface is needed, this assumption might be revisited.

As for the configuration variable, we could reuse $wgRestrictedGroups. CentralAuth already reuses local-group-specific patterns in MediaWiki, such as rendering group names from MediaWiki:Group-<name> etc.

Acceptance criteria

  • CentralAuth reads conditions for global groups from $wgRestrictedGroups (from the central wiki).

Event Timeline

from $wgRestrictedGroups

We may want a different configuration name per T410076#11616259.

from $wgRestrictedGroups

We may want a different configuration name per T410076#11616259.

Yes, I've seen that comment, but it was with regard to making local and global groups use unique names. Using $wgRestrictedGroups for both local and global wiki will not require to remove the name collision – just $wgRestrictedGroups['steward'] will apply to both local and global members (which is – I think – advisable).

This behavior is – as far as I understand – fine for both WMF wikis, Miraheze wikis and others, as groups with shared name already share the display name, the group page and probably a few more. So, they are from the user perspective the same groups, just separated into different technical entities.

This behavior is – as far as I understand – fine for both WMF wikis

After internal consultation, it turned out that I forgot about the abusefilter-helper group. We plan to enforce 2FA from the global AFH group, but not from local one.

I still believe that configuring group restrictions in $wgRestrictedGroups is the correct way to go. This will ensure that – for groups who should have common restrictions – the restrictions will be common (i.e., no drift between local and global stewards). But to make it possible to specify which king of groups the restrictions apply to, I'll add support for an additional parameter, tentatively scope, which will allow for that. If it's absent, the restriction is going to apply to all types of groups: T422605: Add 'scope' option in $wgRestrictedGroups.

IMO, narrowing the restrictions scope should be an explicit and conscious decision, so that site admins will not have to keep in mind that certain groups would need two separate configs (e.g., local and global one).

Other point to note:

  1. If the restrictions of global groups drift among wikis, then we may have a loophole like stewards can disable 2FA on some wikis (current OATHManage checks every wikis users is attached on, but we should simply require restrictions of global groups be same among wikis so we only need to check once). Such a drift is easy to prevent though.
  2. Once we start demoting users without 2FA, we should ensure removing of global groups only happens in Meta (i.e. running demoteIneligibleUsers.php in other wikis should not remove global groups).
  1. If the restrictions of global groups drift among wikis, then we may have a loophole like stewards can disable 2FA on some wikis.

I plan to have global group conditions loaded from the central wiki, so that every wiki has the same conditions for global groups. I have that in the WIP code, I missed this detail from the task description.

By drift I meant a case where local steward group gets accidentally different conditions than the global stewards group. (because e.g., $wgRestrictedGroups['steward'] would end up diiferent than hypothetical $wgRestrictedGlobalGroups['steward']).

Such a drift is easy to prevent though; by the way, we should also have a test to ensure if a user requires 2FA due to a local group, they can not disable 2FA from a different wiki.

This already happens (although for local groups, as only local groups are supported now).

  1. Once we start demoting users without 2FA, we should ensure removing of global groups only happens in Meta (i.e. running demoteIneligibleUsers.php in other wikis should not remove global groups).

I plan to have a separate script for global groups, since they are two different concepts, and – at the code level – they are managed in two different ways.

Change #1269336 had a related patch set uploaded (by Mszwarc; author: Mszwarc):

[mediawiki/extensions/CentralAuth@master] GlobalGroupAssignmentService: Support $wgRestrictedGroups conditions

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

Change #1269336 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@master] GlobalGroupAssignmentService: Support $wgRestrictedGroups conditions

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