Page MenuHomePhabricator

System users should be in a dedicated user group
Closed, ResolvedPublic

Description

Currently there's no reliable way to identify system users, both backend and frontend. Copying a comment from @Tgr in https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/476492/, we could introduce a new user group which is automatically assigned inside User::newSystemUser and cannot be removed or assigned otherwise. This way, we'd have two benefits:

  • Users will identify system users at a glance via user groups
  • We could have a new method, User::isSystemUser, which would simply check for the group

Event Timeline

Daimona moved this task from Backlog to Next on the User-Daimona board.

Change 481601 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/core@master] Add a new user group exclusive to system users

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

Daimona moved this task from Next to Under review on the User-Daimona board.

While I read the code in a way that you shouldn't be able to set this permission with the userights permission (because of the way User->getAllGroups works), is this displayed on ListGroupRights correctly? It might be ignored there as well an could look weird with no permissions assigned.

@MGChecker Since the group doesn't appear in any global, ListGroupRights simply skips it. In fact, everything skips it and of course cannot be assigned. My only concern is that Special:ListUsers skips it as well. I guess a cleaner solution would be to add a new global to hold this kind of groups (or just hardcode this group as the only "special" one), and use it where needed (e.g. on ListUsers).

I added 'systemuser' to wgImplicitGroups. This should be saner, although it really prevents this group to be shown e.g. on ListUsers. I think this should be fine for now. If we'll find out that we actually want this group to be displayed, there's still time to do it.

What happens if someone does a rights change on a system user? If it's marked an implicit group it will probably get lost. Or is that Don't Do That Then territory?

Never mind, $wgImplicitGroups doesn't do anything like that. I was probably confusing implicit groups with effective groups but they are not necessarily the same.

Indeed. It's impossible to change anything related to this group (assigned rights, assign or remove the group...).

Change 481601 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/core@master] Add a new user group exclusive to system users

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

"Currently there's no reliable way to identify system users, both backend and frontend" - in the context of T237356: Don't allow thanking system users, https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/548919/ introduces a separate User::isSystemUser that checks based on the current logic:

User:isSystemUser
/**
 * @return bool Whether this user is a system user
 * @since 1.35
 */
public function isSystemUser() {
	// A user is considered to exist as a non-system user if it can
	// authenticate, or has an email set, or has a non-invalid token.
	if ( $this->mEmail || $this->mToken !== self::INVALID_TOKEN ||
		AuthManager::singleton()->userCanAuthenticate( $this->mName )
	) {
		return false;
	}
	return true;
}

this is how User::newSystemUser currently checks if a real user already exists. Would this work?

@Daimona is this still needed, given User::isSystemUser introduced in T237356?

@Daimona is this still needed, given User::isSystemUser introduced in T237356?

Hm, I think User::isSystemUser is enough! As long it consistently identifies all and only the system users, it should be good.

@Daimona is this still needed, given User::isSystemUser introduced in T237356?

Hm, I think User::isSystemUser is enough! As long it consistently identifies all and only the system users, it should be good.

Okay. Once T239526: Note that a user is a system user on Special:Userrights is deployed I'll take a look at some system users to make sure that the note shows up. If it does, I think we can call it good

Change 481601 abandoned by Daimona Eaytoy:

[mediawiki/core@master] Add a new user group exclusive to system users

Reason:

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