diff --git a/django_app/authentication/backends.py b/django_app/authentication/backends.py index 0b9dfe7a..0908db1d 100644 --- a/django_app/authentication/backends.py +++ b/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 diff --git a/django_app/authentication/migrations/0001_initial.py b/django_app/authentication/migrations/0001_initial.py index 3d8bac80..706e3af0 100644 --- a/django_app/authentication/migrations/0001_initial.py +++ b/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): diff --git a/django_app/view_a_licence/mixins.py b/django_app/view_a_licence/mixins.py index 057b77f6..d8a2a108 100644 --- a/django_app/view_a_licence/mixins.py +++ b/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] diff --git a/django_app/view_a_licence/views.py b/django_app/view_a_licence/views.py index 19264815..021468f3 100644 --- a/django_app/view_a_licence/views.py +++ b/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() diff --git a/tests/test_unit/test_view_a_licence/test_views/test_manage_users_view.py b/tests/test_unit/test_view_a_licence/test_views/test_manage_users_view.py index d1a6c243..578bf6d0 100644 --- a/tests/test_unit/test_view_a_licence/test_views/test_manage_users_view.py +++ b/tests/test_unit/test_view_a_licence/test_views/test_manage_users_view.py @@ -33,6 +33,7 @@ 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 @@ -40,6 +41,7 @@ def test_make_active(self, vl_client_logged_in): 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) diff --git a/tests/test_unit/test_view_a_licence/test_views/test_view_authorisation.py b/tests/test_unit/test_view_a_licence/test_views/test_view_authorisation.py index fb212c35..d2825956 100644 --- a/tests/test_unit/test_view_a_licence/test_views/test_view_authorisation.py +++ b/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@example.com", 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"))