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

Vratislav Podzimek vpodzime at redhat.com
Thu Oct 3 14:06:29 UTC 2013


On Thu, 2013-10-03 at 10:03 -0400, David Shea wrote:
> 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.
I think there should be something like CHECK_OK set to None and the same
for True. All checks use the same return values (None/True/error_msg),
right?

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list