From 4b42d1c4cca07396585dbfd6265ecd751b419b06 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Wed, 15 Apr 2015 14:36:28 -0400 Subject: Add a test for a reported (but non-reproducible) failure patching a mailing list's subscription policy. --- src/mailman/rest/tests/test_listconf.py | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) (limited to 'src/mailman/rest/tests') 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) -- cgit v1.2.3-70-g09d2 From ee6061a2385437bc9d6967ddfaf1514f06cb290a Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Wed, 15 Apr 2015 18:27:11 -0400 Subject: A better email validator which actually validates the email address. --- src/mailman/rest/tests/test_validator.py | 18 +++++++++++++++++- src/mailman/rest/validator.py | 6 +++++- 2 files changed, 22 insertions(+), 2 deletions(-) (limited to 'src/mailman/rest/tests') 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): -- cgit v1.2.3-70-g09d2 From 08f457799cd36349a4fd22642f4c05b4eabb306d Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Wed, 15 Apr 2015 22:51:39 -0400 Subject: Plumb the subscription policy through the REST API. --- TODO.rst | 2 +- src/mailman/app/registrar.py | 4 +- src/mailman/app/subscriptions.py | 15 ++- src/mailman/app/tests/test_registrar.py | 47 ++++++--- src/mailman/app/tests/test_subscriptions.py | 32 +++++- src/mailman/commands/eml_confirm.py | 11 ++- src/mailman/commands/tests/test_confirm.py | 3 +- src/mailman/interfaces/registrar.py | 5 +- src/mailman/model/docs/registration.rst | 18 +++- src/mailman/rest/docs/membership.rst | 136 +++++-------------------- src/mailman/rest/helpers.py | 6 ++ src/mailman/rest/members.py | 148 +++++++++++++++++++++++----- src/mailman/rest/tests/test_membership.py | 12 +++ src/mailman/rest/tests/test_users.py | 3 +- src/mailman/runners/docs/command.rst | 2 +- src/mailman/runners/tests/test_confirm.py | 3 +- src/mailman/runners/tests/test_join.py | 10 +- 17 files changed, 287 insertions(+), 170 deletions(-) (limited to 'src/mailman/rest/tests') diff --git a/TODO.rst b/TODO.rst index 362499258..131c52faf 100644 --- a/TODO.rst +++ b/TODO.rst @@ -1,5 +1,5 @@ * TO DO: - - plumb subscription policy through rest api + - rename ISubscriptionService to IMemberSearch or somesuch - get rid of hold_subscription - subsume handle_subscription - workflow for unsubscription diff --git a/src/mailman/app/registrar.py b/src/mailman/app/registrar.py index a3699de91..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, workflow.token_owner + 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, workflow.token_owner + 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 aa9d600bf..1c6b96016 100644 --- a/src/mailman/app/subscriptions.py +++ b/src/mailman/app/subscriptions.py @@ -223,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') @@ -235,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: diff --git a/src/mailman/app/tests/test_registrar.py b/src/mailman/app/tests/test_registrar.py index 1431b5e6f..a2ab2c686 100644 --- a/src/mailman/app/tests/test_registrar.py +++ b/src/mailman/app/tests/test_registrar.py @@ -48,13 +48,14 @@ 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, and the owner of the token. self.assertEqual(self._pendings.count, 0) - token, token_owner = 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) @@ -66,10 +67,11 @@ class TestRegistrar(unittest.TestCase): # there's nothing more to do. self._mlist.subscription_policy = SubscriptionPolicy.open self._anne.verified_on = now() - token, token_owner = self._registrar.register(self._anne) + token, token_owner, rmember = self._registrar.register(self._anne) 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) # There's nothing to confirm. record = self._pendings.confirm(token, expunge=False) @@ -86,16 +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, token_owner = 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. - token, token_owner = 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): @@ -105,16 +109,18 @@ class TestRegistrar(unittest.TestCase): # user. self._mlist.subscription_policy = SubscriptionPolicy.confirm self._anne.verified_on = now() - token, token_owner = 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. - token, token_owner = 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): @@ -123,16 +129,18 @@ class TestRegistrar(unittest.TestCase): # token subscribes the user. self._mlist.subscription_policy = SubscriptionPolicy.moderate self._anne.verified_on = now() - token, token_owner = 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. - token, token_owner = 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): @@ -145,27 +153,30 @@ class TestRegistrar(unittest.TestCase): SubscriptionPolicy.confirm_then_moderate self._anne.verified_on = now() # Runs until subscription confirmation. - token, token_owner = 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, token_owner = 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. - token, token_owner = 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): @@ -177,18 +188,20 @@ class TestRegistrar(unittest.TestCase): SubscriptionPolicy.confirm_then_moderate self._anne.verified_on = now() # Runs until subscription confirmation. - token, token_owner = 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, token_owner = 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. @@ -197,11 +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, token_owner = 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): @@ -210,9 +224,10 @@ class TestRegistrar(unittest.TestCase): self._mlist.subscription_policy = SubscriptionPolicy.confirm self._anne.verified_on = now() # Runs until subscription confirmation. - token, token_owner = 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 fa4fec78f..42984c77c 100644 --- a/src/mailman/app/tests/test_subscriptions.py +++ b/src/mailman/app/tests/test_subscriptions.py @@ -203,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() @@ -598,3 +614,17 @@ approval: # 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 019ce9d2a..6f049b9d7 100644 --- a/src/mailman/interfaces/registrar.py +++ b/src/mailman/interfaces/registrar.py @@ -77,8 +77,9 @@ class IRegistrar(Interface): :type email: ``IUser`` or ``IAddress`` :return: None if the workflow completes with the member being subscribed. If the workflow is paused for user confirmation or - moderator approval, a 2-tuple is returned where the first element - is a ``TokenOwner`` and the second element is the token hash. + 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/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 + on ant@example.com + as MemberRole.member> >>> mlist.members.get_member('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 + on ant@example.com + as MemberRole.member> >>> mlist.members.get_member('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] - - 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_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/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. -- cgit v1.2.3-70-g09d2 From 53d4229bff96677a54fd443322c166c2bdcc3054 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Thu, 16 Apr 2015 15:28:06 -0400 Subject: Checkpointing. --- src/mailman/app/subscriptions.py | 3 +- src/mailman/app/tests/test_subscriptions.py | 16 ++ src/mailman/interfaces/pending.py | 6 + src/mailman/interfaces/registrar.py | 23 +- src/mailman/model/docs/pending.rst | 19 +- src/mailman/model/pending.py | 5 + src/mailman/rest/docs/moderation.rst | 361 ---------------------------- src/mailman/rest/docs/post-moderation.rst | 192 +++++++++++++++ src/mailman/rest/lists.py | 3 +- src/mailman/rest/moderation.py | 252 ------------------- src/mailman/rest/post_moderation.py | 155 ++++++++++++ src/mailman/rest/tests/test_moderation.py | 219 +++++++++++++---- 12 files changed, 577 insertions(+), 677 deletions(-) delete mode 100644 src/mailman/rest/docs/moderation.rst create mode 100644 src/mailman/rest/docs/post-moderation.rst delete mode 100644 src/mailman/rest/moderation.py create mode 100644 src/mailman/rest/post_moderation.py (limited to 'src/mailman/rest/tests') diff --git a/src/mailman/app/subscriptions.py b/src/mailman/app/subscriptions.py index 1c6b96016..46ce549af 100644 --- a/src/mailman/app/subscriptions.py +++ b/src/mailman/app/subscriptions.py @@ -24,7 +24,6 @@ __all__ = [ ] - import uuid import logging @@ -170,6 +169,8 @@ class SubscriptionWorkflow(Workflow): pendable = Pendable( list_id=self.mlist.list_id, address=self.address.email, + hold_date=now().replace(microsecond=0).isoformat(), + token_owner=token_owner.name, ) self.token = getUtility(IPendings).add(pendable, timedelta(days=3650)) diff --git a/src/mailman/app/tests/test_subscriptions.py b/src/mailman/app/tests/test_subscriptions.py index 732266ac3..6b5d88026 100644 --- a/src/mailman/app/tests/test_subscriptions.py +++ b/src/mailman/app/tests/test_subscriptions.py @@ -57,6 +57,22 @@ class TestSubscriptionWorkflow(unittest.TestCase): self.assertEqual(workflow.token_owner, TokenOwner.no_one) self.assertIsNone(workflow.member) + def test_pended_data(self): + # There is a Pendable associated with the held request, and it has + # some data associated with it. + anne = self._user_manager.create_address(self._anne) + workflow = SubscriptionWorkflow(self._mlist, anne) + try: + workflow.run_thru('send_confirmation') + except StopIteration: + pass + self.assertIsNotNone(workflow.token) + pendable = getUtility(IPendings).confirm(workflow.token, expunge=False) + self.assertEqual(pendable['list_id'], 'test.example.com') + self.assertEqual(pendable['address'], 'anne@example.com') + self.assertEqual(pendable['hold_date'], '2005-08-01T07:49:23') + self.assertEqual(pendable['token_owner'], 'subscriber') + def test_user_or_address_required(self): # The `subscriber` attribute must be a user or address. workflow = SubscriptionWorkflow(self._mlist) diff --git a/src/mailman/interfaces/pending.py b/src/mailman/interfaces/pending.py index 222f0dfbf..c921123de 100644 --- a/src/mailman/interfaces/pending.py +++ b/src/mailman/interfaces/pending.py @@ -95,4 +95,10 @@ class IPendings(Interface): def evict(): """Remove all pended items whose lifetime has expired.""" + def __iter__(): + """An iterator over all pendables. + + Each element is a 2-tuple of the form (token, dict). + """ + count = Attribute('The number of pendables in the pendings database.') diff --git a/src/mailman/interfaces/registrar.py b/src/mailman/interfaces/registrar.py index 6f049b9d7..959e0bf6a 100644 --- a/src/mailman/interfaces/registrar.py +++ b/src/mailman/interfaces/registrar.py @@ -75,12 +75,13 @@ class IRegistrar(Interface): :param subscriber: The user or address to subscribe. :type email: ``IUser`` or ``IAddress`` - :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) + :return: A 3-tuple is returned where the first element is the token + hash, the second element is a ``TokenOwner`, and the third element + is the subscribed member. If the subscriber got subscribed + immediately, the token will be None and the member will be + an ``IMember``. If the subscription got held, the token + will be a hash and the member will be None. + :rtype: (str-or-None, ``TokenOwner``, ``IMember``-or-None) :raises MembershipIsBannedError: when the address being subscribed appears in the global or list-centric bans. """ @@ -94,9 +95,13 @@ class IRegistrar(Interface): :param token: A token matching a workflow. :type token: string - :return: The new token for any follow up confirmation, or None if the - user was subscribed. - :rtype: str or None + :return: A 3-tuple is returned where the first element is the token + hash, the second element is a ``TokenOwner`, and the third element + is the subscribed member. If the subscriber got subscribed + immediately, the token will be None and the member will be + an ``IMember``. If the subscription is still being held, the token + will be a hash and the member will be None. + :rtype: (str-or-None, ``TokenOwner``, ``IMember``-or-None) :raises LookupError: when no workflow is associated with the token. """ diff --git a/src/mailman/model/docs/pending.rst b/src/mailman/model/docs/pending.rst index 4a14edb2a..03eee3772 100644 --- a/src/mailman/model/docs/pending.rst +++ b/src/mailman/model/docs/pending.rst @@ -43,10 +43,9 @@ There's exactly one entry in the pendings database now. >>> pendingdb.count 1 -There's not much you can do with tokens except to *confirm* them, which -basically means returning the `IPendable` structure (as a dictionary) from the -database that matches the token. If the token isn't in the database, None is -returned. +You can *confirm* the pending, which means returning the `IPendable` structure +(as a dictionary) from the database that matches the token. If the token +isn't in the database, None is returned. >>> pendable = pendingdb.confirm(b'missing') >>> print(pendable) @@ -83,6 +82,18 @@ expunge it. >>> print(pendingdb.confirm(token_1)) None +You can iterate over all the pendings in the database. + + >>> pendables = list(pendingdb) + >>> def sort_key(item): + ... token, pendable = item + ... return pendable['type'] + >>> sorted_pendables = sorted(pendables, key=sort_key) + >>> for token, pendable in sorted_pendables: + ... print(pendable['type']) + three + two + An event can be given a lifetime when it is pended, otherwise it just uses a default lifetime. diff --git a/src/mailman/model/pending.py b/src/mailman/model/pending.py index bbe95d5f0..04b63f2ca 100644 --- a/src/mailman/model/pending.py +++ b/src/mailman/model/pending.py @@ -166,6 +166,11 @@ class Pendings: store.delete(keyvalue) store.delete(pending) + @dbconnection + def __iter__(self, store): + for pending in store.query(Pended).all(): + yield pending.token, self.confirm(pending.token, expunge=False) + @property @dbconnection def count(self, store): diff --git a/src/mailman/rest/docs/moderation.rst b/src/mailman/rest/docs/moderation.rst deleted file mode 100644 index 0a9fd59a8..000000000 --- a/src/mailman/rest/docs/moderation.rst +++ /dev/null @@ -1,361 +0,0 @@ -========== -Moderation -========== - -There are two kinds of moderation tasks a list administrator may need to -perform. Messages which are held for approval can be accepted, rejected, -discarded, or deferred. Subscription (and sometimes unsubscription) requests -can similarly be accepted, discarded, rejected, or deferred. - - -Message moderation -================== - -Viewing the list of held messages ---------------------------------- - -Held messages can be moderated through the REST API. A mailing list starts -with no held messages. - - >>> ant = create_list('ant@example.com') - >>> transaction.commit() - >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/held') - http_etag: "..." - start: 0 - total_size: 0 - -When a message gets held for moderator approval, it shows up in this list. -:: - - >>> msg = message_from_string("""\ - ... From: anne@example.com - ... To: ant@example.com - ... Subject: Something - ... Message-ID: - ... - ... Something else. - ... """) - - >>> from mailman.app.moderator import hold_message - >>> request_id = hold_message(ant, msg, {'extra': 7}, 'Because') - >>> transaction.commit() - - >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/held') - entry 0: - extra: 7 - hold_date: 2005-08-01T07:49:23 - http_etag: "..." - message_id: - msg: From: anne@example.com - To: ant@example.com - Subject: Something - Message-ID: - X-Message-ID-Hash: GCSMSG43GYWWVUMO6F7FBUSSPNXQCJ6M - - Something else. - - reason: Because - request_id: 1 - sender: anne@example.com - subject: Something - http_etag: "..." - start: 0 - total_size: 1 - -You can get an individual held message by providing the *request id* for that -message. This will include the text of the message. -:: - - >>> def url(request_id): - ... return ('http://localhost:9001/3.0/lists/' - ... 'ant@example.com/held/{0}'.format(request_id)) - - >>> dump_json(url(request_id)) - extra: 7 - hold_date: 2005-08-01T07:49:23 - http_etag: "..." - message_id: - msg: From: anne@example.com - To: ant@example.com - Subject: Something - Message-ID: - X-Message-ID-Hash: GCSMSG43GYWWVUMO6F7FBUSSPNXQCJ6M - - Something else. - - reason: Because - request_id: 1 - sender: anne@example.com - subject: Something - - -Disposing of held messages --------------------------- - -Individual messages can be moderated through the API by POSTing back to the -held message's resource. The POST data requires an action of one of the -following: - - * discard - throw the message away. - * reject - bounces the message back to the original author. - * defer - defer any action on the message (continue to hold it) - * accept - accept the message for posting. - -Let's see what happens when the above message is deferred. - - >>> dump_json(url(request_id), { - ... 'action': 'defer', - ... }) - content-length: 0 - date: ... - server: ... - status: 204 - -The message is still in the moderation queue. - - >>> dump_json(url(request_id)) - extra: 7 - hold_date: 2005-08-01T07:49:23 - http_etag: "..." - message_id: - msg: From: anne@example.com - To: ant@example.com - Subject: Something - Message-ID: - X-Message-ID-Hash: GCSMSG43GYWWVUMO6F7FBUSSPNXQCJ6M - - Something else. - - reason: Because - request_id: 1 - sender: anne@example.com - subject: Something - -The held message can be discarded. - - >>> dump_json(url(request_id), { - ... 'action': 'discard', - ... }) - content-length: 0 - date: ... - server: ... - status: 204 - -Messages can also be accepted via the REST API. Let's hold a new message for -moderation. -:: - - >>> del msg['message-id'] - >>> msg['Message-ID'] = '' - >>> request_id = hold_message(ant, msg) - >>> transaction.commit() - - >>> results = call_http(url(request_id)) - >>> print(results['message_id']) - - - >>> dump_json(url(request_id), { - ... 'action': 'accept', - ... }) - content-length: 0 - date: ... - server: ... - status: 204 - - >>> from mailman.testing.helpers import get_queue_messages - >>> messages = get_queue_messages('pipeline') - >>> len(messages) - 1 - >>> print(messages[0].msg['message-id']) - - -Messages can be rejected via the REST API too. These bounce the message back -to the original author. -:: - - >>> del msg['message-id'] - >>> msg['Message-ID'] = '' - >>> request_id = hold_message(ant, msg) - >>> transaction.commit() - - >>> results = call_http(url(request_id)) - >>> print(results['message_id']) - - - >>> dump_json(url(request_id), { - ... 'action': 'reject', - ... }) - content-length: 0 - date: ... - server: ... - status: 204 - - >>> from mailman.testing.helpers import get_queue_messages - >>> messages = get_queue_messages('virgin') - >>> len(messages) - 1 - >>> print(messages[0].msg['subject']) - Request to mailing list "Ant" rejected - - -Subscription moderation -======================= - -Viewing subscription requests ------------------------------ - -Subscription and unsubscription requests can be moderated via the REST API as -well. A mailing list starts with no pending subscription or unsubscription -requests. - - >>> ant.admin_immed_notify = False - >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/requests') - http_etag: "..." - start: 0 - total_size: 0 - -When Anne tries to subscribe to the Ant list, her subscription is held for -moderator approval. -:: - - >>> from mailman.app.moderator import hold_subscription - >>> from mailman.interfaces.member import DeliveryMode - >>> 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: - delivery_mode: regular - display_name: Anne Person - email: anne@example.com - http_etag: "..." - language: en - request_id: ... - type: subscription - when: 2005-08-01T07:49:23 - http_etag: "..." - start: 0 - total_size: 1 - - -Viewing unsubscription requests -------------------------------- - -Bart tries to leave a mailing list, but he may not be allowed to. - - >>> from mailman.testing.helpers import subscribe - >>> from mailman.app.moderator import hold_unsubscription - >>> bart = subscribe(ant, 'Bart', email='bart@example.com') - >>> unsub_req_id = hold_unsubscription(ant, 'bart@example.com') - >>> transaction.commit() - -The unsubscription request is also available from the mailing list. - - >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/requests') - entry 0: - delivery_mode: regular - display_name: Anne Person - email: anne@example.com - http_etag: "..." - language: en - request_id: ... - type: subscription - when: 2005-08-01T07:49:23 - entry 1: - email: bart@example.com - http_etag: "..." - request_id: ... - type: unsubscription - http_etag: "..." - start: 0 - total_size: 2 - - -Viewing individual requests ---------------------------- - -You can view an individual membership change request by providing the -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)) - delivery_mode: regular - display_name: Anne Person - email: anne@example.com - http_etag: "..." - language: en - request_id: ... - type: subscription - when: 2005-08-01T07:49:23 - -Bart's unsubscription request looks like this. - - >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/' - ... 'requests/{}'.format(unsub_req_id)) - email: bart@example.com - http_etag: "..." - request_id: ... - type: unsubscription - - -Disposing of subscription requests ----------------------------------- - -Similar to held messages, you can dispose of held subscription and -unsubscription requests by POSTing back to the request's resource. The POST -data requires an action of one of the following: - - * discard - throw the request away. - * reject - the request is denied and a notification is sent to the email - address requesting the membership change. - * defer - defer any action on this membership change (continue to hold it). - * accept - accept the membership change. - -Anne's subscription request is accepted. - - >>> dump_json('http://localhost:9001/3.0/lists/' - ... 'ant@example.com/requests/{}'.format(sub_req_id), - ... {'action': 'accept'}) - content-length: 0 - date: ... - server: ... - status: 204 - -Anne is now a member of the mailing list. - - >>> transaction.abort() - >>> ant.members.get_member('anne@example.com') - on ant@example.com - as MemberRole.member> - >>> transaction.abort() - -Bart's unsubscription request is discarded. - - >>> dump_json('http://localhost:9001/3.0/lists/' - ... 'ant@example.com/requests/{}'.format(unsub_req_id), - ... {'action': 'discard'}) - content-length: 0 - date: ... - server: ... - status: 204 - -Bart is still a member of the mailing list. - - >>> transaction.abort() - >>> print(ant.members.get_member('bart@example.com')) - on ant@example.com - as MemberRole.member> - >>> transaction.abort() - -There are no more membership change requests. - - >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/requests') - http_etag: "..." - start: 0 - total_size: 0 diff --git a/src/mailman/rest/docs/post-moderation.rst b/src/mailman/rest/docs/post-moderation.rst new file mode 100644 index 000000000..f082de157 --- /dev/null +++ b/src/mailman/rest/docs/post-moderation.rst @@ -0,0 +1,192 @@ +=============== +Post Moderation +=============== + +Messages which are held for approval can be accepted, rejected, discarded, or +deferred by the list moderators. + + +Viewing the list of held messages +================================= + +Held messages can be moderated through the REST API. A mailing list starts +with no held messages. + + >>> ant = create_list('ant@example.com') + >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/held') + http_etag: "..." + start: 0 + total_size: 0 + +When a message gets held for moderator approval, it shows up in this list. +:: + + >>> msg = message_from_string("""\ + ... From: anne@example.com + ... To: ant@example.com + ... Subject: Something + ... Message-ID: + ... + ... Something else. + ... """) + + >>> from mailman.app.moderator import hold_message + >>> request_id = hold_message(ant, msg, {'extra': 7}, 'Because') + >>> transaction.commit() + + >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/held') + entry 0: + extra: 7 + hold_date: 2005-08-01T07:49:23 + http_etag: "..." + message_id: + msg: From: anne@example.com + To: ant@example.com + Subject: Something + Message-ID: + X-Message-ID-Hash: GCSMSG43GYWWVUMO6F7FBUSSPNXQCJ6M + + Something else. + + reason: Because + request_id: 1 + sender: anne@example.com + subject: Something + http_etag: "..." + start: 0 + total_size: 1 + +You can get an individual held message by providing the *request id* for that +message. This will include the text of the message. +:: + + >>> def url(request_id): + ... return ('http://localhost:9001/3.0/lists/' + ... 'ant@example.com/held/{0}'.format(request_id)) + + >>> dump_json(url(request_id)) + extra: 7 + hold_date: 2005-08-01T07:49:23 + http_etag: "..." + message_id: + msg: From: anne@example.com + To: ant@example.com + Subject: Something + Message-ID: + X-Message-ID-Hash: GCSMSG43GYWWVUMO6F7FBUSSPNXQCJ6M + + Something else. + + reason: Because + request_id: 1 + sender: anne@example.com + subject: Something + + +Disposing of held messages +========================== + +Individual messages can be moderated through the API by POSTing back to the +held message's resource. The POST data requires an action of one of the +following: + + * discard - throw the message away. + * reject - bounces the message back to the original author. + * defer - defer any action on the message (continue to hold it) + * accept - accept the message for posting. + +Let's see what happens when the above message is deferred. + + >>> dump_json(url(request_id), { + ... 'action': 'defer', + ... }) + content-length: 0 + date: ... + server: ... + status: 204 + +The message is still in the moderation queue. + + >>> dump_json(url(request_id)) + extra: 7 + hold_date: 2005-08-01T07:49:23 + http_etag: "..." + message_id: + msg: From: anne@example.com + To: ant@example.com + Subject: Something + Message-ID: + X-Message-ID-Hash: GCSMSG43GYWWVUMO6F7FBUSSPNXQCJ6M + + Something else. + + reason: Because + request_id: 1 + sender: anne@example.com + subject: Something + +The held message can be discarded. + + >>> dump_json(url(request_id), { + ... 'action': 'discard', + ... }) + content-length: 0 + date: ... + server: ... + status: 204 + +Messages can also be accepted via the REST API. Let's hold a new message for +moderation. +:: + + >>> del msg['message-id'] + >>> msg['Message-ID'] = '' + >>> request_id = hold_message(ant, msg) + >>> transaction.commit() + + >>> results = call_http(url(request_id)) + >>> print(results['message_id']) + + + >>> dump_json(url(request_id), { + ... 'action': 'accept', + ... }) + content-length: 0 + date: ... + server: ... + status: 204 + + >>> from mailman.testing.helpers import get_queue_messages + >>> messages = get_queue_messages('pipeline') + >>> len(messages) + 1 + >>> print(messages[0].msg['message-id']) + + +Messages can be rejected via the REST API too. These bounce the message back +to the original author. +:: + + >>> del msg['message-id'] + >>> msg['Message-ID'] = '' + >>> request_id = hold_message(ant, msg) + >>> transaction.commit() + + >>> results = call_http(url(request_id)) + >>> print(results['message_id']) + + + >>> dump_json(url(request_id), { + ... 'action': 'reject', + ... }) + content-length: 0 + date: ... + server: ... + status: 204 + + >>> from mailman.testing.helpers import get_queue_messages + >>> messages = get_queue_messages('virgin') + >>> len(messages) + 1 + >>> print(messages[0].msg['subject']) + Request to mailing list "Ant" rejected diff --git a/src/mailman/rest/lists.py b/src/mailman/rest/lists.py index f6bc27917..0607102cb 100644 --- a/src/mailman/rest/lists.py +++ b/src/mailman/rest/lists.py @@ -42,7 +42,8 @@ from mailman.rest.helpers import ( CollectionMixin, GetterSetter, NotFound, bad_request, child, created, etag, no_content, not_found, okay, paginate, path_to) from mailman.rest.members import AMember, MemberCollection -from mailman.rest.moderation import HeldMessages, SubscriptionRequests +from mailman.rest.post_moderation import HeldMessages +from mailman.rest.sub_moderation import SubscriptionRequests from mailman.rest.validator import Validator from operator import attrgetter from zope.component import getUtility diff --git a/src/mailman/rest/moderation.py b/src/mailman/rest/moderation.py deleted file mode 100644 index 984e2d34a..000000000 --- a/src/mailman/rest/moderation.py +++ /dev/null @@ -1,252 +0,0 @@ -# Copyright (C) 2012-2015 by the Free Software Foundation, Inc. -# -# This file is part of GNU Mailman. -# -# GNU Mailman is free software: you can redistribute it and/or modify it under -# the terms of the GNU General Public License as published by the Free -# Software Foundation, either version 3 of the License, or (at your option) -# any later version. -# -# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT -# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or -# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for -# more details. -# -# You should have received a copy of the GNU General Public License along with -# GNU Mailman. If not, see . - -"""REST API for Message moderation.""" - -__all__ = [ - 'HeldMessage', - 'HeldMessages', - 'MembershipChangeRequest', - 'SubscriptionRequests', - ] - - -from mailman.app.moderator import ( - handle_message, handle_subscription, handle_unsubscription) -from mailman.interfaces.action import Action -from mailman.interfaces.messages import IMessageStore -from mailman.interfaces.requests import IListRequests, RequestType -from mailman.rest.helpers import ( - CollectionMixin, bad_request, child, etag, no_content, not_found, okay) -from mailman.rest.validator import Validator, enum_validator -from zope.component import getUtility - - -HELD_MESSAGE_REQUESTS = (RequestType.held_message,) -MEMBERSHIP_CHANGE_REQUESTS = (RequestType.subscription, - RequestType.unsubscription) - - - -class _ModerationBase: - """Common base class.""" - - def _make_resource(self, request_id, expected_request_types): - requests = IListRequests(self._mlist) - results = requests.get_request(request_id) - if results is None: - return None - key, data = results - resource = dict(key=key, request_id=request_id) - # Flatten the IRequest payload into the JSON representation. - resource.update(data) - # Check for a matching request type, and insert the type name into the - # resource. - request_type = RequestType[resource.pop('_request_type')] - if request_type not in expected_request_types: - return None - resource['type'] = request_type.name - # This key isn't what you think it is. Usually, it's the Pendable - # record's row id, which isn't helpful at all. If it's not there, - # that's fine too. - resource.pop('id', None) - return resource - - - -class _HeldMessageBase(_ModerationBase): - """Held messages are a little different.""" - - def _make_resource(self, request_id): - resource = super(_HeldMessageBase, self)._make_resource( - request_id, HELD_MESSAGE_REQUESTS) - if resource is None: - return None - # Grab the message and insert its text representation into the - # resource. XXX See LP: #967954 - key = resource.pop('key') - msg = getUtility(IMessageStore).get_message_by_id(key) - resource['msg'] = msg.as_string() - # Some of the _mod_* keys we want to rename and place into the JSON - # resource. Others we can drop. Since we're mutating the dictionary, - # we need to make a copy of the keys. When you port this to Python 3, - # you'll need to list()-ify the .keys() dictionary view. - for key in list(resource): - if key in ('_mod_subject', '_mod_hold_date', '_mod_reason', - '_mod_sender', '_mod_message_id'): - resource[key[5:]] = resource.pop(key) - elif key.startswith('_mod_'): - del resource[key] - # Also, held message resources will always be this type, so ignore - # this key value. - del resource['type'] - return resource - - -class HeldMessage(_HeldMessageBase): - """Resource for moderating a held message.""" - - def __init__(self, mlist, request_id): - self._mlist = mlist - self._request_id = request_id - - def on_get(self, request, response): - try: - request_id = int(self._request_id) - except ValueError: - bad_request(response) - return - resource = self._make_resource(request_id) - if resource is None: - not_found(response) - else: - okay(response, etag(resource)) - - def on_post(self, request, response): - try: - validator = Validator(action=enum_validator(Action)) - arguments = validator(request) - except ValueError as error: - bad_request(response, str(error)) - return - requests = IListRequests(self._mlist) - try: - request_id = int(self._request_id) - except ValueError: - bad_request(response) - return - results = requests.get_request(request_id, RequestType.held_message) - if results is None: - not_found(response) - else: - handle_message(self._mlist, request_id, **arguments) - no_content(response) - - - -class HeldMessages(_HeldMessageBase, CollectionMixin): - """Resource for messages held for moderation.""" - - def __init__(self, mlist): - self._mlist = mlist - self._requests = None - - def _resource_as_dict(self, request): - """See `CollectionMixin`.""" - return self._make_resource(request.id) - - def _get_collection(self, request): - requests = IListRequests(self._mlist) - self._requests = requests - return list(requests.of_type(RequestType.held_message)) - - def on_get(self, request, response): - """/lists/listname/held""" - resource = self._make_collection(request) - okay(response, etag(resource)) - - @child(r'^(?P[^/]+)') - def message(self, request, segments, **kw): - return HeldMessage(self._mlist, kw['id']) - - - -class MembershipChangeRequest(_ModerationBase): - """Resource for moderating a membership change.""" - - def __init__(self, mlist, request_id): - self._mlist = mlist - self._request_id = request_id - - def on_get(self, request, response): - try: - request_id = int(self._request_id) - except ValueError: - bad_request(response) - return - resource = self._make_resource(request_id, MEMBERSHIP_CHANGE_REQUESTS) - if resource is None: - not_found(response) - else: - # Remove unnecessary keys. - del resource['key'] - okay(response, etag(resource)) - - def on_post(self, request, response): - try: - validator = Validator(action=enum_validator(Action)) - arguments = validator(request) - except ValueError as error: - bad_request(response, str(error)) - return - requests = IListRequests(self._mlist) - try: - request_id = int(self._request_id) - except ValueError: - bad_request(response) - return - results = requests.get_request(request_id) - if results is None: - not_found(response) - return - key, data = results - try: - request_type = RequestType[data['_request_type']] - except ValueError: - bad_request(response) - return - if request_type is RequestType.subscription: - handle_subscription(self._mlist, request_id, **arguments) - elif request_type is RequestType.unsubscription: - handle_unsubscription(self._mlist, request_id, **arguments) - else: - bad_request(response) - return - no_content(response) - - -class SubscriptionRequests(_ModerationBase, CollectionMixin): - """Resource for membership change requests.""" - - def __init__(self, mlist): - self._mlist = mlist - self._requests = None - - def _resource_as_dict(self, request): - """See `CollectionMixin`.""" - resource = self._make_resource(request.id, MEMBERSHIP_CHANGE_REQUESTS) - # Remove unnecessary keys. - del resource['key'] - return resource - - def _get_collection(self, request): - requests = IListRequests(self._mlist) - self._requests = requests - items = [] - for request_type in MEMBERSHIP_CHANGE_REQUESTS: - for request in requests.of_type(request_type): - items.append(request) - return items - - def on_get(self, request, response): - """/lists/listname/requests""" - resource = self._make_collection(request) - okay(response, etag(resource)) - - @child(r'^(?P[^/]+)') - def subscription(self, request, segments, **kw): - return MembershipChangeRequest(self._mlist, kw['id']) diff --git a/src/mailman/rest/post_moderation.py b/src/mailman/rest/post_moderation.py new file mode 100644 index 000000000..6156fa39f --- /dev/null +++ b/src/mailman/rest/post_moderation.py @@ -0,0 +1,155 @@ +# Copyright (C) 2012-2015 by the Free Software Foundation, Inc. +# +# This file is part of GNU Mailman. +# +# GNU Mailman is free software: you can redistribute it and/or modify it under +# the terms of the GNU General Public License as published by the Free +# Software Foundation, either version 3 of the License, or (at your option) +# any later version. +# +# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along with +# GNU Mailman. If not, see . + +"""REST API for held message moderation.""" + +__all__ = [ + 'HeldMessage', + 'HeldMessages', + ] + + +from mailman.app.moderator import handle_message +from mailman.interfaces.action import Action +from mailman.interfaces.messages import IMessageStore +from mailman.interfaces.requests import IListRequests, RequestType +from mailman.rest.helpers import ( + CollectionMixin, bad_request, child, etag, no_content, not_found, okay) +from mailman.rest.validator import Validator, enum_validator +from zope.component import getUtility + + + +class _ModerationBase: + """Common base class.""" + + def _make_resource(self, request_id): + requests = IListRequests(self._mlist) + results = requests.get_request(request_id) + if results is None: + return None + key, data = results + resource = dict(key=key, request_id=request_id) + # Flatten the IRequest payload into the JSON representation. + resource.update(data) + # Check for a matching request type, and insert the type name into the + # resource. + request_type = RequestType[resource.pop('_request_type')] + if request_type is not RequestType.held_message: + return None + resource['type'] = RequestType.held_message.name + # This key isn't what you think it is. Usually, it's the Pendable + # record's row id, which isn't helpful at all. If it's not there, + # that's fine too. + resource.pop('id', None) + return resource + + + +class _HeldMessageBase(_ModerationBase): + """Held messages are a little different.""" + + def _make_resource(self, request_id): + resource = super(_HeldMessageBase, self)._make_resource(request_id) + if resource is None: + return None + # Grab the message and insert its text representation into the + # resource. XXX See LP: #967954 + key = resource.pop('key') + msg = getUtility(IMessageStore).get_message_by_id(key) + resource['msg'] = msg.as_string() + # Some of the _mod_* keys we want to rename and place into the JSON + # resource. Others we can drop. Since we're mutating the dictionary, + # we need to make a copy of the keys. When you port this to Python 3, + # you'll need to list()-ify the .keys() dictionary view. + for key in list(resource): + if key in ('_mod_subject', '_mod_hold_date', '_mod_reason', + '_mod_sender', '_mod_message_id'): + resource[key[5:]] = resource.pop(key) + elif key.startswith('_mod_'): + del resource[key] + # Also, held message resources will always be this type, so ignore + # this key value. + del resource['type'] + return resource + + +class HeldMessage(_HeldMessageBase): + """Resource for moderating a held message.""" + + def __init__(self, mlist, request_id): + self._mlist = mlist + self._request_id = request_id + + def on_get(self, request, response): + try: + request_id = int(self._request_id) + except ValueError: + bad_request(response) + return + resource = self._make_resource(request_id) + if resource is None: + not_found(response) + else: + okay(response, etag(resource)) + + def on_post(self, request, response): + try: + validator = Validator(action=enum_validator(Action)) + arguments = validator(request) + except ValueError as error: + bad_request(response, str(error)) + return + requests = IListRequests(self._mlist) + try: + request_id = int(self._request_id) + except ValueError: + bad_request(response) + return + results = requests.get_request(request_id, RequestType.held_message) + if results is None: + not_found(response) + else: + handle_message(self._mlist, request_id, **arguments) + no_content(response) + + + +class HeldMessages(_HeldMessageBase, CollectionMixin): + """Resource for messages held for moderation.""" + + def __init__(self, mlist): + self._mlist = mlist + self._requests = None + + def _resource_as_dict(self, request): + """See `CollectionMixin`.""" + return self._make_resource(request.id) + + def _get_collection(self, request): + requests = IListRequests(self._mlist) + self._requests = requests + return list(requests.of_type(RequestType.held_message)) + + def on_get(self, request, response): + """/lists/listname/held""" + resource = self._make_collection(request) + okay(response, etag(resource)) + + @child(r'^(?P[^/]+)') + def message(self, request, segments, **kw): + return HeldMessage(self._mlist, kw['id']) diff --git a/src/mailman/rest/tests/test_moderation.py b/src/mailman/rest/tests/test_moderation.py index c77ae2aca..ab7383251 100644 --- a/src/mailman/rest/tests/test_moderation.py +++ b/src/mailman/rest/tests/test_moderation.py @@ -18,26 +18,28 @@ """REST moderation tests.""" __all__ = [ - 'TestModeration', + 'TestPostModeration', + 'TestSubscriptionModeration', ] import unittest from mailman.app.lifecycle import create_list -from mailman.app.moderator import hold_message, hold_subscription -from mailman.config import config +from mailman.app.moderator import hold_message from mailman.database.transaction import transaction -from mailman.interfaces.member import DeliveryMode -from mailman.interfaces.subscriptions import RequestRecord +from mailman.interfaces.registrar import IRegistrar +from mailman.interfaces.usermanager import IUserManager from mailman.testing.helpers import ( - call_api, specialized_message_from_string as mfs) + call_api, get_queue_messages, specialized_message_from_string as mfs) from mailman.testing.layers import RESTLayer +from mailman.utilities.datetime import now from urllib.error import HTTPError +from zope.component import getUtility -class TestModeration(unittest.TestCase): +class TestPostModeration(unittest.TestCase): layer = RESTLayer def setUp(self): @@ -71,24 +73,6 @@ Something else. call_api('http://localhost:9001/3.0/lists/ant@example.com/held/99') self.assertEqual(cm.exception.code, 404) - def test_subscription_request_as_held_message(self): - # Provide the request id of a subscription request using the held - # message API returns a not-found even though the request id is - # in the database. - held_id = hold_message(self._mlist, self._msg) - subscribe_id = hold_subscription( - 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: - call_api(url.format(subscribe_id)) - self.assertEqual(cm.exception.code, 404) - # But using the held_id returns a valid response. - response, content = call_api(url.format(held_id)) - self.assertEqual(response['message_id'], '') - def test_bad_held_message_action(self): # POSTing to a held message with a bad action. held_id = hold_message(self._mlist, self._msg) @@ -99,43 +83,180 @@ Something else. self.assertEqual(cm.exception.msg, b'Cannot convert parameters: action') - def test_bad_subscription_request_id(self): - # Bad request when request_id is not an integer. + def test_discard(self): + # Discarding a message removes it from the moderation queue. + with transaction(): + held_id = hold_message(self._mlist, self._msg) + url = 'http://localhost:9001/3.0/lists/ant@example.com/held/{}'.format( + held_id) + content, response = call_api(url, dict(action='discard')) + self.assertEqual(response.status, 204) + # Now it's gone. with self.assertRaises(HTTPError) as cm: - call_api('http://localhost:9001/3.0/lists/ant@example.com/' - 'requests/bogus') - self.assertEqual(cm.exception.code, 400) + call_api(url, dict(action='discard')) + self.assertEqual(cm.exception.code, 404) - def test_missing_subscription_request_id(self): - # Bad request when the request_id is not in the database. + + +class TestSubscriptionModeration(unittest.TestCase): + layer = RESTLayer + + def setUp(self): + with transaction(): + self._mlist = create_list('ant@example.com') + self._registrar = IRegistrar(self._mlist) + manager = getUtility(IUserManager) + self._anne = manager.create_address( + 'anne@example.com', 'Anne Person') + self._bart = manager.make_user( + 'bart@example.com', 'Bart Person') + preferred = list(self._bart.addresses)[0] + preferred.verified_on = now() + self._bart.preferred_address = preferred + + def test_no_such_list(self): + # Try to get the requests of a nonexistent list. + with self.assertRaises(HTTPError) as cm: + call_api('http://localhost:9001/3.0/lists/bee@example.com/' + 'requests') + self.assertEqual(cm.exception.code, 404) + + def test_no_such_subscription_token(self): + # Bad request when the token is not in the database. with self.assertRaises(HTTPError) as cm: call_api('http://localhost:9001/3.0/lists/ant@example.com/' - 'requests/99') + 'requests/missing') self.assertEqual(cm.exception.code, 404) def test_bad_subscription_action(self): # POSTing to a held message with a bad action. - held_id = hold_subscription( - 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}' + token, token_owner, member = self._registrar.register(self._anne) + # Anne's subscription request got held. + self.assertIsNone(member) + # Let's try to handle her request, but with a bogus action. + url = 'http://localhost:9001/3.0/lists/ant@example.com/requests/{}' with self.assertRaises(HTTPError) as cm: - call_api(url.format(held_id), {'action': 'bogus'}) + call_api(url.format(token), dict( + action='bogus', + )) self.assertEqual(cm.exception.code, 400) self.assertEqual(cm.exception.msg, b'Cannot convert parameters: action') + def test_list_held_requests(self): + # We can view all the held requests. + token_1, token_owner, member = self._registrar.register(self._anne) + # Anne's subscription request got held. + self.assertIsNone(member) + token_2, token_owner, member = self._registrar.register(self._bart) + content, response = call_api( + 'http://localhost:9001/3.0/lists/ant@example.com/requests') + self.assertEqual(response.status, 200) + self.assertEqual(content['total_size'], 2) + import pdb; pdb.set_trace() + + def test_accept(self): + # POST to the request to accept it. + token, token_owner, member = self._registrar.register(self._anne) + # Anne's subscription request got held. + self.assertIsNone(member) + url = 'http://localhost:9001/3.0/lists/ant@example.com/requests/{}' + with self.assertRaises(HTTPError) as cm: + call_api(url.format(token), dict( + action='accept', + )) + self.assertEqual(cm.exception.code, 204) + # Anne is a member. + self.assertEqual( + self._mlist.get_members('anne@example.com').address, + self._anne) + # The request URL no longer exists. + with self.assertRaises(HTTPError) as cm: + call_api(url.format(token), dict( + action='accept', + )) + self.assertEqual(cm.exception.code, 404) + def test_discard(self): - # Discarding a message removes it from the moderation queue. - with transaction(): - held_id = hold_message(self._mlist, self._msg) - url = 'http://localhost:9001/3.0/lists/ant@example.com/held/{}'.format( - held_id) - content, response = call_api(url, dict(action='discard')) - self.assertEqual(response.status, 204) - # Now it's gone. + # POST to the request to discard it. + token, token_owner, member = self._registrar.register(self._anne) + # Anne's subscription request got held. + self.assertIsNone(member) + url = 'http://localhost:9001/3.0/lists/ant@example.com/requests/{}' with self.assertRaises(HTTPError) as cm: - call_api(url, dict(action='discard')) + call_api(url.format(token), dict( + action='discard', + )) + self.assertEqual(cm.exception.code, 204) + # Anne is not a member. + self.assertIsNone(self._mlist.get_members('anne@example.com')) + # The request URL no longer exists. + with self.assertRaises(HTTPError) as cm: + call_api(url.format(token), dict( + action='discard', + )) + self.assertEqual(cm.exception.code, 404) + + def test_defer(self): + # Defer the decision for some other moderator. + token, token_owner, member = self._registrar.register(self._anne) + # Anne's subscription request got held. + self.assertIsNone(member) + url = 'http://localhost:9001/3.0/lists/ant@example.com/requests/{}' + with self.assertRaises(HTTPError) as cm: + call_api(url.format(token), dict( + action='defer', + )) + self.assertEqual(cm.exception.code, 204) + # Anne is not a member. + self.assertIsNone(self._mlist.get_members('anne@example.com')) + # The request URL still exists. + with self.assertRaises(HTTPError) as cm: + call_api(url.format(token), dict( + action='defer', + )) + self.assertEqual(cm.exception.code, 204) + # And now we can accept it. + with self.assertRaises(HTTPError) as cm: + call_api(url.format(token), dict( + action='accept', + )) + self.assertEqual(cm.exception.code, 204) + # Anne is a member. + self.assertEqual( + self._mlist.get_members('anne@example.com').address, + self._anne) + # The request URL no longer exists. + with self.assertRaises(HTTPError) as cm: + call_api(url.format(token), dict( + action='accept', + )) + self.assertEqual(cm.exception.code, 404) + + def test_reject(self): + # POST to the request to reject it. This leaves a bounce message in + # the virgin queue. + token, token_owner, member = self._registrar.register(self._anne) + # Anne's subscription request got held. + self.assertIsNone(member) + # There are currently no messages in the virgin queue. + items = get_queue_messages('virgin') + self.assertEqual(len(items), 0) + url = 'http://localhost:9001/3.0/lists/ant@example.com/requests/{}' + with self.assertRaises(HTTPError) as cm: + call_api(url.format(token), dict( + action='reject', + )) + self.assertEqual(cm.exception.code, 204) + # Anne is not a member. + self.assertIsNone(self._mlist.get_members('anne@example.com')) + # The request URL no longer exists. + with self.assertRaises(HTTPError) as cm: + call_api(url.format(token), dict( + action='reject', + )) self.assertEqual(cm.exception.code, 404) + # And the rejection message is now in the virgin queue. + items = get_queue_messages('virgin') + self.assertEqual(len(items), 1) + self.assertEqual(str(items[0].msg), '') -- cgit v1.2.3-70-g09d2 From 3857294c3dc8fdf294f23179926f062c4e0a6c90 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Fri, 17 Apr 2015 11:30:23 -0400 Subject: Check pointing new subscription moderation REST API. --- src/mailman/app/subscriptions.py | 5 +- src/mailman/app/tests/test_subscriptions.py | 5 +- src/mailman/rest/docs/post-moderation.rst | 1 + src/mailman/rest/docs/sub-moderation.rst | 8 +- src/mailman/rest/sub_moderation.py | 89 ++++++++++--------- src/mailman/rest/tests/test_moderation.py | 131 ++++++++++++++++++---------- 6 files changed, 147 insertions(+), 92 deletions(-) (limited to 'src/mailman/rest/tests') diff --git a/src/mailman/app/subscriptions.py b/src/mailman/app/subscriptions.py index 46ce549af..8794fb78a 100644 --- a/src/mailman/app/subscriptions.py +++ b/src/mailman/app/subscriptions.py @@ -168,8 +168,9 @@ class SubscriptionWorkflow(Workflow): return pendable = Pendable( list_id=self.mlist.list_id, - address=self.address.email, - hold_date=now().replace(microsecond=0).isoformat(), + email=self.address.email, + display_name=self.address.display_name, + when=now().replace(microsecond=0).isoformat(), token_owner=token_owner.name, ) self.token = getUtility(IPendings).add(pendable, timedelta(days=3650)) diff --git a/src/mailman/app/tests/test_subscriptions.py b/src/mailman/app/tests/test_subscriptions.py index 6b5d88026..2d5a3733b 100644 --- a/src/mailman/app/tests/test_subscriptions.py +++ b/src/mailman/app/tests/test_subscriptions.py @@ -69,8 +69,9 @@ class TestSubscriptionWorkflow(unittest.TestCase): self.assertIsNotNone(workflow.token) pendable = getUtility(IPendings).confirm(workflow.token, expunge=False) self.assertEqual(pendable['list_id'], 'test.example.com') - self.assertEqual(pendable['address'], 'anne@example.com') - self.assertEqual(pendable['hold_date'], '2005-08-01T07:49:23') + self.assertEqual(pendable['email'], 'anne@example.com') + self.assertEqual(pendable['display_name'], '') + self.assertEqual(pendable['when'], '2005-08-01T07:49:23') self.assertEqual(pendable['token_owner'], 'subscriber') def test_user_or_address_required(self): diff --git a/src/mailman/rest/docs/post-moderation.rst b/src/mailman/rest/docs/post-moderation.rst index f082de157..6dd96e71a 100644 --- a/src/mailman/rest/docs/post-moderation.rst +++ b/src/mailman/rest/docs/post-moderation.rst @@ -13,6 +13,7 @@ Held messages can be moderated through the REST API. A mailing list starts with no held messages. >>> ant = create_list('ant@example.com') + >>> transaction.commit() >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/held') http_etag: "..." start: 0 diff --git a/src/mailman/rest/docs/sub-moderation.rst b/src/mailman/rest/docs/sub-moderation.rst index a658ac410..0468d75b5 100644 --- a/src/mailman/rest/docs/sub-moderation.rst +++ b/src/mailman/rest/docs/sub-moderation.rst @@ -42,15 +42,15 @@ The message is being held for moderator approval. The subscription request can be viewed in the REST API. + >>> transaction.commit() >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/requests') entry 0: - delivery_mode: regular display_name: Anne Person email: anne@example.com http_etag: "..." - language: en - request_id: ... - type: subscription + list_id: ant.example.com + token: ... + token_owner: moderator when: 2005-08-01T07:49:23 http_etag: "..." start: 0 diff --git a/src/mailman/rest/sub_moderation.py b/src/mailman/rest/sub_moderation.py index 7bc6554a6..12bd8ab26 100644 --- a/src/mailman/rest/sub_moderation.py +++ b/src/mailman/rest/sub_moderation.py @@ -38,8 +38,11 @@ class _ModerationBase: def __init__(self): self._pendings = getUtility(IPendings) - def _make_resource(self, token): + def _resource_as_dict(self, token): pendable = self._pendings.confirm(token, expunge=False) + if pendable is None: + # This token isn't in the database. + raise LookupError resource = dict(token=token) resource.update(pendable) return resource @@ -50,28 +53,20 @@ class IndividualRequest(_ModerationBase): """Resource for moderating a membership change.""" def __init__(self, mlist, token): + super().__init__() self._mlist = mlist self._registrar = IRegistrar(self._mlist) self._token = token def on_get(self, request, response): + # Get the pended record associated with this token, if it exists in + # the pending table. try: - token, token_owner, member = self._registrar.confirm(self._token) + resource = self._resource_as_dict(self._token) except LookupError: not_found(response) return - try: - request_id = int(self._request_id) - except ValueError: - bad_request(response) - return - resource = self._make_resource(request_id, MEMBERSHIP_CHANGE_REQUESTS) - if resource is None: - not_found(response) - else: - # Remove unnecessary keys. - del resource['key'] - okay(response, etag(resource)) + okay(response, etag(resource)) def on_post(self, request, response): try: @@ -80,32 +75,34 @@ class IndividualRequest(_ModerationBase): except ValueError as error: bad_request(response, str(error)) return - requests = IListRequests(self._mlist) - try: - request_id = int(self._request_id) - except ValueError: - bad_request(response) - return - results = requests.get_request(request_id) - if results is None: - not_found(response) - return - key, data = results - try: - request_type = RequestType[data['_request_type']] - except ValueError: - bad_request(response) - return - if request_type is RequestType.subscription: - handle_subscription(self._mlist, request_id, **arguments) - elif request_type is RequestType.unsubscription: - handle_unsubscription(self._mlist, request_id, **arguments) - else: - bad_request(response) - return - no_content(response) + action = arguments['action'] + if action is Action.defer: + # At least see if the token is in the database. + pendable = self._pendings.confirm(self._token, expunge=False) + if pendable is None: + not_found(response) + else: + no_content(response) + elif action is Action.accept: + try: + self._registrar.confirm(self._token) + except LookupError: + not_found(response) + else: + no_content(response) + elif action is Action.discard: + # At least see if the token is in the database. + pendable = self._pendings.confirm(self._token, expunge=True) + if pendable is None: + not_found(response) + else: + no_content(response) + elif action is Action.reject: + # XXX + no_content(response) + class SubscriptionRequests(_ModerationBase, CollectionMixin): """Resource for membership change requests.""" @@ -116,8 +113,20 @@ class SubscriptionRequests(_ModerationBase, CollectionMixin): def _get_collection(self, request): # There's currently no better way to query the pendings database for # all the entries that are associated with subscription holds on this - # mailing list. Brute force for now. - return [token for token, pendable in getUtility(IPendings)] + # mailing list. Brute force iterating over all the pendables. + collection = [] + for token, pendable in getUtility(IPendings): + if 'token_owner' not in pendable: + # This isn't a subscription hold. + continue + list_id = pendable.get('list_id') + if list_id != self._mlist.list_id: + # Either there isn't a list_id field, in which case it can't + # be a subscription hold, or this is a hold for some other + # mailing list. + continue + collection.append(token) + return collection def on_get(self, request, response): """/lists/listname/requests""" diff --git a/src/mailman/rest/tests/test_moderation.py b/src/mailman/rest/tests/test_moderation.py index ab7383251..cc1f67f7e 100644 --- a/src/mailman/rest/tests/test_moderation.py +++ b/src/mailman/rest/tests/test_moderation.py @@ -145,30 +145,51 @@ class TestSubscriptionModeration(unittest.TestCase): def test_list_held_requests(self): # We can view all the held requests. - token_1, token_owner, member = self._registrar.register(self._anne) - # Anne's subscription request got held. - self.assertIsNone(member) - token_2, token_owner, member = self._registrar.register(self._bart) + with transaction(): + token_1, token_owner, member = self._registrar.register(self._anne) + # Anne's subscription request got held. + self.assertIsNotNone(token_1) + self.assertIsNone(member) + token_2, token_owner, member = self._registrar.register(self._bart) + self.assertIsNotNone(token_2) + self.assertIsNone(member) content, response = call_api( 'http://localhost:9001/3.0/lists/ant@example.com/requests') self.assertEqual(response.status, 200) self.assertEqual(content['total_size'], 2) - import pdb; pdb.set_trace() + tokens = set(json['token'] for json in content['entries']) + self.assertEqual(tokens, {token_1, token_2}) + emails = set(json['email'] for json in content['entries']) + self.assertEqual(emails, {'anne@example.com', 'bart@example.com'}) + + def test_individual_request(self): + # We can view an individual request. + with transaction(): + token, token_owner, member = self._registrar.register(self._anne) + # Anne's subscription request got held. + self.assertIsNotNone(token) + self.assertIsNone(member) + url = 'http://localhost:9001/3.0/lists/ant@example.com/requests/{}' + content, response = call_api(url.format(token)) + self.assertEqual(response.status, 200) + self.assertEqual(content['token'], token) + self.assertEqual(content['token_owner'], token_owner.name) + self.assertEqual(content['email'], 'anne@example.com') def test_accept(self): # POST to the request to accept it. - token, token_owner, member = self._registrar.register(self._anne) + with transaction(): + token, token_owner, member = self._registrar.register(self._anne) # Anne's subscription request got held. self.assertIsNone(member) url = 'http://localhost:9001/3.0/lists/ant@example.com/requests/{}' - with self.assertRaises(HTTPError) as cm: - call_api(url.format(token), dict( - action='accept', - )) - self.assertEqual(cm.exception.code, 204) + content, response = call_api(url.format(token), dict( + action='accept', + )) + self.assertEqual(response.status, 204) # Anne is a member. self.assertEqual( - self._mlist.get_members('anne@example.com').address, + self._mlist.members.get_member('anne@example.com').address, self._anne) # The request URL no longer exists. with self.assertRaises(HTTPError) as cm: @@ -177,19 +198,27 @@ class TestSubscriptionModeration(unittest.TestCase): )) self.assertEqual(cm.exception.code, 404) + def test_accept_bad_token(self): + # Try to accept a request with a bogus token. + with self.assertRaises(HTTPError) as cm: + call_api('http://localhost:9001/3.0/lists/ant@example.com' + '/requests/bogus', + dict(action='accept')) + self.assertEqual(cm.exception.code, 404) + def test_discard(self): # POST to the request to discard it. - token, token_owner, member = self._registrar.register(self._anne) + with transaction(): + token, token_owner, member = self._registrar.register(self._anne) # Anne's subscription request got held. self.assertIsNone(member) url = 'http://localhost:9001/3.0/lists/ant@example.com/requests/{}' - with self.assertRaises(HTTPError) as cm: - call_api(url.format(token), dict( - action='discard', - )) - self.assertEqual(cm.exception.code, 204) + content, response = call_api(url.format(token), dict( + action='discard', + )) + self.assertEqual(response.status, 204) # Anne is not a member. - self.assertIsNone(self._mlist.get_members('anne@example.com')) + self.assertIsNone(self._mlist.members.get_member('anne@example.com')) # The request URL no longer exists. with self.assertRaises(HTTPError) as cm: call_api(url.format(token), dict( @@ -199,32 +228,30 @@ class TestSubscriptionModeration(unittest.TestCase): def test_defer(self): # Defer the decision for some other moderator. - token, token_owner, member = self._registrar.register(self._anne) + with transaction(): + token, token_owner, member = self._registrar.register(self._anne) # Anne's subscription request got held. self.assertIsNone(member) url = 'http://localhost:9001/3.0/lists/ant@example.com/requests/{}' - with self.assertRaises(HTTPError) as cm: - call_api(url.format(token), dict( - action='defer', - )) - self.assertEqual(cm.exception.code, 204) + content, response = call_api(url.format(token), dict( + action='defer', + )) + self.assertEqual(response.status, 204) # Anne is not a member. - self.assertIsNone(self._mlist.get_members('anne@example.com')) + self.assertIsNone(self._mlist.members.get_member('anne@example.com')) # The request URL still exists. - with self.assertRaises(HTTPError) as cm: - call_api(url.format(token), dict( - action='defer', - )) - self.assertEqual(cm.exception.code, 204) + content, response = call_api(url.format(token), dict( + action='defer', + )) + self.assertEqual(response.status, 204) # And now we can accept it. - with self.assertRaises(HTTPError) as cm: - call_api(url.format(token), dict( - action='accept', - )) - self.assertEqual(cm.exception.code, 204) + content, response = call_api(url.format(token), dict( + action='accept', + )) + self.assertEqual(response.status, 204) # Anne is a member. self.assertEqual( - self._mlist.get_members('anne@example.com').address, + self._mlist.members.get_member('anne@example.com').address, self._anne) # The request URL no longer exists. with self.assertRaises(HTTPError) as cm: @@ -233,23 +260,31 @@ class TestSubscriptionModeration(unittest.TestCase): )) self.assertEqual(cm.exception.code, 404) + def test_defer_bad_token(self): + # Try to accept a request with a bogus token. + with self.assertRaises(HTTPError) as cm: + call_api('http://localhost:9001/3.0/lists/ant@example.com' + '/requests/bogus', + dict(action='defer')) + self.assertEqual(cm.exception.code, 404) + def test_reject(self): # POST to the request to reject it. This leaves a bounce message in # the virgin queue. - token, token_owner, member = self._registrar.register(self._anne) + with transaction(): + token, token_owner, member = self._registrar.register(self._anne) # Anne's subscription request got held. self.assertIsNone(member) # There are currently no messages in the virgin queue. items = get_queue_messages('virgin') self.assertEqual(len(items), 0) url = 'http://localhost:9001/3.0/lists/ant@example.com/requests/{}' - with self.assertRaises(HTTPError) as cm: - call_api(url.format(token), dict( - action='reject', - )) - self.assertEqual(cm.exception.code, 204) + content, response = call_api(url.format(token), dict( + action='reject', + )) + self.assertEqual(response.status, 204) # Anne is not a member. - self.assertIsNone(self._mlist.get_members('anne@example.com')) + self.assertIsNone(self._mlist.members.get_member('anne@example.com')) # The request URL no longer exists. with self.assertRaises(HTTPError) as cm: call_api(url.format(token), dict( @@ -260,3 +295,11 @@ class TestSubscriptionModeration(unittest.TestCase): items = get_queue_messages('virgin') self.assertEqual(len(items), 1) self.assertEqual(str(items[0].msg), '') + + def test_reject_bad_token(self): + # Try to accept a request with a bogus token. + with self.assertRaises(HTTPError) as cm: + call_api('http://localhost:9001/3.0/lists/ant@example.com' + '/requests/bogus', + dict(action='reject')) + self.assertEqual(cm.exception.code, 404) -- cgit v1.2.3-70-g09d2 From b37f33558b54c298916f70ff274dd06d18d3e38a Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Fri, 17 Apr 2015 14:12:00 -0400 Subject: Branch his ready. --- src/mailman/app/moderator.py | 22 ++++++++------ src/mailman/app/subscriptions.py | 10 +++++-- src/mailman/app/tests/test_registrar.py | 2 +- src/mailman/rest/docs/sub-moderation.rst | 9 ++---- src/mailman/rest/sub_moderation.py | 14 +++++++-- src/mailman/rest/tests/test_moderation.py | 48 +++++++++++++++++++++++++++---- 6 files changed, 79 insertions(+), 26 deletions(-) (limited to 'src/mailman/rest/tests') diff --git a/src/mailman/app/moderator.py b/src/mailman/app/moderator.py index 0e4d59479..eb848ea08 100644 --- a/src/mailman/app/moderator.py +++ b/src/mailman/app/moderator.py @@ -25,6 +25,7 @@ __all__ = [ 'hold_message', 'hold_subscription', 'hold_unsubscription', + 'send_rejection', ] @@ -125,8 +126,9 @@ def handle_message(mlist, id, action, language = member.preferred_language else: language = None - _refuse(mlist, _('Posting of your message titled "$subject"'), - sender, comment or _('[No reason given]'), language) + send_rejection( + mlist, _('Posting of your message titled "$subject"'), + sender, comment or _('[No reason given]'), language) elif action is Action.accept: # Start by getting the message from the message store. msg = message_store.get_message_by_id(message_id) @@ -236,10 +238,11 @@ def handle_subscription(mlist, id, action, comment=None): pass elif action is Action.reject: key, data = requestdb.get_request(id) - _refuse(mlist, _('Subscription request'), - data['email'], - comment or _('[No reason given]'), - lang=getUtility(ILanguageManager)[data['language']]) + send_rejection( + mlist, _('Subscription request'), + 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']] @@ -307,8 +310,9 @@ def handle_unsubscription(mlist, id, action, comment=None): pass elif action is Action.reject: key, data = requestdb.get_request(id) - _refuse(mlist, _('Unsubscription request'), email, - comment or _('[No reason given]')) + send_rejection( + mlist, _('Unsubscription request'), email, + comment or _('[No reason given]')) elif action is Action.accept: key, data = requestdb.get_request(id) try: @@ -324,7 +328,7 @@ def handle_unsubscription(mlist, id, action, comment=None): -def _refuse(mlist, request, recip, comment, origmsg=None, lang=None): +def send_rejection(mlist, request, recip, comment, origmsg=None, lang=None): # As this message is going to the requester, try to set the language to # his/her language choice, if they are a member. Otherwise use the list's # preferred language. diff --git a/src/mailman/app/subscriptions.py b/src/mailman/app/subscriptions.py index 8794fb78a..1593b4d58 100644 --- a/src/mailman/app/subscriptions.py +++ b/src/mailman/app/subscriptions.py @@ -159,9 +159,13 @@ class SubscriptionWorkflow(Workflow): def _set_token(self, token_owner): assert isinstance(token_owner, TokenOwner) + pendings = getUtility(IPendings) + # Clear out the previous pending token if there is one. + if self.token is not None: + pendings.confirm(self.token) # Create a new token to prevent replay attacks. It seems like this - # should produce the same token, but it won't because the pending adds - # a bit of randomization. + # would produce the same token, but it won't because the pending adds a + # bit of randomization. self.token_owner = token_owner if token_owner is TokenOwner.no_one: self.token = None @@ -173,7 +177,7 @@ class SubscriptionWorkflow(Workflow): when=now().replace(microsecond=0).isoformat(), token_owner=token_owner.name, ) - self.token = getUtility(IPendings).add(pendable, timedelta(days=3650)) + self.token = pendings.add(pendable, timedelta(days=3650)) def _step_sanity_checks(self): # Ensure that we have both an address and a user, even if the address diff --git a/src/mailman/app/tests/test_registrar.py b/src/mailman/app/tests/test_registrar.py index a2ab2c686..e76009454 100644 --- a/src/mailman/app/tests/test_registrar.py +++ b/src/mailman/app/tests/test_registrar.py @@ -59,7 +59,7 @@ class TestRegistrar(unittest.TestCase): 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') + self.assertEqual(record['email'], 'anne@example.com') def test_subscribe(self): # Registering a subscription request where no confirmation or diff --git a/src/mailman/rest/docs/sub-moderation.rst b/src/mailman/rest/docs/sub-moderation.rst index 0468d75b5..52b2b89fd 100644 --- a/src/mailman/rest/docs/sub-moderation.rst +++ b/src/mailman/rest/docs/sub-moderation.rst @@ -65,13 +65,12 @@ You can view an individual membership change request by providing the token >>> dump_json('http://localhost:9001/3.0/lists/ant@example.com/' ... 'requests/{}'.format(token)) - delivery_mode: regular display_name: Anne Person email: anne@example.com http_etag: "..." - language: en - request_id: ... - type: subscription + list_id: ant.example.com + token: ... + token_owner: moderator when: 2005-08-01T07:49:23 @@ -99,11 +98,9 @@ Anne's subscription request is accepted. Anne is now a member of the mailing list. - >>> transaction.abort() >>> ant.members.get_member('anne@example.com') on ant@example.com as MemberRole.member> - >>> transaction.abort() There are no more membership change requests. diff --git a/src/mailman/rest/sub_moderation.py b/src/mailman/rest/sub_moderation.py index 12bd8ab26..ebb09b9b3 100644 --- a/src/mailman/rest/sub_moderation.py +++ b/src/mailman/rest/sub_moderation.py @@ -22,12 +22,14 @@ __all__ = [ ] +from mailman.app.moderator import send_rejection from mailman.interfaces.action import Action from mailman.interfaces.pending import IPendings from mailman.interfaces.registrar import IRegistrar from mailman.rest.helpers import ( CollectionMixin, bad_request, child, etag, no_content, not_found, okay) from mailman.rest.validator import Validator, enum_validator +from mailman.utilities.i18n import _ from zope.component import getUtility @@ -98,8 +100,16 @@ class IndividualRequest(_ModerationBase): else: no_content(response) elif action is Action.reject: - # XXX - no_content(response) + # Like discard but sends a rejection notice to the user. + pendable = self._pendings.confirm(self._token, expunge=True) + if pendable is None: + not_found(response) + else: + no_content(response) + send_rejection( + self._mlist, _('Subscription request'), + pendable['email'], + _('[No reason given]')) diff --git a/src/mailman/rest/tests/test_moderation.py b/src/mailman/rest/tests/test_moderation.py index cc1f67f7e..e1d1f9ab3 100644 --- a/src/mailman/rest/tests/test_moderation.py +++ b/src/mailman/rest/tests/test_moderation.py @@ -28,6 +28,7 @@ import unittest from mailman.app.lifecycle import create_list from mailman.app.moderator import hold_message from mailman.database.transaction import transaction +from mailman.interfaces.mailinglist import SubscriptionPolicy from mailman.interfaces.registrar import IRegistrar from mailman.interfaces.usermanager import IUserManager from mailman.testing.helpers import ( @@ -100,6 +101,7 @@ Something else. class TestSubscriptionModeration(unittest.TestCase): layer = RESTLayer + maxDiff = None def setUp(self): with transaction(): @@ -206,6 +208,38 @@ class TestSubscriptionModeration(unittest.TestCase): dict(action='accept')) self.assertEqual(cm.exception.code, 404) + def test_accept_by_moderator_clears_request_queue(self): + # After accepting a message held for moderator approval, there are no + # more requests to handle. + # + # We start with nothing in the queue. + content, response = call_api( + 'http://localhost:9001/3.0/lists/ant@example.com/requests') + self.assertEqual(content['total_size'], 0) + # Anne tries to subscribe to a list that only requests moderator + # approval. + with transaction(): + self._mlist.subscription_policy = SubscriptionPolicy.moderate + token, token_owner, member = self._registrar.register( + self._anne, + pre_verified=True, pre_confirmed=True) + # There's now one request in the queue, and it's waiting on moderator + # approval. + content, response = call_api( + 'http://localhost:9001/3.0/lists/ant@example.com/requests') + self.assertEqual(content['total_size'], 1) + json = content['entries'][0] + self.assertEqual(json['token_owner'], 'moderator') + self.assertEqual(json['email'], 'anne@example.com') + # The moderator approves the request. + url = 'http://localhost:9001/3.0/lists/ant@example.com/requests/{}' + content, response = call_api(url.format(token), {'action': 'accept'}) + self.assertEqual(response.status, 204) + # And now the request queue is empty. + content, response = call_api( + 'http://localhost:9001/3.0/lists/ant@example.com/requests') + self.assertEqual(content['total_size'], 0) + def test_discard(self): # POST to the request to discard it. with transaction(): @@ -275,9 +309,9 @@ class TestSubscriptionModeration(unittest.TestCase): token, token_owner, member = self._registrar.register(self._anne) # Anne's subscription request got held. self.assertIsNone(member) - # There are currently no messages in the virgin queue. - items = get_queue_messages('virgin') - self.assertEqual(len(items), 0) + # Clear out the virgin queue, which currently contains the + # confirmation message sent to Anne. + get_queue_messages('virgin') url = 'http://localhost:9001/3.0/lists/ant@example.com/requests/{}' content, response = call_api(url.format(token), dict( action='reject', @@ -291,10 +325,14 @@ class TestSubscriptionModeration(unittest.TestCase): action='reject', )) self.assertEqual(cm.exception.code, 404) - # And the rejection message is now in the virgin queue. + # And the rejection message to Anne is now in the virgin queue. items = get_queue_messages('virgin') self.assertEqual(len(items), 1) - self.assertEqual(str(items[0].msg), '') + message = items[0].msg + self.assertEqual(message['From'], 'ant-bounces@example.com') + self.assertEqual(message['To'], 'anne@example.com') + self.assertEqual(message['Subject'], + 'Request to mailing list "Ant" rejected') def test_reject_bad_token(self): # Try to accept a request with a bogus token. -- cgit v1.2.3-70-g09d2