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')