launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06116
[Merge] lp:~wgrant/launchpad/no-passwords into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/no-passwords into lp:launchpad with lp:~wgrant/launchpad/hardcoded-password as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/no-passwords/+merge/88971
The password infrastructure (AccountPassword and friends) is no longer used. Good riddance.
--
https://code.launchpad.net/~wgrant/launchpad/no-passwords/+merge/88971
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/no-passwords into lp:launchpad.
=== modified file 'lib/lp/registry/doc/person-account.txt'
--- lib/lp/registry/doc/person-account.txt 2012-01-15 11:50:42 +0000
+++ lib/lp/registry/doc/person-account.txt 2012-01-18 00:55:42 +0000
@@ -31,17 +31,16 @@
the profile. Sample Person cannot claim it.
>>> login('test@xxxxxxxxxxxxx')
- >>> matsubara.account.reactivate(comment="test", password='ok')
+ >>> matsubara.account.reactivate(comment="test")
Traceback (most recent call last):
...
Unauthorized: ...'launchpad.Special')
-Matsubara can. A password and a preferred email address must be passed
-as arguments.
+Matsubara can.
>>> from zope.security.proxy import removeSecurityProxy
>>> login('matsubara@xxxxxxxxxxxx')
- >>> matsubara.account.reactivate(comment="test", password='ok')
+ >>> matsubara.account.reactivate(comment="test")
>>> matsubara.setPreferredEmail(emailaddress)
>>> import transaction
>>> transaction.commit()
@@ -215,7 +214,6 @@
>>> foobar.reactivate(
... 'User reactivated the account using reset password.',
- ... password="ok",
... preferred_email=foobar_preferredemail)
>>> transaction.commit() # To see the changes on other stores.
>>> foobar.account.status
=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py 2012-01-15 11:50:42 +0000
+++ lib/lp/registry/interfaces/person.py 2012-01-18 00:55:42 +0000
@@ -1817,7 +1817,6 @@
"""Deactivate this person's Launchpad account.
Deactivating an account means:
- - Setting its password to NULL;
- Removing the user from all teams he's a member of;
- Changing all his email addresses' status to NEW;
- Revoking Code of Conduct signatures of that user;
@@ -1827,17 +1826,16 @@
:param comment: An explanation of why the account status changed.
"""
- def reactivate(comment, password, preferred_email):
+ def reactivate(comment, preferred_email):
"""Reactivate this person and its account.
- Set the account status to ACTIVE, the account's password to the given
- one and its preferred email address.
+ Set the account status to ACTIVE, and update the preferred email
+ address.
If the person's name contains a -deactivatedaccount suffix (usually
added by `IPerson`.deactivateAccount(), it is removed.
:param comment: An explanation of why the account status changed.
- :param password: The user's password.
:param preferred_email: The `EmailAddress` to set as the account's
preferred email address. It cannot be None.
"""
@@ -2070,7 +2068,6 @@
def createPersonAndEmail(
email, rationale, comment=None, name=None, displayname=None,
- password=None, passwordEncrypted=False,
hide_email_addresses=False, registrant=None):
"""Create and return an `IPerson` and `IEmailAddress`.
@@ -2091,9 +2088,6 @@
(e.g. "when the foo package was imported into Ubuntu Breezy").
:param name: The person's name.
:param displayname: The person's displayname.
- :param password: The person's password.
- :param passwordEncrypted: Whether or not the given password is
- encrypted.
:param registrant: The user who created this person, if any.
:param hide_email_addresses: Whether or not Launchpad should hide the
person's email addresses from other users.
=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py 2012-01-16 13:11:00 +0000
+++ lib/lp/registry/model/person.py 2012-01-18 00:55:42 +0000
@@ -2516,10 +2516,10 @@
else:
return None
- def reactivate(self, comment, password, preferred_email):
+ def reactivate(self, comment, preferred_email):
"""See `IPersonSpecialRestricted`."""
account = IMasterObject(self.account)
- account.reactivate(comment, password)
+ account.reactivate(comment)
self.setPreferredEmail(preferred_email)
if '-deactivatedaccount' in self.name:
# The name was changed by deactivateAccount(). Restore the
@@ -3153,9 +3153,7 @@
elif person.account.status in [AccountStatus.DEACTIVATED,
AccountStatus.NOACCOUNT]:
- password = ''
- removeSecurityProxy(person.account).reactivate(
- comment, password)
+ removeSecurityProxy(person.account).reactivate(comment)
removeSecurityProxy(person).setPreferredEmail(email)
db_updated = True
else:
@@ -3186,8 +3184,7 @@
return team
def createPersonAndEmail(
- self, email, rationale, comment=None, name=None,
- displayname=None, password=None, passwordEncrypted=False,
+ self, email, rationale, comment=None, name=None, displayname=None,
hide_email_addresses=False, registrant=None):
"""See `IPersonSet`."""
@@ -3208,9 +3205,7 @@
# Convert the PersonCreationRationale to an AccountCreationRationale
account_rationale = getattr(AccountCreationRationale, rationale.name)
- account = getUtility(IAccountSet).new(
- account_rationale, displayname, password=password,
- password_is_encrypted=passwordEncrypted)
+ account = getUtility(IAccountSet).new(account_rationale, displayname)
person = self._newPerson(
name, displayname, hide_email_addresses, rationale=rationale,
=== modified file 'lib/lp/services/database/doc/storm.txt'
--- lib/lp/services/database/doc/storm.txt 2012-01-06 11:08:30 +0000
+++ lib/lp/services/database/doc/storm.txt 2012-01-18 00:55:42 +0000
@@ -16,10 +16,6 @@
... ISlaveStore,
... IStore,
... )
- >>> from lp.services.identity.model.account import (
- ... Account,
- ... AccountPassword,
- ... )
>>> from lp.services.identity.model.emailaddress import EmailAddress
>>> from zope.security.proxy import ProxyFactory
>>> from lp.registry.interfaces.person import IPersonSet
=== modified file 'lib/lp/services/identity/configure.zcml'
--- lib/lp/services/identity/configure.zcml 2011-12-24 17:49:30 +0000
+++ lib/lp/services/identity/configure.zcml 2012-01-18 00:55:42 +0000
@@ -59,7 +59,7 @@
set_attributes="status date_status_set status_comment"/>
<require
permission="launchpad.Edit"
- set_attributes="displayname password"/>
+ set_attributes="displayname"/>
</class>
<securedutility
=== modified file 'lib/lp/services/identity/doc/account.txt'
--- lib/lp/services/identity/doc/account.txt 2012-01-03 11:08:31 +0000
+++ lib/lp/services/identity/doc/account.txt 2012-01-18 00:55:42 +0000
@@ -83,22 +83,6 @@
>>> account
<Account 'No Privileges Person' (Active account)>
-It also has an encrypted password.
-
- >>> print account.password
- K7Qmeansl6RbuPfulfcmyDQOzp70OxVh5Fcf
-
-Ensure the password changes are sticky, as this is a property hiding the
-AccountPassword table.
-
- >>> account.password = None
- >>> print account.password
- None
-
- >>> account.password = u'K7Qmeansl6RbuPfulfcmyDQOzp70OxVh5Fcf'
- >>> print account.password
- K7Qmeansl6RbuPfulfcmyDQOzp70OxVh5Fcf
-
The account has other metadata.
>>> account.date_created
@@ -152,36 +136,12 @@
>>> from lp.services.identity.interfaces.account import (
... AccountCreationRationale)
- >>> from storm.store import Store
>>> login('admin@xxxxxxxxxxxxx')
- >>> passwordless_account = account_set.new(
- ... AccountCreationRationale.USER_CREATED, 'Passwordless')
+ >>> new_account = account_set.new(
+ ... AccountCreationRationale.USER_CREATED, 'New Account')
>>> transaction.commit()
- >>> print passwordless_account.creation_rationale.name
+ >>> print new_account.creation_rationale.name
USER_CREATED
- >>> print passwordless_account.displayname
- Passwordless
- >>> print passwordless_account.password
- None
-
-The new() method accepts the optional parameters of password and
-password_is_encrypted. If password_is_encrypted is False, the default,
-then the method encrypts it for us.
-
- >>> passworded_account = account_set.new(
- ... AccountCreationRationale.OWNER_CREATED_LAUNCHPAD , 'Passworded',
- ... password=u'clear_password')
- >>> Store.of(passworded_account).flush()
- >>> passworded_account.password == u'clear_password'
- False
-
-The method does not encrypt the password if told that it is already
-encrypted, by setting password_is_encrypted to True.
-
- >>> clear_account = account_set.new(
- ... AccountCreationRationale.OWNER_CREATED_LAUNCHPAD , 'Clear',
- ... password=u'clear_password', password_is_encrypted=True)
- >>> Store.of(clear_account).flush()
- >>> print clear_account.password
- clear_password
+ >>> print new_account.displayname
+ New Account
=== modified file 'lib/lp/services/identity/interfaces/account.py'
--- lib/lp/services/identity/interfaces/account.py 2012-01-03 11:42:33 +0000
+++ lib/lp/services/identity/interfaces/account.py 2012-01-18 00:55:42 +0000
@@ -35,10 +35,7 @@
)
from lp import _
-from lp.services.fields import (
- PasswordField,
- StrippedTextLine,
- )
+from lp.services.fields import StrippedTextLine
class AccountSuspendedError(Exception):
@@ -229,9 +226,6 @@
openid_identifiers = Attribute(_("Linked OpenId Identifiers"))
- password = PasswordField(
- title=_("Password."), readonly=False, required=True)
-
class IAccountSpecialRestricted(Interface):
"""Attributes of `IAccount` protected with launchpad.Special."""
@@ -244,14 +238,12 @@
title=_("Why are you deactivating your account?"),
required=False, readonly=False)
- def reactivate(comment, password):
+ def reactivate(comment):
"""Activate this account.
- Set the account status to ACTIVE, the account's password to the given
- one and its preferred email address.
+ Set the account status to ACTIVE.
:param comment: An explanation of why the account status changed.
- :param password: The user's password.
"""
@@ -262,16 +254,11 @@
class IAccountSet(Interface):
"""Creation of and access to `IAccount` providers."""
- def new(rationale, displayname, password=None,
- password_is_encrypted=False):
+ def new(rationale, displayname):
"""Create a new `IAccount`.
:param rationale: An `AccountCreationRationale` value.
:param displayname: The user's display name.
- :param password: A password.
- :param password_is_encrypted: If True, the password parameter has
- already been encrypted using the `IPasswordEncryptor` utility.
- If False, the password will be encrypted automatically.
:return: The newly created `IAccount` provider.
"""
=== modified file 'lib/lp/services/identity/model/account.py'
--- lib/lp/services/identity/model/account.py 2012-01-03 11:08:31 +0000
+++ lib/lp/services/identity/model/account.py 2012-01-18 00:55:42 +0000
@@ -6,16 +6,11 @@
__metaclass__ = type
__all__ = [
'Account',
- 'AccountPassword',
'AccountSet',
]
-from sqlobject import (
- ForeignKey,
- StringCol,
- )
+from sqlobject import StringCol
from storm.locals import ReferenceSet
-from zope.component import getUtility
from zope.interface import implements
from lp.services.database.constants import UTC_NOW
@@ -33,7 +28,6 @@
IAccountSet,
)
from lp.services.openid.model.openididentifier import OpenIdIdentifier
-from lp.services.webapp.interfaces import IPasswordEncryptor
class Account(SQLBase):
@@ -61,55 +55,17 @@
return "<%s '%s' (%s)>" % (
self.__class__.__name__, displayname, self.status)
- def reactivate(self, comment, password):
+ def reactivate(self, comment):
"""See `IAccountSpecialRestricted`."""
self.status = AccountStatus.ACTIVE
self.status_comment = comment
- self.password = password
-
- # The password is actually stored in a separate table for security
- # reasons, so use a property to hide this implementation detail.
- def _get_password(self):
- # We have to force the switch to the auth store, because the
- # AccountPassword table is not visible via the main store
- # for security reasons.
- password = IStore(AccountPassword).find(
- AccountPassword, accountID=self.id).one()
- if password is None:
- return None
- else:
- return password.password
-
- def _set_password(self, value):
- # Making a modification, so we explicitly use the auth store master.
- store = IMasterStore(AccountPassword)
- password = store.find(
- AccountPassword, accountID=self.id).one()
-
- if value is not None and password is None:
- # There is currently no AccountPassword record and we need one.
- AccountPassword(accountID=self.id, password=value)
- elif value is None and password is not None:
- # There is an AccountPassword record that needs removing.
- store.remove(password)
- elif value is not None:
- # There is an AccountPassword record that needs updating.
- password.password = value
- elif value is None and password is None:
- # Nothing to do
- pass
- else:
- assert False, "This should not be reachable."
-
- password = property(_get_password, _set_password)
class AccountSet:
"""See `IAccountSet`."""
implements(IAccountSet)
- def new(self, rationale, displayname, password=None,
- password_is_encrypted=False, openid_identifier=None):
+ def new(self, rationale, displayname, openid_identifier=None):
"""See `IAccountSet`."""
account = Account(
@@ -123,12 +79,6 @@
identifier.identifier = openid_identifier
IMasterStore(OpenIdIdentifier).add(identifier)
- # Create the password record.
- if password is not None:
- if not password_is_encrypted:
- password = getUtility(IPasswordEncryptor).encrypt(password)
- AccountPassword(account=account, password=password)
-
return account
def get(self, id):
@@ -148,14 +98,3 @@
if account is None:
raise LookupError(openid_identifier)
return account
-
-
-class AccountPassword(SQLBase):
- """SQLObject wrapper to the AccountPassword table.
-
- Note that this class is not exported, as the existence of the
- AccountPassword table only needs to be known by this module.
- """
- account = ForeignKey(
- dbName='account', foreignKey='Account', alternateID=True)
- password = StringCol(dbName='password', notNull=True)
=== modified file 'lib/lp/services/webapp/authentication.py'
--- lib/lp/services/webapp/authentication.py 2012-01-18 00:55:42 +0000
+++ lib/lp/services/webapp/authentication.py 2012-01-18 00:55:42 +0000
@@ -9,13 +9,10 @@
'LaunchpadLoginSource',
'LaunchpadPrincipal',
'PlacelessAuthUtility',
- 'SSHADigestEncryptor',
]
import binascii
-import hashlib
-import random
from UserDict import UserDict
from contrib.oauth import OAuthRequest
@@ -47,7 +44,6 @@
CookieAuthPrincipalIdentifiedEvent,
DEFAULT_FLAVOR,
ILaunchpadPrincipal,
- IPasswordEncryptor,
IPlacelessAuthUtility,
IPlacelessLoginSource,
IStoreSelector,
@@ -187,49 +183,6 @@
return getUtility(IPlacelessLoginSource).getPrincipalByLogin(login)
-class SSHADigestEncryptor:
- """SSHA is a modification of the SHA digest scheme with a salt
- starting at byte 20 of the base64-encoded string.
- """
- implements(IPasswordEncryptor)
-
- # Source: http://developer.netscape.com/docs/technote/ldap/pass_sha.html
-
- saltLength = 20
-
- def generate_salt(self):
- # Salt can be any length, but not more than about 37 characters
- # because of limitations of the binascii module.
- # All 256 characters are available.
- salt = ''
- for n in range(self.saltLength):
- salt += chr(random.randrange(256))
- return salt
-
- def encrypt(self, plaintext, salt=None):
- plaintext = str(plaintext)
- if salt is None:
- salt = self.generate_salt()
- v = binascii.b2a_base64(
- hashlib.sha1(plaintext + salt).digest() + salt)
- return v[:-1]
-
- def validate(self, plaintext, encrypted):
- encrypted = str(encrypted)
- plaintext = str(plaintext)
- try:
- ref = binascii.a2b_base64(encrypted)
- except binascii.Error:
- # Not valid base64.
- return False
- salt = ref[20:]
- v = binascii.b2a_base64(
- hashlib.sha1(plaintext + salt).digest() + salt)[:-1]
- pw1 = (v or '').strip()
- pw2 = (encrypted or '').strip()
- return pw1 == pw2
-
-
class LaunchpadLoginSource:
"""A login source that uses the launchpad SQL database to look up
principal information.
=== modified file 'lib/lp/services/webapp/configure.zcml'
--- lib/lp/services/webapp/configure.zcml 2011-12-30 07:32:58 +0000
+++ lib/lp/services/webapp/configure.zcml 2012-01-18 00:55:42 +0000
@@ -169,12 +169,6 @@
/>
<utility
- factory="lp.services.webapp.authentication.SSHADigestEncryptor"
- provides="lp.services.webapp.interfaces.IPasswordEncryptor"
- permission="zope.Public"
- />
-
- <utility
component="lp.services.webapp.authentication.loginSource"
provides="lp.services.webapp.interfaces.IPlacelessLoginSource"
permission="zope.Public"
=== modified file 'lib/lp/services/webapp/interfaces.py'
--- lib/lp/services/webapp/interfaces.py 2012-01-18 00:55:42 +0000
+++ lib/lp/services/webapp/interfaces.py 2012-01-18 00:55:42 +0000
@@ -412,20 +412,6 @@
schema=IBrowserFormNG)
-class IPasswordEncryptor(Interface):
- """An interface representing a password encryption scheme."""
-
- def encrypt(plaintext):
- """Return the encrypted value of plaintext."""
-
- def validate(plaintext, encrypted):
- """Return a true value if the encrypted value of 'plaintext' is
- equivalent to the value of 'encrypted'. In general, if this
- method returns true, it can also be assumed that the value of
- self.encrypt(plaintext) will compare equal to 'encrypted'.
- """
-
-
class IPrincipalIdentifiedEvent(Interface):
"""An event that is sent after a principal has been recovered from the
request's credentials.
=== removed file 'lib/lp/services/webapp/tests/test_encryptor.py'
--- lib/lp/services/webapp/tests/test_encryptor.py 2012-01-01 02:58:52 +0000
+++ lib/lp/services/webapp/tests/test_encryptor.py 1970-01-01 00:00:00 +0000
@@ -1,63 +0,0 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-__metaclass__ = type
-
-
-import binascii
-import hashlib
-import unittest
-
-from zope.app.testing import ztapi
-from zope.app.testing.placelesssetup import PlacelessSetup
-from zope.component import getUtility
-
-from lp.services.webapp.authentication import SSHADigestEncryptor
-from lp.services.webapp.interfaces import IPasswordEncryptor
-
-
-class TestSSHADigestEncryptor(PlacelessSetup, unittest.TestCase):
-
- def setUp(self):
- PlacelessSetup.setUp(self)
- encryptor = SSHADigestEncryptor()
- ztapi.provideUtility(IPasswordEncryptor, encryptor)
-
- def test_encrypt(self):
- encryptor = getUtility(IPasswordEncryptor)
- encrypted1 = encryptor.encrypt('motorhead')
- encrypted2 = encryptor.encrypt('motorhead')
- self.failIfEqual(encrypted1, encrypted2)
- salt = encrypted1[20:]
- v = binascii.b2a_base64(
- hashlib.sha1('motorhead' + salt).digest() + salt)[:-1]
- return (v == encrypted1)
-
- def test_validate(self):
- encryptor = getUtility(IPasswordEncryptor)
- self.assertEqual(encryptor.validate(
- 'motorhead', '+uSsxIfQDRUxG1oDTu1SsQN0P0RTl4SL9XRd'), True)
-
- def test_unicode_encrypt(self):
- encryptor = getUtility(IPasswordEncryptor)
- encrypted1 = encryptor.encrypt(u'motorhead')
- encrypted2 = encryptor.encrypt(u'motorhead')
- self.failIfEqual(encrypted1, encrypted2)
- salt = encrypted1[20:]
- v = binascii.b2a_base64(
- hashlib.sha1('motorhead' + salt).digest() + salt)[:-1]
- return v == encrypted1
-
- def test_unicode_validate(self):
- encryptor = getUtility(IPasswordEncryptor)
- self.assertEqual(encryptor.validate(
- u'motorhead', u'+uSsxIfQDRUxG1oDTu1SsQN0P0RTl4SL9XRd'), True)
-
- def test_nonunicode_password(self):
- encryptor = getUtility(IPasswordEncryptor)
- try:
- encryptor.encrypt(u'motorhead\xc3\xb3')
- except UnicodeEncodeError:
- pass
- else:
- self.fail("uncaught non-ascii text")
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2012-01-17 14:14:49 +0000
+++ lib/lp/testing/factory.py 2012-01-18 00:55:42 +0000
@@ -529,20 +529,16 @@
@with_celebrity_logged_in('admin')
def makeAdministrator(self, name=None, email=None, password=None):
- user = self.makePerson(name=name,
- email=email,
- password=password)
+ user = self.makePerson(name=name, email=email)
administrators = getUtility(ILaunchpadCelebrities).admin
administrators.addMember(user, administrators.teamowner)
return user
def makeRegistryExpert(self, name=None, email='expert@xxxxxxxxxxx',
- password='test'):
+ password=None):
from lp.testing.sampledata import ADMIN_EMAIL
login(ADMIN_EMAIL)
- user = self.makePerson(name=name,
- email=email,
- password=password)
+ user = self.makePerson(name=name, email=email)
registry_team = getUtility(ILaunchpadCelebrities).registry_experts
registry_team.addMember(user, registry_team.teamowner)
return user
@@ -561,14 +557,12 @@
pocket)
return ProxyFactory(location)
- def makeAccount(self, displayname=None, password=None,
- status=AccountStatus.ACTIVE,
+ def makeAccount(self, displayname=None, status=AccountStatus.ACTIVE,
rationale=AccountCreationRationale.UNKNOWN):
"""Create and return a new Account."""
if displayname is None:
displayname = self.getUniqueString('displayname')
- account = getUtility(IAccountSet).new(
- rationale, displayname, password=password)
+ account = getUtility(IAccountSet).new(rationale, displayname)
removeSecurityProxy(account).status = status
self.makeOpenIdIdentifier(account)
return account
@@ -612,10 +606,7 @@
:param email: The email address for the new person.
:param name: The name for the new person.
- :param password: The password for the person.
- This password can be used in setupBrowser in combination
- with the email address to create a browser for this new
- person.
+ :param password: Ignored.
:param email_address_status: If specified, the status of the email
address is set to the email_address_status.
:param displayname: The display name to use for the person.
@@ -630,20 +621,15 @@
email = self.getUniqueEmailAddress()
if name is None:
name = self.getUniqueString('person-name')
- if password is None:
- password = self.getUniqueString('password')
# By default, make the email address preferred.
if (email_address_status is None
or email_address_status == EmailAddressStatus.VALIDATED):
email_address_status = EmailAddressStatus.PREFERRED
- # Set the password to test in order to allow people that have
- # been created this way can be logged in.
person, email = getUtility(IPersonSet).createPersonAndEmail(
email, rationale=PersonCreationRationale.UNKNOWN, name=name,
- password=password, displayname=displayname,
+ displayname=displayname,
hide_email_addresses=hide_email_addresses)
naked_person = removeSecurityProxy(person)
- naked_person._password_cleartext_cached = password
if homepage_content is not None:
naked_person.homepage_content = homepage_content
@@ -721,8 +707,7 @@
# setPreferredEmail no longer activates the account
# automatically.
account = IMasterStore(Account).get(Account, person.accountID)
- account.reactivate(
- "Activated by factory.makePersonByName", password='foo')
+ account.reactivate("Activated by factory.makePersonByName")
person.setPreferredEmail(email)
if not use_default_autosubscribe_policy:
