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 858867f1..7feb0fb0 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 } from '../common/TissueLabels' +import { getTissueDropdownOptions, getTissueIds } from '../common/TissueLabels' import { SharedInstitutionsUserFile } from './SharedInstitutionsUserFile' /** Structure returned from the chunk upload service. */ @@ -31,9 +31,9 @@ declare const urlUserFilesCRUD: string declare const urlUserInstitutions: string declare const urlTissuesCRUD: string declare const urlChunkUpload: string -declare const urlChunkUploadComplete: string -declare const downloadFileURL: string -declare const downloadFileHeaders: string +declare const urlChunkUploadComplete: string +declare const downloadFileURL: string +declare const downloadFileHeaders: string /** * New File Form fields @@ -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)} - - - {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() - }} - /> - ) - : } - - - {(this.state.sharedInstitutionsByFile[userFileRow.id as number] ?? userFileRow.institutions).length > 0 - ? ( - i.name).join(', ') } - )} - /> - ) - : '-'} - - - { - userFileRow.is_public + mapFunction={(userFileRow: DjangoUserFile) => { + const sharedInstitutions = this.state.sharedInstitutionsByFile[userFileRow.id as number] ?? userFileRow.institutions + + 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() + }} /> ) - } - - - {/* Extra information: */} - - {userFileRow.is_owner && ( + : } + + + { + 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/frontend/static/frontend/src/utils/django_interfaces.ts b/src/frontend/static/frontend/src/utils/django_interfaces.ts index bb08cc87..2727d8d7 100644 --- a/src/frontend/static/frontend/src/utils/django_interfaces.ts +++ b/src/frontend/static/frontend/src/utils/django_interfaces.ts @@ -241,6 +241,8 @@ interface DjangoUserFile extends DjangoSimpleUserFile { upload_date?: string, institutions: DjangoInstitutionSimple[], is_owner: boolean, + /** @deprecated Use is_owner. Kept for compatibility with older API consumers. */ + is_private_or_institution_admin: boolean, number_of_rows: number, number_of_samples: number, contains_nan_values: boolean, diff --git a/src/user_files/serializers.py b/src/user_files/serializers.py index 1181b403..0f1daf58 100644 --- a/src/user_files/serializers.py +++ b/src/user_files/serializers.py @@ -82,9 +82,14 @@ def to_representation(self, instance): request = self.context.get('request') if request is not None: user = request.user - data['is_owner'] = instance.user == user + is_owner = instance.user == user + data['is_owner'] = is_owner + # Preserve the existing response field for clients that still + # consume it, but its value now represents ownership only. + data['is_private_or_institution_admin'] = is_owner else: data['is_owner'] = False + data['is_private_or_institution_admin'] = False return data def create(self, validated_data): diff --git a/src/user_files/tests/test_owner_permissions.py b/src/user_files/tests/test_owner_permissions.py index f62b6723..675d54ec 100644 --- a/src/user_files/tests/test_owner_permissions.py +++ b/src/user_files/tests/test_owner_permissions.py @@ -38,27 +38,15 @@ def setUp(self): self.user_file.institutions.add(self.institution) self.client = APIClient() - def test_is_owner_replaces_legacy_permission_flag(self): - """The API exposes ownership without the deprecated combined flag.""" - self.client.force_authenticate(user=self.institution_admin) - admin_response = self.client.get(f'/user-files/{self.user_file.pk}/') - - self.assertEqual(admin_response.status_code, 200) - self.assertFalse(admin_response.data['is_owner']) - self.assertNotIn('is_private_or_institution_admin', admin_response.data) - - self.client.force_authenticate(user=self.owner) - owner_response = self.client.get(f'/user-files/{self.user_file.pk}/') - - self.assertEqual(owner_response.status_code, 200) - self.assertTrue(owner_response.data['is_owner']) - self.assertNotIn('is_private_or_institution_admin', owner_response.data) - def test_institution_admin_can_read_but_cannot_update_or_delete(self): """Institution administrators retain read access but cannot mutate a dataset.""" self.client.force_authenticate(user=self.institution_admin) read_response = self.client.get(f'/user-files/{self.user_file.pk}/') + self.assertEqual(read_response.status_code, 200) + self.assertFalse(read_response.data['is_owner']) + self.assertFalse(read_response.data['is_private_or_institution_admin']) + update_response = self.client.patch( f'/user-files/{self.user_file.pk}/', data={ @@ -75,13 +63,43 @@ def test_institution_admin_can_read_but_cannot_update_or_delete(self): format='json', ) - self.assertEqual(read_response.status_code, 200) self.assertEqual(update_response.status_code, 404) self.assertEqual(delete_response.status_code, 404) self.assertEqual(toggle_response.status_code, 403) self.user_file.refresh_from_db() self.assertEqual(self.user_file.name, 'Shared dataset') + def test_institution_member_cannot_mutate_or_manage_sharing(self): + """Ordinary institution members cannot mutate or manage sharing either.""" + self.client.force_authenticate(user=self.institution_member) + + update_response = self.client.patch( + f'/user-files/{self.user_file.pk}/', + data={ + 'name': 'Changed by member', + 'file_type': FileType.MRNA.value, + 'is_cpg_site_id': False, + }, + format='json', + ) + remove_institution_response = self.client.post( + '/user-files/remove-institution', + data={ + 'userFileId': self.user_file.pk, + 'institutionId': self.institution.pk, + }, + format='json', + ) + toggle_response = self.client.post( + '/user-files/switch-file-public-view/', + data={'userFileId': self.user_file.pk}, + format='json', + ) + + self.assertEqual(update_response.status_code, 404) + self.assertEqual(remove_institution_response.status_code, 403) + self.assertEqual(toggle_response.status_code, 403) + def test_only_owner_can_update_delete_and_manage_sharing(self): """The owner can perform the complete dataset management workflow.""" extra_institution = Institution.objects.create(name='Owner institution') 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