launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #22418
[Merge] lp:~cjwatson/launchpad/email-force-strong-auth into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/email-force-strong-auth into lp:launchpad.
Commit message:
Allow rejecting unsigned email from a given user.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/email-force-strong-auth/+merge/343790
This is a useful strategy when handling incoming email spam from addresses whose associated users are legitimately active (and so can't just be suspended) but which are being forged by spammers.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/email-force-strong-auth into lp:launchpad.
=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py 2017-06-01 12:58:53 +0000
+++ lib/lp/registry/browser/person.py 2018-04-23 08:41:01 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Person-related view classes."""
@@ -1231,7 +1231,9 @@
label = "Review person"
field_names = [
'name', 'display_name',
- 'personal_standing', 'personal_standing_reason']
+ 'personal_standing', 'personal_standing_reason',
+ 'require_strong_email_authentication',
+ ]
custom_widget(
'personal_standing_reason', TextAreaWidget, height=5, width=60)
=== modified file 'lib/lp/registry/browser/tests/person-admin-views.txt'
--- lib/lp/registry/browser/tests/person-admin-views.txt 2015-10-01 10:25:19 +0000
+++ lib/lp/registry/browser/tests/person-admin-views.txt 2018-04-23 08:41:01 +0000
@@ -18,7 +18,8 @@
[]
>>> view.field_names
['name', 'display_name',
- 'personal_standing', 'personal_standing_reason']
+ 'personal_standing', 'personal_standing_reason',
+ 'require_strong_email_authentication']
>>> view.label
'Review person'
@@ -50,6 +51,8 @@
<DBItem PersonalStanding.POOR, ...>
>>> print user.personal_standing_reason
Zaphod's just this guy.
+ >>> user.require_strong_email_authentication
+ False
Non administrators cannot access the +review view
=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml 2017-06-16 11:10:39 +0000
+++ lib/lp/registry/configure.zcml 2018-04-23 08:41:01 +0000
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2017 Canonical Ltd. This software is licensed under the
+<!-- Copyright 2009-2018 Canonical Ltd. This software is licensed under the
GNU Affero General Public License version 3 (see the file LICENSE).
-->
@@ -1028,7 +1028,8 @@
interface="lp.registry.interfaces.person.IPersonLimitedView"/>
<require
permission="launchpad.View"
- interface="lp.registry.interfaces.person.IPersonViewRestricted"/>
+ interface="lp.registry.interfaces.person.IPersonViewRestricted
+ lp.registry.interfaces.person.IPersonModerate"/>
<require
permission="launchpad.View"
interface="lp.registry.interfaces.person.ITeamPublic"/>
@@ -1047,6 +1048,7 @@
<require
permission="launchpad.Moderate"
interface="lp.registry.interfaces.person.IPersonModerateRestricted"
+ set_schema="lp.registry.interfaces.person.IPersonModerate"
set_attributes="name"/>
<require
permission="launchpad.Special"
=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py 2018-04-17 00:26:25 +0000
+++ lib/lp/registry/interfaces/person.py 2018-04-23 08:41:01 +0000
@@ -542,7 +542,7 @@
description=_("The reason the person's standing is what it is."))
-class IPersonSettings(Interface):
+class IPersonSettingsViewRestricted(Interface):
"""Settings for a person (not a team!) that are used relatively rarely.
We store these attributes on a separate object, PersonSettings, to which
@@ -554,6 +554,9 @@
class, too.
We also may want TeamSettings and PersonTeamSettings in the future.
+
+ These attributes need launchpad.View to see, and launchpad.Edit to
+ change.
"""
selfgenerated_bugnotifications = Bool(
@@ -569,6 +572,24 @@
required=False, default=False)
+class IPersonSettingsModerate(Interface):
+ """Settings for a person (not a team!) that are used relatively rarely.
+
+ These attributes need launchpad.View to see, and launchpad.Moderate to
+ change.
+ """
+
+ require_strong_email_authentication = Bool(
+ title=_("Require strong authentication for incoming emails"),
+ description=_(
+ "If this option is set, Launchpad will only accept incoming "
+ "emails from you if it can authenticate them using GPG or DKIM. "
+ "Launchpad administrators may set this if one of your email "
+ "addresses is being forged as the sender address for incoming "
+ "spam."),
+ required=False, default=False)
+
+
class IPersonPublic(IPrivacy):
"""Public attributes for a Person.
@@ -703,7 +724,8 @@
IHasMergeProposals, IHasMugshot,
IHasLocation, IHasRequestedReviews, IObjectWithLocation,
IHasBugs, IHasRecipes, IHasTranslationImports,
- IPersonSettings, IQuestionsPerson, IHasGitRepositories):
+ IPersonSettingsViewRestricted, IQuestionsPerson,
+ IHasGitRepositories):
"""IPerson attributes that require launchpad.View permission."""
account = Object(schema=IAccount)
accountID = Int(title=_('Account ID'), required=True, readonly=True)
@@ -1865,6 +1887,10 @@
"""
+class IPersonModerate(IPersonSettingsModerate):
+ """IPerson attributes that the user can see and moderators can change."""
+
+
class IPersonModerateRestricted(Interface):
"""IPerson attributes that require launchpad.Moderate permission."""
@@ -1884,10 +1910,14 @@
as_of='devel')
+class IPersonSettings(IPersonSettingsViewRestricted, IPersonSettingsModerate):
+ """A person's settings."""
+
+
class IPerson(IPersonPublic, IPersonLimitedView, IPersonViewRestricted,
- IPersonEditRestricted, IPersonModerateRestricted,
- IPersonSpecialRestricted, IHasStanding, ISetLocation,
- IHeadingContext):
+ IPersonEditRestricted, IPersonModerate,
+ IPersonModerateRestricted, IPersonSpecialRestricted,
+ IPersonSettings, IHasStanding, ISetLocation, IHeadingContext):
"""A Person."""
export_as_webservice_entry(plural_name='people')
=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py 2018-04-17 00:26:25 +0000
+++ lib/lp/registry/model/person.py 2018-04-23 08:41:01 +0000
@@ -427,6 +427,8 @@
expanded_notification_footers = BoolCol(notNull=False, default=False)
+ require_strong_email_authentication = BoolCol(notNull=False, default=False)
+
def readonly_settings(message, interface):
"""Make an object that disallows writes to values on the interface.
=== modified file 'lib/lp/registry/tests/test_person.py'
--- lib/lp/registry/tests/test_person.py 2018-04-10 11:10:09 +0000
+++ lib/lp/registry/tests/test_person.py 2018-04-23 08:41:01 +0000
@@ -509,9 +509,9 @@
self.assertEqual(to_person, job.to_person)
self.assertEqual(requester, job.requester)
- def test_selfgenerated_bugnotifications_none_by_default(self):
+ def test_selfgenerated_bugnotifications_false_by_default(self):
# Default for new accounts is to not get any
- # self-generated bug notifications by default.
+ # self-generated bug notifications.
user = self.factory.makePerson()
self.assertFalse(user.selfgenerated_bugnotifications)
@@ -520,6 +520,23 @@
user = self.factory.makePerson()
self.assertFalse(user.expanded_notification_footers)
+ def test_require_strong_email_authentication_false_by_default(self):
+ # Default for new accounts is to not require strong email
+ # authentication.
+ user = self.factory.makePerson()
+ self.assertFalse(user.require_strong_email_authentication)
+
+ def test_require_strong_email_authentication_permissions(self):
+ # A person cannot set require_strong_email_authentication on their
+ # own account, but a registry expert can.
+ user = self.factory.makePerson()
+ with person_logged_in(user):
+ self.assertRaises(
+ Unauthorized, setattr,
+ user, 'require_strong_email_authentication', True)
+ with celebrity_logged_in('registry_experts'):
+ user.require_strong_email_authentication = True
+
def test_canAccess__anonymous(self):
# Anonymous users cannot call Person.canAccess()
person = self.factory.makePerson()
=== added file 'lib/lp/services/mail/errortemplates/person-requires-signature.txt'
--- lib/lp/services/mail/errortemplates/person-requires-signature.txt 1970-01-01 00:00:00 +0000
+++ lib/lp/services/mail/errortemplates/person-requires-signature.txt 2018-04-23 08:41:01 +0000
@@ -0,0 +1,10 @@
+Launchpad only accepts signed email from your address.
+
+If you want to use the Launchpad email interface, you will need to go here
+to import your OpenPGP key and then use it to sign your messages:
+
+ %(import_url)s
+
+If you did not knowingly send email to Launchpad, then spammers may be
+forging messages as if they were sent from your address, perhaps due to
+a compromised address book, and you can safely ignore this message.
=== modified file 'lib/lp/services/mail/helpers.py'
--- lib/lp/services/mail/helpers.py 2015-09-11 16:28:53 +0000
+++ lib/lp/services/mail/helpers.py 2018-04-23 08:41:01 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -178,8 +178,7 @@
return person_term.value
-def ensure_not_weakly_authenticated(signed_msg, context,
- error_template='not-signed.txt'):
+def ensure_not_weakly_authenticated(signed_msg, context):
"""Make sure that the current principal is not weakly authenticated.
NB: While handling an email, the authentication state is stored partly in
=== modified file 'lib/lp/services/mail/incoming.py'
--- lib/lp/services/mail/incoming.py 2015-03-13 19:05:50 +0000
+++ lib/lp/services/mail/incoming.py 2018-04-23 08:41:01 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Functions dealing with mails coming into Launchpad."""
@@ -48,6 +48,7 @@
from lp.services.mail.notification import send_process_error_notification
from lp.services.mail.sendmail import do_paranoid_envelope_to_validation
from lp.services.mail.signedmessage import signed_message_from_string
+from lp.services.webapp import canonical_url
from lp.services.webapp.errorlog import (
ErrorReportingUtility,
ScriptRequest,
@@ -56,7 +57,10 @@
get_current_principal,
setupInteraction,
)
-from lp.services.webapp.interfaces import IPlacelessAuthUtility
+from lp.services.webapp.interfaces import (
+ ILaunchBag,
+ IPlacelessAuthUtility,
+ )
# Match '\n' and '\r' line endings. That is, all '\r' that are not
# followed by a '\n', and all '\n' that are not preceded by a '\r'.
@@ -268,11 +272,20 @@
if dkim_trusted_address:
log.debug('accepting dkim strongly authenticated mail')
setupInteraction(principal, dkim_trusted_address)
- return principal
else:
log.debug("attempt gpg authentication for %r" % person)
- return _gpgAuthenticateEmail(mail, principal, person,
- signature_timestamp_checker)
+ principal = _gpgAuthenticateEmail(
+ mail, principal, person, signature_timestamp_checker)
+
+ if (IWeaklyAuthenticatedPrincipal.providedBy(principal) and
+ person.require_strong_email_authentication):
+ import_url = canonical_url(
+ getUtility(ILaunchBag).user, view_name='+editpgpkeys')
+ error_message = get_error_message(
+ 'person-requires-signature.txt', import_url=import_url)
+ raise IncomingEmailError(error_message)
+
+ return principal
def _gpgAuthenticateEmail(mail, principal, person,
=== modified file 'lib/lp/services/mail/tests/test_signedmessage.py'
--- lib/lp/services/mail/tests/test_signedmessage.py 2016-03-23 17:55:39 +0000
+++ lib/lp/services/mail/tests/test_signedmessage.py 2018-04-23 08:41:01 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test the SignedMessage class."""
@@ -19,6 +19,7 @@
from lp.registry.interfaces.person import IPersonSet
from lp.services.gpg.interfaces import IGPGHandler
+from lp.services.mail.helpers import IncomingEmailError
from lp.services.mail.incoming import (
authenticateEmail,
canonicalise_line_endings,
@@ -46,16 +47,15 @@
def test_unsigned_message(self):
# An unsigned message will not have a signature nor signed content,
- # and generates a weakly authenticated principle.
+ # and generates a weakly authenticated principal.
sender = self.factory.makePerson()
email_message = self.factory.makeEmailMessage(sender=sender)
msg = signed_message_from_string(email_message.as_string())
self.assertIs(None, msg.signedContent)
self.assertIs(None, msg.signature)
- principle = authenticateEmail(msg)
- self.assertEqual(sender, principle.person)
- self.assertTrue(
- IWeaklyAuthenticatedPrincipal.providedBy(principle))
+ principal = authenticateEmail(msg)
+ self.assertEqual(sender, principal.person)
+ self.assertTrue(IWeaklyAuthenticatedPrincipal.providedBy(principal))
self.assertIs(None, msg.signature)
def _get_clearsigned_for_person(self, sender, body=None):
@@ -78,24 +78,22 @@
def test_clearsigned_message_wrong_sender(self):
# If the message is signed, but the key doesn't belong to the sender,
- # the principle is set to the sender, but weakly authenticated.
+ # the principal is set to the sender, but weakly authenticated.
sender = self.factory.makePerson()
msg = self._get_clearsigned_for_person(sender)
- principle = authenticateEmail(msg)
+ principal = authenticateEmail(msg)
self.assertIsNot(None, msg.signature)
- self.assertEqual(sender, principle.person)
- self.assertTrue(
- IWeaklyAuthenticatedPrincipal.providedBy(principle))
+ self.assertEqual(sender, principal.person)
+ self.assertTrue(IWeaklyAuthenticatedPrincipal.providedBy(principal))
def test_clearsigned_message(self):
# The test keys belong to Sample Person.
sender = getUtility(IPersonSet).getByEmail('test@xxxxxxxxxxxxx')
msg = self._get_clearsigned_for_person(sender)
- principle = authenticateEmail(msg)
+ principal = authenticateEmail(msg)
self.assertIsNot(None, msg.signature)
- self.assertEqual(sender, principle.person)
- self.assertFalse(
- IWeaklyAuthenticatedPrincipal.providedBy(principle))
+ self.assertEqual(sender, principal.person)
+ self.assertFalse(IWeaklyAuthenticatedPrincipal.providedBy(principal))
def test_trailing_whitespace(self):
# Trailing whitespace should be ignored when verifying a message's
@@ -106,11 +104,10 @@
'And tabs\t\t\n'
'Also mixed. \t ')
msg = self._get_clearsigned_for_person(sender, body)
- principle = authenticateEmail(msg)
+ principal = authenticateEmail(msg)
self.assertIsNot(None, msg.signature)
- self.assertEqual(sender, principle.person)
- self.assertFalse(
- IWeaklyAuthenticatedPrincipal.providedBy(principle))
+ self.assertEqual(sender, principal.person)
+ self.assertFalse(IWeaklyAuthenticatedPrincipal.providedBy(principal))
def _get_detached_message_for_person(self, sender):
# Return a signed message that contains a detached signature.
@@ -147,21 +144,49 @@
def test_detached_signature_message_wrong_sender(self):
# If the message is signed, but the key doesn't belong to the sender,
- # the principle is set to the sender, but weakly authenticated.
+ # the principal is set to the sender, but weakly authenticated.
sender = self.factory.makePerson()
msg = self._get_detached_message_for_person(sender)
- principle = authenticateEmail(msg)
+ principal = authenticateEmail(msg)
self.assertIsNot(None, msg.signature)
- self.assertEqual(sender, principle.person)
- self.assertTrue(
- IWeaklyAuthenticatedPrincipal.providedBy(principle))
+ self.assertEqual(sender, principal.person)
+ self.assertTrue(IWeaklyAuthenticatedPrincipal.providedBy(principal))
def test_detached_signature_message(self):
# Test a detached correct signature.
sender = getUtility(IPersonSet).getByEmail('test@xxxxxxxxxxxxx')
msg = self._get_detached_message_for_person(sender)
- principle = authenticateEmail(msg)
- self.assertIsNot(None, msg.signature)
- self.assertEqual(sender, principle.person)
- self.assertFalse(
- IWeaklyAuthenticatedPrincipal.providedBy(principle))
+ principal = authenticateEmail(msg)
+ self.assertIsNot(None, msg.signature)
+ self.assertEqual(sender, principal.person)
+ self.assertFalse(IWeaklyAuthenticatedPrincipal.providedBy(principal))
+
+ def test_require_strong_email_authentication_and_signed(self):
+ sender = getUtility(IPersonSet).getByEmail('test@xxxxxxxxxxxxx')
+ sender.require_strong_email_authentication = True
+ msg = self._get_clearsigned_for_person(sender)
+ principal = authenticateEmail(msg)
+ self.assertIsNot(None, msg.signature)
+ self.assertEqual(sender, principal.person)
+ self.assertFalse(IWeaklyAuthenticatedPrincipal.providedBy(principal))
+
+ def test_require_strong_email_authentication_and_unsigned(self):
+ sender = getUtility(IPersonSet).getByEmail('test@xxxxxxxxxxxxx')
+ sender.require_strong_email_authentication = True
+ email_message = self.factory.makeEmailMessage(sender=sender)
+ msg = signed_message_from_string(email_message.as_string())
+ error = self.assertRaises(IncomingEmailError, authenticateEmail, msg)
+ expected_message = (
+ "Launchpad only accepts signed email from your address.\n\n"
+ "If you want to use the Launchpad email interface, you will need "
+ "to go here\n"
+ "to import your OpenPGP key and then use it to sign your "
+ "messages:\n\n"
+ " http://launchpad.dev/~%s/+editpgpkeys\n\n";
+ "If you did not knowingly send email to Launchpad, then spammers "
+ "may be\n"
+ "forging messages as if they were sent from your address, perhaps "
+ "due to\n"
+ "a compromised address book, and you can safely ignore this "
+ "message.\n" % sender.name)
+ self.assertEqual(expected_message, error.message)
Follow ups
