897264 - letters_numbers_specialchars password restriction is incorrect

Status: RESOLVED FIXED

(Bugzilla :: User Accounts, defect)

Description

[Simon Green]

The regular expression used for the letters_numbers_specialchars password is incorrect, as a letter does not need to be used. For example, '1234567!' is would be considered to be a valid password.

Comment 1

[Simon Green]

Attachment: bug897264-v1.patch

Also fixed up the alignment of the error message since I was in that area anyway.

Comment 2

[Frédéric Buclin]

Why do you replace \d by [[:digit:]]?

Comment 3

[Simon Green]

(In reply to Frédéric Buclin from comment #2)
> Why do you replace \d by [[:digit:]]?

For consistency. AFAIK, they are the same (whereas [:alpha:] and \w are different). Plain English '[:digit:]' is also easier to read than the more cryptic '\d'

  -- simon

Comment 4

[Frédéric Buclin]

Comment on attachment 780721 [details] [diff] [review]
bug897264-v1.patch

I don't care about readability. \d is used everywhere in our codebase and is well understood. We don't use [[:digit:]] anywhere else. I don't call that consistency. Please revert this change.

Comment 5

[Simon Green]

Attachment: v2 patch

Comment 6

[Frédéric Buclin]

Comment on attachment 782426 [details] [diff] [review]
v2 patch

>=== modified file 'template/en/default/global/user-error.html.tmpl'

>+      [% IF passregex.search('letters') %]
>+        [% IF passregex.search('specialchars') %]
>+          <li>letter</li>
>+        [% ELSE %]
>+          <li>UPPERCASE letter</li>
>+          <li>lowercase letter</li>
>+        [% END %]
>+      [% END %]
>+      [% IF passregex.search('numbers') %]
>+        <li>digit</li>
>+      [% END %]
>+      [% IF passregex.search('specialchars') %]
>+        <li>special character</li>
>+      [% END %]

As you are now looking twice for 'specialchars', please combine them together:

 [% IF passregex == 'letters_numbers_specialchars' %]
   <li>letter</li>
   <li>special character</li>
 [% ELSIF passregex.search('letters') %]
   <li>UPPERCASE letter</li>
   <li>lowercase letter</li>
 [% END %]

 [% IF passregex.search('numbers') %]
   <li>digit</li>
 [% END %]

Comment 7

[Simon Green]

Attachment: bug897264-v3.patch

Comment 8

[Frédéric Buclin]

Comment on attachment 784690 [details] [diff] [review]
bug897264-v3.patch

r=LpSolit

Comment 9

[Simon Green]

09:20 <@LpSolit> pfff, I reviewed it, you can approve it

Comment 10

[Simon Green]

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/User.pm
modified template/en/default/global/user-error.html.tmpl
Committed revision 8688.                                                       

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/4.4/
modified Bugzilla/User.pm
modified template/en/default/global/user-error.html.tmpl
Committed revision 8588.

Comment 11

[Frédéric Buclin]

Actually, this parameter has been implemented in Bugzilla 4.2, see bug 558803. As it affects the security-level of your passwords, it may be important to fix this bug there too.

Comment 12

[Simon Green]

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/4.2/                         
modified Bugzilla/User.pm
modified template/en/default/global/user-error.html.tmpl
Committed revision 8221.