[PATCH 2/3] Fix issues in password validation. (#1014405)

David Shea dshea at redhat.com
Thu Oct 3 14:03:28 UTC 2013


On 10/03/2013 04:04 AM, Vratislav Podzimek wrote:
> On Wed, 2013-10-02 at 17:22 -0400, David Shea wrote:
>> Redefined validatePassword so that it returns validity, strength and a
>> reason all at once, removing the need to call pwquality more than once
>> for a particular password.
>>
>> Convert the password checking in the root password spoke to use
>> GUICheck, similar to the checks in the user spoke.
>>
>> Fix the handling of messages from libpwquality.
>>
>> Fix the handling of the case where a user password is set by kickstart
>> and overriden by the user.
>>
>> Moved several of the password error strings into constants.
>>
>> This commit also includes some changes that glade automatically made to
>> the GtkLevelBar properties. The behavior of the LevelBars is still the
>> same: some of the removed properties were duplicates, some of them were
>> the default values, and some were not using values in a form that
>> gladeui expects. The interface-requires change is because GtkLevelBar
>> was added in Gtk 3.6.
> I believe it would be quite hard to split this big patch into multiple
> shorter ones, but try to do that next time. It makes searching for
> things in the git history, reverting, review etc. much easier.
Yeah, I guess I should do that. The unrelated glade changes at least I'd 
like in another patch. But it's so much easier to just hit save once.

>   
> -    def _checkPassword(self, editable = None, data = None):
> -        """This method updates the password indicators according
> -        to the passwords entered by the user. It is called by
> -        the changed Gtk event handler.
> +    def _checkPasswordEmpty(self, editable, data):
> +        """Check whether a password has been specified at all."""
> +
> +        # If the password was set by kickstart, skip this check
> +        if self._kickstarted:
> +            return True
> +
> +        if not editable.get_text():
> +            if editable == self.pw:
> +                return PASSWORD_EMPTY_ERROR
> +            else:
> +                return PASSWORD_CONFIRM_ERROR_GUI
> +        else:
> +            return None
> A suggestion for some future patch -- could those checks return some
> constants? I see the same problem we had with the TUI input method here,
> those None and True says nothing about their meaning.
What do you think of having a set of string constants that the checks 
use, kind of like the PASSWORD_EMPTY_ERROR and such in this last patch, 
and a constant like PASSWORD_OK that's set to None? The underlying 
mechanism where whatever's returned becomes an error message could stay 
the same, but the return values would be a little less confusing.

>
>> +        pwstrenth = self.pw_bar.get_value()
> I believe this should be pwstrength.
>
You are correct. At least it's consistently misspelled.


More information about the anaconda-patches mailing list