diff --git a/doc/permissions.rst b/doc/permissions.rst index 1a8c9a25263e43fb5101e6023c3b9f3b800d66af..cdc31e362760f75c816e2388ee706fbfdff57b6f 100644 --- a/doc/permissions.rst +++ b/doc/permissions.rst @@ -74,14 +74,17 @@ Curator A curator can perform changes on **public** data sources even if he/she is not an editor of the data source. The mission of a curator is to keep naming between viruses, hosts coherent, but also edit data source details when needed. - * Edit the list of :term:`viruses <virus>` and :term:`hosts <host>` + * Edit the :term:`viruses <virus>` and :term:`hosts <host>` * Edit the information of a :term:`data source` (except the the list of viewer/editor) + * Edit the mapping Although, an curator cannot: - * See the responses, and thus cannot edit them either - * Edit the mapping + * Edit the responses + * Delete :term:`viruses <virus>` or :term:`hosts <host>` * See the list of viewer/editor, and thus cannot edit it either + * Restore the data source to a previous version + * Download a previous version * Delete the data source * Transfer ownership to an other user diff --git a/src/viralhostrange/viralhostrangedb/static/css/base.css b/src/viralhostrange/viralhostrangedb/static/css/base.css index 8e8a743c88e7fb0bbc15ce35071755a79cfaf7f1..4f0615772b1f124277a700356e694b2f8069b511 100644 --- a/src/viralhostrange/viralhostrangedb/static/css/base.css +++ b/src/viralhostrange/viralhostrangedb/static/css/base.css @@ -170,4 +170,7 @@ a[data-toggle=collapse].card-header { } a[data-toggle=collapse].card-header>:not(i) { color: #212529; +} +.not-allowed { + cursor: not-allowed !important; } \ No newline at end of file diff --git a/src/viralhostrange/viralhostrangedb/templates/viralhostrangedb/datasource_detail.html b/src/viralhostrange/viralhostrangedb/templates/viralhostrangedb/datasource_detail.html index b07d447b8d41979abd7f86a0ada969c21d333e29..b6ec080bacb55b7ffaf4d3d9f6673b22fce244fc 100644 --- a/src/viralhostrange/viralhostrangedb/templates/viralhostrangedb/datasource_detail.html +++ b/src/viralhostrange/viralhostrangedb/templates/viralhostrangedb/datasource_detail.html @@ -150,19 +150,11 @@ </tbody> </table> <div class="btn-group d-flex" role="group" aria-label="Get content"> - {% if request|can_edit:object %} - <a href="{% url 'viralhostrangedb:data-source-data-update' pk=object.pk%}" role="button" - class="btn btn-outline-primary"><i class="fa fa-upload"></i> {%trans "Update content" %}</a> - {%endif%} {% if has_responses %} <a href="{% url 'viralhostrangedb:data-source-download' pk=object.pk%}" role="button" class="float-right btn btn-outline-primary"> <i class="fa fa-download"></i> {%trans "Download it" %} </a> - <a href="http://hub.pages.pasteur.fr/viralhostrangedb/compatible_file.html" target="_blank" style="flex-grow:0" - class="float-right btn btn-outline-primary"> - <i class="fa fa-question-circle"></i> - </a> {%else%} <a href="{% url 'viralhostrangedb:data-source-download' pk=object.pk%}" role="button" class="float-right btn btn-primary"> @@ -173,6 +165,14 @@ <i class="fa fa-question-circle"></i> {%trans "How to fill it" %} </a> {%endif%} + {% if request|is_editor_or_owner_of_ds:object %} + <a href="{% url 'viralhostrangedb:data-source-data-update' pk=object.pk%}" role="button" + class="btn btn-outline-primary"><i class="fa fa-upload"></i> {%trans "Update content" %}</a> + <a href="http://hub.pages.pasteur.fr/viralhostrangedb/compatible_file.html" target="_blank" style="flex-grow:0" + class="float-right btn btn-outline-primary"> + <i class="fa fa-question-circle"></i> + </a> + {%endif%} </div> </div> @@ -221,7 +221,7 @@ <div class="card"> <div class="card-header"> {%trans "Viruses"%} - {% if request|can_edit:object %} + {% if request|is_editor_or_owner_of_ds:object %} <a class="btn btn-xs btn-outline-secondary float-right" href="{% url 'viralhostrangedb:data-source-virus-delete' pk=object.pk%}" role="button" @@ -229,6 +229,8 @@ > <i class="fa fa-trash"></i> {%trans "Delete ..." %} </a> + {%endif%} + {% if request|can_edit:object %} <a class="btn btn-xs btn-outline-secondary float-right" href="{% url 'viralhostrangedb:data-source-virus-update' pk=object.pk%}" role="button" @@ -249,7 +251,7 @@ <div class="card"> <div class="card-header"> {%trans "Hosts"%} - {% if request|can_edit:object %} + {% if request|is_editor_or_owner_of_ds:object %} <a class="btn btn-xs btn-outline-secondary float-right" href="{% url 'viralhostrangedb:data-source-host-delete' pk=object.pk%}" role="button" @@ -257,6 +259,8 @@ > <i class="fa fa-trash"></i> {%trans "Delete ..." %} </a> + {%endif%} + {% if request|can_edit:object %} <a class="btn btn-xs btn-outline-secondary float-right" href="{% url 'viralhostrangedb:data-source-host-update' pk=object.pk%}" role="button" diff --git a/src/viralhostrange/viralhostrangedb/templates/viralhostrangedb/datasource_history.html b/src/viralhostrange/viralhostrangedb/templates/viralhostrangedb/datasource_history.html index 9124b45f04744d2b7e237a7d9a9d62634685d716..16b3464549e903741eea4a5d3f60de9c55f0b35a 100644 --- a/src/viralhostrange/viralhostrangedb/templates/viralhostrangedb/datasource_history.html +++ b/src/viralhostrange/viralhostrangedb/templates/viralhostrangedb/datasource_history.html @@ -29,20 +29,33 @@ <td>{{o.action_time}}</td> <td>{{o.user.last_name|upper}} {{o.user.first_name|title}}</td> <td>{{o|get_change_message_with_action}}</td> + {%with request|is_editor_or_owner_of_ds:o.object_id as can_restore %} <td class="text-center"> {% if o|should_have_backup_file %} {% if o|has_backup_file %} {% if forloop.first %} <i>{%trans 'Current version' %}</i> {% else %} + {%if can_restore or o.user_id is request.user.id%} <a href="{% url 'viralhostrangedb:data-source-history-download' pk=o.object_id log_pk=o.pk%}" class="btn btn-xs btn-outline-primary"> <i class="fa fa-download" aria-hidden="true"></i> {%trans 'Download saved data'%} </a> + {%else%} + <button disabled="disabled" class="btn btn-xs btn-outline-primary disabled not-allowed"> + <i class="fa fa-download" aria-hidden="true"></i> {%trans 'Download saved data'%} + </button> + {%endif%} + {%if can_restore%} <a href="{% url 'viralhostrangedb:data-source-history-restoration' pk=o.object_id log_pk=o.pk%}" class="btn btn-xs btn-outline-danger"> <i class="fa fa-undo" aria-hidden="true"></i> {%trans 'Restore data at this point' %} </a> + {%else%} + <button disabled="disabled" class="btn btn-xs btn-outline-danger disabled not-allowed"> + <i class="fa fa-undo" aria-hidden="true"></i> {%trans 'Restore data at this point'%} + </button> + {%endif%} {%endif%} {% else %} <i>{%trans 'Backup missing' %}</i> @@ -56,6 +69,7 @@ <i>{%trans 'No backup created' %}</i> {%endif%} </td> + {%endwith %} </tr> {%endfor%} </tbody> diff --git a/src/viralhostrange/viralhostrangedb/templatetags/viralhostrange_tags.py b/src/viralhostrange/viralhostrangedb/templatetags/viralhostrange_tags.py index 75c4a6a9267e72bcb3e75e18885e3c4a9306f72a..bbe97295287f1b9546f3c941206c4b633c447630 100644 --- a/src/viralhostrange/viralhostrangedb/templatetags/viralhostrange_tags.py +++ b/src/viralhostrange/viralhostrangedb/templatetags/viralhostrange_tags.py @@ -68,6 +68,28 @@ def can_edit(user, obj): return False +@register.filter +def is_editor_or_owner_of_ds(user, obj_pk): + # used to access datasource history, should follow get_log_entry_with_permission_check_or_404 + # if user is a WSGIRequest, get the attr user + if hasattr(obj_pk,'pk'): + obj_pk=obj_pk.pk + user = getattr(user, "user", user) + if not user.is_authenticated: + return False + try: + return mixins.only_editor_or_owned_queryset_filter( + self=None, + request=None, + queryset=models.DataSource.objects.filter(pk=obj_pk), + user=user, + ).exists() + except Exception as e: + if settings.DEBUG: + raise e + return False + + @register.filter def get_curators_list(_): return business_process.get_curators().order_by("last_name", "first_name") diff --git a/src/viralhostrange/viralhostrangedb/tests/base_test_case.py b/src/viralhostrange/viralhostrangedb/tests/base_test_case.py index 5adbfd6c5840bf8448829c9796a757785c6959b1..777817e2bfc829bd0fb0530e0063e6e051ea04c3 100644 --- a/src/viralhostrange/viralhostrangedb/tests/base_test_case.py +++ b/src/viralhostrange/viralhostrangedb/tests/base_test_case.py @@ -85,6 +85,15 @@ class OneDataSourceTestCase(TestCase): ) Group.objects.get(name="Admin").user_set.add(self.staff_admin) + ################################################################################ + self.u_curator = get_user_model().objects.create( + username="u_curator", + first_name="curatorF", + last_name="curatorL", + email="u_curator@a.b", + ) + business_process.set_curator(self.u_curator, True) + ################################################################################ self.user = get_user_model().objects.create( username="user", diff --git a/src/viralhostrange/viralhostrangedb/tests/test_changes_notifications.py b/src/viralhostrange/viralhostrangedb/tests/test_changes_notifications.py index c814fdf496b0fa4e52edb1dcf316f80f2d2c0104..e8928331fb58ac54362810c2c18b2b5294e24cf9 100644 --- a/src/viralhostrange/viralhostrangedb/tests/test_changes_notifications.py +++ b/src/viralhostrange/viralhostrangedb/tests/test_changes_notifications.py @@ -38,13 +38,13 @@ class DataSourceChangeNotificationTestCase(base_test_case.OneDataSourceTestCase) assert e.errno == errno.EEXIST, e super().setUp() ################################################################################ - self.u_curator = get_user_model().objects.create( - username="u_curator", - first_name="Dany", - last_name="u_curatorL", - email="u_curator@a.b", - ) - business_process.set_curator(self.u_curator, True) + # self.u_curator = get_user_model().objects.create( + # username="u_curator", + # first_name="Dany", + # last_name="u_curatorL", + # email="u_curator@a.b", + # ) + # business_process.set_curator(self.u_curator, True) self.ds.public = True self.ds.save() diff --git a/src/viralhostrange/viralhostrangedb/tests/test_history.py b/src/viralhostrange/viralhostrangedb/tests/test_history.py index 6f366bd3e122aa168c87c1c51ec0d223d14428e1..90e1cbfa73db68a10adc8caf0a89b4fd68492332 100644 --- a/src/viralhostrange/viralhostrangedb/tests/test_history.py +++ b/src/viralhostrange/viralhostrangedb/tests/test_history.py @@ -62,6 +62,20 @@ class DataSourceHistoryTestCase(base_test_case.OneDataSourceTestCase): self.assertEqual(self.client.get(url_restore).status_code, 200) self.assertEqual(self.client.post(url_restore, dict(agree="True")).status_code, 302) + self.client.force_login(self.u_curator) + self.ds.public = True + self.ds.save() + self.assertEqual(self.client.get(url_see).status_code, 200) + self.assertEqual(self.client.get(url_get).status_code, 404) + self.assertEqual(self.client.get(url_restore).status_code, 404) + self.assertEqual(self.client.post(url_restore, dict(agree="True")).status_code, 404) + self.ds.public = False + self.ds.save() + self.assertEqual(self.client.get(url_see).status_code, 404) + self.assertEqual(self.client.get(url_get).status_code, 404) + self.assertEqual(self.client.get(url_restore).status_code, 404) + self.assertEqual(self.client.post(url_restore, dict(agree="True")).status_code, 404) + self.client.force_login(self.user) self.assertEqual(self.client.get(url_see).status_code, 200) self.assertEqual(self.client.get(url_get).status_code, 200) diff --git a/src/viralhostrange/viralhostrangedb/tests/test_templatetags.py b/src/viralhostrange/viralhostrangedb/tests/test_templatetags.py index e0fc3b69da46a1197b5126f53f6235eff425f20e..723594245ca512d8b4aa9a383e4fb557fb80d0ba 100644 --- a/src/viralhostrange/viralhostrangedb/tests/test_templatetags.py +++ b/src/viralhostrange/viralhostrangedb/tests/test_templatetags.py @@ -4,7 +4,7 @@ from django.test import TestCase, override_settings from django.utils import timezone from viralhostrangedb import models, business_process -from viralhostrangedb.templatetags.viralhostrange_tags import can_edit, can_see, can_delete +from viralhostrangedb.templatetags.viralhostrange_tags import can_edit, can_see, can_delete, is_editor_or_owner_of_ds class TemplateTagsTests(TestCase): @@ -84,6 +84,21 @@ class TemplateTagsTests(TestCase): business_process.set_curator(self.user, True) self.assertTrue(can_edit(self.user, self.d_toto)) + @override_settings(DEBUG=False) + def test_is_editor_or_owner_of_ds(self): + self.assertFalse(is_editor_or_owner_of_ds(self.user, self.d_toto)) + self.assertTrue(is_editor_or_owner_of_ds(self.toto, self.d_toto)) + + self.assertTrue(is_editor_or_owner_of_ds(self.titi, self.d_toto_grant_titi)) + business_process.set_curator(self.titi, True) + self.assertTrue(is_editor_or_owner_of_ds(self.titi, self.d_toto_grant_titi), "should still be allowed") + + self.assertFalse(is_editor_or_owner_of_ds(self.user, self.d_toto_grant_titi)) + self.assertFalse(is_editor_or_owner_of_ds(self.user, self.d_toto_grant_titi.pk)) + business_process.set_curator(self.user, True) + self.assertFalse(is_editor_or_owner_of_ds(self.user, self.d_toto_grant_titi), "still not allowed") + self.assertFalse(is_editor_or_owner_of_ds(self.user, self.d_toto_grant_titi.pk), "still not allowed") + @override_settings(DEBUG=False) def test_can_delete(self): self.assertFalse(can_delete(self.user, self.d_toto)) diff --git a/src/viralhostrange/viralhostrangedb/views.py b/src/viralhostrange/viralhostrangedb/views.py index 125773ae1b9253d5bb6b8d9bf5ae8cefb4c8a5a6..29dcc24aa3991df9c01e14c29aed0e0e19af6461 100644 --- a/src/viralhostrange/viralhostrangedb/views.py +++ b/src/viralhostrange/viralhostrangedb/views.py @@ -421,7 +421,8 @@ def data_source_history_list(request, pk): ) -def get_log_entry_with_permission_check_or_404(request, pk, log_pk): +@login_required +def get_log_entry_with_permission_check_or_404(request, pk, log_pk, allow_curator=False, user_id=None): if request.user.is_superuser and not models.DataSource.objects.filter(pk=pk).exists(): # Allows superuser to access to back up of deleted data source after they have been deleted data_source = None @@ -430,15 +431,31 @@ def get_log_entry_with_permission_check_or_404(request, pk, log_pk): self=None, request=request, queryset=models.DataSource.objects, + allow_curator=allow_curator, ), pk=pk) + log_entries = LogEntry.objects.filter(object_id=pk, + content_type=ContentType.objects.get_for_model(models.DataSource)) + if user_id: + log_entries = log_entries.filter(user_id=user_id) return data_source, get_object_or_404( - LogEntry.objects.filter(object_id=pk, content_type=ContentType.objects.get_for_model(models.DataSource)), + log_entries, pk=log_pk, ) +@login_required def data_source_history_download(request, pk, log_pk): - data_source, le = get_log_entry_with_permission_check_or_404(request, pk, log_pk) + try: + data_source, le = get_log_entry_with_permission_check_or_404(request, pk, log_pk) + except Http404: + # if we cannot get it, then allows curators, but only on their own entry + data_source, le = get_log_entry_with_permission_check_or_404( + request, + pk, + log_pk, + allow_curator=True, + user_id=request.user.id, + ) file_path = business_process.get_backup_file_path(le, test_if_exists=True) if file_path is None: