diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 0221c2d50..81d3b9b61 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -3,6 +3,7 @@ import logging from django.contrib.auth.models import AbstractUser from django.db import models from django.db.models import Q +from django.forms import ValidationError from registrar.models.domain_information import DomainInformation from registrar.models.user_domain_role import UserDomainRole @@ -229,6 +230,16 @@ class User(AbstractUser): def has_contact_info(self): return bool(self.title or self.email or self.phone) + def clean(self): + """Extends clean method to perform additional validation, which can raise errors in django admin.""" + super().clean() + + if self.portfolio is None and self._get_portfolio_permissions(): + raise ValidationError("When portfolio roles or additional permissions are assigned, portfolio is required.") + + if self.portfolio is not None and not self._get_portfolio_permissions(): + raise ValidationError("When portfolio is assigned, portfolio roles or additional permissions are required.") + def _get_portfolio_permissions(self): """ Retrieve the permissions for the user's portfolio roles. diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 8c69517e9..5167aac99 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1,3 +1,4 @@ +from django.forms import ValidationError from django.test import TestCase from django.db.utils import IntegrityError from django.db import transaction @@ -1348,6 +1349,7 @@ class TestUser(TestCase): self.user.phone = None self.assertFalse(self.user.has_contact_info()) + @less_console_noise_decorator def test_has_portfolio_permission(self): """ 0. Returns False when user does not have a permission @@ -1401,6 +1403,37 @@ class TestUser(TestCase): Portfolio.objects.all().delete() + @less_console_noise_decorator + def test_user_with_portfolio_but_no_roles(self): + # Create an instance of User with a portfolio but no roles or additional permissions + portfolio, _ = Portfolio.objects.get_or_create(creator=self.user, organization_name="Hotel California") + + self.user.portfolio = portfolio + self.user.portfolio_roles = [] + + # Test if the ValidationError is raised with the correct message + with self.assertRaises(ValidationError) as cm: + self.user.clean() + + self.assertEqual( + cm.exception.message, "When portfolio is assigned, portfolio roles or additional permissions are required." + ) + Portfolio.objects.all().delete() + + @less_console_noise_decorator + def test_user_with_portfolio_roles_but_no_portfolio(self): + # Create an instance of User with a portfolio role but no portfolio + self.user.portfolio = None + self.user.portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + + # Test if the ValidationError is raised with the correct message + with self.assertRaises(ValidationError) as cm: + self.user.clean() + + self.assertEqual( + cm.exception.message, "When portfolio roles or additional permissions are assigned, portfolio is required." + ) + class TestContact(TestCase): @less_console_noise_decorator diff --git a/src/zap.conf b/src/zap.conf index d6d22995c..c97897aeb 100644 --- a/src/zap.conf +++ b/src/zap.conf @@ -71,6 +71,7 @@ 10038 OUTOFSCOPE http://app:8080/domain_requests/ 10038 OUTOFSCOPE http://app:8080/domains/ 10038 OUTOFSCOPE http://app:8080/organization/ +10038 OUTOFSCOPE http://app:8080/suborganization/ # This URL always returns 404, so include it as well. 10038 OUTOFSCOPE http://app:8080/todo # OIDC isn't configured in the test environment and DEBUG=True so this gives a 500 without CSP headers