diff options
Diffstat (limited to 'src')
28 files changed, 636 insertions, 313 deletions
diff --git a/src/mailman/app/docs/moderator.rst b/src/mailman/app/docs/moderator.rst index 490c9630a..82d29074a 100644 --- a/src/mailman/app/docs/moderator.rst +++ b/src/mailman/app/docs/moderator.rst @@ -238,14 +238,16 @@ Holding subscription requests For closed lists, subscription requests will also be held for moderator approval. In this case, several pieces of information related to the subscription must be provided, including the subscriber's address and real -name, their password (possibly hashed), what kind of delivery option they are -choosing and their preferred language. +name, what kind of delivery option they are choosing and their preferred +language. >>> from mailman.app.moderator import hold_subscription >>> from mailman.interfaces.member import DeliveryMode + >>> from mailman.interfaces.subscriptions import RequestRecord >>> req_id = hold_subscription( - ... mlist, 'fred@example.org', 'Fred Person', - ... '{NONE}abcxyz', DeliveryMode.regular, 'en') + ... mlist, + ... RequestRecord('fred@example.org', 'Fred Person', + ... DeliveryMode.regular, 'en')) Disposing of membership change requests @@ -269,8 +271,9 @@ The held subscription can also be discarded. Gwen tries to subscribe to the mailing list, but... >>> req_id = hold_subscription( - ... mlist, 'gwen@example.org', 'Gwen Person', - ... '{NONE}zyxcba', DeliveryMode.regular, 'en') + ... mlist, + ... RequestRecord('gwen@example.org', 'Gwen Person', + ... DeliveryMode.regular, 'en')) ...her request is rejected... @@ -305,8 +308,9 @@ mailing list. >>> mlist.send_welcome_message = False >>> req_id = hold_subscription( - ... mlist, 'herb@example.org', 'Herb Person', - ... 'abcxyz', DeliveryMode.regular, 'en') + ... mlist, + ... RequestRecord('herb@example.org', 'Herb Person', + ... DeliveryMode.regular, 'en')) The moderators accept the subscription request. @@ -399,8 +403,9 @@ list is configured to send them. Iris tries to subscribe to the mailing list. - >>> req_id = hold_subscription(mlist, 'iris@example.org', 'Iris Person', - ... 'password', DeliveryMode.regular, 'en') + >>> req_id = hold_subscription(mlist, + ... RequestRecord('iris@example.org', 'Iris Person', + ... DeliveryMode.regular, 'en')) There's now a message in the virgin queue, destined for the list owner. @@ -491,8 +496,9 @@ can get a welcome message. >>> mlist.admin_notify_mchanges = False >>> mlist.send_welcome_message = True - >>> req_id = hold_subscription(mlist, 'kate@example.org', 'Kate Person', - ... 'password', DeliveryMode.regular, 'en') + >>> req_id = hold_subscription(mlist, + ... RequestRecord('kate@example.org', 'Kate Person', + ... DeliveryMode.regular, 'en')) >>> handle_subscription(mlist, req_id, Action.accept) >>> messages = get_queue_messages('virgin') >>> len(messages) diff --git a/src/mailman/app/membership.py b/src/mailman/app/membership.py index 996493dc4..b64291d7c 100644 --- a/src/mailman/app/membership.py +++ b/src/mailman/app/membership.py @@ -40,8 +40,7 @@ from zope.component import getUtility -def add_member(mlist, email, display_name, password, delivery_mode, language, - role=MemberRole.member): +def add_member(mlist, record, role=MemberRole.member): """Add a member right now. The member's subscription must be approved by whatever policy the list @@ -49,16 +48,8 @@ def add_member(mlist, email, display_name, password, delivery_mode, language, :param mlist: The mailing list to add the member to. :type mlist: `IMailingList` - :param email: The email address to subscribe. - :type email: str - :param display_name: The subscriber's full name. - :type display_name: str - :param password: The subscriber's plain text password. - :type password: str - :param delivery_mode: The delivery mode the subscriber has chosen. - :type delivery_mode: DeliveryMode - :param language: The language that the subscriber is going to use. - :type language: str + :param record: a subscription request record. + :type record: RequestRecord :param role: The membership role for this subscription. :type role: `MemberRole` :return: The just created member. @@ -69,62 +60,74 @@ def add_member(mlist, email, display_name, password, delivery_mode, language, :raises MembershipIsBannedError: if the membership is not allowed. """ # Check to see if the email address is banned. - if IBanManager(mlist).is_banned(email): - raise MembershipIsBannedError(mlist, email) - # See if there's already a user linked with the given address. + if IBanManager(mlist).is_banned(record.email): + raise MembershipIsBannedError(mlist, record.email) + # Make sure there is a user linked with the given address. user_manager = getUtility(IUserManager) - user = user_manager.get_user(email) - if user is None: - # A user linked to this address does not yet exist. Is the address - # itself known but just not linked to a user? - address = user_manager.get_address(email) - if address is None: - # Nope, we don't even know about this address, so create both the - # user and address now. - user = user_manager.create_user(email, display_name) - # Do it this way so we don't have to flush the previous change. - address = list(user.addresses)[0] - else: - # The address object exists, but it's not linked to a user. - # Create the user and link it now. - user = user_manager.create_user() - user.display_name = ( - display_name if display_name else address.display_name) - user.link(address) - # Encrypt the password using the currently selected hash scheme. - user.password = config.password_context.encrypt(password) - user.preferences.preferred_language = language - member = mlist.subscribe(address, role) - member.preferences.delivery_mode = delivery_mode - else: - # The user exists and is linked to the case-insensitive address. - # We're looking for two versions of the email address, the case - # preserved version and the case insensitive version. We'll - # subscribe the version with matching case if it exists, otherwise - # we'll use one of the matching case-insensitively ones. It's - # undefined which one we pick. - case_preserved = None - case_insensitive = None - for address in user.addresses: - if address.original_email == email: - case_preserved = address - if address.email == email.lower(): - case_insensitive = address - assert case_preserved is not None or case_insensitive is not None, ( - 'Could not find a linked address for: {}'.format(email)) - address = (case_preserved if case_preserved is not None - else case_insensitive) - # Create the member and set the appropriate preferences. It's - # possible we're subscribing the lower cased version of the address; - # if that's already subscribed re-issue the exception with the correct - # email address (i.e. the one passed in here). - try: - member = mlist.subscribe(address, role) - except AlreadySubscribedError as error: - raise AlreadySubscribedError( - error.fqdn_listname, email, error.role) - member.preferences.preferred_language = language - member.preferences.delivery_mode = delivery_mode + user = user_manager.make_user(record.email, record.display_name) + # Encrypt the password using the currently selected hash scheme. + user.preferences.preferred_language = record.language + # Subscribe the address, not the user. + address = user_manager.get_address(record.email) + if address is None or address.user is not user: + raise AssertionError( + 'User should have had linked address: {0}'.format(address)) + # Create the member and set the appropriate preferences. + member = mlist.subscribe(address, role) + member.preferences.preferred_language = record.language + member.preferences.delivery_mode = record.delivery_mode +# user = user_manager.get_user(email) +# if user is None: +# # A user linked to this address does not yet exist. Is the address +# # itself known but just not linked to a user? +# address = user_manager.get_address(email) +# if address is None: +# # Nope, we don't even know about this address, so create both the +# # user and address now. +# user = user_manager.create_user(email, display_name) +# # Do it this way so we don't have to flush the previous change. +# address = list(user.addresses)[0] +# else: +# # The address object exists, but it's not linked to a user. +# # Create the user and link it now. +# user = user_manager.create_user() +# user.display_name = ( +# display_name if display_name else address.display_name) +# user.link(address) +# # Encrypt the password using the currently selected hash scheme. +# user.password = config.password_context.encrypt(password) +# user.preferences.preferred_language = language +# member = mlist.subscribe(address, role) +# member.preferences.delivery_mode = delivery_mode +# else: +# # The user exists and is linked to the case-insensitive address. +# # We're looking for two versions of the email address, the case +# # preserved version and the case insensitive version. We'll +# # subscribe the version with matching case if it exists, otherwise +# # we'll use one of the matching case-insensitively ones. It's +# # undefined which one we pick. +# case_preserved = None +# case_insensitive = None +# for address in user.addresses: +# if address.original_email == email: +# case_preserved = address +# if address.email == email.lower(): +# case_insensitive = address +# assert case_preserved is not None or case_insensitive is not None, ( +# 'Could not find a linked address for: {}'.format(email)) +# address = (case_preserved if case_preserved is not None +# else case_insensitive) +# # Create the member and set the appropriate preferences. It's +# # possible we're subscribing the lower cased version of the address; +# # if that's already subscribed re-issue the exception with the correct +# # email address (i.e. the one passed in here). +# try: +# member = mlist.subscribe(address, role) +# except AlreadySubscribedError as error: +# raise AlreadySubscribedError( +# error.fqdn_listname, email, error.role) +# member.preferences.preferred_language = language +# member.preferences.delivery_mode = delivery_mode return member diff --git a/src/mailman/app/moderator.py b/src/mailman/app/moderator.py index b55388e6a..0e4d59479 100644 --- a/src/mailman/app/moderator.py +++ b/src/mailman/app/moderator.py @@ -44,6 +44,7 @@ from mailman.interfaces.member import ( AlreadySubscribedError, DeliveryMode, NotAMemberError) from mailman.interfaces.messages import IMessageStore from mailman.interfaces.requests import IListRequests, RequestType +from mailman.interfaces.subscriptions import RequestRecord from mailman.utilities.datetime import now from mailman.utilities.i18n import make from zope.component import getUtility @@ -192,26 +193,26 @@ def handle_message(mlist, id, action, -def hold_subscription(mlist, address, display_name, password, mode, language): +def hold_subscription(mlist, record): data = dict(when=now().isoformat(), - address=address, - display_name=display_name, - password=password, - delivery_mode=mode.name, - language=language) - # Now hold this request. We'll use the address as the key. + email=record.email, + display_name=record.display_name, + delivery_mode=record.delivery_mode.name, + language=record.language) + # Now hold this request. We'll use the email address as the key. requestsdb = IListRequests(mlist) request_id = requestsdb.hold_request( - RequestType.subscription, address, data) + RequestType.subscription, record.email, data) vlog.info('%s: held subscription request from %s', - mlist.fqdn_listname, address) + mlist.fqdn_listname, record.email) # Possibly notify the administrator in default list language if mlist.admin_immed_notify: + email = record.email # XXX: seems unnecessary subject = _( - 'New subscription request to $mlist.display_name from $address') + 'New subscription request to $mlist.display_name from $email') text = make('subauth.txt', mailing_list=mlist, - username=address, + username=record.email, listname=mlist.fqdn_listname, admindb_url=mlist.script_url('admindb'), ) @@ -236,19 +237,19 @@ def handle_subscription(mlist, id, action, comment=None): elif action is Action.reject: key, data = requestdb.get_request(id) _refuse(mlist, _('Subscription request'), - data['address'], + data['email'], comment or _('[No reason given]'), lang=getUtility(ILanguageManager)[data['language']]) elif action is Action.accept: key, data = requestdb.get_request(id) delivery_mode = DeliveryMode[data['delivery_mode']] - address = data['address'] + email = data['email'] display_name = data['display_name'] language = getUtility(ILanguageManager)[data['language']] - password = data['password'] try: - add_member(mlist, address, display_name, password, - delivery_mode, language) + add_member( + mlist, + RequestRecord(email, display_name, delivery_mode, language)) except AlreadySubscribedError: # The address got subscribed in some other way after the original # request was made and accepted. @@ -256,9 +257,9 @@ def handle_subscription(mlist, id, action, comment=None): else: if mlist.admin_notify_mchanges: send_admin_subscription_notice( - mlist, address, display_name, language) + mlist, email, display_name, language) slog.info('%s: new %s, %s %s', mlist.fqdn_listname, - delivery_mode, formataddr((display_name, address)), + delivery_mode, formataddr((display_name, email)), 'via admin approval') else: raise AssertionError('Unexpected action: {0}'.format(action)) @@ -267,20 +268,20 @@ def handle_subscription(mlist, id, action, comment=None): -def hold_unsubscription(mlist, address): - data = dict(address=address) +def hold_unsubscription(mlist, email): + data = dict(email=email) requestsdb = IListRequests(mlist) request_id = requestsdb.hold_request( - RequestType.unsubscription, address, data) + RequestType.unsubscription, email, data) vlog.info('%s: held unsubscription request from %s', - mlist.fqdn_listname, address) + mlist.fqdn_listname, email) # Possibly notify the administrator of the hold if mlist.admin_immed_notify: subject = _( - 'New unsubscription request from $mlist.display_name by $address') + 'New unsubscription request from $mlist.display_name by $email') text = make('unsubauth.txt', mailing_list=mlist, - address=address, + email=email, listname=mlist.fqdn_listname, admindb_url=mlist.script_url('admindb'), ) @@ -297,7 +298,7 @@ def hold_unsubscription(mlist, address): def handle_unsubscription(mlist, id, action, comment=None): requestdb = IListRequests(mlist) key, data = requestdb.get_request(id) - address = data['address'] + email = data['email'] if action is Action.defer: # Nothing to do. return @@ -306,16 +307,16 @@ def handle_unsubscription(mlist, id, action, comment=None): pass elif action is Action.reject: key, data = requestdb.get_request(id) - _refuse(mlist, _('Unsubscription request'), address, + _refuse(mlist, _('Unsubscription request'), email, comment or _('[No reason given]')) elif action is Action.accept: key, data = requestdb.get_request(id) try: - delete_member(mlist, address) + delete_member(mlist, email) except NotAMemberError: # User has already been unsubscribed. pass - slog.info('%s: deleted %s', mlist.fqdn_listname, address) + slog.info('%s: deleted %s', mlist.fqdn_listname, email) else: raise AssertionError('Unexpected action: {0}'.format(action)) # Delete the request from the database. diff --git a/src/mailman/app/subscriptions.py b/src/mailman/app/subscriptions.py index bcb9e6585..eff7c12af 100644 --- a/src/mailman/app/subscriptions.py +++ b/src/mailman/app/subscriptions.py @@ -19,28 +19,35 @@ __all__ = [ 'SubscriptionService', + 'SubscriptionWorkflow', 'handle_ListDeletingEvent', ] +from collections import deque from operator import attrgetter -from passlib.utils import generate_password as generate +#from passlib.utils import generate_password as generate from sqlalchemy import and_, or_ from uuid import UUID from zope.component import getUtility from zope.interface import implementer from mailman.app.membership import add_member, delete_member -from mailman.config import config +#from mailman.config import config +from mailman.app.moderator import hold_subscription from mailman.core.constants import system_preferences from mailman.database.transaction import dbconnection +from mailman.interfaces.address import IAddress from mailman.interfaces.listmanager import ( IListManager, ListDeletingEvent, NoSuchListError) +from mailman.interfaces.mailinglist import SubscriptionPolicy from mailman.interfaces.member import DeliveryMode, MemberRole from mailman.interfaces.subscriptions import ( - ISubscriptionService, MissingUserError) + ISubscriptionService, MissingUserError, RequestRecord) +from mailman.interfaces.user import IUser from mailman.interfaces.usermanager import IUserManager from mailman.model.member import Member +from mailman.utilities.datetime import now @@ -54,6 +61,118 @@ def _membership_sort_key(member): +class SubscriptionWorkflow: + """Workflow of a subscription request.""" + + def __init__(self, mlist, subscriber, + pre_verified, pre_confirmed, pre_approved): + self.mlist = mlist + # The subscriber must be either an IUser or IAddress. + if IAddress.providedBy(subscriber): + self.address = subscriber + self.user = self.address.user + elif IUser.providedBy(subscriber): + self.address = subscriber.preferred_address + self.user = subscriber + self.subscriber = subscriber + self.pre_verified = pre_verified + self.pre_confirmed = pre_confirmed + self.pre_approved = pre_approved + # Prepare the state machine. + self._next = deque() + self._next.append(self._verification_check) + + def __iter__(self): + return self + + def _pop(self): + step = self._next.popleft() + # step could be a partial or a method. + name = getattr(step, 'func', step).__name__ + return step, name + + def __next__(self): + try: + step, name = self._pop() + step() + except IndexError: + raise StopIteration + except: + raise + + def _maybe_set_preferred_address(self): + 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) + self.user.preferred_address = self.address + elif self.user.preferred_address is None: + assert self.address is not None, 'No address or user' + # The address has a linked user, but no preferred address is set + # yet. This is required, so use the address. + self.user.preferred_address = self.address + + def _verification_check(self): + if self.address.verified_on is not None: + # The address is already verified. Give the user a preferred + # address if it doesn't already have one. We may still have to do + # a subscription confirmation check. See below. + self._maybe_set_preferred_address() + else: + # The address is not yet verified. Maybe we're pre-verifying it. + # If so, we also want to give the user a preferred address if it + # doesn't already have one. We may still have to do a + # subscription confirmation check. See below. + if self.pre_verified: + self.address.verified_on = now() + self._maybe_set_preferred_address() + else: + # Since the address was not already verified, and not + # pre-verified, we have to send a confirmation check, which + # doubles as a verification step. Skip to that now. + self._next.append(self._send_confirmation) + return + self._next.append(self._confirmation_check) + + def _confirmation_check(self): + # Must the user confirm their subscription request? If the policy is + # open subscriptions, then we need neither confirmation nor moderator + # approval, so just subscribe them now. + if self.mlist.subscription_policy == SubscriptionPolicy.open: + self._next.append(self._do_subscription) + elif self.pre_confirmed: + # No confirmation is necessary. We can skip to seeing whether a + # moderator confirmation is necessary. + self._next.append(self._moderation_check) + else: + self._next.append(self._send_confirmation) + + def _moderation_check(self): + # Does the moderator need to approve the subscription request? + if self.mlist.subscription_policy in ( + SubscriptionPolicy.moderate, + SubscriptionPolicy.confirm_then_moderate): + self._next.append(self._get_moderator_approval) + else: + # The moderator does not need to approve the subscription, so go + # ahead and do that now. + self._next.append(self._do_subscription) + + def _get_moderator_approval(self): + # In order to get the moderator's approval, we need to hold the + # subscription request in the database + request = RequestRecord( + self.address.email, self.subscriber.display_name, + DeliveryMode.regular, 'en') + hold_subscription(self._mlist, request) + + def _do_subscription(self): + # We can immediately subscribe the user to the mailing list. + self.mlist.subscribe(self.subscriber) + + + @implementer(ISubscriptionService) class SubscriptionService: """Subscription services for the REST API.""" @@ -148,16 +267,11 @@ class SubscriptionService: if isinstance(subscriber, str): if display_name is None: display_name, at, domain = subscriber.partition('@') - # Because we want to keep the REST API simple, there is no - # password or language given to us. We'll use the system's - # default language for the user's default language. We'll set the - # password to a system default. This will have to get reset since - # it can't be retrieved. Note that none of these are used unless - # the address is completely new to us. - password = generate(int(config.passwords.password_length)) - return add_member(mlist, subscriber, display_name, password, - delivery_mode, - system_preferences.preferred_language, role) + return add_member( + mlist, + RequestRecord(subscriber, display_name, delivery_mode, + system_preferences.preferred_language), + role) else: # We have to assume it's a UUID. assert isinstance(subscriber, UUID), 'Not a UUID' diff --git a/src/mailman/app/tests/test_bounces.py b/src/mailman/app/tests/test_bounces.py index a84ef24c2..119fcf7f9 100644 --- a/src/mailman/app/tests/test_bounces.py +++ b/src/mailman/app/tests/test_bounces.py @@ -36,15 +36,15 @@ import unittest from mailman.app.bounces import ( ProbeVERP, StandardVERP, bounce_message, maybe_forward, send_probe) from mailman.app.lifecycle import create_list -from mailman.app.membership import add_member from mailman.config import config from mailman.interfaces.bounce import UnrecognizedBounceDisposition from mailman.interfaces.languages import ILanguageManager -from mailman.interfaces.member import DeliveryMode, MemberRole +from mailman.interfaces.member import MemberRole from mailman.interfaces.pending import IPendings from mailman.interfaces.usermanager import IUserManager from mailman.testing.helpers import ( - LogFileMark, get_queue_messages, specialized_message_from_string as mfs) + LogFileMark, get_queue_messages, specialized_message_from_string as mfs, + subscribe_ex) from mailman.testing.layers import ConfigLayer from zope.component import getUtility @@ -193,9 +193,8 @@ class TestSendProbe(unittest.TestCase): def setUp(self): self._mlist = create_list('test@example.com') self._mlist.send_welcome_message = False - self._member = add_member(self._mlist, 'anne@example.com', - 'Anne Person', 'xxx', - DeliveryMode.regular, 'en') + self._member = subscribe_ex( + self._mlist, 'Anne', email='anne@example.com') self._msg = mfs("""\ From: bouncer@example.com To: anne@example.com @@ -285,9 +284,8 @@ class TestSendProbeNonEnglish(unittest.TestCase): def setUp(self): self._mlist = create_list('test@example.com') - self._member = add_member(self._mlist, 'anne@example.com', - 'Anne Person', 'xxx', - DeliveryMode.regular, 'en') + self._member = subscribe_ex( + self._mlist, 'Anne', email='anne@example.com') self._msg = mfs("""\ From: bouncer@example.com To: anne@example.com @@ -351,9 +349,8 @@ class TestProbe(unittest.TestCase): def setUp(self): self._mlist = create_list('test@example.com') self._mlist.send_welcome_message = False - self._member = add_member(self._mlist, 'anne@example.com', - 'Anne Person', 'xxx', - DeliveryMode.regular, 'en') + self._member = subscribe_ex( + self._mlist, 'Anne', email='anne@example.com') self._msg = mfs("""\ From: bouncer@example.com To: anne@example.com diff --git a/src/mailman/app/tests/test_membership.py b/src/mailman/app/tests/test_membership.py index cdf0641ea..73e2ced07 100644 --- a/src/mailman/app/tests/test_membership.py +++ b/src/mailman/app/tests/test_membership.py @@ -19,7 +19,6 @@ __all__ = [ 'TestAddMember', - 'TestAddMemberPassword', 'TestDeleteMember', ] @@ -33,6 +32,7 @@ from mailman.interfaces.bans import IBanManager from mailman.interfaces.member import ( AlreadySubscribedError, DeliveryMode, MemberRole, MembershipIsBannedError, NotAMemberError) +from mailman.interfaces.subscriptions import RequestRecord from mailman.interfaces.usermanager import IUserManager from mailman.testing.layers import ConfigLayer from zope.component import getUtility @@ -48,9 +48,11 @@ class TestAddMember(unittest.TestCase): def test_add_member_new_user(self): # Test subscribing a user to a mailing list when the email address has # not yet been associated with a user. - member = add_member(self._mlist, 'aperson@example.com', - 'Anne Person', '123', DeliveryMode.regular, - system_preferences.preferred_language) + member = add_member( + self._mlist, + RequestRecord('aperson@example.com', 'Anne Person', + DeliveryMode.regular, + system_preferences.preferred_language)) self.assertEqual(member.address.email, 'aperson@example.com') self.assertEqual(member.list_id, 'test.example.com') self.assertEqual(member.role, MemberRole.member) @@ -60,9 +62,11 @@ class TestAddMember(unittest.TestCase): # already been associated with a user. user_manager = getUtility(IUserManager) user_manager.create_user('aperson@example.com', 'Anne Person') - member = add_member(self._mlist, 'aperson@example.com', - 'Anne Person', '123', DeliveryMode.regular, - system_preferences.preferred_language) + member = add_member( + self._mlist, + RequestRecord('aperson@example.com', 'Anne Person', + DeliveryMode.regular, + system_preferences.preferred_language)) self.assertEqual(member.address.email, 'aperson@example.com') self.assertEqual(member.list_id, 'test.example.com') @@ -71,9 +75,11 @@ class TestAddMember(unittest.TestCase): # subscribe to the mailing list. IBanManager(self._mlist).ban('anne@example.com') with self.assertRaises(MembershipIsBannedError) as cm: - add_member(self._mlist, 'anne@example.com', 'Anne Person', - '123', DeliveryMode.regular, - system_preferences.preferred_language) + add_member( + self._mlist, + RequestRecord('anne@example.com', 'Anne Person', + DeliveryMode.regular, + system_preferences.preferred_language)) self.assertEqual( str(cm.exception), 'anne@example.com is not allowed to subscribe to test@example.com') @@ -84,17 +90,21 @@ class TestAddMember(unittest.TestCase): IBanManager(None).ban('anne@example.com') self.assertRaises( MembershipIsBannedError, - add_member, self._mlist, 'anne@example.com', 'Anne Person', - '123', DeliveryMode.regular, system_preferences.preferred_language) + add_member, self._mlist, + RequestRecord('anne@example.com', 'Anne Person', + DeliveryMode.regular, + system_preferences.preferred_language)) def test_add_member_banned_from_different_list(self): # Test that members who are banned by on a different list can still be # subscribed to other mlists. sample_list = create_list('sample@example.com') IBanManager(sample_list).ban('anne@example.com') - member = add_member(self._mlist, 'anne@example.com', - 'Anne Person', '123', DeliveryMode.regular, - system_preferences.preferred_language) + member = add_member( + self._mlist, + RequestRecord('anne@example.com', 'Anne Person', + DeliveryMode.regular, + system_preferences.preferred_language)) self.assertEqual(member.address.email, 'anne@example.com') def test_add_member_banned_by_pattern(self): @@ -102,33 +112,41 @@ class TestAddMember(unittest.TestCase): IBanManager(self._mlist).ban('^.*@example.com') self.assertRaises( MembershipIsBannedError, - add_member, self._mlist, 'anne@example.com', 'Anne Person', - '123', DeliveryMode.regular, system_preferences.preferred_language) + add_member, self._mlist, + RequestRecord('anne@example.com', 'Anne Person', + DeliveryMode.regular, + system_preferences.preferred_language)) def test_add_member_globally_banned_by_pattern(self): # Addresses matching global regexp ban patterns cannot subscribe. IBanManager(None).ban('^.*@example.com') self.assertRaises( MembershipIsBannedError, - add_member, self._mlist, 'anne@example.com', 'Anne Person', - '123', DeliveryMode.regular, system_preferences.preferred_language) + add_member, self._mlist, + RequestRecord('anne@example.com', 'Anne Person', + DeliveryMode.regular, + system_preferences.preferred_language)) def test_add_member_banned_from_different_list_by_pattern(self): # Addresses matching regexp ban patterns on one list can still # subscribe to other mailing lists. sample_list = create_list('sample@example.com') IBanManager(sample_list).ban('^.*@example.com') - member = add_member(self._mlist, 'anne@example.com', - 'Anne Person', '123', DeliveryMode.regular, - system_preferences.preferred_language) + member = add_member( + self._mlist, + RequestRecord('anne@example.com', 'Anne Person', + DeliveryMode.regular, + system_preferences.preferred_language)) self.assertEqual(member.address.email, 'anne@example.com') def test_add_member_moderator(self): # Test adding a moderator to a mailing list. - member = add_member(self._mlist, 'aperson@example.com', - 'Anne Person', '123', DeliveryMode.regular, - system_preferences.preferred_language, - MemberRole.moderator) + member = add_member( + self._mlist, + RequestRecord('aperson@example.com', 'Anne Person', + DeliveryMode.regular, + system_preferences.preferred_language), + MemberRole.moderator) self.assertEqual(member.address.email, 'aperson@example.com') self.assertEqual(member.list_id, 'test.example.com') self.assertEqual(member.role, MemberRole.moderator) @@ -136,29 +154,37 @@ class TestAddMember(unittest.TestCase): def test_add_member_twice(self): # Adding a member with the same role twice causes an # AlreadySubscribedError to be raised. - add_member(self._mlist, 'aperson@example.com', - 'Anne Person', '123', DeliveryMode.regular, - system_preferences.preferred_language, - MemberRole.member) + add_member( + self._mlist, + RequestRecord('aperson@example.com', 'Anne Person', + DeliveryMode.regular, + system_preferences.preferred_language), + MemberRole.member) with self.assertRaises(AlreadySubscribedError) as cm: - add_member(self._mlist, 'aperson@example.com', - 'Anne Person', '123', DeliveryMode.regular, - system_preferences.preferred_language, - MemberRole.member) + add_member( + self._mlist, + RequestRecord('aperson@example.com', 'Anne Person', + DeliveryMode.regular, + system_preferences.preferred_language), + MemberRole.member) self.assertEqual(cm.exception.fqdn_listname, 'test@example.com') self.assertEqual(cm.exception.email, 'aperson@example.com') self.assertEqual(cm.exception.role, MemberRole.member) def test_add_member_with_different_roles(self): # Adding a member twice with different roles is okay. - member_1 = add_member(self._mlist, 'aperson@example.com', - 'Anne Person', '123', DeliveryMode.regular, - system_preferences.preferred_language, - MemberRole.member) - member_2 = add_member(self._mlist, 'aperson@example.com', - 'Anne Person', '123', DeliveryMode.regular, - system_preferences.preferred_language, - MemberRole.owner) + member_1 = add_member( + self._mlist, + RequestRecord('aperson@example.com', 'Anne Person', + DeliveryMode.regular, + system_preferences.preferred_language), + MemberRole.member) + member_2 = add_member( + self._mlist, + RequestRecord('aperson@example.com', 'Anne Person', + DeliveryMode.regular, + system_preferences.preferred_language), + MemberRole.owner) self.assertEqual(member_1.list_id, member_2.list_id) self.assertEqual(member_1.address, member_2.address) self.assertEqual(member_1.user, member_2.user) @@ -171,44 +197,39 @@ class TestAddMember(unittest.TestCase): # test subscribes the lower case address and ensures the original # mixed case address can't be subscribed. email = 'APerson@example.com' - add_member(self._mlist, email.lower(), 'Ann Person', '123', - DeliveryMode.regular, system_preferences.preferred_language) + add_member( + self._mlist, + RequestRecord(email.lower(), 'Ann Person', + DeliveryMode.regular, + system_preferences.preferred_language)) with self.assertRaises(AlreadySubscribedError) as cm: - add_member(self._mlist, email, 'Ann Person', '123', - DeliveryMode.regular, - system_preferences.preferred_language) - self.assertEqual(cm.exception.email, email) - + add_member( + self._mlist, + RequestRecord(email, 'Ann Person', + DeliveryMode.regular, + system_preferences.preferred_language)) + self.assertEqual(cm.exception.email, email.lower()) + def test_add_member_with_lower_case_email(self): # LP: #1425359 - Mailman is case-perserving, case-insensitive. This # test subscribes the mixed case address and ensures the lower cased # address can't be added. email = 'APerson@example.com' - add_member(self._mlist, email, 'Ann Person', '123', - DeliveryMode.regular, system_preferences.preferred_language) + add_member( + self._mlist, + RequestRecord(email, 'Ann Person', + DeliveryMode.regular, + system_preferences.preferred_language)) with self.assertRaises(AlreadySubscribedError) as cm: - add_member(self._mlist, email.lower(), 'Ann Person', '123', - DeliveryMode.regular, - system_preferences.preferred_language) + add_member( + self._mlist, + RequestRecord(email.lower(), 'Ann Person', + DeliveryMode.regular, + system_preferences.preferred_language)) self.assertEqual(cm.exception.email, email.lower()) -class TestAddMemberPassword(unittest.TestCase): - layer = ConfigLayer - - def setUp(self): - self._mlist = create_list('test@example.com') - - def test_add_member_password(self): - # Test that the password stored with the new user is encrypted. - member = add_member(self._mlist, 'anne@example.com', - 'Anne Person', 'abc', DeliveryMode.regular, - system_preferences.preferred_language) - self.assertEqual(member.user.password, '{plaintext}abc') - - - class TestDeleteMember(unittest.TestCase): layer = ConfigLayer diff --git a/src/mailman/app/tests/test_moderation.py b/src/mailman/app/tests/test_moderation.py index 72a25253e..a371d8520 100644 --- a/src/mailman/app/tests/test_moderation.py +++ b/src/mailman/app/tests/test_moderation.py @@ -19,16 +19,21 @@ __all__ = [ 'TestModeration', + 'TestUnsubscription', ] import unittest from mailman.app.lifecycle import create_list -from mailman.app.moderator import handle_message, hold_message +from mailman.app.moderator import ( + handle_message, handle_subscription, handle_unsubscription, hold_message, + hold_subscription, hold_unsubscription) from mailman.interfaces.action import Action +from mailman.interfaces.member import DeliveryMode from mailman.interfaces.messages import IMessageStore from mailman.interfaces.requests import IListRequests +from mailman.interfaces.subscriptions import RequestRecord from mailman.runners.incoming import IncomingRunner from mailman.runners.outgoing import OutgoingRunner from mailman.runners.pipeline import PipelineRunner @@ -148,3 +153,26 @@ Message-ID: <alpha> 'Forward of moderated message') self.assertEqual(messages[0].msgdata['recipients'], ['zack@example.com']) + + + +class TestUnsubscription(unittest.TestCase): + """Test unsubscription requests.""" + + layer = SMTPLayer + + def setUp(self): + self._mlist = create_list('test@example.com') + self._request_db = IListRequests(self._mlist) + + def test_unsubscribe_defer(self): + # When unsubscriptions must be approved by the moderator, but the + # moderator defers this decision. + token = hold_subscription( + self._mlist, + RequestRecord('anne@example.org', 'Anne Person', + DeliveryMode.regular, 'en')) + handle_subscription(self._mlist, token, Action.accept) + # Now hold and handle an unsubscription request. + token = hold_unsubscription(self._mlist, 'anne@example.org') + handle_unsubscription(self._mlist, token, Action.defer) diff --git a/src/mailman/app/tests/test_notifications.py b/src/mailman/app/tests/test_notifications.py index e46a50ebd..19c11d0a4 100644 --- a/src/mailman/app/tests/test_notifications.py +++ b/src/mailman/app/tests/test_notifications.py @@ -28,11 +28,11 @@ import tempfile import unittest from mailman.app.lifecycle import create_list -from mailman.app.membership import add_member from mailman.config import config from mailman.interfaces.languages import ILanguageManager -from mailman.interfaces.member import DeliveryMode, MemberRole -from mailman.testing.helpers import get_queue_messages +from mailman.interfaces.member import MemberRole +from mailman.interfaces.usermanager import IUserManager +from mailman.testing.helpers import get_queue_messages, subscribe, subscribe_ex from mailman.testing.layers import ConfigLayer from zope.component import getUtility @@ -42,6 +42,7 @@ class TestNotifications(unittest.TestCase): """Test notifications.""" layer = ConfigLayer + maxDiff = None def setUp(self): self._mlist = create_list('test@example.com') @@ -78,8 +79,7 @@ Welcome to the $list_name mailing list. shutil.rmtree(self.var_dir) def test_welcome_message(self): - add_member(self._mlist, 'anne@example.com', 'Anne Person', - 'password', DeliveryMode.regular, 'en') + subscribe(self._mlist, 'Anne', email='anne@example.com') # Now there's one message in the virgin queue. messages = get_queue_messages('virgin') self.assertEqual(len(messages), 1) @@ -104,8 +104,12 @@ Welcome to the Test List mailing list. # Add the xx language and subscribe Anne using it. manager = getUtility(ILanguageManager) manager.add('xx', 'us-ascii', 'Xlandia') - add_member(self._mlist, 'anne@example.com', 'Anne Person', - 'password', DeliveryMode.regular, 'xx') + # We can't use the subscribe_ex() helper because that would send the + # welcome message before we set the member's preferred language. + address = getUtility(IUserManager).create_address( + 'anne@example.com', 'Anne Person') + address.preferences.preferred_language = 'xx' + self._mlist.subscribe(address) # Now there's one message in the virgin queue. messages = get_queue_messages('virgin') self.assertEqual(len(messages), 1) @@ -118,27 +122,29 @@ Welcome to the Test List mailing list. def test_no_welcome_message_to_owners(self): # Welcome messages go only to mailing list members, not to owners. - add_member(self._mlist, 'anne@example.com', 'Anne Person', - 'password', DeliveryMode.regular, 'xx', - MemberRole.owner) + member = subscribe_ex( + self._mlist, 'Anne', MemberRole.owner, email='anne@example.com') + member.preferences.preferred_language = 'xx' # There is no welcome message in the virgin queue. messages = get_queue_messages('virgin') self.assertEqual(len(messages), 0) def test_no_welcome_message_to_nonmembers(self): # Welcome messages go only to mailing list members, not to nonmembers. - add_member(self._mlist, 'anne@example.com', 'Anne Person', - 'password', DeliveryMode.regular, 'xx', - MemberRole.nonmember) + member = subscribe_ex( + self._mlist, 'Anne', MemberRole.nonmember, + email='anne@example.com') + member.preferences.preferred_language = 'xx' # There is no welcome message in the virgin queue. messages = get_queue_messages('virgin') self.assertEqual(len(messages), 0) def test_no_welcome_message_to_moderators(self): # Welcome messages go only to mailing list members, not to moderators. - add_member(self._mlist, 'anne@example.com', 'Anne Person', - 'password', DeliveryMode.regular, 'xx', - MemberRole.moderator) + member = subscribe_ex( + self._mlist, 'Anne', MemberRole.moderator, + email='anne@example.com') + member.preferences.preferred_language = 'xx' # There is no welcome message in the virgin queue. messages = get_queue_messages('virgin') self.assertEqual(len(messages), 0) diff --git a/src/mailman/app/tests/test_subscriptions.py b/src/mailman/app/tests/test_subscriptions.py index 8ba5f52ff..72892b886 100644 --- a/src/mailman/app/tests/test_subscriptions.py +++ b/src/mailman/app/tests/test_subscriptions.py @@ -18,7 +18,8 @@ """Tests for the subscription service.""" __all__ = [ - 'TestJoin' + 'TestJoin', + 'TestSubscriptionWorkflow', ] @@ -26,11 +27,14 @@ import uuid import unittest from mailman.app.lifecycle import create_list +from mailman.app.subscriptions import SubscriptionWorkflow from mailman.interfaces.address import InvalidEmailAddressError from mailman.interfaces.member import MemberRole, MissingPreferredAddressError from mailman.interfaces.subscriptions import ( MissingUserError, ISubscriptionService) from mailman.testing.layers import ConfigLayer +from mailman.interfaces.mailinglist import SubscriptionPolicy +from mailman.interfaces.usermanager import IUserManager from zope.component import getUtility @@ -65,3 +69,32 @@ class TestJoin(unittest.TestCase): self._service.join, 'test.example.com', anne.user.user_id, role=MemberRole.owner) + + + +class TestSubscriptionWorkflow(unittest.TestCase): + layer = ConfigLayer + + def setUp(self): + self._mlist = create_list('test@example.com') + self._anne = 'anne@example.com' + self._user_manager = getUtility(IUserManager) + + 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') + 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. + list(workflow) + self.assertIsNotNone(anne.verified_on) + self.assertIsNotNone(anne.user) + self.assertIsNotNone(self._mlist.subscribers.get_member(self._anne)) diff --git a/src/mailman/chains/docs/moderation.rst b/src/mailman/chains/docs/moderation.rst index 1fe7e40cb..fefc4f543 100644 --- a/src/mailman/chains/docs/moderation.rst +++ b/src/mailman/chains/docs/moderation.rst @@ -32,12 +32,11 @@ deferred. The default setting for the moderation action of new members is determined by the mailing list's settings. By default, a mailing list is not set to moderate new member postings. - >>> from mailman.app.membership import add_member - >>> from mailman.interfaces.member import DeliveryMode - >>> member = add_member(mlist, 'anne@example.com', 'Anne', 'aaa', - ... DeliveryMode.regular, 'en') + >>> from mailman.testing.helpers import subscribe_ex + >>> member = subscribe_ex(mlist, 'Anne', email='anne@example.com') >>> member - <Member: Anne <anne@example.com> on test@example.com as MemberRole.member> + <Member: Anne Person <anne@example.com> + on test@example.com as MemberRole.member> >>> print(member.moderation_action) Action.defer diff --git a/src/mailman/commands/cli_members.py b/src/mailman/commands/cli_members.py index e4cad5966..ccacbeeb8 100644 --- a/src/mailman/commands/cli_members.py +++ b/src/mailman/commands/cli_members.py @@ -27,15 +27,14 @@ import codecs from email.utils import formataddr, parseaddr from mailman.app.membership import add_member -from mailman.config import config from mailman.core.i18n import _ from mailman.database.transaction import transactional from mailman.interfaces.command import ICLISubCommand from mailman.interfaces.listmanager import IListManager from mailman.interfaces.member import ( AlreadySubscribedError, DeliveryMode, DeliveryStatus) +from mailman.interfaces.subscriptions import RequestRecord from operator import attrgetter -from passlib.utils import generate_password as generate from zope.component import getUtility from zope.interface import implementer @@ -193,12 +192,11 @@ class Members: continue # Parse the line and ensure that the values are unicodes. display_name, email = parseaddr(line) - # Give the user a default, user-friendly password. - password = generate(int(config.passwords.password_length)) try: - add_member(mlist, email, display_name, password, - DeliveryMode.regular, - mlist.preferred_language.code) + add_member(mlist, + RequestRecord(email, display_name, + DeliveryMode.regular, + mlist.preferred_language.code)) except AlreadySubscribedError: # It's okay if the address is already subscribed, just # print a warning and continue. diff --git a/src/mailman/commands/docs/members.rst b/src/mailman/commands/docs/members.rst index c90418181..0f4a4afa1 100644 --- a/src/mailman/commands/docs/members.rst +++ b/src/mailman/commands/docs/members.rst @@ -34,17 +34,9 @@ options. To start with, there are no members of the mailing list. Once the mailing list add some members, they will be displayed. :: - >>> from mailman.interfaces.member import DeliveryMode - >>> from mailman.app.membership import add_member - >>> add_member(mlist1, 'anne@example.com', 'Anne Person', 'xxx', - ... DeliveryMode.regular, mlist1.preferred_language.code) - <Member: Anne Person <anne@example.com> - on test1@example.com as MemberRole.member> - >>> add_member(mlist1, 'bart@example.com', 'Bart Person', 'xxx', - ... DeliveryMode.regular, mlist1.preferred_language.code) - <Member: Bart Person <bart@example.com> - on test1@example.com as MemberRole.member> - + >>> from mailman.testing.helpers import subscribe + >>> subscribe(mlist1, 'Anne', email='anne@example.com') + >>> subscribe(mlist1, 'Bart', email='bart@example.com') >>> command.process(args) Anne Person <anne@example.com> Bart Person <bart@example.com> @@ -52,11 +44,7 @@ Once the mailing list add some members, they will be displayed. Members are displayed in alphabetical order based on their address. :: - >>> add_member(mlist1, 'anne@aaaxample.com', 'Anne Person', 'xxx', - ... DeliveryMode.regular, mlist1.preferred_language.code) - <Member: Anne Person <anne@aaaxample.com> - on test1@example.com as MemberRole.member> - + >>> subscribe(mlist1, 'Anne', email='anne@aaaxample.com') >>> command.process(args) Anne Person <anne@aaaxample.com> Anne Person <anne@example.com> @@ -92,6 +80,7 @@ Filtering on delivery mode You can limit output to just the regular non-digest members... + >>> from mailman.interfaces.member import DeliveryMode >>> args.regular = True >>> member = mlist1.members.get_member('anne@example.com') >>> member.preferences.delivery_mode = DeliveryMode.plaintext_digests @@ -136,21 +125,16 @@ status is enabled... :: >>> from mailman.interfaces.member import DeliveryStatus + >>> from mailman.testing.helpers import subscribe_ex >>> member = mlist1.members.get_member('anne@aaaxample.com') >>> member.preferences.delivery_status = DeliveryStatus.by_moderator >>> member = mlist1.members.get_member('bart@example.com') >>> member.preferences.delivery_status = DeliveryStatus.by_user - >>> member = add_member( - ... mlist1, 'cris@example.com', 'Cris Person', 'xxx', - ... DeliveryMode.regular, mlist1.preferred_language.code) + >>> member = subscribe_ex(mlist1, 'Cris', email='cris@example.com') >>> member.preferences.delivery_status = DeliveryStatus.unknown - >>> member = add_member( - ... mlist1, 'dave@example.com', 'Dave Person', 'xxx', - ... DeliveryMode.regular, mlist1.preferred_language.code) + >>> member = subscribe_ex(mlist1, 'Dave', email='dave@example.com') >>> member.preferences.delivery_status = DeliveryStatus.enabled - >>> member = add_member( - ... mlist1, 'elly@example.com', 'Elly Person', 'xxx', - ... DeliveryMode.regular, mlist1.preferred_language.code) + >>> member = subscribe_ex(mlist1, 'Elly', email='elly@example.com') >>> member.preferences.delivery_status = DeliveryStatus.by_bounces >>> args.nomail = 'enabled' diff --git a/src/mailman/interfaces/mailinglist.py b/src/mailman/interfaces/mailinglist.py index 23d2fadf4..f112b2a11 100644 --- a/src/mailman/interfaces/mailinglist.py +++ b/src/mailman/interfaces/mailinglist.py @@ -25,6 +25,7 @@ __all__ = [ 'IMailingList', 'Personalization', 'ReplyToMunging', + 'SubscriptionPolicy', ] @@ -53,6 +54,18 @@ class ReplyToMunging(Enum): explicit_header = 2 +class SubscriptionPolicy(Enum): + # Neither confirmation, nor moderator approval is required. + open = 0 + # The user must confirm the subscription. + confirm = 1 + # The moderator must approve the subscription. + moderate = 2 + # The user must first confirm their subscription, and then if that is + # successful, the moderator must also approve it. + confirm_then_moderate = 3 + + class IMailingList(Interface): """A mailing list.""" @@ -234,6 +247,9 @@ class IMailingList(Interface): deliver disabled or not, or of the type of digest they are to receive.""") + subscription_policy = Attribute( + """The policy for subscribing new members to the list.""") + subscribers = Attribute( """An iterator over all IMembers subscribed to this list, with any role. diff --git a/src/mailman/interfaces/subscriptions.py b/src/mailman/interfaces/subscriptions.py index c72a902cb..677f591ef 100644 --- a/src/mailman/interfaces/subscriptions.py +++ b/src/mailman/interfaces/subscriptions.py @@ -19,9 +19,12 @@ __all__ = [ 'ISubscriptionService', + 'RequestRecord', ] +from collections import namedtuple + from mailman.interfaces.errors import MailmanError from mailman.interfaces.member import DeliveryMode, MemberRole from zope.interface import Interface @@ -40,6 +43,19 @@ class MissingUserError(MailmanError): +_RequestRecord = namedtuple( + 'RequestRecord', + 'email display_name delivery_mode, language') +def RequestRecord(email, display_name='', + delivery_mode=DeliveryMode.regular, + language=None): + if language is None: + from mailman.core.constants import system_preferences + language = system_preferences.preferred_language + return _RequestRecord(email, display_name, delivery_mode, language) + + + class ISubscriptionService(Interface): """General Subscription services.""" diff --git a/src/mailman/interfaces/usermanager.py b/src/mailman/interfaces/usermanager.py index 798d1d127..5f3a324cc 100644 --- a/src/mailman/interfaces/usermanager.py +++ b/src/mailman/interfaces/usermanager.py @@ -43,6 +43,22 @@ class IUserManager(Interface): registered. """ + def make_user(email, display_name=None): + """Create a new user linked to an address object. + + If ``email`` is already associated with an existing `IAddress` + object, use that, otherwise create a new `IAddress`. If the + address object already points to an `IUser` return it. If a new + `IUser` is created, link the address to the user. + + :param email: The email address. + :type email: str + :param display_name: The display name. + :type display_name: str + :return: the IUser object that exists or was created. + :rtype: IUser + """ + def delete_user(user): """Delete the given user. diff --git a/src/mailman/model/docs/usermanager.rst b/src/mailman/model/docs/usermanager.rst index ba328b54b..8e40b621e 100644 --- a/src/mailman/model/docs/usermanager.rst +++ b/src/mailman/model/docs/usermanager.rst @@ -179,3 +179,25 @@ There are now four members in the system. Sort them by address then role. test.example.com bperson@example.com MemberRole.owner test.example.com eperson@example.com MemberRole.member test.example.com fperson@example.com MemberRole.member + + +Creating a new user +=================== + +A common situation (especially during the subscription life cycle) is to +create a user linked to an address, with a preferred address. Say for +example, we are asked to subscribe a new address we have never seen before. + + >>> cris = user_manager.make_user('cris@example.com', 'Cris Person') + +Since we've never seen ``cris@example.com`` before, this call creates a new +user with the given email and display name. + + >>> cris + <User "Cris Person" (5) at ...> + +The user has a single unverified address object. + + >>> for address in cris.addresses: + ... print(repr(address)) + <Address: Cris Person <cris@example.com> [not verified] at ...> diff --git a/src/mailman/model/mailinglist.py b/src/mailman/model/mailinglist.py index a204d54cd..a5ecfddbe 100644 --- a/src/mailman/model/mailinglist.py +++ b/src/mailman/model/mailinglist.py @@ -38,7 +38,7 @@ from mailman.interfaces.domain import IDomainManager from mailman.interfaces.languages import ILanguageManager from mailman.interfaces.mailinglist import ( IAcceptableAlias, IAcceptableAliasSet, IListArchiver, IListArchiverSet, - IMailingList, Personalization, ReplyToMunging) + IMailingList, Personalization, ReplyToMunging, SubscriptionPolicy) from mailman.interfaces.member import ( AlreadySubscribedError, MemberRole, MissingPreferredAddressError, SubscriptionEvent) @@ -197,6 +197,8 @@ class MailingList(Model): self._list_id = '{0}.{1}'.format(listname, hostname) # For the pending database self.next_request_id = 1 + # XXX Make this a database column: Enum(SubscriptionPolicy) + self.subscription_policy = None # We need to set up the rosters. Normally, this method will get called # when the MailingList object is loaded from the database, but when the # constructor is called, SQLAlchemy's `load` event isn't triggered. @@ -455,6 +457,8 @@ class MailingList(Model): return self.owners elif role is MemberRole.moderator: return self.moderators + elif role is MemberRole.nonmember: + return self.nonmembers else: raise TypeError('Undefined MemberRole: {}'.format(role)) diff --git a/src/mailman/model/roster.py b/src/mailman/model/roster.py index 91211c665..ef24d896b 100644 --- a/src/mailman/model/roster.py +++ b/src/mailman/model/roster.py @@ -99,9 +99,7 @@ class AbstractRoster: @dbconnection def get_member(self, store, address): """See `IRoster`.""" - results = store.query(Member).filter( - Member.list_id == self._mlist.list_id, - Member.role == self.role, + results = self._query().filter( Address.email == address, Member.address_id == Address.id) if results.count() == 0: diff --git a/src/mailman/model/tests/test_usermanager.py b/src/mailman/model/tests/test_usermanager.py index 90fcdac0c..31f1a7275 100644 --- a/src/mailman/model/tests/test_usermanager.py +++ b/src/mailman/model/tests/test_usermanager.py @@ -33,18 +33,50 @@ from zope.component import getUtility class TestUserManager(unittest.TestCase): layer = ConfigLayer + def setUp(self): + self._usermanager = getUtility(IUserManager) + def test_create_user_with_existing_address(self): # LP: #1418280. If a user is created when an email address is passed # in, and that address already exists, the user object should not get # created. - manager = getUtility(IUserManager) # Create the address we're going to try to duplicate. - manager.create_address('anne@example.com') + self._usermanager.create_address('anne@example.com') # There are no users. - self.assertEqual(len(list(manager.users)), 0) + self.assertEqual(len(list(self._usermanager.users)), 0) # Now create the user with an already existing address. with self.assertRaises(ExistingAddressError) as cm: - manager.create_user('anne@example.com') + self._usermanager.create_user('anne@example.com') self.assertEqual(cm.exception.address, 'anne@example.com') # There are still no users. - self.assertEqual(len(list(manager.users)), 0) + self.assertEqual(len(list(self._usermanager.users)), 0) + + def test_make_new_user(self): + # Neither the user nor address objects exist yet. + self.assertIsNone(self._usermanager.get_user('anne@example.com')) + self.assertIsNone(self._usermanager.get_address('anne@example.com')) + user = self._usermanager.make_user('anne@example.com', 'Anne Person') + self.assertIn('anne@example.com', + [address.email for address in user.addresses]) + addresses = list(user.addresses) + self.assertEqual(len(addresses), 1) + address = addresses[0] + self.assertEqual(address.email, 'anne@example.com') + self.assertEqual(address.display_name, 'Anne Person') + self.assertEqual(address.user.display_name, 'Anne Person') + self.assertIs(address.user, user) + + def test_make_linked_user(self): + # The address exists, but there is no linked user. + self.assertIsNone(self._usermanager.get_user('anne@example.com')) + address = self._usermanager.create_address('anne@example.com') + user = self._usermanager.make_user('anne@example.com', 'Anne Person') + self.assertIsNotNone(address.user) + self.assertIs(user, address.user) + self.assertIn(address, user.addresses) + self.assertEqual(user.display_name, 'Anne Person') + + def test_make_user_exists(self): + user = self._usermanager.create_user('anne@example.com', 'Anne Person') + other_user = self._usermanager.make_user('anne@example.com') + self.assertIs(user, other_user) diff --git a/src/mailman/model/usermanager.py b/src/mailman/model/usermanager.py index 11557bc25..9b82f10c2 100644 --- a/src/mailman/model/usermanager.py +++ b/src/mailman/model/usermanager.py @@ -46,6 +46,27 @@ class UserManager: user.link(address) return user + def make_user(self, email, display_name=None): + """See `IUserManager`.""" + # See if there's already a user linked with the given address. + user = self.get_user(email) + if user is None: + # A user linked to this address does not yet exist. Is the + # address itself known but just not linked to a user? + address = self.get_address(email) + if address is None: + # Nope, we don't even know about this address, so create both + # the user and address now. + return self.create_user(email, display_name) + # The address exists, but it's not yet linked to a user. Create + # the empty user object and link them together. + user = self.create_user() + user.display_name = ( + display_name if display_name else address.display_name) + user.link(address) + return user + return user + @dbconnection def delete_user(self, store, user): """See `IUserManager`.""" diff --git a/src/mailman/mta/tests/test_delivery.py b/src/mailman/mta/tests/test_delivery.py index 77d31d3a3..0ac8489a2 100644 --- a/src/mailman/mta/tests/test_delivery.py +++ b/src/mailman/mta/tests/test_delivery.py @@ -28,13 +28,11 @@ import tempfile import unittest from mailman.app.lifecycle import create_list -from mailman.app.membership import add_member from mailman.config import config from mailman.interfaces.mailinglist import Personalization -from mailman.interfaces.member import DeliveryMode from mailman.mta.deliver import Deliver from mailman.testing.helpers import ( - specialized_message_from_string as mfs) + specialized_message_from_string as mfs, subscribe_ex) from mailman.testing.layers import ConfigLayer @@ -63,9 +61,8 @@ class TestIndividualDelivery(unittest.TestCase): self._mlist = create_list('test@example.com') self._mlist.personalize = Personalization.individual # Make Anne a member of this mailing list. - self._anne = add_member(self._mlist, - 'anne@example.org', 'Anne Person', - 'xyz', DeliveryMode.regular, 'en') + self._anne = subscribe_ex( + self._mlist, 'Anne', email='anne@example.org') # Clear out any results from the previous test. del _deliveries[:] self._msg = mfs("""\ diff --git a/src/mailman/rest/docs/moderation.rst b/src/mailman/rest/docs/moderation.rst index 6aec921f0..bdb8d9f1c 100644 --- a/src/mailman/rest/docs/moderation.rst +++ b/src/mailman/rest/docs/moderation.rst @@ -219,21 +219,21 @@ moderator approval. >>> from mailman.app.moderator import hold_subscription >>> from mailman.interfaces.member import DeliveryMode - >>> sub_req_id = hold_subscription( - ... ant, 'anne@example.com', 'Anne Person', - ... 'password', DeliveryMode.regular, 'en') + >>> from mailman.interfaces.subscriptions import RequestRecord + >>> sub_req_id = hold_subscription(ant, + ... RequestRecord('anne@example.com', 'Anne Person', + ... DeliveryMode.regular, 'en')) >>> transaction.commit() The subscription request is available from the mailing list. >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/requests') entry 0: - address: anne@example.com delivery_mode: regular display_name: Anne Person + email: anne@example.com http_etag: "..." language: en - password: password request_id: ... type: subscription when: 2005-08-01T07:49:23 @@ -247,10 +247,9 @@ Viewing unsubscription requests Bart tries to leave a mailing list, but he may not be allowed to. - >>> from mailman.app.membership import add_member + >>> from mailman.testing.helpers import subscribe >>> from mailman.app.moderator import hold_unsubscription - >>> bart = add_member(ant, 'bart@example.com', 'Bart Person', - ... 'password', DeliveryMode.regular, 'en') + >>> bart = subscribe(ant, 'Bart', email='bart@example.com') >>> unsub_req_id = hold_unsubscription(ant, 'bart@example.com') >>> transaction.commit() @@ -258,17 +257,16 @@ The unsubscription request is also available from the mailing list. >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/requests') entry 0: - address: anne@example.com delivery_mode: regular display_name: Anne Person + email: anne@example.com http_etag: "..." language: en - password: password request_id: ... type: subscription when: 2005-08-01T07:49:23 entry 1: - address: bart@example.com + email: bart@example.com http_etag: "..." request_id: ... type: unsubscription @@ -285,12 +283,11 @@ request id. Anne's subscription request looks like this. >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/' ... 'requests/{}'.format(sub_req_id)) - address: anne@example.com delivery_mode: regular display_name: Anne Person + email: anne@example.com http_etag: "..." language: en - password: password request_id: ... type: subscription when: 2005-08-01T07:49:23 @@ -299,7 +296,7 @@ Bart's unsubscription request looks like this. >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/' ... 'requests/{}'.format(unsub_req_id)) - address: bart@example.com + email: bart@example.com http_etag: "..." request_id: ... type: unsubscription diff --git a/src/mailman/rest/root.py b/src/mailman/rest/root.py index 0861a9a5b..9ec84da68 100644 --- a/src/mailman/rest/root.py +++ b/src/mailman/rest/root.py @@ -182,6 +182,7 @@ class TopLevel: @child() def lists(self, request, segments): """/<api>/lists + /<api>/lists/styles /<api>/lists/<list> /<api>/lists/<list>/... """ diff --git a/src/mailman/rest/tests/test_moderation.py b/src/mailman/rest/tests/test_moderation.py index 262a7ec60..c77ae2aca 100644 --- a/src/mailman/rest/tests/test_moderation.py +++ b/src/mailman/rest/tests/test_moderation.py @@ -29,6 +29,7 @@ from mailman.app.moderator import hold_message, hold_subscription from mailman.config import config from mailman.database.transaction import transaction from mailman.interfaces.member import DeliveryMode +from mailman.interfaces.subscriptions import RequestRecord from mailman.testing.helpers import ( call_api, specialized_message_from_string as mfs) from mailman.testing.layers import RESTLayer @@ -76,8 +77,9 @@ Something else. # in the database. held_id = hold_message(self._mlist, self._msg) subscribe_id = hold_subscription( - self._mlist, 'bperson@example.net', 'Bart Person', 'xyz', - DeliveryMode.regular, 'en') + self._mlist, + RequestRecord('bperson@example.net', 'Bart Person', + DeliveryMode.regular, 'en')) config.db.store.commit() url = 'http://localhost:9001/3.0/lists/ant@example.com/held/{0}' with self.assertRaises(HTTPError) as cm: @@ -114,8 +116,9 @@ Something else. def test_bad_subscription_action(self): # POSTing to a held message with a bad action. held_id = hold_subscription( - self._mlist, 'cperson@example.net', 'Cris Person', 'xyz', - DeliveryMode.regular, 'en') + self._mlist, + RequestRecord('cperson@example.net', 'Cris Person', + DeliveryMode.regular, 'en')) config.db.store.commit() url = 'http://localhost:9001/3.0/lists/ant@example.com/requests/{0}' with self.assertRaises(HTTPError) as cm: diff --git a/src/mailman/runners/docs/outgoing.rst b/src/mailman/runners/docs/outgoing.rst index 7c3d1a989..598622775 100644 --- a/src/mailman/runners/docs/outgoing.rst +++ b/src/mailman/runners/docs/outgoing.rst @@ -11,24 +11,13 @@ a *delivery module*, essentially a pluggable interface for determining how the recipient set will be batched, whether messages will be personalized and VERP'd, etc. The outgoing runner doesn't itself support retrying but it can move messages to the 'retry queue' for handling delivery failures. -:: + >>> from mailman.testing.helpers import subscribe >>> mlist = create_list('test@example.com') - >>> from mailman.app.membership import add_member - >>> from mailman.interfaces.member import DeliveryMode - >>> add_member(mlist, 'aperson@example.com', 'Anne Person', - ... 'password', DeliveryMode.regular, 'en') - <Member: Anne Person <aperson@example.com> - on test@example.com as MemberRole.member> - >>> add_member(mlist, 'bperson@example.com', 'Bart Person', - ... 'password', DeliveryMode.regular, 'en') - <Member: Bart Person <bperson@example.com> - on test@example.com as MemberRole.member> - >>> add_member(mlist, 'cperson@example.com', 'Cris Person', - ... 'password', DeliveryMode.regular, 'en') - <Member: Cris Person <cperson@example.com> - on test@example.com as MemberRole.member> + >>> subscribe(mlist, 'Anne', email='aperson@example.com') + >>> subscribe(mlist, 'Bart', email='bperson@example.com') + >>> subscribe(mlist, 'Cris', email='cperson@example.com') Normally, messages would show up in the outgoing queue after the message has been processed by the rule set and pipeline. But we can simulate that here by diff --git a/src/mailman/styles/base.py b/src/mailman/styles/base.py index 50cddbc32..7a77af609 100644 --- a/src/mailman/styles/base.py +++ b/src/mailman/styles/base.py @@ -41,7 +41,8 @@ from mailman.interfaces.archiver import ArchivePolicy from mailman.interfaces.autorespond import ResponseAction from mailman.interfaces.bounce import UnrecognizedBounceDisposition from mailman.interfaces.digests import DigestFrequency -from mailman.interfaces.mailinglist import Personalization, ReplyToMunging +from mailman.interfaces.mailinglist import ( + Personalization, ReplyToMunging, SubscriptionPolicy) from mailman.interfaces.nntp import NewsgroupModeration @@ -75,6 +76,7 @@ class BasicOperation: mlist.personalize = Personalization.none mlist.default_member_action = Action.defer mlist.default_nonmember_action = Action.hold + mlist.subscription_policy = SubscriptionPolicy.confirm # Notify the administrator of pending requests and membership changes. mlist.admin_immed_notify = True mlist.admin_notify_mchanges = False diff --git a/src/mailman/templates/en/unsubauth.txt b/src/mailman/templates/en/unsubauth.txt index 5975a2ce8..e6a6a82f2 100644 --- a/src/mailman/templates/en/unsubauth.txt +++ b/src/mailman/templates/en/unsubauth.txt @@ -1,7 +1,7 @@ Your authorization is required for a mailing list unsubscription request approval: - By: $address + By: $email From: $listname At your convenience, visit: diff --git a/src/mailman/testing/helpers.py b/src/mailman/testing/helpers.py index a869c8d55..476211a08 100644 --- a/src/mailman/testing/helpers.py +++ b/src/mailman/testing/helpers.py @@ -33,6 +33,7 @@ __all__ = [ 'reset_the_world', 'specialized_message_from_string', 'subscribe', + 'subscribe_ex', 'temporary_db', 'wait_for_webservice', ] @@ -435,10 +436,11 @@ class chdir: -def subscribe(mlist, first_name, role=MemberRole.member): +def subscribe(mlist, first_name, role=MemberRole.member, email=None): """Helper for subscribing a sample person to a mailing list.""" user_manager = getUtility(IUserManager) - email = '{0}person@example.com'.format(first_name[0].lower()) + email = ('{0}person@example.com'.format(first_name[0].lower()) + if email is None else email) full_name = '{0} Person'.format(first_name) with transaction(): person = user_manager.get_user(email) @@ -446,13 +448,30 @@ def subscribe(mlist, first_name, role=MemberRole.member): address = user_manager.get_address(email) if address is None: person = user_manager.create_user(email, full_name) - preferred_address = list(person.addresses)[0] - mlist.subscribe(preferred_address, role) + subscription_address = list(person.addresses)[0] else: - mlist.subscribe(address, role) + subscription_address = address else: - preferred_address = list(person.addresses)[0] - mlist.subscribe(preferred_address, role) + subscription_address = list(person.addresses)[0] + # We can't return the newly created member because that will + # implicitly open a new transaction, which can break doctests. If you + # really need the newly created member, look it up. + mlist.subscribe(subscription_address, role) + + +def subscribe_ex(mlist, first_name, role=MemberRole.member, email=None): + """Like ``subscribe()`` but returns the newly created member object. + + Only use this in contexts where you can accept the opening of an implicit + transaction (i.e. *not* in REST tests) unless you explicitly close said + transaction. Otherwise you will lock the database. + """ + # Blarg. I wish we didn't have to duplicate this logic. + email = ('{0}person@example.com'.format(first_name[0].lower()) + if email is None else email) + subscribe(mlist, first_name, role, email) + roster = mlist.get_roster(role) + return roster.get_member(email) |