276543 - createaccount.cgi should not display page if account creation is disabled

Status: RESOLVED FIXED

(Bugzilla :: User Accounts, defect)

Description

[Kevin O.]

The createemailregexp parameter is checked before an account is created. If the
parameter is blank, only an administrator may create accounts. This policy is good.

In addition, there does not appear to be a link on any page to self-register if
the paramteter is blank. Also good.

However, if a knowning individual goes to the createacount.cgi script directly,
(via, e.g., http://{somebugzilla}/createacount.cgi) the page is displayed. While
Bugzilla does refuse to create an account after the data is posted, it displays
the entire page, including the e-mail address of the maintainer.

I would recommend that if account creation is disabled for the general public
via the empty createemailregexp parameter, the error page be displayed
immediately. Thus, someone directly accessing the page would not be able to
garner the address of the maintainer from this approach.


Reproduce: Always

Steps:
1. Set te createemailregexp to be blank
2. Attempt to access the page createaccount.cgi directly by entering into the
address bar
3. Page is displayed
4. If one attempts to create an account, the error page is displayed.

Comment 1

[Olav Vitters]

Attachment: Patch v1 against Bugzilla HEAD 28 March 2005

Moves the empty Param('createemailregexp') check to be earlier.

Comment 2

[Frédéric Buclin]

Comment on attachment 178808 [details] [diff] [review]
Patch v1 against Bugzilla HEAD 28 March 2005

>+unless (Param('createemailregexp')) {
>+    # Just in case someone already has an account, let them get the correct
>+    # footer on the error message
>+    Bugzilla->login();
>+    ThrowUserError("account_creation_disabled");
>+}

I would not repeat this comment again. I would put Bugzilla->login() out of
"unless () {...}" so that we have the correct footer in all cases. Moreover, as
$createexp is defined later in the code, move it here so that you can use "if
(!$createexp) {...}".

>     my $createexp = Param('createemailregexp');
>-    if (!($createexp)
>-        || ($login !~ /$createexp/)) {
>+    if ($login !~ /$createexp/) {
>         ThrowUserError("account_creation_disabled");

Move "$createexp = Param('createemailregexp');" earlier in the code as said
above.


This is not part of this bug, but I would also move "Bugzilla->logout();" just
before the "# Create account" comment. This way, we again have the correct
footer if the new account creation fails, but the user is logged out before the
new account is created.

Comment 3

[Frédéric Buclin]

I don't like having the form displayed if new accounts cannot be created.

Comment 4

[Olav Vitters]

Attachment: Patch v2 addresses comments by LpSolit (HEAD 28 March)

Changes:
 - Moves Bugzilla->login() out of unless statements
 - Moves 'my $createexp = Param('createemailregexp');' to first usage of param
 - Moves Bugzilla->logout(); to just before account creation
 - Fixes indentation of first unless statement (1 line)

Comment 5

[Frédéric Buclin]

Comment on attachment 178933 [details] [diff] [review]
Patch v2 addresses comments by LpSolit (HEAD 28 March)

r=LpSolit

Comment 6

[Olav Vitters]

Attachment: Patch for 2.18 branch

Comment 7

[Frédéric Buclin]

Comment on attachment 178994 [details] [diff] [review]
Patch for 2.18 branch

r=LpSolit

Comment 8

[Frédéric Buclin]

Tip:

Checking in createaccount.cgi;
/cvsroot/mozilla/webtools/bugzilla/createaccount.cgi,v  <--  createaccount.cgi
new revision: 1.38; previous revision: 1.37
done


2.18:

Checking in createaccount.cgi;
/cvsroot/mozilla/webtools/bugzilla/createaccount.cgi,v  <--  createaccount.cgi
new revision: 1.33.2.1; previous revision: 1.33
done

Comment 9

[csjoshi]

This is not part of this bug, but I would also move "Bugzilla->logout();" just before the "# Create account" comment http://bestheadlightbulbs.net/hid-headlights/ This way, we again have the correct footer if the new account creation fails, but the user is logged out before the new account is created.