launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #29750
[Merge] ~cjwatson/launchpad:signing-key-ui into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:signing-key-ui into launchpad:master.
Commit message:
Show modern archive signing keys
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1920266 in Launchpad itself: "Signing key not appearing on my PPA page"
https://bugs.launchpad.net/launchpad/+bug/1920266
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/438770
Since commit 96b7e00f45b9db262a286028a460a7ff39fcb8b8 (or in fact since 2020-09-25, when that feature rule was enabled on production), archive signing keys have been generated using the signing service and stored in the `SigningKey` table, rather than being generated locally and stored in the `GPGKey` table. However, the web UI hadn't been updated to match this, and so PPAs whose keys were generated after this time - which in fact means PPAs whose owners hadn't had any populated PPA prior to this time - didn't show signing key information.
The UI now handles both old and new key storage methods.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:signing-key-ui into launchpad:master.
diff --git a/lib/lp/archivepublisher/archivegpgsigningkey.py b/lib/lp/archivepublisher/archivegpgsigningkey.py
index 4ee076a..aae8b30 100644
--- a/lib/lp/archivepublisher/archivegpgsigningkey.py
+++ b/lib/lp/archivepublisher/archivegpgsigningkey.py
@@ -275,6 +275,7 @@ class ArchiveGPGSigningKey(SignableArchive):
default_ppa.signing_key_fingerprint
)
del get_property_cache(self.archive).signing_key
+ del get_property_cache(self.archive).signing_key_display_name
if default_ppa.signing_key_fingerprint is None:
d = IArchiveGPGSigningKey(default_ppa).generateSigningKey(
@@ -368,6 +369,7 @@ class ArchiveGPGSigningKey(SignableArchive):
self.archive.signing_key_owner = key_owner
self.archive.signing_key_fingerprint = key.fingerprint
del get_property_cache(self.archive).signing_key
+ del get_property_cache(self.archive).signing_key_display_name
def _setupSigningKey(self, signing_key, async_keyserver=False):
"""Mandatory setup for signing keys.
diff --git a/lib/lp/archivepublisher/tests/test_archivegpgsigningkey.py b/lib/lp/archivepublisher/tests/test_archivegpgsigningkey.py
index b1f0875..e4832f4 100644
--- a/lib/lp/archivepublisher/tests/test_archivegpgsigningkey.py
+++ b/lib/lp/archivepublisher/tests/test_archivegpgsigningkey.py
@@ -380,12 +380,17 @@ class TestArchiveGPGSigningKey(TestCaseWithFactory):
displayname="Celso \xe1\xe9\xed\xf3\xfa Providelo"
)
archive = self.factory.makeArchive(owner=owner)
+ self.assertIsNone(archive.signing_key_display_name)
yield IArchiveGPGSigningKey(archive).generateSigningKey(
log=logger, async_keyserver=True
)
# The key is stored in the database.
self.assertIsNotNone(archive.signing_key_owner)
self.assertIsNotNone(archive.signing_key_fingerprint)
+ self.assertEqual(
+ "1024R/0D57E99656BEFB0897606EE9A022DD1F5001B46D",
+ archive.signing_key_display_name,
+ )
# The key is stored as a GPGKey, not a SigningKey.
self.assertIsNotNone(
getUtility(IGPGKeySet).getByFingerprint(
@@ -421,6 +426,8 @@ class TestArchiveGPGSigningKey(TestCaseWithFactory):
)
default_ppa = self.factory.makeArchive(owner=owner)
another_ppa = self.factory.makeArchive(owner=owner)
+ self.assertIsNone(default_ppa.signing_key_display_name)
+ self.assertIsNone(another_ppa.signing_key_display_name)
yield IArchiveGPGSigningKey(another_ppa).generateSigningKey(
log=logger, async_keyserver=True
)
@@ -430,6 +437,9 @@ class TestArchiveGPGSigningKey(TestCaseWithFactory):
signing_key=Not(Is(None)),
signing_key_owner=Not(Is(None)),
signing_key_fingerprint=Not(Is(None)),
+ signing_key_display_name=Equals(
+ "1024R/0D57E99656BEFB0897606EE9A022DD1F5001B46D"
+ ),
),
)
self.assertIsNotNone(
@@ -448,6 +458,7 @@ class TestArchiveGPGSigningKey(TestCaseWithFactory):
signing_key=default_ppa.signing_key,
signing_key_owner=default_ppa.signing_key_owner,
signing_key_fingerprint=default_ppa.signing_key_fingerprint,
+ signing_key_display_name=default_ppa.signing_key_display_name,
),
)
@@ -469,12 +480,17 @@ class TestArchiveGPGSigningKey(TestCaseWithFactory):
}
logger = BufferLogger()
archive = self.factory.makeArchive()
+ self.assertIsNone(archive.signing_key_display_name)
yield IArchiveGPGSigningKey(archive).generateSigningKey(
log=logger, async_keyserver=True
)
# The key is stored in the database.
self.assertIsNotNone(archive.signing_key_owner)
self.assertIsNotNone(archive.signing_key_fingerprint)
+ self.assertEqual(
+ "4096R/33C0A61893A5DC5EB325B29E415A12CAC2F30234",
+ archive.signing_key_display_name,
+ )
# The key is stored as a SigningKey, not a GPGKey.
self.assertIsNone(
getUtility(IGPGKeySet).getByFingerprint(
@@ -516,6 +532,8 @@ class TestArchiveGPGSigningKey(TestCaseWithFactory):
logger = BufferLogger()
default_ppa = self.factory.makeArchive()
another_ppa = self.factory.makeArchive(owner=default_ppa.owner)
+ self.assertIsNone(default_ppa.signing_key_display_name)
+ self.assertIsNone(another_ppa.signing_key_display_name)
yield IArchiveGPGSigningKey(another_ppa).generateSigningKey(
log=logger, async_keyserver=True
)
@@ -525,6 +543,9 @@ class TestArchiveGPGSigningKey(TestCaseWithFactory):
signing_key=Is(None),
signing_key_owner=Not(Is(None)),
signing_key_fingerprint=Not(Is(None)),
+ signing_key_display_name=Equals(
+ "4096R/33C0A61893A5DC5EB325B29E415A12CAC2F30234"
+ ),
),
)
self.assertIsNone(
@@ -544,5 +565,8 @@ class TestArchiveGPGSigningKey(TestCaseWithFactory):
signing_key_fingerprint=Equals(
default_ppa.signing_key_fingerprint
),
+ signing_key_display_name=Equals(
+ default_ppa.signing_key_display_name
+ ),
),
)
diff --git a/lib/lp/soyuz/browser/tests/test_personal_archive_subscription.py b/lib/lp/soyuz/browser/tests/test_personal_archive_subscription.py
index 813aaf5..4bf4496 100644
--- a/lib/lp/soyuz/browser/tests/test_personal_archive_subscription.py
+++ b/lib/lp/soyuz/browser/tests/test_personal_archive_subscription.py
@@ -3,13 +3,21 @@
"""Tests for the PersonalArchiveSubscription components and view."""
-from lp.app.interfaces.launchpad import IPrivacy
+from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
+
+from lp.app.interfaces.launchpad import ILaunchpadCelebrities, IPrivacy
+from lp.services.config import config
+from lp.services.signing.enums import SigningKeyType
from lp.soyuz.browser.archivesubscription import PersonalArchiveSubscription
from lp.testing import TestCaseWithFactory, person_logged_in
+from lp.testing.gpgkeys import test_pubkey_from_email
from lp.testing.layers import DatabaseFunctionalLayer
+from lp.testing.pages import extract_text, find_tag_by_id
+from lp.testing.views import create_initialized_view
-class TestSomething(TestCaseWithFactory):
+class TestPersonArchiveSubscriptionView(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
@@ -22,3 +30,62 @@ class TestSomething(TestCaseWithFactory):
pas = PersonalArchiveSubscription(subscriber, pppa)
privacy = IPrivacy(pas)
self.assertTrue(privacy.private)
+
+ def test_signed_with_local_key(self):
+ owner = self.factory.makePerson(name="archiveowner")
+ subscriber = self.factory.makePerson(name="subscriber")
+ pppa = self.factory.makeArchive(owner=owner, private=True, name="pppa")
+ key = self.factory.makeGPGKey(owner=owner)
+ removeSecurityProxy(pppa).signing_key_fingerprint = key.fingerprint
+ removeSecurityProxy(pppa).signing_key_owner = getUtility(
+ ILaunchpadCelebrities
+ ).ppa_key_guard
+ with person_logged_in(owner):
+ pppa.newSubscription(subscriber, owner)
+ with person_logged_in(subscriber):
+ pppa.newAuthToken(subscriber)
+ pas = PersonalArchiveSubscription(subscriber, pppa)
+ view = create_initialized_view(pas, "+index", principal=subscriber)
+ signing_key = find_tag_by_id(view(), "signing-key")
+ self.assertEqual(
+ "This repository is signed with\n%s%s/%s OpenPGP key."
+ % (key.keysize, key.algorithm.title, key.fingerprint),
+ extract_text(signing_key),
+ )
+ self.assertEqual(
+ "https://%s/pks/lookup?fingerprint=on&op=index&search=0x%s";
+ % (config.gpghandler.public_host, key.fingerprint),
+ signing_key.a["href"],
+ )
+
+ def test_signed_with_signing_service_key(self):
+ owner = self.factory.makePerson(name="archiveowner")
+ subscriber = self.factory.makePerson(name="subscriber")
+ pppa = self.factory.makeArchive(owner=owner, private=True, name="pppa")
+ test_key = test_pubkey_from_email("test@xxxxxxxxxxxxx")
+ key = self.factory.makeSigningKey(
+ SigningKeyType.OPENPGP,
+ fingerprint="A419AE861E88BC9E04B9C26FBA2B9389DFD20543",
+ public_key=test_key,
+ )
+ removeSecurityProxy(pppa).signing_key_fingerprint = key.fingerprint
+ removeSecurityProxy(pppa).signing_key_owner = getUtility(
+ ILaunchpadCelebrities
+ ).ppa_key_guard
+ with person_logged_in(owner):
+ pppa.newSubscription(subscriber, owner)
+ with person_logged_in(subscriber):
+ pppa.newAuthToken(subscriber)
+ pas = PersonalArchiveSubscription(subscriber, pppa)
+ view = create_initialized_view(pas, "+index", principal=subscriber)
+ signing_key = find_tag_by_id(view(), "signing-key")
+ self.assertEqual(
+ "This repository is signed with\n1024D/%s OpenPGP key."
+ % key.fingerprint,
+ extract_text(signing_key),
+ )
+ self.assertEqual(
+ "https://%s/pks/lookup?fingerprint=on&op=index&search=0x%s";
+ % (config.gpghandler.public_host, key.fingerprint),
+ signing_key.a["href"],
+ )
diff --git a/lib/lp/soyuz/configure.zcml b/lib/lp/soyuz/configure.zcml
index 01a0a45..9d38192 100644
--- a/lib/lp/soyuz/configure.zcml
+++ b/lib/lp/soyuz/configure.zcml
@@ -137,6 +137,8 @@
signing_key
signing_key_fingerprint
signing_key_owner
+ signing_key_keyserver_url
+ signing_key_display_name
archive
sources
sourceFileUrls
diff --git a/lib/lp/soyuz/interfaces/archive.py b/lib/lp/soyuz/interfaces/archive.py
index f31793b..f824ceb 100644
--- a/lib/lp/soyuz/interfaces/archive.py
+++ b/lib/lp/soyuz/interfaces/archive.py
@@ -535,6 +535,12 @@ class IArchiveSubscriberView(Interface):
signing_key = Object(
title=_("Repository signing key."), required=False, schema=IGPGKey
)
+ signing_key_keyserver_url = TextLine(
+ title=_("Keyserver URL for archive signing key."), required=False
+ )
+ signing_key_display_name = TextLine(
+ title=_("Display name for archive signing key."), required=False
+ )
@export_read_operation()
@operation_for_version("devel")
diff --git a/lib/lp/soyuz/model/archive.py b/lib/lp/soyuz/model/archive.py
index d96fc8e..94993a6 100644
--- a/lib/lp/soyuz/model/archive.py
+++ b/lib/lp/soyuz/model/archive.py
@@ -11,6 +11,7 @@ __all__ = [
"validate_ppa",
]
+import logging
import re
import typing
from datetime import datetime
@@ -98,6 +99,8 @@ from lp.services.gpg.interfaces import IGPGHandler
from lp.services.job.interfaces.job import JobStatus
from lp.services.librarian.model import LibraryFileAlias, LibraryFileContent
from lp.services.propertycache import cachedproperty, get_property_cache
+from lp.services.signing.enums import SigningKeyType
+from lp.services.signing.interfaces.signingkey import ISigningKeySet
from lp.services.tokens import create_token
from lp.services.webapp.authorization import check_permission
from lp.services.webapp.interfaces import ILaunchBag
@@ -422,6 +425,40 @@ class Archive(SQLBase):
return key_data.decode("UTF-8")
@property
+ def signing_key_keyserver_url(self):
+ if self.signing_key_fingerprint is not None:
+ return getUtility(IGPGHandler).getURLForKeyInServer(
+ self.signing_key_fingerprint, public=True
+ )
+
+ @cachedproperty
+ def signing_key_display_name(self):
+ if self.signing_key is not None:
+ # Signing key is stored as a GPGKey.
+ return self.signing_key.displayname
+ elif self.signing_key_fingerprint is not None:
+ signing_key = getUtility(ISigningKeySet).get(
+ SigningKeyType.OPENPGP, self.signing_key_fingerprint
+ )
+ if signing_key is not None:
+ # Signing key is stored on the signing service.
+ try:
+ pyme_key = getUtility(IGPGHandler).importPublicKey(
+ signing_key.public_key
+ )
+ except Exception:
+ logging.getLogger(__name__).exception(
+ "Failed to import public key %s from SigningKey",
+ self.signing_key_fingerprint,
+ )
+ else:
+ return "%s%s/%s" % (
+ pyme_key.keysize,
+ pyme_key.algorithm.title,
+ pyme_key.fingerprint,
+ )
+
+ @property
def is_ppa(self):
"""See `IArchive`."""
return self.purpose == ArchivePurpose.PPA
diff --git a/lib/lp/soyuz/stories/ppa/xx-ubuntu-ppas.rst b/lib/lp/soyuz/stories/ppa/xx-ubuntu-ppas.rst
index b89f0c4..09aeab0 100644
--- a/lib/lp/soyuz/stories/ppa/xx-ubuntu-ppas.rst
+++ b/lib/lp/soyuz/stories/ppa/xx-ubuntu-ppas.rst
@@ -655,6 +655,48 @@ And further down, next to the key id, we link to that same pop-up help:
>>> print(anon_browser.getLink("What is this?").url)
http://launchpad.test/+help-soyuz/ppa-sources-list.html
+Try the same again, but this time using the signing service.
+
+ >>> from lp.app.interfaces.launchpad import ILaunchpadCelebrities
+ >>> from lp.services.propertycache import get_property_cache
+ >>> from lp.services.signing.enums import SigningKeyType
+ >>> from lp.testing.gpgkeys import test_pubkey_from_email
+
+ >>> login("foo.bar@xxxxxxxxxxxxx")
+ >>> test_key = test_pubkey_from_email("test@xxxxxxxxxxxxx")
+ >>> signing_key = factory.makeSigningKey(
+ ... SigningKeyType.OPENPGP,
+ ... fingerprint="A419AE861E88BC9E04B9C26FBA2B9389DFD20543",
+ ... public_key=test_key,
+ ... )
+ >>> removeSecurityProxy(no_priv.archive).signing_key_owner = getUtility(
+ ... ILaunchpadCelebrities
+ ... ).ppa_key_guard
+ >>> removeSecurityProxy(
+ ... no_priv.archive
+ ... ).signing_key_fingerprint = signing_key.fingerprint
+ >>> del get_property_cache(no_priv.archive).signing_key
+ >>> del get_property_cache(no_priv.archive).signing_key_display_name
+ >>> logout()
+
+ >>> anon_browser.reload()
+
+ >>> signing_key_section = find_tag_by_id(
+ ... anon_browser.contents, "signing-key"
+ ... )
+
+ >>> print(extract_text(signing_key_section))
+ Signing key: 1024D/A419AE861E88BC9E04B9C26FBA2B9389DFD20543
+ (What is this?)
+ Fingerprint: A419AE861E88BC9E04B9C26FBA2B9389DFD20543
+
+ >>> print(
+ ... anon_browser.getLink(
+ ... "1024D/A419AE861E88BC9E04B9C26FBA2B9389DFD20543"
+ ... ).url
+ ... ) # noqa
+ https://keyserver.ubuntu.com/pks/lookup?fingerprint=on&op=index&search=0xA419AE861E88BC9E04B9C26FBA2B9389DFD20543
+
Single-publication PPAs
-----------------------
diff --git a/lib/lp/soyuz/templates/archive-index.pt b/lib/lp/soyuz/templates/archive-index.pt
index f7e38b3..8f52ffa 100644
--- a/lib/lp/soyuz/templates/archive-index.pt
+++ b/lib/lp/soyuz/templates/archive-index.pt
@@ -105,19 +105,18 @@ sudo apt update
sources. </p>
<tal:entries replace="structure view/sources_list_entries" />
<dl id="signing-key"
- tal:define="signing_key context/signing_key"
- tal:condition="signing_key">
+ tal:condition="context/signing_key_fingerprint">
<dt>Signing key:</dt>
<dd>
- <a tal:attributes="href signing_key/keyserverURL">
- <code tal:content="signing_key/displayname"
+ <a tal:attributes="href context/signing_key_keyserver_url">
+ <code tal:content="context/signing_key_display_name"
>1024R/23456789</code>
</a>
(<a href="/+help-soyuz/ppa-sources-list.html"
target="help">What is this?</a>)
</dd>
<dt>Fingerprint:</dt>
- <dd tal:content="signing_key/fingerprint"/>
+ <dd tal:content="context/signing_key_fingerprint"/>
</dl>
<div id="archive-dependencies"
tal:condition="not: context/dependencies/is_empty">
diff --git a/lib/lp/soyuz/templates/person-archive-subscription.pt b/lib/lp/soyuz/templates/person-archive-subscription.pt
index efc9003..7810d3b 100644
--- a/lib/lp/soyuz/templates/person-archive-subscription.pt
+++ b/lib/lp/soyuz/templates/person-archive-subscription.pt
@@ -37,11 +37,10 @@
</p>
<tal:entries replace="structure view/sources_list_entries" />
<div id="signing-key"
- tal:define="signing_key context/archive/signing_key"
- tal:condition="signing_key">
+ tal:condition="context/archive/signing_key_fingerprint">
<p>This repository is signed with
- <a tal:attributes="href signing_key/keyserverURL">
- <code tal:content="signing_key/displayname">
+ <a tal:attributes="href context/archive/signing_key_keyserver_url">
+ <code tal:content="context/archive/signing_key_display_name">
1024R/23456789
</code></a> OpenPGP key.
</p>
