Merge pull request #222 from Kinto/override_signer_per_collection

Fix overriding of per-bucket signers (ref #215)

README.rst

@@ -233,8 +233,9 @@ Settings can be prefixed with bucket id:
 
 .. code-block:: ini
 
-    kinto.signer.<bucket-id>.signer_backend = kinto_signer.signer.autograph
-    kinto.signer.<bucket-id>.autograph.server_url = http://172.11.20.1:8888
+    kinto.signer.signer_backend = kinto_signer.signer.autograph
+    kinto.signer.autograph.server_url = http://172.11.20.1:8888
+
     kinto.signer.<bucket-id>.autograph.hawk_id = bob
     kinto.signer.<bucket-id>.autograph.hawk_secret = a-secret
 

kinto_signer/__init__.py

@@ -1,3 +1,4 @@
+import re
 import pkg_resources
 import functools
 
@@ -15,33 +16,7 @@
 __version__ = pkg_resources.get_distribution(__package__).version
 
 
-def _signer_dotted_location(settings, resource):
-    """
-    Returns the Python dotted location for the specified `resource`, along
-    the associated settings prefix.
-
-    If a ``signer_backend`` setting is defined for a particular bucket
-    or a particular collection, then use the same prefix for every other
-    settings names.
-
-    .. note::
-
-        This means that every signer settings must be duplicated for each
-        dedicated signer.
-    """
-    prefix = 'signer.'
-    bucket_wide = '{bucket}.'.format(**resource['source'])
-    collection_wide = '{bucket}_{collection}.'.format(**resource['source'])
-
-    prefixes = [prefix + collection_wide, prefix + bucket_wide, prefix]
-
-    backend_setting_value = utils.get_first_matching_setting('signer_backend', settings, prefixes)
-
-    # Fallback to the local ECDSA signer.
-    default_signer_module = "kinto_signer.signer.local_ecdsa"
-    signer_dotted_location = backend_setting_value or default_signer_module
-
-    return signer_dotted_location, prefixes
+DEFAULT_SIGNER = "kinto_signer.signer.local_ecdsa"
 
 
 def includeme(config):
@@ -63,15 +38,48 @@ def includeme(config):
         "to_review_enabled": False,
         "group_check_enabled": False,
     }
+
     global_settings = {}
 
     config.registry.signers = {}
     for key, resource in resources.items():
-        # Load the signers associated to each resource.
-        dotted_location, prefixes = _signer_dotted_location(settings, resource)
-        signer_module = config.maybe_dotted(dotted_location)
-        backend = signer_module.load_from_settings(settings, prefixes=prefixes)
-        config.registry.signers[key] = backend
+
+        server_wide = 'signer.'
+        bucket_wide = 'signer.{bucket}.'.format(**resource['source'])
+
+        if resource['source']['collection'] is not None:
+            collection_wide = 'signer.{bucket}_{collection}.'.format(**resource['source'])
+            signers_prefixes = [(key, [collection_wide, bucket_wide, server_wide])]
+        else:
+            # If collection is None, it means the resource was configured for the whole bucket.
+            signers_prefixes = [(key, [bucket_wide, server_wide])]
+            # Iterate on settings to see if a specific signer config exists for
+            # a collection within this bucket.
+            bid = resource['source']['bucket']
+            # Match setting names like signer.stage_specific.autograph.hawk_id
+            matched = [re.search('signer\.{0}_([^\.]+)\.(.+)'.format(bid), k)
+                       for k, v in settings.items()]
+            for cid, unprefixed_setting_name in [m.groups() for m in matched if m]:
+                if unprefixed_setting_name in listeners.REVIEW_SETTINGS:
+                    # No need to have a custom signer for specific review settings.
+                    continue
+                # A specific signer will be instantiated and stored in the registry
+                # with collection URI key since at least one of its parameter is specific.
+                signer_key = "/buckets/{0}/collections/{1}".format(bid, cid)
+                # Define the list of prefixes for this collection. This will allow
+                # to mix collection specific with global defaults for signer settings.
+                collection_wide = 'signer.{0}_{1}.'.format(bid, cid)
+                signers_prefixes += [(signer_key, [collection_wide, bucket_wide, server_wide])]
+
+        # Instantiates the signers associated to this resource.
+        for signer_key, prefixes in signers_prefixes:
+            dotted_location = utils.get_first_matching_setting('signer_backend',
+                                                               settings,
+                                                               prefixes,
+                                                               default=DEFAULT_SIGNER)
+            signer_module = config.maybe_dotted(dotted_location)
+            backend = signer_module.load_from_settings(settings, prefixes=prefixes)
+            config.registry.signers[signer_key] = backend
 
         # Load the setttings associated to each resource.
         for setting in listeners.REVIEW_SETTINGS:

kinto_signer/listeners.py

@@ -46,11 +46,9 @@ def pick_resource_and_signer(request, resources, bucket_id, collection_id):
     # Review might have been configured explictly for this collection,
     if collection_key in resources:
         resource = resources[collection_key]
-        signer = request.registry.signers[collection_key]
     elif bucket_key in resources:
         # Or via its bucket.
         resource = copy.deepcopy(resources[bucket_key])
-        signer = request.registry.signers[bucket_key]
         # Since it was configured per bucket, we want to make this
         # resource look as if it was configured explicitly for this
         # collection.
@@ -65,6 +63,11 @@ def pick_resource_and_signer(request, resources, bucket_id, collection_id):
             if setting_key in settings:
                 resource[setting] = settings[setting_key]
 
+    if collection_key in request.registry.signers:
+        signer = request.registry.signers[collection_key]
+    elif bucket_key in request.registry.signers:
+        signer = request.registry.signers[bucket_key]
+
     return resource, signer
 
 

kinto_signer/utils.py

@@ -129,8 +129,9 @@ def parse_resources(raw_resources):
     return resources
 
 
-def get_first_matching_setting(setting_name, settings, prefixes):
+def get_first_matching_setting(setting_name, settings, prefixes, default=None):
     for prefix in prefixes:
         prefixed_setting_name = '{}{}'.format(prefix, setting_name)
         if prefixed_setting_name in settings:
             return settings[prefixed_setting_name]
+    return default

tests/test_signoff_flow.py

@@ -7,24 +7,19 @@
 from .support import BaseWebTest, get_user_headers
 
 
-def _patch_autograph():
-    # Patch calls to Autograph.
-    patch = mock.patch('kinto_signer.signer.autograph.requests')
-    mocked = patch.start()
-    mocked.post.return_value.json.return_value = [{
-        "signature": "",
-        "hash_algorithm": "",
-        "signature_encoding": "",
-        "content-signature": "",
-        "x5u": ""}]
-    return patch
-
-
 class PostgresWebTest(BaseWebTest):
     def setUp(self):
         super(PostgresWebTest, self).setUp()
-        patch = _patch_autograph()
+        # Patch calls to Autograph.
+        patch = mock.patch('kinto_signer.signer.autograph.requests')
         self.addCleanup(patch.stop)
+        self.mocked_autograph = patch.start()
+        self.mocked_autograph.post.return_value.json.return_value = [{
+            "signature": "",
+            "hash_algorithm": "",
+            "signature_encoding": "",
+            "content-signature": "",
+            "x5u": ""}]
 
         self.headers = get_user_headers('tarte:en-pion')
         resp = self.app.get("/", headers=self.headers)
@@ -510,6 +505,7 @@ def get_app_settings(cls, extras=None):
         settings['signer.to_review_enabled'] = 'true'
         settings['signer.stage_specific.to_review_enabled'] = 'false'
 
+        settings['signer.stage_specific.autograph.hawk_id'] = 'for-specific'
         return settings
 
     def test_destination_does_not_exist_at_first(self):
@@ -526,9 +522,20 @@ def test_collection_with_same_name_gets_created_when_signed(self):
                             {"data": {"status": "to-sign"}},
                             headers=self.other_headers)
         self.app.get(self.destination_collection, headers=self.headers, status=200)
+        assert self.mocked_autograph.post.called
 
     def test_review_settings_can_be_overriden_for_a_specific_collection(self):
         # review is not enabled for this particular one, sign directly!
         self.app.put_json(self.source_bucket + "/collections/specific",
                           {"data": {"status": "to-sign"}},
                           headers=self.headers)
+
+    def test_signer_can_be_specified_per_collection(self):
+        self.app.put_json(self.source_bucket + "/collections/specific",
+                          {"data": {"status": "to-sign"}},
+                          headers=self.headers)
+
+        args, kwargs = self.mocked_autograph.post.call_args_list[0]
+        assert args[0].startswith('http://localhost:8000')  # global.
+        assert kwargs['auth'].credentials['id'] == 'for-specific'
+        assert kwargs['auth'].credentials['key'].startswith('fs5w')  # global in signer.ini