rest: string → UUID conversion for resource.id to be unique per user

This changes the UUID5 based mechanism so it depends on the user trying
to CRUD the resource. This makes sure that when using this kind of
transformation, the resource id is converted to a unique id for the
user, while preventing conflicting if every user wants to create a
"foobar" resource.

Change-Id: Iebaf3b9f8e0a198af0156008710e0c1253dc5f9d
Closes-Bug: #1617918

gnocchi/indexer/alembic/versions/397987e38570_no_more_slash_and_reencode.py

@@ -13,7 +13,7 @@
 #    under the License.
 #
 
-"""no-more-slash
+"""Remove slashes from original resource IDs, recompute their id with creator
 
 Revision ID: 397987e38570
 Revises: aba5a217ca9b
@@ -49,7 +49,8 @@ resource_table = sa.Table(
               sqlalchemy_utils.types.uuid.UUIDType(),
               nullable=False),
     sa.Column('original_resource_id', sa.String(255)),
-    sa.Column('type', sa.String(255))
+    sa.Column('type', sa.String(255)),
+    sa.Column('creator', sa.String(255))
 )
 
 resourcehistory_table = sa.Table(
@@ -100,15 +101,28 @@ def upgrade():
                       nullable=False),
         )
 
-    for resource in connection.execute(resource_table.select().where(
+    for resource in connection.execute(resource_table.select()):
-            resource_table.c.original_resource_id.like('%/%'))):
+
+        if resource_table.c.original_resource_id is None:
+            # statsd resource has no original_resource_id and is NULL
+            continue
+
+        try:
+            orig_as_uuid = uuid.UUID(
+                str(resource_table.c.original_resource_id))
+        except ValueError:
+            pass
+        else:
+            if orig_as_uuid == resource_table.c.id:
+                continue
+
         new_original_resource_id = resource.original_resource_id.replace(
             '/', '_')
         if six.PY2:
             new_original_resource_id = new_original_resource_id.encode('utf-8')
         new_id = sa.literal(uuidtype.process_bind_param(
-            str(uuid.uuid5(utils.RESOURCE_ID_NAMESPACE,
+            str(utils.ResourceUUID(
-                           new_original_resource_id)),
+                new_original_resource_id, resource.creator)),
             connection.dialect))
 
         # resource table

gnocchi/rest/__init__.py

@@ -14,6 +14,7 @@
 # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 # License for the specific language governing permissions and limitations
 # under the License.
+import functools
 import itertools
 import uuid
 
@@ -848,8 +849,10 @@ class ResourceController(rest.RestController):
 
     def __init__(self, resource_type, id):
         self._resource_type = resource_type
+        creator = pecan.request.auth_helper.get_current_user(
+            pecan.request.headers)
         try:
-            self.id = utils.ResourceUUID(id)
+            self.id = utils.ResourceUUID(id, creator)
         except ValueError:
             abort(404, indexer.NoSuchResource(id))
         self.metric = NamedMetricController(str(self.id), self._resource_type)
@@ -929,8 +932,15 @@ def schema_for(resource_type):
     return ResourceSchema(resource_type.schema)
 
 
-def ResourceID(value):
+def ResourceUUID(value, creator):
-    return (six.text_type(value), utils.ResourceUUID(value))
+    try:
+        return utils.ResourceUUID(value, creator)
+    except ValueError as e:
+        raise voluptuous.Invalid(e)
+
+
+def ResourceID(value, creator):
+    return (six.text_type(value), ResourceUUID(value, creator))
 
 
 class ResourcesController(rest.RestController):
@@ -946,7 +956,9 @@ class ResourcesController(rest.RestController):
         # NOTE(sileht): we need to copy the dict because when change it
         # and we don't want that next patch call have the "id"
         schema = dict(schema_for(self._resource_type))
-        schema["id"] = ResourceID
+        creator = pecan.request.auth_helper.get_current_user(
+            pecan.request.headers)
+        schema["id"] = functools.partial(ResourceID, creator=creator)
 
         body = deserialize_and_validate(schema)
         body["original_resource_id"], body["id"] = body["id"]
@@ -956,8 +968,6 @@ class ResourcesController(rest.RestController):
         }
         target.update(body)
         enforce("create resource", target)
-        creator = pecan.request.auth_helper.get_current_user(
-            pecan.request.headers)
         rid = body['id']
         del body['id']
         try:
@@ -1003,8 +1013,7 @@ class ResourcesController(rest.RestController):
     def delete(self, **kwargs):
         # NOTE(sileht): Don't allow empty filter, this is going to delete
         # the entire database.
-        attr_filter = deserialize_and_validate(
+        attr_filter = deserialize_and_validate(ResourceSearchSchema)
-            SearchResourceTypeController.ResourceSearchSchema)
 
         # the voluptuous checks everything, but it is better to
         # have this here.
@@ -1129,16 +1138,16 @@ class QueryStringSearchAttrFilter(object):
         return cls._parsed_query2dict(parsed_query)
 
 
-def _ResourceSearchSchema(v):
+def ResourceSearchSchema(v):
-    """Helper method to indirect the recursivity of the search schema"""
+    return _ResourceSearchSchema()(v)
-    return SearchResourceTypeController.ResourceSearchSchema(v)
 
 
-class SearchResourceTypeController(rest.RestController):
+def _ResourceSearchSchema():
-    def __init__(self, resource_type):
+    user = pecan.request.auth_helper.get_current_user(
-        self._resource_type = resource_type
+        pecan.request.headers)
+    _ResourceUUID = functools.partial(ResourceUUID, creator=user)
 
-    ResourceSearchSchema = voluptuous.Schema(
+    return voluptuous.Schema(
         voluptuous.All(
             voluptuous.Length(min=0, max=1),
             {
@@ -1155,31 +1164,36 @@ class SearchResourceTypeController(rest.RestController):
                     voluptuous.Length(min=1, max=1),
                     voluptuous.Any(
                         {"id": voluptuous.Any(
-                            utils.ResourceUUID, [utils.ResourceUUID]),
+                            [_ResourceUUID], _ResourceUUID),
                          voluptuous.Extra: voluptuous.Extra})),
                 voluptuous.Any(
                     u"and", u"∨",
                     u"or", u"∧",
                     u"not",
                 ): voluptuous.All(
-                    [_ResourceSearchSchema], voluptuous.Length(min=1)
+                    [ResourceSearchSchema], voluptuous.Length(min=1)
                 )
             }
         )
     )
 
-    @classmethod
+
-    def parse_and_validate_qs_filter(cls, query):
+class SearchResourceTypeController(rest.RestController):
+    def __init__(self, resource_type):
+        self._resource_type = resource_type
+
+    @staticmethod
+    def parse_and_validate_qs_filter(query):
         try:
             attr_filter = QueryStringSearchAttrFilter.parse(query)
         except InvalidQueryStringSearchAttrFilter as e:
             raise abort(400, e)
-        return voluptuous.Schema(cls.ResourceSearchSchema,
+        return voluptuous.Schema(ResourceSearchSchema,
                                  required=True)(attr_filter)
 
     def _search(self, **kwargs):
         if pecan.request.body:
-            attr_filter = deserialize_and_validate(self.ResourceSearchSchema)
+            attr_filter = deserialize_and_validate(ResourceSearchSchema)
         elif kwargs.get("filter"):
             attr_filter = self.parse_and_validate_qs_filter(kwargs["filter"])
         else:
@@ -1328,13 +1342,16 @@ class SearchMetricController(rest.RestController):
 
 
 class ResourcesMetricsMeasuresBatchController(rest.RestController):
-    MeasuresBatchSchema = voluptuous.Schema(
-        {ResourceID: {six.text_type: MeasuresListSchema}}
-    )
-
     @pecan.expose('json')
     def post(self, create_metrics=False):
-        body = deserialize_and_validate(self.MeasuresBatchSchema)
+        creator = pecan.request.auth_helper.get_current_user(
+            pecan.request.headers)
+        MeasuresBatchSchema = voluptuous.Schema(
+            {functools.partial(ResourceID, creator=creator):
+             {six.text_type: MeasuresListSchema}}
+        )
+
+        body = deserialize_and_validate(MeasuresBatchSchema)
 
         known_metrics = []
         unknown_metrics = []
@@ -1349,8 +1366,6 @@ class ResourcesMetricsMeasuresBatchController(rest.RestController):
 
             known_names = [m.name for m in metrics]
             if strutils.bool_from_string(create_metrics):
-                creator = pecan.request.auth_helper.get_current_user(
-                    pecan.request.headers)
                 already_exists_names = []
                 for name in names:
                     if name not in known_names:

gnocchi/tests/gabbi/gabbits/base.yaml

@@ -134,13 +134,13 @@ tests:
     project_id: f3d41b77-0cc1-4f0b-b94a-1d5be9c0e3ea
   status: 201
   response_headers:
-    location: $SCHEME://$NETLOC/v1/resource/generic/8d835270-2834-5e55-a693-fd0cf91cba3d
+    location: $SCHEME://$NETLOC/v1/resource/generic/2d869568-70d4-5ed6-9891-7d7a3bbf572d
   response_json_paths:
     type: generic
     started_at: "2014-01-03T02:02:02+00:00"
     project_id: f3d41b77-0cc1-4f0b-b94a-1d5be9c0e3ea
     created_by_project_id: 99d13f22-3618-4288-82b8-6512ded77e4f
-    id: 8d835270-2834-5e55-a693-fd0cf91cba3d
+    id: 2d869568-70d4-5ed6-9891-7d7a3bbf572d
     original_resource_id: 1.2.3.4
 
 - name: get status denied

gnocchi/tests/gabbi/gabbits/batch-measures.yaml

@@ -244,7 +244,7 @@ tests:
       response_json_paths:
           $.description.cause: "Unknown resources"
           $.description.detail:
-            - resource_id: "301dbf9a-4fce-52b6-9010-4484c469dcec"
+            - resource_id: "6b8e287d-c01a-538c-979b-a819ee49de5d"
               original_resource_id: "foobar"
 
     - name: push measurements to named metrics and resource with create_metrics with wrong measure objects

gnocchi/tests/gabbi/gabbits/resource-type.yaml

@@ -6,6 +6,11 @@
 fixtures:
     - ConfigFixture
 
+defaults:
+  request_headers:
+      x-user-id: 0fbb231484614b1a80131fc22f6afc9c
+      x-project-id: f3d41b770cc14f0bb94a1d5be9c0e3ea
+
 tests:
 
     - name: list resource type
@@ -222,8 +227,6 @@ tests:
     - name: post invalid resource
       POST: /v1/resource/my_custom_resource
       request_headers:
-          x-user-id: 0fbb2314-8461-4b1a-8013-1fc22f6afc9c
-          x-project-id: f3d41b77-0cc1-4f0b-b94a-1d5be9c0e3ea
           content-type: application/json
       data:
           id: d11edfca-4393-4fda-b94d-b05a3a1b3747
@@ -239,8 +242,6 @@ tests:
     - name: post invalid resource uuid
       POST: $LAST_URL
       request_headers:
-          x-user-id: 0fbb2314-8461-4b1a-8013-1fc22f6afc9c
-          x-project-id: f3d41b77-0cc1-4f0b-b94a-1d5be9c0e3ea
           content-type: application/json
       data:
           id: d11edfca-4393-4fda-b94d-b05a3a1b3747
@@ -258,8 +259,6 @@ tests:
     - name: post custom resource
       POST: $LAST_URL
       request_headers:
-          x-user-id: 0fbb2314-8461-4b1a-8013-1fc22f6afc9c
-          x-project-id: f3d41b77-0cc1-4f0b-b94a-1d5be9c0e3ea
           content-type: application/json
       data:
           id: d11edfca-4393-4fda-b94d-b05a3a1b3747
@@ -276,8 +275,6 @@ tests:
     - name: patch custom resource
       PATCH: /v1/resource/my_custom_resource/d11edfca-4393-4fda-b94d-b05a3a1b3747
       request_headers:
-          x-user-id: 0fbb2314-8461-4b1a-8013-1fc22f6afc9c
-          x-project-id: f3d41b77-0cc1-4f0b-b94a-1d5be9c0e3ea
           content-type: application/json
       data:
           name: foo
@@ -303,8 +300,6 @@ tests:
     - name: post resource with default
       POST: /v1/resource/my_custom_resource
       request_headers:
-          x-user-id: 0fbb2314-8461-4b1a-8013-1fc22f6afc9c
-          x-project-id: f3d41b77-0cc1-4f0b-b94a-1d5be9c0e3ea
           content-type: application/json
       data:
           id: c4110aec-6e5c-43fa-b8c5-ffdfbca3ce59

gnocchi/tests/gabbi/gabbits/transformedids.yaml

@@ -66,8 +66,26 @@ tests:
           project_id: f3d41b770cc14f0bb94a1d5be9c0e3ea
       status: 400
       response_strings:
-        - "Invalid input: not a valid value for dictionary value @ data["
+        - "'/' is not supported in resource id"
-        - "'id']  "
+
+
+    - name: post new resource non uuid again different user
+      POST: /v1/resource/generic
+      request_headers:
+        x-user-id: 0fbb231484614b1a80131fc22f6afc9b
+        x-project-id: f3d41b770cc14f0bb94a1d5be9c0e3ea
+      data:
+          id: generic zero
+          metrics:
+              cpu.util:
+                  archive_policy_name: medium
+      status: 201
+      response_json_paths:
+          created_by_user_id: 0fbb231484614b1a80131fc22f6afc9b
+          created_by_project_id: f3d41b770cc14f0bb94a1d5be9c0e3ea
+      response_headers:
+          # is a UUID
+          location: /v1/resource/generic/[a-f0-9-]{36}/
 
     - name: post new resource non uuid
       POST: /v1/resource/generic
@@ -151,7 +169,7 @@ tests:
                   archive_policy_name: medium
       status: 400
       response_strings:
-          - not a valid value for
+        - transformable resource id >255 max allowed characters for dictionary value
 
     - name: post long non uuid resource id
       POST: $LAST_URL

gnocchi/utils.py

@@ -39,23 +39,24 @@ LOG = log.getLogger(__name__)
 RESOURCE_ID_NAMESPACE = uuid.UUID('0a7a15ff-aa13-4ac2-897c-9bdf30ce175b')
 
 
-def ResourceUUID(value):
+def ResourceUUID(value, creator):
     if isinstance(value, uuid.UUID):
         return value
     if '/' in value:
         raise ValueError("'/' is not supported in resource id")
     try:
-        try:
+        return uuid.UUID(value)
-            return uuid.UUID(value)
+    except ValueError:
-        except ValueError:
+        if len(value) <= 255:
-            if len(value) <= 255:
+            # value/creator must be str (unicode) in Python 3 and str (bytes)
-                if six.PY2:
+            # in Python 2. It's not logical, I know.
-                    value = value.encode('utf-8')
+            if six.PY2:
-                return uuid.uuid5(RESOURCE_ID_NAMESPACE, value)
+                value = value.encode('utf-8')
-            raise ValueError(
+                creator = creator.encode('utf-8')
-                'transformable resource id >255 max allowed characters')
+            return uuid.uuid5(RESOURCE_ID_NAMESPACE,
-    except Exception as e:
+                              value + "\x00" + creator)
-        raise ValueError(e)
+        raise ValueError(
+            'transformable resource id >255 max allowed characters')
 
 
 def UUID(value):

releasenotes/notes/uuid5-change-8a8c467d2b2d4c85.yaml

@@ -0,0 +1,12 @@
+---
+issues:
+  - >-
+    The conversion mechanism provided by the API to convert non-UUID resource
+    id to UUID is now also based on the user creating/accessing the resource.
+    This makes sure that the conversion generates a unique UUID for the user
+    and that several users can use the same string as `original_resource_id`.
+upgrade:
+  - >-
+    Since `original_resource_id` is now unique per creator, that means users
+    cannot refer to resource by using the `original_resource_id` if the
+    resource was not created by them.

run-upgrade-tests.sh

@@ -109,12 +109,16 @@ RESOURCE_IDS=(
     "5a301761-cccc-46e2-8900-8b4f6fe6675a"
 )
 # NOTE(sileht): / are now _
-[ "$have_resource_type_post" ] && RESOURCE_ID_EXT="5a301761_dddd_46e2_8900_8b4f6fe6675a"
+# NOTE(jdanjou): and we reencode for admin:admin, but we cannot authenticate as
+# admin:admin in basic since ":" is forbidden in any username, so let's use the direct
+# computed ID
+[ "$have_resource_type_post" ] && RESOURCE_ID_EXT="517920a9-2e50-58b8-88e8-25fd7aae1d8f"
+
 dump_data $GNOCCHI_DATA/new
 
 # NOTE(sileht): change the output of the old gnocchi to compare with the new without '/'
 $GSED -i -e "s,5a301761/dddd/46e2/8900/8b4f6fe6675a,5a301761_dddd_46e2_8900_8b4f6fe6675a,g" \
-      -e "s,19235bb9-35ca-5f55-b7db-165cfb033c86,fe1bdabf-d94c-5b3a-af1e-06bdff53f228,g" $GNOCCHI_DATA/old/resources.list
+      -e "s,19235bb9-35ca-5f55-b7db-165cfb033c86,517920a9-2e50-58b8-88e8-25fd7aae1d8f,g" $GNOCCHI_DATA/old/resources.list
 
 echo "* Checking output difference between Gnocchi $old_version and $new_version"
 diff -uNr $GNOCCHI_DATA/old $GNOCCHI_DATA/new