Page MenuHomePhabricator

Investigate implementing requirements for membership of local groups
Closed, ResolvedPublic

Description

Background

Some groups have requirements for members, for example a minimum edit count or enabling 2FA. These are implemented ad hoc, sometimes via software restrictions and sometimes via manual checking. To reduce manual burden on users who assign groups, and to ensure policies are adhered to, investigate building a framework within MediaWiki for configuring user requirements per group.

Questions

Scope:

  • Other than temporary-account-ip-viewer and 2FA-requiring groups, which other groups have requirements?
  • What are the different ways that group requirements are enforced?

2FA:

  • OATHAuth calculates on-the-fly whether a user should be entitled to a group's privileges, based on whether they have 2FA enabled. How would this fit in to a more generic framework? How could we remove a user from a group, or prevent them being added, if they do not meet this requirement?

General technical approach (see also T393615#10801569):

  • Can/should we re-use UserGroupManager::recCheckCondition and UserGroupManager::checkCondition?
  • How could we handle requirements checking when adding a user to a group? (UserGroupManager::addUserToGroup)
    • Can we re-use designs for temporary-account-ip-viewer on Special:UserRights?
    • Can we provide a useful error message via ApiUserrights?
    • How could we allow performers from certain groups to be exempt from target requirements checking?
  • How could we handle requirements checking when checking a user's groups? (UserGroupManager::getUserEffectiveGroups)
    • If this fails, how could we remove the user from the group, update the expiry, warn the user, etc?
  • Global groups are defined differently (DB table rather than config), and membership is handled by different software (CentralAuth's SpecialGlobalGroupMembership). Name conflicts are allowed between local and global groups. Would any of this be re-usable for global groups, or would it need to be re-implemented?

Event Timeline

Scope:

  • Other than temporary-account-ip-viewer and 2FA-requiring groups, which other groups have requirements?
  • What are the different ways that group requirements are enforced?

Summary

I didn't find other group/rights requirements being enforced by software.

Details

I checked hooks run by UserGroupManager and PermissionManager.

Hooks run from UserGroupManager:

  • onUserAddGroup, onUserRemoveGroup - unused
  • onGetAutompromoteGroups, onAutopromoteCondition - we won't change these
  • onUserEffectiveGroups - only OATHAuth uses this (for 2FA)
  • onUserGroupsChanged - lots of handlers; check they don't change the groups

Hooks run from PermissionManager:

  • onUserGetRights, onUserGetAllRights, onUserCan, onUserPrivilegedGroups - has a few handlers, but none restrict rights, just add
  • onUserPrivilegedGroups - used by EventLogging for logging certain activities more closely
  • onUserGetRightsRemove - no handlers, but docs say it's intended for removing rights
  • other hooks are not relevant to restricting rights

2FA:
OATHAuth calculates on-the-fly whether a user should be entitled to a group's privileges, based on whether they have 2FA enabled. How would this fit in to a more generic framework? How could we remove a user from a group, or prevent them being added, if they do not meet this requirement?

Summary

We can fit 2FA requirements into a more generic framework by allowing OATHAuth to extend a core config, and hook into a core service that checks this config, similar to how extensions can hook into autopromote.

We can prevent a user being added via this config.

If they are added and then lose 2FA, we should delegate the consequence of this to OATHAuth. There would likely be very different consequences for different groups. It may be too rigid to define one consequence for all types of restricted groups.


General technical approach (see also T393615#10801569):
Can/should we re-use UserGroupManager::recCheckCondition and UserGroupManager::checkCondition?

Yes. We should make a new service, e.g. UserEligibilityChecker, and move UserGroupManager::recCheckCondition and ::checkCondition to this service: T406547.

It could be used for checking autopromote, restricted groups members, users who are attempting to change restricted group memberships (as per the suggestion in T393615#10801595) and restricted rights.

How could we handle requirements checking when adding a user to a group? (UserGroupManager::addUserToGroup)

I'm imagining some kind of RestrictedGroupValidator service that would use UserEligibilityChecker. It could be used when checking if a group can be added for a target, if a performer can perform a change, and if a group member can actually use their group.

Can we re-use designs for temporary-account-ip-viewer on Special:UserRights?

Yes: T406003, T117884

How could we allow performers from certain groups to be exempt from target requirements checking?

We can let the config determine this. Note that it will not then be possible to enforce that a members pass the conditions for this group, because we won't know whether they were legitimately added by a user who bypassed the checks.

How could we handle requirements checking when checking a user's groups? (UserGroupManager::getUserEffectiveGroups)

Call the RestrictedGroupValidator from getUserEffectiveGroups, as mentioned above. Note that it won't be possible for groups where another user is allowed to add the user to the group without meeting the conditions.

If this fails, how could we remove the user from the group, update the expiry, warn the user, etc?

I think it should be possible to handle this case-by-case. We could by default just treat them as they are not in the group (as OATHAuth currently does). But we should run a hook so that extensions can add their own reaction (e.g. OATHAuth may want to remove users who fail 2FA checks from the group).

Filling in the missing questions:

Can we provide a useful error message via ApiUserrights?

I think we should be able to, following work done to share logic between the special pages and the APIs (task TBC).

Global groups are defined differently (DB table rather than config), and membership is handled by different software (CentralAuth's SpecialGlobalGroupMembership). Name conflicts are allowed between local and global groups. Would any of this be re-usable for global groups, or would it need to be re-implemented?

We should definitely re-use things like eligibility checking and group validity checking.

If we keep allowing group names to conflict then we will need a separate config for restricted global groups, and then whichever service checks the config will need to be able to be adapted to check either config. This might be a lower lift than requiring that global and local group names cannot conflict.

I'll close this task, and further discussion can happen on T406544: Create a way to technically enforce policies for restricted groups, where the concrete proposal that came from this investigation is outlined.