diff --git a/src/registrar/assets/src/js/getgov/main.js b/src/registrar/assets/src/js/getgov/main.js index 490874220..a496b76f3 100644 --- a/src/registrar/assets/src/js/getgov/main.js +++ b/src/registrar/assets/src/js/getgov/main.js @@ -11,7 +11,7 @@ import { initDomainRequestsTable } from './table-domain-requests.js'; import { initMembersTable } from './table-members.js'; import { initMemberDomainsTable } from './table-member-domains.js'; import { initEditMemberDomainsTable } from './table-edit-member-domains.js'; -import { initPortfolioNewMemberPageToggle, initAddNewMemberPageListeners, initPortfolioMemberPageRadio } from './portfolio-member-page.js'; +import { initPortfolioNewMemberPageToggle, initAddNewMemberPageListeners, initPortfolioMemberPage } from './portfolio-member-page.js'; import { initDomainRequestForm } from './domain-request-form.js'; import { initDomainManagersPage } from './domain-managers.js'; import { initDomainDNSSEC } from './domain-dnssec.js'; @@ -56,8 +56,10 @@ initDomainDNSSEC(); initFormErrorHandling(); +// Init the portfolio member page +initPortfolioMemberPage(); + // Init the portfolio new member page -initPortfolioMemberPageRadio(); initPortfolioNewMemberPageToggle(); initAddNewMemberPageListeners(); diff --git a/src/registrar/assets/src/js/getgov/portfolio-member-page.js b/src/registrar/assets/src/js/getgov/portfolio-member-page.js index 96961e5dc..faf2c7b18 100644 --- a/src/registrar/assets/src/js/getgov/portfolio-member-page.js +++ b/src/registrar/assets/src/js/getgov/portfolio-member-page.js @@ -193,10 +193,14 @@ export function initAddNewMemberPageListeners() { } // Initalize the radio for the member pages -export function initPortfolioMemberPageRadio() { +export function initPortfolioMemberPage() { document.addEventListener("DOMContentLoaded", () => { let memberForm = document.getElementById("member_form"); - let newMemberForm = document.getElementById("add_member_form") + let newMemberForm = document.getElementById("add_member_form"); + let editSelfWarningModal = document.getElementById("toggle-member-permissions-edit-self"); + let editSelfWarningModalConfirm = document.getElementById("member-permissions-edit-self"); + + // Init the radio if (memberForm || newMemberForm) { hookupRadioTogglerListener( 'role', @@ -206,5 +210,36 @@ export function initPortfolioMemberPageRadio() { } ); } + + // Init the "edit self" warning modal, which triggers when the user is trying to edit themselves. + // The dom will include these elements when this occurs. + // NOTE: This logic does not trigger when the user is the ONLY admin in the portfolio. + // This is because info alerts are used rather than modals in this case. + if (memberForm && editSelfWarningModal) { + // Only show the warning modal when the user is changing their ROLE. + var canSubmit = document.querySelector(`input[name="role"]:checked`)?.value != "organization_member"; + let radioButtons = document.querySelectorAll(`input[name="role"]`); + radioButtons.forEach(function (radioButton) { + radioButton.addEventListener("change", function() { + let selectedValue = radioButton.checked ? radioButton.value : null; + canSubmit = selectedValue != "organization_member"; + }); + }); + + // Prevent form submission assuming org member is selected for role, and open the modal. + memberForm.addEventListener("submit", function(e) { + if (!canSubmit) { + e.preventDefault(); + editSelfWarningModal.click(); + } + }); + + // Hook the confirm button on the modal to form submission. + editSelfWarningModalConfirm.addEventListener("click", function() { + canSubmit = true; + memberForm.submit(); + }); + } }); + } diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py index db1f58d88..43a391ae0 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -411,6 +411,27 @@ class PortfolioMemberForm(BasePortfolioMemberForm): model = UserPortfolioPermission fields = ["roles", "additional_permissions"] + def clean(self): + """ + Override of clean to ensure that the user isn't removing themselves + if they're the only portfolio admin + """ + super().clean() + role = self.cleaned_data.get("role") + if self.instance and hasattr(self.instance, "user") and hasattr(self.instance, "portfolio"): + if role and self.instance.user.is_only_admin_of_portfolio(self.instance.portfolio): + # This is how you associate a validation error to a particular field. + # The alternative is to do this in clean_role, but execution order matters. + raise forms.ValidationError( + { + "role": forms.ValidationError( + "You can't change your member access because you're " + "the only admin for this organization. " + "To change your access, you'll need to add another admin." + ) + } + ) + class PortfolioInvitedMemberForm(BasePortfolioMemberForm): """ diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index d5476ab9a..c59ab376b 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -474,7 +474,7 @@ class User(AbstractUser): admin_count = admins.count() # Check if the current user is in the list of admins - if admin_count == 1 and admins.first().user == self: + if admin_count == 1 and admins.first() and admins.first().user == self: return True # The user is the only admin # If there are other admins or the user is not the only one diff --git a/src/registrar/templates/portfolio_member.html b/src/registrar/templates/portfolio_member.html index 1b9ffd653..1afe96161 100644 --- a/src/registrar/templates/portfolio_member.html +++ b/src/registrar/templates/portfolio_member.html @@ -98,6 +98,18 @@ Organization member {% if portfolio_permission %} + {% if member and member.id == request.user.id and is_only_admin %} +
+
+

+ You're the only admin for this organization. + Organizations must have at least one admin. + To remove yourself or change your member access, + you'll need to add another admin. +

+
+
+ {% endif %} {% include "includes/summary_item.html" with title='Member access and permissions' permissions=True value=portfolio_permission edit_link=edit_url editable=has_edit_members_portfolio_permission %} {% elif portfolio_invitation %} {% include "includes/summary_item.html" with title='Member access and permissions' permissions=True value=portfolio_invitation edit_link=edit_url editable=has_edit_members_portfolio_permission %} diff --git a/src/registrar/templates/portfolio_member_permissions.html b/src/registrar/templates/portfolio_member_permissions.html index e5ae5864e..59a8b1fd8 100644 --- a/src/registrar/templates/portfolio_member_permissions.html +++ b/src/registrar/templates/portfolio_member_permissions.html @@ -75,11 +75,25 @@ +

Member access

 + + {% if member and member.id == request.user.id and is_only_admin %} +
+
+

+ You're the only admin for this organization. + Organizations must have at least one admin. + To remove yourself or change your member access, + you'll need to add another admin. +

+
+
+ {% endif %} Select the level of access for this member. * @@ -109,4 +123,22 @@ + + {% comment %} If an admin is trying to edit themselves, show a modal {% endcomment %} + {% if member and member.id == request.user.id and not is_only_admin %} + + + {% endif %} {% endblock portfolio_content%} diff --git a/src/registrar/tests/test_forms.py b/src/registrar/tests/test_forms.py index 1c8b127ae..6e4b6adae 100644 --- a/src/registrar/tests/test_forms.py +++ b/src/registrar/tests/test_forms.py @@ -477,8 +477,8 @@ class TestBasePortfolioMemberForms(TestCase): self.assertTrue(form.is_valid(), f"Form {form_class.__name__} failed validation with data: {data}") return form - def _assert_form_has_error(self, form_class, data, field_name): - form = form_class(data=data) + def _assert_form_has_error(self, form_class, data, field_name, instance=None): + form = form_class(data=data, instance=instance) self.assertFalse(form.is_valid()) self.assertIn(field_name, form.errors) @@ -504,17 +504,23 @@ class TestBasePortfolioMemberForms(TestCase): "domain_permissions": "", # Simulate missing field "member_permissions": "", # Simulate missing field } + user_portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create( + portfolio=self.portfolio, user=self.user + ) + portfolio_invitation, _ = PortfolioInvitation.objects.get_or_create(portfolio=self.portfolio, email="hi@ho") # Check required fields for all forms - self._assert_form_has_error(PortfolioMemberForm, data, "domain_request_permissions") - self._assert_form_has_error(PortfolioMemberForm, data, "domain_permissions") - self._assert_form_has_error(PortfolioMemberForm, data, "member_permissions") - self._assert_form_has_error(PortfolioInvitedMemberForm, data, "domain_request_permissions") - self._assert_form_has_error(PortfolioInvitedMemberForm, data, "domain_permissions") - self._assert_form_has_error(PortfolioInvitedMemberForm, data, "member_permissions") - self._assert_form_has_error(PortfolioNewMemberForm, data, "domain_request_permissions") - self._assert_form_has_error(PortfolioNewMemberForm, data, "domain_permissions") - self._assert_form_has_error(PortfolioNewMemberForm, data, "member_permissions") + self._assert_form_has_error(PortfolioMemberForm, data, "domain_request_permissions", user_portfolio_permission) + self._assert_form_has_error(PortfolioMemberForm, data, "domain_permissions", user_portfolio_permission) + self._assert_form_has_error(PortfolioMemberForm, data, "member_permissions", user_portfolio_permission) + self._assert_form_has_error( + PortfolioInvitedMemberForm, data, "domain_request_permissions", portfolio_invitation + ) + self._assert_form_has_error(PortfolioInvitedMemberForm, data, "domain_permissions", portfolio_invitation) + self._assert_form_has_error(PortfolioInvitedMemberForm, data, "member_permissions", portfolio_invitation) + self._assert_form_has_error(PortfolioNewMemberForm, data, "domain_request_permissions", portfolio_invitation) + self._assert_form_has_error(PortfolioNewMemberForm, data, "domain_permissions", portfolio_invitation) + self._assert_form_has_error(PortfolioNewMemberForm, data, "member_permissions", portfolio_invitation) @less_console_noise_decorator def test_clean_validates_required_fields_for_admin_role(self): @@ -529,7 +535,6 @@ class TestBasePortfolioMemberForms(TestCase): portfolio=self.portfolio, user=self.user ) portfolio_invitation, _ = PortfolioInvitation.objects.get_or_create(portfolio=self.portfolio, email="hi@ho") - data = { "role": UserPortfolioRoleChoices.ORGANIZATION_ADMIN.value, } @@ -677,6 +682,7 @@ class TestBasePortfolioMemberForms(TestCase): @less_console_noise_decorator def test_invalid_data_for_member(self): """Test invalid form submission for a member role with missing permissions.""" + portfolio_invitation, _ = PortfolioInvitation.objects.get_or_create(portfolio=self.portfolio, email="hi@ho") data = { "email": "hi@ho.com", "portfolio": self.portfolio.id, @@ -685,5 +691,5 @@ class TestBasePortfolioMemberForms(TestCase): "member_permissions": "", # Missing field "domain_permissions": "", # Missing field } - self._assert_form_has_error(PortfolioMemberForm, data, "domain_request_permissions") - self._assert_form_has_error(PortfolioInvitedMemberForm, data, "member_permissions") + self._assert_form_has_error(PortfolioMemberForm, data, "domain_request_permissions", portfolio_invitation) + self._assert_form_has_error(PortfolioInvitedMemberForm, data, "member_permissions", portfolio_invitation) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index f490f51ad..81be0d3ad 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -1826,10 +1826,7 @@ class TestPortfolioMemberDeleteView(WebTest): ) self.assertEqual(response.status_code, 400) - expected_error_message = ( - "There must be at least one admin in your organization. Give another member admin " - "permissions, make sure they log into the registrar, and then remove this member." - ) + expected_error_message = "the only admin for this organization" self.assertContains(response, expected_error_message, status_code=400) # assert that send_portfolio_admin_removal_emails is not called @@ -2155,17 +2152,14 @@ class TestPortfolioMemberDeleteView(WebTest): self.assertEqual(response.status_code, 302) - expected_error_message = ( - "There must be at least one admin in your organization. Give another member admin " - "permissions, make sure they log into the registrar, and then remove this member." - ) + expected_error_message = "the only admin for this organization." args, kwargs = mock_error.call_args # Check if first arg is a WSGIRequest, confirms request object passed correctly # WSGIRequest protocol is basically the HTTPRequest but in Django form (ie POST '/member/1/delete') self.assertIsInstance(args[0], WSGIRequest) # Check that the error message matches the expected error message - self.assertEqual(args[1], expected_error_message) + self.assertIn(expected_error_message, args[1]) # Location is used for a 3xx HTTP status code to indicate that the URL was redirected # and then confirm that we're still on the Manage Members page @@ -4639,6 +4633,17 @@ class TestPortfolioMemberEditView(WebTest): # Get the user's admin permission admin_permission = UserPortfolioPermission.objects.get(user=self.user, portfolio=self.portfolio) + # Create a second permission so the user isn't just deleting themselves + member = create_test_user() + UserPortfolioPermission.objects.create( + user=member, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + + # First, verify that the change modal is on the page + response = self.client.get(reverse("member-permissions", kwargs={"member_pk": admin_permission.id})) + self.assertEqual(response.status_code, 200) + self.assertContains(response, "Yes, change my access") + response = self.client.post( reverse("member-permissions", kwargs={"member_pk": admin_permission.id}), { @@ -4652,6 +4657,39 @@ class TestPortfolioMemberEditView(WebTest): self.assertEqual(response.status_code, 302) self.assertEqual(response["Location"], reverse("home")) + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + def test_admin_removing_own_admin_role_only_admin(self): + """Tests that admin removing their own admin role when they are the only admin + throws a validation error. + """ + + self.client.force_login(self.user) + + # Get the user's admin permission + admin_permission = UserPortfolioPermission.objects.get(user=self.user, portfolio=self.portfolio) + + # First, verify that the info alert is present on the page + response = self.client.get(reverse("member-permissions", kwargs={"member_pk": admin_permission.id})) + self.assertEqual(response.status_code, 200) + self.assertContains(response, "To remove yourself or change your member access") + + # Then, verify that the right form error is shown + response = self.client.post( + reverse("member-permissions", kwargs={"member_pk": admin_permission.id}), + { + "role": UserPortfolioRoleChoices.ORGANIZATION_MEMBER, + "domain_permissions": UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS, + "member_permissions": "no_access", + "domain_request_permissions": "no_access", + }, + ) + + self.assertEqual(response.status_code, 200) + error_message = "the only admin for this organization" + self.assertIn(error_message, str(response.context["form"].errors)) + class TestPortfolioInvitedMemberEditView(WebTest): """Tests for the edit invited member page on portfolios""" diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index 952793084..2a9b9684d 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -114,6 +114,7 @@ class PortfolioMemberView(DetailView, View): "member_has_view_members_portfolio_permission": member_has_view_members_portfolio_permission, "member_has_edit_members_portfolio_permission": member_has_edit_members_portfolio_permission, "member_has_view_all_domains_portfolio_permission": member_has_view_all_domains_portfolio_permission, + "is_only_admin": request.user.is_only_admin_of_portfolio(portfolio_permission.portfolio), }, ) @@ -160,8 +161,8 @@ class PortfolioMemberDeleteView(View): ) if member.is_only_admin_of_portfolio(portfolio): return ( - "There must be at least one admin in your organization. Give another member admin " - "permissions, make sure they log into the registrar, and then remove this member." + "You can't remove yourself because you're the only admin for this organization. " + "To remove yourself, you'll need to add another admin." ) return None @@ -257,9 +258,7 @@ class PortfolioMemberEditView(DetailView, View): def get(self, request, member_pk): portfolio_permission = get_object_or_404(UserPortfolioPermission, pk=member_pk) user = portfolio_permission.user - form = self.form_class(instance=portfolio_permission) - return render( request, self.template_name, @@ -267,6 +266,7 @@ class PortfolioMemberEditView(DetailView, View): "form": form, "member": user, "portfolio_permission": portfolio_permission, + "is_only_admin": request.user.is_only_admin_of_portfolio(portfolio_permission.portfolio), }, ) @@ -307,15 +307,17 @@ class PortfolioMemberEditView(DetailView, View): form.save() messages.success(self.request, "The member access and permission changes have been saved.") return redirect("member", member_pk=member_pk) if not removing_admin_role_on_self else redirect("home") - - return render( - request, - self.template_name, - { - "form": form, - "member": user, # Pass the user object again to the template - }, - ) + else: + return render( + request, + self.template_name, + { + "form": form, + "member": user, + "portfolio_permission": portfolio_permission, + "is_only_admin": request.user.is_only_admin_of_portfolio(portfolio_permission.portfolio), + }, + ) def _handle_exceptions(self, exception): """Handle exceptions raised during the process."""