5 files changed, 69 insertions, 28 deletions

diff --git a/src/mailman_pgp/config/mailman_pgp.cfg b/src/mailman_pgp/config/mailman_pgp.cfg
index 756d16e..d2aa84d 100644
--- a/src/mailman_pgp/config/mailman_pgp.cfg
+++ b/src/mailman_pgp/config/mailman_pgp.cfg
@@ -89,7 +89,12 @@ change_request_lifetime: 1d
 
 
 [rest]
-# Allow the acessing/modification of a list private key through the REST API?
+# Allow the acessing of a list private key through the REST API.
 # This is necessary for the django-pgpmailman web ui to allow a list owner
-# to change/export the list private key.
-expose_private_key: yes
\ No newline at end of file
+# to export the list private key.
+allow_read_private_key: yes
+
+# Allow the modification of a list private key through the REST API?
+# This is necessary for the django-pgpmailman web ui to allow a list owner
+# to change the list private key.
+allow_write_private_key: yes
\ No newline at end of file
diff --git a/src/mailman_pgp/config/schema.cfg b/src/mailman_pgp/config/schema.cfg
index 92c3f1a..ebd4377 100644
--- a/src/mailman_pgp/config/schema.cfg
+++ b/src/mailman_pgp/config/schema.cfg
@@ -57,4 +57,6 @@ change_request_lifetime: lazr.config.as_timedelta
 
 
 [rest]
-expose_private_key: lazr.config.as_boolean
\ No newline at end of file
+allow_read_private_key: lazr.config.as_boolean
+
+allow_write_private_key: lazr.config.as_boolean
\ No newline at end of file
diff --git a/src/mailman_pgp/rest/lists.py b/src/mailman_pgp/rest/lists.py
index fbece1d..c7c632a 100644
--- a/src/mailman_pgp/rest/lists.py
+++ b/src/mailman_pgp/rest/lists.py
@@ -21,9 +21,9 @@ from mailman.interfaces.action import Action
 from mailman.interfaces.listmanager import IListManager
 from mailman.interfaces.member import MemberRole
 from mailman.rest.helpers import (accepted, bad_request,
-                                  child, ChildError, CollectionMixin, etag,
+                                  child, CollectionMixin, etag,
                                   GetterSetter, no_content, not_found,
-                                  NotFound, okay)
+                                  NotFound, okay, forbidden)
 from mailman.rest.validator import (enum_validator, PatchValidator,
                                     UnknownPATCHRequestError, Validator)
 from pgpy.errors import PGPError
@@ -38,7 +38,8 @@ from mailman_pgp.utils.rest import enumflag_validator, workflow_validator
 from mailman_pgp.workflows.key_change import (KeyChangeModWorkflow,
                                               KeyChangeWorkflow)
 
-CONFIGURATION = dict(
+ATTRIBUTES = dict(
+        list_id=GetterSetter(None),
         unsigned_msg_action=GetterSetter(enum_validator(Action)),
         inline_pgp_action=GetterSetter(enum_validator(Action)),
         expired_sig_action=GetterSetter(enum_validator(Action)),
@@ -54,24 +55,25 @@ CONFIGURATION = dict(
         key_signing_allowed=GetterSetter(enumflag_validator(MemberRole))
 )
 
+VALIDATORS = ATTRIBUTES.copy()
+for attribute, gettersetter in list(VALIDATORS.items()):
+    if gettersetter.decoder is None:
+        del VALIDATORS[attribute]
+
 
 class _PGPListBase(CollectionMixin):
     def _resource_as_dict(self, emlist):
         """See `CollectionMixin`."""
-        return dict(list_id=emlist.list_id,
-                    unsigned_msg_action=emlist.unsigned_msg_action,
-                    inline_pgp_action=emlist.inline_pgp_action,
-                    expired_sig_action=emlist.expired_sig_action,
-                    revoked_sig_action=emlist.revoked_sig_action,
-                    invalid_sig_action=emlist.invalid_sig_action,
-                    duplicate_sig_action=emlist.duplicate_sig_action,
-                    strip_original_sig=emlist.strip_original_sig,
-                    sign_outgoing=emlist.sign_outgoing,
-                    nonencrypted_msg_action=emlist.nonencrypted_msg_action,
-                    encrypt_outgoing=emlist.encrypt_outgoing,
-                    self_link=self.api.path_to(
-                            '/plugins/{}/lists/{}'.format(config.name,
-                                                          emlist.list_id)))
+        resource = {}
+        for attribute, getter in ATTRIBUTES.items():
+            value = getter.get(emlist, attribute)
+            if isinstance(value, (set, frozenset)):
+                value = list(iter(value))
+            resource[attribute] = value
+        resource['self_link'] = self.api.path_to(
+                '/plugins/{}/lists/{}'.format(config.name,
+                                              emlist.list_id))
+        return resource
 
     def _get_collection(self, request):
         """See `CollectionMixin`."""
@@ -112,7 +114,7 @@ class APGPList(_PGPListBase):
         if self._mlist is None:
             not_found(response)
         else:
-            validator = Validator(**CONFIGURATION)
+            validator = Validator(**VALIDATORS)
             try:
                 with transaction():
                     validator.update(self._mlist, request)
@@ -127,7 +129,7 @@ class APGPList(_PGPListBase):
             not_found(response)
         else:
             try:
-                validator = PatchValidator(request, CONFIGURATION)
+                validator = PatchValidator(request, VALIDATORS)
             except UnknownPATCHRequestError as error:
                 bad_request(response,
                             'Unknown attribute: {}'.format(error.attribute))
@@ -144,8 +146,6 @@ class APGPList(_PGPListBase):
     def key(self, context, segments):
         if self._mlist is None:
             return NotFound(), []
-        if not config.get_value('rest', 'expose_private_key'):
-            return ChildError(403), []
         return AListKey(self._mlist), []
 
     @child()
@@ -164,6 +164,9 @@ class AListKey:
 
     def on_get(self, request, response):
         """/lists/<list_id>/key"""
+        if not config.get_value('rest', 'allow_read_private_key'):
+            forbidden(response)
+            return
         key = self._mlist.key
         if key is None:
             not_found(response)
@@ -174,6 +177,9 @@ class AListKey:
 
     def on_put(self, request, response):
         """/lists/<list_id>/key"""
+        if not config.get_value('rest', 'allow_write_private_key'):
+            forbidden(response)
+            return
         try:
             validator = Validator(key=GetterSetter(key_from_blob))
             values = validator(request)
diff --git a/src/mailman_pgp/rest/tests/test_lists.py b/src/mailman_pgp/rest/tests/test_lists.py
index 7b21023..8e3ba31 100644
--- a/src/mailman_pgp/rest/tests/test_lists.py
+++ b/src/mailman_pgp/rest/tests/test_lists.py
@@ -90,6 +90,29 @@ class TestListConfig(TestCase):
             self.pgp_list = PGPMailingList.for_list(self.mlist)
             self.pgp_list.key = self.list_key
 
+    def test_get(self):
+        json, response = call_api(
+                'http://localhost:9001/3.1/plugins/pgp/lists/'
+                'test.example.com')
+        cfg = dict(unsigned_msg_action='reject',
+                   inline_pgp_action='defer',
+                   expired_sig_action='reject',
+                   revoked_sig_action='reject',
+                   invalid_sig_action='reject',
+                   duplicate_sig_action='reject',
+                   strip_original_sig=False,
+                   sign_outgoing=False,
+                   nonencrypted_msg_action='reject',
+                   encrypt_outgoing=True,
+                   key_change_workflow='pgp-key-change-mod-workflow',
+                   key_signing_allowed=['moderator', 'owner'])
+
+        for key in cfg:
+            value = json[key]
+            if isinstance(value, list):
+                value = sorted(value)
+            self.assertEqual(cfg[key], value)
+
     def test_put(self):
         cfg = dict(unsigned_msg_action='defer',
                    inline_pgp_action='defer',
diff --git a/src/mailman_pgp/testing/mailman_pgp.cfg b/src/mailman_pgp/testing/mailman_pgp.cfg
index 3e34b48..aac192e 100644
--- a/src/mailman_pgp/testing/mailman_pgp.cfg
+++ b/src/mailman_pgp/testing/mailman_pgp.cfg
@@ -90,7 +90,12 @@ change_request_lifetime: 1d
 
 
 [rest]
-# Allow the acessing/modification of a list private key through the REST API?
+# Allow the acessing of a list private key through the REST API.
 # This is necessary for the django-pgpmailman web ui to allow a list owner
-# to change/export the list private key.
-expose_private_key: yes
+# to export the list private key.
+allow_read_private_key: yes
+
+# Allow the modification of a list private key through the REST API?
+# This is necessary for the django-pgpmailman web ui to allow a list owner
+# to change the list private key.
+allow_write_private_key: yes