[node-patches] Change in ovirt-node[master]: ui: Add security check to ConfirmedEntry

fabiand at fedoraproject.org fabiand at fedoraproject.org
Tue Aug 13 14:23:33 UTC 2013


Fabian Deutsch has uploaded a new change for review.

Change subject: ui: Add security check to ConfirmedEntry
......................................................................

ui: Add security check to ConfirmedEntry

Previously it was cumbersome to get the effects of incorrect/unsave
passwords on the UI straight. By pulling the password check code into
the ConfirmedEntry widget and adding a Notice field, the whole process
is simpliefied from a "user" perspektive (plugin writer).

Change-Id: I302ec06290d6c1dd26c7b4e8b3c13727ec8b1131
Signed-off-by: Fabian Deutsch <fabiand at fedoraproject.org>
---
M src/ovirt/node/ui/__init__.py
M src/ovirt/node/utils/security.py
2 files changed, 44 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-node refs/changes/28/18028/1

diff --git a/src/ovirt/node/ui/__init__.py b/src/ovirt/node/ui/__init__.py
index 3e7bdd3..498079d 100644
--- a/src/ovirt/node/ui/__init__.py
+++ b/src/ovirt/node/ui/__init__.py
@@ -19,7 +19,7 @@
 # MA  02110-1301, USA.  A copy of the GNU General Public License is
 # also available at http://www.gnu.org/copyleft/gpl.html.
 from ovirt.node import base
-from ovirt.node.utils import console
+from ovirt.node.utils import console, security
 
 """
 This contains abstract UI Elements
@@ -307,23 +307,27 @@
     """A container for elements which must be identical"""
 
     on_change = None
+    on_password_security_change = None
 
     _primary = None
     _secondary = None
 
     _changes = None
 
-    def __init__(self, path, label, is_password=False):
-        children = []
+    min_length = 0
+
+    def __init__(self, path, label, is_password=False, min_length=0):
         self.on_change = self.new_signal()
+        self.on_password_security_change = self.new_signal()
+        self._changes = {}
+
+        children = []
         entry_class = PasswordEntry if is_password else Entry
 
         self._primary = entry_class("%s[0]" % path,
                                     label)
         self._secondary = entry_class("%s[1]" % path,
                                       "Confirm %s" % label)
-
-        self._changes = {}
 
         for child in [self._primary, self._secondary]:
             self._changes[child.path] = ""
@@ -333,6 +337,29 @@
             children.append(child)
 
         self.on_change.connect(ChangeAction())
+
+        if is_password:
+            # If it's a password then also do checks!
+            # This work by adding a function which does the pw check
+            # and calls a specific signal (on_password_security_change)
+            self.min_length = min_length
+
+            self._notice = Notice("%s.notice" % path, "")
+            children.append(self._notice)
+
+            def do_security_check(target, changes):
+                pw, pwc = target.values()
+                msg = ""
+                is_secure = False
+                try:
+                    msg = security.password_check(pw, pwc,
+                                                  min_length=self.min_length)
+                    is_secure = True
+                except ValueError as e:
+                    msg = e.message
+                self._notice.text(msg)
+                self.on_password_security_change(is_secure)
+            self.on_change.connect(do_security_check)
 
         super(ConfirmedEntry, self).__init__(path, children)
 
@@ -346,6 +373,9 @@
             self.valid(False)
         self.on_change({self.path: self.value()})
 
+    def values(self):
+        return [v for _, v in sorted(self._changes.items())]
+
     def value(self, new_value=None):
         if new_value is not None:
             pass
diff --git a/src/ovirt/node/utils/security.py b/src/ovirt/node/utils/security.py
index f4e590e..19f3b66 100644
--- a/src/ovirt/node/utils/security.py
+++ b/src/ovirt/node/utils/security.py
@@ -37,8 +37,12 @@
     Returns:
         A message about a possibly weak password
 
-    >>> password_check("", "") is None
+    >>> password_check("", "", min_length=0) is None
     True
+
+    >>> password_check("", "")
+    Traceback (most recent call last):
+    ValueError: Password must be at least 1 characters
 
     >>> msg = password_check("foo", "foo")
     >>> "You have provided a weak password" in msg
@@ -58,7 +62,7 @@
     '''
     message = None
 
-    if len(password) is 0 and min_length is not 0:
+    if len(password) is 0 and min_length is 0:
         pass
     elif len(password) < min_length:
         raise ValueError("Password must be at least %d characters" %
@@ -70,12 +74,13 @@
     else:
         try:
             cracklib.FascistCheck(password)
-        except ValueError:
+        except ValueError as e:
             message = "You have provided a weak password!\n"
             message += "Strong passwords contain a mix of uppercase,\n"
             message += "lowercase, numeric and punctuation characters.\n"
             message += "They are six or more characters long and\n"
-            message += "do not contain dictionary words"
+            message += "do not contain dictionary words. \n"
+            message += "Reason: %s" % e
 
     return message
 


-- 
To view, visit http://gerrit.ovirt.org/18028
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I302ec06290d6c1dd26c7b4e8b3c13727ec8b1131
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-node
Gerrit-Branch: master
Gerrit-Owner: Fabian Deutsch <fabiand at fedoraproject.org>



More information about the node-patches mailing list