diff --git a/allianceauth/groupmanagement/admin.py b/allianceauth/groupmanagement/admin.py index 44750aab..0f38bfe7 100644 --- a/allianceauth/groupmanagement/admin.py +++ b/allianceauth/groupmanagement/admin.py @@ -1,14 +1,18 @@ +from django import forms from django.apps import apps from django.contrib.auth.models import Permission from django.contrib import admin from django.contrib.auth.models import Group as BaseGroup, User +from django.core.exceptions import ValidationError from django.db.models import Count from django.db.models.functions import Lower from django.db.models.signals import pre_save, post_save, pre_delete, \ post_delete, m2m_changed from django.dispatch import receiver +from django.utils.timezone import now +from django.utils.translation import gettext_lazy as _ -from .models import AuthGroup +from .models import AuthGroup, ReservedGroupName from .models import GroupRequest if 'eve_autogroups' in apps.app_configs: @@ -70,8 +74,7 @@ if _has_auto_groups: managedalliancegroup__isnull=True, managedcorpgroup__isnull=True ) - else: - return queryset + return queryset class HasLeaderFilter(admin.SimpleListFilter): @@ -90,11 +93,22 @@ class HasLeaderFilter(admin.SimpleListFilter): return queryset.filter(authgroup__group_leaders__isnull=False) elif value == 'no': return queryset.filter(authgroup__group_leaders__isnull=True) - else: - return queryset + return queryset + + +class GroupAdminForm(forms.ModelForm): + def clean_name(self): + my_name = self.cleaned_data['name'] + if ReservedGroupName.objects.filter(name__iexact=my_name).exists(): + raise ValidationError( + _("This name has been reserved and can not be used for groups."), + code='reserved_name' + ) + return my_name class GroupAdmin(admin.ModelAdmin): + form = GroupAdminForm list_select_related = ('authgroup',) ordering = ('name',) list_display = ( @@ -209,6 +223,41 @@ class GroupRequestAdmin(admin.ModelAdmin): _leave_request.boolean = True +class ReservedGroupNameAdminForm(forms.ModelForm): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.fields['created_by'].initial = self.current_user.username + self.fields['created_at'].initial = _("(auto)") + + created_by = forms.CharField(disabled=True) + created_at = forms.CharField(disabled=True) + + def clean_name(self): + my_name = self.cleaned_data['name'].lower() + if Group.objects.filter(name__iexact=my_name).exists(): + raise ValidationError( + _("There already exists a group with that name."), code='already_exists' + ) + return my_name + + def clean_created_at(self): + return now() + + +@admin.register(ReservedGroupName) +class ReservedGroupNameAdmin(admin.ModelAdmin): + form = ReservedGroupNameAdminForm + list_display = ("name", "created_by", "created_at") + + def get_form(self, request, *args, **kwargs): + form = super().get_form(request, *args, **kwargs) + form.current_user = request.user + return form + + def has_change_permission(self, *args, **kwargs) -> bool: + return False + + @receiver(pre_save, sender=Group) def redirect_pre_save(sender, signal=None, *args, **kwargs): pre_save.send(BaseGroup, *args, **kwargs) diff --git a/allianceauth/groupmanagement/migrations/0018_reservedgroupname.py b/allianceauth/groupmanagement/migrations/0018_reservedgroupname.py new file mode 100644 index 00000000..0349b9e2 --- /dev/null +++ b/allianceauth/groupmanagement/migrations/0018_reservedgroupname.py @@ -0,0 +1,24 @@ +# Generated by Django 3.2.9 on 2021-11-25 18:38 + +from django.db import migrations, models +import django.utils.timezone + + +class Migration(migrations.Migration): + + dependencies = [ + ('groupmanagement', '0017_improve_groups_documentation'), + ] + + operations = [ + migrations.CreateModel( + name='ReservedGroupName', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('name', models.CharField(help_text='Name that can not be used for groups.', max_length=150, unique=True, verbose_name='name')), + ('reason', models.TextField(help_text='Reason why this name is reserved.', verbose_name='reason')), + ('created_by', models.CharField(help_text='Name of the user who created this entry.', max_length=255, verbose_name='created by')), + ('created_at', models.DateTimeField(default=django.utils.timezone.now, help_text='Date when this entry was created', verbose_name='created at')), + ], + ), + ] diff --git a/allianceauth/groupmanagement/models.py b/allianceauth/groupmanagement/models.py index c023c643..9befa8d0 100644 --- a/allianceauth/groupmanagement/models.py +++ b/allianceauth/groupmanagement/models.py @@ -4,8 +4,7 @@ from django.conf import settings from django.contrib.auth.models import Group from django.contrib.auth.models import User from django.db import models -from django.db.models.signals import post_save -from django.dispatch import receiver +from django.utils.timezone import now from django.utils.translation import gettext_lazy as _ from allianceauth.authentication.models import State @@ -181,18 +180,35 @@ class AuthGroup(models.Model): ) -@receiver(post_save, sender=Group) -def create_auth_group(sender, instance, created, **kwargs): - """ - Creates the AuthGroup model when a group is created - """ - if created: - AuthGroup.objects.create(group=instance) +class ReservedGroupName(models.Model): + """Name that can not be used for groups. + This enables AA to ignore groups on other services (e.g. Discord) with that name. + """ + name = models.CharField( + _('name'), + max_length=150, + unique=True, + help_text=_("Name that can not be used for groups.") + ) + reason = models.TextField( + _('reason'), help_text=_("Reason why this name is reserved.") + ) + created_by = models.CharField( + _('created by'), + max_length=255, + help_text="Name of the user who created this entry." + ) + created_at = models.DateTimeField( + _('created at'), default=now, help_text=_("Date when this entry was created") + ) -@receiver(post_save, sender=Group) -def save_auth_group(sender, instance, **kwargs): - """ - Ensures AuthGroup model is saved automatically - """ - instance.authgroup.save() + def __str__(self) -> str: + return self.name + + def save(self, *args, **kwargs) -> None: + if Group.objects.filter(name__iexact=self.name).exists(): + raise RuntimeError( + f"Save failed. There already exists a group with the name: {self.name}." + ) + super().save(*args, **kwargs) diff --git a/allianceauth/groupmanagement/signals.py b/allianceauth/groupmanagement/signals.py index 99af3d5d..60bbeb80 100644 --- a/allianceauth/groupmanagement/signals.py +++ b/allianceauth/groupmanagement/signals.py @@ -1,11 +1,33 @@ import logging +from django.contrib.auth.models import Group +from django.db.models.signals import pre_save, post_save from django.dispatch import receiver + from allianceauth.authentication.signals import state_changed +from .models import AuthGroup, ReservedGroupName logger = logging.getLogger(__name__) +@receiver(pre_save, sender=Group) +def find_new_name_for_conflicting_groups(sender, instance, **kwargs): + """Find new name for a group which name is already reserved.""" + new_name = instance.name + num = 0 + while ReservedGroupName.objects.filter(name__iexact=new_name).exists(): + num += 1 + new_name = f"{instance.name}_{num}" + instance.name = new_name + + +@receiver(post_save, sender=Group) +def create_auth_group(sender, instance, created, **kwargs): + """Create the AuthGroup model when a group is created.""" + if created: + AuthGroup.objects.create(group=instance) + + @receiver(state_changed) def check_groups_on_state_change(sender, user, state, **kwargs): logger.debug( diff --git a/allianceauth/groupmanagement/tests/test_admin.py b/allianceauth/groupmanagement/tests/test_admin.py index 5b32ba10..02c5a848 100644 --- a/allianceauth/groupmanagement/tests/test_admin.py +++ b/allianceauth/groupmanagement/tests/test_admin.py @@ -10,9 +10,10 @@ from allianceauth.authentication.models import CharacterOwnership, State from allianceauth.eveonline.models import ( EveCharacter, EveCorporationInfo, EveAllianceInfo ) - from ..admin import HasLeaderFilter, GroupAdmin, Group from . import get_admin_change_view_url +from ..models import ReservedGroupName + if 'allianceauth.eveonline.autogroups' in settings.INSTALLED_APPS: _has_auto_groups = True @@ -396,3 +397,108 @@ class TestGroupAdmin(TestCase): c.login(username='superuser', password='secret') response = c.get(get_admin_change_view_url(self.group_1)) self.assertEqual(response.status_code, 200) + + def test_should_create_new_group(self): + # given + user = User.objects.create_superuser("bruce") + self.client.force_login(user) + # when + response = self.client.post( + "/admin/groupmanagement/group/add/", + data={ + "name": "new group", + "authgroup-TOTAL_FORMS": 1, + "authgroup-INITIAL_FORMS": 0, + "authgroup-MIN_NUM_FORMS": 0, + "authgroup-MAX_NUM_FORMS": 1, + } + ) + # then + self.assertEqual(response.status_code, 302) + self.assertEqual(response.url, "/admin/groupmanagement/group/") + self.assertTrue(Group.objects.filter(name="new group").exists()) + + def test_should_not_allow_creating_new_group_with_reserved_name(self): + # given + ReservedGroupName.objects.create( + name="new group", reason="dummy", created_by="bruce" + ) + user = User.objects.create_superuser("bruce") + self.client.force_login(user) + # when + response = self.client.post( + "/admin/groupmanagement/group/add/", + data={ + "name": "New group", + "authgroup-TOTAL_FORMS": 1, + "authgroup-INITIAL_FORMS": 0, + "authgroup-MIN_NUM_FORMS": 0, + "authgroup-MAX_NUM_FORMS": 1, + } + ) + # then + self.assertContains( + response, "This name has been reserved and can not be used for groups" + ) + self.assertFalse(Group.objects.filter(name="new group").exists()) + + def test_should_not_allow_changing_name_of_existing_group_to_reserved_name(self): + # given + ReservedGroupName.objects.create( + name="new group", reason="dummy", created_by="bruce" + ) + group = Group.objects.create(name="dummy") + user = User.objects.create_superuser("bruce") + self.client.force_login(user) + # when + response = self.client.post( + f"/admin/groupmanagement/group/{group.pk}/change/", + data={ + "name": "new group", + "authgroup-TOTAL_FORMS": 1, + "authgroup-INITIAL_FORMS": 0, + "authgroup-MIN_NUM_FORMS": 0, + "authgroup-MAX_NUM_FORMS": 1, + } + ) + # then + self.assertContains( + response, "This name has been reserved and can not be used for groups" + ) + self.assertFalse(Group.objects.filter(name="new group").exists()) + + +class TestReservedGroupNameAdmin(TestCase): + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.user = User.objects.create_superuser("bruce") + + def test_should_create_new_entry(self): + # given + self.client.force_login(self.user) + # when + response = self.client.post( + "/admin/groupmanagement/reservedgroupname/add/", + data={"name": "Test", "reason": "dummy"} + ) + # then + self.assertEqual(response.status_code, 302) + self.assertEqual(response.url, "/admin/groupmanagement/reservedgroupname/") + obj = ReservedGroupName.objects.get(name="test") + self.assertEqual(obj.name, "test") + self.assertEqual(obj.created_by, self.user.username) + self.assertTrue(obj.created_at) + + def test_should_not_allow_names_of_existing_groups(self): + # given + Group.objects.create(name="Already taken") + self.client.force_login(self.user) + # when + response = self.client.post( + "/admin/groupmanagement/reservedgroupname/add/", + data={"name": "already taken", "reason": "dummy"} + ) + # then + self.assertContains(response, "There already exists a group with that name") + self.assertFalse(ReservedGroupName.objects.filter(name="already taken").exists()) diff --git a/allianceauth/groupmanagement/tests/test_models.py b/allianceauth/groupmanagement/tests/test_models.py index b8179550..dcf9a1e2 100644 --- a/allianceauth/groupmanagement/tests/test_models.py +++ b/allianceauth/groupmanagement/tests/test_models.py @@ -5,7 +5,7 @@ from django.test import TestCase, override_settings from allianceauth.tests.auth_utils import AuthUtils -from ..models import GroupRequest, RequestLog +from ..models import GroupRequest, RequestLog, ReservedGroupName MODULE_PATH = "allianceauth.groupmanagement.models" @@ -284,3 +284,22 @@ class TestAuthGroupRequestApprovers(TestCase): leaders = child_group.authgroup.group_request_approvers() # then self.assertSetEqual(leaders, set()) + + +class TestReservedGroupName(TestCase): + def test_should_return_name(self): + # given + obj = ReservedGroupName(name="test", reason="abc", created_by="xxx") + # when + result = str(obj) + # then + self.assertEqual(result, "test") + + def test_should_not_allow_creating_reserved_name_for_existing_group(self): + # given + Group.objects.create(name="Dummy") + # when + with self.assertRaises(RuntimeError): + ReservedGroupName.objects.create( + name="dummy", reason="abc", created_by="xxx" + ) diff --git a/allianceauth/groupmanagement/tests/test_signals.py b/allianceauth/groupmanagement/tests/test_signals.py index e9e47f36..efa35dda 100644 --- a/allianceauth/groupmanagement/tests/test_signals.py +++ b/allianceauth/groupmanagement/tests/test_signals.py @@ -6,6 +6,27 @@ from allianceauth.eveonline.autogroups.models import AutogroupsConfig from allianceauth.tests.auth_utils import AuthUtils +from ..models import ReservedGroupName + + +class TestGroupSignals(TestCase): + def test_should_create_authgroup_when_group_is_created(self): + # when + group = Group.objects.create(name="test") + # then + self.assertEqual(group.authgroup.group, group) + + def test_should_rename_group_that_conflicts_with_reserved_name(self): + # given + ReservedGroupName.objects.create(name="test", reason="dummy", created_by="xyz") + ReservedGroupName.objects.create(name="test_1", reason="dummy", created_by="xyz") + # when + group = Group.objects.create(name="Test") + # then + self.assertNotEqual(group.name, "test") + self.assertNotEqual(group.name, "test_1") + + class TestCheckGroupsOnStateChange(TestCase): @classmethod diff --git a/allianceauth/groupmanagement/tests/test_views.py b/allianceauth/groupmanagement/tests/test_views.py index b90775e1..0c723ec9 100644 --- a/allianceauth/groupmanagement/tests/test_views.py +++ b/allianceauth/groupmanagement/tests/test_views.py @@ -1,10 +1,7 @@ -from unittest.mock import Mock, patch - from django.test import RequestFactory, TestCase from django.urls import reverse from allianceauth.tests.auth_utils import AuthUtils -from esi.models import Token from .. import views diff --git a/allianceauth/services/modules/discord/discord_client/helpers.py b/allianceauth/services/modules/discord/discord_client/helpers.py index 859b3d01..50e7b7dc 100644 --- a/allianceauth/services/modules/discord/discord_client/helpers.py +++ b/allianceauth/services/modules/discord/discord_client/helpers.py @@ -1,4 +1,5 @@ from copy import copy +from typing import Set, Iterable class DiscordRoles: @@ -39,7 +40,7 @@ class DiscordRoles: def __len__(self): return len(self._roles.keys()) - def has_roles(self, role_ids: set) -> bool: + def has_roles(self, role_ids: Set[int]) -> bool: """returns true if this objects contains all roles defined by given role_ids incl. managed roles """ @@ -47,13 +48,22 @@ class DiscordRoles: all_role_ids = self._roles.keys() return role_ids.issubset(all_role_ids) - def ids(self) -> set: + def ids(self) -> Set[int]: """return a set of all role IDs""" return set(self._roles.keys()) - def subset(self, role_ids: set = None, managed_only: bool = False) -> object: - """returns a new object containing the subset of roles as defined - by given role IDs and/or including managed roles only + def subset( + self, + role_ids: Iterable[int] = None, + managed_only: bool = False, + role_names: Iterable[str] = None + ) -> "DiscordRoles": + """returns a new object containing the subset of roles + + Args: + - role_ids: role ids must be in the provided list + - managed_only: roles must be managed + - role_names: role names must match provided list (not case sensitive) """ if role_ids is not None: role_ids = {int(id) for id in role_ids} @@ -74,15 +84,21 @@ class DiscordRoles: if role_id in role_ids and role['managed'] ]) - else: - return copy(self) + elif role_ids is None and managed_only is False and role_names is not None: + role_names = {self.sanitize_role_name(name).lower() for name in role_names} + return type(self)([ + role for role in self._roles.values() + if role["name"].lower() in role_names + ]) - def union(self, other: object) -> object: + return copy(self) + + def union(self, other: object) -> "DiscordRoles": """returns a new roles object that is the union of this roles object with other""" return type(self)(list(self) + list(other)) - def difference(self, other: object) -> object: + def difference(self, other: object) -> "DiscordRoles": """returns a new roles object that only contains the roles that exist in the current objects, but not in other """ @@ -94,11 +110,10 @@ class DiscordRoles: role_name = self.sanitize_role_name(role_name) if role_name in self._roles_by_name: return self._roles_by_name[role_name] - else: - return dict() + return dict() @classmethod - def create_from_matched_roles(cls, matched_roles: list) -> None: + def create_from_matched_roles(cls, matched_roles: list) -> "DiscordRoles": """returns a new object created from the given list of matches roles matches_roles must be a list of tuples in the form: (role, created) @@ -107,7 +122,7 @@ class DiscordRoles: return cls(raw_roles) @staticmethod - def _assert_valid_role(role: dict): + def _assert_valid_role(role: dict) -> None: if not isinstance(role, dict): raise TypeError('Roles must be of type dict: %s' % role) diff --git a/allianceauth/services/modules/discord/discord_client/tests/__init__.py b/allianceauth/services/modules/discord/discord_client/tests/__init__.py index 7ba14d17..48be4f00 100644 --- a/allianceauth/services/modules/discord/discord_client/tests/__init__.py +++ b/allianceauth/services/modules/discord/discord_client/tests/__init__.py @@ -6,7 +6,7 @@ TEST_BOT_TOKEN = 'abcdefhijlkmnopqastzvwxyz1234567890ABCDEFGHOJKLMNOPQRSTUVWXY' TEST_ROLE_ID = 654321012345678912 -def create_role(id: int, name: str, managed=False): +def create_role(id: int, name: str, managed=False) -> dict: return { 'id': int(id), 'name': str(name), @@ -21,8 +21,10 @@ def create_matched_role(role, created=False) -> tuple: ROLE_ALPHA = create_role(1, 'alpha') ROLE_BRAVO = create_role(2, 'bravo') ROLE_CHARLIE = create_role(3, 'charlie') +ROLE_CHARLIE_2 = create_role(4, 'Charlie') # Discord roles are case sensitive ROLE_MIKE = create_role(13, 'mike', True) + ALL_ROLES = [ROLE_ALPHA, ROLE_BRAVO, ROLE_CHARLIE, ROLE_MIKE] diff --git a/allianceauth/services/modules/discord/discord_client/tests/test_helpers.py b/allianceauth/services/modules/discord/discord_client/tests/test_helpers.py index e19140cf..6a65cca2 100644 --- a/allianceauth/services/modules/discord/discord_client/tests/test_helpers.py +++ b/allianceauth/services/modules/discord/discord_client/tests/test_helpers.py @@ -1,6 +1,14 @@ from unittest import TestCase -from . import ROLE_ALPHA, ROLE_BRAVO, ROLE_CHARLIE, ROLE_MIKE, ALL_ROLES, create_role +from . import ( + ROLE_ALPHA, + ROLE_BRAVO, + ROLE_CHARLIE, + ROLE_CHARLIE_2, + ROLE_MIKE, + ALL_ROLES, + create_role +) from .. import DiscordRoles @@ -143,6 +151,16 @@ class TestSubset(TestCase): expected = {1, 2, 3, 13} self.assertSetEqual(roles.ids(), expected) + def test_should_return_role_names_only(self): + # given + all_roles = DiscordRoles([ + ROLE_ALPHA, ROLE_BRAVO, ROLE_CHARLIE, ROLE_MIKE, ROLE_CHARLIE_2 + ]) + # when + roles = all_roles.subset(role_names={"bravo", "charlie"}) + # then + self.assertSetEqual(roles.ids(), {2, 3, 4}) + class TestHasRoles(TestCase): diff --git a/allianceauth/services/modules/discord/models.py b/allianceauth/services/modules/discord/models.py index 321cd183..33389845 100644 --- a/allianceauth/services/modules/discord/models.py +++ b/allianceauth/services/modules/discord/models.py @@ -6,11 +6,12 @@ from django.contrib.auth.models import User from django.db import models from django.utils.translation import gettext_lazy +from allianceauth.groupmanagement.models import ReservedGroupName from allianceauth.notifications import notify from . import __title__ from .app_settings import DISCORD_GUILD_ID -from .discord_client import DiscordApiBackoff, DiscordRoles +from .discord_client import DiscordApiBackoff, DiscordClient, DiscordRoles from .discord_client.helpers import match_or_create_roles_from_names from .managers import DiscordUserManager from .utils import LoggerAddTag @@ -109,11 +110,16 @@ class DiscordUser(models.Model): - False on error or raises exception """ client = DiscordUser.objects._bot_client() + member_roles = self._determine_member_roles(client) + if member_roles is None: + return None + return self._update_roles_if_needed(client, state_name, member_roles) + + def _determine_member_roles(self, client: DiscordClient) -> DiscordRoles: + """Determine the roles of the current member / user.""" member_info = client.guild_member(guild_id=DISCORD_GUILD_ID, user_id=self.uid) if member_info is None: - # User is no longer a member - return None - + return None # User is no longer a member guild_roles = DiscordRoles(client.guild_roles(guild_id=DISCORD_GUILD_ID)) logger.debug('Current guild roles: %s', guild_roles.ids()) if 'roles' in member_info: @@ -128,10 +134,13 @@ class DiscordUser(models.Model): set(member_info['roles']).difference(guild_roles.ids()) ) ) - member_roles = guild_roles.subset(member_info['roles']) - else: - raise RuntimeError('member_info from %s is not valid' % self.user) + return guild_roles.subset(member_info['roles']) + raise RuntimeError('member_info from %s is not valid' % self.user) + def _update_roles_if_needed( + self, client: DiscordClient, state_name: str, member_roles: DiscordRoles + ) -> bool: + """Update the roles of this member/user if needed.""" requested_roles = match_or_create_roles_from_names( client=client, guild_id=DISCORD_GUILD_ID, @@ -143,10 +152,13 @@ class DiscordUser(models.Model): 'Requested roles for user %s: %s', self.user, requested_roles.ids() ) logger.debug('Current roles user %s: %s', self.user, member_roles.ids()) + reserved_role_names = ReservedGroupName.objects.values_list("name", flat=True) + member_roles_reserved = member_roles.subset(role_names=reserved_role_names) member_roles_managed = member_roles.subset(managed_only=True) - if requested_roles != member_roles.difference(member_roles_managed): + member_roles_persistent = member_roles_managed.union(member_roles_reserved) + if requested_roles != member_roles.difference(member_roles_persistent): logger.debug('Need to update roles for user %s', self.user) - new_roles = requested_roles.union(member_roles_managed) + new_roles = requested_roles.union(member_roles_persistent) success = client.modify_guild_member( guild_id=DISCORD_GUILD_ID, user_id=self.uid, @@ -157,10 +169,8 @@ class DiscordUser(models.Model): else: logger.warning('Failed to update roles for %s', self.user) return success - - else: - logger.info('No need to update roles for user %s', self.user) - return True + logger.info('No need to update roles for user %s', self.user) + return True def update_username(self) -> bool: """Updates the username incl. the discriminator @@ -171,7 +181,6 @@ class DiscordUser(models.Model): - None if user is no longer a member of the Discord server - False on error or raises exception """ - client = DiscordUser.objects._bot_client() user_info = client.guild_member(guild_id=DISCORD_GUILD_ID, user_id=self.uid) if user_info is None: diff --git a/allianceauth/services/modules/discord/tests/__init__.py b/allianceauth/services/modules/discord/tests/__init__.py index aae30b3f..ab6023db 100644 --- a/allianceauth/services/modules/discord/tests/__init__.py +++ b/allianceauth/services/modules/discord/tests/__init__.py @@ -9,6 +9,7 @@ from ..discord_client.tests import ( # noqa ROLE_ALPHA, ROLE_BRAVO, ROLE_CHARLIE, + ROLE_CHARLIE_2, ROLE_MIKE, ALL_ROLES, create_user_info diff --git a/allianceauth/services/modules/discord/tests/test_models.py b/allianceauth/services/modules/discord/tests/test_models.py index 332b8cb3..bf23f573 100644 --- a/allianceauth/services/modules/discord/tests/test_models.py +++ b/allianceauth/services/modules/discord/tests/test_models.py @@ -5,6 +5,7 @@ from requests.exceptions import HTTPError from django.test import TestCase from allianceauth.tests.auth_utils import AuthUtils +from allianceauth.groupmanagement.models import ReservedGroupName from . import ( TEST_USER_NAME, @@ -15,7 +16,8 @@ from . import ( ROLE_ALPHA, ROLE_BRAVO, ROLE_CHARLIE, - ROLE_MIKE + ROLE_CHARLIE_2, + ROLE_MIKE, ) from ..discord_client import DiscordClient, DiscordApiBackoff from ..discord_client.tests import create_matched_role @@ -294,25 +296,33 @@ class TestUpdateGroups(TestCase): args, kwargs = mock_DiscordClient.return_value.modify_guild_member.call_args self.assertEqual(set(kwargs['role_ids']), {1, 2}) - def test_update_if_needed_and_preserve_managed_roles( + def test_should_update_and_preserve_managed_and_reserved_roles( self, mock_user_group_names, mock_DiscordClient ): - roles_current = [1, 13] + # given + roles_current = [1, 3, 4, 13] mock_user_group_names.return_value = [] mock_DiscordClient.return_value.match_or_create_roles_from_names\ .return_value = self.roles_requested - mock_DiscordClient.return_value.guild_roles.return_value = self.guild_roles - mock_DiscordClient.return_value.guild_member.return_value = \ - {'roles': roles_current} + mock_DiscordClient.return_value.guild_roles.return_value = [ + ROLE_ALPHA, ROLE_BRAVO, ROLE_CHARLIE, ROLE_MIKE, ROLE_CHARLIE_2 + ] + mock_DiscordClient.return_value.guild_member.return_value = { + 'roles': roles_current + } mock_DiscordClient.return_value.modify_guild_member.return_value = True - + ReservedGroupName.objects.create( + name="charlie", reason="dummy", created_by="xyz" + ) + # when result = self.discord_user.update_groups() + # then self.assertTrue(result) self.assertTrue(mock_DiscordClient.return_value.modify_guild_member.called) args, kwargs = mock_DiscordClient.return_value.modify_guild_member.call_args - self.assertEqual(set(kwargs['role_ids']), {1, 2, 13}) + self.assertEqual(set(kwargs['role_ids']), {1, 2, 3, 4, 13}) def test_dont_update_if_not_needed( self, diff --git a/allianceauth_model.png b/allianceauth_model.png index 09b93574..e3fa337f 100644 Binary files a/allianceauth_model.png and b/allianceauth_model.png differ diff --git a/docs/features/core/groupmanagement.md b/docs/features/core/groupmanagement.md deleted file mode 100644 index 54e58054..00000000 --- a/docs/features/core/groupmanagement.md +++ /dev/null @@ -1,73 +0,0 @@ -# Group Management - -In order to access group management, users need to be either a superuser, granted the `auth | user | group_management ( Access to add members to groups within the alliance )` permission or a group leader (discussed later). - -## Group Requests - -When a user joins or leaves a group which is not marked as "Open", their group request will have to be approved manually by a user with the `group_management` permission or by a group leader of the group they are requesting. - -## Group Membership - -The group membership tab gives an overview of all of the non-internal groups. - -![Group overview](/_static/images/features/core/groupmanagement/group-membership.png) - -### Group Member Management - -Clicking on the blue eye will take you to the group member management screen. Here you can see a list of people who are in the group, and remove members where necessary. - -![Group overview](/_static/images/features/core/groupmanagement/group-member-management.png) - -### Group Audit Log - -Whenever a user Joins, Leaves, or is Removed from a group, this is logged. To find the audit log for a given group, click the light-blue button to the right of the Group Member Management (blue eye) button. - -These logs contain the Date and Time the action was taken (in EVE/UTC), the user which submitted the request being acted upon (requestor), the user's main character, the type of request (join, leave or removed), the action taken (accept, reject or remove), and the user that took the action (actor). - -![Audit Log Example](/_static/images/features/core/groupmanagement/group_audit_log.png) - -## Group Leaders - -Group leaders have the same abilities as users with the `group_management` permission, _however_, they will only be able to: - -- Approve requests for groups they are a leader of. -- View the Group Membership and Group Members of groups they are leaders of. - -This allows you to more finely control who has access to manage which groups. - -## Settings - -Here is a list of available settings for Group Management. They can be configured by adding them to your AA settings file (``local.py``). -Note that all settings are optional and the app will use the documented default settings if they are not used. - -```eval_rst -+---------------------------------------------+-------------------------------------------------------------------------------------------+------------+ -| Name | Description | Default | -+=============================================+===========================================================================================+============+ -| ``GROUPMANAGEMENT_AUTO_LEAVE`` | Group members can leave their group without needing confirmation from their group leaders | ``False`` | -+---------------------------------------------+-------------------------------------------------------------------------------------------+------------+ -| ``GROUPMANAGEMENT_REQUESTS_NOTIFICATION`` | Send Auth notifications to all group leaders for join and leave requests. | ``False`` | -+---------------------------------------------+-------------------------------------------------------------------------------------------+------------+ -``` - -## Permissions - -Group Management should be mostly done using group leaders, a series of permissions are included below for thoroughness. - -```eval_rst -+--------------------------------+-------------------+------------------------------------------------------------------------------------+ -| Permission | Admin Site | Auth Site | -+================================+===================+====================================================================================+ -| auth.group_management | None | Can Approve and Deny all Group Requests, Can view and manage all group memberships | -+--------------------------------+-------------------+------------------------------------------------------------------------------------+ -| groupmanagement.request_groups | None | Can Request Non-Public Groups | -+--------------------------------+-------------------+------------------------------------------------------------------------------------+ -| groupmanagement.add_group | Can Add Models | None | -+--------------------------------+-------------------+------------------------------------------------------------------------------------+ -| groupmanagement.change_group | Can Edit Models | None | -+--------------------------------+-------------------+------------------------------------------------------------------------------------+ -| groupmanagement.delete_group | Can Delete Models | None | -+--------------------------------+-------------------+------------------------------------------------------------------------------------+ -| groupmanagement.view_group | Can View Models | None | -+--------------------------------+-------------------+------------------------------------------------------------------------------------+ -``` diff --git a/docs/features/core/groups.md b/docs/features/core/groups.md index 71fe6469..25fd16c2 100644 --- a/docs/features/core/groups.md +++ b/docs/features/core/groups.md @@ -1,8 +1,8 @@ # Groups -Group Management is one of the core tasks of Alliance Auth. Many of Alliance Auth's services allow for synchronising of group membership, allowing you to grant permissions or roles in services to access certain aspects of them. +Group Management is one of the core tasks of Alliance Auth. Many of Alliance Auth's services allow for synchronizing of group membership, allowing you to grant permissions or roles in services to access certain aspects of them. -## User Organized Groups +## Creating groups Administrators can create custom groups for users to join. Examples might be groups like `Leadership`, `CEO` or `Scouts`. @@ -38,7 +38,81 @@ The key difference is that the group is completely unmanaged by Auth. **Once a m Most people won't have a use for public groups, though it can be useful if you wish to allow public access to some services. You can grant service permissions on a public group to allow this behavior. -## Permission +```eval_rst +.. _ref-reserved-group-names: +``` + +## Reserved group names + +When using Alliance Auth to manage external services like Discord, Auth will automatically duplicate groups on those services. E.g. on Discord Auth will create roles of the same name as groups. However, there may be cases where you want to manage groups on external services by yourself or by another bot. For those cases you can define a list of reserved group names. Auth will ensure that you can not create groups with a reserved name. You will find this list on the admin site under groupmanagement. + +```eval_rst +.. note:: + While this feature can help to avoid naming conflicts with groups on external services, the respective service component in Alliance Auth also needs to be build in such a way that it knows how to prevent these conflicts. Currently only the Discord service has this ability. +``` + +## Managing groups + +In order to access group management, users need to be either a superuser, granted the `auth | user | group_management ( Access to add members to groups within the alliance )` permission or a group leader (discussed later). + +### Group Requests + +When a user joins or leaves a group which is not marked as "Open", their group request will have to be approved manually by a user with the `group_management` permission or by a group leader of the group they are requesting. + +### Group Membership + +The group membership tab gives an overview of all of the non-internal groups. + +![Group overview](/_static/images/features/core/groupmanagement/group-membership.png) + +#### Group Member Management + +Clicking on the blue eye will take you to the group member management screen. Here you can see a list of people who are in the group, and remove members where necessary. + +![Group overview](/_static/images/features/core/groupmanagement/group-member-management.png) + +#### Group Audit Log + +Whenever a user Joins, Leaves, or is Removed from a group, this is logged. To find the audit log for a given group, click the light-blue button to the right of the Group Member Management (blue eye) button. + +These logs contain the Date and Time the action was taken (in EVE/UTC), the user which submitted the request being acted upon (requestor), the user's main character, the type of request (join, leave or removed), the action taken (accept, reject or remove), and the user that took the action (actor). + +![Audit Log Example](/_static/images/features/core/groupmanagement/group_audit_log.png) + +### Group Leaders + +Group leaders have the same abilities as users with the `group_management` permission, _however_, they will only be able to: + +- Approve requests for groups they are a leader of. +- View the Group Membership and Group Members of groups they are leaders of. + +This allows you to more finely control who has access to manage which groups. + +### Auto Leave + +By default in AA, Both requests and leaves for non-open groups must be approved by a group manager. If you wish to allow users to leave groups without requiring approvals, add the following lines to your `local.py` + +```python +## Allows users to freely leave groups without requiring approval. +AUTO_LEAVE = True +``` + +## Settings + +Here is a list of available settings for Group Management. They can be configured by adding them to your AA settings file (``local.py``). +Note that all settings are optional and the app will use the documented default settings if they are not used. + +```eval_rst ++---------------------------------------------+---------------------------------------------------------------------------+------------+ +| Name | Description | Default | ++=============================================+===========================================================================+============+ +| ``GROUPMANAGEMENT_REQUESTS_NOTIFICATION`` | Send Auth notifications to all group leaders for join and leave requests. | ``False`` | ++---------------------------------------------+---------------------------------------------------------------------------+------------+ +| ``GROUPMANAGEMENT_AUTO_LEAVE`` | Allows users to freely leave groups without requiring approval.. | ``False`` | ++---------------------------------------------+---------------------------------------------------------------------------+------------+ +``` + +## Permissions In order to join a group other than a public group, the permission `groupmanagement.request_groups` (`Can request non-public groups` in the admin panel) must be active on their account, either via a group or directly applied to their User account. @@ -48,3 +122,15 @@ When a user loses this permission, they will be removed from all groups _except_ .. note:: By default, the ``groupmanagement.request_groups`` permission is applied to the ``Member`` group. In most instances this, and perhaps adding it to the ``Blue`` group, should be all that is ever needed. It is unsupported and NOT advisable to apply this permission to a public group. See #697 for more information. ``` + +Group Management should be mostly done using group leaders, a series of permissions are included below for thoroughness: + +```eval_rst ++--------------------------------+-------------------+------------------------------------------------------------------------------------+ +| Permission | Admin Site | Auth Site | ++================================+===================+====================================================================================+ +| auth.group_management | None | Can Approve and Deny all Group Requests, Can view and manage all group memberships | ++--------------------------------+-------------------+------------------------------------------------------------------------------------+ +| groupmanagement.request_groups | None | Can Request Non-Public Groups | ++--------------------------------+-------------------+------------------------------------------------------------------------------------+ +``` diff --git a/docs/features/core/index.md b/docs/features/core/index.md index ec2003c7..1c518e3a 100644 --- a/docs/features/core/index.md +++ b/docs/features/core/index.md @@ -9,7 +9,6 @@ Managing access to applications and services is one of the core functions of **A dashboard states groups - groupmanagement analytics notifications ``` diff --git a/docs/features/services/discord.md b/docs/features/services/discord.md index 3d47cc95..a62a1d23 100644 --- a/docs/features/services/discord.md +++ b/docs/features/services/discord.md @@ -92,6 +92,28 @@ If you want users to have their Discord nickname changed to their in-game charac Once users link their accounts you’ll notice Roles get populated on Discord. These are the equivalent to groups on every other service. The default permissions should be enough for members to use text and audio communications. Add more permissions to the roles as desired through the server management window. +By default Alliance Auth is taking over full control of role assignments on Discord. This means that users on Discord can in general only have roles that correlate to groups on Auth. However, there are two exceptions to this rule. + +### Internal Discord roles + +First, users will keep their so called "Discord managed roles". Those are internal roles created by Discord e.g. for Nitro. + +### Excluding roles from being managed by Auth + +Second, it is possible to exclude Discord roles from being managed by Auth at all. This can be useful if you have other bots on your Discord server that are using their own roles and which would otherwise conflict with Auth. This would also allow you to manage a role manually on Discord if you so chose. + +To exclude roles from being managed by Auth you only have to add them to the list of reserved group names in Group Management. + +```eval_rst +.. note:: + Role names on Discord are case sensitive, while reserved group names on Auth are not. Therefore reserved group names will cover all roles regardless of their case. For example if you have reserved the group name "alpha", then the Discord roles "alpha" and "Alpha" will both be persisted. +``` + +```eval_rst +.. seealso:: + For more information see :ref:`ref-reserved-group-names`. +``` + ## Tasks The Discord service contains a number of tasks that can be run to manually perform updates to all users. @@ -159,7 +181,7 @@ This indicates your callback URL doesn't match. Ensure the `DISCORD_CALLBACK_URL ### "Add/Remove" Errors in Discord Service -If you are recieving errors in your Notifications after verifying that your settings are all correct try the following: +If you are receiving errors in your Notifications after verifying that your settings are all correct try the following: - Ensure that the bot's role in Discord is at the top of the roles list. Each time you add it to your server you will need to do this again. - Make sure that the bot is not trying to modify the Owner of the discord, as it will fail. A holding discord account added with invite link will mitigate this.