diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/mailman/app/subscriptions.py | 14 | ||||
| -rw-r--r-- | src/mailman/docs/NEWS.rst | 2 | ||||
| -rw-r--r-- | src/mailman/interfaces/subscriptions.py | 10 | ||||
| -rw-r--r-- | src/mailman/rest/members.py | 4 | ||||
| -rw-r--r-- | src/mailman/rest/tests/test_membership.py | 49 |
5 files changed, 65 insertions, 14 deletions
diff --git a/src/mailman/app/subscriptions.py b/src/mailman/app/subscriptions.py index 6a414c26a..a75d4361d 100644 --- a/src/mailman/app/subscriptions.py +++ b/src/mailman/app/subscriptions.py @@ -39,7 +39,8 @@ from mailman.interfaces.mailinglist import SubscriptionPolicy from mailman.interfaces.member import MembershipIsBannedError from mailman.interfaces.pending import IPendable, IPendings from mailman.interfaces.registrar import ConfirmationNeededEvent -from mailman.interfaces.subscriptions import ISubscriptionService, SubscriptionPendingError, TokenOwner +from mailman.interfaces.subscriptions import ( + ISubscriptionService, SubscriptionPendingError, TokenOwner) from mailman.interfaces.user import IUser from mailman.interfaces.usermanager import IUserManager from mailman.interfaces.workflow import IWorkflowStateManager @@ -185,12 +186,13 @@ class SubscriptionWorkflow(Workflow): # Is this email address banned? if IBanManager(self.mlist).is_banned(self.address.email): raise MembershipIsBannedError(self.mlist, self.address.email) - # Check if the request for this email is already pending under - # moderation - pendings = getUtility(IPendings).find(mlist = self.mlist,pend_type = 'subscription') + # Check if there is already a subscription request for this email. + pendings = getUtility(IPendings).find( + mlist=self.mlist, + pend_type='subscription') for token, pendable in pendings: - if pendable['email'] == self.address.email and pendable['token_owner'] == 'moderator': - raise SubscriptionPendingError(self.address.email, self.mlist) + if pendable['email'] == self.address.email: + raise SubscriptionPendingError(self.mlist, self.address.email) # Start out with the subscriber being the token owner. self.push('verification_checks') diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst index 79adfae43..a8a7ca89f 100644 --- a/src/mailman/docs/NEWS.rst +++ b/src/mailman/docs/NEWS.rst @@ -165,6 +165,8 @@ REST * Header match rules for individual mailing lists are now exposed in the REST API. Given by Aurélien Bompard. (Closes: #192) * Expose `goodbye_message_uri` in the REST API. Given by Harshit Bansal. + * New subscription requests are rejected if there is already one pending. + With thanks to Anirudh Dahiya. (Closes #199) Other ----- diff --git a/src/mailman/interfaces/subscriptions.py b/src/mailman/interfaces/subscriptions.py index 9ab40291c..5ce046a30 100644 --- a/src/mailman/interfaces/subscriptions.py +++ b/src/mailman/interfaces/subscriptions.py @@ -47,14 +47,11 @@ class MissingUserError(MailmanError): class SubscriptionPendingError(MailmanError): - def __init__(self, email, list_id): + def __init__(self, mlist, email): super().__init__() + self.mlist = mlist self.email = email - self.list_id = list_id - - def __str__(self): - return 'Subscription request for email {} is pending for mailing list {}'.format( - self.email,self.list_id.fqdn_listname) + class TooManyMembersError(MembershipError): def __init__(self, subscriber, list_id, role): @@ -63,6 +60,7 @@ class TooManyMembersError(MembershipError): self.list_id = list_id self.role = role + _RequestRecord = namedtuple( 'RequestRecord', diff --git a/src/mailman/rest/members.py b/src/mailman/rest/members.py index 9bbefbfc1..22db939b3 100644 --- a/src/mailman/rest/members.py +++ b/src/mailman/rest/members.py @@ -32,7 +32,6 @@ from mailman.interfaces.listmanager import IListManager from mailman.interfaces.member import ( AlreadySubscribedError, DeliveryMode, MemberRole, MembershipError, MembershipIsBannedError, MissingPreferredAddressError) -from mailman.interfaces.pending import IPendings from mailman.interfaces.registrar import IRegistrar from mailman.interfaces.subscriptions import ( ISubscriptionService, RequestRecord, SubscriptionPendingError, TokenOwner) @@ -47,6 +46,7 @@ from mailman.rest.validator import ( from uuid import UUID from zope.component import getUtility + class _MemberBase(CollectionMixin): """Shared base class for member representations.""" @@ -280,7 +280,7 @@ class AllMembers(_MemberBase): bad_request(response, b'Membership is banned') return except SubscriptionPendingError: - conflict(response ,b'Subscription request already pending') + conflict(response, b'Subscription request already pending') return if token is None: assert token_owner is TokenOwner.no_one, token_owner diff --git a/src/mailman/rest/tests/test_membership.py b/src/mailman/rest/tests/test_membership.py index 740fe82c5..57f641007 100644 --- a/src/mailman/rest/tests/test_membership.py +++ b/src/mailman/rest/tests/test_membership.py @@ -30,7 +30,10 @@ from mailman.app.lifecycle import create_list from mailman.config import config from mailman.database.transaction import transaction from mailman.interfaces.bans import IBanManager +from mailman.interfaces.mailinglist import SubscriptionPolicy from mailman.interfaces.member import DeliveryMode, MemberRole +from mailman.interfaces.registrar import IRegistrar +from mailman.interfaces.subscriptions import TokenOwner from mailman.interfaces.usermanager import IUserManager from mailman.testing.helpers import ( TestableMaster, call_api, get_lmtp_client, make_testable_runner, @@ -217,6 +220,52 @@ class TestMembership(unittest.TestCase): 'http://localhost:9001/3.0/addresses/anne@example.com') self.assertEqual(entry_0['list_id'], 'test.example.com') + def test_duplicate_pending_subscription(self): + # Issue #199 - a member's subscription is already pending and they try + # to subscribe again. + registrar = IRegistrar(self._mlist) + with transaction(): + self._mlist.subscription_policy = SubscriptionPolicy.moderate + anne = self._usermanager.create_address('anne@example.com') + token, token_owner, member = registrar.register( + anne, pre_verified=True, pre_confirmed=True) + self.assertEqual(token_owner, TokenOwner.moderator) + self.assertIsNone(member) + with self.assertRaises(HTTPError) as cm: + call_api('http://localhost:9001/3.0/members', { + 'list_id': 'test.example.com', + 'subscriber': 'anne@example.com', + 'pre_verified': True, + 'pre_confirmed': True, + }) + self.assertEqual(cm.exception.code, 409) + self.assertEqual(cm.exception.reason, + b'Subscription request already pending') + + def test_duplicate_other_pending_subscription(self): + # Issue #199 - a member's subscription is already pending and they try + # to subscribe again. Unlike above, this pend is waiting for the user + # to confirm their subscription. + registrar = IRegistrar(self._mlist) + with transaction(): + self._mlist.subscription_policy = ( + SubscriptionPolicy.confirm_then_moderate) + anne = self._usermanager.create_address('anne@example.com') + token, token_owner, member = registrar.register( + anne, pre_verified=True) + self.assertEqual(token_owner, TokenOwner.subscriber) + self.assertIsNone(member) + with self.assertRaises(HTTPError) as cm: + call_api('http://localhost:9001/3.0/members', { + 'list_id': 'test.example.com', + 'subscriber': 'anne@example.com', + 'pre_verified': True, + 'pre_confirmed': True, + }) + self.assertEqual(cm.exception.code, 409) + self.assertEqual(cm.exception.reason, + b'Subscription request already pending') + def test_member_changes_preferred_address(self): with transaction(): anne = self._usermanager.create_user('anne@example.com') |