Project

General

Profile

Actions

Bug #7518

closed

Wt::Auth::PasswordStrengthValidator does not respect setMandatory

Added by Adrian Guzowski almost 4 years ago. Updated over 3 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
Start date:
04/16/2020
Due date:
% Done:

0%

Estimated time:

Description

Given a following sample:

auto v { std::make_shared<Wt::Auth::PasswordStrengthValidator>() };
v->setMandatory(false);

auto result { v->validate(Wt::WString::Empty) };

result is always InvalidEmpty.

Consider a case, when you i.e have a form of changing a password, but it is optional until you type it - with current validator state it is impossible to do so, or I don't see a proper way to do it.

Actions #1

Updated by Roel Standaert almost 4 years ago

Do you mean that the result is just "Invalid"? I don't see a situation where it returns InvalidEmpty, but if you can also reproduce that that would be helpful.

It would be better if:

  1. It returns InvalidEmpty if mandatory and empty (and mandatory should be the default)
  2. It returns Valid if not mandatory and empty or if the password is strong enough
  3. It returns Invalid if not empty but the password is not strong enough
Actions #2

Updated by Adrian Guzowski almost 4 years ago

Roel Standaert wrote:

Do you mean that the result is just "Invalid"? I don't see a situation where it returns InvalidEmpty, but if you can also reproduce that that would be helpful.

It would be better if:

# It returns InvalidEmpty if mandatory and empty (and mandatory should be the default)

# It returns Valid if not mandatory and empty or if the password is strong enough

# It returns Invalid if not empty but the password is not strong enough

Sorry, I assumed it returns InvalidEmpty since value is actually empty. But the issue remains, as it does not pass validation when both input is empty and validator is explicitly set to not be mandatory.

For the time being, I created a simple wrapper to workaround it with following implementation:

    class OptionalPasswordStrengthValidator
        : public Wt::Auth::PasswordStrengthValidator
    {
    public:
        OptionalPasswordStrengthValidator()
            : Wt::Auth::PasswordStrengthValidator()
        {
            setMandatory(false);
        }

        Wt::WValidator::Result validate(const Wt::WString& password, const Wt::WString& loginName, const std::string& email) const override
        {
            if (!isMandatory() && password.empty())
                return Wt::WValidator::Result(Wt::ValidationState::Valid);

            return Wt::Auth::PasswordStrengthValidator::validate(password, loginName, email);
        }
    };

While it works, it doesn't feel 100% right to return a valid status. Nonetheless this is the most straightforward solution.

For the fix, I'd just set Wt::PasswordStrengthValidator to be always mandatory, perform the check in "validate" method whether input is empty and validator is mandatory. It won't change default behaviour, but will allow to make it truly optional.

Actions #3

Updated by Adrian Guzowski almost 4 years ago

Clarification: "to be always mandatory" -> "to be mandatory by default"

Actions #4

Updated by Roel Standaert almost 4 years ago

  • Status changed from New to Resolved

I just pushed a commit that should fix this.

Actions #5

Updated by Roel Standaert almost 4 years ago

While it works, it doesn't feel 100% right to return a valid status. Nonetheless this is the most straightforward solution.

I believe that is the way our validators in general currently work, though: they always do the validation, and will return valid if left empty and not mandatory.

Actions #6

Updated by Adrian Guzowski almost 4 years ago

Adrian Guzowski wrote:

Clarification: "to be always mandatory" -> "to be mandatory by default"

Thanks!

Roel Standaert wrote:

> While it works, it doesn't feel 100% right to return a valid status. Nonetheless this is the most straightforward solution.

I believe that is the way our validators in general currently work, though: they always do the validation, and will return valid if left empty and not mandatory.

Yeah, seems so, but that may be misleading at first, until you get the inner workings of the framework. Anyway, as long as it works... ;)

Actions #7

Updated by Adrian Guzowski almost 4 years ago

Wrong quote! Too bad comment edits are not available.

Actions #8

Updated by Roel Standaert over 3 years ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF