Merge branch 'fix_show_available_groups_for_user_only' into 'master'

Fix: Users can be assigned to groups depite not matching state restrictions

See merge request allianceauth/allianceauth!1402

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)
 
 

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
 
@@ -542,6 +543,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):

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()