From 5c7a43f53a80b0cde7a02212b14de6753bbb8fa7 Mon Sep 17 00:00:00 2001 From: Mathieu Pillard Date: Thu, 21 May 2026 15:27:54 +0200 Subject: [PATCH 1/4] Add bulk banning functionality to the admin Also make banning through the changelist action use the same async task underneath to scale to a larger number of users. --- src/olympia/lib/settings_base.py | 1 + src/olympia/users/admin.py | 71 +++++++++++++++++-- src/olympia/users/forms.py | 13 ++++ src/olympia/users/models.py | 38 +++++----- src/olympia/users/tasks.py | 10 +++ .../admin/users/userprofile/bulk_ban.html | 36 ++++++++++ .../userprofile/change_list_object_tools.html | 12 ++++ static/css/admin-user.less | 4 ++ static/css/admin/users.css | 11 +++ 9 files changed, 174 insertions(+), 22 deletions(-) create mode 100644 src/olympia/users/templates/admin/users/userprofile/bulk_ban.html create mode 100644 src/olympia/users/templates/admin/users/userprofile/change_list_object_tools.html create mode 100644 static/css/admin-user.less create mode 100644 static/css/admin/users.css diff --git a/src/olympia/lib/settings_base.py b/src/olympia/lib/settings_base.py index b5dbfe2998c3..b04d760d9a74 100644 --- a/src/olympia/lib/settings_base.py +++ b/src/olympia/lib/settings_base.py @@ -769,6 +769,7 @@ def get_language_url_map(): 'olympia.activity.tasks.create_ratinglog': {'queue': 'adhoc'}, 'olympia.files.tasks.extract_host_permissions': {'queue': 'adhoc'}, 'olympia.lib.crypto.tasks.bump_and_resign_addons': {'queue': 'adhoc'}, + 'olympia.users.tasks.bulk_ban': {'queue': 'adhoc'}, 'olympia.users.tasks.restrict_banned_users': {'queue': 'adhoc'}, # Misc AMO tasks. 'olympia.blocklist.tasks.monitor_remote_settings': {'queue': 'amo'}, diff --git a/src/olympia/users/admin.py b/src/olympia/users/admin.py index be7b7f30e45b..1f4a3dc479d0 100644 --- a/src/olympia/users/admin.py +++ b/src/olympia/users/admin.py @@ -6,7 +6,7 @@ from django.db import models from django.db.models import Count, Q from django.db.utils import IntegrityError -from django.forms import TextInput +from django.forms import HiddenInput, TextInput from django.http import ( Http404, HttpResponseForbidden, @@ -25,6 +25,8 @@ from olympia.activity.models import ActivityLog, RequestFingerprintLog from olympia.addons.models import Addon, AddonUser from olympia.amo.admin import AMOModelAdmin +from olympia.amo.celery import create_chunked_tasks_signatures +from olympia.amo.templatetags.jinja_helpers import vite_asset from olympia.amo.utils import backup_storage_enabled, create_signed_url_for_file_backup from olympia.api.models import APIKey, APIKeyConfirmation from olympia.bandwagon.models import Collection @@ -45,6 +47,7 @@ UserProfile, UserRestrictionHistory, ) +from .tasks import bulk_ban class GroupUserInline(admin.TabularInline): @@ -107,6 +110,9 @@ def picture_link(self, obj): @admin.register(UserProfile) class UserAdmin(AMOModelAdmin): + class Media: + css = {'all': (vite_asset('css/admin-user.less'),)} + list_filter = ('banned',) list_display = ( '__str__', @@ -242,6 +248,11 @@ def wrapper(*args, **kwargs): urlpatterns = super().get_urls() custom_urlpatterns = [ + re_path( + r'^bulk_ban/$', + wrap(self.bulk_ban_view), + name='users_userprofile_bulk_ban', + ), re_path( r'^(?P.+)/ban/$', wrap(self.ban_view), @@ -284,6 +295,19 @@ def get_actions(self, request): return actions + def changelist_view(self, request, extra_context=None): + extra_context = extra_context or {} + extra_context['has_users_edit_permission'] = acl.action_allowed_for( + request.user, amo.permissions.USERS_EDIT + ) + extra_context['has_users_ban_permission'] = acl.action_allowed_for( + request.user, amo.permissions.USERS_BAN + ) + return super().changelist_view( + request, + extra_context=extra_context, + ) + def change_view(self, request, object_id, form_url='', extra_context=None): extra_context = extra_context or {} extra_context['has_users_edit_permission'] = acl.action_allowed_for( @@ -425,10 +449,49 @@ def delete_picture_view(self, request, object_id, extra_context=None): reverse('admin:users_userprofile_change', args=(obj.pk,)) ) + def bulk_ban_view(self, request, extra_context=None): + if not acl.action_allowed_for(request.user, amo.permissions.USERS_BAN): + return HttpResponseForbidden() + + user_ids_to_ban = None + + if request.method == 'POST': + form = forms.BulkBanForm(request.POST) + if form.is_valid(): + user_ids_to_ban = form.cleaned_data['user_ids'] + form.fields['user_ids'].widget = HiddenInput() + if request.POST.get('post'): + self._trigger_bulk_ban_task( + request, + ) + return HttpResponseRedirect( + reverse('admin:users_userprofile_changelist') + ) + else: + form = forms.BulkBanForm() + context = { + 'title': 'Bulk ban users', + 'form': form, + 'user_ids_to_ban': user_ids_to_ban, + **self.admin_site.each_context(request), + 'opts': self.opts, + 'media': self.media + form.media, + **(extra_context or {}), + } + return TemplateResponse( + request, 'admin/users/userprofile/bulk_ban.html', context + ) + + def _trigger_bulk_ban_task(self, request, user_ids): + workflow = create_chunked_tasks_signatures(bulk_ban, list(user_ids), 50) + workflow.apply_async() + self.message_user( + request, + f'Bulk-ban for {len(user_ids)} user id(s) will be processed shortly.', + ) + def ban_action(self, request, qs): - qs.ban_and_disable_related_content(hard_block_addons=True) - kw = {'users': ', '.join(str(user) for user in qs)} - self.message_user(request, 'The users "%(users)s" have been banned.' % kw) + self._trigger_bulk_ban_task(request, qs.values_list('id', flat=True)) ban_action.short_description = 'Ban selected users' diff --git a/src/olympia/users/forms.py b/src/olympia/users/forms.py index 6637c5552421..e0def00fdb11 100644 --- a/src/olympia/users/forms.py +++ b/src/olympia/users/forms.py @@ -61,3 +61,16 @@ def clean(self): data['network'] = f'{ip_address}/32' return data + + +class BulkBanForm(forms.Form): + user_ids = forms.CharField( + label='', required=True, widget=forms.Textarea(attrs={'rows': 30, 'cols': 80}) + ) + + def clean_user_ids(self): + data = self.cleaned_data.get('user_ids', '') + user_ids = {id_ for id_ in data.splitlines() if id_.isdigit()} + if not user_ids: + raise forms.ValidationError('This field must contain a least one user id') + return user_ids diff --git a/src/olympia/users/models.py b/src/olympia/users/models.py index a405d64b3a79..5f2e8c35457b 100644 --- a/src/olympia/users/models.py +++ b/src/olympia/users/models.py @@ -161,21 +161,22 @@ def ban_and_disable_related_content( BannedUserContent.objects.bulk_create( [BannedUserContent(user=user) for user in users], ignore_conflicts=True ) - EmailUserRestriction.objects.bulk_create( - [ - EmailUserRestriction( - email_pattern=EmailUserRestriction.normalize_email(user.email), - restriction_type=restriction_type, - reason=f'Automatically added because of user {user.pk} ban', - ) - for user in users - for restriction_type in [ - RESTRICTION_TYPES.ADDON_SUBMISSION, - RESTRICTION_TYPES.RATING, - ] - ], - ignore_conflicts=True, - ) + if user.email: + EmailUserRestriction.objects.bulk_create( + [ + EmailUserRestriction( + email_pattern=EmailUserRestriction.normalize_email(user.email), + restriction_type=restriction_type, + reason=f'Automatically added because of user {user.pk} ban', + ) + for user in users + for restriction_type in [ + RESTRICTION_TYPES.ADDON_SUBMISSION, + RESTRICTION_TYPES.RATING, + ] + ], + ignore_conflicts=True, + ) # Collect affected addons addon_ids = set( @@ -318,9 +319,10 @@ def unban_and_reenable_related_content(self, *, skip_activity_log=False): user.deleted = False user.banned = None user.save() - EmailUserRestriction.objects.filter( - email_pattern=EmailUserRestriction.normalize_email(user.email) - ).delete() + if user.email: + EmailUserRestriction.objects.filter( + email_pattern=EmailUserRestriction.normalize_email(user.email) + ).delete() if blocklist_submissions_pks: user_responsible = core.get_user() or get_task_user() revert_published_blocklist_submissions.delay( diff --git a/src/olympia/users/tasks.py b/src/olympia/users/tasks.py index 5c883d964220..d041fbbbc827 100644 --- a/src/olympia/users/tasks.py +++ b/src/olympia/users/tasks.py @@ -278,3 +278,13 @@ def bulk_add_disposable_email_domains(entries: list[tuple[str, str]], batch_size f'Processed {len(processed_domains)} domains: ' f'{[obj.domain for obj in processed_domains]}' ) + + +def bulk_ban(user_ids): + task_log.info( + '[1@None] Bulk-banning users %d-%d [%d].', + ids[0], + ids[-1], + len(ids), + ) + UserProfile.objects.filter(pk__in=chunk).ban_and_disable_related_content() diff --git a/src/olympia/users/templates/admin/users/userprofile/bulk_ban.html b/src/olympia/users/templates/admin/users/userprofile/bulk_ban.html new file mode 100644 index 000000000000..f2eae1dff725 --- /dev/null +++ b/src/olympia/users/templates/admin/users/userprofile/bulk_ban.html @@ -0,0 +1,36 @@ +{% extends "admin/base_site.html" %} +{% load i18n admin_urls static %} + +{% block extrahead %} + {{ block.super }} + {{ media }} + +{% endblock %} + +{% block bodyclass %}{{ block.super }} app-{{ opts.app_label }} model-{{ opts.model_name }} bulk-ban-confirmation{% endblock %} + +{% block breadcrumbs %} +
+Home +› {{ opts.app_config.verbose_name }} +› {{ opts.verbose_name_plural|capfirst }} +› Bulk ban +
+{% endblock %} + +{% block content %} +
+ {% csrf_token %} + {% if user_ids_to_ban %} +

You're about to bulk-ban {{ user_ids_to_ban|length }} user(s). Are you sure ?

+ {{ form.as_p }} + + + No, take me back + {% else %} +

Paste user ids to ban below, separated by newlines (only lines containing digits will be considered). A confirmation will be displayed on the next page.

+ {{ form.as_p }} + + {% endif %} +
+{% endblock content %} diff --git a/src/olympia/users/templates/admin/users/userprofile/change_list_object_tools.html b/src/olympia/users/templates/admin/users/userprofile/change_list_object_tools.html new file mode 100644 index 000000000000..4f2045942e5e --- /dev/null +++ b/src/olympia/users/templates/admin/users/userprofile/change_list_object_tools.html @@ -0,0 +1,12 @@ +{% extends "admin/change_list_object_tools.html" %} +{% load i18n admin_urls %} + +{% block object-tools-items %} + {{ block.super }} + {% if has_users_ban_permission %} +
  • + {% url 'admin:users_userprofile_bulk_ban' as ban_url %} + Bulk ban +
    • + {% endif %} +{% endblock %} diff --git a/static/css/admin-user.less b/static/css/admin-user.less new file mode 100644 index 000000000000..cffcf2f68692 --- /dev/null +++ b/static/css/admin-user.less @@ -0,0 +1,4 @@ +@import url('./admin/amoadmin.css'); +@import url('./admin/l10n.css'); +@import url('./admin/pagination.css'); +@import url('./admin/users.css'); diff --git a/static/css/admin/users.css b/static/css/admin/users.css new file mode 100644 index 000000000000..9afc758d2404 --- /dev/null +++ b/static/css/admin/users.css @@ -0,0 +1,11 @@ +.bulk-ban-confirmation .cancel-link { + display: inline-block; + vertical-align: middle; + height: 0.9375rem; + line-height: 0.9375rem; + border-radius: 4px; + padding: 10px 15px; + color: var(--button-fg); + background: var(--close-button-bg); + margin: 0 0 0 10px; +} From 5388c2e63d9ea1f927a96159319e221cc7536e63 Mon Sep 17 00:00:00 2001 From: Mathieu Pillard Date: Fri, 22 May 2026 16:48:27 +0200 Subject: [PATCH 2/4] First pass of tests/fixes --- src/olympia/users/models.py | 32 +++++++++++++------------- src/olympia/users/tasks.py | 4 ++-- src/olympia/users/tests/test_models.py | 3 ++- 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/olympia/users/models.py b/src/olympia/users/models.py index 5f2e8c35457b..ae6b1d53ae27 100644 --- a/src/olympia/users/models.py +++ b/src/olympia/users/models.py @@ -161,22 +161,22 @@ def ban_and_disable_related_content( BannedUserContent.objects.bulk_create( [BannedUserContent(user=user) for user in users], ignore_conflicts=True ) - if user.email: - EmailUserRestriction.objects.bulk_create( - [ - EmailUserRestriction( - email_pattern=EmailUserRestriction.normalize_email(user.email), - restriction_type=restriction_type, - reason=f'Automatically added because of user {user.pk} ban', - ) - for user in users - for restriction_type in [ - RESTRICTION_TYPES.ADDON_SUBMISSION, - RESTRICTION_TYPES.RATING, - ] - ], - ignore_conflicts=True, - ) + EmailUserRestriction.objects.bulk_create( + [ + EmailUserRestriction( + email_pattern=EmailUserRestriction.normalize_email(user.email), + restriction_type=restriction_type, + reason=f'Automatically added because of user {user.pk} ban', + ) + for user in users + for restriction_type in [ + RESTRICTION_TYPES.ADDON_SUBMISSION, + RESTRICTION_TYPES.RATING, + ] + if user.email + ], + ignore_conflicts=True, + ) # Collect affected addons addon_ids = set( diff --git a/src/olympia/users/tasks.py b/src/olympia/users/tasks.py index d041fbbbc827..6292e5bd7627 100644 --- a/src/olympia/users/tasks.py +++ b/src/olympia/users/tasks.py @@ -280,11 +280,11 @@ def bulk_add_disposable_email_domains(entries: list[tuple[str, str]], batch_size ) -def bulk_ban(user_ids): +def bulk_ban(ids): task_log.info( '[1@None] Bulk-banning users %d-%d [%d].', ids[0], ids[-1], len(ids), ) - UserProfile.objects.filter(pk__in=chunk).ban_and_disable_related_content() + UserProfile.objects.filter(pk__in=ids).ban_and_disable_related_content() diff --git a/src/olympia/users/tests/test_models.py b/src/olympia/users/tests/test_models.py index c2061d8b757c..3b7f0c954ca9 100644 --- a/src/olympia/users/tests/test_models.py +++ b/src/olympia/users/tests/test_models.py @@ -240,6 +240,7 @@ def test_ban_and_disable_related_content_bulk( occupation='some job too', read_dev_agreement=datetime.now(), ) + user_no_email = user_factory(email=None, deleted=True) user_innocent = user_factory() addon_multi = addon_factory( users=UserProfile.objects.filter(id__in=[user_multi.id, user_innocent.id]) @@ -259,7 +260,7 @@ def test_ban_and_disable_related_content_bulk( # Now that everything is set up, disable/delete related content. UserProfile.objects.filter( - pk__in=(user_sole.pk, user_multi.pk) + pk__in=(user_sole.pk, user_multi.pk, user_no_email.pk) ).ban_and_disable_related_content(hard_block_addons=hard_block_addons) assert copy_file_to_backup_storage_mock.call_count == 2 From 541b6d02622266c1283876841d59fcd8e8399ea2 Mon Sep 17 00:00:00 2001 From: Mathieu Pillard Date: Fri, 22 May 2026 16:49:43 +0200 Subject: [PATCH 3/4] Should be a task --- src/olympia/users/tasks.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/olympia/users/tasks.py b/src/olympia/users/tasks.py index 6292e5bd7627..a0def442def4 100644 --- a/src/olympia/users/tasks.py +++ b/src/olympia/users/tasks.py @@ -280,6 +280,7 @@ def bulk_add_disposable_email_domains(entries: list[tuple[str, str]], batch_size ) +@task def bulk_ban(ids): task_log.info( '[1@None] Bulk-banning users %d-%d [%d].', From 7ef1d854dc1c1821d3a3c1b5b767f0a63d264402 Mon Sep 17 00:00:00 2001 From: Mathieu Pillard Date: Fri, 22 May 2026 21:25:06 +0200 Subject: [PATCH 4/4] Avoiding create_chunked_signatures --- src/olympia/users/admin.py | 15 ++++++++------- src/olympia/users/tasks.py | 1 + 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/olympia/users/admin.py b/src/olympia/users/admin.py index 1f4a3dc479d0..915cd3efcf80 100644 --- a/src/olympia/users/admin.py +++ b/src/olympia/users/admin.py @@ -25,9 +25,12 @@ from olympia.activity.models import ActivityLog, RequestFingerprintLog from olympia.addons.models import Addon, AddonUser from olympia.amo.admin import AMOModelAdmin -from olympia.amo.celery import create_chunked_tasks_signatures from olympia.amo.templatetags.jinja_helpers import vite_asset -from olympia.amo.utils import backup_storage_enabled, create_signed_url_for_file_backup +from olympia.amo.utils import ( + backup_storage_enabled, + chunked, + create_signed_url_for_file_backup, +) from olympia.api.models import APIKey, APIKeyConfirmation from olympia.bandwagon.models import Collection from olympia.constants.activity import LOG_STORE_IPS @@ -461,9 +464,7 @@ def bulk_ban_view(self, request, extra_context=None): user_ids_to_ban = form.cleaned_data['user_ids'] form.fields['user_ids'].widget = HiddenInput() if request.POST.get('post'): - self._trigger_bulk_ban_task( - request, - ) + self._trigger_bulk_ban_task(request, user_ids_to_ban) return HttpResponseRedirect( reverse('admin:users_userprofile_changelist') ) @@ -483,8 +484,8 @@ def bulk_ban_view(self, request, extra_context=None): ) def _trigger_bulk_ban_task(self, request, user_ids): - workflow = create_chunked_tasks_signatures(bulk_ban, list(user_ids), 50) - workflow.apply_async() + for chunk in chunked(user_ids, 50): + bulk_ban.delay(list(chunk)) self.message_user( request, f'Bulk-ban for {len(user_ids)} user id(s) will be processed shortly.', diff --git a/src/olympia/users/tasks.py b/src/olympia/users/tasks.py index a0def442def4..5f587c0d7b55 100644 --- a/src/olympia/users/tasks.py +++ b/src/olympia/users/tasks.py @@ -281,6 +281,7 @@ def bulk_add_disposable_email_domains(entries: list[tuple[str, str]], batch_size @task +@use_primary_db def bulk_ban(ids): task_log.info( '[1@None] Bulk-banning users %d-%d [%d].',