launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #20129
[Merge] lp:~wgrant/launchpad/devel-add-read-ff into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/devel-add-read-ff into lp:launchpad.
Commit message:
Add feature flag to read GPG keys from gpgservice.
Requested reviews:
William Grant (wgrant): code
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/devel-add-read-ff/+merge/289603
Polished version of https://code.launchpad.net/~thomir/launchpad/devel-add-read-ff/+merge/288989
--
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg 2016-03-14 19:40:14 +0000
+++ database/schema/security.cfg 2016-03-21 05:51:58 +0000
@@ -975,6 +975,7 @@
public.livefs = SELECT
public.livefsbuild = SELECT, UPDATE
public.livefsfile = SELECT
+public.openididentifier = SELECT
public.packageset = SELECT
public.packagesetgroup = SELECT
public.packagesetinclusion = SELECT
@@ -1056,6 +1057,7 @@
public.job = SELECT, INSERT, UPDATE
public.libraryfilealias = SELECT
public.libraryfilecontent = SELECT
+public.openididentifier = SELECT
public.packagecopyjob = SELECT
public.packageset = SELECT, INSERT
public.packagesetgroup = SELECT, INSERT
@@ -1113,6 +1115,7 @@
public.libraryfilecontent = SELECT, INSERT
public.message = SELECT, INSERT, UPDATE
public.messagechunk = SELECT, INSERT, UPDATE
+public.openididentifier = SELECT
public.packagecopyjob = SELECT, UPDATE
public.packagediff = SELECT, INSERT
public.packageset = SELECT, INSERT
@@ -1216,6 +1219,7 @@
public.logintoken = SELECT, INSERT, UPDATE
public.message = SELECT, INSERT, UPDATE
public.milestone = SELECT, INSERT, UPDATE
+public.openididentifier = SELECT
public.packageupload = SELECT, INSERT, UPDATE
public.packageuploadbuild = SELECT, INSERT, UPDATE
public.packageuploadcustom = SELECT, INSERT, UPDATE
@@ -1390,6 +1394,7 @@
public.messagechunk = SELECT, INSERT
public.milestone = SELECT
public.milestonetag = SELECT
+public.openididentifier = SELECT
public.packagecopyjob = SELECT, INSERT
public.packagediff = SELECT, INSERT, UPDATE, DELETE
public.packageset = SELECT
@@ -1502,6 +1507,7 @@
public.messagechunk = SELECT, INSERT
public.milestone = SELECT
public.milestonetag = SELECT
+public.openididentifier = SELECT
public.packagecopyjob = SELECT, INSERT, UPDATE
public.packagediff = SELECT, UPDATE
public.packageset = SELECT
@@ -1804,6 +1810,7 @@
public.messagechunk = SELECT, INSERT
public.milestone = SELECT
public.milestonetag = SELECT, INSERT, DELETE
+public.openididentifier = SELECT
public.packageset = SELECT
public.packagesetgroup = SELECT
public.packagesetinclusion = SELECT
@@ -1922,6 +1929,7 @@
public.libraryfilecontent = SELECT, INSERT
public.message = SELECT, INSERT
public.messagechunk = SELECT, INSERT
+public.openididentifier = SELECT
public.person = SELECT
public.product = SELECT
public.productseries = SELECT
=== modified file 'lib/lp/registry/browser/tests/test_gpgkey.py'
--- lib/lp/registry/browser/tests/test_gpgkey.py 2016-02-10 00:51:55 +0000
+++ lib/lp/registry/browser/tests/test_gpgkey.py 2016-03-21 05:51:58 +0000
@@ -6,28 +6,39 @@
__metaclass__ = type
from testtools.matchers import (
+ Contains,
+ Equals,
+ HasLength,
Not,
Raises,
raises,
)
+from zope.component import getUtility
+from lp.registry.interfaces.person import IPersonSet
+from lp.registry.interfaces.gpg import IGPGKeySet
from lp.services.features.testing import FeatureFixture
from lp.services.gpg.interfaces import (
+ GPG_DATABASE_READONLY_FEATURE_FLAG,
+ GPG_READ_FROM_GPGSERVICE_FEATURE_FLAG,
+ GPG_WRITE_TO_GPGSERVICE_FEATURE_FLAG,
+ GPGKeyAlgorithm,
GPGReadOnly,
- GPG_DATABASE_READONLY_FEATURE_FLAG,
)
+from lp.services.verification.interfaces.authtoken import LoginTokenType
+from lp.services.verification.interfaces.logintoken import ILoginTokenSet
from lp.services.webapp import canonical_url
from lp.testing import (
login_person,
TestCaseWithFactory,
)
-from lp.testing.layers import DatabaseFunctionalLayer
+from lp.testing.layers import LaunchpadFunctionalLayer
from lp.testing.views import create_initialized_view
class TestCanonicalUrl(TestCaseWithFactory):
- layer = DatabaseFunctionalLayer
+ layer = LaunchpadFunctionalLayer
def test_canonical_url(self):
# The canonical URL of a GPG key is ???
@@ -41,7 +52,7 @@
class TestPersonGPGView(TestCaseWithFactory):
- layer = DatabaseFunctionalLayer
+ layer = LaunchpadFunctionalLayer
def test_edit_pgp_keys_login_redirect(self):
"""+editpgpkeys should redirect to force you to re-authenticate."""
@@ -60,8 +71,9 @@
}))
person = self.factory.makePerson()
login_person(person)
- view = create_initialized_view(person, "+editpgpkeys", principal=person,
- method='POST', have_fresh_login=True)
+ view = create_initialized_view(
+ person, "+editpgpkeys", principal=person, method='POST',
+ have_fresh_login=True)
self.assertThat(view.render, raises(GPGReadOnly))
def test_gpgkeys_GET_readonly_with_feature_flag_set(self):
@@ -70,6 +82,98 @@
}))
person = self.factory.makePerson()
login_person(person)
- view = create_initialized_view(person, "+editpgpkeys", principal=person,
- method='GET', have_fresh_login=True)
+ view = create_initialized_view(
+ person, "+editpgpkeys", principal=person, method='GET',
+ have_fresh_login=True)
self.assertThat(view.render, Not(Raises()))
+
+
+class GPGKeySetTests(TestCaseWithFactory):
+
+ layer = LaunchpadFunctionalLayer
+
+ def test_can_add_keys_for_test(self):
+ # IGPGKeySet.new _only_ writes to the launchpad database, so this test
+ # only works if the read_from_gpgservice FF is *not* set. Once this is
+ # the default codepath this test should be deleted.
+ self.useFixture(FeatureFixture({
+ GPG_READ_FROM_GPGSERVICE_FEATURE_FLAG: None,
+ }))
+ keyset = getUtility(IGPGKeySet)
+ person = self.factory.makePerson()
+ fingerprint = "DEADBEEF12345678DEADBEEF12345678DEADBEEF"
+ keyset.new(person.id, "F0A432C2", fingerprint, 4096,
+ GPGKeyAlgorithm.R, True, True)
+
+ keys = keyset.getGPGKeysForPerson(person)
+
+ self.assertThat(keys, HasLength(1))
+ self.assertThat(keys[0].fingerprint, Equals(fingerprint))
+
+ def test_sampledata_contains_gpgkeys(self):
+ keyset = getUtility(IGPGKeySet)
+ personset = getUtility(IPersonSet)
+ foobar = personset.getByName('name16')
+ keys = keyset.getGPGKeysForPerson(foobar)
+
+ self.assertThat(keys, HasLength(1))
+ self.assertThat(keys[0].keyid, Equals('12345678'))
+ self.assertThat(keys[0].fingerprint,
+ Equals('ABCDEF0123456789ABCDDCBA0000111112345678'))
+
+ def test_can_retrieve_keys_by_fingerprint(self):
+ keyset = getUtility(IGPGKeySet)
+ person = self.factory.makePerson()
+ key = self.factory.makeGPGKey(person)
+
+ retrieved_key = keyset.getByFingerprint(key.fingerprint)
+
+ self.assertThat(retrieved_key.owner.name, Equals(person.name))
+ self.assertThat(retrieved_key.fingerprint, Equals(key.fingerprint))
+
+ def test_getGPGKeysForPerson_retrieves_active_keys(self):
+ keyset = getUtility(IGPGKeySet)
+ person = self.factory.makePerson()
+ key = self.factory.makeGPGKey(person)
+
+ keys = keyset.getGPGKeysForPerson(person)
+
+ self.assertThat(keys, HasLength(1))
+ self.assertThat(keys, Contains(key))
+
+ def test_getGPGKeysForPerson_retrieves_inactive_keys(self):
+ keyset = getUtility(IGPGKeySet)
+ person = self.factory.makePerson()
+ key = self.factory.makeGPGKey(person)
+ keyset.deactivate(key)
+
+ active_keys = keyset.getGPGKeysForPerson(person)
+ inactive_keys = keyset.getGPGKeysForPerson(person, active=False)
+
+ self.assertThat(active_keys, HasLength(0))
+ self.assertThat(inactive_keys, HasLength(1))
+ self.assertThat(inactive_keys, Contains(key))
+
+ def test_getGPGKeysForPerson_excludes_keys_with_logintoken(self):
+ keyset = getUtility(IGPGKeySet)
+ email = 'foo@xxxxxxx'
+ person = self.factory.makePerson(email)
+ key = self.factory.makeGPGKey(person)
+ keyset.deactivate(key)
+ getUtility(ILoginTokenSet).new(
+ person, email, email, LoginTokenType.VALIDATEGPG, key.fingerprint)
+
+ inactive_keys = keyset.getGPGKeysForPerson(person, active=False)
+ self.assertThat(inactive_keys, HasLength(0))
+
+
+class GPGKeySetWithGPGServiceTests(GPGKeySetTests):
+
+ """A copy of GPGKeySetTests, but with gpgservice used."""
+
+ def setUp(self):
+ super(GPGKeySetWithGPGServiceTests, self).setUp()
+ self.useFixture(FeatureFixture({
+ GPG_READ_FROM_GPGSERVICE_FEATURE_FLAG: True,
+ GPG_WRITE_TO_GPGSERVICE_FEATURE_FLAG: True,
+ }))
=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml 2016-02-28 19:13:17 +0000
+++ lib/lp/registry/configure.zcml 2016-03-21 05:51:58 +0000
@@ -1284,6 +1284,11 @@
permission="launchpad.Edit"
set_attributes="active can_encrypt"/>
</class>
+ <class
+ class="lp.registry.model.gpgkey.GPGServiceKey">
+ <allow
+ interface="lp.registry.interfaces.gpg.IGPGKey"/>
+ </class>
<!-- GPGKeySet -->
=== removed file 'lib/lp/registry/doc/gpgkey.txt'
--- lib/lp/registry/doc/gpgkey.txt 2016-03-01 12:46:32 +0000
+++ lib/lp/registry/doc/gpgkey.txt 1970-01-01 00:00:00 +0000
@@ -1,33 +0,0 @@
-= GPG Keys =
-
-Launchpad models GPG keys in a GPGKey class.
-
- >>> from zope.component import getUtility
- >>> from lp.registry.interfaces.gpg import IGPGKeySet
- >>> from lp.registry.interfaces.person import IPersonSet
- >>> from lp.services.gpg.interfaces import GPGKeyAlgorithm
- >>> personset = getUtility(IPersonSet)
- >>> foobar = personset.getByName('name16')
- >>> gpgkeyset = getUtility(IGPGKeySet)
- >>> keys = gpgkeyset.getGPGKeysForPerson(foobar)
- >>> [key.keyid for key in keys]
- [u'12345678']
-
-Adding new keys is pretty easy:
-
- >>> name12 = personset.getByName('name12')
- >>> gpgkeyset.new(name12.id, u'DEADBEEF',
- ... 'DEADBEEF12345678DEADBEEF12345678DEADBEEF', 1024,
- ... GPGKeyAlgorithm.LITTLE_G)
- <GPGKey...>
- >>> gpgkeyset.new(name12.id, u'DEADBEED',
- ... 'DEADBEED12345678DEADBEED12345678DEADBEED', 2048,
- ... GPGKeyAlgorithm.G)
- <GPGKey...>
-
-As is pulling it based on the key ID:
-
- >>> key = gpgkeyset.getByFingerprint(
- ... 'DEADBEEF12345678DEADBEEF12345678DEADBEEF')
- >>> key.owner.name
- u'name12'
=== modified file 'lib/lp/registry/interfaces/gpg.py'
--- lib/lp/registry/interfaces/gpg.py 2016-03-14 19:40:14 +0000
+++ lib/lp/registry/interfaces/gpg.py 2016-03-21 05:51:58 +0000
@@ -39,7 +39,6 @@
export_as_webservice_entry('gpg_key')
- id = Int(title=_("Database id"), required=True, readonly=True)
keysize = Int(title=_("Keysize"), required=True)
algorithm = Choice(title=_("Algorithm"), required=True,
vocabulary='GpgAlgorithm')
@@ -84,6 +83,9 @@
inactive ones.
"""
+ def getOwnerIdForPerson(person):
+ """return an owner id string suitable for sending to gpgservice."""
+
def getByFingerprints(fingerprints):
"""Get multiple OpenPGP keys by their fingerprints."""
=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py 2016-01-26 15:47:37 +0000
+++ lib/lp/registry/interfaces/person.py 2016-03-21 05:51:58 +0000
@@ -775,10 +775,10 @@
inactivesignatures = Attribute("Retrieve own Inactive CoC Signatures.")
signedcocs = Attribute("List of Signed Code Of Conduct")
gpg_keys = exported(
- CollectionField(
+ doNotSnapshot(CollectionField(
title=_("List of valid OpenPGP keys ordered by ID"),
readonly=False, required=False,
- value_type=Reference(schema=IGPGKey)))
+ value_type=Reference(schema=IGPGKey))))
pending_gpg_keys = CollectionField(
title=_("Set of fingerprints pending confirmation"),
readonly=False, required=False,
=== modified file 'lib/lp/registry/model/gpgkey.py'
--- lib/lp/registry/model/gpgkey.py 2016-03-16 00:32:05 +0000
+++ lib/lp/registry/model/gpgkey.py 2016-03-21 05:51:58 +0000
@@ -8,7 +8,6 @@
BoolCol,
ForeignKey,
IntCol,
- SQLObjectNotFound,
StringCol,
)
from zope.component import getUtility
@@ -18,6 +17,8 @@
IGPGKey,
IGPGKeySet,
)
+from lp.registry.interfaces.person import IPersonSet
+from lp.services.config import config
from lp.services.database.enumcol import EnumCol
from lp.services.database.interfaces import IStore
from lp.services.database.sqlbase import (
@@ -27,12 +28,14 @@
from lp.services.features import getFeatureFlag
from lp.services.gpg.interfaces import (
GPG_WRITE_TO_GPGSERVICE_FEATURE_FLAG,
+ GPG_READ_FROM_GPGSERVICE_FEATURE_FLAG,
GPGKeyAlgorithm,
IGPGClient,
IGPGHandler,
)
from lp.services.openid.interfaces.openid import IOpenIDPersistentIdentity
from lp.services.openid.model.openididentifier import OpenIdIdentifier
+from lp.services.verification.interfaces.logintoken import ILoginTokenSet
@implementer(IGPGKey)
@@ -65,6 +68,58 @@
return '%s%s/%s' % (self.keysize, self.algorithm.title, self.keyid)
+@implementer(IGPGKey)
+class GPGServiceKey:
+
+ def __init__(self, key_data):
+ self._key_data = key_data
+
+ @property
+ def active(self):
+ return self._key_data['enabled']
+
+ @property
+ def keysize(self):
+ return self._key_data['size']
+
+ @property
+ def algorithm(self):
+ return GPGKeyAlgorithm.items[self._key_data['algorithm']]
+
+ @property
+ def keyid(self):
+ return self._key_data['id']
+
+ @property
+ def fingerprint(self):
+ return self._key_data['fingerprint']
+
+ @property
+ def displayname(self):
+ return '%s%s/%s' % (self.keysize, self.algorithm.title, self.keyid)
+
+ @property
+ def keyserverURL(self):
+ return getUtility(
+ IGPGHandler).getURLForKeyInServer(self.fingerprint, public=True)
+
+ @property
+ def can_encrypt(self):
+ return self._key_data['can_encrypt']
+
+ @property
+ def owner(self):
+ return getUtility(IPersonSet).getByOpenIDIdentifier(
+ self._key_data['owner'])
+
+ @property
+ def ownerID(self):
+ return self.owner.id
+
+ def __eq__(self, other):
+ return self.fingerprint == other.fingerprint
+
+
@implementer(IGPGKeySet)
class GPGKeySet:
@@ -79,8 +134,13 @@
def activate(self, requester, key, can_encrypt):
"""See `IGPGKeySet`."""
fingerprint = key.fingerprint
- lp_key = self.getByFingerprint(fingerprint)
+ # XXX: This is a little ugly - we can't use getByFingerprint
+ # here since if the READ_FROM_GPGSERVICE FF is set we'll get a
+ # GPGServiceKey object instead of a GPGKey object, and we need
+ # to change the database representation in all cases.
+ lp_key = GPGKey.selectOneBy(fingerprint=fingerprint)
if lp_key:
+ assert lp_key.owner == requester
is_new = False
# Then the key already exists, so let's reactivate it.
lp_key.active = True
@@ -95,50 +155,116 @@
ownerID, keyid, fingerprint, keysize, algorithm,
can_encrypt=can_encrypt)
if getFeatureFlag(GPG_WRITE_TO_GPGSERVICE_FEATURE_FLAG):
+ # XXX: Further to the comment above, if READ_FROM_GPGSERVICE FF is
+ # set then we need to duplicate the block above but reading from
+ # the gpgservice instead of the database:
client = getUtility(IGPGClient)
- openid_identifier = self.getOwnerIdForPerson(lp_key.owner)
- client.addKeyForOwner(openid_identifier, key.fingerprint)
+ owner_id = self.getOwnerIdForPerson(requester)
+ # Users with more than one openid identifier may be re-activating
+ # a key that was previously deactivated with their non-default
+ # openid identifier. If that's the case, use the same openid
+ # identifier rather than the default one - this happens even if the
+ # read FF is not set:
+ key_data = client.getKeyByFingerprint(fingerprint)
+ if key_data:
+ owner_id = key_data['owner']
+ allowed_owner_ids = self._getAllOwnerIdsForPerson(requester)
+ assert owner_id in allowed_owner_ids
+ gpgservice_key = GPGServiceKey(
+ client.addKeyForOwner(owner_id, key.fingerprint))
+ if getFeatureFlag(GPG_READ_FROM_GPGSERVICE_FEATURE_FLAG):
+ is_new = key_data is None
+ lp_key = gpgservice_key
return lp_key, is_new
def deactivate(self, key):
- key.active = False
+ # key could be a GPGServiceKey, which doesn't allow us to set it's
+ # active attribute. Retrieve it by fingerprint:
+ lp_key = GPGKey.selectOneBy(fingerprint=key.fingerprint)
+ lp_key.active = False
if getFeatureFlag(GPG_WRITE_TO_GPGSERVICE_FEATURE_FLAG):
+ # Users with more than one openid identifier may be
+ # deactivating a key that is associated with their
+ # non-default openid identifier. If that's the case, use
+ # the same openid identifier rather than the default one:
client = getUtility(IGPGClient)
- openid_identifier = self.getOwnerIdForPerson(key.owner)
+ key_data = client.getKeyByFingerprint(key.fingerprint)
+ if not key_data:
+ # We get here if we're asked to deactivate a key that was never
+ # activated. This should probably never happen.
+ return
+ openid_identifier = key_data['owner']
client.disableKeyForOwner(openid_identifier, key.fingerprint)
def getByFingerprint(self, fingerprint, default=None):
"""See `IGPGKeySet`"""
- result = GPGKey.selectOneBy(fingerprint=fingerprint)
- if result is None:
- return default
- return result
+ if getFeatureFlag(GPG_READ_FROM_GPGSERVICE_FEATURE_FLAG):
+ key_data = getUtility(IGPGClient).getKeyByFingerprint(fingerprint)
+ return GPGServiceKey(key_data) if key_data else default
+ else:
+ result = GPGKey.selectOneBy(fingerprint=fingerprint)
+ if result is None:
+ return default
+ return result
def getByFingerprints(self, fingerprints):
"""See `IGPGKeySet`"""
- return IStore(GPGKey).find(
- GPGKey, GPGKey.fingerprint.is_in(fingerprints))
+ fingerprints = list(fingerprints)
+ if getFeatureFlag(GPG_READ_FROM_GPGSERVICE_FEATURE_FLAG):
+ client = getUtility(IGPGClient)
+ return [
+ GPGServiceKey(key_data)
+ for key_data in client.getKeysByFingerprints(fingerprints)]
+ else:
+ return list(IStore(GPGKey).find(
+ GPGKey, GPGKey.fingerprint.is_in(fingerprints)))
def getGPGKeysForPerson(self, owner, active=True):
- if active is False:
- query = """
- active = false
- AND fingerprint NOT IN
- (SELECT fingerprint FROM LoginToken
- WHERE fingerprint IS NOT NULL
- AND requester = %s
- AND date_consumed is NULL
- )
- """ % sqlvalues(owner.id)
+ if getFeatureFlag(GPG_READ_FROM_GPGSERVICE_FEATURE_FLAG):
+ client = getUtility(IGPGClient)
+ owner_ids = self._getAllOwnerIdsForPerson(owner)
+ if not owner_ids:
+ return []
+ gpg_keys = []
+ for owner_id in owner_ids:
+ key_data_list = client.getKeysForOwner(owner_id)['keys']
+ gpg_keys.extend([
+ GPGServiceKey(d) for d in key_data_list
+ if d['enabled'] == active])
+ if active is False:
+ login_tokens = getUtility(ILoginTokenSet).getPendingGPGKeys(
+ owner.id)
+ token_fingerprints = [t.fingerprint for t in login_tokens]
+ return [
+ k for k in gpg_keys
+ if k.fingerprint not in token_fingerprints]
+ return gpg_keys
else:
- query = 'active=true'
-
- query += ' AND owner=%s' % sqlvalues(owner.id)
-
- return list(GPGKey.select(query, orderBy='id'))
+ if active is False:
+ query = """
+ active = false
+ AND fingerprint NOT IN
+ (SELECT fingerprint FROM LoginToken
+ WHERE fingerprint IS NOT NULL
+ AND requester = %s
+ AND date_consumed is NULL
+ )
+ """ % sqlvalues(owner.id)
+ else:
+ query = 'active=true'
+ query += ' AND owner=%s' % sqlvalues(owner.id)
+ return list(GPGKey.select(query, orderBy='id'))
def getOwnerIdForPerson(self, owner):
"""See IGPGKeySet."""
url = IOpenIDPersistentIdentity(owner).openid_identity_url
assert url is not None
return url
+
+ def _getAllOwnerIdsForPerson(self, owner):
+ identifiers = IStore(OpenIdIdentifier).find(
+ OpenIdIdentifier, account=owner.account)
+ openid_provider_root = config.launchpad.openid_provider_root
+ return [
+ openid_provider_root + '+id/' + i.identifier.encode('ascii')
+ for i in identifiers]
=== modified file 'lib/lp/registry/stories/gpg-coc/gpg-with-gpgservice-ff.txt'
--- lib/lp/registry/stories/gpg-coc/gpg-with-gpgservice-ff.txt 2016-02-22 23:45:24 +0000
+++ lib/lp/registry/stories/gpg-coc/gpg-with-gpgservice-ff.txt 2016-03-21 05:51:58 +0000
@@ -7,9 +7,13 @@
tests will be deleted.
>>> from lp.services.features.testing import FeatureFixture
- >>> from lp.services.gpg.interfaces import GPG_WRITE_TO_GPGSERVICE_FEATURE_FLAG
+ >>> from lp.services.gpg.interfaces import (
+ ... GPG_WRITE_TO_GPGSERVICE_FEATURE_FLAG,
+ ... GPG_READ_FROM_GPGSERVICE_FEATURE_FLAG,
+ ... )
>>> feature_fixture = FeatureFixture(
- ... {GPG_WRITE_TO_GPGSERVICE_FEATURE_FLAG: True})
+ ... {GPG_WRITE_TO_GPGSERVICE_FEATURE_FLAG: True,
+ ... GPG_READ_FROM_GPGSERVICE_FEATURE_FLAG: True})
>>> feature_fixture.setUp()
== Setup ==
=== modified file 'lib/lp/services/gpg/configure.zcml'
--- lib/lp/services/gpg/configure.zcml 2016-02-18 03:35:43 +0000
+++ lib/lp/services/gpg/configure.zcml 2016-03-21 05:51:58 +0000
@@ -24,7 +24,7 @@
</class>
<securedutility
- class="lp.services.gpg.handler.GPGClient"
+ class="lp.services.gpg.handler.LPGPGClient"
provides="lp.services.gpg.interfaces.IGPGClient">
<allow interface="lp.services.gpg.interfaces.IGPGClient" />
</securedutility>
=== modified file 'lib/lp/services/gpg/handler.py'
--- lib/lp/services/gpg/handler.py 2016-03-18 00:48:59 +0000
+++ lib/lp/services/gpg/handler.py 2016-03-21 05:51:58 +0000
@@ -11,7 +11,6 @@
]
import atexit
-import base64
import httplib
import os
import shutil
@@ -22,12 +21,10 @@
import tempfile
import urllib
import urllib2
-from urlparse import urljoin
import gpgme
+from gpgservice_client import GPGClient
from lazr.restful.utils import get_current_browser_request
-import requests
-from requests.status_codes import codes as http_codes
from zope.interface import implementer
from lp.app.validators.email import valid_email
@@ -39,7 +36,6 @@
GPGKeyNotFoundError,
GPGKeyRevoked,
GPGKeyTemporarilyNotFoundError,
- GPGServiceException,
GPGUploadFailure,
GPGVerificationError,
IGPGClient,
@@ -51,7 +47,6 @@
SecretGPGKeyImportDetected,
valid_fingerprint,
)
-from lp.services.openid.model.openididentifier import OpenIdIdentifier
from lp.services.timeline.requesttimeline import get_request_timeline
from lp.services.timeout import (
TimeoutError,
@@ -640,132 +635,12 @@
return fingerprint
-def sanitize_fingerprint_or_raise(fingerprint):
- """Check the sanity of 'fingerprint'.
-
- If 'fingerprint' is a valid fingerprint, the sanitised version will be
- returned (see sanitize_fingerprint).
-
- Otherwise, a ValueError will be raised.
- """
- sane_fingerprint = sanitize_fingerprint(fingerprint)
- if sane_fingerprint is None:
- raise ValueError("Invalid fingerprint: %r." % fingerprint)
- return sane_fingerprint
-
-
@implementer(IGPGClient)
-class GPGClient:
+class LPGPGClient(GPGClient):
"""See IGPGClient."""
- def __init__(self):
- self.write_hooks = set()
-
- def getKeysForOwner(self, owner_id):
- """See IGPGClient."""
- path = '/users/%s/keys' % self._encode_owner_id(owner_id)
- resp = self._request('get', path)
- if resp.status_code != http_codes['OK']:
- self.raise_for_error(resp)
- return resp.json()
-
- def addKeyForOwner(self, owner_id, fingerprint):
- """See IGPGClient."""
- fingerprint = sanitize_fingerprint_or_raise(fingerprint)
- path = '/users/%s/keys' % self._encode_owner_id(owner_id)
- data = dict(fingerprint=fingerprint)
- resp = self._request('post', path, data)
- if resp.status_code == http_codes['CREATED']:
- self._notify_writes()
- # status 200 (OK) is returned if the key was already enabled:
- elif resp.status_code != http_codes['OK']:
- self.raise_for_error(resp)
-
- def disableKeyForOwner(self, owner_id, fingerprint):
- """See IGPGClient."""
- fingerprint = sanitize_fingerprint_or_raise(fingerprint)
- path = '/users/%s/keys/%s' % (self._encode_owner_id(owner_id), fingerprint)
- resp = self._request('delete', path)
- if resp.status_code == http_codes['OK']:
- self._notify_writes()
- else:
- self.raise_for_error(resp)
-
- def getKeyByFingerprint(self, fingerprint):
- fingerprint = sanitize_fingerprint_or_raise(fingerprint)
- path = '/keys/%s' % fingerprint
- resp = self._request('get', path)
- if resp.status_code == http_codes['OK']:
- return resp.json()
- elif resp.status_code == http_codes['NOT_FOUND']:
- return None
- else:
- self.raise_for_error(resp)
-
- def registerWriteHook(self, hook_callable):
- """See IGPGClient."""
- if not callable(hook_callable):
- raise TypeError("'hook_callable' parameter must be a callable.")
- self.write_hooks.add(hook_callable)
-
- def unregisterWriteHook(self, hook_callable):
- """See IGPGClient."""
- if hook_callable not in self.write_hooks:
- raise ValueError("%r not registered.")
- self.write_hooks.remove(hook_callable)
-
- def addKeyForTest(self, owner_id, keyid, fingerprint, keysize, algorithm, enabled,
- can_encrypt):
- """See IGPGClient."""
- document = {'keys': [{
- 'owner': owner_id,
- 'id': keyid,
- 'fingerprint': fingerprint,
- 'size': keysize,
- 'algorithm': algorithm,
- 'enabled': enabled,
- 'can_encrypt': can_encrypt}]}
- path = '/test/add_keys'
- resp = self._request('post', path, document)
- if resp.status_code == http_codes['NOT_FOUND']:
- raise RuntimeError(
- "gpgservice was not configured with test endpoints enabled.")
- elif resp.status_code != http_codes['OK']:
- self.raise_for_error(resp)
-
- def _notify_writes(self):
- errors = []
- for hook in self.write_hooks:
- try:
- hook()
- except Exception as e:
- errors.append(str(e))
- if errors:
- raise Exception("The operation succeeded, but one or more write"
- " hooks failed: %s" % ', '.join(errors))
-
- def _encode_owner_id(self, owner_id):
- return base64.b64encode(owner_id, altchars='-_')
-
- def raise_for_error(self, response):
- """Raise GPGServiceException based on what's in 'response'."""
- if response.headers['Content-Type'] == 'application/json':
- message = response.json()['status']
- else:
- message = "Unhandled service error. HTTP Status: %d HTTP Body: %s" % (
- response.status_code, response.content)
- raise GPGServiceException(message)
-
- @property
- def timeout(self):
- # Perhaps this should be from config?
+ def get_endpoint(self):
+ return "http://{}".format(config.gpgservice.api_endpoint)
+
+ def get_timeout(self):
return 30.0
-
- @property
- def endpoint(self):
- return "http://{}".format(config.gpgservice.api_endpoint)
-
- def _request(self, method, path, data=None, **kwargs):
- response = getattr(requests, method)(
- urljoin(self.endpoint, path), json=data, timeout=self.timeout, **kwargs)
- return response
=== modified file 'lib/lp/services/gpg/interfaces.py'
--- lib/lp/services/gpg/interfaces.py 2016-03-16 00:32:46 +0000
+++ lib/lp/services/gpg/interfaces.py 2016-03-21 05:51:58 +0000
@@ -3,6 +3,7 @@
__all__ = [
'GPG_DATABASE_READONLY_FEATURE_FLAG',
+ 'GPG_READ_FROM_GPGSERVICE_FEATURE_FLAG',
'GPG_WRITE_TO_GPGSERVICE_FEATURE_FLAG',
'GPGKeyAlgorithm',
'GPGKeyDoesNotExistOnServer',
@@ -28,6 +29,7 @@
import httplib
import re
+from gpgservice_client import GPGServiceException
from lazr.enum import (
DBEnumeratedType,
DBItem,
@@ -52,6 +54,7 @@
GPG_DATABASE_READONLY_FEATURE_FLAG = u"gpg.database_read_only"
GPG_WRITE_TO_GPGSERVICE_FEATURE_FLAG = u"gpg.write_to_gpgservice"
+GPG_READ_FROM_GPGSERVICE_FEATURE_FLAG = u"gpg.read_from_gpgservice"
def valid_fingerprint(fingerprint):
@@ -425,15 +428,6 @@
comment = Attribute("The comment portion of this user ID")
-class GPGServiceException(Exception):
-
- """Raised when we get an error from the gpgservice.
-
- More specific errors for commonly encountered errors may be added once we
- actually integrate gpgservice with the rest of launchpad.
- """
-
-
class IGPGClient(Interface):
"""A client for querying a gpgservice instance."""
@@ -467,6 +461,13 @@
:raises ValueError: if the fingerprint isn't valid.
"""
+ def getKeysByFingerprints(fingerprints):
+ """Bulk retrieve GPG keys by a list of fingerprints.
+
+ :param fingerprints: A list of fingerprints to retrieve.
+ :returns: A list of keys that were found.
+ """
+
def registerWriteHook(hook_callable):
"""Register a write hook.
=== modified file 'lib/lp/services/gpg/tests/test_gpghandler.py'
--- lib/lp/services/gpg/tests/test_gpghandler.py 2016-03-18 00:48:59 +0000
+++ lib/lp/services/gpg/tests/test_gpghandler.py 2016-03-21 05:51:58 +0000
@@ -16,6 +16,11 @@
from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
+from lp.registry.interfaces.gpg import IGPGKeySet
+from lp.registry.interfaces.person import IPersonSet
+from lp.services.database.constants import THIRTY_DAYS_AGO
+from lp.services.database.interfaces import IMasterStore
+from lp.services.features.testing import FeatureFixture
from lp.services.gpg.interfaces import (
GPGKeyDoesNotExistOnServer,
GPGKeyTemporarilyNotFoundError,
@@ -24,6 +29,7 @@
IGPGHandler,
)
from lp.services.log.logger import BufferLogger
+from lp.services.openid.model.openididentifier import OpenIdIdentifier
from lp.services.timeout import (
get_default_timeout_function,
set_default_timeout_function,
@@ -277,7 +283,9 @@
def test_get_key_for_user_with_sampledata(self):
client = getUtility(IGPGClient)
- data = client.getKeysForOwner('name16_oid')
+ person = getUtility(IPersonSet).getByName('name16')
+ openid_id = getUtility(IGPGKeySet).getOwnerIdForPerson(person)
+ data = client.getKeysForOwner(openid_id)
self.assertThat(data, ContainsDict({'keys': HasLength(1)}))
def test_get_key_for_unknown_user(self):
@@ -339,7 +347,8 @@
def test_adding_invalid_fingerprint_raises_ValueError(self):
client = getUtility(IGPGClient)
self.assertThat(
- lambda: client.addKeyForOwner(self.get_random_owner_id_string(), ''),
+ lambda: client.addKeyForOwner(
+ self.get_random_owner_id_string(), ''),
raises(ValueError("Invalid fingerprint: ''.")))
def test_adding_duplicate_fingerprint_raises_GPGServiceException(self):
@@ -351,7 +360,8 @@
client.addKeyForOwner(user_one, fingerprint)
self.assertThat(
lambda: client.addKeyForOwner(user_two, fingerprint),
- raises(GPGServiceException("Error: Fingerprint already in database.")))
+ raises(GPGServiceException(
+ "Error: Fingerprint already in database.")))
def test_disabling_active_key(self):
self.useFixture(KeyServerTac())
@@ -382,7 +392,8 @@
def test_disabling_invalid_fingerprint_raises_ValueError(self):
client = getUtility(IGPGClient)
self.assertThat(
- lambda: client.disableKeyForOwner(self.get_random_owner_id_string(), ''),
+ lambda: client.disableKeyForOwner(
+ self.get_random_owner_id_string(), ''),
raises(ValueError("Invalid fingerprint: ''."))
)
@@ -409,15 +420,76 @@
raises(ValueError))
def test_can_add_IGPGKey_to_test_enabled_gpgservice(self):
+ self.useFixture(
+ FeatureFixture({'gpg.write_to_gpgservice': True}))
client = getUtility(IGPGClient)
person = self.factory.makePerson()
+ # With the feature flag enabled, the following creates a
+ # gpg key on the gpgservice.
gpgkey = self.factory.makeGPGKey(person)
- user = self.get_random_owner_id_string()
- client.addKeyForTest(user, gpgkey.keyid, gpgkey.fingerprint,
- gpgkey.keysize, gpgkey.algorithm.name,
- gpgkey.active, gpgkey.can_encrypt)
-
+ user = getUtility(IGPGKeySet).getOwnerIdForPerson(person)
key = client.getKeyByFingerprint(gpgkey.fingerprint)
self.assertThat(
key, ContainsDict({'owner': Equals(user),
'fingerprint': Equals(gpgkey.fingerprint)}))
+
+ def makePersonWithMultipleGPGKeysInDifferentOpenids(self):
+ """Make a person with multiple GPG keys owned by
+ different openid identifiers. This happens as a result
+ of an account merge.
+
+ :returns: an IPerson instance with two keys under
+ different openid identifiers.
+ """
+ person = self.factory.makePerson()
+ self.factory.makeGPGKey(person)
+ # Create a second openid identifier from 30 days ago.
+ # This simulates the account merge:
+ identifier = OpenIdIdentifier()
+ identifier.account = person.account
+ identifier.identifier = u'openid_identifier'
+ identifier.date_created = THIRTY_DAYS_AGO
+ IMasterStore(OpenIdIdentifier).add(identifier)
+ self.factory.makeGPGKey(person)
+ return person
+
+ def test_can_retrieve_keys_for_all_openid_identifiers(self):
+ person = self.makePersonWithMultipleGPGKeysInDifferentOpenids()
+ keys = getUtility(IGPGKeySet).getGPGKeysForPerson(person)
+ self.assertThat(keys, HasLength(2))
+
+ def test_can_deactivate_all_keys_with_multiple_openid_identifiers(self):
+ person = self.makePersonWithMultipleGPGKeysInDifferentOpenids()
+ keyset = getUtility(IGPGKeySet)
+ key_one, key_two = keyset.getGPGKeysForPerson(person)
+ keyset.deactivate(key_one)
+ keyset.deactivate(key_two)
+ key_one, key_two = keyset.getGPGKeysForPerson(person, active=False)
+
+ self.assertFalse(key_one.active)
+ self.assertFalse(key_two.active)
+
+ def test_can_reactivate_all_keys_with_multiple_openid_identifiers(self):
+ person = self.makePersonWithMultipleGPGKeysInDifferentOpenids()
+ keyset = getUtility(IGPGKeySet)
+ for k in keyset.getGPGKeysForPerson(person):
+ keyset.deactivate(k)
+ for k in keyset.getGPGKeysForPerson(person, active=False):
+ keyset.activate(person, k, k.can_encrypt)
+ key_one, key_two = keyset.getGPGKeysForPerson(person)
+
+ self.assertTrue(key_one.active)
+ self.assertTrue(key_two.active)
+
+ def test_cannot_reactivate_someone_elses_key(self):
+ person1 = self.factory.makePerson()
+ key = self.factory.makeGPGKey(person1)
+ person2 = self.factory.makePerson()
+
+ keyset = getUtility(IGPGKeySet)
+ keyset.deactivate(key)
+ self.assertRaises(
+ AssertionError,
+ keyset.activate,
+ person2, key, key.can_encrypt
+ )
=== modified file 'lib/lp/services/verification/browser/tests/test_logintoken.py'
--- lib/lp/services/verification/browser/tests/test_logintoken.py 2016-02-10 02:37:07 +0000
+++ lib/lp/services/verification/browser/tests/test_logintoken.py 2016-03-21 05:51:58 +0000
@@ -21,12 +21,14 @@
from lp.services.verification.interfaces.authtoken import LoginTokenType
from lp.services.verification.interfaces.logintoken import ILoginTokenSet
from lp.testing import (
- login_person,
person_logged_in,
TestCaseWithFactory,
)
from lp.testing.deprecated import LaunchpadFormHarness
-from lp.testing.layers import DatabaseFunctionalLayer
+from lp.testing.layers import (
+ DatabaseFunctionalLayer,
+ LaunchpadFunctionalLayer,
+ )
from lp.testing.views import create_initialized_view
@@ -37,7 +39,7 @@
token to be consumed (so it can't be used again) when the user hits
Cancel.
"""
- layer = DatabaseFunctionalLayer
+ layer = LaunchpadFunctionalLayer
def setUp(self):
TestCaseWithFactory.setUp(self)
@@ -82,7 +84,7 @@
class LoginTokenReadOnlyTests(TestCaseWithFactory):
- layer = DatabaseFunctionalLayer
+ layer = LaunchpadFunctionalLayer
def test_continue_action_failed_with_gpg_database_in_ro_mode(self):
self.useFixture(FeatureFixture({
=== modified file 'lib/lp/soyuz/tests/test_archive_subscriptions.py'
--- lib/lp/soyuz/tests/test_archive_subscriptions.py 2015-09-24 11:30:24 +0000
+++ lib/lp/soyuz/tests/test_archive_subscriptions.py 2016-03-21 05:51:58 +0000
@@ -21,7 +21,10 @@
StormStatementRecorder,
TestCaseWithFactory,
)
-from lp.testing.layers import DatabaseFunctionalLayer
+from lp.testing.layers import (
+ DatabaseFunctionalLayer,
+ LaunchpadFunctionalLayer,
+ )
from lp.testing.mail_helpers import pop_notifications
from lp.testing.matchers import HasQueryCount
from lp.testing.pages import (
@@ -129,7 +132,7 @@
class PrivateArtifactsViewTestCase(BrowserTestCase):
""" Tests that private team archives can be viewed."""
- layer = DatabaseFunctionalLayer
+ layer = LaunchpadFunctionalLayer
def setUp(self):
"""Create a test archive."""
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2016-03-09 01:37:56 +0000
+++ lib/lp/testing/factory.py 2016-03-21 05:51:58 +0000
@@ -178,6 +178,7 @@
)
from lp.registry.interfaces.distroseriesparent import IDistroSeriesParentSet
from lp.registry.interfaces.gpg import IGPGKeySet
+from lp.registry.model.gpgkey import GPGServiceKey
from lp.registry.interfaces.mailinglist import (
IMailingListSet,
MailingListStatus,
@@ -233,8 +234,10 @@
from lp.services.database.sqlbase import flush_database_updates
from lp.services.features import getFeatureFlag
from lp.services.gpg.interfaces import (
+ GPG_READ_FROM_GPGSERVICE_FEATURE_FLAG,
GPG_WRITE_TO_GPGSERVICE_FEATURE_FLAG,
GPGKeyAlgorithm,
+ IGPGClient,
IGPGHandler,
)
from lp.services.identity.interfaces.account import (
@@ -603,6 +606,11 @@
client.addKeyForTest(
openid_identifier, key.keyid, key.fingerprint, key.keysize,
key.algorithm.name, key.active, key.can_encrypt)
+ # Sadly client.addKeyForTest does not return the key that
+ # was added:
+ if getFeatureFlag(GPG_READ_FROM_GPGSERVICE_FEATURE_FLAG):
+ return GPGServiceKey(
+ client.getKeyByFingerprint(key.fingerprint))
return key
def makePerson(
=== modified file 'lib/lp/testing/gpgkeys/__init__.py'
--- lib/lp/testing/gpgkeys/__init__.py 2016-02-24 22:43:07 +0000
+++ lib/lp/testing/gpgkeys/__init__.py 2016-03-21 05:51:58 +0000
@@ -31,6 +31,7 @@
from lp.services.gpg.interfaces import (
GPG_WRITE_TO_GPGSERVICE_FEATURE_FLAG,
GPGKeyAlgorithm,
+ IGPGClient,
IGPGHandler,
)
=== modified file 'lib/lp/testing/gpgservice/_fixture.py'
--- lib/lp/testing/gpgservice/_fixture.py 2016-02-16 05:36:37 +0000
+++ lib/lp/testing/gpgservice/_fixture.py 2016-03-21 05:51:58 +0000
@@ -36,8 +36,8 @@
def setUp(self):
super(GPGKeyServiceFixture, self).setUp()
- # Write service config to a file on disk. This file gets deleted when the
- # fixture ends.
+ # Write service config to a file on disk. This file gets deleted
+ # when the fixture ends.
service_config = _get_default_service_config()
self._config_file = NamedTemporaryFile()
self.addCleanup(self._config_file.close)
@@ -94,12 +94,15 @@
raise RuntimeError("Service not responding: %r" % errors)
def reset_service_database(self):
- """Reset the gpgservice instance database to the launchpad sampledata."""
+ """Reset the gpgservice instance database to the launchpad sampledata.
+ """
conn = httplib.HTTPConnection(self.bind_address)
test_data = {
'keys': [
{
- 'owner': 'name16_oid',
+ 'owner':
+ config.launchpad.openid_provider_root
+ + '+id/name16_oid',
'id': '12345678',
'fingerprint': 'ABCDEF0123456789ABCDDCBA0000111112345678',
'size': 1024,
=== modified file 'lib/lp/testing/gpgservice/tests/test_fixture.py'
--- lib/lp/testing/gpgservice/tests/test_fixture.py 2016-03-16 20:41:49 +0000
+++ lib/lp/testing/gpgservice/tests/test_fixture.py 2016-03-21 05:51:58 +0000
@@ -47,9 +47,10 @@
def test_fixture_can_create_test_data(self):
fixture = self.useFixture(GPGKeyServiceFixture())
conn = httplib.HTTPConnection(fixture.bind_address)
- conn.request(
- 'GET',
- '/users/%s/keys' % base64.b64encode('name16_oid', altchars='-_'))
+ user = base64.b64encode(
+ config.launchpad.openid_provider_root + '+id/name16_oid',
+ altchars='-_')
+ conn.request('GET', '/users/%s/keys' % user)
resp = conn.getresponse()
self.assertEqual(200, resp.status)
data = json.loads(resp.read())
=== modified file 'setup.py'
--- setup.py 2016-02-05 20:28:29 +0000
+++ setup.py 2016-03-21 05:51:58 +0000
@@ -41,6 +41,7 @@
'FeedParser',
'feedvalidator',
'funkload',
+ 'gpgservice-client',
'html5browser',
'httmock',
'pygpgme',
=== modified file 'versions.cfg'
--- versions.cfg 2016-03-14 02:01:04 +0000
+++ versions.cfg 2016-03-21 05:51:58 +0000
@@ -39,6 +39,7 @@
FormEncode = 1.2.4
funkload = 1.16.1
gpgservice = 0.1.2
+gpgservice-client = 0.0.2
grokcore.component = 1.6
gunicorn = 19.4.5
html5browser = 0.0.9
References
