letters_numbers_specialchars password restriction is incorrect

RESOLVED FIXED in Bugzilla 4.2

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mail, Assigned: mail)

Tracking

Bugzilla 4.2
Dependency tree / graph
Bug Flags:
approval +
approval4.4 +
approval4.2 +

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

6 years ago
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.

Updated

6 years ago
OS: Linux → All
Hardware: x86_64 → All
Target Milestone: --- → Bugzilla 4.4
(Assignee)

Comment 1

6 years ago
Posted patch bug897264-v1.patch (obsolete) — Splinter Review
Also fixed up the alignment of the error message since I was in that area anyway.
Attachment #780721 - Flags: review?(dkl)

Comment 2

6 years ago
Why do you replace \d by [[:digit:]]?
(Assignee)

Comment 3

6 years ago
(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

6 years ago
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.
Attachment #780721 - Flags: review?(dkl) → review-
(Assignee)

Comment 5

6 years ago
Posted patch v2 patch (obsolete) — Splinter Review
Attachment #780721 - Attachment is obsolete: true
Attachment #782426 - Flags: review?(LpSolit)

Comment 6

6 years ago
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 %]
Attachment #782426 - Flags: review?(LpSolit) → review-
(Assignee)

Comment 7

6 years ago
Attachment #782426 - Attachment is obsolete: true
Attachment #784690 - Flags: review?(LpSolit)

Comment 8

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

r=LpSolit
Attachment #784690 - Flags: review?(LpSolit) → review+

Updated

6 years ago
Flags: approval?
Flags: approval4.4?
(Assignee)

Comment 9

6 years ago
09:20 <@LpSolit> pfff, I reviewed it, you can approve it
Flags: approval?
Flags: approval4.4?
Flags: approval4.4+
Flags: approval+
(Assignee)

Comment 10

6 years ago
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
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.
Depends on: 558803
Flags: approval4.2?
Target Milestone: Bugzilla 4.4 → Bugzilla 4.2
Flags: approval4.2? → approval4.2+
(Assignee)

Comment 12

6 years ago
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.
You need to log in before you can comment on or make changes to this bug.