From 3ed0f873f37189101859c530b6e9903c6b48a7cc Mon Sep 17 00:00:00 2001 From: Adarnof Date: Thu, 22 Feb 2018 14:25:43 -0500 Subject: [PATCH] Capture signals sent by admin proxy models. This will prevent those weird missing UserProfile and AuthGroup errors. Add logging to authentication signals. Correct reverse migration authservicesinfo creation. Rename proxy models so they look better on the admin site. --- allianceauth/authentication/admin.py | 23 +++++----- .../migrations/0015_user_profiles.py | 6 +-- allianceauth/authentication/signals.py | 43 ++++++++++++++++--- allianceauth/groupmanagement/admin.py | 19 +++++--- .../migrations/0007_on_delete.py | 2 +- .../services/migrations/0002_nameformatter.py | 1 + allianceauth/services/signals.py | 6 ++- 7 files changed, 72 insertions(+), 28 deletions(-) diff --git a/allianceauth/authentication/admin.py b/allianceauth/authentication/admin.py index ab47f598..15afcca7 100644 --- a/allianceauth/authentication/admin.py +++ b/allianceauth/authentication/admin.py @@ -1,6 +1,6 @@ from django.contrib import admin from django.contrib.auth.admin import UserAdmin as BaseUserAdmin -from django.contrib.auth.models import User, Permission +from django.contrib.auth.models import User as BaseUser, Permission as BasePermission from django.utils.text import slugify from allianceauth.services.hooks import ServicesHook @@ -120,7 +120,7 @@ class CharacterOwnershipAdmin(admin.ModelAdmin): class PermissionAdmin(admin.ModelAdmin): actions = None - readonly_fields = [field.name for field in Permission._meta.fields] + readonly_fields = [field.name for field in BasePermission._meta.fields] list_display = ('admin_name', 'name', 'codename', 'content_type') list_filter = ('content_type__app_label',) @@ -136,21 +136,22 @@ class PermissionAdmin(admin.ModelAdmin): # Hack to allow registration of django.contrib.auth models in our authentication app -class ProxyUser(User): +class User(BaseUser): class Meta: proxy = True - verbose_name = User._meta.verbose_name - verbose_name_plural = User._meta.verbose_name_plural + verbose_name = BaseUser._meta.verbose_name + verbose_name_plural = BaseUser._meta.verbose_name_plural -class ProxyPermission(Permission): +class Permission(BasePermission): class Meta: proxy = True - verbose_name = Permission._meta.verbose_name - verbose_name_plural = Permission._meta.verbose_name_plural + verbose_name = BasePermission._meta.verbose_name + verbose_name_plural = BasePermission._meta.verbose_name_plural + try: - admin.site.unregister(User) + admin.site.unregister(BaseUser) finally: - admin.site.register(ProxyUser, UserAdmin) - admin.site.register(ProxyPermission, PermissionAdmin) \ No newline at end of file + admin.site.register(User, UserAdmin) + admin.site.register(Permission, PermissionAdmin) \ No newline at end of file diff --git a/allianceauth/authentication/migrations/0015_user_profiles.py b/allianceauth/authentication/migrations/0015_user_profiles.py index 5f57fba6..96c2577b 100644 --- a/allianceauth/authentication/migrations/0015_user_profiles.py +++ b/allianceauth/authentication/migrations/0015_user_profiles.py @@ -145,7 +145,7 @@ def recreate_authservicesinfo(apps, schema_editor): User = apps.get_model('auth', 'User') # recreate all missing AuthServicesInfo models - AuthServicesInfo.objects.bulk_create([AuthServicesInfo(user=u.pk) for u in User.objects.all()]) + AuthServicesInfo.objects.bulk_create([AuthServicesInfo(user_id=u.pk) for u in User.objects.all()]) # repopulate main characters for profile in UserProfile.objects.exclude(main_character__isnull=True).select_related('user', 'main_character'): @@ -233,7 +233,7 @@ class Migration(migrations.Migration): ), migrations.RunPython(disable_passwords, migrations.RunPython.noop), migrations.CreateModel( - name='ProxyPermission', + name='Permission', fields=[ ], options={ @@ -247,7 +247,7 @@ class Migration(migrations.Migration): ], ), migrations.CreateModel( - name='ProxyUser', + name='User', fields=[ ], options={ diff --git a/allianceauth/authentication/signals.py b/allianceauth/authentication/signals.py index c2e16fb1..4193d2c3 100644 --- a/allianceauth/authentication/signals.py +++ b/allianceauth/authentication/signals.py @@ -6,12 +6,12 @@ from django.db.models import Q from django.db.models.signals import post_save, pre_delete, m2m_changed, pre_save from django.dispatch import receiver, Signal from esi.models import Token +from allianceauth.authentication.admin import User as AdminUser from allianceauth.eveonline.models import EveCharacter logger = logging.getLogger(__name__) - state_changed = Signal(providing_args=['user', 'state']) @@ -32,23 +32,27 @@ def trigger_state_check(state): @receiver(m2m_changed, sender=State.member_characters.through) def state_member_characters_changed(sender, instance, action, *args, **kwargs): if action.startswith('post_'): + logger.debug('State {} member characters changed. Re-evaluating membership.'.format(instance)) trigger_state_check(instance) @receiver(m2m_changed, sender=State.member_corporations.through) def state_member_corporations_changed(sender, instance, action, *args, **kwargs): if action.startswith('post_'): + logger.debug('State {} member corporations changed. Re-evaluating membership.'.format(instance)) trigger_state_check(instance) @receiver(m2m_changed, sender=State.member_alliances.through) def state_member_alliances_changed(sender, instance, action, *args, **kwargs): if action.startswith('post_'): + logger.debug('State {} member alliances changed. Re-evaluating membership.'.format(instance)) trigger_state_check(instance) @receiver(post_save, sender=State) def state_saved(sender, instance, *args, **kwargs): + logger.debug('State {} saved. Re-evaluating membership.'.format(instance)) trigger_state_check(instance) @@ -59,19 +63,26 @@ def reassess_on_profile_save(sender, instance, created, *args, **kwargs): if not created: update_fields = kwargs.pop('update_fields', []) or [] if 'state' not in update_fields: + logger.debug('Profile for {} saved without state change. Re-evaluating state.'.format(instance.user)) instance.assign_state() -@receiver(post_save, sender=User) def create_required_models(sender, instance, created, *args, **kwargs): # ensure all users have a model if created: + logger.debug('User {} created. Creating default UserProfile.'.format(instance)) UserProfile.objects.get_or_create(user=instance) +post_save.connect(create_required_models, sender=User) +post_save.connect(create_required_models, sender=AdminUser) + + @receiver(post_save, sender=Token) def record_character_ownership(sender, instance, created, *args, **kwargs): if created: + logger.debug('New token for {0} character {1} saved. Evaluating ownership.'.format(instance.user, + instance.character_name)) if instance.user: query = Q(owner_hash=instance.character_owner_hash) & Q(user=instance.user) else: @@ -80,10 +91,15 @@ def record_character_ownership(sender, instance, created, *args, **kwargs): CharacterOwnership.objects.filter(character__character_id=instance.character_id).exclude(query).delete() # create character if needed if EveCharacter.objects.filter(character_id=instance.character_id).exists() is False: + logger.debug('Token is for a new character. Creating model for {0} ({1})'.format(instance.character_name, + instance.character_id)) EveCharacter.objects.create_character(instance.character_id) char = EveCharacter.objects.get(character_id=instance.character_id) # check if we need to create ownership - if instance.user and not CharacterOwnership.objects.filter(character__character_id=instance.character_id).exists(): + if instance.user and not CharacterOwnership.objects.filter( + character__character_id=instance.character_id).exists(): + logger.debug("Character {0} is not yet owned. Assigning ownership to {1}".format(instance.character_name, + instance.user)) CharacterOwnership.objects.update_or_create(character=char, defaults={'owner_hash': instance.character_owner_hash, 'user': instance.user}) @@ -92,6 +108,8 @@ def record_character_ownership(sender, instance, created, *args, **kwargs): @receiver(pre_delete, sender=CharacterOwnership) def validate_main_character(sender, instance, *args, **kwargs): if instance.user.profile.main_character == instance.character: + logger.debug("Ownership of a main character {0} has been revoked. Resetting {1} main character.".format( + instance.character, instance.user)) # clear main character as user no longer owns them instance.user.profile.main_character = None instance.user.profile.save() @@ -100,30 +118,45 @@ def validate_main_character(sender, instance, *args, **kwargs): @receiver(pre_delete, sender=Token) def validate_main_character_token(sender, instance, *args, **kwargs): if UserProfile.objects.filter(main_character__character_id=instance.character_id).exists(): + logger.debug( + "Token for a main character {0} is being deleted. Ensuring there are valid tokens to refresh.".format( + instance.character_name)) profile = UserProfile.objects.get(main_character__character_id=instance.character_id) - if not Token.objects.filter(character_id=instance.character_id).filter(user=profile.user).exclude(pk=instance.pk).exists(): + if not Token.objects.filter(character_id=instance.character_id).filter(user=profile.user).exclude( + pk=instance.pk).require_valid().exists(): + logger.debug( + "No remaining tokens to validate {0} ownership of main character {1}. Resetting main character.".format( + profile.user, profile.main_character)) # clear main character as we can no longer verify ownership profile.main_character = None profile.save() -@receiver(pre_save, sender=User) def assign_state_on_active_change(sender, instance, *args, **kwargs): # set to guest state if inactive, assign proper state if reactivated if instance.pk: old_instance = User.objects.get(pk=instance.pk) if old_instance.is_active != instance.is_active: if instance.is_active: + logger.debug("User {0} has been activated. Assigning state.".format(instance)) instance.profile.assign_state() else: + logger.debug( + "User {0} has been deactivated. Revoking state and assigning to guest state.".format(instance)) instance.profile.state = get_guest_state() instance.profile.save(update_fields=['state']) +pre_save.connect(assign_state_on_active_change, sender=User) +pre_save.connect(assign_state_on_active_change, sender=AdminUser) + + @receiver(post_save, sender=EveCharacter) def check_state_on_character_update(sender, instance, *args, **kwargs): # if this is a main character updating, check that user's state try: + logger.debug("Character {0} has been saved. Assessing owner's state for changes.".format(instance)) instance.userprofile.assign_state() except UserProfile.DoesNotExist: + logger.debug("Character {0} is not a main character. No state assessment required.".format(instance)) pass diff --git a/allianceauth/groupmanagement/admin.py b/allianceauth/groupmanagement/admin.py index c7287386..3146293b 100644 --- a/allianceauth/groupmanagement/admin.py +++ b/allianceauth/groupmanagement/admin.py @@ -1,7 +1,8 @@ from django.contrib import admin -from django.contrib.auth.models import Group +from django.contrib.auth.models import Group as BaseGroup +from django.db.models.signals import post_save -from .models import AuthGroup +from .models import AuthGroup, save_auth_group, create_auth_group from .models import GroupRequest @@ -12,17 +13,21 @@ class AuthGroupAdmin(admin.ModelAdmin): filter_horizontal = ('group_leaders',) -class ProxyGroup(Group): +class Group(BaseGroup): class Meta: proxy = True - verbose_name = Group._meta.verbose_name - verbose_name_plural = Group._meta.verbose_name_plural + verbose_name = BaseGroup._meta.verbose_name + verbose_name_plural = BaseGroup._meta.verbose_name_plural try: - admin.site.unregister(Group) + admin.site.unregister(BaseGroup) finally: - admin.site.register(ProxyGroup) + admin.site.register(Group) admin.site.register(GroupRequest) admin.site.register(AuthGroup, AuthGroupAdmin) + + +post_save.connect(create_auth_group, sender=Group) +post_save.connect(save_auth_group, sender=Group) diff --git a/allianceauth/groupmanagement/migrations/0007_on_delete.py b/allianceauth/groupmanagement/migrations/0007_on_delete.py index 022e93bd..c1e4d789 100644 --- a/allianceauth/groupmanagement/migrations/0007_on_delete.py +++ b/allianceauth/groupmanagement/migrations/0007_on_delete.py @@ -15,7 +15,7 @@ class Migration(migrations.Migration): operations = [ migrations.CreateModel( - name='ProxyGroup', + name='Group', fields=[ ], options={ diff --git a/allianceauth/services/migrations/0002_nameformatter.py b/allianceauth/services/migrations/0002_nameformatter.py index 8fb0dd45..bd84c65c 100644 --- a/allianceauth/services/migrations/0002_nameformatter.py +++ b/allianceauth/services/migrations/0002_nameformatter.py @@ -11,6 +11,7 @@ class Migration(migrations.Migration): dependencies = [ ('services', '0001_squashed_0003_delete_groupcache'), + ('authentication', '0015_user_profiles'), ] operations = [ diff --git a/allianceauth/services/signals.py b/allianceauth/services/signals.py index 55478592..138e9c0d 100644 --- a/allianceauth/services/signals.py +++ b/allianceauth/services/signals.py @@ -8,6 +8,7 @@ from django.db.models.signals import pre_save from django.dispatch import receiver from .hooks import ServicesHook from .tasks import disable_user +from allianceauth.authentication.admin import User as AdminUser from allianceauth.authentication.models import State, UserProfile from allianceauth.authentication.signals import state_changed @@ -140,7 +141,6 @@ def pre_delete_user(sender, instance, *args, **kwargs): disable_user(instance) -@receiver(pre_save, sender=User) def pre_save_user(sender, instance, *args, **kwargs): logger.debug("Received pre_save from %s" % instance) # check if user is being marked active/inactive @@ -154,3 +154,7 @@ def pre_save_user(sender, instance, *args, **kwargs): disable_user(instance) except User.DoesNotExist: pass + + +pre_save.connect(pre_save_user, sender=User) +pre_save.connect(pre_save_user, sender=AdminUser)