From 78f4ccb50a94c71d3d4b9f79df2f96d403d587de Mon Sep 17 00:00:00 2001 From: Josue Balandrano Coronel Date: Tue, 26 Feb 2019 15:49:07 -0600 Subject: [PATCH] Expose LoginFailure model to Django's Admin UI to allow admin users unlock student accounts that have been locked out for excessive login failures. This change will allow to quickly unlock student accounts when time is of the essence (e.g. Timed exams). --- common/djangoapps/student/admin.py | 116 +++++++++++++++++- .../migrations/0020_auto_20190227_2019.py | 19 +++ common/djangoapps/student/models.py | 29 +++++ .../student/tests/test_admin_views.py | 70 ++++++++++- .../loginfailures/change_form_template.html | 32 +++++ 5 files changed, 263 insertions(+), 3 deletions(-) create mode 100644 common/djangoapps/student/migrations/0020_auto_20190227_2019.py create mode 100644 common/templates/admin/student/loginfailures/change_form_template.html diff --git a/common/djangoapps/student/admin.py b/common/djangoapps/student/admin.py index 716c9eb89b..65868444fa 100644 --- a/common/djangoapps/student/admin.py +++ b/common/djangoapps/student/admin.py @@ -1,14 +1,19 @@ """ Django admin pages for student app """ +from functools import wraps from config_models.admin import ConfigurationModelAdmin from django import forms +from django.db import router, transaction from django.contrib import admin from django.contrib.admin.sites import NotRegistered +from django.contrib.admin.utils import unquote from django.contrib.auth import get_user_model from django.contrib.auth.admin import UserAdmin as BaseUserAdmin from django.contrib.auth.forms import ReadOnlyPasswordHashField, UserChangeForm as BaseUserChangeForm from django.db import models +from django.http import HttpResponseRedirect from django.http.request import QueryDict -from django.utils.translation import ugettext_lazy as _ +from django.utils.translation import ugettext_lazy as _, ngettext +from django.urls import reverse from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey @@ -27,7 +32,8 @@ from student.models import ( RegistrationCookieConfiguration, UserAttribute, UserProfile, - UserTestGroup + UserTestGroup, + LoginFailures, ) from student.roles import REGISTERED_ACCESS_ROLES from xmodule.modulestore.django import modulestore @@ -316,6 +322,112 @@ class CourseEnrollmentAllowedAdmin(admin.ModelAdmin): model = CourseEnrollmentAllowed +@admin.register(LoginFailures) +class LoginFailuresAdmin(admin.ModelAdmin): + """Admin interface for the LoginFailures model. """ + list_display = ('user', 'failure_count', 'lockout_until') + raw_id_fields = ('user',) + search_fields = ('user__username', 'user__email', 'user__first_name', 'user__last_name') + actions = ['unlock_student_accounts'] + change_form_template = 'admin/student/loginfailures/change_form_template.html' + + class _Feature(object): + """ + Inner feature class to implement decorator. + """ + @classmethod + def is_enabled(cls, func): + """ + Check if feature is enabled. + """ + @wraps(func) + def decorator(*args, **kwargs): + """Decorator class to return""" + if not LoginFailures.is_feature_enabled(): + return False + return func(*args, **kwargs) + return decorator + + @_Feature.is_enabled + def has_module_permission(self, request): + """ + Only enabled if feature is enabled. + """ + return super(LoginFailuresAdmin, self).has_module_permission(request) + + @_Feature.is_enabled + def has_delete_permission(self, request, obj=None): + """ + Only enabled if feature is enabled. + """ + return super(LoginFailuresAdmin, self).has_delete_permission(request, obj) + + @_Feature.is_enabled + def has_change_permission(self, request, obj=None): + """ + Only enabled if feature is enabled. + """ + return super(LoginFailuresAdmin, self).has_change_permission(request, obj) + + @_Feature.is_enabled + def has_add_permission(self, request): + """ + Only enabled if feature is enabled. + """ + return super(LoginFailuresAdmin, self).has_add_permission(request) + + def unlock_student_accounts(self, request, queryset): + """ + Unlock student accounts with login failures. + """ + count = 0 + with transaction.atomic(using=router.db_for_write(self.model)): + for obj in queryset: + self.unlock_student(request, obj=obj) + count += 1 + self.message_user( + request, + ngettext( + '%(count)d student account was unlocked.', + '%(count)d student accounts were unlocked.', + count + ) % { + 'count': count + } + ) + + def change_view(self, request, object_id, form_url='', extra_context=None): + """ + Change View. + + This is overridden so we can add a custom button to unlock an account in the record's details. + """ + if '_unlock' in request.POST: + with transaction.atomic(using=router.db_for_write(self.model)): + self.unlock_student(request, object_id=object_id) + url = reverse('admin:student_loginfailures_changelist', current_app=self.admin_site.name) + return HttpResponseRedirect(url) + return super(LoginFailuresAdmin, self).change_view(request, object_id, form_url, extra_context) + + def get_actions(self, request): + """ + Get actions for model admin and remove delete action. + """ + actions = super(LoginFailuresAdmin, self).get_actions(request) + if 'delete_selected' in actions: + del actions['delete_selected'] + return actions + + def unlock_student(self, request, object_id=None, obj=None): + """ + Unlock student account. + """ + if object_id: + obj = self.get_object(request, unquote(object_id)) + + self.model.clear_lockout_counter(obj.user) + + admin.site.register(UserTestGroup) admin.site.register(Registration) admin.site.register(PendingNameChange) diff --git a/common/djangoapps/student/migrations/0020_auto_20190227_2019.py b/common/djangoapps/student/migrations/0020_auto_20190227_2019.py new file mode 100644 index 0000000000..7d83469426 --- /dev/null +++ b/common/djangoapps/student/migrations/0020_auto_20190227_2019.py @@ -0,0 +1,19 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.20 on 2019-02-27 20:19 +from __future__ import unicode_literals + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('student', '0019_auto_20181221_0540'), + ] + + operations = [ + migrations.AlterModelOptions( + name='loginfailures', + options={'verbose_name': 'Login Failure', 'verbose_name_plural': 'Login Failures'}, + ), + ] diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 09ccc4ccaf..540eae7dc0 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -854,6 +854,7 @@ EVENT_NAME_ENROLLMENT_DEACTIVATED = 'edx.course.enrollment.deactivated' EVENT_NAME_ENROLLMENT_MODE_CHANGED = 'edx.course.enrollment.mode_changed' +@six.python_2_unicode_compatible class LoginFailures(models.Model): """ This model will keep track of failed login attempts. @@ -930,6 +931,34 @@ class LoginFailures(models.Model): except ObjectDoesNotExist: return + def __repr__(self): + """Repr -> LoginFailures(username, count, date)""" + date_str = '-' + if self.lockout_until is not None: + date_str = self.lockout_until.isoformat() + + return u'LoginFailures({username}, {count}, {date})'.format( + username=unicode(self.user.username, 'utf-8'), + count=self.failure_count, + date=date_str + ) + + def __str__(self): + """Str -> Username: count - date.""" + date_str = '-' + if self.lockout_until is not None: + date_str = self.lockout_until.isoformat() + + return u'{username}: {count} - {date}'.format( + username=unicode(self.user.username, 'utf-8'), + count=self.failure_count, + date=date_str + ) + + class Meta: + verbose_name = 'Login Failure' + verbose_name_plural = 'Login Failures' + class CourseEnrollmentException(Exception): pass diff --git a/common/djangoapps/student/tests/test_admin_views.py b/common/djangoapps/student/tests/test_admin_views.py index e60170f8e2..b9fae98969 100644 --- a/common/djangoapps/student/tests/test_admin_views.py +++ b/common/djangoapps/student/tests/test_admin_views.py @@ -1,15 +1,18 @@ """ Tests student admin.py """ +import datetime import ddt from django.contrib.admin.sites import AdminSite from django.contrib.auth.models import User +from django.conf import settings from django.forms import ValidationError from django.urls import reverse -from django.test import TestCase +from django.test import TestCase, override_settings from mock import Mock from student.admin import COURSE_ENROLLMENT_ADMIN_SWITCH, UserAdmin +from student.models import LoginFailures from student.tests.factories import CourseEnrollmentFactory, UserFactory from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory @@ -298,3 +301,68 @@ class CourseEnrollmentAdminTest(SharedModuleStoreTestCase): reverse('admin:student_courseenrollment_change', args=(self.course_enrollment.id, )), data=data, ) + + +@ddt.ddt +class LoginFailuresAdminTest(TestCase): + """Test Login Failures Admin.""" + + @classmethod + def setUpClass(cls): + """Setup class""" + super(LoginFailuresAdminTest, cls).setUpClass() + cls.user = UserFactory.create(is_staff=True, is_superuser=True) + cls.user.save() + + def setUp(self): + """Setup.""" + super(LoginFailuresAdminTest, self).setUp() + self.client.login(username=self.user.username, password='test') + user = UserFactory.create() + LoginFailures.objects.create(user=self.user, failure_count=10, lockout_until=datetime.datetime.now()) + LoginFailures.objects.create(user=user, failure_count=2) + + def tearDown(self): + """Tear Down.""" + super(LoginFailuresAdminTest, self).tearDown() + LoginFailures.objects.all().delete() + + @ddt.data( + reverse('admin:student_loginfailures_changelist'), + reverse('admin:student_loginfailures_add'), + reverse('admin:student_loginfailures_change', args=(1,)), + reverse('admin:student_loginfailures_delete', args=(1,)), + ) + def test_feature_disabled(self, url): + """Test if feature is disabled there's no access to the admin module.""" + response = self.client.get(url) + self.assertEqual(response.status_code, 403) + response = self.client.post(url) + self.assertEqual(response.status_code, 403) + + @override_settings(FEATURES={'ENABLE_MAX_FAILED_LOGIN_ATTEMPTS': True}) + def test_unlock_student_accounts(self): + """Test batch unlock student accounts.""" + url = reverse('admin:student_loginfailures_changelist') + self.client.post( + url, + data={ + 'action': 'unlock_student_accounts', + '_selected_action': [unicode(o.pk) for o in LoginFailures.objects.all()] + }, + follow=True + ) + count = LoginFailures.objects.count() + self.assertEqual(count, 0) + + @override_settings(FEATURES={'ENABLE_MAX_FAILED_LOGIN_ATTEMPTS': True}) + def test_unlock_account(self): + """Test unlock single student account.""" + url = reverse('admin:student_loginfailures_change', args=(1, )) + start_count = LoginFailures.objects.count() + self.client.post( + url, + data={'_unlock': 1} + ) + count = LoginFailures.objects.count() + self.assertEqual(count, start_count - 1) diff --git a/common/templates/admin/student/loginfailures/change_form_template.html b/common/templates/admin/student/loginfailures/change_form_template.html new file mode 100644 index 0000000000..ce3b4766c2 --- /dev/null +++ b/common/templates/admin/student/loginfailures/change_form_template.html @@ -0,0 +1,32 @@ +{% extends 'admin/change_form.html' %} +{% load i18n admin_urls %} +{% block submit_buttons_top %} +
+ {% if original.lockout_until %} + + {% endif %} + + {% trans 'Close' %} + +
+{% endblock %} +{% block submit_buttons_bottom %} +
+ {% if original.lockout_until %} +

+ +

+ {% endif %} + + {% trans 'Close' %} + +
+{% endblock %}