292059 - No locking in createaccount.cgi

Status: RESOLVED FIXED

(Bugzilla :: User Accounts, defect)

Description

[Gervase Markham [:gerv]]

We have two bits of account creation code - in Bugzilla/Users.pm and
editusers.cgi. editusers.cgi locks the profiles table while it checks for
non-existence of the user and then creates it if necessary. The code called from
createaccount.cgi (in Bugzilla/Users.pm) does not. There is therefore
theoretically a race condition, which might be hit if, for example, the user
double-clicks the button and the web browser sends both requests.

If we are just to add locking, it needs to be in createaccount.cgi rather than
Users.pm because of the way the code is structured. However, we may also want to
take this opportunity to change things to only have a single place where users
are created.

Comment 1

[Frédéric Buclin]

Attachment: patch, v1

The code in createaccount.cgi and editusers.cgi is actually too different to be
merged. For example, the user has to be logged out when he creates a new
account. He also needs a "forgot password" message if the account already
exists, etc...

Comment 2

[Teemu Mannermaa (:wicked)]

Comment on attachment 189241 [details] [diff] [review]
patch, v1

This patch didn't apply cleanly to either 2.20 branch (hunk 1 and 4) or trunk
(hunk 1). I applied failed hunk 1 manually on tip and tested account creation.
Even though this suffles things aroung, it would still seem to work correctly.

>Index: createaccount.cgi
> # Shut up misguided -w warnings about "used only once":
>-use vars qw(
>-  $template
>-  $vars
>-);
>+use vars qw($template $vars);

Nit: Imho, nice cleanup but unneeded.

> # Just in case someone already has an account, let them get the correct footer
>-# on an error message.  The user is logged out just before the account is
>+# on an error message.  The user will be logged out when the account is
> # actually created.

Nit: Maybe remove the double blank too? Also, perhaps add word "after" so it's
clear logout happens after account creation now..

Comment 3

[Frédéric Buclin]

Attachment: patch for the tip, v2

Comment 4

[Teemu Mannermaa (:wicked)]

Comment on attachment 190705 [details] [diff] [review]
patch for the tip, v2

Tested on trunk, seems to work. Does not apply cleanly on 2.20 branch.

Comment 5

[Frédéric Buclin]

Attachment: 2.20 backport, v2

same patch as for the tip.

Comment 6

[Teemu Mannermaa (:wicked)]

Comment on attachment 190710 [details] [diff] [review]
2.20 backport, v2

Tested to be working on 2.20 branch.

Comment 7

[Frédéric Buclin]

tip:

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


2.20rc1:

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