Publically joinable Groups (#697)

* Add public field to AuthGroup

* Add permission for users to join non-public groups

By default this permission will be applied to the "Member" group to
maintain the current behaviour.

* Allow users to join public groups

Users without the 'groupmanagement.request_groups' permission will be
able to join groups marked as public but will not be able to see or join
any other groups.

* Prevent None state change from purging groups

Currently when a user drops from Blue or Member state all groups and
permissions are discarded. This softens that approach by not removing
public groups and creates a distinction between the two activities. An
argument could maybe be made for not removing permissions on a state
change, but that is beyond the scope of this change.

* Correct syntax for removing filtered groups

* Add unit tests for disable user and member

* Update services signals tests

* Correct mocking call

* Remove permissions checking from menu item

authentication/tasks.py

@@ -20,7 +20,28 @@ def generate_alliance_group_name(alliancename):
 
 
 def disable_member(user):
+    """
+    Disable a member who is transitioning to a NONE state.
+    :param user: django.contrib.auth.models.User to disable
+    :return:
+    """
     logger.debug("Disabling member %s" % user)
+    if user.user_permissions.all().exists():
+        logger.info("Clearning user %s permission to deactivate user." % user)
+        user.user_permissions.clear()
+    if user.groups.all().exists():
+        logger.info("Clearing all non-public user %s groups to disable member." % user)
+        user.groups.remove(*user.groups.filter(authgroup__public=False))
+    validate_services(user, None)
+
+
+def disable_user(user):
+    """
+    Disable a user who is being set inactive or deleted
+    :param user: django.contrib.auth.models.User to disable
+    :return:
+    """
+    logger.debug("Disabling user %s" % user)
     if user.user_permissions.all().exists():
         logger.info("Clearning user %s permission to deactivate user." % user)
         user.user_permissions.clear()

authentication/tests.py

@@ -1 +0,0 @@
-# Create your tests here.

authentication/tests/test_tasks.py

@@ -0,0 +1,84 @@
+from __future__ import unicode_literals
+
+try:
+    # Py3
+    from unittest import mock
+except ImportError:
+    # Py2
+    import mock
+
+from django.test import TestCase
+from django.contrib.auth.models import Group, Permission
+
+from alliance_auth.tests.auth_utils import AuthUtils
+
+from authentication.tasks import disable_member, disable_user
+
+
+class AuthenticationTasksTestCase(TestCase):
+    def setUp(self):
+        self.member = AuthUtils.create_member('auth_member')
+        self.none_user = AuthUtils.create_user('none_user', disconnect_signals=True)
+
+    @mock.patch('services.signals.transaction')
+    def test_disable_member(self, transaction):
+        # Inert signals action
+        transaction.on_commit.side_effect = lambda fn: fn()
+
+        # Add permission
+        perm = Permission.objects.create(codename='test_perm', name='test perm', content_type_id=1)
+
+        # Add public group
+        pub_group = Group.objects.create(name="A Public group")
+        pub_group.authgroup.internal = False
+        pub_group.authgroup.public = True
+        pub_group.save()
+
+        # Setup member
+        self.member.user_permissions.add(perm)
+        self.member.groups.add(pub_group)
+
+        # Pre assertion
+        self.assertIn(pub_group, self.member.groups.all())
+        self.assertGreater(len(self.member.groups.all()), 1)
+
+        # Act
+        disable_member(self.member)
+
+        # Assert
+        self.assertIn(pub_group, self.member.groups.all())
+        # Everything but the single public group wiped
+        self.assertEqual(len(self.member.groups.all()), 1)
+        # All permissions wiped
+        self.assertEqual(len(self.member.user_permissions.all()), 0)
+
+    @mock.patch('services.signals.transaction')
+    def test_disable_user(self, transaction):
+        # Inert signals action
+        transaction.on_commit.side_effect = lambda fn: fn()
+
+        # Add permission
+        perm = Permission.objects.create(codename='test_perm', name='test perm', content_type_id=1)
+
+        # Add public group
+        pub_group = Group.objects.create(name="A Public group")
+        pub_group.authgroup.internal = False
+        pub_group.authgroup.public = True
+        pub_group.save()
+
+        # Setup member
+        self.member.user_permissions.add(perm)
+        self.member.groups.add(pub_group)
+
+        # Pre assertion
+        self.assertIn(pub_group, self.member.groups.all())
+        self.assertGreater(len(self.member.groups.all()), 1)
+
+        # Act
+        disable_user(self.member)
+
+        # Assert
+        # All groups wiped
+        self.assertEqual(len(self.member.groups.all()), 0)
+        # All permissions wiped
+        self.assertEqual(len(self.member.user_permissions.all()), 0)

groupmanagement/migrations/0005_authgroup_public.py

@@ -0,0 +1,20 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.10.5 on 2017-02-04 06:11
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('groupmanagement', '0004_authgroup'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='authgroup',
+            name='public',
+            field=models.BooleanField(default=False, help_text='Group is public. Any registered user is able to join this group, with visibility based on the other options set for this group.<br> Auth will not remove users from this group automatically when they are no longer authenticated.'),
+        ),
+    ]

groupmanagement/migrations/0006_request_groups_perm.py

@@ -0,0 +1,44 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.10.5 on 2017-02-04 07:17
+from __future__ import unicode_literals
+
+from django.db import migrations
+from django.conf import settings
+from django.core.exceptions import ObjectDoesNotExist
+from django.contrib.auth.management import create_permissions
+
+import logging
+
+logger = logging.getLogger(__name__)
+
+
+def add_default_member_permission(apps, schema_editor):
+    for app_config in apps.get_app_configs():
+        app_config.models_module = True
+        create_permissions(app_config, apps=apps, verbosity=0)
+        app_config.models_module = None
+
+    Group = apps.get_model("auth", "Group")
+    Permission = apps.get_model("auth", "Permission")
+
+    try:
+        perm = Permission.objects.get(codename='request_groups', name='Can request non-public groups')
+        group = Group.objects.get(name=getattr(settings, str('DEFAULT_AUTH_GROUP'), 'Member'))
+        group.permissions.add(perm)
+    except ObjectDoesNotExist:
+        logger.warning('Failed to add default request_groups permission to Member group')
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('groupmanagement', '0005_authgroup_public'),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name='authgroup',
+            options={'permissions': (('request_groups', 'Can request non-public groups'),)},
+        ),
+        migrations.RunPython(add_default_member_permission),
+    ]

groupmanagement/models.py

@@ -32,6 +32,10 @@ class AuthGroup(models.Model):
     Internal - not requestable by users, at all. Covers Corp_, Alliance_, Members etc groups.
     Groups are internal by default
 
+    Public - Other options are respected, but any user will be able to become and remain a member, even if they
+    have no API etc entered. Auth will not manage these groups automatically so user removal is up to
+    group managers/leaders.
+
     Not Internal and:
         Hidden - users cannot view, can request if they have the direct link.
         Not Hidden - Users can view and request the group
@@ -49,6 +53,11 @@ class AuthGroup(models.Model):
     open = models.BooleanField(default=False,
                                help_text="Group is open and users will be automatically added upon request. <br>"
                                          "If the group is not open users will need their request manually approved.")
+    public = models.BooleanField(default=False,
+                                 help_text="Group is public. Any registered user is able to join this group, with "
+                                           "visibility based on the other options set for this group.<br> Auth will "
+                                           "not remove users from this group automatically when they are no longer "
+                                           "authenticated.")
     # Group leaders have management access to this group
     group_leaders = models.ManyToManyField(User, related_name='leads_groups', blank=True,
                                            help_text="Group leaders can process group requests for this group "
@@ -60,6 +69,11 @@ class AuthGroup(models.Model):
     def __str__(self):
         return self.group.name
 
+    class Meta:
+        permissions = (
+            ("request_groups", u"Can request non-public groups"),
+        )
+
 
 @receiver(post_save, sender=Group)
 def create_auth_group(sender, instance, created, **kwargs):

groupmanagement/views.py

@@ -12,6 +12,7 @@ from django.http import Http404
 from groupmanagement.managers import GroupManager
 from groupmanagement.models import GroupRequest
 from authentication.models import AuthServicesInfo
+from authentication.managers import UserState
 from eveonline.managers import EveManager
 
 import logging
@@ -270,7 +271,14 @@ def groups_view(request):
     logger.debug("groups_view called by user %s" % request.user)
     groups = []
 
-    for group in GroupManager.get_joinable_groups():
+    group_query = GroupManager.get_joinable_groups()
+
+    if not request.user.has_perm('groupmanagement.request_groups'):
+        # Filter down to public groups only for non-members
+        group_query = group_query.filter(authgroup__public=True)
+        logger.debug("Not a member, only public groups will be available")
+
+    for group in group_query:
         # Exclude hidden
         if not group.authgroup.hidden:
             group_request = GroupRequest.objects.filter(user=request.user).filter(group=group)
@@ -290,6 +298,12 @@ def group_request_add(request, group_id):
                        (request.user, group_id))
         messages.warning(request, "You cannot join that group")
         return redirect('auth_groups')
+    if not request.user.has_perm('groupmanagement.request_groups') and not group.authgroup.public:
+        # Does not have the required permission, trying to join a non-public group
+        logger.warning("User %s attempted to join group id %s but it is not a public group" %
+                       (request.user, group_id))
+        messages.warning(request, "You cannot join that group")
+        return redirect('auth_groups')
     if group.authgroup.open:
         logger.info("%s joining %s as is an open group" % (request.user, group))
         request.user.groups.add(group)

services/signals.py

@@ -10,7 +10,7 @@ from django.db.models.signals import pre_save
 from django.dispatch import receiver
 
 from alliance_auth.hooks import get_hooks
-from authentication.tasks import disable_member
+from authentication.tasks import disable_user
 from authentication.tasks import set_state
 
 logger = logging.getLogger(__name__)
@@ -39,7 +39,7 @@ def m2m_changed_user_groups(sender, instance, action, *args, **kwargs):
 @receiver(pre_delete, sender=User)
 def pre_delete_user(sender, instance, *args, **kwargs):
     logger.debug("Received pre_delete from %s" % instance)
-    disable_member(instance)
+    disable_user(instance)
 
 
 @receiver(pre_save, sender=User)
@@ -53,7 +53,7 @@ def pre_save_user(sender, instance, *args, **kwargs):
         old_instance = User.objects.get(pk=instance.pk)
         if old_instance.is_active and not instance.is_active:
             logger.info("Disabling services for inactivation of user %s" % instance)
-            disable_member(instance)
+            disable_user(instance)
         elif instance.is_active and not old_instance.is_active:
             logger.info("Assessing state of reactivated user %s" % instance)
             set_state(instance)

services/tests/test_signals.py

@@ -47,27 +47,27 @@ class ServicesSignalsTestCase(TestCase):
         args, kwargs = svc.update_groups.call_args
         self.assertEqual(self.member, args[0])
 
-    @mock.patch('services.signals.disable_member')
+    @mock.patch('services.signals.disable_user')
-    def test_pre_delete_user(self, disable_member):
+    def test_pre_delete_user(self, disable_user):
         """
         Test that disable_member is called when a user is deleted
         """
         self.none_user.delete()
 
-        self.assertTrue(disable_member.called)
+        self.assertTrue(disable_user.called)
-        args, kwargs = disable_member.call_args
+        args, kwargs = disable_user.call_args
         self.assertEqual(self.none_user, args[0])
 
-    @mock.patch('services.signals.disable_member')
+    @mock.patch('services.signals.disable_user')
-    def test_pre_save_user_inactivation(self, disable_member):
+    def test_pre_save_user_inactivation(self, disable_user):
         """
         Test a user set inactive has disable_member called
         """
         self.member.is_active = False
         self.member.save()  # Signal Trigger
 
-        self.assertTrue(disable_member.called)
+        self.assertTrue(disable_user.called)
-        args, kwargs = disable_member.call_args
+        args, kwargs = disable_user.call_args
         self.assertEqual(self.member, args[0])
 
     @mock.patch('services.signals.set_state')

stock/templates/public/base.html

@@ -106,15 +106,11 @@
                                 <i class="fa fa-dashboard fa-fw grayiconecolor"></i>{% trans " Dashboard" %}
                             </a>
                         </li>
-
+                        <li>
-                        {% if STATE == MEMBER_STATE or user.is_superuser %}
+                            <a class="{% navactive request 'auth_groups' %}" href="{% url 'auth_groups' %}">
-                            <li>
+                                <i class="fa fa-cogs fa-fw fa-sitemap grayiconecolor"></i>{% trans " Groups" %}
-                                <a class="{% navactive request 'auth_groups' %}" href="{% url 'auth_groups' %}">
+                            </a>
-                                    <i class="fa fa-cogs fa-fw fa-sitemap grayiconecolor"></i>{% trans " Groups" %}
+                        </li>
-                                </a>
-                            </li>
-                        {% endif %}
-
                         <li>
                             <a class="{% navactive request 'auth_help' %}" href="{% url 'auth_help' %}">
                                 <i class="fa fa-question fa-fw grayiconecolor"></i>{% trans " Help" %}

stock/templates/registered/groups.html

@@ -10,48 +10,44 @@
 {% block content %}
     <div class="col-lg-12">
         <h1 class="page-header text-center">{% trans "Available Groups" %}</h1>
-        {% if STATE == MEMBER_STATE or user.is_superuser %}
+        {% if groups %}
-            {% if groups %}
+        <table class="table">
-            <table class="table">
+            <tr>
-                <tr>
+                <th class="text-center">{% trans "Name" %}</th>
-                    <th class="text-center">{% trans "Name" %}</th>
+                <th class="text-center">{% trans "Description" %}</th>
-                    <th class="text-center">{% trans "Description" %}</th>
+                <th class="text-center">{% trans "Action" %}</th>
-                    <th class="text-center">{% trans "Action" %}</th>
+            </tr>
-                </tr>
 
-                {% for g in groups %}
+            {% for g in groups %}
-                    <tr>
+                <tr>
-                        <td class="text-center">{{ g.group.name }}</td>
+                    <td class="text-center">{{ g.group.name }}</td>
-                        <td class="text-center">{{ g.group.authgroup.description }}</td>
+                    <td class="text-center">{{ g.group.authgroup.description }}</td>
-                        <td class="text-center">
+                    <td class="text-center">
-                            {% if g.group in user.groups.all %}
+                        {% if g.group in user.groups.all %}
-                                {% if not g.request %}
+                            {% if not g.request %}
-                                    <a href="{% url 'auth_group_request_leave' g.group.id %}" class="btn btn-danger">
+                                <a href="{% url 'auth_group_request_leave' g.group.id %}" class="btn btn-danger">
-                                        {% trans "Leave" %}
+                                    {% trans "Leave" %}
-                                    </a>
-                                {% else %}
-                                    <button type="button" class="btn btn-primary" disabled>
-                                        {{ g.request.status }}
-                                    </button>
-                                {% endif %}
-                            {% elif not g.request %}
-                                <a href="{% url 'auth_group_request_add' g.group.id %}" class="btn btn-success">
-                                    {% trans "Request" %}
                                 </a>
                             {% else %}
                                 <button type="button" class="btn btn-primary" disabled>
                                     {{ g.request.status }}
                                 </button>
                             {% endif %}
-                        </td>
+                        {% elif not g.request %}
-                    </tr>
+                            <a href="{% url 'auth_group_request_add' g.group.id %}" class="btn btn-success">
-                {% endfor %}
+                                {% trans "Request" %}
-            </table>
+                            </a>
-            {% else %}
+                        {% else %}
-            <div class="alert alert-warning text-center">No groups available.</div>
+                            <button type="button" class="btn btn-primary" disabled>
-            {% endif %}
+                                {{ g.request.status }}
+                            </button>
+                        {% endif %}
+                    </td>
+                </tr>
+            {% endfor %}
+        </table>
         {% else %}
-            <div class="alert alert-danger" role="alert">{% trans "You are not a member." %}</div>
+        <div class="alert alert-warning text-center">No groups available.</div>
         {% endif %}
     </div>