Methanal

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

  1. verified-password-input
  2. Merge into trunk
Proposed by Jonathan Jacobs
Status: Merged
Approved by: Tristan Seligmann
Approved revision: 114
Merged at revision: not available
Proposed branch: lp:~jjacobs/methanal/verified-password-input
Merge into: lp:methanal
Diff against target: 306 lines
4 files modified
methanal/js/Methanal/Tests/TestView.js (+119/-5)
methanal/js/Methanal/View.js (+91/-0)
methanal/themes/methanal-base/methanal-verified-password-input.html (+15/-0)
methanal/view.py (+32/-0)
To merge this branch: bzr merge lp:~jjacobs/methanal/verified-password-input
Reviewer Review Type Date Requested Status
Tristan Seligmann Approve
Forrest Aldridge (community) Needs Fixing
Review via email: mp+13138@code.launchpad.net

This proposal supersedes a proposal from 2009-10-09.

To post a comment you must log in.
Revision history for this message
Forrest Aldridge (forrest-aldridge) wrote : Posted in a previous version of this proposal

 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
Revision history for this message
Tristan Seligmann (mithrandi) wrote :

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
Revision history for this message
Forrest Aldridge (forrest-aldridge) wrote :

 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
Revision history for this message
Jonathan Jacobs (jjacobs) wrote :

> 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.

Revision history for this message
Jonathan Jacobs (jjacobs) wrote :

> 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.

Revision history for this message
Tristan Seligmann (mithrandi) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'methanal/js/Methanal/Tests/TestView.js'
2--- methanal/js/Methanal/Tests/TestView.js 2009-10-04 11:18:09 +0000
3+++ methanal/js/Methanal/Tests/TestView.js 2009-10-09 22:42:12 +0000
4@@ -168,14 +168,14 @@
5 * Set the input node's value to C{value} and passes the result of
6 * C{control.getValue} to C{control.baseValidator}.
7 */
8- function assertValidInput(self, control, value) {
9- var oldValue = control.inputNode.value;
10+ function assertValidInput(self, control, value, msg) {
11 control.inputNode.value = value;
12- var msg = (Methanal.Util.repr(value) + ' is NOT valid input for ' +
13- Methanal.Util.repr(control));
14+ if (msg === undefined) {
15+ msg = (Methanal.Util.repr(value) + ' is NOT valid input for ' +
16+ Methanal.Util.repr(control));
17+ }
18 self.assertIdentical(
19 control.baseValidator(control.getValue()), undefined, msg);
20- control.inputNode.value = oldValue;
21 },
22
23
24@@ -754,6 +754,120 @@
25
26
27 /**
28+ * Tests for L{Methanal.View.VerifiedPasswordInput}.
29+ */
30+Methanal.Tests.TestView.BaseTestTextInput.subclass(Methanal.Tests.TestView, 'TestVerifiedPasswordInput').methods(
31+ function setUp(self) {
32+ self.controlType = Methanal.View.VerifiedPasswordInput;
33+ },
34+
35+
36+ /**
37+ * Assert that C{password}, and optionally C{confirmPassword}, are a good
38+ * input.
39+ */
40+ function assertGoodPassword(self, control, password, confirmPassword) {
41+ if (confirmPassword === undefined) {
42+ confirmPassword = password;
43+ }
44+ control._confirmPasswordNode.value = confirmPassword;
45+ self.assertValidInput(
46+ control, password,
47+ Methanal.Util.repr(password) + ' is NOT a good password');
48+ },
49+
50+
51+ /**
52+ * Assert that C{password}, and optionally C{confirmPassword}, are a bad
53+ * input.
54+ */
55+ function assertBadPassword(self, control, password, confirmPassword) {
56+ if (confirmPassword === undefined) {
57+ confirmPassword = password;
58+ }
59+ control._confirmPasswordNode.value = confirmPassword;
60+ self.assertInvalidInput(
61+ control, password,
62+ Methanal.Util.repr(password) + ' IS a good password');
63+ },
64+
65+
66+ function createControl(self, args) {
67+ var control = Methanal.Tests.TestView.TestVerifiedPasswordInput.upcall(
68+ self, 'createControl', args);
69+ Methanal.Tests.TestView.makeWidgetChildNode(
70+ control, 'input', 'confirmPassword');
71+ return control;
72+ },
73+
74+
75+ /**
76+ * Validation will fail under the following conditions:
77+ * 1. The input and confirmPasswordNode node values don't match.
78+ * 2. If either of the above node values have no length (are blank).
79+ */
80+ function test_inputValidation(self) {
81+ self.testControl({value: null},
82+ function (control) {
83+ // Test condition 1
84+ self.assertBadPassword(control, 'match', 'no match');
85+ self.assertGoodPassword(control, 'match', 'match');
86+ // Test condition 2
87+ self.assertBadPassword(control, '', '');
88+ });
89+ },
90+
91+
92+ /**
93+ * Changing the password strength criteria results in different validation
94+ * criteria for the control.
95+ */
96+ function test_strengthCriteria(self) {
97+ // Override the default criteria of 5 or more characters.
98+ self.testControl({value: null, minPasswordLength: 3},
99+ function (control) {
100+ self.assertBadPassword(control, '12');
101+ self.assertGoodPassword(control, '123');
102+
103+ control.setStrengthCriteria(['ALPHA']);
104+ self.assertGoodPassword(control, 'Abc');
105+ self.assertBadPassword(control, '123');
106+
107+ control.setStrengthCriteria(['NUMERIC']);
108+ self.assertBadPassword(control, 'Abc');
109+ self.assertGoodPassword(control, '123');
110+
111+ control.setStrengthCriteria(['ALPHA', 'NUMERIC']);
112+ self.assertBadPassword(control, 'Abc');
113+ self.assertBadPassword(control, '123');
114+ self.assertGoodPassword(control, 'Abc123');
115+
116+ control.setStrengthCriteria(['MIXEDCASE']);
117+ self.assertGoodPassword(control, 'Abc');
118+ self.assertGoodPassword(control, 'abC');
119+ self.assertBadPassword(control, 'abc');
120+ self.assertBadPassword(control, '123');
121+
122+ control.setStrengthCriteria(['SYMBOLS']);
123+ self.assertGoodPassword(control, '!@#_');
124+ self.assertBadPassword(control, ' ');
125+ self.assertBadPassword(control, 'abc');
126+
127+ control.setStrengthCriteria([]);
128+ self.assertGoodPassword(control, '!@#_');
129+ self.assertGoodPassword(control, 'abc');
130+ self.assertGoodPassword(control, '123');
131+
132+ self.assertThrows(Methanal.View.InvalidStrengthCriterion,
133+ function () {
134+ control.setStrengthCriteria(['DANGERWILLROBINSON']);
135+ });
136+ });
137+ });
138+
139+
140+
141+/**
142 * Tests for L{Methanal.View.InputContainer}.
143 */
144 Methanal.Tests.TestView.BaseTestTextInput.subclass(Methanal.Tests.TestView, 'TestFormGroup').methods(
145
146=== modified file 'methanal/js/Methanal/View.js'
147--- methanal/js/Methanal/View.js 2009-10-05 00:15:43 +0000
148+++ methanal/js/Methanal/View.js 2009-10-09 22:42:12 +0000
149@@ -1793,3 +1793,94 @@
150 return 'Percentage values must be between 0% and 100%'
151 }
152 });
153+
154+
155+
156+/**
157+ * An invalid password strength criterion was specified.
158+ */
159+Divmod.Error.subclass(Methanal.View, 'InvalidStrengthCriterion');
160+
161+
162+
163+/**
164+ * Password input with a verification field and strength checking.
165+ */
166+Methanal.View.TextInput.subclass(Methanal.View, 'VerifiedPasswordInput').methods(
167+ function __init__(self, node, args) {
168+ Methanal.View.VerifiedPasswordInput.upcall(
169+ self, '__init__', node, args);
170+ self._minPasswordLength = args.minPasswordLength || 5;
171+ self.setStrengthCriteria(args.strengthCriteria || []);
172+ },
173+
174+
175+ function nodeInserted(self) {
176+ self._confirmPasswordNode = self.nodeById('confirmPassword');
177+ Methanal.View.VerifiedPasswordInput.upcall(self, 'nodeInserted');
178+ },
179+
180+
181+ /**
182+ * Set the password strength criteria.
183+ *
184+ * @type criteria: C{Array} of C{String}
185+ * @param criteria: An array of names, matching those found in
186+ * L{Methanal.View.VerifiedPasswordInput.STRENGTH_CRITERIA}, indicating
187+ * the password strength criteria
188+ */
189+ function setStrengthCriteria(self, criteria) {
190+ var fns = Methanal.View.VerifiedPasswordInput.STRENGTH_CRITERIA;
191+ for (var i = 0; i < criteria.length; ++i) {
192+ var criterion = criteria[i];
193+ if (fns[criterion] === undefined) {
194+ criterion = Methanal.Util.repr(criterion);
195+ throw Methanal.View.InvalidStrengthCriterion(criterion);
196+ }
197+ }
198+ self._strengthCriteria = criteria;
199+ },
200+
201+
202+ /**
203+ * Override this method to change the definition of a 'strong' password.
204+ */
205+ function passwordIsStrong(self, password) {
206+ if (password.length < self._minPasswordLength) {
207+ return false;
208+ }
209+
210+ var fns = Methanal.View.VerifiedPasswordInput.STRENGTH_CRITERIA;
211+ for (var i = 0; i < self._strengthCriteria.length; ++i) {
212+ var fn = fns[self._strengthCriteria[i]];
213+ if (!fn(password)) {
214+ return false;
215+ }
216+ }
217+ return true;
218+ },
219+
220+
221+ /**
222+ * This default validator ensures that the password is strong and that
223+ * the password given in both fields have length > 0 and match exactly.
224+ */
225+ function baseValidator(self, value) {
226+ if (value !== self._confirmPasswordNode.value || value === null ||
227+ self._confirmPasswordNode.value === null) {
228+ return 'Passwords do not match.';
229+ }
230+
231+ if (!self.passwordIsStrong(value)) {
232+ return 'Password is too weak.';
233+ }
234+ });
235+
236+
237+
238+Methanal.View.VerifiedPasswordInput.STRENGTH_CRITERIA = {
239+ 'ALPHA': function (value) { return /[a-zA-Z]/.test(value); },
240+ 'NUMERIC': function (value) { return /[0-9]/.test(value); },
241+ 'MIXEDCASE': function (value) {
242+ return /[a-z]/.test(value) && /[A-Z]/.test(value); },
243+ 'SYMBOLS': function (value) { return /[^A-Za-z0-9\s]/.test(value); }};
244
245=== added file 'methanal/themes/methanal-base/methanal-verified-password-input.html'
246--- methanal/themes/methanal-base/methanal-verified-password-input.html 1970-01-01 00:00:00 +0000
247+++ methanal/themes/methanal-base/methanal-verified-password-input.html 2009-10-09 22:42:12 +0000
248@@ -0,0 +1,15 @@
249+<div xmlns:nevow="http://nevow.com/ns/nevow/0.1" xmlns:athena="http://divmod.org/ns/athena/0.7" nevow:render="liveElement">
250+ <input class="methanal-input" type="password">
251+ <nevow:attr name="value" nevow:render="value" />
252+ <athena:handler event="onchange" handler="onChange" />
253+ <athena:handler event="onkeyup" handler="onKeyUp" />
254+ <athena:handler event="onblur" handler="onBlur" />
255+ <athena:handler event="onfocus" handler="onFocus" />
256+ </input>
257+ <input class="methanal-input" type="password" id="confirmPassword">
258+ <nevow:attr name="value" nevow:render="value" />
259+ <athena:handler event="onchange" handler="onChange" />
260+ </input>
261+ <span style="display: none;" class="friendly-representation" id="displayValue" />
262+ <span class="methanal-error" id="error" />
263+</div>
264
265=== modified file 'methanal/view.py'
266--- methanal/view.py 2009-10-04 10:17:26 +0000
267+++ methanal/view.py 2009-10-09 22:42:12 +0000
268@@ -511,6 +511,38 @@
269
270
271
272+class VerifiedPasswordInput(TextInput):
273+ """
274+ Password input with verification and strength checking.
275+
276+ @type minPasswordLength: C{int}
277+ @ivar minPasswordLength: Minimum acceptable password length, or C{None}
278+ to use the default client-side value
279+
280+ @type strengthCriteria: C{list} of C{unicode}
281+ @ivar strengthCriteria: A list of criteria names for password strength
282+ testing, or C{None} for no additional strength criteria. See
283+ L{Methanal.View.VerifiedPasswordInput.STRENGTH_CRITERIA} in the
284+ Javascript source for possible values
285+ """
286+ fragmentName = 'methanal-verified-password-input'
287+ jsClass = u'Methanal.View.VerifiedPasswordInput'
288+
289+
290+ def __init__(self, minPasswordLength=None, strengthCriteria=None, **kw):
291+ super(VerifiedPasswordInput, self).__init__(**kw)
292+ self.minPasswordLength = minPasswordLength
293+ if strengthCriteria is None:
294+ strengthCriteria = []
295+ self.strengthCriteria = strengthCriteria
296+
297+
298+ def getArgs(self):
299+ return {u'minPasswordLength': self.minPasswordLength,
300+ u'strengthCriteria': self.strengthCriteria}
301+
302+
303+
304 class ChoiceInput(FormInput):
305 """
306 Abstract input with multiple options.

Subscribers

People subscribed via source and target branches