From 8f626c0a0e8810134677ab815c91bcad9830a85b Mon Sep 17 00:00:00 2001 From: Juan Nicolas Herrera <54152074+juanNH@users.noreply.github.com> Date: Fri, 14 Aug 2026 01:24:36 -0300 Subject: [PATCH 1/2] Restrict dataset management to owners --- src/common/access_control.py | 16 +- .../components/files-manager/FilesManager.tsx | 92 +++++------ .../frontend/src/utils/django_interfaces.ts | 2 + src/user_files/serializers.py | 15 +- .../tests/test_owner_permissions.py | 152 ++++++++++++++++++ src/user_files/views.py | 25 ++- 6 files changed, 219 insertions(+), 83 deletions(-) create mode 100644 src/user_files/tests/test_owner_permissions.py diff --git a/src/common/access_control.py b/src/common/access_control.py index a486b559..8b5ae075 100644 --- a/src/common/access_control.py +++ b/src/common/access_control.py @@ -92,14 +92,10 @@ def user_file_visibility_q(user: AbstractBaseUser) -> Q: def can_edit_user_file(user_file: Any, user: AbstractBaseUser) -> bool: - """Return whether the user may edit/delete a UserFile.""" - if not user or not user.is_authenticated: - return False + """Return whether the user may mutate a UserFile. - return ( - user_file.user_id == user.id - or user_file.institutions.filter( - users=user, - institutionadministration__is_institution_admin=True, - ).exists() - ) + Institution membership controls visibility, but it does not grant + ownership or mutation permissions. Dataset edits, deletion, visibility + changes, and sharing changes are therefore restricted to the owner. + """ + return bool(user and user.is_authenticated and user_file.user_id == user.id) diff --git a/src/frontend/static/frontend/src/components/files-manager/FilesManager.tsx b/src/frontend/static/frontend/src/components/files-manager/FilesManager.tsx index 9b3827dc..0a928feb 100644 --- a/src/frontend/static/frontend/src/components/files-manager/FilesManager.tsx +++ b/src/frontend/static/frontend/src/components/files-manager/FilesManager.tsx @@ -15,7 +15,7 @@ import { PopupIcons } from '../common/PopupIcons' import { SwitchPublicButton } from '../common/SwitchPublicButton' import { useIntl, IntlShape } from 'react-intl' import { DeleteButton } from '../common/DeleteButton' -import { getTissueDropdownOptions, getTissueIds, TissueLabels } from '../common/TissueLabels' +import { getTissueDropdownOptions, getTissueIds } from '../common/TissueLabels' import { SharedInstitutionsUserFile } from './SharedInstitutionsUserFile' /** Structure returned from the chunk upload service. */ @@ -34,7 +34,6 @@ declare const urlChunkUpload: string declare const urlChunkUploadComplete: string declare const downloadFileURL: string declare const downloadFileHeaders: string -declare const currentUserId: string /** * New File Form fields @@ -842,53 +841,43 @@ class FilesManager extends React.Component {getFileTypeName(userFileRow.file_type)} - - - {userFileRow.institutions.length > 0 && ( - i.name).join(', ') } - )} - /> - )} - - - } - tagType={TagType.FILE} - onTagSelect={(tagId) => this.updateUserFileTag(userFileRow, tagId)} - onTagCreated={() => this.getUserTags()} - onTagEdited={() => this.getUserTags()} - onTagDeleted={(deletedTagId) => { - if (this.state.newFile.newTag === deletedTagId) { - this.handleAddFileInputsChange('newTag', null) - } - - this.getUserTags() - }} - /> - - - {(this.state.sharedInstitutionsByFile[userFileRow.id as number] ?? userFileRow.institutions).length > 0 - ? ( - i.name).join(', ') } - )} - /> - ) - : '-'} + + + {(this.state.sharedInstitutionsByFile[userFileRow.id as number] ?? userFileRow.institutions).length > 0 + ? ( + i.name).join(', ') } + )} + /> + ) + : '-'} + + + {userFileRow.is_owner + ? ( + } + tagType={TagType.FILE} + onTagSelect={(tagId) => this.updateUserFileTag(userFileRow, tagId)} + onTagCreated={() => this.getUserTags()} + onTagEdited={() => this.getUserTags()} + onTagDeleted={(deletedTagId) => { + if (this.state.newFile.newTag === deletedTagId) { + this.handleAddFileInputsChange('newTag', null) + } + + this.getUserTags() + }} + /> + ) + : } - - - + { userFileRow.is_public ? ( @@ -918,7 +907,7 @@ class FilesManager extends React.Component { columnName: userFileRow.column_used_as_index } )} /> - {Number(currentUserId) === userFileRow.user.id && ( + {userFileRow.is_owner && ( onClick={() => this.openShareInstitutions(userFileRow)} /> )} - {/* Users can modify or delete own files or the ones which belongs to an - Institution which the user is admin of */} - {userFileRow.is_private_or_institution_admin && ( + {/* Only the dataset owner can modify or delete it. */} + {userFileRow.is_owner && ( <> {/* Shows a edit button if specified */} UserFile: return get_object_or_404(queryset, pk=user_file_pk) -def get_own_or_as_admin_user_files(user: AbstractBaseUser): +def get_own_user_files(user: AbstractBaseUser): """ - Returns only the User's Files which were uploaded by himself or belongs to an Institution which his is the admin of + Returns only the User's Files which were uploaded by himself. @param user: User to retrieve his Datasets @return: User's Files """ - return UserFile.objects.filter(Q(user=user) | ( - Q(institutions__institutionadministration__user=user) - & Q(institutions__institutionadministration__is_institution_admin=True) - )).distinct() + return UserFile.objects.filter(user=user) def get_user_files(user: AbstractBaseUser, public_only: bool, private_only: bool, with_survival_only: bool) -> QuerySet: @@ -157,14 +154,14 @@ def get_queryset(self): class UserFileDetail(generics.RetrieveUpdateDestroyAPIView): """ - REST endpoint: get, modify or delete for UserFile model. A User can modify or delete ONLY the files which were - uploaded by himself or belongs to an Institution which his is the admin of + REST endpoint: get, modify or delete for UserFile model. Only the owner + can modify or delete a UserFile; institutional visibility is read-only. """ def get_queryset(self): if self.request.method == 'GET': return UserFile.objects.filter(user_file_visibility_q(self.request.user)).distinct() - return get_own_or_as_admin_user_files(self.request.user) + return get_own_user_files(self.request.user) serializer_class = UserFileSerializer permission_classes = [permissions.IsAuthenticated] @@ -180,7 +177,7 @@ def get_queryset(self): """Return the current user's institutions not already shared with the dataset.""" user_file_id = self.kwargs.get('user_file_id') user_file = get_object_or_404(UserFile, pk=user_file_id) - if not can_edit_shared_resource(user_file, self.request.user): + if not can_edit_user_file(user_file, self.request.user): raise PermissionDenied('You do not have permission to modify this dataset.') shared_institution_ids = user_file.institutions.values_list('pk', flat=True) @@ -220,7 +217,7 @@ def post(request): ) user_file = get_object_or_404(UserFile, pk=user_file_id) - if not can_edit_shared_resource(user_file, request.user): + if not can_edit_user_file(user_file, request.user): raise PermissionDenied('You do not have permission to modify this dataset.') institution = get_object_or_404( @@ -244,7 +241,7 @@ def post(request): user_file_id = request.data.get('userFileId') user_file = get_object_or_404(UserFile, pk=user_file_id) - if not can_edit_shared_resource(user_file, request.user): + if not can_edit_user_file(user_file, request.user): raise PermissionDenied('You do not have permission to modify this dataset.') institution = get_object_or_404(Institution, pk=institution_id) @@ -293,7 +290,7 @@ def post(request): user_file_id = data.get('userFileId') user_file: UserFile = get_object_or_404(UserFile, id=user_file_id) - if user_file.user.id != request.user.id: + if not can_edit_user_file(user_file, request.user): raise PermissionDenied('You do not have permission to modify this user file.') user_file.is_public = not user_file.is_public From 251792f13b0046a690f201fc90168efdf2c01515 Mon Sep 17 00:00:00 2001 From: Juan Nicolas Herrera <54152074+juanNH@users.noreply.github.com> Date: Tue, 15 Sep 2026 13:30:28 -0300 Subject: [PATCH 2/2] Address dataset review comments --- .../components/files-manager/FilesManager.tsx | 258 +++++++++--------- src/user_files/views.py | 15 +- 2 files changed, 139 insertions(+), 134 deletions(-) diff --git a/src/frontend/static/frontend/src/components/files-manager/FilesManager.tsx b/src/frontend/static/frontend/src/components/files-manager/FilesManager.tsx index 0a928feb..c03c5604 100644 --- a/src/frontend/static/frontend/src/components/files-manager/FilesManager.tsx +++ b/src/frontend/static/frontend/src/components/files-manager/FilesManager.tsx @@ -836,146 +836,150 @@ class FilesManager extends React.Component searchPlaceholder={intl.formatMessage({ id: 'files.manager.search.placeholder' })} urlToRetrieveData={urlUserFilesCRUD} updateWSKey='update_user_files' - mapFunction={(userFileRow: DjangoUserFile) => ( - - - - {getFileTypeName(userFileRow.file_type)} - - - {(this.state.sharedInstitutionsByFile[userFileRow.id as number] ?? userFileRow.institutions).length > 0 - ? ( - i.name).join(', ') } - )} - /> - ) - : '-'} - - - {userFileRow.is_owner - ? ( - } - tagType={TagType.FILE} - onTagSelect={(tagId) => this.updateUserFileTag(userFileRow, tagId)} - onTagCreated={() => this.getUserTags()} - onTagEdited={() => this.getUserTags()} - onTagDeleted={(deletedTagId) => { - if (this.state.newFile.newTag === deletedTagId) { - this.handleAddFileInputsChange('newTag', null) - } + mapFunction={(userFileRow: DjangoUserFile) => { + const sharedInstitutions = this.state.sharedInstitutionsByFile[userFileRow.id as number] ?? userFileRow.institutions - this.getUserTags() - }} - /> - ) - : } - - - { - userFileRow.is_public - ? ( - - ) - : ( - - ) - } - - - {/* Extra information: */} - - {userFileRow.is_owner && ( + return ( + + + + {getFileTypeName(userFileRow.file_type)} + + + {sharedInstitutions.length > 0 + ? ( + i.name).join(', ') } + )} + /> + ) + : '-'} + + + {userFileRow.is_owner + ? ( + } + tagType={TagType.FILE} + onTagSelect={(tagId) => this.updateUserFileTag(userFileRow, tagId)} + onTagCreated={() => this.getUserTags()} + onTagEdited={() => this.getUserTags()} + onTagDeleted={(deletedTagId) => { + if (this.state.newFile.newTag === deletedTagId) { + this.handleAddFileInputsChange('newTag', null) + } + + this.getUserTags() + }} + /> + ) + : } + + + { + userFileRow.is_public + ? ( + + ) + : ( + + ) + } + + + {/* Extra information: */} this.openShareInstitutions(userFileRow)} + title={intl.formatMessage( + { id: 'files.manager.tooltip.indexColumn' }, + { columnName: userFileRow.column_used_as_index } + )} /> - )} - {/* Only the dataset owner can modify or delete it. */} - {userFileRow.is_owner && ( - <> - {/* Shows a edit button if specified */} + {userFileRow.is_owner && ( this.editFile(userFileRow)} + color='blue' + title={intl.formatMessage({ id: 'files.manager.sharedInstitutions.action' })} + onClick={() => this.openShareInstitutions(userFileRow)} /> - - )} - - - {/* Shows a download button if specified */} + )} + {/* Only the dataset owner can modify or delete it. */} + {userFileRow.is_owner && ( + <> + {/* Shows a edit button if specified */} window.open(`${downloadFileURL}${userFileRow.id}`, '_blank')} + color='yellow' + title={intl.formatMessage({ id: 'common.edit' })} + onClick={() => this.editFile(userFileRow)} /> - {/* Public switch */} - - {/* Shows a delete button if specified */} - {!userFileRow.is_public && ( - this.confirmFileDeletion(userFileRow)} - ownerId={userFileRow.user.id} - /> - )} - + )} - /> - {/* NaNs warning */} - {userFileRow.contains_nan_values && ( - + {/* Shows a download button if specified */} + window.open(`${downloadFileURL}${userFileRow.id}`, '_blank')} + /> + {/* Public switch */} + + {/* Shows a delete button if specified */} + {!userFileRow.is_public && ( + this.confirmFileDeletion(userFileRow)} + ownerId={userFileRow.user.id} + /> + )} + + )} /> - )} - - - )} + + {/* NaNs warning */} + {userFileRow.contains_nan_values && ( + + )} + + + ) + }} /> diff --git a/src/user_files/views.py b/src/user_files/views.py index f4b95073..4af3fb80 100644 --- a/src/user_files/views.py +++ b/src/user_files/views.py @@ -87,20 +87,21 @@ def get_user_files(user: AbstractBaseUser, public_only: bool, private_only: bool Returns the User's files objects from DB @param user: User to retrieve his Datasets @param public_only: If True retrieves only the public Datasets - @param private_only: If True retrieves only the uploaded Datasets by the User, otherwise, returns all his datasets - and the Datasets of the Institutions the user belongs to. If public_only is set to True, this parameter is ignored + @param private_only: If True retrieves only the uploaded Datasets by the User, otherwise, returns all public + datasets and the Datasets of the Institutions the user belongs to. If public_only is set to True, this parameter + is ignored @param with_survival_only: If True retrieves only the clinical Datasets which have at least on survival column tuple @return: QuerySet of UserFile objects """ if public_only: # Gets public Datasets filter_condition = Q(is_public=True) - else: - # Gets UserFiles which belongs to current user + elif private_only: + # Gets UserFiles which belong to the current user filter_condition = Q(user=user) - if not private_only: - # Gets public datasets too - filter_condition = user_file_visibility_q(user) + else: + # Gets public and institution datasets too + filter_condition = user_file_visibility_q(user) # Filters by survival data user_files_objects = UserFile.objects