Provide a way to opt-in to EFF e-mail (#4082)

* Add eff email flags

* add eff_sign_up

* add requests dep to certbot

* make pylint happy

* Add EFF subscribe uri

* add POST to EFF and write tests

* log EFF e-mail submission

* Add eff module and tests

* cleanup client tests

* offer subscription when changing e-mail

* cleanup client.py and tests

* expand e-mail prompt

certbot/cli.py

@@ -880,6 +880,12 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
     helpful.add(
         ["register", "unregister", "automation"], "-m", "--email",
         help=config_help("email"))
+    helpful.add(["register", "automation"], "--eff-email", action="store_true",
+                default=None, dest="eff_email",
+                help="Share your e-mail address with EFF")
+    helpful.add(["register", "automation"], "--no-eff-email", action="store_false",
+                default=None, dest="eff_email",
+                help="Don't share your e-mail address with EFF")
     helpful.add(
         ["automation", "certonly", "run"],
         "--keep-until-expiring", "--keep", "--reinstall",

certbot/client.py

@@ -15,15 +15,16 @@ import certbot
 
 from certbot import account
 from certbot import auth_handler
+from certbot import cli
 from certbot import constants
 from certbot import crypto_util
-from certbot import errors
+from certbot import eff
 from certbot import error_handler
+from certbot import errors
 from certbot import interfaces
-from certbot import util
 from certbot import reverter
 from certbot import storage
-from certbot import cli
+from certbot import util
 
 from certbot.display import ops as display_ops
 from certbot.display import enhancements
@@ -139,6 +140,8 @@ def register(config, account_storage, tos_cb=None):
     account.report_new_account(acc, config)
     account_storage.save(acc)
 
+    eff.handle_subscription(config)
+
     return acc, acme
 
 

certbot/constants.py

@@ -106,3 +106,6 @@ RENEWAL_CONFIGS_DIR = "renewal"
 
 FORCE_INTERACTIVE_FLAG = "--force-interactive"
 """Flag to disable TTY checking in IDisplay."""
+
+EFF_SUBSCRIBE_URI = "https://supporters.eff.org/subscribe/certbot"
+"""EFF URI used to submit the e-mail address of users who opt-in."""

certbot/eff.py

@@ -0,0 +1,95 @@
+"""Subscribes users to the EFF newsletter."""
+import logging
+
+import requests
+import zope.component
+
+from certbot import constants
+from certbot import interfaces
+
+
+logger = logging.getLogger(__name__)
+
+
+def handle_subscription(config):
+    """High level function to take care of EFF newsletter subscriptions.
+
+    The user may be asked if they want to sign up for the newsletter if
+    they have not already specified.
+
+    :param .IConfig config: Client configuration.
+
+    """
+    if config.email is None:
+        if config.eff_email:
+            _report_failure("you didn't provide an e-mail address")
+        return
+    if config.eff_email is None:
+        config.eff_email = _want_subscription()
+    if config.eff_email:
+        subscribe(config.email)
+
+
+def _want_subscription():
+    """Does the user want to be subscribed to the EFF newsletter?
+
+    :returns: True if we should subscribe the user, otherwise, False
+    :rtype: bool
+
+    """
+    prompt = (
+        'Would you be willing to share your email address with the '
+        "Electronic Frontier Foundation, a founding partner of the Let's "
+        'Encrypt project and the non-profit organization that develops '
+        "Cerbot? We'd like to send you email about EFF and our work to "
+        'encrypt the web, protect its users and defend digital rights.')
+    display = zope.component.getUtility(interfaces.IDisplay)
+    return display.yesno(prompt, default=False)
+
+
+def subscribe(email):
+    """Subscribe the user to the EFF mailing list.
+
+    :param str email: the e-mail address to subscribe
+
+    """
+    url = constants.EFF_SUBSCRIBE_URI
+    data = {'data_type': 'json',
+            'email': email,
+            'form_id': 'eff_supporters_library_subscribe_form'}
+    logger.debug('Sending POST request to %s:\n%s', url, data)
+    _check_response(requests.post(url, data=data))
+
+
+def _check_response(response):
+    """Check for errors in the server's response.
+
+    If an error occurred, it will be reported to the user.
+
+    :param requests.Response response: the server's response to the
+        subscription request
+
+    """
+    logger.debug('Received response:\n%s', response.content)
+    if response.ok:
+        if not response.json()['status']:
+            _report_failure('your e-mail address appears to be invalid')
+    else:
+        _report_failure()
+
+
+def _report_failure(reason=None):
+    """Notify the user of failing to sign them up for the newsletter.
+
+    :param reason: a phrase describing what the problem was
+        beginning with a lowercase letter and no closing punctuation
+    :type reason: `str` or `None`
+
+    """
+    msg = ['We were unable to subscribe you the EFF mailing list']
+    if reason is not None:
+        msg.append(' because ')
+        msg.append(reason)
+    msg.append('. You can try again later by visiting https://act.eff.org.')
+    reporter = zope.component.getUtility(interfaces.IReporter)
+    reporter.add_message(''.join(msg), reporter.LOW_PRIORITY)

certbot/main.py

@@ -24,6 +24,7 @@ from certbot import crypto_util
 from certbot import colored_logging
 from certbot import configuration
 from certbot import constants
+from certbot import eff
 from certbot import errors
 from certbot import hooks
 from certbot import interfaces
@@ -474,6 +475,7 @@ def register(config, unused_plugins):
     acc.regr = acme_client.acme.update_registration(acc.regr.update(
         body=acc.regr.body.update(contact=('mailto:' + config.email,))))
     account_storage.save_regr(acc)
+    eff.handle_subscription(config)
     add_msg("Your e-mail address was updated to {0}.".format(config.email))
 
 

certbot/tests/client_test.py

@@ -44,20 +44,25 @@ class RegisterTest(unittest.TestCase):
     def test_no_tos(self):
         with mock.patch("certbot.client.acme_client.Client") as mock_client:
             mock_client.register().terms_of_service = "http://tos"
-            with mock.patch("certbot.account.report_new_account"):
-                self.tos_cb.return_value = False
-                self.assertRaises(errors.Error, self._call)
+            with mock.patch("certbot.eff.handle_subscription") as mock_handle:
+                with mock.patch("certbot.account.report_new_account"):
+                    self.tos_cb.return_value = False
+                    self.assertRaises(errors.Error, self._call)
+                    self.assertFalse(mock_handle.called)
 
-                self.tos_cb.return_value = True
-                self._call()
+                    self.tos_cb.return_value = True
+                    self._call()
+                    self.assertTrue(mock_handle.called)
 
-                self.tos_cb = None
-                self._call()
+                    self.tos_cb = None
+                    self._call()
+                    self.assertEqual(mock_handle.call_count, 2)
 
     def test_it(self):
         with mock.patch("certbot.client.acme_client.Client"):
             with mock.patch("certbot.account.report_new_account"):
-                self._call()
+                with mock.patch("certbot.eff.handle_subscription"):
+                    self._call()
 
     @mock.patch("certbot.account.report_new_account")
     @mock.patch("certbot.client.display_ops.get_email")
@@ -67,9 +72,11 @@ class RegisterTest(unittest.TestCase):
         msg = "DNS problem: NXDOMAIN looking up MX for example.com"
         mx_err = messages.Error.with_code('invalidContact', detail=msg)
         with mock.patch("certbot.client.acme_client.Client") as mock_client:
-            mock_client().register.side_effect = [mx_err, mock.MagicMock()]
-            self._call()
-            self.assertEqual(mock_get_email.call_count, 1)
+            with mock.patch("certbot.eff.handle_subscription") as mock_handle:
+                mock_client().register.side_effect = [mx_err, mock.MagicMock()]
+                self._call()
+                self.assertEqual(mock_get_email.call_count, 1)
+                self.assertTrue(mock_handle.called)
 
     @mock.patch("certbot.account.report_new_account")
     def test_email_invalid_noninteractive(self, _rep):
@@ -77,8 +84,9 @@ class RegisterTest(unittest.TestCase):
         msg = "DNS problem: NXDOMAIN looking up MX for example.com"
         mx_err = messages.Error.with_code('invalidContact', detail=msg)
         with mock.patch("certbot.client.acme_client.Client") as mock_client:
-            mock_client().register.side_effect = [mx_err, mock.MagicMock()]
-            self.assertRaises(errors.Error, self._call)
+            with mock.patch("certbot.eff.handle_subscription"):
+                mock_client().register.side_effect = [mx_err, mock.MagicMock()]
+                self.assertRaises(errors.Error, self._call)
 
     def test_needs_email(self):
         self.config.email = None
@@ -86,21 +94,25 @@ class RegisterTest(unittest.TestCase):
 
     @mock.patch("certbot.client.logger")
     def test_without_email(self, mock_logger):
-        with mock.patch("certbot.client.acme_client.Client"):
-            with mock.patch("certbot.account.report_new_account"):
-                self.config.email = None
-                self.config.register_unsafely_without_email = True
-                self.config.dry_run = False
-                self._call()
-                mock_logger.warning.assert_called_once_with(mock.ANY)
+        with mock.patch("certbot.eff.handle_subscription") as mock_handle:
+            with mock.patch("certbot.client.acme_client.Client"):
+                with mock.patch("certbot.account.report_new_account"):
+                    self.config.email = None
+                    self.config.register_unsafely_without_email = True
+                    self.config.dry_run = False
+                    self._call()
+                    mock_logger.warning.assert_called_once_with(mock.ANY)
+                    self.assertTrue(mock_handle.called)
 
     def test_unsupported_error(self):
         from acme import messages
         msg = "Test"
         mx_err = messages.Error(detail=msg, typ="malformed", title="title")
         with mock.patch("certbot.client.acme_client.Client") as mock_client:
-            mock_client().register.side_effect = [mx_err, mock.MagicMock()]
-            self.assertRaises(messages.Error, self._call)
+            with mock.patch("certbot.eff.handle_subscription") as mock_handle:
+                mock_client().register.side_effect = [mx_err, mock.MagicMock()]
+                self.assertRaises(messages.Error, self._call)
+        self.assertFalse(mock_handle.called)
 
 
 class ClientTestCommon(unittest.TestCase):

certbot/tests/eff_test.py

@@ -0,0 +1,135 @@
+"""Tests for certbot.eff."""
+import unittest
+
+import mock
+
+from certbot import constants
+from certbot.tests import util
+
+
+class HandleSubscriptionTest(unittest.TestCase):
+    """Tests for certbot.eff.handle_subscription."""
+    def setUp(self):
+        self.email = 'certbot@example.org'
+        self.config = mock.Mock(email=self.email, eff_email=None)
+
+    def _call(self):
+        from certbot.eff import handle_subscription
+        return handle_subscription(self.config)
+
+    @util.patch_get_utility()
+    @mock.patch('certbot.eff.subscribe')
+    def test_failure(self, mock_subscribe, mock_get_utility):
+        self.config.email = None
+        self.config.eff_email = True
+        self._call()
+        self.assertFalse(mock_subscribe.called)
+        self.assertFalse(mock_get_utility().yesno.called)
+        actual = mock_get_utility().add_message.call_args[0][0]
+        expected_part = "because you didn't provide an e-mail address"
+        self.assertTrue(expected_part in actual)
+
+    @mock.patch('certbot.eff.subscribe')
+    def test_no_subscribe_with_no_prompt(self, mock_subscribe):
+        self.config.eff_email = False
+        with util.patch_get_utility() as mock_get_utility:
+            self._call()
+        self.assertFalse(mock_subscribe.called)
+        self._assert_no_get_utility_calls(mock_get_utility)
+
+    @util.patch_get_utility()
+    @mock.patch('certbot.eff.subscribe')
+    def test_subscribe_with_no_prompt(self, mock_subscribe, mock_get_utility):
+        self.config.eff_email = True
+        self._call()
+        self._assert_subscribed(mock_subscribe)
+        self._assert_no_get_utility_calls(mock_get_utility)
+
+    def _assert_no_get_utility_calls(self, mock_get_utility):
+        self.assertFalse(mock_get_utility().yesno.called)
+        self.assertFalse(mock_get_utility().add_message.called)
+
+    @util.patch_get_utility()
+    @mock.patch('certbot.eff.subscribe')
+    def test_subscribe_with_prompt(self, mock_subscribe, mock_get_utility):
+        mock_get_utility().yesno.return_value = True
+        self._call()
+        self._assert_subscribed(mock_subscribe)
+        self.assertFalse(mock_get_utility().add_message.called)
+        self._assert_correct_yesno_call(mock_get_utility)
+
+    def _assert_subscribed(self, mock_subscribe):
+        self.assertTrue(mock_subscribe.called)
+        self.assertEqual(mock_subscribe.call_args[0][0], self.email)
+
+    @util.patch_get_utility()
+    @mock.patch('certbot.eff.subscribe')
+    def test_no_subscribe_with_prompt(self, mock_subscribe, mock_get_utility):
+        mock_get_utility().yesno.return_value = False
+        self._call()
+        self.assertFalse(mock_subscribe.called)
+        self.assertFalse(mock_get_utility().add_message.called)
+        self._assert_correct_yesno_call(mock_get_utility)
+
+    def _assert_correct_yesno_call(self, mock_get_utility):
+        self.assertTrue(mock_get_utility().yesno.called)
+        call_args, call_kwargs = mock_get_utility().yesno.call_args
+        actual = call_args[0]
+        expected_part = 'Electronic Frontier Foundation'
+        self.assertTrue(expected_part in actual)
+        self.assertFalse(call_kwargs.get('default', True))
+
+
+class SubscribeTest(unittest.TestCase):
+    """Tests for certbot.eff.subscribe."""
+    def setUp(self):
+        self.email = 'certbot@example.org'
+        self.json = {'status': True}
+        self.response = mock.Mock(ok=True)
+        self.response.json.return_value = self.json
+
+    @mock.patch('certbot.eff.requests.post')
+    def _call(self, mock_post):
+        mock_post.return_value = self.response
+
+        from certbot.eff import subscribe
+        subscribe(self.email)
+        self._check_post_call(mock_post)
+
+    def _check_post_call(self, mock_post):
+        self.assertEqual(mock_post.call_count, 1)
+        call_args, call_kwargs = mock_post.call_args
+        self.assertEqual(call_args[0], constants.EFF_SUBSCRIBE_URI)
+
+        data = call_kwargs.get('data')
+        self.assertFalse(data is None)
+        self.assertEqual(data.get('email'), self.email)
+
+    @util.patch_get_utility()
+    def test_bad_status(self, mock_get_utility):
+        self.json['status'] = False
+        self._call()  # pylint: disable=no-value-for-parameter
+        actual = self._get_reported_message(mock_get_utility)
+        expected_part = 'because your e-mail address appears to be invalid.'
+        self.assertTrue(expected_part in actual)
+
+    @util.patch_get_utility()
+    def test_not_ok(self, mock_get_utility):
+        self.response.ok = False
+        self._call()  # pylint: disable=no-value-for-parameter
+        actual = self._get_reported_message(mock_get_utility)
+        unexpected_part = 'because'
+        self.assertFalse(unexpected_part in actual)
+
+    def _get_reported_message(self, mock_get_utility):
+        self.assertTrue(mock_get_utility().add_message.called)
+        return mock_get_utility().add_message.call_args[0][0]
+
+    @util.patch_get_utility()
+    def test_subscribe(self, mock_get_utility):
+        self._call()  # pylint: disable=no-value-for-parameter
+        self.assertFalse(mock_get_utility.called)
+
+
+if __name__ == '__main__':
+    unittest.main()  # pragma: no cover

certbot/tests/main_test.py

@@ -1145,9 +1145,9 @@ class MainTest(unittest.TestCase):  # pylint: disable=too-many-public-methods
     def test_update_registration_with_email(self, mock_utility, mock_email):
         email = "user@example.com"
         mock_email.return_value = email
-        with mock.patch('certbot.main.client') as mocked_client:
-            with mock.patch('certbot.main.account') as mocked_account:
-                with mock.patch('certbot.main._determine_account') as mocked_det:
+        with mock.patch('certbot.eff.handle_subscription') as mock_handle:
+            with mock.patch('certbot.main._determine_account') as mocked_det:
+                with mock.patch('certbot.main.account') as mocked_account:
                     with mock.patch('certbot.main.client') as mocked_client:
                         mocked_storage = mock.MagicMock()
                         mocked_account.AccountFileStorage.return_value = mocked_storage
@@ -1168,6 +1168,7 @@ class MainTest(unittest.TestCase):  # pylint: disable=too-many-public-methods
                         self.assertTrue(mocked_storage.save_regr.called)
                         self.assertTrue(
                             email in mock_utility().add_message.call_args[0][0])
+                        self.assertTrue(mock_handle.called)
 
 
 class UnregisterTest(unittest.TestCase):

setup.py

@@ -31,6 +31,9 @@ changes = read_file(os.path.join(here, 'CHANGES.rst'))
 version = meta['version']
 
 # Please update tox.ini when modifying dependency version requirements
+# This package relies on requests, however, it isn't specified here to avoid
+# masking the more specific request requirements in acme. See
+# https://github.com/pypa/pip/issues/988 for more info.
 install_requires = [
     'acme=={0}'.format(version),
     # We technically need ConfigArgParse 0.10.0 for Python 2.6 support, but