Mailing List Archive: Wikipedia: Wikitech

PHPUnit tests now fixed

Index | Next | Previous | View Threaded

dnessett at yahoo

Aug 6, 2009, 9:44 AM

Post #1 of 11 (935 views)
Permalink

PHPUnit tests now fixed

I have fixed 5 bugs in /tests/ and added one feature to run-tests.php (a --runall option so testers can run the PHPUnit tests without using make - although make test still works). With these changes all of the tests in /tests/ now work. A unified diff patch is attached to bug ticket 20077.

I had to make an architectural decision when enhancing run-test.php with the --runall option. The bug ticked describes this decision and suggests two other ways to achieve the same objective. I chose the approach implemented by the patch because it required no changes to the directory structure of /tests/. However, I actually prefer the second possibility. So, if senior developers could look at the bug ticket description and give me some feedback (especially if they also think the second option is better), that would be great.

I also would appreciate some feedback on the following question. One of the tests referenced the global variables $wgDBadminname and $wgDBadminuser. When I ran the configuration script during Mediawiki installation on my machine, the LocalSettings.php file created defined the globals $wgDBname and $wgDBuser. So, I changed the test to use these variables rather than the 'admin' versions. However, I don't remember if the script gave me a choice to use the 'admin' versions or not. Also, if the configuration script has changed, then some installations may use the 'admin' versions and some may not. In either case, I would have to modify the bug fix to accept both types of global variable. If someone would fill me in, I can make any required changes to the bug fix.

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

happy-melon at live

Aug 6, 2009, 9:56 AM

Post #2 of 11 (906 views)
Permalink

Re: PHPUnit tests now fixed [In reply to]

"dan nessett" <dnessett [at] yahoo> wrote in message
news:630381.19130.qm [at] web32503
> I also would appreciate some feedback on the following question. One of
> the tests referenced the global variables $wgDBadminname and
> $wgDBadminuser. When I ran the configuration script during Mediawiki
> installation on my machine, the LocalSettings.php file created defined the
> globals $wgDBname and $wgDBuser. So, I changed the test to use these
> variables rather than the 'admin' versions. However, I don't remember if
> the script gave me a choice to use the 'admin' versions or not. Also, if
> the configuration script has changed, then some installations may use the
> 'admin' versions and some may not. In either case, I would have to modify
> the bug fix to accept both types of global variable. If someone would fill
> me in, I can make any required changes to the bug fix.

I think these are to allow you to define a separate MySQL user (in
adminsettings.php) that can be given higher privileges, as a security
device. IIRC this is now deprecated (cf bug18768).

--HM

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

innocentkiller at gmail

Aug 6, 2009, 10:05 AM

Post #3 of 11 (907 views)
Permalink

Re: PHPUnit tests now fixed [In reply to]

On Thu, Aug 6, 2009 at 12:56 PM, Happy-melon<happy-melon [at] live> wrote:
> "dan nessett" <dnessett [at] yahoo> wrote in message
> news:630381.19130.qm [at] web32503
>> I also would appreciate some feedback on the following question. One of
>> the tests referenced the global variables $wgDBadminname and
>> $wgDBadminuser. When I ran the configuration script during Mediawiki
>> installation on my machine, the LocalSettings.php file created defined the
>> globals $wgDBname and $wgDBuser. So, I changed the test to use these
>> variables rather than the 'admin' versions. However, I don't remember if
>> the script gave me a choice to use the 'admin' versions or not. Also, if
>> the configuration script has changed, then some installations may use the
>> 'admin' versions and some may not. In either case, I would have to modify
>> the bug fix to accept both types of global variable. If someone would fill
>> me in, I can make any required changes to the bug fix.
>
> I think these are to allow you to define a separate MySQL user (in
> adminsettings.php) that can be given higher privileges, as a security
> device. IIRC this is now deprecated (cf bug18768).
>
> --HM
>
>
>
> _______________________________________________
> Wikitech-l mailing list
> Wikitech-l [at] lists
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>

Yes, it's been deprecated. AdminSettings will still load in maintenance
environments (commandLine.inc and Maintenance.php) if it still exists,
but it is no longer _required_ for anything. These variables can safely
be set in LocalSettings.

HM is right on what these users are for. Some (not all) maintenance
scripts require higher permissions than your normal $wgDBuser, so
$wgDBadminuser is supposed to have those privileges.

In practice, I've found that the vast majority of maintenance scripts
don't actually need this much permission. I'm trying to clean that
up over time, so we're not using a user with higher permissions when
it's not needed. (see the Maintenance constants and the getDbType()
function).

-Chad

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Simetrical+wikilist at gmail

Aug 6, 2009, 10:20 AM

Post #4 of 11 (904 views)
Permalink

Re: PHPUnit tests now fixed [In reply to]

On Thu, Aug 6, 2009 at 1:05 PM, Chad<innocentkiller [at] gmail> wrote:
> HM is right on what these users are for. Some (not all) maintenance
> scripts require higher permissions than your normal $wgDBuser, so
> $wgDBadminuser is supposed to have those privileges.

$wgDBuser needs to have DELETE rights on pretty much all tables, so
what's the security gain of bothering with a different user for ALTER
TABLE/CREATE TABLE/etc.? $wgDBadminuser doesn't need to be able to
create new databases or reconfigure replication or anything, right?

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

innocentkiller at gmail

Aug 6, 2009, 10:30 AM

Post #5 of 11 (901 views)
Permalink

Re: PHPUnit tests now fixed [In reply to]

On Thu, Aug 6, 2009 at 1:20 PM, Aryeh
Gregor<Simetrical+wikilist [at] gmail> wrote:
> On Thu, Aug 6, 2009 at 1:05 PM, Chad<innocentkiller [at] gmail> wrote:
>> HM is right on what these users are for. Some (not all) maintenance
>> scripts require higher permissions than your normal $wgDBuser, so
>> $wgDBadminuser is supposed to have those privileges.
>
> $wgDBuser needs to have DELETE rights on pretty much all tables, so
> what's the security gain of bothering with a different user for ALTER
> TABLE/CREATE TABLE/etc.? $wgDBadminuser doesn't need to be able to
> create new databases or reconfigure replication or anything, right?
>
> _______________________________________________
> Wikitech-l mailing list
> Wikitech-l [at] lists
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>

Depends on which maintenance script you're talking about. Update.php
certainly does, as does renameDbPrefix (just to grab one off the top of
my head). The vast majority of scripts can function just fine with normal
DB access. Some (mcc and digit2html, to name a few) don't need any
DB access at all.

-Chad

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

brion at wikimedia

Aug 6, 2009, 12:04 PM

Post #6 of 11 (901 views)
Permalink

Re: PHPUnit tests now fixed [In reply to]

On 8/6/09 10:30 AM, Chad wrote:
> Depends on which maintenance script you're talking about. Update.php
> certainly does, as does renameDbPrefix (just to grab one off the top of
> my head). The vast majority of scripts can function just fine with normal
> DB access. Some (mcc and digit2html, to name a few) don't need any
> DB access at all.

Generally, only updaters need create/alter/etc privs; a few script that
do mass rebuilds of indexes will also want to do things like dropping
and re-adding indexes -- for example rebuildTextIndex drops the fulltext
index on the searchindex table, then re-adds it after refilling the
table's contents.

And a few "extreme" maintenance ops like MySQL replication master
switches of course will need all kinds of fun privs. ;)

-- brion

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

innocentkiller at gmail

Aug 6, 2009, 12:10 PM

Post #7 of 11 (902 views)
Permalink

Re: PHPUnit tests now fixed [In reply to]

On Thu, Aug 6, 2009 at 3:04 PM, Brion Vibber<brion [at] wikimedia> wrote:
> On 8/6/09 10:30 AM, Chad wrote:
>> Depends on which maintenance script you're talking about. Update.php
>> certainly does, as does renameDbPrefix (just to grab one off the top of
>> my head). The vast majority of scripts can function just fine with normal
>> DB access. Some (mcc and digit2html, to name a few) don't need any
>> DB access at all.
>
> Generally, only updaters need create/alter/etc privs; a few script that
> do mass rebuilds of indexes will also want to do things like dropping
> and re-adding indexes -- for example rebuildTextIndex drops the fulltext
> index on the searchindex table, then re-adds it after refilling the
> table's contents.
>
> And a few "extreme" maintenance ops like MySQL replication master
> switches of course will need all kinds of fun privs. ;)
>
> -- brion
>
> _______________________________________________
> Wikitech-l mailing list
> Wikitech-l [at] lists
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>

Right, which is what my idea behind getDbType() was (which still needs
actual implementation, it's more an idea than practice at the moment).
If we don't need root DB access, we shouldn't be using it! If we don't need
DB access at all, don't bother connecting.

-Chad

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

brion at wikimedia

Aug 6, 2009, 12:14 PM

Post #8 of 11 (902 views)
Permalink

Re: PHPUnit tests now fixed [In reply to]

On 8/6/09 12:10 PM, Chad wrote:
> Right, which is what my idea behind getDbType() was (which still needs
> actual implementation, it's more an idea than practice at the moment).
> If we don't need root DB access, we shouldn't be using it! If we don't need
> DB access at all, don't bother connecting.

nom nom nom...

Might be good to be able to req increased privileges on command line if
we can't do it with default user as well.

-- brion

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Simetrical+wikilist at gmail

Aug 6, 2009, 1:31 PM

Post #9 of 11 (901 views)
Permalink

Re: PHPUnit tests now fixed [In reply to]

On Thu, Aug 6, 2009 at 1:30 PM, Chad<innocentkiller [at] gmail> wrote:
> Depends on which maintenance script you're talking about. Update.php
> certainly does, as does renameDbPrefix (just to grab one off the top of
> my head).

update.php shouldn't need access other than to the wiki database,
should it? Giving the normal wiki MySQL user rights to ALTER TABLE,
etc. isn't a security risk. If we have a script to rename the DB
prefix or fiddle with replication or whatever, then yeah, that will
need root access (or at least significantly more access than the wiki
should have on a multi-user/multi-app setup). But the overwhelming
majority of admins won't need to use that. In that case, I think
AdminSettings.php is certainly a good idea, so it could be readable
only to root and not the web server. Maybe the logic for these few
maintenance scripts should go like

1) Check in LocalSettings.php for admin login. (This is a bad idea if
you have databases the web server isn't supposed to access! But
probably fine for typical sites with only databases accessible to the
web server anyway.)

2) Check in AdminSettings.php if that exists.

3) Try /root/.my.cnf or ~/.my.cnf, just in case that works, but don't
fail fatally if a login is given there but doesn't have the needed
privileges.

4) Prompt the user for a login.

I don't think this needs too much effort invested in it, though, since
only very few admins should need to run scripts that need more than
normal DB access.

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Platonides at gmail

Aug 7, 2009, 2:18 AM

Post #10 of 11 (889 views)
Permalink

Re: PHPUnit tests now fixed [In reply to]

Aryeh Gregor wrote:
> In that case, I think
> AdminSettings.php is certainly a good idea, so it could be readable
> only to root and not the web server.

only by /root/?

If an attacker has read access to your AdminSettings.php he might as
well have write permissions.
He just needs to change your php files and wait until you run a
maintenance script to get mailed your /etc/shadow, or rm -rf / you.

Maintenance scripts shouldn't be run as root. OTOH that would be a good
method if you used a specific account eg. 'WikiAdmin'. Still, you might
get funny permissions when running scripts that deal with files.

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Simetrical+wikilist at gmail

Aug 7, 2009, 3:37 AM

Post #11 of 11 (888 views)
Permalink

Re: PHPUnit tests now fixed [In reply to]

On Fri, Aug 7, 2009 at 5:18 AM, Platonides<Platonides [at] gmail> wrote:
> only by /root/?

Well, or otherwise not readable by the web server, like 640
root:admins. You're right that there's no reason to run a PHP script
as root if you only need root DB access, of course.

> Maintenance scripts shouldn't be run as root. OTOH that would be a good
> method if you used a specific account eg. 'WikiAdmin'. Still, you might
> get funny permissions when running scripts that deal with files.

Well, when I run maintenance scripts I usually don't bother sudoing to
www-data anyway, personally. Practically none of the scripts touch
files.

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads