Users do not have to belong to the editors group to request review anymore (fixes #150)

CHANGELOG.rst

@@ -4,6 +4,14 @@ CHANGELOG
 This document describes changes between each past release as well as
 the version control of each dependency.
 
+28.0.0 (2021-12-10)
+===================
+
+**Breaking Changes**
+
+- Users do not have to belong to the ``editors`` group to request review anymore (#1952).
+  The metadata in the ``signer`` capability does not have the ``editors_group`` fields anymore.
+
 
 27.0.0 (2021-12-10)
 ===================

config/example.ini

@@ -152,7 +152,6 @@ kinto.attachment.base_path = /tmp
 # Kinto signer
 #
 kinto.signer.resources = /buckets/source -> /buckets/preview -> /buckets/destination
-kinto.signer.editors_group = {collection_id}-editors
 kinto.signer.reviewers_group = {collection_id}-reviewers
 
 kinto.signer.to_review_enabled = true

kinto_remote_settings/README.rst

@@ -220,26 +220,23 @@ makes sure that:
 
 * the collection is reviewed before being signed
 * the user asking for review is the not the one approving the review
-* the user asking for review belongs to a group ``editors`` and
-  the one approving the review belongs to ``reviewers``.
+* the user approving the review belongs to ``reviewers``.
 
 +----------------------------------+---------------+--------------------------------------------------------------------------+
 | Setting name                     | Default       | What does it do?                                                         |
 +==================================+===============+==========================================================================+
 | kinto.signer.to_review_enabled   | ``false``     | If ``true``, the collection ``status`` must be set to ``to-review`` by a |
 |                                  |               | different user before being set to ``to-sign``.                          |
 +----------------------------------+---------------+--------------------------------------------------------------------------+
-| kinto.signer.editors_group       | ``editors``   | The group id that is required for changing status to ``to-review``       |
-+----------------------------------+---------------+--------------------------------------------------------------------------+
 | kinto.signer.reviewers_group     | ``reviewers`` | The group id that is required for changing status to ``to-sign``         |
 +----------------------------------+---------------+--------------------------------------------------------------------------+
 
 .. warning::
 
-    The ``editors`` and ``reviewers`` groups are defined in the **source bucket**
-    (e.g. ``/buckets/staging/groups/editors``).
+    The ``reviewers`` group is defined in the **source bucket**
+    (e.g. ``/buckets/staging/groups/reviewers``).
 
-    The ``editors`` and ``reviewers`` groups can have placeholders that are resolved
+    The ``reviewers`` group can have placeholders that are resolved
     with the source **source bucket/collection**
     (e.g. ``group:/buckets/{bucket_id}/groups/{collection_id}-reviewers``).
 
@@ -272,7 +269,7 @@ collection will be enabled.
 .. image:: workflow.png
 
 
-The editors and reviewers groups are automatically created when the source collection is created.
+The reviewers group is automatically created when the source collection is created.
 
 
 Multiple certificates

kinto_remote_settings/signer/__init__.py

@@ -94,7 +94,6 @@ def includeme(config):
     # Determine which are the settings that apply to all buckets/collections.
     defaults = {
         "reviewers_group": "reviewers",
-        "editors_group": "editors",
         "to_review_enabled": False,
     }
     global_settings = {}
@@ -199,9 +198,8 @@ def includeme(config):
 
     config.add_subscriber(
         functools.partial(
-            listeners.create_editors_reviewers_groups,
+            listeners.create_reviewers_groups,
             resources=resources,
-            editors_group=global_settings["editors_group"],
             reviewers_group=global_settings["reviewers_group"],
         ),
         ResourceChanged,

kinto_remote_settings/signer/listeners.py

@@ -14,7 +14,6 @@
 
 REVIEW_SETTINGS = (
     "reviewers_group",
-    "editors_group",
     "to_review_enabled",
 )
 
@@ -248,7 +247,6 @@ def check_collection_status(
     event,
     resources,
     to_review_enabled,
-    editors_group,
     reviewers_group,
 ):
     """Make sure status changes are allowed."""
@@ -279,16 +277,10 @@ def check_collection_status(
 
         # to-review and group checking.
         _to_review_enabled = resource.get("to_review_enabled", to_review_enabled)
-        _editors_group = resource_group(
-            resource, "editors_group", default=editors_group
-        )
         _reviewers_group = resource_group(
             resource, "reviewers_group", default=reviewers_group
         )
-        # Member of groups have their URIs in their principals.
-        editors_group_uri = instance_uri(
-            event.request, "group", bucket_id=payload["bucket_id"], id=_editors_group
-        )
+        # Member of reviewers group have their URIs in their principals.
         reviewers_group_uri = instance_uri(
             event.request, "group", bucket_id=payload["bucket_id"], id=_reviewers_group
         )
@@ -307,8 +299,7 @@ def check_collection_status(
 
         # 2. work-in-progress -> to-review
         elif new_status == STATUS.TO_REVIEW:
-            if editors_group_uri not in user_principals:
-                raise_forbidden(message="Not in %s group" % _editors_group)
+            pass
 
         # 3. to-review -> work-in-progress
         # 3. to-review -> to-sign
@@ -561,7 +552,7 @@ def set_work_in_progress_status(event, resources):
     updater.update_source_status(STATUS.WORK_IN_PROGRESS, event.request)
 
 
-def create_editors_reviewers_groups(event, resources, editors_group, reviewers_group):
+def create_reviewers_groups(event, resources, reviewers_group):
     if event.request.prefixed_userid == PLUGIN_USERID:
         return
 
@@ -586,9 +577,6 @@ def create_editors_reviewers_groups(event, resources, editors_group, reviewers_g
         if resource is None:
             continue
 
-        _editors_group = resource_group(
-            resource, "editors_group", default=editors_group
-        )
         _reviewers_group = resource_group(
             resource, "reviewers_group", default=reviewers_group
         )
@@ -599,32 +587,27 @@ def create_editors_reviewers_groups(event, resources, editors_group, reviewers_g
             return
 
         group_perms = {"write": [current_user_id]}
-        for group, members in (
-            (_editors_group, [current_user_id]),
-            (_reviewers_group, []),
-        ):
-            ensure_resource_exists(
-                request=event.request,
-                resource_name="group",
-                parent_id=bucket_uri,
-                obj={"id": group, "members": members},
-                permissions=group_perms,
-                matchdict={"bucket_id": bid, "id": group},
-            )
+        ensure_resource_exists(
+            request=event.request,
+            resource_name="group",
+            parent_id=bucket_uri,
+            obj={"id": _reviewers_group, "members": []},
+            permissions=group_perms,
+            matchdict={"bucket_id": bid, "id": _reviewers_group},
+        )
 
-        # Allow those groups to write to the source collection.
+        # Allow reviewers to write to the source collection.
         permission = event.request.registry.permission
         collection_uri = instance_uri(
             event.request,
             "collection",
             bucket_id=bid,
             id=resource["source"]["collection"],
         )
-        for group in (_editors_group, _reviewers_group):
-            group_principal = instance_uri(
-                event.request, "group", bucket_id=bid, id=group
-            )
-            permission.add_principal_to_ace(collection_uri, "write", group_principal)
+        group_principal = instance_uri(
+            event.request, "group", bucket_id=bid, id=_reviewers_group
+        )
+        permission.add_principal_to_ace(collection_uri, "write", group_principal)
 
 
 def cleanup_preview_destination(event, resources):

kinto_remote_settings/signer/tests/functional.py

@@ -106,7 +106,6 @@ def setUp(self):
             user_principal(self.someone_client),
             user_principal(self.source),
         ]
-        create_group(self.source, "editors", members=principals)
         create_group(self.source, "reviewers", members=principals)
 
         # Create some data on the source collection and send it.
@@ -303,7 +302,6 @@ def setUp(self):
         # Give the permission to write in collection to anybody
         self.source.create_bucket()
         principals = [user_principal(self.editor_client), user_principal(self.source)]
-        create_group(self.source, "editors", members=principals)
         create_group(self.source, "reviewers", members=principals)
 
         perms = {"write": ["system.Authenticated"]}
@@ -376,9 +374,6 @@ def setUpClass(cls):
     def setUp(self):
         perms = {"write": ["system.Authenticated"]}
         self.client.create_bucket()
-        create_group(
-            self.client, "editors", members=[self.anna_principal, self.client_principal]
-        )
         create_group(
             self.client,
             "reviewers",
@@ -407,10 +402,6 @@ def test_whole_workflow(self):
         status = self.client.get_collection()["data"]["status"]
         assert status == "signed"
 
-    def test_only_editors_can_ask_for_review(self):
-        with self.assertRaises(KintoException):
-            self.elsa_client.patch_collection(data={"status": "to-review"})
-
     def test_status_can_be_maintained_as_to_review(self):
         self.anna_client.patch_collection(data={"status": "to-review"})
         self.elsa_client.patch_collection(data={"status": "to-review"})
@@ -562,11 +553,9 @@ def test_anyone_can_create_collections(self):
         collection = self.julia_client.create_collection(id="pim")
         assert self.julia_principal in collection["permissions"]["write"]
 
-    def test_editors_and_reviewers_groups_are_created(self):
+    def test_reviewers_group_is_created(self):
         self.julia_client.create_collection(id="pam")
-        editors_group = self.julia_client.get_group(id="editors")
         reviewers_group = self.julia_client.get_group(id="reviewers")
-        assert self.julia_principal in editors_group["data"]["members"]
         assert self.julia_principal not in reviewers_group["data"]["members"]
 
     def test_preview_and_destination_collections_are_signed(self):
@@ -588,7 +577,6 @@ def test_collection_can_be_deleted(self):
         # The following objects are still here, thus not raising:
         self.anon_client.get_collection(bucket="preview", id="poum")
         self.anon_client.get_collection(bucket="prod", id="poum")
-        self.julia_client.get_group(id="editors")
         self.julia_client.get_group(id="reviewers")
 
     def test_full_review_test(self):

kinto_remote_settings/signer/tests/test_plugin_setup.py

@@ -38,7 +38,6 @@ def test_capability_is_exposed(self):
             "url": ("https://github.com/Kinto/kinto-signer#kinto-signer"),
             "version": __version__,
             "to_review_enabled": False,
-            "editors_group": "editors",
             "reviewers_group": "{bucket_id}-{collection_id}-reviewers",
             "resources": [
                 {
@@ -232,7 +231,7 @@ def test_a_statsd_timer_is_used_for_signature_if_configured(self):
     def test_includeme_raises_value_error_if_unknown_placeholder(self):
         settings = {
             "signer.resources": "/buckets/sb1/collections/sc1 -> /buckets/db1/collections/dc1",  # noqa: 501
-            "signer.editors_group": "{datetime}_group",
+            "signer.reviewers_group": "{datetime}_group",
             "signer.ecdsa.public_key": "/path/to/key",
             "signer.ecdsa.private_key": "/path/to/private",
         }
@@ -460,7 +459,6 @@ class SourceCollectionDeletion(BaseWebTest, unittest.TestCase):
     def get_app_settings(cls, extras=None):
         settings = super(cls, SourceCollectionDeletion).get_app_settings(extras)
         settings["signer.to_review_enabled"] = "true"
-        settings["signer.stage.editors_group"] = "something"
         return settings
 
     def setUp(self):
@@ -491,11 +489,6 @@ def setUp(self):
 
         self.app.put_json("/buckets/stage", headers=self.headers)
 
-        self.app.put_json(
-            "/buckets/stage/groups/something",
-            {"data": {"members": [self.other_userid]}},
-            headers=self.headers,
-        )
         self.app.put_json(
             "/buckets/stage/groups/reviewers",
             {"data": {"members": [self.userid]}},