Merge lp:~jjacobs/methanal/verified-password-input into lp:methanal

[Jonathan Jacobs]

[Forrest Aldridge (forrest-aldridge)]

 1. The docstring in TestView.js on line 834 is an incomplete sentence and is slightly unclear.
 2. This is a matter of preference really, but I think it would be clearer if the variable c in View.js in lines 1828-1831 were renamed to 'criterion'

review: Needs Fixing

[Tristan Seligmann (mithrandi)]

1. Saving and restoring control._confirmPasswordNode seems pointless; nothing relies on this behaviour, and the first password input isn't saved and restored, so it's somewhat inconsistent.

2. In general, the pattern "throw new Error('Foo bar:' + c);" is better written as "throw new FooBar(c);"; embedding the exception "class" in text makes it challenging to catch the specific exception without doing something yucky like substring matching. In this specific case, setStrengthCriteria should probably throw UnknownStrengthCriterion or InvalidStrengthCriterion or something like that.

review: Needs Fixing

[Forrest Aldridge (forrest-aldridge)]

 1. The docstring in TestView.js on line 834 is an incomplete sentence and is slightly unclear.
 2. This is a matter of preference really, but I think it would be clearer if the variable c in View.js in lines 1828-1831 were renamed to 'criterion'

review: Needs Fixing

[Jonathan Jacobs (jjacobs)]

> 1. The docstring in TestView.js on line 834 is an incomplete sentence and is
> slightly unclear.

Oops, sorry.

> 2. This is a matter of preference really, but I think it would be clearer if
> the variable c in View.js in lines 1828-1831 were renamed to 'criterion'

It was probably renamed to reduce it's width and then something else happened that stopped that from mattering.

[Jonathan Jacobs (jjacobs)]

> 1. Saving and restoring control._confirmPasswordNode seems pointless; nothing
> relies on this behaviour, and the first password input isn't saved and
> restored, so it's somewhat inconsistent.

This behaviour has been removed, it didn't seem to matter to anything, I guess if you need a clean slate, write another test case.

> 2. In general, the pattern "throw new Error('Foo bar:' + c);" is better
> written as "throw new FooBar(c);"; embedding the exception "class" in text
> makes it challenging to catch the specific exception without doing something
> yucky like substring matching. In this specific case, setStrengthCriteria
> should probably throw UnknownStrengthCriterion or InvalidStrengthCriterion or
> something like that.

I was being lazy.

[Tristan Seligmann (mithrandi)]

review: Approve

=== modified file 'methanal/js/Methanal/Tests/TestView.js'
--- methanal/js/Methanal/Tests/TestView.js	2009-10-04 11:18:09 +0000
+++ methanal/js/Methanal/Tests/TestView.js	2009-10-09 22:42:12 +0000
@@ -168,14 +168,14 @@
      * Set the input node's value to C{value} and passes the result of
      * C{control.getValue} to C{control.baseValidator}.
      */
-    function assertValidInput(self, control, value) {
-        var oldValue = control.inputNode.value;
+    function assertValidInput(self, control, value, msg) {
         control.inputNode.value = value;
-        var msg = (Methanal.Util.repr(value) + ' is NOT valid input for ' +
-            Methanal.Util.repr(control));
+        if (msg === undefined) {
+            msg = (Methanal.Util.repr(value) + ' is NOT valid input for ' +
+                Methanal.Util.repr(control));
+        }
         self.assertIdentical(
             control.baseValidator(control.getValue()), undefined, msg);
-        control.inputNode.value = oldValue;
     },
 
 
@@ -754,6 +754,120 @@
 
 
 /**
+ * Tests for L{Methanal.View.VerifiedPasswordInput}.
+ */
+Methanal.Tests.TestView.BaseTestTextInput.subclass(Methanal.Tests.TestView, 'TestVerifiedPasswordInput').methods(
+    function setUp(self) {
+        self.controlType = Methanal.View.VerifiedPasswordInput;
+    },
+    
+
+    /**
+     * Assert that C{password}, and optionally C{confirmPassword}, are a good
+     * input.
+     */
+    function assertGoodPassword(self, control, password, confirmPassword) {
+        if (confirmPassword === undefined) {
+            confirmPassword = password;
+        }
+        control._confirmPasswordNode.value = confirmPassword;
+        self.assertValidInput(
+            control, password,
+            Methanal.Util.repr(password) + ' is NOT a good password');
+    },
+
+
+    /**
+     * Assert that C{password}, and optionally C{confirmPassword}, are a bad
+     * input.
+     */
+    function assertBadPassword(self, control, password, confirmPassword) {
+        if (confirmPassword === undefined) {
+            confirmPassword = password;
+        }
+        control._confirmPasswordNode.value = confirmPassword;
+        self.assertInvalidInput(
+            control, password,
+            Methanal.Util.repr(password) + ' IS a good password');
+    },
+
+
+    function createControl(self, args) {
+        var control = Methanal.Tests.TestView.TestVerifiedPasswordInput.upcall(
+            self, 'createControl', args);
+        Methanal.Tests.TestView.makeWidgetChildNode(
+            control, 'input', 'confirmPassword');
+        return control;
+    },
+
+
+    /**
+     * Validation will fail under the following conditions:
+     *     1. The input and confirmPasswordNode node values don't match.
+     *     2. If either of the above node values have no length (are blank).
+     */
+    function test_inputValidation(self) {
+        self.testControl({value: null},
+            function (control) {
+                // Test condition 1
+                self.assertBadPassword(control, 'match', 'no match');
+                self.assertGoodPassword(control, 'match', 'match');
+                // Test condition 2
+                self.assertBadPassword(control, '', '');
+            });
+    },
+    
+    
+    /**
+     * Changing the password strength criteria results in different validation
+     * criteria for the control.
+     */
+    function test_strengthCriteria(self) {
+        // Override the default criteria of 5 or more characters.
+        self.testControl({value: null, minPasswordLength: 3},
+            function (control) {
+                self.assertBadPassword(control, '12');
+                self.assertGoodPassword(control, '123');
+
+                control.setStrengthCriteria(['ALPHA']);
+                self.assertGoodPassword(control, 'Abc');
+                self.assertBadPassword(control, '123');
+
+                control.setStrengthCriteria(['NUMERIC']);
+                self.assertBadPassword(control, 'Abc');
+                self.assertGoodPassword(control, '123');
+
+                control.setStrengthCriteria(['ALPHA', 'NUMERIC']);
+                self.assertBadPassword(control, 'Abc');
+                self.assertBadPassword(control, '123');
+                self.assertGoodPassword(control, 'Abc123');
+
+                control.setStrengthCriteria(['MIXEDCASE']);
+                self.assertGoodPassword(control, 'Abc');
+                self.assertGoodPassword(control, 'abC');
+                self.assertBadPassword(control, 'abc');
+                self.assertBadPassword(control, '123');
+
+                control.setStrengthCriteria(['SYMBOLS']);
+                self.assertGoodPassword(control, '!@#_');
+                self.assertBadPassword(control, ' ');
+                self.assertBadPassword(control, 'abc');
+
+                control.setStrengthCriteria([]);
+                self.assertGoodPassword(control, '!@#_');
+                self.assertGoodPassword(control, 'abc');
+                self.assertGoodPassword(control, '123');
+
+                self.assertThrows(Methanal.View.InvalidStrengthCriterion,
+                    function () {
+                        control.setStrengthCriteria(['DANGERWILLROBINSON']);
+                    });
+            });
+    });
+
+
+
+/**
  * Tests for L{Methanal.View.InputContainer}.
  */
 Methanal.Tests.TestView.BaseTestTextInput.subclass(Methanal.Tests.TestView, 'TestFormGroup').methods(
 
=== modified file 'methanal/js/Methanal/View.js'
--- methanal/js/Methanal/View.js	2009-10-05 00:15:43 +0000
+++ methanal/js/Methanal/View.js	2009-10-09 22:42:12 +0000
@@ -1793,3 +1793,94 @@
             return 'Percentage values must be between 0% and 100%'
         }
     });
+
+
+
+/**
+ * An invalid password strength criterion was specified.
+ */
+Divmod.Error.subclass(Methanal.View, 'InvalidStrengthCriterion');
+
+
+
+/**
+ * Password input with a verification field and strength checking.
+ */
+Methanal.View.TextInput.subclass(Methanal.View, 'VerifiedPasswordInput').methods(
+    function __init__(self, node, args) {
+        Methanal.View.VerifiedPasswordInput.upcall(
+            self, '__init__', node, args);
+        self._minPasswordLength = args.minPasswordLength || 5;
+        self.setStrengthCriteria(args.strengthCriteria || []);
+    },
+
+
+    function nodeInserted(self) {
+        self._confirmPasswordNode = self.nodeById('confirmPassword');
+        Methanal.View.VerifiedPasswordInput.upcall(self, 'nodeInserted');
+    },
+
+
+    /**
+     * Set the password strength criteria.
+     *
+     * @type  criteria: C{Array} of C{String}
+     * @param criteria: An array of names, matching those found in
+     *     L{Methanal.View.VerifiedPasswordInput.STRENGTH_CRITERIA}, indicating
+     *     the password strength criteria
+     */
+    function setStrengthCriteria(self, criteria) {
+        var fns = Methanal.View.VerifiedPasswordInput.STRENGTH_CRITERIA;
+        for (var i = 0; i < criteria.length; ++i) {
+            var criterion = criteria[i];
+            if (fns[criterion] === undefined) {
+                criterion = Methanal.Util.repr(criterion);
+                throw Methanal.View.InvalidStrengthCriterion(criterion);
+            }
+        }
+        self._strengthCriteria = criteria;
+    },
+
+
+    /**
+     * Override this method to change the definition of a 'strong' password.
+     */
+    function passwordIsStrong(self, password) {
+        if (password.length < self._minPasswordLength) {
+            return false;
+        }
+
+        var fns = Methanal.View.VerifiedPasswordInput.STRENGTH_CRITERIA;
+        for (var i = 0; i < self._strengthCriteria.length; ++i) {
+            var fn = fns[self._strengthCriteria[i]];
+            if (!fn(password)) {
+                return false;
+            }
+        }
+        return true;
+    },
+
+
+    /**
+     * This default validator ensures that the password is strong and that
+     * the password given in both fields have length > 0 and match exactly.
+     */
+    function baseValidator(self, value) {
+        if (value !== self._confirmPasswordNode.value || value === null ||
+            self._confirmPasswordNode.value === null) {
+            return 'Passwords do not match.';
+        }
+
+        if (!self.passwordIsStrong(value)) {
+            return 'Password is too weak.';
+        }
+    });
+
+
+
+Methanal.View.VerifiedPasswordInput.STRENGTH_CRITERIA = {
+    'ALPHA':     function (value) { return /[a-zA-Z]/.test(value); },
+    'NUMERIC':   function (value) { return /[0-9]/.test(value); },
+    'MIXEDCASE': function (value) {
+        return /[a-z]/.test(value) && /[A-Z]/.test(value); },
+    'SYMBOLS':   function (value) { return /[^A-Za-z0-9\s]/.test(value); }};
 
=== added file 'methanal/themes/methanal-base/methanal-verified-password-input.html'
--- methanal/themes/methanal-base/methanal-verified-password-input.html	1970-01-01 00:00:00 +0000
+++ methanal/themes/methanal-base/methanal-verified-password-input.html	2009-10-09 22:42:12 +0000
@@ -0,0 +1,15 @@
+<div xmlns:nevow="http://nevow.com/ns/nevow/0.1" xmlns:athena="http://divmod.org/ns/athena/0.7" nevow:render="liveElement">
+    <input class="methanal-input" type="password">
+        <nevow:attr name="value" nevow:render="value" />
+        <athena:handler event="onchange" handler="onChange" />
+        <athena:handler event="onkeyup" handler="onKeyUp" />
+        <athena:handler event="onblur" handler="onBlur" />
+        <athena:handler event="onfocus" handler="onFocus" />
+    </input>
+    <input class="methanal-input" type="password" id="confirmPassword">
+        <nevow:attr name="value" nevow:render="value" />
+        <athena:handler event="onchange" handler="onChange" />
+    </input>
+    <span style="display: none;" class="friendly-representation" id="displayValue" />
+    <span class="methanal-error" id="error" />
+</div>
 
=== modified file 'methanal/view.py'
--- methanal/view.py	2009-10-04 10:17:26 +0000
+++ methanal/view.py	2009-10-09 22:42:12 +0000
@@ -511,6 +511,38 @@
 
 
 
+class VerifiedPasswordInput(TextInput):
+    """
+    Password input with verification and strength checking.
+
+    @type minPasswordLength: C{int}
+    @ivar minPasswordLength: Minimum acceptable password length, or C{None}
+        to use the default client-side value
+
+    @type strengthCriteria: C{list} of C{unicode}
+    @ivar strengthCriteria: A list of criteria names for password strength
+        testing, or C{None} for no additional strength criteria. See
+        L{Methanal.View.VerifiedPasswordInput.STRENGTH_CRITERIA} in the
+        Javascript source for possible values
+    """
+    fragmentName = 'methanal-verified-password-input'
+    jsClass = u'Methanal.View.VerifiedPasswordInput'
+
+
+    def __init__(self, minPasswordLength=None, strengthCriteria=None, **kw):
+        super(VerifiedPasswordInput, self).__init__(**kw)
+        self.minPasswordLength = minPasswordLength
+        if strengthCriteria is None:
+            strengthCriteria = []
+        self.strengthCriteria = strengthCriteria
+
+
+    def getArgs(self):
+        return {u'minPasswordLength': self.minPasswordLength,
+                u'strengthCriteria':  self.strengthCriteria}
+
+
+
 class ChoiceInput(FormInput):
     """
     Abstract input with multiple options.