Merge lp:~wgrant/launchpad/destroy-person-password into lp:launchpad

Proposed by William Grant
Status: Merged
Approved by: Curtis Hovey
Approved revision: no longer in the source branch.
Merged at revision: 14679
Proposed branch: lp:~wgrant/launchpad/destroy-person-password
Merge into: lp:launchpad
Diff against target: 467 lines (+14/-229)
11 files modified
lib/lp/app/doc/launchpadform.txt (+1/-3)
lib/lp/app/widgets/password.py (+0/-129)
lib/lp/app/widgets/templates/passwordchange.pt (+0/-35)
lib/lp/app/widgets/tests/test_widget_doctests.py (+0/-1)
lib/lp/registry/browser/person.py (+4/-9)
lib/lp/registry/browser/tests/person-admin-views.txt (+4/-17)
lib/lp/registry/doc/person-account.txt (+3/-3)
lib/lp/registry/doc/person.txt (+1/-4)
lib/lp/registry/interfaces/person.py (+0/-3)
lib/lp/registry/model/person.py (+1/-22)
lib/lp/testing/factory.py (+0/-3)
To merge this branch: bzr merge lp:~wgrant/launchpad/destroy-person-password
Reviewer Review Type Date Requested Status
Curtis Hovey (community) code. Approve
Review via email: mp+88685@code.launchpad.net

Commit message

Remove admin password reset support. Launchpad doesn't manage passwords.

Description of the change

Launchpad hasn't managed passwords since April 2010, but the admin password reset form has remained until now. This branch kills it and some related stuff.

To post a comment you must log in.
Revision history for this message
Curtis Hovey (sinzui) wrote :

Thank you very much for this branch. This fixes a few bugs. I will link the ones that I find.

review: Approve (code.)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/app/doc/launchpadform.txt'
--- lib/lp/app/doc/launchpadform.txt 2012-01-06 11:08:30 +0000
+++ lib/lp/app/doc/launchpadform.txt 2012-01-17 00:44:26 +0000
@@ -132,11 +132,9 @@
132132
133 >>> from zope.app.form.browser import TextWidget133 >>> from zope.app.form.browser import TextWidget
134 >>> from lp.app.browser.launchpadform import custom_widget134 >>> from lp.app.browser.launchpadform import custom_widget
135 >>> from lp.app.widgets.password import PasswordChangeWidget
136135
137 >>> class FormTestView3(LaunchpadFormView):136 >>> class FormTestView3(LaunchpadFormView):
138 ... schema = IFormTest137 ... schema = IFormTest
139 ... custom_widget('password', PasswordChangeWidget)
140 ... custom_widget('displayname', TextWidget, displayWidth=50)138 ... custom_widget('displayname', TextWidget, displayWidth=50)
141139
142 >>> context = FormTest()140 >>> context = FormTest()
@@ -149,7 +147,7 @@
149 >>> view.widgets['displayname'].displayWidth147 >>> view.widgets['displayname'].displayWidth
150 50148 50
151 >>> view.widgets['password']149 >>> view.widgets['password']
152 <...PasswordChangeWidget object at ...>150 <...TextWidget object at ...>
153151
154152
155== Using Another Context ==153== Using Another Context ==
156154
=== removed file 'lib/lp/app/widgets/password.py'
--- lib/lp/app/widgets/password.py 2011-12-24 17:49:30 +0000
+++ lib/lp/app/widgets/password.py 1970-01-01 00:00:00 +0000
@@ -1,129 +0,0 @@
1# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""
5Custom Password widgets.
6
7TODO: Consider folding this back into Zope3 -- StuartBishop 20050520
8"""
9
10__metaclass__ = type
11
12from z3c.ptcompat import ViewPageTemplateFile
13from zope.app.form.browser import PasswordWidget
14from zope.app.form.browser.interfaces import ITextBrowserWidget
15from zope.app.form.interfaces import WidgetInputError
16from zope.component import getUtility
17from zope.interface import implements
18from zope.schema.interfaces import ValidationError
19
20from lp import _
21from lp.services.webapp.interfaces import (
22 IMultiLineWidgetLayout,
23 IPasswordEncryptor,
24 )
25
26
27class PasswordMismatch(ValidationError):
28 __doc__ = _("Passwords do not match.")
29
30 def __repr__(self):
31 return repr(self.__doc__)
32
33
34class RequiredPasswordMissing(ValidationError):
35 __doc__ = _("You must enter a password.")
36
37 def __repr__(self):
38 return repr(self.__doc__)
39
40
41class PasswordChangeWidget(PasswordWidget):
42 """A password change widget.
43
44 Text is not echoed to the user, and two text boxes are used to ensure
45 the password is entered correctly.
46 """
47 implements(ITextBrowserWidget, IMultiLineWidgetLayout)
48 type = 'password change'
49 display_label = False
50
51 __call__ = ViewPageTemplateFile('templates/passwordchange.pt')
52
53 def hasInput(self):
54 """We always have input if there is an existing value
55
56 No input indicates unchanged.
57 """
58 if PasswordWidget.hasInput(self):
59 return True
60
61 # If we don't have input from the user, we lie because we will
62 # use the existing value.
63 return bool(self._getCurrentPassword())
64
65 def _getCurrentPassword(self):
66 # Yesh... indirection up the wazoo to do something this simple.
67 # Returns the current password.
68 return self.context.get(self.context.context) or None
69
70 def getInputValue(self):
71 """Ensure both text boxes contain the same value and inherited checks
72
73 >>> from lp.services.webapp.servers import (
74 ... LaunchpadTestRequest)
75 >>> from zope.schema import Field
76 >>> field = Field(__name__='foo', title=u'Foo')
77
78 The widget will only return a value if both of the text boxes
79 contain the same value. It returns the value encrypted.
80
81 >>> request = LaunchpadTestRequest(form={
82 ... 'field.foo': u'My Password',
83 ... 'field.foo_dupe': u'My Password',
84 ... })
85 >>> widget = PasswordChangeWidget(field, request)
86 >>> crypted_pw = widget.getInputValue()
87 >>> encryptor = getUtility(IPasswordEncryptor)
88 >>> encryptor.validate(u'My Password', crypted_pw)
89 True
90
91 Otherwise it raises the exception required by IInputWidget
92
93 >>> request = LaunchpadTestRequest(form={
94 ... 'field.foo': u'My Password', 'field.foo_dupe': u'No Match'})
95 >>> widget = PasswordChangeWidget(field, request)
96 >>> widget.getInputValue()
97 Traceback (most recent call last):
98 [...]
99 WidgetInputError: ('foo', u'Foo', u'Passwords do not match.')
100 """
101 value1 = self.request.form_ng.getOne(self.name, None)
102 value2 = self.request.form_ng.getOne('%s_dupe' % self.name, None)
103 if value1 != value2:
104 self._error = WidgetInputError(
105 self.context.__name__, self.label, PasswordMismatch()
106 )
107 raise self._error
108
109 # If the user hasn't entered a password, we use the existing one
110 # if it is there. If it is not there, we are creating a new password
111 # and we raise an error
112 if not value1:
113 current_password = self._getCurrentPassword()
114 if current_password:
115 return current_password
116 else:
117 self._error = WidgetInputError(
118 self.context.__name__, self.label,
119 RequiredPasswordMissing()
120 )
121 raise self._error
122
123 # Do any other validation
124 value = PasswordWidget.getInputValue(self)
125 assert value == value1, 'Form system has changed'
126
127 # If we have matching plaintext, encrypt it and return the password
128 encryptor = getUtility(IPasswordEncryptor)
129 return encryptor.encrypt(value)
1300
=== removed file 'lib/lp/app/widgets/templates/passwordchange.pt'
--- lib/lp/app/widgets/templates/passwordchange.pt 2009-07-28 22:44:12 +0000
+++ lib/lp/app/widgets/templates/passwordchange.pt 1970-01-01 00:00:00 +0000
@@ -1,35 +0,0 @@
1<tal:comment condition="nothing">
2 Note that we don't use view/_getFormValue to fill in the value
3 for security reasons
4</tal:comment>
5<table style="margin-top:1em;">
6 <tr>
7 <td colspan="2">
8 <label tal:content="string: ${view/label}:"
9 tal:attributes="for view/name;">Password:</label><br />
10 <input type="password" value="" tal:attributes="
11 name view/name;
12 id view/name;
13 class view/cssClass;
14 style view/style;
15 size view/displayWidth;
16 maxlength view/displayMaxWidth|nothing;
17 "/>
18 </td>
19 </tr>
20 <tr>
21 <td colspan="2">
22 <label tal:attributes="for string:${view/name}_dupe;">
23 Retype the password:
24 </label><br />
25 <input type="password" value="" tal:attributes="
26 name string:${view/name}_dupe;
27 id string:${view/name}_dupe;
28 class view/cssClass;
29 style view/style;
30 size view/displayWidth;
31 maxlength view/displayMaxWidth|nothing;
32 "/>
33 </td>
34 </tr>
35</table>
360
=== modified file 'lib/lp/app/widgets/tests/test_widget_doctests.py'
--- lib/lp/app/widgets/tests/test_widget_doctests.py 2011-12-28 17:03:06 +0000
+++ lib/lp/app/widgets/tests/test_widget_doctests.py 2012-01-17 00:44:26 +0000
@@ -12,7 +12,6 @@
12def test_suite():12def test_suite():
13 suite = unittest.TestSuite()13 suite = unittest.TestSuite()
14 suite.layer = DatabaseFunctionalLayer14 suite.layer = DatabaseFunctionalLayer
15 suite.addTest(doctest.DocTestSuite('lp.app.widgets.password'))
16 suite.addTest(doctest.DocTestSuite('lp.app.widgets.textwidgets'))15 suite.addTest(doctest.DocTestSuite('lp.app.widgets.textwidgets'))
17 suite.addTest(doctest.DocTestSuite('lp.app.widgets.date'))16 suite.addTest(doctest.DocTestSuite('lp.app.widgets.date'))
18 return suite17 return suite
1918
=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py 2011-12-30 06:14:56 +0000
+++ lib/lp/registry/browser/person.py 2012-01-17 00:44:26 +0000
@@ -155,7 +155,6 @@
155 LaunchpadRadioWidgetWithDescription,155 LaunchpadRadioWidgetWithDescription,
156 )156 )
157from lp.app.widgets.location import LocationWidget157from lp.app.widgets.location import LocationWidget
158from lp.app.widgets.password import PasswordChangeWidget
159from lp.blueprints.browser.specificationtarget import HasSpecificationsView158from lp.blueprints.browser.specificationtarget import HasSpecificationsView
160from lp.blueprints.enums import SpecificationFilter159from lp.blueprints.enums import SpecificationFilter
161from lp.bugs.browser.bugtask import BugTaskSearchListingView160from lp.bugs.browser.bugtask import BugTaskSearchListingView
@@ -971,8 +970,7 @@
971970
972 usedfor = IPersonEditMenu971 usedfor = IPersonEditMenu
973 facet = 'overview'972 facet = 'overview'
974 links = ('personal', 'email_settings',973 links = ('personal', 'email_settings', 'sshkeys', 'gpgkeys')
975 'sshkeys', 'gpgkeys', 'passwords')
976974
977 def personal(self):975 def personal(self):
978 target = '+edit'976 target = '+edit'
@@ -1277,7 +1275,6 @@
1277 label = "Review person's account"1275 label = "Review person's account"
1278 custom_widget(1276 custom_widget(
1279 'status_comment', TextAreaWidget, height=5, width=60)1277 'status_comment', TextAreaWidget, height=5, width=60)
1280 custom_widget('password', PasswordChangeWidget)
12811278
1282 def __init__(self, context, request):1279 def __init__(self, context, request):
1283 """See `LaunchpadEditFormView`."""1280 """See `LaunchpadEditFormView`."""
@@ -1290,7 +1287,7 @@
1290 # Set fields to be displayed.1287 # Set fields to be displayed.
1291 self.field_names = ['status', 'status_comment']1288 self.field_names = ['status', 'status_comment']
1292 if self.viewed_by_admin:1289 if self.viewed_by_admin:
1293 self.field_names = ['displayname', 'password'] + self.field_names1290 self.field_names = ['displayname'] + self.field_names
12941291
1295 @property1292 @property
1296 def is_viewing_person(self):1293 def is_viewing_person(self):
@@ -1334,10 +1331,8 @@
1334 """Update the IAccount."""1331 """Update the IAccount."""
1335 if (data['status'] == AccountStatus.SUSPENDED1332 if (data['status'] == AccountStatus.SUSPENDED
1336 and self.context.status != AccountStatus.SUSPENDED):1333 and self.context.status != AccountStatus.SUSPENDED):
1337 # Setting the password to a clear value makes it impossible to1334 # The preferred email address is removed to ensure no email
1338 # login. The preferred email address is removed to ensure no1335 # is sent to the user.
1339 # email is sent to the user.
1340 data['password'] = 'invalid'
1341 self.person.setPreferredEmail(None)1336 self.person.setPreferredEmail(None)
1342 self.request.response.addInfoNotification(1337 self.request.response.addInfoNotification(
1343 u'The account "%s" has been suspended.' % (1338 u'The account "%s" has been suspended.' % (
13441339
=== modified file 'lib/lp/registry/browser/tests/person-admin-views.txt'
--- lib/lp/registry/browser/tests/person-admin-views.txt 2011-12-24 17:49:30 +0000
+++ lib/lp/registry/browser/tests/person-admin-views.txt 2012-01-17 00:44:26 +0000
@@ -31,7 +31,6 @@
31The PersonAdministerView allows Launchpad admins to change some31The PersonAdministerView allows Launchpad admins to change some
32of a user's attributes.32of a user's attributes.
3333
34 >>> old_password = user.password
35 >>> form = {34 >>> form = {
36 ... 'field.name': 'zaphod',35 ... 'field.name': 'zaphod',
37 ... 'field.displayname': 'Zaphod Beeblebrox',36 ... 'field.displayname': 'Zaphod Beeblebrox',
@@ -78,7 +77,7 @@
78 >>> print view.errors77 >>> print view.errors
79 []78 []
80 >>> view.field_names79 >>> view.field_names
81 ['displayname', 'password', 'status', 'status_comment']80 ['displayname', 'status', 'status_comment']
82 >>> view.label81 >>> view.label
83 "Review person's account"82 "Review person's account"
8483
@@ -97,32 +96,25 @@
9796
98The admin can change the user's account information.97The admin can change the user's account information.
9998
100 >>> old_password = user.password
101 >>> form = {99 >>> form = {
102 ... 'field.displayname': 'The Zaphod Beeblebrox',100 ... 'field.displayname': 'The Zaphod Beeblebrox',
103 ... 'field.password': 'secret1',
104 ... 'field.password_dupe': 'secret1',
105 ... 'field.status': 'ACTIVE',101 ... 'field.status': 'ACTIVE',
106 ... 'field.actions.change': 'Change',102 ... 'field.actions.change': 'Change',
107 ... }103 ... }
108 >>> view = create_initialized_view(user, '+reviewaccount', form=form)104 >>> view = create_initialized_view(user, '+reviewaccount', form=form)
109 >>> print view.errors105 >>> print view.errors
110 []106 []
111 >>> user.password != old_password
112 True
113 >>> print user.displayname107 >>> print user.displayname
114 Zaphod Beeblebrox108 Zaphod Beeblebrox
115109
116An admin can suspend a user's account using the +reviewaccount view. When110An admin can suspend a user's account using the +reviewaccount view. When
117an account is suspended, the preferred email address is disabled and the111an account is suspended, the preferred email address is disabled.
118password is changed too.
119112
120 >>> user.account_status113 >>> user.account_status
121 <DBItem AccountStatus.ACTIVE, ...>114 <DBItem AccountStatus.ACTIVE, ...>
122 >>> print user.account_status_comment115 >>> print user.account_status_comment
123 None116 None
124117
125 >>> old_password = user.password
126 >>> form = {118 >>> form = {
127 ... 'field.displayname': 'Zaphod Beeblebrox',119 ... 'field.displayname': 'Zaphod Beeblebrox',
128 ... 'field.status': 'SUSPENDED',120 ... 'field.status': 'SUSPENDED',
@@ -137,16 +129,13 @@
137 <DBItem AccountStatus.SUSPENDED, ...>129 <DBItem AccountStatus.SUSPENDED, ...>
138 >>> print user.account_status_comment130 >>> print user.account_status_comment
139 Wanted by the galactic police.131 Wanted by the galactic police.
140 >>> user.password != old_password
141 True
142 >>> print user.preferredemail132 >>> print user.preferredemail
143 None133 None
144134
145An admin can reactivate a disabled user's account too. Unlike the act of135An admin can reactivate a disabled user's account too. Unlike the act of
146suspension, reactivation does not change the user's password or email136suspension, reactivation does not change the user's email addresses; the
147addresses; the user must use the reset password feature to restore these.137user must log in to restore these.
148138
149 >>> old_password = user.password
150 >>> form = {139 >>> form = {
151 ... 'field.displayname': 'Zaphod Beeblebrox',140 ... 'field.displayname': 'Zaphod Beeblebrox',
152 ... 'field.status': 'ACTIVE',141 ... 'field.status': 'ACTIVE',
@@ -160,7 +149,5 @@
160 <DBItem AccountStatus.ACTIVE, ...>149 <DBItem AccountStatus.ACTIVE, ...>
161 >>> print user.account_status_comment150 >>> print user.account_status_comment
162 Zaphod's a hoopy frood.151 Zaphod's a hoopy frood.
163 >>> user.password == old_password
164 True
165 >>> print user.preferredemail152 >>> print user.preferredemail
166 None153 None
167154
=== modified file 'lib/lp/registry/doc/person-account.txt'
--- lib/lp/registry/doc/person-account.txt 2012-01-03 12:44:03 +0000
+++ lib/lp/registry/doc/person-account.txt 2012-01-17 00:44:26 +0000
@@ -12,7 +12,9 @@
12process. Matsubara's account was created during a code import.12process. Matsubara's account was created during a code import.
1313
14 >>> from zope.component import getUtility14 >>> from zope.component import getUtility
15 >>> from lp.services.identity.interfaces.emailaddress import IEmailAddressSet15 >>> from lp.services.identity.interfaces.emailaddress import (
16 ... IEmailAddressSet,
17 ... )
16 >>> from lp.registry.interfaces.person import IPersonSet18 >>> from lp.registry.interfaces.person import IPersonSet
1719
18 >>> emailset = getUtility(IEmailAddressSet)20 >>> emailset = getUtility(IEmailAddressSet)
@@ -22,8 +24,6 @@
22 False24 False
23 >>> matsubara.account_status25 >>> matsubara.account_status
24 <DBItem AccountStatus.NOACCOUNT, ...>26 <DBItem AccountStatus.NOACCOUNT, ...>
25 >>> print matsubara.password
26 None
27 >>> print matsubara.preferredemail27 >>> print matsubara.preferredemail
28 None28 None
2929
3030
=== modified file 'lib/lp/registry/doc/person.txt'
--- lib/lp/registry/doc/person.txt 2012-01-05 00:23:45 +0000
+++ lib/lp/registry/doc/person.txt 2012-01-17 00:44:26 +0000
@@ -142,7 +142,7 @@
142 >>> from lp.services.identity.model.account import Account142 >>> from lp.services.identity.model.account import Account
143 >>> from lp.services.database.lpstorm import IMasterStore143 >>> from lp.services.database.lpstorm import IMasterStore
144 >>> account = IMasterStore(Account).get(Account, p.accountID)144 >>> account = IMasterStore(Account).get(Account, p.accountID)
145 >>> account.reactivate("Activated by doc test.", password=p.password)145 >>> account.reactivate("Activated by doc test.", account.password)
146 >>> p.account_status146 >>> p.account_status
147 <DBItem AccountStatus.ACTIVE...147 <DBItem AccountStatus.ACTIVE...
148148
@@ -485,9 +485,6 @@
485 >>> verifyObject(ITeam, not_a_person)485 >>> verifyObject(ITeam, not_a_person)
486 True486 True
487487
488 >>> print removeSecurityProxy(not_a_person).password
489 None
490
491The team owner is also added as an administrator of its team.488The team owner is also added as an administrator of its team.
492489
493 >>> [member.name for member in not_a_person.adminmembers]490 >>> [member.name for member in not_a_person.adminmembers]
494491
=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py 2011-12-30 06:14:56 +0000
+++ lib/lp/registry/interfaces/person.py 2012-01-17 00:44:26 +0000
@@ -145,7 +145,6 @@
145 is_public_person_or_closed_team,145 is_public_person_or_closed_team,
146 LogoImageUpload,146 LogoImageUpload,
147 MugshotImageUpload,147 MugshotImageUpload,
148 PasswordField,
149 PersonChoice,148 PersonChoice,
150 PublicPersonChoice,149 PublicPersonChoice,
151 StrippedTextLine,150 StrippedTextLine,
@@ -766,8 +765,6 @@
766 """IPerson attributes that require launchpad.View permission."""765 """IPerson attributes that require launchpad.View permission."""
767 account = Object(schema=IAccount)766 account = Object(schema=IAccount)
768 accountID = Int(title=_('Account ID'), required=True, readonly=True)767 accountID = Int(title=_('Account ID'), required=True, readonly=True)
769 password = PasswordField(
770 title=_('Password'), required=True, readonly=False)
771 karma = exported(768 karma = exported(
772 Int(title=_('Karma'), readonly=True,769 Int(title=_('Karma'), readonly=True,
773 description=_('The cached total karma for this person.')))770 description=_('The cached total karma for this person.')))
774771
=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py 2012-01-12 08:13:59 +0000
+++ lib/lp/registry/model/person.py 2012-01-17 00:44:26 +0000
@@ -260,10 +260,7 @@
260 IEmailAddressSet,260 IEmailAddressSet,
261 InvalidEmailAddress,261 InvalidEmailAddress,
262 )262 )
263from lp.services.identity.model.account import (263from lp.services.identity.model.account import Account
264 Account,
265 AccountPassword,
266 )
267from lp.services.identity.model.emailaddress import (264from lp.services.identity.model.emailaddress import (
268 EmailAddress,265 EmailAddress,
269 HasOwnerMixin,266 HasOwnerMixin,
@@ -528,24 +525,6 @@
528 mugshot = ForeignKey(525 mugshot = ForeignKey(
529 dbName='mugshot', foreignKey='LibraryFileAlias', default=None)526 dbName='mugshot', foreignKey='LibraryFileAlias', default=None)
530527
531 def _get_password(self):
532 # We have to remove the security proxy because the password is
533 # needed before we are authenticated. I'm not overly worried because
534 # this method is scheduled for demolition -- StuartBishop 20080514
535 password = IStore(AccountPassword).find(
536 AccountPassword, accountID=self.accountID).one()
537 if password is None:
538 return None
539 else:
540 return password.password
541
542 def _set_password(self, value):
543 account = IMasterStore(Account).get(Account, self.accountID)
544 assert account is not None, 'No account for this Person.'
545 account.password = value
546
547 password = property(_get_password, _set_password)
548
549 def _get_account_status(self):528 def _get_account_status(self):
550 account = IStore(Account).get(Account, self.accountID)529 account = IStore(Account).get(Account, self.accountID)
551 if account is not None:530 if account is not None:
552531
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2012-01-05 09:02:37 +0000
+++ lib/lp/testing/factory.py 2012-01-17 00:44:26 +0000
@@ -647,9 +647,6 @@
647 if homepage_content is not None:647 if homepage_content is not None:
648 naked_person.homepage_content = homepage_content648 naked_person.homepage_content = homepage_content
649649
650 assert person.password is not None, (
651 'Password not set. Wrong default auth Store?')
652
653 if (time_zone is not None or latitude is not None or650 if (time_zone is not None or latitude is not None or
654 longitude is not None):651 longitude is not None):
655 naked_person.setLocation(latitude, longitude, time_zone, person)652 naked_person.setLocation(latitude, longitude, time_zone, person)