zydronium/manage.get.gov
1
0
Fork 0
mirror of https://github.com/cisagov/manage.get.gov.git synced 2025-07-25 20:18:38 +02:00
Code Issues Projects Releases Packages Wiki Activity

Merge pull request #3706 from cisagov/backup/3622-warn-org-admins

Browse source 
#3622: Members page, warn org admins when they are modifying themselves - [BACKUP]
This commit is contained in:
zandercymatics 2025-03-28 07:58:35 -06:00 • committed by GitHub
commit d07fab387a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 189 additions and 41 deletions
Download patch file Download diff file Expand all files Collapse all files

6
src/registrar/assets/src/js/getgov/main.js
View file

@ -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();

39
src/registrar/assets/src/js/getgov/portfolio-member-page.js
View file

@ -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();
});
}
});
}

21
src/registrar/forms/portfolio.py
View file

@ -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):
"""

2
src/registrar/models/user.py
View file

@ -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

12
src/registrar/templates/portfolio_member.html
View file

@ -98,6 +98,18 @@ Organization member
</address>
{% if portfolio_permission %}
{% if member and member.id == request.user.id and is_only_admin %}
<div class="usa-alert usa-alert--info usa-alert--slim">
<div class="usa-alert__body">
<p class="usa-alert__text ">
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.
</p>
</div>
</div>
{% 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 %}

32
src/registrar/templates/portfolio_member_permissions.html
View file

@ -75,11 +75,25 @@
<!-- Member email -->
</fieldset>
<!-- Member access radio buttons (Toggles other sections) -->
<fieldset class="usa-fieldset">
<legend>
<h2 class="margin-top-0">Member access</h2>
</legend>
{% if member and member.id == request.user.id and is_only_admin %}
<div class="usa-alert usa-alert--info usa-alert--slim margin-top-1 margin-bottom-1">
<div class="usa-alert__body">
<p class="usa-alert__text ">
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.
</p>
</div>
</div>
{% endif %}
<em>Select the level of access for this member. <abbr class="usa-hint usa-hint--required" title="required">*</abbr></em>
@ -109,4 +123,22 @@
</div>
</form>
</div>
{% 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 %}
<a
id="toggle-member-permissions-edit-self"
href="#modal-member-permissions-edit-self"
class="display-none"
aria-controls="modal-member-permissions-edit-self"
data-open-modal
>Edit self</a>
<div
class="usa-modal"
id="modal-member-permissions-edit-self"
data-force-action
>
{% include 'includes/modal.html' with modal_heading="Are you sure you want to change your member access?" modal_description="You’ve selected basic access, which means you’ll no longer be able to manage member permissions. This action cannot be undone." modal_button_id="member-permissions-edit-self" modal_button_text="Yes, change my access" %}
</div>
{% endif %}
{% endblock portfolio_content%}

34
src/registrar/tests/test_forms.py
View file

@ -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)

56
src/registrar/tests/test_views_portfolio.py
View file

@ -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"""

28
src/registrar/views/portfolios.py
View file

@ -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."""