Page MenuHomePhabricator

exception in installer when creating initial mw user
Closed, ResolvedPublic

Description

I was going through the installer. On the step that makes the user account, it errors with

[afd113f5] /w/mw-config/index.php?page=Name DBAccessError from line 364 of /w/includes/db/LBFactory.php: Mediawiki tried to access the database via wfGetDB(). This is not allowed.

Backtrace:

#0 /w/includes/GlobalFunctions.php(3614): LBFactoryFake->getMainLB(boolean)
#1 /w/includes/User.php(348): wfGetLB()
#2 /w/includes/User.php(2991): User->load()
#3 /w/includes/User.php(3006): User->getGroups()
#4 /w/includes/password/UserPasswordPolicy.php(103): User->getEffectiveGroups()
#5 /w/includes/password/UserPasswordPolicy.php(74): UserPasswordPolicy->getPoliciesForUser(User)
#6 /w/includes/User.php(859): UserPasswordPolicy->checkUserPassword(User, string)
#7 /w/includes/User.php(807): User->checkPasswordValidity(string)
#8 /w/includes/installer/WebInstallerPage.php(912): User->getPasswordValidity(string)
#9 /w/includes/installer/WebInstallerPage.php(739): WebInstallerName->submit()
#10 /w/includes/installer/WebInstaller.php(280): WebInstallerName->execute()
#11 /w/mw-config/index.php(77): WebInstaller->execute(array)
#12 //w/mw-config/index.php(36): wfInstallerMain()
#13 {main}

Event Timeline

Bawolff created this task.Jun 27 2015, 11:33 PM
Bawolff raised the priority of this task from to Needs Triage.
Bawolff updated the task description. (Show Details)
Bawolff added subscribers: Bawolff, csteipp, demon.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 27 2015, 11:33 PM
Bawolff renamed this task from exception in installer if admin password not strong enough to exception in installer when creating initial mw user.Jun 28 2015, 2:02 AM
Bawolff updated the task description. (Show Details)
Bawolff set Security to None.
demon added a comment.Jun 29 2015, 5:20 AM

Ugh, why on earth does checking password validity require a DB hit for a not-yet-existing user?

Because we want to check what group they're in. I didn't think about that
when putting that patch in. I think we'll assume the user is a sysop +
crat, and enforce that, but I haven't looked through the installer in a
while to see how easy that is.

demon added a comment.Jun 29 2015, 2:45 PM

Because we want to check what group they're in. I didn't think about that
when putting that patch in. I think we'll assume the user is a sysop +
crat, and enforce that, but I haven't looked through the installer in a
while to see how easy that is.

Yeah that makes sense for an existing user. But it makes it impossible to check a not-yet-existing user.

Change 221797 had a related patch set uploaded (by CSteipp):
Check install user's password as sysop/bureaucrat

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

Change 219440 had a related patch set uploaded (by Legoktm):
WebInstallerName: Fix user password validity check.

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

I'm ok with either patch (both feel equally hacky, might be the more accurate statement). Both methods should work ok with createAndPromote.php, which will need to be updated.

Does anyone have strong feel whether adding yet another hack to User vs making the password checking a little less clean is preferable?

Change 221797 merged by jenkins-bot:
Check install user's password as sysop/bureaucrat

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

TTO closed this task as Resolved.Oct 29 2015, 12:40 PM
TTO claimed this task.
TTO added a subscriber: TTO.

This particular issue looks like it was fixed in rMW6a69a4eb733b.

TTO reassigned this task from TTO to csteipp.Oct 29 2015, 12:40 PM
kaldari removed a subscriber: kaldari.Oct 29 2015, 9:03 PM

Change 219440 abandoned by TTO:
WebInstallerName: Fix user password validity check.

Reason:
This commit is old; it seems to make a bunch of random changes without much explanation why they are being done; the owner has gone; and the linked task T104092 appears to have been resolved in the meantime.

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

Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptJan 9 2017, 2:43 PM
Sulhan removed a subscriber: Sulhan.Jan 9 2017, 6:35 PM