Page MenuHomePhabricator

Use a more complex password for WikiAdmin in selenium tests
Closed, ResolvedPublic

Description

As @zeljkofilipin found for me in https://phabricator.wikimedia.org/source/mediawiki/browse/master/tests/selenium/wdio.conf.jenkins.js;c9e13ac9f8a6a47faddeab957649a654d626db8b$3 it seems we use testpass as a password for selenium tests

This means my upcoming change for T151425: Enlarge Popular Password File to 100,000 entries and enforce the new minimum in the config in https://gerrit.wikimedia.org/r/#/c/414603/ breaks the selenium tests, which is expected (as this password is in the blacklist!)

We need to use a more complex password (or, more specifically, something less common that is not blacklisted in https://github.com/wikimedia/mediawiki-libs-PasswordBlacklist/blob/master/scripts/data/10_million_password_list_top_100000.txt ), in wherever we run install.php, and then slo in wdio.conf.jenkins.js as linked above

Event Timeline

Restricted Application added a subscriber: Aklapper. ยท View Herald TranscriptFeb 28 2018, 3:45 PM

Change 415312 had a related patch set uploaded (by Reedy; owner: Reedy):
[integration/jenkins@master] Use a more complex password for wikiadmin user

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

Change 415313 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] Use a more complex password for wikiadmin user

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

Suggesting we use testwikijenkinspass unless we have any other objections/suggestions (I'm as happy to use something like X0od636Cga from a password generator)

Core patch will need to be merged not long after the CI patch, as selenium tests will fail for the period inbetween (and probably some rebasing needed)

I am confused. Your patch https://gerrit.wikimedia.org/r/#/c/414603/ runs Selenium tests https://integration.wikimedia.org/ci/job/mediawiki-core-qunit-selenium-jessie/15707/console and they seem all happy:

18:35:29 [chrome #0-0] User
18:35:29 [chrome #0-0] โœ“ should be able to create account
18:35:29 [chrome #0-0] โœ“ should be able to log in
18:35:29 [chrome #0-0] โœ“ should be able to change preferences
18:36:01 [chrome #0-1] Page
18:36:01 [chrome #0-1] โœ“ should be creatable
18:36:01 [chrome #0-1] โœ“ should be re-creatable
18:36:01 [chrome #0-1] โœ“ should be editable
18:36:01 [chrome #0-1] โœ“ should have history
18:36:01 [chrome #0-1] โœ“ should be deletable
18:36:01 [chrome #0-1] โœ“ should be restorable

Eg even with your patch, MediaWiki managed to install with the dummy testpass password and Selenium is able to use it to create pages. Or to say otherwise the blacklist patch https://gerrit.wikimedia.org/r/#/c/414603/ does not blacklist the password?

See the tests on PS5 - https://integration.wikimedia.org/ci/job/mediawiki-core-qunit-selenium-jessie/15499/console

I commented out the password policies as can be seen in the diff between PS5 and PS11, so that they weren't being run/enforced on sysop users. So I could concentrate on fixing the other failures. See https://gerrit.wikimedia.org/r/#/c/414603/5..11/includes/DefaultSettings.php

I'm happy to uncomment them again if you want to see them fail... And then we can get the other patches merged ;)

Ah good thanks :] Lets sync all of that with Zeljko and I tomorrow!

This is on hold until @hashar has the time to do it. :)

@Reedy @zeljkofilipin Merging/deploying this change is going to be complicated, because changing the job will break master and older branches, and we can't change the code pass unless the job is changed.

We could do this by simply changing the job and breaking it and then fixing the code in master and backporting it everywhere.

But rather than 2 job changes and 5 commits, might I suggest to instead update the Jenkins job to set the MEDIAWIKI_PASSWORD environment variable? The mediawiki/core code is already set up in a way that prefers the env var when present, so this one change will make everything work.

Afterwards, we can remove the then-unused code from mediawiki/core, but it won't be a blocker, and won't need backports. The only change we need is for the Jenkins config to use a better password and expose it via the environment variable.

Change 415313 abandoned by Krinkle:
Use a more complex password for wikiadmin user in Selenium tests

Reason:
Redundant after https://gerrit.wikimedia.org/r/#/c/415312/ - will submit a clean up commit afterwards.

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

Works for me, yeah. Obviously (you mention a cleanup commit) it'll need the password changing in master, as when the T151425 gets merged (depending on defaults chosen), so it doesn't break tests everywhere :)

Obviously [..] it'll need the password changing in master

No, it won't :) The env has precedence over the hardcoded string in mediawiki@master. Once https://gerrit.wikimedia.org/r/415312 lands in integration/jenkins, the password is both changed and used correctly at the same time.

[...] you mention a cleanup commit

Yep, up for review at https://gerrit.wikimedia.org/r/426833.

Change 415312 merged by jenkins-bot:
[integration/jenkins@master] Expose wikiadmin user/pass via env and use a more complex password

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

Krinkle claimed this task.