in practice on d.o infrastructure, there are a set of us who sometimes need to run update.php. it's kind of a pain to have to keep editing the file (especially since there's no emacs installed on those machines!) to change the $user->uid == 1 each time someone else (me, killes, etc) needs to run an update for something.

i know it's a little late in the game, but this is *such* a small diff, totally self-contained to update.php, etc, etc. it'd sure save some hassle maintaining d.o infrastructure, and i'm guessing lots of other sites are in a situation where they want to allow more than 1 user to be able to run update.php without manually editing the file each time.

if this gets bumped to 6.x-dev, i'd understand, but i'd probably install it on the live copy for d.o, anyway. ;)

cheers,
-derek

CommentFileSizeAuthor
#7 update_super_users_array.patch_1.txt1.08 KBdww
update_super_users_array.patch.txt989 bytesdww
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

killes@www.drop.org’s picture

killes@www.drop.org CreditAttribution: killes@www.drop.org commented

I like the idea. We can always apply it locally should it not get into D5.

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented

should that configuration go into settings.php? ... i'd like to see this applied to core, and not just on d.o. really, what i want is http://drupal.org/node/67234 but we are too late for that now.

dww’s picture

dww
we/he/they
CreditAttribution: dww commented

i was just trying to keep this patch as small as possible. we already have the notion that site admins can edit update.php to toggle the $access_check = TRUE; line in place, instead of having that live in settings.php. frankly, i prefer to keep such things as close to the code/functionality as possible.

OTOH, settings.php would make this better for multi-sites, i guess. but, i don't really know enough about how multi-site is used in the real world, and especially how it interacts with update.php, to say for sure.

i agree, http://drupal.org/node/67234 would be even better, but that's far too big a change this late in the game for D5. hopefully we can use this issue as the starting point, and morph the functionality into #67234 for D6 once we've got a little experience and feedback on how this is being used.

RobRoy’s picture

RobRoy CreditAttribution: RobRoy commented

Something like this would be better done in settings.php from a multi-site POV, or at least have a warning in update.php because if you allow array(1, 2) to update on siteA.com and create a siteB.com later down the road where UID 2 is just some Joe Schmo you'll run into a security issue.

dww’s picture

dww
we/he/they
CreditAttribution: dww commented

i'm not going to re-roll for settings.php. if it's going to start getting this complicated, we should just "won't fix" this issue and commit something like http://drupal.org/files/issues/update-script-access-rights_3.patch from http://drupal.org/node/67234 instead. that's only a 3 line diff, and then it's just another permission you can assign to whatever site-specific roles you want...

RobRoy’s picture

RobRoy CreditAttribution: RobRoy commented

Yeah, I wasn't implying you should re-roll 'er for settings.php. Just wanted to give you a multi-site perspective. I think if this is going into core, just a warning on top saying something like this would be enough:

"WARNING: If you are using Drupal in a multi-site setting, you must ensure that any user IDs added here are trusted users across ALL sites."

That is a quick fix for the meantime until something like http://drupal.org/node/67234 gets in.

dww’s picture

dww
we/he/they
CreditAttribution: dww commented
FileSize
update_super_users_array.patch_1.txt1.08 KB

ok, here's a new patch with another line in the comment about multi-site. that's as far as i'm taking this. ;)

forngren’s picture

forngren CreditAttribution: forngren commented
Status: Needs review » Closed (won't fix)

Huge -1 from me, this is really a hack, not a patch IMNO. Besides, this patch can potentially lock out user/1 and cause security problems for multisites. Admins that are poking in update.php should be aware of what they are doing anyway.

We should really wait for http://drupal.org/node/67234, is it possible to get it in before 5.x is released? It's a really small patch and less likely to encounter problems than this one. Sorry, but I think "Won't fix" is the best solution for everyone. Feel free to change if you disagree.