Page MenuHomePhabricator

$wgLDAPUseLDAPGroups produces MediaWiki groups with invalid whitespace in names
Open, Needs TriagePublic

Description

LdapAuthentication creates MediaWiki groups to sync with LDAP groups, using the $wgLDAPUseLDAPGroups setting, When the LDAP group name contains whitespace, LdapAuthentication will happily create a MediaWiki group with the identical name, even though whitespace is invalid in MediaWiki group names.

As a result, Special:UserRights cannot handle these groups. If you try to add a group, the add silently fails. If you are a member of one of these groups, and you perform ANY submission on Special:UserRights, you get unexpectedly removed from the group.

LdapAuthentication should modify the group names to get rid of any whitespace (for example, replacing spaces with underscores) when creating the MediaWiki groups.

Details

Related Gerrit Patches:

Event Timeline

maiden_taiwan raised the priority of this task from to Needs Triage.
maiden_taiwan updated the task description. (Show Details)
maiden_taiwan added a subscriber: maiden_taiwan.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 22 2015, 5:44 PM

The MediaWiki restriction on whitespace, as far as I can tell, has to do with Special:UserRights. (This is a guess.) This page creates a checkbox for each user group, with name="wgGroup-$groupname". In other words, if the group name contains whitespace, you get invalid HTML4 by putting whitespace into the "name" attribute of the tag. If MediaWiki would sanitize the name, would this MediaWiki restriction on group names go away?

maiden_taiwan added a comment.EditedJan 22 2015, 7:18 PM

Suggested fix:

  • Supply a function, perhaps named LdapAuthentication::transformLDAPGroupName($name), that converts a space to an underscore. Make it public and static so other extensions can call it too.
public static function transformLDAPGroupName($name) {
  return str_replace(' ', '_', $name);
}
  • At the end of function getGroups(), for each member of $this->userLDAPGroups["short"] and $this->allLDAPGroups["short"], replace space characters with underscores. Supply a global variable $wgLDAPGroupsFixWhitespace (boolean) to turn this behavior on and off.
if ($this->getConf(LDAPGroupsFixWhitespace) {
  if (isset($this->userLDAPGroups) && isset($this->userLDAPGroups["short"])) {
    foreach ($this->userLDAPGroups["short"] as $key => $value) {
      $this->userLDAPGroups["short"][$key] = self::transformLDAPGroupName($value);
    }
  }
  if (isset($this->allLDAPGroups) && isset($this->allLDAPGroups["short"])) {
    foreach ($this->allLDAPGroups["short"] as $key => $value) {
      $this->allLDAPGroups["short"][$key] = self::transformLDAPGroupName($value);
    }
  }
}

Uploading a patch against LdapAuthentication 2.0d (latest).

Thanks for taking a look at the code!

You are very welcome to use developer access to submit this as a Git branch directly into Gerrit.

Putting your branch in Git makes it easier to review it quickly. If you don't want to set up Git/Gerrit, you can also use the Gerrit Patch Uploader. Thanks again!

gerritbot added a subscriber: gerritbot.

Change 187917 had a related patch set uploaded (by Gerrit Patch Uploader):
Phabricator T87376: Don't create invalid MediaWiki group names containing whitespace: change spaces to underscores.

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

Patch-For-Review

Change 187917 had a related patch set uploaded (by Luke081515):
Don't create invalid MediaWiki group names containing whitespace: change spaces to underscores.

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