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

David Shea dshea at redhat.com
Wed Oct 2 21:22:35 UTC 2013


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.
---
 pyanaconda/constants.py                 |  12 ++
 pyanaconda/ui/gui/spokes/password.glade |  16 +--
 pyanaconda/ui/gui/spokes/password.py    | 227 ++++++++++++++++++--------------
 pyanaconda/ui/gui/spokes/user.glade     |  17 +--
 pyanaconda/ui/gui/spokes/user.py        | 109 +++++++--------
 pyanaconda/ui/tui/spokes/__init__.py    |  38 +++---
 pyanaconda/users.py                     |  85 +++++++-----
 7 files changed, 280 insertions(+), 224 deletions(-)

diff --git a/pyanaconda/constants.py b/pyanaconda/constants.py
index 0d1956b..13440e2 100644
--- a/pyanaconda/constants.py
+++ b/pyanaconda/constants.py
@@ -130,3 +130,15 @@ FIRSTBOOT_ENVIRON = "firstboot"
 
 # Tainted hardware
 UNSUPPORTED_HW = 1 << 28
+
+# Password validation
+PASSWORD_MIN_LEN = 6
+PASSWORD_EMPTY_ERROR = _("The password is empty.")
+PASSWORD_CONFIRM_ERROR_GUI = _("The passwords do not match.")
+PASSWORD_CONFIRM_ERROR_TUI = _("The passwords you entered were different.  Please try again.")
+PASSWORD_WEAK = _("The password you have provided is weak. You will have to press Done twice to confirm it.")
+PASSWORD_WEAK_WITH_ERROR = _("The password you have provided is weak: %s. You will have to press Done twice to confirm it.")
+PASSWORD_WEAK_CONFIRM = _("You have provided a weak password. Press Done again to use anyway.")
+PASSWORD_WEAK_CONFIRM_WITH_ERROR = _("You have provided a weak password: %s. Press Done again to use anyway.")
+
+PASSWORD_STRENGTH_DESC = [_("Empty"), _("Weak"), _("Fair"), _("Good"), _("Strong")]
diff --git a/pyanaconda/ui/gui/spokes/password.glade b/pyanaconda/ui/gui/spokes/password.glade
index 37942eb..b57b7e4 100644
--- a/pyanaconda/ui/gui/spokes/password.glade
+++ b/pyanaconda/ui/gui/spokes/password.glade
@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <interface>
-  <!-- interface-requires gtk+ 3.0 -->
+  <!-- interface-requires gtk+ 3.6 -->
   <!-- interface-requires AnacondaWidgets 1.0 -->
   <object class="AnacondaSpokeWindow" id="passwordWindow">
     <property name="startup_id">filler</property>
@@ -87,7 +87,7 @@
                         <property name="can_focus">True</property>
                         <property name="visibility">False</property>
                         <property name="invisible_char">●</property>
-                        <signal name="changed" handler="_checkPassword" swapped="no"/>
+                        <signal name="changed" handler="_updatePwQuality" swapped="no"/>
                       </object>
                       <packing>
                         <property name="left_attach">1</property>
@@ -103,7 +103,6 @@
                         <property name="visibility">False</property>
                         <property name="invisible_char">●</property>
                         <property name="activates_default">True</property>
-                        <signal name="changed" handler="_checkPassword" swapped="no"/>
                       </object>
                       <packing>
                         <property name="left_attach">1</property>
@@ -134,15 +133,10 @@
                           <object class="GtkLevelBar" id="password_bar">
                             <property name="visible">True</property>
                             <property name="can_focus">False</property>
-                            <property name="orientation">vertical</property>
-                            <property name="spacing">2</property>
-                            <property name="mode">GTK_LEVEL_BAR_MODE_DISCRETE</property>
-                            <property name="min-value">0</property>
-                            <property name="max-value">4</property>
-                            <property name="orientation">GTK_ORIENTATION_HORIZONTAL</property>
-                            <property name="value">2</property>
-                            <property name="halign">fill</property>
                             <property name="valign">center</property>
+                            <property name="value">2</property>
+                            <property name="max_value">4</property>
+                            <property name="mode">discrete</property>
                           </object>
                           <packing>
                             <property name="expand">True</property>
diff --git a/pyanaconda/ui/gui/spokes/password.py b/pyanaconda/ui/gui/spokes/password.py
index b828317..c64df56 100644
--- a/pyanaconda/ui/gui/spokes/password.py
+++ b/pyanaconda/ui/gui/spokes/password.py
@@ -20,13 +20,16 @@
 #
 
 from pyanaconda.i18n import _, N_
-from pyanaconda.users import cryptPassword, validatePassword, checkPassword
-from pwquality import PWQError
+from pyanaconda.users import cryptPassword, validatePassword
 
 from pyanaconda.ui.gui.spokes import NormalSpoke
 from pyanaconda.ui.gui.categories.user_settings import UserSettingsCategory
 from pyanaconda.ui.common import FirstbootSpokeMixIn
 
+from pyanaconda.constants import PASSWORD_EMPTY_ERROR, PASSWORD_CONFIRM_ERROR_GUI,\
+        PASSWORD_STRENGTH_DESC, PASSWORD_WEAK, PASSWORD_WEAK_WITH_ERROR,\
+        PASSWORD_WEAK_CONFIRM, PASSWORD_WEAK_CONFIRM_WITH_ERROR
+
 __all__ = ["PasswordSpoke"]
 
 
@@ -43,9 +46,6 @@ class PasswordSpoke(FirstbootSpokeMixIn, NormalSpoke):
 
     def __init__(self, *args):
         NormalSpoke.__init__(self, *args)
-        self._password = None
-        self._error = False
-        self._oldweak = None
         self._kickstarted = False
 
     def initialize(self):
@@ -54,6 +54,35 @@ class PasswordSpoke(FirstbootSpokeMixIn, NormalSpoke):
         self.pw = self.builder.get_object("pw")
         self.confirm = self.builder.get_object("confirm")
 
+        # Install the password checks:
+        # - Has a password been specified?
+        # - If a password has been specified and there is data in the confirm box, do they match?
+        # - How strong is the password?
+        # - Is there any data in the confirm box?
+        self.add_check(self.pw, self._checkPasswordEmpty)
+
+        # The password confirmation needs to be checked whenever either of the password
+        # fields change. Separate checks are created for each field so that edits on either
+        # will trigger a new check and so that the last edited field will get focus when
+        # Done is clicked. The checks are saved here so that either check can trigger the
+        # other check in order to reset the status on both when either field is changed.
+        # The check_data field is used as a flag to prevent infinite recursion.
+        self._confirm_check = self.add_check(self.confirm, self._checkPasswordConfirm)
+        self._password_check = self.add_check(self.pw, self._checkPasswordConfirm)
+
+        # Keep a reference for this check, since it has to be manually run for the
+        # click Done twice check.
+        self._pwStrengthCheck = self.add_check(self.pw, self._checkPasswordStrength)
+
+        self.add_check(self.confirm, self._checkPasswordEmpty)
+
+        # Counter for the click Done twice check override
+        self._waivePasswordClicks = 0
+
+        # Password validation data
+        self._pwq_error = None
+        self._pwq_valid = True
+
         self._kickstarted = self.data.rootpw.seen
         if self._kickstarted:
             self.pw.set_placeholder_text(_("The password is set."))
@@ -69,13 +98,11 @@ class PasswordSpoke(FirstbootSpokeMixIn, NormalSpoke):
 
     def refresh(self):
         self.pw.grab_focus()
-        self._checkPassword()
+        self.pw.emit("changed")
 
     @property
     def status(self):
-        if self._error:
-            return _("Error setting root password")
-        elif self.data.rootpw.password:
+        if self.data.rootpw.password:
             return _("Root password is set")
         elif self.data.rootpw.lock:
             return _("Root account is disabled")
@@ -88,10 +115,11 @@ class PasswordSpoke(FirstbootSpokeMixIn, NormalSpoke):
                             if "wheel" in user.groups)
 
     def apply(self):
-        if self._password is None and self._kickstarted:
+        pw = self.pw.get_text()
+        if (not pw) and (self._kickstarted):
             return
 
-        self.data.rootpw.password = cryptPassword(self._password)
+        self.data.rootpw.password = cryptPassword(self.pw.get_text())
         self.data.rootpw.isCrypted = True
         self.data.rootpw.lock = False
 
@@ -106,108 +134,115 @@ class PasswordSpoke(FirstbootSpokeMixIn, NormalSpoke):
     def completed(self):
         return bool(self.data.rootpw.password or self.data.rootpw.lock)
 
-    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
+
+    def _checkPasswordConfirm(self, editable=None, reset_status=None):
+        """Check whether the password matches the confirmation data."""
+
+        # This check is triggered by changes to either the password field or the
+        # confirmation field. If this method is being run from a successful check
+        # to reset the status, just return success
+        if reset_status:
+            return None
+
+        # Skip the check if no password is required
+        if self._kickstarted:
+            result = None
+        elif self.confirm.get_text() and (self.pw.get_text() != self.confirm.get_text()):
+            result = PASSWORD_CONFIRM_ERROR_GUI
+        else:
+            result = None
+
+        # If the check succeeded, reset the status of the other check object
+        if result is None:
+            if editable == self.confirm:
+                self._password_check.update_check_status(check_data=True)
+            else:
+                self._confirm_check.update_check_status(check_data=True)
+
+        return result
+
+    def _updatePwQuality(self, editable=None, data=None):
+        """Update the password quality information.
+
+           This function is called by the ::changed signal handler on the
+           password field.
         """
-        try:
-            strength = checkPassword(self.pw.get_text())
-            _pwq_error = None
-        except PWQError as e:
-            _pwq_error = e.message
-            strength = 0
-
-        if strength < 50:
+
+        pwtext = self.pw.get_text()
+
+        # Reset the counter used for the "press Done twice" logic
+        self._waivePasswordClicks = 0
+
+        self._pwq_valid, strength, self._pwq_error = validatePassword(pwtext, "root")
+
+        if not pwtext:
+            val = 0
+        elif strength < 50:
             val = 1
-            text = _("Weak")
-            self._error = _("The password you have provided is weak")
-            if _pwq_error:
-                self._error += ": %s. " % _pwq_error
-            else:
-                self._error += ". "
-            self._error += _("You will have to press Done twice to confirm it.")
         elif strength < 75:
             val = 2
-            text = _("Fair")
-            self._error = False
         elif strength < 90:
             val = 3
-            text = _("Good")
-            self._error = False
         else:
             val = 4
-            text = _("Strong")
-            self._error = False
-
-        if not self.pw.get_text():
-            val = 0
-            text = _("Empty")
-            self._error = _("The password is empty.")
-        elif self.confirm.get_text() and self.pw.get_text() != self.confirm.get_text():
-            self._error = _("The passwords do not match.")
+        text = PASSWORD_STRENGTH_DESC[val]
 
         self.pw_bar.set_value(val)
         self.pw_label.set_text(text)
 
-        self.clear_info()
-        if self._error:
-            self.set_warning(self._error)
-            self.window.show_all()
-            return False
+    def _checkPasswordStrength(self, editable=None, data=None):
+        """Update the error message based on password strength.
 
-        return True
+           Convert the strength set by _updatePwQuality into an error message.
+        """
 
-    def _validatePassword(self):
-        # Do various steps to validate the password
-        # sets self._error to an error string
-        # Return True if valid, False otherwise
-        self._error = False
         pw = self.pw.get_text()
         confirm = self.confirm.get_text()
 
-        if not pw and not confirm:
-            if self._kickstarted:
-                return True
+        # Skip the check if no password is required
+        if (not pw and not confirm) and self._kickstarted:
+            return None
+
+        # Check for validity errors
+        if (not self._pwq_valid) and (self._pwq_error):
+            return self._pwq_error
+
+        pwstrenth = self.pw_bar.get_value()
+
+        if pwstrenth < 2:
+            # If done has been clicked twice, waive the check
+            if self._waivePasswordClicks > 1:
+                return None
+            elif self._waivePasswordClicks == 1:
+                if self._pwq_error:
+                    return PASSWORD_WEAK_CONFIRM_WITH_ERROR % self._pwq_error
+                else:
+                    return PASSWORD_WEAK_CONFIRM
             else:
-                self._error = _("You must provide and confirm a password.")
-                return False
-
-        try:
-            self._error = validatePassword(pw, confirm)
-        except PWQError as e:
-            if pw == self._oldweak:
-                # We got a second attempt with the same weak password
-                pass
-            else:
-                self._error = _("You have provided a weak password: %s. "
-                                " Press Done again to use anyway.") % e.message
-                self._oldweak = pw
-                return False
-
-        if self._error:
-            return False
-
-        # the self._checkPassword function is used to indicate the password
-        # strength and need of hitting the Done button twice so use it here as
-        # well
-        if not self._checkPassword() and pw != self._oldweak:
-            # check failed and the Done button was clicked for the first time
-            self._oldweak = pw
-            return False
-
-        # if no errors, clear the info for next time we go into the spoke
-        self._password = pw
-        self.clear_info()
-        self._error = False
-        return True
+                if self._pwq_error:
+                    return PASSWORD_WEAK_WITH_ERROR % self._pwq_error
+                else:
+                    return PASSWORD_WEAK
+        else:
+            return None
 
     def on_back_clicked(self, button):
-        if self._validatePassword():
-            self.clear_info()
-            NormalSpoke.on_back_clicked(self, button)
-        else:
-            self.clear_info()
-            self.set_warning(self._error)
-            self.pw.grab_focus()
-            self.window.show_all()
+        # Add a click and re-check the password strength
+        self._waivePasswordClicks += 1
+        self._pwStrengthCheck.update_check_status()
+
+        NormalSpoke.on_back_clicked(self, button)
diff --git a/pyanaconda/ui/gui/spokes/user.glade b/pyanaconda/ui/gui/spokes/user.glade
index 377f9f2..d2f701f 100644
--- a/pyanaconda/ui/gui/spokes/user.glade
+++ b/pyanaconda/ui/gui/spokes/user.glade
@@ -102,7 +102,7 @@
                         <property name="invisible_char">●</property>
                         <property name="invisible_char_set">True</property>
                         <property name="caps_lock_warning">False</property>
-                        <signal name="changed" handler="_guessNames" swapped="no"/>
+                        <signal name="changed" handler="full_name_changed" swapped="no"/>
                       </object>
                       <packing>
                         <property name="left_attach">1</property>
@@ -117,7 +117,7 @@
                         <property name="can_focus">True</property>
                         <property name="invisible_char">●</property>
                         <property name="invisible_char_set">True</property>
-                        <signal name="changed" handler="_guessNameDisabler" swapped="no"/>
+                        <signal name="changed" handler="username_changed" swapped="no"/>
                       </object>
                       <packing>
                         <property name="left_attach">1</property>
@@ -172,7 +172,7 @@
                         <property name="can_focus">True</property>
                         <property name="visibility">False</property>
                         <property name="invisible_char">●</property>
-                        <signal name="changed" handler="_updatePwQuality" swapped="no"/>
+                        <signal name="changed" handler="password_changed" swapped="no"/>
                       </object>
                       <packing>
                         <property name="left_attach">1</property>
@@ -219,7 +219,7 @@
                         <property name="xalign">0</property>
                         <property name="active">True</property>
                         <property name="draw_indicator">True</property>
-                        <signal name="toggled" handler="_passwordDisabler" swapped="no"/>
+                        <signal name="toggled" handler="usepassword_toggled" swapped="no"/>
                       </object>
                       <packing>
                         <property name="left_attach">1</property>
@@ -236,13 +236,10 @@
                           <object class="GtkLevelBar" id="password_bar">
                             <property name="visible">True</property>
                             <property name="can_focus">False</property>
-                            <property name="mode">GTK_LEVEL_BAR_MODE_DISCRETE</property>
-                            <property name="min-value">0</property>
-                            <property name="max-value">4</property>
-                            <property name="orientation">GTK_ORIENTATION_HORIZONTAL</property>
-                            <property name="value">2</property>
-                            <property name="halign">fill</property>
                             <property name="valign">center</property>
+                            <property name="value">2</property>
+                            <property name="max_value">4</property>
+                            <property name="mode">discrete</property>
                           </object>
                           <packing>
                             <property name="expand">True</property>
diff --git a/pyanaconda/ui/gui/spokes/user.py b/pyanaconda/ui/gui/spokes/user.py
index aca7d95..eb4c2e0 100644
--- a/pyanaconda/ui/gui/spokes/user.py
+++ b/pyanaconda/ui/gui/spokes/user.py
@@ -29,11 +29,12 @@ from pyanaconda.ui.common import FirstbootSpokeMixIn
 from pyanaconda.ui.gui.utils import enlightbox
 
 from pykickstart.constants import FIRSTBOOT_RECONFIG
-from pyanaconda.constants import ANACONDA_ENVIRON, FIRSTBOOT_ENVIRON
+from pyanaconda.constants import ANACONDA_ENVIRON, FIRSTBOOT_ENVIRON,\
+        PASSWORD_EMPTY_ERROR, PASSWORD_CONFIRM_ERROR_GUI, PASSWORD_STRENGTH_DESC,\
+        PASSWORD_WEAK, PASSWORD_WEAK_WITH_ERROR, PASSWORD_WEAK_CONFIRM,\
+        PASSWORD_WEAK_CONFIRM_WITH_ERROR
 from pyanaconda.regexes import GECOS_VALID, USERNAME_VALID, GROUPNAME_VALID, GROUPLIST_FANCY_PARSE
 
-import pwquality
-
 __all__ = ["UserSpoke", "AdvancedUserDialog"]
 
 def _checkUsername(editable, data):
@@ -220,7 +221,6 @@ class UserSpoke(FirstbootSpokeMixIn, NormalSpoke):
     def __init__(self, *args):
         NormalSpoke.__init__(self, *args)
         self._oldweak = None
-        self._error = False
 
     def initialize(self):
         NormalSpoke.initialize(self)
@@ -248,13 +248,10 @@ class UserSpoke(FirstbootSpokeMixIn, NormalSpoke):
             self.username: True
             }
 
-        # set up passphrase quality checker
-        self._pwq = pwquality.PWQSettings()
-        self._pwq.read_config()
-
         # Updated during the password changed event and used by the password
         # field validity checker
         self._pwq_error = None
+        self._pwq_valid = True
 
         self.pw_bar = self.builder.get_object("password_bar")
         self.pw_label = self.builder.get_object("password_label")
@@ -281,7 +278,7 @@ class UserSpoke(FirstbootSpokeMixIn, NormalSpoke):
         # - if a password is specified and there is data in the confirm box, do they match?
         # - if a password is specified and the confirm box is empty or match, how strong is it?
         # - if a password is required, is there any data in the confirm box?
-        self.add_check(self.pw, self._checkPasswordEmpty, None)
+        self.add_check(self.pw, self._checkPasswordEmpty)
         
         # The password confirmation needs to be checked whenever either of the password
         # fields change. Separate checks are created on each field so that edits on
@@ -289,14 +286,14 @@ class UserSpoke(FirstbootSpokeMixIn, NormalSpoke):
         # when Done is clicked. Whichever check is run needs to run the other check in
         # order to reset the status. The check_data field is used as a flag to prevent
         # infinite recursion.
-        self._confirm_check = self.add_check(self.confirm, self._checkPasswordConfirm, None)
-        self._password_check = self.add_check(self.pw, self._checkPasswordConfirm, None)
+        self._confirm_check = self.add_check(self.confirm, self._checkPasswordConfirm)
+        self._password_check = self.add_check(self.pw, self._checkPasswordConfirm)
 
         # Keep a reference to this check, since it has to be manually run for the
         # click Done twice check.
-        self._pwStrengthCheck = self.add_check(self.pw, self._checkPasswordStrength, None)
+        self._pwStrengthCheck = self.add_check(self.pw, self._checkPasswordStrength)
 
-        self.add_check(self.confirm, self._checkPasswordEmpty, None)
+        self.add_check(self.confirm, self._checkPasswordEmpty)
 
         # Allow empty usernames so the spoke can be exited without creating a user
         self.add_check(self.username, _checkUsername,
@@ -332,9 +329,7 @@ class UserSpoke(FirstbootSpokeMixIn, NormalSpoke):
 
     @property
     def status(self):
-        if self._error:
-            return _("Error creating user account: %s") % self._error
-        elif len(self.data.user.userList) == 0:
+        if len(self.data.user.userList) == 0:
             return _("No user will be created")
         elif self._wheel.name in self.data.user.userList[0].groups:
             return _("Administrator %s will be created") % self.data.user.userList[0].name
@@ -400,52 +395,48 @@ class UserSpoke(FirstbootSpokeMixIn, NormalSpoke):
     def completed(self):
         return len(self.data.user.userList) > 0
 
-    def _updatePwQuality(self, editable=None, data=None):
+    def _updatePwQuality(self):
         """This method updates the password indicators according
-        to the password entered by the user. It is called by
-        the changed Gtk event handler.
+        to the password entered by the user.
         """
         pwtext = self.pw.get_text()
+        username = self.username.get_text()
 
         # Reset the counter used for the "press Done twice" logic
         self._waivePasswordClicks = 0
 
-        try:
-            strength = self._pwq.check(pwtext, None, None)
-            self._pwq_error = None
-        except pwquality.PWQError as e:
-            self._pwq_error = e.message
-            strength = 0
+        self._pwq_valid, strength, self._pwq_error = validatePassword(pwtext, username)
 
         if not pwtext:
             val = 0
-            text = _("Empty")
         elif strength < 50:
             val = 1
-            text = _("Weak")
         elif strength < 75:
             val = 2
-            text = _("Fair")
         elif strength < 90:
             val = 3
-            text = _("Good")
         else:
             val = 4
-            text = _("Strong")
+        text = PASSWORD_STRENGTH_DESC[val]
 
         self.pw_bar.set_value(val)
         self.pw_label.set_text(text)
 
-    def _passwordDisabler(self, editable = None, data = None):
+    def usepassword_toggled(self, togglebutton=None, data=None):
         """Called by Gtk callback when the "Use password" check
         button is toggled. It will make password entries in/sensitive."""
 
         self.pw.set_sensitive(self.usepassword.get_active())
         self.confirm.set_sensitive(self.usepassword.get_active())
+
+        # Re-check the password
         self.pw.emit("changed")
-        self.confirm.emit("changed")
 
-    def _guessNameDisabler(self, editable = None, data = None):
+    def password_changed(self, editable=None, data=None):
+        """Update the password strength level bar"""
+        self._updatePwQuality()
+
+    def username_changed(self, editable=None, data=None):
         """Called by Gtk callback when the username or hostname
         entry changes. It disables the guess algorithm if the
         user added his own text there and reenable it when the
@@ -458,16 +449,22 @@ class UserSpoke(FirstbootSpokeMixIn, NormalSpoke):
             self.guesser[editable] = False
             self.b_advanced.set_sensitive(True)
 
-    def _guessNames(self, editable = None, data = None):
+            # Re-run the password checks against the new username
+            self.pw.emit("changed")
+
+    def full_name_changed(self, editable=None, data=None):
         """Called by Gtk callback when the full name field changes.
         It guesses the username and hostname, strips diacritics
         and make those lowercase.
         """
-        fullname = self.fullname.get_text()
-        username = guess_username(fullname)
 
-        # after the text is updated in guesser, the guess has to be reenabled
+        # Setting the text in the username field emits the ::changed signal,
+        # causing the guesser to be disabled. set_text emits the signal on
+        # the underlying Editable synchronously, so just re-enable the guesser
+        # after it returns.
         if self.guesser[self.username]:
+            fullname = self.fullname.get_text()
+            username = guess_username(fullname)
             self.username.set_text(username)
             self.guesser[self.username] = True
 
@@ -486,9 +483,9 @@ class UserSpoke(FirstbootSpokeMixIn, NormalSpoke):
             return None
         elif not editable.get_text():
             if editable == self.pw:
-                return _("The password is empty")
+                return PASSWORD_EMPTY_ERROR
             else:
-                return _("The passwords do not match.")
+                return PASSWORD_CONFIRM_ERROR_GUI
         else:
             return None
 
@@ -502,10 +499,11 @@ class UserSpoke(FirstbootSpokeMixIn, NormalSpoke):
             return None
         
         # Skip the check if no password is required
-        if (not self.usepassword.get_active()) or self._user.password_kickstarted:
+        if (not self.usepassword.get_active()) or \
+                ((not self.pw.get_text()) and (self._user.password_kickstarted)):
             result = None
         elif self.confirm.get_text() and (self.pw.get_text() != self.confirm.get_text()):
-            result = _("The passwords do not match.")
+            result = PASSWORD_CONFIRM_ERROR_GUI
         else:
             result = None
 
@@ -531,41 +529,34 @@ class UserSpoke(FirstbootSpokeMixIn, NormalSpoke):
          """
 
         # Skip the check if no password is required
-        if (not self.usepassword.get_active()) or self._user.password_kickstarted:
+        if (not self.usepassword.get_active()) or \
+                ((not self.pw.get_text()) and (self._user.password_kickstarted)):
             return None
 
+        # If the password fails the validity check, fail this check
+        if (not self._pwq_valid) and (self._pwq_error):
+            return self._pwq_error
+
         pwstrength = self.pw_bar.get_value()
         
-        # If the password passed the pwquality test, see if validatePassword
-        # catches anything else.
-        if pwstrength >= 2:
-            self._pwq_error = validatePassword(self.pw.get_text())
-            if self._pwq_error:
-                pwstrength = 0
-
         if pwstrength < 2:
             # If Done has been clicked twice, waive the check
             if self._waivePasswordClicks > 1:
                 return None
             elif self._waivePasswordClicks == 1:
                 if self._pwq_error:
-                    return _("You have provided a weak password: %s. "
-                            " Press Done again to use anyway.") % self._pwq_error
+                    return PASSWORD_WEAK_CONFIRM_WITH_ERROR % self._pwq_error
                 else:
-                    return _("You have provided a weak password. "
-                            " Press Done again to use anyway.")
+                    return PASSWORD_WEAK_CONFIRM
             else:
-                error = _("The password you have provided is weak")
                 if self._pwq_error:
-                    error += ": %s. " % self._pwq_error
+                    return PASSWORD_WEAK_WITH_ERROR % self._pwq_error
                 else:
-                    error += ". "
-                error += _("You will have to press Done twice to confirm it.")
-                return error
+                    return PASSWORD_WEAK
         else:
             return None
 
-    def on_advanced_clicked(self, _button):
+    def on_advanced_clicked(self, _button, user_data=None):
         """Handler for the Advanced.. button. It starts the Advanced dialog
         for setting homedit, uid, gid and groups.
         """
diff --git a/pyanaconda/ui/tui/spokes/__init__.py b/pyanaconda/ui/tui/spokes/__init__.py
index 773ea67..6db3f5b 100644
--- a/pyanaconda/ui/tui/spokes/__init__.py
+++ b/pyanaconda/ui/tui/spokes/__init__.py
@@ -21,8 +21,7 @@
 from pyanaconda.ui.tui import simpleline as tui
 from pyanaconda.ui.tui.tuiobject import TUIObject, YesNoDialog
 from pyanaconda.ui.common import Spoke, StandaloneSpoke, NormalSpoke, PersonalizationSpoke, collect
-from pyanaconda.users import validatePassword, checkPassword
-from pwquality import PWQError
+from pyanaconda.users import validatePassword
 import re
 from collections import namedtuple
 from pyanaconda.iutil import setdeepattr, getdeepattr
@@ -110,20 +109,27 @@ class EditTUIDialog(NormalTUISpoke):
         if entry.aux == self.PASSWORD:
             pw = self._app.raw_input(_("%s: ") % entry.title, hidden=True)
             confirm = self._app.raw_input(_("%s (confirm): ") % entry.title, hidden=True)
-            error = None
-            # just returning an error is either blank or mismatched
-            # passwords.  Raising is because of poor quality.
-            try:
-                error = validatePassword(pw, confirm)
-                if error:
-                    print(error)
-                    return None
-                strength = checkPassword(pw)
-                if strength < 50:
-                    raise PWQError("The password you have provided is weak.")
-            except PWQError as e:
-                error = _("You have provided a weak password: %s. " % e.message)
-                error += _("\nWould you like to use it anyway?")
+
+            if (pw and not confirm) or (confirm and not pw):
+                print(_("You must enter your root password and confirm it by typing"
+                        " it a second time to continue."))
+                return None
+            if (pw != confirm):
+                print(_("The passwords you entered were different. Please try again."))
+                return None
+
+            valid, strength, message = validatePassword(pw, user=None)
+            if not valid:
+                print(message)
+                return None
+
+            if strength < 50:
+                if message:
+                    error = _("You have provided a weak password: %s.\n"
+                              "Would you like to use it anyway?") % message
+                else:
+                    error = _("You have provided a weak password.\n"
+                              "Would you like to use it anyway?")
                 question_window = YesNoDialog(self._app, error)
                 self._app.switch_screen_modal(question_window)
                 if not question_window.answer:
diff --git a/pyanaconda/users.py b/pyanaconda/users.py
index 67baf72..335f121 100644
--- a/pyanaconda/users.py
+++ b/pyanaconda/users.py
@@ -30,6 +30,7 @@ from pyanaconda import iutil
 import pwquality
 from pyanaconda.iutil import strip_accents
 from pyanaconda.i18n import _
+from pyanaconda.constants import PASSWORD_MIN_LEN
 
 import logging
 log = logging.getLogger("anaconda")
@@ -111,47 +112,67 @@ def cryptPassword(password, algo=None):
 
     return crypt.crypt (password, saltstr)
 
-def validatePassword(pw, confirm=None, minlen=6, user="root"):
-    # Do various steps to validate the password
-    # Return an error string, or None for no errors
-    # If inital checks pass, pwquality will be tested.  Raises
-    # from pwquality will pass up to the calling code
+def validatePassword(pw, user="root", settings=None):
+    """Check the quality of a password.
 
-    # if both pw and confirm are blank, password is disabled.
-    if (pw and confirm == '') or (confirm and not pw):
-        error = _("You must enter your root password "
-                  "and confirm it by typing it a second "
-                  "time to continue.")
-        return error
+       This function does three things: given a password and an optional
+       username, it will tell if this password can be used at all, how
+       strong the password is on a scale of 1-100, and, if the password is
+       unusable, why it is unusuable.
 
-    if confirm != None and pw != confirm:
-        error = _("The passwords you entered were "
-                  "different.  Please try again.")
-        return error
+       This function uses libpwquality to check the password strength.
+       pwquality will raise a PWQError on a weak password, which, honestly,
+       is kind of dumb behavior. A weak password isn't exceptional, it's what
+       we're asking about! Anyway, this function does not raise PWQError. If
+       the password fails the PWQSettings conditions, the first member of the
+       return tuple will be False and the second member of the tuple will be 0.
+
+       :param pw: the password to check
+       :type pw: string
+
+       :param user: the username for which the password is being set. If no
+                    username is provided, "root" will be used. Use user=None
+                    to disable the username check.
+       :type user: string
+
+       :param settings: an optional PWQSettings object
+       :type settings: pwquality.PWQSettings
+
+       :returns: A tuple containing (bool(valid), int(score), str(message))
+       :rtype: tuple
+    """
+
+    valid = True
+    message = None
+    strength = 0
+
+    if settings is None:
+        # Generate a default PWQSettings once and save it as a member of this function
+        if not hasattr(validatePassword, "pwqsettings"):
+            validatePassword.pwqsettings = pwquality.PWQSettings()
+            validatePassword.pwqsettings.read_config()
+            validatePassword.pwqsettings.minlen = PASSWORD_MIN_LEN
+        settings = validatePassword.pwqsettings
 
     legal = string.digits + string.ascii_letters + string.punctuation + " "
     for letter in pw:
         if letter not in legal:
-            error = _("Requested password contains "
+            message = _("Requested password contains "
                       "non-ASCII characters, which are "
                       "not allowed.")
-            return error
-
-    if pw:
-        settings = pwquality.PWQSettings()
-        settings.read_config()
-        settings.minlen = minlen
-        settings.check(pw, None, user)
-
-    return None
+            valid = False
+            break
 
-def checkPassword(pw):
-    """ Check the quality of a password passed in and return a numeric
-        value.
-    """
-    pwq = pwquality.PWQSettings()
-    pwq.read_config()
-    return pwq.check(pw, None, None)
+    if valid:
+        try:
+            strength = settings.check(pw, None, user)
+        except pwquality.PWQError as e:
+            # Leave valid alone here: the password is weak but can still
+            # be accepted.
+            # PWQError values are built as a tuple of (int, str)
+            message = e[1]
+
+    return (valid, strength, message)
 
 def guess_username(fullname):
     fullname = fullname.split()
-- 
1.8.3.1



More information about the anaconda-patches mailing list