Closed Bug 87795 Opened 23 years ago Closed 18 years ago

Creating an account should send token and wait for confirmation (prevent user account abuse)

Categories

(Bugzilla :: User Accounts, enhancement, P1)

2.13
enhancement

Tracking

()

VERIFIED FIXED
Bugzilla 3.0

People

(Reporter: CodeMachine, Assigned: LpSolit)

References

Details

Attachments

(2 files, 2 obsolete files)

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.
Target Milestone: --- → Bugzilla 2.18
Severity: normal → enhancement
Priority: -- → P3
-> Bugzilla product, User Accounts component, reassigning.
Assignee: tara → myk
Component: Bugzilla → User Accounts
Product: Webtools → Bugzilla
Version: Bugzilla 2.13 → 2.12
restoring version field lost in product change
Version: 2.12 → 2.13
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 :)
Can the last login date already be obtained using the lastused column in the
logincookies table?
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.
*** Bug 137499 has been marked as a duplicate of this bug. ***
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.
Adding summary from dupe since it's more precise.
Summary: Prevent account abuse. → Creating an account should wait for confirmation (prevent account abuse)
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).
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.
Target Milestone: Bugzilla 2.18 → Bugzilla 2.20
*** Bug 243303 has been marked as a duplicate of this bug. ***
sneaking a few additional words into the summary to make it easier to find
Summary: Creating an account should wait for confirmation (prevent account abuse) → Creating an account should send token and wait for confirmation (prevent user account abuse)
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.
Target Milestone: Bugzilla 2.20 → Bugzilla 2.22
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).
Priority: P3 → P1
The trunk is now frozen to prepare Bugzilla 2.22. Enhancement bugs are retargetted to 2.24.
Target Milestone: Bugzilla 2.22 → Bugzilla 2.24
Blocks: 174326
No longer blocks: 174326
/me wants to see this bug fixed for 2.24.
Assignee: myk → bugzilla-mozilla
QA Contact: mattyt-bugzilla → default-qa
Attached patch patch, v1 (obsolete) — Splinter Review
Assignee: bugzilla-mozilla → LpSolit
Attachment #56077 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #232354 - Flags: review?(mkanat)
Attachment #232354 - Flags: review?(bugzilla-mozilla)
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.
Attachment #232354 - Flags: review?(mkanat) → review+
> 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 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.
Attachment #232354 - Flags: review?(bugzilla-mozilla) → review+
Already requesting approval as justdave is on vacation and myk is usually away during the week-end. ;)
Flags: approval?
Flags: approval? → approval+
Attached patch patch, v2Splinter Review
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().
Attachment #232354 - Attachment is obsolete: true
Attachment #234586 - Flags: review?(mkanat)
Comment on attachment 234586 [details] [diff] [review]
patch, v2

carrying forward bkor's r+ per discussion on IRC.
Attachment #234586 - Flags: review+
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.
Attachment #234586 - Flags: review?(mkanat) → review+
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
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Keywords: relnote
Resolution: --- → FIXED
Blocks: 349336
Blocks: 349337
Added to relnotes in bug 349423. Please let me know if I missed any critical information about this bug in the release notes.
Keywords: relnote
Flags: documentation?
Flags: testcase?
Attachment #246322 - Flags: review?(documentation)
Attachment #246322 - Flags: review?(documentation) → review+
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
Flags: documentation? → documentation+
Flags: testcase? → testcase+
Status: RESOLVED → VERIFIED
Stop writing useless comments or your account will be locked!
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: