diff options
| author | J08nY | 2017-07-05 01:07:37 +0200 |
|---|---|---|
| committer | J08nY | 2017-08-30 13:18:40 +0200 |
| commit | d173b42e5ff5d90167091e6a015d394c2ddd8678 (patch) | |
| tree | aa1f5da977690822e80ce61b7b4cfddd0375cb62 | |
| parent | 34bf9690fd808d1ece8f6c2d674605fc25018894 (diff) | |
| download | mailman-d173b42e5ff5d90167091e6a015d394c2ddd8678.tar.gz mailman-d173b42e5ff5d90167091e6a015d394c2ddd8678.tar.zst mailman-d173b42e5ff5d90167091e6a015d394c2ddd8678.zip | |
29 files changed, 620 insertions, 719 deletions
diff --git a/src/mailman/app/docs/moderator.rst b/src/mailman/app/docs/moderator.rst index ce25a4711..6fcaf0b08 100644 --- a/src/mailman/app/docs/moderator.rst +++ b/src/mailman/app/docs/moderator.rst @@ -224,7 +224,7 @@ Fred is a member of the mailing list... >>> 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) + ... fred, pre_verified=True, pre_confirmed=True) >>> member <Member: Fred Person <fred@example.com> on ant@example.com as MemberRole.member> @@ -301,16 +301,16 @@ Usually, the list administrators want to be notified when there are membership change requests they need to moderate. These notifications are sent when the list is configured to send them. - >>> from mailman.interfaces.mailinglist import SubscriptionPolicy + >>> from mailman.workflows.subscription import ModerationSubscriptionPolicy >>> mlist.admin_immed_notify = True - >>> mlist.subscription_policy = SubscriptionPolicy.moderate + >>> mlist.subscription_policy = ModerationSubscriptionPolicy Gwen tries to subscribe to the mailing list. >>> gwen = getUtility(IUserManager).create_address( ... 'gwen@example.com', 'Gwen Person') >>> token, token_owner, member = registrar.register( - ... gwen, pre_verified=True, pre_confirmed=True) + ... gwen, pre_verified=True) Her subscription must be approved by the list administrator, so she is not yet a member of the mailing list. @@ -414,7 +414,7 @@ message. >>> herb = getUtility(IUserManager).create_address( ... 'herb@example.com', 'Herb Person') >>> token, token_owner, member = registrar.register( - ... herb, pre_verified=True, pre_confirmed=True, pre_approved=True) + ... herb, pre_verified=True, pre_approved=True) >>> messages = get_queue_messages('virgin') >>> len(messages) 1 diff --git a/src/mailman/app/subscriptions.py b/src/mailman/app/subscriptions.py index c89e1e79f..e1c1d8993 100644 --- a/src/mailman/app/subscriptions.py +++ b/src/mailman/app/subscriptions.py @@ -28,8 +28,6 @@ from mailman.interfaces.subscriptions import ( from mailman.interfaces.template import ITemplateLoader from mailman.interfaces.workflows import IWorkflowStateManager from mailman.utilities.string import expand -from mailman.workflows.builtin import (SubscriptionWorkflow, - UnSubscriptionWorkflow) from mailman.workflows.common import (PendableSubscription, PendableUnsubscription) from public import public @@ -43,23 +41,16 @@ class SubscriptionManager: def __init__(self, mlist): self._mlist = mlist - def register(self, subscriber=None, *, - pre_verified=False, pre_confirmed=False, pre_approved=False): + def register(self, subscriber=None, **kwargs): """See `ISubscriptionManager`.""" - workflow = SubscriptionWorkflow( - self._mlist, subscriber, - pre_verified=pre_verified, - pre_confirmed=pre_confirmed, - pre_approved=pre_approved) + workflow = self._mlist.subscription_policy(self._mlist, subscriber, + **kwargs) 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) + def unregister(self, subscriber=None, **kwargs): + workflow = self._mlist.unsubscription_policy(self._mlist, subscriber, + **kwargs) list(workflow) return workflow.token, workflow.token_owner, workflow.member @@ -72,9 +63,9 @@ class SubscriptionManager: workflow_type = pendable.get('type') assert workflow_type in (PendableSubscription.PEND_TYPE, PendableUnsubscription.PEND_TYPE) - workflow = (SubscriptionWorkflow + workflow = (self._mlist.subscription_policy if workflow_type == PendableSubscription.PEND_TYPE - else UnSubscriptionWorkflow)(self._mlist) + else self._mlist.unsubscription_policy)(self._mlist) workflow.token = token workflow.restore() # In order to just run the whole workflow, all we need to do diff --git a/src/mailman/app/tests/test_moderation.py b/src/mailman/app/tests/test_moderation.py index 5448a1d79..7d7524a10 100644 --- a/src/mailman/app/tests/test_moderation.py +++ b/src/mailman/app/tests/test_moderation.py @@ -162,7 +162,7 @@ class TestUnsubscription(unittest.TestCase): user_manager = getUtility(IUserManager) anne = user_manager.create_address('anne@example.org', 'Anne Person') token, token_owner, member = self._manager.register( - anne, pre_verified=True, pre_confirmed=True, pre_approved=True) + anne, pre_verified=True, pre_confirmed=True) self.assertIsNone(token) self.assertEqual(member.address.email, 'anne@example.org') bart = user_manager.create_user('bart@example.com', 'Bart User') diff --git a/src/mailman/app/tests/test_subscriptions.py b/src/mailman/app/tests/test_subscriptions.py index 0519ec385..23487ab1c 100644 --- a/src/mailman/app/tests/test_subscriptions.py +++ b/src/mailman/app/tests/test_subscriptions.py @@ -21,9 +21,7 @@ import unittest from contextlib import suppress from mailman.app.lifecycle import create_list -from mailman.app.subscriptions import SubscriptionWorkflow from mailman.interfaces.bans import IBanManager -from mailman.interfaces.mailinglist import SubscriptionPolicy from mailman.interfaces.member import MemberRole, MembershipIsBannedError from mailman.interfaces.pending import IPendings from mailman.interfaces.subscriptions import TokenOwner @@ -32,6 +30,9 @@ from mailman.testing.helpers import ( LogFileMark, get_queue_messages, set_preferred) from mailman.testing.layers import ConfigLayer from mailman.utilities.datetime import now +from mailman.workflows.subscription import ( + ConfirmModerationSubscriptionPolicy, ConfirmSubscriptionPolicy, + ModerationSubscriptionPolicy, OpenSubscriptionPolicy) from unittest.mock import patch from zope.component import getUtility @@ -55,7 +56,7 @@ class TestSubscriptionWorkflow(unittest.TestCase): def test_start_state(self): # The workflow starts with no tokens or member. - workflow = SubscriptionWorkflow(self._mlist) + workflow = ConfirmSubscriptionPolicy(self._mlist) self.assertIsNone(workflow.token) self.assertEqual(workflow.token_owner, TokenOwner.no_one) self.assertIsNone(workflow.member) @@ -64,7 +65,7 @@ class TestSubscriptionWorkflow(unittest.TestCase): # There is a Pendable associated with the held request, and it has # some data associated with it. anne = self._user_manager.create_address(self._anne) - workflow = SubscriptionWorkflow(self._mlist, anne) + workflow = ConfirmSubscriptionPolicy(self._mlist, anne) with suppress(StopIteration): workflow.run_thru('send_confirmation') self.assertIsNotNone(workflow.token) @@ -79,14 +80,14 @@ class TestSubscriptionWorkflow(unittest.TestCase): def test_user_or_address_required(self): # The `subscriber` attribute must be a user or address. - workflow = SubscriptionWorkflow(self._mlist) + workflow = ConfirmSubscriptionPolicy(self._mlist) self.assertRaises(AssertionError, list, workflow) def test_sanity_checks_address(self): # Ensure that the sanity check phase, when given an IAddress, ends up # with a linked user. anne = self._user_manager.create_address(self._anne) - workflow = SubscriptionWorkflow(self._mlist, anne) + workflow = ConfirmSubscriptionPolicy(self._mlist, anne) self.assertIsNotNone(workflow.address) self.assertIsNone(workflow.user) workflow.run_thru('sanity_checks') @@ -99,7 +100,7 @@ class TestSubscriptionWorkflow(unittest.TestCase): # preferred address, ends up with an address. anne = self._user_manager.make_user(self._anne) address = set_preferred(anne) - workflow = SubscriptionWorkflow(self._mlist, anne) + workflow = ConfirmSubscriptionPolicy(self._mlist, anne) # The constructor sets workflow.address because the user has a # preferred address. self.assertEqual(workflow.address, address) @@ -113,7 +114,7 @@ class TestSubscriptionWorkflow(unittest.TestCase): # preferred address, but with at least one linked address, gets an # address. anne = self._user_manager.make_user(self._anne) - workflow = SubscriptionWorkflow(self._mlist, anne) + workflow = ConfirmSubscriptionPolicy(self._mlist, anne) self.assertIsNone(workflow.address) self.assertEqual(workflow.user, anne) workflow.run_thru('sanity_checks') @@ -127,7 +128,7 @@ class TestSubscriptionWorkflow(unittest.TestCase): anne = self._user_manager.make_user(self._anne) anne.link(self._user_manager.create_address('anne@example.net')) anne.link(self._user_manager.create_address('anne@example.org')) - workflow = SubscriptionWorkflow(self._mlist, anne) + workflow = ConfirmSubscriptionPolicy(self._mlist, anne) self.assertIsNone(workflow.address) self.assertEqual(workflow.user, anne) workflow.run_thru('sanity_checks') @@ -139,21 +140,21 @@ class TestSubscriptionWorkflow(unittest.TestCase): def test_sanity_checks_user_without_addresses(self): # It is an error to try to subscribe a user with no linked addresses. user = self._user_manager.create_user() - workflow = SubscriptionWorkflow(self._mlist, user) + workflow = ConfirmSubscriptionPolicy(self._mlist, user) self.assertRaises(AssertionError, workflow.run_thru, 'sanity_checks') def test_sanity_checks_globally_banned_address(self): # An exception is raised if the address is globally banned. anne = self._user_manager.create_address(self._anne) IBanManager(None).ban(self._anne) - workflow = SubscriptionWorkflow(self._mlist, anne) + workflow = ConfirmSubscriptionPolicy(self._mlist, anne) self.assertRaises(MembershipIsBannedError, list, workflow) def test_sanity_checks_banned_address(self): # An exception is raised if the address is banned by the mailing list. anne = self._user_manager.create_address(self._anne) IBanManager(self._mlist).ban(self._anne) - workflow = SubscriptionWorkflow(self._mlist, anne) + workflow = ConfirmSubscriptionPolicy(self._mlist, anne) self.assertRaises(MembershipIsBannedError, list, workflow) def test_verification_checks_with_verified_address(self): @@ -161,7 +162,7 @@ class TestSubscriptionWorkflow(unittest.TestCase): # confirmation checks. anne = self._user_manager.create_address(self._anne) anne.verified_on = now() - workflow = SubscriptionWorkflow(self._mlist, anne) + workflow = ConfirmSubscriptionPolicy(self._mlist, anne) workflow.run_thru('verification_checks') with patch.object(workflow, '_step_confirmation_checks') as step: next(workflow) @@ -171,7 +172,8 @@ class TestSubscriptionWorkflow(unittest.TestCase): # When the address is not yet verified, but the pre-verified flag is # passed to the workflow, we skip to the confirmation checks. anne = self._user_manager.create_address(self._anne) - workflow = SubscriptionWorkflow(self._mlist, anne, pre_verified=True) + workflow = ConfirmSubscriptionPolicy(self._mlist, anne, + pre_verified=True) workflow.run_thru('verification_checks') with patch.object(workflow, '_step_confirmation_checks') as step: next(workflow) @@ -184,7 +186,7 @@ class TestSubscriptionWorkflow(unittest.TestCase): # A confirmation message must be sent to the user which will also # verify their address. anne = self._user_manager.create_address(self._anne) - workflow = SubscriptionWorkflow(self._mlist, anne) + workflow = ConfirmSubscriptionPolicy(self._mlist, anne) workflow.run_thru('verification_checks') with patch.object(workflow, '_step_send_confirmation') as step: next(workflow) @@ -195,10 +197,11 @@ class TestSubscriptionWorkflow(unittest.TestCase): def test_confirmation_checks_open_list(self): # A subscription to an open list does not need to be confirmed or # moderated. - self._mlist.subscription_policy = SubscriptionPolicy.open + self._mlist.subscription_policy = OpenSubscriptionPolicy anne = self._user_manager.create_address(self._anne) - workflow = SubscriptionWorkflow(self._mlist, anne, pre_verified=True) - workflow.run_thru('confirmation_checks') + workflow = self._mlist.subscription_policy(self._mlist, anne, + pre_verified=True) + workflow.run_thru('verification_checks') with patch.object(workflow, '_step_do_subscription') as step: next(workflow) step.assert_called_once_with() @@ -206,10 +209,11 @@ class TestSubscriptionWorkflow(unittest.TestCase): def test_confirmation_checks_no_user_confirmation_needed(self): # A subscription to a list which does not need user confirmation skips # to the moderation checks. - self._mlist.subscription_policy = SubscriptionPolicy.moderate + self._mlist.subscription_policy = ModerationSubscriptionPolicy anne = self._user_manager.create_address(self._anne) - workflow = SubscriptionWorkflow(self._mlist, anne, pre_verified=True) - workflow.run_thru('confirmation_checks') + workflow = self._mlist.subscription_policy(self._mlist, anne, + pre_verified=True) + workflow.run_thru('verification_checks') with patch.object(workflow, '_step_moderation_checks') as step: next(workflow) step.assert_called_once_with() @@ -218,11 +222,11 @@ class TestSubscriptionWorkflow(unittest.TestCase): # The subscription policy requires user confirmation, but their # subscription is pre-confirmed. Since moderation is not required, # the user will be immediately subscribed. - self._mlist.subscription_policy = SubscriptionPolicy.confirm + self._mlist.subscription_policy = ConfirmSubscriptionPolicy anne = self._user_manager.create_address(self._anne) - workflow = SubscriptionWorkflow(self._mlist, anne, - pre_verified=True, - pre_confirmed=True) + workflow = self._mlist.subscription_policy(self._mlist, anne, + pre_verified=True, + pre_confirmed=True) workflow.run_thru('confirmation_checks') with patch.object(workflow, '_step_do_subscription') as step: next(workflow) @@ -233,11 +237,11 @@ class TestSubscriptionWorkflow(unittest.TestCase): # subscription is pre-confirmed. Since moderation is required, that # check will be performed. self._mlist.subscription_policy = ( - SubscriptionPolicy.confirm_then_moderate) + ConfirmModerationSubscriptionPolicy) anne = self._user_manager.create_address(self._anne) - workflow = SubscriptionWorkflow(self._mlist, anne, - pre_verified=True, - pre_confirmed=True) + workflow = self._mlist.subscription_policy(self._mlist, anne, + pre_verified=True, + pre_confirmed=True) workflow.run_thru('confirmation_checks') with patch.object(workflow, '_step_moderation_checks') as step: next(workflow) @@ -247,11 +251,11 @@ class TestSubscriptionWorkflow(unittest.TestCase): # The subscription policy requires user confirmation and moderation, # but their subscription is pre-confirmed. self._mlist.subscription_policy = ( - SubscriptionPolicy.confirm_then_moderate) + ConfirmModerationSubscriptionPolicy) anne = self._user_manager.create_address(self._anne) - workflow = SubscriptionWorkflow(self._mlist, anne, - pre_verified=True, - pre_confirmed=True) + workflow = self._mlist.subscription_policy(self._mlist, anne, + pre_verified=True, + pre_confirmed=True) workflow.run_thru('confirmation_checks') with patch.object(workflow, '_step_moderation_checks') as step: next(workflow) @@ -260,9 +264,10 @@ class TestSubscriptionWorkflow(unittest.TestCase): def test_confirmation_checks_confirmation_needed(self): # The subscription policy requires confirmation and the subscription # is not pre-confirmed. - self._mlist.subscription_policy = SubscriptionPolicy.confirm + self._mlist.subscription_policy = ConfirmSubscriptionPolicy anne = self._user_manager.create_address(self._anne) - workflow = SubscriptionWorkflow(self._mlist, anne, pre_verified=True) + workflow = self._mlist.subscription_policy(self._mlist, anne, + pre_verified=True) workflow.run_thru('confirmation_checks') with patch.object(workflow, '_step_send_confirmation') as step: next(workflow) @@ -272,9 +277,10 @@ class TestSubscriptionWorkflow(unittest.TestCase): # The subscription policy requires confirmation and moderation, and the # subscription is not pre-confirmed. self._mlist.subscription_policy = ( - SubscriptionPolicy.confirm_then_moderate) + ConfirmModerationSubscriptionPolicy) anne = self._user_manager.create_address(self._anne) - workflow = SubscriptionWorkflow(self._mlist, anne, pre_verified=True) + workflow = self._mlist.subscription_policy(self._mlist, anne, + pre_verified=True) workflow.run_thru('confirmation_checks') with patch.object(workflow, '_step_send_confirmation') as step: next(workflow) @@ -282,11 +288,11 @@ class TestSubscriptionWorkflow(unittest.TestCase): def test_moderation_checks_pre_approved(self): # The subscription is pre-approved by the moderator. - self._mlist.subscription_policy = SubscriptionPolicy.moderate + self._mlist.subscription_policy = ModerationSubscriptionPolicy anne = self._user_manager.create_address(self._anne) - workflow = SubscriptionWorkflow(self._mlist, anne, - pre_verified=True, - pre_approved=True) + workflow = self._mlist.subscription_policy(self._mlist, anne, + pre_verified=True, + pre_approved=True) workflow.run_thru('moderation_checks') with patch.object(workflow, '_step_do_subscription') as step: next(workflow) @@ -294,9 +300,10 @@ class TestSubscriptionWorkflow(unittest.TestCase): def test_moderation_checks_approval_required(self): # The moderator must approve the subscription. - self._mlist.subscription_policy = SubscriptionPolicy.moderate + self._mlist.subscription_policy = ModerationSubscriptionPolicy anne = self._user_manager.create_address(self._anne) - workflow = SubscriptionWorkflow(self._mlist, anne, pre_verified=True) + workflow = self._mlist.subscription_policy(self._mlist, anne, + pre_verified=True) workflow.run_thru('moderation_checks') with patch.object(workflow, '_step_get_moderator_approval') as step: next(workflow) @@ -306,9 +313,10 @@ class TestSubscriptionWorkflow(unittest.TestCase): # An open subscription policy plus a pre-verified address means the # user gets subscribed to the mailing list without any further # confirmations or approvals. - self._mlist.subscription_policy = SubscriptionPolicy.open + self._mlist.subscription_policy = OpenSubscriptionPolicy anne = self._user_manager.create_address(self._anne) - workflow = SubscriptionWorkflow(self._mlist, anne, pre_verified=True) + workflow = self._mlist.subscription_policy(self._mlist, anne, + pre_verified=True) # Consume the entire state machine. list(workflow) # Anne is now a member of the mailing list. @@ -323,11 +331,11 @@ class TestSubscriptionWorkflow(unittest.TestCase): # An moderation-requiring subscription policy plus a pre-verified and # pre-approved address means the user gets subscribed to the mailing # list without any further confirmations or approvals. - self._mlist.subscription_policy = SubscriptionPolicy.moderate + self._mlist.subscription_policy = ModerationSubscriptionPolicy anne = self._user_manager.create_address(self._anne) - workflow = SubscriptionWorkflow(self._mlist, anne, - pre_verified=True, - pre_approved=True) + workflow = self._mlist.subscription_policy(self._mlist, anne, + pre_verified=True, + pre_approved=True) # Consume the entire state machine. list(workflow) # Anne is now a member of the mailing list. @@ -343,12 +351,12 @@ class TestSubscriptionWorkflow(unittest.TestCase): # pre-approved address means the user gets subscribed to the mailing # list without any further confirmations or approvals. self._mlist.subscription_policy = ( - SubscriptionPolicy.confirm_then_moderate) + ConfirmModerationSubscriptionPolicy) anne = self._user_manager.create_address(self._anne) - workflow = SubscriptionWorkflow(self._mlist, anne, - pre_verified=True, - pre_confirmed=True, - pre_approved=True) + workflow = self._mlist.subscription_policy(self._mlist, anne, + pre_verified=True, + pre_confirmed=True, + pre_approved=True) # Consume the entire state machine. list(workflow) # Anne is now a member of the mailing list. @@ -362,12 +370,10 @@ class TestSubscriptionWorkflow(unittest.TestCase): def test_do_subscription_cleanups(self): # Once the user is subscribed, the token, and its associated pending # database record will be removed from the database. - self._mlist.subscription_policy = SubscriptionPolicy.open + self._mlist.subscription_policy = OpenSubscriptionPolicy anne = self._user_manager.create_address(self._anne) - workflow = SubscriptionWorkflow(self._mlist, anne, - pre_verified=True, - pre_confirmed=True, - pre_approved=True) + workflow = self._mlist.subscription_policy(self._mlist, anne, + pre_verified=True) # Consume the entire state machine. list(workflow) # Anne is now a member of the mailing list. @@ -382,11 +388,10 @@ class TestSubscriptionWorkflow(unittest.TestCase): # 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 subscribed. - self._mlist.subscription_policy = SubscriptionPolicy.moderate + self._mlist.subscription_policy = ModerationSubscriptionPolicy anne = self._user_manager.create_address(self._anne) - workflow = SubscriptionWorkflow(self._mlist, anne, - pre_verified=True, - pre_confirmed=True) + workflow = self._mlist.subscription_policy(self._mlist, anne, + pre_verified=True) # Consume the entire state machine. list(workflow) # The user is not currently subscribed to the mailing list. @@ -399,7 +404,7 @@ class TestSubscriptionWorkflow(unittest.TestCase): # Create a new workflow with the previous workflow's save token, and # restore its state. This models an approved subscription and should # result in the user getting subscribed. - approved_workflow = SubscriptionWorkflow(self._mlist) + approved_workflow = self._mlist.subscription_policy(self._mlist) approved_workflow.token = workflow.token approved_workflow.restore() list(approved_workflow) @@ -415,11 +420,10 @@ class TestSubscriptionWorkflow(unittest.TestCase): # When the subscription is held for moderator approval, a message is # logged. mark = LogFileMark('mailman.subscribe') - self._mlist.subscription_policy = SubscriptionPolicy.moderate + self._mlist.subscription_policy = ModerationSubscriptionPolicy anne = self._user_manager.create_address(self._anne) - workflow = SubscriptionWorkflow(self._mlist, anne, - pre_verified=True, - pre_confirmed=True) + workflow = self._mlist.subscription_policy(self._mlist, anne, + pre_verified=True) # Consume the entire state machine. list(workflow) self.assertIn( @@ -434,14 +438,13 @@ class TestSubscriptionWorkflow(unittest.TestCase): # When the subscription 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.subscription_policy = SubscriptionPolicy.moderate + self._mlist.subscription_policy = ModerationSubscriptionPolicy anne = self._user_manager.create_address(self._anne) bart = self._user_manager.create_user('bart@example.com', 'Bart User') address = set_preferred(bart) self._mlist.subscribe(address, MemberRole.moderator) - workflow = SubscriptionWorkflow(self._mlist, anne, - pre_verified=True, - pre_confirmed=True) + workflow = self._mlist.subscription_policy(self._mlist, anne, + pre_verified=True) # Consume the entire state machine. list(workflow) # Find the moderator message. @@ -452,13 +455,13 @@ class TestSubscriptionWorkflow(unittest.TestCase): else: raise AssertionError('No moderator email found') self.assertEqual( - item.msgdata['recipients'], {'test-owner@example.com'}) + item.msgdata['recipients'], {'test-owner@example.com'}) 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 subscription request to Test from anne@example.com') + message['Subject'], + 'New subscription request to Test from anne@example.com') self.assertEqual(message.get_payload(), """\ Your authorization is required for a mailing list subscription request approval: @@ -474,11 +477,10 @@ approval: # When the subscription 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.subscription_policy = SubscriptionPolicy.moderate + self._mlist.subscription_policy = ModerationSubscriptionPolicy anne = self._user_manager.create_address(self._anne) - workflow = SubscriptionWorkflow(self._mlist, anne, - pre_verified=True, - pre_confirmed=True) + workflow = self._mlist.subscription_policy(self._mlist, anne, + pre_verified=True) # Consume the entire state machine. list(workflow) get_queue_messages('virgin', expected_count=0) @@ -491,14 +493,14 @@ approval: anne = self._user_manager.create_address(self._anne) self.assertIsNone(anne.verified_on) # Run the workflow to model the confirmation step. - workflow = SubscriptionWorkflow(self._mlist, anne) + workflow = self._mlist.subscription_policy(self._mlist, anne) list(workflow) items = get_queue_messages('virgin', expected_count=1) message = items[0].msg token = workflow.token self.assertEqual(message['Subject'], 'confirm {}'.format(token)) self.assertEqual( - message['From'], 'test-confirm+{}@example.com'.format(token)) + 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 @@ -511,15 +513,16 @@ approval: anne = self._user_manager.create_address(self._anne) self.assertIsNone(anne.verified_on) # Run the workflow to model the confirmation step. - workflow = SubscriptionWorkflow(self._mlist, anne, pre_confirmed=True) + workflow = self._mlist.subscription_policy(self._mlist, anne, + pre_confirmed=True) 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)) + message['Subject'], 'confirm {}'.format(workflow.token)) self.assertEqual( - message['From'], 'test-confirm+{}@example.com'.format(token)) + 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 @@ -527,19 +530,20 @@ approval: def test_send_confirmation_pre_verified(self): # A confirmation message gets sent even when the address is verified # when the subscription must be confirmed. - self._mlist.subscription_policy = SubscriptionPolicy.confirm + self._mlist.subscription_policy = ConfirmSubscriptionPolicy anne = self._user_manager.create_address(self._anne) self.assertIsNone(anne.verified_on) # Run the workflow to model the confirmation step. - workflow = SubscriptionWorkflow(self._mlist, anne, pre_verified=True) + workflow = self._mlist.subscription_policy(self._mlist, anne, + pre_verified=True) 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)) + message['Subject'], 'confirm {}'.format(workflow.token)) self.assertEqual( - message['From'], 'test-confirm+{}@example.com'.format(token)) + 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 @@ -551,11 +555,11 @@ approval: anne = self._user_manager.create_address(self._anne) self.assertIsNone(anne.verified_on) # Run the workflow to model the confirmation step. - workflow = SubscriptionWorkflow(self._mlist, anne) + workflow = self._mlist.subscription_policy(self._mlist, anne) list(workflow) # The address is still not verified. self.assertIsNone(anne.verified_on) - confirm_workflow = SubscriptionWorkflow(self._mlist) + confirm_workflow = self._mlist.subscription_policy(self._mlist) confirm_workflow.token = workflow.token confirm_workflow.restore() confirm_workflow.run_thru('do_confirm_verify') @@ -569,11 +573,11 @@ approval: set_preferred(anne) # Run the workflow to model the confirmation step. There is no # subscriber attribute yet. - workflow = SubscriptionWorkflow(self._mlist, anne) + workflow = self._mlist.subscription_policy(self._mlist, anne) list(workflow) self.assertEqual(workflow.subscriber, anne) # Do a confirmation workflow, which should now set the subscriber. - confirm_workflow = SubscriptionWorkflow(self._mlist) + confirm_workflow = self._mlist.subscription_policy(self._mlist) confirm_workflow.token = workflow.token confirm_workflow.restore() confirm_workflow.run_thru('do_confirm_verify') @@ -584,10 +588,10 @@ approval: # Subscriptions to the mailing list must be confirmed. Once that's # done, the user's address (which is not initially verified) gets # subscribed to the mailing list. - self._mlist.subscription_policy = SubscriptionPolicy.confirm + self._mlist.subscription_policy = ConfirmSubscriptionPolicy anne = self._user_manager.create_address(self._anne) self.assertIsNone(anne.verified_on) - workflow = SubscriptionWorkflow(self._mlist, anne) + workflow = self._mlist.subscription_policy(self._mlist, anne) list(workflow) # Anne is not yet a member. member = self._mlist.regular_members.get_member(self._anne) @@ -597,7 +601,7 @@ approval: self.assertIsNotNone(workflow.token) self.assertEqual(workflow.token_owner, TokenOwner.subscriber) # Confirm. - confirm_workflow = SubscriptionWorkflow(self._mlist) + confirm_workflow = self._mlist.subscription_policy(self._mlist) confirm_workflow.token = workflow.token confirm_workflow.restore() list(confirm_workflow) @@ -615,9 +619,10 @@ approval: # the user confirming their subscription, and then the moderator # approving it, that different tokens are used in these two cases. self._mlist.subscription_policy = ( - SubscriptionPolicy.confirm_then_moderate) + ConfirmModerationSubscriptionPolicy) anne = self._user_manager.create_address(self._anne) - workflow = SubscriptionWorkflow(self._mlist, anne, pre_verified=True) + workflow = self._mlist.subscription_policy(self._mlist, anne, + pre_verified=True) # Run the state machine up to the first confirmation, and cache the # confirmation token. list(workflow) @@ -630,7 +635,7 @@ approval: self.assertIsNotNone(workflow.token) self.assertEqual(workflow.token_owner, TokenOwner.subscriber) # The old token will not work for moderator approval. - moderator_workflow = SubscriptionWorkflow(self._mlist) + moderator_workflow = self._mlist.subscription_policy(self._mlist) moderator_workflow.token = token moderator_workflow.restore() list(moderator_workflow) @@ -641,7 +646,7 @@ approval: # that there's a new token for the next steps. self.assertNotEqual(token, moderator_workflow.token) # The old token won't work. - final_workflow = SubscriptionWorkflow(self._mlist) + final_workflow = self._mlist.subscription_policy(self._mlist) final_workflow.token = token self.assertRaises(LookupError, final_workflow.restore) # Running this workflow will fail. @@ -666,11 +671,11 @@ approval: 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.subscription_policy = SubscriptionPolicy.confirm + self._mlist.subscription_policy = ConfirmSubscriptionPolicy anne = self._user_manager.create_address(self._anne) - workflow = SubscriptionWorkflow( - self._mlist, anne, - pre_verified=True, pre_confirmed=True, pre_approved=True) + workflow = self._mlist.subscription_policy( + self._mlist, anne, + pre_verified=True, pre_confirmed=True) list(workflow) # Anne was subscribed. self.assertIsNone(workflow.token) @@ -680,17 +685,18 @@ approval: def test_restore_user_absorbed(self): # The subscribing user is absorbed (and thus deleted) before the # moderator approves the subscription. - self._mlist.subscription_policy = SubscriptionPolicy.moderate + self._mlist.subscription_policy = ModerationSubscriptionPolicy anne = self._user_manager.create_user(self._anne) bill = self._user_manager.create_user('bill@example.com') set_preferred(bill) # anne subscribes. - workflow = SubscriptionWorkflow(self._mlist, anne, pre_verified=True) + workflow = self._mlist.subscription_policy(self._mlist, anne, + pre_verified=True) list(workflow) # bill absorbs anne. bill.absorb(anne) # anne's subscription request is approved. - approved_workflow = SubscriptionWorkflow(self._mlist) + approved_workflow = self._mlist.subscription_policy(self._mlist) approved_workflow.token = workflow.token approved_workflow.restore() self.assertEqual(approved_workflow.user, bill) @@ -700,19 +706,19 @@ approval: def test_restore_address_absorbed(self): # The subscribing user is absorbed (and thus deleted) before the # moderator approves the subscription. - self._mlist.subscription_policy = SubscriptionPolicy.moderate + self._mlist.subscription_policy = ModerationSubscriptionPolicy anne = self._user_manager.create_user(self._anne) anne_address = anne.addresses[0] bill = self._user_manager.create_user('bill@example.com') # anne subscribes. - workflow = SubscriptionWorkflow( - self._mlist, anne_address, pre_verified=True) + workflow = self._mlist.subscription_policy( + self._mlist, anne_address, pre_verified=True) list(workflow) # bill absorbs anne. bill.absorb(anne) self.assertIn(anne_address, bill.addresses) # anne's subscription request is approved. - approved_workflow = SubscriptionWorkflow(self._mlist) + approved_workflow = self._mlist.subscription_policy(self._mlist) approved_workflow.token = workflow.token approved_workflow.restore() self.assertEqual(approved_workflow.user, bill) diff --git a/src/mailman/app/tests/test_unsubscriptions.py b/src/mailman/app/tests/test_unsubscriptions.py index 4ca657190..2e210e90b 100644 --- a/src/mailman/app/tests/test_unsubscriptions.py +++ b/src/mailman/app/tests/test_unsubscriptions.py @@ -21,14 +21,15 @@ 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 mailman.workflows.unsubscription import ( + ConfirmModerationUnsubscriptionPolicy, ConfirmUnsubscriptionPolicy, + ModerationUnsubscriptionPolicy, OpenUnsubscriptionPolicy) from unittest.mock import patch from zope.component import getUtility @@ -40,7 +41,7 @@ class TestUnSubscriptionWorkflow(unittest.TestCase): def setUp(self): self._mlist = create_list('test@example.com') self._mlist.admin_immed_notify = False - self._mlist.unsubscription_policy = SubscriptionPolicy.open + self._mlist.unsubscription_policy = OpenUnsubscriptionPolicy self._mlist.send_welcome_message = False self._anne = 'anne@example.com' self._user_manager = getUtility(IUserManager) @@ -58,7 +59,7 @@ class TestUnSubscriptionWorkflow(unittest.TestCase): def test_start_state(self): # Test the workflow starts with no tokens or members. - workflow = UnSubscriptionWorkflow(self._mlist) + workflow = self._mlist.unsubscription_policy(self._mlist) self.assertEqual(workflow.token_owner, TokenOwner.no_one) self.assertIsNone(workflow.token) self.assertIsNone(workflow.member) @@ -67,8 +68,8 @@ class TestUnSubscriptionWorkflow(unittest.TestCase): # 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) + self._mlist.unsubscription_policy = ConfirmUnsubscriptionPolicy + workflow = self._mlist.unsubscription_policy(self._mlist, self.anne) with suppress(StopIteration): workflow.run_thru('send_confirmation') self.assertIsNotNone(workflow.token) @@ -84,23 +85,23 @@ class TestUnSubscriptionWorkflow(unittest.TestCase): 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) + workflow = OpenUnsubscriptionPolicy(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) + workflow = self._mlist.unsubscription_policy(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') + self._mlist.unsubscription_policy = OpenUnsubscriptionPolicy + workflow = self._mlist.unsubscription_policy(self._mlist, self.anne) + workflow.run_thru('subscription_checks') with patch.object(workflow, '_step_do_unsubscription') as step: next(workflow) step.assert_called_once_with() @@ -108,10 +109,9 @@ class TestUnSubscriptionWorkflow(unittest.TestCase): 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') + self._mlist.unsubscription_policy = ModerationUnsubscriptionPolicy + workflow = self._mlist.unsubscription_policy(self._mlist, self.anne) + workflow.run_thru('subscription_checks') with patch.object(workflow, '_step_moderation_checks') as step: next(workflow) step.assert_called_once_with() @@ -120,8 +120,8 @@ class TestUnSubscriptionWorkflow(unittest.TestCase): # 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.unsubscription_policy = ConfirmUnsubscriptionPolicy + workflow = self._mlist.unsubscription_policy( self._mlist, self.anne, pre_confirmed=True) workflow.run_thru('confirmation_checks') with patch.object(workflow, '_step_do_unsubscription') as step: @@ -133,19 +133,19 @@ class TestUnSubscriptionWorkflow(unittest.TestCase): # unsubscription is pre-confirmed. Since moderation is required, that # check will be performed. self._mlist.unsubscription_policy = ( - SubscriptionPolicy.confirm_then_moderate) - workflow = UnSubscriptionWorkflow( + ConfirmModerationUnsubscriptionPolicy) + workflow = self._mlist.unsubscription_policy( self._mlist, self.anne, pre_confirmed=True) workflow.run_thru('confirmation_checks') - with patch.object(workflow, '_step_do_unsubscription') as step: + with patch.object(workflow, '_step_moderation_checks') 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) + self._mlist.unsubscription_policy = ConfirmUnsubscriptionPolicy + workflow = self._mlist.unsubscription_policy(self._mlist, self.anne) workflow.run_thru('confirmation_checks') with patch.object(workflow, '_step_send_confirmation') as step: next(workflow) @@ -153,17 +153,17 @@ class TestUnSubscriptionWorkflow(unittest.TestCase): 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') + self._mlist.unsubscription_policy = ModerationUnsubscriptionPolicy + workflow = self._mlist.unsubscription_policy(self._mlist, self.anne) + workflow.run_thru('subscription_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) + self._mlist.unsubscription_policy = ModerationUnsubscriptionPolicy + workflow = self._mlist.unsubscription_policy(self._mlist, self.anne) workflow.run_thru('moderation_checks') with patch.object(workflow, '_step_get_moderator_approval') as step: next(workflow) @@ -172,8 +172,8 @@ class TestUnSubscriptionWorkflow(unittest.TestCase): 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) + self._mlist.unsubscription_policy = OpenUnsubscriptionPolicy + workflow = self._mlist.unsubscription_policy(self._mlist, self.anne) list(workflow) member = self._mlist.regular_members.get_member(self._anne) self.assertIsNone(member) @@ -182,9 +182,9 @@ class TestUnSubscriptionWorkflow(unittest.TestCase): # 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) + self._mlist.unsubscription_policy = ModerationUnsubscriptionPolicy + workflow = self._mlist.unsubscription_policy(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) @@ -198,10 +198,10 @@ class TestUnSubscriptionWorkflow(unittest.TestCase): # 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) + ConfirmModerationUnsubscriptionPolicy) + workflow = self._mlist.unsubscription_policy(self._mlist, self.anne, + pre_approved=True, + pre_confirmed=True) list(workflow) member = self._mlist.regular_members.get_member(self._anne) self.assertIsNone(member) @@ -212,10 +212,8 @@ class TestUnSubscriptionWorkflow(unittest.TestCase): 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) + self._mlist.unsubscription_policy = OpenUnsubscriptionPolicy + workflow = self._mlist.unsubscription_policy(self._mlist, self.anne) # Run the workflow. list(workflow) # Anne is now unsubscribed from the list. @@ -229,9 +227,9 @@ class TestUnSubscriptionWorkflow(unittest.TestCase): # 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) + self._mlist.unsubscription_policy = ModerationUnsubscriptionPolicy + workflow = self._mlist.unsubscription_policy( + self._mlist, self.anne) # Run the entire workflow. list(workflow) # The user is currently subscribed to the mailing list. @@ -244,7 +242,7 @@ class TestUnSubscriptionWorkflow(unittest.TestCase): # 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 = self._mlist.unsubscription_policy(self._mlist) approved_workflow.token = workflow.token approved_workflow.restore() list(approved_workflow) @@ -260,9 +258,9 @@ class TestUnSubscriptionWorkflow(unittest.TestCase): # 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) + self._mlist.unsubscription_policy = ModerationUnsubscriptionPolicy + workflow = self._mlist.unsubscription_policy( + self._mlist, self.anne) # Run the entire workflow. list(workflow) self.assertIn( @@ -277,9 +275,9 @@ class TestUnSubscriptionWorkflow(unittest.TestCase): # 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) + self._mlist.unsubscription_policy = ModerationUnsubscriptionPolicy + workflow = self._mlist.unsubscription_policy( + self._mlist, self.anne) # Consume the entire state machine. list(workflow) items = get_queue_messages('virgin', expected_count=1) @@ -305,9 +303,9 @@ request approval: # 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) + self._mlist.unsubscription_policy = ModerationUnsubscriptionPolicy + workflow = self._mlist.unsubscription_policy( + self._mlist, self.anne) # Consume the entire state machine. list(workflow) get_queue_messages('virgin', expected_count=0) @@ -318,9 +316,9 @@ request approval: def test_send_confirmation(self): # A confirmation message gets sent when the unsubscription must be # confirmed. - self._mlist.unsubscription_policy = SubscriptionPolicy.confirm + self._mlist.unsubscription_policy = ConfirmUnsubscriptionPolicy # Run the workflow to model the confirmation step. - workflow = UnSubscriptionWorkflow(self._mlist, self.anne) + workflow = self._mlist.unsubscription_policy(self._mlist, self.anne) list(workflow) items = get_queue_messages('virgin', expected_count=1) message = items[0].msg @@ -336,8 +334,8 @@ request approval: 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) + self._mlist.unsubscription_policy = ConfirmUnsubscriptionPolicy + workflow = self._mlist.unsubscription_policy(self._mlist, self.anne) list(workflow) # Anne is a member. member = self._mlist.regular_members.get_member(self._anne) @@ -347,7 +345,7 @@ request approval: self.assertIsNotNone(workflow.token) self.assertEqual(workflow.token_owner, TokenOwner.subscriber) # Confirm. - confirm_workflow = UnSubscriptionWorkflow(self._mlist) + confirm_workflow = self._mlist.unsubscription_policy(self._mlist) confirm_workflow.token = workflow.token confirm_workflow.restore() list(confirm_workflow) @@ -363,8 +361,8 @@ request approval: # 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) + self._mlist.unsubscription_policy = ConfirmUnsubscriptionPolicy + workflow = self._mlist.unsubscription_policy(self._mlist, address) list(workflow) # Bart is a member. member = self._mlist.regular_members.get_member( @@ -375,7 +373,7 @@ request approval: self.assertIsNotNone(workflow.token) self.assertEqual(workflow.token_owner, TokenOwner.subscriber) # Confirm. - confirm_workflow = UnSubscriptionWorkflow(self._mlist) + confirm_workflow = self._mlist.unsubscription_policy(self._mlist) confirm_workflow.token = workflow.token confirm_workflow.restore() list(confirm_workflow) @@ -390,8 +388,8 @@ request approval: 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) + self._mlist.unsubscription_policy = ConfirmUnsubscriptionPolicy + workflow = self._mlist.unsubscription_policy(self._mlist, self.anne) list(workflow) # Anne is a member. member = self._mlist.regular_members.get_member(self._anne) @@ -403,7 +401,7 @@ request approval: # Unsubscribe Anne out of band. member.unsubscribe() # Confirm. - confirm_workflow = UnSubscriptionWorkflow(self._mlist) + confirm_workflow = self._mlist.unsubscription_policy(self._mlist) confirm_workflow.token = workflow.token confirm_workflow.restore() list(confirm_workflow) @@ -414,8 +412,8 @@ request approval: 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) + self._mlist.unsubscription_policy = ConfirmUnsubscriptionPolicy + workflow = self._mlist.unsubscription_policy(self._mlist, self.anne) list(workflow) # Anne is a member. member = self._mlist.regular_members.get_member(self._anne) @@ -425,7 +423,7 @@ request approval: self.assertIsNotNone(workflow.token) self.assertEqual(workflow.token_owner, TokenOwner.subscriber) # Confirm. - confirm_workflow = UnSubscriptionWorkflow(self._mlist) + confirm_workflow = self._mlist.unsubscription_policy(self._mlist) confirm_workflow.token = workflow.token confirm_workflow.restore() confirm_workflow.run_until('do_unsubscription') @@ -443,8 +441,8 @@ request approval: # 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) + ConfirmModerationUnsubscriptionPolicy) + workflow = self._mlist.unsubscription_policy(self._mlist, self.anne) # Run the state machine up to the first confirmation, and cache the # confirmation token. list(workflow) @@ -457,7 +455,7 @@ request approval: 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 = self._mlist.unsubscription_policy(self._mlist) moderator_workflow.token = token moderator_workflow.restore() list(moderator_workflow) @@ -468,7 +466,7 @@ request approval: # 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 = self._mlist.unsubscription_policy(self._mlist) final_workflow.token = token self.assertRaises(LookupError, final_workflow.restore) # Running this workflow will fail. @@ -492,9 +490,9 @@ request approval: 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) + self._mlist.unsubscription_policy = ConfirmUnsubscriptionPolicy + workflow = self._mlist.unsubscription_policy( + self._mlist, self.anne, pre_confirmed=True) list(workflow) # Anne was unsubscribed. self.assertIsNone(workflow.token) @@ -504,11 +502,11 @@ request approval: 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) + self._mlist.unsubscription_policy = ModerationUnsubscriptionPolicy + workflow = self._mlist.unsubscription_policy(self._mlist, address) # Get moderator approval. list(workflow) - approved_workflow = UnSubscriptionWorkflow(self._mlist) + approved_workflow = self._mlist.unsubscription_policy(self._mlist) approved_workflow.token = workflow.token approved_workflow.restore() list(approved_workflow) diff --git a/src/mailman/app/tests/test_workflowmanager.py b/src/mailman/app/tests/test_workflowmanager.py index 38ed2e468..e21e428f6 100644 --- a/src/mailman/app/tests/test_workflowmanager.py +++ b/src/mailman/app/tests/test_workflowmanager.py @@ -20,7 +20,6 @@ import unittest 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.subscriptions import ISubscriptionManager, TokenOwner @@ -28,6 +27,9 @@ from mailman.interfaces.usermanager import IUserManager from mailman.testing.helpers import get_queue_messages from mailman.testing.layers import ConfigLayer from mailman.utilities.datetime import now +from mailman.workflows.subscription import ( + ConfirmModerationSubscriptionPolicy, ConfirmSubscriptionPolicy, + ModerationSubscriptionPolicy, OpenSubscriptionPolicy) from zope.component import getUtility @@ -60,7 +62,7 @@ class TestRegistrar(unittest.TestCase): # Registering a subscription request where no confirmation or # moderation steps are needed, leaves us with no token or owner, since # there's nothing more to do. - self._mlist.subscription_policy = SubscriptionPolicy.open + self._mlist.subscription_policy = OpenSubscriptionPolicy self._anne.verified_on = now() token, token_owner, rmember = self._registrar.register(self._anne) self.assertIsNone(token) @@ -82,7 +84,7 @@ class TestRegistrar(unittest.TestCase): # (because she does not have a verified address), but not the moderator # to approve. Running the workflow gives us a token. Confirming the # token subscribes the user. - self._mlist.subscription_policy = SubscriptionPolicy.open + self._mlist.subscription_policy = OpenSubscriptionPolicy token, token_owner, rmember = self._registrar.register(self._anne) self.assertIsNotNone(token) self.assertEqual(token_owner, TokenOwner.subscriber) @@ -102,7 +104,7 @@ class TestRegistrar(unittest.TestCase): # (because of list policy), but not the moderator to approve. Running # the workflow gives us a token. Confirming the token subscribes the # user. - self._mlist.subscription_policy = SubscriptionPolicy.confirm + self._mlist.subscription_policy = ConfirmSubscriptionPolicy self._anne.verified_on = now() token, token_owner, rmember = self._registrar.register(self._anne) self.assertIsNotNone(token) @@ -122,7 +124,7 @@ class TestRegistrar(unittest.TestCase): # We have a subscription request which requires the moderator to # approve. Running the workflow gives us a token. Confirming the # token subscribes the user. - self._mlist.subscription_policy = SubscriptionPolicy.moderate + self._mlist.subscription_policy = ModerationSubscriptionPolicy self._anne.verified_on = now() token, token_owner, rmember = self._registrar.register(self._anne) self.assertIsNotNone(token) @@ -145,7 +147,7 @@ class TestRegistrar(unittest.TestCase): # token runs the workflow a little farther, but still gives us a # token. Confirming again subscribes the user. self._mlist.subscription_policy = ( - SubscriptionPolicy.confirm_then_moderate) + ConfirmModerationSubscriptionPolicy) self._anne.verified_on = now() # Runs until subscription confirmation. token, token_owner, rmember = self._registrar.register(self._anne) @@ -180,7 +182,7 @@ class TestRegistrar(unittest.TestCase): # sees when they approve the subscription. This prevents the user # from using a replay attack to subvert moderator approval. self._mlist.subscription_policy = ( - SubscriptionPolicy.confirm_then_moderate) + ConfirmModerationSubscriptionPolicy) self._anne.verified_on = now() # Runs until subscription confirmation. token, token_owner, rmember = self._registrar.register(self._anne) @@ -216,7 +218,7 @@ class TestRegistrar(unittest.TestCase): def test_discard_waiting_for_confirmation(self): # While waiting for a user to confirm their subscription, we discard # the workflow. - self._mlist.subscription_policy = SubscriptionPolicy.confirm + self._mlist.subscription_policy = ConfirmSubscriptionPolicy self._anne.verified_on = now() # Runs until subscription confirmation. token, token_owner, rmember = self._registrar.register(self._anne) @@ -233,7 +235,7 @@ class TestRegistrar(unittest.TestCase): def test_admin_notify_mchanges(self): # When a user gets subscribed via the subscription policy workflow, # the list administrators get an email notification. - self._mlist.subscription_policy = SubscriptionPolicy.open + self._mlist.subscription_policy = OpenSubscriptionPolicy self._mlist.admin_notify_mchanges = True self._mlist.send_welcome_message = False token, token_owner, member = self._registrar.register( @@ -253,7 +255,7 @@ anne@example.com has been successfully subscribed to Ant. # Even when a user gets subscribed via the subscription policy # workflow, the list administrators won't get an email notification if # they don't want one. - self._mlist.subscription_policy = SubscriptionPolicy.open + self._mlist.subscription_policy = OpenSubscriptionPolicy self._mlist.admin_notify_mchanges = False self._mlist.send_welcome_message = False # Bart is an administrator of the mailing list. diff --git a/src/mailman/commands/docs/membership.rst b/src/mailman/commands/docs/membership.rst index 18c473c5b..e1266d690 100644 --- a/src/mailman/commands/docs/membership.rst +++ b/src/mailman/commands/docs/membership.rst @@ -221,9 +221,9 @@ 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 - >>> mlist.unsubscription_policy = SubscriptionPolicy.open + >>> from mailman.workflows.unsubscription import OpenUnsubscriptionPolicy + >>> mlist_2.unsubscription_policy = OpenUnsubscriptionPolicy + >>> mlist.unsubscription_policy = OpenUnsubscriptionPolicy >>> results = Results() >>> print(leave.process(mlist_2, msg, {}, (), results)) ContinueProcessing.yes diff --git a/src/mailman/commands/tests/test_eml_confirm.py b/src/mailman/commands/tests/test_eml_confirm.py index 1ef392b76..d17a084f7 100644 --- a/src/mailman/commands/tests/test_eml_confirm.py +++ b/src/mailman/commands/tests/test_eml_confirm.py @@ -24,7 +24,6 @@ from mailman.commands.eml_confirm import Confirm 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.subscriptions import ISubscriptionManager from mailman.interfaces.usermanager import IUserManager from mailman.runners.command import CommandRunner, Results @@ -32,6 +31,7 @@ from mailman.testing.helpers import ( get_queue_messages, make_testable_runner, specialized_message_from_string as mfs, subscribe) from mailman.testing.layers import ConfigLayer +from mailman.workflows.subscription import ConfirmModerationSubscriptionPolicy from zope.component import getUtility @@ -109,7 +109,7 @@ class TestEmailResponses(unittest.TestCase): def test_confirm_then_moderate_workflow(self): # Issue #114 describes a problem when confirming the moderation email. self._mlist.subscription_policy = ( - SubscriptionPolicy.confirm_then_moderate) + ConfirmModerationSubscriptionPolicy) bart = getUtility(IUserManager).create_address( 'bart@example.com', 'Bart Person') # Clear any previously queued confirmation messages. diff --git a/src/mailman/commands/tests/test_eml_membership.py b/src/mailman/commands/tests/test_eml_membership.py index 6cf4802c6..d90e3319e 100644 --- a/src/mailman/commands/tests/test_eml_membership.py +++ b/src/mailman/commands/tests/test_eml_membership.py @@ -22,11 +22,11 @@ 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 mailman.workflows.unsubscription import ConfirmUnsubscriptionPolicy from zope.component import getUtility @@ -38,7 +38,7 @@ class TestLeave(unittest.TestCase): self._command = Leave() def test_confirm_leave_not_a_member(self): - self._mlist.unsubscription_policy = SubscriptionPolicy.confirm + self._mlist.unsubscription_policy = ConfirmUnsubscriptionPolicy # 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. diff --git a/src/mailman/database/alembic/versions/ccb9e28c44f4_mailinglist_sub_unsub_policies.py b/src/mailman/database/alembic/versions/ccb9e28c44f4_mailinglist_sub_unsub_policies.py new file mode 100644 index 000000000..3eab48de0 --- /dev/null +++ b/src/mailman/database/alembic/versions/ccb9e28c44f4_mailinglist_sub_unsub_policies.py @@ -0,0 +1,141 @@ +# Copyright (C) 2015-2017 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/>. + +"""Change the subscription_policy and unsubscription_policy attribute type. + +Revision ID: ccb9e28c44f4 +Revises: 7c5b39d1ecc4 +Create Date: 2017-07-05 00:02:18.238155 + +Changes the [un]subscription_policy MailingList attributes to a string keyä +of the workflow name from an Enum. +""" + +import sqlalchemy as sa + +from alembic import op + +from mailman.database.types import SAUnicode +from mailman.interfaces.mailinglist import SubscriptionPolicy +from mailman.workflows.subscription import ( + ConfirmModerationSubscriptionPolicy, ConfirmSubscriptionPolicy, + ModerationSubscriptionPolicy, OpenSubscriptionPolicy) +from mailman.workflows.unsubscription import ( + ConfirmModerationUnsubscriptionPolicy, ConfirmUnsubscriptionPolicy, + ModerationUnsubscriptionPolicy, OpenUnsubscriptionPolicy) + +# revision identifiers, used by Alembic. +revision = 'ccb9e28c44f4' +down_revision = '7c5b39d1ecc4' + + +mlist_table = sa.sql.table( + 'mailinglist', + sa.sql.column('id', sa.Integer), + sa.sql.column('old_subscription_policy', sa.Integer), + sa.sql.column('old_unsubscription_policy', sa.Integer), + sa.sql.column('subscription_policy', SAUnicode), + sa.sql.column('unsubscription_policy', SAUnicode) + ) + +sub_map = { + SubscriptionPolicy.open.value: OpenSubscriptionPolicy.name, + SubscriptionPolicy.confirm.value: ConfirmSubscriptionPolicy.name, + SubscriptionPolicy.moderate.value: ModerationSubscriptionPolicy.name, + SubscriptionPolicy.confirm_then_moderate.value: + ConfirmModerationSubscriptionPolicy.name + } + +sub_inv_map = {v: k for k, v in sub_map.items()} + +unsub_map = { + SubscriptionPolicy.open.value: OpenUnsubscriptionPolicy.name, + SubscriptionPolicy.confirm.value: ConfirmUnsubscriptionPolicy.name, + SubscriptionPolicy.moderate.value: ModerationUnsubscriptionPolicy.name, + SubscriptionPolicy.confirm_then_moderate.value: + ConfirmModerationUnsubscriptionPolicy.name + } + +unsub_inv_map = {v: k for k, v in unsub_map.items()} + + +def upgrade(): + with op.batch_alter_table('mailinglist') as batch_op: + batch_op.alter_column('subscription_policy', + new_column_name='old_subscription_policy', + existing_type=sa.Integer, existing_nullable=True) + batch_op.alter_column('unsubscription_policy', + new_column_name='old_unsubscription_policy', + existing_type=sa.Integer, existing_nullable=True) + + with op.batch_alter_table('mailinglist') as batch_op: + batch_op.add_column(sa.Column('subscription_policy', SAUnicode)) + batch_op.add_column(sa.Column('unsubscription_policy', SAUnicode)) + + connection = op.get_bind() + for (table_id, old_sub_policy, old_unsub_policy, sub_policy, + unsub_policy) in connection.execute(mlist_table.select()).fetchall(): + new_sub = sub_map.get(old_sub_policy, + ConfirmSubscriptionPolicy.name) + + new_unsub = unsub_map.get(old_unsub_policy, + ConfirmUnsubscriptionPolicy.name) + + connection.execute(mlist_table.update().where( + mlist_table.c.id == table_id).values( + subscription_policy=new_sub, + unsubscription_policy=new_unsub + )) + + with op.batch_alter_table('mailinglist') as batch_op: + batch_op.drop_column('old_subscription_policy') + batch_op.drop_column('old_unsubscription_policy') + + +def downgrade(): + with op.batch_alter_table('mailinglist') as batch_op: + batch_op.alter_column('subscription_policy', + new_column_name='old_subscription_policy', + existing_type=SAUnicode) + batch_op.alter_column('unsubscription_policy', + new_column_name='old_unsubscription_policy', + existing_type=SAUnicode) + + with op.batch_alter_table('mailinglist') as batch_op: + batch_op.add_column(sa.Column('subscription_policy', sa.Integer, + nullable=True)) + batch_op.add_column(sa.Column('unsubscription_policy', sa.Integer, + nullable=True)) + + connection = op.get_bind() + for (table_id, old_sub_policy, old_unsub_policy, sub_policy, + unsub_policy) in connection.execute(mlist_table.select()).fetchall(): + + new_sub = sub_inv_map.get(old_sub_policy, + SubscriptionPolicy.confirm.value) + new_unsub = unsub_inv_map.get(old_unsub_policy, + SubscriptionPolicy.confirm.value) + + connection.execute(mlist_table.update().where( + mlist_table.c.id == table_id).values( + subscription_policy=new_sub, + unsubscription_policy=new_unsub + )) + + with op.batch_alter_table('mailinglist') as batch_op: + batch_op.drop_column('old_subscription_policy') + batch_op.drop_column('old_unsubscription_policy') diff --git a/src/mailman/database/tests/test_migrations.py b/src/mailman/database/tests/test_migrations.py index 5b686d13b..104487350 100644 --- a/src/mailman/database/tests/test_migrations.py +++ b/src/mailman/database/tests/test_migrations.py @@ -32,10 +32,13 @@ from mailman.database.transaction import transaction from mailman.database.types import Enum, SAUnicode from mailman.interfaces.action import Action from mailman.interfaces.cache import ICacheManager +from mailman.interfaces.mailinglist import SubscriptionPolicy from mailman.interfaces.member import MemberRole from mailman.interfaces.template import ITemplateManager from mailman.interfaces.usermanager import IUserManager from mailman.testing.layers import ConfigLayer +from mailman.workflows.subscription import ModerationSubscriptionPolicy +from mailman.workflows.unsubscription import OpenUnsubscriptionPolicy from warnings import catch_warnings, simplefilter from zope.component import getUtility @@ -594,3 +597,54 @@ class TestMigrations(unittest.TestCase): self.assertEqual(token, 'abcde') self.assertEqual(step, None) self.assertEqual(data, 'another data') + + def test_ccb9e28c44f4_mailinglist_sub_unsub_policies_downgrade(self): + # Downgrade to the tested revision. + alembic.command.downgrade(alembic_cfg, 'ccb9e28c44f4') + # Create our example list. + with transaction(): + mlist = create_list('test@example.com') + mlist.subscription_policy = ModerationSubscriptionPolicy + mlist.unsubscription_policy = OpenUnsubscriptionPolicy + # Downgrade, should keep the policies, since they have a value in the + # SubscriptionPolicy enum. + alembic.command.downgrade(alembic_cfg, '7c5b39d1ecc4') + old_mlist_table = sa.sql.table( + 'mailinglist', + sa.sql.column('id', sa.Integer), + sa.sql.column('subscription_policy', sa.Integer), + sa.sql.column('unsubscription_policy', sa.Integer) + ) + table_id, sub_policy, unsub_policy = config.db.store.execute( + old_mlist_table.select()).fetchone() + self.assertEqual(sub_policy, SubscriptionPolicy.moderate.value) + self.assertEqual(unsub_policy, SubscriptionPolicy.open.value) + + def test_ccb9e28c44f4_mailinglist_sub_unsub_policies_upgrade(self): + old_mlist_table = sa.sql.table( + 'mailinglist', + sa.sql.column('id', sa.Integer), + sa.sql.column('subscription_policy', sa.Integer), + sa.sql.column('unsubscription_policy', sa.Integer) + ) + with transaction(): + # Downgrade to a revision below the one tested. + alembic.command.downgrade(alembic_cfg, '7c5b39d1ecc4') + + config.db.store.execute( + old_mlist_table.insert().values( + subscription_policy=SubscriptionPolicy.moderate.value, + unsubscription_policy=SubscriptionPolicy.open.value) + ) + # Upgrade and test that the new names and types are there. + alembic.command.upgrade(alembic_cfg, 'ccb9e28c44f4') + new_mlist_table = sa.sql.table( + 'mailinglist', + sa.sql.column('id', sa.Integer), + sa.sql.column('subscription_policy', SAUnicode), + sa.sql.column('unsubscription_policy', SAUnicode) + ) + table_id, sub_policy, unsub_policy = config.db.store.execute( + new_mlist_table.select()).fetchone() + self.assertEqual(sub_policy, ModerationSubscriptionPolicy.name) + self.assertEqual(unsub_policy, OpenUnsubscriptionPolicy.name) diff --git a/src/mailman/interfaces/subscriptions.py b/src/mailman/interfaces/subscriptions.py index 8de517c09..40c66cb1f 100644 --- a/src/mailman/interfaces/subscriptions.py +++ b/src/mailman/interfaces/subscriptions.py @@ -218,8 +218,7 @@ class ISubscriptionManager(Interface): To use this, adapt an ``IMailingList`` to this interface. """ - def register(subscriber=None, *, - pre_verified=False, pre_confirmed=False, pre_approved=False): + def register(subscriber=None, **kwargs): """Subscribe an address or user according to subscription policies. The mailing list's subscription policy is used to subscribe @@ -270,8 +269,7 @@ class ISubscriptionManager(Interface): appears in the global or list-centric bans. """ - def unregister(subscriber=None, *, - pre_confirmed=False, pre_approved=False): + def unregister(subscriber=None, **kwargs): """Unsubscribe an address or user according to subscription policies. The mailing list's unsubscription policy is used to unsubscribe diff --git a/src/mailman/model/docs/subscriptions.rst b/src/mailman/model/docs/subscriptions.rst index 1e1810a7a..e23236b3b 100644 --- a/src/mailman/model/docs/subscriptions.rst +++ b/src/mailman/model/docs/subscriptions.rst @@ -33,8 +33,8 @@ 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 + >>> from mailman.workflows.subscription import OpenSubscriptionPolicy + >>> mlist.subscription_policy = OpenSubscriptionPolicy Anne attempts to join the mailing list. A unique token is created which represents this work flow. @@ -81,7 +81,8 @@ Bart verifies his address and makes it his preferred address. 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 + >>> from mailman.workflows.subscription import ModerationSubscriptionPolicy + >>> mlist.subscription_policy = ModerationSubscriptionPolicy 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 @@ -118,7 +119,8 @@ interface, but with a different name. If the mailing list's unsubscription policy is open, unregistering the subscription takes effect immediately. - >>> mlist.unsubscription_policy = SubscriptionPolicy.open + >>> from mailman.workflows.unsubscription import OpenUnsubscriptionPolicy + >>> mlist.unsubscription_policy = OpenUnsubscriptionPolicy >>> token, token_owner, member = manager.unregister(anne) >>> print(mlist.members.get_member('anne@example.com')) None @@ -127,7 +129,8 @@ 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 + >>> from mailman.workflows.unsubscription import ConfirmUnsubscriptionPolicy + >>> mlist.unsubscription_policy = ConfirmUnsubscriptionPolicy >>> token, token_owner, member = manager.unregister(bart) Bart hasn't confirmed yet, so he's still a member of the list. diff --git a/src/mailman/model/mailinglist.py b/src/mailman/model/mailinglist.py index 2e50575d5..0764be0bb 100644 --- a/src/mailman/model/mailinglist.py +++ b/src/mailman/model/mailinglist.py @@ -34,13 +34,15 @@ from mailman.interfaces.languages import ILanguageManager from mailman.interfaces.mailinglist import ( DMARCMitigateAction, IAcceptableAlias, IAcceptableAliasSet, IHeaderMatch, IHeaderMatchList, IListArchiver, IListArchiverSet, - IMailingList, Personalization, ReplyToMunging, SubscriptionPolicy) + IMailingList, Personalization, ReplyToMunging) from mailman.interfaces.member import ( AlreadySubscribedError, MemberRole, MissingPreferredAddressError, SubscriptionEvent) from mailman.interfaces.mime import FilterType from mailman.interfaces.nntp import NewsgroupModeration from mailman.interfaces.user import IUser +from mailman.interfaces.workflows import (ISubscriptionWorkflow, + IUnsubscriptionWorkflow) from mailman.model import roster from mailman.model.digests import OneLastDigest from mailman.model.member import Member @@ -179,11 +181,11 @@ class MailingList(Model): send_goodbye_message = Column(Boolean) send_welcome_message = Column(Boolean) subject_prefix = Column(SAUnicode) - subscription_policy = Column(Enum(SubscriptionPolicy)) + _subscription_policy = Column('subscription_policy', SAUnicode) topics = Column(PickleType) topics_bodylines_limit = Column(Integer) topics_enabled = Column(Boolean) - unsubscription_policy = Column(Enum(SubscriptionPolicy)) + _unsubscription_policy = Column('unsubscription_policy', SAUnicode) # ORM relationships. header_matches = relationship( 'HeaderMatch', backref='mailing_list', @@ -494,6 +496,34 @@ class MailingList(Model): notify(SubscriptionEvent(self, member)) return member + @property + def subscription_policy(self): + return config.workflows[self._subscription_policy] + + @subscription_policy.setter + def subscription_policy(self, value): + if isinstance(value, str): + cls = config.workflows.get(value, None) + else: + cls = value + if (cls is None or not ISubscriptionWorkflow.implementedBy(cls)): + raise ValueError('Invalid subscription policy: {}'.format(value)) + self._subscription_policy = cls.name + + @property + def unsubscription_policy(self): + return config.workflows[self._unsubscription_policy] + + @unsubscription_policy.setter + def unsubscription_policy(self, value): + if isinstance(value, str): + cls = config.workflows.get(value, None) + else: + cls = value + if (cls is None or not IUnsubscriptionWorkflow.implementedBy(cls)): + raise ValueError('Invalid unsubscription policy: {}'.format(value)) + self._unsubscription_policy = cls.name + @public @implementer(IAcceptableAlias) diff --git a/src/mailman/rest/docs/membership.rst b/src/mailman/rest/docs/membership.rst index b0b5e1254..6e24d14f9 100644 --- a/src/mailman/rest/docs/membership.rst +++ b/src/mailman/rest/docs/membership.rst @@ -642,7 +642,6 @@ won't have to approve her subscription request. ... 'display_name': 'Elly Person', ... 'pre_verified': True, ... 'pre_confirmed': True, - ... 'pre_approved': True, ... }) content-length: 0 content-type: application/json; charset=UTF-8 @@ -699,7 +698,6 @@ list with her preferred address. ... 'subscriber': user_id, ... 'pre_verified': True, ... 'pre_confirmed': True, - ... 'pre_approved': True, ... }) content-length: 0 content-type: application/json; charset=UTF-8 diff --git a/src/mailman/rest/docs/sub-moderation.rst b/src/mailman/rest/docs/sub-moderation.rst index 92c0c8849..3c6f30aef 100644 --- a/src/mailman/rest/docs/sub-moderation.rst +++ b/src/mailman/rest/docs/sub-moderation.rst @@ -13,8 +13,8 @@ A mailing list starts with no pending subscription or unsubscription requests. >>> ant = create_list('ant@example.com') >>> ant.admin_immed_notify = False - >>> from mailman.interfaces.mailinglist import SubscriptionPolicy - >>> ant.subscription_policy = SubscriptionPolicy.moderate + >>> from mailman.workflows.subscription import ModerationSubscriptionPolicy + >>> ant.subscription_policy = ModerationSubscriptionPolicy >>> transaction.commit() >>> dump_json('http://localhost:9001/3.0/lists/ant.example.com/requests') http_etag: "..." @@ -31,7 +31,6 @@ is returned to track her subscription request. ... 'subscriber': 'anne@example.com', ... 'display_name': 'Anne Person', ... 'pre_verified': True, - ... 'pre_confirmed': True, ... }) http_etag: ... token: 0000000000000000000000000000000000000001 diff --git a/src/mailman/rest/listconf.py b/src/mailman/rest/listconf.py index b62d34529..70f04999c 100644 --- a/src/mailman/rest/listconf.py +++ b/src/mailman/rest/listconf.py @@ -24,14 +24,17 @@ from mailman.interfaces.archiver import ArchivePolicy from mailman.interfaces.autorespond import ResponseAction from mailman.interfaces.digests import DigestFrequency from mailman.interfaces.mailinglist import ( - DMARCMitigateAction, IAcceptableAliasSet, IMailingList, ReplyToMunging, - SubscriptionPolicy) + DMARCMitigateAction, IAcceptableAliasSet, IMailingList, ReplyToMunging) from mailman.interfaces.template import ITemplateManager +from mailman.interfaces.workflows import ISubscriptionWorkflow from mailman.rest.helpers import ( GetterSetter, bad_request, etag, no_content, not_found, okay) from mailman.rest.validator import ( PatchValidator, ReadOnlyPATCHRequestError, UnknownPATCHRequestError, - Validator, enum_validator, list_of_strings_validator) + Validator, enum_validator, list_of_strings_validator, policy_validator) +from mailman.workflows.subscription import ( + ConfirmModerationSubscriptionPolicy, ConfirmSubscriptionPolicy, + ModerationSubscriptionPolicy, OpenSubscriptionPolicy) from public import public from zope.component import getUtility @@ -86,6 +89,20 @@ class URIAttributeMapper(GetterSetter): getUtility(ITemplateManager).set(template_name, obj.list_id, value) +class SubscriptionPolicyMapper(GetterSetter): + + def get(self, obj, attribute): + assert IMailingList.providedBy(obj), obj + old_sub_map = { + OpenSubscriptionPolicy: 'open', + ConfirmSubscriptionPolicy: 'confirm', + ModerationSubscriptionPolicy: 'moderate', + ConfirmModerationSubscriptionPolicy: 'confirm_then_moderate' + } + cls = getattr(obj, attribute) + return old_sub_map.get(cls, cls.name) + + # Additional validators for converting from web request strings to internal # data types. See below for details. @@ -179,7 +196,8 @@ ATTRIBUTES = dict( request_address=GetterSetter(None), send_welcome_message=GetterSetter(as_boolean), subject_prefix=GetterSetter(str), - subscription_policy=GetterSetter(enum_validator(SubscriptionPolicy)), + subscription_policy=SubscriptionPolicyMapper( + policy_validator(ISubscriptionWorkflow)), volume=GetterSetter(None), ) diff --git a/src/mailman/rest/members.py b/src/mailman/rest/members.py index 3ff712259..2d5b95115 100644 --- a/src/mailman/rest/members.py +++ b/src/mailman/rest/members.py @@ -17,7 +17,6 @@ """REST for members.""" -from lazr.config import as_boolean from mailman.app.membership import add_member, delete_member from mailman.interfaces.action import Action from mailman.interfaces.address import IAddress @@ -198,22 +197,43 @@ class AllMembers(_MemberBase): def on_post(self, request, response): """Create a new member.""" - try: - validator = Validator( - list_id=str, - subscriber=subscriber_validator(self.api), - display_name=str, - delivery_mode=enum_validator(DeliveryMode), - role=enum_validator(MemberRole), - pre_verified=as_boolean, - pre_confirmed=as_boolean, - pre_approved=as_boolean, - _optional=('delivery_mode', 'display_name', 'role', - 'pre_verified', 'pre_confirmed', 'pre_approved')) - arguments = validator(request) - except ValueError as error: - bad_request(response, str(error)) + # Validate the params manually as the subscription_policy workflow + # attributes are dynamic, so parse all known and optional attirbutes + # and leave the rest to policy_kwargs. + required_arguments = ['list_id', 'subscriber'] + known_arguments = dict(list_id=str, + subscriber=subscriber_validator(self.api), + display_name=str, + delivery_mode=enum_validator(DeliveryMode), + role=enum_validator(MemberRole)) + + arguments = {} + policy_kwargs = {} + + wrong = [] + for key, value in request.params.items(): + try: + if key in known_arguments: + converter = known_arguments[key] + arguments[key] = converter(value) + else: + policy_kwargs[key] = value + except ValueError: + wrong.append(key) + + if len(wrong) > 0: + params = ', '.join(wrong) + bad_request(response, + 'Cannot convert parameters: {}'.format(params)) return + + missing = [key for key in required_arguments if key not in arguments] + if len(missing) > 0: + bad_request(response, 'Missing parameters: {}'.format(missing)) + return + # XXX policy_kwargs are string, while subscription workflows expects + # whatever. + # Dig the mailing list out of the arguments. list_id = arguments.pop('list_id') mlist = getUtility(IListManager).get_by_list_id(list_id) @@ -247,20 +267,13 @@ class AllMembers(_MemberBase): # nonmembers go through the legacy API for now. role = arguments.pop('role', MemberRole.member) if role is MemberRole.member: - # Get the pre_ flags for the subscription workflow. - pre_verified = arguments.pop('pre_verified', False) - pre_confirmed = arguments.pop('pre_confirmed', False) - pre_approved = arguments.pop('pre_approved', False) # Now we can run the registration process until either the # subscriber is subscribed, or the workflow is paused for # verification, confirmation, or approval. registrar = ISubscriptionManager(mlist) try: token, token_owner, member = registrar.register( - subscriber, - pre_verified=pre_verified, - pre_confirmed=pre_confirmed, - pre_approved=pre_approved) + subscriber, **policy_kwargs) except AlreadySubscribedError: conflict(response, b'Member already subscribed') return diff --git a/src/mailman/rest/tests/test_listconf.py b/src/mailman/rest/tests/test_listconf.py index 782effd29..bb88feb61 100644 --- a/src/mailman/rest/tests/test_listconf.py +++ b/src/mailman/rest/tests/test_listconf.py @@ -22,11 +22,11 @@ import unittest from mailman.app.lifecycle import create_list from mailman.database.transaction import transaction from mailman.interfaces.digests import DigestFrequency -from mailman.interfaces.mailinglist import ( - IAcceptableAliasSet, SubscriptionPolicy) +from mailman.interfaces.mailinglist import IAcceptableAliasSet from mailman.interfaces.template import ITemplateManager from mailman.testing.helpers import call_api from mailman.testing.layers import RESTLayer +from mailman.workflows.subscription import ConfirmModerationSubscriptionPolicy from urllib.error import HTTPError from zope.component import getUtility @@ -197,7 +197,7 @@ class TestConfiguration(unittest.TestCase): self.assertEqual(response.status_code, 204) # And now we verify that it has the requested setting. self.assertEqual(self._mlist.subscription_policy, - SubscriptionPolicy.confirm_then_moderate) + ConfirmModerationSubscriptionPolicy) def test_patch_attribute_double(self): with self.assertRaises(HTTPError) as cm: diff --git a/src/mailman/rest/tests/test_membership.py b/src/mailman/rest/tests/test_membership.py index e5a2ce283..bb0134ac5 100644 --- a/src/mailman/rest/tests/test_membership.py +++ b/src/mailman/rest/tests/test_membership.py @@ -23,7 +23,6 @@ from mailman.app.lifecycle import create_list from mailman.config import config 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.subscriptions import ISubscriptionManager, TokenOwner from mailman.interfaces.usermanager import IUserManager @@ -33,6 +32,8 @@ from mailman.testing.helpers import ( set_preferred, subscribe, wait_for_webservice) from mailman.testing.layers import ConfigLayer, RESTLayer from mailman.utilities.datetime import now +from mailman.workflows.subscription import ( + ConfirmModerationSubscriptionPolicy, ModerationSubscriptionPolicy) from urllib.error import HTTPError from zope.component import getUtility @@ -93,7 +94,6 @@ class TestMembership(unittest.TestCase): 'subscriber': 'anne@example.com', 'pre_verified': True, 'pre_confirmed': True, - 'pre_approved': True, }) self.assertEqual(cm.exception.code, 409) self.assertEqual(cm.exception.reason, 'Member already subscribed') @@ -108,7 +108,6 @@ class TestMembership(unittest.TestCase): 'subscriber': 'anne@example.com', 'pre_verified': False, 'pre_confirmed': False, - 'pre_approved': False, }) self.assertEqual(cm.exception.code, 409) self.assertEqual(cm.exception.reason, 'Member already subscribed') @@ -123,7 +122,6 @@ class TestMembership(unittest.TestCase): 'subscriber': '00000000000000000000000000000001', 'pre_verified': True, 'pre_confirmed': True, - 'pre_approved': True, }) self.assertEqual(cm.exception.code, 400) self.assertEqual(cm.exception.reason, 'User has no preferred address') @@ -135,7 +133,6 @@ class TestMembership(unittest.TestCase): 'subscriber': '00000000000000000000000000000801', 'pre_verified': True, 'pre_confirmed': True, - 'pre_approved': True, }) self.assertEqual(cm.exception.code, 400) self.assertEqual(cm.exception.reason, 'No such user') @@ -153,7 +150,6 @@ class TestMembership(unittest.TestCase): 'subscriber': 'ANNE@example.com', 'pre_verified': True, 'pre_confirmed': True, - 'pre_approved': True, }) self.assertEqual(cm.exception.code, 409) self.assertEqual(cm.exception.reason, 'Member already subscribed') @@ -171,7 +167,6 @@ class TestMembership(unittest.TestCase): 'subscriber': 'anne@example.com', 'pre_verified': True, 'pre_confirmed': True, - 'pre_approved': True, }) self.assertEqual(cm.exception.code, 409) self.assertEqual(cm.exception.reason, 'Member already subscribed') @@ -195,7 +190,6 @@ class TestMembership(unittest.TestCase): 'display_name': 'Hugh Person', 'pre_verified': True, 'pre_confirmed': True, - 'pre_approved': True, }) self.assertEqual(json, None) self.assertEqual(response.status_code, 201) @@ -231,10 +225,10 @@ class TestMembership(unittest.TestCase): # to subscribe again. registrar = ISubscriptionManager(self._mlist) with transaction(): - self._mlist.subscription_policy = SubscriptionPolicy.moderate + self._mlist.subscription_policy = ModerationSubscriptionPolicy anne = self._usermanager.create_address('anne@example.com') token, token_owner, member = registrar.register( - anne, pre_verified=True, pre_confirmed=True) + anne, pre_verified=True) self.assertEqual(token_owner, TokenOwner.moderator) self.assertIsNone(member) with self.assertRaises(HTTPError) as cm: @@ -242,7 +236,6 @@ class TestMembership(unittest.TestCase): 'list_id': 'test.example.com', 'subscriber': 'anne@example.com', 'pre_verified': True, - 'pre_confirmed': True, }) self.assertEqual(cm.exception.code, 409) self.assertEqual(cm.exception.reason, @@ -255,7 +248,7 @@ class TestMembership(unittest.TestCase): registrar = ISubscriptionManager(self._mlist) with transaction(): self._mlist.subscription_policy = ( - SubscriptionPolicy.confirm_then_moderate) + ConfirmModerationSubscriptionPolicy) anne = self._usermanager.create_address('anne@example.com') token, token_owner, member = registrar.register( anne, pre_verified=True) @@ -645,7 +638,6 @@ class TestAPI31Members(unittest.TestCase): 'subscriber': '00000000000000000000000000000001', 'pre_verified': True, 'pre_confirmed': True, - 'pre_approved': True, }) self.assertEqual(response.status_code, 201) self.assertEqual( @@ -681,7 +673,6 @@ class TestAPI31Members(unittest.TestCase): 'subscriber': '1', 'pre_verified': True, 'pre_confirmed': True, - 'pre_approved': True, }) # This is a bad request because the `subscriber` value isn't something # that's known to the system, in API 3.1. It's not technically a 404 diff --git a/src/mailman/rest/tests/test_moderation.py b/src/mailman/rest/tests/test_moderation.py index 4f6894ebb..a7abc9a80 100644 --- a/src/mailman/rest/tests/test_moderation.py +++ b/src/mailman/rest/tests/test_moderation.py @@ -24,7 +24,6 @@ from mailman.app.lifecycle import create_list 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.requests import IListRequests, RequestType from mailman.interfaces.subscriptions import ISubscriptionManager from mailman.interfaces.usermanager import IUserManager @@ -32,6 +31,7 @@ from mailman.testing.helpers import ( call_api, get_queue_messages, set_preferred, specialized_message_from_string as mfs) from mailman.testing.layers import RESTLayer +from mailman.workflows.subscription import ModerationSubscriptionPolicy from pkg_resources import resource_filename from urllib.error import HTTPError from zope.component import getUtility @@ -315,10 +315,9 @@ class TestSubscriptionModeration(unittest.TestCase): # Anne tries to subscribe to a list that only requests moderator # approval. with transaction(): - self._mlist.subscription_policy = SubscriptionPolicy.moderate + self._mlist.subscription_policy = ModerationSubscriptionPolicy token, token_owner, member = self._registrar.register( - self._anne, - pre_verified=True, pre_confirmed=True) + self._anne, pre_verified=True) # There's now one request in the queue, and it's waiting on moderator # approval. json, response = call_api( diff --git a/src/mailman/rest/tests/test_users.py b/src/mailman/rest/tests/test_users.py index 97f4b76d2..4c97e5c1d 100644 --- a/src/mailman/rest/tests/test_users.py +++ b/src/mailman/rest/tests/test_users.py @@ -437,7 +437,7 @@ class TestLP1074374(unittest.TestCase): list_id='test.example.com', subscriber='anne@example.com', role='member', - pre_verified=True, pre_confirmed=True, pre_approved=True)) + pre_verified=True, pre_confirmed=True)) # This is not the Anne you're looking for. (IOW, the new Anne is a # different user). json, response = call_api( diff --git a/src/mailman/rest/validator.py b/src/mailman/rest/validator.py index b325fdc84..d5f684df9 100644 --- a/src/mailman/rest/validator.py +++ b/src/mailman/rest/validator.py @@ -16,15 +16,35 @@ # GNU Mailman. If not, see <http://www.gnu.org/licenses/>. """REST web form validation.""" - +from mailman.config import config from mailman.interfaces.address import IEmailValidator from mailman.interfaces.errors import MailmanError from mailman.interfaces.languages import ILanguageManager +from mailman.interfaces.workflows import (ISubscriptionWorkflow, + IUnsubscriptionWorkflow) +from mailman.workflows.subscription import ( + ConfirmModerationSubscriptionPolicy, ConfirmSubscriptionPolicy, + ModerationSubscriptionPolicy, OpenSubscriptionPolicy) +from mailman.workflows.unsubscription import ( + ConfirmModerationUnsubscriptionPolicy, ConfirmUnsubscriptionPolicy, + ModerationUnsubscriptionPolicy, OpenUnsubscriptionPolicy) from public import public from zope.component import getUtility COMMASPACE = ', ' +OLD_SUB_MAP = { + 'open': OpenSubscriptionPolicy, + 'confirm': ConfirmSubscriptionPolicy, + 'moderate': ModerationSubscriptionPolicy, + 'confirm_then_moderate': ConfirmModerationSubscriptionPolicy + } +OLD_UNSUB_MAP = { + 'open': OpenUnsubscriptionPolicy, + 'confirm': ConfirmUnsubscriptionPolicy, + 'moderate': ModerationUnsubscriptionPolicy, + 'confirm_then_moderate': ConfirmModerationUnsubscriptionPolicy + } @public @@ -100,6 +120,33 @@ def list_of_strings_validator(values): @public +class policy_validator: + """""" + + def __init__(self, policy_interface): + self._policy_interface = policy_interface + if policy_interface is ISubscriptionWorkflow: + self._old_map = OLD_SUB_MAP + elif policy_interface is IUnsubscriptionWorkflow: + self._old_map = OLD_UNSUB_MAP + else: + raise ValueError('Expected a workflow interface.') + + def __call__(self, policy): + if self._policy_interface.implementedBy(policy): + return policy + if (policy in config.workflows and + self._policy_interface.implementedBy( + config.workflows[policy])): + return config.workflows[policy] + # For backwards compatibility. + policy = self._old_map.get(policy, None) + if policy is not None: + return policy + raise ValueError('Unknown policy: {}'.format(policy)) + + +@public class Validator: """A validator of parameter input.""" diff --git a/src/mailman/runners/docs/command.rst b/src/mailman/runners/docs/command.rst index 0dbce2ee1..73bacf5b4 100644 --- a/src/mailman/runners/docs/command.rst +++ b/src/mailman/runners/docs/command.rst @@ -166,8 +166,8 @@ Similarly, to leave a mailing list, the user need only email the ``-leave`` or ... ... """) - >>> from mailman.interfaces.mailinglist import SubscriptionPolicy - >>> mlist.unsubscription_policy = SubscriptionPolicy.open + >>> from mailman.workflows.unsubscription import OpenUnsubscriptionPolicy + >>> mlist.unsubscription_policy = OpenUnsubscriptionPolicy >>> filebase = inject_message( ... mlist, msg, switchboard='command', subaddress='leave') >>> command.run() diff --git a/src/mailman/runners/tests/test_leave.py b/src/mailman/runners/tests/test_leave.py index 7069e6710..c20637c3f 100644 --- a/src/mailman/runners/tests/test_leave.py +++ b/src/mailman/runners/tests/test_leave.py @@ -23,13 +23,14 @@ 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 mailman.workflows.unsubscription import (ConfirmUnsubscriptionPolicy, + OpenUnsubscriptionPolicy) from zope.component import getUtility @@ -65,7 +66,7 @@ class TestLeave(unittest.TestCase): def test_leave(self): with transaction(): - self._mlist.unsubscription_policy = SubscriptionPolicy.confirm + self._mlist.unsubscription_policy = ConfirmUnsubscriptionPolicy anne = getUtility(IUserManager).create_user('anne@example.org') set_preferred(anne) self._mlist.subscribe(anne.preferred_address) @@ -100,7 +101,7 @@ leave # 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 + self._mlist.unsubscription_policy = OpenUnsubscriptionPolicy anne = getUtility(IUserManager).create_user('anne@example.org') set_preferred(anne) self._mlist.subscribe(anne.preferred_address) diff --git a/src/mailman/styles/base.py b/src/mailman/styles/base.py index 9e012385d..740a56044 100644 --- a/src/mailman/styles/base.py +++ b/src/mailman/styles/base.py @@ -31,8 +31,10 @@ from mailman.interfaces.autorespond import ResponseAction from mailman.interfaces.bounce import UnrecognizedBounceDisposition from mailman.interfaces.digests import DigestFrequency from mailman.interfaces.mailinglist import ( - DMARCMitigateAction, Personalization, ReplyToMunging, SubscriptionPolicy) + DMARCMitigateAction, Personalization, ReplyToMunging) from mailman.interfaces.nntp import NewsgroupModeration +from mailman.workflows.subscription import ConfirmSubscriptionPolicy +from mailman.workflows.unsubscription import ConfirmUnsubscriptionPolicy from public import public @@ -66,8 +68,8 @@ class BasicOperation: mlist.personalize = Personalization.none mlist.default_member_action = Action.defer mlist.default_nonmember_action = Action.hold - mlist.subscription_policy = SubscriptionPolicy.confirm - mlist.unsubscription_policy = SubscriptionPolicy.confirm + mlist.subscription_policy = ConfirmSubscriptionPolicy + mlist.unsubscription_policy = ConfirmUnsubscriptionPolicy # Notify the administrator of pending requests and membership changes. mlist.admin_immed_notify = True mlist.admin_notify_mchanges = False diff --git a/src/mailman/utilities/importer.py b/src/mailman/utilities/importer.py index 578e95dff..c5e73bcc5 100644 --- a/src/mailman/utilities/importer.py +++ b/src/mailman/utilities/importer.py @@ -35,14 +35,16 @@ from mailman.interfaces.digests import DigestFrequency from mailman.interfaces.errors import MailmanError from mailman.interfaces.languages import ILanguageManager from mailman.interfaces.mailinglist import ( - IAcceptableAliasSet, IHeaderMatchList, Personalization, ReplyToMunging, - SubscriptionPolicy) + IAcceptableAliasSet, IHeaderMatchList, Personalization, ReplyToMunging) from mailman.interfaces.member import DeliveryMode, DeliveryStatus, MemberRole from mailman.interfaces.nntp import NewsgroupModeration from mailman.interfaces.template import ITemplateManager from mailman.interfaces.usermanager import IUserManager from mailman.utilities.filesystem import makedirs from mailman.utilities.i18n import search +from mailman.workflows.subscription import ( + ConfirmModerationSubscriptionPolicy, ConfirmSubscriptionPolicy, + ModerationSubscriptionPolicy, OpenSubscriptionPolicy) from public import public from sqlalchemy import Boolean from zope.component import getUtility @@ -138,6 +140,15 @@ def action_to_chain(value): }[value] +def sub_policy_to_class(value): + return { + 0: OpenSubscriptionPolicy, + 1: ConfirmSubscriptionPolicy, + 2: ModerationSubscriptionPolicy, + 3: ConfirmModerationSubscriptionPolicy + }[value] + + def check_language_code(code): if code is None: return None @@ -180,7 +191,7 @@ TYPES = dict( personalize=Personalization, preferred_language=check_language_code, reply_goes_to_list=ReplyToMunging, - subscription_policy=SubscriptionPolicy, + subscription_policy=sub_policy_to_class, ) diff --git a/src/mailman/utilities/tests/test_import.py b/src/mailman/utilities/tests/test_import.py index b1b4764ff..6db0c4a4a 100644 --- a/src/mailman/utilities/tests/test_import.py +++ b/src/mailman/utilities/tests/test_import.py @@ -33,8 +33,7 @@ from mailman.interfaces.bans import IBanManager from mailman.interfaces.bounce import UnrecognizedBounceDisposition from mailman.interfaces.domain import IDomainManager from mailman.interfaces.languages import ILanguageManager -from mailman.interfaces.mailinglist import ( - IAcceptableAliasSet, SubscriptionPolicy) +from mailman.interfaces.mailinglist import IAcceptableAliasSet from mailman.interfaces.member import DeliveryMode, DeliveryStatus from mailman.interfaces.nntp import NewsgroupModeration from mailman.interfaces.template import ITemplateLoader, ITemplateManager @@ -44,6 +43,9 @@ from mailman.testing.layers import ConfigLayer from mailman.utilities.filesystem import makedirs from mailman.utilities.importer import ( Import21Error, check_language_code, import_config_pck) +from mailman.workflows.subscription import ( + ConfirmModerationSubscriptionPolicy, ConfirmSubscriptionPolicy, + ModerationSubscriptionPolicy, OpenSubscriptionPolicy) from pickle import load from pkg_resources import resource_filename from unittest import mock @@ -295,32 +297,32 @@ class TestBasicImport(unittest.TestCase): self.assertEqual(self._mlist.encode_ascii_prefixes, True) def test_subscription_policy_open(self): - self._mlist.subscription_policy = SubscriptionPolicy.confirm + self._mlist.subscription_policy = ConfirmSubscriptionPolicy self._pckdict['subscribe_policy'] = 0 self._import() self.assertEqual(self._mlist.subscription_policy, - SubscriptionPolicy.open) + OpenSubscriptionPolicy) def test_subscription_policy_confirm(self): - self._mlist.subscription_policy = SubscriptionPolicy.open + self._mlist.subscription_policy = OpenSubscriptionPolicy self._pckdict['subscribe_policy'] = 1 self._import() self.assertEqual(self._mlist.subscription_policy, - SubscriptionPolicy.confirm) + ConfirmSubscriptionPolicy) def test_subscription_policy_moderate(self): - self._mlist.subscription_policy = SubscriptionPolicy.open + self._mlist.subscription_policy = OpenSubscriptionPolicy self._pckdict['subscribe_policy'] = 2 self._import() self.assertEqual(self._mlist.subscription_policy, - SubscriptionPolicy.moderate) + ModerationSubscriptionPolicy) def test_subscription_policy_confirm_then_moderate(self): - self._mlist.subscription_policy = SubscriptionPolicy.open + self._mlist.subscription_policy = OpenSubscriptionPolicy self._pckdict['subscribe_policy'] = 3 self._import() self.assertEqual(self._mlist.subscription_policy, - SubscriptionPolicy.confirm_then_moderate) + ConfirmModerationSubscriptionPolicy) def test_header_matches(self): # This test containes real cases of header_filter_rules. diff --git a/src/mailman/workflows/builtin.py b/src/mailman/workflows/builtin.py deleted file mode 100644 index 33e428d0b..000000000 --- a/src/mailman/workflows/builtin.py +++ /dev/null @@ -1,403 +0,0 @@ -# Copyright (C) 2015-2017 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/>. - -"""The built in subscription and unsubscription workflows.""" - -import logging - -from email.utils import formataddr -from mailman.app.membership import delete_member -from mailman.core.i18n import _ -from mailman.email.message import UserNotification -from mailman.interfaces.address import IAddress -from mailman.interfaces.bans import IBanManager -from mailman.interfaces.mailinglist import SubscriptionPolicy -from mailman.interfaces.member import (AlreadySubscribedError, MemberRole, - MembershipIsBannedError, - NotAMemberError) -from mailman.interfaces.pending import IPendings -from mailman.interfaces.subscriptions import ( - SubscriptionConfirmationNeededEvent, SubscriptionPendingError, - TokenOwner, UnsubscriptionConfirmationNeededEvent) -from mailman.interfaces.template import ITemplateLoader -from mailman.interfaces.user import IUser -from mailman.interfaces.usermanager import IUserManager -from mailman.interfaces.workflows import (ISubscriptionWorkflow, - IUnsubscriptionWorkflow) -from mailman.utilities.datetime import now -from mailman.utilities.string import expand, wrap -from mailman.workflows.common import (SubscriptionWorkflowCommon, - WhichSubscriber) -from public import public -from zope.component import getUtility -from zope.event import notify -from zope.interface import implementer - - -log = logging.getLogger('mailman.subscribe') - - -@public -@implementer(ISubscriptionWorkflow) -class SubscriptionWorkflow(SubscriptionWorkflowCommon): - """Workflow of a subscription request.""" - - name = 'subscription-default' - description = '' - - 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 - # verified. - if self.user is None: - # The address has no linked user so create one, link it, and set - # the user's preferred address. - assert self.address is not None, 'No address or user' - self.user = getUtility(IUserManager).make_user(self.address.email) - if self.address is None: - assert self.user.preferred_address is None, ( - "Preferred address exists, but wasn't used in constructor") - addresses = list(self.user.addresses) - if len(addresses) == 0: - raise AssertionError('User has no addresses: {}'.format( - self.user)) - # This is rather arbitrary, but we have no choice. - self.address = addresses[0] - assert self.user is not None and self.address is not None, ( - 'Insane sanity check results') - # Is this subscriber already a member? - if (self.which is WhichSubscriber.user and - self.user.preferred_address is not None): - subscriber = self.user - else: - subscriber = self.address - if self.mlist.is_subscribed(subscriber): - # 2017-04-22 BAW: This branch actually *does* get covered, as I've - # verified by a full coverage run, but diffcov for some reason - # claims that the test added in the branch that added this code - # does not cover the change. That seems like a bug in diffcov. - raise AlreadySubscribedError( # pragma: no cover - self.mlist.fqdn_listname, - self.address.email, - MemberRole.member) - # Is this email address banned? - if IBanManager(self.mlist).is_banned(self.address.email): - raise MembershipIsBannedError(self.mlist, self.address.email) - # Check if there is already a subscription request for this email. - pendings = getUtility(IPendings).find( - mlist=self.mlist, - pend_type='subscription') - for token, pendable in pendings: - if pendable['email'] == self.address.email: - raise SubscriptionPendingError(self.mlist, self.address.email) - # Start out with the subscriber being the token owner. - self.push('verification_checks') - - def _step_verification_checks(self): - # Is the address already verified, or is the pre-verified flag set? - if self.address.verified_on is None: - if self.pre_verified: - self.address.verified_on = now() - else: - # The address being subscribed is not yet verified, so we need - # to send a validation email that will also confirm that the - # user wants to be subscribed to this mailing list. - self.push('send_confirmation') - return - self.push('confirmation_checks') - - def _step_confirmation_checks(self): - # If the list's subscription policy is open, then the user can be - # subscribed right here and now. - if self.mlist.subscription_policy is SubscriptionPolicy.open: - self.push('do_subscription') - return - # If we do not need the user's confirmation, then skip to the - # moderation checks. - if self.mlist.subscription_policy is SubscriptionPolicy.moderate: - self.push('moderation_checks') - return - # If the subscription has been pre-confirmed, then we can skip the - # confirmation check can be skipped. If moderator approval is - # required we need to check that, otherwise we can go straight to - # subscription. - if self.pre_confirmed: - next_step = ( - 'moderation_checks' - if self.mlist.subscription_policy is - SubscriptionPolicy.confirm_then_moderate # noqa: E131 - else 'do_subscription') - self.push(next_step) - return - # The user must confirm their subscription. - self.push('send_confirmation') - - def _step_moderation_checks(self): - # Does the moderator need to approve the subscription request? - assert self.mlist.subscription_policy in ( - SubscriptionPolicy.moderate, - SubscriptionPolicy.confirm_then_moderate, - ), self.mlist.subscription_policy - if self.pre_approved: - self.push('do_subscription') - else: - self.push('get_moderator_approval') - - def _step_get_moderator_approval(self): - # Here's the next step in the workflow, assuming the moderator - # approves of the subscription. If they don't, the workflow and - # subscription request will just be thrown away. - self._set_token(TokenOwner.moderator) - self.push('subscribe_from_restored') - self.save() - log.info('{}: held subscription request from {}'.format( - self.mlist.fqdn_listname, self.address.email)) - # Possibly send a notification to the list moderators. - if self.mlist.admin_immed_notify: - subject = _( - 'New subscription 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:subscribe', 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) - # The workflow must stop running here. - raise StopIteration - - def _step_subscribe_from_restored(self): - # Prevent replay attacks. - self._set_token(TokenOwner.no_one) - # Restore a little extra state that can't be stored in the database - # (because the order of setattr() on restore is indeterminate), then - # subscribe the user. - if self.which is WhichSubscriber.address: - self.subscriber = self.address - else: - assert self.which is WhichSubscriber.user - self.subscriber = self.user - self.push('do_subscription') - - def _step_do_subscription(self): - # We can immediately subscribe the user to the mailing list. - self.member = self.mlist.subscribe(self.subscriber) - 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(SubscriptionConfirmationNeededEvent( - self.mlist, self.token, self.address.email)) - # Now we wait for the confirmation. - 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) - # The user has confirmed their subscription request, and also verified - # their email address if necessary. This latter needs to be set on the - # IAddress, but there's nothing more to do about the confirmation step. - # We just continue along with the workflow. - if self.address.verified_on is None: - self.address.verified_on = now() - # The next step depends on the mailing list's subscription policy. - next_step = ('moderation_checks' - if self.mlist.subscription_policy in ( - SubscriptionPolicy.moderate, - SubscriptionPolicy.confirm_then_moderate, - ) - else 'do_subscription') - self.push(next_step) - - -@public -@implementer(IUnsubscriptionWorkflow) -class UnSubscriptionWorkflow(SubscriptionWorkflowCommon): - """Workflow of a unsubscription request.""" - - name = 'unsubscription-default' - description = '' - - 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) - # 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') |
