Allow users with Admin:Advanced to delete users with relations to other models

Note that the behaviour depends on which model is affected - some are kept,
others are deleted because of UserProfile.delete_or_disable_related_content().
This commit is contained in:
Mathieu Pillard 2018-08-21 14:54:16 +02:00
Родитель 471a2e666b
Коммит c09a7fc1ae
4 изменённых файлов: 77 добавлений и 2 удалений

Просмотреть файл

@ -73,6 +73,10 @@ class AddonAdmin(admin.ModelAdmin):
status_with_admin_manage_link.short_description = _(u'Status') status_with_admin_manage_link.short_description = _(u'Status')
class AddonUserAdmin(admin.ModelAdmin):
raw_id_fields = ('addon', 'user')
class FeatureAdmin(admin.ModelAdmin): class FeatureAdmin(admin.ModelAdmin):
raw_id_fields = ('addon',) raw_id_fields = ('addon',)
list_filter = ('application', 'locale') list_filter = ('application', 'locale')

Просмотреть файл

@ -84,7 +84,14 @@ PERMISSIONS_LIST = [
DJANGO_PERMISSIONS_MAPPING = defaultdict(lambda: SUPERPOWERS) DJANGO_PERMISSIONS_MAPPING = defaultdict(lambda: SUPERPOWERS)
DJANGO_PERMISSIONS_MAPPING.update({ DJANGO_PERMISSIONS_MAPPING.update({
'abuse.delete_abusereport': ADMIN_ADVANCED,
# Note that ActivityLog's ModelAdmin actually forbids deletion entirely.
# This is just here to allow deletion of users, because django checks
# foreign keys even though users are only soft-deleted and related objects
# will be kept.
'activity.delete_activitylog': ADMIN_ADVANCED,
'addons.change_addon': ADDONS_EDIT, 'addons.change_addon': ADDONS_EDIT,
'addons.delete_addonuser': ADMIN_ADVANCED,
# Users with Admin:Curation can do anything to ReplacementAddon. # Users with Admin:Curation can do anything to ReplacementAddon.
# In addition, the modeladmin will also check for Addons:Edit and give them # In addition, the modeladmin will also check for Addons:Edit and give them
# read-only access to the changelist (obj=None passed to the # read-only access to the changelist (obj=None passed to the
@ -100,8 +107,11 @@ DJANGO_PERMISSIONS_MAPPING.update({
'discovery.change_discoveryitem': DISCOVERY_EDIT, 'discovery.change_discoveryitem': DISCOVERY_EDIT,
'discovery.delete_discoveryitem': DISCOVERY_EDIT, 'discovery.delete_discoveryitem': DISCOVERY_EDIT,
'reviewers.delete_reviewerscore': ADMIN_ADVANCED,
'users.change_userprofile': USERS_EDIT, 'users.change_userprofile': USERS_EDIT,
'users.delete_userprofile': ADMIN_ADVANCED, 'users.delete_userprofile': ADMIN_ADVANCED,
'ratings.change_rating': RATINGS_MODERATE, 'ratings.change_rating': RATINGS_MODERATE,
'ratings.delete_rating': ADMIN_ADVANCED,
}) })

Просмотреть файл

@ -419,6 +419,8 @@ class UserProfile(OnChangeMixin, ModelBase, AbstractBaseUser):
addon.delete() addon.delete()
else: else:
addon.force_disable() addon.force_disable()
else:
addon.addonuser_set.filter(user=self).delete()
user_responsible = core.get_user() user_responsible = core.get_user()
self._ratings_all.all().delete(user_responsible=user_responsible) self._ratings_all.all().delete(user_responsible=user_responsible)
self.delete_picture() self.delete_picture()

Просмотреть файл

@ -10,15 +10,16 @@ import mock
from pyquery import PyQuery as pq from pyquery import PyQuery as pq
from olympia import amo, core from olympia import amo, core
from olympia.abuse.models import AbuseReport from olympia.abuse.models import AbuseReport
from olympia.activity.models import ActivityLog from olympia.activity.models import ActivityLog
from olympia.addons.models import AddonUser
from olympia.amo.tests import ( from olympia.amo.tests import (
addon_factory, TestCase, user_factory, version_factory) addon_factory, collection_factory, TestCase, user_factory, version_factory)
from olympia.amo.urlresolvers import reverse from olympia.amo.urlresolvers import reverse
from olympia.bandwagon.models import Collection from olympia.bandwagon.models import Collection
from olympia.ratings.models import Rating from olympia.ratings.models import Rating
from olympia.reviewers.models import ReviewerScore
from olympia.users.admin import UserAdmin from olympia.users.admin import UserAdmin
from olympia.users.models import UserProfile from olympia.users.models import UserProfile
@ -95,6 +96,7 @@ class TestUserAdmin(TestCase):
core.set_user(user) core.set_user(user)
response = self.client.get(self.delete_url, follow=True) response = self.client.get(self.delete_url, follow=True)
assert response.status_code == 200 assert response.status_code == 200
assert 'Cannot delete user' not in response.content
response = self.client.post(self.delete_url, {'post': 'yes'}, response = self.client.post(self.delete_url, {'post': 'yes'},
follow=True) follow=True)
assert response.status_code == 200 assert response.status_code == 200
@ -109,6 +111,63 @@ class TestUserAdmin(TestCase):
assert alog.action == amo.LOG.ADMIN_USER_ANONYMIZED.id assert alog.action == amo.LOG.ADMIN_USER_ANONYMIZED.id
assert alog.arguments == [self.user] assert alog.arguments == [self.user]
def test_can_delete_with_related_objects_with_admin_advanced_permission(
self):
# Add related instances...
addon = addon_factory()
addon_with_other_authors = addon_factory()
AddonUser.objects.create(
addon=addon_with_other_authors, user=user_factory())
relations_that_should_be_deleted = [
AddonUser.objects.create(
addon=addon_with_other_authors, user=self.user),
Rating.objects.create(
addon=addon_factory(), rating=5, user=self.user),
addon, # Has no other author, should be deleted.
collection_factory(author=self.user)
]
relations_that_should_survive = [
AbuseReport.objects.create(reporter=self.user),
AbuseReport.objects.create(user=self.user),
ActivityLog.create(user=self.user, action=amo.LOG.USER_EDITED),
ReviewerScore.objects.create(user=self.user, score=42),
addon_with_other_authors, # Has other authors, should be kept.
# Bit of a weird case, but because the user was the only author of
# this add-on, the addonuser relation is kept, and both the add-on
# and the user are soft-deleted. This is in contrast with the case
# where the user is *not* the only author, in which case the
# addonuser relation is deleted, but the add-on is left intact.
AddonUser.objects.create(addon=addon, user=self.user),
]
# Now test as normal.
user = user_factory()
assert not self.user.deleted
self.grant_permission(user, 'Admin:Tools')
self.grant_permission(user, 'Admin:Advanced')
self.client.login(email=user.email)
core.set_user(user)
response = self.client.get(self.delete_url, follow=True)
assert response.status_code == 200
assert 'Cannot delete user' not in response.content
response = self.client.post(self.delete_url, {'post': 'yes'},
follow=True)
assert response.status_code == 200
self.user.reload()
assert self.user.deleted
assert self.user.email is None
alog = ActivityLog.objects.filter(
action=amo.LOG.ADMIN_USER_ANONYMIZED.id).get()
assert alog.arguments == [self.user]
# Test the related instances we created earlier.
for obj in relations_that_should_be_deleted:
assert not obj.__class__.objects.filter(pk=obj.pk).exists()
for obj in relations_that_should_survive:
assert obj.__class__.objects.filter(pk=obj.pk).exists()
def test_get_actions(self): def test_get_actions(self):
user_admin = UserAdmin(UserProfile, admin.site) user_admin = UserAdmin(UserProfile, admin.site)
request = RequestFactory().get('/') request = RequestFactory().get('/')