Bug 1194987: Editing your email address and make it point to a non-existent email...

r=gerv a=sgreen

diff --git a/Bugzilla/Auth/Verify.pm b/Bugzilla/Auth/Verify.pm
index e44fb06aef9057214f0c6d5f105d901054c07e3a..9dc83273b47bc83d9437cebbc6f41c214fba1fb3 100644 (file)
--- a/Bugzilla/Auth/Verify.pm
+++ b/Bugzilla/Auth/Verify.pm
@@ -106,21 +106,24 @@ sub create_or_update_user {
 
     my $user = new Bugzilla::User($user_id);
 
-    # Now that we have a valid User, we need to see if any data has to be
-    # updated.
+    # Now that we have a valid User, we need to see if any data has to be updated.
+    my $changed = 0;
+
     if ($username && lc($user->login) ne lc($username)) {
         validate_email_syntax($username)
           || return { failure => AUTH_ERROR, error => 'auth_invalid_email',
                       details => {addr => $username} };
         $user->set_login($username);
+        $changed = 1;
     }
     if ($real_name && $user->name ne $real_name) {
         # $real_name is more than likely tainted, but we only use it
         # in a placeholder and we never use it after this.
         trick_taint($real_name);
         $user->set_name($real_name);
+        $changed = 1;
     }
-    $user->update();
+    $user->update() if $changed;
 
     return { user => $user };
 }

diff --git a/Bugzilla/Mailer.pm b/Bugzilla/Mailer.pm
index 593a1067a09a760295455da5f00a1e0ca7332798..20175db155728d176100a3bcc24043ea78f44860 100644 (file)
--- a/Bugzilla/Mailer.pm
+++ b/Bugzilla/Mailer.pm
@@ -260,17 +260,14 @@ sub build_thread_marker {
 
 sub send_staged_mail {
     my $dbh = Bugzilla->dbh;
-    my @ids;
-    my $emails
-        = $dbh->selectall_arrayref("SELECT id, message FROM mail_staging");
 
-    foreach my $row (@$emails) {
-        MessageToMTA($row->[1]);
-        push(@ids, $row->[0]);
-    }
+    my $emails = $dbh->selectall_arrayref('SELECT id, message FROM mail_staging');
+    my $sth = $dbh->prepare('DELETE FROM mail_staging WHERE id = ?');
 
-    if (@ids) {
-        $dbh->do("DELETE FROM mail_staging WHERE " . $dbh->sql_in('id', \@ids));
+    foreach my $email (@$emails) {
+        my ($id, $message) = @$email;
+        MessageToMTA($message);
+        $sth->execute($id);
     }
 }
 

diff --git a/Bugzilla/Token.pm b/Bugzilla/Token.pm
index 67a201b53f68ec9d1c6f00ba4342f0fadd0cfc73..be76be645da4c023af6e82f9032b887dacf1b940 100644 (file)
--- a/Bugzilla/Token.pm
+++ b/Bugzilla/Token.pm
@@ -34,6 +34,8 @@ use parent qw(Exporter);
 # 62 = 0-9, a-z, A-Z.
 use constant TOKEN_LENGTH => 22;
 
+use constant SEND_NOW => 1;
+
 ################################################################################
 # Public Functions
 ################################################################################
@@ -122,43 +124,49 @@ sub issue_new_user_account_token {
     # who made the request, and so it is reasonable to send the email in the same
     # language used to view the "Create a New Account" page (we cannot use their
     # user prefs as the user has no account yet!).
-    MessageToMTA($message);
+    MessageToMTA($message, SEND_NOW);
 }
 
 sub IssueEmailChangeToken {
-    my ($user, $new_email) = @_;
-    my $email_suffix = Bugzilla->params->{'emailsuffix'};
-    my $old_email = $user->login;
-
-    my ($token, $token_ts) = _create_token($user->id, 'emailold', $old_email . ":" . $new_email);
+    my $new_email = shift;
+    my $user = Bugzilla->user;
 
-    my $newtoken = _create_token($user->id, 'emailnew', $old_email . ":" . $new_email);
+    my ($token, $token_ts) = _create_token($user->id, 'emailold', $user->login . ":$new_email");
+    my $newtoken = _create_token($user->id, 'emailnew', $user->login . ":$new_email");
 
     # Mail the user the token along with instructions for using it.
 
     my $template = Bugzilla->template_inner($user->setting('lang'));
     my $vars = {};
 
-    $vars->{'oldemailaddress'} = $old_email . $email_suffix;
-    $vars->{'newemailaddress'} = $new_email . $email_suffix;
+    $vars->{'newemailaddress'} = $new_email . Bugzilla->params->{'emailsuffix'};
     $vars->{'expiration_ts'} = ctime($token_ts + MAX_TOKEN_AGE * 86400);
-    $vars->{'token'} = $token;
-    $vars->{'emailaddress'} = $old_email . $email_suffix;
+
+    # First send an email to the new address. If this one doesn't exist,
+    # then the whole process must stop immediately. This means the email must
+    # be sent immediately and must not be stored in the queue.
+    $vars->{'token'} = $newtoken;
 
     my $message;
-    $template->process("account/email/change-old.txt.tmpl", $vars, \$message)
+    $template->process('account/email/change-new.txt.tmpl', $vars, \$message)
       || ThrowTemplateError($template->error());
 
-    MessageToMTA($message);
+    MessageToMTA($message, SEND_NOW);
 
-    $vars->{'token'} = $newtoken;
-    $vars->{'emailaddress'} = $new_email . $email_suffix;
+    # If we come here, then the new address exists. We now email the current
+    # address, but we don't want to stop the process if it no longer exists,
+    # to give a chance to the user to confirm the email address change.
+    $vars->{'token'} = $token;
 
-    $message = "";
-    $template->process("account/email/change-new.txt.tmpl", $vars, \$message)
+    $message = '';
+    $template->process('account/email/change-old.txt.tmpl', $vars, \$message)
       || ThrowTemplateError($template->error());
 
-    MessageToMTA($message);
+    eval { MessageToMTA($message, SEND_NOW); };
+
+    # Give the user a chance to cancel the process even if he never got
+    # the email above. The token is required.
+    return $token;
 }
 
 # Generates a random token, adds it to the tokens table, and sends it
@@ -543,17 +551,15 @@ Bugzilla::Token - Provides different routines to manage tokens.
  Returns:     Nothing. It throws an error if the same user made the same
               request in the last few minutes.
 
-=item C<sub IssueEmailChangeToken($user, $new_email)>
+=item C<sub IssueEmailChangeToken($new_email)>
 
  Description: Sends two distinct tokens per email to the old and new email
               addresses to confirm the email address change for the given
               user. These tokens remain valid for the next MAX_TOKEN_AGE days.
 
- Params:      $user      - User object of the user requesting a new
-                           email address.
-              $new_email - The new email address of the user.
+ Params:      $new_email - The new email address of the user.
 
- Returns:     Nothing.
+ Returns:     The token to cancel the request.
 
 =item C<IssuePasswordToken($user)>
 

diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 01d5fdf4e4a2ed862fe37f1e58b74e3e2c293785..ca9b5aa28fe2745d7a5f31e29a083eab387cc1ac 100644 (file)
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -2429,6 +2429,9 @@ sub check_account_creation_enabled {
 
 sub check_and_send_account_creation_confirmation {
     my ($self, $login) = @_;
+    my $dbh = Bugzilla->dbh;
+
+    $dbh->bz_start_transaction;
 
     $login = $self->check_login_name($login);
     my $creation_regexp = Bugzilla->params->{'createemailregexp'};
@@ -2443,6 +2446,8 @@ sub check_and_send_account_creation_confirmation {
     # Create and send a token for this new account.
     require Bugzilla::Token;
     Bugzilla::Token::issue_new_user_account_token($login);
+
+    $dbh->bz_commit_transaction;
 }
 
 # This is used in a few performance-critical areas where we don't want to

diff --git a/template/en/default/account/email/change-new.txt.tmpl b/template/en/default/account/email/change-new.txt.tmpl
index a6c54a7731b332ca4372008d969e962e178932e0..f32abd80d423adf2e3b32b1630d6be74820ef709 100644 (file)
--- a/template/en/default/account/email/change-new.txt.tmpl
+++ b/template/en/default/account/email/change-new.txt.tmpl
@@ -7,12 +7,12 @@
   #%]
 
 From: [% Param('mailfrom') %]
-To: [% emailaddress %]
+To: [% newemailaddress %]
 Subject: [% terms.Bugzilla %] Change Email Address Request
 X-Bugzilla-Type: admin
 
 [%+ terms.Bugzilla %] has received a request to change the email address
-for the account [% oldemailaddress %] to your address.
+for the account [% user.email %] to your address.
 
 To confirm the change, visit the following link:
 

diff --git a/template/en/default/account/email/change-old.txt.tmpl b/template/en/default/account/email/change-old.txt.tmpl
index 24b90a2b0dbba5d33da12e3d0b24232685850110..400004c5ac3d093312ff05b8868b2a45b30b81d6 100644 (file)
--- a/template/en/default/account/email/change-old.txt.tmpl
+++ b/template/en/default/account/email/change-old.txt.tmpl
@@ -7,7 +7,7 @@
   #%]
 
 From: [% Param('mailfrom') %]
-To: [% emailaddress %]
+To: [% user.email %]
 Subject: [% terms.Bugzilla %] Change Email Address Request
 Importance: High
 X-MSMail-Priority: High

diff --git a/template/en/default/account/prefs/prefs.html.tmpl b/template/en/default/account/prefs/prefs.html.tmpl
index 8d90d7b6e9bde051ce2137525e24502036aed129..ea47b7663ab0283f46d4c04244a7142b0b99269d 100644 (file)
--- a/template/en/default/account/prefs/prefs.html.tmpl
+++ b/template/en/default/account/prefs/prefs.html.tmpl
@@ -75,6 +75,10 @@
         In order to confirm your request, we have sent an email to your
         new email address. As a precaution, an email has also been sent
         to your old address allowing you to cancel this change if needed.
+        If you don't receive the email, you can
+        <a href="token.cgi?t=[% email_token FILTER uri %]&amp;a=cxlem">
+        cancel the email address change</a> from here if you wish (especially
+        if you mistyped the new email address).
       </p>
     [% END %]
   </div>

diff --git a/userprefs.cgi b/userprefs.cgi
index 56157dfcd94c2dfdfacafdfe63695df0421db2bd..4b1a7d542e91f210917f0d1ab4e0a986dc3e6c6a 100755 (executable)
--- a/userprefs.cgi
+++ b/userprefs.cgi
@@ -112,8 +112,7 @@ sub SaveAccount {
             is_available_username($new_login_name)
               || ThrowUserError("account_exists", {email => $new_login_name});
 
-            Bugzilla::Token::IssueEmailChangeToken($user, $new_login_name);
-
+            $vars->{'email_token'} = Bugzilla::Token::IssueEmailChangeToken($new_login_name);
             $vars->{'email_changes_saved'} = 1;
         }
     }