diff options
50 files changed, 1646 insertions, 587 deletions
diff --git a/coverage.ini b/coverage.ini index 3deaacf0c..a981671b2 100644 --- a/coverage.ini +++ b/coverage.ini @@ -8,6 +8,7 @@ omit = .tox/*/lib/python3.5/site-packages/* */test_*.py /tmp/* + */testing/*.py [report] exclude_lines = diff --git a/src/mailman/app/docs/moderator.rst b/src/mailman/app/docs/moderator.rst index 988e8bebc..ce25a4711 100644 --- a/src/mailman/app/docs/moderator.rst +++ b/src/mailman/app/docs/moderator.rst @@ -221,8 +221,8 @@ Fred is a member of the mailing list... >>> mlist.send_welcome_message = False >>> fred = getUtility(IUserManager).create_address( ... 'fred@example.com', 'Fred Person') - >>> from mailman.interfaces.registrar import IRegistrar - >>> registrar = IRegistrar(mlist) + >>> from mailman.interfaces.subscriptions import ISubscriptionManager + >>> registrar = ISubscriptionManager(mlist) >>> token, token_owner, member = registrar.register( ... fred, pre_verified=True, pre_confirmed=True, pre_approved=True) >>> member diff --git a/src/mailman/app/events.py b/src/mailman/app/events.py index 8b95bd4be..f3323b6ce 100644 --- a/src/mailman/app/events.py +++ b/src/mailman/app/events.py @@ -18,8 +18,7 @@ """Global events.""" from mailman import public -from mailman.app import ( - domain, membership, moderator, registrar, subscriptions) +from mailman.app import domain, membership, moderator, subscriptions from mailman.core import i18n, switchboard from mailman.languages import manager as language_manager from mailman.styles import manager as style_manager @@ -37,8 +36,9 @@ def initialize(): membership.handle_SubscriptionEvent, moderator.handle_ListDeletingEvent, passwords.handle_ConfigurationUpdatedEvent, - registrar.handle_ConfirmationNeededEvent, style_manager.handle_ConfigurationUpdatedEvent, subscriptions.handle_ListDeletingEvent, + subscriptions.handle_SubscriptionConfirmationNeededEvent, + subscriptions.handle_UnsubscriptionConfirmationNeededEvent, switchboard.handle_ConfigurationUpdatedEvent, ]) diff --git a/src/mailman/app/registrar.py b/src/mailman/app/registrar.py deleted file mode 100644 index 7cefbd518..000000000 --- a/src/mailman/app/registrar.py +++ /dev/null @@ -1,105 +0,0 @@ -# Copyright (C) 2007-2016 by the Free Software Foundation, Inc. -# -# This file is part of GNU Mailman. -# -# GNU Mailman is free software: you can redistribute it and/or modify it under -# the terms of the GNU General Public License as published by the Free -# Software Foundation, either version 3 of the License, or (at your option) -# any later version. -# -# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT -# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or -# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for -# more details. -# -# You should have received a copy of the GNU General Public License along with -# GNU Mailman. If not, see <http://www.gnu.org/licenses/>. - -"""Implementation of the IRegistrar interface.""" - -import logging - -from mailman import public -from mailman.app.subscriptions import SubscriptionWorkflow -from mailman.database.transaction import flush -from mailman.email.message import UserNotification -from mailman.interfaces.pending import IPendable, IPendings -from mailman.interfaces.registrar import ConfirmationNeededEvent, IRegistrar -from mailman.interfaces.template import ITemplateLoader -from mailman.interfaces.workflow import IWorkflowStateManager -from mailman.utilities.string import expand -from zope.component import getUtility -from zope.interface import implementer - - -log = logging.getLogger('mailman.error') - - -@implementer(IPendable) -class PendableRegistration(dict): - PEND_TYPE = 'registration' - - -@public -@implementer(IRegistrar) -class Registrar: - """Handle registrations and confirmations for subscriptions.""" - - def __init__(self, mlist): - self._mlist = mlist - - def register(self, subscriber=None, *, - pre_verified=False, pre_confirmed=False, pre_approved=False): - """See `IRegistrar`.""" - workflow = SubscriptionWorkflow( - self._mlist, subscriber, - pre_verified=pre_verified, - pre_confirmed=pre_confirmed, - pre_approved=pre_approved) - list(workflow) - return workflow.token, workflow.token_owner, workflow.member - - def confirm(self, token): - """See `IRegistrar`.""" - workflow = SubscriptionWorkflow(self._mlist) - workflow.token = token - workflow.restore() - list(workflow) - return workflow.token, workflow.token_owner, workflow.member - - def discard(self, token): - """See `IRegistrar`.""" - with flush(): - getUtility(IPendings).confirm(token) - getUtility(IWorkflowStateManager).discard( - SubscriptionWorkflow.__name__, token) - - -@public -def handle_ConfirmationNeededEvent(event): - if not isinstance(event, ConfirmationNeededEvent): - return - # There are three ways for a user to confirm their subscription. They - # can reply to the original message and let the VERP'd return address - # encode the token, they can reply to the robot and keep the token in - # the Subject header, or they can click on the URL in the body of the - # message and confirm through the web. - subject = 'confirm {}'.format(event.token) - confirm_address = event.mlist.confirm_address(event.token) - email_address = event.email - # Send a verification email to the address. - template = getUtility(ITemplateLoader).get( - 'list:user:action:confirm', event.mlist) - text = expand(template, event.mlist, dict( - token=event.token, - subject=subject, - confirm_email=confirm_address, - user_email=email_address, - # For backward compatibility. - confirm_address=confirm_address, - email_address=email_address, - domain_name=event.mlist.domain.mail_host, - contact_address=event.mlist.owner_address, - )) - msg = UserNotification(email_address, confirm_address, subject, text) - msg.send(event.mlist, add_precedence=False) diff --git a/src/mailman/app/subscriptions.py b/src/mailman/app/subscriptions.py index a397c4fde..9e915a25d 100644 --- a/src/mailman/app/subscriptions.py +++ b/src/mailman/app/subscriptions.py @@ -24,18 +24,21 @@ from datetime import timedelta from email.utils import formataddr from enum import Enum from mailman import public +from mailman.app.membership import delete_member from mailman.app.workflow import Workflow from mailman.core.i18n import _ +from mailman.database.transaction import flush from mailman.email.message import UserNotification from mailman.interfaces.address import IAddress from mailman.interfaces.bans import IBanManager from mailman.interfaces.listmanager import ListDeletingEvent from mailman.interfaces.mailinglist import SubscriptionPolicy -from mailman.interfaces.member import MembershipIsBannedError +from mailman.interfaces.member import MembershipIsBannedError, NotAMemberError from mailman.interfaces.pending import IPendable, IPendings -from mailman.interfaces.registrar import ConfirmationNeededEvent from mailman.interfaces.subscriptions import ( - ISubscriptionService, SubscriptionPendingError, TokenOwner) + ISubscriptionManager, ISubscriptionService, + SubscriptionConfirmationNeededEvent, SubscriptionPendingError, TokenOwner, + UnsubscriptionConfirmationNeededEvent) from mailman.interfaces.template import ITemplateLoader from mailman.interfaces.user import IUser from mailman.interfaces.usermanager import IUserManager @@ -56,27 +59,21 @@ class WhichSubscriber(Enum): @implementer(IPendable) -class Pendable(dict): +class PendableSubscription(dict): PEND_TYPE = 'subscription' -@public -class SubscriptionWorkflow(Workflow): - """Workflow of a subscription request.""" +@implementer(IPendable) +class PendableUnsubscription(dict): + PEND_TYPE = 'unsubscription' - INITIAL_STATE = 'sanity_checks' - SAVE_ATTRIBUTES = ( - 'pre_approved', - 'pre_confirmed', - 'pre_verified', - 'address_key', - 'subscriber_key', - 'user_key', - 'token_owner_key', - ) - def __init__(self, mlist, subscriber=None, *, - pre_verified=False, pre_confirmed=False, pre_approved=False): +class _SubscriptionWorkflowCommon(Workflow): + """Common support between subscription and unsubscription.""" + + PENDABLE_CLASS = None + + def __init__(self, mlist, subscriber): super().__init__() self.mlist = mlist self.address = None @@ -94,9 +91,6 @@ class SubscriptionWorkflow(Workflow): self.user = subscriber self.which = WhichSubscriber.user self.subscriber = subscriber - self.pre_verified = pre_verified - self.pre_confirmed = pre_confirmed - self.pre_approved = pre_approved @property def user_key(self): @@ -150,7 +144,7 @@ class SubscriptionWorkflow(Workflow): if token_owner is TokenOwner.no_one: self.token = None return - pendable = Pendable( + pendable = self.PENDABLE_CLASS( list_id=self.mlist.list_id, email=self.address.email, display_name=self.address.display_name, @@ -159,6 +153,30 @@ class SubscriptionWorkflow(Workflow): ) self.token = pendings.add(pendable, timedelta(days=3650)) + +@public +class SubscriptionWorkflow(_SubscriptionWorkflowCommon): + """Workflow of a subscription request.""" + + PENDABLE_CLASS = PendableSubscription + INITIAL_STATE = 'sanity_checks' + SAVE_ATTRIBUTES = ( + 'pre_approved', + 'pre_confirmed', + 'pre_verified', + 'address_key', + 'subscriber_key', + 'user_key', + 'token_owner_key', + ) + + def __init__(self, mlist, subscriber=None, *, + pre_verified=False, pre_confirmed=False, pre_approved=False): + super().__init__(mlist, subscriber) + self.pre_verified = pre_verified + self.pre_confirmed = pre_confirmed + self.pre_approved = pre_approved + def _step_sanity_checks(self): # Ensure that we have both an address and a user, even if the address # is not verified. We can't set the preferred address until it is @@ -288,15 +306,15 @@ class SubscriptionWorkflow(Workflow): def _step_do_subscription(self): # We can immediately subscribe the user to the mailing list. self.member = self.mlist.subscribe(self.subscriber) - # This workflow is done so throw away any associated state. - getUtility(IWorkflowStateManager).restore(self.name, self.token) + assert self.token is None and self.token_owner is TokenOwner.no_one, ( + 'Unexpected active token at end of subscription workflow') def _step_send_confirmation(self): self._set_token(TokenOwner.subscriber) self.push('do_confirm_verify') self.save() # Triggering this event causes the confirmation message to be sent. - notify(ConfirmationNeededEvent( + notify(SubscriptionConfirmationNeededEvent( self.mlist, self.token, self.address.email)) # Now we wait for the confirmation. raise StopIteration @@ -329,6 +347,233 @@ class SubscriptionWorkflow(Workflow): @public +class UnSubscriptionWorkflow(_SubscriptionWorkflowCommon): + """Workflow of a unsubscription request.""" + + PENDABLE_CLASS = PendableUnsubscription + INITIAL_STATE = 'subscription_checks' + SAVE_ATTRIBUTES = ( + 'pre_approved', + 'pre_confirmed', + 'address_key', + 'user_key', + 'subscriber_key', + 'token_owner_key', + ) + + def __init__(self, mlist, subscriber=None, *, + pre_approved=False, pre_confirmed=False): + super().__init__(mlist, subscriber) + if IAddress.providedBy(subscriber) or IUser.providedBy(subscriber): + self.member = self.mlist.regular_members.get_member( + self.address.email) + self.pre_confirmed = pre_confirmed + self.pre_approved = pre_approved + + def _step_subscription_checks(self): + assert self.mlist.is_subscribed(self.subscriber) + self.push('confirmation_checks') + + def _step_confirmation_checks(self): + # If list's unsubscription policy is open, the user can unsubscribe + # right now. + if self.mlist.unsubscription_policy is SubscriptionPolicy.open: + self.push('do_unsubscription') + return + # If we don't need the user's confirmation, then skip to the moderation + # checks. + if self.mlist.unsubscription_policy is SubscriptionPolicy.moderate: + self.push('moderation_checks') + return + # If the request is pre-confirmed, then the user can unsubscribe right + # now. + if self.pre_confirmed: + self.push('do_unsubscription') + return + # The user must confirm their un-subsbcription. + self.push('send_confirmation') + + def _step_send_confirmation(self): + self._set_token(TokenOwner.subscriber) + self.push('do_confirm_verify') + self.save() + notify(UnsubscriptionConfirmationNeededEvent( + self.mlist, self.token, self.address.email)) + raise StopIteration + + def _step_moderation_checks(self): + # Does the moderator need to approve the unsubscription request? + assert self.mlist.unsubscription_policy in ( + SubscriptionPolicy.moderate, + SubscriptionPolicy.confirm_then_moderate, + ), self.mlist.unsubscription_policy + if self.pre_approved: + self.push('do_unsubscription') + else: + self.push('get_moderator_approval') + + def _step_get_moderator_approval(self): + self._set_token(TokenOwner.moderator) + self.push('unsubscribe_from_restored') + self.save() + log.info('{}: held unsubscription request from {}'.format( + self.mlist.fqdn_listname, self.address.email)) + if self.mlist.admin_immed_notify: + subject = _( + 'New unsubscription request to $self.mlist.display_name ' + 'from $self.address.email') + username = formataddr( + (self.subscriber.display_name, self.address.email)) + template = getUtility(ITemplateLoader).get( + 'list:admin:action:unsubscribe', self.mlist) + text = wrap(expand(template, self.mlist, dict( + member=username, + ))) + # This message should appear to come from the <list>-owner so as + # to avoid any useless bounce processing. + msg = UserNotification( + self.mlist.owner_address, self.mlist.owner_address, + subject, text, self.mlist.preferred_language) + msg.send(self.mlist, tomoderators=True) + # The workflow must stop running here + raise StopIteration + + def _step_do_confirm_verify(self): + # Restore a little extra state that can't be stored in the database + # (because the order of setattr() on restore is indeterminate), then + # continue with the confirmation/verification step. + if self.which is WhichSubscriber.address: + self.subscriber = self.address + else: + assert self.which is WhichSubscriber.user + self.subscriber = self.user + # Reset the token so it can't be used in a replay attack. + self._set_token(TokenOwner.no_one) + # Restore the member object. + self.member = self.mlist.regular_members.get_member(self.address.email) + # It's possible the member was already unsubscribed while we were + # waiting for the confirmation. + if self.member is None: + return + # The user has confirmed their unsubscription request + next_step = ('moderation_checks' + if self.mlist.unsubscription_policy in ( + SubscriptionPolicy.moderate, + SubscriptionPolicy.confirm_then_moderate, + ) + else 'do_unsubscription') + self.push(next_step) + + def _step_do_unsubscription(self): + try: + delete_member(self.mlist, self.address.email) + except NotAMemberError: + # The member has already been unsubscribed. + pass + self.member = None + assert self.token is None and self.token_owner is TokenOwner.no_one, ( + 'Unexpected active token at end of subscription workflow') + + def _step_unsubscribe_from_restored(self): + # Prevent replay attacks. + self._set_token(TokenOwner.no_one) + if self.which is WhichSubscriber.address: + self.subscriber = self.address + else: + assert self.which is WhichSubscriber.user + self.subscriber = self.user + self.push('do_unsubscription') + + +@public +@implementer(ISubscriptionManager) +class SubscriptionManager: + def __init__(self, mlist): + self._mlist = mlist + + def register(self, subscriber=None, *, + pre_verified=False, pre_confirmed=False, pre_approved=False): + """See `ISubscriptionManager`.""" + workflow = SubscriptionWorkflow( + self._mlist, subscriber, + pre_verified=pre_verified, + pre_confirmed=pre_confirmed, + pre_approved=pre_approved) + list(workflow) + return workflow.token, workflow.token_owner, workflow.member + + def unregister(self, subscriber=None, *, + pre_confirmed=False, pre_approved=False): + workflow = UnSubscriptionWorkflow( + self._mlist, subscriber, + pre_confirmed=pre_confirmed, + pre_approved=pre_approved) + list(workflow) + return workflow.token, workflow.token_owner, workflow.member + + def confirm(self, token): + if token is None: + raise LookupError + pendable = getUtility(IPendings).confirm(token, expunge=False) + if pendable is None: + raise LookupError + workflow_type = pendable.get('type') + assert workflow_type in (PendableSubscription.PEND_TYPE, + PendableUnsubscription.PEND_TYPE) + workflow = (SubscriptionWorkflow + if workflow_type == PendableSubscription.PEND_TYPE + else UnSubscriptionWorkflow)(self._mlist) + workflow.token = token + workflow.restore() + # In order to just run the whole workflow, all we need to do + # is iterate over the workflow object. On calling the __next__ + # over the workflow iterator it automatically executes the steps + # that needs to be done. + list(workflow) + return workflow.token, workflow.token_owner, workflow.member + + def discard(self, token): + with flush(): + getUtility(IPendings).confirm(token) + getUtility(IWorkflowStateManager).discard(token) + + +def _handle_confirmation_needed_events(event, template_name): + subject = 'confirm {}'.format(event.token) + confirm_address = event.mlist.confirm_address(event.token) + email_address = event.email + # Send a verification email to the address. + template = getUtility(ITemplateLoader).get(template_name, event.mlist) + text = expand(template, event.mlist, dict( + token=event.token, + subject=subject, + confirm_email=confirm_address, + user_email=email_address, + # For backward compatibility. + confirm_address=confirm_address, + email_address=email_address, + domain_name=event.mlist.domain.mail_host, + contact_address=event.mlist.owner_address, + )) + msg = UserNotification(email_address, confirm_address, subject, text) + msg.send(event.mlist, add_precedence=False) + + +@public +def handle_SubscriptionConfirmationNeededEvent(event): + if not isinstance(event, SubscriptionConfirmationNeededEvent): + return + _handle_confirmation_needed_events(event, 'list:user:action:subscribe') + + +@public +def handle_UnsubscriptionConfirmationNeededEvent(event): + if not isinstance(event, UnsubscriptionConfirmationNeededEvent): + return + _handle_confirmation_needed_events(event, 'list:user:action:unsubscribe') + + +@public def handle_ListDeletingEvent(event): """Delete a mailing list's members when the list is being deleted.""" diff --git a/src/mailman/app/tests/test_moderation.py b/src/mailman/app/tests/test_moderation.py index 931b85fd0..20584da49 100644 --- a/src/mailman/app/tests/test_moderation.py +++ b/src/mailman/app/tests/test_moderation.py @@ -24,8 +24,8 @@ from mailman.app.moderator import ( handle_message, handle_unsubscription, hold_message, hold_unsubscription) from mailman.interfaces.action import Action from mailman.interfaces.messages import IMessageStore -from mailman.interfaces.registrar import IRegistrar from mailman.interfaces.requests import IListRequests +from mailman.interfaces.subscriptions import ISubscriptionManager from mailman.interfaces.usermanager import IUserManager from mailman.runners.incoming import IncomingRunner from mailman.runners.outgoing import OutgoingRunner @@ -153,17 +153,21 @@ class TestUnsubscription(unittest.TestCase): def setUp(self): self._mlist = create_list('test@example.com') - self._registrar = IRegistrar(self._mlist) + self._manager = ISubscriptionManager(self._mlist) def test_unsubscribe_defer(self): # When unsubscriptions must be approved by the moderator, but the # moderator defers this decision. anne = getUtility(IUserManager).create_address( 'anne@example.org', 'Anne Person') - token, token_owner, member = self._registrar.register( + token, token_owner, member = self._manager.register( anne, pre_verified=True, pre_confirmed=True, pre_approved=True) self.assertIsNone(token) self.assertEqual(member.address.email, 'anne@example.org') # Now hold and handle an unsubscription request. token = hold_unsubscription(self._mlist, 'anne@example.org') handle_unsubscription(self._mlist, token, Action.defer) + + def test_bogus_token(self): + # Try to handle an unsubscription with a bogus token. + self.assertRaises(LookupError, self._manager.confirm, None) diff --git a/src/mailman/app/tests/test_subscriptions.py b/src/mailman/app/tests/test_subscriptions.py index 9f02593a9..19198307d 100644 --- a/src/mailman/app/tests/test_subscriptions.py +++ b/src/mailman/app/tests/test_subscriptions.py @@ -45,6 +45,13 @@ class TestSubscriptionWorkflow(unittest.TestCase): self._mlist.admin_immed_notify = False self._anne = 'anne@example.com' self._user_manager = getUtility(IUserManager) + self._expected_pendings_count = 0 + + def tearDown(self): + # There usually should be no pending after all is said and done, but + # some tests don't complete the workflow. + self.assertEqual(getUtility(IPendings).count, + self._expected_pendings_count) def test_start_state(self): # The workflow starts with no tokens or member. @@ -67,6 +74,8 @@ class TestSubscriptionWorkflow(unittest.TestCase): self.assertEqual(pendable['display_name'], '') self.assertEqual(pendable['when'], '2005-08-01T07:49:23') self.assertEqual(pendable['token_owner'], 'subscriber') + # The token is still in the database. + self._expected_pendings_count = 1 def test_user_or_address_required(self): # The `subscriber` attribute must be a user or address. @@ -359,8 +368,6 @@ class TestSubscriptionWorkflow(unittest.TestCase): pre_verified=True, pre_confirmed=True, pre_approved=True) - # Cache the token. - token = workflow.token # Consume the entire state machine. list(workflow) # Anne is now a member of the mailing list. @@ -370,13 +377,6 @@ class TestSubscriptionWorkflow(unittest.TestCase): # The workflow is done, so it has no token. self.assertIsNone(workflow.token) self.assertEqual(workflow.token_owner, TokenOwner.no_one) - # The pendable associated with the token has been evicted. - self.assertIsNone(getUtility(IPendings).confirm(token, expunge=False)) - # There is no saved workflow associated with the token. This shows up - # as an exception when we try to restore the workflow. - new_workflow = SubscriptionWorkflow(self._mlist) - new_workflow.token = token - self.assertRaises(LookupError, new_workflow.restore) def test_moderator_approves(self): # The workflow runs until moderator approval is required, at which @@ -426,6 +426,9 @@ class TestSubscriptionWorkflow(unittest.TestCase): 'test@example.com: held subscription request from anne@example.com', mark.readline() ) + # The state machine stopped at the moderator approval so there will be + # one token still in the database. + self._expected_pendings_count = 1 def test_get_moderator_approval_notifies_moderators(self): # When the subscription is held for moderator approval, and the list @@ -452,6 +455,9 @@ approval: For: anne@example.com List: test@example.com """) + # The state machine stopped at the moderator approval so there will be + # one token still in the database. + self._expected_pendings_count = 1 def test_get_moderator_approval_no_notifications(self): # When the subscription is held for moderator approval, and the list @@ -465,6 +471,9 @@ approval: # Consume the entire state machine. list(workflow) get_queue_messages('virgin', expected_count=0) + # The state machine stopped at the moderator approval so there will be + # one token still in the database. + self._expected_pendings_count = 1 def test_send_confirmation(self): # A confirmation message gets sent when the address is not verified. @@ -481,6 +490,9 @@ approval: message['From'], 'test-confirm+{}@example.com'.format(token)) # The confirmation message is not `Precedence: bulk`. self.assertIsNone(message['precedence']) + # The state machine stopped at the moderator approval so there will be + # one token still in the database. + self._expected_pendings_count = 1 def test_send_confirmation_pre_confirmed(self): # A confirmation message gets sent when the address is not verified @@ -497,6 +509,9 @@ approval: message['Subject'], 'confirm {}'.format(workflow.token)) self.assertEqual( message['From'], 'test-confirm+{}@example.com'.format(token)) + # The state machine stopped at the moderator approval so there will be + # one token still in the database. + self._expected_pendings_count = 1 def test_send_confirmation_pre_verified(self): # A confirmation message gets sent even when the address is verified @@ -514,6 +529,9 @@ approval: message['Subject'], 'confirm {}'.format(workflow.token)) self.assertEqual( message['From'], 'test-confirm+{}@example.com'.format(token)) + # The state machine stopped at the moderator approval so there will be + # one token still in the database. + self._expected_pendings_count = 1 def test_do_confirm_verify_address(self): # The address is not yet verified, nor are we pre-verifying. A diff --git a/src/mailman/app/tests/test_unsubscriptions.py b/src/mailman/app/tests/test_unsubscriptions.py new file mode 100644 index 000000000..1fb80e778 --- /dev/null +++ b/src/mailman/app/tests/test_unsubscriptions.py @@ -0,0 +1,522 @@ +# Copyright (C) 2016 by the Free Software Foundation, Inc. +# +# This file is part of GNU Mailman. +# +# GNU Mailman is free software: you can redistribute it and/or modify it under +# the terms of the GNU General Public License as published by the Free +# Software Foundation, either version 3 of the License, or (at your option) +# any later version. +# +# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along with +# GNU Mailman. If not, see <http://www.gnu.org/licenses/>. + +"""Test for unsubscription service.""" + +import unittest + +from contextlib import suppress +from mailman.app.lifecycle import create_list +from mailman.app.subscriptions import UnSubscriptionWorkflow +from mailman.interfaces.mailinglist import SubscriptionPolicy +from mailman.interfaces.pending import IPendings +from mailman.interfaces.subscriptions import TokenOwner +from mailman.interfaces.usermanager import IUserManager +from mailman.testing.helpers import LogFileMark, get_queue_messages +from mailman.testing.layers import ConfigLayer +from mailman.utilities.datetime import now +from unittest.mock import patch +from zope.component import getUtility + + +class TestUnSubscriptionWorkflow(unittest.TestCase): + layer = ConfigLayer + maxDiff = None + + def setUp(self): + self._mlist = create_list('test@example.com') + self._mlist.admin_immed_notify = False + self._mlist.unsubscription_policy = SubscriptionPolicy.open + self._mlist.send_welcome_message = False + self._anne = 'anne@example.com' + self._user_manager = getUtility(IUserManager) + self.anne = self._user_manager.create_user(self._anne) + self.anne.addresses[0].verified_on = now() + self.anne.preferred_address = self.anne.addresses[0] + self._mlist.subscribe(self.anne) + self._expected_pendings_count = 0 + + def tearDown(self): + # There usually should be no pending after all is said and done, but + # some tests don't complete the workflow. + self.assertEqual(getUtility(IPendings).count, + self._expected_pendings_count) + + def test_start_state(self): + # Test the workflow starts with no tokens or members. + workflow = UnSubscriptionWorkflow(self._mlist) + self.assertEqual(workflow.token_owner, TokenOwner.no_one) + self.assertIsNone(workflow.token) + self.assertIsNone(workflow.member) + + def test_pended_data(self): + # Test there is a Pendable object associated with a held + # unsubscription request and it has some valid data associated with + # it. + self._mlist.unsubscription_policy = SubscriptionPolicy.confirm + workflow = UnSubscriptionWorkflow(self._mlist, self.anne) + with suppress(StopIteration): + workflow.run_thru('send_confirmation') + self.assertIsNotNone(workflow.token) + pendable = getUtility(IPendings).confirm(workflow.token, expunge=False) + self.assertEqual(pendable['list_id'], 'test.example.com') + self.assertEqual(pendable['email'], 'anne@example.com') + self.assertEqual(pendable['display_name'], '') + self.assertEqual(pendable['when'], '2005-08-01T07:49:23') + self.assertEqual(pendable['token_owner'], 'subscriber') + # The token is still in the database. + self._expected_pendings_count = 1 + + def test_user_or_address_required(self): + # The `subscriber` attribute must be a user or address that is provided + # to the workflow. + workflow = UnSubscriptionWorkflow(self._mlist) + self.assertRaises(AssertionError, list, workflow) + + def test_user_is_subscribed_to_unsubscribe(self): + # A user must be subscribed to a list when trying to unsubscribe. + addr = self._user_manager.create_address('aperson@example.org') + addr.verfied_on = now() + workflow = UnSubscriptionWorkflow(self._mlist, addr) + self.assertRaises(AssertionError, + workflow.run_thru, 'subscription_checks') + + def test_confirmation_checks_open_list(self): + # An unsubscription from an open list does not need to be confirmed or + # moderated. + self._mlist.unsubscription_policy = SubscriptionPolicy.open + workflow = UnSubscriptionWorkflow(self._mlist, self.anne) + workflow.run_thru('confirmation_checks') + with patch.object(workflow, '_step_do_unsubscription') as step: + next(workflow) + step.assert_called_once_with() + + def test_confirmation_checks_no_user_confirmation_needed(self): + # An unsubscription from a list which does not need user confirmation + # skips to the moderation checks. + self._mlist.unsubscription_policy = SubscriptionPolicy.moderate + workflow = UnSubscriptionWorkflow(self._mlist, self.anne, + pre_confirmed=True) + workflow.run_thru('confirmation_checks') + with patch.object(workflow, '_step_moderation_checks') as step: + next(workflow) + step.assert_called_once_with() + + def test_confirmation_checks_confirm_pre_confirmed(self): + # The unsubscription policy requires user-confirmation, but their + # unsubscription is pre-confirmed. Since moderation is not reuqired, + # the user will be immediately unsubscribed. + self._mlist.unsubscription_policy = SubscriptionPolicy.confirm + workflow = UnSubscriptionWorkflow( + self._mlist, self.anne, pre_confirmed=True) + workflow.run_thru('confirmation_checks') + with patch.object(workflow, '_step_do_unsubscription') as step: + next(workflow) + step.assert_called_once_with() + + def test_confirmation_checks_confirm_then_moderate_pre_confirmed(self): + # The unsubscription policy requires user confirmation, but their + # unsubscription is pre-confirmed. Since moderation is required, that + # check will be performed. + self._mlist.unsubscription_policy = ( + SubscriptionPolicy.confirm_then_moderate) + workflow = UnSubscriptionWorkflow( + self._mlist, self.anne, pre_confirmed=True) + workflow.run_thru('confirmation_checks') + with patch.object(workflow, '_step_do_unsubscription') as step: + next(workflow) + step.assert_called_once_with() + + def test_send_confirmation_checks_confirm_list(self): + # The unsubscription policy requires user confirmation and the + # unsubscription is not pre-confirmed. + self._mlist.unsubscription_policy = SubscriptionPolicy.confirm + workflow = UnSubscriptionWorkflow(self._mlist, self.anne) + workflow.run_thru('confirmation_checks') + with patch.object(workflow, '_step_send_confirmation') as step: + next(workflow) + step.assert_called_once_with() + + def test_moderation_checks_moderated_list(self): + # The unsubscription policy requires moderation. + self._mlist.unsubscription_policy = SubscriptionPolicy.moderate + workflow = UnSubscriptionWorkflow(self._mlist, self.anne) + workflow.run_thru('confirmation_checks') + with patch.object(workflow, '_step_moderation_checks') as step: + next(workflow) + step.assert_called_once_with() + + def test_moderation_checks_approval_required(self): + # The moderator must approve the subscription request. + self._mlist.unsubscription_policy = SubscriptionPolicy.moderate + workflow = UnSubscriptionWorkflow(self._mlist, self.anne) + workflow.run_thru('moderation_checks') + with patch.object(workflow, '_step_get_moderator_approval') as step: + next(workflow) + step.assert_called_once_with() + + def test_do_unsusbcription(self): + # An open unsubscription policy means the user gets unsubscribed to + # the mailing list without any further confirmations or approvals. + self._mlist.unsubscription_policy = SubscriptionPolicy.open + workflow = UnSubscriptionWorkflow(self._mlist, self.anne) + list(workflow) + member = self._mlist.regular_members.get_member(self._anne) + self.assertIsNone(member) + + def test_do_unsubscription_pre_approved(self): + # A moderation-requiring subscription policy plus a pre-approved + # address means the user gets unsubscribed from the mailing list + # without any further confirmation or approvals. + self._mlist.unsubscription_policy = SubscriptionPolicy.moderate + workflow = UnSubscriptionWorkflow(self._mlist, self.anne, + pre_approved=True) + list(workflow) + # Anne is now unsubscribed form the mailing list. + member = self._mlist.regular_members.get_member(self._anne) + self.assertIsNone(member) + # No further token is needed. + self.assertIsNone(workflow.token) + self.assertEqual(workflow.token_owner, TokenOwner.no_one) + + def test_do_unsubscription_pre_approved_pre_confirmed(self): + # A moderation-requiring unsubscription policy plus a pre-appvoed + # address means the user gets unsubscribed to the mailing list without + # any further confirmations or approvals. + self._mlist.unsubscription_policy = ( + SubscriptionPolicy.confirm_then_moderate) + workflow = UnSubscriptionWorkflow(self._mlist, self.anne, + pre_approved=True, + pre_confirmed=True) + list(workflow) + member = self._mlist.regular_members.get_member(self._anne) + self.assertIsNone(member) + # No further token is needed. + self.assertIsNone(workflow.token) + self.assertEqual(workflow.token_owner, TokenOwner.no_one) + + def test_do_unsubscription_cleanups(self): + # Once the user is unsubscribed, the token and its associated pending + # database record will be removed from the database. + self._mlist.unsubscription_policy = SubscriptionPolicy.open + workflow = UnSubscriptionWorkflow(self._mlist, self.anne, + pre_approved=True, + pre_confirmed=True) + # Run the workflow. + list(workflow) + # Anne is now unsubscribed from the list. + member = self._mlist.regular_members.get_member(self._anne) + self.assertIsNone(member) + # Workflow is done, so it has no token. + self.assertIsNone(workflow.token) + self.assertEqual(workflow.token_owner, TokenOwner.no_one) + + def test_moderator_approves(self): + # The workflow runs until moderator approval is required, at which + # point the workflow is saved. Once the moderator approves, the + # workflow resumes and the user is unsubscribed. + self._mlist.unsubscription_policy = SubscriptionPolicy.moderate + workflow = UnSubscriptionWorkflow( + self._mlist, self.anne, pre_confirmed=True) + # Run the entire workflow. + list(workflow) + # The user is currently subscribed to the mailing list. + member = self._mlist.regular_members.get_member(self._anne) + self.assertIsNotNone(member) + self.assertIsNotNone(workflow.member) + # The token is owned by the moderator. + self.assertIsNotNone(workflow.token) + self.assertEqual(workflow.token_owner, TokenOwner.moderator) + # Create a new workflow with the previous workflow's save token, and + # restore its state. This models an approved un-sunscription request + # and should result in the user getting subscribed. + approved_workflow = UnSubscriptionWorkflow(self._mlist) + approved_workflow.token = workflow.token + approved_workflow.restore() + list(approved_workflow) + # Now the user is unsubscribed from the mailing list. + member = self._mlist.regular_members.get_member(self._anne) + self.assertIsNone(member) + self.assertEqual(approved_workflow.member, member) + # No further token is needed. + self.assertIsNone(approved_workflow.token) + self.assertEqual(approved_workflow.token_owner, TokenOwner.no_one) + + def test_get_moderator_approval_log_on_hold(self): + # When the unsubscription is held for moderator approval, a message is + # logged. + mark = LogFileMark('mailman.subscribe') + self._mlist.unsubscription_policy = SubscriptionPolicy.moderate + workflow = UnSubscriptionWorkflow( + self._mlist, self.anne, pre_confirmed=True) + # Run the entire workflow. + list(workflow) + self.assertIn( + 'test@example.com: held unsubscription request from anne@example.com', + mark.readline() + ) + # The state machine stopped at the moderator approval step so there + # will be one token still in the database. + self._expected_pendings_count = 1 + + def test_get_moderator_approval_notifies_moderators(self): + # When the unsubscription is held for moderator approval, and the list + # is so configured, a notification is sent to the list moderators. + self._mlist.admin_immed_notify = True + self._mlist.unsubscription_policy = SubscriptionPolicy.moderate + workflow = UnSubscriptionWorkflow( + self._mlist, self.anne, pre_confirmed=True) + # Consume the entire state machine. + list(workflow) + items = get_queue_messages('virgin', expected_count=1) + message = items[0].msg + self.assertEqual(message['From'], 'test-owner@example.com') + self.assertEqual(message['To'], 'test-owner@example.com') + self.assertEqual( + message['Subject'], + 'New unsubscription request to Test from anne@example.com') + self.assertEqual(message.get_payload(), """\ +Your authorization is required for a mailing list unsubscription +request approval: + + For: anne@example.com + List: test@example.com +""") + # The state machine stopped at the moderator approval so there will be + # one token still in the database. + self._expected_pendings_count = 1 + + def test_get_moderator_approval_no_notifications(self): + # When the unsubscription request is held for moderator approval, and + # the list is so configured, a notification is sent to the list + # moderators. + self._mlist.admin_immed_notify = False + self._mlist.unsubscription_policy = SubscriptionPolicy.moderate + workflow = UnSubscriptionWorkflow( + self._mlist, self.anne, pre_confirmed=True) + # Consume the entire state machine. + list(workflow) + get_queue_messages('virgin', expected_count=0) + # The state machine stopped at the moderator approval so there will be + # one token still in the database. + self._expected_pendings_count = 1 + + def test_send_confirmation(self): + # A confirmation message gets sent when the unsubscription must be + # confirmed. + self._mlist.unsubscription_policy = SubscriptionPolicy.confirm + # Run the workflow to model the confirmation step. + workflow = UnSubscriptionWorkflow(self._mlist, self.anne) + list(workflow) + items = get_queue_messages('virgin', expected_count=1) + message = items[0].msg + token = workflow.token + self.assertEqual( + message['Subject'], 'confirm {}'.format(workflow.token)) + self.assertEqual( + message['From'], 'test-confirm+{}@example.com'.format(token)) + # The state machine stopped at the member confirmation step so there + # will be one token still in the database. + self._expected_pendings_count = 1 + + def test_do_confirmation_unsubscribes_user(self): + # Unsubscriptions to the mailing list must be confirmed. Once that's + # done, the user's address is unsubscribed. + self._mlist.unsubscription_policy = SubscriptionPolicy.confirm + workflow = UnSubscriptionWorkflow(self._mlist, self.anne) + list(workflow) + # Anne is a member. + member = self._mlist.regular_members.get_member(self._anne) + self.assertIsNotNone(member) + self.assertEqual(member, workflow.member) + # The token is owned by the subscriber. + self.assertIsNotNone(workflow.token) + self.assertEqual(workflow.token_owner, TokenOwner.subscriber) + # Confirm. + confirm_workflow = UnSubscriptionWorkflow(self._mlist) + confirm_workflow.token = workflow.token + confirm_workflow.restore() + list(confirm_workflow) + # Anne is now unsubscribed. + member = self._mlist.regular_members.get_member(self._anne) + self.assertIsNone(member) + # No further token is needed. + self.assertIsNone(confirm_workflow.token) + self.assertEqual(confirm_workflow.token_owner, TokenOwner.no_one) + + def test_do_confirmation_unsubscribes_address(self): + # Unsubscriptions to the mailing list must be confirmed. Once that's + # done, the address is unsubscribed. + address = self.anne.register('anne.person@example.com') + self._mlist.subscribe(address) + self._mlist.unsubscription_policy = SubscriptionPolicy.confirm + workflow = UnSubscriptionWorkflow(self._mlist, address) + list(workflow) + # Bart is a member. + member = self._mlist.regular_members.get_member( + 'anne.person@example.com') + self.assertIsNotNone(member) + self.assertEqual(member, workflow.member) + # The token is owned by the subscriber. + self.assertIsNotNone(workflow.token) + self.assertEqual(workflow.token_owner, TokenOwner.subscriber) + # Confirm. + confirm_workflow = UnSubscriptionWorkflow(self._mlist) + confirm_workflow.token = workflow.token + confirm_workflow.restore() + list(confirm_workflow) + # Bart is now unsubscribed. + member = self._mlist.regular_members.get_member( + 'anne.person@example.com') + self.assertIsNone(member) + # No further token is needed. + self.assertIsNone(confirm_workflow.token) + self.assertEqual(confirm_workflow.token_owner, TokenOwner.no_one) + + def test_do_confirmation_nonmember(self): + # Attempt to confirm the unsubscription of a member who has already + # been unsubscribed. + self._mlist.unsubscription_policy = SubscriptionPolicy.confirm + workflow = UnSubscriptionWorkflow(self._mlist, self.anne) + list(workflow) + # Anne is a member. + member = self._mlist.regular_members.get_member(self._anne) + self.assertIsNotNone(member) + self.assertEqual(member, workflow.member) + # The token is owned by the subscriber. + self.assertIsNotNone(workflow.token) + self.assertEqual(workflow.token_owner, TokenOwner.subscriber) + # Unsubscribe Anne out of band. + member.unsubscribe() + # Confirm. + confirm_workflow = UnSubscriptionWorkflow(self._mlist) + confirm_workflow.token = workflow.token + confirm_workflow.restore() + list(confirm_workflow) + # No further token is needed. + self.assertIsNone(confirm_workflow.token) + self.assertEqual(confirm_workflow.token_owner, TokenOwner.no_one) + + def test_do_confirmation_nonmember_final_step(self): + # Attempt to confirm the unsubscription of a member who has already + # been unsubscribed. + self._mlist.unsubscription_policy = SubscriptionPolicy.confirm + workflow = UnSubscriptionWorkflow(self._mlist, self.anne) + list(workflow) + # Anne is a member. + member = self._mlist.regular_members.get_member(self._anne) + self.assertIsNotNone(member) + self.assertEqual(member, workflow.member) + # The token is owned by the subscriber. + self.assertIsNotNone(workflow.token) + self.assertEqual(workflow.token_owner, TokenOwner.subscriber) + # Confirm. + confirm_workflow = UnSubscriptionWorkflow(self._mlist) + confirm_workflow.token = workflow.token + confirm_workflow.restore() + confirm_workflow.run_until('do_unsubscription') + self.assertEqual(member, confirm_workflow.member) + # Unsubscribe Anne out of band. + member.unsubscribe() + list(confirm_workflow) + self.assertIsNone(confirm_workflow.member) + # No further token is needed. + self.assertIsNone(confirm_workflow.token) + self.assertEqual(confirm_workflow.token_owner, TokenOwner.no_one) + + def test_prevent_confirmation_replay_attacks(self): + # Ensure that if the workflow requires two confirmations, e.g. first + # the user confirming their subscription, and then the moderator + # approving it, that different tokens are used in these two cases. + self._mlist.unsubscription_policy = ( + SubscriptionPolicy.confirm_then_moderate) + workflow = UnSubscriptionWorkflow(self._mlist, self.anne) + # Run the state machine up to the first confirmation, and cache the + # confirmation token. + list(workflow) + token = workflow.token + # Anne is still a member of the mailing list. + member = self._mlist.regular_members.get_member(self._anne) + self.assertIsNotNone(member) + self.assertIsNotNone(workflow.member) + # The token is owned by the subscriber. + self.assertIsNotNone(workflow.token) + self.assertEqual(workflow.token_owner, TokenOwner.subscriber) + # The old token will not work for moderator approval. + moderator_workflow = UnSubscriptionWorkflow(self._mlist) + moderator_workflow.token = token + moderator_workflow.restore() + list(moderator_workflow) + # The token is owned by the moderator. + self.assertIsNotNone(moderator_workflow.token) + self.assertEqual(moderator_workflow.token_owner, TokenOwner.moderator) + # While we wait for the moderator to approve the subscription, note + # that there's a new token for the next steps. + self.assertNotEqual(token, moderator_workflow.token) + # The old token won't work. + final_workflow = UnSubscriptionWorkflow(self._mlist) + final_workflow.token = token + self.assertRaises(LookupError, final_workflow.restore) + # Running this workflow will fail. + self.assertRaises(AssertionError, list, final_workflow) + # Anne is still not unsubscribed. + member = self._mlist.regular_members.get_member(self._anne) + self.assertIsNotNone(member) + self.assertIsNone(final_workflow.member) + # However, if we use the new token, her unsubscription request will be + # approved by the moderator. + final_workflow.token = moderator_workflow.token + final_workflow.restore() + list(final_workflow) + # And now Anne is unsubscribed. + member = self._mlist.regular_members.get_member(self._anne) + self.assertIsNone(member) + # No further token is needed. + self.assertIsNone(final_workflow.token) + self.assertEqual(final_workflow.token_owner, TokenOwner.no_one) + + def test_confirmation_needed_and_pre_confirmed(self): + # The subscription policy is 'confirm' but the subscription is + # pre-confirmed so the moderation checks can be skipped. + self._mlist.unsubscription_policy = SubscriptionPolicy.confirm + workflow = UnSubscriptionWorkflow( + self._mlist, self.anne, pre_confirmed=True, pre_approved=True) + list(workflow) + # Anne was unsubscribed. + self.assertIsNone(workflow.token) + self.assertEqual(workflow.token_owner, TokenOwner.no_one) + self.assertIsNone(workflow.member) + + def test_confirmation_needed_moderator_address(self): + address = self.anne.register('anne.person@example.com') + self._mlist.subscribe(address) + self._mlist.unsubscription_policy = SubscriptionPolicy.moderate + workflow = UnSubscriptionWorkflow(self._mlist, address) + # Get moderator approval. + list(workflow) + approved_workflow = UnSubscriptionWorkflow(self._mlist) + approved_workflow.token = workflow.token + approved_workflow.restore() + list(approved_workflow) + self.assertEqual(approved_workflow.subscriber, address) + # Anne was unsubscribed. + self.assertIsNone(approved_workflow.token) + self.assertEqual(approved_workflow.token_owner, TokenOwner.no_one) + self.assertIsNone(approved_workflow.member) + member = self._mlist.regular_members.get_member( + 'anne.person@example.com') + self.assertIsNone(member) diff --git a/src/mailman/app/tests/test_registrar.py b/src/mailman/app/tests/test_workflowmanager.py index 2107f9648..4193beefe 100644 --- a/src/mailman/app/tests/test_registrar.py +++ b/src/mailman/app/tests/test_workflowmanager.py @@ -23,8 +23,7 @@ from mailman.app.lifecycle import create_list from mailman.interfaces.mailinglist import SubscriptionPolicy from mailman.interfaces.member import MemberRole from mailman.interfaces.pending import IPendings -from mailman.interfaces.registrar import IRegistrar -from mailman.interfaces.subscriptions import TokenOwner +from mailman.interfaces.subscriptions import ISubscriptionManager, TokenOwner from mailman.interfaces.usermanager import IUserManager from mailman.testing.helpers import get_queue_messages from mailman.testing.layers import ConfigLayer @@ -39,7 +38,7 @@ class TestRegistrar(unittest.TestCase): def setUp(self): self._mlist = create_list('ant@example.com') - self._registrar = IRegistrar(self._mlist) + self._registrar = ISubscriptionManager(self._mlist) self._pendings = getUtility(IPendings) self._anne = getUtility(IUserManager).create_address( 'anne@example.com') diff --git a/src/mailman/app/workflow.py b/src/mailman/app/workflow.py index eec8a14a8..36fd7d611 100644 --- a/src/mailman/app/workflow.py +++ b/src/mailman/app/workflow.py @@ -110,7 +110,7 @@ class Workflow: # Stop executing, but not before we push the last state back # onto the deque. Otherwise, resuming the state machine would # skip this step. - self._next.appendleft(step) + self._next.appendleft(name) break results.append(step()) return results @@ -132,15 +132,11 @@ class Workflow: raise AssertionError( "Can't save a workflow state with more than one step " "in the queue") - state_manager.save( - self.__class__.__name__, - self.token, - step, - json.dumps(data)) + state_manager.save(self.token, step, json.dumps(data)) def restore(self): state_manager = getUtility(IWorkflowStateManager) - state = state_manager.restore(self.__class__.__name__, self.token) + state = state_manager.restore(self.token) if state is None: # The token doesn't exist in the database. raise LookupError(self.token) diff --git a/src/mailman/commands/docs/membership.rst b/src/mailman/commands/docs/membership.rst index 49e80511d..5799e1e17 100644 --- a/src/mailman/commands/docs/membership.rst +++ b/src/mailman/commands/docs/membership.rst @@ -113,11 +113,11 @@ Mailman has sent her the confirmation message. <BLANKLINE> anne@example.com <BLANKLINE> - Before you can start using GNU Mailman at this site, you must first - confirm that this is your email address. You can do this by replying to - this message, keeping the Subject header intact. + Before you can start using GNU Mailman at this site, you must first confirm + that this is your email address. You can do this by replying to this me... + keeping the Subject header intact. <BLANKLINE> - If you do not wish to register this email address simply disregard this + If you do not wish to register this email address, simply disregard this message. If you think you are being maliciously subscribed to the list, or have any other questions, you may contact <BLANKLINE> @@ -217,9 +217,12 @@ list. ``unsubscribe`` is an alias for ``leave``. You may be asked to confirm your request. Anne is a member of the ``baker@example.com`` mailing list, when she decides -to leave it. She sends a message to the ``-leave`` address for the list and -is sent a confirmation message for her request. +to leave it. Because the mailing list allows for *open* unsubscriptions +(i.e. no confirmation is needed), when she sends a message to the ``-leave`` +address for the list, she is immediately removed. + >>> from mailman.interfaces.mailinglist import SubscriptionPolicy + >>> mlist_2.unsubscription_policy = SubscriptionPolicy.open >>> results = Results() >>> print(leave.process(mlist_2, msg, {}, (), results)) ContinueProcessing.yes diff --git a/src/mailman/commands/eml_confirm.py b/src/mailman/commands/eml_confirm.py index 6a3e389bd..6787b0987 100644 --- a/src/mailman/commands/eml_confirm.py +++ b/src/mailman/commands/eml_confirm.py @@ -20,8 +20,7 @@ from mailman import public from mailman.core.i18n import _ from mailman.interfaces.command import ContinueProcessing, IEmailCommand -from mailman.interfaces.registrar import IRegistrar -from mailman.interfaces.subscriptions import TokenOwner +from mailman.interfaces.subscriptions import ISubscriptionManager, TokenOwner from zope.interface import implementer @@ -50,7 +49,8 @@ class Confirm: tokens.add(token) results.confirms = tokens try: - new_token, token_owner, member = IRegistrar(mlist).confirm(token) + new_token, token_owner, member = ISubscriptionManager( + mlist).confirm(token) if new_token is None: assert token_owner is TokenOwner.no_one, token_owner assert member is not None, member diff --git a/src/mailman/commands/eml_membership.py b/src/mailman/commands/eml_membership.py index 7d3704e14..e658e7b58 100644 --- a/src/mailman/commands/eml_membership.py +++ b/src/mailman/commands/eml_membership.py @@ -22,8 +22,8 @@ from mailman import public from mailman.core.i18n import _ from mailman.interfaces.command import ContinueProcessing, IEmailCommand from mailman.interfaces.member import DeliveryMode, MemberRole -from mailman.interfaces.registrar import IRegistrar -from mailman.interfaces.subscriptions import ISubscriptionService +from mailman.interfaces.subscriptions import ( + ISubscriptionManager, ISubscriptionService) from mailman.interfaces.usermanager import IUserManager from zope.component import getUtility from zope.interface import implementer @@ -31,7 +31,7 @@ from zope.interface import implementer def match_subscriber(email, display_name): # Return something matching the email which should be used as the - # subscriber by the IRegistrar interface. + # subscriber by the ISubscriptionManager interface. manager = getUtility(IUserManager) # Is there a user with a preferred address matching the email? user = manager.get_user(email) @@ -101,7 +101,7 @@ used. print(_('$person is already a member'), file=results) return ContinueProcessing.yes subscriber = match_subscriber(email, display_name) - IRegistrar(mlist).register(subscriber) + ISubscriptionManager(mlist).register(subscriber) print(_('Confirmation email sent to $person'), file=results) return ContinueProcessing.yes @@ -173,6 +173,7 @@ You may be asked to confirm your request.""") print(_('Invalid or unverified email address: $email'), file=results) return ContinueProcessing.no + already_left = msgdata.setdefault('leaves', set()) for user_address in user.addresses: # Only recognize verified addresses. if user_address.verified_on is None: @@ -181,14 +182,28 @@ You may be asked to confirm your request.""") if member is not None: break else: - # None of the user's addresses are subscribed to this mailing list. - print(_( - '$self.name: $email is not a member of $mlist.fqdn_listname'), - file=results) - return ContinueProcessing.no - member.unsubscribe() - person = formataddr((user.display_name, email)) # noqa: F841 - print(_('$person left $mlist.fqdn_listname'), file=results) + # There are two possible situations. Either none of the user's + # addresses are subscribed to this mailing list, or this command + # email *already* unsubscribed the user from the mailing list. + # E.g. if a message was sent to the -leave address and it + # contained the 'leave' command. Don't send a bogus response in + # this case, just ignore subsequent leaves of the same address. + if email not in already_left: + print(_('$self.name: $email is not a member of ' + '$mlist.fqdn_listname'), file=results) + return ContinueProcessing.no + if email in already_left: + return ContinueProcessing.yes + # Ignore any subsequent 'leave' commands. + already_left.add(email) + manager = ISubscriptionManager(mlist) + token, token_owner, member = manager.unregister(user_address) + person = formataddr((user.display_name, email)) # noqa + if member is None: + print(_('$person left $mlist.fqdn_listname'), file=results) + else: + print(_('Confirmation email sent to $person to leave' + ' $mlist.fqdn_listname'), file=results) return ContinueProcessing.yes diff --git a/src/mailman/commands/tests/test_confirm.py b/src/mailman/commands/tests/test_confirm.py index 7cce4c3c7..e0f816f24 100644 --- a/src/mailman/commands/tests/test_confirm.py +++ b/src/mailman/commands/tests/test_confirm.py @@ -25,7 +25,7 @@ from mailman.config import config from mailman.email.message import Message from mailman.interfaces.command import ContinueProcessing from mailman.interfaces.mailinglist import SubscriptionPolicy -from mailman.interfaces.registrar import IRegistrar +from mailman.interfaces.subscriptions import ISubscriptionManager from mailman.interfaces.usermanager import IUserManager from mailman.runners.command import CommandRunner, Results from mailman.testing.helpers import get_queue_messages, make_testable_runner @@ -42,8 +42,8 @@ class TestConfirm(unittest.TestCase): self._mlist = create_list('test@example.com') anne = getUtility(IUserManager).create_address( 'anne@example.com', 'Anne Person') - self._token, token_owner, member = IRegistrar(self._mlist).register( - anne) + self._token, token_owner, member = ISubscriptionManager( + self._mlist).register(anne) self._command = Confirm() # Clear the virgin queue. get_queue_messages('virgin') @@ -88,8 +88,8 @@ class TestEmailResponses(unittest.TestCase): 'bart@example.com', 'Bart Person') # Clear any previously queued confirmation messages. get_queue_messages('virgin') - self._token, token_owner, member = IRegistrar(self._mlist).register( - bart) + self._token, token_owner, member = ISubscriptionManager( + self._mlist).register(bart) # There should now be one email message in the virgin queue, i.e. the # confirmation message sent to Bart. items = get_queue_messages('virgin', expected_count=1) diff --git a/src/mailman/commands/tests/test_membership.py b/src/mailman/commands/tests/test_membership.py new file mode 100644 index 000000000..14074b3a9 --- /dev/null +++ b/src/mailman/commands/tests/test_membership.py @@ -0,0 +1,54 @@ +# Copyright (C) 2016 by the Free Software Foundation, Inc. +# +# This file is part of GNU Mailman. +# +# GNU Mailman is free software: you can redistribute it and/or modify it under +# the terms of the GNU General Public License as published by the Free +# Software Foundation, either version 3 of the License, or (at your option) +# any later version. +# +# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along with +# GNU Mailman. If not, see <http://www.gnu.org/licenses/>. + +"""Test the Leave command.""" + +import unittest + +from mailman.app.lifecycle import create_list +from mailman.commands.eml_membership import Leave +from mailman.email.message import Message +from mailman.interfaces.mailinglist import SubscriptionPolicy +from mailman.interfaces.usermanager import IUserManager +from mailman.runners.command import Results +from mailman.testing.helpers import set_preferred +from mailman.testing.layers import ConfigLayer +from zope.component import getUtility + + +class TestLeave(unittest.TestCase): + layer = ConfigLayer + + def setUp(self): + self._mlist = create_list('ant@example.com') + self._command = Leave() + + def test_confirm_leave_not_a_member(self): + self._mlist.unsubscription_policy = SubscriptionPolicy.confirm + # Try to unsubscribe someone who is not a member. Anne is a real + # user, with a validated address, but she is not a member of the + # mailing list. + anne = getUtility(IUserManager).create_user('anne@example.com') + set_preferred(anne) + # Initiate an unsubscription. + msg = Message() + msg['From'] = 'anne@example.com' + results = Results() + self._command.process(self._mlist, msg, {}, (), results) + self.assertEqual( + str(results).splitlines()[-1], + 'leave: anne@example.com is not a member of ant@example.com') diff --git a/src/mailman/config/configure.zcml b/src/mailman/config/configure.zcml index f31e79525..2fd0c8788 100644 --- a/src/mailman/config/configure.zcml +++ b/src/mailman/config/configure.zcml @@ -48,8 +48,8 @@ <adapter for="mailman.interfaces.mailinglist.IMailingList" - provides="mailman.interfaces.registrar.IRegistrar" - factory="mailman.app.registrar.Registrar" + provides="mailman.interfaces.subscriptions.ISubscriptionManager" + factory="mailman.app.subscriptions.SubscriptionManager" /> <utility diff --git a/src/mailman/database/alembic/versions/16c2b25c7b_list_subscription_policy.py b/src/mailman/database/alembic/versions/16c2b25c7b_list_subscription_policy.py index fef188bd0..591033ce1 100644 --- a/src/mailman/database/alembic/versions/16c2b25c7b_list_subscription_policy.py +++ b/src/mailman/database/alembic/versions/16c2b25c7b_list_subscription_policy.py @@ -25,7 +25,6 @@ def upgrade(): # SQLite may not have removed it when downgrading. op.add_column('mailinglist', sa.Column( 'subscription_policy', Enum(SubscriptionPolicy), nullable=True)) - # Now migrate the data. Don't import the table definition from the # models, it may break this migration when the model is updated in the # future (see the Alembic doc). diff --git a/src/mailman/database/alembic/versions/448a93984c35_unsubscription_workflow.py b/src/mailman/database/alembic/versions/448a93984c35_unsubscription_workflow.py new file mode 100644 index 000000000..dc57c8aee --- /dev/null +++ b/src/mailman/database/alembic/versions/448a93984c35_unsubscription_workflow.py @@ -0,0 +1,45 @@ +"""unsubscription_workflow + +Revision ID: 448a93984c35 +Revises: fa0d96e28631 +Create Date: 2016-06-02 14:34:24.154723 +""" + +import sqlalchemy as sa + +from alembic import op +from mailman.database.helpers import exists_in_db +from mailman.database.types import Enum, SAUnicode +from mailman.interfaces.mailinglist import SubscriptionPolicy + + +# revision identifiers, used by Alembic. +revision = '448a93984c35' +down_revision = 'fa0d96e28631' + + +def upgrade(): + if not exists_in_db(op.get_bind(), 'mailinglist', 'unsubscription_policy'): + # SQLite may not have removed it when downgrading. + op.add_column('mailinglist', sa.Column( + 'unsubscription_policy', Enum(SubscriptionPolicy), nullable=True)) + # Now migrate the data. Don't import the table definition from the + # models, it may break this migration when the model is updated in the + # future (see the Alembic doc). + mlist = sa.sql.table( + 'mailinglist', + sa.sql.column('unsubscription_policy', Enum(SubscriptionPolicy)) + ) + # There were no enforced subscription policy before, so all lists are + # considered open. + op.execute(mlist.update().values( + {'unsubscription_policy': + op.inline_literal(SubscriptionPolicy.open)})) + with op.batch_alter_table('workflowstate') as batch_op: + batch_op.drop_column('name') + + +def downgrade(): + with op.batch_alter_table('mailinglist') as batch_op: + batch_op.drop_column('unsubscription_policy') + op.add_column('workflowstate', sa.Column('name', SAUnicode)) diff --git a/src/mailman/database/alembic/versions/fa0d96e28631_template_manager.py b/src/mailman/database/alembic/versions/fa0d96e28631_template_manager.py index 58d688453..8fb8c1056 100644 --- a/src/mailman/database/alembic/versions/fa0d96e28631_template_manager.py +++ b/src/mailman/database/alembic/versions/fa0d96e28631_template_manager.py @@ -1,7 +1,7 @@ """File cache and template manager. Revision ID: fa0d96e28631 -Revises: bfda02ab3a9b +Revises: 7b254d88f122 Create Date: 2016-02-21 16:21:48.277654 """ diff --git a/src/mailman/database/tests/test_migrations.py b/src/mailman/database/tests/test_migrations.py index 2ad41ae53..874f3a9ce 100644 --- a/src/mailman/database/tests/test_migrations.py +++ b/src/mailman/database/tests/test_migrations.py @@ -208,8 +208,8 @@ class TestMigrations(unittest.TestCase): def test_70af5a4e5790_data_paths(self): # Create a couple of mailing lists through the standard API. with transaction(): - ant = create_list('ant@example.com') - bee = create_list('bee@example.com') + create_list('ant@example.com') + create_list('bee@example.com') # Downgrade and verify that the old data paths exist. alembic.command.downgrade(alembic_cfg, '47294d3a604') self.assertTrue(os.path.exists( @@ -222,11 +222,18 @@ class TestMigrations(unittest.TestCase): self.assertFalse(os.path.exists( os.path.join(config.LIST_DATA_DIR, 'ant@example.com'))) self.assertFalse(os.path.exists( - os.path.join(config.LIST_DATA_DIR, 'ant@example.com'))) - self.assertTrue(os.path.exists(ant.data_path)) - self.assertTrue(os.path.exists(bee.data_path)) + os.path.join(config.LIST_DATA_DIR, 'bee@example.com'))) + self.assertTrue(os.path.exists( + os.path.join(config.LIST_DATA_DIR, 'ant.example.com'))) + self.assertTrue(os.path.exists( + os.path.join(config.LIST_DATA_DIR, 'bee.example.com'))) def test_7b254d88f122_moderation_action(self): + # Create a mailing list through the standard API. + with transaction(): + ant = create_list('ant@example.com') + default_member_action = ant.default_member_action + default_nonmember_action = ant.default_nonmember_action sa.sql.table( 'mailinglist', sa.sql.column('id', sa.Integer), @@ -246,8 +253,6 @@ class TestMigrations(unittest.TestCase): with transaction(): # Start at the previous revision. alembic.command.downgrade(alembic_cfg, 'd4fbb4fd34ca') - # Create a mailing list through the standard API. - ant = create_list('ant@example.com') # Create some members. anne = user_manager.create_address('anne@example.com') bart = user_manager.create_address('bart@example.com') @@ -258,22 +263,40 @@ class TestMigrations(unittest.TestCase): # Assign some moderation actions to the members created above. config.db.store.execute(member_table.insert().values([ {'address_id': anne.id, 'role': MemberRole.owner, - 'list_id': ant.list_id, 'moderation_action': Action.accept}, + 'list_id': 'ant.example.com', + 'moderation_action': Action.accept}, {'address_id': bart.id, 'role': MemberRole.moderator, - 'list_id': ant.list_id, 'moderation_action': Action.accept}, + 'list_id': 'ant.example.com', + 'moderation_action': Action.accept}, {'address_id': cris.id, 'role': MemberRole.member, - 'list_id': ant.list_id, 'moderation_action': Action.defer}, + 'list_id': 'ant.example.com', + 'moderation_action': Action.defer}, {'address_id': dana.id, 'role': MemberRole.nonmember, - 'list_id': ant.list_id, 'moderation_action': Action.hold}, + 'list_id': 'ant.example.com', + 'moderation_action': Action.hold}, ])) # Cris and Dana have actions which match the list default action for # members and nonmembers respectively. - self.assertEqual( - ant.members.get_member('cris@example.com').moderation_action, - ant.default_member_action) - self.assertEqual( - ant.nonmembers.get_member('dana@example.com').moderation_action, - ant.default_nonmember_action) + query = sa.sql.select([member_table.c.moderation_action]).where( + sa.and_( + member_table.c.role == MemberRole.member, + member_table.c.list_id == 'ant.example.com', + # cris@example.com + member_table.c.address_id == 3, + ) + ) + action = config.db.store.execute(query).fetchone()[0] + self.assertEqual(action, default_member_action) + query = sa.sql.select([member_table.c.moderation_action]).where( + sa.and_( + member_table.c.role == MemberRole.nonmember, + member_table.c.list_id == 'ant.example.com', + # dana@example.com + member_table.c.address_id == 4, + ) + ) + action = config.db.store.execute(query).fetchone()[0] + self.assertEqual(action, default_nonmember_action) # Upgrade and check the moderation_actions. Cris's and Dana's # actions have been set to None to fall back to the list defaults. alembic.command.upgrade(alembic_cfg, '7b254d88f122') @@ -300,11 +323,6 @@ class TestMigrations(unittest.TestCase): ]) def test_fa0d96e28631_upgrade_uris(self): - with transaction(): - # Start at the previous revision. - alembic.command.downgrade(alembic_cfg, '7b254d88f122') - # Create a mailing list through the standard API. - create_list('ant@example.com') mlist_table = sa.sql.table( 'mailinglist', sa.sql.column('id', sa.Integer), @@ -317,15 +335,18 @@ class TestMigrations(unittest.TestCase): sa.sql.column('welcome_message_uri', sa.Unicode), ) with transaction(): - config.db.store.execute(mlist_table.update().where( - mlist_table.c.list_id == 'ant.example.com').values( - digest_footer_uri='mailman:///digest_footer.txt', - digest_header_uri='mailman:///digest_header.txt', - footer_uri='mailman:///footer.txt', - header_uri='mailman:///header.txt', - goodbye_message_uri='mailman:///goodbye.txt', - welcome_message_uri='mailman:///welcome.txt', - )) + # Start at the previous revision. + alembic.command.downgrade(alembic_cfg, '7b254d88f122') + # Create a mailing list with some URIs. + config.db.store.execute(mlist_table.insert().values( + list_id='ant.example.com', + digest_footer_uri='mailman:///digest_footer.txt', + digest_header_uri='mailman:///digest_header.txt', + footer_uri='mailman:///footer.txt', + header_uri='mailman:///header.txt', + goodbye_message_uri='mailman:///goodbye.txt', + welcome_message_uri='mailman:///welcome.txt', + )) # Now upgrade and check to see if the values got into the template # table correctly. alembic.command.upgrade(alembic_cfg, 'fa0d96e28631') @@ -366,14 +387,25 @@ class TestMigrations(unittest.TestCase): ]) def test_fa0d96e28631_upgrade_no_uris(self): + mlist_table = sa.sql.table( + 'mailinglist', + sa.sql.column('id', sa.Integer), + sa.sql.column('list_id', sa.Unicode), + sa.sql.column('digest_footer_uri', sa.Unicode), + sa.sql.column('digest_header_uri', sa.Unicode), + sa.sql.column('footer_uri', sa.Unicode), + sa.sql.column('header_uri', sa.Unicode), + sa.sql.column('goodbye_message_uri', sa.Unicode), + sa.sql.column('welcome_message_uri', sa.Unicode), + ) # None of the URL parameters are defined. with transaction(): # Start at the previous revision. alembic.command.downgrade(alembic_cfg, '7b254d88f122') - # Create a mailing list through the standard API. - create_list('ant@example.com') - # Now upgrade and check to see if the values got into the template - # table correctly. + # Create a mailing list without any URLs. + config.db.store.execute(mlist_table.insert().values( + list_id='ant.example.com')) + # Now upgrade. There are no templates. alembic.command.upgrade(alembic_cfg, 'fa0d96e28631') template_table = sa.sql.table( 'template', diff --git a/src/mailman/docs/DATABASE.rst b/src/mailman/docs/DATABASE.rst index cc7d09eb9..c85731fb3 100644 --- a/src/mailman/docs/DATABASE.rst +++ b/src/mailman/docs/DATABASE.rst @@ -71,12 +71,6 @@ database in MySQL via:: mysql> CREATE DATABASE mailman; -In some cases, our test suite requires the default collation of the database -to be set to `utf8_unicode_ci`. You can change it via the MySQL command line -like this:: - - mysql> ALTER DATABASE mailman DEFAULT COLLATE utf8_unicode_ci; - You would also need the Python driver `pymysql` for MySQL.:: $ pip install pymysql diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst index 49f309770..ee8e1c09b 100644 --- a/src/mailman/docs/NEWS.rst +++ b/src/mailman/docs/NEWS.rst @@ -117,7 +117,6 @@ Command line Database -------- - * MySQL is now an officially supported database. Given by Abhilash Raj. Interfaces @@ -243,6 +242,7 @@ Other * The ``mailman members`` command can now be used to display members based on subscription roles. Also, the positional "list" argument can now accept list names or list-ids. + * Unsubscriptions can now be confirmed and/or moderated. (Closes #213) 3.0.0 -- "Show Don't Tell" diff --git a/src/mailman/interfaces/mailinglist.py b/src/mailman/interfaces/mailinglist.py index 19db025d1..877016f41 100644 --- a/src/mailman/interfaces/mailinglist.py +++ b/src/mailman/interfaces/mailinglist.py @@ -46,6 +46,7 @@ class ReplyToMunging(Enum): @public class SubscriptionPolicy(Enum): + """All subscription/unsubscription policies for a mailing list.""" # Neither confirmation, nor moderator approval is required. open = 0 # The user must confirm the subscription. @@ -256,6 +257,9 @@ class IMailingList(Interface): subscription_policy = Attribute( """The policy for subscribing new members to the list.""") + unsubscription_policy = Attribute( + """The policy for unsubscribing members from the list.""") + subscribers = Attribute( """An iterator over all IMembers subscribed to this list, with any role. @@ -270,6 +274,18 @@ class IMailingList(Interface): :rtype: Roster """ + def is_subscribed(subscriber, role=MemberRole.member): + """Is the given address or user subscribed to the mailing list? + + :param subscriber: The address or user to check. + :type subscriber: `IUser` or `IAddress` + :param role: The role being checked (e.g. a member, owner, or + moderator of a mailing list). + :type role: `MemberRole` + :return: A flag indicating whether the subscriber is already + subscribed to the mailing list or not. + """ + def subscribe(subscriber, role=MemberRole.member): """Subscribe the given address or user to the mailing list. @@ -278,7 +294,7 @@ class IMailingList(Interface): has one, otherwise no address for the user appears in the rosters. :type subscriber: `IUser` or `IAddress` :param role: The role being subscribed to (e.g. a member, owner, or - moderator of a mailing list. + moderator of a mailing list). :type role: `MemberRole` :return: The member object representing the subscription. :rtype: `IMember` diff --git a/src/mailman/interfaces/registrar.py b/src/mailman/interfaces/registrar.py deleted file mode 100644 index e2cec5fbb..000000000 --- a/src/mailman/interfaces/registrar.py +++ /dev/null @@ -1,112 +0,0 @@ -# Copyright (C) 2007-2016 by the Free Software Foundation, Inc. -# -# This file is part of GNU Mailman. -# -# GNU Mailman is free software: you can redistribute it and/or modify it under -# the terms of the GNU General Public License as published by the Free -# Software Foundation, either version 3 of the License, or (at your option) -# any later version. -# -# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT -# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or -# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for -# more details. -# -# You should have received a copy of the GNU General Public License along with -# GNU Mailman. If not, see <http://www.gnu.org/licenses/>. - -"""Interface describing a user registration service. - -This is a higher level interface to user registration, address confirmation, -etc. than the IUserManager. The latter does no validation, syntax checking, -or confirmation, while this interface does. -""" - -from mailman import public -from zope.interface import Interface - - -@public -class ConfirmationNeededEvent: - """Triggered when an address needs confirmation. - - Addresses must be verified before they can receive messages or post - to mailing list. The confirmation message is sent to the user when - this event is triggered. - """ - def __init__(self, mlist, token, email): - self.mlist = mlist - self.token = token - self.email = email - - -@public -class IRegistrar(Interface): - """Interface for subscribing addresses and users. - - This is a higher level interface to user registration, email address - confirmation, etc. than the IUserManager. The latter does no validation, - syntax checking, or confirmation, while this interface does. - - To use this, adapt an ``IMailingList`` to this interface. - """ - - def register(subscriber=None, *, - pre_verified=False, pre_confirmed=False, pre_approved=False): - """Subscribe an address or user according to subscription policies. - - The mailing list's subscription policy is used to subscribe - `subscriber` to the given mailing list. The subscriber can be - an ``IUser``, in which case the user must have a preferred - address, and that preferred address will be subscribed. The - subscriber can also be an ``IAddress``, in which case the - address will be subscribed. - - The workflow may pause (i.e. be serialized, saved, and - suspended) when some out-of-band confirmation step is required. - For example, if the user must confirm, or the moderator must - approve the subscription. Use the ``confirm(token)`` method to - resume the workflow. - - :param subscriber: The user or address to subscribe. - :type email: ``IUser`` or ``IAddress`` - :return: A 3-tuple is returned where the first element is the token - hash, the second element is a ``TokenOwner`, and the third element - is the subscribed member. If the subscriber got subscribed - immediately, the token will be None and the member will be - an ``IMember``. If the subscription got held, the token - will be a hash and the member will be None. - :rtype: (str-or-None, ``TokenOwner``, ``IMember``-or-None) - :raises MembershipIsBannedError: when the address being subscribed - appears in the global or list-centric bans. - """ - - def confirm(token): - """Continue any paused workflow. - - Confirmation may occur after the user confirms their - subscription request, or their email address must be verified, - or the moderator must approve the subscription request. - - :param token: A token matching a workflow. - :type token: string - :return: A 3-tuple is returned where the first element is the token - hash, the second element is a ``TokenOwner`, and the third element - is the subscribed member. If the subscriber got subscribed - immediately, the token will be None and the member will be - an ``IMember``. If the subscription is still being held, the token - will be a hash and the member will be None. - :rtype: (str-or-None, ``TokenOwner``, ``IMember``-or-None) - :raises LookupError: when no workflow is associated with the token. - """ - - def discard(token): - """Discard the workflow matched to the given `token`. - - :param token: A token matching a pending event with a type of - 'registration'. - :raises LookupError: when no workflow is associated with the token. - """ - - def evict(): - """Evict all saved workflows which have expired.""" diff --git a/src/mailman/interfaces/subscriptions.py b/src/mailman/interfaces/subscriptions.py index 382a23ac0..e707cf706 100644 --- a/src/mailman/interfaces/subscriptions.py +++ b/src/mailman/interfaces/subscriptions.py @@ -15,7 +15,7 @@ # You should have received a copy of the GNU General Public License along with # GNU Mailman. If not, see <http://www.gnu.org/licenses/>. -"""Membership interface for REST.""" +"""Subscription management.""" from collections import namedtuple from enum import Enum @@ -78,8 +78,35 @@ class TokenOwner(Enum): @public +class SubscriptionConfirmationNeededEvent: + """Triggered when a subscription needs confirmation. + + Addresses must be verified before they can receive messages or post + to mailing list. The confirmation message is sent to the user when + this event is triggered. + """ + def __init__(self, mlist, token, email): + self.mlist = mlist + self.token = token + self.email = email + + +@public +class UnsubscriptionConfirmationNeededEvent: + """Triggered when an unsubscription request needs confirmation. + + The confirmation message is sent to the user when this event is + triggered. + """ + def __init__(self, mlist, token, email): + self.mlist = mlist + self.token = token + self.email = email + + +@public class ISubscriptionService(Interface): - """General Subscription services.""" + """General subscription services.""" def get_members(): """Return a sequence of all members of all mailing lists. @@ -178,3 +205,138 @@ class ISubscriptionService(Interface): :rtype: 2-tuple of (set-of-strings, set-of-strings) :raises NoSuchListError: if the named mailing list does not exist. """ + + +@public +class ISubscriptionManager(Interface): + """Handling subscription and unsubscription of addresses and users. + + This is a higher level interface to user registration and + unregistration, email address confirmation, etc. than the + `IUserManager`. The latter does no validation, syntax checking, or + confirmation, while this interface does. + + To use this, adapt an ``IMailingList`` to this interface. + """ + def register(subscriber=None, *, + pre_verified=False, pre_confirmed=False, pre_approved=False): + """Subscribe an address or user according to subscription policies. + + The mailing list's subscription policy is used to subscribe + `subscriber` to the given mailing list. The subscriber can be + an ``IUser``, in which case the user must have a preferred + address, and that preferred address will be subscribed. The + subscriber can also be an ``IAddress``, in which case the + address will be subscribed. + + The workflow may pause (i.e. be serialized, saved, and + suspended) when some out-of-band confirmation step is required. + For example, if the user must confirm, or the moderator must + approve the subscription. Use the ``confirm(token)`` method to + resume the workflow. + + :param subscriber: The user or address to subscribe. + :type email: ``IUser`` or ``IAddress`` + :param pre_verified: A flag indicating whether the subscriber's email + address should be considered pre-verified. Normally a never + before seen email address must be verified by mail-back + confirmation. Setting this flag to True automatically verifies + such addresses without the mail-back. (A confirmation message may + still be sent under other conditions.) + :type pre_verified: bool + :param pre_confirmed: A flag indicating whether, when required by the + subscription policy, a subscription request should be considered + pre-confirmed. Normally in such cases, a mail-back confirmation + message is sent to the subscriber, which must be positively + acknowledged by some manner. Setting this flag to True + automatically confirms the subscription request. (A confirmation + message may still be sent under other conditions.) + :type pre_confirmed: bool + :param pre_approved: A flag indicating whether, when required by the + subscription policy, a subscription request should be considered + pre-approved. Normally in such cases, the list administrator is + notified that an approval is necessary, which must be positively + acknowledged in some manner. Setting this flag to True + automatically approves the subscription request. + :type pre_approved: bool + :return: A 3-tuple is returned where the first element is the token + hash, the second element is a ``TokenOwner`, and the third element + is the subscribed member. If the subscriber got subscribed + immediately, the token will be None and the member will be + an ``IMember``. If the subscription got held, the token + will be a hash and the member will be None. + :rtype: (str-or-None, ``TokenOwner``, ``IMember``-or-None) + :raises MembershipIsBannedError: when the address being subscribed + appears in the global or list-centric bans. + """ + + def unregister(subscriber=None, *, + pre_confirmed=False, pre_approved=False): + """Unsubscribe an address or user according to subscription policies. + + The mailing list's unsubscription policy is used to unsubscribe + `subscriber` from the given mailing list. The subscriber can be + an ``IUser`` or an ``IAddress``, and must already be subscribed to the + mailing list. + + The workflow may pause (i.e. be serialized, saved, and + suspended) when some out-of-band confirmation step is required. + For example, if the user must confirm, or the moderator must + approve the unsubscription. Use the ``confirm(token)`` method to + resume the workflow. + + :param subscriber: The user or address to unsubscribe. + :type email: ``IUser`` or ``IAddress`` + :param pre_confirmed: A flag indicating whether, when required by the + unsubscription policy, an unsubscription request should be + considered pre-confirmed. Normally in such cases, a mail-back + confirmation message is sent to the subscriber, which must be + positively acknowledged by some manner. Setting this flag to True + automatically confirms the unsubscription request. (A confirmation + message may still be sent under other conditions.) + :type pre_confirmed: bool + :param pre_approved: A flag indicating whether, when required by the + unsubscription policy, an unsubscription request should be + considered pre-approved. Normally in such cases, the list + administrator is notified that an approval is necessary, which + must be positively acknowledged in some manner. Setting this flag + to True automatically approves the unsubscription request. + :type pre_approved: bool + :return: A 3-tuple is returned where the first element is the token + hash, the second element is a ``TokenOwner`, and the third element + is the unsubscribing member. If the subscriber got unsubscribed + immediately, the token will be None and the member will be + an ``IMember``. If the unsubscription got held, the token + will be a hash and the member will be None. + :rtype: (str-or-None, ``TokenOwner``, ``IMember``-or-None) + """ + + def confirm(token): + """Continue any paused workflow. + + Confirmation may occur after the user confirms their + subscription request, or their email address must be verified, + or the moderator must approve the subscription request. + + :param token: A token matching a workflow. + :type token: string + :return: A 3-tuple is returned where the first element is the token + hash, the second element is a ``TokenOwner`, and the third element + is the subscribed member. If the subscriber got subscribed + immediately, the token will be None and the member will be + an ``IMember``. If the subscription is still being held, the token + will be a hash and the member will be None. + :rtype: (str-or-None, ``TokenOwner``, ``IMember``-or-None) + :raises LookupError: when no workflow is associated with the token. + """ + + def discard(token): + """Discard the workflow matched to the given `token`. + + :param token: A token matching a pending event with a type of + 'registration'. + :raises LookupError: when no workflow is associated with the token. + """ + + def evict(): + """Evict all saved workflows which have expired.""" diff --git a/src/mailman/interfaces/template.py b/src/mailman/interfaces/template.py index 442f1eeda..e576eb882 100644 --- a/src/mailman/interfaces/template.py +++ b/src/mailman/interfaces/template.py @@ -171,7 +171,7 @@ ALL_TEMPLATES = { 'list:admin:notice:unrecognized', 'list:admin:notice:unsubscribe', 'list:member:digest:masthead', - 'list:user:action:confirm', + 'list:user:action:subscribe', 'list:user:action:unsubscribe', 'list:user:notice:hold', 'list:user:notice:no-more-today', diff --git a/src/mailman/interfaces/workflow.py b/src/mailman/interfaces/workflow.py index bf2138959..9846f4683 100644 --- a/src/mailman/interfaces/workflow.py +++ b/src/mailman/interfaces/workflow.py @@ -25,8 +25,6 @@ from zope.interface import Attribute, Interface class IWorkflowState(Interface): """The state of a workflow.""" - name = Attribute('The name of the workflow.') - token = Attribute('A unique key identifying the workflow instance.') step = Attribute("This workflow's next step.") @@ -38,11 +36,9 @@ class IWorkflowState(Interface): class IWorkflowStateManager(Interface): """The workflow states manager.""" - def save(name, token, step, data=None): + def save(token, step, data=None): """Save the state of a workflow. - :param name: The name of the workflow. - :type name: str :param token: A unique token identifying this workflow instance. :type token: str :param step: The next step for this workflow. @@ -51,11 +47,9 @@ class IWorkflowStateManager(Interface): :type data: str """ - def restore(name, token): + def restore(token): """Get the saved state for a workflow or None if nothing was saved. - :param name: The name of the workflow. - :type name: str :param token: A unique token identifying this workflow instance. :type token: str :return: The saved state associated with this name/token pair, or None @@ -63,11 +57,9 @@ class IWorkflowStateManager(Interface): :rtype: ``IWorkflowState`` """ - def discard(name, token): + def discard(token): """Throw away the saved state for a workflow. - :param name: The name of the workflow. - :type name: str :param token: A unique token identifying this workflow instance. :type token: str """ diff --git a/src/mailman/model/docs/registration.rst b/src/mailman/model/docs/registration.rst deleted file mode 100644 index 4b1e13520..000000000 --- a/src/mailman/model/docs/registration.rst +++ /dev/null @@ -1,100 +0,0 @@ -============ -Registration -============ - -When a user wants to join a mailing list, they must register and verify their -email address. Then depending on how the mailing list is configured, they may -need to confirm their subscription and have it approved by the list -moderator. The ``IRegistrar`` interface manages this work flow. - - >>> from mailman.interfaces.registrar import IRegistrar - -Registrars adapt mailing lists. - - >>> from mailman.interfaces.mailinglist import SubscriptionPolicy - >>> mlist = create_list('ant@example.com') - >>> mlist.send_welcome_message = False - >>> mlist.subscription_policy = SubscriptionPolicy.open - >>> registrar = IRegistrar(mlist) - -Usually, addresses are registered, but users with preferred addresses can be -registered too. - - >>> from mailman.interfaces.usermanager import IUserManager - >>> from zope.component import getUtility - >>> anne = getUtility(IUserManager).create_address( - ... 'anne@example.com', 'Anne Person') - - -Register an email address -========================= - -When the registration steps involve confirmation or moderator approval, the -process will pause until these steps are completed. A unique token is created -which represents this work flow. - -Anne attempts to join the mailing list. - - >>> token, token_owner, member = registrar.register(anne) - -Because her email address has not yet been verified, she has not yet become a -member of the mailing list. - - >>> print(member) - None - >>> print(mlist.members.get_member('anne@example.com')) - None - -Once she verifies her email address, she will become a member of the mailing -list. In this case, verifying implies that she also confirms her wish to join -the mailing list. - - >>> token, token_owner, member = registrar.confirm(token) - >>> member - <Member: Anne Person <anne@example.com> on ant@example.com - as MemberRole.member> - >>> mlist.members.get_member('anne@example.com') - <Member: Anne Person <anne@example.com> on ant@example.com - as MemberRole.member> - - -Register a user -=============== - -Users can also register, but they must have a preferred address. The mailing -list will deliver messages to this preferred address. - - >>> bart = getUtility(IUserManager).make_user( - ... 'bart@example.com', 'Bart Person') - -Bart verifies his address and makes it his preferred address. - - >>> from mailman.utilities.datetime import now - >>> preferred = list(bart.addresses)[0] - >>> preferred.verified_on = now() - >>> bart.preferred_address = preferred - -The mailing list's subscription policy does not require Bart to confirm his -subscription, but the moderate does want to approve all subscriptions. - - >>> mlist.subscription_policy = SubscriptionPolicy.moderate - -Now when Bart registers as a user for the mailing list, a token will still be -generated, but this is only used by the moderator. At first, Bart is not -subscribed to the mailing list. - - >>> token, token_owner, member = registrar.register(bart) - >>> print(member) - None - >>> print(mlist.members.get_member('bart@example.com')) - None - -When the moderator confirms Bart's subscription, he joins the mailing list. - - >>> token, token_owner, member = registrar.confirm(token) - >>> member - <Member: Bart Person <bart@example.com> on ant@example.com - as MemberRole.member> - >>> mlist.members.get_member('bart@example.com') - <Member: Bart Person <bart@example.com> on ant@example.com - as MemberRole.member> diff --git a/src/mailman/model/docs/subscriptions.rst b/src/mailman/model/docs/subscriptions.rst index 14cb36b71..1e1810a7a 100644 --- a/src/mailman/model/docs/subscriptions.rst +++ b/src/mailman/model/docs/subscriptions.rst @@ -1,14 +1,156 @@ -===================== +=============== + Subscriptions +=============== + +When a user wants to join a mailing list, they must register and verify their +email address. Then depending on how the mailing list is configured, they may +need to confirm their subscription and have it approved by the list moderator. +The ``ISubscriptionManager`` interface manages this work flow. + + >>> from mailman.interfaces.subscriptions import ISubscriptionManager + +To begin, adapt a mailing list to an ``ISubscriptionManager``. This is a +named interface because the same interface manages both subscriptions and +unsubscriptions. + + >>> mlist = create_list('ant@example.com') + >>> manager = ISubscriptionManager(mlist) + +Either addresses or users with a preferred address can be registered. + + >>> from mailman.interfaces.usermanager import IUserManager + >>> from zope.component import getUtility + >>> anne = getUtility(IUserManager).create_address( + ... 'anne@example.com', 'Anne Person') + + +Subscribing an email address +============================ + +The subscription process requires that the email address be verified. It may +also require confirmation or moderator approval depending on the mailing +list's subscription policy. For example, an open subscription policy does not +require confirmation or approval, but the email address must still be +verified, and the process will pause until these steps are completed. + + >>> from mailman.interfaces.mailinglist import SubscriptionPolicy + >>> mlist.subscription_policy = SubscriptionPolicy.open + +Anne attempts to join the mailing list. A unique token is created which +represents this work flow. + + >>> token, token_owner, member = manager.register(anne) + +Because her email address has not yet been verified, she has not yet become a +member of the mailing list. + + >>> print(member) + None + >>> print(mlist.members.get_member('anne@example.com')) + None + +Once she verifies her email address, she will become a member of the mailing +list. When the subscription policy requires confirmation, the verification +process implies that she also confirms her wish to join the mailing list. + + >>> token, token_owner, member = manager.confirm(token) + >>> member + <Member: Anne Person <anne@example.com> on ant@example.com + as MemberRole.member> + >>> mlist.members.get_member('anne@example.com') + <Member: Anne Person <anne@example.com> on ant@example.com + as MemberRole.member> + + +Subscribing a user +================== + +Users can also register, but they must have a preferred address. The mailing +list will deliver messages to this preferred address. + + >>> bart = getUtility(IUserManager).make_user( + ... 'bart@example.com', 'Bart Person') + +Bart verifies his address and makes it his preferred address. + + >>> from mailman.utilities.datetime import now + >>> preferred = list(bart.addresses)[0] + >>> preferred.verified_on = now() + >>> bart.preferred_address = preferred + +The mailing list's subscription policy does not require Bart to confirm his +subscription, but the moderate does want to approve all subscriptions. + + >>> mlist.subscription_policy = SubscriptionPolicy.moderate + +Now when Bart registers as a user for the mailing list, a token will still be +generated, but this is only used by the moderator. At first, Bart is not +subscribed to the mailing list. + + >>> token, token_owner, member = manager.register(bart) + >>> print(member) + None + >>> print(mlist.members.get_member('bart@example.com')) + None + +When the moderator confirms Bart's subscription, he joins the mailing list. + + >>> token, token_owner, member = manager.confirm(token) + >>> member + <Member: Bart Person <bart@example.com> on ant@example.com + as MemberRole.member> + >>> mlist.members.get_member('bart@example.com') + <Member: Bart Person <bart@example.com> on ant@example.com + as MemberRole.member> + + +Unsubscribing +============= + +Similarly, unsubscribing a user depends on the mailing list's unsubscription +policy. Of course, since the address or user is already subscribed, implying +that their email address is already verified, that step is not required. To +begin with unsubscribing, you need to adapt the mailing list to the same +interface, but with a different name. + + >>> manager = ISubscriptionManager(mlist) + +If the mailing list's unsubscription policy is open, unregistering the +subscription takes effect immediately. + + >>> mlist.unsubscription_policy = SubscriptionPolicy.open + >>> token, token_owner, member = manager.unregister(anne) + >>> print(mlist.members.get_member('anne@example.com')) + None + +Usually though, the member must confirm their unsubscription request, to +prevent an attacker from unsubscribing them from the list without their +knowledge. + + >>> mlist.unsubscription_policy = SubscriptionPolicy.confirm + >>> token, token_owner, member = manager.unregister(bart) + +Bart hasn't confirmed yet, so he's still a member of the list. + + >>> mlist.members.get_member('bart@example.com') + <Member: Bart Person <bart@example.com> on ant@example.com + as MemberRole.member> + +Once Bart confirms, he's unsubscribed from the mailing list. + + >>> token, token_owner, member = manager.confirm(token) + >>> print(mlist.members.get_member('bart@example.com')) + None + + Subscription services ===================== The ``ISubscriptionService`` utility provides higher level convenience methods useful for searching, retrieving, iterating, and removing memberships across -all mailing lists on th esystem. Adding new users is handled by the -``IRegistrar`` interface. +all mailing lists on the system. >>> from mailman.interfaces.subscriptions import ISubscriptionService - >>> from zope.component import getUtility >>> service = getUtility(ISubscriptionService) You can use the service to get all members of all mailing lists, for any @@ -30,7 +172,7 @@ When there are some members, of any role on any mailing list, they can be retrieved through the subscription service. >>> from mailman.app.lifecycle import create_list - >>> ant = create_list('ant@example.com') + >>> ant = mlist >>> bee = create_list('bee@example.com') >>> cat = create_list('cat@example.com') diff --git a/src/mailman/model/mailinglist.py b/src/mailman/model/mailinglist.py index 933384797..eec58ad04 100644 --- a/src/mailman/model/mailinglist.py +++ b/src/mailman/model/mailinglist.py @@ -178,6 +178,7 @@ class MailingList(Model): topics = Column(PickleType) topics_bodylines_limit = Column(Integer) topics_enabled = Column(Boolean) + unsubscription_policy = Column(Enum(SubscriptionPolicy)) # ORM relationships. header_matches = relationship( 'HeaderMatch', backref='mailing_list', @@ -440,31 +441,46 @@ class MailingList(Model): else: raise ValueError('Undefined MemberRole: {}'.format(role)) - @dbconnection - def subscribe(self, store, subscriber, role=MemberRole.member): - """See `IMailingList`.""" + def _get_subscriber(self, store, subscriber, role): + """Get some information about a user/address. + + Returns a 2-tuple of (member, email) for the given subscriber. If the + subscriber is is not an ``IAddress`` or ``IUser``, then a 2-tuple of + (None, None) is returned. If the subscriber is not already + subscribed, then (None, email) is returned. If the subscriber is an + ``IUser`` and does not have a preferred address, (member, None) is + returned. + """ + member = None + email = None if IAddress.providedBy(subscriber): member = store.query(Member).filter( Member.role == role, Member.list_id == self._list_id, Member._address == subscriber).first() - if member: - raise AlreadySubscribedError( - self.fqdn_listname, subscriber.email, role) + email = subscriber.email elif IUser.providedBy(subscriber): if subscriber.preferred_address is None: raise MissingPreferredAddressError(subscriber) + email = subscriber.preferred_address.email member = store.query(Member).filter( Member.role == role, Member.list_id == self._list_id, Member._user == subscriber).first() - if member: - raise AlreadySubscribedError( - self.fqdn_listname, - subscriber.preferred_address.email, - role) - else: - raise ValueError('subscriber must be an address or user') + return member, email + + @dbconnection + def is_subscribed(self, store, subscriber, role=MemberRole.member): + """See `IMailingList`.""" + member, email = self._get_subscriber(store, subscriber, role) + return member is not None + + @dbconnection + def subscribe(self, store, subscriber, role=MemberRole.member): + """See `IMailingList`.""" + member, email = self._get_subscriber(store, subscriber, role) + if member is not None: + raise AlreadySubscribedError(self.fqdn_listname, email, role) member = Member(role=role, list_id=self._list_id, subscriber=subscriber) diff --git a/src/mailman/model/pending.py b/src/mailman/model/pending.py index 9d8315605..7e7f0b2eb 100644 --- a/src/mailman/model/pending.py +++ b/src/mailman/model/pending.py @@ -130,8 +130,8 @@ class Pendings: # Iterate on PendedKeyValue entries that are associated with the # pending object's ID. Watch out for type conversions. for keyvalue in pending.key_values: - # The `type` key is special and served. It is not JSONified. See - # the IPendable interface for details. + # The `type` key is special and reserved. It is not JSONified. + # See the IPendable interface for details. if keyvalue.key == 'type': value = keyvalue.value else: diff --git a/src/mailman/model/tests/test_mailinglist.py b/src/mailman/model/tests/test_mailinglist.py index c9aecc93b..0a5c8a5e9 100644 --- a/src/mailman/model/tests/test_mailinglist.py +++ b/src/mailman/model/tests/test_mailinglist.py @@ -92,6 +92,19 @@ class TestMailingList(unittest.TestCase): self.assertIn('Anne Person <aperson@example.com>', items[0].msg.get_payload()) + def test_is_subscribed(self): + manager = getUtility(IUserManager) + user = manager.create_user('anne@example.com', 'Anne Person') + set_preferred(user) + self.assertEqual(False, self._mlist.is_subscribed(user)) + self._mlist.subscribe(user) + self.assertEqual(True, self._mlist.is_subscribed(user)) + address = manager.create_address('anne2@example.com', 'Anne Person') + address.verfied_on = now() + self.assertEqual(False, self._mlist.is_subscribed(address)) + self._mlist.subscribe(address) + self.assertEqual(True, self._mlist.is_subscribed(address)) + class TestListArchiver(unittest.TestCase): layer = ConfigLayer diff --git a/src/mailman/model/tests/test_workflow.py b/src/mailman/model/tests/test_workflow.py index 4c8c6776f..6aade980b 100644 --- a/src/mailman/model/tests/test_workflow.py +++ b/src/mailman/model/tests/test_workflow.py @@ -32,115 +32,88 @@ class TestWorkflow(unittest.TestCase): def test_save_restore_workflow(self): # Save and restore a workflow. - name = 'ant' token = 'bee' step = 'cat' data = 'dog' - self._manager.save(name, token, step, data) - state = self._manager.restore(name, token) - self.assertEqual(state.name, name) + self._manager.save(token, step, data) + state = self._manager.restore(token) self.assertEqual(state.token, token) self.assertEqual(state.step, step) self.assertEqual(state.data, data) def test_save_restore_workflow_without_step(self): # Save and restore a workflow that contains no step. - name = 'ant' token = 'bee' data = 'dog' - self._manager.save(name, token, data=data) - state = self._manager.restore(name, token) - self.assertEqual(state.name, name) + self._manager.save(token, data=data) + state = self._manager.restore(token) self.assertEqual(state.token, token) self.assertIsNone(state.step) self.assertEqual(state.data, data) def test_save_restore_workflow_without_data(self): # Save and restore a workflow that contains no data. - name = 'ant' token = 'bee' step = 'cat' - self._manager.save(name, token, step) - state = self._manager.restore(name, token) - self.assertEqual(state.name, name) + self._manager.save(token, step) + state = self._manager.restore(token) self.assertEqual(state.token, token) self.assertEqual(state.step, step) self.assertIsNone(state.data) def test_save_restore_workflow_without_step_or_data(self): # Save and restore a workflow that contains no step or data. - name = 'ant' token = 'bee' - self._manager.save(name, token) - state = self._manager.restore(name, token) - self.assertEqual(state.name, name) + self._manager.save(token) + state = self._manager.restore(token) self.assertEqual(state.token, token) self.assertIsNone(state.step) self.assertIsNone(state.data) - def test_restore_workflow_with_no_matching_name(self): - # Try to restore a workflow that has no matching name in the database. - name = 'ant' - token = 'bee' - self._manager.save(name, token) - state = self._manager.restore('ewe', token) - self.assertIsNone(state) - def test_restore_workflow_with_no_matching_token(self): # Try to restore a workflow that has no matching token in the database. - name = 'ant' - token = 'bee' - self._manager.save(name, token) - state = self._manager.restore(name, 'fly') - self.assertIsNone(state) - - def test_restore_workflow_with_no_matching_token_or_name(self): - # Try to restore a workflow that has no matching token or name in the - # database. - name = 'ant' token = 'bee' - self._manager.save(name, token) - state = self._manager.restore('ewe', 'fly') + self._manager.save(token) + state = self._manager.restore('fly') self.assertIsNone(state) def test_restore_removes_record(self): - name = 'ant' token = 'bee' self.assertEqual(self._manager.count, 0) - self._manager.save(name, token) + self._manager.save(token) self.assertEqual(self._manager.count, 1) - self._manager.restore(name, token) + self._manager.restore(token) self.assertEqual(self._manager.count, 0) def test_save_after_restore(self): - name = 'ant' token = 'bee' self.assertEqual(self._manager.count, 0) - self._manager.save(name, token) + self._manager.save(token) self.assertEqual(self._manager.count, 1) - self._manager.restore(name, token) + self._manager.restore(token) self.assertEqual(self._manager.count, 0) - self._manager.save(name, token) + self._manager.save(token) self.assertEqual(self._manager.count, 1) def test_discard(self): - # Discard some workflow state. This is use by IRegistrar.discard(). - self._manager.save('ant', 'token', 'one') - self._manager.save('bee', 'token', 'two') - self._manager.save('ant', 'nekot', 'three') - self._manager.save('bee', 'nekot', 'four') + # Discard some workflow state. This is use by + # ISubscriptionManager.discard(). + self._manager.save('token1', 'one') + self._manager.save('token2', 'two') + self._manager.save('token3', 'three') + self._manager.save('token4', 'four') self.assertEqual(self._manager.count, 4) - self._manager.discard('bee', 'token') + self._manager.discard('token2') self.assertEqual(self._manager.count, 3) - state = self._manager.restore('ant', 'token') + state = self._manager.restore('token1') self.assertEqual(state.step, 'one') - state = self._manager.restore('bee', 'token') + state = self._manager.restore('token2') self.assertIsNone(state) - state = self._manager.restore('ant', 'nekot') + state = self._manager.restore('token3') self.assertEqual(state.step, 'three') - state = self._manager.restore('bee', 'nekot') + state = self._manager.restore('token4') self.assertEqual(state.step, 'four') def test_discard_missing_workflow(self): - self._manager.discard('bogus-name', 'bogus-token') + self._manager.discard('bogus-token') self.assertEqual(self._manager.count, 0) diff --git a/src/mailman/model/workflow.py b/src/mailman/model/workflow.py index 1072fa548..53763a0e8 100644 --- a/src/mailman/model/workflow.py +++ b/src/mailman/model/workflow.py @@ -33,7 +33,6 @@ class WorkflowState(Model): __tablename__ = 'workflowstate' - name = Column(SAUnicode, primary_key=True) token = Column(SAUnicode, primary_key=True) step = Column(SAUnicode) data = Column(SAUnicode) @@ -45,23 +44,23 @@ class WorkflowStateManager: """See `IWorkflowStateManager`.""" @dbconnection - def save(self, store, name, token, step=None, data=None): + def save(self, store, token, step=None, data=None): """See `IWorkflowStateManager`.""" - state = WorkflowState(name=name, token=token, step=step, data=data) + state = WorkflowState(token=token, step=step, data=data) store.add(state) @dbconnection - def restore(self, store, name, token): + def restore(self, store, token): """See `IWorkflowStateManager`.""" - state = store.query(WorkflowState).get((name, token)) + state = store.query(WorkflowState).get(token) if state is not None: store.delete(state) return state @dbconnection - def discard(self, store, name, token): + def discard(self, store, token): """See `IWorkflowStateManager`.""" - state = store.query(WorkflowState).get((name, token)) + state = store.query(WorkflowState).get(token) if state is not None: store.delete(state) diff --git a/src/mailman/rest/docs/templates.rst b/src/mailman/rest/docs/templates.rst index 7ac0670e1..eefff09b7 100644 --- a/src/mailman/rest/docs/templates.rst +++ b/src/mailman/rest/docs/templates.rst @@ -479,7 +479,7 @@ below. Here are all the supported template names: (e.g. "French", "English", "Italian") * ``user_name`` - the recipient's display name if available -* ``list:user:action:confirm`` +* ``list:user:action:subscribe`` The message sent to subscribers when a subscription confirmation is required. @@ -490,6 +490,17 @@ below. Here are all the supported template names: to; this corresponds to the ``Reply-To`` header * ``user_email`` - the email address being confirmed +* ``list:user:action:unsubscribe`` + The message sent to subscribers when an unsubscription confirmation is + required. + + * ``token`` - the unique confirmation token + * ``subject`` - the ``Subject`` heading for the confirmation email, which + includes the confirmation token + * ``confirm_email`` - the email address to send the confirmation response + to; this corresponds to the ``Reply-To`` header + * ``user_email`` - the email address being confirmed + * ``list:user:notice:goodbye`` The notice sent to a member when they unsubscribe from a mailing list. diff --git a/src/mailman/rest/members.py b/src/mailman/rest/members.py index 11ad17c14..b35440af4 100644 --- a/src/mailman/rest/members.py +++ b/src/mailman/rest/members.py @@ -25,9 +25,9 @@ from mailman.interfaces.listmanager import IListManager from mailman.interfaces.member import ( AlreadySubscribedError, DeliveryMode, MemberRole, MembershipError, MembershipIsBannedError, MissingPreferredAddressError) -from mailman.interfaces.registrar import IRegistrar from mailman.interfaces.subscriptions import ( - ISubscriptionService, RequestRecord, SubscriptionPendingError, TokenOwner) + ISubscriptionManager, ISubscriptionService, RequestRecord, + SubscriptionPendingError, TokenOwner) from mailman.interfaces.user import IUser, UnverifiedAddressError from mailman.interfaces.usermanager import IUserManager from mailman.rest.helpers import ( @@ -253,7 +253,7 @@ class AllMembers(_MemberBase): # Now we can run the registration process until either the # subscriber is subscribed, or the workflow is paused for # verification, confirmation, or approval. - registrar = IRegistrar(mlist) + registrar = ISubscriptionManager(mlist) try: token, token_owner, member = registrar.register( subscriber, diff --git a/src/mailman/rest/sub_moderation.py b/src/mailman/rest/sub_moderation.py index f5ef072ff..55b6a7609 100644 --- a/src/mailman/rest/sub_moderation.py +++ b/src/mailman/rest/sub_moderation.py @@ -23,7 +23,7 @@ from mailman.core.i18n import _ from mailman.interfaces.action import Action from mailman.interfaces.member import AlreadySubscribedError from mailman.interfaces.pending import IPendings -from mailman.interfaces.registrar import IRegistrar +from mailman.interfaces.subscriptions import ISubscriptionManager from mailman.rest.helpers import ( CollectionMixin, bad_request, child, conflict, etag, no_content, not_found, okay) @@ -54,7 +54,7 @@ class IndividualRequest(_ModerationBase): def __init__(self, mlist, token): super().__init__() self._mlist = mlist - self._registrar = IRegistrar(self._mlist) + self._registrar = ISubscriptionManager(self._mlist) self._token = token def on_get(self, request, response): diff --git a/src/mailman/rest/tests/test_domains.py b/src/mailman/rest/tests/test_domains.py index 41723b07c..c707eaadb 100644 --- a/src/mailman/rest/tests/test_domains.py +++ b/src/mailman/rest/tests/test_domains.py @@ -288,7 +288,7 @@ class TestDomainTemplates(unittest.TestCase): 'list:member:digest:masthead': '', 'list:member:regular:footer': 'http://example.org/footer', 'list:member:regular:header': 'http://example.org/header', - 'list:user:action:confirm': '', + 'list:user:action:subscribe': '', 'list:user:action:unsubscribe': '', 'list:user:notice:goodbye': 'http://example.org/goodbye', 'list:user:notice:hold': '', diff --git a/src/mailman/rest/tests/test_lists.py b/src/mailman/rest/tests/test_lists.py index 787009855..da6e95f3b 100644 --- a/src/mailman/rest/tests/test_lists.py +++ b/src/mailman/rest/tests/test_lists.py @@ -674,7 +674,7 @@ class TestListTemplates(unittest.TestCase): 'list:member:digest:masthead': '', 'list:member:regular:footer': 'http://example.org/footer', 'list:member:regular:header': 'http://example.org/header', - 'list:user:action:confirm': '', + 'list:user:action:subscribe': '', 'list:user:action:unsubscribe': '', 'list:user:notice:goodbye': 'http://example.org/goodbye', 'list:user:notice:hold': '', diff --git a/src/mailman/rest/tests/test_membership.py b/src/mailman/rest/tests/test_membership.py index 1ea70e90b..f705fdcf6 100644 --- a/src/mailman/rest/tests/test_membership.py +++ b/src/mailman/rest/tests/test_membership.py @@ -25,8 +25,7 @@ from mailman.database.transaction import transaction from mailman.interfaces.bans import IBanManager from mailman.interfaces.mailinglist import SubscriptionPolicy from mailman.interfaces.member import DeliveryMode, MemberRole -from mailman.interfaces.registrar import IRegistrar -from mailman.interfaces.subscriptions import TokenOwner +from mailman.interfaces.subscriptions import ISubscriptionManager, TokenOwner from mailman.interfaces.usermanager import IUserManager from mailman.runners.incoming import IncomingRunner from mailman.testing.helpers import ( @@ -215,7 +214,7 @@ class TestMembership(unittest.TestCase): def test_duplicate_pending_subscription(self): # Issue #199 - a member's subscription is already pending and they try # to subscribe again. - registrar = IRegistrar(self._mlist) + registrar = ISubscriptionManager(self._mlist) with transaction(): self._mlist.subscription_policy = SubscriptionPolicy.moderate anne = self._usermanager.create_address('anne@example.com') @@ -238,7 +237,7 @@ class TestMembership(unittest.TestCase): # Issue #199 - a member's subscription is already pending and they try # to subscribe again. Unlike above, this pend is waiting for the user # to confirm their subscription. - registrar = IRegistrar(self._mlist) + registrar = ISubscriptionManager(self._mlist) with transaction(): self._mlist.subscription_policy = ( SubscriptionPolicy.confirm_then_moderate) diff --git a/src/mailman/rest/tests/test_moderation.py b/src/mailman/rest/tests/test_moderation.py index 9f9da6b18..e0c3f1ccf 100644 --- a/src/mailman/rest/tests/test_moderation.py +++ b/src/mailman/rest/tests/test_moderation.py @@ -24,8 +24,8 @@ from mailman.app.moderator import hold_message from mailman.database.transaction import transaction from mailman.interfaces.bans import IBanManager from mailman.interfaces.mailinglist import SubscriptionPolicy -from mailman.interfaces.registrar import IRegistrar from mailman.interfaces.requests import IListRequests, RequestType +from mailman.interfaces.subscriptions import ISubscriptionManager from mailman.interfaces.usermanager import IUserManager from mailman.testing.helpers import ( call_api, get_queue_messages, set_preferred, @@ -150,7 +150,7 @@ class TestSubscriptionModeration(unittest.TestCase): def setUp(self): with transaction(): self._mlist = create_list('ant@example.com') - self._registrar = IRegistrar(self._mlist) + self._registrar = ISubscriptionManager(self._mlist) manager = getUtility(IUserManager) self._anne = manager.create_address( 'anne@example.com', 'Anne Person') diff --git a/src/mailman/rest/tests/test_root.py b/src/mailman/rest/tests/test_root.py index 840212825..eb246cb1d 100644 --- a/src/mailman/rest/tests/test_root.py +++ b/src/mailman/rest/tests/test_root.py @@ -288,7 +288,7 @@ class TestSiteTemplates(unittest.TestCase): 'list:member:digest:masthead': '', 'list:member:regular:footer': 'http://example.org/footer', 'list:member:regular:header': 'http://example.org/header', - 'list:user:action:confirm': '', + 'list:user:action:subscribe': '', 'list:user:action:unsubscribe': '', 'list:user:notice:goodbye': 'http://example.org/goodbye', 'list:user:notice:hold': '', diff --git a/src/mailman/runners/docs/command.rst b/src/mailman/runners/docs/command.rst index c97e6454c..2243d4ff4 100644 --- a/src/mailman/runners/docs/command.rst +++ b/src/mailman/runners/docs/command.rst @@ -140,14 +140,15 @@ address, and the other is the results of his email command. >>> len(messages) 2 - >>> from mailman.interfaces.registrar import IRegistrar - >>> registrar = IRegistrar(mlist) + >>> from mailman.interfaces.subscriptions import ISubscriptionManager + + >>> manager = ISubscriptionManager(mlist) >>> for item in messages: ... subject = item.msg['subject'] ... print('Subject:', subject) ... if 'confirm' in str(subject): ... token = str(subject).split()[1].strip() - ... new_token, token_owner, member = registrar.confirm(token) + ... new_token, token_owner, member = manager.confirm(token) ... assert new_token is None, 'Confirmation failed' Subject: The results of your email commands Subject: confirm ... @@ -165,27 +166,29 @@ Similarly, to leave a mailing list, the user need only email the ``-leave`` or ... ... """) + >>> from mailman.interfaces.mailinglist import SubscriptionPolicy + >>> mlist.unsubscribe_policy = SubscriptionPolicy.open >>> filebase = inject_message( ... mlist, msg, switchboard='command', subaddress='leave') >>> command.run() >>> messages = get_queue_messages('virgin') >>> len(messages) - 1 + 2 >>> print(messages[0].msg.as_string()) + MIME-Version: 1.0 + ... + Subject: You have been unsubscribed from the Test mailing list + From: test-bounces@example.com + To: dperson@example.com + ... + + >>> print(messages[1].msg.as_string()) Subject: The results of your email commands From: test-bounces@example.com To: dperson@example.com ... <BLANKLINE> - The results of your email command are provided below. - <BLANKLINE> - - Original message details: - From: dperson@example.com - Subject: n/a - Date: ... - Message-ID: ... - <BLANKLINE> - Results: Dirk Person <dperson@example.com> left test@example.com <BLANKLINE> diff --git a/src/mailman/runners/tests/test_confirm.py b/src/mailman/runners/tests/test_confirm.py index 7dc2403fc..af6f03eb5 100644 --- a/src/mailman/runners/tests/test_confirm.py +++ b/src/mailman/runners/tests/test_confirm.py @@ -24,7 +24,7 @@ from email.iterators import body_line_iterator from mailman.app.lifecycle import create_list from mailman.config import config from mailman.database.transaction import transaction -from mailman.interfaces.registrar import IRegistrar +from mailman.interfaces.subscriptions import ISubscriptionManager from mailman.interfaces.usermanager import IUserManager from mailman.runners.command import CommandRunner from mailman.testing.helpers import ( @@ -47,7 +47,7 @@ class TestConfirm(unittest.TestCase): self._mlist = create_list('test@example.com') self._mlist.send_welcome_message = False anne = getUtility(IUserManager).create_address('anne@example.org') - registrar = IRegistrar(self._mlist) + registrar = ISubscriptionManager(self._mlist) self._token, token_owner, member = registrar.register(anne) def test_confirm_with_re_prefix(self): diff --git a/src/mailman/runners/tests/test_join.py b/src/mailman/runners/tests/test_join.py index 0f24f3847..1e249d68f 100644 --- a/src/mailman/runners/tests/test_join.py +++ b/src/mailman/runners/tests/test_join.py @@ -23,8 +23,8 @@ from email.iterators import body_line_iterator from mailman.app.lifecycle import create_list from mailman.config import config from mailman.interfaces.member import DeliveryMode -from mailman.interfaces.registrar import IRegistrar -from mailman.interfaces.subscriptions import ISubscriptionService, TokenOwner +from mailman.interfaces.subscriptions import ( + ISubscriptionManager, ISubscriptionService, TokenOwner) from mailman.interfaces.usermanager import IUserManager from mailman.runners.command import CommandRunner from mailman.testing.helpers import ( @@ -145,7 +145,8 @@ class TestJoinWithDigests(unittest.TestCase): subject_words = str(items[1].msg['subject']).split() self.assertEqual(subject_words[0], 'confirm') token = subject_words[1] - token, token_owner, rmember = IRegistrar(self._mlist).confirm(token) + token, token_owner, rmember = ISubscriptionManager( + self._mlist).confirm(token) self.assertIsNone(token) self.assertEqual(token_owner, TokenOwner.no_one) # Now, make sure that Anne is a member of the list and is receiving diff --git a/src/mailman/runners/tests/test_leave.py b/src/mailman/runners/tests/test_leave.py new file mode 100644 index 000000000..008e8eb57 --- /dev/null +++ b/src/mailman/runners/tests/test_leave.py @@ -0,0 +1,119 @@ +# Copyright (C) 2016 by the Free Software Foundation, Inc. +# +# This file is part of GNU Mailman. +# +# GNU Mailman is free software: you can redistribute it and/or modify it under +# the terms of the GNU General Public License as published by the Free +# Software Foundation, either version 3 of the License, or (at your option) +# any later version. +# +# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along with +# GNU Mailman. If not, see <http://www.gnu.org/licenses/>. + +"""Test mailing list un-subscriptions.""" + +import unittest + +from email.iterators import body_line_iterator +from mailman.app.lifecycle import create_list +from mailman.config import config +from mailman.database.transaction import transaction +from mailman.interfaces.mailinglist import SubscriptionPolicy +from mailman.interfaces.usermanager import IUserManager +from mailman.runners.command import CommandRunner +from mailman.testing.helpers import ( + get_queue_messages, make_testable_runner, set_preferred, + specialized_message_from_string as mfs) +from mailman.testing.layers import ConfigLayer +from zope.component import getUtility + + +def confirmation_line(msg): + confirmation_lines = [] + in_results = False + for line in body_line_iterator(msg): + line = line.strip() + if in_results: + if line.startswith('- Done'): + break + if len(line) > 0: + confirmation_lines.append(line) + if line.strip() == '- Results:': + in_results = True + # There should be exactly one confirmation line. + assert len(confirmation_lines) == 1, confirmation_lines + return confirmation_lines[0] + + +class TestLeave(unittest.TestCase): + """Test mailing list un-subscriptions""" + + layer = ConfigLayer + + def setUp(self): + with transaction(): + self._mlist = create_list('test@example.com') + self._mlist.send_goodbye_message = False + self._mlist.send_welcome_message = False + self._commandq = config.switchboards['command'] + self._runner = make_testable_runner(CommandRunner, 'command') + + def test_leave(self): + with transaction(): + self._mlist.unsubscription_policy = SubscriptionPolicy.confirm + anne = getUtility(IUserManager).create_user('anne@example.org') + set_preferred(anne) + self._mlist.subscribe(anne.preferred_address) + msg = mfs("""\ +From: anne@example.org +To: test-leave@example.com + +leave +""") + self._commandq.enqueue(msg, dict(listid='test.example.com', + subaddress='leave')) + self._runner.run() + # Two messages have been sent, both to anne@example.org. The first + # asks for her confirmation of her unsubscription event. The second + # is the results of her email command. + items = get_queue_messages('virgin', sort_on='subject', + expected_count=2) + if items[0].msg['from'] == 'test-bounces@example.com': + results = items[0].msg + confirmation = items[1].msg + else: + results = items[1].msg + confirmation = items[0].msg + self.assertTrue(str(confirmation['subject']).startswith('confirm')) + line = confirmation_line(results) + # The confirmation line should name Anne's email address. + self.assertIn('anne@example.org', line) + + def test_double_leave(self): + # In this case, the user can be unsubscribed immediately because the + # policy does not require confirmation, however because the email is + # sent to the -leave address and it contains the 'leave' command, we + # should only process one command per email. + with transaction(): + self._mlist.unsubscription_policy = SubscriptionPolicy.open + anne = getUtility(IUserManager).create_user('anne@example.org') + set_preferred(anne) + self._mlist.subscribe(anne.preferred_address) + msg = mfs("""\ +From: anne@example.org +To: test-leave@example.com + +leave +""") + self._commandq.enqueue(msg, dict(listid='test.example.com', + subaddress='leave')) + self._runner.run() + items = get_queue_messages('virgin', sort_on='subject', + expected_count=1) + line = confirmation_line(items[0].msg) + self.assertEqual(line, 'anne@example.org left test@example.com') diff --git a/src/mailman/styles/base.py b/src/mailman/styles/base.py index 6620e85f2..b4c1c336d 100644 --- a/src/mailman/styles/base.py +++ b/src/mailman/styles/base.py @@ -67,6 +67,7 @@ class BasicOperation: mlist.default_member_action = Action.defer mlist.default_nonmember_action = Action.hold mlist.subscription_policy = SubscriptionPolicy.confirm + mlist.unsubscription_policy = SubscriptionPolicy.open # Notify the administrator of pending requests and membership changes. mlist.admin_immed_notify = True mlist.admin_notify_mchanges = False diff --git a/src/mailman/templates/en/list:user:action:confirm.txt b/src/mailman/templates/en/list:user:action:subscribe.txt index ce384fea2..10ec5a3b5 100644 --- a/src/mailman/templates/en/list:user:action:confirm.txt +++ b/src/mailman/templates/en/list:user:action:subscribe.txt @@ -10,7 +10,7 @@ Before you can start using GNU Mailman at this site, you must first confirm that this is your email address. You can do this by replying to this message, keeping the Subject header intact. -If you do not wish to register this email address simply disregard this +If you do not wish to register this email address, simply disregard this message. If you think you are being maliciously subscribed to the list, or have any other questions, you may contact diff --git a/src/mailman/templates/en/list:user:action:unsubscribe.txt b/src/mailman/templates/en/list:user:action:unsubscribe.txt index 97ebd30b2..583b83c82 100644 --- a/src/mailman/templates/en/list:user:action:unsubscribe.txt +++ b/src/mailman/templates/en/list:user:action:unsubscribe.txt @@ -1,19 +1,17 @@ -Mailing list removal confirmation notice for mailing list $listname +Email Address Unsubscription Confirmation -We have received a request for the removal of your email address, "${email}" -from the $listaddr mailing list. To confirm that you want to be removed from -this mailing list, simply reply to this message, keeping the Subject header -intact. +Hello, this is the GNU Mailman server at $domain. + +We have received an unsubscription request for the email address -Or include the following line -- and only the following line -- in a -message to $requestaddr: + $user_email - confirm $cookie +Before GNU Mailman can unsubscribe you, you must first confirm your request. +You can do this by replying to this message, keeping the Subject header +intact. -Note that simply sending a reply to this message should work from most mail -readers, since that usually leaves the Subject line in the right form -(additional "Re:" text in the Subject is okay). +If you do not wish to unsubscribe this email address, simply disregard this +message. If you think you are being maliciously unsubscribed from the list, +or have any other questions, you may contact -If you do not wish to be removed from this list, please simply disregard this -message. If you think you are being maliciously removed from the list, or -have any other questions, send them to $listadmin + $owner_email diff --git a/src/mailman/testing/helpers.py b/src/mailman/testing/helpers.py index 6496484ca..d9cf974f5 100644 --- a/src/mailman/testing/helpers.py +++ b/src/mailman/testing/helpers.py @@ -18,6 +18,7 @@ """Various test helpers.""" import os +import sys import json import time import uuid @@ -118,8 +119,11 @@ def get_queue_messages(queue_name, sort_on=None, expected_count=None): messages.append(_Bag(msg=msg, msgdata=msgdata)) queue.finish(filebase) if expected_count is not None: - assert len(messages) == expected_count, 'Wanted {}, got {}'.format( - expected_count, len(messages)) + if len(messages) != expected_count: + for item in messages: + print(item.msg, file=sys.stderr) + raise AssertionError('Wanted {}, got {}'.format( + expected_count, len(messages))) if sort_on is not None: messages.sort(key=lambda item: str(item.msg[sort_on])) return messages |