diff --git a/readthedocs/core/admin.py b/readthedocs/core/admin.py index 3793a3207b5..6b6b533c332 100644 --- a/readthedocs/core/admin.py +++ b/readthedocs/core/admin.py @@ -67,6 +67,7 @@ def is_banned(self, obj): return hasattr(obj, 'profile') and obj.profile.banned is_banned.short_description = 'Banned' + is_banned.boolean = True def ban_user(self, request, queryset): users = [] diff --git a/readthedocs/projects/admin.py b/readthedocs/projects/admin.py index 284b81eefd9..c881189e63e 100644 --- a/readthedocs/projects/admin.py +++ b/readthedocs/projects/admin.py @@ -2,8 +2,13 @@ from __future__ import absolute_import from django.contrib import admin +from django.contrib import messages +from django.contrib.admin.actions import delete_selected +from django.utils.translation import ugettext_lazy as _ from guardian.admin import GuardedModelAdmin +from readthedocs.core.models import UserProfile +from readthedocs.core.utils import broadcast from readthedocs.builds.models import Version from readthedocs.redirects.models import Redirect from readthedocs.notifications.views import SendNotificationView @@ -11,6 +16,7 @@ from .notifications import ResourceUsageNotification from .models import (Project, ImportedFile, ProjectRelationship, EmailHook, WebHook, Domain) +from .tasks import remove_dir class ProjectSendNotificationView(SendNotificationView): @@ -65,6 +71,30 @@ class DomainInline(admin.TabularInline): # return instance.click_ratio * 100 +class ProjectOwnerBannedFilter(admin.SimpleListFilter): + + """Filter for projects with banned owners + + There are problems adding `users__profile__banned` to the `list_filter` + attribute, so we'll create a basic filter to capture banned owners. + """ + + title = 'project owner banned' + parameter_name = 'project_owner_banned' + + OWNER_BANNED = 'true' + + def lookups(self, request, model_admin): + return ( + (self.OWNER_BANNED, _('Yes')), + ) + + def queryset(self, request, queryset): + if self.value() == self.OWNER_BANNED: + return queryset.filter(users__profile__banned=True) + return queryset + + class ProjectAdmin(GuardedModelAdmin): """Project model admin view""" @@ -72,13 +102,14 @@ class ProjectAdmin(GuardedModelAdmin): prepopulated_fields = {'slug': ('name',)} list_display = ('name', 'repo', 'repo_type', 'allow_comments', 'featured', 'theme') list_filter = ('repo_type', 'allow_comments', 'featured', 'privacy_level', - 'documentation_type', 'programming_language') + 'documentation_type', 'programming_language', + ProjectOwnerBannedFilter) list_editable = ('featured',) search_fields = ('slug', 'repo') inlines = [ProjectRelationshipInline, RedirectInline, VersionInline, DomainInline] raw_id_fields = ('users', 'main_language_project') - actions = ['send_owner_email'] + actions = ['send_owner_email', 'ban_owner'] def send_owner_email(self, request, queryset): view = ProjectSendNotificationView.as_view( @@ -88,6 +119,51 @@ def send_owner_email(self, request, queryset): send_owner_email.short_description = 'Notify project owners' + def ban_owner(self, request, queryset): + """Ban project owner + + This will only ban single owners, because a malicious user could add a + user as a co-owner of the project. We don't want to induce and + collatoral damage when flagging users. + """ + total = 0 + for project in queryset: + if project.users.count() == 1: + count = (UserProfile.objects + .filter(user__projects=project) + .update(banned=True)) + total += count + else: + messages.add_message(request, messages.ERROR, + 'Project has multiple owners: {0}'.format(project)) + if total == 0: + messages.add_message(request, messages.ERROR, 'No users banned') + else: + messages.add_message(request, messages.INFO, + 'Banned {0} user(s)'.format(total)) + + ban_owner.short_description = 'Ban project owner' + + def delete_selected_and_artifacts(self, request, queryset): + """Remove HTML/etc artifacts from application instances + + Prior to the query delete, broadcast tasks to delete HTML artifacts from + application instances. + """ + if request.POST.get('post'): + for project in queryset: + broadcast(type='app', task=remove_dir, args=[project.doc_path]) + return delete_selected(self, request, queryset) + + def get_actions(self, request): + actions = super(ProjectAdmin, self).get_actions(request) + actions['delete_selected'] = ( + self.__class__.delete_selected_and_artifacts, + 'delete_selected', + delete_selected.short_description + ) + return actions + class ImportedFileAdmin(admin.ModelAdmin): diff --git a/readthedocs/rtd_tests/tests/projects/__init__.py b/readthedocs/rtd_tests/tests/projects/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/readthedocs/rtd_tests/tests/projects/test_admin_actions.py b/readthedocs/rtd_tests/tests/projects/test_admin_actions.py new file mode 100644 index 00000000000..fb2d63555b0 --- /dev/null +++ b/readthedocs/rtd_tests/tests/projects/test_admin_actions.py @@ -0,0 +1,80 @@ +import mock +import django_dynamic_fixture as fixture +from django.contrib.admin.helpers import ACTION_CHECKBOX_NAME +from django.contrib.auth.models import User +from django.core import urlresolvers +from django.test import TestCase + +from readthedocs.core.models import UserProfile +from readthedocs.projects.models import Project + + +class ProjectAdminActionsTest(TestCase): + + @classmethod + def setUpTestData(cls): + cls.owner = fixture.get(User) + cls.profile = fixture.get(UserProfile, user=cls.owner, banned=False) + cls.admin = fixture.get(User, is_staff=True, is_superuser=True) + cls.project = fixture.get( + Project, + main_language_project=None, + users=[cls.owner], + ) + + def setUp(self): + self.client.force_login(self.admin) + + def test_project_ban_owner(self): + self.assertFalse(self.owner.profile.banned) + action_data = { + ACTION_CHECKBOX_NAME: [self.project.pk], + 'action': 'ban_owner', + 'index': 0, + } + resp = self.client.post( + urlresolvers.reverse('admin:projects_project_changelist'), + action_data + ) + self.assertTrue(self.project.users.filter(profile__banned=True).exists()) + self.assertFalse(self.project.users.filter(profile__banned=False).exists()) + + def test_project_ban_multiple_owners(self): + owner_b = fixture.get(User) + profile_b = fixture.get(UserProfile, user=owner_b, banned=False) + self.project.users.add(owner_b) + self.assertFalse(self.owner.profile.banned) + self.assertFalse(owner_b.profile.banned) + action_data = { + ACTION_CHECKBOX_NAME: [self.project.pk], + 'action': 'ban_owner', + 'index': 0, + } + resp = self.client.post( + urlresolvers.reverse('admin:projects_project_changelist'), + action_data + ) + self.assertFalse(self.project.users.filter(profile__banned=True).exists()) + self.assertEqual(self.project.users.filter(profile__banned=False).count(), 2) + + @mock.patch('readthedocs.projects.admin.remove_dir') + def test_project_delete(self, remove_dir): + """Test project and artifacts are removed""" + action_data = { + ACTION_CHECKBOX_NAME: [self.project.pk], + 'action': 'delete_selected', + 'index': 0, + 'post': 'yes', + } + resp = self.client.post( + urlresolvers.reverse('admin:projects_project_changelist'), + action_data + ) + self.assertFalse(Project.objects.filter(pk=self.project.pk).exists()) + remove_dir.apply_async.assert_has_calls([ + mock.call( + kwargs={}, + queue='celery', + args=[self.project.doc_path] + ), + ])