Last Comment Bug 575947 - Users with passwords length less than 6 characters can't login after migration from 3.4.x or older to 3.6 or newer
: Users with passwords length less than 6 characters can't login after migratio...
Status: RESOLVED FIXED
:
Product: Bugzilla
Classification: Server Software
Component: User Accounts (show other bugs)
: 3.6
: All All
: -- normal (vote)
: Bugzilla 3.6
Assigned To: Frédéric Buclin
: default-qa
Mentors:
: 637016 (view as bug list)
Depends on:
Blocks: 604522
  Show dependency treegraph
 
Reported: 2010-06-30 04:23 PDT by Andrey Buranov
Modified: 2011-02-27 05:31 PST (History)
5 users (show)
mkanat: approval+
mkanat: approval4.0+
mkanat: blocking4.0+
mkanat: approval3.6+
mkanat: blocking3.6.3+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch, v1 (9.65 KB, patch)
2010-09-14 10:32 PDT, Frédéric Buclin
no flags Details | Diff | Splinter Review
patch, v2 (6.99 KB, patch)
2010-10-09 07:59 PDT, Frédéric Buclin
mkanat: review-
Details | Diff | Splinter Review
patch, v3 (6.25 KB, patch)
2010-10-13 17:04 PDT, Frédéric Buclin
mkanat: review+
Details | Diff | Splinter Review

Description Andrey Buranov 2010-06-30 04:23:20 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.6) Gecko/20100625 Firefox/3.6.6
Build Identifier: 3.6.1

I just finished the test migration from version 2.22.3 to 3.6.1 and discovered that I can't login with my password that have less than 6 characters. For Users with password length more than 6 characters all looks fine.

The problem is sufficiently important because I can't check the passwords for all several hundred users and change it manually. And it was no any words about this issue during migration process while checksetup.pl script worked.

How can I avoid this problem during migration or remove this security requirement just after?

Reproducible: Always

Steps to Reproduce:
1. You have not customized bugzilla installation based on version 2.2.23 (may be some other) and user with password length less than 6 characters
2. Migrate it to version 3.6.1
3. Try to login by any user with password length less than 6 characters
Actual Results:  
Now it's impossible to login for users with password length less than 6 characters.

Expected Results:  
It should be possible to login for users with password length less than 6 characters. Or may be they should be somehow automatically informed about this requirement...
Comment 1 Frédéric Buclin 2010-06-30 04:35:57 PDT
When logging in, there is no check about the password length, so old passwords are still working with 3.6.1. This check is only done when changing your password.
Comment 2 Frédéric Buclin 2010-09-09 16:53:38 PDT
OK, I just realized what's going on. When upgrading from 3.2 or older to 3.6 or newer, we move from a DB storing passwords encrypted with crypt() to a DB storing passwords encrypted with SHA-256. When Bugzilla detects that the stored password has been encrypted with crypt(), it encrypts it to SHA-256, see Bugzilla::Auth::Verify::DB::check_credentials(). To do that, it calls $user->set_password(), which itself calls validate_password() via _check_password(). But validate_password() checks the password length, and realizes that it's now smaller than the increased (from 3 to 6) minimum length. As a consequence, the password is rejected despite it's not a new one.

There are two alternatives here:
1) pass a 2nd argument NO_CHECK to set_password() so that it doesn't call validate_password();
2) or force the user to enter a new password because it's too short.

Any preference?
Comment 3 Reed Loden [:reed] (use needinfo?) 2010-09-09 16:55:15 PDT
The latter. Should present an intermediary form requiring the user to create a new password. This form will be useful for future changes as well, such as bug 558803.
Comment 4 Frédéric Buclin 2010-09-10 04:27:37 PDT
Taking! I have a patch ready. I need to test it carefully first before attaching it here.
Comment 5 Frédéric Buclin 2010-09-14 10:32:07 PDT
Created attachment 475121 [details] [diff] [review]
patch, v1

Rather than displaying an intermediate page, which wouldn't work with WS, I prefer to use the traditional way to change the password, i.e. by email. So if the password is too short, an error is thrown, asking the user to follow the URL in the email he got to enter a new password. Refactoring the code would be too invasive for 3.6 and 4.0.
Comment 6 Max Kanat-Alexander 2010-10-01 04:24:09 PDT
Comment on attachment 475121 [details] [diff] [review]
patch, v1

Why not just return AUTH_ERROR with the error password_too_short, and have that error message link to the reset password page?
Comment 7 Frédéric Buclin 2010-10-01 10:21:10 PDT
(In reply to comment #6)
> Why not just return AUTH_ERROR with the error password_too_short, and have that
> error message link to the reset password page?

You mean only changing the error thrown? Because you still need a new constant for this kind of problem. My idea was to avoid a useless extra step, and send an email immediately.
Comment 8 Max Kanat-Alexander 2010-10-01 16:50:29 PDT
Ah, okay. But at some point, this code will pretty much become obsolete (because everybody will have upgraded their passwords), and it will only apply to a small number of users to start with (people who have passwords shorter than six characters). So it would be best to keep it as simple and as small as possible, and I think the simplest fix is just to link them to the password reset form, no?
Comment 9 Frédéric Buclin 2010-10-02 10:22:07 PDT
(In reply to comment #8)
> Ah, okay. But at some point, this code will pretty much become obsolete

An admin is free to change the USER_PASSWORD_MIN_LENGTH constant. Also, assuming we move to 8 characters in the future, the code would still be applicable. I will simply change the error message to let the user ask for a new password, to make the patch less invasive.
Comment 10 Max Kanat-Alexander 2010-10-02 10:28:39 PDT
Okay, that sounds good.
Comment 11 Frédéric Buclin 2010-10-09 07:59:23 PDT
Created attachment 482040 [details] [diff] [review]
patch, v2
Comment 12 Max Kanat-Alexander 2010-10-09 08:26:42 PDT
Comment on attachment 482040 [details] [diff] [review]
patch, v2

Cool, this is looking better.

Could you explain why you can't use AUTH_ERROR?
Comment 13 Frédéric Buclin 2010-10-09 08:32:05 PDT
(In reply to comment #12)
> Could you explain why you can't use AUTH_ERROR?

AUTH_ERROR is for code errors, which calls ThrowCodeError(). Here, we want ThrowUserError(), as the error is not a code error.
Comment 14 Max Kanat-Alexander 2010-10-10 17:13:09 PDT
Comment on attachment 482040 [details] [diff] [review]
patch, v2

Oh, you're right, I didn't realize that.

I'm not sure that a new constant is the best way to go, though. I'm going to think about it for a bit.
Comment 15 Max Kanat-Alexander 2010-10-13 15:46:02 PDT
Comment on attachment 482040 [details] [diff] [review]
patch, v2

Okay, so here's my thoughts on it:

Instead of calling it AUTH_PASSWORD_TOO_SHORT, call it AUTH_USER_ERROR, and make it work just like AUTH_ERROR but throw user errors instead. Or alternately, allow passing a particular hash key back along with AUTH_ERROR to do UserError instead of CodeError.

The error's id should be password_too_short instead of password_force_change.

The WebService description of the error should just say that the user is required to change their password for one reason or another (this will let customizers re-use this if they want to force password changes).
Comment 16 Frédéric Buclin 2010-10-13 17:04:36 PDT
Created attachment 483021 [details] [diff] [review]
patch, v3
Comment 17 Max Kanat-Alexander 2010-10-13 17:09:06 PDT
Comment on attachment 483021 [details] [diff] [review]
patch, v3

Yeah, this looks great! :-) Thanks!!
Comment 18 Frédéric Buclin 2010-10-13 17:44:32 PDT
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Auth.pm
modified Bugzilla/Auth/Verify/DB.pm
modified Bugzilla/WebService/Constants.pm
modified Bugzilla/WebService/User.pm
modified template/en/default/account/email/confirm-new.html.tmpl
modified template/en/default/account/password/set-forgotten-password.html.tmpl
modified template/en/default/global/user-error.html.tmpl
Committed revision 7541.


Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.0/
modified Bugzilla/Auth.pm
modified Bugzilla/Auth/Verify/DB.pm
modified Bugzilla/WebService/Constants.pm
modified Bugzilla/WebService/User.pm
modified template/en/default/account/email/confirm-new.html.tmpl
modified template/en/default/account/password/set-forgotten-password.html.tmpl
modified template/en/default/global/user-error.html.tmpl
Committed revision 7440.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/3.6/
modified Bugzilla/Auth.pm
modified Bugzilla/Auth/Verify/DB.pm
modified Bugzilla/WebService/Constants.pm
modified Bugzilla/WebService/User.pm
modified template/en/default/account/email/confirm-new.html.tmpl
modified template/en/default/account/password/set-forgotten-password.html.tmpl
modified template/en/default/global/user-error.html.tmpl
Committed revision 7187.
Comment 19 stanislav.seltser 2011-02-21 08:50:01 PST
how do i change this constant USER_PASSWORD_MIN_LENGTH? i could not find a place in the code where it can be changed. it it stored in db table?

i had upgraded to bugzilla 4.0 and now none of my users can login including myself because passwords were too short
Comment 20 stanislav.seltser 2011-02-21 08:51:21 PST
another question is bugzilla-4.0 distribution on the web site reflects the patch you guys had made?
Comment 21 stanislav.seltser 2011-02-21 08:55:05 PST
even worst problem is that my email server is not yet setup on upgraded bugzilla instance so i cannot even get email with new password.
Comment 22 Frédéric Buclin 2011-02-21 11:08:18 PST
(In reply to comment #21)
> even worst problem is that my email server is not yet setup on upgraded
> bugzilla instance so i cannot even get email with new password.

This is a question for the support mailing-list, so additional questions or problems should be redirected there. But in short: fix your email server, then click the forgot password link to set a new one. We intentionnaly reject too short passwords for security reasons. *If and only if* you don't care too much about security can you edit the USER_PASSWORD_MIN_LENGTH constant in Bugzilla/Constants.pm.
Comment 23 Max Kanat-Alexander 2011-02-27 05:31:02 PST
*** Bug 637016 has been marked as a duplicate of this bug. ***

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