diff --git a/admin/files/tasks.py b/admin/files/tasks.py new file mode 100644 index 00000000000..5ee35bb545a --- /dev/null +++ b/admin/files/tasks.py @@ -0,0 +1,31 @@ +import logging + +from django.apps import apps +from django.conf import settings +from django.db import transaction + +from framework.celery_tasks import app + +logger = logging.getLogger(__name__) + + +@app.task(max_retries=5, default_retry_delay=60) +def purge_file_version_task(version_pk): + + from google.cloud.storage.client import Client + from google.oauth2.service_account import Credentials + + FileVersion = apps.get_model('osf.FileVersion') + with transaction.atomic(): + version = FileVersion.objects.filter(pk=version_pk).first() + if not version or version.purged: + return 0 + + creds_path = getattr(settings, 'GCS_CREDS', None) + if not creds_path: + logger.error(f'GCS_CREDS not configured; cannot purge FileVersion {version_pk}') + return 0 + + creds = Credentials.from_service_account_file(creds_path) + client = Client(credentials=creds) + return version._purge(client=client) diff --git a/admin/files/urls.py b/admin/files/urls.py index 1a4a90dadda..5c3d4fd48cc 100644 --- a/admin/files/urls.py +++ b/admin/files/urls.py @@ -5,5 +5,7 @@ urlpatterns = [ re_path(r'^$', views.FileSearchView.as_view(), name='search'), - re_path(r'^(?P\w+)/$', views.FileView.as_view(), name='file') + re_path(r'^(?P\w+)/$', views.FileView.as_view(), name='file'), + re_path(r'^(?P\w+)/delete/$', views.FileDeleteView.as_view(), name='file-delete'), + re_path(r'^(?P\w+)/versions/(?P[\w-]+)/delete/$', views.FileVersionDeleteView.as_view(), name='file-version-delete'), ] diff --git a/admin/files/views.py b/admin/files/views.py index 772d0544245..414f9804c81 100644 --- a/admin/files/views.py +++ b/admin/files/views.py @@ -1,12 +1,24 @@ -from django.urls import NoReverseMatch from django.contrib import messages +from django.db import transaction +from django.db.models import F from django.shortcuts import redirect -from django.views.generic import FormView -from django.urls import reverse_lazy -from admin.base.forms import GuidForm +from django.urls import NoReverseMatch, reverse, reverse_lazy +from django.utils import timezone +from django.views.generic import FormView, View from django.contrib.auth.mixins import PermissionRequiredMixin + +from admin.base.forms import GuidForm from admin.base.views import GuidView -from osf.models import Guid, GuidMetadataRecord, BaseFileNode +from admin.files.tasks import purge_file_version_task +from framework.postcommit_tasks.handlers import enqueue_postcommit_task +from osf.models import Guid, GuidMetadataRecord, BaseFileNode, NodeLog +from osf.models.admin_log_entry import ( + update_admin_log, + FILE_REMOVED, + FILE_VERSION_REMOVED, +) +from osf.models.files import File, FileVersion, TrashedFile, TrashedFolder +from website.files.exceptions import FileNodeCheckedOutError, FileNodeIsPrimaryFile class FileSearchView(PermissionRequiredMixin, FormView): @@ -52,12 +64,120 @@ def get_context_data(self, **kwargs): ) file = context['object'] node = file.target - latest_modified = file.versions.latest('created') + is_trashed = isinstance(file, (TrashedFile, TrashedFolder)) + # Annotate version_id because django templates prohibit accessing attributes that start with underscores + versions = file.versions.all().order_by('-created').annotate(version_id=F('_id')) if isinstance(file, File) else FileVersion.objects.none() + + selected_version_id = self.request.GET.get('version') + selected_version = versions.filter(version_id=selected_version_id).first() if selected_version_id else None + if selected_version is None: + selected_version = versions.first() + context.update({ 'guid': guid, 'node_id': node._id if node else None, 'node': node, 'file_metadata': metadata_record, - 'version': latest_modified.location.get('version', '') + 'version': selected_version.location.get('version', '') if selected_version else '', + 'versions': versions, + 'selected_version': selected_version, + 'is_trashed': is_trashed, }) return context + + +class FileDeleteView(FileMixin, View): + """ Allows authorized users to delete a file or folder (soft delete / trash). + """ + permission_required = 'osf.delete_basefilenode' + raise_exception = True + + def post(self, request, *args, **kwargs): + file = self.get_object() + if isinstance(file, (TrashedFile, TrashedFolder)): + messages.error(request, 'This file has already been deleted.') + return redirect(self.get_success_url()) + + node = file.target + file_path = getattr(file, 'materialized_path', None) or getattr(file, 'path', None) or '' + guid = self.kwargs['guid'] + + try: + with transaction.atomic(): + file.delete(user=request.user) + if node is not None and hasattr(node, 'add_log'): + params = dict(getattr(node, 'log_params', {})) + params.update({ + 'pathType': 'file', + 'path': file_path, + }) + node.add_log( + action=NodeLog.FILE_REMOVED, + auth=None, + foreign_user=NodeLog.SUPPORT_USER_LABEL, + params=params, + log_date=timezone.now(), + should_hide=False, + ) + except FileNodeCheckedOutError: + messages.error(request, 'This file is checked out and cannot be deleted until it is checked in.') + return redirect(self.get_success_url()) + except FileNodeIsPrimaryFile: + messages.error(request, 'This file is the primary file of a preprint and cannot be deleted.') + return redirect(self.get_success_url()) + + update_admin_log( + user_id=request.user.id, + object_id=guid, + object_repr='BaseFileNode', + message=f'File {guid} deleted by admin.', + action_flag=FILE_REMOVED, + ) + messages.success(request, 'File deleted.') + return redirect(reverse('home')) + + +class FileVersionDeleteView(FileMixin, View): + """ Allows authorized users to delete a single version of a file, unlinking + it from the file and enqueueing a task to purge the underlying storage blob. + """ + permission_required = 'osf.delete_fileversion' + raise_exception = True + + def post(self, request, *args, **kwargs): + file = self.get_object() + guid = self.kwargs['guid'] + + if not isinstance(file, File): + messages.error(request, 'Only individual files have versions.') + return redirect(self.get_success_url()) + + version_id = self.kwargs.get('version_id') + version = FileVersion.load(version_id) + if version is None: + messages.error(request, 'Version not found.') + return redirect(self.get_success_url()) + + through = version.get_basefilenode_version(file) + if through is None: + messages.error(request, 'This version does not belong to this file.') + return redirect(self.get_success_url()) + + if file.versions.count() <= 1: + messages.error(request, 'Cannot delete the only version of a file. Delete the whole file instead.') + return redirect(self.get_success_url()) + + with transaction.atomic(): + through.delete() + + enqueue_postcommit_task(purge_file_version_task, (version.pk,), {}, celery=True) + + update_admin_log( + user_id=request.user.id, + object_id=guid, + object_repr='FileVersion', + message=f'Version {version_id} of file {guid} unlinked by admin; GCS purge enqueued.', + action_flag=FILE_VERSION_REMOVED, + ) + messages.success(request, 'File version deleted.') + return redirect(self.get_success_url()) diff --git a/admin/nodes/views.py b/admin/nodes/views.py index 2adec026b21..05d96259e25 100644 --- a/admin/nodes/views.py +++ b/admin/nodes/views.py @@ -148,6 +148,8 @@ def get_context_data(self, **kwargs): children = AbstractNode.objects.filter( id__in=[child.id for child in children] ).prefetch_related('guids').annotate(guid=F('guids___id')) + node_files = node.files.filter(deleted__isnull=True).prefetch_related('guids').annotate( + guid=F('guids___id')).order_by('name')[:200] context.update({ 'SPAM_STATUS': SpamStatus, 'STORAGE_LIMITS': settings.StorageLimits, @@ -156,6 +158,7 @@ def get_context_data(self, **kwargs): 'annotated_contributors': node.contributor_set.prefetch_related('user__guids').annotate( guid=F('user__guids___id')), 'children': children, + 'node_files': node_files, 'permissions': API_CONTRIBUTOR_PERMISSIONS, 'has_update_permission': self.request.user.has_perm('osf.change_node'), }) diff --git a/admin/templates/files/file.html b/admin/templates/files/file.html index 3c16db23443..4b1408e8f0d 100644 --- a/admin/templates/files/file.html +++ b/admin/templates/files/file.html @@ -30,7 +30,64 @@

File: {{ object.name }}

Version - {{ version }} + + {% if versions %} +
+ +
+ {% if selected_version %} +

+ {{ version }} · uploaded {{ selected_version.created | date:'SHORT_DATETIME_FORMAT' }} + by {{ selected_version.creator }} · {{ selected_version.size }} bytes +

+ {% endif %} + {% if perms.osf.delete_fileversion and not is_trashed %} + {% if versions|length > 1 and selected_version %} + + Delete Version + + + {% else %} + + {% endif %} + {% endif %} + {% else %} + {{ version }} + {% endif %} + {% if node_id %} @@ -67,6 +124,37 @@

File: {{ object.name }}

+ + {% if perms.osf.delete_basefilenode and not is_trashed %} + + Delete File + + + {% elif is_trashed %} +

This file has already been deleted.

+ {% endif %} {% endblock content %} diff --git a/admin/templates/nodes/file_list.html b/admin/templates/nodes/file_list.html new file mode 100644 index 00000000000..1fc3d9a38d9 --- /dev/null +++ b/admin/templates/nodes/file_list.html @@ -0,0 +1,35 @@ + + +

Files

+ {% if node_files %} + + + + + + + + + + + {% for file in node_files %} + + + + + + + {% endfor %} + +
NameProviderGuidActions
{{ file.name }}{{ file.provider }}{{ file.guid }} + {% if file.guid %} + + Manage + + {% endif %} +
+ {% else %} +

No files found.

+ {% endif %} + + diff --git a/admin/templates/nodes/node.html b/admin/templates/nodes/node.html index 68e451a59d4..57af20e8215 100644 --- a/admin/templates/nodes/node.html +++ b/admin/templates/nodes/node.html @@ -140,6 +140,7 @@

{{ node.type|cut:'osf.'|title }}: {{ node.title }} diff --git a/admin_tests/files/__init__.py b/admin_tests/files/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/admin_tests/files/test_tasks.py b/admin_tests/files/test_tasks.py new file mode 100644 index 00000000000..e56394e4c5e --- /dev/null +++ b/admin_tests/files/test_tasks.py @@ -0,0 +1,51 @@ +from unittest import mock + +from django.test import override_settings +from django.utils import timezone + +from admin.files.tasks import purge_file_version_task +from api_tests.utils import create_test_file +from osf.models.files import BaseFileVersionsThrough +from tests.base import AdminTestCase +from osf_tests.factories import AuthUserFactory, ProjectFactory + + +class TestPurgeFileVersionTask(AdminTestCase): + def setUp(self): + super().setUp() + self.node = ProjectFactory() + self.user = AuthUserFactory() + self.test_file = create_test_file(self.node, self.user) + self.version = self.test_file.versions.first() + BaseFileVersionsThrough.objects.filter( + basefilenode=self.test_file, fileversion=self.version + ).delete() + + @override_settings(GCS_CREDS=None) + def test_no_gcs_creds_configured_is_noop(self): + with mock.patch.object(type(self.version), '_purge') as mock_purge: + result = purge_file_version_task(self.version.pk) + mock_purge.assert_not_called() + assert result == 0 + + @override_settings(GCS_CREDS='/fake/path/to/creds.json') + def test_purges_with_gcs_client(self): + fake_client = mock.Mock() + with mock.patch('google.oauth2.service_account.Credentials.from_service_account_file') as mock_creds, \ + mock.patch('google.cloud.storage.client.Client', return_value=fake_client), \ + mock.patch.object(type(self.version), '_purge', return_value=1337) as mock_purge: + result = purge_file_version_task(self.version.pk) + + mock_creds.assert_called_once_with('/fake/path/to/creds.json') + mock_purge.assert_called_once_with(client=fake_client) + assert result == 1337 + + @override_settings(GCS_CREDS='/fake/path/to/creds.json') + def test_already_purged_version_is_noop(self): + self.version.purged = timezone.now() + self.version.save() + + with mock.patch.object(type(self.version), '_purge') as mock_purge: + result = purge_file_version_task(self.version.pk) + mock_purge.assert_not_called() + assert result == 0 diff --git a/admin_tests/files/test_views.py b/admin_tests/files/test_views.py new file mode 100644 index 00000000000..6c8ff1e0709 --- /dev/null +++ b/admin_tests/files/test_views.py @@ -0,0 +1,247 @@ +import pytest +from unittest import mock + +from django.core.exceptions import PermissionDenied +from django.contrib.auth.models import Permission +from django.contrib.contenttypes.models import ContentType +from django.test import RequestFactory +from django.urls import reverse + +from addons.osfstorage import settings as osfstorage_settings +from admin.files.views import FileDeleteView, FileVersionDeleteView, FileView +from admin_tests.utilities import setup_log_view +from api_tests.utils import create_test_file +from osf.models import AdminLogEntry, BaseFileNode, FileVersion, NodeLog +from osf.models.files import BaseFileVersionsThrough, TrashedFile +from tests.base import AdminTestCase +from osf_tests.factories import AuthUserFactory, PreprintFactory, ProjectFactory + + +def patch_messages(request): + from django.contrib.messages.storage.fallback import FallbackStorage + setattr(request, 'session', 'session') + messages = FallbackStorage(request) + setattr(request, '_messages', messages) + + +def add_permission(user, codename, model=BaseFileNode): + permission = Permission.objects.filter( + codename=codename, + content_type_id=ContentType.objects.get_for_model(model).id + ).first() + user.user_permissions.add(permission) + user.save() + + +def add_second_version(test_file, user): + return test_file.create_version(user, { + 'object': 'second-object-id', + 'service': 'cloud', + 'bucket': 'us-bucket', + osfstorage_settings.WATERBUTLER_RESOURCE: 'osf', + }, { + 'size': 42, + 'contentType': 'img/png', + }) + + +class TestFileDeleteView(AdminTestCase): + def setUp(self): + super().setUp() + self.node = ProjectFactory() + self.admin_user = AuthUserFactory() + self.test_file = create_test_file(self.node, self.admin_user) + self.guid = self.test_file.get_guid(create=True)._id + self.request = RequestFactory().post('/fake_path') + self.request.user = self.admin_user + patch_messages(self.request) + self.plain_view = FileDeleteView + self.view = setup_log_view(self.plain_view(), self.request, guid=self.guid) + self.url = reverse('files:file-delete', kwargs={'guid': self.guid}) + + def test_delete_file(self): + count = AdminLogEntry.objects.count() + response = self.view.post(self.request) + # `delete()` recasts the row to TrashedFile (TypedModels STI), so re-fetch + # via the base manager instead of refresh_from_db() on the original typed instance. + deleted_file = BaseFileNode.objects.get(id=self.test_file.id) + assert isinstance(deleted_file, TrashedFile) + assert AdminLogEntry.objects.count() == count + 1 + assert response.url == reverse('home') + log = self.node.logs.filter( + action=NodeLog.FILE_REMOVED, + foreign_user=NodeLog.SUPPORT_USER_LABEL, + ).latest('date') + assert log.user is None + assert log.foreign_user == NodeLog.SUPPORT_USER_LABEL + + def test_delete_already_trashed_file_is_noop(self): + self.test_file.delete(user=self.admin_user) + count = AdminLogEntry.objects.count() + response = self.view.post(self.request) + assert AdminLogEntry.objects.count() == count + assert response.url == reverse('files:file', kwargs={'guid': self.guid}) + + def test_delete_checked_out_file_blocked(self): + self.test_file.checkout = self.admin_user + self.test_file.save() + count = AdminLogEntry.objects.count() + response = self.view.post(self.request) + self.test_file.refresh_from_db() + assert not isinstance(self.test_file, TrashedFile) + assert AdminLogEntry.objects.count() == count + assert response.url == reverse('files:file', kwargs={'guid': self.guid}) + + def test_delete_preprint_primary_file_blocked(self): + preprint = PreprintFactory() + primary_file = preprint.primary_file + primary_file_guid = primary_file.get_guid(create=True)._id + view = setup_log_view(self.plain_view(), self.request, guid=primary_file_guid) + count = AdminLogEntry.objects.count() + response = view.post(self.request) + primary_file.refresh_from_db() + assert not isinstance(primary_file, TrashedFile) + assert AdminLogEntry.objects.count() == count + assert response.url == reverse('files:file', kwargs={'guid': primary_file_guid}) + + def test_no_user_permissions_raises_error(self): + user = AuthUserFactory() + request = RequestFactory().post(self.url) + request.user = user + + with pytest.raises(PermissionDenied): + self.plain_view.as_view()(request, guid=self.guid) + + def test_correct_view_permissions(self): + user = AuthUserFactory() + add_permission(user, 'delete_basefilenode') + add_permission(user, 'view_basefilenode') + + request = RequestFactory().post(self.url) + patch_messages(request) + request.user = user + + response = self.plain_view.as_view()(request, guid=self.guid) + assert response.status_code == 302 + + +class TestFileVersionDeleteView(AdminTestCase): + def setUp(self): + super().setUp() + self.node = ProjectFactory() + self.admin_user = AuthUserFactory() + self.test_file = create_test_file(self.node, self.admin_user) + self.guid = self.test_file.get_guid(create=True)._id + self.first_version = self.test_file.versions.first() + self.request = RequestFactory().post('/fake_path') + self.request.user = self.admin_user + patch_messages(self.request) + self.plain_view = FileVersionDeleteView + + def _make_view(self, version_id): + return setup_log_view( + self.plain_view(), self.request, + guid=self.guid, version_id=version_id, + ) + + def test_delete_version_unlinks_and_enqueues_purge(self): + second_version = add_second_version(self.test_file, self.admin_user) + view = self._make_view(self.first_version._id) + count = AdminLogEntry.objects.count() + + with mock.patch('admin.files.views.enqueue_postcommit_task') as mock_enqueue: + view.post(self.request) + + assert not BaseFileVersionsThrough.objects.filter( + basefilenode=self.test_file, fileversion=self.first_version + ).exists() + assert BaseFileVersionsThrough.objects.filter( + basefilenode=self.test_file, fileversion=second_version + ).exists() + mock_enqueue.assert_called_once() + assert mock_enqueue.call_args[0][1] == (self.first_version.pk,) + assert AdminLogEntry.objects.count() == count + 1 + + def test_cannot_delete_sole_version(self): + view = self._make_view(self.first_version._id) + count = AdminLogEntry.objects.count() + + with mock.patch('admin.files.views.enqueue_postcommit_task') as mock_enqueue: + view.post(self.request) + + assert BaseFileVersionsThrough.objects.filter( + basefilenode=self.test_file, fileversion=self.first_version + ).exists() + mock_enqueue.assert_not_called() + assert AdminLogEntry.objects.count() == count + + def test_version_belonging_to_different_file_blocked(self): + other_file = create_test_file(self.node, self.admin_user, filename='other_file') + add_second_version(self.test_file, self.admin_user) + view = self._make_view(other_file.versions.first()._id) + count = AdminLogEntry.objects.count() + + with mock.patch('admin.files.views.enqueue_postcommit_task') as mock_enqueue: + view.post(self.request) + + mock_enqueue.assert_not_called() + assert AdminLogEntry.objects.count() == count + + def test_no_user_permissions_raises_error(self): + add_second_version(self.test_file, self.admin_user) + user = AuthUserFactory() + url = reverse('files:file-version-delete', kwargs={ + 'guid': self.guid, 'version_id': self.first_version._id, + }) + request = RequestFactory().post(url) + request.user = user + + with pytest.raises(PermissionDenied): + self.plain_view.as_view()(request, guid=self.guid, version_id=self.first_version._id) + + def test_correct_view_permissions(self): + add_second_version(self.test_file, self.admin_user) + user = AuthUserFactory() + add_permission(user, 'delete_fileversion', model=FileVersion) + add_permission(user, 'view_basefilenode') + + url = reverse('files:file-version-delete', kwargs={ + 'guid': self.guid, 'version_id': self.first_version._id, + }) + request = RequestFactory().post(url) + patch_messages(request) + request.user = user + + response = self.plain_view.as_view()(request, guid=self.guid, version_id=self.first_version._id) + assert response.status_code == 302 + + +class TestFileViewVersionSelection(AdminTestCase): + def setUp(self): + super().setUp() + self.node = ProjectFactory() + self.user = AuthUserFactory() + add_permission(self.user, 'view_basefilenode') + self.test_file = create_test_file(self.node, self.user) + self.guid = self.test_file.get_guid(create=True)._id + self.first_version = self.test_file.versions.first() + self.second_version = add_second_version(self.test_file, self.user) + + def _get_context(self, query_string=''): + url = reverse('files:file', kwargs={'guid': self.guid}) + query_string + request = RequestFactory().get(url) + request.user = self.user + response = FileView.as_view()(request, guid=self.guid) + return response.context_data + + def test_defaults_to_latest_version(self): + context = self._get_context() + assert context['selected_version'].pk == self.second_version.pk + + def test_selecting_specific_version_via_query_param(self): + context = self._get_context(f'?version={self.first_version._id}') + assert context['selected_version'].pk == self.first_version.pk + + def test_invalid_version_falls_back_to_latest(self): + context = self._get_context('?version=does-not-exist') + assert context['selected_version'].pk == self.second_version.pk diff --git a/osf/migrations/__init__.py b/osf/migrations/__init__.py index 95bfd49a76d..d9c8c0b3ed0 100644 --- a/osf/migrations/__init__.py +++ b/osf/migrations/__init__.py @@ -76,6 +76,8 @@ def get_admin_write_permissions(): 'delete_brand', 'change_node', 'delete_node', + 'delete_basefilenode', + 'delete_fileversion', 'change_user', 'change_conference', 'mark_spam', diff --git a/osf/models/admin_log_entry.py b/osf/models/admin_log_entry.py index f6cd288cc43..ac8ec81056d 100644 --- a/osf/models/admin_log_entry.py +++ b/osf/models/admin_log_entry.py @@ -40,6 +40,9 @@ MANUAL_ARCHIVE_RESTART = 90 +FILE_REMOVED = 100 +FILE_VERSION_REMOVED = 101 + def update_admin_log(user_id, object_id, object_repr, message, action_flag=UNKNOWN): AdminLogEntry.objects.log_action( user_id=user_id,