diff --git a/lms/djangoapps/discussion/admin.py b/lms/djangoapps/discussion/admin.py new file mode 100644 index 000000000000..0234217527b6 --- /dev/null +++ b/lms/djangoapps/discussion/admin.py @@ -0,0 +1,329 @@ +""" +Django Admin configuration for discussion moderation models. + +Following edX best practices: +- Read-only for most users (view-only audit logs) +- Write access restricted to superusers +- Staff can view but not modify +""" + +from django.contrib import admin +from django.utils.html import format_html +from django.utils.translation import gettext_lazy as _ + +from lms.djangoapps.discussion.models import ( + DiscussionBan, + DiscussionBanException, + DiscussionModerationLog, +) + + +class ReadOnlyForNonSuperuserMixin: + """ + Mixin to make admin read-only for non-superusers. + + Superusers can add/change/delete, but regular staff can only view. + This is useful for audit/compliance access while preventing accidental changes. + """ + + def has_add_permission(self, request): + """Only superusers can add new records.""" + if request.user.is_superuser: + return super().has_add_permission(request) + return False + + def has_change_permission(self, request, obj=None): + """Only superusers can modify records. Staff can view.""" + if request.user.is_superuser: + return super().has_change_permission(request, obj) + # Staff users can view (needed for list view) but fields will be readonly + return request.user.is_staff + + def has_delete_permission(self, request, obj=None): + """Only superusers can delete records.""" + if request.user.is_superuser: + return super().has_delete_permission(request, obj) + return False + + def get_readonly_fields(self, request, obj=None): + """Make all fields readonly for non-superusers.""" + if not request.user.is_superuser: + # Return all fields as readonly for staff (non-superuser) + return [field.name for field in self.model._meta.fields] + return super().get_readonly_fields(request, obj) + + +@admin.register(DiscussionBan) +class DiscussionBanAdmin(ReadOnlyForNonSuperuserMixin, admin.ModelAdmin): + """ + Admin interface for Discussion Bans. + + Permissions: + - Superusers: Full access (view, add, change, delete) + - Staff: View-only (for audit/support purposes) + - Others: No access + """ + + list_display = [ + 'id', + 'user_link', + 'scope', + 'course_or_org', + 'is_active', + 'banned_at', + 'banned_by_link', + 'reason_preview', + ] + + list_filter = [ + 'scope', + 'is_active', + 'banned_at', + ] + + search_fields = [ + 'user__username', + 'user__email', + 'course_id', + 'org_key', + 'reason', + 'banned_by__username', + ] + + readonly_fields = [ + 'banned_at', + 'unbanned_at', + 'created', + 'modified', + ] + + fieldsets = ( + (_('Ban Information'), { + 'fields': ( + 'user', + 'scope', + 'course_id', + 'org_key', + 'is_active', + ) + }), + (_('Moderation Details'), { + 'fields': ( + 'banned_by', + 'reason', + 'banned_at', + 'unbanned_by', + 'unbanned_at', + ) + }), + (_('Timestamps'), { + 'fields': ( + 'created', + 'modified', + ), + 'classes': ('collapse',), + }), + ) + + date_hierarchy = 'banned_at' + + def user_link(self, obj): + """Display user with link to user admin.""" + if obj.user: + from django.urls import reverse + url = reverse('admin:auth_user_change', args=[obj.user.id]) + return format_html('{}', url, obj.user.username) + return '-' + user_link.short_description = _('User') + + def banned_by_link(self, obj): + """Display moderator with link to user admin.""" + if obj.banned_by: + from django.urls import reverse + url = reverse('admin:auth_user_change', args=[obj.banned_by.id]) + return format_html('{}', url, obj.banned_by.username) + return '-' + banned_by_link.short_description = _('Banned By') + + def course_or_org(self, obj): + """Display either course_id or organization based on scope.""" + if obj.scope == 'course': + return obj.course_id or '-' + else: + return obj.org_key or '-' + course_or_org.short_description = _('Course/Org') + + def reason_preview(self, obj): + """Display truncated reason.""" + if obj.reason: + return obj.reason[:100] + '...' if len(obj.reason) > 100 else obj.reason + return '-' + reason_preview.short_description = _('Reason') + + +@admin.register(DiscussionBanException) +class DiscussionBanExceptionAdmin(ReadOnlyForNonSuperuserMixin, admin.ModelAdmin): + """ + Admin interface for Ban Exceptions. + + Allows viewing course-specific exceptions to organization-level bans. + """ + + list_display = [ + 'id', + 'ban_link', + 'course_id', + 'unbanned_by_link', + 'created', + ] + + list_filter = [ + 'created', + ] + + search_fields = [ + 'ban__user__username', + 'course_id', + 'unbanned_by__username', + 'reason', + ] + + readonly_fields = [ + 'created', + 'modified', + ] + + fieldsets = ( + (_('Exception Information'), { + 'fields': ( + 'ban', + 'course_id', + 'unbanned_by', + 'reason', + ) + }), + (_('Timestamps'), { + 'fields': ( + 'created', + 'modified', + ), + 'classes': ('collapse',), + }), + ) + + date_hierarchy = 'created' + + def ban_link(self, obj): + """Display link to parent ban.""" + if obj.ban: + from django.urls import reverse + url = reverse('admin:discussion_discussionban_change', args=[obj.ban.id]) + return format_html( + 'Ban #{} - {}', url, obj.ban.id, obj.ban.user.username + ) + return '-' + ban_link.short_description = _('Parent Ban') + + def unbanned_by_link(self, obj): + """Display unbanner with link.""" + if obj.unbanned_by: + from django.urls import reverse + url = reverse('admin:auth_user_change', args=[obj.unbanned_by.id]) + return format_html('{}', url, obj.unbanned_by.username) + return '-' + unbanned_by_link.short_description = _('Unbanned By') + + +@admin.register(DiscussionModerationLog) +class DiscussionModerationLogAdmin(ReadOnlyForNonSuperuserMixin, admin.ModelAdmin): + """ + Admin interface for Moderation Audit Logs. + + IMPORTANT: This is an audit log and should be READ-ONLY for all users + (even superusers in production). Only use for compliance/investigation. + """ + + list_display = [ + 'id', + 'action_type', + 'target_user_link', + 'moderator_link', + 'course_id', + 'scope', + 'created', + ] + + list_filter = [ + 'action_type', + 'scope', + 'created', + ] + + search_fields = [ + 'target_user__username', + 'target_user__email', + 'moderator__username', + 'course_id', + 'reason', + ] + + readonly_fields = [ + 'action_type', + 'target_user', + 'moderator', + 'course_id', + 'scope', + 'reason', + 'metadata', + 'created', + ] + + fieldsets = ( + (_('Action Details'), { + 'fields': ( + 'action_type', + 'target_user', + 'moderator', + 'course_id', + 'scope', + ) + }), + (_('Context'), { + 'fields': ( + 'reason', + 'metadata', + ) + }), + (_('Timestamp'), { + 'fields': ('created',), + }), + ) + + date_hierarchy = 'created' + + # Disable add/delete for audit logs - even for superusers + def has_add_permission(self, request): + """Audit logs should never be manually created.""" + return False + + def has_delete_permission(self, request, obj=None): + """Audit logs should never be deleted.""" + return False + + def target_user_link(self, obj): + """Display target user with link.""" + if obj.target_user: + from django.urls import reverse + url = reverse('admin:auth_user_change', args=[obj.target_user.id]) + return format_html('{}', url, obj.target_user.username) + return '-' + target_user_link.short_description = _('Target User') + + def moderator_link(self, obj): + """Display moderator with link.""" + if obj.moderator: + from django.urls import reverse + url = reverse('admin:auth_user_change', args=[obj.moderator.id]) + return format_html('{}', url, obj.moderator.username) + return '-' + moderator_link.short_description = _('Moderator') diff --git a/lms/djangoapps/discussion/migrations/0001_initial.py b/lms/djangoapps/discussion/migrations/0001_initial.py new file mode 100644 index 000000000000..63d162ca9f69 --- /dev/null +++ b/lms/djangoapps/discussion/migrations/0001_initial.py @@ -0,0 +1,131 @@ +# Generated migration for discussion moderation models + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone +import model_utils.fields +import opaque_keys.edx.django.models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name='DiscussionBan', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created', model_utils.fields.AutoCreatedField(default=django.utils.timezone.now, editable=False, verbose_name='created')), + ('modified', model_utils.fields.AutoLastModifiedField(default=django.utils.timezone.now, editable=False, verbose_name='modified')), + ('course_id', opaque_keys.edx.django.models.CourseKeyField(blank=True, db_index=True, help_text='Specific course for course-level bans, NULL for org-level bans', max_length=255, null=True)), + ('org_key', models.CharField(blank=True, db_index=True, help_text="Organization name for org-level bans (e.g., 'HarvardX'), NULL for course-level", max_length=255, null=True)), + ('scope', models.CharField(choices=[('course', 'Course'), ('organization', 'Organization')], db_index=True, default='course', max_length=20)), + ('is_active', models.BooleanField(db_index=True, default=True)), + ('reason', models.TextField()), + ('banned_at', models.DateTimeField(auto_now_add=True)), + ('unbanned_at', models.DateTimeField(blank=True, null=True)), + ('banned_by', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='bans_issued', to=settings.AUTH_USER_MODEL)), + ('unbanned_by', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='bans_reversed', to=settings.AUTH_USER_MODEL)), + ('user', models.ForeignKey(db_index=True, on_delete=django.db.models.deletion.CASCADE, related_name='discussion_bans', to=settings.AUTH_USER_MODEL)), + ], + options={ + 'verbose_name': 'Discussion Ban', + 'verbose_name_plural': 'Discussion Bans', + 'db_table': 'discussion_user_ban', + }, + ), + migrations.CreateModel( + name='DiscussionModerationLog', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('action_type', models.CharField(choices=[('ban_user', 'Ban User'), ('unban_user', 'Unban User'), ('ban_exception', 'Ban Exception Created'), ('bulk_delete', 'Bulk Delete')], db_index=True, max_length=50)), + ('course_id', opaque_keys.edx.django.models.CourseKeyField(db_index=True, max_length=255)), + ('scope', models.CharField(blank=True, max_length=20, null=True)), + ('reason', models.TextField(blank=True, null=True)), + ('metadata', models.JSONField(blank=True, null=True)), + ('created', models.DateTimeField(auto_now_add=True, db_index=True)), + ('moderator', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='moderation_actions_performed', to=settings.AUTH_USER_MODEL)), + ('target_user', models.ForeignKey(db_index=True, on_delete=django.db.models.deletion.CASCADE, related_name='moderation_actions_received', to=settings.AUTH_USER_MODEL)), + ], + options={ + 'verbose_name': 'Discussion Moderation Log', + 'verbose_name_plural': 'Discussion Moderation Logs', + 'db_table': 'discussion_moderation_log', + }, + ), + migrations.CreateModel( + name='DiscussionBanException', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created', model_utils.fields.AutoCreatedField(default=django.utils.timezone.now, editable=False, verbose_name='created')), + ('modified', model_utils.fields.AutoLastModifiedField(default=django.utils.timezone.now, editable=False, verbose_name='modified')), + ('course_id', opaque_keys.edx.django.models.CourseKeyField(db_index=True, help_text='Specific course where user is unbanned despite org-level ban', max_length=255)), + ('reason', models.TextField(blank=True, null=True)), + ('ban', models.ForeignKey(help_text='The organization-level ban this exception applies to', on_delete=django.db.models.deletion.CASCADE, related_name='exceptions', to='discussion.discussionban')), + ('unbanned_by', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='ban_exceptions_created', to=settings.AUTH_USER_MODEL)), + ], + options={ + 'verbose_name': 'Discussion Ban Exception', + 'verbose_name_plural': 'Discussion Ban Exceptions', + 'db_table': 'discussion_ban_exception', + }, + ), + migrations.AddIndex( + model_name='discussionmoderationlog', + index=models.Index(fields=['target_user', '-created'], name='idx_target_user'), + ), + migrations.AddIndex( + model_name='discussionmoderationlog', + index=models.Index(fields=['moderator', '-created'], name='idx_moderator'), + ), + migrations.AddIndex( + model_name='discussionmoderationlog', + index=models.Index(fields=['course_id', '-created'], name='idx_course'), + ), + migrations.AddIndex( + model_name='discussionmoderationlog', + index=models.Index(fields=['action_type', '-created'], name='idx_action_type'), + ), + migrations.AddConstraint( + model_name='discussionbanexception', + constraint=models.UniqueConstraint(fields=('ban', 'course_id'), name='unique_ban_exception'), + ), + migrations.AddIndex( + model_name='discussionbanexception', + index=models.Index(fields=['ban', 'course_id'], name='idx_ban_course'), + ), + migrations.AddIndex( + model_name='discussionbanexception', + index=models.Index(fields=['course_id'], name='idx_exception_course'), + ), + migrations.AddIndex( + model_name='discussionban', + index=models.Index(fields=['user', 'is_active'], name='idx_user_active'), + ), + migrations.AddIndex( + model_name='discussionban', + index=models.Index(fields=['course_id', 'is_active'], name='idx_course_active'), + ), + migrations.AddIndex( + model_name='discussionban', + index=models.Index(fields=['org_key', 'is_active'], name='idx_org_active'), + ), + migrations.AddIndex( + model_name='discussionban', + index=models.Index(fields=['scope', 'is_active'], name='idx_scope_active'), + ), + migrations.AddConstraint( + model_name='discussionban', + constraint=models.UniqueConstraint(condition=models.Q(('is_active', True), ('scope', 'course')), fields=('user', 'course_id'), name='unique_active_course_ban'), + ), + migrations.AddConstraint( + model_name='discussionban', + constraint=models.UniqueConstraint(condition=models.Q(('is_active', True), ('scope', 'organization')), fields=('user', 'org_key'), name='unique_active_org_ban'), + ), + ] diff --git a/lms/djangoapps/discussion/migrations/__init__.py b/lms/djangoapps/discussion/migrations/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/lms/djangoapps/discussion/models.py b/lms/djangoapps/discussion/models.py new file mode 100644 index 000000000000..73134894f152 --- /dev/null +++ b/lms/djangoapps/discussion/models.py @@ -0,0 +1,309 @@ +""" +Django models for discussion moderation features. +""" +from django.contrib.auth import get_user_model +from django.core.exceptions import ValidationError +from django.db import models +from django.utils.translation import gettext_lazy as _ +from model_utils.models import TimeStampedModel +from opaque_keys.edx.django.models import CourseKeyField + +User = get_user_model() + + +class DiscussionBan(TimeStampedModel): + """ + Tracks users banned from course or organization discussions. + + Uses edX standard patterns: + - TimeStampedModel for created/modified timestamps + - CourseKeyField for course_id + - Soft delete pattern with is_active flag + """ + + SCOPE_COURSE = 'course' + SCOPE_ORGANIZATION = 'organization' + SCOPE_CHOICES = [ + (SCOPE_COURSE, _('Course')), + (SCOPE_ORGANIZATION, _('Organization')), + ] + + # Core Fields + user = models.ForeignKey( + User, + on_delete=models.CASCADE, + related_name='discussion_bans', + db_index=True, + ) + course_id = CourseKeyField( + max_length=255, + db_index=True, + null=True, + blank=True, + help_text="Specific course for course-level bans, NULL for org-level bans" + ) + org_key = models.CharField( + max_length=255, + db_index=True, + null=True, + blank=True, + help_text="Organization name for org-level bans (e.g., 'HarvardX'), NULL for course-level" + ) + scope = models.CharField( + max_length=20, + choices=SCOPE_CHOICES, + default=SCOPE_COURSE, + db_index=True, + ) + is_active = models.BooleanField(default=True, db_index=True) + + # Metadata + banned_by = models.ForeignKey( + User, + on_delete=models.SET_NULL, + null=True, + related_name='bans_issued', + ) + reason = models.TextField() + banned_at = models.DateTimeField(auto_now_add=True) + unbanned_at = models.DateTimeField(null=True, blank=True) + unbanned_by = models.ForeignKey( + User, + on_delete=models.SET_NULL, + null=True, + blank=True, + related_name='bans_reversed', + ) + + class Meta: + db_table = 'discussion_user_ban' + indexes = [ + models.Index(fields=['user', 'is_active'], name='idx_user_active'), + models.Index(fields=['course_id', 'is_active'], name='idx_course_active'), + models.Index(fields=['org_key', 'is_active'], name='idx_org_active'), + models.Index(fields=['scope', 'is_active'], name='idx_scope_active'), + ] + constraints = [ + # Prevent duplicate course-level bans + models.UniqueConstraint( + fields=['user', 'course_id'], + condition=models.Q(is_active=True, scope='course'), + name='unique_active_course_ban' + ), + # Prevent duplicate org-level bans + models.UniqueConstraint( + fields=['user', 'org_key'], + condition=models.Q(is_active=True, scope='organization'), + name='unique_active_org_ban' + ), + # Note: Scope-based field validation is done in clean() method + # CheckConstraints don't work well with CourseKeyField due to opaque_keys limitations + ] + verbose_name = _('Discussion Ban') + verbose_name_plural = _('Discussion Bans') + + def __str__(self): + if self.scope == self.SCOPE_COURSE: + return f"Ban: {self.user.username} in {self.course_id} (course-level)" + else: + return f"Ban: {self.user.username} in {self.org_key} (org-level)" + + def clean(self): + """Validate scope-based field requirements.""" + super().clean() + if self.scope == self.SCOPE_COURSE: + if not self.course_id: + raise ValidationError(_("Course-level bans require course_id")) + elif self.scope == self.SCOPE_ORGANIZATION: + if not self.org_key: + raise ValidationError(_("Organization-level bans require organization")) + if self.course_id: + raise ValidationError(_("Organization-level bans should not have course_id set")) + + @classmethod + def is_user_banned(cls, user, course_id, check_org=True): + """ + Check if user is banned from discussions. + + Priority: + 1. Check for course-level exception to org ban (allows user) + 2. Organization-level ban (applies to all courses in org) + 3. Course-level ban (applies to specific course) + + Args: + user: User object + course_id: CourseKey or string + check_org: If True, also check organization-level bans + + Returns: + bool: True if user has active ban + """ + from openedx.core.djangoapps.content.course_overviews.models import CourseOverview + from opaque_keys.edx.keys import CourseKey + + # Normalize course_id to CourseKey + if isinstance(course_id, str): + course_id = CourseKey.from_string(course_id) + + # Check organization-level ban first (higher priority) + if check_org: + # Try to get organization from CourseOverview, fallback to CourseKey + try: + course = CourseOverview.objects.get(id=course_id) + org_name = course.org + except CourseOverview.DoesNotExist: + # Fallback: extract org directly from course_id + org_name = course_id.org + + # Check if org-level ban exists + org_ban = cls.objects.filter( + user=user, + org_key=org_name, + scope=cls.SCOPE_ORGANIZATION, + is_active=True + ).first() + + if org_ban: + # Check if there's an exception for this specific course + if DiscussionBanException.objects.filter( + ban=org_ban, + course_id=course_id + ).exists(): + # Exception exists - user is allowed in this course + return False + # Org ban applies, no exception + return True + + # Check course-level ban + if cls.objects.filter( + user=user, + course_id=course_id, + scope=cls.SCOPE_COURSE, + is_active=True + ).exists(): + return True + + return False + + +class DiscussionBanException(TimeStampedModel): + """ + Tracks course-level exceptions to organization-level bans. + + Allows moderators to unban a user from specific courses while + maintaining an organization-wide ban for all other courses. + + Uses edX standard patterns: + - TimeStampedModel for created/modified timestamps + + Example: + - User banned from all HarvardX courses (org-level ban) + - Exception created for HarvardX+CS50+2024 + - User can participate in CS50 but remains banned in all other HarvardX courses + """ + + # Core Fields + ban = models.ForeignKey( + 'DiscussionBan', + on_delete=models.CASCADE, + related_name='exceptions', + help_text="The organization-level ban this exception applies to" + ) + course_id = CourseKeyField( + max_length=255, + db_index=True, + help_text="Specific course where user is unbanned despite org-level ban" + ) + + # Metadata + unbanned_by = models.ForeignKey( + User, + on_delete=models.SET_NULL, + null=True, + related_name='ban_exceptions_created', + ) + reason = models.TextField(null=True, blank=True) + + class Meta: + db_table = 'discussion_ban_exception' + constraints = [ + models.UniqueConstraint( + fields=['ban', 'course_id'], + name='unique_ban_exception' + ), + ] + indexes = [ + models.Index(fields=['ban', 'course_id'], name='idx_ban_course'), + models.Index(fields=['course_id'], name='idx_exception_course'), + ] + verbose_name = _('Discussion Ban Exception') + verbose_name_plural = _('Discussion Ban Exceptions') + + def __str__(self): + return f"Exception: {self.ban.user.username} allowed in {self.course_id}" + + def clean(self): + """Validate that exception only applies to organization-level bans.""" + super().clean() + if self.ban.scope != 'organization': + raise ValidationError(_("Exceptions can only be created for organization-level bans")) + + +class DiscussionModerationLog(models.Model): + """ + Audit log for discussion moderation actions. + + Tracks ban, unban, and bulk delete actions for compliance. + """ + + ACTION_BAN = 'ban_user' + ACTION_UNBAN = 'unban_user' + ACTION_BAN_EXCEPTION = 'ban_exception' + ACTION_BULK_DELETE = 'bulk_delete' + + ACTION_CHOICES = [ + (ACTION_BAN, _('Ban User')), + (ACTION_UNBAN, _('Unban User')), + (ACTION_BAN_EXCEPTION, _('Ban Exception Created')), + (ACTION_BULK_DELETE, _('Bulk Delete')), + ] + + # Action Details + action_type = models.CharField(max_length=50, choices=ACTION_CHOICES, db_index=True) + target_user = models.ForeignKey( + User, + on_delete=models.CASCADE, + related_name='moderation_actions_received', + db_index=True, + ) + moderator = models.ForeignKey( + User, + on_delete=models.SET_NULL, + null=True, + related_name='moderation_actions_performed', + db_index=True, + ) + course_id = CourseKeyField(max_length=255, db_index=True) + + # Context + scope = models.CharField(max_length=20, null=True, blank=True) + reason = models.TextField(null=True, blank=True) + metadata = models.JSONField(null=True, blank=True) # Task IDs, counts, etc. + + # Timestamp + created = models.DateTimeField(auto_now_add=True, db_index=True) + + class Meta: + db_table = 'discussion_moderation_log' + indexes = [ + models.Index(fields=['target_user', '-created'], name='idx_target_user'), + models.Index(fields=['moderator', '-created'], name='idx_moderator'), + models.Index(fields=['course_id', '-created'], name='idx_course'), + models.Index(fields=['action_type', '-created'], name='idx_action_type'), + ] + verbose_name = _('Discussion Moderation Log') + verbose_name_plural = _('Discussion Moderation Logs') + + def __str__(self): + moderator_name = self.moderator.username if self.moderator else 'System' + return f"{self.action_type}: {self.target_user.username} by {moderator_name}" diff --git a/lms/djangoapps/discussion/rest_api/emails.py b/lms/djangoapps/discussion/rest_api/emails.py new file mode 100644 index 000000000000..e4ebcc21a567 --- /dev/null +++ b/lms/djangoapps/discussion/rest_api/emails.py @@ -0,0 +1,170 @@ +""" +Email notifications for discussion moderation actions. +""" +import logging + +from django.conf import settings +from django.contrib.auth import get_user_model + +log = logging.getLogger(__name__) +User = get_user_model() + +# Try to import ACE at module level for easier testing +try: + from edx_ace import ace + from edx_ace.recipient import Recipient + from edx_ace.message import Message + ACE_AVAILABLE = True +except ImportError: + ace = None + Recipient = None + Message = None + ACE_AVAILABLE = False + + +def send_ban_escalation_email( + banned_user_id, + moderator_id, + course_id, + scope, + reason, + threads_deleted, + comments_deleted +): + """ + Send email to partner-support when user is banned. + + Uses ACE (Automated Communications Engine) for templated emails if available, + otherwise falls back to Django's email system. + + Args: + banned_user_id: ID of the banned user + moderator_id: ID of the moderator who applied the ban + course_id: Course ID where ban was applied + scope: 'course' or 'organization' + reason: Reason for the ban + threads_deleted: Number of threads deleted + comments_deleted: Number of comments deleted + """ + # Check if email notifications are enabled + if not getattr(settings, 'DISCUSSION_MODERATION_BAN_EMAIL_ENABLED', True): + log.info( + "Ban email notifications disabled by settings. " + "User %s banned in course %s (scope: %s)", + banned_user_id, course_id, scope + ) + return + + try: + banned_user = User.objects.get(id=banned_user_id) + moderator = User.objects.get(id=moderator_id) + + # Get escalation email from settings + escalation_email = getattr( + settings, + 'DISCUSSION_MODERATION_ESCALATION_EMAIL', + 'partner-support@edx.org' + ) + + # Try using ACE first (preferred method for edX) + if ACE_AVAILABLE and ace is not None: + message = Message( + app_label='discussion', + name='ban_escalation', + recipient=Recipient(lms_user_id=None, email_address=escalation_email), + context={ + 'banned_username': banned_user.username, + 'banned_email': banned_user.email, + 'banned_user_id': banned_user_id, + 'moderator_username': moderator.username, + 'moderator_email': moderator.email, + 'moderator_id': moderator_id, + 'course_id': str(course_id), + 'scope': scope, + 'reason': reason or 'No reason provided', + 'threads_deleted': threads_deleted, + 'comments_deleted': comments_deleted, + 'total_deleted': threads_deleted + comments_deleted, + } + ) + + ace.send(message) + log.info( + "Ban escalation email sent via ACE to %s for user %s in course %s", + escalation_email, banned_user.username, course_id + ) + + else: + # Fallback to Django's email system if ACE is not available + from django.core.mail import send_mail + from django.template.loader import render_to_string + from django.template import TemplateDoesNotExist + + context = { + 'banned_username': banned_user.username, + 'banned_email': banned_user.email, + 'banned_user_id': banned_user_id, + 'moderator_username': moderator.username, + 'moderator_email': moderator.email, + 'moderator_id': moderator_id, + 'course_id': str(course_id), + 'scope': scope, + 'reason': reason or 'No reason provided', + 'threads_deleted': threads_deleted, + 'comments_deleted': comments_deleted, + 'total_deleted': threads_deleted + comments_deleted, + } + + # Try to render template, fall back to plain text if template doesn't exist + try: + email_body = render_to_string( + 'discussion/ban_escalation_email.txt', + context + ) + except TemplateDoesNotExist: + # Plain text fallback + banned_user_info = "{} ({})".format(banned_user.username, banned_user.email) + moderator_info = "{} ({})".format(moderator.username, moderator.email) + email_body = """ +A user has been banned from discussions: + +Banned User: {} +Moderator: {} +Course: {} +Scope: {} +Reason: {} +Content Deleted: {} threads, {} comments + +Please review this moderation action and follow up as needed. +""".format( + banned_user_info, + moderator_info, + course_id, + scope, + reason or 'No reason provided', + threads_deleted, + comments_deleted + ) + + subject = f'Discussion Ban Alert: {banned_user.username} in {course_id}' + from_email = getattr(settings, 'DEFAULT_FROM_EMAIL', 'no-reply@example.com') + + send_mail( + subject=subject, + message=email_body, + from_email=from_email, + recipient_list=[escalation_email], + fail_silently=False, + ) + + log.info( + "Ban escalation email sent via Django mail to %s for user %s in course %s", + escalation_email, banned_user.username, course_id + ) + + except User.DoesNotExist as e: + log.error("Failed to send ban escalation email: User not found - %s", str(e)) + raise + except Exception as exc: + log.error("Failed to send ban escalation email: %s", str(exc), exc_info=True) + raise diff --git a/lms/djangoapps/discussion/rest_api/permissions.py b/lms/djangoapps/discussion/rest_api/permissions.py index cfcea5b32834..31cef934ce75 100644 --- a/lms/djangoapps/discussion/rest_api/permissions.py +++ b/lms/djangoapps/discussion/rest_api/permissions.py @@ -6,7 +6,7 @@ from opaque_keys.edx.keys import CourseKey from rest_framework import permissions -from common.djangoapps.student.models import CourseAccessRole, CourseEnrollment +from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.student.roles import ( CourseInstructorRole, CourseStaffRole, @@ -189,42 +189,90 @@ def has_permission(self, request, view): def can_take_action_on_spam(user, course_id): """ - Returns if the user has access to take action against forum spam posts + Returns if the user has access to take action against forum spam posts. + + Grants access to: + - Global Staff (user.is_staff or GlobalStaff role) + - Course Staff for the specific course + - Course Instructors for the specific course + - Forum Moderators for the specific course + - Forum Administrators for the specific course + Parameters: user: User object course_id: CourseKey or string of course_id + + Returns: + bool: True if user can take action on spam, False otherwise """ - if GlobalStaff().has_user(user): + # Global staff have universal access + if GlobalStaff().has_user(user) or user.is_staff: return True if isinstance(course_id, str): course_id = CourseKey.from_string(course_id) - org_id = course_id.org - course_ids = CourseEnrollment.objects.filter(user=user).values_list('course_id', flat=True) - course_ids = [c_id for c_id in course_ids if c_id.org == org_id] + + # Check if user is Course Staff or Instructor for this specific course + if CourseStaffRole(course_id).has_user(user): + return True + + if CourseInstructorRole(course_id).has_user(user): + return True + + # Check forum moderator/administrator roles for this specific course user_roles = set( Role.objects.filter( users=user, - course_id__in=course_ids, - ).values_list('name', flat=True).distinct() + course_id=course_id, + ).values_list('name', flat=True) ) - if bool(user_roles & {FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR}): - return True - if CourseAccessRole.objects.filter(user=user, course_id__in=course_ids, role__in=["instructor", "staff"]).exists(): + if user_roles & {FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR}: return True + return False class IsAllowedToBulkDelete(permissions.BasePermission): """ - Permission that checks if the user is staff or an admin. + Permission that checks if the user is allowed to perform bulk delete and ban operations. + + Grants access to: + - Global Staff (superusers) + - Course Staff + - Course Instructors + - Forum Moderators + - Forum Administrators + + Denies access to: + - Unauthenticated users + - Regular students + - Community TAs (they can moderate individual posts but not bulk delete) """ def has_permission(self, request, view): - """Returns true if the user can bulk delete posts""" + """ + Returns True if the user can bulk delete posts and ban users. + + For ViewSet actions, course_id may come from: + 1. URL kwargs (view.kwargs.get('course_id')) + 2. Query parameters (request.query_params.get('course_id')) + 3. Request body (request.data.get('course_id')) + """ if not request.user.is_authenticated: return False - course_id = view.kwargs.get("course_id") + # Try to get course_id from different sources + course_id = ( + view.kwargs.get("course_id") or + request.query_params.get("course_id") or + (request.data.get("course_id") if hasattr(request, 'data') else None) + ) + + # If no course_id provided, we can't check permissions yet + # Let the view handle validation of required course_id + if not course_id: + # For safety, only allow global staff to proceed without course_id + return GlobalStaff().has_user(request.user) or request.user.is_staff + return can_take_action_on_spam(request.user, course_id) diff --git a/lms/djangoapps/discussion/rest_api/serializers.py b/lms/djangoapps/discussion/rest_api/serializers.py index 9c2668d0b226..67c2a411bc0b 100644 --- a/lms/djangoapps/discussion/rest_api/serializers.py +++ b/lms/djangoapps/discussion/rest_api/serializers.py @@ -153,8 +153,8 @@ def filter_spam_urls_from_html(html_string): is_spam = False for domain in settings.DISCUSSION_SPAM_URLS: escaped = domain.replace(".", r"\.") - domain_pattern = rf"(\w+\.)*{escaped}(?:/\S*)*" - patterns.append(re.compile(rf"(https?://)?{domain_pattern}", re.IGNORECASE)) + domain_pattern = r"(\w+\.)*{}(?:/\S*)*".format(escaped) + patterns.append(re.compile(r"(https?:)?{}".format(domain_pattern), re.IGNORECASE)) for a_tag in soup.find_all("a", href=True): href = a_tag.get('href') @@ -944,3 +944,56 @@ class CourseMetadataSerailizer(serializers.Serializer): child=ReasonCodeSeralizer(), help_text="A list of reasons that can be specified by moderators for editing a post, response, or comment", ) + + +class BulkDeleteBanRequestSerializer(serializers.Serializer): + """Request payload for bulk delete + ban action.""" + + user_id = serializers.IntegerField(required=True) + course_id = serializers.CharField(max_length=255, required=True) + ban_user = serializers.BooleanField(default=False) + ban_scope = serializers.ChoiceField( + choices=['course', 'organization'], + default='course', + help_text="Scope of the ban: 'course' for course-level or 'organization' for organization-level" + ) + reason = serializers.CharField( + required=False, + allow_blank=True, + max_length=1000 + ) + + def validate(self, data): + """Validate that reason is provided when ban_user is True.""" + if data.get('ban_user'): + reason = data.get('reason', '').strip() + if not reason: + raise serializers.ValidationError({ + 'reason': "Reason is required when banning a user." + }) + + # Validate that organization-level bans require elevated permissions + if data.get('ban_scope') == 'organization': + request = self.context.get('request') + if request and not GlobalStaff().has_user(request.user): + raise serializers.ValidationError({ + 'ban_scope': "Organization-level bans require global staff permissions." + }) + + return data + + +class BannedUserSerializer(serializers.Serializer): + """Banned user information for list view.""" + + id = serializers.IntegerField(read_only=True) + username = serializers.CharField(source='user.username', read_only=True) + email = serializers.EmailField(source='user.email', read_only=True) + user_id = serializers.IntegerField(source='user.id', read_only=True) + course_id = serializers.CharField(read_only=True) + organization = serializers.CharField(source='org_key', read_only=True) + scope = serializers.CharField(read_only=True) + reason = serializers.CharField(read_only=True) + banned_at = serializers.DateTimeField(read_only=True) + banned_by_username = serializers.CharField(source='banned_by.username', read_only=True) + is_active = serializers.BooleanField(read_only=True) diff --git a/lms/djangoapps/discussion/rest_api/tasks.py b/lms/djangoapps/discussion/rest_api/tasks.py index cd725a3513dc..c4b72a287143 100644 --- a/lms/djangoapps/discussion/rest_api/tasks.py +++ b/lms/djangoapps/discussion/rest_api/tasks.py @@ -5,6 +5,8 @@ from celery import shared_task from django.contrib.auth import get_user_model +from django.db import transaction +from django.utils import timezone from edx_django_utils.monitoring import set_code_owner_attribute from opaque_keys.edx.locator import CourseKey from eventtracking import tracker @@ -92,22 +94,222 @@ def send_response_endorsed_notifications(thread_id, response_id, course_key_str, notification_sender.send_response_endorsed_notification() -@shared_task +@shared_task( + bind=True, # Enable retry context and access to task instance + max_retries=3, # Retry up to 3 times on failure + default_retry_delay=60, # Wait 60 seconds between retries + autoretry_for=(OSError, TimeoutError), # Only retry on transient network/IO errors + retry_backoff=True, # Exponential backoff between retries + retry_jitter=True, # Add randomization to retry delays +) @set_code_owner_attribute -def delete_course_post_for_user(user_id, username, course_ids, event_data=None): +def delete_course_post_for_user( # pylint: disable=too-many-statements + self, + user_id, + username=None, + course_ids=None, + event_data=None, + # NEW PARAMETERS (backward compatible - all have defaults): + ban_user=False, + ban_scope='course', + moderator_id=None, + reason=None, +): """ - Deletes all posts for user in a course. + Delete all discussion posts for a user and optionally ban them. + + BACKWARD COMPATIBLE: Existing callers without ban_user parameter + will experience no change in behavior. + + Args: + self: Task instance (when bind=True) + user_id: User whose posts to delete + username: Username of the user (optional, will be fetched if not provided) + course_ids: List of course IDs (API sends single course wrapped in array) + event_data: Event tracking metadata + ban_user: If True, create ban record (NEW) + ban_scope: 'course' or 'organization' (NEW) + moderator_id: Moderator applying ban (NEW) + reason: Ban reason (NEW) """ + from django.db.utils import OperationalError, InterfaceError + event_data = event_data or {} - log.info(f"<> Deleting all posts for {username} in course {course_ids}") - threads_deleted = Thread.delete_user_threads(user_id, course_ids) - comments_deleted = Comment.delete_user_comments(user_id, course_ids) - log.info(f"<> Deleted {threads_deleted} posts and {comments_deleted} comments for {username} " - f"in course {course_ids}") - event_data.update({ - "number_of_posts_deleted": threads_deleted, - "number_of_comments_deleted": comments_deleted, - }) - event_name = 'edx.discussion.bulk_delete_user_posts' - tracker.emit(event_name, event_data) - segment.track('None', event_name, event_data) + + try: + user = User.objects.get(id=user_id) + if username is None: + username = user.username + + log.info( + "Task %s: Deleting posts for user=%s, courses=%s, ban=%s", + self.request.id, username, course_ids, ban_user + ) + + # Phase 1: Delete content (EXISTING - unchanged) + threads_deleted = Thread.delete_user_threads(user_id, course_ids) + comments_deleted = Comment.delete_user_comments(user_id, course_ids) + + log.info( + "Task %s: Deleted %d threads and %d comments for %s in courses %s", + self.request.id, threads_deleted, comments_deleted, username, course_ids + ) + + # Phase 2: Create ban record (NEW - only if ban_user=True) + if ban_user and moderator_id: + from lms.djangoapps.discussion.models import DiscussionBan + from openedx.core.djangoapps.content.course_overviews.models import CourseOverview + + with transaction.atomic(): + banned_user = User.objects.get(id=user_id) + moderator = User.objects.get(id=moderator_id) + + # Extract organization from course for consistency + course_key = CourseKey.from_string(course_ids[0]) + try: + course = CourseOverview.objects.get(id=course_key) + org_name = course.org + except CourseOverview.DoesNotExist: + # Fallback to extracting org from course key + org_name = course_key.org + + # Determine ban scope fields + if ban_scope == 'organization': + # Org-level ban: use course's org, leave course_id NULL + ban_kwargs = { + 'user': banned_user, + 'org_key': org_name, + 'scope': 'organization', + } + lookup_kwargs = { + 'user': banned_user, + 'org_key': org_name, + 'scope': 'organization', + } + else: + # Course-level ban: set course_id + ban_kwargs = { + 'user': banned_user, + 'course_id': course_ids[0], + 'org_key': org_name, # Denormalized for reporting + 'scope': 'course', + } + lookup_kwargs = { + 'user': banned_user, + 'course_id': course_ids[0], + 'scope': 'course', + } + + # Create or update ban + ban, created = DiscussionBan.objects.get_or_create( + **lookup_kwargs, + defaults={ + **ban_kwargs, + 'banned_by': moderator, + 'reason': reason or 'No reason provided', + 'is_active': True, + 'banned_at': timezone.now(), + } + ) + + if not created and not ban.is_active: + # Reactivate previously lifted ban + ban.is_active = True + ban.banned_by = moderator + ban.reason = reason or ban.reason + ban.banned_at = timezone.now() + ban.unbanned_at = None + ban.unbanned_by = None + ban.save() + + log.info( + "Task %s: Created/updated ban (id=%d) for user=%s, scope=%s", + self.request.id, ban.id, username, ban_scope + ) + + # Phase 3: Audit logging (NEW) + if ban_user and moderator_id: + from lms.djangoapps.discussion.models import DiscussionModerationLog + + with transaction.atomic(): + DiscussionModerationLog.objects.create( + action_type=DiscussionModerationLog.ACTION_BAN, + target_user_id=user_id, + moderator_id=moderator_id, + course_id=course_ids[0], + scope=ban_scope, + reason=reason, + metadata={ + 'threads_deleted': threads_deleted, + 'comments_deleted': comments_deleted, + 'task_id': self.request.id, + } + ) + + # Phase 4: Event tracking (ENHANCED) + event_data.update({ + "number_of_posts_deleted": threads_deleted, + "number_of_comments_deleted": comments_deleted, + 'ban_applied': ban_user, + 'ban_scope': ban_scope if ban_user else None, + }) + event_name = 'edx.discussion.bulk_delete_user_posts' + tracker.emit(event_name, event_data) + segment.track('None', event_name, event_data) + + # Phase 5: Email notification (NEW) + if ban_user and moderator_id: + # Check if email notifications are enabled before attempting to send + from django.conf import settings as django_settings + if getattr(django_settings, 'DISCUSSION_MODERATION_BAN_EMAIL_ENABLED', True): + from lms.djangoapps.discussion.rest_api.emails import send_ban_escalation_email + + try: + send_ban_escalation_email( + banned_user_id=user_id, + moderator_id=moderator_id, + course_id=course_ids[0], + scope=ban_scope, + reason=reason, + threads_deleted=threads_deleted, + comments_deleted=comments_deleted, + ) + except (OSError, ValueError, TypeError) as email_exc: + # Log but don't fail the task if email fails + # Catches: SMTP errors (OSError), template errors (ValueError), data errors (TypeError) + log.error( + "Task %s: Failed to send ban escalation email: %s", + self.request.id, str(email_exc) + ) + else: + log.info( + "Task %s: Email notifications disabled, skipping ban escalation email", + self.request.id + ) + + log.info( + "Task %s completed: user=%s, threads=%d, comments=%d, ban=%s", + self.request.id, username, threads_deleted, comments_deleted, ban_user + ) + + return { + 'threads_deleted': threads_deleted, + 'comments_deleted': comments_deleted, + 'ban_applied': ban_user, + 'task_id': self.request.id, + } + + except (OperationalError, InterfaceError, OSError, TimeoutError) as exc: + # Transient errors - let Celery retry + log.warning( + "Task %s retrying due to transient error: user_id=%s, error=%s", + self.request.id, user_id, str(exc) + ) + raise + except Exception as exc: + # Permanent errors - log and fail immediately + log.error( + "Task %s failed permanently: user_id=%s, error=%s", + self.request.id, user_id, str(exc), exc_info=True + ) + raise diff --git a/lms/djangoapps/discussion/rest_api/tests/test_moderation_emails.py b/lms/djangoapps/discussion/rest_api/tests/test_moderation_emails.py new file mode 100644 index 000000000000..9256596945e2 --- /dev/null +++ b/lms/djangoapps/discussion/rest_api/tests/test_moderation_emails.py @@ -0,0 +1,238 @@ +""" +Tests for discussion moderation email notifications. +""" +from unittest import mock +from django.test import override_settings +from django.core import mail + +from lms.djangoapps.discussion.rest_api.emails import send_ban_escalation_email +from common.djangoapps.student.tests.factories import UserFactory +from xmodule.modulestore.tests.factories import CourseFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase + + +class BanEscalationEmailTest(ModuleStoreTestCase): + """Tests for send_ban_escalation_email function.""" + + def setUp(self): + super().setUp() + self.course = CourseFactory.create(org='TestX', number='CS101', run='2024') + self.course_key = str(self.course.id) + self.banned_user = UserFactory.create(username='spammer', email='spammer@example.com') + self.moderator = UserFactory.create(username='moderator', email='mod@example.com') + + @override_settings(DISCUSSION_MODERATION_BAN_EMAIL_ENABLED=False) + def test_email_disabled_by_setting(self): + """Test that email is not sent when DISCUSSION_MODERATION_BAN_EMAIL_ENABLED is False.""" + # Clear outbox + mail.outbox = [] + + # Try to send email + send_ban_escalation_email( + banned_user_id=self.banned_user.id, + moderator_id=self.moderator.id, + course_id=self.course_key, + scope='course', + reason='Spam', + threads_deleted=5, + comments_deleted=10 + ) + + # No email should be sent + self.assertEqual(len(mail.outbox), 0) + + @override_settings( + DISCUSSION_MODERATION_BAN_EMAIL_ENABLED=True, + DISCUSSION_MODERATION_ESCALATION_EMAIL='partner-support@edx.org' + ) + @mock.patch('lms.djangoapps.discussion.rest_api.emails.ace') + def test_email_sent_via_ace(self, mock_ace_module): + """Test that email is sent via ACE when available.""" + # Create mock ACE send function + mock_send = mock.MagicMock() + mock_ace_module.send = mock_send + + send_ban_escalation_email( + banned_user_id=self.banned_user.id, + moderator_id=self.moderator.id, + course_id=self.course_key, + scope='course', + reason='Posting scam links', + threads_deleted=3, + comments_deleted=7 + ) + + # ACE send should be called + mock_send.assert_called_once() + + # Get the message argument + call_args = mock_send.call_args + message = call_args[0][0] + + # Verify message properties + self.assertEqual(message.recipient.email_address, 'partner-support@edx.org') + self.assertEqual(message.context['banned_username'], 'spammer') + self.assertEqual(message.context['moderator_username'], 'moderator') + self.assertEqual(message.context['scope'], 'course') + self.assertEqual(message.context['reason'], 'Posting scam links') + self.assertEqual(message.context['threads_deleted'], 3) + self.assertEqual(message.context['comments_deleted'], 7) + self.assertEqual(message.context['total_deleted'], 10) + + @override_settings( + DISCUSSION_MODERATION_BAN_EMAIL_ENABLED=True, + DISCUSSION_MODERATION_ESCALATION_EMAIL='custom-support@example.com', + DEFAULT_FROM_EMAIL='noreply@edx.org' + ) + @mock.patch('lms.djangoapps.discussion.rest_api.emails.ace', None) + def test_email_fallback_to_django_mail(self): + """Test that email falls back to Django mail when ACE is not available.""" + # Clear outbox + mail.outbox = [] + + # Simulate ACE not being importable by making the import fail + import sys + original_modules = sys.modules.copy() + + # Remove ace modules if present + ace_modules = [key for key in sys.modules if key.startswith('edx_ace')] + for mod in ace_modules: + sys.modules.pop(mod, None) + + try: + send_ban_escalation_email( + banned_user_id=self.banned_user.id, + moderator_id=self.moderator.id, + course_id=self.course_key, + scope='organization', + reason='Multiple violations', + threads_deleted=15, + comments_deleted=25 + ) + finally: + # Restore modules + sys.modules.update(original_modules) + + # Email should be sent via Django + self.assertEqual(len(mail.outbox), 1) + + email = mail.outbox[0] + self.assertIn('custom-support@example.com', email.to) + self.assertEqual(email.from_email, 'noreply@edx.org') + self.assertIn('spammer', email.body) + self.assertIn('moderator', email.body) + self.assertIn('Multiple violations', email.body) + self.assertIn('ORGANIZATION', email.body) + self.assertIn('15', email.body) # threads_deleted + self.assertIn('25', email.body) # comments_deleted + + @override_settings( + DISCUSSION_MODERATION_BAN_EMAIL_ENABLED=True, + DISCUSSION_MODERATION_ESCALATION_EMAIL='support@example.com' + ) + @mock.patch('lms.djangoapps.discussion.rest_api.emails.ace', None) + def test_email_handles_missing_reason(self): + """Test that email handles empty/None reason gracefully.""" + mail.outbox = [] + + # Send with empty reason (will use Django mail since ace is None) + send_ban_escalation_email( + banned_user_id=self.banned_user.id, + moderator_id=self.moderator.id, + course_id=self.course_key, + scope='course', + reason='', + threads_deleted=1, + comments_deleted=0 + ) + + self.assertEqual(len(mail.outbox), 1) + email = mail.outbox[0] + # Should use default text + self.assertIn('No reason provided', email.body) + + @override_settings( + DISCUSSION_MODERATION_BAN_EMAIL_ENABLED=True, + DISCUSSION_MODERATION_ESCALATION_EMAIL='support@example.com' + ) + @mock.patch('lms.djangoapps.discussion.rest_api.emails.ace', None) + def test_email_with_org_level_ban(self): + """Test email for organization-level ban.""" + mail.outbox = [] + + send_ban_escalation_email( + banned_user_id=self.banned_user.id, + moderator_id=self.moderator.id, + course_id=self.course_key, + scope='organization', + reason='Org-wide spam campaign', + threads_deleted=50, + comments_deleted=100 + ) + + self.assertEqual(len(mail.outbox), 1) + email = mail.outbox[0] + self.assertIn('ORGANIZATION', email.body) + self.assertIn('Org-wide spam campaign', email.body) + + @override_settings( + DISCUSSION_MODERATION_BAN_EMAIL_ENABLED=True, + DISCUSSION_MODERATION_ESCALATION_EMAIL='support@example.com' + ) + @mock.patch('lms.djangoapps.discussion.rest_api.emails.ace', None) + def test_email_failure_logged(self): + """Test that email failures are properly logged.""" + with mock.patch('django.core.mail.send_mail', side_effect=Exception("SMTP error")): + with self.assertLogs('lms.djangoapps.discussion.rest_api.emails', level='ERROR') as logs: + with self.assertRaises(Exception): + send_ban_escalation_email( + banned_user_id=self.banned_user.id, + moderator_id=self.moderator.id, + course_id=self.course_key, + scope='course', + reason='Test', + threads_deleted=1, + comments_deleted=1 + ) + + # Verify error was logged + self.assertTrue(any('Failed to send ban escalation email' in log for log in logs.output)) + + @override_settings(DISCUSSION_MODERATION_BAN_EMAIL_ENABLED=True) + def test_email_with_invalid_user_id(self): + """Test that email handles invalid user IDs gracefully.""" + with self.assertRaises(Exception): + send_ban_escalation_email( + banned_user_id=99999, # Non-existent user + moderator_id=self.moderator.id, + course_id=self.course_key, + scope='course', + reason='Test', + threads_deleted=0, + comments_deleted=0 + ) + + @override_settings( + DISCUSSION_MODERATION_BAN_EMAIL_ENABLED=True, + DISCUSSION_MODERATION_ESCALATION_EMAIL='test@example.com' + ) + @mock.patch('lms.djangoapps.discussion.rest_api.emails.ace', None) + def test_email_subject_format(self): + """Test that email subject is properly formatted.""" + mail.outbox = [] + + send_ban_escalation_email( + banned_user_id=self.banned_user.id, + moderator_id=self.moderator.id, + course_id=self.course_key, + scope='course', + reason='Test ban', + threads_deleted=1, + comments_deleted=1 + ) + + self.assertEqual(len(mail.outbox), 1) + email = mail.outbox[0] + # Subject should contain username and course + self.assertIn('spammer', email.subject) + self.assertIn(self.course_key, email.subject) diff --git a/lms/djangoapps/discussion/rest_api/tests/test_moderation_permissions.py b/lms/djangoapps/discussion/rest_api/tests/test_moderation_permissions.py new file mode 100644 index 000000000000..20a520d2d5bd --- /dev/null +++ b/lms/djangoapps/discussion/rest_api/tests/test_moderation_permissions.py @@ -0,0 +1,213 @@ +""" +Tests for discussion moderation permissions. +""" +from unittest.mock import Mock + +from rest_framework.test import APIRequestFactory + +from common.djangoapps.student.roles import CourseStaffRole, CourseInstructorRole, GlobalStaff +from common.djangoapps.student.tests.factories import UserFactory +from lms.djangoapps.discussion.rest_api.permissions import ( + IsAllowedToBulkDelete, + can_take_action_on_spam, +) +from openedx.core.djangoapps.django_comment_common.models import Role +from xmodule.modulestore.tests.factories import CourseFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase + + +class CanTakeActionOnSpamTest(ModuleStoreTestCase): + """Tests for can_take_action_on_spam permission helper function.""" + + def setUp(self): + super().setUp() + self.course = CourseFactory.create(org='TestX', number='CS101', run='2024') + self.course_key = self.course.id + + def test_global_staff_has_permission(self): + """Global staff should have permission.""" + user = UserFactory.create(is_staff=True) + self.assertTrue(can_take_action_on_spam(user, self.course_key)) + + def test_global_staff_role_has_permission(self): + """Users with GlobalStaff role should have permission.""" + user = UserFactory.create() + GlobalStaff().add_users(user) + self.assertTrue(can_take_action_on_spam(user, self.course_key)) + + def test_course_staff_has_permission(self): + """Course staff should have permission for their course.""" + user = UserFactory.create() + CourseStaffRole(self.course_key).add_users(user) + self.assertTrue(can_take_action_on_spam(user, self.course_key)) + + def test_course_instructor_has_permission(self): + """Course instructors should have permission for their course.""" + user = UserFactory.create() + CourseInstructorRole(self.course_key).add_users(user) + self.assertTrue(can_take_action_on_spam(user, self.course_key)) + + def test_forum_moderator_has_permission(self): + """Forum moderators should have permission for their course.""" + user = UserFactory.create() + role = Role.objects.create(name='Moderator', course_id=self.course_key) + role.users.add(user) + self.assertTrue(can_take_action_on_spam(user, self.course_key)) + + def test_forum_administrator_has_permission(self): + """Forum administrators should have permission for their course.""" + user = UserFactory.create() + role = Role.objects.create(name='Administrator', course_id=self.course_key) + role.users.add(user) + self.assertTrue(can_take_action_on_spam(user, self.course_key)) + + def test_regular_student_no_permission(self): + """Regular students should not have permission.""" + user = UserFactory.create() + self.assertFalse(can_take_action_on_spam(user, self.course_key)) + + def test_community_ta_no_permission(self): + """Community TAs should not have bulk delete permission.""" + user = UserFactory.create() + role = Role.objects.create(name='Community TA', course_id=self.course_key) + role.users.add(user) + self.assertFalse(can_take_action_on_spam(user, self.course_key)) + + def test_staff_different_course_no_permission(self): + """Staff from a different course should not have permission.""" + other_course = CourseFactory.create(org='OtherX', number='CS201', run='2024') + user = UserFactory.create() + CourseStaffRole(other_course.id).add_users(user) + self.assertFalse(can_take_action_on_spam(user, self.course_key)) + + def test_accepts_string_course_id(self): + """Function should accept string course_id and convert it.""" + user = UserFactory.create() + CourseStaffRole(self.course_key).add_users(user) + self.assertTrue(can_take_action_on_spam(user, str(self.course_key))) + + +class IsAllowedToBulkDeleteTest(ModuleStoreTestCase): + """Tests for IsAllowedToBulkDelete permission class.""" + + def setUp(self): + super().setUp() + self.course = CourseFactory.create(org='TestX', number='CS101', run='2024') + self.course_key = str(self.course.id) + self.factory = APIRequestFactory() + self.permission = IsAllowedToBulkDelete() + + def _create_view_with_kwargs(self, course_id=None): + """Helper to create a mock view with kwargs.""" + view = Mock() + view.kwargs = {'course_id': course_id} if course_id else {} + return view + + def _create_request_with_data(self, user, course_id=None, method='POST'): + """Helper to create a request with data.""" + if method == 'POST': + request = self.factory.post('/api/discussion/v1/moderation/bulk-delete-ban/') + else: + request = self.factory.get('/api/discussion/v1/moderation/banned-users/') + + request.user = user + request.data = {'course_id': course_id} if course_id else {} + request.query_params = {'course_id': course_id} if course_id and method == 'GET' else {} + return request + + def test_unauthenticated_user_denied(self): + """Unauthenticated users should be denied.""" + request = self.factory.post('/api/discussion/v1/moderation/bulk-delete-ban/') + request.user = Mock(is_authenticated=False) + view = self._create_view_with_kwargs() + + self.assertFalse(self.permission.has_permission(request, view)) + + def test_global_staff_with_course_id_in_data(self): + """Global staff should have permission when course_id is in request data.""" + user = UserFactory.create(is_staff=True) + request = self._create_request_with_data(user, self.course_key) + view = self._create_view_with_kwargs() + + self.assertTrue(self.permission.has_permission(request, view)) + + def test_course_staff_with_course_id_in_data(self): + """Course staff should have permission when course_id is in request data.""" + user = UserFactory.create() + CourseStaffRole(self.course.id).add_users(user) + request = self._create_request_with_data(user, self.course_key) + view = self._create_view_with_kwargs() + + self.assertTrue(self.permission.has_permission(request, view)) + + def test_course_instructor_with_course_id_in_data(self): + """Course instructors should have permission when course_id is in request data.""" + user = UserFactory.create() + CourseInstructorRole(self.course.id).add_users(user) + request = self._create_request_with_data(user, self.course_key) + view = self._create_view_with_kwargs() + + self.assertTrue(self.permission.has_permission(request, view)) + + def test_forum_moderator_with_course_id_in_data(self): + """Forum moderators should have permission when course_id is in request data.""" + user = UserFactory.create() + role = Role.objects.create(name='Moderator', course_id=self.course.id) + role.users.add(user) + request = self._create_request_with_data(user, self.course_key) + view = self._create_view_with_kwargs() + + self.assertTrue(self.permission.has_permission(request, view)) + + def test_regular_student_denied(self): + """Regular students should be denied.""" + user = UserFactory.create() + request = self._create_request_with_data(user, self.course_key) + view = self._create_view_with_kwargs() + + self.assertFalse(self.permission.has_permission(request, view)) + + def test_course_id_in_url_kwargs(self): + """Permission should work when course_id is in URL kwargs.""" + user = UserFactory.create() + CourseStaffRole(self.course.id).add_users(user) + request = self.factory.get('/api/discussion/v1/moderation/banned-users/') + request.user = user + request.data = {} + request.query_params = {} + view = self._create_view_with_kwargs(self.course_key) + + self.assertTrue(self.permission.has_permission(request, view)) + + def test_course_id_in_query_params(self): + """Permission should work when course_id is in query parameters.""" + user = UserFactory.create() + CourseStaffRole(self.course.id).add_users(user) + request = self._create_request_with_data(user, self.course_key, method='GET') + view = self._create_view_with_kwargs() + + self.assertTrue(self.permission.has_permission(request, view)) + + def test_no_course_id_only_global_staff_allowed(self): + """When no course_id provided, only global staff should be allowed.""" + # Global staff allowed + global_staff = UserFactory.create(is_staff=True) + request = self._create_request_with_data(global_staff) + view = self._create_view_with_kwargs() + self.assertTrue(self.permission.has_permission(request, view)) + + # Regular user denied + regular_user = UserFactory.create() + request = self._create_request_with_data(regular_user) + view = self._create_view_with_kwargs() + self.assertFalse(self.permission.has_permission(request, view)) + + def test_staff_different_course_denied(self): + """Staff from different course should be denied.""" + other_course = CourseFactory.create(org='OtherX', number='CS201', run='2024') + user = UserFactory.create() + CourseStaffRole(other_course.id).add_users(user) + request = self._create_request_with_data(user, self.course_key) + view = self._create_view_with_kwargs() + + self.assertFalse(self.permission.has_permission(request, view)) diff --git a/lms/djangoapps/discussion/rest_api/tests/test_moderation_serializers.py b/lms/djangoapps/discussion/rest_api/tests/test_moderation_serializers.py new file mode 100644 index 000000000000..70ef1960a8c6 --- /dev/null +++ b/lms/djangoapps/discussion/rest_api/tests/test_moderation_serializers.py @@ -0,0 +1,227 @@ +""" +Tests for discussion moderation serializers. +""" + +from django.test import TestCase + +from lms.djangoapps.discussion.models import DiscussionBan +from lms.djangoapps.discussion.rest_api.serializers import ( + BulkDeleteBanRequestSerializer, + BannedUserSerializer, +) +from common.djangoapps.student.tests.factories import UserFactory +from xmodule.modulestore.tests.factories import CourseFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase + + +class BulkDeleteBanRequestSerializerTest(TestCase): + """Tests for BulkDeleteBanRequestSerializer.""" + + def setUp(self): + super().setUp() + self.user = UserFactory.create() + self.course_id = 'course-v1:edX+DemoX+Demo_Course' + + def test_valid_data_without_ban(self): + """Test serializer with valid data for delete only.""" + data = { + 'user_id': self.user.id, + 'course_id': self.course_id, + 'ban_user': False + } + + serializer = BulkDeleteBanRequestSerializer(data=data) + self.assertTrue(serializer.is_valid()) + self.assertEqual(serializer.validated_data['user_id'], self.user.id) + self.assertFalse(serializer.validated_data['ban_user']) + + def test_valid_data_with_ban_and_reason(self): + """Test serializer with ban enabled and reason provided.""" + data = { + 'user_id': self.user.id, + 'course_id': self.course_id, + 'ban_user': True, + 'ban_scope': 'course', + 'reason': 'Posting spam content' + } + + serializer = BulkDeleteBanRequestSerializer(data=data) + self.assertTrue(serializer.is_valid()) + self.assertTrue(serializer.validated_data['ban_user']) + self.assertEqual(serializer.validated_data['reason'], 'Posting spam content') + + def test_missing_reason_when_ban_true(self): + """Test that reason is required when ban_user is True.""" + data = { + 'user_id': self.user.id, + 'course_id': self.course_id, + 'ban_user': True, + 'ban_scope': 'course' + } + + serializer = BulkDeleteBanRequestSerializer(data=data) + self.assertFalse(serializer.is_valid()) + self.assertIn('reason', serializer.errors) + + def test_empty_reason_when_ban_true(self): + """Test that empty reason is rejected when ban_user is True.""" + data = { + 'user_id': self.user.id, + 'course_id': self.course_id, + 'ban_user': True, + 'ban_scope': 'course', + 'reason': '' + } + + serializer = BulkDeleteBanRequestSerializer(data=data) + self.assertFalse(serializer.is_valid()) + self.assertIn('reason', serializer.errors) + + def test_missing_required_fields(self): + """Test that required fields are validated.""" + data = {} + + serializer = BulkDeleteBanRequestSerializer(data=data) + self.assertFalse(serializer.is_valid()) + self.assertIn('user_id', serializer.errors) + self.assertIn('course_id', serializer.errors) + + def test_ban_scope_choices(self): + """Test that ban_scope accepts valid choices.""" + for scope in ['course', 'organization']: + data = { + 'user_id': self.user.id, + 'course_id': self.course_id, + 'ban_user': True, + 'ban_scope': scope, + 'reason': 'Test' + } + + serializer = BulkDeleteBanRequestSerializer(data=data) + self.assertTrue(serializer.is_valid(), f"Scope {scope} should be valid") + + def test_invalid_ban_scope(self): + """Test that invalid ban_scope is rejected.""" + data = { + 'user_id': self.user.id, + 'course_id': self.course_id, + 'ban_user': True, + 'ban_scope': 'invalid_scope', + 'reason': 'Test' + } + + serializer = BulkDeleteBanRequestSerializer(data=data) + self.assertFalse(serializer.is_valid()) + self.assertIn('ban_scope', serializer.errors) + + def test_default_values(self): + """Test default values for optional fields.""" + data = { + 'user_id': self.user.id, + 'course_id': self.course_id + } + + serializer = BulkDeleteBanRequestSerializer(data=data) + self.assertTrue(serializer.is_valid()) + self.assertFalse(serializer.validated_data['ban_user']) + self.assertEqual(serializer.validated_data['ban_scope'], 'course') + + +class BannedUserSerializerTest(ModuleStoreTestCase): + """Tests for BannedUserSerializer.""" + + def setUp(self): + super().setUp() + self.course = CourseFactory.create(org='TestX', number='CS101', run='2024') + self.course_key = self.course.id + self.user = UserFactory.create(username='banneduser', email='banned@example.com') + self.moderator = UserFactory.create(username='moderator') + + def test_serialize_course_ban(self): + """Test serializing a course-level ban.""" + ban = DiscussionBan.objects.create( + user=self.user, + course_id=self.course_key, + scope='course', + banned_by=self.moderator, + reason='Spam posting', + is_active=True + ) + + serializer = BannedUserSerializer(ban) + data = serializer.data + + self.assertEqual(data['id'], ban.id) + self.assertEqual(data['username'], 'banneduser') + self.assertEqual(data['email'], 'banned@example.com') + self.assertEqual(data['user_id'], self.user.id) + self.assertEqual(data['scope'], 'course') + self.assertEqual(data['reason'], 'Spam posting') + self.assertEqual(data['banned_by_username'], 'moderator') + self.assertTrue(data['is_active']) + + def test_serialize_org_ban(self): + """Test serializing an organization-level ban.""" + ban = DiscussionBan.objects.create( + user=self.user, + org_key='TestX', + scope='organization', + banned_by=self.moderator, + reason='Repeated violations', + is_active=True + ) + + serializer = BannedUserSerializer(ban) + data = serializer.data + + self.assertEqual(data['organization'], 'TestX') + self.assertEqual(data['scope'], 'organization') + self.assertIsNone(data['course_id']) + + def test_serialize_multiple_bans(self): + """Test serializing multiple bans.""" + DiscussionBan.objects.create( + user=self.user, + course_id=self.course_key, + scope='course', + banned_by=self.moderator, + reason='Ban 1' + ) + + user2 = UserFactory.create() + DiscussionBan.objects.create( + user=user2, + course_id=self.course_key, + scope='course', + banned_by=self.moderator, + reason='Ban 2' + ) + + bans = DiscussionBan.objects.filter(course_id=self.course_key) + serializer = BannedUserSerializer(bans, many=True) + + self.assertEqual(len(serializer.data), 2) + + def test_read_only_fields(self): + """Test that all fields are read-only.""" + ban = DiscussionBan.objects.create( + user=self.user, + course_id=self.course_key, + scope='course', + banned_by=self.moderator, + reason='Test' + ) + + # Try to modify data via serializer + data = { + 'username': 'modified', + 'reason': 'modified reason' + } + + # Serializer is read-only, so validation should fail for writes + # But reading should work fine + serializer = BannedUserSerializer(ban) + original_data = serializer.data + + self.assertEqual(original_data['username'], 'banneduser') + self.assertEqual(original_data['reason'], 'Test') diff --git a/lms/djangoapps/discussion/rest_api/tests/test_moderation_tasks.py b/lms/djangoapps/discussion/rest_api/tests/test_moderation_tasks.py new file mode 100644 index 000000000000..284715335d13 --- /dev/null +++ b/lms/djangoapps/discussion/rest_api/tests/test_moderation_tasks.py @@ -0,0 +1,320 @@ +""" +Tests for discussion moderation tasks. +""" +from unittest import mock +from django.test import override_settings + +from lms.djangoapps.discussion.rest_api.tasks import delete_course_post_for_user +from lms.djangoapps.discussion.models import DiscussionBan, DiscussionModerationLog +from common.djangoapps.student.tests.factories import UserFactory +from xmodule.modulestore.tests.factories import CourseFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase + + +class DeleteCoursePostForUserTaskTest(ModuleStoreTestCase): + """Tests for delete_course_post_for_user Celery task.""" + + def setUp(self): + super().setUp() + self.course = CourseFactory.create(org='TestX', number='CS101', run='2024') + self.course_key = str(self.course.id) + self.user = UserFactory.create() + self.moderator = UserFactory.create() + + @mock.patch('openedx.core.djangoapps.django_comment_common.comment_client.Comment.delete_user_comments') + @mock.patch('openedx.core.djangoapps.django_comment_common.comment_client.thread.Thread.delete_user_threads') + @mock.patch('lms.djangoapps.discussion.rest_api.tasks.tracker.emit') + @mock.patch('lms.djangoapps.discussion.rest_api.tasks.segment.track') + def test_delete_posts_without_ban(self, mock_segment, mock_tracker, + mock_delete_threads, mock_delete_comments): + """Test deleting posts without banning the user.""" + # Mock forum service responses + mock_delete_threads.return_value = 2 + mock_delete_comments.return_value = 3 + + # Execute task + result = delete_course_post_for_user.apply(kwargs={ + 'user_id': self.user.id, + 'username': self.user.username, + 'course_ids': [self.course_key], + 'ban_user': False + }).get() + + # Verify deletions were called + mock_delete_threads.assert_called_once_with(self.user.id, [self.course_key]) + mock_delete_comments.assert_called_once_with(self.user.id, [self.course_key]) + + # Verify no ban was created + self.assertFalse(DiscussionBan.objects.filter(user=self.user).exists()) + + # Verify result + self.assertEqual(result['threads_deleted'], 2) + self.assertEqual(result['comments_deleted'], 3) + self.assertFalse(result['ban_applied']) + + @override_settings(DISCUSSION_MODERATION_BAN_EMAIL_ENABLED=True) + @mock.patch('openedx.core.djangoapps.django_comment_common.comment_client.Comment.delete_user_comments') + @mock.patch('openedx.core.djangoapps.django_comment_common.comment_client.thread.Thread.delete_user_threads') + @mock.patch('lms.djangoapps.discussion.rest_api.tasks.tracker.emit') + @mock.patch('lms.djangoapps.discussion.rest_api.tasks.segment.track') + @mock.patch('lms.djangoapps.discussion.rest_api.emails.send_ban_escalation_email') + def test_delete_posts_with_course_ban(self, mock_send_email, mock_segment, + mock_tracker, mock_delete_threads, + mock_delete_comments): + """Test deleting posts with course-level ban.""" + # Mock forum service responses + mock_delete_threads.return_value = 1 + mock_delete_comments.return_value = 2 + + # Execute task with ban + result = delete_course_post_for_user.apply(kwargs={ + 'user_id': self.user.id, + 'username': self.user.username, + 'course_ids': [self.course_key], + 'ban_user': True, + 'ban_scope': 'course', + 'moderator_id': self.moderator.id, + 'reason': 'Posting spam' + }).get() + + # Verify ban was created + ban = DiscussionBan.objects.get(user=self.user, course_id=self.course_key) + self.assertTrue(ban.is_active) + self.assertEqual(ban.scope, 'course') + self.assertEqual(ban.reason, 'Posting spam') + + # Verify moderation log was created + log = DiscussionModerationLog.objects.get(target_user=self.user) + self.assertEqual(log.action_type, 'ban_user') + self.assertEqual(log.moderator, self.moderator) + self.assertIsNotNone(log.metadata) + if log.metadata: + self.assertEqual(log.metadata.get('threads_deleted'), 1) + self.assertEqual(log.metadata.get('comments_deleted'), 2) + + # Verify email was sent + mock_send_email.assert_called_once() + + # Verify result includes ban info + self.assertTrue(result['ban_applied']) + + @override_settings(DISCUSSION_MODERATION_BAN_EMAIL_ENABLED=True) + @mock.patch('openedx.core.djangoapps.django_comment_common.comment_client.Comment.delete_user_comments') + @mock.patch('openedx.core.djangoapps.django_comment_common.comment_client.thread.Thread.delete_user_threads') + @mock.patch('lms.djangoapps.discussion.rest_api.tasks.tracker.emit') + @mock.patch('lms.djangoapps.discussion.rest_api.tasks.segment.track') + @mock.patch('lms.djangoapps.discussion.rest_api.emails.send_ban_escalation_email') + def test_delete_posts_with_org_ban(self, mock_send_email, mock_segment, + mock_tracker, mock_delete_threads, + mock_delete_comments): + """Test deleting posts with organization-level ban.""" + mock_delete_threads.return_value = 5 + mock_delete_comments.return_value = 10 + + # Execute task with org-level ban + result = delete_course_post_for_user.apply(kwargs={ + 'user_id': self.user.id, + 'username': self.user.username, + 'course_ids': [self.course_key], + 'ban_user': True, + 'ban_scope': 'organization', + 'moderator_id': self.moderator.id, + 'reason': 'Multiple violations' + }).get() + + # Verify org-level ban was created + ban = DiscussionBan.objects.get(user=self.user, org_key='TestX') + self.assertEqual(ban.scope, 'organization') + self.assertIsNone(ban.course_id) + + # Verify result + self.assertTrue(result['ban_applied']) + self.assertEqual(result['threads_deleted'], 5) + + @mock.patch('openedx.core.djangoapps.django_comment_common.comment_client.Comment.delete_user_comments') + @mock.patch('openedx.core.djangoapps.django_comment_common.comment_client.thread.Thread.delete_user_threads') + @mock.patch('lms.djangoapps.discussion.rest_api.tasks.tracker.emit') + @mock.patch('lms.djangoapps.discussion.rest_api.tasks.segment.track') + def test_no_posts_to_delete(self, mock_segment, mock_tracker, + mock_delete_threads, mock_delete_comments): + """Test task when user has no posts.""" + mock_delete_threads.return_value = 0 + mock_delete_comments.return_value = 0 + + result = delete_course_post_for_user.apply(kwargs={ + 'user_id': self.user.id, + 'course_ids': [self.course_key], + 'ban_user': False + }).get() + + self.assertEqual(result['threads_deleted'], 0) + self.assertEqual(result['comments_deleted'], 0) + + @mock.patch('openedx.core.djangoapps.django_comment_common.comment_client.Comment.delete_user_comments') + @mock.patch('openedx.core.djangoapps.django_comment_common.comment_client.thread.Thread.delete_user_threads') + def test_task_handles_failure(self, mock_delete_threads, mock_delete_comments): + """Test that task raises exception on failure.""" + # Simulate forum service error + mock_delete_threads.side_effect = Exception("Forum service error") + + with self.assertRaises(Exception): + delete_course_post_for_user.apply(kwargs={ + 'user_id': self.user.id, + 'course_ids': [self.course_key], + 'ban_user': False + }).get() + + @mock.patch('openedx.core.djangoapps.django_comment_common.comment_client.Comment.delete_user_comments') + @mock.patch('openedx.core.djangoapps.django_comment_common.comment_client.thread.Thread.delete_user_threads') + @mock.patch('lms.djangoapps.discussion.rest_api.tasks.tracker.emit') + @mock.patch('lms.djangoapps.discussion.rest_api.tasks.segment.track') + def test_backward_compatibility_without_ban_param(self, mock_segment, mock_tracker, + mock_delete_threads, + mock_delete_comments): + """Test that task works without ban_user parameter (backward compatibility).""" + mock_delete_threads.return_value = 1 + mock_delete_comments.return_value = 0 + + # Call without ban_user parameter (defaults to False) + result = delete_course_post_for_user.apply(kwargs={ + 'user_id': self.user.id, + 'course_ids': [self.course_key] + }).get() + + # Should work and not ban user + self.assertFalse(DiscussionBan.objects.filter(user=self.user).exists()) + self.assertEqual(result['threads_deleted'], 1) + self.assertFalse(result['ban_applied']) + + @mock.patch('openedx.core.djangoapps.django_comment_common.comment_client.Comment.delete_user_comments') + @mock.patch('openedx.core.djangoapps.django_comment_common.comment_client.thread.Thread.delete_user_threads') + @mock.patch('lms.djangoapps.discussion.rest_api.tasks.tracker.emit') + @mock.patch('lms.djangoapps.discussion.rest_api.tasks.segment.track') + @mock.patch('lms.djangoapps.discussion.rest_api.emails.send_ban_escalation_email') + def test_email_failure_doesnt_break_task(self, mock_send_email, mock_segment, + mock_tracker, mock_delete_threads, + mock_delete_comments): + """Test that email send failure doesn't break the task.""" + mock_delete_threads.return_value = 1 + mock_delete_comments.return_value = 1 + mock_send_email.side_effect = Exception("SMTP error") + + # Task should complete despite email failure + result = delete_course_post_for_user.apply(kwargs={ + 'user_id': self.user.id, + 'course_ids': [self.course_key], + 'ban_user': True, + 'moderator_id': self.moderator.id, + 'reason': 'Test' + }).get() + + # Ban should still be created + self.assertTrue(DiscussionBan.objects.filter(user=self.user).exists()) + self.assertTrue(result['ban_applied']) + + @mock.patch('openedx.core.djangoapps.django_comment_common.comment_client.Comment.delete_user_comments') + @mock.patch('openedx.core.djangoapps.django_comment_common.comment_client.thread.Thread.delete_user_threads') + @mock.patch('lms.djangoapps.discussion.rest_api.tasks.tracker.emit') + @mock.patch('lms.djangoapps.discussion.rest_api.tasks.segment.track') + def test_reactivates_inactive_ban(self, mock_segment, mock_tracker, + mock_delete_threads, mock_delete_comments): + """Test that an inactive ban is reactivated.""" + # Create an inactive ban + inactive_ban = DiscussionBan.objects.create( + user=self.user, + course_id=self.course_key, + scope='course', + banned_by=self.moderator, + reason='Old ban', + is_active=False + ) + + mock_delete_threads.return_value = 1 + mock_delete_comments.return_value = 0 + + # Re-ban the user + delete_course_post_for_user.apply(kwargs={ + 'user_id': self.user.id, + 'course_ids': [self.course_key], + 'ban_user': True, + 'moderator_id': self.moderator.id, + 'reason': 'New ban' + }).get() + + # Ban should be reactivated + inactive_ban.refresh_from_db() + self.assertTrue(inactive_ban.is_active) + self.assertEqual(inactive_ban.reason, 'New ban') + + @override_settings(DISCUSSION_MODERATION_BAN_EMAIL_ENABLED=False) + @mock.patch('openedx.core.djangoapps.django_comment_common.comment_client.Comment.delete_user_comments') + @mock.patch('openedx.core.djangoapps.django_comment_common.comment_client.thread.Thread.delete_user_threads') + @mock.patch('lms.djangoapps.discussion.rest_api.tasks.tracker.emit') + @mock.patch('lms.djangoapps.discussion.rest_api.tasks.segment.track') + @mock.patch('lms.djangoapps.discussion.rest_api.emails.send_ban_escalation_email') + def test_email_not_sent_when_disabled(self, mock_send_email, mock_segment, + mock_tracker, mock_delete_threads, + mock_delete_comments): + """Test that email is not sent when DISCUSSION_MODERATION_BAN_EMAIL_ENABLED is False.""" + mock_delete_threads.return_value = 1 + mock_delete_comments.return_value = 1 + + # Execute task with ban + delete_course_post_for_user.apply(kwargs={ + 'user_id': self.user.id, + 'username': self.user.username, + 'course_ids': [self.course_key], + 'ban_user': True, + 'ban_scope': 'course', + 'moderator_id': self.moderator.id, + 'reason': 'Spam' + }).get() + + # Ban should be created + self.assertTrue(DiscussionBan.objects.filter(user=self.user).exists()) + + # Email should NOT be sent when setting is False + mock_send_email.assert_not_called() + + @override_settings( + DISCUSSION_MODERATION_BAN_EMAIL_ENABLED=True, + DISCUSSION_MODERATION_ESCALATION_EMAIL='custom-support@example.com' + ) + @mock.patch('openedx.core.djangoapps.django_comment_common.comment_client.Comment.delete_user_comments') + @mock.patch('openedx.core.djangoapps.django_comment_common.comment_client.thread.Thread.delete_user_threads') + @mock.patch('lms.djangoapps.discussion.rest_api.tasks.tracker.emit') + @mock.patch('lms.djangoapps.discussion.rest_api.tasks.segment.track') + @mock.patch('lms.djangoapps.discussion.rest_api.emails.send_ban_escalation_email') + def test_email_sent_to_custom_address(self, mock_send_email, mock_segment, + mock_tracker, mock_delete_threads, + mock_delete_comments): + """Test that email respects custom escalation email setting.""" + mock_delete_threads.return_value = 2 + mock_delete_comments.return_value = 3 + + # Execute task with ban + result = delete_course_post_for_user.apply(kwargs={ + 'user_id': self.user.id, + 'username': self.user.username, + 'course_ids': [self.course_key], + 'ban_user': True, + 'ban_scope': 'course', + 'moderator_id': self.moderator.id, + 'reason': 'Policy violation' + }).get() + + # Email should be called with correct parameters + mock_send_email.assert_called_once_with( + banned_user_id=self.user.id, + moderator_id=self.moderator.id, + course_id=self.course_key, + scope='course', + reason='Policy violation', + threads_deleted=2, + comments_deleted=3, + ) + + # Verify result includes ban info + self.assertTrue(result['ban_applied']) + self.assertEqual(result['threads_deleted'], 2) + self.assertEqual(result['comments_deleted'], 3) diff --git a/lms/djangoapps/discussion/rest_api/tests/test_moderation_views.py b/lms/djangoapps/discussion/rest_api/tests/test_moderation_views.py new file mode 100644 index 000000000000..f73ade284785 --- /dev/null +++ b/lms/djangoapps/discussion/rest_api/tests/test_moderation_views.py @@ -0,0 +1,469 @@ +""" +Tests for discussion moderation views. + +INTEGRATION TESTS - Requires MongoDB and full infrastructure. +These tests use ModuleStoreTestCase to create real courses in MongoDB and test +the full integration of the moderation views with course data, enrollments, and permissions. + +Note: These tests are slower (~60s per test when MongoDB is not available) but provide +comprehensive integration testing. For faster unit tests, see test_moderation_views_v2.py. +""" + +from unittest import mock +from django.contrib.auth import get_user_model +from django.urls import reverse +from rest_framework import status +from rest_framework.test import APIClient + +from lms.djangoapps.discussion.models import DiscussionBan, DiscussionModerationLog +from lms.djangoapps.discussion.toggles import ENABLE_DISCUSSION_BAN +from common.djangoapps.student.roles import CourseStaffRole, CourseInstructorRole +from common.djangoapps.student.tests.factories import UserFactory, CourseEnrollmentFactory +from common.djangoapps.util.testing import UrlResetMixin +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + +User = get_user_model() + + +@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) +class DiscussionModerationViewSetTest(UrlResetMixin, ModuleStoreTestCase): + """Integration tests for DiscussionModerationViewSet.""" + + @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) + def setUp(self): + super().setUp() + self.client = APIClient() + + # Create course with MongoDB (integration test) + self.course = CourseFactory.create( + org='TestX', + course='CS101', + run='2024' + ) + self.course_key = self.course.id + + # Create users + self.student = UserFactory.create(username='student') + self.moderator = UserFactory.create(username='moderator') + self.admin = UserFactory.create(username='admin', is_staff=True) + + # Create enrollments + CourseEnrollmentFactory.create(user=self.student, course_id=self.course_key) + CourseEnrollmentFactory.create(user=self.moderator, course_id=self.course_key) + + # Add moderator to course staff role + CourseStaffRole(self.course_key).add_users(self.moderator) + + def test_bulk_delete_ban_permission_denied_for_student(self): + """Test that students cannot access bulk delete/ban endpoint.""" + self.client.force_authenticate(user=self.student) + + url = reverse('discussion-moderation-bulk-delete-ban') + data = { + 'user_id': self.student.id, + 'course_id': str(self.course_key), + 'ban_user': False + } + + response = self.client.post(url, data, format='json') + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + @mock.patch('lms.djangoapps.discussion.rest_api.views.delete_course_post_for_user.apply_async') + def test_bulk_delete_without_ban(self, mock_task): + """Test bulk delete without banning the user.""" + # Mock the task to return a task-like object with an ID + expected_task_id = 'test-task-id-123' + mock_task.return_value = mock.Mock(id=expected_task_id) + + self.client.force_authenticate(user=self.moderator) + + url = reverse('discussion-moderation-bulk-delete-ban') + data = { + 'user_id': self.student.id, + 'course_id': str(self.course_key), + 'ban_user': False + } + + response = self.client.post(url, data, format='json') + + self.assertEqual(response.status_code, status.HTTP_202_ACCEPTED) + self.assertIn('message', response.data) + self.assertEqual(response.data['task_id'], expected_task_id) + + # Verify task was called with correct kwargs + mock_task.assert_called_once_with( + kwargs={ + 'user_id': self.student.id, + 'username': self.student.username, + 'course_ids': [str(self.course_key)], + 'ban_user': False, + 'ban_scope': 'course', + 'moderator_id': self.moderator.id, + 'reason': '', + } + ) + + # Verify no ban was created + self.assertFalse(DiscussionBan.objects.filter(user=self.student).exists()) + + @mock.patch('lms.djangoapps.discussion.rest_api.views.delete_course_post_for_user.apply_async') + @mock.patch.object(ENABLE_DISCUSSION_BAN, 'is_enabled', return_value=True) + def test_bulk_delete_with_course_ban(self, mock_waffle, mock_task): + """Test bulk delete with course-level ban.""" + # Mock task return value + expected_task_id = 'task-id-456' + mock_task.return_value = mock.Mock(id=expected_task_id) + + self.client.force_authenticate(user=self.moderator) + + url = reverse('discussion-moderation-bulk-delete-ban') + data = { + 'user_id': self.student.id, + 'course_id': str(self.course_key), + 'ban_user': True, + 'ban_scope': 'course', + 'reason': 'Posting spam content' + } + + response = self.client.post(url, data, format='json') + + # Endpoint returns 202 Accepted for async operations + self.assertEqual(response.status_code, status.HTTP_202_ACCEPTED) + self.assertEqual(response.data['status'], 'success') + self.assertIn('message', response.data) + self.assertEqual(response.data['task_id'], expected_task_id) + + # Verify task was called with correct kwargs + mock_task.assert_called_once_with( + kwargs={ + 'user_id': self.student.id, + 'username': self.student.username, + 'course_ids': [str(self.course_key)], + 'ban_user': True, + 'ban_scope': 'course', + 'moderator_id': self.moderator.id, + 'reason': 'Posting spam content', + } + ) + + # Note: Ban and moderation log are created by the Celery task, not by the view + # Those should be tested in test_moderation_tasks.py + + @mock.patch('lms.djangoapps.discussion.rest_api.views.delete_course_post_for_user.apply_async') + @mock.patch.object(ENABLE_DISCUSSION_BAN, 'is_enabled', return_value=True) + def test_bulk_delete_with_org_ban(self, mock_waffle, mock_task): + """Test bulk delete with organization-level ban.""" + mock_task.return_value = mock.Mock(id='org-ban-task-id') + + self.client.force_authenticate(user=self.moderator) + + url = reverse('discussion-moderation-bulk-delete-ban') + data = { + 'user_id': self.student.id, + 'course_id': str(self.course_key), + 'ban_user': True, + 'ban_scope': 'organization', + 'reason': 'Multiple violations across courses' + } + + response = self.client.post(url, data, format='json') + + self.assertEqual(response.status_code, status.HTTP_202_ACCEPTED) + + # Verify task was called with org scope + mock_task.assert_called_once() + call_kwargs = mock_task.call_args[1]['kwargs'] + self.assertEqual(call_kwargs['ban_scope'], 'organization') + + # Note: Ban is created by the Celery task, not by the view + + def test_bulk_delete_invalid_data(self): + """Test bulk delete with invalid data returns 400.""" + self.client.force_authenticate(user=self.moderator) + + url = reverse('discussion-moderation-bulk-delete-ban') + data = { + 'user_id': self.student.id, + 'course_id': str(self.course_key), + 'ban_user': True, + 'ban_scope': 'course' + # Missing required 'reason' field + } + + response = self.client.post(url, data, format='json') + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn('reason', response.data) + + def test_banned_users_list(self): + """Test listing banned users for a course.""" + # Create some bans + DiscussionBan.objects.create( + user=self.student, + course_id=self.course_key, + scope='course', + banned_by=self.moderator, + reason='Test ban', + is_active=True + ) + + other_user = UserFactory.create() + DiscussionBan.objects.create( + user=other_user, + course_id=self.course_key, + scope='course', + banned_by=self.moderator, + reason='Another ban', + is_active=True + ) + + # Inactive ban (should not appear) + inactive_user = UserFactory.create() + DiscussionBan.objects.create( + user=inactive_user, + course_id=self.course_key, + scope='course', + banned_by=self.moderator, + reason='Inactive', + is_active=False + ) + + self.client.force_authenticate(user=self.moderator) + url = reverse('discussion-moderation-banned-users', kwargs={'course_id': str(self.course_key)}) + + response = self.client.get(url) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data['count'], 2) + self.assertEqual(len(response.data['results']), 2) + + usernames = [ban['username'] for ban in response.data['results']] + self.assertIn('student', usernames) + self.assertIn(other_user.username, usernames) + self.assertNotIn(inactive_user.username, usernames) + + def test_banned_users_includes_org_bans(self): + """Test that banned users list includes organization-level bans.""" + # Create org-level ban + DiscussionBan.objects.create( + user=self.student, + org_key='TestX', + scope='organization', + banned_by=self.admin, + reason='Org ban', + is_active=True + ) + + self.client.force_authenticate(user=self.moderator) + url = reverse('discussion-moderation-banned-users', kwargs={'course_id': str(self.course_key)}) + + response = self.client.get(url) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data['count'], 1) + self.assertEqual(len(response.data['results']), 1) + self.assertEqual(response.data['results'][0]['scope'], 'organization') + + def test_unban_user_course_level(self): + """Test unbanning a user from a course.""" + ban = DiscussionBan.objects.create( + user=self.student, + course_id=self.course_key, + scope='course', + banned_by=self.moderator, + reason='Test ban', + is_active=True + ) + + self.client.force_authenticate(user=self.moderator) + url = reverse('discussion-moderation-unban-user', kwargs={'pk': ban.id}) + data = { + 'user_id': self.student.id, + 'reason': 'Ban appeal approved' + } + + response = self.client.post(url, data, format='json') + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + # Verify ban was deactivated + ban.refresh_from_db() + self.assertFalse(ban.is_active) + self.assertIsNotNone(ban.unbanned_at) + + # Verify moderation log + log = DiscussionModerationLog.objects.filter( + target_user=self.student, + action_type='unban_user' + ).first() + self.assertIsNotNone(log) + self.assertEqual(log.moderator, self.moderator) + + def test_unban_user_org_level_creates_exception(self): + """Test unbanning from org-level ban creates an exception.""" + org_ban = DiscussionBan.objects.create( + user=self.student, + org_key='TestX', + scope='organization', + banned_by=self.admin, + reason='Org ban', + is_active=True + ) + + self.client.force_authenticate(user=self.moderator) + url = reverse('discussion-moderation-unban-user', kwargs={'pk': org_ban.id}) + data = { + 'user_id': self.student.id, + 'course_id': str(self.course_key), + 'reason': 'Exception for this course' + } + + response = self.client.post(url, data, format='json') + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + # Org ban should still be active + org_ban.refresh_from_db() + self.assertTrue(org_ban.is_active) + + # Exception should exist + from lms.djangoapps.discussion.models import DiscussionBanException + exception = DiscussionBanException.objects.filter( + ban=org_ban, + course_id=self.course_key + ).first() + self.assertIsNotNone(exception) + + def test_unban_user_not_banned(self): + """Test unbanning a user who is not banned.""" + self.client.force_authenticate(user=self.moderator) + # Use a non-existent ban ID + url = reverse('discussion-moderation-unban-user', kwargs={'pk': 99999}) + data = { + 'user_id': self.student.id, + 'reason': 'Trying to unban' + } + + response = self.client.post(url, data, format='json') + + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + def test_unban_user_missing_reason(self): + """Test that reason is required for unbanning.""" + ban = DiscussionBan.objects.create( + user=self.student, + course_id=self.course_key, + scope='course', + banned_by=self.moderator, + reason='Test ban', + is_active=True + ) + + self.client.force_authenticate(user=self.moderator) + url = reverse('discussion-moderation-unban-user', kwargs={'pk': ban.id}) + data = { + 'user_id': self.student.id + # Missing 'reason' + } + + response = self.client.post(url, data, format='json') + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + def test_permission_course_staff(self): + """Test that course staff can access moderation endpoints.""" + staff_user = UserFactory.create() + CourseStaffRole(self.course_key).add_users(staff_user) + + self.client.force_authenticate(user=staff_user) + url = reverse('discussion-moderation-banned-users', kwargs={'course_id': str(self.course_key)}) + + response = self.client.get(url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + def test_permission_course_instructor(self): + """Test that course instructors can access moderation endpoints.""" + instructor = UserFactory.create() + CourseInstructorRole(self.course_key).add_users(instructor) + + self.client.force_authenticate(user=instructor) + url = reverse('discussion-moderation-banned-users', kwargs={'course_id': str(self.course_key)}) + + response = self.client.get(url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + @mock.patch('lms.djangoapps.discussion.rest_api.views.delete_course_post_for_user.apply_async') + def test_bulk_delete_ban_feature_disabled(self, mock_task): + """Test that ban is rejected when waffle flag is disabled.""" + with mock.patch.object(ENABLE_DISCUSSION_BAN, 'is_enabled', return_value=False): + self.client.force_authenticate(user=self.moderator) + + url = reverse('discussion-moderation-bulk-delete-ban') + data = { + 'user_id': self.student.id, + 'course_id': str(self.course_key), + 'ban_user': True, + 'ban_scope': 'course', + 'reason': 'Spam content' + } + + response = self.client.post(url, data, format='json') + + # Should return 403 when feature is disabled + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertIn('not enabled', response.data['error'].lower()) + + # Verify task was not called + mock_task.assert_not_called() + + # Verify no ban was created + self.assertFalse(DiscussionBan.objects.filter(user=self.student).exists()) + + @mock.patch('lms.djangoapps.discussion.rest_api.views.delete_course_post_for_user.apply_async') + def test_bulk_delete_ban_feature_enabled(self, mock_task): + """Test that ban works when waffle flag is enabled.""" + mock_task.return_value = mock.Mock(id='enabled-feature-task') + + with mock.patch.object(ENABLE_DISCUSSION_BAN, 'is_enabled', return_value=True): + self.client.force_authenticate(user=self.moderator) + + url = reverse('discussion-moderation-bulk-delete-ban') + data = { + 'user_id': self.student.id, + 'course_id': str(self.course_key), + 'ban_user': True, + 'ban_scope': 'course', + 'reason': 'Spam content' + } + + response = self.client.post(url, data, format='json') + + # Should succeed when feature is enabled + self.assertEqual(response.status_code, status.HTTP_202_ACCEPTED) + + # Verify task was called + mock_task.assert_called_once() + + @mock.patch('lms.djangoapps.discussion.rest_api.views.delete_course_post_for_user.apply_async') + def test_bulk_delete_without_ban_works_regardless_of_flag(self, mock_task): + """Test that delete without ban works even when flag is disabled.""" + mock_task.return_value = mock.Mock(id='no-ban-task') + + with mock.patch.object(ENABLE_DISCUSSION_BAN, 'is_enabled', return_value=False): + self.client.force_authenticate(user=self.moderator) + + url = reverse('discussion-moderation-bulk-delete-ban') + data = { + 'user_id': self.student.id, + 'course_id': str(self.course_key), + 'ban_user': False # Not banning + } + + response = self.client.post(url, data, format='json') + + # Should work fine - flag only controls ban feature + self.assertEqual(response.status_code, status.HTTP_202_ACCEPTED) + + # Verify task was called + mock_task.assert_called_once() diff --git a/lms/djangoapps/discussion/rest_api/urls.py b/lms/djangoapps/discussion/rest_api/urls.py index f102dc41f249..763a9fc3587f 100644 --- a/lms/djangoapps/discussion/rest_api/urls.py +++ b/lms/djangoapps/discussion/rest_api/urls.py @@ -18,6 +18,7 @@ CourseTopicsViewV3, CourseView, CourseViewV2, + DiscussionModerationViewSet, LearnerThreadView, ReplaceUsernamesView, RetireUserView, @@ -30,6 +31,22 @@ ROUTER.register("comments", CommentViewSet, basename="comment") urlpatterns = [ + # Moderation endpoints (defined first to avoid router conflicts) + path( + 'v1/moderation/bulk-delete-ban/', + DiscussionModerationViewSet.as_view({'post': 'bulk_delete_ban'}), + name='discussion-moderation-bulk-delete-ban' + ), + re_path( + fr'^v1/moderation/banned-users/{settings.COURSE_ID_PATTERN}$', + DiscussionModerationViewSet.as_view({'get': 'banned_users'}), + name='discussion-moderation-banned-users' + ), + path( + 'v1/moderation//unban/', + DiscussionModerationViewSet.as_view({'post': 'unban_user'}), + name='discussion-moderation-unban-user' + ), re_path( r"^v1/courses/{}/settings$".format( settings.COURSE_ID_PATTERN diff --git a/lms/djangoapps/discussion/rest_api/utils.py b/lms/djangoapps/discussion/rest_api/utils.py index 0f02a0dcdcf2..a9bf600de9ff 100644 --- a/lms/djangoapps/discussion/rest_api/utils.py +++ b/lms/djangoapps/discussion/rest_api/utils.py @@ -453,8 +453,8 @@ def verify_recaptcha_token(token: str) -> bool: """ try: site_key = get_captcha_site_key_by_platform(get_platform_from_request()) - url = (f"https://recaptchaenterprise.googleapis.com/v1/projects/{settings.RECAPTCHA_PROJECT_ID}/assessments" - f"?key={settings.RECAPTCHA_PRIVATE_KEY}") + url = ("https://recaptchaenterprise.googleapis.com/v1/projects/{}/assessments" + "?key={}").format(settings.RECAPTCHA_PROJECT_ID, settings.RECAPTCHA_PRIVATE_KEY) data = { "event": { "token": token, diff --git a/lms/djangoapps/discussion/rest_api/views.py b/lms/djangoapps/discussion/rest_api/views.py index ba9818124e08..b55bc68f613a 100644 --- a/lms/djangoapps/discussion/rest_api/views.py +++ b/lms/djangoapps/discussion/rest_api/views.py @@ -9,6 +9,7 @@ from django.contrib.auth import get_user_model from django.core.exceptions import BadRequest, ValidationError from django.shortcuts import get_object_or_404 +from django.utils import timezone from drf_yasg import openapi from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser @@ -31,6 +32,7 @@ from lms.djangoapps.discussion.rest_api.permissions import IsAllowedToBulkDelete from lms.djangoapps.discussion.rest_api.tasks import delete_course_post_for_user from lms.djangoapps.discussion.toggles import ONLY_VERIFIED_USERS_CAN_POST +from lms.djangoapps.discussion.toggles import ENABLE_DISCUSSION_BAN from lms.djangoapps.discussion.django_comment_client import settings as cc_settings from lms.djangoapps.discussion.django_comment_client.utils import get_group_id_for_comments_service from lms.djangoapps.instructor.access import update_forum_role @@ -1615,3 +1617,471 @@ def post(self, request, course_id): {"comment_count": comment_count, "thread_count": thread_count}, status=status.HTTP_202_ACCEPTED ) + + +class DiscussionModerationViewSet(DeveloperErrorViewMixin, ViewSet): + """ + **Use Cases** + + Perform bulk moderation actions on discussion posts and manage user bans. + + **Example Requests** + + POST /api/discussion/v1/moderation/bulk-delete-ban/ + GET /api/discussion/v1/moderation/banned-users/?course_id=course-v1:edX+DemoX+Demo + POST /api/discussion/v1/moderation/123/unban/ + """ + + authentication_classes = ( + JwtAuthentication, BearerAuthentication, SessionAuthentication, + ) + permission_classes = (permissions.IsAuthenticated, IsAllowedToBulkDelete) + + def get_permissions(self): + """ + Return permission instances for the view. + + For unban_user action, we only need IsAuthenticated because we check + course-specific permissions inside the action method after retrieving the ban. + """ + if self.action in ['unban_user', 'banned_users']: + return [permissions.IsAuthenticated()] + return super().get_permissions() + + @apidocs.schema( + body=openapi.Schema( + type=openapi.TYPE_OBJECT, + required=['user_id', 'course_id'], + properties={ + 'user_id': openapi.Schema( + type=openapi.TYPE_INTEGER, + description='ID of the user whose posts should be deleted' + ), + 'course_id': openapi.Schema( + type=openapi.TYPE_STRING, + description='Course ID (e.g., course-v1:edX+DemoX+Demo_Course)' + ), + 'ban_user': openapi.Schema( + type=openapi.TYPE_BOOLEAN, + description='If true, ban the user after deleting posts', + default=False + ), + 'ban_scope': openapi.Schema( + type=openapi.TYPE_STRING, + description='Scope of ban: "course" or "organization"', + enum=['course', 'organization'], + default='course' + ), + 'reason': openapi.Schema( + type=openapi.TYPE_STRING, + description='Reason for ban (required if ban_user is true)', + max_length=1000 + ), + }, + ), + responses={ + 202: openapi.Response( + description='Deletion task queued successfully', + schema=openapi.Schema( + type=openapi.TYPE_OBJECT, + properties={ + 'status': openapi.Schema(type=openapi.TYPE_STRING, example='success'), + 'message': openapi.Schema(type=openapi.TYPE_STRING), + 'task_id': openapi.Schema(type=openapi.TYPE_STRING), + }, + ), + ), + 400: 'Invalid request data or missing required parameters.', + 401: 'The requester is not authenticated.', + 403: 'The requester does not have permission to perform bulk delete.', + 404: 'The specified user does not exist.', + }, + ) + def bulk_delete_ban(self, request): + """ + Delete all user posts in a course and optionally ban the user. + + **Use Cases** + + * Remove all discussion content from a spam account + * Ban user from course or organization discussions + * Bulk cleanup of policy-violating content + + **Example Request** + + POST /api/discussion/v1/moderation/bulk-delete-ban/ + + ```json + { + "user_id": 12345, + "course_id": "course-v1:HarvardX+CS50+2024", + "ban_user": true, + "ban_scope": "course", + "reason": "Posting spam and scam content" + } + ``` + + **Response Values** + + * status: Success status of the request + * message: Human-readable message about the queued task + * task_id: Celery task ID for tracking the asynchronous operation + + **Notes** + + * This operation is asynchronous and returns a task ID + * If ban_user is true, a ban record will be created after content deletion + * Reason is required when ban_user is true + * Email notification is sent to partner-support upon ban + """ + from lms.djangoapps.discussion.rest_api.serializers import BulkDeleteBanRequestSerializer + + serializer = BulkDeleteBanRequestSerializer(data=request.data) + if not serializer.is_valid(): + return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) + + validated_data = serializer.validated_data + + # Check if ban feature is enabled for this course + if validated_data['ban_user']: + course_key = CourseKey.from_string(validated_data['course_id']) + if not ENABLE_DISCUSSION_BAN.is_enabled(course_key): + return Response( + {'error': 'Discussion ban feature is not enabled for this course'}, + status=status.HTTP_403_FORBIDDEN + ) + + # Enqueue Celery task (backward compatible with new parameters) + task = delete_course_post_for_user.apply_async( + kwargs={ + 'user_id': validated_data['user_id'], + 'username': get_object_or_404(User, id=validated_data['user_id']).username, + 'course_ids': [validated_data['course_id']], + 'ban_user': validated_data['ban_user'], + 'ban_scope': validated_data.get('ban_scope', 'course'), + 'moderator_id': request.user.id, + 'reason': validated_data.get('reason', ''), + } + ) + + message = ( + 'Deletion task queued. User will be banned upon completion.' + if validated_data['ban_user'] + else 'Deletion task queued.' + ) + return Response({ + 'status': 'success', + 'message': message, + 'task_id': task.id, + }, status=status.HTTP_202_ACCEPTED) + + @apidocs.schema( + parameters=[ + apidocs.string_parameter( + 'course_id', + apidocs.ParameterLocation.QUERY, + description='Course ID to filter banned users (required)' + ), + apidocs.string_parameter( + 'scope', + apidocs.ParameterLocation.QUERY, + description='Filter by ban scope: "course" or "organization"' + ), + ], + responses={ + 200: openapi.Response( + description='List of banned users', + schema=openapi.Schema( + type=openapi.TYPE_OBJECT, + properties={ + 'count': openapi.Schema( + type=openapi.TYPE_INTEGER, + description='Total number of banned users' + ), + 'results': openapi.Schema( + type=openapi.TYPE_ARRAY, + description='Array of banned user records', + items=openapi.Schema( + type=openapi.TYPE_OBJECT, + properties={ + 'id': openapi.Schema(type=openapi.TYPE_INTEGER), + 'username': openapi.Schema(type=openapi.TYPE_STRING), + 'email': openapi.Schema(type=openapi.TYPE_STRING), + 'user_id': openapi.Schema(type=openapi.TYPE_INTEGER), + 'course_id': openapi.Schema(type=openapi.TYPE_STRING), + 'organization': openapi.Schema(type=openapi.TYPE_STRING), + 'scope': openapi.Schema(type=openapi.TYPE_STRING), + 'reason': openapi.Schema(type=openapi.TYPE_STRING), + 'banned_at': openapi.Schema(type=openapi.TYPE_STRING, format='date-time'), + 'banned_by_username': openapi.Schema(type=openapi.TYPE_STRING), + 'is_active': openapi.Schema(type=openapi.TYPE_BOOLEAN), + }, + ), + ), + }, + ), + ), + 400: 'Missing required course_id parameter.', + 401: 'The requester is not authenticated.', + 403: 'The requester does not have permission to view banned users.', + }, + ) + def banned_users(self, request, course_id=None): + """ + Retrieve list of banned users for a specific course. + + **Use Cases** + + * View all currently banned users in a course + * Filter banned users by scope (course-level vs organization-level) + * Audit moderation actions + + **Example Requests** + + GET /api/discussion/v1/moderation/banned-users/course-v1:HarvardX+CS50+2024 + GET /api/discussion/v1/moderation/banned-users/course-v1:edX+DemoX+Demo?scope=course + + **Response Values** + + * count: Total number of active bans for the course + * results: Array of ban records with user information + + **Notes** + + * Only returns active bans (is_active=True) + * Course-level bans are specific to one course + * Organization-level bans apply to all courses in the organization + """ + from lms.djangoapps.discussion.models import DiscussionBan + from lms.djangoapps.discussion.rest_api.serializers import BannedUserSerializer + from lms.djangoapps.discussion.rest_api.permissions import can_take_action_on_spam + + if not course_id: + return Response( + {'error': 'course_id parameter is required'}, + status=status.HTTP_400_BAD_REQUEST + ) + + course_key = CourseKey.from_string(course_id) + + # Permission check: user must be able to moderate in this course + if not can_take_action_on_spam(request.user, course_key): + return Response( + {'error': 'You do not have permission to view banned users in this course'}, + status=status.HTTP_403_FORBIDDEN + ) + + organization = course_key.org + + # Include both course-level bans AND org-level bans for this organization + from django.db.models import Q + queryset = DiscussionBan.objects.filter( + Q(course_id=course_key) | Q(org_key=organization, scope='organization'), + is_active=True + ).select_related('user', 'banned_by') + + # Optional scope filter + scope = request.query_params.get('scope') + if scope in ['course', 'organization']: + queryset = queryset.filter(scope=scope) + + serializer = BannedUserSerializer(queryset, many=True) + + return Response({ + 'count': queryset.count(), + 'results': serializer.data + }) + + @apidocs.schema( + parameters=[ + apidocs.string_parameter( + 'pk', + apidocs.ParameterLocation.PATH, + description='Ban ID to unban' + ), + ], + body=openapi.Schema( + type=openapi.TYPE_OBJECT, + properties={ + 'course_id': openapi.Schema( + type=openapi.TYPE_STRING, + description='Course ID for organization-level ban exceptions' + ), + 'reason': openapi.Schema( + type=openapi.TYPE_STRING, + description='Reason for unbanning' + ), + }, + required=['reason'], + ), + responses={ + 200: openapi.Response( + description='User unbanned successfully', + schema=openapi.Schema( + type=openapi.TYPE_OBJECT, + properties={ + 'status': openapi.Schema(type=openapi.TYPE_STRING, example='success'), + 'message': openapi.Schema(type=openapi.TYPE_STRING), + 'exception_created': openapi.Schema( + type=openapi.TYPE_BOOLEAN, + description='True if org-level ban exception was created' + ), + }, + ), + ), + 401: 'The requester is not authenticated.', + 403: 'The requester does not have permission to unban users.', + 404: 'Active ban not found with the specified ID.', + }, + ) + def unban_user(self, request, pk=None): + """ + Unban a user from discussions or create course-level exception. + + **Use Cases** + + * Lift a course-level ban completely + * Lift an organization-level ban completely + * Create course-specific exception to organization-level ban + * Process user appeals + + **Example Requests** + + POST /api/discussion/v1/moderation/123/unban/ + + ```json + { + "reason": "User appeal approved - first offense" + } + ``` + + Create exception for org-level ban: + + ```json + { + "course_id": "course-v1:HarvardX+CS50+2024", + "reason": "Exception approved for CS50 only" + } + ``` + + **Response Values** + + * status: Success status of the operation + * message: Human-readable message describing the action taken + * exception_created: Boolean indicating if an org-level exception was created + + **Notes** + + * For course-level bans: Deactivates the ban completely + * For org-level bans without course_id: Deactivates entire org-level ban + * For org-level bans with course_id: Creates exception allowing user in that course only + * All unban actions are logged in DiscussionModerationLog + """ + from lms.djangoapps.discussion.models import DiscussionBan, DiscussionBanException, DiscussionModerationLog + from lms.djangoapps.discussion.rest_api.permissions import can_take_action_on_spam + + try: + ban = DiscussionBan.objects.get(id=pk, is_active=True) + except DiscussionBan.DoesNotExist: + return Response( + {'error': 'Active ban not found'}, + status=status.HTTP_404_NOT_FOUND + ) + + course_id = request.data.get('course_id') + reason = request.data.get('reason', '').strip() + + # Permission check: depends on ban type and what user is trying to do + if ban.course_id: + # Course-level ban - check permissions for that specific course + if not can_take_action_on_spam(request.user, ban.course_id): + return Response( + {'error': 'You do not have permission to unban users in this course'}, + status=status.HTTP_403_FORBIDDEN + ) + else: + # Org-level ban + from common.djangoapps.student.roles import GlobalStaff + if course_id: + # Creating exception for specific course - check permissions in that course + if not can_take_action_on_spam(request.user, course_id): + return Response( + {'error': 'You do not have permission to create exceptions in this course'}, + status=status.HTTP_403_FORBIDDEN + ) + else: + # Fully unbanning org-level ban - only global staff can do this + if not (GlobalStaff().has_user(request.user) or request.user.is_staff): + return Response( + {'error': 'Only global staff can fully unban organization-level bans'}, + status=status.HTTP_403_FORBIDDEN + ) + + # Validate that reason is provided + if not reason: + return Response( + {'error': 'reason field is required'}, + status=status.HTTP_400_BAD_REQUEST + ) + + exception_created = False + + # For org-level bans with course_id: create exception instead of full unban + if ban.scope == 'organization' and course_id: + course_key = CourseKey.from_string(course_id) + + # Create exception for this specific course + exception, created = DiscussionBanException.objects.get_or_create( + ban=ban, + course_id=course_key, + defaults={ + 'unbanned_by': request.user, + 'reason': reason, + } + ) + + exception_created = True + message = ( + f'User {ban.user.username} unbanned from {course_id} ' + f'(org-level ban still active for other courses)' + ) + + # Audit log for exception + DiscussionModerationLog.objects.create( + action_type=DiscussionModerationLog.ACTION_BAN_EXCEPTION, + target_user=ban.user, + moderator=request.user, + course_id=course_key, + scope='organization', + reason=f"Exception to org ban: {reason}", + metadata={ + 'ban_id': ban.id, + 'exception_id': exception.id, + 'exception_created': created, + 'organization': ban.org_key + } + ) + else: + # Full unban (course-level or complete org-level unban) + ban.is_active = False + ban.unbanned_at = timezone.now() + ban.unbanned_by = request.user + ban.save() + + message = f'User {ban.user.username} unbanned successfully' + + # Audit log + DiscussionModerationLog.objects.create( + action_type=DiscussionModerationLog.ACTION_UNBAN, + target_user=ban.user, + moderator=request.user, + course_id=ban.course_id, + scope=ban.scope, + reason=f"Unban: {reason}", + ) + + return Response({ + 'status': 'success', + 'message': message, + 'exception_created': exception_created + }) diff --git a/lms/djangoapps/discussion/templates/discussion/ban_escalation_email.txt b/lms/djangoapps/discussion/templates/discussion/ban_escalation_email.txt new file mode 100644 index 000000000000..f8a02b664b18 --- /dev/null +++ b/lms/djangoapps/discussion/templates/discussion/ban_escalation_email.txt @@ -0,0 +1,28 @@ +DISCUSSION BAN ALERT +================================================================================ + +A user has been banned from course discussions. + +Banned User: {{ banned_username }} ({{ banned_email }}) +User ID: {{ banned_user_id }} + +Moderator: {{ moderator_username }} ({{ moderator_email }}) +Moderator ID: {{ moderator_id }} + +Course ID: {{ course_id }} +Ban Scope: {{ scope|upper }} + +Reason: {{ reason }} + +Content Deleted: +- Threads: {{ threads_deleted }} +- Comments: {{ comments_deleted }} +- Total: {{ total_deleted }} + +================================================================================ + +ACTION REQUIRED: +Please review this moderation action and follow up as needed. If this ban was +applied in error or requires adjustment, contact the moderator or course staff. + +================================================================================ diff --git a/lms/djangoapps/discussion/templates/discussion/edx_ace/ban_escalation/email/body.html b/lms/djangoapps/discussion/templates/discussion/edx_ace/ban_escalation/email/body.html new file mode 100644 index 000000000000..d6a57962433f --- /dev/null +++ b/lms/djangoapps/discussion/templates/discussion/edx_ace/ban_escalation/email/body.html @@ -0,0 +1,82 @@ +{% extends 'ace_common/edx_ace/common/base_body.html' %} + +{% load i18n %} +{% load django_markup %} +{% load static %} + +{% block content %} + + + + +
+

+ {% filter force_escape %} + {% blocktrans %} + Discussion Ban Alert + {% endblocktrans %} + {% endfilter %} +

+ +

+ {% filter force_escape %} + {% blocktrans %} + A user has been banned from course discussions. Please review this moderation action. + {% endblocktrans %} + {% endfilter %} +

+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
Banned User{{ banned_username }} ({{ banned_email }})
User ID{{ banned_user_id }}
Moderator{{ moderator_username }} ({{ moderator_email }})
Moderator ID{{ moderator_id }}
Course ID{{ course_id }}
Ban Scope + {{ scope|upper }}{% if scope == 'organization' %} (All courses in organization){% endif %} +
Reason{{ reason }}
Content Deleted + {{ threads_deleted }} thread{{ threads_deleted|pluralize }}, + {{ comments_deleted }} comment{{ comments_deleted|pluralize }} +
+ Total: {{ total_deleted }} item{{ total_deleted|pluralize }} +
+ +

+ {% trans "Action Required:" as action_required %}{{ action_required|force_escape }} + {% trans "Please review this moderation action and follow up as needed. If this ban was applied in error or requires adjustment, contact the moderator or course staff." as review_instructions %}{{ review_instructions|force_escape }} +

+ + {% block google_analytics_pixel %} + {% if ga_tracking_pixel_url %} + + {% endif %} + {% endblock %} +
+{% endblock %} diff --git a/lms/djangoapps/discussion/templates/discussion/edx_ace/ban_escalation/email/body.txt b/lms/djangoapps/discussion/templates/discussion/edx_ace/ban_escalation/email/body.txt new file mode 100644 index 000000000000..f8a02b664b18 --- /dev/null +++ b/lms/djangoapps/discussion/templates/discussion/edx_ace/ban_escalation/email/body.txt @@ -0,0 +1,28 @@ +DISCUSSION BAN ALERT +================================================================================ + +A user has been banned from course discussions. + +Banned User: {{ banned_username }} ({{ banned_email }}) +User ID: {{ banned_user_id }} + +Moderator: {{ moderator_username }} ({{ moderator_email }}) +Moderator ID: {{ moderator_id }} + +Course ID: {{ course_id }} +Ban Scope: {{ scope|upper }} + +Reason: {{ reason }} + +Content Deleted: +- Threads: {{ threads_deleted }} +- Comments: {{ comments_deleted }} +- Total: {{ total_deleted }} + +================================================================================ + +ACTION REQUIRED: +Please review this moderation action and follow up as needed. If this ban was +applied in error or requires adjustment, contact the moderator or course staff. + +================================================================================ diff --git a/lms/djangoapps/discussion/templates/discussion/edx_ace/ban_escalation/email/from_name.txt b/lms/djangoapps/discussion/templates/discussion/edx_ace/ban_escalation/email/from_name.txt new file mode 100644 index 000000000000..fb090bda4e0e --- /dev/null +++ b/lms/djangoapps/discussion/templates/discussion/edx_ace/ban_escalation/email/from_name.txt @@ -0,0 +1 @@ +edX Discussion Moderation \ No newline at end of file diff --git a/lms/djangoapps/discussion/templates/discussion/edx_ace/ban_escalation/email/head.html b/lms/djangoapps/discussion/templates/discussion/edx_ace/ban_escalation/email/head.html new file mode 100644 index 000000000000..366ada7ad92e --- /dev/null +++ b/lms/djangoapps/discussion/templates/discussion/edx_ace/ban_escalation/email/head.html @@ -0,0 +1 @@ +{% extends 'ace_common/edx_ace/common/base_head.html' %} diff --git a/lms/djangoapps/discussion/templates/discussion/edx_ace/ban_escalation/email/subject.txt b/lms/djangoapps/discussion/templates/discussion/edx_ace/ban_escalation/email/subject.txt new file mode 100644 index 000000000000..a3e4a972368b --- /dev/null +++ b/lms/djangoapps/discussion/templates/discussion/edx_ace/ban_escalation/email/subject.txt @@ -0,0 +1 @@ +Discussion Moderation Alert: User Banned \ No newline at end of file diff --git a/lms/djangoapps/discussion/tests/test_models.py b/lms/djangoapps/discussion/tests/test_models.py new file mode 100644 index 000000000000..e7ebb3b5cc4b --- /dev/null +++ b/lms/djangoapps/discussion/tests/test_models.py @@ -0,0 +1,367 @@ +""" +Tests for discussion moderation models. +""" +from django.contrib.auth import get_user_model +from django.core.exceptions import ValidationError +from django.db import IntegrityError + +from lms.djangoapps.discussion.models import ( + DiscussionBan, + DiscussionBanException, + DiscussionModerationLog, +) +from common.djangoapps.student.tests.factories import UserFactory +from xmodule.modulestore.tests.factories import CourseFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase + +User = get_user_model() + + +class DiscussionBanModelTest(ModuleStoreTestCase): + """Tests for DiscussionBan model.""" + + def setUp(self): + super().setUp() + self.course = CourseFactory.create(org='TestX', number='CS101', run='2024') + self.course_key = self.course.id + self.user = UserFactory.create(username='testuser') + self.moderator = UserFactory.create(username='moderator') + + def test_create_course_level_ban(self): + """Test creating a course-level ban.""" + ban = DiscussionBan.objects.create( + user=self.user, + course_id=self.course_key, + scope='course', + banned_by=self.moderator, + reason='Spam posting', + is_active=True + ) + + self.assertEqual(ban.user, self.user) + self.assertEqual(ban.course_id, self.course_key) + self.assertEqual(ban.scope, 'course') + self.assertEqual(ban.banned_by, self.moderator) + self.assertTrue(ban.is_active) + self.assertIsNotNone(ban.banned_at) + self.assertIsNone(ban.unbanned_at) + + def test_create_organization_level_ban(self): + """Test creating an organization-level ban.""" + ban = DiscussionBan.objects.create( + user=self.user, + org_key='TestX', + scope='organization', + banned_by=self.moderator, + reason='Repeated violations', + is_active=True + ) + + self.assertEqual(ban.scope, 'organization') + self.assertEqual(ban.org_key, 'TestX') + self.assertIsNone(ban.course_id) + + def test_unique_constraint_course_ban(self): + """Test that duplicate active course-level bans are prevented.""" + DiscussionBan.objects.create( + user=self.user, + course_id=self.course_key, + scope='course', + banned_by=self.moderator, + reason='First ban', + is_active=True + ) + + with self.allow_transaction_exception(): + with self.assertRaises(IntegrityError): + DiscussionBan.objects.create( + user=self.user, + course_id=self.course_key, + scope='course', + banned_by=self.moderator, + reason='Second ban', + is_active=True + ) + + def test_unique_constraint_allows_inactive_duplicates(self): + """Test that inactive bans don't violate unique constraint.""" + DiscussionBan.objects.create( + user=self.user, + course_id=self.course_key, + scope='course', + banned_by=self.moderator, + reason='First ban', + is_active=False + ) + + # Should not raise IntegrityError + ban = DiscussionBan.objects.create( + user=self.user, + course_id=self.course_key, + scope='course', + banned_by=self.moderator, + reason='Second ban', + is_active=True + ) + + self.assertTrue(ban.is_active) + + def test_clean_course_ban_requires_course_id(self): + """Test that course-level bans require course_id.""" + ban = DiscussionBan( + user=self.user, + scope='course', + banned_by=self.moderator, + reason='Test' + ) + + with self.assertRaises(ValidationError): + ban.clean() + + def test_clean_org_ban_requires_organization(self): + """Test that organization-level bans require organization.""" + ban = DiscussionBan( + user=self.user, + scope='organization', + banned_by=self.moderator, + reason='Test' + ) + + with self.assertRaises(ValidationError): + ban.clean() + + def test_clean_org_ban_rejects_course_id(self): + """Test that organization-level bans should not have course_id.""" + ban = DiscussionBan( + user=self.user, + course_id=self.course_key, + org_key='TestX', + scope='organization', + banned_by=self.moderator, + reason='Test' + ) + + with self.assertRaises(ValidationError): + ban.clean() + + def test_is_user_banned_course_level(self): + """Test is_user_banned for course-level bans.""" + self.assertFalse(DiscussionBan.is_user_banned(self.user, self.course_key)) + + DiscussionBan.objects.create( + user=self.user, + course_id=self.course_key, + scope='course', + banned_by=self.moderator, + reason='Test', + is_active=True + ) + + self.assertTrue(DiscussionBan.is_user_banned(self.user, self.course_key)) + + def test_is_user_banned_organization_level(self): + """Test is_user_banned for organization-level bans.""" + DiscussionBan.objects.create( + user=self.user, + org_key='TestX', + scope='organization', + banned_by=self.moderator, + reason='Test', + is_active=True + ) + + self.assertTrue(DiscussionBan.is_user_banned(self.user, self.course_key)) + + def test_is_user_banned_with_exception(self): + """Test that ban exceptions override organization-level bans.""" + org_ban = DiscussionBan.objects.create( + user=self.user, + org_key='TestX', + scope='organization', + banned_by=self.moderator, + reason='Test', + is_active=True + ) + + # User is banned + self.assertTrue(DiscussionBan.is_user_banned(self.user, self.course_key)) + + # Create exception for this course + DiscussionBanException.objects.create( + ban=org_ban, + course_id=self.course_key, + unbanned_by=self.moderator, + reason='Exception for CS101' + ) + + # User is no longer banned in this course + self.assertFalse(DiscussionBan.is_user_banned(self.user, self.course_key)) + + def test_str_representation_course(self): + """Test string representation of course-level ban.""" + ban = DiscussionBan.objects.create( + user=self.user, + course_id=self.course_key, + scope='course', + banned_by=self.moderator, + reason='Test' + ) + + expected = f"Ban: {self.user.username} in {self.course_key} (course-level)" + self.assertEqual(str(ban), expected) + + def test_str_representation_org(self): + """Test string representation of organization-level ban.""" + ban = DiscussionBan.objects.create( + user=self.user, + org_key='TestX', + scope='organization', + banned_by=self.moderator, + reason='Test' + ) + + expected = f"Ban: {self.user.username} in TestX (org-level)" + self.assertEqual(str(ban), expected) + + +class DiscussionBanExceptionModelTest(ModuleStoreTestCase): + """Tests for DiscussionBanException model.""" + + def setUp(self): + super().setUp() + self.course = CourseFactory.create(org='TestX', number='CS101', run='2024') + self.course_key = self.course.id + self.user = UserFactory.create() + self.moderator = UserFactory.create() + + self.org_ban = DiscussionBan.objects.create( + user=self.user, + org_key='TestX', + scope='organization', + banned_by=self.moderator, + reason='Org-level ban' + ) + + def test_create_exception(self): + """Test creating a ban exception.""" + exception = DiscussionBanException.objects.create( + ban=self.org_ban, + course_id=self.course_key, + unbanned_by=self.moderator, + reason='Special exception' + ) + + self.assertEqual(exception.ban, self.org_ban) + self.assertEqual(exception.course_id, self.course_key) + self.assertEqual(exception.unbanned_by, self.moderator) + + def test_unique_constraint(self): + """Test that duplicate exceptions are prevented.""" + DiscussionBanException.objects.create( + ban=self.org_ban, + course_id=self.course_key, + unbanned_by=self.moderator + ) + + with self.allow_transaction_exception(): + with self.assertRaises(IntegrityError): + DiscussionBanException.objects.create( + ban=self.org_ban, + course_id=self.course_key, + unbanned_by=self.moderator + ) + + def test_clean_requires_org_ban(self): + """Test that exceptions only work for organization-level bans.""" + course_ban = DiscussionBan.objects.create( + user=self.user, + course_id=self.course_key, + scope='course', + banned_by=self.moderator, + reason='Course ban' + ) + + exception = DiscussionBanException( + ban=course_ban, + course_id=self.course_key, + unbanned_by=self.moderator + ) + + with self.assertRaises(ValidationError): + exception.clean() + + def test_str_representation(self): + """Test string representation.""" + exception = DiscussionBanException.objects.create( + ban=self.org_ban, + course_id=self.course_key, + unbanned_by=self.moderator + ) + + expected = f"Exception: {self.user.username} allowed in {self.course_key}" + self.assertEqual(str(exception), expected) + + +class DiscussionModerationLogModelTest(ModuleStoreTestCase): + """Tests for DiscussionModerationLog model.""" + + def setUp(self): + super().setUp() + self.course = CourseFactory.create() + self.course_key = self.course.id + self.user = UserFactory.create() + self.moderator = UserFactory.create() + + def test_create_ban_log(self): + """Test creating a ban action log.""" + log = DiscussionModerationLog.objects.create( + action_type=DiscussionModerationLog.ACTION_BAN, + target_user=self.user, + moderator=self.moderator, + course_id=self.course_key, + scope='course', + reason='Spam', + metadata={'posts_deleted': 10} + ) + + self.assertEqual(log.action_type, 'ban_user') + self.assertEqual(log.target_user, self.user) + self.assertEqual(log.moderator, self.moderator) + self.assertEqual(log.metadata['posts_deleted'], 10) + + def test_create_unban_log(self): + """Test creating an unban action log.""" + log = DiscussionModerationLog.objects.create( + action_type=DiscussionModerationLog.ACTION_UNBAN, + target_user=self.user, + moderator=self.moderator, + course_id=self.course_key, + scope='course', + reason='Appeal approved' + ) + + self.assertEqual(log.action_type, 'unban_user') + + def test_query_by_user(self): + """Test querying logs by target user.""" + DiscussionModerationLog.objects.create( + action_type=DiscussionModerationLog.ACTION_BAN, + target_user=self.user, + moderator=self.moderator, + course_id=self.course_key + ) + + logs = DiscussionModerationLog.objects.filter(target_user=self.user) + self.assertEqual(logs.count(), 1) + + def test_str_representation(self): + """Test string representation.""" + log = DiscussionModerationLog.objects.create( + action_type=DiscussionModerationLog.ACTION_BAN, + target_user=self.user, + moderator=self.moderator, + course_id=self.course_key + ) + + expected = f"ban_user: {self.user.username} by {self.moderator.username}" + self.assertEqual(str(log), expected) diff --git a/lms/djangoapps/discussion/toggles.py b/lms/djangoapps/discussion/toggles.py index 61286686b759..9de8d0250555 100644 --- a/lms/djangoapps/discussion/toggles.py +++ b/lms/djangoapps/discussion/toggles.py @@ -37,3 +37,20 @@ # .. toggle_creation_date: 2025-07-29 # .. toggle_target_removal_date: 2026-07-29 ENABLE_RATE_LIMIT_IN_DISCUSSION = CourseWaffleFlag(f'{WAFFLE_FLAG_NAMESPACE}.enable_rate_limit', __name__) + + +# .. toggle_name: discussions.enable_discussion_ban +# .. toggle_implementation: CourseWaffleFlag +# .. toggle_default: False +# .. toggle_description: Waffle flag to enable ban user functionality in discussion moderation. +# When enabled, moderators can ban users from discussions at course or organization level +# during bulk delete operations. This addresses crypto spam attacks and harassment. +# .. toggle_use_cases: opt_in +# .. toggle_creation_date: 2024-11-24 +# .. toggle_target_removal_date: 2025-06-01 +# .. toggle_warning: This feature requires proper moderator training to prevent misuse. +# Ensure DISCUSSION_MODERATION_BAN_EMAIL_ENABLED is configured appropriately for your environment. +# .. toggle_tickets: COSMO2-736 +ENABLE_DISCUSSION_BAN = CourseWaffleFlag( + f'{WAFFLE_FLAG_NAMESPACE}.enable_discussion_ban', __name__ +) diff --git a/lms/envs/common.py b/lms/envs/common.py index 3dde7156b93e..cb6cc78e9fb1 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -3489,6 +3489,36 @@ AVAILABLE_DISCUSSION_TOURS = [] +############## DISCUSSION MODERATION ############## + +# .. toggle_name: settings.DISCUSSION_MODERATION_BAN_EMAIL_ENABLED +# .. toggle_implementation: DjangoSetting +# .. toggle_default: True +# .. toggle_description: Enable/disable email notifications when users are banned from discussions. +# Set to False in development/test environments to prevent spam to partner-support@edx.org. +# When enabled, escalation emails are sent to DISCUSSION_MODERATION_ESCALATION_EMAIL address. +# .. toggle_use_cases: opt_in +# .. toggle_creation_date: 2024-11-24 +# .. toggle_tickets: COSMO2-736 +DISCUSSION_MODERATION_BAN_EMAIL_ENABLED = True + +# .. setting_name: DISCUSSION_MODERATION_ESCALATION_EMAIL +# .. setting_default: 'partner-support@edx.org' +# .. setting_description: Email address to receive ban escalation notifications when users are banned +# from discussions. Override in development to use a test email address. +# .. setting_use_cases: opt_in +# .. setting_creation_date: 2024-11-24 +# .. setting_tickets: COSMO2-736 +DISCUSSION_MODERATION_ESCALATION_EMAIL = 'partner-support@edx.org' + +# .. setting_name: DISCUSSION_MODERATION_BAN_REASON_MAX_LENGTH +# .. setting_default: 1000 +# .. setting_description: Maximum character length for ban reason text. +# .. setting_use_cases: opt_in +# .. setting_creation_date: 2024-11-24 +# .. setting_tickets: COSMO2-736 +DISCUSSION_MODERATION_BAN_REASON_MAX_LENGTH = 1000 + ############## NOTIFICATIONS ############## NOTIFICATION_TYPE_ICONS = {} DEFAULT_NOTIFICATION_ICON_URL = "" diff --git a/lms/envs/devstack.py b/lms/envs/devstack.py index b15533855c7b..b20db319ff37 100644 --- a/lms/envs/devstack.py +++ b/lms/envs/devstack.py @@ -40,6 +40,10 @@ CLEAR_REQUEST_CACHE_ON_TASK_COMPLETION = False HTTPS = 'off' +# Disable ban emails in local development to prevent spam +DISCUSSION_MODERATION_BAN_EMAIL_ENABLED = False +DISCUSSION_MODERATION_ESCALATION_EMAIL = 'devnull@example.com' + LMS_ROOT_URL = f'http://{LMS_BASE}' LMS_INTERNAL_ROOT_URL = LMS_ROOT_URL ENTERPRISE_API_URL = f'{LMS_INTERNAL_ROOT_URL}/enterprise/api/v1/' diff --git a/lms/envs/test.py b/lms/envs/test.py index 218a7e8461b8..67b56a954404 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -72,6 +72,10 @@ # the one in cms/envs/test.py ENABLE_DISCUSSION_SERVICE = False +# Disable ban emails in tests to prevent spam and speed up tests +DISCUSSION_MODERATION_BAN_EMAIL_ENABLED = False +DISCUSSION_MODERATION_ESCALATION_EMAIL = 'test@example.com' + ENABLE_SERVICE_STATUS = True ENABLE_VERIFIED_CERTIFICATES = True