3 files changed, 90 insertions, 49 deletions

diff --git a/src/mailman_pgp/workflows/base.py b/src/mailman_pgp/workflows/base.py
index 3c276cf..20db5f9 100644
--- a/src/mailman_pgp/workflows/base.py
+++ b/src/mailman_pgp/workflows/base.py
@@ -45,7 +45,7 @@ Token: {}
 """
 
 
-class PubkeyMixin:
+class SetPubkeyMixin:
     def __init__(self, pubkey=None, pre_confirmed=False):
         self.pubkey = pubkey
         self.pubkey_confirmed = pre_confirmed
@@ -69,15 +69,9 @@ class PubkeyMixin:
         if pgp_address is not None:
             if not pgp_address.key:
                 self.push('send_key_request')
-            else:
-                if not pgp_address.key_confirmed:
-                    self.push('send_key_confirm_request')
         else:
             if not self.pubkey:
                 self.push('send_key_request')
-            else:
-                if not self.pubkey_confirmed:
-                    self.push('send_key_confirm_request')
 
     def _step_send_key_request(self):
         self._set_token(TokenOwner.subscriber)
@@ -103,6 +97,19 @@ class PubkeyMixin:
             self.push('send_key_request')
         else:
             self.pubkey = pgp_address.key
+
+
+class ConfirmPubkeyMixin:
+    def __init__(self, pre_confirmed=False):
+        self.pubkey_confirmed = pre_confirmed
+
+    def _step_pubkey_confirmation(self):
+        pgp_address = PGPAddress.for_address(self.address)
+
+        if pgp_address is not None:
+            if not pgp_address.key_confirmed and not self.pubkey_confirmed:
+                self.push('send_key_confirm_request')
+        else:
             if not self.pubkey_confirmed:
                 self.push('send_key_confirm_request')
 
diff --git a/src/mailman_pgp/workflows/subscription.py b/src/mailman_pgp/workflows/subscription.py
index a7565f3..c4138e6 100644
--- a/src/mailman_pgp/workflows/subscription.py
+++ b/src/mailman_pgp/workflows/subscription.py
@@ -24,12 +24,13 @@ from mailman.workflows.common import (ConfirmationMixin, ModerationMixin,
 from public import public
 from zope.interface import implementer
 
-from mailman_pgp.workflows.base import PubkeyMixin
+from mailman_pgp.workflows.base import ConfirmPubkeyMixin, SetPubkeyMixin
 
 
 @public
 @implementer(ISubscriptionWorkflow)
-class OpenSubscriptionPolicy(SubscriptionBase, VerificationMixin, PubkeyMixin):
+class OpenSubscriptionPolicy(SubscriptionBase, VerificationMixin,
+                             SetPubkeyMixin, ConfirmPubkeyMixin):
     """"""
 
     name = 'pgp-policy-open'
@@ -51,11 +52,12 @@ class OpenSubscriptionPolicy(SubscriptionBase, VerificationMixin, PubkeyMixin):
                  pubkey_pre_confirmed=False):
         SubscriptionBase.__init__(self, mlist, subscriber)
         VerificationMixin.__init__(self, pre_verified=pre_verified)
-        PubkeyMixin.__init__(self, pubkey=pubkey,
-                             pre_confirmed=pubkey_pre_confirmed)
+        SetPubkeyMixin.__init__(self, pubkey=pubkey)
+        ConfirmPubkeyMixin.__init__(self, pre_confirmed=pubkey_pre_confirmed)
 
     def _step_prepare(self):
         self.push('do_subscription')
+        self.push('pubkey_confirmation')
         self.push('pubkey_checks')
         self.push('verification_checks')
         self.push('sanity_checks')
@@ -64,7 +66,8 @@ class OpenSubscriptionPolicy(SubscriptionBase, VerificationMixin, PubkeyMixin):
 @public
 @implementer(ISubscriptionWorkflow)
 class ConfirmSubscriptionPolicy(SubscriptionBase, VerificationMixin,
-                                ConfirmationMixin, PubkeyMixin):
+                                ConfirmationMixin, SetPubkeyMixin,
+                                ConfirmPubkeyMixin):
     """"""
 
     name = 'pgp-policy-confirm'
@@ -88,11 +91,12 @@ class ConfirmSubscriptionPolicy(SubscriptionBase, VerificationMixin,
         SubscriptionBase.__init__(self, mlist, subscriber)
         VerificationMixin.__init__(self, pre_verified=pre_verified)
         ConfirmationMixin.__init__(self, pre_confirmed=pre_confirmed)
-        PubkeyMixin.__init__(self, pubkey=pubkey,
-                             pre_confirmed=pubkey_pre_confirmed)
+        SetPubkeyMixin.__init__(self, pubkey=pubkey)
+        ConfirmPubkeyMixin.__init__(self, pre_confirmed=pubkey_pre_confirmed)
 
     def _step_prepare(self):
         self.push('do_subscription')
+        self.push('pubkey_confirmation')
         self.push('pubkey_checks')
         self.push('confirmation_checks')
         self.push('verification_checks')
@@ -102,7 +106,8 @@ class ConfirmSubscriptionPolicy(SubscriptionBase, VerificationMixin,
 @public
 @implementer(ISubscriptionWorkflow)
 class ModerationSubscriptionPolicy(SubscriptionBase, VerificationMixin,
-                                   ModerationMixin, PubkeyMixin):
+                                   ModerationMixin, SetPubkeyMixin,
+                                   ConfirmPubkeyMixin):
     """"""
 
     name = 'pgp-policy-moderate'
@@ -126,12 +131,13 @@ class ModerationSubscriptionPolicy(SubscriptionBase, VerificationMixin,
         SubscriptionBase.__init__(self, mlist, subscriber)
         VerificationMixin.__init__(self, pre_verified=pre_verified)
         ModerationMixin.__init__(self, pre_approved=pre_approved)
-        PubkeyMixin.__init__(self, pubkey=pubkey,
-                             pre_confirmed=pubkey_pre_confirmed)
+        SetPubkeyMixin.__init__(self, pubkey=pubkey)
+        ConfirmPubkeyMixin.__init__(self, pre_confirmed=pubkey_pre_confirmed)
 
     def _step_prepare(self):
         self.push('do_subscription')
         self.push('moderation_checks')
+        self.push('pubkey_confirmation')
         self.push('pubkey_checks')
         self.push('verification_checks')
         self.push('sanity_checks')
@@ -141,7 +147,7 @@ class ModerationSubscriptionPolicy(SubscriptionBase, VerificationMixin,
 @implementer(ISubscriptionWorkflow)
 class ConfirmModerationSubscriptionPolicy(SubscriptionBase, VerificationMixin,
                                           ConfirmationMixin, ModerationMixin,
-                                          PubkeyMixin):
+                                          SetPubkeyMixin, ConfirmPubkeyMixin):
     """"""
 
     name = 'pgp-policy-confirm-moderate'
@@ -167,12 +173,13 @@ class ConfirmModerationSubscriptionPolicy(SubscriptionBase, VerificationMixin,
         VerificationMixin.__init__(self, pre_verified=pre_verified)
         ConfirmationMixin.__init__(self, pre_confirmed=pre_confirmed)
         ModerationMixin.__init__(self, pre_approved=pre_approved)
-        PubkeyMixin.__init__(self, pubkey=pubkey,
-                             pre_confirmed=pubkey_pre_confirmed)
+        SetPubkeyMixin.__init__(self, pubkey=pubkey)
+        ConfirmPubkeyMixin.__init__(self, pre_confirmed=pubkey_pre_confirmed)
 
     def _step_prepare(self):
         self.push('do_subscription')
         self.push('moderation_checks')
+        self.push('pubkey_confirmation')
         self.push('pubkey_checks')
         self.push('confirmation_checks')
         self.push('verification_checks')
diff --git a/src/mailman_pgp/workflows/tests/test_base.py b/src/mailman_pgp/workflows/tests/test_base.py
index ef3a009..913ab67 100644
--- a/src/mailman_pgp/workflows/tests/test_base.py
+++ b/src/mailman_pgp/workflows/tests/test_base.py
@@ -37,8 +37,7 @@ from mailman_pgp.workflows.base import KEY_REQUEST
 from mailman_pgp.workflows.subscription import ConfirmSubscriptionPolicy
 
 
-class TestPubkeyMixin(unittest.TestCase):
-    layer = PGPConfigLayer
+class PubkeyMixinSetup():
 
     def setUp(self):
         with mm_transaction():
@@ -55,19 +54,9 @@ class TestPubkeyMixin(unittest.TestCase):
         self.sender_key = load_key('rsa_1024.priv.asc')
         self.sender = self.um.create_address('rsa-1024b@example.org')
 
-    def test_pended_data(self):
-        workflow = ConfirmSubscriptionPolicy(self.mlist, self.sender,
-                                             pre_verified=True,
-                                             pre_confirmed=True)
-        with suppress(StopIteration):
-            workflow.run_thru('send_key_request')
-        self.assertIsNotNone(workflow.token)
-        pendable = getUtility(IPendings).confirm(workflow.token, expunge=False)
-        self.assertEqual(pendable['list_id'], 'test.example.com')
-        self.assertEqual(pendable['email'], 'rsa-1024b@example.org')
-        self.assertEqual(pendable['display_name'], '')
-        self.assertEqual(pendable['when'], '2005-08-01T07:49:23')
-        self.assertEqual(pendable['token_owner'], 'subscriber')
+
+class TestSetPubkeyMixin(PubkeyMixinSetup, unittest.TestCase):
+    layer = PGPConfigLayer
 
     def test_key_request_sent(self):
         workflow = ConfirmSubscriptionPolicy(self.mlist, self.sender,
@@ -81,17 +70,6 @@ class TestPubkeyMixin(unittest.TestCase):
         self.assertEqual(message['Subject'], 'key set {}'.format(token))
         self.assertEqual(message.get_payload(), KEY_REQUEST)
 
-    def test_key_request_pubkey_set(self):
-        workflow = ConfirmSubscriptionPolicy(self.mlist, self.sender,
-                                             pre_verified=True,
-                                             pre_confirmed=True,
-                                             pubkey=self.sender_key.pubkey,
-                                             pubkey_pre_confirmed=True)
-        workflow.run_thru('pubkey_checks')
-        with patch.object(workflow, '_step_do_subscription') as step:
-            next(workflow)
-        step.assert_called_once_with()
-
     def test_receive_key(self):
         workflow = ConfirmSubscriptionPolicy(self.mlist, self.sender,
                                              pre_verified=True,
@@ -111,6 +89,7 @@ class TestPubkeyMixin(unittest.TestCase):
         self.assertEqual(receive_workflow.pubkey.fingerprint,
                          self.sender_key.pubkey.fingerprint)
 
+        receive_workflow.run_thru('pubkey_confirmation')
         with patch.object(receive_workflow,
                           '_step_send_key_confirm_request') as step:
             next(receive_workflow)
@@ -146,11 +125,26 @@ class TestPubkeyMixin(unittest.TestCase):
         receive_workflow = ConfirmSubscriptionPolicy(self.mlist)
         receive_workflow.token = workflow.token
         receive_workflow.restore()
-        receive_workflow.run_thru('receive_key')
+        receive_workflow.run_thru('pubkey_confirmation')
         with patch.object(receive_workflow, '_step_do_subscription') as step:
             next(receive_workflow)
         step.assert_called_once_with()
 
+
+class TestConfirmPubkeyMixin(PubkeyMixinSetup, unittest.TestCase):
+    layer = PGPConfigLayer
+
+    def test_key_request_pubkey_set(self):
+        workflow = ConfirmSubscriptionPolicy(self.mlist, self.sender,
+                                             pre_verified=True,
+                                             pre_confirmed=True,
+                                             pubkey=self.sender_key.pubkey,
+                                             pubkey_pre_confirmed=True)
+        workflow.run_thru('pubkey_confirmation')
+        with patch.object(workflow, '_step_do_subscription') as step:
+            next(workflow)
+        step.assert_called_once_with()
+
     def test_send_key_confirm_request(self):
         workflow = ConfirmSubscriptionPolicy(self.mlist, self.sender,
                                              pre_verified=True,
@@ -187,6 +181,39 @@ class TestPubkeyMixin(unittest.TestCase):
             next(receive_workflow)
         step.assert_called_once_with()
 
+
+class TestBothPubkeyMixins(PubkeyMixinSetup, unittest.TestCase):
+    layer = PGPConfigLayer
+
+    def test_pended_data_key_request(self):
+        workflow = ConfirmSubscriptionPolicy(self.mlist, self.sender,
+                                             pre_verified=True,
+                                             pre_confirmed=True)
+        with suppress(StopIteration):
+            workflow.run_thru('send_key_request')
+        self.assertIsNotNone(workflow.token)
+        pendable = getUtility(IPendings).confirm(workflow.token, expunge=False)
+        self.assertEqual(pendable['list_id'], 'test.example.com')
+        self.assertEqual(pendable['email'], 'rsa-1024b@example.org')
+        self.assertEqual(pendable['display_name'], '')
+        self.assertEqual(pendable['when'], '2005-08-01T07:49:23')
+        self.assertEqual(pendable['token_owner'], 'subscriber')
+
+    def test_pended_data_key_confirmation(self):
+        workflow = ConfirmSubscriptionPolicy(self.mlist, self.sender,
+                                             pre_verified=True,
+                                             pre_confirmed=True,
+                                             pubkey=self.sender_key.pubkey)
+        with suppress(StopIteration):
+            workflow.run_thru('send_key_confirm_request')
+        self.assertIsNotNone(workflow.token)
+        pendable = getUtility(IPendings).confirm(workflow.token, expunge=False)
+        self.assertEqual(pendable['list_id'], 'test.example.com')
+        self.assertEqual(pendable['email'], 'rsa-1024b@example.org')
+        self.assertEqual(pendable['display_name'], '')
+        self.assertEqual(pendable['when'], '2005-08-01T07:49:23')
+        self.assertEqual(pendable['token_owner'], 'subscriber')
+
     def test_exisitng_pgp_address(self):
         workflow = ConfirmSubscriptionPolicy(self.mlist, self.sender,
                                              pre_verified=True,
@@ -198,7 +225,7 @@ class TestPubkeyMixin(unittest.TestCase):
             pgp_address.key_confirmed = True
             t.add(pgp_address)
 
-        workflow.run_thru('pubkey_checks')
+        workflow.run_thru('pubkey_confirmation')
         with patch.object(workflow, '_step_do_subscription') as step:
             next(workflow)
         step.assert_called_once_with()
@@ -213,7 +240,7 @@ class TestPubkeyMixin(unittest.TestCase):
             pgp_address.key = self.sender_key.pubkey
             t.add(pgp_address)
 
-        workflow.run_thru('pubkey_checks')
+        workflow.run_thru('pubkey_confirmation')
         with patch.object(workflow, '_step_send_key_confirm_request') as step:
             next(workflow)
         step.assert_called_once_with()