diff options
| author | Barry Warsaw | 2015-04-15 22:52:34 -0400 |
|---|---|---|
| committer | Barry Warsaw | 2015-04-15 22:52:34 -0400 |
| commit | 546b25e343c3c2aafbd29de440c1188711e98870 (patch) | |
| tree | c4d2270bce3862e71a252d941e3942860a8c8565 /src | |
| parent | 2f2e4aa6684a0930395d56a77078aa39ee7786a5 (diff) | |
| parent | 08f457799cd36349a4fd22642f4c05b4eabb306d (diff) | |
| download | mailman-546b25e343c3c2aafbd29de440c1188711e98870.tar.gz mailman-546b25e343c3c2aafbd29de440c1188711e98870.tar.zst mailman-546b25e343c3c2aafbd29de440c1188711e98870.zip | |
Diffstat (limited to 'src')
21 files changed, 585 insertions, 461 deletions
diff --git a/src/mailman/app/docs/subscriptions.rst b/src/mailman/app/docs/subscriptions.rst index eaccdc3cc..2fc59d9a7 100644 --- a/src/mailman/app/docs/subscriptions.rst +++ b/src/mailman/app/docs/subscriptions.rst @@ -2,9 +2,10 @@ Subscription services ===================== -The `ISubscriptionService` utility provides higher level convenience methods -useful for searching, retrieving, iterating, adding, and removing -memberships. +The ``ISubscriptionService`` utility provides higher level convenience methods +useful for searching, retrieving, iterating, and removing memberships across +all mailing lists on th esystem. Adding new users is handled by the +``IRegistrar`` interface. >>> from mailman.interfaces.subscriptions import ISubscriptionService >>> from zope.component import getUtility @@ -22,183 +23,154 @@ membership role. At first, there are no memberships. None -Adding new members -================== - -The service can be used to subscribe new members, by default with the `member` -role. At a minimum, a mailing list and an address for the new user is -required. - - >>> mlist = create_list('test@example.com') - >>> anne = service.join('test.example.com', 'anne@example.com') - >>> anne - <Member: anne <anne@example.com> on test@example.com as MemberRole.member> +Listing members +=============== -The real name of the new member can be given. +When there are some members, of any role on any mailing list, they can be +retrieved through the subscription service. - >>> bart = service.join('test.example.com', 'bart@example.com', - ... 'Bart Person') - >>> bart - <Member: Bart Person <bart@example.com> - on test@example.com as MemberRole.member> + >>> from mailman.app.lifecycle import create_list + >>> ant = create_list('ant@example.com') + >>> bee = create_list('bee@example.com') + >>> cat = create_list('cat@example.com') -Other roles can also be subscribed. +Some people become members. >>> from mailman.interfaces.member import MemberRole - >>> anne_owner = service.join('test.example.com', 'anne@example.com', - ... role=MemberRole.owner) - >>> anne_owner - <Member: anne <anne@example.com> on test@example.com as MemberRole.owner> + >>> from mailman.testing.helpers import subscribe + >>> anne_1 = subscribe(ant, 'Anne') + >>> anne_2 = subscribe(ant, 'Anne', MemberRole.owner) + >>> bart_1 = subscribe(ant, 'Bart', MemberRole.moderator) + >>> bart_2 = subscribe(bee, 'Bart', MemberRole.owner) + >>> anne_3 = subscribe(cat, 'Anne', email='anne@example.com') + >>> cris_1 = subscribe(cat, 'Cris') -And all the subscribed members can now be displayed. +The service can be used to iterate over them. - >>> service.get_members() - [<Member: anne <anne@example.com> on test@example.com as MemberRole.owner>, - <Member: anne <anne@example.com> on test@example.com - as MemberRole.member>, - <Member: Bart Person <bart@example.com> on test@example.com - as MemberRole.member>] - >>> sum(1 for member in service) - 3 - >>> print(service.get_member(UUID(int=3))) - <Member: anne <anne@example.com> on test@example.com as MemberRole.owner> + >>> for member in service.get_members(): + ... print(member) + <Member: Anne Person <aperson@example.com> + on ant@example.com as MemberRole.owner> + <Member: Bart Person <bperson@example.com> + on ant@example.com as MemberRole.moderator> + <Member: Anne Person <aperson@example.com> + on ant@example.com as MemberRole.member> + <Member: Bart Person <bperson@example.com> + on bee@example.com as MemberRole.owner> + <Member: Anne Person <anne@example.com> + on cat@example.com as MemberRole.member> + <Member: Cris Person <cperson@example.com> + on cat@example.com as MemberRole.member> -New members can also be added by providing an existing user id instead of an -email address. However, the user must have a preferred email address. -:: +The service can also be used to get the information about a single member. - >>> from mailman.utilities.datetime import now - >>> address = list(bart.user.addresses)[0] - >>> address.verified_on = now() - >>> bart.user.preferred_address = address - >>> service.join('test.example.com', bart.user.user_id, - ... role=MemberRole.owner) - <Member: Bart Person <bart@example.com> - on test@example.com as MemberRole.owner> + >>> print(service.get_member(bart_2.member_id)) + <Member: Bart Person <bperson@example.com> + on bee@example.com as MemberRole.owner> +There is an iteration shorthand for getting all the members. -Removing members -================ - -Regular members can also be removed. - - >>> cris = service.join('test.example.com', 'cris@example.com') - >>> service.get_members() - [<Member: anne <anne@example.com> on test@example.com - as MemberRole.owner>, - <Member: Bart Person <bart@example.com> on test@example.com - as MemberRole.owner>, - <Member: anne <anne@example.com> on test@example.com - as MemberRole.member>, - <Member: Bart Person <bart@example.com> on test@example.com - as MemberRole.member>, - <Member: cris <cris@example.com> on test@example.com - as MemberRole.member>] - >>> sum(1 for member in service) - 5 - >>> service.leave('test.example.com', 'cris@example.com') - >>> service.get_members() - [<Member: anne <anne@example.com> on test@example.com - as MemberRole.owner>, - <Member: Bart Person <bart@example.com> on test@example.com - as MemberRole.owner>, - <Member: anne <anne@example.com> on test@example.com - as MemberRole.member>, - <Member: Bart Person <bart@example.com> on test@example.com - as MemberRole.member>] - >>> sum(1 for member in service) - 4 + >>> for member in service: + ... print(member) + <Member: Anne Person <aperson@example.com> + on ant@example.com as MemberRole.owner> + <Member: Bart Person <bperson@example.com> + on ant@example.com as MemberRole.moderator> + <Member: Anne Person <aperson@example.com> + on ant@example.com as MemberRole.member> + <Member: Bart Person <bperson@example.com> + on bee@example.com as MemberRole.owner> + <Member: Anne Person <anne@example.com> + on cat@example.com as MemberRole.member> + <Member: Cris Person <cperson@example.com> + on cat@example.com as MemberRole.member> Finding members =============== -If you know the member id for a specific member, you can get that member. - - >>> service.get_member(UUID(int=3)) - <Member: anne <anne@example.com> on test@example.com as MemberRole.owner> - -If you know the member's address, you can find all their memberships, based on -specific search criteria. We start by subscribing Anne to a couple of new -mailing lists. +The subscription service can be used to find memberships based on specific +search criteria. For example, we can find all the mailing lists that Anne is +a member of with her ``aperson@example.com`` address. - >>> mlist2 = create_list('foo@example.com') - >>> mlist3 = create_list('bar@example.com') - >>> address = list(anne.user.addresses)[0] - >>> address.verified_on = now() - >>> anne.user.preferred_address = address - >>> mlist.subscribe(anne.user, MemberRole.moderator) - <Member: anne <anne@example.com> on test@example.com - as MemberRole.moderator> - >>> mlist2.subscribe(anne.user, MemberRole.member) - <Member: anne <anne@example.com> on foo@example.com as MemberRole.member> - >>> mlist3.subscribe(anne.user, MemberRole.owner) - <Member: anne <anne@example.com> on bar@example.com as MemberRole.owner> - -And now we can find all of Anne's memberships. - - >>> service.find_members('anne@example.com') - [<Member: anne <anne@example.com> on bar@example.com as MemberRole.owner>, - <Member: anne <anne@example.com> on foo@example.com as MemberRole.member>, - <Member: anne <anne@example.com> on test@example.com - as MemberRole.member>, - <Member: anne <anne@example.com> on test@example.com - as MemberRole.owner>, - <Member: anne <anne@example.com> on test@example.com - as MemberRole.moderator>] + >>> for member in service.find_members('aperson@example.com'): + ... print(member) + <Member: Anne Person <aperson@example.com> + on ant@example.com as MemberRole.member> + <Member: Anne Person <aperson@example.com> + on ant@example.com as MemberRole.owner> There may be no matching memberships. - >>> service.find_members('cris@example.com') + >>> service.find_members('dave@example.com') [] Memberships can also be searched for by user id. - >>> service.find_members(UUID(int=1)) - [<Member: anne <anne@example.com> on bar@example.com as MemberRole.owner>, - <Member: anne <anne@example.com> on foo@example.com as MemberRole.member>, - <Member: anne <anne@example.com> on test@example.com - as MemberRole.member>, - <Member: anne <anne@example.com> on test@example.com - as MemberRole.owner>, - <Member: anne <anne@example.com> on test@example.com - as MemberRole.moderator>] + >>> for member in service.find_members(anne_1.user.user_id): + ... print(member) + <Member: Anne Person <aperson@example.com> + on ant@example.com as MemberRole.member> + <Member: Anne Person <aperson@example.com> + on ant@example.com as MemberRole.owner> You can find all the memberships for a specific mailing list. - >>> service.find_members(list_id='test.example.com') - [<Member: anne <anne@example.com> on test@example.com - as MemberRole.member>, - <Member: anne <anne@example.com> on test@example.com as MemberRole.owner>, - <Member: anne <anne@example.com> on test@example.com - as MemberRole.moderator>, - <Member: Bart Person <bart@example.com> on test@example.com - as MemberRole.member>, - <Member: Bart Person <bart@example.com> on test@example.com - as MemberRole.owner>] + >>> for member in service.find_members(list_id='ant.example.com'): + ... print(member) + <Member: Anne Person <aperson@example.com> + on ant@example.com as MemberRole.member> + <Member: Anne Person <aperson@example.com> + on ant@example.com as MemberRole.owner> + <Member: Bart Person <bperson@example.com> + on ant@example.com as MemberRole.moderator> You can find all the memberships for an address on a specific mailing list, but you have to give it the list id, not the fqdn listname since the former is stable but the latter could change if the list is moved. - >>> service.find_members('anne@example.com', 'test.example.com') - [<Member: anne <anne@example.com> on test@example.com - as MemberRole.member>, - <Member: anne <anne@example.com> on test@example.com - as MemberRole.owner>, - <Member: anne <anne@example.com> on test@example.com - as MemberRole.moderator>] + >>> for member in service.find_members( + ... 'bperson@example.com', 'ant.example.com'): + ... print(member) + <Member: Bart Person <bperson@example.com> + on ant@example.com as MemberRole.moderator> You can find all the memberships for an address with a specific role. - >>> service.find_members('anne@example.com', role=MemberRole.owner) - [<Member: anne <anne@example.com> on bar@example.com as MemberRole.owner>, - <Member: anne <anne@example.com> on test@example.com - as MemberRole.owner>] + >>> for member in service.find_members( + ... list_id='ant.example.com', role=MemberRole.owner): + ... print(member) + <Member: Anne Person <aperson@example.com> + on ant@example.com as MemberRole.owner> You can also find a specific membership by all three criteria. - >>> service.find_members('anne@example.com', 'test.example.com', - ... MemberRole.owner) - [<Member: anne <anne@example.com> on test@example.com - as MemberRole.owner>] + >>> for member in service.find_members( + ... 'bperson@example.com', 'bee.example.com', MemberRole.owner): + ... print(member) + <Member: Bart Person <bperson@example.com> + on bee@example.com as MemberRole.owner> + + +Removing members +================ + +Members can be removed via this service. + + >>> len(service.get_members()) + 6 + >>> service.leave('cat.example.com', 'cperson@example.com') + >>> len(service.get_members()) + 5 + >>> for member in service: + ... print(member) + <Member: Anne Person <aperson@example.com> + on ant@example.com as MemberRole.owner> + <Member: Bart Person <bperson@example.com> + on ant@example.com as MemberRole.moderator> + <Member: Anne Person <aperson@example.com> + on ant@example.com as MemberRole.member> + <Member: Bart Person <bperson@example.com> + on bee@example.com as MemberRole.owner> + <Member: Anne Person <anne@example.com> + on cat@example.com as MemberRole.member> diff --git a/src/mailman/app/registrar.py b/src/mailman/app/registrar.py index ae4322d22..240742bc0 100644 --- a/src/mailman/app/registrar.py +++ b/src/mailman/app/registrar.py @@ -63,7 +63,7 @@ class Registrar: pre_confirmed=pre_confirmed, pre_approved=pre_approved) list(workflow) - return workflow.token + return workflow.token, workflow.token_owner, workflow.member def confirm(self, token): """See `IRegistrar`.""" @@ -71,7 +71,7 @@ class Registrar: workflow.token = token workflow.restore() list(workflow) - return workflow.token + return workflow.token, workflow.token_owner, workflow.member def discard(self, token): """See `IRegistrar`.""" diff --git a/src/mailman/app/subscriptions.py b/src/mailman/app/subscriptions.py index 3138c513b..1c6b96016 100644 --- a/src/mailman/app/subscriptions.py +++ b/src/mailman/app/subscriptions.py @@ -31,9 +31,8 @@ import logging from email.utils import formataddr from enum import Enum from datetime import timedelta -from mailman.app.membership import add_member, delete_member +from mailman.app.membership import delete_member from mailman.app.workflow import Workflow -from mailman.core.constants import system_preferences from mailman.core.i18n import _ from mailman.database.transaction import dbconnection from mailman.email.message import UserNotification @@ -42,12 +41,10 @@ from mailman.interfaces.bans import IBanManager from mailman.interfaces.listmanager import ( IListManager, ListDeletingEvent, NoSuchListError) from mailman.interfaces.mailinglist import SubscriptionPolicy -from mailman.interfaces.member import ( - DeliveryMode, MemberRole, MembershipIsBannedError) +from mailman.interfaces.member import MembershipIsBannedError from mailman.interfaces.pending import IPendable, IPendings from mailman.interfaces.registrar import ConfirmationNeededEvent -from mailman.interfaces.subscriptions import ( - ISubscriptionService, MissingUserError, RequestRecord) +from mailman.interfaces.subscriptions import ISubscriptionService, TokenOwner from mailman.interfaces.user import IUser from mailman.interfaces.usermanager import IUserManager from mailman.interfaces.workflow import IWorkflowStateManager @@ -56,7 +53,6 @@ from mailman.utilities.datetime import now from mailman.utilities.i18n import make from operator import attrgetter from sqlalchemy import and_, or_ -from uuid import UUID from zope.component import getUtility from zope.event import notify from zope.interface import implementer @@ -97,6 +93,7 @@ class SubscriptionWorkflow(Workflow): 'address_key', 'subscriber_key', 'user_key', + 'token_owner_key', ) def __init__(self, mlist, subscriber=None, *, @@ -106,6 +103,8 @@ class SubscriptionWorkflow(Workflow): 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 @@ -151,6 +150,29 @@ class SubscriptionWorkflow(Workflow): 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) + # Create a new token to prevent replay attacks. It seems like this + # should 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 = Pendable( + list_id=self.mlist.list_id, + address=self.address.email, + ) + self.token = getUtility(IPendings).add(pendable, timedelta(days=3650)) + 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 @@ -174,13 +196,7 @@ class SubscriptionWorkflow(Workflow): # Is this email address banned? if IBanManager(self.mlist).is_banned(self.address.email): raise MembershipIsBannedError(self.mlist, self.address.email) - # Create a pending record. This will give us the hash token we can use - # to uniquely name this workflow. - pendable = Pendable( - list_id=self.mlist.list_id, - address=self.address.email, - ) - self.token = getUtility(IPendings).add(pendable, timedelta(days=3650)) + # Start out with the subscriber being the token owner. self.push('verification_checks') def _step_verification_checks(self): @@ -207,10 +223,16 @@ class SubscriptionWorkflow(Workflow): if self.mlist.subscription_policy is SubscriptionPolicy.moderate: self.push('moderation_checks') return - # If the subscription has been pre-confirmed, then we can skip to the - # moderation checks. + # If the subscription has been pre-confirmed, then we can skip the + # confirmation check can be skipped. If moderator approval is + # required we need to check that, otherwise we can go straight to + # subscription. if self.pre_confirmed: - self.push('moderation_checks') + next_step = ('moderation_checks' + if self.mlist.subscription_policy is + SubscriptionPolicy.confirm_then_moderate + else 'do_subscription') + self.push(next_step) return # The user must confirm their subscription. self.push('send_confirmation') @@ -219,7 +241,8 @@ class SubscriptionWorkflow(Workflow): # Does the moderator need to approve the subscription request? assert self.mlist.subscription_policy in ( SubscriptionPolicy.moderate, - SubscriptionPolicy.confirm_then_moderate) + SubscriptionPolicy.confirm_then_moderate, + ), self.mlist.subscription_policy if self.pre_approved: self.push('do_subscription') else: @@ -229,6 +252,7 @@ class SubscriptionWorkflow(Workflow): # 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( @@ -255,6 +279,8 @@ class SubscriptionWorkflow(Workflow): 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. @@ -267,12 +293,12 @@ class SubscriptionWorkflow(Workflow): def _step_do_subscription(self): # We can immediately subscribe the user to the mailing list. - self.mlist.subscribe(self.subscriber) + self.member = self.mlist.subscribe(self.subscriber) # This workflow is done so throw away any associated state. getUtility(IWorkflowStateManager).restore(self.name, self.token) - self.token = None 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. @@ -290,14 +316,8 @@ class SubscriptionWorkflow(Workflow): else: assert self.which is WhichSubscriber.user self.subscriber = self.user - # Create a new token to prevent replay attacks. It seems like this - # should produce the same token, but it won't because the pending adds - # a bit of randomization. - pendable = Pendable( - list_id=self.mlist.list_id, - address=self.address.email, - ) - self.token = getUtility(IPendings).add(pendable, timedelta(days=3650)) + # 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. @@ -314,6 +334,7 @@ class SubscriptionWorkflow(Workflow): self.push(next_step) + @implementer(ISubscriptionService) class SubscriptionService: """Subscription services for the REST API.""" @@ -396,31 +417,6 @@ class SubscriptionService: for member in self.get_members(): yield member - def join(self, list_id, subscriber, - display_name=None, - delivery_mode=DeliveryMode.regular, - role=MemberRole.member): - """See `ISubscriptionService`.""" - mlist = getUtility(IListManager).get_by_list_id(list_id) - if mlist is None: - raise NoSuchListError(list_id) - # Is the subscriber an email address or user id? - if isinstance(subscriber, str): - if display_name is None: - display_name, at, domain = subscriber.partition('@') - 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' - user = getUtility(IUserManager).get_user_by_id(subscriber) - if user is None: - raise MissingUserError(subscriber) - return mlist.subscribe(user, role) - def leave(self, list_id, email): """See `ISubscriptionService`.""" mlist = getUtility(IListManager).get_by_list_id(list_id) diff --git a/src/mailman/app/tests/test_registrar.py b/src/mailman/app/tests/test_registrar.py index 4f5e1e3f9..a2ab2c686 100644 --- a/src/mailman/app/tests/test_registrar.py +++ b/src/mailman/app/tests/test_registrar.py @@ -28,6 +28,7 @@ from mailman.app.lifecycle import create_list from mailman.interfaces.mailinglist import SubscriptionPolicy from mailman.interfaces.pending import IPendings from mailman.interfaces.registrar import IRegistrar +from mailman.interfaces.subscriptions import TokenOwner from mailman.interfaces.usermanager import IUserManager from mailman.testing.layers import ConfigLayer from mailman.utilities.datetime import now @@ -47,37 +48,34 @@ class TestRegistrar(unittest.TestCase): self._anne = getUtility(IUserManager).create_address( 'anne@example.com') - def test_unique_token(self): + def test_initial_conditions(self): # Registering a subscription request provides a unique token associated - # with a pendable. + # with a pendable, and the owner of the token. self.assertEqual(self._pendings.count, 0) - token = self._registrar.register(self._anne) + token, token_owner, member = self._registrar.register(self._anne) self.assertIsNotNone(token) + self.assertEqual(token_owner, TokenOwner.subscriber) + self.assertIsNone(member) self.assertEqual(self._pendings.count, 1) record = self._pendings.confirm(token, expunge=False) self.assertEqual(record['list_id'], self._mlist.list_id) self.assertEqual(record['address'], 'anne@example.com') - def test_no_token(self): + def test_subscribe(self): # Registering a subscription request where no confirmation or - # moderation steps are needed, leaves us with no token, since there's - # nothing more to do. + # moderation steps are needed, leaves us with no token or owner, since + # there's nothing more to do. self._mlist.subscription_policy = SubscriptionPolicy.open self._anne.verified_on = now() - token = self._registrar.register(self._anne) + token, token_owner, rmember = self._registrar.register(self._anne) self.assertIsNone(token) - record = self._pendings.confirm(token, expunge=False) - self.assertIsNone(record) - - def test_is_subscribed(self): - # Where no confirmation or moderation steps are needed, registration - # happens immediately. - self._mlist.subscription_policy = SubscriptionPolicy.open - self._anne.verified_on = now() - status = self._registrar.register(self._anne) - self.assertIsNone(status) + self.assertEqual(token_owner, TokenOwner.no_one) member = self._mlist.regular_members.get_member('anne@example.com') + self.assertEqual(rmember, member) self.assertEqual(member.address, self._anne) + # There's nothing to confirm. + record = self._pendings.confirm(token, expunge=False) + self.assertIsNone(record) def test_no_such_token(self): # Given a token which is not in the database, a LookupError is raised. @@ -90,13 +88,18 @@ class TestRegistrar(unittest.TestCase): # to approve. Running the workflow gives us a token. Confirming the # token subscribes the user. self._mlist.subscription_policy = SubscriptionPolicy.open - token = self._registrar.register(self._anne) + token, token_owner, rmember = self._registrar.register(self._anne) self.assertIsNotNone(token) + self.assertEqual(token_owner, TokenOwner.subscriber) + self.assertIsNone(rmember) member = self._mlist.regular_members.get_member('anne@example.com') self.assertIsNone(member) # Now confirm the subscription. - self._registrar.confirm(token) + token, token_owner, rmember = self._registrar.confirm(token) + self.assertIsNone(token) + self.assertEqual(token_owner, TokenOwner.no_one) member = self._mlist.regular_members.get_member('anne@example.com') + self.assertEqual(rmember, member) self.assertEqual(member.address, self._anne) def test_confirm_because_confirm(self): @@ -106,13 +109,18 @@ class TestRegistrar(unittest.TestCase): # user. self._mlist.subscription_policy = SubscriptionPolicy.confirm self._anne.verified_on = now() - token = self._registrar.register(self._anne) + token, token_owner, rmember = self._registrar.register(self._anne) self.assertIsNotNone(token) + self.assertEqual(token_owner, TokenOwner.subscriber) + self.assertIsNone(rmember) member = self._mlist.regular_members.get_member('anne@example.com') self.assertIsNone(member) # Now confirm the subscription. - self._registrar.confirm(token) + token, token_owner, rmember = self._registrar.confirm(token) + self.assertIsNone(token) + self.assertEqual(token_owner, TokenOwner.no_one) member = self._mlist.regular_members.get_member('anne@example.com') + self.assertEqual(rmember, member) self.assertEqual(member.address, self._anne) def test_confirm_because_moderation(self): @@ -121,13 +129,18 @@ class TestRegistrar(unittest.TestCase): # token subscribes the user. self._mlist.subscription_policy = SubscriptionPolicy.moderate self._anne.verified_on = now() - token = self._registrar.register(self._anne) + token, token_owner, rmember = self._registrar.register(self._anne) self.assertIsNotNone(token) + self.assertEqual(token_owner, TokenOwner.moderator) + self.assertIsNone(rmember) member = self._mlist.regular_members.get_member('anne@example.com') self.assertIsNone(member) # Now confirm the subscription. - self._registrar.confirm(token) + token, token_owner, rmember = self._registrar.confirm(token) + self.assertIsNone(token) + self.assertEqual(token_owner, TokenOwner.no_one) member = self._mlist.regular_members.get_member('anne@example.com') + self.assertEqual(rmember, member) self.assertEqual(member.address, self._anne) def test_confirm_because_confirm_then_moderation(self): @@ -140,22 +153,30 @@ class TestRegistrar(unittest.TestCase): SubscriptionPolicy.confirm_then_moderate self._anne.verified_on = now() # Runs until subscription confirmation. - token = self._registrar.register(self._anne) + token, token_owner, rmember = self._registrar.register(self._anne) self.assertIsNotNone(token) + self.assertEqual(token_owner, TokenOwner.subscriber) + self.assertIsNone(rmember) member = self._mlist.regular_members.get_member('anne@example.com') self.assertIsNone(member) # Now confirm the subscription, and wait for the moderator to approve # the subscription. She is still not subscribed. - new_token = self._registrar.confirm(token) + new_token, token_owner, rmember = self._registrar.confirm(token) # The new token, used for the moderator to approve the message, is not # the same as the old token. self.assertNotEqual(new_token, token) + self.assertIsNotNone(new_token) + self.assertEqual(token_owner, TokenOwner.moderator) + self.assertIsNone(rmember) member = self._mlist.regular_members.get_member('anne@example.com') self.assertIsNone(member) # Confirm once more, this time as the moderator approving the # subscription. Now she's a member. - self._registrar.confirm(new_token) + token, token_owner, rmember = self._registrar.confirm(new_token) + self.assertIsNone(token) + self.assertEqual(token_owner, TokenOwner.no_one) member = self._mlist.regular_members.get_member('anne@example.com') + self.assertEqual(rmember, member) self.assertEqual(member.address, self._anne) def test_confirm_then_moderate_with_different_tokens(self): @@ -167,16 +188,20 @@ class TestRegistrar(unittest.TestCase): SubscriptionPolicy.confirm_then_moderate self._anne.verified_on = now() # Runs until subscription confirmation. - token = self._registrar.register(self._anne) + token, token_owner, rmember = self._registrar.register(self._anne) self.assertIsNotNone(token) + self.assertEqual(token_owner, TokenOwner.subscriber) + self.assertIsNone(rmember) member = self._mlist.regular_members.get_member('anne@example.com') self.assertIsNone(member) # Now confirm the subscription, and wait for the moderator to approve # the subscription. She is still not subscribed. - new_token = self._registrar.confirm(token) + new_token, token_owner, rmember = self._registrar.confirm(token) # The status is not true because the user has not yet been subscribed # to the mailing list. self.assertIsNotNone(new_token) + self.assertEqual(token_owner, TokenOwner.moderator) + self.assertIsNone(rmember) member = self._mlist.regular_members.get_member('anne@example.com') self.assertIsNone(member) # The new token is different than the old token. @@ -185,10 +210,12 @@ class TestRegistrar(unittest.TestCase): self.assertRaises(LookupError, self._registrar.confirm, token) # Confirm once more, this time with the new token, as the moderator # approving the subscription. Now she's a member. - done_token = self._registrar.confirm(new_token) + done_token, token_owner, rmember = self._registrar.confirm(new_token) # The token is None, signifying that the member has been subscribed. self.assertIsNone(done_token) + self.assertEqual(token_owner, TokenOwner.no_one) member = self._mlist.regular_members.get_member('anne@example.com') + self.assertEqual(rmember, member) self.assertEqual(member.address, self._anne) def test_discard_waiting_for_confirmation(self): @@ -197,8 +224,10 @@ class TestRegistrar(unittest.TestCase): self._mlist.subscription_policy = SubscriptionPolicy.confirm self._anne.verified_on = now() # Runs until subscription confirmation. - token = self._registrar.register(self._anne) + token, token_owner, rmember = self._registrar.register(self._anne) self.assertIsNotNone(token) + self.assertEqual(token_owner, TokenOwner.subscriber) + self.assertIsNone(rmember) member = self._mlist.regular_members.get_member('anne@example.com') self.assertIsNone(member) # Now discard the subscription request. diff --git a/src/mailman/app/tests/test_subscriptions.py b/src/mailman/app/tests/test_subscriptions.py index a4971d793..42984c77c 100644 --- a/src/mailman/app/tests/test_subscriptions.py +++ b/src/mailman/app/tests/test_subscriptions.py @@ -34,7 +34,7 @@ from mailman.interfaces.member import ( MemberRole, MembershipIsBannedError, MissingPreferredAddressError) from mailman.interfaces.pending import IPendings from mailman.interfaces.subscriptions import ( - MissingUserError, ISubscriptionService) + ISubscriptionService, MissingUserError, TokenOwner) from mailman.testing.helpers import LogFileMark, get_queue_messages from mailman.testing.layers import ConfigLayer from mailman.interfaces.mailinglist import SubscriptionPolicy @@ -45,39 +45,6 @@ from zope.component import getUtility -class TestJoin(unittest.TestCase): - layer = ConfigLayer - - def setUp(self): - self._mlist = create_list('test@example.com') - self._service = getUtility(ISubscriptionService) - - def test_join_user_with_bogus_id(self): - # When `subscriber` is a missing user id, an exception is raised. - with self.assertRaises(MissingUserError) as cm: - self._service.join('test.example.com', uuid.UUID(int=99)) - self.assertEqual(cm.exception.user_id, uuid.UUID(int=99)) - - def test_join_user_with_invalid_email_address(self): - # When `subscriber` is a string that is not an email address, an - # exception is raised. - with self.assertRaises(InvalidEmailAddressError) as cm: - self._service.join('test.example.com', 'bogus') - self.assertEqual(cm.exception.email, 'bogus') - - def test_missing_preferred_address(self): - # A user cannot join a mailing list if they have no preferred address. - anne = self._service.join( - 'test.example.com', 'anne@example.com', 'Anne Person') - # Try to join Anne as a user with a different role. Her user has no - # preferred address, so this will fail. - self.assertRaises(MissingPreferredAddressError, - self._service.join, - 'test.example.com', anne.user.user_id, - role=MemberRole.owner) - - - class TestSubscriptionWorkflow(unittest.TestCase): layer = ConfigLayer maxDiff = None @@ -88,6 +55,13 @@ class TestSubscriptionWorkflow(unittest.TestCase): self._anne = 'anne@example.com' self._user_manager = getUtility(IUserManager) + def test_start_state(self): + # The workflow starts with no tokens or member. + workflow = SubscriptionWorkflow(self._mlist) + self.assertIsNone(workflow.token) + self.assertEqual(workflow.token_owner, TokenOwner.no_one) + self.assertIsNone(workflow.member) + def test_user_or_address_required(self): # The `subscriber` attribute must be a user or address. workflow = SubscriptionWorkflow(self._mlist) @@ -229,13 +203,29 @@ class TestSubscriptionWorkflow(unittest.TestCase): def test_confirmation_checks_confirm_pre_confirmed(self): # The subscription policy requires user confirmation, but their - # subscription is pre-confirmed. + # subscription is pre-confirmed. Since moderation is not required, + # the user will be immediately subscribed. self._mlist.subscription_policy = SubscriptionPolicy.confirm anne = self._user_manager.create_address(self._anne) workflow = SubscriptionWorkflow(self._mlist, anne, pre_verified=True, pre_confirmed=True) workflow.run_thru('confirmation_checks') + with patch.object(workflow, '_step_do_subscription') as step: + next(workflow) + step.assert_called_once_with() + + def test_confirmation_checks_confirm_then_moderate_pre_confirmed(self): + # The subscription policy requires user confirmation, but their + # subscription is pre-confirmed. Since moderation is required, that + # check will be performed. + self._mlist.subscription_policy = \ + SubscriptionPolicy.confirm_then_moderate + anne = self._user_manager.create_address(self._anne) + workflow = SubscriptionWorkflow(self._mlist, anne, + pre_verified=True, + pre_confirmed=True) + workflow.run_thru('confirmation_checks') with patch.object(workflow, '_step_moderation_checks') as step: next(workflow) step.assert_called_once_with() @@ -311,6 +301,10 @@ class TestSubscriptionWorkflow(unittest.TestCase): # Anne is now a member of the mailing list. member = self._mlist.regular_members.get_member(self._anne) self.assertEqual(member.address, anne) + self.assertEqual(workflow.member, member) + # No further token is needed. + self.assertIsNone(workflow.token) + self.assertEqual(workflow.token_owner, TokenOwner.no_one) def test_do_subscription_pre_approved(self): # An moderation-requiring subscription policy plus a pre-verified and @@ -326,6 +320,10 @@ class TestSubscriptionWorkflow(unittest.TestCase): # Anne is now a member of the mailing list. member = self._mlist.regular_members.get_member(self._anne) self.assertEqual(member.address, anne) + self.assertEqual(workflow.member, member) + # No further token is needed. + self.assertIsNone(workflow.token) + self.assertEqual(workflow.token_owner, TokenOwner.no_one) def test_do_subscription_pre_approved_pre_confirmed(self): # An moderation-requiring subscription policy plus a pre-verified and @@ -343,6 +341,10 @@ class TestSubscriptionWorkflow(unittest.TestCase): # Anne is now a member of the mailing list. member = self._mlist.regular_members.get_member(self._anne) self.assertEqual(member.address, anne) + self.assertEqual(workflow.member, member) + # No further token is needed. + self.assertIsNone(workflow.token) + self.assertEqual(workflow.token_owner, TokenOwner.no_one) def test_do_subscription_cleanups(self): # Once the user is subscribed, the token, and its associated pending @@ -360,8 +362,10 @@ class TestSubscriptionWorkflow(unittest.TestCase): # Anne is now a member of the mailing list. member = self._mlist.regular_members.get_member(self._anne) self.assertEqual(member.address, anne) + self.assertEqual(workflow.member, member) # The workflow is done, so it has no token. self.assertIsNone(workflow.token) + self.assertEqual(workflow.token_owner, TokenOwner.no_one) # The pendable associated with the token has been evicted. self.assertIsNone(getUtility(IPendings).confirm(token, expunge=False)) # There is no saved workflow associated with the token. This shows up @@ -384,6 +388,10 @@ class TestSubscriptionWorkflow(unittest.TestCase): # The user is not currently subscribed to the mailing list. member = self._mlist.regular_members.get_member(self._anne) self.assertIsNone(member) + self.assertIsNone(workflow.member) + # The token is owned by the moderator. + self.assertIsNotNone(workflow.token) + self.assertEqual(workflow.token_owner, TokenOwner.moderator) # Create a new workflow with the previous workflow's save token, and # restore its state. This models an approved subscription and should # result in the user getting subscribed. @@ -394,6 +402,10 @@ class TestSubscriptionWorkflow(unittest.TestCase): # Now the user is subscribed to the mailing list. member = self._mlist.regular_members.get_member(self._anne) self.assertEqual(member.address, anne) + self.assertEqual(approved_workflow.member, member) + # No further token is needed. + self.assertIsNone(approved_workflow.token) + self.assertEqual(approved_workflow.token_owner, TokenOwner.no_one) def test_get_moderator_approval_log_on_hold(self): # When the subscription is held for moderator approval, a message is @@ -529,14 +541,26 @@ approval: self.assertIsNone(anne.verified_on) workflow = SubscriptionWorkflow(self._mlist, anne) list(workflow) - self.assertIsNone(self._mlist.regular_members.get_member(self._anne)) + # Anne is not yet a member. + member = self._mlist.regular_members.get_member(self._anne) + self.assertIsNone(member) + self.assertIsNone(workflow.member) + # The token is owned by the subscriber. + self.assertIsNotNone(workflow.token) + self.assertEqual(workflow.token_owner, TokenOwner.subscriber) + # Confirm. confirm_workflow = SubscriptionWorkflow(self._mlist) confirm_workflow.token = workflow.token confirm_workflow.restore() list(confirm_workflow) self.assertIsNotNone(anne.verified_on) - self.assertEqual( - self._mlist.regular_members.get_member(self._anne).address, anne) + # Anne is now a member. + member = self._mlist.regular_members.get_member(self._anne) + self.assertEqual(member.address, anne) + self.assertEqual(confirm_workflow.member, member) + # No further token is needed. + self.assertIsNone(confirm_workflow.token) + self.assertEqual(confirm_workflow.token_owner, TokenOwner.no_one) def test_prevent_confirmation_replay_attacks(self): # Ensure that if the workflow requires two confirmations, e.g. first @@ -553,11 +577,18 @@ approval: # Anne is not yet a member of the mailing list. member = self._mlist.regular_members.get_member(self._anne) self.assertIsNone(member) + self.assertIsNone(workflow.member) + # The token is owned by the subscriber. + self.assertIsNotNone(workflow.token) + self.assertEqual(workflow.token_owner, TokenOwner.subscriber) # The old token will not work for moderator approval. moderator_workflow = SubscriptionWorkflow(self._mlist) moderator_workflow.token = token moderator_workflow.restore() list(moderator_workflow) + # The token is owned by the moderator. + self.assertIsNotNone(moderator_workflow.token) + self.assertEqual(moderator_workflow.token_owner, TokenOwner.moderator) # While we wait for the moderator to approve the subscription, note # that there's a new token for the next steps. self.assertNotEqual(token, moderator_workflow.token) @@ -570,6 +601,7 @@ approval: # Anne is still not subscribed. member = self._mlist.regular_members.get_member(self._anne) self.assertIsNone(member) + self.assertIsNone(final_workflow.member) # However, if we use the new token, her subscription request will be # approved by the moderator. final_workflow.token = moderator_workflow.token @@ -578,3 +610,21 @@ approval: # And now Anne is a member. member = self._mlist.regular_members.get_member(self._anne) self.assertEqual(member.address.email, self._anne) + self.assertEqual(final_workflow.member, member) + # No further token is needed. + self.assertIsNone(final_workflow.token) + self.assertEqual(final_workflow.token_owner, TokenOwner.no_one) + + def test_confirmation_needed_and_pre_confirmed(self): + # The subscription policy is 'confirm' but the subscription is + # pre-confirmed so the moderation checks can be skipped. + self._mlist.subscription_policy = SubscriptionPolicy.confirm + anne = self._user_manager.create_address(self._anne) + workflow = SubscriptionWorkflow( + self._mlist, anne, + pre_verified=True, pre_confirmed=True, pre_approved=True) + list(workflow) + # Anne was subscribed. + self.assertIsNone(workflow.token) + self.assertEqual(workflow.token_owner, TokenOwner.no_one) + self.assertEqual(workflow.member.address, anne) diff --git a/src/mailman/commands/eml_confirm.py b/src/mailman/commands/eml_confirm.py index 077fab9a6..ddf0db0e2 100644 --- a/src/mailman/commands/eml_confirm.py +++ b/src/mailman/commands/eml_confirm.py @@ -25,6 +25,7 @@ __all__ = [ from mailman.core.i18n import _ from mailman.interfaces.command import ContinueProcessing, IEmailCommand from mailman.interfaces.registrar import IRegistrar +from mailman.interfaces.subscriptions import TokenOwner from zope.interface import implementer @@ -53,7 +54,15 @@ class Confirm: tokens.add(token) results.confirms = tokens try: - succeeded = (IRegistrar(mlist).confirm(token) is None) + token, token_owner, member = IRegistrar(mlist).confirm(token) + if token is None: + assert token_owner is TokenOwner.no_one, token_owner + assert member is not None, member + succeeded = True + else: + assert token_owner is not TokenOwner.no_one, token_owner + assert member is None, member + succeeded = False except LookupError: # The token must not exist in the database. succeeded = False diff --git a/src/mailman/commands/tests/test_confirm.py b/src/mailman/commands/tests/test_confirm.py index 98a26bf7d..e980141b0 100644 --- a/src/mailman/commands/tests/test_confirm.py +++ b/src/mailman/commands/tests/test_confirm.py @@ -46,7 +46,8 @@ class TestConfirm(unittest.TestCase): self._mlist = create_list('test@example.com') anne = getUtility(IUserManager).create_address( 'anne@example.com', 'Anne Person') - self._token = IRegistrar(self._mlist).register(anne) + self._token, token_owner, member = IRegistrar(self._mlist).register( + anne) self._command = Confirm() # Clear the virgin queue. get_queue_messages('virgin') diff --git a/src/mailman/interfaces/registrar.py b/src/mailman/interfaces/registrar.py index ff3f26898..6f049b9d7 100644 --- a/src/mailman/interfaces/registrar.py +++ b/src/mailman/interfaces/registrar.py @@ -52,9 +52,11 @@ class IRegistrar(Interface): This is a higher level interface to user registration, email address confirmation, etc. than the IUserManager. The latter does no validation, syntax checking, or confirmation, while this interface does. + + To use this, adapt an ``IMailingList`` to this interface. """ - def register(mlist, subscriber=None, *, + def register(subscriber=None, *, pre_verified=False, pre_confirmed=False, pre_approved=False): """Subscribe an address or user according to subscription policies. @@ -71,13 +73,14 @@ class IRegistrar(Interface): approve the subscription. Use the ``confirm(token)`` method to resume the workflow. - :param mlist: The mailing list to subscribe to. - :type mlist: `IMailingList` :param subscriber: The user or address to subscribe. :type email: ``IUser`` or ``IAddress`` - :return: The confirmation token string, or None if the workflow - completes (i.e. the member has been subscribed). - :rtype: str or None + :return: None if the workflow completes with the member being + subscribed. If the workflow is paused for user confirmation or + moderator approval, a 3-tuple is returned where the first element + is a ``TokenOwner`` the second element is the token hash, and the + third element is the subscribed member. + :rtype: None or 2-tuple of (TokenOwner, str) :raises MembershipIsBannedError: when the address being subscribed appears in the global or list-centric bans. """ diff --git a/src/mailman/interfaces/subscriptions.py b/src/mailman/interfaces/subscriptions.py index 677f591ef..e6ffd29ce 100644 --- a/src/mailman/interfaces/subscriptions.py +++ b/src/mailman/interfaces/subscriptions.py @@ -19,14 +19,16 @@ __all__ = [ 'ISubscriptionService', + 'MissingUserError', 'RequestRecord', + 'TokenOwner', ] from collections import namedtuple - +from enum import Enum from mailman.interfaces.errors import MailmanError -from mailman.interfaces.member import DeliveryMode, MemberRole +from mailman.interfaces.member import DeliveryMode from zope.interface import Interface @@ -56,6 +58,14 @@ def RequestRecord(email, display_name='', +class TokenOwner(Enum): + """Who 'owns' the token returned from the registrar?""" + no_one = 0 + subscriber = 1 + moderator = 2 + + + class ISubscriptionService(Interface): """General Subscription services.""" @@ -104,44 +114,6 @@ class ISubscriptionService(Interface): def __iter__(): """See `get_members()`.""" - def join(list_id, subscriber, display_name=None, - delivery_mode=DeliveryMode.regular, - role=MemberRole.member): - """Subscribe to a mailing list. - - A user for the address is created if it is not yet known to Mailman, - however newly registered addresses will not yet be validated. No - confirmation message will be sent to the address, and the approval of - the subscription request is still dependent on the policy of the - mailing list. - - :param list_id: The list id of the mailing list the user is - subscribing to. - :type list_id: string - :param subscriber: The email address or user id of the user getting - subscribed. - :type subscriber: string or int - :param display_name: The name of the user. This is only used if a new - user is created, and it defaults to the local part of the email - address if not given. - :type display_name: string - :param delivery_mode: The delivery mode for this subscription. This - can be one of the enum values of `DeliveryMode`. If not given, - regular delivery is assumed. - :type delivery_mode: string - :param role: The membership role for this subscription. - :type role: `MemberRole` - :return: The just created member. - :rtype: `IMember` - :raises AlreadySubscribedError: if the user is already subscribed to - the mailing list. - :raises InvalidEmailAddressError: if the email address is not valid. - :raises MembershipIsBannedError: if the membership is not allowed. - :raises MissingUserError: when a bogus user id is given. - :raises NoSuchListError: if the named mailing list does not exist. - :raises ValueError: when `delivery_mode` is invalid. - """ - def leave(list_id, email): """Unsubscribe from a mailing list. diff --git a/src/mailman/model/docs/registration.rst b/src/mailman/model/docs/registration.rst index fc7ad6f1a..4b1e13520 100644 --- a/src/mailman/model/docs/registration.rst +++ b/src/mailman/model/docs/registration.rst @@ -35,11 +35,13 @@ which represents this work flow. Anne attempts to join the mailing list. - >>> token = registrar.register(anne) + >>> token, token_owner, member = registrar.register(anne) Because her email address has not yet been verified, she has not yet become a member of the mailing list. + >>> print(member) + None >>> print(mlist.members.get_member('anne@example.com')) None @@ -47,7 +49,10 @@ Once she verifies her email address, she will become a member of the mailing list. In this case, verifying implies that she also confirms her wish to join the mailing list. - >>> registrar.confirm(token) + >>> token, token_owner, member = registrar.confirm(token) + >>> member + <Member: Anne Person <anne@example.com> on ant@example.com + as MemberRole.member> >>> mlist.members.get_member('anne@example.com') <Member: Anne Person <anne@example.com> on ant@example.com as MemberRole.member> @@ -78,13 +83,18 @@ Now when Bart registers as a user for the mailing list, a token will still be generated, but this is only used by the moderator. At first, Bart is not subscribed to the mailing list. - >>> token = registrar.register(bart) + >>> token, token_owner, member = registrar.register(bart) + >>> print(member) + None >>> print(mlist.members.get_member('bart@example.com')) None When the moderator confirms Bart's subscription, he joins the mailing list. - >>> registrar.confirm(token) + >>> token, token_owner, member = registrar.confirm(token) + >>> member + <Member: Bart Person <bart@example.com> on ant@example.com + as MemberRole.member> >>> mlist.members.get_member('bart@example.com') <Member: Bart Person <bart@example.com> on ant@example.com as MemberRole.member> diff --git a/src/mailman/rest/docs/membership.rst b/src/mailman/rest/docs/membership.rst index 3cb83a0c8..f3a09cfe9 100644 --- a/src/mailman/rest/docs/membership.rst +++ b/src/mailman/rest/docs/membership.rst @@ -598,14 +598,17 @@ A user can be subscribed to a mailing list via the REST API, either by a specific address, or more generally by their preferred address. A subscribed user is called a member. -Elly wants to subscribes to the `ant` mailing list. Since Elly's email -address is not yet known to Mailman, a user is created for her. By default, -get gets a regular delivery. +The list owner wants to subscribe Elly to the `ant` mailing list. Since +Elly's email address is not yet known to Mailman, a user is created for her. +By default, get gets a regular delivery. >>> dump_json('http://localhost:9001/3.0/members', { ... 'list_id': 'ant.example.com', ... 'subscriber': 'eperson@example.com', ... 'display_name': 'Elly Person', + ... 'pre_verified': True, + ... 'pre_confirmed': True, + ... 'pre_approved': True, ... }) content-length: 0 date: ... @@ -659,6 +662,9 @@ list with her preferred address. >>> dump_json('http://localhost:9001/3.0/members', { ... 'list_id': 'ant.example.com', ... 'subscriber': user_id, + ... 'pre_verified': True, + ... 'pre_confirmed': True, + ... 'pre_approved': True, ... }) content-length: 0 date: ... @@ -729,94 +735,6 @@ Elly is no longer a member of the mailing list. set() -Digest delivery -=============== - -Fred joins the `ant` mailing list but wants MIME digest delivery. -:: - - >>> transaction.abort() - >>> dump_json('http://localhost:9001/3.0/members', { - ... 'list_id': 'ant.example.com', - ... 'subscriber': 'fperson@example.com', - ... 'display_name': 'Fred Person', - ... 'delivery_mode': 'mime_digests', - ... }) - content-length: 0 - date: ... - location: http://localhost:9001/3.0/members/10 - server: ... - status: 201 - - >>> fred = user_manager.get_user('fperson@example.com') - >>> memberships = list(fred.memberships.members) - >>> len(memberships) - 1 - -Fred is getting MIME deliveries. - - >>> memberships[0] - <Member: Fred Person <fperson@example.com> - on ant@example.com as MemberRole.member> - >>> print(memberships[0].delivery_mode) - DeliveryMode.mime_digests - - >>> dump_json('http://localhost:9001/3.0/members/10') - address: http://localhost:9001/3.0/addresses/fperson@example.com - delivery_mode: mime_digests - email: fperson@example.com - http_etag: "..." - list_id: ant.example.com - member_id: 10 - role: member - self_link: http://localhost:9001/3.0/members/10 - user: http://localhost:9001/3.0/users/7 - -Fred wants to change his delivery from MIME digest back to regular delivery. -This can be done by PATCH'ing his member with the `delivery_mode` parameter. -:: - - >>> transaction.abort() - >>> dump_json('http://localhost:9001/3.0/members/10', { - ... 'delivery_mode': 'regular', - ... }, method='PATCH') - content-length: 0 - date: ... - server: ... - status: 204 - - >>> dump_json('http://localhost:9001/3.0/members/10') - address: http://localhost:9001/3.0/addresses/fperson@example.com - delivery_mode: regular - email: fperson@example.com - http_etag: "..." - list_id: ant.example.com - member_id: 10 - role: member - self_link: http://localhost:9001/3.0/members/10 - user: http://localhost:9001/3.0/users/7 - -If a PATCH request changes no attributes, nothing happens. -:: - - >>> dump_json('http://localhost:9001/3.0/members/10', {}, method='PATCH') - content-length: 0 - date: ... - server: ... - status: 204 - - >>> dump_json('http://localhost:9001/3.0/members/10') - address: http://localhost:9001/3.0/addresses/fperson@example.com - delivery_mode: regular - email: fperson@example.com - http_etag: "..." - list_id: ant.example.com - member_id: 10 - role: member - self_link: http://localhost:9001/3.0/members/10 - user: http://localhost:9001/3.0/users/7 - - Changing delivery address ========================= @@ -849,30 +767,30 @@ addresses. >>> dump_json('http://localhost:9001/3.0/members') entry 0: ... - entry 5: + entry 4: address: http://localhost:9001/3.0/addresses/herb@example.com delivery_mode: regular email: herb@example.com http_etag: "..." list_id: ant.example.com - member_id: 11 + member_id: 10 role: member - self_link: http://localhost:9001/3.0/members/11 - user: http://localhost:9001/3.0/users/8 + self_link: http://localhost:9001/3.0/members/10 + user: http://localhost:9001/3.0/users/7 ... - entry 10: + entry 9: address: http://localhost:9001/3.0/addresses/herb@example.com delivery_mode: regular email: herb@example.com http_etag: "..." list_id: bee.example.com - member_id: 12 + member_id: 11 role: member - self_link: http://localhost:9001/3.0/members/12 - user: http://localhost:9001/3.0/users/8 + self_link: http://localhost:9001/3.0/members/11 + user: http://localhost:9001/3.0/users/7 http_etag: "..." start: 0 - total_size: 11 + total_size: 10 In order to change all of his subscriptions to use a different email address, Herb must iterate through his memberships explicitly. @@ -883,13 +801,13 @@ Herb must iterate through his memberships explicitly. >>> memberships = [entry['self_link'] for entry in content['entries']] >>> for url in sorted(memberships): ... print(url) + http://localhost:9001/3.0/members/10 http://localhost:9001/3.0/members/11 - http://localhost:9001/3.0/members/12 For each membership resource, the subscription address is changed by PATCH'ing the `address` attribute. - >>> dump_json('http://localhost:9001/3.0/members/11', { + >>> dump_json('http://localhost:9001/3.0/members/10', { ... 'address': 'hperson@example.com', ... }, method='PATCH') content-length: 0 @@ -897,7 +815,7 @@ the `address` attribute. server: ... status: 204 - >>> dump_json('http://localhost:9001/3.0/members/12', { + >>> dump_json('http://localhost:9001/3.0/members/11', { ... 'address': 'hperson@example.com', ... }, method='PATCH') content-length: 0 @@ -924,20 +842,20 @@ his membership ids have not changed. email: hperson@example.com http_etag: "..." list_id: ant.example.com - member_id: 11 + member_id: 10 role: member - self_link: http://localhost:9001/3.0/members/11 - user: http://localhost:9001/3.0/users/8 + self_link: http://localhost:9001/3.0/members/10 + user: http://localhost:9001/3.0/users/7 entry 1: address: http://localhost:9001/3.0/addresses/hperson@example.com delivery_mode: regular email: hperson@example.com http_etag: "..." list_id: bee.example.com - member_id: 12 + member_id: 11 role: member - self_link: http://localhost:9001/3.0/members/12 - user: http://localhost:9001/3.0/users/8 + self_link: http://localhost:9001/3.0/members/11 + user: http://localhost:9001/3.0/users/7 http_etag: "..." start: 0 total_size: 2 diff --git a/src/mailman/rest/helpers.py b/src/mailman/rest/helpers.py index edd57b76b..c737fcbc7 100644 --- a/src/mailman/rest/helpers.py +++ b/src/mailman/rest/helpers.py @@ -300,6 +300,12 @@ def not_found(response, body=b'404 Not Found'): response.body = body +def accepted(response, body=None): + response.status = falcon.HTTP_202 + if body is not None: + response.body = body + + def bad_request(response, body='400 Bad Request'): response.status = falcon.HTTP_400 if body is not None: diff --git a/src/mailman/rest/members.py b/src/mailman/rest/members.py index a0b5d4f4e..d6a57a673 100644 --- a/src/mailman/rest/members.py +++ b/src/mailman/rest/members.py @@ -25,18 +25,20 @@ __all__ = [ ] -from mailman.app.membership import delete_member -from mailman.interfaces.address import InvalidEmailAddressError -from mailman.interfaces.listmanager import IListManager, NoSuchListError +from mailman.app.membership import add_member, delete_member +from mailman.interfaces.address import IAddress, InvalidEmailAddressError +from mailman.interfaces.listmanager import IListManager from mailman.interfaces.member import ( AlreadySubscribedError, DeliveryMode, MemberRole, MembershipError, - NotAMemberError) -from mailman.interfaces.subscriptions import ISubscriptionService -from mailman.interfaces.user import UnverifiedAddressError + MembershipIsBannedError, NotAMemberError) +from mailman.interfaces.registrar import IRegistrar +from mailman.interfaces.subscriptions import ( + ISubscriptionService, RequestRecord, TokenOwner) +from mailman.interfaces.user import IUser, UnverifiedAddressError from mailman.interfaces.usermanager import IUserManager from mailman.rest.helpers import ( - CollectionMixin, NotFound, bad_request, child, conflict, created, etag, - no_content, not_found, okay, paginate, path_to) + CollectionMixin, NotFound, accepted, bad_request, child, conflict, + created, etag, no_content, not_found, okay, paginate, path_to) from mailman.rest.preferences import Preferences, ReadOnlyPreferences from mailman.rest.validator import ( Validator, enum_validator, subscriber_validator) @@ -202,7 +204,6 @@ class AllMembers(_MemberBase): def on_post(self, request, response): """Create a new member.""" - service = getUtility(ISubscriptionService) try: validator = Validator( list_id=str, @@ -210,22 +211,125 @@ class AllMembers(_MemberBase): display_name=str, delivery_mode=enum_validator(DeliveryMode), role=enum_validator(MemberRole), - _optional=('delivery_mode', 'display_name', 'role')) - member = service.join(**validator(request)) - except AlreadySubscribedError: - conflict(response, b'Member already subscribed') - except NoSuchListError: - bad_request(response, b'No such list') - except InvalidEmailAddressError: - bad_request(response, b'Invalid email address') + pre_verified=bool, + pre_confirmed=bool, + pre_approved=bool, + _optional=('delivery_mode', 'display_name', 'role', + 'pre_verified', 'pre_confirmed', 'pre_approved')) + arguments = validator(request) except ValueError as error: bad_request(response, str(error)) + return + # Dig the mailing list out of the arguments. + list_id = arguments.pop('list_id') + mlist = getUtility(IListManager).get_by_list_id(list_id) + if mlist is None: + bad_request(response, b'No such list') + return + # Figure out what kind of subscriber is being registered. Either it's + # a user via their preferred email address or it's an explicit address. + # If it's a UUID, then it must be associated with an existing user. + subscriber = arguments.pop('subscriber') + user_manager = getUtility(IUserManager) + # We use the display name if there is one. + display_name = arguments.pop('display_name', '') + if isinstance(subscriber, UUID): + user = user_manager.get_user_by_id(subscriber) + if user is None: + bad_request(response, b'No such user') + return + subscriber = user else: - # The member_id are UUIDs. We need to use the integer equivalent - # in the URL. - member_id = member.member_id.int - location = path_to('members/{0}'.format(member_id)) - created(response, location) + # This must be an email address. See if there's an existing + # address object associated with this email. + address = user_manager.get_address(subscriber) + if address is None: + # Create a new address, which of course will not be validated. + address = user_manager.create_address( + subscriber, display_name) + subscriber = address + # What role are we subscribing? Regular members go through the + # subscription policy workflow while owners, moderators, and + # nonmembers go through the legacy API for now. + role = arguments.pop('role', MemberRole.member) + if role is MemberRole.member: + # Get the pre_ flags for the subscription workflow. + pre_verified = arguments.pop('pre_verified', False) + pre_confirmed = arguments.pop('pre_confirmed', False) + pre_approved = arguments.pop('pre_approved', False) + # Now we can run the registration process until either the + # subscriber is subscribed, or the workflow is paused for + # verification, confirmation, or approval. + registrar = IRegistrar(mlist) + try: + token, token_owner, member = registrar.register( + subscriber, + pre_verified=pre_verified, + pre_confirmed=pre_confirmed, + pre_approved=pre_approved) + except AlreadySubscribedError: + conflict(response, b'Member already subscribed') + return + if token is None: + assert token_owner is TokenOwner.no_one, token_owner + # The subscription completed. Let's get the resulting member + # and return the location to the new member. Member ids are + # UUIDs and need to be converted to URLs because JSON doesn't + # directly support UUIDs. + member_id = member.member_id.int + location = path_to('members/{0}'.format(member_id)) + created(response, location) + return + # The member could not be directly subscribed because there are + # some out-of-band steps that need to be completed. E.g. the user + # must confirm their subscription or the moderator must approve + # it. In this case, an HTTP 202 Accepted is exactly the code that + # we should use, and we'll return both the confirmation token and + # the "token owner" so the client knows who should confirm it. + assert token is not None, token + assert token_owner is not TokenOwner.no_one, token_owner + assert member is None, member + content = dict(token=token, token_owner=token_owner.name) + accepted(response, etag(content)) + return + # 2015-04-15 BAW: We're subscribing some role other than a regular + # member. Use the legacy API for this for now. + assert role in (MemberRole.owner, + MemberRole.moderator, + MemberRole.nonmember) + # 2015-04-15 BAW: We're limited to using an email address with this + # legacy API, so if the subscriber is a user, the user must have a + # preferred address, which we'll use, even though it will subscribe + # the explicit address. It is an error if the user does not have a + # preferred address. + # + # If the subscriber is an address object, just use that. + if IUser.providedBy(subscriber): + if subscriber.preferred_address is None: + bad_request(response, b'User without preferred address') + return + email = subscriber.preferred_address.email + else: + assert IAddress.providedBy(subscriber) + email = subscriber.email + delivery_mode = arguments.pop('delivery_mode', DeliveryMode.regular) + record = RequestRecord(email, display_name, delivery_mode) + try: + member = add_member(mlist, record, role) + except InvalidEmailAddressError: + bad_request(response, b'Invalid email address') + return + except MembershipIsBannedError: + bad_request(response, b'Membership is banned') + return + # The subscription completed. Let's get the resulting member + # and return the location to the new member. Member ids are + # UUIDs and need to be converted to URLs because JSON doesn't + # directly support UUIDs. + member_id = member.member_id.int + location = path_to('members/{0}'.format(member_id)) + created(response, location) + return def on_get(self, request, response): """/members""" diff --git a/src/mailman/rest/tests/test_listconf.py b/src/mailman/rest/tests/test_listconf.py index ddb43a8ea..860adac57 100644 --- a/src/mailman/rest/tests/test_listconf.py +++ b/src/mailman/rest/tests/test_listconf.py @@ -26,7 +26,8 @@ import unittest from mailman.app.lifecycle import create_list from mailman.database.transaction import transaction -from mailman.interfaces.mailinglist import IAcceptableAliasSet +from mailman.interfaces.mailinglist import ( + IAcceptableAliasSet, SubscriptionPolicy) from mailman.testing.helpers import call_api from mailman.testing.layers import RESTLayer @@ -90,3 +91,20 @@ class TestConfiguration(unittest.TestCase): # All three acceptable aliases were set. self.assertEqual(set(IAcceptableAliasSet(self._mlist).aliases), set(aliases)) + + def test_patch_subscription_policy(self): + # The new subscription_policy value can be patched. + # + # To start with, the subscription policy is confirm by default. + resource, response = call_api( + 'http://localhost:9001/3.0/lists/test@example.com/config') + self.assertEqual(resource['subscription_policy'], 'confirm') + # Let's patch it to do some moderation. + resource, response = call_api( + 'http://localhost:9001/3.0/lists/test@example.com/config', dict( + subscription_policy='confirm_then_moderate'), + method='PATCH') + self.assertEqual(response.status, 204) + # And now we verify that it has the requested setting. + self.assertEqual(self._mlist.subscription_policy, + SubscriptionPolicy.confirm_then_moderate) diff --git a/src/mailman/rest/tests/test_membership.py b/src/mailman/rest/tests/test_membership.py index a77dea3b5..4542677b6 100644 --- a/src/mailman/rest/tests/test_membership.py +++ b/src/mailman/rest/tests/test_membership.py @@ -100,6 +100,9 @@ class TestMembership(unittest.TestCase): call_api('http://localhost:9001/3.0/members', { 'list_id': 'test.example.com', 'subscriber': 'anne@example.com', + 'pre_verified': True, + 'pre_confirmed': True, + 'pre_approved': True, }) self.assertEqual(cm.exception.code, 409) self.assertEqual(cm.exception.reason, b'Member already subscribed') @@ -115,6 +118,9 @@ class TestMembership(unittest.TestCase): call_api('http://localhost:9001/3.0/members', { 'list_id': 'test.example.com', 'subscriber': 'ANNE@example.com', + 'pre_verified': True, + 'pre_confirmed': True, + 'pre_approved': True, }) self.assertEqual(cm.exception.code, 409) self.assertEqual(cm.exception.reason, b'Member already subscribed') @@ -130,6 +136,9 @@ class TestMembership(unittest.TestCase): call_api('http://localhost:9001/3.0/members', { 'list_id': 'test.example.com', 'subscriber': 'anne@example.com', + 'pre_verified': True, + 'pre_confirmed': True, + 'pre_approved': True, }) self.assertEqual(cm.exception.code, 409) self.assertEqual(cm.exception.reason, b'Member already subscribed') @@ -151,6 +160,9 @@ class TestMembership(unittest.TestCase): 'list_id': 'test.example.com', 'subscriber': 'hugh/person@example.com', 'display_name': 'Hugh Person', + 'pre_verified': True, + 'pre_confirmed': True, + 'pre_approved': True, }) self.assertEqual(content, None) self.assertEqual(response.status, 201) diff --git a/src/mailman/rest/tests/test_users.py b/src/mailman/rest/tests/test_users.py index af2c9f0d1..ac8d018e8 100644 --- a/src/mailman/rest/tests/test_users.py +++ b/src/mailman/rest/tests/test_users.py @@ -376,7 +376,8 @@ class TestLP1074374(unittest.TestCase): call_api('http://localhost:9001/3.0/members', dict( list_id='test.example.com', subscriber='anne@example.com', - role='member')) + role='member', + pre_verified=True, pre_confirmed=True, pre_approved=True)) # This is not the Anne you're looking for. (IOW, the new Anne is a # different user). content, response = call_api( diff --git a/src/mailman/rest/tests/test_validator.py b/src/mailman/rest/tests/test_validator.py index c670fc77c..2d515f828 100644 --- a/src/mailman/rest/tests/test_validator.py +++ b/src/mailman/rest/tests/test_validator.py @@ -24,8 +24,11 @@ __all__ = [ import unittest -from mailman.rest.validator import list_of_strings_validator +from mailman.interfaces.usermanager import IUserManager +from mailman.rest.validator import ( + list_of_strings_validator, subscriber_validator) from mailman.testing.layers import RESTLayer +from zope.component import getUtility @@ -46,3 +49,16 @@ class TestValidators(unittest.TestCase): # Strings are required. self.assertRaises(ValueError, list_of_strings_validator, 7) self.assertRaises(ValueError, list_of_strings_validator, ['ant', 7]) + + def test_subscriber_validator_uuid(self): + # Convert from an existing user id to a UUID. + anne = getUtility(IUserManager).make_user('anne@example.com') + uuid = subscriber_validator(str(anne.user_id.int)) + self.assertEqual(anne.user_id, uuid) + + def test_subscriber_validator_bad_uuid(self): + self.assertRaises(ValueError, subscriber_validator, 'not-a-thing') + + def test_subscriber_validator_email_address(self): + self.assertEqual(subscriber_validator('anne@example.com'), + 'anne@example.com') diff --git a/src/mailman/rest/validator.py b/src/mailman/rest/validator.py index 720d7adc1..1d5ad4ef9 100644 --- a/src/mailman/rest/validator.py +++ b/src/mailman/rest/validator.py @@ -29,6 +29,7 @@ __all__ = [ from mailman.core.errors import ( ReadOnlyPATCHRequestError, UnknownPATCHRequestError) +from mailman.interfaces.address import IEmailValidator from mailman.interfaces.languages import ILanguageManager from uuid import UUID from zope.component import getUtility @@ -59,7 +60,10 @@ def subscriber_validator(subscriber): try: return UUID(int=int(subscriber)) except ValueError: - return subscriber + # It must be an email address. + if getUtility(IEmailValidator).is_valid(subscriber): + return subscriber + raise ValueError def language_validator(code): diff --git a/src/mailman/runners/docs/command.rst b/src/mailman/runners/docs/command.rst index 19d772b00..c97e6454c 100644 --- a/src/mailman/runners/docs/command.rst +++ b/src/mailman/runners/docs/command.rst @@ -147,7 +147,7 @@ address, and the other is the results of his email command. ... print('Subject:', subject) ... if 'confirm' in str(subject): ... token = str(subject).split()[1].strip() - ... new_token = registrar.confirm(token) + ... new_token, token_owner, member = registrar.confirm(token) ... assert new_token is None, 'Confirmation failed' Subject: The results of your email commands Subject: confirm ... diff --git a/src/mailman/runners/tests/test_confirm.py b/src/mailman/runners/tests/test_confirm.py index 71ec5988d..b24d14a52 100644 --- a/src/mailman/runners/tests/test_confirm.py +++ b/src/mailman/runners/tests/test_confirm.py @@ -53,7 +53,8 @@ class TestConfirm(unittest.TestCase): self._mlist = create_list('test@example.com') self._mlist.send_welcome_message = False anne = getUtility(IUserManager).create_address('anne@example.org') - self._token = IRegistrar(self._mlist).register(anne) + registrar = IRegistrar(self._mlist) + self._token, token_owner, member = registrar.register(anne) def test_confirm_with_re_prefix(self): subject = 'Re: confirm {0}'.format(self._token) diff --git a/src/mailman/runners/tests/test_join.py b/src/mailman/runners/tests/test_join.py index c3b52cf0c..1067517e2 100644 --- a/src/mailman/runners/tests/test_join.py +++ b/src/mailman/runners/tests/test_join.py @@ -30,7 +30,7 @@ from mailman.app.lifecycle import create_list from mailman.config import config from mailman.interfaces.member import DeliveryMode from mailman.interfaces.registrar import IRegistrar -from mailman.interfaces.subscriptions import ISubscriptionService +from mailman.interfaces.subscriptions import ISubscriptionService, TokenOwner from mailman.interfaces.usermanager import IUserManager from mailman.runners.command import CommandRunner from mailman.testing.helpers import ( @@ -160,14 +160,16 @@ class TestJoinWithDigests(unittest.TestCase): subject_words = str(messages[1].msg['subject']).split() self.assertEqual(subject_words[0], 'confirm') token = subject_words[1] - status = IRegistrar(self._mlist).confirm(token) - self.assertIsNone(status, 'Confirmation failed') + token, token_owner, rmember = IRegistrar(self._mlist).confirm(token) + self.assertIsNone(token) + self.assertEqual(token_owner, TokenOwner.no_one) # Now, make sure that Anne is a member of the list and is receiving # digest deliveries. members = getUtility(ISubscriptionService).find_members( 'anne@example.org') self.assertEqual(len(members), 1) - return members[0] + self.assertEqual(rmember, members[0]) + return rmember def test_join_with_implicit_no_digests(self): # Test the digest=mime argument to the join command. |
