From c0a6d7dedfe7bb59c1837d924ed5c6c2b2796846 Mon Sep 17 00:00:00 2001 From: J08nY Date: Thu, 6 Jul 2017 19:30:22 +0200 Subject: Move workflows from app.subscriptions to workflows.builtin. --- src/mailman/workflows/common.py | 123 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 src/mailman/workflows/common.py (limited to 'src/mailman/workflows/common.py') diff --git a/src/mailman/workflows/common.py b/src/mailman/workflows/common.py new file mode 100644 index 000000000..940152272 --- /dev/null +++ b/src/mailman/workflows/common.py @@ -0,0 +1,123 @@ +# 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 . + +"""Common support between subscription and unsubscription.""" + +import uuid + +from datetime import timedelta +from enum import Enum +from mailman.interfaces.address import IAddress +from mailman.interfaces.pending import IPendings +from mailman.interfaces.subscriptions import TokenOwner +from mailman.interfaces.user import IUser +from mailman.interfaces.usermanager import IUserManager +from mailman.utilities.datetime import now +from mailman.workflows.base import Workflow +from zope.component import getUtility + + +class WhichSubscriber(Enum): + address = 1 + user = 2 + + +class SubscriptionWorkflowCommon(Workflow): + """Common support between subscription and unsubscription.""" + + PENDABLE_CLASS = None + + def __init__(self, mlist, subscriber): + super().__init__() + self.mlist = mlist + self.address = None + self.user = None + self.which = None + self.member = None + self._set_token(TokenOwner.no_one) + # The subscriber must be either an IUser or IAddress. + if IAddress.providedBy(subscriber): + self.address = subscriber + self.user = self.address.user + self.which = WhichSubscriber.address + elif IUser.providedBy(subscriber): + self.address = subscriber.preferred_address + self.user = subscriber + self.which = WhichSubscriber.user + self.subscriber = subscriber + + @property + def user_key(self): + # For save. + return self.user.user_id.hex + + @user_key.setter + def user_key(self, hex_key): + # For restore. + uid = uuid.UUID(hex_key) + self.user = getUtility(IUserManager).get_user_by_id(uid) + if self.user is None: + self.user = self.address.user + + @property + def address_key(self): + # For save. + return self.address.email + + @address_key.setter + def address_key(self, email): + # For restore. + self.address = getUtility(IUserManager).get_address(email) + assert self.address is not None + + @property + def subscriber_key(self): + return self.which.value + + @subscriber_key.setter + def subscriber_key(self, key): + self.which = WhichSubscriber(key) + + @property + def token_owner_key(self): + return self.token_owner.value + + @token_owner_key.setter + def token_owner_key(self, value): + self.token_owner = TokenOwner(value) + + def _set_token(self, token_owner): + assert isinstance(token_owner, TokenOwner) + pendings = getUtility(IPendings) + # Clear out the previous pending token if there is one. + if self.token is not None: + pendings.confirm(self.token) + # Create a new token to prevent replay attacks. It seems like this + # would produce the same token, but it won't because the pending adds a + # bit of randomization. + self.token_owner = token_owner + if token_owner is TokenOwner.no_one: + self.token = None + return + pendable = self.PENDABLE_CLASS( + list_id=self.mlist.list_id, + email=self.address.email, + display_name=self.address.display_name, + when=now().replace(microsecond=0).isoformat(), + token_owner=token_owner.name, + ) + self.token = pendings.add(pendable, timedelta(days=3650)) -- cgit v1.2.3-70-g09d2 From 31854e7fadc147ec0dd0c79347f632091cf41461 Mon Sep 17 00:00:00 2001 From: J08nY Date: Fri, 30 Jun 2017 00:50:57 +0200 Subject: Make workflows implement their interfaces. --- src/mailman/app/subscriptions.py | 6 +++--- src/mailman/workflows/base.py | 6 +++++- src/mailman/workflows/builtin.py | 16 +++++----------- src/mailman/workflows/common.py | 28 ++++++++++++++++++++++++---- 4 files changed, 37 insertions(+), 19 deletions(-) (limited to 'src/mailman/workflows/common.py') diff --git a/src/mailman/app/subscriptions.py b/src/mailman/app/subscriptions.py index 90811722d..c89e1e79f 100644 --- a/src/mailman/app/subscriptions.py +++ b/src/mailman/app/subscriptions.py @@ -28,10 +28,10 @@ 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 (PendableSubscription, - PendableUnsubscription, - SubscriptionWorkflow, +from mailman.workflows.builtin import (SubscriptionWorkflow, UnSubscriptionWorkflow) +from mailman.workflows.common import (PendableSubscription, + PendableUnsubscription) from public import public from zope.component import getUtility from zope.interface import implementer diff --git a/src/mailman/workflows/base.py b/src/mailman/workflows/base.py index 5988dfa43..e374d58b4 100644 --- a/src/mailman/workflows/base.py +++ b/src/mailman/workflows/base.py @@ -23,9 +23,11 @@ import logging from collections import deque -from mailman.interfaces.workflows import IWorkflowStateManager +from mailman.interfaces.workflows import IWorkflow, IWorkflowStateManager +from mailman.utilities.modules import abstract_component from public import public from zope.component import getUtility +from zope.interface import implementer COMMASPACE = ', ' @@ -33,6 +35,8 @@ log = logging.getLogger('mailman.error') @public +@abstract_component +@implementer(IWorkflow) class Workflow: """Generic workflow.""" diff --git a/src/mailman/workflows/builtin.py b/src/mailman/workflows/builtin.py index e16a888fc..33e428d0b 100644 --- a/src/mailman/workflows/builtin.py +++ b/src/mailman/workflows/builtin.py @@ -29,13 +29,15 @@ from mailman.interfaces.mailinglist import SubscriptionPolicy from mailman.interfaces.member import (AlreadySubscribedError, MemberRole, MembershipIsBannedError, NotAMemberError) -from mailman.interfaces.pending import IPendable, IPendings +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, @@ -49,17 +51,8 @@ from zope.interface import implementer log = logging.getLogger('mailman.subscribe') -@implementer(IPendable) -class PendableSubscription(dict): - PEND_TYPE = 'subscription' - - -@implementer(IPendable) -class PendableUnsubscription(dict): - PEND_TYPE = 'unsubscription' - - @public +@implementer(ISubscriptionWorkflow) class SubscriptionWorkflow(SubscriptionWorkflowCommon): """Workflow of a subscription request.""" @@ -269,6 +262,7 @@ class SubscriptionWorkflow(SubscriptionWorkflowCommon): @public +@implementer(IUnsubscriptionWorkflow) class UnSubscriptionWorkflow(SubscriptionWorkflowCommon): """Workflow of a unsubscription request.""" diff --git a/src/mailman/workflows/common.py b/src/mailman/workflows/common.py index 940152272..c57ce374d 100644 --- a/src/mailman/workflows/common.py +++ b/src/mailman/workflows/common.py @@ -22,13 +22,17 @@ import uuid from datetime import timedelta from enum import Enum from mailman.interfaces.address import IAddress -from mailman.interfaces.pending import IPendings +from mailman.interfaces.pending import IPendable, IPendings from mailman.interfaces.subscriptions import TokenOwner from mailman.interfaces.user import IUser from mailman.interfaces.usermanager import IUserManager +from mailman.interfaces.workflows import (ISubscriptionWorkflow, + IUnsubscriptionWorkflow, IWorkflow) from mailman.utilities.datetime import now from mailman.workflows.base import Workflow from zope.component import getUtility +from zope.interface import implementer +from zope.interface.exceptions import DoesNotImplement class WhichSubscriber(Enum): @@ -36,11 +40,19 @@ class WhichSubscriber(Enum): user = 2 +@implementer(IPendable) +class PendableSubscription(dict): + PEND_TYPE = 'subscription' + + +@implementer(IPendable) +class PendableUnsubscription(dict): + PEND_TYPE = 'unsubscription' + + class SubscriptionWorkflowCommon(Workflow): """Common support between subscription and unsubscription.""" - PENDABLE_CLASS = None - def __init__(self, mlist, subscriber): super().__init__() self.mlist = mlist @@ -113,7 +125,15 @@ class SubscriptionWorkflowCommon(Workflow): if token_owner is TokenOwner.no_one: self.token = None return - pendable = self.PENDABLE_CLASS( + + if ISubscriptionWorkflow.implementedBy(self.__class__): + pendable_class = PendableSubscription + elif IUnsubscriptionWorkflow.implementedBy(self.__class__): + pendable_class = PendableUnsubscription + else: + raise DoesNotImplement(IWorkflow) + + pendable = pendable_class( list_id=self.mlist.list_id, email=self.address.email, display_name=self.address.display_name, -- cgit v1.2.3-70-g09d2 From e8a0de7723c6e1d42ec2bf6fd4a4d8f22f237b78 Mon Sep 17 00:00:00 2001 From: J08nY Date: Tue, 4 Jul 2017 02:43:03 +0200 Subject: Split subscription workflow into mixins. --- src/mailman/workflows/common.py | 192 +++++++++++++++++++++++++++- src/mailman/workflows/subscription.py | 231 ++++++++++++++++++++++++++++++++++ 2 files changed, 422 insertions(+), 1 deletion(-) create mode 100644 src/mailman/workflows/subscription.py (limited to 'src/mailman/workflows/common.py') diff --git a/src/mailman/workflows/common.py b/src/mailman/workflows/common.py index c57ce374d..79d57d0ae 100644 --- a/src/mailman/workflows/common.py +++ b/src/mailman/workflows/common.py @@ -18,23 +18,37 @@ """Common support between subscription and unsubscription.""" import uuid +import logging from datetime import timedelta +from email.utils import formataddr from enum import Enum +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.member import (AlreadySubscribedError, MemberRole, + MembershipIsBannedError) from mailman.interfaces.pending import IPendable, IPendings -from mailman.interfaces.subscriptions import TokenOwner +from mailman.interfaces.subscriptions import ( + SubscriptionConfirmationNeededEvent, SubscriptionPendingError, TokenOwner) +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, IWorkflow) from mailman.utilities.datetime import now +from mailman.utilities.string import expand, wrap from mailman.workflows.base import Workflow from zope.component import getUtility +from zope.event import notify from zope.interface import implementer from zope.interface.exceptions import DoesNotImplement +log = logging.getLogger('mailman.subscribe') + + class WhichSubscriber(Enum): address = 1 user = 2 @@ -141,3 +155,179 @@ class SubscriptionWorkflowCommon(Workflow): token_owner=token_owner.name, ) self.token = pendings.add(pendable, timedelta(days=3650)) + + +class SubscriptionBase(SubscriptionWorkflowCommon): + + 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. + + 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') + + +class RequestMixin: + + 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() + self.verified = True + self.confirmed = True + + +class VerificationMixin(RequestMixin): + + def __init__(self, pre_verified=False): + self.verified = pre_verified + + 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.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 + + +class ConfirmationMixin(RequestMixin): + + def __init__(self, pre_confirmed=False): + self.confirmed = pre_confirmed + + def _step_confirmation_checks(self): + # If the subscription has been pre-confirmed, then we can skip the + # confirmation check can be skipped. + if self.confirmed: + return + # The user must confirm their subscription. + self.push('send_confirmation') + + +class ModerationMixin: + + def __init__(self, pre_approved=False): + self.approved = pre_approved + + def _step_moderation_checks(self): + # Does the moderator need to approve the subscription request? + if self.approved: + return + # A moderator must confirm the subscription. + 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 -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 diff --git a/src/mailman/workflows/subscription.py b/src/mailman/workflows/subscription.py new file mode 100644 index 000000000..f780c96a0 --- /dev/null +++ b/src/mailman/workflows/subscription.py @@ -0,0 +1,231 @@ +# 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 . + +"""""" + +from mailman.core.i18n import _ +from mailman.interfaces.workflows import ISubscriptionWorkflow +from mailman.workflows.common import (ConfirmationMixin, ModerationMixin, + SubscriptionBase, VerificationMixin) +from public import public +from zope.interface import implementer + + +@public +@implementer(ISubscriptionWorkflow) +class OpenSubscriptionPolicy(SubscriptionBase, VerificationMixin): + """""" + + name = 'sub-policy-open' + description = _('An open subscription policy, only requires verification.') + initial_state = 'prepare' + save_attributes = ( + 'verified', + 'address_key', + 'subscriber_key', + 'user_key', + 'token_owner_key', + ) + + def __init__(self, mlist, subscriber=None, *, + pre_verified=False): + """ + + :param mlist: + :param subscriber: The user or address to subscribe. + :type subscriber: ``IUser`` or ``IAddress`` + :param pre_verified: A flag indicating whether the subscriber's email + address should be considered pre-verified. Normally a never + before seen email address must be verified by mail-back + confirmation. Setting this flag to True automatically verifies + such addresses without the mail-back. (A confirmation message may + still be sent under other conditions.) + :type pre_verified: bool + """ + SubscriptionBase.__init__(self, mlist, subscriber) + VerificationMixin.__init__(self, pre_verified=pre_verified) + + def _step_prepare(self): + self.push('do_subscription') + self.push('verification_checks') + self.push('sanity_checks') + + +@public +@implementer(ISubscriptionWorkflow) +class ConfirmSubscriptionPolicy(SubscriptionBase, ConfirmationMixin, + VerificationMixin): + """""" + + name = 'sub-policy-confirm' + description = _('An subscription policy that requires confirmation.') + initial_state = 'prepare' + save_attributes = ( + 'verified', + 'confirmed', + 'address_key', + 'subscriber_key', + 'user_key', + 'token_owner_key', + ) + + def __init__(self, mlist, subscriber=None, *, + pre_verified=False, pre_confirmed=False): + """ + + :param mlist: + :param subscriber: The user or address to subscribe. + :type subscriber: ``IUser`` or ``IAddress`` + :param pre_verified: A flag indicating whether the subscriber's email + address should be considered pre-verified. Normally a never + before seen email address must be verified by mail-back + confirmation. Setting this flag to True automatically verifies + such addresses without the mail-back. (A confirmation message may + still be sent under other conditions.) + :type pre_verified: bool + :param pre_confirmed: A flag indicating whether, when required by the + subscription policy, a subscription request should be considered + pre-confirmed. Normally in such cases, a mail-back confirmation + message is sent to the subscriber, which must be positively + acknowledged by some manner. Setting this flag to True + automatically confirms the subscription request. (A confirmation + message may still be sent under other conditions.) + :type pre_confirmed: bool + """ + SubscriptionBase.__init__(self, mlist, subscriber) + VerificationMixin.__init__(self, pre_verified=pre_verified) + ConfirmationMixin.__init__(self, pre_confirmed=pre_confirmed) + + def _step_prepare(self): + self.push('do_subscription') + self.push('confirmation_checks') + self.push('verification_checks') + self.push('sanity_checks') + + +@public +@implementer(ISubscriptionWorkflow) +class ModerationSubscriptionPolicy(SubscriptionBase, ModerationMixin, + VerificationMixin): + """""" + + name = 'sub-policy-moderate' + description = _('A subscription policy that requires moderation.') + initial_state = 'prepare' + save_attributes = ( + 'approved', + 'verified', + 'address_key', + 'subscriber_key', + 'user_key', + 'token_owner_key', + ) + + def __init__(self, mlist, subscriber=None, *, + pre_verified=False, pre_approved=False): + """ + + :param mlist: + :param subscriber: The user or address to subscribe. + :type subscriber: ``IUser`` or ``IAddress`` + :param pre_verified: A flag indicating whether the subscriber's email + address should be considered pre-verified. Normally a never + before seen email address must be verified by mail-back + confirmation. Setting this flag to True automatically verifies + such addresses without the mail-back. (A confirmation message may + still be sent under other conditions.) + :type pre_verified: bool + :param pre_approved: A flag indicating whether, when required by the + subscription policy, a subscription request should be considered + pre-approved. Normally in such cases, the list administrator is + notified that an approval is necessary, which must be positively + acknowledged in some manner. Setting this flag to True + automatically approves the subscription request. + :type pre_approved: bool + """ + SubscriptionBase.__init__(self, mlist, subscriber) + VerificationMixin.__init__(self, pre_verified=pre_verified) + ModerationMixin.__init__(self, pre_approved=pre_approved) + + def _step_prepare(self): + self.push('do_subscription') + self.push('moderation_checks') + self.push('verification_checks') + self.push('sanity_checks') + + +@public +@implementer(ISubscriptionWorkflow) +class ConfirmModerationSubscriptionPolicy(SubscriptionBase, ConfirmationMixin, + ModerationMixin, VerificationMixin): + """""" + + name = 'sub-policy-confirm-moderate' + description = _( + 'A subscription policy that requires moderation after confirmation.') + initial_state = 'prepare' + save_attributes = ( + 'approved', + 'confirmed', + '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): + """ + + :param mlist: + :param subscriber: The user or address to subscribe. + :type subscriber: ``IUser`` or ``IAddress`` + :param pre_verified: A flag indicating whether the subscriber's email + address should be considered pre-verified. Normally a never + before seen email address must be verified by mail-back + confirmation. Setting this flag to True automatically verifies + such addresses without the mail-back. (A confirmation message may + still be sent under other conditions.) + :type pre_verified: bool + :param pre_confirmed: A flag indicating whether, when required by the + subscription policy, a subscription request should be considered + pre-confirmed. Normally in such cases, a mail-back confirmation + message is sent to the subscriber, which must be positively + acknowledged by some manner. Setting this flag to True + automatically confirms the subscription request. (A confirmation + message may still be sent under other conditions.) + :type pre_confirmed: bool + :param pre_approved: A flag indicating whether, when required by the + subscription policy, a subscription request should be considered + pre-approved. Normally in such cases, the list administrator is + notified that an approval is necessary, which must be positively + acknowledged in some manner. Setting this flag to True + automatically approves the subscription request. + :type pre_approved: bool + """ + SubscriptionBase.__init__(self, mlist, subscriber) + VerificationMixin.__init__(self, pre_verified=pre_verified) + ConfirmationMixin.__init__(self, pre_confirmed=pre_confirmed) + ModerationMixin.__init__(self, pre_approved=pre_approved) + + def _step_prepare(self): + self.push('do_subscription') + self.push('moderation_checks') + self.push('confirmation_checks') + self.push('verification_checks') + self.push('sanity_checks') -- cgit v1.2.3-70-g09d2 From bf8550eaa2b65dd6c546eaf153dfefc8d2f4fe5f Mon Sep 17 00:00:00 2001 From: J08nY Date: Tue, 4 Jul 2017 19:45:11 +0200 Subject: Split unsubscription workflow into mixins. --- src/mailman/workflows/common.py | 121 +++++++++++++++++++- src/mailman/workflows/unsubscription.py | 190 ++++++++++++++++++++++++++++++++ 2 files changed, 309 insertions(+), 2 deletions(-) create mode 100644 src/mailman/workflows/unsubscription.py (limited to 'src/mailman/workflows/common.py') diff --git a/src/mailman/workflows/common.py b/src/mailman/workflows/common.py index 79d57d0ae..d6c372b1e 100644 --- a/src/mailman/workflows/common.py +++ b/src/mailman/workflows/common.py @@ -23,15 +23,19 @@ import logging from datetime import timedelta from email.utils import formataddr from enum import Enum + +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.member import (AlreadySubscribedError, MemberRole, - MembershipIsBannedError) + MembershipIsBannedError, + NotAMemberError) from mailman.interfaces.pending import IPendable, IPendings from mailman.interfaces.subscriptions import ( - SubscriptionConfirmationNeededEvent, SubscriptionPendingError, TokenOwner) + SubscriptionConfirmationNeededEvent, SubscriptionPendingError, TokenOwner, + UnsubscriptionConfirmationNeededEvent) from mailman.interfaces.template import ITemplateLoader from mailman.interfaces.user import IUser from mailman.interfaces.usermanager import IUserManager @@ -331,3 +335,116 @@ class ModerationMixin: else: assert self.which is WhichSubscriber.user self.subscriber = self.user + + +class UnsubscriptionBase(SubscriptionWorkflowCommon): + + def __init__(self, mlist, subscriber): + super().__init__(mlist, subscriber) + if IAddress.providedBy(subscriber) or IUser.providedBy(subscriber): + self.member = self.mlist.regular_members.get_member( + self.address.email) + + def _step_subscription_checks(self): + assert self.mlist.is_subscribed(self.subscriber) + + 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') + + +class UnRequestMixin: + + 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_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 + self.confirmed = True + + +class UnConfirmationMixin(UnRequestMixin): + + def __init__(self, pre_confirmed=False): + self.confirmed = pre_confirmed + + def _step_confirmation_checks(self): + # If the unsubscription has been pre-confirmed, then we can skip the + # confirmation check can be skipped. + if self.confirmed: + return + # The user must confirm their unsubscription. + self.push('send_confirmation') + + +class UnModerationMixin(UnRequestMixin): + + def __init__(self, pre_approved=False): + self.approved = pre_approved + + def _step_moderation_checks(self): + # Does the moderator need to approve the unsubscription request? + if not self.approved: + 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 -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_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 diff --git a/src/mailman/workflows/unsubscription.py b/src/mailman/workflows/unsubscription.py new file mode 100644 index 000000000..3c4ad39de --- /dev/null +++ b/src/mailman/workflows/unsubscription.py @@ -0,0 +1,190 @@ +# 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 . + +"""""" + +from mailman.core.i18n import _ +from mailman.interfaces.workflows import IUnsubscriptionWorkflow +from mailman.workflows.common import (UnConfirmationMixin, UnModerationMixin, + UnsubscriptionBase) +from public import public +from zope.interface import implementer + + +@public +@implementer(IUnsubscriptionWorkflow) +class OpenUnsubscriptionPolicy(UnsubscriptionBase): + """""" + + name = 'unsub-policy-open' + description = _( + 'An open unsubscription policy, only requires verification.') + initial_state = 'prepare' + save_attributes = ( + 'address_key', + 'subscriber_key', + 'user_key', + 'token_owner_key', + ) + + def __init__(self, mlist, subscriber=None): + """ + + :param mlist: + :param subscriber: The user or address to unsubscribe. + :type subscriber: ``IUser`` or ``IAddress`` + """ + UnsubscriptionBase.__init__(self, mlist, subscriber) + + def _step_prepare(self): + self.push('do_unsubscription') + self.push('subscription_checks') + + +@public +@implementer(IUnsubscriptionWorkflow) +class ConfirmUnsubscriptionPolicy(UnsubscriptionBase, UnConfirmationMixin): + """""" + + name = 'unsub-policy-confirm' + description = _('An unsubscription policy that requires confirmation.') + initial_state = 'prepare' + save_attributes = ( + 'confirmed', + 'address_key', + 'subscriber_key', + 'user_key', + 'token_owner_key', + ) + + def __init__(self, mlist, subscriber=None, *, + pre_confirmed=False): + """ + + :param mlist: + :param subscriber: The user or address to unsubscribe. + :type subscriber: ``IUser`` or ``IAddress`` + :param pre_confirmed: A flag indicating whether, when required by the + unsubscription policy, an unsubscription request should be + considered pre-confirmed. Normally in such cases, a mail-back + confirmation message is sent to the subscriber, which must be + positively acknowledged by some manner. Setting this flag to True + automatically confirms the unsubscription request. (A confirmation + message may still be sent under other conditions.) + :type pre_confirmed: bool + """ + UnsubscriptionBase.__init__(self, mlist, subscriber) + UnConfirmationMixin.__init__(self, pre_confirmed=pre_confirmed) + + def _step_prepare(self): + self.push('do_unsubscription') + self.push('confirmation_checks') + self.push('subscription_checks') + + +@public +@implementer(IUnsubscriptionWorkflow) +class ModerationUnsubscriptionPolicy(UnsubscriptionBase, UnModerationMixin): + """""" + + name = 'unsub-policy-moderate' + description = _('An unsubscription policy that requires moderation.') + initial_state = 'prepare' + save_attributes = ( + 'approved', + 'address_key', + 'subscriber_key', + 'user_key', + 'token_owner_key', + ) + + def __init__(self, mlist, subscriber=None, *, + pre_approved=False): + """ + + :param mlist: + :param subscriber: The user or address to unsubscribe. + :type subscriber: ``IUser`` or ``IAddress`` + :param pre_approved: A flag indicating whether, when required by the + unsubscription policy, an unsubscription request should be + considered pre-approved. Normally in such cases, the list + administrator is notified that an approval is necessary, which + must be positively acknowledged in some manner. Setting this flag + to True automatically approves the unsubscription request. + :type pre_approved: bool + """ + UnsubscriptionBase.__init__(self, mlist, subscriber) + UnModerationMixin.__init__(self, pre_approved=pre_approved) + + def _step_prepare(self): + self.push('do_unsubscription') + self.push('moderation_checks') + self.push('subscription_checks') + + +@public +@implementer(IUnsubscriptionWorkflow) +class ConfirmModerationUnsubscriptionPolicy(UnsubscriptionBase, + UnConfirmationMixin, + UnModerationMixin): + """""" + + name = 'unsub-policy-confirm-moderate' + description = _( + 'An unsubscription policy, requires moderation after confirmation.') + initial_state = 'prepare' + save_attributes = ( + 'approved', + 'confirmed', + 'address_key', + 'subscriber_key', + 'user_key', + 'token_owner_key', + ) + + def __init__(self, mlist, subscriber=None, *, + pre_confirmed=False, pre_approved=False): + """ + + :param mlist: + :param subscriber: The user or address to unsubscribe. + :type subscriber: ``IUser`` or ``IAddress`` + :param pre_confirmed: A flag indicating whether, when required by the + unsubscription policy, an unsubscription request should be + considered pre-confirmed. Normally in such cases, a mail-back + confirmation message is sent to the subscriber, which must be + positively acknowledged by some manner. Setting this flag to True + automatically confirms the unsubscription request. (A confirmation + message may still be sent under other conditions.) + :type pre_confirmed: bool + :param pre_approved: A flag indicating whether, when required by the + unsubscription policy, an unsubscription request should be + considered pre-approved. Normally in such cases, the list + administrator is notified that an approval is necessary, which + must be positively acknowledged in some manner. Setting this flag + to True automatically approves the unsubscription request. + :type pre_approved: bool + """ + UnsubscriptionBase.__init__(self, mlist, subscriber) + UnConfirmationMixin.__init__(self, pre_confirmed=pre_confirmed) + UnModerationMixin.__init__(self, pre_approved=pre_approved) + + def _step_prepare(self): + self.push('do_unsubscription') + self.push('moderation_checks') + self.push('confirmation_checks') + self.push('subscription_checks') -- cgit v1.2.3-70-g09d2 From 7e47b06fbd20ac349b653f4cfcd2157229be1176 Mon Sep 17 00:00:00 2001 From: J08nY Date: Thu, 6 Jul 2017 18:13:47 +0200 Subject: Refactor the duplicate workflow mixins. --- src/mailman/workflows/common.py | 228 +++++++++++--------------------- src/mailman/workflows/unsubscription.py | 18 +-- 2 files changed, 88 insertions(+), 158 deletions(-) (limited to 'src/mailman/workflows/common.py') diff --git a/src/mailman/workflows/common.py b/src/mailman/workflows/common.py index d6c372b1e..8593d0997 100644 --- a/src/mailman/workflows/common.py +++ b/src/mailman/workflows/common.py @@ -58,6 +58,11 @@ class WhichSubscriber(Enum): user = 2 +class WhichWorkflow(Enum): + subscription = 1 + unsubscription = 2 + + @implementer(IPendable) class PendableSubscription(dict): PEND_TYPE = 'subscription' @@ -130,6 +135,16 @@ class SubscriptionWorkflowCommon(Workflow): def token_owner_key(self, value): self.token_owner = TokenOwner(value) + def _restore_subscriber(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 + def _set_token(self, token_owner): assert isinstance(token_owner, TokenOwner) pendings = getUtility(IPendings) @@ -163,6 +178,10 @@ class SubscriptionWorkflowCommon(Workflow): class SubscriptionBase(SubscriptionWorkflowCommon): + def __init__(self, mlist, subscriber): + super().__init__(mlist, subscriber) + self._workflow = WhichWorkflow.subscription + 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 @@ -217,27 +236,49 @@ class SubscriptionBase(SubscriptionWorkflowCommon): 'Unexpected active token at end of subscription workflow') +class UnsubscriptionBase(SubscriptionWorkflowCommon): + + def __init__(self, mlist, subscriber): + super().__init__(mlist, subscriber) + if IAddress.providedBy(subscriber) or IUser.providedBy(subscriber): + self.member = self.mlist.regular_members.get_member( + self.address.email) + self._workflow = WhichWorkflow.unsubscription + + def _step_subscription_checks(self): + assert self.mlist.is_subscribed(self.subscriber) + + 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') + + class RequestMixin: def _step_send_confirmation(self): self._set_token(TokenOwner.subscriber) self.push('do_confirm_verify') self.save() + if self._workflow is WhichWorkflow.subscription: + event_class = SubscriptionConfirmationNeededEvent + else: + event_class = UnsubscriptionConfirmationNeededEvent + # Triggering this event causes the confirmation message to be sent. - notify(SubscriptionConfirmationNeededEvent( - self.mlist, self.token, self.address.email)) + notify(event_class( + 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 + # Restore a little extra state that can't be stored in the database. + self._restore_subscriber() # 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 @@ -249,6 +290,10 @@ class RequestMixin: self.verified = True self.confirmed = True + if self._workflow is WhichWorkflow.unsubscription: + self.member = self.mlist.regular_members.get_member( + self.address.email) + class VerificationMixin(RequestMixin): @@ -265,7 +310,6 @@ class VerificationMixin(RequestMixin): # 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 class ConfirmationMixin(RequestMixin): @@ -275,11 +319,10 @@ class ConfirmationMixin(RequestMixin): def _step_confirmation_checks(self): # If the subscription has been pre-confirmed, then we can skip the - # confirmation check can be skipped. - if self.confirmed: - return - # The user must confirm their subscription. - self.push('send_confirmation') + # confirmation check. + if not self.confirmed: + # The user must confirm their subscription. + self.push('send_confirmation') class ModerationMixin: @@ -288,146 +331,36 @@ class ModerationMixin: self.approved = pre_approved def _step_moderation_checks(self): - # Does the moderator need to approve the subscription request? - if self.approved: - return - # A moderator must confirm the subscription. - self.push('get_moderator_approval') + # Does the moderator need to approve the request? + if not self.approved: + 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. + # approves of the request. If they don't, the workflow and + # 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 -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 - - -class UnsubscriptionBase(SubscriptionWorkflowCommon): - - def __init__(self, mlist, subscriber): - super().__init__(mlist, subscriber) - if IAddress.providedBy(subscriber) or IUser.providedBy(subscriber): - self.member = self.mlist.regular_members.get_member( - self.address.email) - - def _step_subscription_checks(self): - assert self.mlist.is_subscribed(self.subscriber) - - 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') - - -class UnRequestMixin: - - def _step_send_confirmation(self): - self._set_token(TokenOwner.subscriber) - self.push('do_confirm_verify') + self.push('restore') self.save() - notify(UnsubscriptionConfirmationNeededEvent( - self.mlist, self.token, self.address.email)) - 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 + if self._workflow is WhichWorkflow.subscription: + workflow_name = 'subscription' + template_name = 'list:admin:action:subscribe' 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 - self.confirmed = True - - -class UnConfirmationMixin(UnRequestMixin): - - def __init__(self, pre_confirmed=False): - self.confirmed = pre_confirmed - - def _step_confirmation_checks(self): - # If the unsubscription has been pre-confirmed, then we can skip the - # confirmation check can be skipped. - if self.confirmed: - return - # The user must confirm their unsubscription. - self.push('send_confirmation') - + workflow_name = 'unsubscription' + template_name = 'list:admin:action:unsubscribe' -class UnModerationMixin(UnRequestMixin): - - def __init__(self, pre_approved=False): - self.approved = pre_approved - - def _step_moderation_checks(self): - # Does the moderator need to approve the unsubscription request? - if not self.approved: - 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)) + log.info('{}: held {} request from {}'.format( + self.mlist.fqdn_listname, workflow_name, self.address.email)) + # Possibly send a notification to the list moderators. if self.mlist.admin_immed_notify: subject = _( - 'New unsubscription request to $self.mlist.display_name ' + 'New $workflow_name 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) + template_name, self.mlist) text = wrap(expand(template, self.mlist, dict( member=username, ))) @@ -437,14 +370,11 @@ class UnModerationMixin(UnRequestMixin): self.mlist.owner_address, self.mlist.owner_address, subject, text, self.mlist.preferred_language) msg.send(self.mlist) - # The workflow must stop running here + # The workflow must stop running here. raise StopIteration - def _step_unsubscribe_from_restored(self): + def _step_restore(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 + # Restore a little extra state that can't be stored in the database. + self._restore_subscriber() diff --git a/src/mailman/workflows/unsubscription.py b/src/mailman/workflows/unsubscription.py index 3c4ad39de..45dad92f5 100644 --- a/src/mailman/workflows/unsubscription.py +++ b/src/mailman/workflows/unsubscription.py @@ -19,7 +19,7 @@ from mailman.core.i18n import _ from mailman.interfaces.workflows import IUnsubscriptionWorkflow -from mailman.workflows.common import (UnConfirmationMixin, UnModerationMixin, +from mailman.workflows.common import (ConfirmationMixin, ModerationMixin, UnsubscriptionBase) from public import public from zope.interface import implementer @@ -57,7 +57,7 @@ class OpenUnsubscriptionPolicy(UnsubscriptionBase): @public @implementer(IUnsubscriptionWorkflow) -class ConfirmUnsubscriptionPolicy(UnsubscriptionBase, UnConfirmationMixin): +class ConfirmUnsubscriptionPolicy(UnsubscriptionBase, ConfirmationMixin): """""" name = 'unsub-policy-confirm' @@ -88,7 +88,7 @@ class ConfirmUnsubscriptionPolicy(UnsubscriptionBase, UnConfirmationMixin): :type pre_confirmed: bool """ UnsubscriptionBase.__init__(self, mlist, subscriber) - UnConfirmationMixin.__init__(self, pre_confirmed=pre_confirmed) + ConfirmationMixin.__init__(self, pre_confirmed=pre_confirmed) def _step_prepare(self): self.push('do_unsubscription') @@ -98,7 +98,7 @@ class ConfirmUnsubscriptionPolicy(UnsubscriptionBase, UnConfirmationMixin): @public @implementer(IUnsubscriptionWorkflow) -class ModerationUnsubscriptionPolicy(UnsubscriptionBase, UnModerationMixin): +class ModerationUnsubscriptionPolicy(UnsubscriptionBase, ModerationMixin): """""" name = 'unsub-policy-moderate' @@ -128,7 +128,7 @@ class ModerationUnsubscriptionPolicy(UnsubscriptionBase, UnModerationMixin): :type pre_approved: bool """ UnsubscriptionBase.__init__(self, mlist, subscriber) - UnModerationMixin.__init__(self, pre_approved=pre_approved) + ModerationMixin.__init__(self, pre_approved=pre_approved) def _step_prepare(self): self.push('do_unsubscription') @@ -139,8 +139,8 @@ class ModerationUnsubscriptionPolicy(UnsubscriptionBase, UnModerationMixin): @public @implementer(IUnsubscriptionWorkflow) class ConfirmModerationUnsubscriptionPolicy(UnsubscriptionBase, - UnConfirmationMixin, - UnModerationMixin): + ConfirmationMixin, + ModerationMixin): """""" name = 'unsub-policy-confirm-moderate' @@ -180,8 +180,8 @@ class ConfirmModerationUnsubscriptionPolicy(UnsubscriptionBase, :type pre_approved: bool """ UnsubscriptionBase.__init__(self, mlist, subscriber) - UnConfirmationMixin.__init__(self, pre_confirmed=pre_confirmed) - UnModerationMixin.__init__(self, pre_approved=pre_approved) + ConfirmationMixin.__init__(self, pre_confirmed=pre_confirmed) + ModerationMixin.__init__(self, pre_approved=pre_approved) def _step_prepare(self): self.push('do_unsubscription') -- cgit v1.2.3-70-g09d2 From eddbcf479421e234100ee7cd9b425b9c057b04ee Mon Sep 17 00:00:00 2001 From: J08nY Date: Thu, 6 Jul 2017 20:01:54 +0200 Subject: Remove [Un]SubscriptionConfirmationNeeded events, send msg in workflows. --- src/mailman/app/events.py | 2 -- src/mailman/app/subscriptions.py | 42 +-------------------------------- src/mailman/interfaces/subscriptions.py | 27 --------------------- src/mailman/workflows/common.py | 33 ++++++++++++++++++-------- 4 files changed, 24 insertions(+), 80 deletions(-) (limited to 'src/mailman/workflows/common.py') diff --git a/src/mailman/app/events.py b/src/mailman/app/events.py index bf7f2ece8..a3fca30a8 100644 --- a/src/mailman/app/events.py +++ b/src/mailman/app/events.py @@ -38,7 +38,5 @@ def initialize(): passwords.handle_ConfigurationUpdatedEvent, style_manager.handle_ConfigurationUpdatedEvent, subscriptions.handle_ListDeletingEvent, - subscriptions.handle_SubscriptionConfirmationNeededEvent, - subscriptions.handle_UnsubscriptionConfirmationNeededEvent, switchboard.handle_ConfigurationUpdatedEvent, ]) diff --git a/src/mailman/app/subscriptions.py b/src/mailman/app/subscriptions.py index e1c1d8993..cc28ea859 100644 --- a/src/mailman/app/subscriptions.py +++ b/src/mailman/app/subscriptions.py @@ -18,16 +18,11 @@ """Handle subscriptions.""" from mailman.database.transaction import flush -from mailman.email.message import UserNotification from mailman.interfaces.listmanager import ListDeletingEvent from mailman.interfaces.pending import IPendings from mailman.interfaces.subscriptions import ( - ISubscriptionManager, ISubscriptionService, - SubscriptionConfirmationNeededEvent, - UnsubscriptionConfirmationNeededEvent) -from mailman.interfaces.template import ITemplateLoader + ISubscriptionManager, ISubscriptionService) from mailman.interfaces.workflows import IWorkflowStateManager -from mailman.utilities.string import expand from mailman.workflows.common import (PendableSubscription, PendableUnsubscription) from public import public @@ -81,41 +76,6 @@ class SubscriptionManager: getUtility(IWorkflowStateManager).discard(token) -def _handle_confirmation_needed_events(event, template_name): - subject = 'confirm {}'.format(event.token) - confirm_address = event.mlist.confirm_address(event.token) - email_address = event.email - # Send a verification email to the address. - template = getUtility(ITemplateLoader).get(template_name, event.mlist) - text = expand(template, event.mlist, dict( - token=event.token, - subject=subject, - confirm_email=confirm_address, - user_email=email_address, - # For backward compatibility. - confirm_address=confirm_address, - email_address=email_address, - domain_name=event.mlist.domain.mail_host, - contact_address=event.mlist.owner_address, - )) - msg = UserNotification(email_address, confirm_address, subject, text) - msg.send(event.mlist, add_precedence=False) - - -@public -def handle_SubscriptionConfirmationNeededEvent(event): - if not isinstance(event, SubscriptionConfirmationNeededEvent): - return - _handle_confirmation_needed_events(event, 'list:user:action:subscribe') - - -@public -def handle_UnsubscriptionConfirmationNeededEvent(event): - if not isinstance(event, UnsubscriptionConfirmationNeededEvent): - return - _handle_confirmation_needed_events(event, 'list:user:action:unsubscribe') - - @public def handle_ListDeletingEvent(event): """Delete a mailing list's members when the list is being deleted.""" diff --git a/src/mailman/interfaces/subscriptions.py b/src/mailman/interfaces/subscriptions.py index 40c66cb1f..4a82da096 100644 --- a/src/mailman/interfaces/subscriptions.py +++ b/src/mailman/interfaces/subscriptions.py @@ -77,33 +77,6 @@ class TokenOwner(Enum): moderator = 2 -@public -class SubscriptionConfirmationNeededEvent: - """Triggered when a subscription needs confirmation. - - Addresses must be verified before they can receive messages or post - to mailing list. The confirmation message is sent to the user when - this event is triggered. - """ - def __init__(self, mlist, token, email): - self.mlist = mlist - self.token = token - self.email = email - - -@public -class UnsubscriptionConfirmationNeededEvent: - """Triggered when an unsubscription request needs confirmation. - - The confirmation message is sent to the user when this event is - triggered. - """ - def __init__(self, mlist, token, email): - self.mlist = mlist - self.token = token - self.email = email - - @public class ISubscriptionService(Interface): """General subscription services.""" diff --git a/src/mailman/workflows/common.py b/src/mailman/workflows/common.py index 8593d0997..41733f6e5 100644 --- a/src/mailman/workflows/common.py +++ b/src/mailman/workflows/common.py @@ -33,9 +33,8 @@ from mailman.interfaces.member import (AlreadySubscribedError, MemberRole, MembershipIsBannedError, NotAMemberError) from mailman.interfaces.pending import IPendable, IPendings -from mailman.interfaces.subscriptions import ( - SubscriptionConfirmationNeededEvent, SubscriptionPendingError, TokenOwner, - UnsubscriptionConfirmationNeededEvent) +from mailman.interfaces.subscriptions import (SubscriptionPendingError, + TokenOwner) from mailman.interfaces.template import ITemplateLoader from mailman.interfaces.user import IUser from mailman.interfaces.usermanager import IUserManager @@ -45,7 +44,6 @@ from mailman.utilities.datetime import now from mailman.utilities.string import expand, wrap from mailman.workflows.base import Workflow from zope.component import getUtility -from zope.event import notify from zope.interface import implementer from zope.interface.exceptions import DoesNotImplement @@ -266,13 +264,28 @@ class RequestMixin: self.push('do_confirm_verify') self.save() if self._workflow is WhichWorkflow.subscription: - event_class = SubscriptionConfirmationNeededEvent + template_name = 'list:user:action:subscribe' else: - event_class = UnsubscriptionConfirmationNeededEvent - - # Triggering this event causes the confirmation message to be sent. - notify(event_class( - self.mlist, self.token, self.address.email)) + template_name = 'list:user:action:unsubscribe' + + subject = 'confirm {}'.format(self.token) + confirm_address = self.mlist.confirm_address(self.token) + email_address = self.address.email + # Send a verification email to the address. + template = getUtility(ITemplateLoader).get(template_name, self.mlist) + text = expand(template, self.mlist, dict( + token=self.token, + subject=subject, + confirm_email=confirm_address, + user_email=email_address, + # For backward compatibility. + confirm_address=confirm_address, + email_address=email_address, + domain_name=self.mlist.domain.mail_host, + contact_address=self.mlist.owner_address, + )) + msg = UserNotification(email_address, confirm_address, subject, text) + msg.send(self.mlist, add_precedence=False) # Now we wait for the confirmation. raise StopIteration -- cgit v1.2.3-70-g09d2 From 139a4b484415843d4f0dcf723ed7b56fc52b2547 Mon Sep 17 00:00:00 2001 From: J08nY Date: Wed, 12 Jul 2017 18:57:01 +0200 Subject: Save workflows name in Pendable PEND_TYPE. - Saves workflow name in as Pendables type, so that it is correctly restored even if the MailingLists sub/unsub policy changes while it was pending. --- src/mailman/app/subscriptions.py | 15 +++++++------ src/mailman/rest/docs/sub-moderation.rst | 4 ++-- src/mailman/rest/sub_moderation.py | 14 +++++++++++-- src/mailman/workflows/common.py | 36 +++++++++++++++++++------------- 4 files changed, 45 insertions(+), 24 deletions(-) (limited to 'src/mailman/workflows/common.py') diff --git a/src/mailman/app/subscriptions.py b/src/mailman/app/subscriptions.py index cc28ea859..a0c0c8059 100644 --- a/src/mailman/app/subscriptions.py +++ b/src/mailman/app/subscriptions.py @@ -17,6 +17,7 @@ """Handle subscriptions.""" +from mailman.config import config from mailman.database.transaction import flush from mailman.interfaces.listmanager import ListDeletingEvent from mailman.interfaces.pending import IPendings @@ -56,11 +57,13 @@ class SubscriptionManager: if pendable is None: raise LookupError workflow_type = pendable.get('type') - assert workflow_type in (PendableSubscription.PEND_TYPE, - PendableUnsubscription.PEND_TYPE) - workflow = (self._mlist.subscription_policy - if workflow_type == PendableSubscription.PEND_TYPE - else self._mlist.unsubscription_policy)(self._mlist) + if workflow_type in {PendableSubscription.PEND_TYPE, + PendableUnsubscription.PEND_TYPE}: + workflow = (self._mlist.subscription_policy + if workflow_type == PendableSubscription.PEND_TYPE + else self._mlist.unsubscription_policy)(self._mlist) + else: + workflow = config.workflows[workflow_type](self._mlist) workflow.token = token workflow.restore() # In order to just run the whole workflow, all we need to do @@ -84,6 +87,6 @@ def handle_ListDeletingEvent(event): return # Find all the members still associated with the mailing list. members = getUtility(ISubscriptionService).find_members( - list_id=event.mailing_list.list_id) + list_id=event.mailing_list.list_id) for member in members: member.unsubscribe() diff --git a/src/mailman/rest/docs/sub-moderation.rst b/src/mailman/rest/docs/sub-moderation.rst index 3c6f30aef..8bf95fcd2 100644 --- a/src/mailman/rest/docs/sub-moderation.rst +++ b/src/mailman/rest/docs/sub-moderation.rst @@ -47,7 +47,7 @@ The subscription request can be viewed in the REST API. list_id: ant.example.com token: 0000000000000000000000000000000000000001 token_owner: moderator - type: subscription + type: sub-policy-moderate when: 2005-08-01T07:49:23 http_etag: "..." start: 0 @@ -68,7 +68,7 @@ You can view an individual membership change request by providing the token list_id: ant.example.com token: 0000000000000000000000000000000000000001 token_owner: moderator - type: subscription + type: sub-policy-moderate when: 2005-08-01T07:49:23 diff --git a/src/mailman/rest/sub_moderation.py b/src/mailman/rest/sub_moderation.py index 47745b729..ad1c5751b 100644 --- a/src/mailman/rest/sub_moderation.py +++ b/src/mailman/rest/sub_moderation.py @@ -16,13 +16,16 @@ # GNU Mailman. If not, see . """REST API for held subscription requests.""" +from itertools import chain from mailman.app.moderator import send_rejection +from mailman.config import config from mailman.core.i18n import _ from mailman.interfaces.action import Action from mailman.interfaces.member import AlreadySubscribedError from mailman.interfaces.pending import IPendings from mailman.interfaces.subscriptions import ISubscriptionManager +from mailman.interfaces.workflows import ISubscriptionWorkflow from mailman.rest.helpers import ( CollectionMixin, bad_request, child, conflict, etag, no_content, not_found, okay) @@ -122,8 +125,15 @@ class SubscriptionRequests(_ModerationBase, CollectionMixin): self._mlist = mlist def _get_collection(self, request): - pendings = getUtility(IPendings).find( - mlist=self._mlist, pend_type='subscription') + sub_workflows = [workflow_class + for workflow_class in config.workflows.values() + if ISubscriptionWorkflow.implementedBy(workflow_class) + ] + generators = [getUtility(IPendings).find(mlist=self._mlist, + pend_type=sub_workflow.name) + for + sub_workflow in sub_workflows] + pendings = chain.from_iterable(generators) return [token for token, pendable in pendings] def on_get(self, request, response): diff --git a/src/mailman/workflows/common.py b/src/mailman/workflows/common.py index 41733f6e5..1f63d3d1d 100644 --- a/src/mailman/workflows/common.py +++ b/src/mailman/workflows/common.py @@ -23,8 +23,10 @@ import logging from datetime import timedelta from email.utils import formataddr from enum import Enum +from itertools import chain from mailman.app.membership import delete_member +from mailman.config import config from mailman.core.i18n import _ from mailman.email.message import UserNotification from mailman.interfaces.address import IAddress @@ -38,14 +40,12 @@ from mailman.interfaces.subscriptions import (SubscriptionPendingError, 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, IWorkflow) +from mailman.interfaces.workflows import ISubscriptionWorkflow from mailman.utilities.datetime import now from mailman.utilities.string import expand, wrap from mailman.workflows.base import Workflow from zope.component import getUtility from zope.interface import implementer -from zope.interface.exceptions import DoesNotImplement log = logging.getLogger('mailman.subscribe') @@ -157,14 +157,7 @@ class SubscriptionWorkflowCommon(Workflow): self.token = None return - if ISubscriptionWorkflow.implementedBy(self.__class__): - pendable_class = PendableSubscription - elif IUnsubscriptionWorkflow.implementedBy(self.__class__): - pendable_class = PendableUnsubscription - else: - raise DoesNotImplement(IWorkflow) - - pendable = pendable_class( + pendable = self.pendable_class()( list_id=self.mlist.list_id, email=self.address.email, display_name=self.address.display_name, @@ -173,6 +166,13 @@ class SubscriptionWorkflowCommon(Workflow): ) self.token = pendings.add(pendable, timedelta(days=3650)) + @classmethod + def pendable_class(cls): + @implementer(IPendable) + class Pendable(dict): + PEND_TYPE = cls.name + return Pendable + class SubscriptionBase(SubscriptionWorkflowCommon): @@ -219,9 +219,17 @@ class SubscriptionBase(SubscriptionWorkflowCommon): 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') + # Look at all known subscription workflows, because any pending + # subscription workflow is exclusive. + sub_workflows = [workflow_class + for workflow_class in config.workflows.values() + if ISubscriptionWorkflow.implementedBy(workflow_class) + ] + generators = [getUtility(IPendings).find(mlist=self.mlist, + pend_type=sub_workflow.name) + for + sub_workflow in sub_workflows] + pendings = chain.from_iterable(generators) for token, pendable in pendings: if pendable['email'] == self.address.email: raise SubscriptionPendingError(self.mlist, self.address.email) -- cgit v1.2.3-70-g09d2 From b902d7858d8302d248add89a5983c521c3581c4c Mon Sep 17 00:00:00 2001 From: J08nY Date: Tue, 25 Jul 2017 23:08:35 +0200 Subject: Add more tests for coverage. --- src/mailman/app/subscriptions.py | 2 +- src/mailman/model/tests/test_mailinglist.py | 24 +++++++++++++ src/mailman/rest/tests/test_validator.py | 44 ++++++++++++++++++++++- src/mailman/workflows/common.py | 2 +- src/mailman/workflows/tests/test_subscriptions.py | 14 +++++++- 5 files changed, 82 insertions(+), 4 deletions(-) (limited to 'src/mailman/workflows/common.py') diff --git a/src/mailman/app/subscriptions.py b/src/mailman/app/subscriptions.py index a0c0c8059..ba62e9f52 100644 --- a/src/mailman/app/subscriptions.py +++ b/src/mailman/app/subscriptions.py @@ -59,7 +59,7 @@ class SubscriptionManager: workflow_type = pendable.get('type') if workflow_type in {PendableSubscription.PEND_TYPE, PendableUnsubscription.PEND_TYPE}: - workflow = (self._mlist.subscription_policy + workflow = (self._mlist.subscription_policy # pragma: nocover if workflow_type == PendableSubscription.PEND_TYPE else self._mlist.unsubscription_policy)(self._mlist) else: diff --git a/src/mailman/model/tests/test_mailinglist.py b/src/mailman/model/tests/test_mailinglist.py index 72232cb53..a402c6dc4 100644 --- a/src/mailman/model/tests/test_mailinglist.py +++ b/src/mailman/model/tests/test_mailinglist.py @@ -28,6 +28,8 @@ from mailman.interfaces.mailinglist import ( from mailman.interfaces.member import ( AlreadySubscribedError, MemberRole, MissingPreferredAddressError) from mailman.interfaces.usermanager import IUserManager +from mailman.interfaces.workflows import ( + ISubscriptionWorkflow, IUnsubscriptionWorkflow) from mailman.testing.helpers import ( configuration, get_queue_messages, set_preferred) from mailman.testing.layers import ConfigLayer @@ -408,3 +410,25 @@ class TestHeaderMatch(unittest.TestCase): header_matches = IHeaderMatchList(self._mlist) with self.assertRaises(IndexError): del header_matches[0] + + def test_subscription_policy(self): + for workflow_class in config.workflows: + if ISubscriptionWorkflow.implementedBy(workflow_class): + self._mlist.subscription_policy = workflow_class.name + self.assertEqual(self._mlist.subscription_policy, + workflow_class) + + def test_subscription_policy_invalid(self): + with self.assertRaises(ValueError): + self._mlist.subscription_policy = 'not-a-subscription-policy' + + def test_unsubscription_policy(self): + for workflow_class in config.workflows: + if IUnsubscriptionWorkflow.implementedBy(workflow_class): + self._mlist.unsubscription_policy = workflow_class.name + self.assertEqual(self._mlist.unsubscription_policy, + workflow_class) + + def test_unsubscription_policy_invalid(self): + with self.assertRaises(ValueError): + self._mlist.unsubscription_policy = 'not-an-unsubscription-policy' diff --git a/src/mailman/rest/tests/test_validator.py b/src/mailman/rest/tests/test_validator.py index 0d197032e..95855d170 100644 --- a/src/mailman/rest/tests/test_validator.py +++ b/src/mailman/rest/tests/test_validator.py @@ -22,9 +22,14 @@ import unittest from mailman.core.api import API30, API31 from mailman.interfaces.action import Action from mailman.interfaces.usermanager import IUserManager +from mailman.interfaces.workflows import (ISubscriptionWorkflow, + IUnsubscriptionWorkflow) from mailman.rest.validator import ( - enum_validator, list_of_strings_validator, subscriber_validator) + enum_validator, list_of_strings_validator, policy_validator, + subscriber_validator) from mailman.testing.layers import RESTLayer +from mailman.workflows.subscription import ConfirmSubscriptionPolicy +from mailman.workflows.unsubscription import ConfirmUnsubscriptionPolicy from zope.component import getUtility @@ -91,3 +96,40 @@ class TestValidators(unittest.TestCase): def test_enum_validator_blank(self): self.assertEqual(enum_validator(Action, allow_blank=True)(''), None) + + def test_policy_validator_wrong_policy_class(self): + self.assertRaises(ValueError, policy_validator, None) + + def test_policy_validator_sub_name(self): + self.assertEqual(policy_validator(ISubscriptionWorkflow)( + ConfirmSubscriptionPolicy.name), ConfirmSubscriptionPolicy) + + def test_policy_validator_sub_class(self): + self.assertEqual(policy_validator(ISubscriptionWorkflow)( + ConfirmSubscriptionPolicy), ConfirmSubscriptionPolicy) + + def test_policy_validator_sub_backward_compat(self): + self.assertEqual(policy_validator(ISubscriptionWorkflow)('confirm'), + ConfirmSubscriptionPolicy) + + def test_policy_validator_sub_wrong_policy(self): + validator = policy_validator(ISubscriptionWorkflow) + with self.assertRaises(ValueError): + validator('not a subscription policy') + + def test_policy_validator_unsub_name(self): + self.assertEqual(policy_validator(IUnsubscriptionWorkflow)( + ConfirmUnsubscriptionPolicy.name), ConfirmUnsubscriptionPolicy) + + def test_policy_validator_unsub_class(self): + self.assertEqual(policy_validator(IUnsubscriptionWorkflow)( + ConfirmUnsubscriptionPolicy), ConfirmUnsubscriptionPolicy) + + def test_policy_validator_unsub_backward_compat(self): + self.assertEqual(policy_validator(IUnsubscriptionWorkflow)('confirm'), + ConfirmUnsubscriptionPolicy) + + def test_policy_validator_unsub_wrong_policy(self): + validator = policy_validator(IUnsubscriptionWorkflow) + with self.assertRaises(ValueError): + validator('not an unsubscription policy') diff --git a/src/mailman/workflows/common.py b/src/mailman/workflows/common.py index 1f63d3d1d..c250785c2 100644 --- a/src/mailman/workflows/common.py +++ b/src/mailman/workflows/common.py @@ -211,7 +211,7 @@ class SubscriptionBase(SubscriptionWorkflowCommon): # 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 + raise AlreadySubscribedError( # pragma: nocover self.mlist.fqdn_listname, self.address.email, MemberRole.member) diff --git a/src/mailman/workflows/tests/test_subscriptions.py b/src/mailman/workflows/tests/test_subscriptions.py index 23487ab1c..65569691b 100644 --- a/src/mailman/workflows/tests/test_subscriptions.py +++ b/src/mailman/workflows/tests/test_subscriptions.py @@ -24,7 +24,9 @@ from mailman.app.lifecycle import create_list from mailman.interfaces.bans import IBanManager from mailman.interfaces.member import MemberRole, MembershipIsBannedError from mailman.interfaces.pending import IPendings -from mailman.interfaces.subscriptions import TokenOwner +from mailman.interfaces.subscriptions import ( + SubscriptionPendingError, + TokenOwner) from mailman.interfaces.usermanager import IUserManager from mailman.testing.helpers import ( LogFileMark, get_queue_messages, set_preferred) @@ -157,6 +159,16 @@ class TestSubscriptionWorkflow(unittest.TestCase): workflow = ConfirmSubscriptionPolicy(self._mlist, anne) self.assertRaises(MembershipIsBannedError, list, workflow) + def test_sanity_checks_already_requested(self): + # An exception is raised if there is already a subscription request. + anne = self._user_manager.create_address(self._anne) + workflow = ConfirmSubscriptionPolicy(self._mlist, anne) + list(workflow) + other_workflow = ConfirmSubscriptionPolicy(self._mlist, anne) + self.assertRaises(SubscriptionPendingError, list, other_workflow) + # The original workflow token is still in the database. + self._expected_pendings_count = 1 + def test_verification_checks_with_verified_address(self): # When the address is already verified, we skip straight to the # confirmation checks. -- cgit v1.2.3-70-g09d2