LoginToboggan validates the submitted password at registration time to enforce that it only contain certain characters; however the change password form in user/edit makes no such checks and you can set your password to anything. The check doesn't seem particularly useful, and excludes a number of reasonable seeming punctuation characters (like !, $, %, ^, etc) from being used as the registration-time password.

Am I missing something? It'd be nice to allow more punctuation, and if the password format is going to be validated it should really be validated in user/edit too according to the same criteria..

CommentFileSizeAuthor
#10 logintoboggan_passwordchange_validate.patch856 bytesrobert castelo
#6 logintoboggan_passwordchange_validate.diff1.44 KBtorne
#5 logintoboggan_pw_chars.diff1.65 KBtorne

Comments

hunmonk’s picture

hunmonk commented
Category: bug » task

this is not a bug, it's a current feature. my personal opinion is that some kind of checking should be done in for passwords in core.

hunmonk’s picture

hunmonk commented
Title: Password validation doesn't make much sense » Remove password validation?
torne’s picture

torne commented

Yah, that would be nice, but the filter it uses at the moment is annoyingly restrictive. Couldn't it at least accept more punctuation? None of the passwords I use were accepted :)

hunmonk’s picture

hunmonk commented

you are welcome to submit a patch for it if you like, and i'll review--it's not something i'm interested in coding, as it works fine for me as is :)

torne’s picture

torne commented
Title: Remove password validation? » Improve password validation
Status: Active » Needs review
StatusFileSize
new logintoboggan_pw_chars.diff1.65 KB

Here you go then. Two seperate patches: this one enlarges the range of accepted characters in the password to include all printable non-space characters (basically, punctuation as well as letters/numbers). Tested on my server, seems to be happy :)

torne’s picture

torne commented
StatusFileSize
new logintoboggan_passwordchange_validate.diff1.44 KB

...and this one adds an extra validation function to user/edit to enforce the same password checks when the user (or an admin) changes their password. It only does the check if the password is being changed, obviously.

Hope this is useful :)

hunmonk’s picture

hunmonk commented
Version: 4.7.x-1.x-dev » 7.x-1.x-dev
Status: Needs review » Fixed

patch applied to 4.7 and HEAD in logintoboggan. can you please submit the core patch as a core issue?

torne’s picture

torne commented
Status: Fixed » Reviewed & tested by the community

It's not a core patch, it's a logintoboggan patch. It adds validation to user/edit via hook_form_alter and uses the same password validation function that's already implemented in logintoboggan.

hunmonk’s picture

hunmonk commented
Status: Reviewed & tested by the community » Fixed

heh, that's what i get for not reading the patch... :)

applied to 4.7/HEAD--thanks!

robert castelo’s picture

robert castelo commented
Status: Fixed » Reviewed & tested by the community
StatusFileSize
new logintoboggan_passwordchange_validate.patch856 bytes

Try changing your password.

I get an error message saying the password is too short, even when it's not.

Patch above breaks validation because $pass is not a string, it's an array containing ['pass1'] and ['pass2'], the password and confirm password fields.

Here's a new patch that fixes the problem.

hunmonk’s picture

hunmonk commented
Status: Reviewed & tested by the community » Needs work

@Robert Castelo: i am unable to replicate this bug--password validation appears to be working fine. please note that function password_confirm_validate is called before function logintoboggan_user_edit_validate--this function eliminates the array and reduces the password to a string field, so i don't believe your fix is necessary.

marking as needs work, but please close if you confirm the above.

torne’s picture

torne commented

Confirming that it works for me too...

patchak’s picture

patchak commented

Well,

I just installed the latest 4.7 version of login toboggan and now editing a user,s account is impossible as it always asks for the password, and even when you supply another one it always says it's too short.

Please help!

hunmonk’s picture

hunmonk commented

once again, i am unable to confirm this issue. i see no problems in the password validation workflow, and am receiving no errors. i cannot debug if i cannot recreate!

i have just committed a change to the 4.7 and HEAD versions of the module that now makes password length validation optional, and configurable--check the settings for the module. please install the updated version and report back if the problem still exists. if you're still having a problem, please include detailed steps on how to reproduce!

Bèr Kessels’s picture

Bèr Kessels commented

-- on user/#/edit I get the same validation error as Robert Castelo did: logintobogan is telling me the PW is too short (make sure when replicating that you have the minimum length enabled).

I am not sure if I should rather start a new issue, but since I found references to that issue in this thread, I thought I should best report it here.

hunmonk’s picture

hunmonk commented

please include _exact_ steps on how to reproduce, and drupal/apache/mysql/php versions -- i'd like all this since this issue has been so hard to tackle :)

do you have any other contrib modules installed where you're experiencing the problem? if so, which ones?

hunmonk’s picture

hunmonk commented
Version: 7.x-1.x-dev » 5.x-1.x-dev
Category: task » bug
Priority: Minor » Normal
Status: Needs work » Closed (fixed)

i'm closing this issue. nobody has reported back in almost a year, and i'm sure i'd have heard something more by now if it was a widespread problem. please feel free to re-open if more information comes to light.

vmenelas’s picture

vmenelas commented

yea