The default bug view has changed. See this FAQ.

letters_numbers_specialchars password restriction is incorrect

RESOLVED FIXED in Bugzilla 4.2

Status

()

Bugzilla
User Accounts
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: Simon Green, Assigned: Simon Green)

Tracking

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

Details

Attachments

(1 attachment, 2 obsolete attachments)

2.14 KB, patch
Frédéric Buclin
: review+
Details | Diff | Splinter Review
(Assignee)

Description

4 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

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

Comment 1

4 years ago
Created attachment 780721 [details] [diff] [review]
bug897264-v1.patch

Also fixed up the alignment of the error message since I was in that area anyway.
Attachment #780721 - Flags: review?(dkl)

Comment 2

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

Comment 3

4 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

4 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

4 years ago
Created attachment 782426 [details] [diff] [review]
v2 patch
Attachment #780721 - Attachment is obsolete: true
Attachment #782426 - Flags: review?(LpSolit)

Comment 6

4 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

4 years ago
Created attachment 784690 [details] [diff] [review]
bug897264-v3.patch
Attachment #782426 - Attachment is obsolete: true
Attachment #784690 - Flags: review?(LpSolit)

Comment 8

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

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

Updated

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

Comment 9

4 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

4 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: 4 years ago
Resolution: --- → FIXED

Comment 11

4 years ago
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

4 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.