Last Comment Bug 87795 - Creating an account should send token and wait for confirmation (prevent user account abuse)
: Creating an account should send token and wait for confirmation (prevent user...
Status: VERIFIED FIXED
:
Product: Bugzilla
Classification: Server Software
Component: User Accounts (show other bugs)
: 2.13
: All All
: P1 enhancement (vote)
: Bugzilla 3.0
Assigned To: Frédéric Buclin
: default-qa
Mentors:
: 137499 243303 (view as bug list)
Depends on:
Blocks: 349336 349337
  Show dependency treegraph
 
Reported: 2001-06-26 02:12 PDT by Matthew Tuck [:CodeMachine]
Modified: 2015-05-16 09:11 PDT (History)
11 users (show)
myk: approval+
LpSolit: documentation+
LpSolit: testcase+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
editusers.cgi: patch to display 'lastused' time in action=list mode (4.20 KB, patch)
2001-11-01 07:47 PST, James A. Laska
no flags Details | Diff | Splinter Review
patch, v1 (28.53 KB, patch)
2006-08-05 12:47 PDT, Frédéric Buclin
mkanat: review+
bugzilla-mozilla: review+
Details | Diff | Splinter Review
patch, v2 (31.15 KB, patch)
2006-08-19 07:25 PDT, Frédéric Buclin
mkanat: review+
LpSolit: review+
Details | Diff | Splinter Review
documentation patch, v1 (4.42 KB, patch)
2006-11-22 12:24 PST, Frédéric Buclin
justdave: review+
Details | Diff | Splinter Review

Description Matthew Tuck [:CodeMachine] 2001-06-26 02:12:52 PDT
We should do something to prevent someone from creating accounts for other
people they shouldn't, and those accounts polluting the system.

Two options present themselves:

(1) Put a 'last login date' or 'everloggedin' option on the profiles table. 
Then after a specific period, delete the account if it has never been used (and
hence can be deleted safely one would hope).
(2) Before creating the account, store the email address, real name, password,
and a unique token in a temporary table, and send a confirmation email to the
user including a validation URL with the token embedded which will create the
account.

Either mechanism would require a clean up job to clean up old accounts/tokens. 
This could be done in a cron job or it could be tied to another task, for
example when an account is created and the cleanup hasn't occurred for a day.

I would prefer option 2.  Unvalidated accounts will take less space, clean up
will be faster, the max userid number will be less, and there's no chance a bug
might make the account be used somewhere while being marked as never logged in.
Comment 1 Andreas Franke (gone) 2001-08-28 22:08:40 PDT
-> Bugzilla product, User Accounts component, reassigning.
Comment 2 Dave Miller [:justdave] (justdave@bugzilla.org) 2001-08-28 22:16:42 PDT
restoring version field lost in product change
Comment 3 James A. Laska 2001-10-26 06:04:55 PDT
Matthew, 

Wouldn't option (1) be much easier to implement and kill two birds with one
stone?  I would like to see a last_loggin_ts, and a ever_logged_in in the
profiles table.  That same field could be used by a cron job to kick off after a
specified time (Param('account_timeout')) and email the user that if they do not
log into their account, the account will be disabled/removed.  Not sold on
deletion of the user account since bugzilla's motto should be to never lose
data/information about a bug or person.  It may be advisable to mark the account
as disabled.

In any case, I would like to see a last_loggin_ts (that's how I stumpled upon
this bug).  This field would be updated by CGI::confirm_login().  This
information would be displayed on the editusers.cgi?action=edit interface.  We
could extend this further by displaying last_modified_ts in the editusers.cgi
interface.  This could be achieved by a select on the bugs and bugs_activity
table for the most recent timestamp with that bug_id.  Hey, call me an
information freak, but we have the data...lets show it :)
Comment 4 James A. Laska 2001-11-01 07:10:24 PST
Can the last login date already be obtained using the lastused column in the
logincookies table?
Comment 5 James A. Laska 2001-11-01 07:47:13 PST
Created attachment 56077 [details] [diff] [review]
editusers.cgi: patch to display 'lastused' time in action=list mode
Comment 6 Matthew Tuck [:CodeMachine] 2001-11-29 07:24:53 PST
I believe this was fixed in the 2.14 timeframe when the password handling code
was written.  Dave, any thoughts?

I'm not sure how we trim out old login cookies, we should find out and work out
whether we want a lastloggedin field.  I agree it would be useful.
Comment 7 Matthew Tuck [:CodeMachine] 2002-04-20 03:50:56 PDT
*** Bug 137499 has been marked as a duplicate of this bug. ***
Comment 8 Bradley Baetz (:bbaetz) 2002-04-20 04:22:39 PDT
See my suggestion in bug 137499 - don't create accounts at all, just create a
holding entry in the tokens table. Only create teh account once that is
confirmed.
Comment 9 Andreas Franke (gone) 2002-04-21 05:22:33 PDT
Adding summary from dupe since it's more precise.
Comment 10 Dave Miller [:justdave] (justdave@bugzilla.org) 2002-07-03 22:49:45 PDT
This should be extremely trivial to do now with the token code working the way
it does now.  When a user signs up for a new account, they enter nothing but
their email address.  It emails them the token.  They click the link in the
token, which gives them a page where they enter their real name and choose a
password (which also eliminates the randomly generated password that's too hard
to remember which half the people never bother to change).  Maybe even choose
some of their initial preferences on this page (such as their preferred format
for the query page).
Comment 11 Joel Peshkin 2004-03-18 16:04:12 PST
Unloved bugs targetted for 2.18 but untouched since 9-15-2003 are being
retargeted to 2.20
If you plan to act on one immediately, go ahead and pull it back to 2.18.
Comment 12 Dave Miller [:justdave] (justdave@bugzilla.org) 2004-05-11 07:59:27 PDT
*** Bug 243303 has been marked as a duplicate of this bug. ***
Comment 13 Dave Miller [:justdave] (justdave@bugzilla.org) 2004-05-11 08:00:42 PDT
sneaking a few additional words into the summary to make it easier to find
Comment 14 Dave Miller [:justdave] (justdave@bugzilla.org) 2004-09-18 17:54:20 PDT
Bugzilla 2.20 feature set is now frozen as of 15 Sept 2004.  Anything flagged
enhancement that hasn't already landed is being pushed out.  If this bug is
otherwise ready to land, we'll handle it on a case-by-case basis, please set the
blocking2.20 flag to '?' if you think it qualifies.
Comment 15 Dave Miller [:justdave] (justdave@bugzilla.org) 2004-12-21 17:06:53 PST
This could potentially be a legal issue in the new age of online privacy
issues...  We need to ensure that an account can't actually be used (such as for
CCs and the like) without the consent of the person who owns the email address
(in otherwords, don't create the account until they confirm their intent to use it).
Comment 16 Frédéric Buclin 2005-11-17 07:20:39 PST
The trunk is now frozen to prepare Bugzilla 2.22. Enhancement bugs are retargetted to 2.24.
Comment 17 Frédéric Buclin 2006-02-01 09:56:57 PST
/me wants to see this bug fixed for 2.24.
Comment 18 Frédéric Buclin 2006-08-05 12:47:52 PDT
Created attachment 232354 [details] [diff] [review]
patch, v1
Comment 19 Max Kanat-Alexander 2006-08-14 12:12:21 PDT
Comment on attachment 232354 [details] [diff] [review]
patch, v1

>Index: template/en/default/account/created.html.tmpl
> [snip]
> No action from you in the
>+  next [% constants.MAX_TOKEN_AGE FILTER html %] days will automatically cancel
>+  this request.

  Nit: The way you said that, it means, "It's impossible for you to cancel this request in the next three days." :-)

>Index: template/en/default/account/email/confirm-new.html.tmpl

>+<div>
>+  To complete the creation of your user account, you must enter a password in the
>+  form below. You can also enter your real name, which is optional.<p>

  Nit: Say "choose a password in the form below" instead. Otherwise people might get confused and think they need to know some password they don't know.

>Index: template/en/default/account/email/request-new.txt.tmpl
> [snip]
>+Subject: [% terms.Bugzilla %] Create User Account Request

  That's a slightly awkward subject line. Perhaps something like "Bugzilla: Confirm Account Creation"

>+[%+ terms.Bugzilla %] has received a request to create a user account
>+using your email address ([% email %]).

>+To confirm this user account creation, visit the following link:

  Nit: "To confirm that you want to create an account using that email address,"


  Everything else looks good. :-) It would be nice to find some way to directly display the login box (or directly log them in) after they create their account, instead of just showing them a message that they can do so.
Comment 20 Max Kanat-Alexander 2006-08-14 12:17:36 PDT
> No action from you in the
>+  next [% constants.MAX_TOKEN_AGE FILTER html %] days will automatically cancel
>+  this request.

  You could instead say:

  If you take no action in the next three days, this request will automatically be canceled.
Comment 21 Olav Vitters 2006-08-18 13:39:46 PDT
Comment on attachment 232354 [details] [diff] [review]
patch, v1

Nits:
 * should focus the field when it requests an email address using javascript
 * same for the page that asks for realname, password
 * should specify the IP address and maybe the time the request was made
 * 'Request Submitted' is not clear enough for a user
 * perhaps log in *automatically* after specifying the name + password (for another bug)
 * It is possible to mailbomb someone just by refreshing the page a lot. Because this is possible currently (just CC the account to bugs that get a lot of mail), It is not a review- .. however with this interface it is far easier to do.
 
The r+ is based upon my current inspection. I haven't closely looked line-by-line.
Comment 22 Frédéric Buclin 2006-08-18 14:08:41 PDT
Already requesting approval as justdave is on vacation and myk is usually away during the week-end. ;)
Comment 23 Frédéric Buclin 2006-08-19 07:25:10 PDT
Created attachment 234586 [details] [diff] [review]
patch, v2

This patch addresses all comments made by mkanat and bkor, except the "automatically log in the user" one, which should be fixed in a separate bug. Also, as per our discussion on IRC, the IP address won't be displayed in the email, for consistency with other emails sent by Bugzilla.

Requesting review from mkanat for the mailbomb thing in Token::issue_new_user_account_token().
Comment 24 Frédéric Buclin 2006-08-19 07:26:18 PDT
Comment on attachment 234586 [details] [diff] [review]
patch, v2

carrying forward bkor's r+ per discussion on IRC.
Comment 25 Max Kanat-Alexander 2006-08-19 10:59:13 PDT
Comment on attachment 234586 [details] [diff] [review]
patch, v2

Okay, that looks fine, but when you check it in, make the "10" into a constant instead of it being a magic number inside the subroutine.
Comment 26 Frédéric Buclin 2006-08-19 11:11:58 PDT
Checking in createaccount.cgi;
/cvsroot/mozilla/webtools/bugzilla/createaccount.cgi,v  <--  createaccount.cgi
new revision: 1.52; previous revision: 1.51
done
Checking in token.cgi;
/cvsroot/mozilla/webtools/bugzilla/token.cgi,v  <--  token.cgi
new revision: 1.45; previous revision: 1.44
done
Checking in userprefs.cgi;
/cvsroot/mozilla/webtools/bugzilla/userprefs.cgi,v  <--  userprefs.cgi
new revision: 1.105; previous revision: 1.104
done
Checking in Bugzilla/BugMail.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/BugMail.pm,v  <--  BugMail.pm
new revision: 1.90; previous revision: 1.89
done
Checking in Bugzilla/Constants.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Constants.pm,v  <--  Constants.pm
new revision: 1.49; previous revision: 1.48
done
Checking in Bugzilla/Token.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Token.pm,v  <--  Token.pm
new revision: 1.47; previous revision: 1.46
done
Checking in Bugzilla/User.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/User.pm,v  <--  User.pm
new revision: 1.127; previous revision: 1.126
done
Checking in Bugzilla/DB/Schema.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB/Schema.pm,v  <--  Schema.pm
new revision: 1.62; previous revision: 1.61
done
Checking in Bugzilla/Install/DB.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install/DB.pm,v  <--  DB.pm
new revision: 1.8; previous revision: 1.7
done
Checking in template/en/default/account/cancel-token.txt.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/account/cancel-token.txt.tmpl,v  <--  cancel-token.txt.tmpl
new revision: 1.10; previous revision: 1.9
done
Checking in template/en/default/account/create.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/account/create.html.tmpl,v  <--  create.html.tmpl
new revision: 1.10; previous revision: 1.9
done
Checking in template/en/default/account/created.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/account/created.html.tmpl,v  <--  created.html.tmpl
new revision: 1.7; previous revision: 1.6
done
RCS file: /cvsroot/mozilla/webtools/bugzilla/template/en/default/account/email/confirm-new.html.tmpl,v
done
Checking in template/en/default/account/email/confirm-new.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/account/email/confirm-new.html.tmpl,v  <--  confirm-new.html.tmpl
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/webtools/bugzilla/template/en/default/account/email/request-new.txt.tmpl,v
done
Checking in template/en/default/account/email/request-new.txt.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/account/email/request-new.txt.tmpl,v  <--  request-new.txt.tmpl
initial revision: 1.1
done
Removing template/en/default/email/password.txt.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/email/password.txt.tmpl,v  <--  password.txt.tmpl
new revision: delete; previous revision: 1.1
done
Checking in template/en/default/global/messages.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/messages.html.tmpl,v  <--  messages.html.tmpl
new revision: 1.38; previous revision: 1.37
done
Checking in template/en/default/global/user-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/user-error.html.tmpl,v  <--  user-error.html.tmpl
new revision: 1.182; previous revision: 1.181
done
Comment 27 Max Kanat-Alexander 2006-09-07 17:55:06 PDT
Added to relnotes in bug 349423. Please let me know if I missed any critical information about this bug in the release notes.
Comment 28 Frédéric Buclin 2006-11-22 12:24:56 PST
Created attachment 246322 [details] [diff] [review]
documentation patch, v1
Comment 29 Frédéric Buclin 2006-11-22 12:38:12 PST
Checking in docs/xml/using.xml;
/cvsroot/mozilla/webtools/bugzilla/docs/xml/using.xml,v  <--  using.xml
new revision: 1.53; previous revision: 1.52
done
Comment 30 Hongnguyen 2015-05-16 07:55:40 PDT Comment hidden (spam)
Comment 31 Hongnguyen 2015-05-16 09:08:34 PDT Comment hidden (spam)
Comment 32 Hongnguyen 2015-05-16 09:10:54 PDT Comment hidden (spam)
Comment 33 Frédéric Buclin 2015-05-16 09:11:04 PDT
Stop writing useless comments or your account will be locked!

Note You need to log in before you can comment on or make changes to this bug.