From abac8d5e7e4f5804b552110c0bc9516a9c964f67 Mon Sep 17 00:00:00 2001 From: marcelkornblum Date: Thu, 12 Oct 2023 09:32:52 +0100 Subject: [PATCH 1/6] interim --- .../management/commands/clean_user_records.py | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 src/user/management/commands/clean_user_records.py diff --git a/src/user/management/commands/clean_user_records.py b/src/user/management/commands/clean_user_records.py new file mode 100644 index 000000000..77823b3a0 --- /dev/null +++ b/src/user/management/commands/clean_user_records.py @@ -0,0 +1,82 @@ +from typing import List, Tuple + +from django.core.management.base import BaseCommand +from django.db.models import Count + +from user.models import User +from peoplefinder.models import Person + + +class Command(BaseCommand): + help = "Clean up and archive User and Person records" + + def add_arguments(self, parser): + parser.add_argument("days", type=int) + + def handle(self, *args, **options): + days = options["days"] + self.deleted_users = 0 + self.deleted_profiles = 0 + self.archived = 0 + + self.delete_users_and_profiles() + self.archive_users(login_older_than=days) + + self.stdout.write( + self.style.SUCCESS( + "Job finished successfully\n" + f"{self.deleted_users} User records deleted\n" + f"{self.deleted_profiles} Person records deleted\n" + f"{self.archived} User and Person records with logins older than {days} days have been archived\n" + ) + ) + + def delete_users_and_profiles(self): + + self.created += 1 + + + def archive_users(self, login_older_than=90): + ... + + + + + + +all_users = User.objects.values_list('email', flat=True) +all_users_d = all_users.distinct() +all_users_np = all_users.filter(profile__isnull=True) +all_valid_users = User.objects.filter(profile__isnull=False) + +all_pf = Person.objects.all() +all_pf_nu = all_pf.filter(user__isnull=True) +inactive_profiles = all_valid_users.filter(profile__is_active=False) + +duplicate_emails = User.objects.values('email').annotate(email_count=Count('email')).filter(email_count__gt=1) + + +len(all_users) +14935 +len(all_users_d) +13540 +len(all_users_np) +3231 +len(all_valid_users) +11704 + +len(all_pf) +12128 +len(inactive_profiles) +1952 + +all_valid_users.filter(is_active=True).count() +11702 +all_valid_users.filter(profile__is_active=True).count() +9752 +all_valid_users.filter(email__in=list(duplicate_emails)) + + + +all_valid_users.filter(profile__is_active=True).order_by("last_login").values_list("last_login") + Date: Thu, 12 Oct 2023 17:06:56 +0100 Subject: [PATCH 2/6] first stab and stub for mgmt commands --- src/user/management/commands/archive_users.py | 83 +++++++++++++ .../management/commands/clean_user_records.py | 113 +++++++----------- 2 files changed, 129 insertions(+), 67 deletions(-) create mode 100644 src/user/management/commands/archive_users.py diff --git a/src/user/management/commands/archive_users.py b/src/user/management/commands/archive_users.py new file mode 100644 index 000000000..e526e47c5 --- /dev/null +++ b/src/user/management/commands/archive_users.py @@ -0,0 +1,83 @@ +from django.core.management.base import BaseCommand +from django.utils import timezone + +from user.models import User +from peoplefinder.models import Person + + +class Command(BaseCommand): + help = "Clean up and archive User and Person records" + + def add_arguments(self, parser): + parser.add_argument("days", type=int, default=90) + parser.add_argument("batch_size", type=int, default=25) + parser.add_argument("batch_start", type=int, default=0) + + def handle(self, *args, **options): + days = options["days"] + batch_size = options["batch_size"] + batch_start = options["batch_start"] + + self.sync_inactive_users_profiles() + + login_cutoff_date = timezone.now() - timedelta(days=days) + users_to_check = User.objects.filter(is_active=True).filter( + last_login__lt=login_cutoff_date + ) + number_of_users = users_to_check.count() + + if batch_size == 0: + batch = users_to_check + self.stdout.write( + self.style.NOTICE( + f"Checking all {number_of_users} User records with last_login older than {days} ago" + ) + ) + else: + batch = users_to_check[batch_start:batch_size] + self.stdout.write( + self.style.NOTICE( + f"Checking {batch_size} (from record {batch_start}) of {number_of_users} User records with last_login older than {days} ago" + ) + ) + + deactivated = 0 + ignored = 0 + for user in batch: + # check against SSO API endpoint to see if user is active there + # if sso record is inactive: + # user.is_active = False + # user.save() + # deactivated += 1 + # else: + # ignored += 1 + ... + + self.stdout.write( + self.style.SUCCESS( + "Job finished successfully\n" + f"{self.deactivated} User records marked as inactive\n" + f"{self.ignored} User records left as active\n" + ) + ) + + def sync_inactive_users_profiles(self): + mismatched_users = User.objects.filter(profile__is_active=False).filter( + is_active=True + ) + mismatched_users.update(active=False) + self.stdout.write( + self.style.SUCCESS( + f"Updated {mismatched_users.count()} active User records with inactive Person profiles to inactive" + ) + ) + + mismatched_profiles = Person.objects.filter(is_active=True).filter( + user__is_active=False + ) + mismatched_profiles.update(active=False) + self.stdout.write( + self.style.SUCCESS( + f"Updated {mismatched_profiles.count()} active Person records with inactive User records to inactive" + ) + ) diff --git a/src/user/management/commands/clean_user_records.py b/src/user/management/commands/clean_user_records.py index 77823b3a0..103311425 100644 --- a/src/user/management/commands/clean_user_records.py +++ b/src/user/management/commands/clean_user_records.py @@ -1,5 +1,3 @@ -from typing import List, Tuple - from django.core.management.base import BaseCommand from django.db.models import Count @@ -8,75 +6,56 @@ class Command(BaseCommand): - help = "Clean up and archive User and Person records" - - def add_arguments(self, parser): - parser.add_argument("days", type=int) + help = "Clean up User and Person records" def handle(self, *args, **options): - days = options["days"] - self.deleted_users = 0 - self.deleted_profiles = 0 - self.archived = 0 + users_without_profile = User.objects.filter(profile__isnull=True) + self.do_delete(users_without_profile, "User") - self.delete_users_and_profiles() - self.archive_users(login_older_than=days) + profiles_without_user = Person.objects.filter(user__isnull=True) + self.do_delete(profiles_without_user, "Person") + no_duplicate_emails = ( + User.objects.values("email") + .annotate(email_count=Count("email")) + .filter(email_count__gt=1) + .count() + ) + if no_duplicate_emails > 0: + self.stdout.write( + self.style.NOTICE( + f"Found {no_duplicate_emails} User records with duplicate emails" + ) + ) + + def do_delete(self, queryset, model_name): self.stdout.write( - self.style.SUCCESS( - "Job finished successfully\n" - f"{self.deleted_users} User records deleted\n" - f"{self.deleted_profiles} Person records deleted\n" - f"{self.archived} User and Person records with logins older than {days} days have been archived\n" + self.style.NOTICE( + f"Found {queryset.count()} {model_name} records with no associated Person profile" ) ) - - def delete_users_and_profiles(self): - - self.created += 1 - - - def archive_users(self, login_older_than=90): - ... - - - - - - -all_users = User.objects.values_list('email', flat=True) -all_users_d = all_users.distinct() -all_users_np = all_users.filter(profile__isnull=True) -all_valid_users = User.objects.filter(profile__isnull=False) - -all_pf = Person.objects.all() -all_pf_nu = all_pf.filter(user__isnull=True) -inactive_profiles = all_valid_users.filter(profile__is_active=False) - -duplicate_emails = User.objects.values('email').annotate(email_count=Count('email')).filter(email_count__gt=1) - - -len(all_users) -14935 -len(all_users_d) -13540 -len(all_users_np) -3231 -len(all_valid_users) -11704 - -len(all_pf) -12128 -len(inactive_profiles) -1952 - -all_valid_users.filter(is_active=True).count() -11702 -all_valid_users.filter(profile__is_active=True).count() -9752 -all_valid_users.filter(email__in=list(duplicate_emails)) - - - -all_valid_users.filter(profile__is_active=True).order_by("last_login").values_list("last_login") - Date: Thu, 19 Oct 2023 12:14:36 +0100 Subject: [PATCH 3/6] connect to speculatively useful sso endpoint --- src/user/management/commands/archive_users.py | 52 +++++++++++++------ 1 file changed, 35 insertions(+), 17 deletions(-) diff --git a/src/user/management/commands/archive_users.py b/src/user/management/commands/archive_users.py index e526e47c5..5d8d3bbad 100644 --- a/src/user/management/commands/archive_users.py +++ b/src/user/management/commands/archive_users.py @@ -1,3 +1,8 @@ +from datetime import timedelta +import requests +from urllib.parse import urlparse, urlunparse + +from django.conf import settings from django.core.management.base import BaseCommand from django.utils import timezone @@ -10,13 +15,13 @@ class Command(BaseCommand): def add_arguments(self, parser): parser.add_argument("days", type=int, default=90) - parser.add_argument("batch_size", type=int, default=25) - parser.add_argument("batch_start", type=int, default=0) + parser.add_argument("limit", type=int, default=25) + parser.add_argument("offset", type=int, default=0) def handle(self, *args, **options): days = options["days"] - batch_size = options["batch_size"] - batch_start = options["batch_start"] + limit = options["limit"] + offset = options["offset"] self.sync_inactive_users_profiles() @@ -26,7 +31,7 @@ def handle(self, *args, **options): ) number_of_users = users_to_check.count() - if batch_size == 0: + if limit == 0: batch = users_to_check self.stdout.write( self.style.NOTICE( @@ -34,30 +39,43 @@ def handle(self, *args, **options): ) ) else: - batch = users_to_check[batch_start:batch_size] + batch = users_to_check[offset:limit] self.stdout.write( self.style.NOTICE( - f"Checking {batch_size} (from record {batch_start}) of {number_of_users} User records with last_login older than {days} ago" + f"Checking {limit} (from record {offset}) of {number_of_users} User records with last_login older than {days} ago" ) ) deactivated = 0 ignored = 0 + # check against SSO API endpoint to see if user is active there + authbroker_url = urlparse(settings.AUTHBROKER_URL) + url = urlunparse(authbroker_url._replace(path="/introspect/")) + headers = {"Authorization": f"bearer {settings.AUTHBROKER_INTROSPECTION_TOKEN}"} + for user in batch: - # check against SSO API endpoint to see if user is active there - # if sso record is inactive: - # user.is_active = False - # user.save() - # deactivated += 1 - # else: - # ignored += 1 - ... + params = {"email": user.email} + response = requests.get(url, params, headers=headers, timeout=5) + if response.status_code == 200: + resp_json = response.json() + if not resp_json["is_active"]: + user.is_active = False + user.save() + deactivated += 1 + else: + ignored += 1 + else: + self.stdout.write( + self.style.ERROR( + f"SSO introspect endpoint returned {response.status_code} status code for {user.email}" + ) + ) self.stdout.write( self.style.SUCCESS( "Job finished successfully\n" - f"{self.deactivated} User records marked as inactive\n" - f"{self.ignored} User records left as active\n" + f"{deactivated} User records marked as inactive\n" + f"{ignored} User records left as active\n" ) ) From 09a3030c67acb41736254dfdfe66083cb4e1c4f5 Mon Sep 17 00:00:00 2001 From: marcelkornblum Date: Thu, 19 Oct 2023 12:52:22 +0100 Subject: [PATCH 4/6] mark profiles and users inactive at the same time --- src/peoplefinder/services/person.py | 3 +++ src/peoplefinder/views/profile.py | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/peoplefinder/services/person.py b/src/peoplefinder/services/person.py index 8ffa15775..054629432 100644 --- a/src/peoplefinder/services/person.py +++ b/src/peoplefinder/services/person.py @@ -335,6 +335,9 @@ def profile_deleted( person.is_active = False person.became_inactive = timezone.now() person.save() + if person.user: + person.user.is_active = False + person.user.save() AuditLogService().log(AuditLog.Action.DELETE, deleted_by, person) diff --git a/src/peoplefinder/views/profile.py b/src/peoplefinder/views/profile.py index a523a3bf4..81f3cd07b 100644 --- a/src/peoplefinder/views/profile.py +++ b/src/peoplefinder/views/profile.py @@ -515,6 +515,9 @@ def action(self): self.person.is_active = True self.person.became_inactive = None self.person.save() + if self.person.user: + self.person.user.is_active = True + self.person.user.save() class ProfileUpdateUserView(SuccessMessageMixin, HtmxFormView): From 7166f8e7960500a2413b95b3cd66cc684c8723a0 Mon Sep 17 00:00:00 2001 From: marcelkornblum Date: Thu, 19 Oct 2023 15:19:35 +0100 Subject: [PATCH 5/6] connect user and profile is_active saving in both directions --- src/peoplefinder/admin.py | 6 +++++- src/peoplefinder/models.py | 4 ++++ src/user/models.py | 6 ++++++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/peoplefinder/admin.py b/src/peoplefinder/admin.py index c612a8a29..941a65da1 100644 --- a/src/peoplefinder/admin.py +++ b/src/peoplefinder/admin.py @@ -4,12 +4,13 @@ from peoplefinder.forms.admin import TeamModelForm from peoplefinder.models import LegacyAuditLog, NewNetwork, Person, Team, TeamMember from peoplefinder.services.team import TeamService +from user.models import User class PersonModelAdmin(admin.ModelAdmin): """Admin page for the Person model.""" - list_display = ["full_name", "email"] + list_display = ["full_name", "email", "is_active"] list_filter = ["is_active"] search_fields = [ "first_name", @@ -21,6 +22,9 @@ class PersonModelAdmin(admin.ModelAdmin): @admin.action(description="Mark selected people as active") def make_active(self, request, queryset): queryset.filter(is_active=False).update(is_active=True, became_inactive=None) + User.objects.filter(profile__pk__in=queryset.values("pk")).update( + is_active=True + ) class LegacyAuditLogModelAdmin(admin.ModelAdmin): diff --git a/src/peoplefinder/models.py b/src/peoplefinder/models.py index 09ac15f39..2df18d347 100644 --- a/src/peoplefinder/models.py +++ b/src/peoplefinder/models.py @@ -783,6 +783,10 @@ def save(self, *args, **kwargs): from peoplefinder.services.person import PersonService self.profile_completion = PersonService().get_profile_completion(person=self) + + if self.user and self.is_active != self.user.is_active: + self.user.is_active = self.is_active + self.user.save() return super().save(*args, **kwargs) def get_first_name_display(self) -> str: diff --git a/src/user/models.py b/src/user/models.py index 55ae8cf1e..0b721bbe0 100644 --- a/src/user/models.py +++ b/src/user/models.py @@ -24,5 +24,11 @@ class Meta: def __str__(self): return f"{self.first_name} {self.last_name}" + def save(self, *args, **kwargs): + if self.profile and self.is_active != self.profile.is_active: + self.profile.is_active = self.is_active + self.profile.save() + return super().save(*args, **kwargs) + register(User) From e173e08db1b9ea24dbb4b136d826d33281507dfd Mon Sep 17 00:00:00 2001 From: marcelkornblum Date: Thu, 19 Oct 2023 16:19:07 +0100 Subject: [PATCH 6/6] try now --- src/user/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/user/models.py b/src/user/models.py index 0b721bbe0..795c5452e 100644 --- a/src/user/models.py +++ b/src/user/models.py @@ -25,7 +25,7 @@ def __str__(self): return f"{self.first_name} {self.last_name}" def save(self, *args, **kwargs): - if self.profile and self.is_active != self.profile.is_active: + if hasattr(self, "profile") and self.is_active != self.profile.is_active: self.profile.is_active = self.is_active self.profile.save() return super().save(*args, **kwargs)