[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