custom_user: remove User's deleted field (#51452)

This commit is contained in:
Benjamin Dauvergne 2021-02-26 11:15:02 +01:00
parent 8e0f6d3cb7
commit 8a543226bf
21 changed files with 100 additions and 83 deletions

View File

@ -273,7 +273,7 @@ class UserCreationForm(BaseUserForm):
class AuthenticUserAdmin(UserAdmin):
fieldsets = (
(None, {'fields': ('uuid', 'ou', 'password', 'deleted')}),
(None, {'fields': ('uuid', 'ou', 'password')}),
(_('Personal info'), {'fields': ('username', 'first_name', 'last_name', 'email')}),
(_('Permissions'), {'fields': ('is_active', 'is_staff', 'is_superuser',
'groups')}),
@ -285,8 +285,8 @@ class AuthenticUserAdmin(UserAdmin):
'fields': ('ou', 'username', 'first_name', 'last_name', 'email', 'password1', 'password2')}),
)
readonly_fields = ('uuid',)
list_filter = UserAdmin.list_filter + (UserRealmListFilter, ExternalUserListFilter) + ('deleted',)
list_display = ['__str__', 'ou', 'first_name', 'last_name', 'email', 'deleted']
list_filter = UserAdmin.list_filter + (UserRealmListFilter, ExternalUserListFilter)
list_display = ['__str__', 'ou', 'first_name', 'last_name', 'email']
actions = UserAdmin.actions + ['mark_as_inactive']
def get_fieldsets(self, request, obj=None):

View File

@ -168,8 +168,7 @@ class RegistrationSerializer(serializers.Serializer):
raise serializers.ValidationError(
_('Email is required'))
if User.objects.filter(
email__iexact=data['email'],
deleted__isnull=True).exists():
email__iexact=data['email']).exists():
raise serializers.ValidationError(
_('Account already exists'))
@ -178,8 +177,7 @@ class RegistrationSerializer(serializers.Serializer):
raise serializers.ValidationError(
_('Email is required in this ou'))
if User.objects.filter(
ou=ou, email__iexact=data['email'],
deleted__isnull=True).exists():
ou=ou, email__iexact=data['email']).exists():
raise serializers.ValidationError(
_('Account already exists in this ou'))
@ -189,8 +187,7 @@ class RegistrationSerializer(serializers.Serializer):
raise serializers.ValidationError(
_('Username is required'))
if User.objects.filter(
username=data['username'],
deleted__isnull=True).exists():
username=data['username']).exists():
raise serializers.ValidationError(
_('Account already exists'))
@ -199,8 +196,7 @@ class RegistrationSerializer(serializers.Serializer):
raise serializers.ValidationError(
_('Username is required in this ou'))
if User.objects.filter(
ou=ou, username=data['username'],
deleted__isnull=True).exists():
ou=ou, username=data['username']).exists():
raise serializers.ValidationError(
_('Account already exists in this ou'))
return data
@ -345,7 +341,7 @@ class PasswordChangeSerializer(serializers.Serializer):
def validate(self, data):
User = get_user_model()
qs = User.objects.filter(email=data['email'], deleted__isnull=True)
qs = User.objects.filter(email=data['email'])
if data['ou']:
qs = qs.filter(ou=data['ou'])
try:
@ -532,10 +528,10 @@ class BaseUserSerializer(serializers.ModelSerializer):
and data.get('email')
and (not self.instance or data.get('email') != self.instance.email)):
if app_settings.A2_EMAIL_IS_UNIQUE and qs.filter(
email=data['email'], deleted__isnull=True).exists():
email=data['email']).exists():
already_used = True
if ou and ou.email_is_unique and qs.filter(
ou=ou, email=data['email'], deleted__isnull=True).exists():
ou=ou, email=data['email']).exists():
already_used = True
errors = {}
@ -741,7 +737,7 @@ class FreeTextSearchFilter(BaseFilterBackend):
class UsersAPI(api_mixins.GetOrCreateMixinView, HookMixin, ExceptionHandlerMixin, ModelViewSet):
queryset = User.objects.filter(deleted__isnull=True)
queryset = User.objects.all()
ordering_fields = ['username', 'first_name', 'last_name', 'modified', 'date_joined']
lookup_field = 'uuid'
serializer_class = BaseUserSerializer

View File

@ -116,7 +116,7 @@ You can use <tt>event:login</tt> to find all events of type <tt>login</tt>.'''
return models.EventQuerySet._which_references_query(users)
def search_by_email(self, email):
users = User.objects.filter(email__iexact=email.lower(), deleted__isnull=True)
users = User.objects.filter(email__iexact=email.lower())
yield (self.query_for_users(users) | Q(data__email__iexact=email.lower()))
search_by_event.documentation = _(

View File

@ -23,7 +23,7 @@ def get_user_queryset():
qs = User.objects.all()
qs = qs.filter(deleted__isnull=True)
qs = qs.filter()
if app_settings.A2_USER_FILTER:
qs = qs.filter(**app_settings.A2_USER_FILTER)

View File

@ -46,8 +46,7 @@ class ModelBackend(ModelBackend):
try:
if app_settings.A2_ACCEPT_EMAIL_AUTHENTICATION \
and UserModel._meta.get_field('email'):
queries.append(models.Q(**{
'email__iexact': username, 'deleted__isnull': True}))
queries.append(models.Q(**{'email__iexact': username}))
except models.FieldDoesNotExist:
pass
@ -91,8 +90,6 @@ class ModelBackend(ModelBackend):
user = UserModel._default_manager.get(pk=user_id)
except UserModel.DoesNotExist:
return None
if user.deleted:
return None
return user
def get_saml2_authn_context(self):

View File

@ -653,12 +653,10 @@ class UserCsvImporter(object):
# lookup by field/attribute
key_value = row[header_key].value
if header_key.field:
users = User.objects.filter(
**{header_key.name: key_value}, deleted__isnull=True)
users = User.objects.filter(**{header_key.name: key_value})
elif header_key.attribute:
atvs = AttributeValue.objects.filter(attribute__name=header_key.name, content=key_value)
users = User.objects.filter(
attribute_values__in=atvs, deleted__isnull=True)
users = User.objects.filter(attribute_values__in=atvs)
users = users[:2]
if users:

View File

@ -118,8 +118,7 @@ class UserQuerySet(models.QuerySet):
name = '%s %s' % (first_name, last_name)
name = unicodedata.normalize('NFKD', name).encode('ascii', 'ignore').decode('ascii').lower()
qs = self.filter(deleted__isnull=True)
qs = qs.annotate(name=Lower(Unaccent(ImmutableConcat('first_name', Value(' '), 'last_name'))))
qs = self.annotate(name=Lower(Unaccent(ImmutableConcat('first_name', Value(' '), 'last_name'))))
qs = qs.filter(name__trigram_similar=name)
qs = qs.annotate(dist=TrigramDistance('name', name))
qs = qs.order_by('dist')

View File

@ -0,0 +1,54 @@
# Generated by Django 2.2.19 on 2021-02-26 10:02
from django.db import migrations
from authentic2 import app_settings
def delete_users(apps, schema_editor):
# to run schema and data altering statements in the same transaction, you
# must execute all constraints/triggers immediately, likely caused by
# on_cascade=models.DELETE
schema_editor.execute('SET CONSTRAINTS ALL IMMEDIATE')
User = apps.get_model('custom_user', 'User')
DeletedUser = apps.get_model('custom_user', 'DeletedUser')
def delete_user(self):
deleted_user = DeletedUser(
old_user_id=self.id)
if 'email' in app_settings.A2_USER_DELETED_KEEP_DATA:
deleted_user.old_email = self.email.rsplit('#', 1)[0]
if 'uuid' in app_settings.A2_USER_DELETED_KEEP_DATA:
deleted_user.old_uuid = self.uuid
# save LDAP account references
external_ids = self.userexternalid_set.order_by('id')
if external_ids.exists():
deleted_user.old_data = {'external_ids': []}
for external_id in external_ids:
deleted_user.old_data['external_ids'].append(
{
'source': external_id.source,
'external_id': external_id.external_id,
}
)
external_ids.delete()
deleted_user.save()
self.delete()
for user in User.objects.filter(deleted__isnull=False):
delete_user(user)
class Migration(migrations.Migration):
dependencies = [
('custom_user', '0025_user_deactivation'),
]
operations = [
migrations.RunPython(delete_users, migrations.RunPython.noop),
migrations.RemoveField(
model_name='user',
name='deleted',
),
]

View File

@ -189,10 +189,6 @@ class User(AbstractBaseUser, PermissionMixin):
verbose_name=_('Last account deletion alert'),
null=True,
blank=True)
deleted = models.DateTimeField(
verbose_name=_('Deletion date'),
null=True,
blank=True)
deactivation = models.DateTimeField(
verbose_name=_('Deactivation datetime'),
null=True,
@ -298,7 +294,7 @@ class User(AbstractBaseUser, PermissionMixin):
email_qs = qs.filter(ou=self.ou)
try:
try:
email_qs.get(email__iexact=self.email, deleted__isnull=True)
email_qs.get(email__iexact=self.email)
except MultipleObjectsReturned:
pass
except model.DoesNotExist:

View File

@ -48,7 +48,6 @@ class PasswordResetForm(forms.Form):
users = get_user_queryset()
active_users = users.filter(
Q(email__iexact=email) | Q(username__iexact=email),
deleted__isnull=True,
is_active=True)
for user in active_users:
# we don't set the password to a random string, as some users should not have

View File

@ -110,7 +110,7 @@ class RegistrationCompletionFormNoPassword(profile_forms.BaseUserForm):
if app_settings.A2_REGISTRATION_EMAIL_IS_UNIQUE:
exist = False
try:
User.objects.get(email__iexact=email, deleted__isnull=True)
User.objects.get(email__iexact=email)
except User.DoesNotExist:
pass
except User.MultipleObjectsReturned:

View File

@ -508,7 +508,7 @@ class Command(BaseCommand):
if not emails:
return
users = qs.annotate(iemail=Lower('email'))
count = users.filter(iemail__in=emails, deleted__isnull=True).count()
count = users.filter(iemail__in=emails).count()
self.warning('%s, found %%d user accounts with same email:' % msg, count)
for email in emails:
self.notice('- %s :', email)

View File

@ -79,7 +79,6 @@ class UsersView(HideOUColumnMixin, BaseTableView):
def get_queryset(self):
qs = super(UsersView, self).get_queryset()
qs = qs.filter(deleted__isnull=True)
qs = qs.select_related('ou')
qs = qs.prefetch_related('roles', 'roles__parent_relation__parent')
return qs
@ -277,11 +276,6 @@ class UserDetailView(OtherActionsMixin, BaseDetailView):
template_name = 'authentic2/manager/user_detail.html'
slug_field = 'uuid'
def get_queryset(self):
qs = super(UserDetailView, self).get_queryset()
qs = qs.filter(deleted__isnull=True)
return qs
@property
def title(self):
return self.object.get_full_name()

View File

@ -741,7 +741,7 @@ def send_registration_mail(request, email, ou, template_names=None, next_url=Non
**kwargs)
# existing accounts
existing_accounts = User.objects.filter(email=email, deleted__isnull=True)
existing_accounts = User.objects.filter(email=email)
if not app_settings.A2_EMAIL_IS_UNIQUE:
existing_accounts = existing_accounts.filter(ou=ou, email=email)

View File

@ -209,12 +209,9 @@ class EmailChangeVerifyView(TemplateView):
user = User.objects.get(pk=user_pk)
non_unique = False
if app_settings.A2_EMAIL_IS_UNIQUE:
non_unique = User.objects.filter(
email=email, deleted__isnull=True).exclude(pk=user_pk).exists()
non_unique = User.objects.filter(email=email).exclude(pk=user_pk).exists()
elif user.ou and user.ou.email_is_unique:
non_unique = User.objects.filter(
email=email, ou=user.ou, deleted__isnull=True).exclude(
pk=user_pk).exists()
non_unique = User.objects.filter(email=email, ou=user.ou).exclude(pk=user_pk).exists()
if non_unique:
raise ValidationError(_('This email is already used by another account.'))
old_email = user.email
@ -950,8 +947,7 @@ class RegistrationCompletionView(CreateView):
self.ou = OU.objects.get(pk=self.token['ou'])
else:
self.ou = get_default_ou()
self.users = User.objects.filter(email__iexact=self.email, deleted__isnull=True) \
.order_by('date_joined')
self.users = User.objects.filter(email__iexact=self.email).order_by('date_joined')
if self.ou:
self.users = self.users.filter(ou=self.ou)
self.email_is_unique = app_settings.A2_EMAIL_IS_UNIQUE \
@ -1231,9 +1227,6 @@ class ValidateDeletionView(TemplateView):
user_pk = deletion_token['user_pk']
self.user = get_user_model().objects.get(pk=user_pk)
# A user account wont be deactived twice
if self.user.deleted:
raise ValidationError(
_('This account has previously been deleted.'))
if not self.user.is_active:
raise ValidationError(
_('This account is inactive, it cannot be deleted.'))
@ -1254,7 +1247,7 @@ class ValidateDeletionView(TemplateView):
return utils.redirect(request, 'auth_homepage')
def post(self, request, *args, **kwargs):
if 'cancel' not in request.POST and not self.user.deleted:
if 'cancel' not in request.POST:
utils.send_account_deletion_mail(self.request, self.user)
logger.info(u'deletion of account %s performed', self.user)
hooks.call_hooks('event', name='delete-account', user=self.user)

View File

@ -414,7 +414,7 @@ class LoginOrLinkView(PopupViewMixin, FcOAuthSessionViewMixin, View):
if not user and self.user_info.get('email') and email_is_unique:
email = self.user_info['email']
User = get_user_model()
qs = User.objects.filter(email__iexact=email, deleted__isnull=True)
qs = User.objects.filter(email__iexact=email)
if not a2_app_settings.A2_EMAIL_IS_UNIQUE and default_ou.email_is_unique:
qs = qs.filter(ou=default_ou)

View File

@ -91,7 +91,6 @@ class SerializerTests(TestCase):
'user_permissions': [],
'password': '',
'ou': None,
'deleted': None,
'deactivation': None,
}
},

View File

@ -417,7 +417,7 @@ def test_api_users_create(settings, app, api_user):
'last_name_verified', 'date_joined', 'last_login',
'username', 'password', 'email', 'is_active', 'title',
'title_verified', 'modified', 'email_verified',
'last_account_deletion_alert', 'deleted',
'last_account_deletion_alert',
'deactivation']) == set(resp.json.keys())
assert resp.json['first_name'] == payload['first_name']
assert resp.json['last_name'] == payload['last_name']
@ -485,7 +485,7 @@ def test_api_users_create(settings, app, api_user):
'last_name_verified', 'date_joined', 'last_login',
'username', 'password', 'email', 'is_active', 'title',
'title_verified', 'modified', 'email_verified',
'last_account_deletion_alert', 'deleted',
'last_account_deletion_alert',
'deactivation']) == set(resp.json.keys())
user = get_user_model().objects.get(pk=resp.json['id'])
assert AttributeValue.objects.with_owner(user).filter(verified=True).count() == 3

View File

@ -123,14 +123,12 @@ def test_clean_unused_account_user_logs_in(app, db, simple_user, mailoutbox, fre
# the day of deletion, nothing happens
freezer.move_to('2018-01-02')
simple_user.refresh_from_db()
assert not simple_user.deleted
assert len(mailoutbox) == 1
# when new alert delay is reached, user gets alerted again
freezer.move_to('2018-01-04')
call_command('clean-unused-accounts')
simple_user.refresh_from_db()
assert not simple_user.deleted
assert len(mailoutbox) == 2
@ -140,7 +138,6 @@ def test_clean_unused_account_disabled_by_default(db, simple_user, mailoutbox):
call_command('clean-unused-accounts')
simple_user.refresh_from_db()
assert not simple_user.deleted
assert len(mailoutbox) == 0
@ -155,13 +152,11 @@ def test_clean_unused_account_always_alert(db, simple_user, mailoutbox, freezer)
# even if account last login in past deletion delay, an alert is always sent first
call_command('clean-unused-accounts')
simple_user.refresh_from_db()
assert not simple_user.deleted
assert len(mailoutbox) == 1
# and calling again as no effect, since one day must pass before account is deleted
call_command('clean-unused-accounts')
simple_user.refresh_from_db()
assert not simple_user.deleted
assert len(mailoutbox) == 1

View File

@ -73,7 +73,6 @@ def test_user_delete(db):
assert deleted_user.old_user_id == user_id
assert deleted_user.old_uuid == user_uuid
assert deleted_user.old_email == 'foo@example.net'
assert User.objects.filter(email='foo@example.net', deleted__isnull=False).count() == 0
@pytest.fixture

View File

@ -18,26 +18,7 @@ import mock
from django.contrib.auth.models import AbstractUser
from django.db.utils import ProgrammingError
def test_migration_0019_add_user_deleted(transactional_db, migration):
old_apps = migration.before([
('authentic2', '0026_token'),
('custom_user', '0018_user_last_account_deletion_alert'),
])
User = old_apps.get_model('custom_user', 'User')
DeletedUser = old_apps.get_model('authentic2', 'DeletedUser')
user = User.objects.create()
DeletedUser.objects.create(user=user)
new_apps = migration.before([
('authentic2', '0027_remove_deleteduser'),
('custom_user', '0019_add_user_deleted'),
])
NewUser = new_apps.get_model('custom_user', 'User')
new_user = NewUser.objects.get(id=user.id)
assert new_user.deleted
from django.utils.timezone import now
def test_migration_0028_trigram_unaccent_index(transactional_db, migration):
@ -63,3 +44,20 @@ def test_migration_custom_user_0021_set_unusable_password(transactional_db, migr
User = new_apps.get_model('custom_user', 'User')
user = User.objects.get()
assert not AbstractUser.has_usable_password(user)
def test_migration_custom_user_0026_remove_user_deleted(transactional_db, migration):
old_apps = migration.before([('custom_user', '0025_user_deactivation')])
User = old_apps.get_model('custom_user', 'User')
DeletedUser = old_apps.get_model('custom_user', 'DeletedUser')
User.objects.create(deleted=now())
User.objects.create()
assert User.objects.count() == 2
assert DeletedUser.objects.count() == 0
new_apps = migration.apply([('custom_user', '0026_remove_user_deleted')])
User = new_apps.get_model('custom_user', 'User')
DeletedUser = new_apps.get_model('custom_user', 'DeletedUser')
assert User.objects.count() == 1
assert DeletedUser.objects.count() == 1