Project

General

Profile

Actions

feature request #9862

closed

centralized password policy enforcement validator

Added by Andreas Kohlbecker over 2 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
New
Category:
cdmlib
Target version:
Start date:
Due date:
% Done:

100%

Estimated time:
Severity:
normal
Tags:

Description

Currently rules to validate the security of passwords are only implemented in the TaxEditor (eu.etaxonomy.taxeditor.ui.password.PasswordWizardPage.PasswordValidator).

For consistent password security rules a class to validate the password security should be implemented centrally in the cdmlib.

Passay is an often used password policy enforcement library for java


Files

clipboard-202201211646-6i1sw.png (28.9 KB) clipboard-202201211646-6i1sw.png Andreas Kohlbecker, 01/21/2022 04:46 PM
clipboard-202201211648-goxaw.png (22.4 KB) clipboard-202201211648-goxaw.png Andreas Kohlbecker, 01/21/2022 04:48 PM

Related issues

Related to EDIT - feature request #9859: Password Recovery UI ClosedAndreas Kohlbecker

Actions
Actions #1

Updated by Andreas Kohlbecker over 2 years ago

Actions #2

Updated by Andreas Kohlbecker over 2 years ago

  • Description updated (diff)
Actions #3

Updated by Andreas Kohlbecker over 2 years ago

  • Status changed from New to Resolved
  • % Done changed from 0 to 50
Actions #4

Updated by Andreas Kohlbecker over 2 years ago

the 4b4dfff4 also contains a @ValidPassword annotation which can be used at model classes

please review

Actions #5

Updated by Andreas Kohlbecker over 2 years ago

  • Assignee changed from Andreas Kohlbecker to Andreas Müller
Actions #6

Updated by Andreas Müller over 2 years ago

  • Description updated (diff)
Actions #7

Updated by Andreas Müller over 2 years ago

  • Status changed from Resolved to Feedback
  • Assignee changed from Andreas Müller to Andreas Kohlbecker
  • % Done changed from 50 to 80

Generally it works. Some minor issues:

  • The validator class should always named like {ValidationAnnotationName}Validator, I did rename it to ValidPasswordValidator
  • I added null check as PasswordData throws exception for null value
  • I added max length as the password field in the DB only accepts 256 characters (and also the error message does not not look nice if saying '... should be between 8 and 210000000 or so)
  • tests were still missing, I added some first tests

Also validation did not yet use hibernate bean validation at all yet. This is necessary for Level1 validation as existing data may create errors otherwise. But we can use Level2 validation already and in future switch to Level1 validation. I added the according annotation to User.password field.

Note: Generally vaadin still seems to workaround the integrated hibernate bean validation by using the public static method in the validator. In future we may want to integrate bean validation deeper into vaadin UIs. For now this is perfectly ok as it allows fast implementation but code is still at the correct place in model.validation.

Please review if you agree with my minor changes.

Actions #8

Updated by Andreas Müller over 2 years ago

One more issue. I wonder if we really need to integrate a full new library for this function. There seem to be simple regex solutions which are more lightweight like here: https://stackoverflow.com/questions/10877668/password-validation-using-hibernate-bean-validation

Actions #9

Updated by Andreas Kohlbecker over 2 years ago

Andreas Müller wrote in #note-8:

One more issue. I wonder if we really need to integrate a full new library for this function. There seem to be simple regex solutions which are more lightweight like here: https://stackoverflow.com/questions/10877668/password-validation-using-hibernate-bean-validation

Password rules can be fairly complex, so that it is not always easily understandable why a password has been rejected. This library helps a lot to identify what is wrong:

passowrd = "aaa"

password = "1aaaaaaaa"

"1aaaaaaaaA" is still not a good password, so it would make sense to add rules for variation, or entropy. This is also supported by this library and nothing you can achieve with regexes.

BTW: I will do the review of your implementation next week.

Actions #10

Updated by Andreas Müller over 2 years ago

Yes I agree, it is probably better although for possible future developments to have a more sophisticated library at hand.

Actions #11

Updated by Andreas Kohlbecker over 2 years ago

  • Status changed from Resolved to Closed
  • % Done changed from 80 to 100

I agree with your modifications and all open questions are solved, so we can close this ticket now

Actions #12

Updated by Andreas Müller over 2 years ago

  • Target version changed from Release 5.51 to Release 5.29
Actions

Also available in: Atom PDF