refactor: add validation to project blueprints

Author: Panaetius

components/renku_data_services/authz/authz.py

@@ -26,6 +26,7 @@
 )
 from sanic.log import logger
 from sqlalchemy.ext.asyncio import AsyncSession
+from ulid import ULID
 
 from renku_data_services import base_models
 from renku_data_services.authz.config import AuthzConfig
@@ -132,8 +133,8 @@ class AuthzOperation(StrEnum):
 
 class _AuthzConverter:
     @staticmethod
-    def project(id: str) -> ObjectReference:
-        return ObjectReference(object_type=ResourceType.project.value, object_id=id)
+    def project(id: ULID) -> ObjectReference:
+        return ObjectReference(object_type=ResourceType.project.value, object_id=str(id))
 
     @staticmethod
     def user(id: str | None) -> ObjectReference:
@@ -170,9 +171,9 @@ def user_namespace(id: ULID) -> ObjectReference:
         return ObjectReference(object_type=ResourceType.user_namespace, object_id=str(id))
 
     @staticmethod
-    def to_object(resource_type: ResourceType, resource_id: str | int) -> ObjectReference:
+    def to_object(resource_type: ResourceType, resource_id: str | ULID | int) -> ObjectReference:
         match (resource_type, resource_id):
-            case (ResourceType.project, sid) if isinstance(sid, str):
+            case (ResourceType.project, sid) if isinstance(sid, ULID):
                 return _AuthzConverter.project(sid)
             case (ResourceType.user, sid) if isinstance(sid, str) or sid is None:
                 return _AuthzConverter.user(sid)
@@ -242,23 +243,26 @@ async def decorated_function(
     return decorator
 
 
+T = TypeVar("T", str, ULID)
+
+
 def _is_allowed(
     operation: Scope,
 ) -> Callable[
-    [Callable[Concatenate["Authz", base_models.APIUser, ResourceType, str, _P], Awaitable[_T]]],
-    Callable[Concatenate["Authz", base_models.APIUser, ResourceType, str, _P], Awaitable[_T]],
+    [Callable[Concatenate["Authz", base_models.APIUser, ResourceType, T, _P], Awaitable[_T]]],
+    Callable[Concatenate["Authz", base_models.APIUser, ResourceType, T, _P], Awaitable[_T]],
 ]:
     """A decorator that checks if the operation on a resource is allowed or not."""
 
     def decorator(
-        f: Callable[Concatenate["Authz", base_models.APIUser, ResourceType, str, _P], Awaitable[_T]],
-    ) -> Callable[Concatenate["Authz", base_models.APIUser, ResourceType, str, _P], Awaitable[_T]]:
+        f: Callable[Concatenate["Authz", base_models.APIUser, ResourceType, T, _P], Awaitable[_T]],
+    ) -> Callable[Concatenate["Authz", base_models.APIUser, ResourceType, T, _P], Awaitable[_T]]:
         @wraps(f)
         async def decorated_function(
             self: "Authz",
             user: base_models.APIUser,
             resource_type: ResourceType,
-            resource_id: str,
+            resource_id: T,
             *args: _P.args,
             **kwargs: _P.kwargs,
         ) -> _T:
@@ -294,7 +298,7 @@ def client(self) -> AsyncClient:
         return self._client
 
     async def _has_permission(
-        self, user: base_models.APIUser, resource_type: ResourceType, resource_id: str | None, scope: Scope
+        self, user: base_models.APIUser, resource_type: ResourceType, resource_id: str | ULID | None, scope: Scope
     ) -> tuple[bool, ZedToken | None]:
         """Checks whether the provided user has a specific permission on the specific resource."""
         if not resource_id:
@@ -317,7 +321,7 @@ async def _has_permission(
         return response.permissionship == CheckPermissionResponse.PERMISSIONSHIP_HAS_PERMISSION, response.checked_at
 
     async def has_permission(
-        self, user: base_models.APIUser, resource_type: ResourceType, resource_id: str, scope: Scope
+        self, user: base_models.APIUser, resource_type: ResourceType, resource_id: str | ULID, scope: Scope
     ) -> bool:
         """Checks whether the provided user has a specific permission on the specific resource."""
         res, _ = await self._has_permission(user, resource_type, resource_id, scope)
@@ -386,12 +390,13 @@ async def members(
         self,
         user: base_models.APIUser,
         resource_type: ResourceType,
-        resource_id: str,
+        resource_id: _T,
         role: Role | None = None,
         *,
         zed_token: ZedToken | None = None,
     ) -> list[Member]:
         """Get all users that are members of a resource, if role is None then all roles are retrieved."""
+        resource_id: str = str(resource_id)
         consistency = Consistency(at_least_as_fresh=zed_token) if zed_token else Consistency(fully_consistent=True)
         sub_filter = SubjectFilter(subject_type=ResourceType.user.value)
         rel_filter = RelationshipFilter(
@@ -500,7 +505,7 @@ async def _get_authz_change(
                             )
                         authz_change.extend(db_repo.authz._add_user_namespace(res.namespace))
                 case _:
-                    resource_id: str | None = "unknown"
+                    resource_id: str | ULID | None = "unknown"
                     if isinstance(result, (Project, Namespace, Group)):
                         resource_id = result.id
                     elif isinstance(result, (ProjectUpdate, NamespaceUpdate, GroupUpdate)):
@@ -619,7 +624,7 @@ async def _remove_project(
     ) -> _AuthzChange:
         """Remove the relationships associated with the project."""
         consistency = Consistency(at_least_as_fresh=zed_token) if zed_token else Consistency(fully_consistent=True)
-        rel_filter = RelationshipFilter(resource_type=ResourceType.project.value, optional_resource_id=project.id)
+        rel_filter = RelationshipFilter(resource_type=ResourceType.project.value, optional_resource_id=str(project.id))
         responses: AsyncIterable[ReadRelationshipsResponse] = self.client.ReadRelationships(
             ReadRelationshipsRequest(consistency=consistency, relationship_filter=rel_filter)
         )
@@ -640,6 +645,7 @@ async def _update_project_visibility(
         self, user: base_models.APIUser, project: Project, *, zed_token: ZedToken | None = None
     ) -> _AuthzChange:
         """Update the visibility of the project in the authorization database."""
+        project_id = str(project.id)
         consistency = Consistency(at_least_as_fresh=zed_token) if zed_token else Consistency(fully_consistent=True)
         project_res = _AuthzConverter.project(project.id)
         all_users_sub = SubjectReference(object=_AuthzConverter.all_users())
@@ -668,7 +674,7 @@ async def _update_project_visibility(
         )
         rel_filter = RelationshipFilter(
             resource_type=ResourceType.project.value,
-            optional_resource_id=project.id,
+            optional_resource_id=project_id,
             optional_subject_filter=SubjectFilter(
                 subject_type=ResourceType.user.value, optional_subject_id=all_users_sub.object.object_id
             ),
@@ -678,7 +684,7 @@ async def _update_project_visibility(
         )
         rel_filter = RelationshipFilter(
             resource_type=ResourceType.project.value,
-            optional_resource_id=project.id,
+            optional_resource_id=project_id,
             optional_subject_filter=SubjectFilter(
                 subject_type=ResourceType.anonymous_user.value,
                 optional_subject_id=anon_users_sub.object.object_id,
@@ -724,11 +730,12 @@ async def _update_project_namespace(
         self, user: base_models.APIUser, project: Project, *, zed_token: ZedToken | None = None
     ) -> _AuthzChange:
         """Update the namespace/group of the project in the authorization database."""
+        project_id = str(project.id)
         consistency = Consistency(at_least_as_fresh=zed_token) if zed_token else Consistency(fully_consistent=True)
         project_res = _AuthzConverter.project(project.id)
         project_namespace_filter = RelationshipFilter(
             resource_type=ResourceType.project.value,
-            optional_resource_id=project.id,
+            optional_resource_id=str(project_id),
             optional_relation=_Relation.project_namespace.value,
         )
         current_namespace: ReadRelationshipsResponse | None = await anext(
@@ -808,7 +815,7 @@ async def upsert_project_members(
         self,
         user: base_models.APIUser,
         resource_type: ResourceType,
-        resource_id: str,
+        resource_id: ULID,
         members: list[Member],
         *,
         zed_token: ZedToken | None = None,
@@ -823,7 +830,8 @@ async def upsert_project_members(
         undo: list[RelationshipUpdate] = []
         output: list[MembershipChange] = []
         expected_user_roles = {_Relation.viewer.value, _Relation.owner.value, _Relation.editor.value}
-        existing_owners_rels = await self._get_resource_owners(resource_type, resource_id, consistency)
+        resource_id_str = str(resource_id)
+        existing_owners_rels = await self._get_resource_owners(resource_type, resource_id_str, consistency)
         n_existing_owners = len(existing_owners_rels)
         for member in members:
             rel = Relationship(
@@ -833,7 +841,7 @@ async def upsert_project_members(
             )
             existing_rel_filter = RelationshipFilter(
                 resource_type=resource_type.value,
-                optional_resource_id=resource_id,
+                optional_resource_id=resource_id_str,
                 optional_subject_filter=SubjectFilter(
                     subject_type=ResourceType.user, optional_subject_id=member.user_id
                 ),
@@ -926,12 +934,13 @@ async def remove_project_members(
         self,
         user: base_models.APIUser,
         resource_type: ResourceType,
-        resource_id: str,
+        resource_id: ULID,
         user_ids: list[str],
         *,
         zed_token: ZedToken | None = None,
     ) -> list[MembershipChange]:
         """Remove the specific members from the project, then return the list of members that were removed."""
+        resource_id: str = str(resource_id)
         consistency = Consistency(at_least_as_fresh=zed_token) if zed_token else Consistency(fully_consistent=True)
         add_members: list[RelationshipUpdate] = []
         remove_members: list[RelationshipUpdate] = []

components/renku_data_services/base_api/auth.py

@@ -44,30 +44,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], Awaitable[_T]],
-) -> Callable[Concatenate[Request, _P], Awaitable[_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 requried "
-                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], Awaitable[_T]],
 ) -> Callable[Concatenate[Request, _P], Awaitable[_T]]:

components/renku_data_services/message_queue/converters.py

@@ -41,7 +41,7 @@ def to_events(
                     Event(
                         "project.created",
                         v2.ProjectCreated(
-                            id=project.id,
+                            id=str(project.id),
                             name=project.name,
                             namespace=project.namespace.slug,
                             slug=project.slug,
@@ -56,7 +56,7 @@ def to_events(
                     Event(
                         "projectAuth.added",
                         v2.ProjectMemberAdded(
-                            projectId=project.id,
+                            projectId=str(project.id),
                             userId=project.created_by,
                             role=v2.MemberRole.OWNER,
                         ),
@@ -67,7 +67,7 @@ def to_events(
                     Event(
                         "project.updated",
                         v2.ProjectUpdated(
-                            id=project.id,
+                            id=str(project.id),
                             name=project.name,
                             namespace=project.namespace.slug,
                             slug=project.slug,
@@ -79,7 +79,7 @@ def to_events(
                     )
                 ]
             case v2.ProjectRemoved:
-                return [Event("project.removed", v2.ProjectRemoved(id=project.id))]
+                return [Event("project.removed", v2.ProjectRemoved(id=str(project.id)))]
             case _:
                 raise errors.EventError(message=f"Trying to convert a project to an unknown event type {event_type}")
 

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)