Bug 670669 - Changing the e-mail address under account prefs does not require current password if can_change_password is false

r=dkl, a=simon

4 files changed, 32 insertions, 22 deletions

diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index d6c1f1225..01d5fdf4e 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -2357,6 +2357,19 @@ sub account_ip_login_failures {
     return $self->{account_ip_login_failures};
 }
 
+sub check_current_password {
+    my $self = shift;
+    my $password = shift || ThrowUserError("current_password_required");
+
+    my $cryptpwd
+        = $self->cryptpassword || ThrowCodeError("unable_to_retrieve_password");
+
+    if (bz_crypt($password, $cryptpwd) ne $cryptpwd) {
+        ThrowUserError("current_password_incorrect");
+    }
+
+}
+
 ###############
 # Subroutines #
 ###############
@@ -3103,6 +3116,11 @@ set_groups.
 
 C<bool> - Sets C<disable_mail> to the inverse of the boolean provided.
 
+=item C<check_current_password>
+
+C<string> - Throws an error if the supplied password does not match the
+user's current password.
+
 =back
 
 =head1 CLASS FUNCTIONS
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 57c06c204..710928562 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -1478,11 +1478,11 @@
       See the list of available <a href="describekeywords.cgi?show_inactive_keywords=1">keywords</a>.
     [% END %]
 
-  [% ELSIF error == "old_password_incorrect" %]
+  [% ELSIF error == "current_password_incorrect" %]
     [% title = "Incorrect Password" %]
     You did not enter your current password correctly.
 
-  [% ELSIF error == "old_password_required" %]
+  [% ELSIF error == "current_password_required" %]
     [% title = "Old Password Required" %]
     You must enter your old password to change your email address.
 
diff --git a/token.cgi b/token.cgi
index 830ecfccb..eba336d98 100755
--- a/token.cgi
+++ b/token.cgi
@@ -210,14 +210,11 @@ sub changeEmail {
     $dbh->bz_start_transaction();
     
     my $user = Bugzilla::User->check({ id => $userid });
-    my $realpassword = $user->cryptpassword;
     my $cgipassword  = $cgi->param('password');
 
     # Make sure the user who wants to change the email address
     # is the real account owner.
-    if (bz_crypt($cgipassword, $realpassword) ne $realpassword) {
-        ThrowUserError("old_password_incorrect");
-    }
+    $user->check_current_password($cgipassword);
 
     # The new email address should be available as this was 
     # confirmed initially so cancel token if it is not still available
diff --git a/userprefs.cgi b/userprefs.cgi
index 71b274c01..56157dfcd 100755
--- a/userprefs.cgi
+++ b/userprefs.cgi
@@ -74,29 +74,24 @@ sub SaveAccount {
     my $user = Bugzilla->user;
 
     my $oldpassword = $cgi->param('old_password');
+    my $verified_password;
     my $pwd1 = $cgi->param('new_password1');
     my $pwd2 = $cgi->param('new_password2');
     my $new_login_name = trim($cgi->param('new_login_name'));
 
     if ($user->authorizer->can_change_password
-        && ($oldpassword ne "" || $pwd1 ne "" || $pwd2 ne ""))
+        && ($pwd1 ne "" || $pwd2 ne ""))
     {
-        my $oldcryptedpwd = $user->cryptpassword;
-        $oldcryptedpwd || ThrowCodeError("unable_to_retrieve_password");
+        $user->check_current_password($oldpassword);
+        $verified_password = 1;
 
-        if (bz_crypt($oldpassword, $oldcryptedpwd) ne $oldcryptedpwd) {
-            ThrowUserError("old_password_incorrect");
-        }
-
-        if ($pwd1 ne "" || $pwd2 ne "") {
-            $pwd1 || ThrowUserError("new_password_missing");
-            validate_password($pwd1, $pwd2);
+        $pwd1 || ThrowUserError("new_password_missing");
+        validate_password($pwd1, $pwd2);
 
-            if ($oldpassword ne $pwd1) {
-                $user->set_password($pwd1);
-                # Invalidate all logins except for the current one
-                Bugzilla->logout(LOGOUT_KEEP_CURRENT);
-            }
+        if ($oldpassword ne $pwd1) {
+            $user->set_password($pwd1);
+            # Invalidate all logins except for the current one
+            Bugzilla->logout(LOGOUT_KEEP_CURRENT);
         }
     }
 
@@ -105,7 +100,7 @@ sub SaveAccount {
         && $new_login_name)
     {
         if ($user->login ne $new_login_name) {
-            $oldpassword || ThrowUserError("old_password_required");
+            $verified_password || $user->check_current_password($oldpassword);
 
             # Block multiple email changes for the same user.
             if (Bugzilla::Token::HasEmailChangeToken($user->id)) {