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

RESOLVED FIXED in Bugzilla 2.18

Status

()

Bugzilla
User Accounts
RESOLVED FIXED
13 years ago
6 months ago

People

(Reporter: Kevin O., Assigned: Olav Vitters)

Tracking

2.18
Bugzilla 2.18
Bug Flags:
approval +
blocking2.20 +
approval2.18 +
blocking2.18.1 +

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

13 years ago
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.
(Assignee)

Comment 1

13 years ago
Created attachment 178808 [details] [diff] [review]
Patch v1 against Bugzilla HEAD 28 March 2005

Moves the empty Param('createemailregexp') check to be earlier.
Assignee: myk → bugzilla-mozilla
Status: UNCONFIRMED → ASSIGNED
Attachment #178808 - Flags: review?(kiko)

Comment 2

13 years ago
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.
Attachment #178808 - Flags: review?(kiko) → review-

Comment 3

13 years ago
I don't like having the form displayed if new accounts cannot be created.
Severity: minor → normal
Flags: blocking2.20?
Flags: blocking2.18.1?
OS: Windows XP → All
Hardware: PC → All
Target Milestone: --- → Bugzilla 2.18

Updated

13 years ago
Summary: createaccount.cgi should not display pageif account creation is disabled → createaccount.cgi should not display page if account creation is disabled
(Assignee)

Comment 4

13 years ago
Created attachment 178933 [details] [diff] [review]
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)
Attachment #178808 - Attachment is obsolete: true
Attachment #178933 - Flags: review?(LpSolit)

Comment 5

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

r=LpSolit
Attachment #178933 - Flags: review?(LpSolit) → review+

Updated

13 years ago
Flags: approval?
Flags: approval2.18?
Flags: blocking2.20?
Flags: blocking2.20+
Flags: blocking2.18.1?
Flags: blocking2.18.1+
Flags: approval?
Flags: approval2.18?
Flags: approval2.18+
Flags: approval+
(Assignee)

Comment 6

13 years ago
Created attachment 178994 [details] [diff] [review]
Patch for 2.18 branch

Comment 7

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

r=LpSolit
Attachment #178994 - Flags: review+

Comment 8

13 years ago
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
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
QA Contact: matty_is_a_geek → default-qa

Comment 9

6 months ago
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.
You need to log in before you can comment on or make changes to this bug.