From 32edeb911b1ff4af21b31bb6739dc4976cc9f2f4 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 29 Nov 2024 11:09:04 -0700 Subject: [PATCH] Add some unit tests --- .../models/user_portfolio_permission.py | 7 +- src/registrar/tests/test_admin.py | 132 ++++++++++++++++++ 2 files changed, 137 insertions(+), 2 deletions(-) diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index b3d679720..a5fb0a31b 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -162,13 +162,16 @@ class UserPortfolioPermission(TimeStampedModel): # so called "forbidden" ones. But just member on their own cannot. # The solution to this is to only grab what is only COMMONLY "forbidden". # This will scale if we add more roles in the future. + # This is thes same as applying the `&` operator across all sets for each role. common_forbidden_perms = set.intersection( - *(set(cls.FORBIDDEN_PORTFOLIO_ROLE_PERMISSIONS.get(role, [])) for role in roles) + *[set(cls.FORBIDDEN_PORTFOLIO_ROLE_PERMISSIONS.get(role, [])) for role in roles] ) # Check if the users current permissions overlap with any forbidden permissions + # by getting the intersection between current user permissions, and forbidden ones. + # This is the same as portfolio_permissions & common_forbidden_perms. portfolio_permissions = set(cls.get_portfolio_permissions(roles, additional_permissions)) - return portfolio_permissions & common_forbidden_perms + return portfolio_permissions.intersection(common_forbidden_perms) def clean(self): """Extends clean method to perform additional validation, which can raise errors in django admin.""" diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 84b85e412..eef59e871 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -25,6 +25,7 @@ from registrar.admin import ( TransitionDomainAdmin, UserGroupAdmin, PortfolioAdmin, + UserPortfolioPermissionAdmin, ) from registrar.models import ( Domain, @@ -65,6 +66,8 @@ from django.contrib.sessions.backends.db import SessionStore from django.contrib.auth import get_user_model from unittest.mock import ANY, patch, Mock +from registrar.models.utility.portfolio_helper import validate_portfolio_invitation, validate_user_portfolio_permission +from django.forms import ValidationError import logging @@ -187,6 +190,93 @@ class TestDomainInvitationAdmin(TestCase): self.assertContains(response, retrieved_html, count=1) +class TestUserPortfolioPermissionAdmin(TestCase): + """Tests for the PortfolioInivtationAdmin class""" + + def setUp(self): + """Create a client object""" + self.factory = RequestFactory() + self.admin = ListHeaderAdmin(model=UserPortfolioPermissionAdmin, admin_site=AdminSite()) + self.client = Client(HTTP_HOST="localhost:8080") + self.superuser = create_superuser() + self.portfolio = Portfolio.objects.create(organization_name="Test Portfolio", creator=self.superuser) + + def tearDown(self): + """Delete all DomainInvitation objects""" + Portfolio.objects.all().delete() + PortfolioInvitation.objects.all().delete() + Contact.objects.all().delete() + User.objects.all().delete() + + @less_console_noise_decorator + def test_validate_user_portfolio_permission(self): + """Tests validation of user portfolio permission""" + + # Test validation fails when portfolio missing but permissions are present + permission = UserPortfolioPermission(user=self.superuser, roles=["organization_admin"], portfolio=None) + with self.assertRaises(ValidationError) as err: + validate_user_portfolio_permission(permission) + self.assertEqual( + str(err.exception), + "When portfolio roles or additional permissions are assigned, portfolio is required.", + ) + + # Test validation fails when portfolio present but no permissions are present + permission = UserPortfolioPermission(user=self.superuser, roles=None, portfolio=self.portfolio) + with self.assertRaises(ValidationError) as err: + validate_user_portfolio_permission(permission) + self.assertEqual( + str(err.exception), + "When portfolio is assigned, portfolio roles or additional permissions are required.", + ) + + # Test validation fails with forbidden permissions for single role + forbidden_member_roles = UserPortfolioPermission.FORBIDDEN_PORTFOLIO_ROLE_PERMISSIONS.get( + UserPortfolioRoleChoices.ORGANIZATION_MEMBER + ) + permission = UserPortfolioPermission( + user=self.superuser, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + additional_permissions=forbidden_member_roles, + portfolio=self.portfolio, + ) + with self.assertRaises(ValidationError) as err: + permission.clean() + self.assertEqual( + str(err.exception), + "These permissions cannot be assigned to Member: " + "", + ) + + @less_console_noise_decorator + def test_get_forbidden_permissions_with_multiple_roles(self): + """Tests that forbidden permissions are properly handled when a user has multiple roles""" + # Get forbidden permissions for member role + member_forbidden = UserPortfolioPermission.FORBIDDEN_PORTFOLIO_ROLE_PERMISSIONS.get( + UserPortfolioRoleChoices.ORGANIZATION_MEMBER + ) + + # Test with both admin and member roles + roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN, UserPortfolioRoleChoices.ORGANIZATION_MEMBER] + + # These permissions would be forbidden for member alone, but should be allowed + # when combined with admin role + permissions = UserPortfolioPermission.get_forbidden_permissions( + roles=roles, additional_permissions=member_forbidden + ) + + # Should return empty set since no permissions are commonly forbidden between admin and member + self.assertEqual(permissions, set()) + + # Verify the same permissions are forbidden when only member role is present + member_only_permissions = UserPortfolioPermission.get_forbidden_permissions( + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], additional_permissions=member_forbidden + ) + + # Should return the forbidden permissions for member role + self.assertEqual(member_only_permissions, set(member_forbidden)) + + class TestPortfolioInvitationAdmin(TestCase): """Tests for the PortfolioInvitationAdmin class as super user @@ -204,9 +294,11 @@ class TestPortfolioInvitationAdmin(TestCase): def setUp(self): """Create a client object""" self.client = Client(HTTP_HOST="localhost:8080") + self.portfolio = Portfolio.objects.create(organization_name="Test Portfolio", creator=self.superuser) def tearDown(self): """Delete all DomainInvitation objects""" + Portfolio.objects.all().delete() PortfolioInvitation.objects.all().delete() Contact.objects.all().delete() @@ -214,6 +306,46 @@ class TestPortfolioInvitationAdmin(TestCase): def tearDownClass(self): User.objects.all().delete() + @less_console_noise_decorator + def test_portfolio_invitation_clean(self): + """Tests validation of portfolio invitation permissions""" + + # Test validation fails when portfolio missing but permissions present + invitation = PortfolioInvitation(email="test@example.com", roles=["organization_admin"], portfolio=None) + with self.assertRaises(ValidationError) as err: + invitation.clean() + self.assertEqual( + str(err.exception), + "When portfolio roles or additional permissions are assigned, portfolio is required.", + ) + + # Test validation fails when portfolio present but no permissions + invitation = PortfolioInvitation(email="test@example.com", roles=None, portfolio=self.portfolio) + with self.assertRaises(ValidationError) as err: + invitation.clean() + self.assertEqual( + str(err.exception), + "When portfolio is assigned, portfolio roles or additional permissions are required.", + ) + + # Test validation fails with forbidden permissions + forbidden_member_roles = UserPortfolioPermission.FORBIDDEN_PORTFOLIO_ROLE_PERMISSIONS.get( + UserPortfolioRoleChoices.ORGANIZATION_MEMBER + ) + invitation = PortfolioInvitation( + email="test@example.com", + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + additional_permissions=forbidden_member_roles, + portfolio=self.portfolio, + ) + with self.assertRaises(ValidationError) as err: + invitation.clean() + self.assertEqual( + str(err.exception), + "These permissions cannot be assigned to Member: " + "", + ) + @less_console_noise_decorator def test_has_model_description(self): """Tests if this model has a model description on the table view"""