From 2b9ef541cf80e5bcf413c2ba234856a7bc77baad Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Mon, 13 Apr 2015 16:23:50 -0400 Subject: [PATCH] SubscriptionPolicy is largely done, modulo bugs, and the unknown-unknown of course. This commit contains test failures because the next step is to hook everything up. Changes here include: * confirm.txt removes the URL since we can't know that. * Changes to the ConfirmationNeededEvent * Create the Pendable early in the workflow. * Clean up test. * Update IRegistrar docstring. --- src/mailman/app/registrar.py | 17 ++-- src/mailman/app/subscriptions.py | 65 ++++++++----- src/mailman/app/tests/test_subscriptions.py | 141 ++++++++++++++++------------ src/mailman/interfaces/registrar.py | 19 +--- src/mailman/templates/en/confirm.txt | 4 +- 5 files changed, 137 insertions(+), 109 deletions(-) diff --git a/src/mailman/app/registrar.py b/src/mailman/app/registrar.py index 9272b95d7..db68432d3 100644 --- a/src/mailman/app/registrar.py +++ b/src/mailman/app/registrar.py @@ -156,18 +156,17 @@ def handle_ConfirmationNeededEvent(event): # the Subject header, or they can click on the URL in the body of the # message and confirm through the web. subject = 'confirm ' + event.token - mlist = getUtility(IListManager).get_by_list_id(event.pendable['list_id']) - confirm_address = mlist.confirm_address(event.token) + confirm_address = event.mlist.confirm_address(event.token) # For i18n interpolation. - confirm_url = mlist.domain.confirm_url(event.token) - email_address = event.pendable['email'] - domain_name = mlist.domain.mail_host - contact_address = mlist.owner_address + confirm_url = event.mlist.domain.confirm_url(event.token) + email_address = event.email + domain_name = event.mlist.domain.mail_host + contact_address = event.mlist.owner_address # Send a verification email to the address. template = getUtility(ITemplateLoader).get( 'mailman:///{0}/{1}/confirm.txt'.format( - mlist.fqdn_listname, - mlist.preferred_language.code)) + event.mlist.fqdn_listname, + event.mlist.preferred_language.code)) text = _(template) msg = UserNotification(email_address, confirm_address, subject, text) - msg.send(mlist) + msg.send(event.mlist) diff --git a/src/mailman/app/subscriptions.py b/src/mailman/app/subscriptions.py index 9df85c819..6c60a71a4 100644 --- a/src/mailman/app/subscriptions.py +++ b/src/mailman/app/subscriptions.py @@ -45,6 +45,7 @@ from mailman.interfaces.mailinglist import SubscriptionPolicy from mailman.interfaces.member import ( DeliveryMode, MemberRole, MembershipIsBannedError) from mailman.interfaces.pending import IPendable, IPendings +from mailman.interfaces.registrar import ConfirmationNeededEvent from mailman.interfaces.subscriptions import ( ISubscriptionService, MissingUserError, RequestRecord) from mailman.interfaces.user import IUser @@ -56,6 +57,7 @@ from operator import attrgetter from sqlalchemy import and_, or_ from uuid import UUID from zope.component import getUtility +from zope.event import notify from zope.interface import implementer @@ -171,6 +173,14 @@ class SubscriptionWorkflow(Workflow): # Is this email address banned? if IBanManager(self.mlist).is_banned(self.address.email): raise MembershipIsBannedError(self.mlist, self.address.email) + # Create a pending record. This will give us the hash token we can use + # to uniquely name this workflow. + pendable = Pendable( + when=now().isoformat(), + list_id=self.mlist.list_id, + address=self.address.email, + ) + self.token = getUtility(IPendings).add(pendable, timedelta(days=3650)) self.push('verification_checks') def _step_verification_checks(self): @@ -215,24 +225,7 @@ class SubscriptionWorkflow(Workflow): else: self.push('get_moderator_approval') - def _step_do_subscription(self): - # We can immediately subscribe the user to the mailing list. - self.mlist.subscribe(self.subscriber) - def _step_get_moderator_approval(self): - # Getting the moderator's approval requires several steps. We'll need - # to suspend this workflow for an indeterminate amount of time while - # we wait for that approval. We need a unique token for this - # suspended workflow, so we'll create a minimal pending record. We - # also might need to send an email notification to the list - # moderators. - # - # Start by creating the pending record. This will give us a hash - # token we can use to uniquely name this workflow. It only needs to - # contain the current date, which we'll use to expire requests from - # the database, say if the moderator never approves the request. - pendable = Pendable(when=now().isoformat()) - self.token = getUtility(IPendings).add(pendable, timedelta(days=3650)) # 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. @@ -272,12 +265,42 @@ class SubscriptionWorkflow(Workflow): self.subscriber = self.user self.push('do_subscription') + def _step_do_subscription(self): + # We can immediately subscribe the user to the mailing list. + self.mlist.subscribe(self.subscriber) + def _step_send_confirmation(self): - self._next.append('moderation_check') + self.push('do_confirm_verify') self.save() - self._next.clear() # stop iteration until we get confirmation - # XXX: create the Pendable, send the ConfirmationNeededEvent - # (see Registrar.register) + # Triggering this event causes the confirmation message to be sent. + notify(ConfirmationNeededEvent( + 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 + # 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_check' + if self.mlist.subscription_policy in ( + SubscriptionPolicy.moderate, + SubscriptionPolicy.confirm_then_moderate, + ) + else 'do_subscription') + self.push(next_step) @implementer(ISubscriptionService) diff --git a/src/mailman/app/tests/test_subscriptions.py b/src/mailman/app/tests/test_subscriptions.py index 31218d2d7..bdfa72e29 100644 --- a/src/mailman/app/tests/test_subscriptions.py +++ b/src/mailman/app/tests/test_subscriptions.py @@ -32,7 +32,6 @@ from mailman.interfaces.address import InvalidEmailAddressError from mailman.interfaces.bans import IBanManager from mailman.interfaces.member import ( MemberRole, MembershipIsBannedError, MissingPreferredAddressError) -from mailman.interfaces.requests import IListRequests, RequestType from mailman.interfaces.subscriptions import ( MissingUserError, ISubscriptionService) from mailman.testing.helpers import LogFileMark, get_queue_messages @@ -426,70 +425,88 @@ approval: items = get_queue_messages('virgin') self.assertEqual(len(items), 0) - # XXX + def test_send_confirmation(self): + # A confirmation message gets sent when the address is not verified. + 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) + list(workflow) + items = get_queue_messages('virgin') + self.assertEqual(len(items), 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)) - @unittest.expectedFailure - def test_preverified_address_joins_open_list(self): - # The mailing list has an open subscription policy, so the subscriber - # becomes a member with no human intervention. - self._mlist.subscription_policy = SubscriptionPolicy.open - anne = self._user_manager.create_address(self._anne, 'Anne Person') + def test_send_confirmation_pre_confirmed(self): + # A confirmation message gets sent when the address is not verified + # but the subscription is pre-confirmed. + anne = self._user_manager.create_address(self._anne) self.assertIsNone(anne.verified_on) - self.assertIsNone(anne.user) - self.assertIsNone(self._mlist.subscribers.get_member(self._anne)) - workflow = SubscriptionWorkflow( - self._mlist, anne, - pre_verified=True, pre_confirmed=False, pre_approved=False) - # Run the state machine to the end. The result is that her address - # will be verified, linked to a user, and subscribed to the mailing - # list. + # Run the workflow to model the confirmation step. + workflow = SubscriptionWorkflow(self._mlist, anne, pre_confirmed=True) list(workflow) - self.assertIsNotNone(anne.verified_on) - self.assertIsNotNone(anne.user) - self.assertIsNotNone(self._mlist.subscribers.get_member(self._anne)) - - @unittest.expectedFailure - def test_verified_address_joins_moderated_list(self): - # The mailing list is moderated but the subscriber is not a verified - # address and the subscription request is not pre-verified. - # A confirmation email must be sent, it will serve as the verification - # email too. - anne = self._user_manager.create_address(self._anne, 'Anne Person') - request_db = IListRequests(self._mlist) - def _do_check(): - anne.verified_on = now() - self.assertIsNone(self._mlist.subscribers.get_member(self._anne)) - workflow = SubscriptionWorkflow( - self._mlist, anne, - pre_verified=False, pre_confirmed=True, pre_approved=False) - # Run the state machine to the end. - list(workflow) - # Look in the requests db - requests = list(request_db.of_type(RequestType.subscription)) - self.assertEqual(len(requests), 1) - self.assertEqual(requests[0].key, anne.email) - request_db.delete_request(requests[0].id) - self._mlist.subscription_policy = SubscriptionPolicy.moderate - _do_check() - self._mlist.subscription_policy = \ - SubscriptionPolicy.confirm_then_moderate - _do_check() + items = get_queue_messages('virgin') + self.assertEqual(len(items), 1) + message = items[0].msg + token = workflow.token + self.assertEqual( + message['Subject'], 'confirm {}'.format(workflow.token)) + self.assertEqual( + message['From'], 'test-confirm+{}@example.com'.format(token)) - @unittest.expectedFailure - def test_confirmation_required(self): - # Tests subscriptions where user confirmation is required - self._mlist.subscription_policy = \ - SubscriptionPolicy.confirm_then_moderate - anne = self._user_manager.create_address(self._anne, 'Anne Person') - self.assertIsNone(self._mlist.subscribers.get_member(self._anne)) - workflow = SubscriptionWorkflow( - self._mlist, anne, - pre_verified=True, pre_confirmed=False, pre_approved=True) - # Run the state machine to the end. + 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 + 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) list(workflow) - # A confirmation request must be pending - # TODO: test it - # Now restore and re-run the state machine as if we got the confirmation - workflow.restore() + items = get_queue_messages('virgin') + self.assertEqual(len(items), 1) + message = items[0].msg + token = workflow.token + self.assertEqual( + message['Subject'], 'confirm {}'.format(workflow.token)) + self.assertEqual( + message['From'], 'test-confirm+{}@example.com'.format(token)) + + def test_do_confirm_verify_address(self): + # The address is not yet verified, nor are we pre-verifying. A + # confirmation message will be sent. When the user confirms their + # subscription request, the address will end up being verified. + 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) list(workflow) - self.assertIsNotNone(self._mlist.subscribers.get_member(self._anne)) + # The address is still not verified. + self.assertIsNone(anne.verified_on) + confirm_workflow = SubscriptionWorkflow(self._mlist) + confirm_workflow.token = workflow.token + confirm_workflow.restore() + confirm_workflow.run_thru('do_confirm_verify') + # The address is now verified. + self.assertIsNotNone(anne.verified_on) + + def test_do_confirmation_subscribes_user(self): + # 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 + anne = self._user_manager.create_address(self._anne) + self.assertIsNone(anne.verified_on) + workflow = SubscriptionWorkflow(self._mlist, anne) + list(workflow) + self.assertIsNone(self._mlist.regular_members.get_member(self._anne)) + confirm_workflow = SubscriptionWorkflow(self._mlist) + confirm_workflow.token = workflow.token + confirm_workflow.restore() + list(confirm_workflow) + self.assertIsNotNone(anne.verified_on) + self.assertEqual( + self._mlist.regular_members.get_member(self._anne).address, anne) diff --git a/src/mailman/interfaces/registrar.py b/src/mailman/interfaces/registrar.py index 7d3cf9c25..504442f7e 100644 --- a/src/mailman/interfaces/registrar.py +++ b/src/mailman/interfaces/registrar.py @@ -35,23 +35,14 @@ from zope.interface import Interface class ConfirmationNeededEvent: """Triggered when an address needs confirmation. - Addresses must be verified before they can receive messages or post to - mailing list. When an address is registered with Mailman, via the - `IRegistrar` interface, an `IPendable` is created which represents the - pending registration. This pending registration is stored in the - database, keyed by a token. Then this event is triggered. - - There may be several ways to confirm an email address. On some sites, - registration may immediately produce a verification, e.g. because it is on - a known intranet. Or verification may occur via external database lookup - (e.g. LDAP). On most public mailing lists, a mail-back confirmation is - sent to the address, and only if they reply to the mail-back, or click on - an embedded link, is the registered address confirmed. + Addresses must be verified before they can receive messages or post + to mailing list. The confirmation message is sent to the user when + this event is triggered. """ - def __init__(self, mlist, pendable, token): + def __init__(self, mlist, token, email): self.mlist = mlist - self.pendable = pendable self.token = token + self.email = email diff --git a/src/mailman/templates/en/confirm.txt b/src/mailman/templates/en/confirm.txt index d02cb462b..7c8bee75f 100644 --- a/src/mailman/templates/en/confirm.txt +++ b/src/mailman/templates/en/confirm.txt @@ -8,9 +8,7 @@ We have received a registration request for the email address Before you can start using GNU Mailman at this site, you must first confirm that this is your email address. You can do this by replying to this message, -keeping the Subject header intact. Or you can visit this web page - - $confirm_url +keeping the Subject header intact. If you do not wish to register this email address simply disregard this message. If you think you are being maliciously subscribed to the list, or -- 2.11.4.GIT