fix(permission-db): update following code review

antarest/launcher/service.py

@@ -40,7 +40,7 @@
 from antarest.launcher.repository import JobResultRepository
 from antarest.launcher.ssh_client import calculates_slurm_load
 from antarest.launcher.ssh_config import SSHConfigDTO
-from antarest.study.repository import StudyFilter
+from antarest.study.repository import QueryUser, StudyFilter, build_query_user_from_params
 from antarest.study.service import StudyService
 from antarest.study.storage.utils import assert_permission, extract_output_name, find_single_output_path
 
@@ -312,7 +312,9 @@ def _filter_from_user_permission(self, job_results: List[JobResult], user: Optio
         if study_ids:
             studies = {
                 study.id: study
-                for study in self.study_service.repository.get_all(study_filter=StudyFilter(study_ids=study_ids))
+                for study in self.study_service.repository.get_all(
+                    study_filter=StudyFilter(study_ids=study_ids, query_user=build_query_user_from_params(user))
+                )
             }
         else:
             studies = {}

antarest/study/repository.py

@@ -7,7 +7,9 @@
 from sqlalchemy.orm import Session, joinedload, with_polymorphic  # type: ignore
 
 from antarest.core.interfaces.cache import ICache
+from antarest.core.jwt import JWTUser
 from antarest.core.model import PublicMode
+from antarest.core.requests import RequestParameters
 from antarest.core.utils.fastapi_sqlalchemy import db
 from antarest.login.model import Group
 from antarest.study.model import DEFAULT_WORKSPACE_NAME, RawStudy, Study, StudyAdditionalData, Tag
@@ -35,6 +37,42 @@ def escape_like(string: str, escape_char: str = "\\") -> str:
     return string.replace(escape_char, escape_char * 2).replace("%", escape_char + "%").replace("_", escape_char + "_")
 
 
+class QueryUser(BaseModel, frozen=True, extra="forbid"):
+    """
+    This class object is build to pass on the user identity and its associated groups information
+    into the listing function get_all below
+    """
+
+    is_admin: bool = False
+    user_id: t.Optional[int] = None
+    user_groups: t.Sequence[str] = ()
+
+
+def build_query_user_from_params(params: t.Union[RequestParameters, JWTUser]) -> QueryUser:
+    """
+    This function makes it easier to pass on user ids and groups into the repository filtering function by
+    extracting the associated `QueryUser` object.
+    Args:
+        params: `RequestParameters` or `JWTUser` holding user ids and groups
+
+    Returns: `QueryUser`
+
+    """
+    if isinstance(params, RequestParameters):
+        user = params.user
+    else:
+        user = params
+
+    if user:
+        return QueryUser(
+            is_admin=user.is_site_admin() or user.is_admin_token(),
+            user_id=user.id,
+            user_groups=user.groups,
+        )
+    else:
+        return QueryUser()
+
+
 class StudyFilter(BaseModel, frozen=True, extra="forbid"):
     """Study filter class gathering the main filtering parameters
 
@@ -51,6 +89,7 @@ class StudyFilter(BaseModel, frozen=True, extra="forbid"):
         exists: if raw study missing
         workspace: optional workspace of the study
         folder: optional folder prefix of the study
+        query_user: query user id, groups and admins status
     """
 
     name: str = ""
@@ -65,17 +104,7 @@ class StudyFilter(BaseModel, frozen=True, extra="forbid"):
     exists: t.Optional[bool] = None
     workspace: str = ""
     folder: str = ""
-
-
-class QueryUser(BaseModel, frozen=True, extra="forbid"):
-    """
-    This class object is build to pass on the user identity and its associated groups information
-    into the listing function get_all below
-    """
-
-    is_admin: bool = False
-    user_id: t.Optional[int] = None
-    user_groups: t.Optional[t.Sequence[str]] = None
+    query_user: QueryUser = QueryUser()
 
 
 class StudySortBy(str, enum.Enum):
@@ -194,7 +223,6 @@ def get_all(
         study_filter: StudyFilter = StudyFilter(),
         sort_by: t.Optional[StudySortBy] = None,
         pagination: StudyPagination = StudyPagination(),
-        query_user: QueryUser = QueryUser(),
     ) -> t.Sequence[Study]:
         """
         Retrieve studies based on specified filters, sorting, and pagination.
@@ -203,7 +231,6 @@ def get_all(
             study_filter: composed of all filtering criteria.
             sort_by: how the user would like the results to be sorted.
             pagination: specifies the number of results to displayed in each page and the actually displayed page.
-            query_user: user id and groups info
 
         Returns:
             The matching studies in proper order and pagination.
@@ -258,11 +285,11 @@ def get_all(
             q = q.filter(entity.version.in_(study_filter.versions))
 
         # permissions filtering
-        if not query_user.is_admin:
-            if query_user.user_id is not None:
+        if not study_filter.query_user.is_admin:
+            if study_filter.query_user.user_id is not None:
                 condition_1 = entity.public_mode != PublicMode.NONE
-                condition_2 = entity.owner_id == query_user.user_id
-                condition_3 = Group.id.in_(query_user.user_groups or [])
+                condition_2 = entity.owner_id == study_filter.query_user.user_id
+                condition_3 = Group.id.in_(study_filter.query_user.user_groups or [])
                 if study_filter.groups:
                     q0 = q.filter(condition_3)
                     q = q0.union(q.filter(or_(condition_1, condition_2)))

antarest/study/service.py

@@ -95,7 +95,14 @@
     StudyMetadataPatchDTO,
     StudySimResultDTO,
 )
-from antarest.study.repository import QueryUser, StudyFilter, StudyMetadataRepository, StudyPagination, StudySortBy
+from antarest.study.repository import (
+    QueryUser,
+    StudyFilter,
+    StudyMetadataRepository,
+    StudyPagination,
+    StudySortBy,
+    build_query_user_from_params,
+)
 from antarest.study.storage.matrix_profile import adjust_matrix_columns_index
 from antarest.study.storage.rawstudy.model.filesystem.config.model import FileStudyTreeConfigDTO
 from antarest.study.storage.rawstudy.model.filesystem.folder_node import ChildNotFoundError
@@ -461,29 +468,10 @@ def get_studies_information(
         """
         logger.info("Retrieving matching studies")
         studies: t.Dict[str, StudyMetadataDTO] = {}
-
-        # retrieve user id and groups
-        user_id = None
-        user_groups = None
-        if params.user:
-            if params.user.id:
-                user_id = params.user.id
-            if params.user.groups:
-                user_groups = [group.id for group in params.user.groups]
-        else:
-            logger.error("FAIL permission: user is not logged")
-            raise UserHasNotPermissionError()
-
-        query_user = QueryUser(
-            is_admin=params.user.is_site_admin() or params.user.is_admin_token(),
-            user_id=user_id,
-            user_groups=user_groups,
-        )
         matching_studies = self.repository.get_all(
             study_filter=study_filter,
             sort_by=sort_by,
             pagination=pagination,
-            query_user=query_user,
         )
         logger.info("Studies retrieved")
         for study in matching_studies:
@@ -720,8 +708,15 @@ def get_input_matrix_startdate(
         return get_start_date(file_study, output_id, level)
 
     def remove_duplicates(self) -> None:
+        """
+        This function removes duplicates of non archived raw studies ith same path on drive from the db. \n
+        **Warnings: This function provides all admin `READ` rights over the studies.**
+
+        Returns:
+
+        """
         study_paths: t.Dict[str, t.List[str]] = {}
-        for study in self.repository.get_all(query_user=QueryUser(is_admin=True)):
+        for study in self.repository.get_all(study_filter=StudyFilter(query_user=QueryUser(is_admin=True))):
             if isinstance(study, RawStudy) and not study.archived:
                 path = str(study.path)
                 if path not in study_paths:
@@ -731,9 +726,9 @@ def remove_duplicates(self) -> None:
         for studies_with_same_path in study_paths.values():
             if len(studies_with_same_path) > 1:
                 logger.info(f"Found studies {studies_with_same_path} with same path, de duplicating")
-                for study_name in studies_with_same_path[1:]:
-                    logger.info(f"Removing study {study_name}")
-                    self.repository.delete(study_name)
+                for study_id in studies_with_same_path[1:]:
+                    logger.info(f"Removing study {study_id}")
+                    self.repository.delete(study_id)
 
     def sync_studies_on_disk(self, folders: t.List[StudyFolder], directory: t.Optional[Path] = None) -> None:
         """
@@ -2161,10 +2156,23 @@ def update_matrix(
             raise BadEditInstructionException(str(exc)) from exc
 
     def check_and_update_all_study_versions_in_database(self, params: RequestParameters) -> None:
+        """
+        This function updates studies version on the db. \n
+        **Warnings: Only users with Admins rights should be able to run this function.**
+        Args:
+            params: `RequestParameters` holding user ids and groups
+
+        Returns:
+
+        Raises: `UserHasNotPermissionError` if params user is not admin.
+
+        """
         if params.user and not params.user.is_site_admin():
             logger.error(f"User {params.user.id} is not site admin")
             raise UserHasNotPermissionError()
-        studies = self.repository.get_all(study_filter=StudyFilter(managed=False), query_user=QueryUser(is_amin=True))
+        studies = self.repository.get_all(
+            study_filter=StudyFilter(managed=False, query_user=build_query_user_from_params(params))
+        )
 
         for study in studies:
             storage = self.storage_service.raw_study_service

antarest/study/storage/auto_archive_service.py

@@ -28,8 +28,9 @@ def __init__(self, study_service: StudyService, config: Config):
     def _try_archive_studies(self) -> None:
         old_date = datetime.datetime.utcnow() - datetime.timedelta(days=self.config.storage.auto_archive_threshold_days)
         with db():
+            # in this part full `Read` rights over studies are granted to this function
             studies: t.Sequence[Study] = self.study_service.repository.get_all(
-                study_filter=StudyFilter(managed=True), query_user=QueryUser(is_admin=True)
+                study_filter=StudyFilter(managed=True, query_user=QueryUser(is_admin=True))
             )
             # list of study IDs and boolean indicating if it's a raw study (True) or a variant (False)
             study_ids_to_archive = [

antarest/study/storage/rawstudy/watcher.py

@@ -94,6 +94,7 @@ def _loop(self) -> None:
                 "Removing duplicates, this is a temporary fix that should be removed when previous duplicates are removed"
             )
             with db():
+                # in this part full `Read` rights over studies are granted to this function
                 self.study_service.remove_duplicates()
         except Exception as e:
             logger.error("Unexpected error when removing duplicates", exc_info=e)

antarest/study/web/studies_blueprint.py

@@ -15,7 +15,7 @@
 from antarest.core.filetransfer.service import FileTransferManager
 from antarest.core.jwt import JWTUser
 from antarest.core.model import PublicMode
-from antarest.core.requests import RequestParameters
+from antarest.core.requests import RequestParameters, UserHasNotPermissionError
 from antarest.core.utils.utils import BadArchiveContent, sanitize_uuid
 from antarest.core.utils.web import APITag
 from antarest.login.auth import Auth
@@ -28,7 +28,7 @@
     StudyMetadataPatchDTO,
     StudySimResultDTO,
 )
-from antarest.study.repository import StudyFilter, StudyPagination, StudySortBy
+from antarest.study.repository import StudyFilter, StudyPagination, StudySortBy, build_query_user_from_params
 from antarest.study.service import StudyService
 from antarest.study.storage.rawstudy.model.filesystem.config.model import FileStudyTreeConfigDTO
 
@@ -144,6 +144,10 @@ def get_studies(
 
         user_list = [int(v) for v in _split_comma_separated_values(users)]
 
+        if not params.user:
+            logger.error("FAIL permission: user is not logged")
+            raise UserHasNotPermissionError()
+
         study_filter = StudyFilter(
             name=name,
             managed=managed,
@@ -157,6 +161,7 @@ def get_studies(
             exists=exists,
             workspace=workspace,
             folder=folder,
+            query_user=build_query_user_from_params(params),
         )
 
         matching_studies = study_service.get_studies_information(