diff --git a/packages/postgres-database/src/simcore_postgres_database/utils_folders.py b/packages/postgres-database/src/simcore_postgres_database/utils_folders.py index 4437a2e9353..e0f59cdcfd2 100644 --- a/packages/postgres-database/src/simcore_postgres_database/utils_folders.py +++ b/packages/postgres-database/src/simcore_postgres_database/utils_folders.py @@ -10,20 +10,22 @@ import sqlalchemy as sa from aiopg.sa.connection import SAConnection from aiopg.sa.result import RowProxy -from psycopg2.errors import ForeignKeyViolation from pydantic import ( BaseModel, ConstrainedStr, Field, NonNegativeInt, PositiveInt, + ValidationError, parse_obj_as, ) from pydantic.errors import PydanticErrorMixin from simcore_postgres_database.utils_ordering import OrderByDict +from sqlalchemy import Column, func from sqlalchemy.dialects import postgresql -from sqlalchemy.sql.elements import ColumnElement -from sqlalchemy.sql.selectable import ScalarSelect +from sqlalchemy.dialects.postgresql import BOOLEAN, INTEGER +from sqlalchemy.sql.elements import ColumnElement, Label +from sqlalchemy.sql.selectable import CTE from .models.folders import folders, folders_access_rights, folders_to_projects from .models.groups import GroupType, groups @@ -47,11 +49,13 @@ * FolderNotFoundError * FolderNotSharedWithGidError * InsufficientPermissionsError + * NoAccessForGroupsFoundError * BaseCreateFolderError * FolderAlreadyExistsError * ParentFolderIsNotWritableError * CouldNotCreateFolderError * GroupIdDoesNotExistError + * RootFolderRequiresAtLeastOnePrimaryGroupError * BaseMoveFolderError * CannotMoveFolderSharedViaNonPrimaryGroupError * BaseAddProjectError @@ -72,15 +76,19 @@ class FolderAccessError(FoldersError): class FolderNotFoundError(FolderAccessError): - msg_template = "no entry found for folder_id={folder_id}, gid={gid} and product_name={product_name}" + msg_template = "no entry found for folder_id={folder_id}, gids={gids} and product_name={product_name}" class FolderNotSharedWithGidError(FolderAccessError): - msg_template = "folder_id={folder_id} was not shared with gid={gid}" + msg_template = "folder_id={folder_id} was not shared with gids={gids}" class InsufficientPermissionsError(FolderAccessError): - msg_template = "could not find a parent for folder_id={folder_id} and gid={gid}, with permissions={permissions}" + msg_template = "could not find a parent for folder_id={folder_id} and gids={gids}, with permissions={permissions}" + + +class NoAccessForGroupsFoundError(FolderAccessError): + msg_template = "No parent found for folder_id={folder_id} and gids={gids}, with permissions={permissions}" class BaseCreateFolderError(FoldersError): @@ -88,7 +96,7 @@ class BaseCreateFolderError(FoldersError): class FolderAlreadyExistsError(BaseCreateFolderError): - msg_template = "A folder='{folder}' with parent='{parent}' for group='{gid}' in product_name={product_name} already exists" + msg_template = "A folder='{folder}' with parent='{parent}' in product_name={product_name} already exists" class ParentFolderIsNotWritableError(BaseCreateFolderError): @@ -99,8 +107,15 @@ class CouldNotCreateFolderError(BaseCreateFolderError): msg_template = "Could not create folder='{folder}' and parent='{parent}'" -class GroupIdDoesNotExistError(BaseCreateFolderError): - msg_template = "Provided group id '{gid}' does not exist " +class NoGroupIDFoundError(BaseCreateFolderError): + msg_template = "None of the provided gids='{gids}' was found" + + +class RootFolderRequiresAtLeastOnePrimaryGroupError(BaseCreateFolderError): + msg_template = ( + "No parent={parent} defined and groupIDs={gids} did not contain a PRIMARY group. " + "Cannot create a folder isnide the 'root' wihtout using the user's group." + ) class BaseMoveFolderError(FoldersError): @@ -259,8 +274,8 @@ def _requires(*permissions: _FolderPermissions) -> _FolderPermissions: return _or_dicts_list(permissions) -def _get_and_calsue_with_only_true_entries( - permissions: _FolderPermissions, table +def _get_filter_for_enabled_permissions( + permissions: _FolderPermissions, table: sa.Table | CTE ) -> ColumnElement | bool: clauses: list[ColumnElement] = [] @@ -299,12 +314,6 @@ class FolderEntry(BaseModel): my_access_rights: _FolderPermissions access_rights: dict[_GroupID, _FolderPermissions] - access_via_gid: _GroupID = Field( - ..., - description="used to compute my_access_rights, should be used by the fronted", - ) - gid: _GroupID = Field(..., description="actual gid of this entry") - class Config: orm_mode = True @@ -329,30 +338,11 @@ async def _get_resolved_access_rights( gid: _GroupID, *, permissions: _FolderPermissions | None, - enforece_all_permissions: bool, ) -> _ResolvedAccessRights | None: # Define the anchor CTE access_rights_cte = ( sa.select( - [ - folders_access_rights.c.folder_id, - folders_access_rights.c.gid, - folders_access_rights.c.traversal_parent_id, - folders_access_rights.c.original_parent_id, - folders_access_rights.c.read, - folders_access_rights.c.write, - folders_access_rights.c.delete, - sa.literal_column("0").label("level"), - ] - ) - .where(folders_access_rights.c.folder_id == sa.bindparam("start_folder_id")) - .cte(name="access_rights_cte", recursive=True) - ) - - # Define the recursive part of the CTE - recursive = sa.select( - [ folders_access_rights.c.folder_id, folders_access_rights.c.gid, folders_access_rights.c.traversal_parent_id, @@ -360,8 +350,22 @@ async def _get_resolved_access_rights( folders_access_rights.c.read, folders_access_rights.c.write, folders_access_rights.c.delete, - sa.literal_column("access_rights_cte.level + 1").label("level"), - ] + sa.literal_column("0").label("level"), + ) + .where(folders_access_rights.c.folder_id == sa.bindparam("start_folder_id")) + .cte(name="access_rights_cte", recursive=True) + ) + + # Define the recursive part of the CTE + recursive = sa.select( + folders_access_rights.c.folder_id, + folders_access_rights.c.gid, + folders_access_rights.c.traversal_parent_id, + folders_access_rights.c.original_parent_id, + folders_access_rights.c.read, + folders_access_rights.c.write, + folders_access_rights.c.delete, + sa.literal_column("access_rights_cte.level + 1").label("level"), ).select_from( folders_access_rights.join( access_rights_cte, @@ -370,36 +374,25 @@ async def _get_resolved_access_rights( ) # Combine anchor and recursive CTE - folder_hierarchy = access_rights_cte.union_all(recursive) - - def _get_permissions_where_clause() -> ColumnElement | bool: - if not permissions: - return True - return ( - sa.and_( - folder_hierarchy.c.read.is_(permissions.read), - folder_hierarchy.c.write.is_(permissions.write), - folder_hierarchy.c.delete.is_(permissions.delete), - ) - if enforece_all_permissions - else _get_and_calsue_with_only_true_entries(permissions, folder_hierarchy) - ) + folder_hierarchy: CTE = access_rights_cte.union_all(recursive) # Final query to filter and order results query = ( sa.select( - [ - folder_hierarchy.c.folder_id, - folder_hierarchy.c.gid, - folder_hierarchy.c.traversal_parent_id, - folder_hierarchy.c.original_parent_id, - folder_hierarchy.c.read, - folder_hierarchy.c.write, - folder_hierarchy.c.delete, - folder_hierarchy.c.level, - ] + folder_hierarchy.c.folder_id, + folder_hierarchy.c.gid, + folder_hierarchy.c.traversal_parent_id, + folder_hierarchy.c.original_parent_id, + folder_hierarchy.c.read, + folder_hierarchy.c.write, + folder_hierarchy.c.delete, + folder_hierarchy.c.level, + ) + .where( + True + if not permissions + else _get_filter_for_enabled_permissions(permissions, folder_hierarchy) ) - .where(_get_permissions_where_clause()) .where(folder_hierarchy.c.original_parent_id.is_(None)) .where(folder_hierarchy.c.gid == gid) .order_by(folder_hierarchy.c.level.asc()) @@ -414,14 +407,14 @@ def _get_permissions_where_clause() -> ColumnElement | bool: ) -async def _check_folder_and_access( +async def _check_and_get_folder_access_by_group( connection: SAConnection, product_name: _ProductName, folder_id: _FolderID, gid: _GroupID, *, + error_reporting_gids: set[_GroupID], permissions: _FolderPermissions, - enforece_all_permissions: bool, ) -> _ResolvedAccessRights: """ Raises: @@ -430,13 +423,13 @@ async def _check_folder_and_access( InsufficientPermissionsError """ folder_entry: int | None = await connection.scalar( - sa.select([folders.c.id]) + sa.select(folders.c.id) .where(folders.c.id == folder_id) .where(folders.c.product_name == product_name) ) if not folder_entry: raise FolderNotFoundError( - folder_id=folder_id, gid=gid, product_name=product_name + folder_id=folder_id, gids=error_reporting_gids, product_name=product_name ) # check if folder was shared @@ -445,10 +438,11 @@ async def _check_folder_and_access( folder_id, gid, permissions=None, - enforece_all_permissions=False, ) if not resolved_access_rights_without_permissions: - raise FolderNotSharedWithGidError(folder_id=folder_id, gid=gid) + raise FolderNotSharedWithGidError( + folder_id=folder_id, gids=error_reporting_gids + ) # check if there are permissions resolved_access_rights = await _get_resolved_access_rights( @@ -456,18 +450,57 @@ async def _check_folder_and_access( folder_id, gid, permissions=permissions, - enforece_all_permissions=enforece_all_permissions, ) if resolved_access_rights is None: raise InsufficientPermissionsError( folder_id=folder_id, - gid=gid, + gids=error_reporting_gids, permissions=_only_true_permissions(permissions), ) return resolved_access_rights +async def _check_and_get_folder_access( + connection: SAConnection, + product_name: _ProductName, + folder_id: _FolderID, + gids: set[_GroupID], + *, + permissions: _FolderPermissions, +) -> _ResolvedAccessRights: + """ + Raises: + FolderNotFoundError + FolderNotSharedWithGidError + InsufficientPermissionsError + NoAccessForGroupsFoundError + """ + folder_access_error = None + + for gid in gids: + try: + return await _check_and_get_folder_access_by_group( + connection, + product_name, + folder_id, + gid, + error_reporting_gids=gids, + permissions=permissions, + ) + except FolderAccessError as e: # noqa: PERF203 + folder_access_error = e + + if folder_access_error: + raise folder_access_error + + raise NoAccessForGroupsFoundError( + folder_id=folder_id, + gids=gids, + permissions=_only_true_permissions(permissions), + ) + + ### ### API DB LAYER ### @@ -477,8 +510,7 @@ async def folder_create( connection: SAConnection, product_name: _ProductName, name: str, - gid: _GroupID, - *, + gids: set[_GroupID], description: str = "", parent: _FolderID | None = None, _required_permissions: _FolderPermissions = _requires( # noqa: B008 @@ -490,15 +522,20 @@ async def folder_create( FolderNotFoundError FolderNotSharedWithGidError InsufficientPermissionsError + NoAccessForGroupsFoundError FolderAlreadyExistsError CouldNotCreateFolderError GroupIdDoesNotExistError + RootFolderRequiresAtLeastOnePrimaryGroupError """ - parse_obj_as(FolderName, name) + try: + parse_obj_as(FolderName, name) + except ValidationError as exc: + raise InvalidFolderNameError(name=name, reason=f"{exc}") from exc async with connection.begin(): entry_exists: int | None = await connection.scalar( - sa.select([folders.c.id]) + sa.select(folders.c.id) .select_from( folders.join( folders_access_rights, @@ -511,47 +548,68 @@ async def folder_create( ) if entry_exists: raise FolderAlreadyExistsError( - product_name=product_name, folder=name, parent=parent, gid=gid + product_name=product_name, folder=name, parent=parent ) + # `permissions_gid` is computed as follows: + # - `folder has a parent?` taken from the resolved access rights of the parent folder + # - `is root folder, a.k.a. no parent?` taken from the user's primary group + permissions_gid = None if parent: - # check if parent has permissions - await _check_folder_and_access( + resolved_access_rights = await _check_and_get_folder_access( connection, product_name, folder_id=parent, - gid=gid, + gids=gids, permissions=_required_permissions, - enforece_all_permissions=False, ) + permissions_gid = resolved_access_rights.gid - # folder entry can now be inserted - try: - folder_id = await connection.scalar( - sa.insert(folders) - .values( - name=name, - description=description, - created_by=gid, - product_name=product_name, + if permissions_gid is None: + groups_results: list[RowProxy] | None = await ( + await connection.execute( + sa.select(groups.c.gid, groups.c.type).where(groups.c.gid.in_(gids)) ) - .returning(folders.c.id) + ).fetchall() + + if not groups_results: + raise NoGroupIDFoundError(gids=gids) + + primary_gid = None + for group in groups_results: + if group["type"] == GroupType.PRIMARY: + primary_gid = group["gid"] + if primary_gid is None: + raise RootFolderRequiresAtLeastOnePrimaryGroupError( + parent=parent, gids=gids + ) + + permissions_gid = primary_gid + + # folder entry can now be inserted + folder_id = await connection.scalar( + sa.insert(folders) + .values( + name=name, + description=description, + created_by=permissions_gid, + product_name=product_name, ) + .returning(folders.c.id) + ) - if not folder_id: - raise CouldNotCreateFolderError(folder=name, parent=parent) + if not folder_id: + raise CouldNotCreateFolderError(folder=name, parent=parent) - await connection.execute( - sa.insert(folders_access_rights).values( - folder_id=folder_id, - gid=gid, - traversal_parent_id=parent, - original_parent_id=parent, - **OWNER_PERMISSIONS.to_dict(), - ) + await connection.execute( + sa.insert(folders_access_rights).values( + folder_id=folder_id, + gid=permissions_gid, + traversal_parent_id=parent, + original_parent_id=parent, + **OWNER_PERMISSIONS.to_dict(), ) - except ForeignKeyViolation as e: - raise GroupIdDoesNotExistError(gid=gid) from e + ) return _FolderID(folder_id) @@ -560,7 +618,7 @@ async def folder_share_or_update_permissions( connection: SAConnection, product_name: _ProductName, folder_id: _FolderID, - sharing_gid: _GroupID, + sharing_gids: set[_GroupID], *, recipient_gid: _GroupID, recipient_role: FolderAccessRole, @@ -573,16 +631,16 @@ async def folder_share_or_update_permissions( FolderNotFoundError FolderNotSharedWithGidError InsufficientPermissionsError + NoAccessForGroupsFoundError """ # NOTE: if the `sharing_gid`` has permissions to share it can share it with any `FolderAccessRole` async with connection.begin(): - await _check_folder_and_access( + await _check_and_get_folder_access( connection, product_name, folder_id=folder_id, - gid=sharing_gid, + gids=sharing_gids, permissions=required_permissions, - enforece_all_permissions=False, ) # update or create permissions entry @@ -611,7 +669,7 @@ async def folder_update( connection: SAConnection, product_name: _ProductName, folder_id: _FolderID, - gid: _GroupID, + gids: set[_GroupID], *, name: str | None = None, description: str | None = None, @@ -624,15 +682,15 @@ async def folder_update( FolderNotFoundError FolderNotSharedWithGidError InsufficientPermissionsError + NoAccessForGroupsFoundError """ async with connection.begin(): - await _check_folder_and_access( + await _check_and_get_folder_access( connection, product_name, folder_id=folder_id, - gid=gid, + gids=gids, permissions=_required_permissions, - enforece_all_permissions=False, ) # do not update if nothing changed @@ -655,7 +713,7 @@ async def folder_delete( connection: SAConnection, product_name: _ProductName, folder_id: _FolderID, - gid: _GroupID, + gids: set[_GroupID], *, _required_permissions: _FolderPermissions = _requires( # noqa: B008 _BasePermissions.DELETE_FOLDER @@ -666,17 +724,17 @@ async def folder_delete( FolderNotFoundError FolderNotSharedWithGidError InsufficientPermissionsError + NoAccessForGroupsFoundError """ childern_folder_ids: list[_FolderID] = [] async with connection.begin(): - await _check_folder_and_access( + await _check_and_get_folder_access( connection, product_name, folder_id=folder_id, - gid=gid, + gids=gids, permissions=_required_permissions, - enforece_all_permissions=False, ) # list all children then delete @@ -692,7 +750,7 @@ async def folder_delete( # first remove all childeren for child_folder_id in childern_folder_ids: - await folder_delete(connection, product_name, child_folder_id, gid) + await folder_delete(connection, product_name, child_folder_id, gids) # as a last step remove the folder per se async with connection.begin(): @@ -703,7 +761,7 @@ async def folder_move( connection: SAConnection, product_name: _ProductName, source_folder_id: _FolderID, - gid: _GroupID, + gids: set[_GroupID], *, destination_folder_id: _FolderID | None, required_permissions_source: _FolderPermissions = _requires( # noqa: B008 @@ -718,34 +776,34 @@ async def folder_move( FolderNotFoundError FolderNotSharedWithGidError InsufficientPermissionsError + NoAccessForGroupsFoundError CannotMoveFolderSharedViaNonPrimaryGroupError: """ async with connection.begin(): - source_access_entry = await _check_folder_and_access( + source_access_entry = await _check_and_get_folder_access( connection, product_name, folder_id=source_folder_id, - gid=gid, + gids=gids, permissions=required_permissions_source, - enforece_all_permissions=False, ) source_access_gid = source_access_entry.gid group_type: GroupType | None = await connection.scalar( - sa.select([groups.c.type]).where(groups.c.gid == source_access_gid) + sa.select(groups.c.type).where(groups.c.gid == source_access_gid) ) + # Might drop primary check if group_type is None or group_type != GroupType.PRIMARY: raise CannotMoveFolderSharedViaNonPrimaryGroupError( group_type=group_type, gid=source_access_gid ) if destination_folder_id: - await _check_folder_and_access( + await _check_and_get_folder_access( connection, product_name, folder_id=destination_folder_id, - gid=gid, + gids=gids, permissions=required_permissions_destination, - enforece_all_permissions=False, ) # set new traversa_parent_id on the source_folder_id which is equal to destination_folder_id @@ -754,7 +812,7 @@ async def folder_move( .where( sa.and_( folders_access_rights.c.folder_id == source_folder_id, - folders_access_rights.c.gid == gid, + folders_access_rights.c.gid.in_(gids), ) ) .values(traversal_parent_id=destination_folder_id) @@ -765,7 +823,7 @@ async def folder_add_project( connection: SAConnection, product_name: _ProductName, folder_id: _FolderID, - gid: _GroupID, + gids: set[_GroupID], *, project_uuid: _ProjectID, required_permissions=_requires( # noqa: B008 @@ -777,16 +835,16 @@ async def folder_add_project( FolderNotFoundError FolderNotSharedWithGidError InsufficientPermissionsError + NoAccessForGroupsFoundError ProjectAlreadyExistsInFolderError """ async with connection.begin(): - await _check_folder_and_access( + await _check_and_get_folder_access( connection, product_name, folder_id=folder_id, - gid=gid, + gids=gids, permissions=required_permissions, - enforece_all_permissions=False, ) # check if already added in folder @@ -814,7 +872,7 @@ async def folder_move_project( connection: SAConnection, product_name: _ProductName, source_folder_id: _FolderID, - gid: _GroupID, + gids: set[_GroupID], *, project_uuid: _ProjectID, destination_folder_id: _FolderID | None, @@ -833,13 +891,12 @@ async def folder_move_project( CannotMoveFolderSharedViaNonPrimaryGroupError: """ async with connection.begin(): - await _check_folder_and_access( + await _check_and_get_folder_access( connection, product_name, folder_id=source_folder_id, - gid=gid, + gids=gids, permissions=_required_permissions_source, - enforece_all_permissions=False, ) if destination_folder_id is None: @@ -848,19 +905,18 @@ async def folder_move_project( connection, product_name, folder_id=source_folder_id, - gid=gid, + gids=gids, project_uuid=project_uuid, ) return async with connection.begin(): - await _check_folder_and_access( + await _check_and_get_folder_access( connection, product_name, folder_id=destination_folder_id, - gid=gid, + gids=gids, permissions=_required_permissions_destination, - enforece_all_permissions=False, ) await connection.execute( @@ -905,7 +961,7 @@ async def folder_remove_project( connection: SAConnection, product_name: _ProductName, folder_id: _FolderID, - gid: _GroupID, + gids: set[_GroupID], *, project_uuid: _ProjectID, required_permissions=_requires( # noqa: B008 @@ -917,15 +973,15 @@ async def folder_remove_project( FolderNotFoundError FolderNotSharedWithGidError InsufficientPermissionsError + NoAccessForGroupsFoundError """ async with connection.begin(): - await _check_folder_and_access( + await _check_and_get_folder_access( connection, product_name, folder_id=folder_id, - gid=gid, + gids=gids, permissions=required_permissions, - enforece_all_permissions=False, ) await connection.execute( @@ -935,48 +991,47 @@ async def folder_remove_project( ) -def _get_subquery_my_access_rights( - access_via_gid: _GroupID, access_via_folder_id: _FolderID | None -) -> ScalarSelect: - return ( +_LIST_GROUP_BY_FIELDS: Final[tuple[Column, ...]] = ( + folders.c.id, + folders.c.name, + folders.c.description, + folders.c.created_by, + folders_access_rights.c.traversal_parent_id, +) +_LIST_SELECT_FIELDS: Final[tuple[Label | Column, ...]] = ( + *_LIST_GROUP_BY_FIELDS, + # access_rights + ( sa.select( - sa.func.jsonb_build_object( - "read", - folders_access_rights.c.read, - "write", - folders_access_rights.c.write, - "delete", - folders_access_rights.c.delete, - ).label("my_access_rights"), - ) - .where( - folders_access_rights.c.folder_id == access_via_folder_id - if access_via_gid and access_via_folder_id - else folders_access_rights.c.folder_id == folders.c.id + sa.func.jsonb_object_agg( + folders_access_rights.c.gid, + sa.func.jsonb_build_object( + "read", + folders_access_rights.c.read, + "write", + folders_access_rights.c.write, + "delete", + folders_access_rights.c.delete, + ), + ).label("access_rights"), ) - .where(folders_access_rights.c.gid == access_via_gid) + .where(folders_access_rights.c.folder_id == folders.c.id) .correlate(folders) .scalar_subquery() - ) - - -_SUBQUERY_ACCESS_RIGHTS: Final[ScalarSelect] = ( - sa.select( - sa.func.jsonb_object_agg( - folders_access_rights.c.gid, - sa.func.jsonb_build_object( - "read", - folders_access_rights.c.read, - "write", - folders_access_rights.c.write, - "delete", - folders_access_rights.c.delete, - ), - ).label("access_rights"), - ) - .where(folders_access_rights.c.folder_id == folders.c.id) - .correlate(folders) - .scalar_subquery() + ).label("access_rights"), + # my_access_rights + func.json_build_object( + "read", + func.max(folders_access_rights.c.read.cast(INTEGER)).cast(BOOLEAN), + "write", + func.max(folders_access_rights.c.write.cast(INTEGER)).cast(BOOLEAN), + "delete", + func.max(folders_access_rights.c.delete.cast(INTEGER)).cast(BOOLEAN), + ).label("my_access_rights"), + # access_created + func.max(folders_access_rights.c.created).label("access_created"), + # access_modified + func.max(folders_access_rights.c.modified).label("access_modified"), ) @@ -984,96 +1039,70 @@ async def folder_list( connection: SAConnection, product_name: _ProductName, folder_id: _FolderID | None, - gid: _GroupID, + gids: set[_GroupID], *, offset: NonNegativeInt, limit: NonNegativeInt, - order_by: OrderByDict = OrderByDict( + order_by: OrderByDict = OrderByDict( # noqa: B008 field="modified", direction=OrderDirection.DESC ), - _required_permissions=_requires(_BasePermissions.LIST_FOLDERS), # noqa: B008 + required_permissions: _FolderPermissions = _requires( # noqa: B008 + _BasePermissions.LIST_FOLDERS + ), ) -> tuple[int, list[FolderEntry]]: """ Raises: FolderNotFoundError FolderNotSharedWithGidError InsufficientPermissionsError + NoAccessForGroupsFoundError """ - # NOTE: when `folder_id is None` list the root folder of `gid` + # NOTE: when `folder_id is None` list the root folder of the `gids` - results: list[FolderEntry] = [] + if folder_id is not None: + await _check_and_get_folder_access( + connection, + product_name, + folder_id=folder_id, + gids=gids, + permissions=required_permissions, + ) - async with connection.begin(): - access_via_gid: _GroupID = gid - access_via_folder_id: _FolderID | None = None + results: list[FolderEntry] = [] - if folder_id: - # this one provides the set of access rights - resolved_access_rights = await _check_folder_and_access( - connection, - product_name, - folder_id=folder_id, - gid=gid, - permissions=_required_permissions, - enforece_all_permissions=False, - ) - access_via_gid = resolved_access_rights.gid - access_via_folder_id = resolved_access_rights.folder_id - - base_query = ( - sa.select( - folders, - folders_access_rights, - folders_access_rights.c.created.label("access_created"), - folders_access_rights.c.modified.label("access_modified"), - sa.literal_column(f"{access_via_gid}").label("access_via_gid"), - _get_subquery_my_access_rights( - access_via_gid, access_via_folder_id - ).label("my_access_rights"), - _SUBQUERY_ACCESS_RIGHTS.label("access_rights"), - ) - .join( - folders_access_rights, folders.c.id == folders_access_rights.c.folder_id - ) - .where( - folders_access_rights.c.traversal_parent_id.is_(None) - if folder_id is None - else folders_access_rights.c.traversal_parent_id == folder_id - ) - .where( - folders_access_rights.c.gid.in_([access_via_gid]) - if folder_id is None - else True - ) - .where( - _get_and_calsue_with_only_true_entries( - _required_permissions, folders_access_rights - ) - if folder_id is None - else True + base_query = ( + sa.select(*_LIST_SELECT_FIELDS) + .join(folders_access_rights, folders.c.id == folders_access_rights.c.folder_id) + .where(folders.c.product_name == product_name) + .where( + folders_access_rights.c.traversal_parent_id.is_(None) + if folder_id is None + else folders_access_rights.c.traversal_parent_id == folder_id + ) + .where(folders_access_rights.c.gid.in_(gids)) + .where( + _get_filter_for_enabled_permissions( + required_permissions, folders_access_rights ) - .where(folders.c.product_name == product_name) ) + .group_by(*_LIST_GROUP_BY_FIELDS) + ) - # Select total count from base_query - subquery = base_query.subquery() - count_query = sa.select(sa.func.count()).select_from(subquery) - count_result = await connection.execute(count_query) - total_count = await count_result.scalar() + # Select total count from base_query + subquery = base_query.subquery() + count_query = sa.select(sa.func.count()).select_from(subquery) + count_result = await connection.execute(count_query) + total_count = await count_result.scalar() - # Ordering and pagination - if order_by["direction"] == OrderDirection.ASC: - list_query = base_query.order_by( - sa.asc(getattr(folders.c, order_by["field"])) - ) - else: - list_query = base_query.order_by( - sa.desc(getattr(folders.c, order_by["field"])) - ) - list_query = list_query.offset(offset).limit(limit) + # Ordering and pagination + if order_by["direction"] == OrderDirection.ASC: + list_query = base_query.order_by(sa.asc(getattr(folders.c, order_by["field"]))) + else: + list_query = base_query.order_by(sa.desc(getattr(folders.c, order_by["field"]))) + list_query = list_query.offset(offset).limit(limit) - async for entry in connection.execute(list_query): - results.append(FolderEntry.from_orm(entry)) # noqa: PERF401s + async for entry in connection.execute(list_query): + results.append(FolderEntry.from_orm(entry)) # noqa: PERF401s return cast(int, total_count), results @@ -1082,56 +1111,42 @@ async def folder_get( connection: SAConnection, product_name: _ProductName, folder_id: _FolderID, - gid: _GroupID, + gids: set[_GroupID], *, - required_permissions=_requires(_BasePermissions.GET_FOLDER), # noqa: B008 + required_permissions: _FolderPermissions = _requires( # noqa: B008 + _BasePermissions.GET_FOLDER + ), ) -> FolderEntry: - async with connection.begin(): - resolved_access_rights: _ResolvedAccessRights = await _check_folder_and_access( - connection, - product_name, - folder_id=folder_id, - gid=gid, - permissions=required_permissions, - enforece_all_permissions=False, - ) - access_via_gid = resolved_access_rights.gid - access_via_folder_id = resolved_access_rights.folder_id - - query = ( - sa.select( - folders, - folders_access_rights, - folders_access_rights.c.created.label("access_created"), - folders_access_rights.c.modified.label("access_modified"), - sa.literal_column(f"{access_via_gid}").label("access_via_gid"), - _get_subquery_my_access_rights( - access_via_gid, access_via_folder_id - ).label("my_access_rights"), - _SUBQUERY_ACCESS_RIGHTS.label("access_rights"), - ) - .join( - folders_access_rights, folders.c.id == folders_access_rights.c.folder_id - ) - .where(folders_access_rights.c.folder_id == folder_id) - .where(folders_access_rights.c.gid.in_([access_via_gid])) - .where( - _get_and_calsue_with_only_true_entries( - required_permissions, folders_access_rights - ) - if folder_id is None - else True + resolved_access_rights: _ResolvedAccessRights = await _check_and_get_folder_access( + connection, + product_name, + folder_id=folder_id, + gids=gids, + permissions=required_permissions, + ) + permissions_gid: _GroupID = resolved_access_rights.gid + + query = ( + sa.select(*_LIST_SELECT_FIELDS) + .join(folders_access_rights, folders.c.id == folders_access_rights.c.folder_id) + .where(folders_access_rights.c.folder_id == folder_id) + .where(folders_access_rights.c.gid == permissions_gid) + .where( + _get_filter_for_enabled_permissions( + required_permissions, folders_access_rights ) - .where(folders.c.product_name == product_name) + if folder_id is None + else True ) + .where(folders.c.product_name == product_name) + .group_by(*_LIST_GROUP_BY_FIELDS) + ) - query_result: RowProxy | None = await ( - await connection.execute(query.params(start_folder_id=folder_id)) - ).fetchone() + query_result: RowProxy | None = await (await connection.execute(query)).fetchone() if query_result is None: raise FolderNotFoundError( - folder_id=folder_id, gid=gid, product_name=product_name + folder_id=folder_id, gids=gids, product_name=product_name ) return FolderEntry.from_orm(query_result) diff --git a/packages/postgres-database/tests/test_utils_folders.py b/packages/postgres-database/tests/test_utils_folders.py index a72721982ca..8c49fd9914f 100644 --- a/packages/postgres-database/tests/test_utils_folders.py +++ b/packages/postgres-database/tests/test_utils_folders.py @@ -3,7 +3,6 @@ # pylint:disable=unused-variable import itertools -import secrets from collections.abc import AsyncIterable, Awaitable, Callable from copy import deepcopy from typing import NamedTuple @@ -13,7 +12,7 @@ import sqlalchemy as sa from aiopg.sa.connection import SAConnection from aiopg.sa.result import RowProxy -from pydantic import BaseModel, Field, NonNegativeInt, ValidationError +from pydantic import BaseModel, Field, NonNegativeInt from pytest_simcore.helpers.faker_factories import random_product from simcore_postgres_database.models.folders import ( folders, @@ -33,12 +32,13 @@ FolderEntry, FolderNotFoundError, FolderNotSharedWithGidError, - GroupIdDoesNotExistError, InsufficientPermissionsError, InvalidFolderNameError, + NoGroupIDFoundError, + RootFolderRequiresAtLeastOnePrimaryGroupError, _FolderID, _FolderPermissions, - _get_and_calsue_with_only_true_entries, + _get_filter_for_enabled_permissions, _get_permissions_from_role, _get_resolved_access_rights, _GroupID, @@ -191,31 +191,25 @@ async def default_product_name( ], ) async def test_folder_create_wrong_folder_name(invalid_name: str): - with pytest.raises((InvalidFolderNameError, ValidationError)): + with pytest.raises(InvalidFolderNameError): await folder_create(Mock(), "mock_product", invalid_name, Mock()) def test__get_where_clause(): assert isinstance( - _get_and_calsue_with_only_true_entries( - VIEWER_PERMISSIONS, folders_access_rights - ), + _get_filter_for_enabled_permissions(VIEWER_PERMISSIONS, folders_access_rights), ColumnElement, ) assert isinstance( - _get_and_calsue_with_only_true_entries( - EDITOR_PERMISSIONS, folders_access_rights - ), + _get_filter_for_enabled_permissions(EDITOR_PERMISSIONS, folders_access_rights), ColumnElement, ) assert isinstance( - _get_and_calsue_with_only_true_entries( - OWNER_PERMISSIONS, folders_access_rights - ), + _get_filter_for_enabled_permissions(OWNER_PERMISSIONS, folders_access_rights), ColumnElement, ) assert isinstance( - _get_and_calsue_with_only_true_entries( + _get_filter_for_enabled_permissions( _FolderPermissions(read=False, write=False, delete=False), folders_access_rights, ), @@ -239,6 +233,19 @@ async def _query_table(table_name: sa.Table, count: NonNegativeInt) -> None: await _query_table(folders_access_rights, access_rights_count or folder_count) +async def _assert_folderpermissions_exists( + connection: SAConnection, folder_id: _FolderID, gids: set[_GroupID] +) -> None: + result = await connection.execute( + folders_access_rights.select() + .where(folders_access_rights.c.folder_id == folder_id) + .where(folders_access_rights.c.gid.in_(gids)) + ) + rows = await result.fetchall() + assert rows is not None + assert len(rows) == 1 + + async def _assert_folder_permissions( connection: SAConnection, *, @@ -247,11 +254,11 @@ async def _assert_folder_permissions( role: FolderAccessRole, ) -> None: result = await connection.execute( - sa.select([folders_access_rights.c.folder_id]) + sa.select(folders_access_rights.c.folder_id) .where(folders_access_rights.c.folder_id == folder_id) .where(folders_access_rights.c.gid == gid) .where( - _get_and_calsue_with_only_true_entries( + _get_filter_for_enabled_permissions( _get_permissions_from_role(role), folders_access_rights ) ) @@ -269,7 +276,7 @@ async def _assert_name_and_description( description: str, ): async with connection.execute( - sa.select([folders.c.name, folders.c.description]).where( + sa.select(folders.c.name, folders.c.description).where( folders.c.id == folder_id ) ) as result_proxy: @@ -378,7 +385,7 @@ async def _( connection, default_product_name, root.name, - root.gid, + {root.gid}, description=root.description, parent=parent, ) @@ -388,7 +395,7 @@ async def _( connection, default_product_name, root_folder_id, - root.gid, + sharing_gids={root.gid}, recipient_gid=gid, recipient_role=role, ) @@ -426,26 +433,110 @@ async def test_folder_create( # 1. when GID is missing no entries should be present missing_gid = 10202023302 await _assert_folder_entires(connection, folder_count=expected_folder_count) - with pytest.raises(GroupIdDoesNotExistError): - await folder_create(connection, product_name, "f1", missing_gid) + with pytest.raises(NoGroupIDFoundError): + await folder_create(connection, product_name, "f1", {missing_gid}) await _assert_folder_entires(connection, folder_count=expected_folder_count) # 2. create a folder and a subfolder of the same name - f1_folder_id = await folder_create(connection, product_name, "f1", owner_gid) + f1_folder_id = await folder_create(connection, product_name, "f1", {owner_gid}) expected_folder_count += 1 await _assert_folder_entires(connection, folder_count=expected_folder_count) await folder_create( - connection, product_name, "f1", owner_gid, parent=f1_folder_id + connection, product_name, "f1", {owner_gid}, parent=f1_folder_id ) expected_folder_count += 1 await _assert_folder_entires(connection, folder_count=expected_folder_count) # 3. inserting already existing folder fails with pytest.raises(FolderAlreadyExistsError): - await folder_create(connection, product_name, "f1", owner_gid) + await folder_create(connection, product_name, "f1", {owner_gid}) await _assert_folder_entires(connection, folder_count=expected_folder_count) +async def test_folder_create_shared_via_groups( + connection: SAConnection, + default_product_name: _ProductName, + get_unique_gids: Callable[[int], tuple[_GroupID, ...]], + make_folders: Callable[[set[MkFolder]], Awaitable[dict[str, _FolderID]]], + create_fake_group: Callable[..., Awaitable[RowProxy]], +): + ####### + # SETUP + ####### + gid_original_owner: _GroupID + (gid_original_owner,) = get_unique_gids(1) + + gid_user: _GroupID = ( + await create_fake_group(connection, type=GroupType.PRIMARY) + ).gid + gid_everyone: _GroupID | None = await connection.scalar( + sa.select(groups.c.gid).where(groups.c.type == GroupType.EVERYONE) + ) + assert gid_everyone + gid_z43: _GroupID = ( + await create_fake_group(connection, type=GroupType.STANDARD) + ).gid + + folder_ids = await make_folders( + { + MkFolder( + name="root", + gid=gid_original_owner, + shared_with={ + gid_z43: FolderAccessRole.OWNER, + gid_everyone: FolderAccessRole.OWNER, + }, + ), + } + ) + + folder_id_root = folder_ids["root"] + + ####### + # TESTS + ####### + + # 1. can create when using one gid with permissions + folder_id_f1 = await folder_create( + connection, + default_product_name, + "f1", + {gid_z43, gid_user}, + parent=folder_id_root, + ) + await _assert_folderpermissions_exists(connection, folder_id_f1, {gid_z43}) + + folder_id_f2 = await folder_create( + connection, + default_product_name, + "f2", + {gid_everyone, gid_user}, + parent=folder_id_root, + ) + await _assert_folderpermissions_exists(connection, folder_id_f2, {gid_everyone}) + + # 2. can create new folder when using both gids with permissions + folder_id_f3 = await folder_create( + connection, + default_product_name, + "f3", + {gid_z43, gid_everyone, gid_user}, + parent=folder_id_root, + ) + await _assert_folderpermissions_exists( + connection, folder_id_f3, {gid_everyone, gid_z43} + ) + + # 3. cannot create a root folder without a primary group + with pytest.raises(RootFolderRequiresAtLeastOnePrimaryGroupError): + await folder_create( + connection, + default_product_name, + "folder_in_root", + {gid_z43, gid_everyone}, + ) + + async def test__get_resolved_access_rights( connection: SAConnection, get_unique_gids: Callable[[int], tuple[_GroupID, ...]], @@ -513,10 +604,6 @@ async def _assert_resolves_to( target_folder_id, gid, permissions=permissions, - # NOTE: this is the more restricitve case - # and we test against exact user roles, - # the APIs use only a subset of the permissions ususally set to True - enforece_all_permissions=True, ) assert resolved_parent assert resolved_parent.folder_id == expected_folder_id @@ -591,14 +678,14 @@ async def test_folder_share_or_update_permissions( connection, default_product_name, folder_id_missing, - sharing_gid=gid_owner, + sharing_gids={gid_owner}, recipient_gid=gid_share_with_error, recipient_role=FolderAccessRole.OWNER, ) await _assert_folder_entires(connection, folder_count=0) # 2. share existing folder with all possible roles - folder_id = await folder_create(connection, default_product_name, "f1", gid_owner) + folder_id = await folder_create(connection, default_product_name, "f1", {gid_owner}) await _assert_folder_entires(connection, folder_count=1) await _assert_folder_permissions( connection, folder_id=folder_id, gid=gid_owner, role=FolderAccessRole.OWNER @@ -608,7 +695,7 @@ async def test_folder_share_or_update_permissions( connection, default_product_name, folder_id, - sharing_gid=gid_owner, + sharing_gids={gid_owner}, recipient_gid=gid_other_owner, recipient_role=FolderAccessRole.OWNER, ) @@ -624,7 +711,7 @@ async def test_folder_share_or_update_permissions( connection, default_product_name, folder_id, - sharing_gid=gid_owner, + sharing_gids={gid_owner}, recipient_gid=gid_editor, recipient_role=FolderAccessRole.EDITOR, ) @@ -637,7 +724,7 @@ async def test_folder_share_or_update_permissions( connection, default_product_name, folder_id, - sharing_gid=gid_owner, + sharing_gids={gid_owner}, recipient_gid=gid_viewer, recipient_role=FolderAccessRole.VIEWER, ) @@ -650,7 +737,7 @@ async def test_folder_share_or_update_permissions( connection, default_product_name, folder_id, - sharing_gid=gid_owner, + sharing_gids={gid_owner}, recipient_gid=gid_no_access, recipient_role=FolderAccessRole.NO_ACCESS, ) @@ -670,7 +757,7 @@ async def test_folder_share_or_update_permissions( connection, default_product_name, folder_id, - sharing_gid=no_access_gids, + sharing_gids={no_access_gids}, recipient_gid=gid_share_with_error, recipient_role=recipient_role, ) @@ -683,7 +770,7 @@ async def test_folder_share_or_update_permissions( connection, default_product_name, folder_id, - sharing_gid=gid_share_with_error, + sharing_gids={gid_share_with_error}, recipient_gid=gid_share_with_error, recipient_role=recipient_role, ) @@ -696,7 +783,7 @@ async def test_folder_share_or_update_permissions( connection, default_product_name, folder_id, - sharing_gid=gid_other_owner, + sharing_gids={gid_other_owner}, recipient_gid=gid_to_drop_permission, recipient_role=FolderAccessRole.NO_ACCESS, ) @@ -727,17 +814,17 @@ async def test_folder_update( missing_folder_id = 1231321332 with pytest.raises(FolderNotFoundError): await folder_update( - connection, default_product_name, missing_folder_id, owner_gid + connection, default_product_name, missing_folder_id, {owner_gid} ) await _assert_folder_entires(connection, folder_count=0) # 2. owner updates created fodler - folder_id = await folder_create(connection, default_product_name, "f1", owner_gid) + folder_id = await folder_create(connection, default_product_name, "f1", {owner_gid}) await _assert_folder_entires(connection, folder_count=1) await _assert_name_and_description(connection, folder_id, name="f1", description="") # nothing changes - await folder_update(connection, default_product_name, folder_id, owner_gid) + await folder_update(connection, default_product_name, folder_id, {owner_gid}) await _assert_name_and_description(connection, folder_id, name="f1", description="") # both changed @@ -745,7 +832,7 @@ async def test_folder_update( connection, default_product_name, folder_id, - owner_gid, + {owner_gid}, name="new_folder", description="new_desc", ) @@ -758,7 +845,7 @@ async def test_folder_update( connection, default_product_name, folder_id, - sharing_gid=owner_gid, + sharing_gids={owner_gid}, recipient_gid=other_owner_gid, recipient_role=FolderAccessRole.OWNER, ) @@ -766,7 +853,7 @@ async def test_folder_update( connection, default_product_name, folder_id, - owner_gid, + {owner_gid}, name="another_owner_name", description="another_owner_description", ) @@ -782,7 +869,7 @@ async def test_folder_update( connection, default_product_name, folder_id, - sharing_gid=owner_gid, + sharing_gids={owner_gid}, recipient_gid=editor_gid, recipient_role=FolderAccessRole.EDITOR, ) @@ -790,7 +877,7 @@ async def test_folder_update( connection, default_product_name, folder_id, - sharing_gid=owner_gid, + sharing_gids={owner_gid}, recipient_gid=viewer_gid, recipient_role=FolderAccessRole.VIEWER, ) @@ -798,7 +885,7 @@ async def test_folder_update( connection, default_product_name, folder_id, - sharing_gid=owner_gid, + sharing_gids={owner_gid}, recipient_gid=no_access_gid, recipient_role=FolderAccessRole.NO_ACCESS, ) @@ -809,7 +896,7 @@ async def test_folder_update( connection, default_product_name, folder_id, - target_user_gid, + {target_user_gid}, name="error_name", description="error_description", ) @@ -825,7 +912,7 @@ async def test_folder_update( connection, default_product_name, folder_id, - share_with_error_gid, + {share_with_error_gid}, name="error_name", description="error_description", ) @@ -855,42 +942,42 @@ async def test_folder_delete( missing_folder_id = 1231321332 with pytest.raises(FolderNotFoundError): await folder_delete( - connection, default_product_name, missing_folder_id, owner_gid + connection, default_product_name, missing_folder_id, {owner_gid} ) await _assert_folder_entires(connection, folder_count=0) # 2. owner deletes folder - folder_id = await folder_create(connection, default_product_name, "f1", owner_gid) + folder_id = await folder_create(connection, default_product_name, "f1", {owner_gid}) await _assert_folder_entires(connection, folder_count=1) - await folder_delete(connection, default_product_name, folder_id, owner_gid) + await folder_delete(connection, default_product_name, folder_id, {owner_gid}) await _assert_folder_entires(connection, folder_count=0) # 3. other owners can delete the folder - folder_id = await folder_create(connection, default_product_name, "f1", owner_gid) + folder_id = await folder_create(connection, default_product_name, "f1", {owner_gid}) await _assert_folder_entires(connection, folder_count=1) await folder_share_or_update_permissions( connection, default_product_name, folder_id, - sharing_gid=owner_gid, + sharing_gids={owner_gid}, recipient_gid=other_owner_gid, recipient_role=FolderAccessRole.OWNER, ) - await folder_delete(connection, default_product_name, folder_id, other_owner_gid) + await folder_delete(connection, default_product_name, folder_id, {other_owner_gid}) await _assert_folder_entires(connection, folder_count=0) # 4. non owner users cannot delete the folder - folder_id = await folder_create(connection, default_product_name, "f1", owner_gid) + folder_id = await folder_create(connection, default_product_name, "f1", {owner_gid}) await _assert_folder_entires(connection, folder_count=1) await folder_share_or_update_permissions( connection, default_product_name, folder_id, - sharing_gid=owner_gid, + sharing_gids={owner_gid}, recipient_gid=editor_gid, recipient_role=FolderAccessRole.EDITOR, ) @@ -898,7 +985,7 @@ async def test_folder_delete( connection, default_product_name, folder_id, - sharing_gid=owner_gid, + sharing_gids={owner_gid}, recipient_gid=viewer_gid, recipient_role=FolderAccessRole.VIEWER, ) @@ -906,7 +993,7 @@ async def test_folder_delete( connection, default_product_name, folder_id, - sharing_gid=owner_gid, + sharing_gids={owner_gid}, recipient_gid=no_access_gid, recipient_role=FolderAccessRole.NO_ACCESS, ) @@ -915,12 +1002,12 @@ async def test_folder_delete( for non_owner_gid in (editor_gid, viewer_gid, no_access_gid): with pytest.raises(InsufficientPermissionsError): await folder_delete( - connection, default_product_name, folder_id, non_owner_gid + connection, default_product_name, folder_id, {non_owner_gid} ) with pytest.raises(FolderNotSharedWithGidError): await folder_delete( - connection, default_product_name, folder_id, share_with_error_gid + connection, default_product_name, folder_id, {share_with_error_gid} ) await _assert_folder_entires(connection, folder_count=1, access_rights_count=4) @@ -965,12 +1052,12 @@ async def _setup_folders() -> _FolderID: folder_id_root_folder = folder_ids["root_folder"] await _assert_folder_entires(connection, folder_count=1, access_rights_count=6) - GIDS_WITH_CREATE_PERMISSIONS: tuple[_GroupID, ...] = ( + GIDS_WITH_CREATE_PERMISSIONS: set[_GroupID] = { gid_owner_a, gid_owner_b, gid_editor_a, gid_editor_b, - ) + } previous_folder_id = folder_id_root_folder for i in range(100): @@ -978,7 +1065,7 @@ async def _setup_folders() -> _FolderID: connection, default_product_name, f"f{i}", - secrets.choice(GIDS_WITH_CREATE_PERMISSIONS), + GIDS_WITH_CREATE_PERMISSIONS, parent=previous_folder_id, ) await _assert_folder_entires( @@ -993,14 +1080,14 @@ async def _setup_folders() -> _FolderID: # 1. delete via `gid_owner_a` folder_id_root_folder = await _setup_folders() await folder_delete( - connection, default_product_name, folder_id_root_folder, gid_owner_a + connection, default_product_name, folder_id_root_folder, {gid_owner_a} ) await _assert_folder_entires(connection, folder_count=0) # 2. delete via shared with `gid_owner_b` folder_id_root_folder = await _setup_folders() await folder_delete( - connection, default_product_name, folder_id_root_folder, gid_owner_b + connection, default_product_name, folder_id_root_folder, {gid_owner_b} ) await _assert_folder_entires(connection, folder_count=0) @@ -1012,7 +1099,7 @@ async def _setup_folders() -> _FolderID: connection, default_product_name, folder_id_root_folder, - no_permissions_gid, + {no_permissions_gid}, ) for no_permissions_gid in (gid_not_shared,): with pytest.raises(FolderNotSharedWithGidError): @@ -1020,7 +1107,7 @@ async def _setup_folders() -> _FolderID: connection, default_product_name, folder_id_root_folder, - no_permissions_gid, + {no_permissions_gid}, ) await _assert_folder_entires(connection, folder_count=101, access_rights_count=106) @@ -1139,7 +1226,7 @@ async def _move_fails_not_shared_with_error( connection, default_product_name, source, - gid, + {gid}, destination_folder_id=destination, ) @@ -1151,7 +1238,7 @@ async def _move_fails_insufficient_permissions_error( connection, default_product_name, source, - gid, + {gid}, destination_folder_id=destination, ) @@ -1170,7 +1257,7 @@ async def _assert_folder_permissions( parent_folder: _FolderID, ) -> None: result = await connection.execute( - sa.select([folders_access_rights.c.folder_id]) + sa.select(folders_access_rights.c.folder_id) .where(folders_access_rights.c.folder_id == folder_id) .where(folders_access_rights.c.gid == gid) .where(folders_access_rights.c.traversal_parent_id == parent_folder) @@ -1188,7 +1275,7 @@ async def _assert_folder_permissions( connection, default_product_name, source, - gid, + {gid}, destination_folder_id=destination, ) @@ -1201,7 +1288,7 @@ async def _assert_folder_permissions( connection, default_product_name, source, - gid, + {gid}, destination_folder_id=source_parent, ) @@ -1297,7 +1384,7 @@ async def _assert_folder_permissions( connection, default_product_name, to_move_folder_id, - to_move_gid, + {to_move_gid}, destination_folder_id=None, ) @@ -1315,7 +1402,7 @@ async def _assert_folder_permissions( connection, default_product_name, to_move_folder_id, - to_move_gid, + {to_move_gid}, destination_folder_id=None, ) @@ -1325,7 +1412,7 @@ async def _assert_folder_permissions( connection, default_product_name, folder_id_not_shared, - to_move_gid, + {to_move_gid}, destination_folder_id=None, ) @@ -1379,7 +1466,7 @@ async def _fails_to_move(gid: _GroupID, destination_folder_id: _FolderID) -> Non connection, default_product_name, folder_id_to_move, - gid, + {gid}, destination_folder_id=destination_folder_id, ) @@ -1398,7 +1485,7 @@ async def _fails_to_move(gid: _GroupID, destination_folder_id: _FolderID) -> Non connection, default_product_name, folder_id_to_move, - gid_not_shared, + {gid_not_shared}, destination_folder_id=folder_id_target_not_shared, ) @@ -1407,7 +1494,7 @@ async def _fails_to_move(gid: _GroupID, destination_folder_id: _FolderID) -> Non connection, default_product_name, folder_id_to_move, - gid_owner, + {gid_owner}, destination_folder_id=folder_id_target_owner, ) @@ -1422,28 +1509,45 @@ async def test_move_group_non_standard_groups_raise_error( ####### # SETUP ####### - (gid_sharing,) = get_unique_gids(1) - gid_primary = (await create_fake_group(connection, type=GroupType.PRIMARY)).gid - gid_everyone = await connection.scalar( - sa.select([groups.c.gid]).where(groups.c.type == GroupType.EVERYONE) + gid_original_owner: _GroupID + (gid_original_owner,) = get_unique_gids(1) + gid_primary: _GroupID = ( + await create_fake_group(connection, type=GroupType.PRIMARY) + ).gid + gid_everyone: _GroupID | None = await connection.scalar( + sa.select(groups.c.gid).where(groups.c.type == GroupType.EVERYONE) ) assert gid_everyone - gid_standard = (await create_fake_group(connection, type=GroupType.STANDARD)).gid + gid_standard: _GroupID = ( + await create_fake_group(connection, type=GroupType.STANDARD) + ).gid folder_ids = await make_folders( { MkFolder( name="SHARING_USER", - gid=gid_sharing, + gid=gid_original_owner, shared_with={ gid_primary: FolderAccessRole.EDITOR, gid_everyone: FolderAccessRole.EDITOR, gid_standard: FolderAccessRole.EDITOR, }, ), - MkFolder(name="PRIMARY", gid=gid_primary), - MkFolder(name="EVERYONE", gid=gid_everyone), - MkFolder(name="STANDARD", gid=gid_standard), + MkFolder( + name="PRIMARY", + gid=gid_original_owner, + shared_with={gid_primary: FolderAccessRole.OWNER}, + ), + MkFolder( + name="EVERYONE", + gid=gid_original_owner, + shared_with={gid_everyone: FolderAccessRole.OWNER}, + ), + MkFolder( + name="STANDARD", + gid=gid_original_owner, + shared_with={gid_standard: FolderAccessRole.OWNER}, + ), } ) @@ -1461,7 +1565,7 @@ async def test_move_group_non_standard_groups_raise_error( connection, default_product_name, folder_id_everyone, - gid_everyone, + {gid_everyone}, destination_folder_id=folder_id_sharing_user, ) assert "EVERYONE" in f"{exc.value}" @@ -1471,7 +1575,7 @@ async def test_move_group_non_standard_groups_raise_error( connection, default_product_name, folder_id_standard, - gid_standard, + {gid_standard}, destination_folder_id=folder_id_sharing_user, ) assert "STANDARD" in f"{exc.value}" @@ -1481,7 +1585,7 @@ async def test_move_group_non_standard_groups_raise_error( connection, default_product_name, folder_id_primary, - gid_primary, + {gid_primary}, destination_folder_id=folder_id_sharing_user, ) @@ -1534,7 +1638,7 @@ async def _add_folder_as(gid: _GroupID) -> None: connection, default_product_name, folder_id_f1, - gid, + {gid}, project_uuid=project_uuid, ) assert await _is_project_present(connection, folder_id_f1, project_uuid) is True @@ -1544,7 +1648,7 @@ async def _remove_folder_as(gid: _GroupID) -> None: connection, default_product_name, folder_id_f1, - gid, + {gid}, project_uuid=project_uuid, ) assert ( @@ -1582,7 +1686,6 @@ async def _remove_folder_as(gid: _GroupID) -> None: class ExpectedValues(NamedTuple): id: _FolderID - access_via_gid: _GroupID my_access_rights: _FolderPermissions access_rights: dict[_GroupID, _FolderPermissions] @@ -1590,7 +1693,6 @@ def __hash__(self): return hash( ( self.id, - self.access_via_gid, self.my_access_rights, tuple(sorted(self.access_rights.items())), ) @@ -1601,7 +1703,6 @@ def __eq__(self, other): return False return ( self.id == other.id - and self.access_via_gid == other.access_via_gid and self.my_access_rights == other.my_access_rights and self.access_rights == other.access_rights ) @@ -1613,7 +1714,6 @@ def _assert_expected_entries( for folder_entry in folders: expected_values = ExpectedValues( folder_entry.id, - folder_entry.access_via_gid, folder_entry.my_access_rights, folder_entry.access_rights, ) @@ -1628,13 +1728,13 @@ async def _list_folder_as( connection: SAConnection, default_product_name: _ProductName, folder_id: _FolderID | None, - gid: _GroupID, + gids: set[_GroupID], offset: NonNegativeInt = ALL_IN_ONE_PAGE_OFFSET, limit: NonNegativeInt = ALL_IN_ONE_PAGE_LIMIT, ) -> list[FolderEntry]: - total_count, folders_db = await folder_list( - connection, default_product_name, folder_id, gid, offset=offset, limit=limit + _, folders_db = await folder_list( + connection, default_product_name, folder_id, gids, offset=offset, limit=limit ) return folders_db @@ -1750,11 +1850,12 @@ async def test_folder_list( for listing_gid in (gid_owner, gid_editor, gid_viewer): # list `root` for gid _assert_expected_entries( - await _list_folder_as(connection, default_product_name, None, listing_gid), + await _list_folder_as( + connection, default_product_name, None, {listing_gid} + ), expected={ ExpectedValues( folder_id_owner_folder, - listing_gid, ACCESS_RIGHTS_BY_GID[listing_gid], { gid_owner: OWNER_PERMISSIONS, @@ -1768,12 +1869,11 @@ async def test_folder_list( # list `owner_folder` for gid _assert_expected_entries( await _list_folder_as( - connection, default_product_name, folder_id_owner_folder, listing_gid + connection, default_product_name, folder_id_owner_folder, {listing_gid} ), expected={ ExpectedValues( fx, - listing_gid, ACCESS_RIGHTS_BY_GID[listing_gid], {gid_owner: OWNER_PERMISSIONS}, ) @@ -1783,12 +1883,11 @@ async def test_folder_list( # list `f10` for gid _assert_expected_entries( await _list_folder_as( - connection, default_product_name, folder_id_f10, listing_gid + connection, default_product_name, folder_id_f10, {listing_gid} ), expected={ ExpectedValues( sub_fx, - listing_gid, ACCESS_RIGHTS_BY_GID[listing_gid], {gid_owner: OWNER_PERMISSIONS}, ) @@ -1799,26 +1898,26 @@ async def test_folder_list( # 2. lisit all levels for `gid_no_access` # can always be ran but should not list any entry _assert_expected_entries( - await _list_folder_as(connection, default_product_name, None, gid_no_access), + await _list_folder_as(connection, default_product_name, None, {gid_no_access}), expected=set(), ) # there are insusficient permissions for folder_id_to_check in ALL_FOLDERS_AND_SUBFOLDERS: with pytest.raises(InsufficientPermissionsError): await _list_folder_as( - connection, default_product_name, folder_id_to_check, gid_no_access + connection, default_product_name, folder_id_to_check, {gid_no_access} ) # 3. lisit all levels for `gid_not_shared`` # can always list the contets of the "root" folder for a gid _assert_expected_entries( - await _list_folder_as(connection, default_product_name, None, gid_not_shared), + await _list_folder_as(connection, default_product_name, None, {gid_not_shared}), expected=set(), ) for folder_id_to_check in ALL_FOLDERS_AND_SUBFOLDERS: with pytest.raises(FolderNotSharedWithGidError): await _list_folder_as( - connection, default_product_name, folder_id_to_check, gid_not_shared + connection, default_product_name, folder_id_to_check, {gid_not_shared} ) # 4. list with pagination @@ -1830,7 +1929,7 @@ async def test_folder_list( connection, default_product_name, folder_id_owner_folder, - gid_owner, + {gid_owner}, offset=offset, limit=limit, ): @@ -1840,7 +1939,7 @@ async def test_folder_list( break one_shot_query = await _list_folder_as( - connection, default_product_name, folder_id_owner_folder, gid_owner + connection, default_product_name, folder_id_owner_folder, {gid_owner} ) assert len(found_folders) == len(one_shot_query) @@ -1905,11 +2004,12 @@ async def test_folder_list_shared_with_different_permissions( for listing_gid in (gid_owner_a, gid_owner_b, gid_owner_c): # list `root` for gid _assert_expected_entries( - await _list_folder_as(connection, default_product_name, None, listing_gid), + await _list_folder_as( + connection, default_product_name, None, {listing_gid} + ), expected={ ExpectedValues( folder_id_f_owner_a, - listing_gid, OWNER_PERMISSIONS, { gid_owner_a: OWNER_PERMISSIONS, @@ -1922,12 +2022,11 @@ async def test_folder_list_shared_with_different_permissions( # list `f_owner_a` for gid _assert_expected_entries( await _list_folder_as( - connection, default_product_name, folder_id_f_owner_a, listing_gid + connection, default_product_name, folder_id_f_owner_a, {listing_gid} ), expected={ ExpectedValues( folder_id_f_owner_b, - listing_gid, OWNER_PERMISSIONS, {gid_owner_b: OWNER_PERMISSIONS}, ), @@ -1936,12 +2035,11 @@ async def test_folder_list_shared_with_different_permissions( # list `f_owner_b` for gid _assert_expected_entries( await _list_folder_as( - connection, default_product_name, folder_id_f_owner_b, listing_gid + connection, default_product_name, folder_id_f_owner_b, {listing_gid} ), expected={ ExpectedValues( folder_id_f_owner_c, - listing_gid, OWNER_PERMISSIONS, { gid_owner_c: OWNER_PERMISSIONS, @@ -1953,12 +2051,11 @@ async def test_folder_list_shared_with_different_permissions( # list `f_owner_c` for gid _assert_expected_entries( await _list_folder_as( - connection, default_product_name, folder_id_f_owner_c, listing_gid + connection, default_product_name, folder_id_f_owner_c, {listing_gid} ), expected={ ExpectedValues( folder_id_f_sub_owner_c, - listing_gid, OWNER_PERMISSIONS, { gid_owner_c: OWNER_PERMISSIONS, @@ -1966,7 +2063,6 @@ async def test_folder_list_shared_with_different_permissions( ), ExpectedValues( folder_id_f_owner_level_2, - listing_gid, OWNER_PERMISSIONS, { gid_owner_level_2: OWNER_PERMISSIONS, @@ -1979,12 +2075,11 @@ async def test_folder_list_shared_with_different_permissions( # list `f_owner_c` for `gid_owner_level_2` _assert_expected_entries( await _list_folder_as( - connection, default_product_name, None, gid_owner_level_2 + connection, default_product_name, None, {gid_owner_level_2} ), expected={ ExpectedValues( folder_id_f_owner_c, - gid_owner_level_2, OWNER_PERMISSIONS, { gid_owner_c: OWNER_PERMISSIONS, @@ -1996,12 +2091,11 @@ async def test_folder_list_shared_with_different_permissions( # list `root` for `gid_owner_level_2` _assert_expected_entries( await _list_folder_as( - connection, default_product_name, folder_id_f_owner_c, gid_owner_level_2 + connection, default_product_name, folder_id_f_owner_c, {gid_owner_level_2} ), expected={ ExpectedValues( folder_id_f_sub_owner_c, - gid_owner_level_2, OWNER_PERMISSIONS, { gid_owner_c: OWNER_PERMISSIONS, @@ -2009,7 +2103,6 @@ async def test_folder_list_shared_with_different_permissions( ), ExpectedValues( folder_id_f_owner_level_2, - gid_owner_level_2, OWNER_PERMISSIONS, { gid_owner_level_2: OWNER_PERMISSIONS, @@ -2019,6 +2112,121 @@ async def test_folder_list_shared_with_different_permissions( ) +async def test_folder_list_in_root_with_different_groups_avoids_duplicate_entries( + connection: SAConnection, + default_product_name: _ProductName, + get_unique_gids: Callable[[int], tuple[_GroupID, ...]], + make_folders: Callable[[set[MkFolder]], Awaitable[dict[str, _FolderID]]], +): + ####### + # SETUP + ####### + + (gid_z43, gid_osparc, gid_user) = get_unique_gids(3) + + await make_folders( + { + MkFolder( + name="f1", + gid=gid_user, + shared_with={ + gid_z43: FolderAccessRole.OWNER, + gid_osparc: FolderAccessRole.OWNER, + }, + ), + MkFolder( + name="f2", + gid=gid_z43, + shared_with={ + gid_osparc: FolderAccessRole.OWNER, + }, + ), + MkFolder( + name="f3", + gid=gid_osparc, + shared_with={ + gid_z43: FolderAccessRole.OWNER, + }, + ), + } + ) + + ####### + # TESTS + ####### + + # 1. gid_z43 and gid_osparc see all folders + for gid_all_folders in (gid_z43, gid_osparc): + entries_z43 = await _list_folder_as( + connection, default_product_name, None, {gid_all_folders} + ) + assert len(entries_z43) == 3 + + # 2. gid_user only sees it's own folder + entries_user = await _list_folder_as( + connection, default_product_name, None, {gid_user} + ) + assert len(entries_user) == 1 + + # 3. all gids see all fodlers + entries_all_groups = await _list_folder_as( + connection, default_product_name, None, {gid_z43, gid_osparc, gid_user} + ) + assert len(entries_all_groups) == 3 + + +async def test_regression_list_folder_parent( + connection: SAConnection, + default_product_name: _ProductName, + get_unique_gids: Callable[[int], tuple[_GroupID, ...]], + make_folders: Callable[[set[MkFolder]], Awaitable[dict[str, _FolderID]]], +): + ####### + # SETUP + ####### + + (gid_user,) = get_unique_gids(1) + + folder_ids = await make_folders( + { + MkFolder( + name="f1", + gid=gid_user, + children={ + MkFolder( + name="f2", + gid=gid_user, + children={ + MkFolder(name="f3", gid=gid_user), + }, + ) + }, + ), + } + ) + + folder_id_f1 = folder_ids["f1"] + folder_id_f2 = folder_ids["f2"] + folder_id_f3 = folder_ids["f3"] + + ####### + # TESTS + ####### + + for folder_id in (None, folder_id_f1, folder_id_f2): + folder_content = await _list_folder_as( + connection, default_product_name, folder_id, {gid_user} + ) + assert len(folder_content) == 1 + assert folder_content[0] + assert folder_content[0].parent_folder == folder_id + + f3_content = await _list_folder_as( + connection, default_product_name, folder_id_f3, {gid_user} + ) + assert len(f3_content) == 0 + + async def test_folder_get( connection: SAConnection, default_product_name: _ProductName, @@ -2077,18 +2285,18 @@ async def test_folder_get( folder_id_sub_f2, ): folder_entries = await _list_folder_as( - connection, default_product_name, folder_id_to_list, gid_owner + connection, default_product_name, folder_id_to_list, {gid_owner} ) for entry in folder_entries: queried_folder = await folder_get( - connection, default_product_name, entry.id, entry.access_via_gid + connection, default_product_name, entry.id, {gid_owner} ) assert entry == queried_folder # 2. query via gid_not_shared with pytest.raises(FolderNotSharedWithGidError): await folder_get( - connection, default_product_name, folder_id_owner_folder, gid_not_shared + connection, default_product_name, folder_id_owner_folder, {gid_not_shared} ) # 3. query with missing folder_id @@ -2100,5 +2308,5 @@ async def test_folder_get( ): with pytest.raises(FolderNotFoundError): await folder_get( - connection, default_product_name, missing_folder_id, gid_to_test + connection, default_product_name, missing_folder_id, {gid_to_test} ) diff --git a/services/web/server/src/simcore_service_webserver/folders/_folders_api.py b/services/web/server/src/simcore_service_webserver/folders/_folders_api.py index 8c0b9de3b5c..ca9d0ce9899 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_folders_api.py +++ b/services/web/server/src/simcore_service_webserver/folders/_folders_api.py @@ -37,7 +37,7 @@ async def create_folder( connection, product_name=product_name, name=folder_name, - gid=user["primary_gid"], + gids={user["primary_gid"]}, description=description if description else "", parent=parent_folder_id, ) @@ -45,7 +45,7 @@ async def create_folder( connection, product_name=product_name, folder_id=folder_id, - gid=user["primary_gid"], + gids={user["primary_gid"]}, ) return FolderGet( folder_id=folder_db.id, @@ -80,7 +80,7 @@ async def get_folder( connection, product_name=product_name, folder_id=folder_id, - gid=user["primary_gid"], + gids={user["primary_gid"]}, ) return FolderGet( folder_id=folder_db.id, @@ -118,7 +118,7 @@ async def list_folders( connection, product_name=product_name, folder_id=folder_id, - gid=user["primary_gid"], + gids={user["primary_gid"]}, offset=offset, limit=limit, order_by=cast(folders_db.OrderByDict, order_by.dict()), @@ -167,7 +167,7 @@ async def update_folder( connection, product_name=product_name, folder_id=folder_id, - gid=user["primary_gid"], + gids={user["primary_gid"]}, name=name, description=description, ) @@ -175,7 +175,7 @@ async def update_folder( connection, product_name=product_name, folder_id=folder_id, - gid=user["primary_gid"], + gids={user["primary_gid"]}, ) return FolderGet( folder_id=folder_db.id, @@ -210,5 +210,5 @@ async def delete_folder( connection, product_name=product_name, folder_id=folder_id, - gid=user["primary_gid"], + gids={user["primary_gid"]}, ) diff --git a/services/web/server/src/simcore_service_webserver/folders/_folders_handlers.py b/services/web/server/src/simcore_service_webserver/folders/_folders_handlers.py index ed8f6108df6..1e67d43c6d8 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_folders_handlers.py +++ b/services/web/server/src/simcore_service_webserver/folders/_folders_handlers.py @@ -27,6 +27,7 @@ from servicelib.mimetype_constants import MIMETYPE_APPLICATION_JSON from servicelib.request_keys import RQT_USERID_KEY from servicelib.rest_constants import RESPONSE_MODEL_POLICY +from simcore_postgres_database.utils_folders import FoldersError from .._constants import RQ_PRODUCT_KEY from .._meta import API_VTAG as VTAG @@ -51,6 +52,9 @@ async def wrapper(request: web.Request) -> web.StreamResponse: except FolderAccessForbiddenError as exc: raise web.HTTPForbidden(reason=f"{exc}") from exc + except FoldersError as exc: + raise web.HTTPBadRequest(reason=f"{exc}") from exc + return wrapper diff --git a/services/web/server/src/simcore_service_webserver/projects/_folders_api.py b/services/web/server/src/simcore_service_webserver/projects/_folders_api.py index 8a13f7f46b8..8a3a1539253 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_folders_api.py +++ b/services/web/server/src/simcore_service_webserver/projects/_folders_api.py @@ -52,7 +52,7 @@ async def replace_project_folder( connection, product_name=product_name, folder_id=folder_id, - gid=user["primary_gid"], + gids={user["primary_gid"]}, project_uuid=project_id, ) return @@ -62,7 +62,7 @@ async def replace_project_folder( connection, product_name=product_name, source_folder_id=_source_folder_id, - gid=user["primary_gid"], + gids={user["primary_gid"]}, project_uuid=project_id, destination_folder_id=folder_id, )