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

Author: Panaetius

components/renku_data_services/authz/authz.py

@@ -559,7 +559,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

@@ -74,7 +74,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,7 +13,6 @@
 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
@@ -93,7 +92,7 @@ async def _get_one(
             headers = {"ETag": project.etag} if project.etag is not None else None
             return validated_json(apispec.Project, self._dump_project(project), 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."""
@@ -111,35 +110,33 @@ async def _get_one_by_namespace_slug(
             headers = {"ETag": project.etag} if project.etag is not None else None
             return validated_json(apispec.Project, self._dump_project(project), 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(
@@ -150,15 +147,14 @@ async def _patch(
             updated_project = project_update.new
             return validated_json(apispec.Project, self._dump_project(updated_project))
 
-        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 = []
 
@@ -178,35 +174,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

components/renku_data_services/project/db.py

@@ -212,7 +212,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))
@@ -243,7 +242,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