launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06099
[Merge] lp:~wgrant/launchpad/destroy-person-password into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/destroy-person-password into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/destroy-person-password/+merge/88685
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.
--
https://code.launchpad.net/~wgrant/launchpad/destroy-person-password/+merge/88685
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/destroy-person-password into lp:launchpad.
=== 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-16 13:30:40 +0000
@@ -132,11 +132,9 @@
>>> from zope.app.form.browser import TextWidget
>>> from lp.app.browser.launchpadform import custom_widget
- >>> from lp.app.widgets.password import PasswordChangeWidget
>>> class FormTestView3(LaunchpadFormView):
... schema = IFormTest
- ... custom_widget('password', PasswordChangeWidget)
... custom_widget('displayname', TextWidget, displayWidth=50)
>>> context = FormTest()
@@ -149,7 +147,7 @@
>>> view.widgets['displayname'].displayWidth
50
>>> view.widgets['password']
- <...PasswordChangeWidget object at ...>
+ <...TextWidget object at ...>
== Using Another Context ==
=== 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 @@
-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""
-Custom Password widgets.
-
-TODO: Consider folding this back into Zope3 -- StuartBishop 20050520
-"""
-
-__metaclass__ = type
-
-from z3c.ptcompat import ViewPageTemplateFile
-from zope.app.form.browser import PasswordWidget
-from zope.app.form.browser.interfaces import ITextBrowserWidget
-from zope.app.form.interfaces import WidgetInputError
-from zope.component import getUtility
-from zope.interface import implements
-from zope.schema.interfaces import ValidationError
-
-from lp import _
-from lp.services.webapp.interfaces import (
- IMultiLineWidgetLayout,
- IPasswordEncryptor,
- )
-
-
-class PasswordMismatch(ValidationError):
- __doc__ = _("Passwords do not match.")
-
- def __repr__(self):
- return repr(self.__doc__)
-
-
-class RequiredPasswordMissing(ValidationError):
- __doc__ = _("You must enter a password.")
-
- def __repr__(self):
- return repr(self.__doc__)
-
-
-class PasswordChangeWidget(PasswordWidget):
- """A password change widget.
-
- Text is not echoed to the user, and two text boxes are used to ensure
- the password is entered correctly.
- """
- implements(ITextBrowserWidget, IMultiLineWidgetLayout)
- type = 'password change'
- display_label = False
-
- __call__ = ViewPageTemplateFile('templates/passwordchange.pt')
-
- def hasInput(self):
- """We always have input if there is an existing value
-
- No input indicates unchanged.
- """
- if PasswordWidget.hasInput(self):
- return True
-
- # If we don't have input from the user, we lie because we will
- # use the existing value.
- return bool(self._getCurrentPassword())
-
- def _getCurrentPassword(self):
- # Yesh... indirection up the wazoo to do something this simple.
- # Returns the current password.
- return self.context.get(self.context.context) or None
-
- def getInputValue(self):
- """Ensure both text boxes contain the same value and inherited checks
-
- >>> from lp.services.webapp.servers import (
- ... LaunchpadTestRequest)
- >>> from zope.schema import Field
- >>> field = Field(__name__='foo', title=u'Foo')
-
- The widget will only return a value if both of the text boxes
- contain the same value. It returns the value encrypted.
-
- >>> request = LaunchpadTestRequest(form={
- ... 'field.foo': u'My Password',
- ... 'field.foo_dupe': u'My Password',
- ... })
- >>> widget = PasswordChangeWidget(field, request)
- >>> crypted_pw = widget.getInputValue()
- >>> encryptor = getUtility(IPasswordEncryptor)
- >>> encryptor.validate(u'My Password', crypted_pw)
- True
-
- Otherwise it raises the exception required by IInputWidget
-
- >>> request = LaunchpadTestRequest(form={
- ... 'field.foo': u'My Password', 'field.foo_dupe': u'No Match'})
- >>> widget = PasswordChangeWidget(field, request)
- >>> widget.getInputValue()
- Traceback (most recent call last):
- [...]
- WidgetInputError: ('foo', u'Foo', u'Passwords do not match.')
- """
- value1 = self.request.form_ng.getOne(self.name, None)
- value2 = self.request.form_ng.getOne('%s_dupe' % self.name, None)
- if value1 != value2:
- self._error = WidgetInputError(
- self.context.__name__, self.label, PasswordMismatch()
- )
- raise self._error
-
- # If the user hasn't entered a password, we use the existing one
- # if it is there. If it is not there, we are creating a new password
- # and we raise an error
- if not value1:
- current_password = self._getCurrentPassword()
- if current_password:
- return current_password
- else:
- self._error = WidgetInputError(
- self.context.__name__, self.label,
- RequiredPasswordMissing()
- )
- raise self._error
-
- # Do any other validation
- value = PasswordWidget.getInputValue(self)
- assert value == value1, 'Form system has changed'
-
- # If we have matching plaintext, encrypt it and return the password
- encryptor = getUtility(IPasswordEncryptor)
- return encryptor.encrypt(value)
=== 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 @@
-<tal:comment condition="nothing">
- Note that we don't use view/_getFormValue to fill in the value
- for security reasons
-</tal:comment>
-<table style="margin-top:1em;">
- <tr>
- <td colspan="2">
- <label tal:content="string: ${view/label}:"
- tal:attributes="for view/name;">Password:</label><br />
- <input type="password" value="" tal:attributes="
- name view/name;
- id view/name;
- class view/cssClass;
- style view/style;
- size view/displayWidth;
- maxlength view/displayMaxWidth|nothing;
- "/>
- </td>
- </tr>
- <tr>
- <td colspan="2">
- <label tal:attributes="for string:${view/name}_dupe;">
- Retype the password:
- </label><br />
- <input type="password" value="" tal:attributes="
- name string:${view/name}_dupe;
- id string:${view/name}_dupe;
- class view/cssClass;
- style view/style;
- size view/displayWidth;
- maxlength view/displayMaxWidth|nothing;
- "/>
- </td>
- </tr>
-</table>
=== 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-16 13:30:40 +0000
@@ -12,7 +12,6 @@
def test_suite():
suite = unittest.TestSuite()
suite.layer = DatabaseFunctionalLayer
- suite.addTest(doctest.DocTestSuite('lp.app.widgets.password'))
suite.addTest(doctest.DocTestSuite('lp.app.widgets.textwidgets'))
suite.addTest(doctest.DocTestSuite('lp.app.widgets.date'))
return suite
=== 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-16 13:30:40 +0000
@@ -155,7 +155,6 @@
LaunchpadRadioWidgetWithDescription,
)
from lp.app.widgets.location import LocationWidget
-from lp.app.widgets.password import PasswordChangeWidget
from lp.blueprints.browser.specificationtarget import HasSpecificationsView
from lp.blueprints.enums import SpecificationFilter
from lp.bugs.browser.bugtask import BugTaskSearchListingView
@@ -971,8 +970,7 @@
usedfor = IPersonEditMenu
facet = 'overview'
- links = ('personal', 'email_settings',
- 'sshkeys', 'gpgkeys', 'passwords')
+ links = ('personal', 'email_settings', 'sshkeys', 'gpgkeys')
def personal(self):
target = '+edit'
@@ -1277,7 +1275,6 @@
label = "Review person's account"
custom_widget(
'status_comment', TextAreaWidget, height=5, width=60)
- custom_widget('password', PasswordChangeWidget)
def __init__(self, context, request):
"""See `LaunchpadEditFormView`."""
@@ -1290,7 +1287,7 @@
# Set fields to be displayed.
self.field_names = ['status', 'status_comment']
if self.viewed_by_admin:
- self.field_names = ['displayname', 'password'] + self.field_names
+ self.field_names = ['displayname'] + self.field_names
@property
def is_viewing_person(self):
@@ -1334,10 +1331,8 @@
"""Update the IAccount."""
if (data['status'] == AccountStatus.SUSPENDED
and self.context.status != AccountStatus.SUSPENDED):
- # Setting the password to a clear value makes it impossible to
- # login. The preferred email address is removed to ensure no
- # email is sent to the user.
- data['password'] = 'invalid'
+ # The preferred email address is removed to ensure no email
+ # is sent to the user.
self.person.setPreferredEmail(None)
self.request.response.addInfoNotification(
u'The account "%s" has been suspended.' % (
=== 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-16 13:30:40 +0000
@@ -31,7 +31,6 @@
The PersonAdministerView allows Launchpad admins to change some
of a user's attributes.
- >>> old_password = user.password
>>> form = {
... 'field.name': 'zaphod',
... 'field.displayname': 'Zaphod Beeblebrox',
@@ -78,7 +77,7 @@
>>> print view.errors
[]
>>> view.field_names
- ['displayname', 'password', 'status', 'status_comment']
+ ['displayname', 'status', 'status_comment']
>>> view.label
"Review person's account"
@@ -97,32 +96,25 @@
The admin can change the user's account information.
- >>> old_password = user.password
>>> form = {
... 'field.displayname': 'The Zaphod Beeblebrox',
- ... 'field.password': 'secret1',
- ... 'field.password_dupe': 'secret1',
... 'field.status': 'ACTIVE',
... 'field.actions.change': 'Change',
... }
>>> view = create_initialized_view(user, '+reviewaccount', form=form)
>>> print view.errors
[]
- >>> user.password != old_password
- True
>>> print user.displayname
Zaphod Beeblebrox
An admin can suspend a user's account using the +reviewaccount view. When
-an account is suspended, the preferred email address is disabled and the
-password is changed too.
+an account is suspended, the preferred email address is disabled.
>>> user.account_status
<DBItem AccountStatus.ACTIVE, ...>
>>> print user.account_status_comment
None
- >>> old_password = user.password
>>> form = {
... 'field.displayname': 'Zaphod Beeblebrox',
... 'field.status': 'SUSPENDED',
@@ -137,16 +129,13 @@
<DBItem AccountStatus.SUSPENDED, ...>
>>> print user.account_status_comment
Wanted by the galactic police.
- >>> user.password != old_password
- True
>>> print user.preferredemail
None
An admin can reactivate a disabled user's account too. Unlike the act of
-suspension, reactivation does not change the user's password or email
-addresses; the user must use the reset password feature to restore these.
+suspension, reactivation does not change the user's email addresses; the
+user must log in to restore these.
- >>> old_password = user.password
>>> form = {
... 'field.displayname': 'Zaphod Beeblebrox',
... 'field.status': 'ACTIVE',
@@ -160,7 +149,5 @@
<DBItem AccountStatus.ACTIVE, ...>
>>> print user.account_status_comment
Zaphod's a hoopy frood.
- >>> user.password == old_password
- True
>>> print user.preferredemail
None
=== 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-16 13:30:40 +0000
@@ -12,7 +12,9 @@
process. Matsubara's account was created during a code import.
>>> from zope.component import getUtility
- >>> from lp.services.identity.interfaces.emailaddress import IEmailAddressSet
+ >>> from lp.services.identity.interfaces.emailaddress import (
+ ... IEmailAddressSet,
+ ... )
>>> from lp.registry.interfaces.person import IPersonSet
>>> emailset = getUtility(IEmailAddressSet)
@@ -22,8 +24,6 @@
False
>>> matsubara.account_status
<DBItem AccountStatus.NOACCOUNT, ...>
- >>> print matsubara.password
- None
>>> print matsubara.preferredemail
None
=== 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-16 13:30:40 +0000
@@ -145,7 +145,6 @@
is_public_person_or_closed_team,
LogoImageUpload,
MugshotImageUpload,
- PasswordField,
PersonChoice,
PublicPersonChoice,
StrippedTextLine,
@@ -766,8 +765,6 @@
"""IPerson attributes that require launchpad.View permission."""
account = Object(schema=IAccount)
accountID = Int(title=_('Account ID'), required=True, readonly=True)
- password = PasswordField(
- title=_('Password'), required=True, readonly=False)
karma = exported(
Int(title=_('Karma'), readonly=True,
description=_('The cached total karma for this person.')))
=== 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-16 13:30:40 +0000
@@ -260,10 +260,7 @@
IEmailAddressSet,
InvalidEmailAddress,
)
-from lp.services.identity.model.account import (
- Account,
- AccountPassword,
- )
+from lp.services.identity.model.account import Account
from lp.services.identity.model.emailaddress import (
EmailAddress,
HasOwnerMixin,
@@ -528,24 +525,6 @@
mugshot = ForeignKey(
dbName='mugshot', foreignKey='LibraryFileAlias', default=None)
- def _get_password(self):
- # We have to remove the security proxy because the password is
- # needed before we are authenticated. I'm not overly worried because
- # this method is scheduled for demolition -- StuartBishop 20080514
- password = IStore(AccountPassword).find(
- AccountPassword, accountID=self.accountID).one()
- if password is None:
- return None
- else:
- return password.password
-
- def _set_password(self, value):
- account = IMasterStore(Account).get(Account, self.accountID)
- assert account is not None, 'No account for this Person.'
- account.password = value
-
- password = property(_get_password, _set_password)
-
def _get_account_status(self):
account = IStore(Account).get(Account, self.accountID)
if account is not None:
=== 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-16 13:30:40 +0000
@@ -647,9 +647,6 @@
if homepage_content is not None:
naked_person.homepage_content = homepage_content
- assert person.password is not None, (
- 'Password not set. Wrong default auth Store?')
-
if (time_zone is not None or latitude is not None or
longitude is not None):
naked_person.setLocation(latitude, longitude, time_zone, person)
