Merge lp:~jjacobs/methanal/verified-password-input into lp:methanal
- verified-password-input
- Merge into trunk
Proposed by
Jonathan Jacobs
Status: | Superseded |
---|---|
Proposed branch: | lp:~jjacobs/methanal/verified-password-input |
Merge into: | lp:methanal |
Diff against target: |
305 lines 4 files modified
methanal/js/Methanal/Tests/TestView.js (+128/-3) methanal/js/Methanal/View.js (+84/-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 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Forrest Aldridge (community) | Needs Fixing | ||
Methanal maintainers | Pending | ||
Review via email: mp+13134@code.launchpad.net |
This proposal has been superseded by a proposal from 2009-10-09.
Commit message
Description of the change
To post a comment you must log in.
Revision history for this message
Forrest Aldridge (forrest-aldridge) wrote : | # |
review:
Needs Fixing
- 111. By Jonathan Jacobs
-
Rename variable for clarity.
- 112. By Jonathan Jacobs
-
Finish incomplete docstring.
- 113. By Jonathan Jacobs
-
Don't bother saving and restoring input node values.
- 114. By Jonathan Jacobs
-
Throw a real exception.
Unmerged revisions
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 17:35:23 +0000 | |||
4 | @@ -168,11 +168,13 @@ | |||
5 | 168 | * Set the input node's value to C{value} and passes the result of | 168 | * Set the input node's value to C{value} and passes the result of |
6 | 169 | * C{control.getValue} to C{control.baseValidator}. | 169 | * C{control.getValue} to C{control.baseValidator}. |
7 | 170 | */ | 170 | */ |
9 | 171 | function assertValidInput(self, control, value) { | 171 | function assertValidInput(self, control, value, msg) { |
10 | 172 | var oldValue = control.inputNode.value; | 172 | var oldValue = control.inputNode.value; |
11 | 173 | control.inputNode.value = value; | 173 | control.inputNode.value = value; |
14 | 174 | var msg = (Methanal.Util.repr(value) + ' is NOT valid input for ' + | 174 | if (msg === undefined) { |
15 | 175 | Methanal.Util.repr(control)); | 175 | msg = (Methanal.Util.repr(value) + ' is NOT valid input for ' + |
16 | 176 | Methanal.Util.repr(control)); | ||
17 | 177 | } | ||
18 | 176 | self.assertIdentical( | 178 | self.assertIdentical( |
19 | 177 | control.baseValidator(control.getValue()), undefined, msg); | 179 | control.baseValidator(control.getValue()), undefined, msg); |
20 | 178 | control.inputNode.value = oldValue; | 180 | control.inputNode.value = oldValue; |
21 | @@ -754,6 +756,129 @@ | |||
22 | 754 | 756 | ||
23 | 755 | 757 | ||
24 | 756 | /** | 758 | /** |
25 | 759 | * Tests for L{Methanal.View.VerifiedPasswordInput}. | ||
26 | 760 | */ | ||
27 | 761 | Methanal.Tests.TestView.BaseTestTextInput.subclass(Methanal.Tests.TestView, 'TestVerifiedPasswordInput').methods( | ||
28 | 762 | function setUp(self) { | ||
29 | 763 | self.controlType = Methanal.View.VerifiedPasswordInput; | ||
30 | 764 | }, | ||
31 | 765 | |||
32 | 766 | |||
33 | 767 | /** | ||
34 | 768 | * Perform some setup tasks for password validation asserting. | ||
35 | 769 | */ | ||
36 | 770 | function _assertPassword(self, fn, control, password, confirmPassword) { | ||
37 | 771 | if (confirmPassword === undefined) { | ||
38 | 772 | confirmPassword = password; | ||
39 | 773 | } | ||
40 | 774 | var oldPasswordValue = control._confirmPasswordNode.value; | ||
41 | 775 | control._confirmPasswordNode.value = confirmPassword; | ||
42 | 776 | fn(); | ||
43 | 777 | control._confirmPasswordNode.value = oldPasswordValue; | ||
44 | 778 | }, | ||
45 | 779 | |||
46 | 780 | |||
47 | 781 | /** | ||
48 | 782 | * Assert that C{password}, and optionally C{confirmPassword}, are a good | ||
49 | 783 | * input. | ||
50 | 784 | */ | ||
51 | 785 | function assertGoodPassword(self, control, password, confirmPassword) { | ||
52 | 786 | self._assertPassword(function () { | ||
53 | 787 | self.assertValidInput( | ||
54 | 788 | control, password, | ||
55 | 789 | Methanal.Util.repr(password) + ' is NOT a good password'); | ||
56 | 790 | }, control, password, confirmPassword); | ||
57 | 791 | }, | ||
58 | 792 | |||
59 | 793 | |||
60 | 794 | /** | ||
61 | 795 | * Assert that C{password}, and optionally C{confirmPassword}, are a bad | ||
62 | 796 | * input. | ||
63 | 797 | */ | ||
64 | 798 | function assertBadPassword(self, control, password, confirmPassword) { | ||
65 | 799 | self._assertPassword(function () { | ||
66 | 800 | self.assertInvalidInput( | ||
67 | 801 | control, password, | ||
68 | 802 | Methanal.Util.repr(password) + ' IS a good password'); | ||
69 | 803 | }, control, password, confirmPassword); | ||
70 | 804 | }, | ||
71 | 805 | |||
72 | 806 | |||
73 | 807 | function createControl(self, args) { | ||
74 | 808 | var control = Methanal.Tests.TestView.TestVerifiedPasswordInput.upcall( | ||
75 | 809 | self, 'createControl', args); | ||
76 | 810 | Methanal.Tests.TestView.makeWidgetChildNode( | ||
77 | 811 | control, 'input', 'confirmPassword'); | ||
78 | 812 | return control; | ||
79 | 813 | }, | ||
80 | 814 | |||
81 | 815 | |||
82 | 816 | /** | ||
83 | 817 | * Validation will fail under the following conditions: | ||
84 | 818 | * 1. The input and confirmPasswordNode node values don't match. | ||
85 | 819 | * 2. If either of the above node values have no length (are blank). | ||
86 | 820 | */ | ||
87 | 821 | function test_inputValidation(self) { | ||
88 | 822 | self.testControl({value: null}, | ||
89 | 823 | function (control) { | ||
90 | 824 | // Test condition 1 | ||
91 | 825 | self.assertBadPassword(control, 'match', 'no match'); | ||
92 | 826 | self.assertGoodPassword(control, 'match', 'match'); | ||
93 | 827 | // Test condition 2 | ||
94 | 828 | self.assertBadPassword(control, '', ''); | ||
95 | 829 | }); | ||
96 | 830 | }, | ||
97 | 831 | |||
98 | 832 | |||
99 | 833 | /** | ||
100 | 834 | * Changing the password strength criteria results in different | ||
101 | 835 | */ | ||
102 | 836 | function test_strengthCriteria(self) { | ||
103 | 837 | // Override the default criteria of 5 or more characters. | ||
104 | 838 | self.testControl({value: null, minPasswordLength: 3}, | ||
105 | 839 | function (control) { | ||
106 | 840 | self.assertBadPassword(control, '12'); | ||
107 | 841 | self.assertGoodPassword(control, '123'); | ||
108 | 842 | |||
109 | 843 | control.setStrengthCriteria(['ALPHA']); | ||
110 | 844 | self.assertGoodPassword(control, 'Abc'); | ||
111 | 845 | self.assertBadPassword(control, '123'); | ||
112 | 846 | |||
113 | 847 | control.setStrengthCriteria(['NUMERIC']); | ||
114 | 848 | self.assertBadPassword(control, 'Abc'); | ||
115 | 849 | self.assertGoodPassword(control, '123'); | ||
116 | 850 | |||
117 | 851 | control.setStrengthCriteria(['ALPHA', 'NUMERIC']); | ||
118 | 852 | self.assertBadPassword(control, 'Abc'); | ||
119 | 853 | self.assertBadPassword(control, '123'); | ||
120 | 854 | self.assertGoodPassword(control, 'Abc123'); | ||
121 | 855 | |||
122 | 856 | control.setStrengthCriteria(['MIXEDCASE']); | ||
123 | 857 | self.assertGoodPassword(control, 'Abc'); | ||
124 | 858 | self.assertGoodPassword(control, 'abC'); | ||
125 | 859 | self.assertBadPassword(control, 'abc'); | ||
126 | 860 | self.assertBadPassword(control, '123'); | ||
127 | 861 | |||
128 | 862 | control.setStrengthCriteria(['SYMBOLS']); | ||
129 | 863 | self.assertGoodPassword(control, '!@#_'); | ||
130 | 864 | self.assertBadPassword(control, ' '); | ||
131 | 865 | self.assertBadPassword(control, 'abc'); | ||
132 | 866 | |||
133 | 867 | control.setStrengthCriteria([]); | ||
134 | 868 | self.assertGoodPassword(control, '!@#_'); | ||
135 | 869 | self.assertGoodPassword(control, 'abc'); | ||
136 | 870 | self.assertGoodPassword(control, '123'); | ||
137 | 871 | |||
138 | 872 | self.assertThrows(Error, | ||
139 | 873 | function () { | ||
140 | 874 | control.setStrengthCriteria(['DANGERWILLROBINSON']); | ||
141 | 875 | }); | ||
142 | 876 | }); | ||
143 | 877 | }); | ||
144 | 878 | |||
145 | 879 | |||
146 | 880 | |||
147 | 881 | /** | ||
148 | 757 | * Tests for L{Methanal.View.InputContainer}. | 882 | * Tests for L{Methanal.View.InputContainer}. |
149 | 758 | */ | 883 | */ |
150 | 759 | Methanal.Tests.TestView.BaseTestTextInput.subclass(Methanal.Tests.TestView, 'TestFormGroup').methods( | 884 | Methanal.Tests.TestView.BaseTestTextInput.subclass(Methanal.Tests.TestView, 'TestFormGroup').methods( |
151 | 760 | 885 | ||
152 | === modified file 'methanal/js/Methanal/View.js' | |||
153 | --- methanal/js/Methanal/View.js 2009-10-05 00:15:43 +0000 | |||
154 | +++ methanal/js/Methanal/View.js 2009-10-09 17:35:23 +0000 | |||
155 | @@ -1793,3 +1793,87 @@ | |||
156 | 1793 | return 'Percentage values must be between 0% and 100%' | 1793 | return 'Percentage values must be between 0% and 100%' |
157 | 1794 | } | 1794 | } |
158 | 1795 | }); | 1795 | }); |
159 | 1796 | |||
160 | 1797 | |||
161 | 1798 | |||
162 | 1799 | /** | ||
163 | 1800 | * Password input with a verification field and strength checking. | ||
164 | 1801 | */ | ||
165 | 1802 | Methanal.View.TextInput.subclass(Methanal.View, 'VerifiedPasswordInput').methods( | ||
166 | 1803 | function __init__(self, node, args) { | ||
167 | 1804 | Methanal.View.VerifiedPasswordInput.upcall( | ||
168 | 1805 | self, '__init__', node, args); | ||
169 | 1806 | self._minPasswordLength = args.minPasswordLength || 5; | ||
170 | 1807 | self.setStrengthCriteria(args.strengthCriteria || []); | ||
171 | 1808 | }, | ||
172 | 1809 | |||
173 | 1810 | |||
174 | 1811 | function nodeInserted(self) { | ||
175 | 1812 | self._confirmPasswordNode = self.nodeById('confirmPassword'); | ||
176 | 1813 | Methanal.View.VerifiedPasswordInput.upcall(self, 'nodeInserted'); | ||
177 | 1814 | }, | ||
178 | 1815 | |||
179 | 1816 | |||
180 | 1817 | /** | ||
181 | 1818 | * Set the password strength criteria. | ||
182 | 1819 | * | ||
183 | 1820 | * @type criteria: C{Array} of C{String} | ||
184 | 1821 | * @param criteria: An array of names, matching those found in | ||
185 | 1822 | * L{Methanal.View.VerifiedPasswordInput.STRENGTH_CRITERIA}, indicating | ||
186 | 1823 | * the password strength criteria | ||
187 | 1824 | */ | ||
188 | 1825 | function setStrengthCriteria(self, criteria) { | ||
189 | 1826 | var fns = Methanal.View.VerifiedPasswordInput.STRENGTH_CRITERIA; | ||
190 | 1827 | for (var i = 0; i < criteria.length; ++i) { | ||
191 | 1828 | var c = criteria[i]; | ||
192 | 1829 | if (fns[c] === undefined) { | ||
193 | 1830 | c = Methanal.Util.repr(c); | ||
194 | 1831 | throw new Error('Unknown strength criterion: ' + c); | ||
195 | 1832 | } | ||
196 | 1833 | } | ||
197 | 1834 | self._strengthCriteria = criteria; | ||
198 | 1835 | }, | ||
199 | 1836 | |||
200 | 1837 | |||
201 | 1838 | /** | ||
202 | 1839 | * Override this method to change the definition of a 'strong' password. | ||
203 | 1840 | */ | ||
204 | 1841 | function passwordIsStrong(self, password) { | ||
205 | 1842 | if (password.length < self._minPasswordLength) { | ||
206 | 1843 | return false; | ||
207 | 1844 | } | ||
208 | 1845 | |||
209 | 1846 | var fns = Methanal.View.VerifiedPasswordInput.STRENGTH_CRITERIA; | ||
210 | 1847 | for (var i = 0; i < self._strengthCriteria.length; ++i) { | ||
211 | 1848 | var fn = fns[self._strengthCriteria[i]]; | ||
212 | 1849 | if (!fn(password)) { | ||
213 | 1850 | return false; | ||
214 | 1851 | } | ||
215 | 1852 | } | ||
216 | 1853 | return true; | ||
217 | 1854 | }, | ||
218 | 1855 | |||
219 | 1856 | |||
220 | 1857 | /** | ||
221 | 1858 | * This default validator ensures that the password is strong and that | ||
222 | 1859 | * the password given in both fields have length > 0 and match exactly. | ||
223 | 1860 | */ | ||
224 | 1861 | function baseValidator(self, value) { | ||
225 | 1862 | if (value !== self._confirmPasswordNode.value || value === null || | ||
226 | 1863 | self._confirmPasswordNode.value === null) { | ||
227 | 1864 | return 'Passwords do not match.'; | ||
228 | 1865 | } | ||
229 | 1866 | |||
230 | 1867 | if (!self.passwordIsStrong(value)) { | ||
231 | 1868 | return 'Password is too weak.'; | ||
232 | 1869 | } | ||
233 | 1870 | }); | ||
234 | 1871 | |||
235 | 1872 | |||
236 | 1873 | |||
237 | 1874 | Methanal.View.VerifiedPasswordInput.STRENGTH_CRITERIA = { | ||
238 | 1875 | 'ALPHA': function (value) { return /[a-zA-Z]/.test(value); }, | ||
239 | 1876 | 'NUMERIC': function (value) { return /[0-9]/.test(value); }, | ||
240 | 1877 | 'MIXEDCASE': function (value) { | ||
241 | 1878 | return /[a-z]/.test(value) && /[A-Z]/.test(value); }, | ||
242 | 1879 | 'SYMBOLS': function (value) { return /[^A-Za-z0-9\s]/.test(value); }}; | ||
243 | 1796 | 1880 | ||
244 | === added file 'methanal/themes/methanal-base/methanal-verified-password-input.html' | |||
245 | --- methanal/themes/methanal-base/methanal-verified-password-input.html 1970-01-01 00:00:00 +0000 | |||
246 | +++ methanal/themes/methanal-base/methanal-verified-password-input.html 2009-10-09 17:35:23 +0000 | |||
247 | @@ -0,0 +1,15 @@ | |||
248 | 1 | <div xmlns:nevow="http://nevow.com/ns/nevow/0.1" xmlns:athena="http://divmod.org/ns/athena/0.7" nevow:render="liveElement"> | ||
249 | 2 | <input class="methanal-input" type="password"> | ||
250 | 3 | <nevow:attr name="value" nevow:render="value" /> | ||
251 | 4 | <athena:handler event="onchange" handler="onChange" /> | ||
252 | 5 | <athena:handler event="onkeyup" handler="onKeyUp" /> | ||
253 | 6 | <athena:handler event="onblur" handler="onBlur" /> | ||
254 | 7 | <athena:handler event="onfocus" handler="onFocus" /> | ||
255 | 8 | </input> | ||
256 | 9 | <input class="methanal-input" type="password" id="confirmPassword"> | ||
257 | 10 | <nevow:attr name="value" nevow:render="value" /> | ||
258 | 11 | <athena:handler event="onchange" handler="onChange" /> | ||
259 | 12 | </input> | ||
260 | 13 | <span style="display: none;" class="friendly-representation" id="displayValue" /> | ||
261 | 14 | <span class="methanal-error" id="error" /> | ||
262 | 15 | </div> | ||
263 | 0 | 16 | ||
264 | === modified file 'methanal/view.py' | |||
265 | --- methanal/view.py 2009-10-04 10:17:26 +0000 | |||
266 | +++ methanal/view.py 2009-10-09 17:35:23 +0000 | |||
267 | @@ -511,6 +511,38 @@ | |||
268 | 511 | 511 | ||
269 | 512 | 512 | ||
270 | 513 | 513 | ||
271 | 514 | class VerifiedPasswordInput(TextInput): | ||
272 | 515 | """ | ||
273 | 516 | Password input with verification and strength checking. | ||
274 | 517 | |||
275 | 518 | @type minPasswordLength: C{int} | ||
276 | 519 | @ivar minPasswordLength: Minimum acceptable password length, or C{None} | ||
277 | 520 | to use the default client-side value | ||
278 | 521 | |||
279 | 522 | @type strengthCriteria: C{list} of C{unicode} | ||
280 | 523 | @ivar strengthCriteria: A list of criteria names for password strength | ||
281 | 524 | testing, or C{None} for no additional strength criteria. See | ||
282 | 525 | L{Methanal.View.VerifiedPasswordInput.STRENGTH_CRITERIA} in the | ||
283 | 526 | Javascript source for possible values | ||
284 | 527 | """ | ||
285 | 528 | fragmentName = 'methanal-verified-password-input' | ||
286 | 529 | jsClass = u'Methanal.View.VerifiedPasswordInput' | ||
287 | 530 | |||
288 | 531 | |||
289 | 532 | def __init__(self, minPasswordLength=None, strengthCriteria=None, **kw): | ||
290 | 533 | super(VerifiedPasswordInput, self).__init__(**kw) | ||
291 | 534 | self.minPasswordLength = minPasswordLength | ||
292 | 535 | if strengthCriteria is None: | ||
293 | 536 | strengthCriteria = [] | ||
294 | 537 | self.strengthCriteria = strengthCriteria | ||
295 | 538 | |||
296 | 539 | |||
297 | 540 | def getArgs(self): | ||
298 | 541 | return {u'minPasswordLength': self.minPasswordLength, | ||
299 | 542 | u'strengthCriteria': self.strengthCriteria} | ||
300 | 543 | |||
301 | 544 | |||
302 | 545 | |||
303 | 514 | class ChoiceInput(FormInput): | 546 | class ChoiceInput(FormInput): |
304 | 515 | """ | 547 | """ |
305 | 516 | Abstract input with multiple options. | 548 | Abstract input with multiple options. |
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'