refactor: add validation to project, storage, repo and session blueprints (#347)

Author: Panaetius

components/renku_data_services/authz/authz.py

@@ -457,7 +457,7 @@ async def _get_members_helper(
             member = Member(
                 user_id=response.relationship.subject.object.object_id,
                 role=member_role,
-                resource_id=response.relationship.resource.object_id,
+                resource_id=ULID.from_str(response.relationship.resource.object_id),
             )
 
             yield member

components/renku_data_services/authz/models.py

@@ -68,7 +68,7 @@ def with_group(self, group_id: ULID) -> "Member":
 class Member(UnsavedMember):
     """Member stored in the database."""
 
-    resource_id: str | ULID
+    resource_id: ULID
 
 
 class Change(Enum):

components/renku_data_services/base_api/auth.py

@@ -71,30 +71,6 @@ async def decorated_function(request: Request, *args: _P.args, **kwargs: _P.kwar
     return decorator
 
 
-def validate_path_project_id(
-    f: Callable[Concatenate[Request, _P], Coroutine[Any, Any, _T]],
-) -> Callable[Concatenate[Request, _P], Coroutine[Any, Any, _T]]:
-    """Decorator for a Sanic handler that validates the project_id path parameter."""
-    _path_project_id_regex = re.compile(r"^[A-Za-z0-9]{26}$")
-
-    @wraps(f)
-    async def decorated_function(request: Request, *args: _P.args, **kwargs: _P.kwargs) -> _T:
-        project_id = cast(str | None, kwargs.get("project_id"))
-        if not project_id:
-            raise errors.ProgrammingError(
-                message="Could not find 'project_id' in the keyword arguments for the handler in order to validate it."
-            )
-        if not _path_project_id_regex.match(project_id):
-            raise errors.ValidationError(
-                message=f"The 'project_id' path parameter {project_id} does not match the required "
-                f"regex {_path_project_id_regex}"
-            )
-
-        return await f(request, *args, **kwargs)
-
-    return decorated_function
-
-
 def validate_path_user_id(
     f: Callable[Concatenate[Request, _P], Coroutine[Any, Any, _T]],
 ) -> Callable[Concatenate[Request, _P], Coroutine[Any, Any, _T]]:

components/renku_data_services/connected_services/apispec.py

@@ -1,6 +1,6 @@
 # generated by datamodel-codegen:
 #   filename:  api.spec.yaml
-#   timestamp: 2024-08-13T13:29:50+00:00
+#   timestamp: 2024-08-20T07:15:22+00:00
 
 from __future__ import annotations
 

components/renku_data_services/crc/apispec.py

@@ -1,6 +1,6 @@
 # generated by datamodel-codegen:
 #   filename:  api.spec.yaml
-#   timestamp: 2024-08-13T13:29:45+00:00
+#   timestamp: 2024-08-20T07:15:17+00:00
 
 from __future__ import annotations
 

components/renku_data_services/platform/apispec.py

@@ -1,6 +1,6 @@
 # generated by datamodel-codegen:
 #   filename:  api.spec.yaml
-#   timestamp: 2024-08-13T13:29:52+00:00
+#   timestamp: 2024-08-20T07:15:25+00:00
 
 from __future__ import annotations
 

components/renku_data_services/project/api.spec.yaml

@@ -144,7 +144,7 @@ paths:
           $ref: "#/components/responses/Error"
       tags:
         - projects
-  /projects/{namespace}/{slug}:
+  /namespaces/{namespace}/projects/{slug}:
     get:
       summary: Get a project by namespace and project slug
       parameters:

components/renku_data_services/project/apispec_base.py

@@ -1,6 +1,7 @@
 """Base models for API specifications."""
 
-from pydantic import BaseModel
+from pydantic import BaseModel, field_validator
+from ulid import ULID
 
 
 class BaseAPISpec(BaseModel):
@@ -13,3 +14,9 @@ class Config:
         # NOTE: By default the pydantic library does not use python for regex but a rust crate
         # this rust crate does not support lookahead regex syntax but we need it in this component
         regex_engine = "python-re"
+
+    @field_validator("id", mode="before", check_fields=False)
+    @classmethod
+    def serialize_id(cls, id: str | ULID) -> str:
+        """Custom serializer that can handle ULIDs."""
+        return str(id)

components/renku_data_services/project/blueprints.py

@@ -3,7 +3,7 @@
 from dataclasses import dataclass
 from typing import Any
 
-from sanic import HTTPResponse, Request, json
+from sanic import HTTPResponse, Request
 from sanic.response import JSONResponse
 from sanic_ext import validate
 from ulid import ULID
@@ -13,13 +13,13 @@
 from renku_data_services.base_api.auth import (
     authenticate,
     only_authenticated,
-    validate_path_project_id,
     validate_path_user_id,
 )
 from renku_data_services.base_api.blueprint import BlueprintFactoryResponse, CustomBlueprint
 from renku_data_services.base_api.etag import if_match_required
 from renku_data_services.base_api.misc import validate_body_root_model, validate_query
 from renku_data_services.base_api.pagination import PaginationRequest, paginate
+from renku_data_services.base_models.validation import validate_and_dump, validated_json
 from renku_data_services.errors import errors
 from renku_data_services.project import apispec
 from renku_data_services.project import models as project_models
@@ -48,22 +48,25 @@ async def _get_all(
             projects, total_num = await self.project_repo.get_projects(
                 user=user, pagination=pagination, namespace=query.namespace
             )
-            return [
-                dict(
-                    id=str(p.id),
-                    name=p.name,
-                    namespace=p.namespace.slug,
-                    slug=p.slug,
-                    creation_date=p.creation_date.isoformat(),
-                    created_by=p.created_by,
-                    repositories=p.repositories,
-                    visibility=p.visibility.value,
-                    description=p.description,
-                    etag=p.etag,
-                    keywords=p.keywords or [],
-                )
-                for p in projects
-            ], total_num
+            return validate_and_dump(
+                apispec.ProjectsList,
+                [
+                    dict(
+                        id=p.id,
+                        name=p.name,
+                        namespace=p.namespace.slug,
+                        slug=p.slug,
+                        creation_date=p.creation_date.isoformat(),
+                        created_by=p.created_by,
+                        repositories=p.repositories,
+                        visibility=p.visibility.value,
+                        description=p.description,
+                        etag=p.etag,
+                        keywords=p.keywords or [],
+                    )
+                    for p in projects
+                ],
+            ), total_num
 
         return "/projects", ["GET"], _get_all
 
@@ -86,9 +89,10 @@ async def _post(_: Request, user: base_models.APIUser, body: apispec.ProjectPost
                 keywords=keywords,
             )
             result = await self.project_repo.insert_project(user, project)
-            return json(
+            return validated_json(
+                apispec.Project,
                 dict(
-                    id=str(result.id),
+                    id=result.id,
                     name=result.name,
                     namespace=result.namespace.slug,
                     slug=result.slug,
@@ -109,18 +113,20 @@ def get_one(self) -> BlueprintFactoryResponse:
         """Get a specific project."""
 
         @authenticate(self.authenticator)
-        @validate_path_project_id
-        async def _get_one(request: Request, user: base_models.APIUser, project_id: str) -> JSONResponse | HTTPResponse:
-            project = await self.project_repo.get_project(user=user, project_id=ULID.from_str(project_id))
+        async def _get_one(
+            request: Request, user: base_models.APIUser, project_id: ULID
+        ) -> JSONResponse | HTTPResponse:
+            project = await self.project_repo.get_project(user=user, project_id=project_id)
 
             etag = request.headers.get("If-None-Match")
             if project.etag is not None and project.etag == etag:
                 return HTTPResponse(status=304)
 
             headers = {"ETag": project.etag} if project.etag is not None else None
-            return json(
+            return validated_json(
+                apispec.Project,
                 dict(
-                    id=str(project.id),
+                    id=project.id,
                     name=project.name,
                     namespace=project.namespace.slug,
                     slug=project.slug,
@@ -135,7 +141,7 @@ async def _get_one(request: Request, user: base_models.APIUser, project_id: str)
                 headers=headers,
             )
 
-        return "/projects/<project_id>", ["GET"], _get_one
+        return "/projects/<project_id:ulid>", ["GET"], _get_one
 
     def get_one_by_namespace_slug(self) -> BlueprintFactoryResponse:
         """Get a specific project by namespace/slug."""
@@ -151,9 +157,10 @@ async def _get_one_by_namespace_slug(
                 return HTTPResponse(status=304)
 
             headers = {"ETag": project.etag} if project.etag is not None else None
-            return json(
+            return validated_json(
+                apispec.Project,
                 dict(
-                    id=str(project.id),
+                    id=project.id,
                     name=project.name,
                     namespace=project.namespace.slug,
                     slug=project.slug,
@@ -168,35 +175,33 @@ async def _get_one_by_namespace_slug(
                 headers=headers,
             )
 
-        return "/projects/<namespace>/<slug:renku_slug>", ["GET"], _get_one_by_namespace_slug
+        return "/namespaces/<namespace>/projects/<slug:renku_slug>", ["GET"], _get_one_by_namespace_slug
 
     def delete(self) -> BlueprintFactoryResponse:
         """Delete a specific project."""
 
         @authenticate(self.authenticator)
         @only_authenticated
-        @validate_path_project_id
-        async def _delete(_: Request, user: base_models.APIUser, project_id: str) -> HTTPResponse:
-            await self.project_repo.delete_project(user=user, project_id=ULID.from_str(project_id))
+        async def _delete(_: Request, user: base_models.APIUser, project_id: ULID) -> HTTPResponse:
+            await self.project_repo.delete_project(user=user, project_id=project_id)
             return HTTPResponse(status=204)
 
-        return "/projects/<project_id>", ["DELETE"], _delete
+        return "/projects/<project_id:ulid>", ["DELETE"], _delete
 
     def patch(self) -> BlueprintFactoryResponse:
         """Partially update a specific project."""
 
         @authenticate(self.authenticator)
         @only_authenticated
-        @validate_path_project_id
         @if_match_required
         @validate(json=apispec.ProjectPatch)
         async def _patch(
-            _: Request, user: base_models.APIUser, project_id: str, body: apispec.ProjectPatch, etag: str
+            _: Request, user: base_models.APIUser, project_id: ULID, body: apispec.ProjectPatch, etag: str
         ) -> JSONResponse:
             body_dict = body.model_dump(exclude_none=True)
 
             project_update = await self.project_repo.update_project(
-                user=user, project_id=ULID.from_str(project_id), etag=etag, payload=body_dict
+                user=user, project_id=project_id, etag=etag, payload=body_dict
             )
             if not isinstance(project_update, project_models.ProjectUpdate):
                 raise errors.ProgrammingError(
@@ -205,9 +210,10 @@ async def _patch(
                 )
 
             updated_project = project_update.new
-            return json(
+            return validated_json(
+                apispec.Project,
                 dict(
-                    id=str(updated_project.id),
+                    id=updated_project.id,
                     name=updated_project.name,
                     namespace=updated_project.namespace.slug,
                     slug=updated_project.slug,
@@ -222,15 +228,14 @@ async def _patch(
                 200,
             )
 
-        return "/projects/<project_id>", ["PATCH"], _patch
+        return "/projects/<project_id:ulid>", ["PATCH"], _patch
 
     def get_all_members(self) -> BlueprintFactoryResponse:
         """List all project members."""
 
         @authenticate(self.authenticator)
-        @validate_path_project_id
-        async def _get_all_members(_: Request, user: base_models.APIUser, project_id: str) -> JSONResponse:
-            members = await self.project_member_repo.get_members(user, ULID.from_str(project_id))
+        async def _get_all_members(_: Request, user: base_models.APIUser, project_id: ULID) -> JSONResponse:
+            members = await self.project_member_repo.get_members(user, project_id)
 
             users = []
 
@@ -250,35 +255,33 @@ async def _get_all_members(_: Request, user: base_models.APIUser, project_id: st
                 ).model_dump(exclude_none=True, mode="json")
                 users.append(user_with_id)
 
-            return json(users)
+            return validated_json(apispec.ProjectMemberListResponse, users)
 
-        return "/projects/<project_id>/members", ["GET"], _get_all_members
+        return "/projects/<project_id:ulid>/members", ["GET"], _get_all_members
 
     def update_members(self) -> BlueprintFactoryResponse:
         """Update or add project members."""
 
         @authenticate(self.authenticator)
-        @validate_path_project_id
         @validate_body_root_model(json=apispec.ProjectMemberListPatchRequest)
         async def _update_members(
-            _: Request, user: base_models.APIUser, project_id: str, body: apispec.ProjectMemberListPatchRequest
+            _: Request, user: base_models.APIUser, project_id: ULID, body: apispec.ProjectMemberListPatchRequest
         ) -> HTTPResponse:
             members = [Member(Role(i.role.value), i.id, project_id) for i in body.root]
-            await self.project_member_repo.update_members(user, ULID.from_str(project_id), members)
+            await self.project_member_repo.update_members(user, project_id, members)
             return HTTPResponse(status=200)
 
-        return "/projects/<project_id>/members", ["PATCH"], _update_members
+        return "/projects/<project_id:ulid>/members", ["PATCH"], _update_members
 
     def delete_member(self) -> BlueprintFactoryResponse:
         """Delete a specific project."""
 
         @authenticate(self.authenticator)
-        @validate_path_project_id
         @validate_path_user_id
         async def _delete_member(
-            _: Request, user: base_models.APIUser, project_id: str, member_id: str
+            _: Request, user: base_models.APIUser, project_id: ULID, member_id: str
         ) -> HTTPResponse:
-            await self.project_member_repo.delete_members(user, ULID.from_str(project_id), [member_id])
+            await self.project_member_repo.delete_members(user, project_id, [member_id])
             return HTTPResponse(status=204)
 
-        return "/projects/<project_id>/members/<member_id>", ["DELETE"], _delete_member
+        return "/projects/<project_id:ulid>/members/<member_id>", ["DELETE"], _delete_member

components/renku_data_services/project/db.py

@@ -199,7 +199,6 @@ async def update_project(
         session: AsyncSession | None = None,
     ) -> models.ProjectUpdate:
         """Update a project entry."""
-        project_id_str: str = str(project_id)
         if not session:
             raise errors.ProgrammingError(message="A database session is required")
         result = await session.scalars(select(schemas.ProjectORM).where(schemas.ProjectORM.id == project_id))
@@ -230,7 +229,7 @@ async def update_project(
 
         if "repositories" in payload:
             payload["repositories"] = [
-                schemas.ProjectRepositoryORM(url=r, project_id=project_id_str, project=project)
+                schemas.ProjectRepositoryORM(url=r, project_id=project_id, project=project)
                 for r in payload["repositories"]
             ]
             # Trigger update for ``updated_at`` column