FilipNest
commented
8 years ago If sites keep getting hacked because of silly passwords then it will reflect bad on the system even if it is people being silly.
I don't agree with this at all. Bad storage and sending and receiving of passwords and user data, support for different login methods, https and two-factor authentication and general code security are the things that a system gets graded on, not how strong its users passwords are (an arbitrary rule). We should be doing audits and work on that stuff, not making people feel secure when they may not be.
It's silly to argue about as it's a minor point so I'm happy to have it merged in if you/others feel it's essential. I've got a few issues remaining though:
Glad you managed to get the library down from 38MB to 2MB though I wonder if it's as effective without all the dictionary code that's been taken out. Also makes it harder to update the package as we've effectively forked it.
I tried typing in a very long password in the field to test it and Chrome crashed. Tried again and it happened again. May be worth investigating. There's an issue in the project's queue about it Script less/unresponsive for long passwords that seems to point to the same thing.
The algorithm is pretty harsh and not very predictable which will annoy people (passwords containing various capitalisation, punctuation and 16 characters are marked as weak). Length is the main issue in password strength. Getting people to put a few numbers in is false security and just makes them harder to remember.
It also uses the word ridiculus as a password strength which is spelt wrong. I also suppose the messages are not translatable which could be a problem. Makes me wonder how we could do client side translations. An HTTP request? Not sure what's best.
Looks good though so happy to have it in if the spelling error is fixed. Slightly worried about the crash issue but that's hopefully minor.
Perhaps a simpler version just checking for password length and using icons instead of words? That'll make it run smoother and avoid the translatability problem.
adam-clarey
To follow on from a previous PR:
(Filips comments) Passwords
I like the password duplicate checker but think this is a major overkill for what we're trying to achieve. The bootstrap-strength-meter js file I suppose is fine as it's small but the password score library is 36.8MB which is only 4MB less than all of Iris plus all its dependencies (the majority of that is the 11MB prettydiff module which I'm going to remove for #216 so this will be much bigger than the whole thing). I also don't think it's core functionality. Especially as password strength is something with lots of different opinions about it. Length is the only important issue but even that should be down to the site owner to advise on.
I'd recommend moving the password strength meter into contrib for people who really need it. The password-confirm box is a good idea but could probably be handled by two password fields and validation to check they're the same without having to define a new field.
Happy for the new field itself as it's a minor addition, however it should be defined like the rest of the extra fields with the registerWidget function documented over at: https://github.com/CityWebConsultants/Iris/wiki/Form-system#extending-json-form-with-custom-fields rather than placed in clientforms.js . This way it's extendable, overwritable and also, eventually once we build in that functionality, will only be used if needed.
For the client side validation system you've put in, are we using JSONForm's own form validation system for this? https://github.com/joshfire/jsonform/wiki#validation there are events before the form goes to the submit handler and others. Might be worth looking into as we should stick to JSONForm's documented system as much as we can.
Response:
I think this is absolutely important if the system is going to be taken seriously. Security is such a hot topic that we have to do everything possible to make this a robust system. If sites keep getting hacked because of silly passwords then it will reflect bad on the system even if it is people being silly.
After removing all the unnecessary password-score files, its now only ~2MB. And these will only be loaded when a 'password-confirm' widget is loaded on a form.
It's also been added as a proper widget so it can be altered if required