Skip to content

Commit 5421279

Browse files
committed
Add some basic spam removal features to admin
This adds actions for: * Mark project owner as banned * Remove project and files on web server Closes #2762
1 parent 3da8ae7 commit 5421279

File tree

5 files changed

+155
-21
lines changed

5 files changed

+155
-21
lines changed

readthedocs/core/admin.py

+1
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ def is_banned(self, obj):
6767
return hasattr(obj, 'profile') and obj.profile.banned
6868

6969
is_banned.short_description = 'Banned'
70+
is_banned.boolean = True
7071

7172
def ban_user(self, request, queryset):
7273
users = []

readthedocs/projects/admin.py

+69-21
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,12 @@
22

33
from __future__ import absolute_import
44
from django.contrib import admin
5+
from django.contrib import messages
6+
from django.contrib.admin.actions import delete_selected
7+
from django.utils.translation import ugettext_lazy as _
58
from guardian.admin import GuardedModelAdmin
69

10+
from readthedocs.core.models import UserProfile
711
from readthedocs.core.utils import broadcast
812
from readthedocs.builds.models import Version
913
from readthedocs.redirects.models import Redirect
@@ -12,7 +16,7 @@
1216
from .notifications import ResourceUsageNotification
1317
from .models import (Project, ImportedFile,
1418
ProjectRelationship, EmailHook, WebHook, Domain)
15-
from .tasks import clear_artifacts
19+
from .tasks import remove_dir
1620

1721

1822
class ProjectSendNotificationView(SendNotificationView):
@@ -67,20 +71,45 @@ class DomainInline(admin.TabularInline):
6771
# return instance.click_ratio * 100
6872

6973

74+
class ProjectOwnerBannedFilter(admin.SimpleListFilter):
75+
76+
"""Filter for projects with banned owners
77+
78+
There are problems adding `users__profile__banned` to the `list_filter`
79+
attribute, so we'll create a basic filter to capture banned owners.
80+
"""
81+
82+
title = 'project owner banned'
83+
parameter_name = 'project_owner_banned'
84+
85+
OWNER_BANNED = 'true'
86+
87+
def lookups(self, request, model_admin):
88+
return (
89+
(self.OWNER_BANNED, _('Yes')),
90+
)
91+
92+
def queryset(self, request, queryset):
93+
if self.value() == self.OWNER_BANNED:
94+
return queryset.filter(users__profile__banned=True)
95+
return queryset
96+
97+
7098
class ProjectAdmin(GuardedModelAdmin):
7199

72100
"""Project model admin view"""
73101

74102
prepopulated_fields = {'slug': ('name',)}
75103
list_display = ('name', 'repo', 'repo_type', 'allow_comments', 'featured', 'theme')
76104
list_filter = ('repo_type', 'allow_comments', 'featured', 'privacy_level',
77-
'documentation_type', 'programming_language')
105+
'documentation_type', 'programming_language',
106+
ProjectOwnerBannedFilter)
78107
list_editable = ('featured',)
79108
search_fields = ('slug', 'repo')
80109
inlines = [ProjectRelationshipInline, RedirectInline,
81110
VersionInline, DomainInline]
82111
raw_id_fields = ('users', 'main_language_project')
83-
actions = ['send_owner_email', 'flag_project', 'flag_project_ban_owner']
112+
actions = ['send_owner_email', 'ban_owner']
84113

85114
def send_owner_email(self, request, queryset):
86115
view = ProjectSendNotificationView.as_view(
@@ -90,26 +119,45 @@ def send_owner_email(self, request, queryset):
90119

91120
send_owner_email.short_description = 'Notify project owners'
92121

93-
def flag_project(self, request, queryset):
94-
"""Flag project as spam
122+
def ban_owner(self, request, queryset):
123+
"""Ban project owner
95124
96-
Delete project and wipe files from web servers
125+
This will only ban single owners, because a malicious user could add a
126+
user as a co-owner of the project. We don't want to induce and
127+
collatoral damage when flagging users.
97128
"""
98-
pass
99-
100-
flag_project.short_description = flag_project.__doc__.splitlines()[0]
101-
102-
def flag_project_ban_owner(self, request, queryset):
103-
"""Flag project as spam and ban owner
104-
105-
This will only ban single owners. Because a malicious user could add a
106-
user as a co-owner of the project, we don't want to induce and
107-
collatoral damage when blowing up projects.
108-
"""
109-
pass
110-
111-
flag_project_ban_owners.short_description = (flag_project_ban_owners
112-
.__doc__.splitlines()[0])
129+
total = 0
130+
for project in queryset:
131+
if project.users.count() == 1:
132+
count = (UserProfile.objects
133+
.filter(user__projects=project)
134+
.update(banned=True))
135+
total += count
136+
else:
137+
messages.add_message(request, messages.ERROR,
138+
'Project has multiple owners: {0}'.format(project))
139+
if total == 0:
140+
messages.add_message(request, messages.ERROR, 'No users banned')
141+
else:
142+
messages.add_message(request, messages.INFO,
143+
'Banned {0} user(s)'.format(total))
144+
145+
ban_owner.short_description = 'Ban project owner'
146+
147+
def delete_selected_and_artifacts(self, request, queryset):
148+
if request.POST.get('post'):
149+
for project in queryset:
150+
broadcast(type='app', task=remove_dir, args=[project.doc_path])
151+
return delete_selected(self, request, queryset)
152+
153+
def get_actions(self, request):
154+
actions = super(ProjectAdmin, self).get_actions(request)
155+
actions['delete_selected'] = (
156+
self.__class__.delete_selected_and_artifacts,
157+
'delete_selected',
158+
delete_selected.short_description
159+
)
160+
return actions
113161

114162

115163
class ImportedFileAdmin(admin.ModelAdmin):

readthedocs/projects/utils.py

+5
Original file line numberDiff line numberDiff line change
@@ -200,3 +200,8 @@ def make_api_project(project_data):
200200
project = Project(**project_data)
201201
project.save = _new_save
202202
return project
203+
204+
205+
def delete_project(project):
206+
broadcast(type='app', task=tasks.remove_dir, args=[project.doc_path])
207+
project.delete()

readthedocs/rtd_tests/tests/projects/__init__.py

Whitespace-only changes.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
import mock
2+
import django_dynamic_fixture as fixture
3+
from django.contrib.admin.helpers import ACTION_CHECKBOX_NAME
4+
from django.contrib.auth.models import User
5+
from django.core import urlresolvers
6+
from django.test import TestCase
7+
8+
from readthedocs.core.models import UserProfile
9+
from readthedocs.projects.models import Project
10+
11+
12+
class ProjectAdminActionsTest(TestCase):
13+
14+
@classmethod
15+
def setUpTestData(cls):
16+
cls.owner = fixture.get(User)
17+
cls.profile = fixture.get(UserProfile, user=cls.owner, banned=False)
18+
cls.admin = fixture.get(User, is_staff=True, is_superuser=True)
19+
cls.project = fixture.get(
20+
Project,
21+
main_language_project=None,
22+
users=[cls.owner],
23+
)
24+
25+
def setUp(self):
26+
self.client.force_login(self.admin)
27+
28+
def test_project_ban_owner(self):
29+
self.assertFalse(self.owner.profile.banned)
30+
action_data = {
31+
ACTION_CHECKBOX_NAME: [self.project.pk],
32+
'action': 'ban_owner',
33+
'index': 0,
34+
}
35+
resp = self.client.post(
36+
urlresolvers.reverse('admin:projects_project_changelist'),
37+
action_data
38+
)
39+
self.assertTrue(self.project.users.filter(profile__banned=True).exists())
40+
self.assertFalse(self.project.users.filter(profile__banned=False).exists())
41+
42+
def test_project_ban_multiple_owners(self):
43+
owner_b = fixture.get(User)
44+
profile_b = fixture.get(UserProfile, user=owner_b, banned=False)
45+
self.project.users.add(owner_b)
46+
self.assertFalse(self.owner.profile.banned)
47+
self.assertFalse(owner_b.profile.banned)
48+
action_data = {
49+
ACTION_CHECKBOX_NAME: [self.project.pk],
50+
'action': 'ban_owner',
51+
'index': 0,
52+
}
53+
resp = self.client.post(
54+
urlresolvers.reverse('admin:projects_project_changelist'),
55+
action_data
56+
)
57+
self.assertFalse(self.project.users.filter(profile__banned=True).exists())
58+
self.assertEqual(self.project.users.filter(profile__banned=False).count(), 2)
59+
60+
@mock.patch('readthedocs.projects.admin.remove_dir')
61+
def test_project_delete(self, remove_dir):
62+
"""Test project and artifacts are removed"""
63+
action_data = {
64+
ACTION_CHECKBOX_NAME: [self.project.pk],
65+
'action': 'delete_selected',
66+
'index': 0,
67+
'post': 'yes',
68+
}
69+
resp = self.client.post(
70+
urlresolvers.reverse('admin:projects_project_changelist'),
71+
action_data
72+
)
73+
self.assertFalse(Project.objects.filter(pk=self.project.pk).exists())
74+
remove_dir.apply_async.assert_has_calls([
75+
mock.call(
76+
kwargs={},
77+
queue='celery',
78+
args=[self.project.doc_path]
79+
),
80+
])

0 commit comments

Comments
 (0)