diff --git a/src/djangooidc/backends.py b/src/djangooidc/backends.py index 41e442f2d..38fde0ced 100644 --- a/src/djangooidc/backends.py +++ b/src/djangooidc/backends.py @@ -21,49 +21,66 @@ class OpenIdConnectBackend(ModelBackend): """ def authenticate(self, request, **kwargs): - logger.debug("kwargs %s" % kwargs) - user = None - if not kwargs or "sub" not in kwargs.keys(): - return user + logger.debug("kwargs %s", kwargs) + + if not kwargs or "sub" not in kwargs: + return None UserModel = get_user_model() username = self.clean_username(kwargs["sub"]) + openid_data = self.extract_openid_data(kwargs) - # Some OP may actually choose to withhold some information, so we must - # test if it is present - openid_data = {"last_login": timezone.now()} - openid_data["first_name"] = kwargs.get("given_name", "") - openid_data["last_name"] = kwargs.get("family_name", "") - openid_data["email"] = kwargs.get("email", "") - openid_data["phone"] = kwargs.get("phone", "") - - # Note that this could be accomplished in one try-except clause, but - # instead we use get_or_create when creating unknown users since it has - # built-in safeguards for multiple threads. if getattr(settings, "OIDC_CREATE_UNKNOWN_USER", True): - args = { - UserModel.USERNAME_FIELD: username, - # defaults _will_ be updated, these are not fallbacks - "defaults": openid_data, - } - - user, created = UserModel.objects.get_or_create(**args) - - if not created: - # If user exists, update existing user - self.update_existing_user(user, args["defaults"]) - else: - # If user is created, configure the user - user = self.configure_user(user, **kwargs) + user = self.get_or_create_user(UserModel, username, openid_data, kwargs) else: - try: - user = UserModel.objects.get_by_natural_key(username) - except UserModel.DoesNotExist: - return None - # run this callback for a each login - user.on_each_login() + user = self.get_user_by_username(UserModel, username) + + if user: + user.on_each_login() + return user + def extract_openid_data(self, kwargs): + """Extract OpenID data from authentication kwargs.""" + return { + "last_login": timezone.now(), + "first_name": kwargs.get("given_name", ""), + "last_name": kwargs.get("family_name", ""), + "email": kwargs.get("email", ""), + "phone": kwargs.get("phone", ""), + } + + def get_or_create_user(self, UserModel, username, openid_data, kwargs): + """Retrieve user by username or email, or create a new user.""" + user = self.get_user_by_username(UserModel, username) + + if not user and openid_data["email"]: + user = self.get_user_by_email(UserModel, openid_data["email"]) + if user: + # if found by email, update the username + setattr(user, UserModel.USERNAME_FIELD, username) + + if not user: + user = UserModel.objects.create(**{UserModel.USERNAME_FIELD: username}, **openid_data) + return self.configure_user(user, **kwargs) + + self.update_existing_user(user, openid_data) + return user + + def get_user_by_username(self, UserModel, username): + """Retrieve user by username.""" + try: + return UserModel.objects.get(**{UserModel.USERNAME_FIELD: username}) + except UserModel.DoesNotExist: + return None + + def get_user_by_email(self, UserModel, email): + """Retrieve user by email.""" + try: + return UserModel.objects.get(email=email) + except UserModel.DoesNotExist: + return None + def update_existing_user(self, user, kwargs): """ Update user fields without overwriting certain fields. diff --git a/src/djangooidc/tests/test_backends.py b/src/djangooidc/tests/test_backends.py index c15106fa9..4e8f80a23 100644 --- a/src/djangooidc/tests/test_backends.py +++ b/src/djangooidc/tests/test_backends.py @@ -1,5 +1,6 @@ from django.test import TestCase from registrar.models import User +from api.tests.common import less_console_noise_decorator from ..backends import OpenIdConnectBackend # Adjust the import path based on your project structure @@ -17,6 +18,7 @@ class OpenIdConnectBackendTestCase(TestCase): def tearDown(self) -> None: User.objects.all().delete() + @less_console_noise_decorator def test_authenticate_with_create_user(self): """Test that authenticate creates a new user if it does not find existing user""" @@ -32,6 +34,7 @@ class OpenIdConnectBackendTestCase(TestCase): self.assertEqual(user.email, "john.doe@example.com") self.assertEqual(user.phone, "123456789") + @less_console_noise_decorator def test_authenticate_with_existing_user(self): """Test that authenticate updates an existing user if it finds one. For this test, given_name and family_name are supplied""" @@ -50,6 +53,30 @@ class OpenIdConnectBackendTestCase(TestCase): self.assertEqual(user.email, "john.doe@example.com") self.assertEqual(user.phone, "123456789") + @less_console_noise_decorator + def test_authenticate_with_existing_user_same_email_different_username(self): + """Test that authenticate updates an existing user if it finds one. + In this case, match is to an existing record with matching email but + a non-matching username. The existing record's username should be udpated. + For this test, given_name and family_name are supplied""" + # Create an existing user with the same username + User.objects.create_user(username="old_username", email="john.doe@example.com") + + # Ensure that the authenticate method updates the existing user + user = self.backend.authenticate(request=None, **self.kwargs) + self.assertIsNotNone(user) + self.assertIsInstance(user, User) + + # Verify that user fields are correctly updated + self.assertEqual(user.first_name, "John") + self.assertEqual(user.last_name, "Doe") + self.assertEqual(user.email, "john.doe@example.com") + self.assertEqual(user.phone, "123456789") + self.assertEqual(user.username, "test_user") + # Assert that a user no longer exists by the old username + self.assertFalse(User.objects.filter(username="old_username").exists()) + + @less_console_noise_decorator def test_authenticate_with_existing_user_with_existing_first_last_phone(self): """Test that authenticate updates an existing user if it finds one. For this test, given_name and family_name are not supplied. @@ -79,6 +106,7 @@ class OpenIdConnectBackendTestCase(TestCase): self.assertEqual(user.email, "john.doe@example.com") self.assertEqual(user.phone, "9999999999") + @less_console_noise_decorator def test_authenticate_with_existing_user_different_name_phone(self): """Test that authenticate updates an existing user if it finds one. For this test, given_name and family_name are supplied and overwrite""" @@ -100,6 +128,7 @@ class OpenIdConnectBackendTestCase(TestCase): self.assertEqual(user.email, "john.doe@example.com") self.assertEqual(user.phone, "123456789") + @less_console_noise_decorator def test_authenticate_with_unknown_user(self): """Test that authenticate returns None when no kwargs are supplied""" # Ensure that the authenticate method handles the case when the user is not found 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 d3422b722..c96677ebc 100644 --- a/src/registrar/assets/src/js/getgov/portfolio-member-page.js +++ b/src/registrar/assets/src/js/getgov/portfolio-member-page.js @@ -87,14 +87,6 @@ export function initAddNewMemberPageListeners() { }); }); - /* - Helper function to capitalize the first letter in a string (for display purposes) - */ - function capitalizeFirstLetter(text) { - if (!text) return ''; // Return empty string if input is falsy - return text.charAt(0).toUpperCase() + text.slice(1); - } - /* Populates contents of the "Add Member" confirmation modal */ @@ -102,10 +94,12 @@ export function initAddNewMemberPageListeners() { const permissionDetailsContainer = document.getElementById("permission_details"); permissionDetailsContainer.innerHTML = ""; // Clear previous content - // Get all permission sections (divs with h3 and radio inputs) - const permissionSections = document.querySelectorAll(`#${permission_details_div_id} > h3`); + if (permission_details_div_id == 'member-basic-permissions') { + // for basic users, display values are based on selections in the form + // Get all permission sections (divs with h3 and radio inputs) + const permissionSections = document.querySelectorAll(`#${permission_details_div_id} > h3`); - permissionSections.forEach(section => { + permissionSections.forEach(section => { // Find the

element text const sectionTitle = section.textContent; @@ -113,31 +107,46 @@ export function initAddNewMemberPageListeners() { const fieldset = section.nextElementSibling; if (fieldset && fieldset.tagName.toLowerCase() === 'fieldset') { - // Get the selected radio button within this fieldset - const selectedRadio = fieldset.querySelector('input[type="radio"]:checked'); + // Get the selected radio button within this fieldset + const selectedRadio = fieldset.querySelector('input[type="radio"]:checked'); - // If a radio button is selected, get its label text - let selectedPermission = "No permission selected"; - if (selectedRadio) { - const label = fieldset.querySelector(`label[for="${selectedRadio.id}"]`); - selectedPermission = label ? label.textContent : "No permission selected"; + // If a radio button is selected, get its label text + let selectedPermission = "No permission selected"; + if (selectedRadio) { + const label = fieldset.querySelector(`label[for="${selectedRadio.id}"]`); + if (label) { + // Get only the text node content (excluding subtext in

) + const mainText = Array.from(label.childNodes) + .filter(node => node.nodeType === Node.TEXT_NODE) + .map(node => node.textContent.trim()) + .join(""); // Combine and trim whitespace + selectedPermission = mainText || "No permission selected"; } - - // Create new elements for the modal content - const titleElement = document.createElement("h4"); - titleElement.textContent = sectionTitle; - titleElement.classList.add("text-primary"); - titleElement.classList.add("margin-bottom-0"); - - const permissionElement = document.createElement("p"); - permissionElement.textContent = selectedPermission; - permissionElement.classList.add("margin-top-0"); - - // Append to the modal content container - permissionDetailsContainer.appendChild(titleElement); - permissionDetailsContainer.appendChild(permissionElement); + } + appendPermissionInContainer(sectionTitle, selectedPermission, permissionDetailsContainer); } - }); + }); + } else { + // for admin users, the permissions are always the same + appendPermissionInContainer('Domains', 'Viewer, all', permissionDetailsContainer); + appendPermissionInContainer('Domain requests', 'Creator', permissionDetailsContainer); + appendPermissionInContainer('Members', 'Manager', permissionDetailsContainer); + } + } + + function appendPermissionInContainer(sectionTitle, permissionDisplay, permissionContainer) { + // Create new elements for the content + const titleElement = document.createElement("h4"); + titleElement.textContent = sectionTitle; + titleElement.classList.add("text-primary", "margin-bottom-0"); + + const permissionElement = document.createElement("p"); + permissionElement.textContent = permissionDisplay; + permissionElement.classList.add("margin-top-0"); + + // Append to the content container + permissionContainer.appendChild(titleElement); + permissionContainer.appendChild(permissionElement); } /* @@ -149,18 +158,25 @@ export function initAddNewMemberPageListeners() { let emailValue = document.getElementById('id_email').value; document.getElementById('modalEmail').textContent = emailValue; - // Get selected radio button for access level + // Get selected radio button for member access level let selectedAccess = document.querySelector('input[name="role"]:checked'); - // Set the selected permission text to 'Basic' or 'Admin' (the value of the selected radio button) - // This value does not have the first letter capitalized so let's capitalize it - let accessText = selectedAccess ? capitalizeFirstLetter(selectedAccess.value) : "No access level selected"; + // Map the access level values to user-friendly labels + const accessLevelMapping = { + organization_admin: "Admin", + organization_member: "Basic", + }; + // Determine the access text based on the selected value + let accessText = selectedAccess + ? accessLevelMapping[selectedAccess.value] || "Unknown access level" + : "No access level selected"; + // Update the modal with the appropriate member access level text document.getElementById('modalAccessLevel').textContent = accessText; // Populate permission details based on access level if (selectedAccess && selectedAccess.value === 'organization_admin') { - populatePermissionDetails('new-member-admin-permissions'); + populatePermissionDetails('admin'); } else { - populatePermissionDetails('new-member-basic-permissions'); + populatePermissionDetails('member-basic-permissions'); } //------- Show the modal @@ -177,22 +193,14 @@ export function initPortfolioMemberPageRadio() { document.addEventListener("DOMContentLoaded", () => { let memberForm = document.getElementById("member_form"); let newMemberForm = document.getElementById("add_member_form") - if (memberForm) { + if (memberForm || newMemberForm) { hookupRadioTogglerListener( 'role', { - 'organization_admin': 'member-admin-permissions', + 'organization_admin': '', 'organization_member': 'member-basic-permissions' } ); - }else if (newMemberForm){ - hookupRadioTogglerListener( - 'role', - { - 'organization_admin': 'new-member-admin-permissions', - 'organization_member': 'new-member-basic-permissions' - } - ); } }); } diff --git a/src/registrar/assets/src/sass/_theme/_accordions.scss b/src/registrar/assets/src/sass/_theme/_accordions.scss index 762618415..ca9990ca9 100644 --- a/src/registrar/assets/src/sass/_theme/_accordions.scss +++ b/src/registrar/assets/src/sass/_theme/_accordions.scss @@ -49,3 +49,30 @@ tr:last-of-type .usa-accordion--more-actions .usa-accordion__content { bottom: -10px; right: 30px; } + +// A CSS only show-more/show-less based on usa-accordion +.usa-accordion--show-more { + width: auto; + .usa-accordion__button[aria-expanded=false], + .usa-accordion__button[aria-expanded=false]:hover, + .usa-accordion__button[aria-expanded=true], + .usa-accordion__button[aria-expanded=true]:hover { + background-image: none; + background-color: transparent; + padding-right: 0; + padding-left: 0; + font-weight: normal; + } + .usa-accordion__button[aria-expanded=true] .expand-more { + display: inline-block; + } + .usa-accordion__button[aria-expanded=true] .expand-less { + display: none; + } + .usa-accordion__button[aria-expanded=false] .expand-more { + display: none; + } + .usa-accordion__button[aria-expanded=false] .expand-less { + display: inline-block; + } +} diff --git a/src/registrar/assets/src/sass/_theme/_tables.scss b/src/registrar/assets/src/sass/_theme/_tables.scss index a8a829a45..37ae22b1b 100644 --- a/src/registrar/assets/src/sass/_theme/_tables.scss +++ b/src/registrar/assets/src/sass/_theme/_tables.scss @@ -105,3 +105,25 @@ th { } } } + +.dotgov-table--cell-padding-2 { + td, th { + padding: units(2); + } +} + +.usa-table--striped tbody tr:nth-child(odd) th, +.usa-table--striped tbody tr:nth-child(odd) td { + background-color: color('primary-lightest'); +} + +.usa-table--bg-transparent { + td, thead th { + background-color: transparent; + } +} + +.usa-table--full-borderless td, +.usa-table--full-borderless th { + border: none !important; +} diff --git a/src/registrar/assets/src/sass/_theme/_typography.scss b/src/registrar/assets/src/sass/_theme/_typography.scss index 5e00bf1b4..22069f726 100644 --- a/src/registrar/assets/src/sass/_theme/_typography.scss +++ b/src/registrar/assets/src/sass/_theme/_typography.scss @@ -11,7 +11,8 @@ address, } h1:not(.usa-alert__heading), -h2:not(.usa-alert__heading), +// .module h2 excludes headers in DJA +h2:not(.usa-alert__heading, .module h2), h3:not(.usa-alert__heading), h4:not(.usa-alert__heading), h5:not(.usa-alert__heading), @@ -45,3 +46,7 @@ h4, .h4 { padding-left: units(1); border-left: 2px solid color('base-lighter'); } + +.font-body-1 { + font-size: size('body', 1); +} diff --git a/src/registrar/fixtures/fixtures_users.py b/src/registrar/fixtures/fixtures_users.py index f1623e674..977bf0858 100644 --- a/src/registrar/fixtures/fixtures_users.py +++ b/src/registrar/fixtures/fixtures_users.py @@ -352,12 +352,37 @@ class UserFixture: @staticmethod def _get_existing_users(users): + # if users match existing users in db by email address, update the users with the username + # from the db. this will prevent duplicate users (with same email) from being added to db. + # it is ok to keep the old username in the db because the username will be updated by oidc process during login + + # Extract email addresses from users + emails = [user.get("email") for user in users] + + # Fetch existing users by email + existing_users_by_email = User.objects.filter(email__in=emails).values_list("email", "username", "id") + + # Create a dictionary to map emails to existing usernames + email_to_existing_user = {user[0]: user[1] for user in existing_users_by_email} + + # Update the users list with the usernames from existing users by email + for user in users: + email = user.get("email") + if email and email in email_to_existing_user: + user["username"] = email_to_existing_user[email] # Update username with the existing one + + # Get the user identifiers (username, id) for the existing users to query the database user_identifiers = [(user.get("username"), user.get("id")) for user in users] + + # Fetch existing users by username or id existing_users = User.objects.filter( username__in=[user[0] for user in user_identifiers] + [user[1] for user in user_identifiers] ).values_list("username", "id") + + # Create sets for usernames and ids that exist existing_usernames = set(user[0] for user in existing_users) existing_user_ids = set(user[1] for user in existing_users) + return existing_usernames, existing_user_ids @staticmethod diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py index e57b56c4f..c9ef280b0 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -4,7 +4,6 @@ import logging from django import forms from django.core.validators import RegexValidator from django.core.validators import MaxLengthValidator -from django.utils.safestring import mark_safe from registrar.forms.utility.combobox import ComboboxWidget from registrar.models import ( @@ -121,47 +120,47 @@ class BasePortfolioMemberForm(forms.ModelForm): widget=forms.RadioSelect, required=True, error_messages={ - "required": "Member access level is required", + "required": "Select the level of access you would like to grant this member.", }, ) - domain_request_permission_admin = forms.ChoiceField( - label=mark_safe(f"Select permission {required_star}"), # nosec + domain_permissions = forms.ChoiceField( choices=[ - (UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS.value, "View all requests"), - (UserPortfolioPermissionChoices.EDIT_REQUESTS.value, "View all requests plus create requests"), + (UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS.value, "Viewer, limited"), + (UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS.value, "Viewer, all"), ], widget=forms.RadioSelect, required=False, + initial=UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS.value, error_messages={ - "required": "Admin domain request permission is required", + "required": "Domain permission is required.", }, ) - member_permission_admin = forms.ChoiceField( - label=mark_safe(f"Select permission {required_star}"), # nosec + domain_request_permissions = forms.ChoiceField( choices=[ - (UserPortfolioPermissionChoices.VIEW_MEMBERS.value, "View all members"), - (UserPortfolioPermissionChoices.EDIT_MEMBERS.value, "View all members plus manage members"), - ], - widget=forms.RadioSelect, - required=False, - error_messages={ - "required": "Admin member permission is required", - }, - ) - - domain_request_permission_member = forms.ChoiceField( - label=mark_safe(f"Select permission {required_star}"), # nosec - choices=[ - (UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS.value, "View all requests"), - (UserPortfolioPermissionChoices.EDIT_REQUESTS.value, "View all requests plus create requests"), ("no_access", "No access"), + (UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS.value, "Viewer"), + (UserPortfolioPermissionChoices.EDIT_REQUESTS.value, "Creator"), ], widget=forms.RadioSelect, required=False, + initial="no_access", error_messages={ - "required": "Basic member permission is required", + "required": "Domain request permission is required.", + }, + ) + + member_permissions = forms.ChoiceField( + choices=[ + ("no_access", "No access"), + (UserPortfolioPermissionChoices.VIEW_MEMBERS.value, "Viewer"), + ], + widget=forms.RadioSelect, + required=False, + initial="no_access", + error_messages={ + "required": "Member permission is required.", }, ) @@ -169,12 +168,11 @@ class BasePortfolioMemberForm(forms.ModelForm): # All of the fields included here have "required=False" by default as they are conditionally required. # see def clean() for more details. ROLE_REQUIRED_FIELDS = { - UserPortfolioRoleChoices.ORGANIZATION_ADMIN: [ - "domain_request_permission_admin", - "member_permission_admin", - ], + UserPortfolioRoleChoices.ORGANIZATION_ADMIN: [], UserPortfolioRoleChoices.ORGANIZATION_MEMBER: [ - "domain_request_permission_member", + "domain_permissions", + "member_permissions", + "domain_request_permissions", ], } @@ -190,15 +188,24 @@ class BasePortfolioMemberForm(forms.ModelForm): Update field descriptions. """ super().__init__(*args, **kwargs) - # Adds a

description beneath each role option - self.fields["role"].descriptions = { - "organization_admin": UserPortfolioRoleChoices.get_role_description( - UserPortfolioRoleChoices.ORGANIZATION_ADMIN - ), - "organization_member": UserPortfolioRoleChoices.get_role_description( - UserPortfolioRoleChoices.ORGANIZATION_MEMBER - ), + + # Adds a

description beneath each option + self.fields["domain_permissions"].descriptions = { + UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS.value: "Can view only the domains they manage", + UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS.value: "Can view all domains for the organization", } + self.fields["domain_request_permissions"].descriptions = { + UserPortfolioPermissionChoices.EDIT_REQUESTS.value: ( + "Can view all domain requests for the organization and create requests" + ), + UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS.value: "Can view all domain requests for the organization", + "no_access": "Cannot view or create domain requests", + } + self.fields["member_permissions"].descriptions = { + UserPortfolioPermissionChoices.VIEW_MEMBERS.value: "Can view all members permissions", + "no_access": "Cannot view member permissions", + } + # Map model instance values to custom form fields if self.instance: self.map_instance_to_initial() @@ -222,8 +229,12 @@ class BasePortfolioMemberForm(forms.ModelForm): self.add_error(field_name, self.fields.get(field_name).error_messages.get("required")) # Edgecase: Member uses a special form value for None called "no_access". - if cleaned_data.get("domain_request_permission_member") == "no_access": - cleaned_data["domain_request_permission_member"] = None + if cleaned_data.get("domain_request_permissions") == "no_access": + cleaned_data["domain_request_permissions"] = None + + # Edgecase: Member uses a special form value for None called "no_access". + if cleaned_data.get("member_permissions") == "no_access": + cleaned_data["member_permissions"] = None # Handle roles cleaned_data["roles"] = [role] @@ -253,7 +264,7 @@ class BasePortfolioMemberForm(forms.ModelForm): "role": "organization_admin" or "organization_member", "member_permission_admin": permission level if admin, "domain_request_permission_admin": permission level if admin, - "domain_request_permission_member": permission level if member + "domain_request_permissions": permission level if member } """ if self.initial is None: @@ -267,12 +278,15 @@ class BasePortfolioMemberForm(forms.ModelForm): UserPortfolioRoleChoices.ORGANIZATION_ADMIN, UserPortfolioRoleChoices.ORGANIZATION_MEMBER, ] - domain_perms = [ + domain_request_perms = [ UserPortfolioPermissionChoices.EDIT_REQUESTS, UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, ] + domain_perms = [ + UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS, + UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, + ] member_perms = [ - UserPortfolioPermissionChoices.EDIT_MEMBERS, UserPortfolioPermissionChoices.VIEW_MEMBERS, ] @@ -282,16 +296,21 @@ class BasePortfolioMemberForm(forms.ModelForm): roles = self.instance.roles or [] selected_role = next((role for role in roles if role in roles), None) self.initial["role"] = selected_role - is_admin = selected_role == UserPortfolioRoleChoices.ORGANIZATION_ADMIN - if is_admin: - selected_domain_permission = next((perm for perm in domain_perms if perm in perms), None) - selected_member_permission = next((perm for perm in member_perms if perm in perms), None) - self.initial["domain_request_permission_admin"] = selected_domain_permission - self.initial["member_permission_admin"] = selected_member_permission - else: - # Edgecase: Member uses a special form value for None called "no_access". This ensures a form selection. - selected_domain_permission = next((perm for perm in domain_perms if perm in perms), "no_access") - self.initial["domain_request_permission_member"] = selected_domain_permission + is_member = selected_role == UserPortfolioRoleChoices.ORGANIZATION_MEMBER + if is_member: + # Edgecase: Member and domain request use a special form value for None called "no_access". + # This ensures a form selection. + selected_domain_permission = next( + (perm for perm in domain_perms if perm in perms), + UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS.value, + ) + selected_domain_request_permission = next( + (perm for perm in domain_request_perms if perm in perms), "no_access" + ) + selected_member_permission = next((perm for perm in member_perms if perm in perms), "no_access") + self.initial["domain_request_permissions"] = selected_domain_request_permission + self.initial["domain_permissions"] = selected_domain_permission + self.initial["member_permissions"] = selected_member_permission class PortfolioMemberForm(BasePortfolioMemberForm): @@ -320,7 +339,7 @@ class PortfolioNewMemberForm(BasePortfolioMemberForm): """ email = forms.EmailField( - label="Enter the email of the member you'd like to invite", + label="Email", max_length=None, error_messages={ "invalid": ("Enter an email address in the required format, like name@example.com."), diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index cb481db7a..0f0b3f112 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1582,11 +1582,9 @@ class Domain(TimeStampedModel, DomainHelper): if self.is_expired() and self.state != self.State.UNKNOWN: # Given expired is not a physical state, but it is displayed as such, # We need custom logic to determine this message. - help_text = ( - "This domain has expired, but it is still online. " "To renew this domain, contact help@get.gov." - ) + help_text = "This domain has expired. Complete the online renewal process to maintain access." elif flag_is_active(request, "domain_renewal") and self.is_expiring(): - help_text = "This domain will expire soon. Contact one of the listed domain managers to renew the domain." + help_text = "This domain is expiring soon. Complete the online renewal process to maintain access." else: help_text = Domain.State.get_help_text(self.state) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 2b5b56a78..1d508f88f 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -171,11 +171,14 @@ class User(AbstractUser): now = timezone.now().date() expiration_window = 60 threshold_date = now + timedelta(days=expiration_window) + acceptable_statuses = [Domain.State.UNKNOWN, Domain.State.DNS_NEEDED, Domain.State.READY] + num_of_expiring_domains = Domain.objects.filter( id__in=domain_ids, expiration_date__isnull=False, expiration_date__lte=threshold_date, expiration_date__gt=now, + state__in=acceptable_statuses, ).count() return num_of_expiring_domains diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index 03a01b80d..c4be90a9b 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -21,16 +21,18 @@ class UserPortfolioPermission(TimeStampedModel): UserPortfolioRoleChoices.ORGANIZATION_ADMIN: [ UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, + UserPortfolioPermissionChoices.EDIT_REQUESTS, UserPortfolioPermissionChoices.VIEW_MEMBERS, + UserPortfolioPermissionChoices.EDIT_MEMBERS, UserPortfolioPermissionChoices.VIEW_PORTFOLIO, UserPortfolioPermissionChoices.EDIT_PORTFOLIO, - # Domain: field specific permissions UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION, UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION, ], # NOTE: Check FORBIDDEN_PORTFOLIO_ROLE_PERMISSIONS before adding roles here. UserPortfolioRoleChoices.ORGANIZATION_MEMBER: [ UserPortfolioPermissionChoices.VIEW_PORTFOLIO, + UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION, ], } @@ -38,9 +40,9 @@ class UserPortfolioPermission(TimeStampedModel): # Used to throw a ValidationError on clean() for UserPortfolioPermission and PortfolioInvitation. FORBIDDEN_PORTFOLIO_ROLE_PERMISSIONS = { UserPortfolioRoleChoices.ORGANIZATION_MEMBER: [ - UserPortfolioPermissionChoices.VIEW_MEMBERS, + UserPortfolioPermissionChoices.EDIT_PORTFOLIO, UserPortfolioPermissionChoices.EDIT_MEMBERS, - UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, + UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION, ], } diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py index 4ae282f21..b3bb07c3d 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -25,23 +25,6 @@ class UserPortfolioRoleChoices(models.TextChoices): logger.warning(f"Invalid portfolio role: {user_portfolio_role}") return f"Unknown ({user_portfolio_role})" - @classmethod - def get_role_description(cls, user_portfolio_role): - """Returns a detailed description for a given role.""" - descriptions = { - cls.ORGANIZATION_ADMIN: ( - "Grants this member access to the organization-wide information " - "on domains, domain requests, and members. Domain management can be assigned separately." - ), - cls.ORGANIZATION_MEMBER: ( - "Grants this member access to the organization. They can be given extra permissions to view all " - "organization domain requests and submit domain requests on behalf of the organization. Basic access " - "members can’t view all members of an organization or manage them. " - "Domain management can be assigned separately." - ), - } - return descriptions.get(user_portfolio_role) - class UserPortfolioPermissionChoices(models.TextChoices): """ """ diff --git a/src/registrar/templates/django/forms/widgets/multiple_input.html b/src/registrar/templates/django/forms/widgets/multiple_input.html index cc0e11989..af98e898b 100644 --- a/src/registrar/templates/django/forms/widgets/multiple_input.html +++ b/src/registrar/templates/django/forms/widgets/multiple_input.html @@ -21,7 +21,7 @@ {% if field and field.field and field.field.descriptions %} {% with description=field.field.descriptions|get_dict_value:option.value %} {% if description %} -

{{ description }}

+

{{ description }}

{% endif %} {% endwith %} {% endif %} diff --git a/src/registrar/templates/domain_base.html b/src/registrar/templates/domain_base.html index 165441c91..58038d0a4 100644 --- a/src/registrar/templates/domain_base.html +++ b/src/registrar/templates/domain_base.html @@ -46,7 +46,7 @@ {# messages block is under the back breadcrumb link #} {% if messages %} {% for message in messages %} -
+
{{ message }}
diff --git a/src/registrar/templates/domain_detail.html b/src/registrar/templates/domain_detail.html index 1d34ef4e4..03df2d59c 100644 --- a/src/registrar/templates/domain_detail.html +++ b/src/registrar/templates/domain_detail.html @@ -49,11 +49,11 @@ {% if has_domain_renewal_flag and domain.is_expired and is_domain_manager %} This domain has expired, but it is still online. {% url 'domain-renewal' pk=domain.id as url %} - Renew to maintain access. + Renew to maintain access. {% elif has_domain_renewal_flag and domain.is_expiring and is_domain_manager %} This domain will expire soon. {% url 'domain-renewal' pk=domain.id as url %} - Renew to maintain access. + Renew to maintain access. {% elif has_domain_renewal_flag and domain.is_expiring and is_portfolio_user %} This domain will expire soon. Contact one of the listed domain managers to renew the domain. {% elif has_domain_renewal_flag and domain.is_expired and is_portfolio_user %} diff --git a/src/registrar/templates/domain_renewal.html b/src/registrar/templates/domain_renewal.html index 30e1be0e4..703c2358f 100644 --- a/src/registrar/templates/domain_renewal.html +++ b/src/registrar/templates/domain_renewal.html @@ -38,11 +38,11 @@ {{ block.super }}

Confirm the following information for accuracy

-

Review these details below. We +

Review the details below. We require that you maintain accurate information for the domain. The details you provide will only be used to support the administration of .gov and won't be made public.

-

If you would like to retire your domain instead, please +

If you would like to retire your domain instead, please contact us.

Required fields are marked with an asterisk (*).

@@ -98,7 +98,7 @@ {% if form.is_policy_acknowledged.errors %} {% for error in form.is_policy_acknowledged.errors %} @@ -131,7 +129,7 @@ name="submit_button" value="next" class="usa-button margin-top-3" - > Submit + > Submit and renew diff --git a/src/registrar/templates/domain_users.html b/src/registrar/templates/domain_users.html index 8f26a7f98..c6e6baa93 100644 --- a/src/registrar/templates/domain_users.html +++ b/src/registrar/templates/domain_users.html @@ -69,7 +69,6 @@ {% if not portfolio %}{{ item.permission.role|title }}{% endif %} - {% if can_delete_users %} Remove @@ -112,18 +112,6 @@ {% csrf_token %} {% endif %} - {% else %} - - {% endif %} {% endfor %} diff --git a/src/registrar/templates/includes/domain_dates.html b/src/registrar/templates/includes/domain_dates.html index b14c091d0..339d75c44 100644 --- a/src/registrar/templates/includes/domain_dates.html +++ b/src/registrar/templates/includes/domain_dates.html @@ -1,7 +1,7 @@ {% if domain.expiration_date or domain.created_at %}

{% if domain.expiration_date %} - Expires: + Date of expiration: {{ domain.expiration_date|date }} {% if domain.is_expired %} (expired){% endif %}
diff --git a/src/registrar/templates/includes/domains_table.html b/src/registrar/templates/includes/domains_table.html index c7c8a2d8e..de3d15eb0 100644 --- a/src/registrar/templates/includes/domains_table.html +++ b/src/registrar/templates/includes/domains_table.html @@ -10,14 +10,14 @@ {% if has_domain_renewal_flag and num_expiring_domains > 0 and has_any_domains_portfolio_permission %} -

+
-
+

{% if num_expiring_domains == 1%} - One domain will expire soon. Go to "Manage" to renew the domain. Show expiring domain. + One domain will expire soon. Go to "Manage" to renew the domain. Show expiring domain. {% else%} - Multiple domains will expire soon. Go to "Manage" to renew the domains. Show expiring domains. + Multiple domains will expire soon. Go to "Manage" to renew the domains. Show expiring domains. {% endif %}

@@ -76,14 +76,14 @@ {% if has_domain_renewal_flag and num_expiring_domains > 0 and not portfolio %} -
+
-
+

{% if num_expiring_domains == 1%} - One domain will expire soon. Go to "Manage" to renew the domain. Show expiring domain. + One domain will expire soon. Go to "Manage" to renew the domain. Show expiring domain. {% else%} - Multiple domains will expire soon. Go to "Manage" to renew the domains. Show expiring domains. + Multiple domains will expire soon. Go to "Manage" to renew the domains. Show expiring domains. {% endif %}

diff --git a/src/registrar/templates/includes/member_basic_permissions.html b/src/registrar/templates/includes/member_basic_permissions.html new file mode 100644 index 000000000..c84f3de09 --- /dev/null +++ b/src/registrar/templates/includes/member_basic_permissions.html @@ -0,0 +1,20 @@ +{% load field_helpers %} +
+

What permissions do you want to add?

+

Configure the permissions for this member. Basic members cannot manage member permissions or organization metadata.

+ +

Domains *

+ {% with group_classes="bg-gray-1 border-bottom-2px border-base-lighter padding-bottom-2 margin-top-0" add_legend_class="usa-sr-only" %} + {% input_with_errors form.domain_permissions %} + {% endwith %} + +

Domain requests *

+ {% with group_classes="bg-gray-1 border-bottom-2px border-base-lighter padding-bottom-2 margin-top-0" add_legend_class="usa-sr-only" %} + {% input_with_errors form.domain_request_permissions %} + {% endwith %} + +

Members *

+ {% with group_classes="bg-gray-1 border-bottom-2px border-base-lighter padding-bottom-2 margin-top-0" add_legend_class="usa-sr-only" %} + {% input_with_errors form.member_permissions %} + {% endwith %} +
diff --git a/src/registrar/templates/includes/member_permissions_matrix.html b/src/registrar/templates/includes/member_permissions_matrix.html new file mode 100644 index 000000000..6c768633f --- /dev/null +++ b/src/registrar/templates/includes/member_permissions_matrix.html @@ -0,0 +1,131 @@ +
+

+ +

+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
Member actions availableAdminBasic
+ View domains they manage + + + + + + + +
View all domains for the organization + + + Optional +
View all domain requests + + + Optional +
Create domain requests + + + Optional +
View all member permissions + + + Optional +
Manage member permissions + + + +
Manage organization metadata (address) + + + +
+
+
diff --git a/src/registrar/templates/includes/member_permissions.html b/src/registrar/templates/includes/member_permissions_summary.html similarity index 51% rename from src/registrar/templates/includes/member_permissions.html rename to src/registrar/templates/includes/member_permissions_summary.html index 4833b5e4b..3a91d16f6 100644 --- a/src/registrar/templates/includes/member_permissions.html +++ b/src/registrar/templates/includes/member_permissions_summary.html @@ -1,26 +1,33 @@

Member access

{% if permissions.roles and 'organization_admin' in permissions.roles %} -

Admin access

+

Admin

{% elif permissions.roles and 'organization_member' in permissions.roles %} -

Basic access

+

Basic

{% else %}

⎯

{% endif %} -

Organization domain requests

+

Domains

+{% if member_has_view_all_domains_portfolio_permission %} +

Viewer, all

+{% else %} +

Viewer, limited

+{% endif %} + +

Domain requests

{% if member_has_edit_request_portfolio_permission %} -

View all requests plus create requests

+

Creator

{% elif member_has_view_all_requests_portfolio_permission %} -

View all requests

+

Viewer

{% else %}

No access

{% endif %} -

Organization members

+

Members

{% if member_has_edit_members_portfolio_permission %} -

View all members plus manage members

+

Manager

{% elif member_has_view_members_portfolio_permission %} -

View all members

+

Viewer

{% else %}

No access

{% endif %} \ No newline at end of file diff --git a/src/registrar/templates/includes/summary_item.html b/src/registrar/templates/includes/summary_item.html index f4975c3dc..26e56fea7 100644 --- a/src/registrar/templates/includes/summary_item.html +++ b/src/registrar/templates/includes/summary_item.html @@ -22,7 +22,7 @@

{{ sub_header_text }}

{% endif %} {% if permissions %} - {% include "includes/member_permissions.html" with permissions=value %} + {% include "includes/member_permissions_summary.html" with permissions=value %} {% elif domain_mgmt %} {% include "includes/member_domain_management.html" with domain_count=value %} {% elif address %} diff --git a/src/registrar/templates/portfolio_member_permissions.html b/src/registrar/templates/portfolio_member_permissions.html index fb50172f0..2bef5b7af 100644 --- a/src/registrar/templates/portfolio_member_permissions.html +++ b/src/registrar/templates/portfolio_member_permissions.html @@ -89,35 +89,10 @@ - -
-

Admin access permissions

-

Member permissions available for admin-level acccess.

+ {% include "includes/member_permissions_matrix.html" %} -

Organization domain requests

- {% with group_classes="usa-form-editable usa-form-editable--no-border bg-gray-1 padding-top-0" %} - {% input_with_errors form.domain_request_permission_admin %} - {% endwith %} - -

Organization members

- {% with group_classes="usa-form-editable usa-form-editable--no-border bg-gray-1 padding-top-0" %} - {% input_with_errors form.member_permission_admin %} - {% endwith %} -
- - -
-

Basic member permissions

-

Member permissions available for basic-level acccess.

- -

Organization domain requests

- {% with group_classes="usa-form-editable usa-form-editable--no-border bg-gray-1 padding-top-0" %} - {% input_with_errors form.domain_request_permission_member %} - {% endwith %} -
+ + {% include "includes/member_basic_permissions.html" %}
diff --git a/src/registrar/templates/portfolio_members_add_new.html b/src/registrar/templates/portfolio_members_add_new.html index ea96cc0cf..464eaefce 100644 --- a/src/registrar/templates/portfolio_members_add_new.html +++ b/src/registrar/templates/portfolio_members_add_new.html @@ -30,88 +30,67 @@ - {% block new_member_header %}

Add a new member

- {% endblock new_member_header %} + +

After adding a new member, an email invitation will be sent to that user with instructions on how to set up an account. All members must keep their contact information updated and be responsive if contacted by the .gov team.

{% include "includes/required_fields.html" %}
+ {% csrf_token %} -
- -

Email

- - - {% csrf_token %} +
+ +

Who would you like to add to the organization?

+ + {% with group_classes="usa-form-editable usa-form-editable--no-border padding-top-0" %} {% input_with_errors form.email %} {% endwith %} -
+
- -
- -

Member Access

- + +
+ +

What level of access would you like to grant this member?

+ - Select the level of access for this member. * +

Select one *

- {% with add_class="usa-radio__input--tile" add_legend_class="usa-sr-only" %} - {% input_with_errors form.role %} - {% endwith %} + {% with add_class="usa-radio__input--tile" add_legend_class="usa-sr-only" %} + {% input_with_errors form.role %} + {% endwith %} +
-
+ {% include "includes/member_permissions_matrix.html" %} - -
-

Admin access permissions

-

Member permissions available for admin-level acccess.

+ + {% include "includes/member_basic_permissions.html" %} + +

Domain management

-

Organization domain requests

- {% with group_classes="usa-form-editable usa-form-editable--no-border bg-gray-1 padding-top-0" %} - {% input_with_errors form.domain_request_permission_admin %} - {% endwith %} +

After you invite this person to your organization, you can assign domain management permissions on their member profile.

-

Organization members

- {% with group_classes="usa-form-editable usa-form-editable--no-border bg-gray-1 padding-top-0" %} - {% input_with_errors form.member_permission_admin %} - {% endwith %} -
+ +
+ Cancel + + + +
- -
-

Basic member permissions

-

Member permissions available for basic-level acccess.

- -

Organization domain requests

- {% with group_classes="usa-form-editable usa-form-editable--no-border bg-gray-1 padding-top-0" %} - {% input_with_errors form.domain_request_permission_member %} - {% endwith %} -
- - -
- Cancel - - - -
1 - - context["can_delete_users"] = can_delete_users - return context - class DomainAddUserView(DomainFormBaseView): """Inside of a domain's user management, a form for adding users. @@ -1310,7 +1321,7 @@ class DomainDeleteUserView(UserDomainRolePermissionDeleteView): """Refreshes the page after a delete is successful""" return reverse("domain-users", kwargs={"pk": self.object.domain.id}) - def get_success_message(self, delete_self=False): + def get_success_message(self): """Returns confirmation content for the deletion event""" # Grab the text representation of the user we want to delete @@ -1320,7 +1331,7 @@ class DomainDeleteUserView(UserDomainRolePermissionDeleteView): # If the user is deleting themselves, return a specific message. # If not, return something more generic. - if delete_self: + if self.delete_self: message = f"You are no longer managing the domain {self.object.domain}." else: message = f"Removed {email_or_name} as a manager for this domain." @@ -1333,22 +1344,35 @@ class DomainDeleteUserView(UserDomainRolePermissionDeleteView): # Delete the object super().form_valid(form) - # Is the user deleting themselves? If so, display a different message - delete_self = self.request.user == self.object.user - - # Email domain managers - # Add a success message - messages.success(self.request, self.get_success_message(delete_self)) + messages.success(self.request, self.get_success_message()) return redirect(self.get_success_url()) def post(self, request, *args, **kwargs): - """Custom post implementation to redirect to home in the event that the user deletes themselves""" + """Custom post implementation to ensure last userdomainrole is not removed and to + redirect to home in the event that the user deletes themselves""" + self.object = self.get_object() # Retrieve the UserDomainRole to delete + + # Is the user deleting themselves? + self.delete_self = self.request.user == self.object.user + + # Check if this is the only UserDomainRole for the domain + if not len(UserDomainRole.objects.filter(domain=self.object.domain)) > 1: + if self.delete_self: + messages.error( + request, + "Domains must have at least one domain manager. " + "To remove yourself, the domain needs another domain manager.", + ) + else: + messages.error(request, "Domains must have at least one domain manager.") + return redirect(self.get_success_url()) + + # normal delete processing in the event that the above condition not reached response = super().post(request, *args, **kwargs) # If the user is deleting themselves, redirect to home - delete_self = self.request.user == self.object.user - if delete_self: + if self.delete_self: return redirect(reverse("home")) return response diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index 5bc16a396..f5db165d1 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -82,6 +82,9 @@ class PortfolioMemberView(PortfolioMemberPermissionView, View): member_has_edit_members_portfolio_permission = member.has_edit_members_portfolio_permission( portfolio_permission.portfolio ) + member_has_view_all_domains_portfolio_permission = member.has_view_all_domains_portfolio_permission( + portfolio_permission.portfolio + ) return render( request, @@ -95,6 +98,7 @@ class PortfolioMemberView(PortfolioMemberPermissionView, View): "member_has_edit_request_portfolio_permission": member_has_edit_request_portfolio_permission, "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, }, ) @@ -346,6 +350,9 @@ class PortfolioInvitedMemberView(PortfolioMemberPermissionView, View): member_has_edit_members_portfolio_permission = ( UserPortfolioPermissionChoices.EDIT_MEMBERS in portfolio_invitation.get_portfolio_permissions() ) + member_has_view_all_domains_portfolio_permission = ( + UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS in portfolio_invitation.get_portfolio_permissions() + ) return render( request, @@ -358,6 +365,7 @@ class PortfolioInvitedMemberView(PortfolioMemberPermissionView, View): "member_has_edit_request_portfolio_permission": member_has_edit_request_portfolio_permission, "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, }, ) diff --git a/src/registrar/views/utility/mixins.py b/src/registrar/views/utility/mixins.py index 11384ca09..08212088b 100644 --- a/src/registrar/views/utility/mixins.py +++ b/src/registrar/views/utility/mixins.py @@ -192,7 +192,8 @@ class DomainPermission(PermissionsLoginMixin): def can_access_domain_via_portfolio(self, pk): """Most views should not allow permission to portfolio users. If particular views allow access to the domain pages, they will need to override - this function.""" + this function. + """ return False def in_editable_state(self, pk): @@ -343,12 +344,6 @@ class UserDeleteDomainRolePermission(PermissionsLoginMixin): if not (has_delete_permission or user_is_analyst_or_superuser): return False - # Check if more than one manager exists on the domain. - # If only one exists, prevent this from happening - has_multiple_managers = len(UserDomainRole.objects.filter(domain=domain_pk)) > 1 - if not has_multiple_managers: - return False - return True