From a68163caa3d773fb0e9a8657165439c8a9400c8d Mon Sep 17 00:00:00 2001
From: Erik Kalkoken <erik.kalkoken@gmail.com>
Date: Sat, 26 Feb 2022 05:19:45 +0000
Subject: [PATCH] Fix: Users can be assigned to groups depite not matching
 state restrictions

---
 allianceauth/authentication/admin.py          | 14 ++++++-
 .../authentication/tests/test_admin.py        | 42 ++++++++++++++++++-
 allianceauth/tests/auth_utils.py              |  2 +-
 3 files changed, 53 insertions(+), 5 deletions(-)

diff --git a/allianceauth/authentication/admin.py b/allianceauth/authentication/admin.py
index 7dc50768..16818d41 100644
--- a/allianceauth/authentication/admin.py
+++ b/allianceauth/authentication/admin.py
@@ -380,6 +380,7 @@ class UserAdmin(BaseUserAdmin):
         'username',
         'character_ownerships__character__character_name'
     )
+    readonly_fields = ('date_joined', 'last_login')
 
     def _characters(self, obj):
         character_ownerships = list(obj.character_ownerships.all())
@@ -425,10 +426,19 @@ class UserAdmin(BaseUserAdmin):
     def has_delete_permission(self, request, obj=None):
         return request.user.has_perm('auth.delete_user')
 
+    def get_object(self, *args , **kwargs):
+        obj = super().get_object(*args , **kwargs)
+        self.obj = obj  # storing current object for use in formfield_for_manytomany
+        return obj
+
     def formfield_for_manytomany(self, db_field, request, **kwargs):
-        """overriding this formfield to have sorted lists in the form"""
         if db_field.name == "groups":
-            kwargs["queryset"] = Group.objects.all().order_by(Lower('name'))
+            groups_qs = Group.objects.filter(authgroup__states__isnull=True)
+            obj_state = self.obj.profile.state
+            if obj_state:
+                matching_groups_qs = Group.objects.filter(authgroup__states=obj_state)
+                groups_qs = groups_qs | matching_groups_qs
+            kwargs["queryset"] = groups_qs.order_by(Lower('name'))
         return super().formfield_for_manytomany(db_field, request, **kwargs)
 
 
diff --git a/allianceauth/authentication/tests/test_admin.py b/allianceauth/authentication/tests/test_admin.py
index e48f3623..e113d25b 100644
--- a/allianceauth/authentication/tests/test_admin.py
+++ b/allianceauth/authentication/tests/test_admin.py
@@ -1,3 +1,4 @@
+from bs4 import BeautifulSoup
 from urllib.parse import quote
 from unittest.mock import patch, MagicMock
 
@@ -188,7 +189,7 @@ class TestCaseWithTestData(TestCase):
             corporation_id=5432,
             corporation_name="Xavier's School for Gifted Youngsters",
             corporation_ticker='MUTNT',
-            alliance_id = None,
+            alliance_id=None,
             faction_id=999,
             faction_name='The X-Men',
         )
@@ -206,6 +207,7 @@ class TestCaseWithTestData(TestCase):
         cls.user_4.profile.save()
         EveFactionInfo.objects.create(faction_id=999, faction_name='The X-Men')
 
+
 def make_generic_search_request(ModelClass: type, search_term: str):
     User.objects.create_superuser(
         username='superuser', password='secret', email='admin@example.com'
@@ -344,7 +346,7 @@ class TestUserAdmin(TestCaseWithTestData):
         self.assertEqual(user_main_organization(self.user_3), expected)
 
     def test_user_main_organization_u4(self):
-        expected="Xavier's School for Gifted Youngsters<br>The X-Men"
+        expected = "Xavier's School for Gifted Youngsters<br>The X-Men"
         self.assertEqual(user_main_organization(self.user_4), expected)
 
     def test_characters_u1(self):
@@ -537,6 +539,42 @@ class TestUserAdmin(TestCaseWithTestData):
         self.assertEqual(response.status_code, expected)
 
 
+class TestUserAdminChangeForm(TestCase):
+    @classmethod
+    def setUpClass(cls) -> None:
+        super().setUpClass()
+        cls.modeladmin = UserAdmin(model=User, admin_site=AdminSite())
+
+    def test_should_show_groups_available_to_user_with_blue_state_only(self):
+        # given
+        superuser = User.objects.create_superuser("Super")
+        user = AuthUtils.create_user("Bruce Wayne")
+        character = AuthUtils.add_main_character_2(
+            user,
+            name="Bruce Wayne",
+            character_id=1001,
+            corp_id=2001,
+            corp_name="Wayne Technologies"
+        )
+        blue_state = State.objects.get(name="Blue")
+        blue_state.member_characters.add(character)
+        member_state = AuthUtils.get_member_state()
+        group_1 = Group.objects.create(name="Group 1")
+        group_2 = Group.objects.create(name="Group 2")
+        group_2.authgroup.states.add(blue_state)
+        group_3 = Group.objects.create(name="Group 3")
+        group_3.authgroup.states.add(member_state)
+        self.client.force_login(superuser)
+        # when
+        response = self.client.get(f"/admin/authentication/user/{user.pk}/change/")
+        # then
+        self.assertEqual(response.status_code, 200)
+        soup = BeautifulSoup(response.rendered_content, features="html.parser")
+        groups_select = soup.find("select", {"id": "id_groups"}).find_all('option')
+        group_ids = {int(option["value"]) for option in groups_select}
+        self.assertSetEqual(group_ids, {group_1.pk, group_2.pk})
+
+
 class TestMakeServicesHooksActions(TestCaseWithTestData):
 
     class MyServicesHookTypeA(ServicesHook):
diff --git a/allianceauth/tests/auth_utils.py b/allianceauth/tests/auth_utils.py
index 808f483c..1ee824ec 100644
--- a/allianceauth/tests/auth_utils.py
+++ b/allianceauth/tests/auth_utils.py
@@ -174,7 +174,7 @@ class AuthUtils:
         alliance_id=None,
         alliance_name='',
         disconnect_signals=False
-    ):
+    ) -> EveCharacter:
         """new version that works in all cases"""
         if disconnect_signals:
             cls.disconnect_signals()