launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06114
[Merge] lp:~wgrant/launchpad/hardcoded-password into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/hardcoded-password into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/hardcoded-password/+merge/88966
This branch drops most of the remaining password support in Launchpad. The major changes are in lp.services.webapp.authentication, where I've disabled basic authentication outside the testrunner, added several more paranoid is-testrunner checks, and replaced the password validation with a string match against 'test'.
--
https://code.launchpad.net/~wgrant/launchpad/hardcoded-password/+merge/88966
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/hardcoded-password into lp:launchpad.
=== modified file 'lib/lp/bugs/scripts/bugexpire.py'
--- lib/lp/bugs/scripts/bugexpire.py 2012-01-01 02:58:52 +0000
+++ lib/lp/bugs/scripts/bugexpire.py 2012-01-17 23:46:25 +0000
@@ -120,8 +120,7 @@
auth_utility = getUtility(IPlacelessAuthUtility)
janitor_email = self.janitor.preferredemail.email
setupInteraction(
- auth_utility.getPrincipalByLogin(
- janitor_email, want_password=False),
+ auth_utility.getPrincipalByLogin(janitor_email),
login=janitor_email)
def _logout(self):
=== modified file 'lib/lp/bugs/scripts/checkwatches/base.py'
--- lib/lp/bugs/scripts/checkwatches/base.py 2012-01-01 02:58:52 +0000
+++ lib/lp/bugs/scripts/checkwatches/base.py 2012-01-17 23:46:25 +0000
@@ -121,7 +121,7 @@
self._login = login
self._principal = (
getUtility(IPlacelessAuthUtility).getPrincipalByLogin(
- self._login, want_password=False))
+ self._login))
self._transaction_manager = transaction_manager
self.logger = logger
=== modified file 'lib/lp/services/webapp/authentication.py'
--- lib/lp/services/webapp/authentication.py 2012-01-03 11:08:31 +0000
+++ lib/lp/services/webapp/authentication.py 2012-01-17 23:46:25 +0000
@@ -45,10 +45,13 @@
AccessLevel,
BasicAuthLoggedInEvent,
CookieAuthPrincipalIdentifiedEvent,
+ DEFAULT_FLAVOR,
ILaunchpadPrincipal,
IPasswordEncryptor,
IPlacelessAuthUtility,
IPlacelessLoginSource,
+ IStoreSelector,
+ MAIN_STORE,
)
@@ -64,13 +67,25 @@
self.nobody.__parent__ = self
def _authenticateUsingBasicAuth(self, credentials, request):
+ # Since there is a hardcoded password, check really really hard
+ # that this is a testrunner.
+ store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
+ db_name = store.execute("SELECT current_database()").get_one()[0]
+ is_test_db = (
+ db_name.startswith('launchpad_ftest')
+ or db_name.startswith('launchpad_empty'))
+ is_dev_domain = config.vhost.mainsite.hostname == 'launchpad.dev'
+ if not is_test_db or not is_dev_domain:
+ raise AssertionError(
+ "Attempted to use basic auth outside the test suite.")
+
login = credentials.getLogin()
if login is not None:
login_src = getUtility(IPlacelessLoginSource)
principal = login_src.getPrincipalByLogin(login)
if principal is not None and principal.person.is_valid_person:
password = credentials.getPassword()
- if principal.validate(password):
+ if password == 'test':
# We send a LoggedInEvent here, when the
# cookie auth below sends a PrincipalIdentified,
# as the login form is never visited for BasicAuth.
@@ -130,7 +145,8 @@
# encoded properly. That's a client error, so we don't really
# care, and we're done.
raise Unauthorized("Bad Basic authentication.")
- if credentials is not None and credentials.getLogin() is not None:
+ if (config.isTestRunner() and credentials is not None
+ and credentials.getLogin() is not None):
return self._authenticateUsingBasicAuth(credentials, request)
else:
# Hack to make us not even think of using a session if there
@@ -166,10 +182,9 @@
utility = getUtility(IPlacelessLoginSource)
return utility.getPrincipals(name)
- def getPrincipalByLogin(self, login, want_password=True):
+ def getPrincipalByLogin(self, login):
"""See IAuthenticationService."""
- utility = getUtility(IPlacelessLoginSource)
- return utility.getPrincipalByLogin(login, want_password=want_password)
+ return getUtility(IPlacelessLoginSource).getPrincipalByLogin(login)
class SSHADigestEncryptor:
@@ -251,16 +266,10 @@
def getPrincipalByLogin(self, login,
access_level=AccessLevel.WRITE_PRIVATE,
- scope=None, want_password=True):
+ scope=None):
"""Return a principal based on the account with the email address
signified by "login".
- :param want_password: If want_password is False, the pricipal
- will have None for a password. Use this when trying to retrieve a
- principal in contexts where we don't need the password and the
- database connection does not have access to the Account or
- AccountPassword tables.
-
:return: None if there is no account with the given email address.
The `access_level` can be used for further restricting the capability
@@ -279,27 +288,18 @@
person = getUtility(IPersonSet).getByEmail(login)
if person is None or person.account is None:
return None
- return self._principalForAccount(
- person.account, access_level, scope, want_password)
+ return self._principalForAccount(person.account, access_level, scope)
- def _principalForAccount(self, account, access_level, scope,
- want_password=True):
+ def _principalForAccount(self, account, access_level, scope):
"""Return a LaunchpadPrincipal for the given account.
The LaunchpadPrincipal will also have the given access level and
scope.
-
- If want_password is True, the principal's password will be set to the
- account's password. Otherwise it's set to None.
"""
naked_account = removeSecurityProxy(account)
- if want_password:
- password = naked_account.password
- else:
- password = None
principal = LaunchpadPrincipal(
naked_account.id, naked_account.displayname,
- naked_account.displayname, account, password,
+ naked_account.displayname, account,
access_level=access_level, scope=scope)
principal.__parent__ = self
return principal
@@ -315,7 +315,7 @@
implements(ILaunchpadPrincipal)
- def __init__(self, id, title, description, account, pwd=None,
+ def __init__(self, id, title, description, account,
access_level=AccessLevel.WRITE_PRIVATE, scope=None):
self.id = id
self.title = title
@@ -324,17 +324,10 @@
self.scope = scope
self.account = account
self.person = IPerson(account, None)
- self.__pwd = pwd
def getLogin(self):
return self.title
- def validate(self, pw):
- encryptor = getUtility(IPasswordEncryptor)
- pw1 = (pw or '').strip()
- pw2 = (self.__pwd or '').strip()
- return encryptor.validate(pw1, pw2)
-
# zope.app.apidoc expects our principals to be adaptable into IAnnotations, so
# we use these dummy adapters here just to make that code not OOPS.
=== modified file 'lib/lp/services/webapp/interaction.py'
--- lib/lp/services/webapp/interaction.py 2011-12-24 17:49:30 +0000
+++ lib/lp/services/webapp/interaction.py 2012-01-17 23:46:25 +0000
@@ -143,7 +143,7 @@
# IPersonSet.getByEmail() and since this is security wrapped, it needs
# an interaction available.
setupInteraction(authutil.unauthenticatedPrincipal())
- principal = authutil.getPrincipalByLogin(email, want_password=False)
+ principal = authutil.getPrincipalByLogin(email)
assert principal is not None, "Invalid login"
if principal.person is not None and principal.person.is_team:
raise AssertionError("Please do not try to login as a team")
=== modified file 'lib/lp/services/webapp/interfaces.py'
--- lib/lp/services/webapp/interfaces.py 2011-12-24 17:49:30 +0000
+++ lib/lp/services/webapp/interfaces.py 2012-01-17 23:46:25 +0000
@@ -494,11 +494,8 @@
login name.
"""
- def getPrincipalByLogin(login, want_password=True):
- """Return a principal based on his login name.
-
- The principal's password is set to None if want_password is False.
- """
+ def getPrincipalByLogin(login):
+ """Return a principal based on his login name."""
class IPlacelessLoginSource(IPrincipalSource):
@@ -507,15 +504,8 @@
between the user id and login name.
"""
- # want_password is temporary. Eventually we will have accounts
- # without passwords at all, authenticated via other means such as external
- # OpenID providers or SSL certificates. Principals having passwords
- # doesn't really make sense.
- def getPrincipalByLogin(login, want_password=True):
- """Return a principal based on his login name.
-
- If want_password is False, the principal's password is set to None.
- """
+ def getPrincipalByLogin(login):
+ """Return a principal based on his login name."""
def getPrincipals(name):
"""Not implemented.
=== modified file 'lib/lp/services/webapp/tests/test_authutility.py'
--- lib/lp/services/webapp/tests/test_authutility.py 2012-01-03 12:25:31 +0000
+++ lib/lp/services/webapp/tests/test_authutility.py 2012-01-17 23:46:25 +0000
@@ -4,29 +4,23 @@
__metaclass__ = type
import base64
-import unittest
-from zope.app.security.basicauthadapter import BasicAuthAdapter
-from zope.app.security.interfaces import ILoginPassword
from zope.app.security.principalregistry import UnauthenticatedPrincipal
-from zope.app.testing import ztapi
-from zope.app.testing.placelesssetup import PlacelessSetup
from zope.component import getUtility
from zope.interface import implements
from zope.publisher.browser import TestRequest
-from zope.publisher.interfaces.http import IHTTPCredentials
from lp.registry.interfaces.person import IPerson
+from lp.services.config import config
from lp.services.identity.interfaces.account import IAccount
-from lp.services.webapp.authentication import (
- LaunchpadPrincipal,
- PlacelessAuthUtility,
- )
+from lp.services.webapp.authentication import LaunchpadPrincipal
from lp.services.webapp.interfaces import (
- IPasswordEncryptor,
IPlacelessAuthUtility,
IPlacelessLoginSource,
)
+from lp.testing import TestCase
+from lp.testing.fixture import ZopeUtilityFixture
+from lp.testing.layers import DatabaseFunctionalLayer
class DummyPerson(object):
@@ -39,14 +33,14 @@
person = DummyPerson()
-Bruce = LaunchpadPrincipal(42, 'bruce', 'Bruce', DummyAccount(), 'bruce!')
+Bruce = LaunchpadPrincipal(42, 'bruce', 'Bruce', DummyAccount())
Bruce.person = Bruce.account.person
class DummyPlacelessLoginSource(object):
implements(IPlacelessLoginSource)
- def getPrincipalByLogin(self, id, want_password=True):
+ def getPrincipalByLogin(self, id):
return Bruce
getPrincipal = getPrincipalByLogin
@@ -55,29 +49,14 @@
return [Bruce]
-class DummyPasswordEncryptor(object):
- implements(IPasswordEncryptor)
-
- def validate(self, plaintext, encrypted):
- return plaintext == encrypted
-
-
-class TestPlacelessAuth(PlacelessSetup, unittest.TestCase):
+class TestPlacelessAuth(TestCase):
+
+ layer = DatabaseFunctionalLayer
def setUp(self):
- PlacelessSetup.setUp(self)
- ztapi.provideUtility(IPasswordEncryptor, DummyPasswordEncryptor())
- ztapi.provideUtility(IPlacelessLoginSource,
- DummyPlacelessLoginSource())
- ztapi.provideUtility(IPlacelessAuthUtility, PlacelessAuthUtility())
- ztapi.provideAdapter(
- IHTTPCredentials, ILoginPassword, BasicAuthAdapter)
-
- def tearDown(self):
- ztapi.unprovideUtility(IPasswordEncryptor)
- ztapi.unprovideUtility(IPlacelessLoginSource)
- ztapi.unprovideUtility(IPlacelessAuthUtility)
- PlacelessSetup.tearDown(self)
+ super(TestPlacelessAuth, self).setUp()
+ self.useFixture(ZopeUtilityFixture(
+ DummyPlacelessLoginSource(), IPlacelessLoginSource, ''))
def _make(self, login, pwd):
dict = {
@@ -87,11 +66,11 @@
return getUtility(IPlacelessAuthUtility), request
def test_authenticate_ok(self):
- authsvc, request = self._make('bruce', 'bruce!')
+ authsvc, request = self._make('bruce', 'test')
self.assertEqual(authsvc.authenticate(request), Bruce)
def test_authenticate_notok(self):
- authsvc, request = self._make('bruce', 'notbruce!')
+ authsvc, request = self._make('bruce', 'nottest')
self.assertEqual(authsvc.authenticate(request), None)
def test_unauthenticatedPrincipal(self):
@@ -100,18 +79,33 @@
UnauthenticatedPrincipal))
def test_unauthorized(self):
- authsvc, request = self._make('bruce', 'bruce!')
+ authsvc, request = self._make('bruce', 'test')
self.assertEqual(authsvc.unauthorized('bruce', request), None)
self.assertEqual(request._response._status, 401)
+ def test_only_for_testrunner(self):
+ # Basic authentication only works on a launchpad_ftest*
+ # database, with a mainsite hostname of launchpad.dev. It has a
+ # hardcoded password, so must never be used on production.
+ try:
+ config.push(
+ "change-rooturl", "[vhost.mainsite]\nhostname: launchpad.net")
+ authsvc, request = self._make('bruce', 'test')
+ self.assertRaisesWithContent(
+ AssertionError,
+ "Attempted to use basic auth outside the test suite.",
+ authsvc.authenticate, request)
+ finally:
+ config.pop("change-rooturl")
+
def test_getPrincipal(self):
- authsvc, request = self._make('bruce', 'bruce!')
+ authsvc, request = self._make('bruce', 'test')
self.assertEqual(authsvc.getPrincipal('bruce'), Bruce)
def test_getPrincipals(self):
- authsvc, request = self._make('bruce', 'bruce!')
+ authsvc, request = self._make('bruce', 'test')
self.assertEqual(authsvc.getPrincipals('bruce'), [Bruce])
def test_getPrincipalByLogin(self):
- authsvc, request = self._make('bruce', 'bruce!')
+ authsvc, request = self._make('bruce', 'test')
self.assertEqual(authsvc.getPrincipalByLogin('bruce'), Bruce)
=== modified file 'lib/lp/testing/fixture.py'
--- lib/lp/testing/fixture.py 2012-01-01 02:58:52 +0000
+++ lib/lp/testing/fixture.py 2012-01-17 23:46:25 +0000
@@ -38,6 +38,7 @@
from zope.component import (
adapter,
getGlobalSiteManager,
+ queryUtility,
provideHandler,
)
from zope.interface import Interface
@@ -218,10 +219,15 @@
def setUp(self):
super(ZopeUtilityFixture, self).setUp()
gsm = getGlobalSiteManager()
+ self._old_component = queryUtility(self.intf, name=self.name)
gsm.registerUtility(self.component, self.intf, self.name)
self.addCleanup(
gsm.unregisterUtility,
self.component, self.intf, self.name)
+ if self._old_component is not None:
+ self.addCleanup(
+ gsm.registerUtility,
+ self._old_component, self.intf, self.name)
class Urllib2Fixture(Fixture):
=== modified file 'lib/lp/testing/tests/test_fixture.py'
--- lib/lp/testing/tests/test_fixture.py 2012-01-01 02:58:52 +0000
+++ lib/lp/testing/tests/test_fixture.py 2012-01-17 23:46:25 +0000
@@ -112,6 +112,20 @@
self.assertEquals(get_mailer(), fake)
self.assertRaises(ComponentLookupError, get_mailer)
+ def test_restores_old(self):
+ def get_mailer():
+ return getGlobalSiteManager().getUtility(
+ IMailDelivery, 'Mail')
+ fake = DummyMailer()
+ fake2 = DummyMailer()
+ # In BaseLayer there should be no mailer by default.
+ self.assertRaises(ComponentLookupError, get_mailer)
+ with ZopeUtilityFixture(fake, IMailDelivery, 'Mail'):
+ with ZopeUtilityFixture(fake2, IMailDelivery, 'Mail'):
+ self.assertEquals(get_mailer(), fake2)
+ self.assertEquals(get_mailer(), fake)
+ self.assertRaises(ComponentLookupError, get_mailer)
+
class TestPGBouncerFixtureWithCA(TestCase):
"""PGBouncerFixture reconnect tests for Component Architecture layers.
=== modified file 'lib/lp/testing/yuixhr.py'
--- lib/lp/testing/yuixhr.py 2012-01-01 02:58:52 +0000
+++ lib/lp/testing/yuixhr.py 2012-01-17 23:46:25 +0000
@@ -111,7 +111,7 @@
request = get_current_browser_request()
assert request is not None, "We do not have a browser request."
authutil = getUtility(IPlacelessAuthUtility)
- principal = authutil.getPrincipalByLogin(email, want_password=False)
+ principal = authutil.getPrincipalByLogin(email)
launchbag = getUtility(IOpenLaunchBag)
launchbag.setLogin(email)
logInPrincipal(request, principal, email)
