Skip to content

Commit 74dbdcd

Browse files
authored
Add some basic spam removal features to admin (readthedocs#3131)
* 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 readthedocs#2762 * Remove unused attempt at using utils * Add docs on admin removal
1 parent 41bd6d3 commit 74dbdcd

File tree

4 files changed

+159
-2
lines changed

4 files changed

+159
-2
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

+78-2
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,21 @@
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
11+
from readthedocs.core.utils import broadcast
712
from readthedocs.builds.models import Version
813
from readthedocs.redirects.models import Redirect
914
from readthedocs.notifications.views import SendNotificationView
1015

1116
from .notifications import ResourceUsageNotification
1217
from .models import (Project, ImportedFile,
1318
ProjectRelationship, EmailHook, WebHook, Domain)
19+
from .tasks import remove_dir
1420

1521

1622
class ProjectSendNotificationView(SendNotificationView):
@@ -65,20 +71,45 @@ class DomainInline(admin.TabularInline):
6571
# return instance.click_ratio * 100
6672

6773

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+
6898
class ProjectAdmin(GuardedModelAdmin):
6999

70100
"""Project model admin view"""
71101

72102
prepopulated_fields = {'slug': ('name',)}
73103
list_display = ('name', 'repo', 'repo_type', 'allow_comments', 'featured', 'theme')
74104
list_filter = ('repo_type', 'allow_comments', 'featured', 'privacy_level',
75-
'documentation_type', 'programming_language')
105+
'documentation_type', 'programming_language',
106+
ProjectOwnerBannedFilter)
76107
list_editable = ('featured',)
77108
search_fields = ('slug', 'repo')
78109
inlines = [ProjectRelationshipInline, RedirectInline,
79110
VersionInline, DomainInline]
80111
raw_id_fields = ('users', 'main_language_project')
81-
actions = ['send_owner_email']
112+
actions = ['send_owner_email', 'ban_owner']
82113

83114
def send_owner_email(self, request, queryset):
84115
view = ProjectSendNotificationView.as_view(
@@ -88,6 +119,51 @@ def send_owner_email(self, request, queryset):
88119

89120
send_owner_email.short_description = 'Notify project owners'
90121

122+
def ban_owner(self, request, queryset):
123+
"""Ban project owner
124+
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.
128+
"""
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+
"""Remove HTML/etc artifacts from application instances
149+
150+
Prior to the query delete, broadcast tasks to delete HTML artifacts from
151+
application instances.
152+
"""
153+
if request.POST.get('post'):
154+
for project in queryset:
155+
broadcast(type='app', task=remove_dir, args=[project.doc_path])
156+
return delete_selected(self, request, queryset)
157+
158+
def get_actions(self, request):
159+
actions = super(ProjectAdmin, self).get_actions(request)
160+
actions['delete_selected'] = (
161+
self.__class__.delete_selected_and_artifacts,
162+
'delete_selected',
163+
delete_selected.short_description
164+
)
165+
return actions
166+
91167

92168
class ImportedFileAdmin(admin.ModelAdmin):
93169

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)