DST-965 - Fixing the manage user page so that group membership is ass…

…igned then and not on account creation

django_app/authentication/backends.py

@@ -89,8 +89,6 @@ def get_or_create_user(self, profile: dict[str, Any]) -> User:
                     is_staff=False,
                 )
                 new_user.set_unusable_password()
-                internal_user_group = Group.objects.get(name=settings.INTERNAL_USER_GROUP_NAME)
-                new_user.groups.add(internal_user_group)
                 new_user.save()
 
                 return new_user

django_app/authentication/migrations/0001_initial.py

@@ -22,6 +22,7 @@ def create_user_groups(apps, schema_editor):
     # give all staff users (currently user management users) access to the admin user group
     for user in User.objects.filter(is_staff=True):
         user.groups.add(admin_user_group)
+        user.groups.add(internal_user_group)
 
 
 def delete_user_groups(apps, schema_editor):

django_app/view_a_licence/mixins.py

@@ -32,4 +32,4 @@ def handle_no_group_membership(self, request):
 class AdminUserOnlyMixin(GroupsRequiredMixin):
     """Mixin to restrict access to admin users only"""
 
-    groups_required = [settings.INTERNAL_USER_GROUP_NAME, settings.ADMIN_USER_GROUP_NAME]
+    groups_required = [settings.ADMIN_USER_GROUP_NAME]

django_app/view_a_licence/views.py

@@ -8,6 +8,7 @@
 )
 from apply_for_a_licence.models import Licence
 from authentication.mixins import LoginRequiredMixin
+from authentication.utils import get_group
 from core.sites import require_view_a_licence
 from django.conf import settings
 from django.contrib.auth.models import User
@@ -108,6 +109,7 @@ def get_context_data(self, **kwargs: object) -> dict[str, Any]:
     def get(self, request: HttpRequest, **kwargs: object) -> HttpResponse:
         if update_user := self.request.GET.get("accept_user", None):
             user_to_accept = User.objects.get(id=update_user)
+            user_to_accept.groups.add(get_group(settings.INTERNAL_USER_GROUP_NAME))
             user_to_accept.is_active = True
             user_to_accept.save()
 

tests/test_unit/test_view_a_licence/test_views/test_manage_users_view.py

@@ -33,13 +33,15 @@ def test_get_context_data(self, vl_client_logged_in, staff_user):
 
     def test_make_active(self, vl_client_logged_in):
         inactive_user = UserFactory.create(is_active=False, is_staff=False)
+        assert not inactive_user.groups.filter(name=settings.INTERNAL_USER_GROUP_NAME).exists()
 
         response = vl_client_logged_in.get(reverse("view_a_licence:manage_users") + f"?accept_user={inactive_user.id}")
         assert response.status_code == 302
         assert response.url == reverse("view_a_licence:manage_users")
 
         inactive_user.refresh_from_db()
         assert inactive_user.is_active
+        assert inactive_user.groups.filter(name=settings.INTERNAL_USER_GROUP_NAME).exists()
 
     def test_delete_user(self, vl_client_logged_in):
         active_user = UserFactory.create(is_active=True, is_staff=True)

tests/test_unit/test_view_a_licence/test_views/test_view_authorisation.py

@@ -119,8 +119,10 @@ def test_unauthorised_user(self, vl_client):
             "John",
             "[email protected]",
             is_active=True,
-            is_staff=False,
+            is_staff=True,
         )
+        # even with internal user group they should not have access
+        test_user.groups.add(get_group(settings.INTERNAL_USER_GROUP_NAME))
 
         vl_client.force_login(test_user, backend="authentication.backends.StaffSSOBackend")
         response = vl_client.get(reverse("view_a_licence:manage_users"))