1009013 - Require a user to change their password if they log in and their current password does not meet the password complexity rules

Status: RESOLVED FIXED

(Bugzilla :: User Accounts, enhancement)

Description

[:glob ✱]

users with existing passwords which don't match the current password complexity rule should be forced to update their password once they log in.

this should only apply to web-page logins (not webservice/api calls).

Comment 2

[Simon Green]

Attachment: bug1009013-v1.patch

Comment 3

[:glob ✱]

Comment on attachment 8459289 [details] [diff] [review]
bug1009013-v1.patch

Review of attachment 8459289 [details] [diff] [review]:
-----------------------------------------------------------------

this looks great; just a few minor changes and clarifications required.

::: Bugzilla/User.pm
@@ +2448,5 @@
>  }
>  
>  sub validate_password {
> +    my $check = validate_password_check(@_);
> +    ThrowUserEorr($check) if $check;

ThrowUserEorr --> ThrowUserError

@@ +3156,5 @@
> +=item C<validate_password_check($passwd1, $passwd2)>
> +
> +Returns undef if a password is valid (i.e. meets Bugzilla's
> +requirements for length and content), else returns the error
> +Untaints C<$passwd1> if successful.

can you reword this slightly so both the actions performed on valid passwords are together (returns undef and untaints $passwd1)

@@ +3159,5 @@
> +requirements for length and content), else returns the error
> +Untaints C<$passwd1> if successful.
> +
> +If a second password is passed in, this function also verifies that
> +the two passwords match. This allows code to handle its own errors.

how does verification that the two passwords match allow the code to handle its own errors?

::: template/en/default/admin/params/auth.html.tmpl
@@ +135,5 @@
>      "letter, a number and a special character.</li></ul>"
> +
> +  password_check_on_login =>
> +    "If set, $terms.Bugzilla will check that the password meets the current " _
> +    "complexity rules and minimum length requirements. If it doesn't, the " _

after requirements add " when the user logs into the Bugzilla web interface" (or similar)

::: template/en/default/global/user-error.html.tmpl
@@ +1450,5 @@
>      The password must be at least
>      [%+ constants.USER_PASSWORD_MIN_LENGTH FILTER html %] characters long.
> +    [% IF locked_user %]
> +    You must <a href="token.cgi?a=reqpw&amp;loginname=[% locked_user.email FILTER uri %]&amp;token=[% issue_hash_token(['reqpw']) FILTER uri %]">
> +    request a new password</a> in order to log in again.

nit: indent

@@ +1471,5 @@
>        [% END %]
>      </ul>
> +    [% IF locked_user %]
> +    You must <a href="token.cgi?a=reqpw&amp;loginname=[% locked_user.email FILTER uri %]&amp;token=[% issue_hash_token(['reqpw']) FILTER uri %]">
> +    request a new password</a> in order to log in again.

nit: indent

Comment 4

[Simon Green]

Attachment: bug1009013-v2.patch

(In reply to Byron Jones ‹:glob› from comment #3)
> Comment on attachment 8459289 [details] [diff] [review]
> bug1009013-v1.patch
> 
> Review of attachment 8459289 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> this looks great; just a few minor changes and clarifications required.
> 
> ::: Bugzilla/User.pm
> @@ +3159,5 @@
> > +requirements for length and content), else returns the error
> > +Untaints C<$passwd1> if successful.
> > +
> > +If a second password is passed in, this function also verifies that
> > +the two passwords match. This allows code to handle its own errors.
> 
> how does verification that the two passwords match allow the code to handle
> its own errors?

I was talking about two different things. Have reworded to make it clearer.

All other issues fixed too.

Comment 5

[Frédéric Buclin]

Does it make sense to have a parameter for that? If an admin wants more secure passwords, then I don't see the logic why this rule should only apply to new accounts.

You should avoid adding new parameters unless they are really needed. Do you remember https://wiki.mozilla.org/Bugzilla:Params (that was in 2007) where we removed some useless parameters to make Bugzilla a bit easier to configure?

Comment 6

[Simon Green]

(In reply to Frédéric Buclin from comment #5)
> Does it make sense to have a parameter for that? If an admin wants more
> secure passwords, then I don't see the logic why this rule should only apply
> to new accounts.

If my company is anything to go by, then yes, it does make sense. We've had situations of very insecure passwords been used, so we want to tighten this up.

However, we have a lot of people that use APIs with their login and password and don't want to disrupt those users that have both APIs and log in via the web UI.
 
> Do you
> remember https://wiki.mozilla.org/Bugzilla:Params (that was in 2007) where
> we removed some useless parameters to make Bugzilla a bit easier to
> configure?

No, that was before my time :)

Comment 7

[Frédéric Buclin]

(In reply to Simon Green (personal account) from comment #6)
> However, we have a lot of people that use APIs with their login and password
> and don't want to disrupt those users that have both APIs and log in via the
> web UI.

So what you say is that it's fine to have these users use unsecure passwords. Generally, you only need one unsecure account to enter a system. In one of my previous jobs, the sysadmin was periodically running a dictionnary against each account, and all accounts with a weak password were automatically disabled and the passwords sent to everybody in the company to show how ridiculous the passwords were. This was a way to educate users. :)

Comment 8

[:glob ✱]

Comment on attachment 8478714 [details] [diff] [review]
bug1009013-v2.patch

Review of attachment 8478714 [details] [diff] [review]:
-----------------------------------------------------------------

r=glob

while i agree with the desire to not add parameters without reason, implementing this change without allowing admins to opt-out may cause issues with large public sites.  i think the parameter is warranted.

Comment 9

[Simon Green]

To ssh://[e-mail]/bugzilla/bugzilla.git
   ab1b842..b8ecdca  master -> master