Checksetup.pl should create maintainer if database is empty.

VERIFIED FIXED in Bugzilla 2.12

Status

()

Bugzilla
Bugzilla-General
P4
enhancement
VERIFIED FIXED
18 years ago
5 years ago

People

(Reporter: Kurt Werle, Assigned: justdave)

Tracking

unspecified
Bugzilla 2.12

Details

(Whiteboard: 2.12)

Attachments

(6 attachments)

(Reporter)

Description

18 years ago
As the subject says...  When you first install Bugzilla, the first user you
create should be made a 'super user' by default.  Right now it is made a generic
unprivileged user - this is not useful...

Updated

18 years ago
Status: NEW → ASSIGNED
Priority: P3 → P2

Comment 1

18 years ago
I need to check into this.  I think the checksetup.pl script does stuff which
helps out here...

Comment 2

18 years ago
tara@tequilarista.org is the new owner of Bugzilla and Bonsai.  (For details,
see my posting in netscape.public.mozilla.webtools,
news://news.mozilla.org/38F5D90D.F40E8C1A%40geocast.com .)
Assignee: terry → tara
Status: ASSIGNED → NEW

Comment 3

18 years ago
If one and only one user exists in the database, checksetup.pl does exactly this
when run.  It's in the docs -- this bug should be resolved.

Comment 4

18 years ago
you were messing with this stuff, can you doublecheck this and close it out?
Assignee: tara → cyeh
(Reporter)

Comment 5

18 years ago
Last comment said would 'you' check this out.  I'm afraid pronouns won't do in such a pulic forum...

In general I'm glad that checksetup.pl works around this problem.  Specifically I still think it'd be nice if it were integrated.  Ah well.

Comment 6

18 years ago
the "you" tara was referring to me when she reassigned the bug. i just took a 
look at checksetup.pl, and it does take care of this. however, you do bring up a 
good point that the initial permissions information could be a little clearer.

we're accepting patches to the documentation. take a look at README and find a 
good place to stick in the info you think is missing.

lowering the priority
Status: NEW → ASSIGNED
Priority: P2 → P4

Comment 7

18 years ago
no comments from original reporter, closing as fixed
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
(Reporter)

Comment 8

18 years ago
Sigh.
The first user created should be made superuser.
Nice there is a workaround, but this is an enhancement request.
This is not a doc bug - even if the doc was real clear about how to work around 
the problem (using checksetup.pl), this still is not the desired functionality.
Considering that the docs say you're required to run checksetup.pl every time you 
run an update, I think the fix to this is, rather than having to log in from the 
web and create a user, then run checksetup.pl again to grant permissions to it, 
that checksetup.pl should prompt the person running it for account information 
for the superuser account and create it on the spot if it has an empty user 
database with no users in it when it runs.  This not only prevents you from 
having to run checksetup.pl twice when you first set it up, but also closes up a 
potential (albeit microscopicly small) security hole, in case someone were to 
beat you to it at logging in as the first user, and then checksetup.pl would fail 
to grant permissions because there was more than one user, or would grant them to 
the wrong user.
oops, meant to add myself to the CC there, too.  Sorry for the spam.

Comment 11

18 years ago
okay, dave@intrec.com has a good suggestion here. we should be prompting for a 
first user at database creation time, that makes it explicit (no more having to 
have read the docs) and user friendly (admin can setup any login/password that 
they like.)

reopening
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 12

18 years ago
changing status to assigned, setting priority to p3
Status: REOPENED → ASSIGNED

Comment 13

18 years ago
Created attachment 9029 [details] [diff] [review]
Patch to checksetup.pl to prompt for and create admin user

Comment 14

18 years ago
I'll have an opportunity to test the patch here sometime next week when I spin 
up a new Bugzilla test installation. I may need to rework it slightly, but the 
concept is sound.

Comment 15

18 years ago
So I've looked at the patch briefly, and it needs some work. You included CGI.pl 
so that you could call InsertNewUser() and MailPassword().  However, including 
CGI.pl and calling those two functions inside of checksetup.pl produces the 
following side-effects:

checksetup.pl has function name collisions with GroupExists() and trim(), where 
the functions used are the ones in globals.pl

MailPassword is intended to be called as a part of the CGI, and spits out html 
tags to console.

checksetup should be self contained if possible, and certainly should have 
function name collisions with stuff previously defined.
Assignee: cyeh → dave
Status: ASSIGNED → NEW
Summary: First user should be made superuser → Checksetup.pl should create maintainer if database is empty.
Whiteboard: 2.12
Changing summary to fit the current direction this bug is going.  Nominating for 
2.12.  Based on discussion in bug 31456, I think this also needs to prompt for a 
maintainer if there isn't anyone set up as one in the database, even if there are 
other users, just in case the original one got his/her privs nuked on accident 
(we've seen it happen).  
Created attachment 23092 [details] [diff] [review]
Patch v2 - self-contained now.
OK, redid this patch...  now it prompts the user for a password, and everything 
is self-contained in checksetup.pl.   As a bonus, it also prompts for an 
administrator address if there isn't anyone in the database with admin privs, no 
matter how many users there are.  And if you specify an email address of someone 
who already exists, it will prompt if you want to grant admin privs to that user 
instead of just blindly creating a new account.

Needs an r=.  Adding 49335 as a dependency because it will automatically be fixed 
when this one is checked in.
Blocks: 49335
Status: NEW → ASSIGNED
Keywords: patch, review
Copying Matt B...  this will require changes to the documentation if this gets 
checked in.
Dangit... make that bug 49935, not 49335.  :)
Blocks: 49935
No longer blocks: 49335
Created attachment 23431 [details] [diff] [review]
patch v3 - fixed comments so they're accurate
*** Bug 49935 has been marked as a duplicate of this bug. ***
No longer blocks: 49935
Created attachment 23470 [details] [diff] [review]
patch v3 again, without the freaking MacBinary header on it this time.
That'll teach me to use Internet Explorer to upload a patch file...

Comment 25

17 years ago
Just applied patch v3 (I was one of those who lost "God" access).  When I ran
checksetup.pl it prompted me for an admin e-mail.  I meant to enter mine but had
a typo in doing so.  It then forced me to create a new user account with that
(non-existant) e-mail address.  Perhaps it should prompt before creating an account?
Created attachment 24194 [details] [diff] [review]
Patch v4
Patch v4 takes Jake's suggestion to prompt for confirmation of the entered email 
address before creating the account.  Also fixes a bug in the Y/N tests that 
would have wound up in an infinite loop if the user already existed and you said 
No to not grant them privs.

Comment 28

17 years ago
One minor comment about your patch: when turning off input echoing, I'd
catch some signals to turn it on again in case of an interrupt. Think about
what happens when the user hits Ctrl-C while asked for the password ... the
terminal is left in a very bad state. So perhaps do something like this:

sub bailout {
    system("stty echo"); # re-enable input echoing
    print "\nInterrupt, bailing out.\n"; # or whatever or no output at all
    exit 1;
}
$SIG{HUP } = \&bailout;
$SIG{INT } = \&bailout;
$SIG{QUIT} = \&bailout;
$SIG{TERM} = \&bailout;

system("stty -echo");  # disable input echoing
# ... ask for password ...
system("stty echo"); # re-enable input echoing

$SIG{HUP } = 'DEFAULT'; # or use a global variable in sub bailout()
$SIG{INT } = 'DEFAULT'; # to disable the stty on interrupt at
$SIG{QUIT} = 'DEFAULT'; # this point
$SIG{TERM} = 'DEFAULT';
Hardware: PC → All
hmm, that's probably a good idea.  I hadn't thought about it at the time because 
every shell I've seen resets the terminal before displaying the prompt.  Then 
again, I know I haven't seen every shell out there, and there's probably some 
that don't.

Comment 30

17 years ago
Just tried Patch v4 and it works as advertised...  I especially like "That's
okay, typos happen.  Give it another shot."

FWIW, r=jake
QA Contact: matty

Comment 31

17 years ago
The patch looks good and works as advertised. I'm ready to check in v4 without 
the shell state restorer code suggested by Stephan. If this is forthcoming I'll 
wait, otherwise I'll check it in RSN. 

Created attachment 25414 [details] [diff] [review]
Patch v5 - adds interrupt hooks to fix echo if aborted

Comment 33

17 years ago
patch submitted to trunk. Thanks to Dave Miller for the patch and Stephan for 
additional input.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago17 years ago
Resolution: --- → FIXED

Comment 34

17 years ago
Sorry for the spam, but I needed to be able to query for all of these correctly.
Target Milestone: --- → Bugzilla 2.12

Updated

17 years ago
Status: RESOLVED → VERIFIED

Comment 35

17 years ago
VERIFIED... I've used it a couple time now both on new and existing installations.
Moving closed bugs to Bugzilla product
Component: Bugzilla → Bugzilla-General
Product: Webtools → Bugzilla
Version: other → unspecified
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.