Page MenuHomePhabricator

NetworkAuth does carry out UserLoginComplete Api action correctly
Open, Needs TriagePublic

Description

  // set cookie and save settings only when this is not a
  // networkauth user
  if (! in_array($username, $this->networkauthusers)) {
    $user->saveSettings();
    $user->setCookies();
  }
  Hooks::run('UserLoginComplete', array(&$user, ""));
}

Passing a blank string is unrighteous because a reference should be passed in case the Hook returns or modifies the value. https://www.mediawiki.org/wiki/Manual:Hooks/UserLoginComplete

should instead be:

  // set cookie and save settings only when this is not a
  // networkauth user
  if (! in_array($username, $this->networkauthusers)) {
    $user->saveSettings();
    $user->setCookies();
  }
  $inject_html = '';
  Hooks::run('UserLoginComplete', array(&$user, &$injected_html));
}

I have managed to produce some errors with this (setup to tricky too explain at the moment and with <sigh> other problems but I think this is reasonably evident - a screenshot

attached).

Event Timeline

Dan.mulholland raised the priority of this task from to Needs Triage.
Dan.mulholland updated the task description. (Show Details)
Dan.mulholland added a subscriber: Dan.mulholland.

Hi @Dan.mulholland, thanks for reporting this and for taking a look at the code!

Where exactly does the code snippet come from that you pasted? Which version of MediaWiki is this about?

This is from the NetworkAuth extension I deployed by git for REL1_26 for MW1.26, the code is here.

Thanks. I don't think that the maintainer has an account in Phabricator so it's unlikely that they will see or find this bug report.

This comment was removed by Paladox.

Change 280476 had a related patch set uploaded (by Paladox):
Fix support for UserLoginComplete Api

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

Change 280476 abandoned by Paladox:
Fix support for UserLoginComplete Api

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

This bug would be better solved by implementing T111482.