*Bulk BZ Change: +Patch to open bugs with patches attached that are missing the keyword*

sumanah wrote:

Thanks for the patch, Michael, and sorry for the wait! Signalling that this patch needs review by adding the "need-review" keyword.

mnewton wrote:

updated patch for 1.17.0

attachment SpecialUserlogin.diff ignored as obsolete

Next time, please try to use the code from subversion so we can apply it more easily r103396

Hi. I reverted this because it called the hook with the argument $u in places before $u is defined (Which broke unit tests). For example when we check if the user didn't enter a name, we don't have a user object yet.

Probably way forward would be to create a new hook to audit situations where the login was rejected out of hand for the user being 'stupid'.

Cheers.

This was re-applied in r103467 & r106446 but wasn't marked FIXED. I've reverted it (again) in r110797, so REOPENED is appropriate.

mnewton wrote:

patch against trunk

I'll submit one more, very stripped down, try at this. Previous patch was blindly calling the hook every time authenticateUserData returned which was obviously overkill.
This should allow logging of failed login attempts with invalid user names.

Attached:

sumanah wrote:

Michael, thanks for the new patch. I have changed the "reviewed" keyword to "need-review" to indicate that the new patch has not yet been reviewed and awaits critique and/or acceptance.

sumanah wrote:

Brion, could I ask you to look at this?

The patch in its current form is not OK. The hook in its current state is documented as passing a User object. If, by chance, there's an extension that decided to use type hinting on the hook call, this would cause errors in those cases. Also, it's just bad design.

However, it is a good idea to be calling this hook all the time, so I'll try and think of a proper solution when I get back from lunch.

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

This restructures User::authenticateUserData so that it always reaches the end of the function. Also, it guarantees that a User object is always passed to the hook.

https://gerrit.wikimedia.org/r/37788 (Gerrit Change I316bbacf4d9fa5558c63fb09e8d0aef98503c556) | change ABANDONED [by Parent5446]

Change 27022 abandoned by Parent5446:
Re-implemented Special:Userlogin using FormSpecialPage.

Reason:
The rest of the login system needs to be fixed before the UI part.

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