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/admin.py b/src/registrar/admin.py index e5c5cafe2..928ead442 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1333,6 +1333,14 @@ class UserPortfolioPermissionAdmin(ListHeaderAdmin): get_roles.short_description = "Roles" # type: ignore + def delete_queryset(self, request, queryset): + """We override the delete method in the model. + When deleting in DJA, if you select multiple items in a table using checkboxes and apply a delete action + the model delete does not get called. This method gets called instead. + This override makes sure our code in the model gets executed in these situations.""" + for obj in queryset: + obj.delete() # Calls the overridden delete method on each instance + class UserDomainRoleAdmin(ListHeaderAdmin, ImportExportModelAdmin): """Custom user domain role admin class.""" @@ -1694,6 +1702,14 @@ class PortfolioInvitationAdmin(BaseInvitationAdmin): # Call the parent save method to save the object super().save_model(request, obj, form, change) + def delete_queryset(self, request, queryset): + """We override the delete method in the model. + When deleting in DJA, if you select multiple items in a table using checkboxes and apply a delete action, + the model delete does not get called. This method gets called instead. + This override makes sure our code in the model gets executed in these situations.""" + for obj in queryset: + obj.delete() # Calls the overridden delete method on each instance + class DomainInformationResource(resources.ModelResource): """defines how each field in the referenced model should be mapped to the corresponding fields in the diff --git a/src/registrar/assets/src/js/getgov/table-base.js b/src/registrar/assets/src/js/getgov/table-base.js index 0abfee9b6..ce4397887 100644 --- a/src/registrar/assets/src/js/getgov/table-base.js +++ b/src/registrar/assets/src/js/getgov/table-base.js @@ -129,7 +129,7 @@ export class BaseTable { this.displayName = itemName; this.sectionSelector = itemName + 's'; this.tableWrapper = document.getElementById(`${this.sectionSelector}__table-wrapper`); - this.tableHeaders = document.querySelectorAll(`#${this.sectionSelector} th[data-sortable]`); + this.tableHeaderSortButtons = document.querySelectorAll(`#${this.sectionSelector} th[data-sortable] button`); this.currentSortBy = 'id'; this.currentOrder = 'asc'; this.currentStatus = []; @@ -303,13 +303,18 @@ export class BaseTable { * A helper that resets sortable table headers * */ - unsetHeader = (header) => { - header.removeAttribute('aria-sort'); - let headerName = header.innerText; - const headerLabel = `${headerName}, sortable column, currently unsorted"`; - const headerButtonLabel = `Click to sort by ascending order.`; - header.setAttribute("aria-label", headerLabel); - header.querySelector('.usa-table__header__button').setAttribute("title", headerButtonLabel); + unsetHeader = (headerSortButton) => { + let header = headerSortButton.closest('th'); + if (header) { + header.removeAttribute('aria-sort'); + let headerName = header.innerText; + const headerLabel = `${headerName}, sortable column, currently unsorted"`; + const headerButtonLabel = `Click to sort by ascending order.`; + header.setAttribute("aria-label", headerLabel); + header.querySelector('.usa-table__header__button').setAttribute("title", headerButtonLabel); + } else { + console.warn('Issue with DOM'); + } }; /** @@ -505,24 +510,21 @@ export class BaseTable { // Add event listeners to table headers for sorting initializeTableHeaders() { - this.tableHeaders.forEach(header => { - header.addEventListener('click', event => { - let button = header.querySelector('.usa-table__header__button') - const sortBy = header.getAttribute('data-sortable'); - let order = 'asc'; - // sort order will be ascending, unless the currently sorted column is ascending, and the user - // is selecting the same column to sort in descending order - if (sortBy === this.currentSortBy) { - order = this.currentOrder === 'asc' ? 'desc' : 'asc'; - } - // load the results with the updated sort - this.loadTable(1, sortBy, order); - // If the click occurs outside of the button, need to simulate a button click in order - // for USWDS listener on the button to execute. - // Check first to see if click occurs outside of the button - if (!button.contains(event.target)) { - // Simulate a button click - button.click(); + this.tableHeaderSortButtons.forEach(tableHeader => { + tableHeader.addEventListener('click', event => { + let header = tableHeader.closest('th'); + if (header) { + const sortBy = header.getAttribute('data-sortable'); + let order = 'asc'; + // sort order will be ascending, unless the currently sorted column is ascending, and the user + // is selecting the same column to sort in descending order + if (sortBy === this.currentSortBy) { + order = this.currentOrder === 'asc' ? 'desc' : 'asc'; + } + // load the results with the updated sort + this.loadTable(1, sortBy, order); + } else { + console.warn('Issue with DOM'); } }); }); @@ -587,9 +589,9 @@ export class BaseTable { // Reset UI and accessibility resetHeaders() { - this.tableHeaders.forEach(header => { + this.tableHeaderSortButtons.forEach(headerSortButton => { // Unset sort UI in headers - this.unsetHeader(header); + this.unsetHeader(headerSortButton); }); // Reset the announcement region this.tableAnnouncementRegion.innerHTML = ''; diff --git a/src/registrar/assets/src/js/getgov/table-member-domains.js b/src/registrar/assets/src/js/getgov/table-member-domains.js index f9b789e1f..d1455c4dc 100644 --- a/src/registrar/assets/src/js/getgov/table-member-domains.js +++ b/src/registrar/assets/src/js/getgov/table-member-domains.js @@ -35,16 +35,19 @@ export class MemberDomainsTable extends BaseTable { showElement(dataWrapper); hideElement(noSearchResultsWrapper); hideElement(noDataWrapper); + this.tableAnnouncementRegion.innerHTML = ''; } else { hideElement(dataWrapper); showElement(noSearchResultsWrapper); hideElement(noDataWrapper); + this.tableAnnouncementRegion.innerHTML = this.noSearchResultsWrapper.innerHTML; } } else { hideElement(searchSection); hideElement(dataWrapper); hideElement(noSearchResultsWrapper); showElement(noDataWrapper); + this.tableAnnouncementRegion.innerHTML = this.noDataWrapper.innerHTML; } }; } diff --git a/src/registrar/assets/src/sass/_theme/_tooltips.scss b/src/registrar/assets/src/sass/_theme/_tooltips.scss index 22b5cf534..e1e31cbec 100644 --- a/src/registrar/assets/src/sass/_theme/_tooltips.scss +++ b/src/registrar/assets/src/sass/_theme/_tooltips.scss @@ -29,7 +29,7 @@ font-weight: 400 !important; } -.domains__table { +.domains__table, .usa-table { /* Trick tooltips in the domains table to do 2 things... 1 - Shrink itself to a padded viewport window diff --git a/src/registrar/assets/src/sass/_theme/_typography.scss b/src/registrar/assets/src/sass/_theme/_typography.scss index 75d5170e8..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), diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index a58e3e2f9..58250e85c 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -25,6 +25,7 @@ from typing import Final from botocore.config import Config import json import logging +import traceback from django.utils.log import ServerFormatter # # # ### @@ -471,7 +472,11 @@ class JsonFormatter(logging.Formatter): "lineno": record.lineno, "message": record.getMessage(), } - return json.dumps(log_record) + # Capture exception info if it exists + if record.exc_info: + log_record["exception"] = "".join(traceback.format_exception(*record.exc_info)) + + return json.dumps(log_record, ensure_ascii=False) class JsonServerFormatter(ServerFormatter): 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/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/portfolio_invitation.py b/src/registrar/models/portfolio_invitation.py index 11c564c36..8feeb0794 100644 --- a/src/registrar/models/portfolio_invitation.py +++ b/src/registrar/models/portfolio_invitation.py @@ -8,6 +8,7 @@ from registrar.models import DomainInvitation, UserPortfolioPermission from .utility.portfolio_helper import ( UserPortfolioPermissionChoices, UserPortfolioRoleChoices, + cleanup_after_portfolio_member_deletion, validate_portfolio_invitation, ) # type: ignore from .utility.time_stamped_model import TimeStampedModel @@ -115,3 +116,27 @@ class PortfolioInvitation(TimeStampedModel): """Extends clean method to perform additional validation, which can raise errors in django admin.""" super().clean() validate_portfolio_invitation(self) + + def delete(self, *args, **kwargs): + + User = get_user_model() + + email = self.email # Capture the email before the instance is deleted + portfolio = self.portfolio # Capture the portfolio before the instance is deleted + + # Call the superclass delete method to actually delete the instance + super().delete(*args, **kwargs) + + if self.status == self.PortfolioInvitationStatus.INVITED: + + # Query the user by email + users = User.objects.filter(email=email) + + if users.count() > 1: + # This should never happen, log an error if more than one object is returned + logger.error(f"Multiple users found with the same email: {email}") + + # Retrieve the first user, or None if no users are found + user = users.first() + + cleanup_after_portfolio_member_deletion(portfolio=portfolio, email=email, user=user) 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 c4be90a9b..11d9c56e3 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -5,6 +5,7 @@ from registrar.models.utility.portfolio_helper import ( UserPortfolioRoleChoices, DomainRequestPermissionDisplay, MemberPermissionDisplay, + cleanup_after_portfolio_member_deletion, validate_user_portfolio_permission, ) from .utility.time_stamped_model import TimeStampedModel @@ -188,3 +189,13 @@ class UserPortfolioPermission(TimeStampedModel): """Extends clean method to perform additional validation, which can raise errors in django admin.""" super().clean() validate_user_portfolio_permission(self) + + def delete(self, *args, **kwargs): + + user = self.user # Capture the user before the instance is deleted + portfolio = self.portfolio # Capture the portfolio before the instance is deleted + + # Call the superclass delete method to actually delete the instance + super().delete(*args, **kwargs) + + cleanup_after_portfolio_member_deletion(portfolio=portfolio, email=user.email, user=user) diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py index 49c2cc1dc..0864bded0 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -212,3 +212,32 @@ def validate_portfolio_invitation(portfolio_invitation): "This user is already assigned to a portfolio invitation. " "Based on current waffle flag settings, users cannot be assigned to multiple portfolios." ) + + +def cleanup_after_portfolio_member_deletion(portfolio, email, user=None): + """ + Cleans up after removing a portfolio member or a portfolio invitation. + + Args: + portfolio: portfolio + user: passed when removing a portfolio member. + email: passed when removing a portfolio invitation, or passed as user.email + when removing a portfolio member. + """ + + DomainInvitation = apps.get_model("registrar.DomainInvitation") + UserDomainRole = apps.get_model("registrar.UserDomainRole") + + # Fetch domain invitations matching the criteria + invitations = DomainInvitation.objects.filter( + email=email, domain__domain_info__portfolio=portfolio, status=DomainInvitation.DomainInvitationStatus.INVITED + ) + + # Call `cancel_invitation` on each invitation + for invitation in invitations: + invitation.cancel_invitation() + invitation.save() + + if user: + # Remove user's domain roles for the current portfolio + UserDomainRole.objects.filter(user=user, domain__domain_info__portfolio=portfolio).delete() 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/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/domain_requests_table.html b/src/registrar/templates/includes/domain_requests_table.html index b026a7a6b..e411f1494 100644 --- a/src/registrar/templates/includes/domain_requests_table.html +++ b/src/registrar/templates/includes/domain_requests_table.html @@ -57,7 +57,7 @@ - {% 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 %}

@@ -64,7 +64,7 @@ {% if user_domain_count and user_domain_count > 0 %}
- + Export as CSV @@ -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 %}

@@ -101,6 +101,7 @@ id="domains__usa-button--filter" aria-expanded="false" aria-controls="filter-status" + aria-label="Status, list 5 items" > Status
- + Export as CSV diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index ef811e083..157420be7 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -164,6 +164,7 @@ class TestPortfolioInvitations(TestCase): DomainInformation.objects.all().delete() Domain.objects.all().delete() UserPortfolioPermission.objects.all().delete() + UserDomainRole.objects.all().delete() Portfolio.objects.all().delete() PortfolioInvitation.objects.all().delete() User.objects.all().delete() @@ -442,6 +443,294 @@ class TestPortfolioInvitations(TestCase): pass + @less_console_noise_decorator + def test_delete_portfolio_invitation_deletes_portfolio_domain_invitations(self): + """Deleting a portfolio invitation causes domain invitations for the same email on the same + portfolio to be canceled.""" + + email_with_no_user = "email-with-no-user@email.gov" + + domain_in_portfolio_1, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_1.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_1, portfolio=self.portfolio + ) + invite_1, _ = DomainInvitation.objects.get_or_create(email=email_with_no_user, domain=domain_in_portfolio_1) + + domain_in_portfolio_2, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_and_invited_2.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_2, portfolio=self.portfolio + ) + invite_2, _ = DomainInvitation.objects.get_or_create(email=email_with_no_user, domain=domain_in_portfolio_2) + + domain_not_in_portfolio, _ = Domain.objects.get_or_create( + name="domain_not_in_portfolio.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create(creator=self.user, domain=domain_not_in_portfolio) + invite_3, _ = DomainInvitation.objects.get_or_create(email=email_with_no_user, domain=domain_not_in_portfolio) + + invitation_of_email_with_no_user, _ = PortfolioInvitation.objects.get_or_create( + email=email_with_no_user, + portfolio=self.portfolio, + roles=[self.portfolio_role_base, self.portfolio_role_admin], + additional_permissions=[self.portfolio_permission_1, self.portfolio_permission_2], + ) + + # The domain invitations start off as INVITED + self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED) + + # Delete member (invite) + invitation_of_email_with_no_user.delete() + + # Reload the objects from the database + invite_1 = DomainInvitation.objects.get(pk=invite_1.pk) + invite_2 = DomainInvitation.objects.get(pk=invite_2.pk) + invite_3 = DomainInvitation.objects.get(pk=invite_3.pk) + + # The domain invitations to the portfolio domains have been canceled + self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.CANCELED) + self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.CANCELED) + + # Invite 3 is unaffected + self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED) + + @less_console_noise_decorator + def test_deleting_a_retrieved_invitation_has_no_side_effects(self): + """Deleting a retrieved portfolio invitation causes no side effects.""" + + domain_in_portfolio_1, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_1.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_1, portfolio=self.portfolio + ) + invite_1, _ = DomainInvitation.objects.get_or_create(email=self.email, domain=domain_in_portfolio_1) + + domain_in_portfolio_2, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_and_invited_2.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_2, portfolio=self.portfolio + ) + invite_2, _ = DomainInvitation.objects.get_or_create(email=self.email, domain=domain_in_portfolio_2) + + domain_in_portfolio_3, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_3.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_3, portfolio=self.portfolio + ) + UserDomainRole.objects.get_or_create( + user=self.user, domain=domain_in_portfolio_3, role=UserDomainRole.Roles.MANAGER + ) + + domain_in_portfolio_4, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_and_invited_4.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_4, portfolio=self.portfolio + ) + UserDomainRole.objects.get_or_create( + user=self.user, domain=domain_in_portfolio_4, role=UserDomainRole.Roles.MANAGER + ) + + domain_not_in_portfolio_1, _ = Domain.objects.get_or_create( + name="domain_not_in_portfolio.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create(creator=self.user, domain=domain_not_in_portfolio_1) + invite_3, _ = DomainInvitation.objects.get_or_create(email=self.email, domain=domain_not_in_portfolio_1) + + domain_not_in_portfolio_2, _ = Domain.objects.get_or_create( + name="domain_not_in_portfolio_2.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create(creator=self.user, domain=domain_not_in_portfolio_2) + UserDomainRole.objects.get_or_create( + user=self.user, domain=domain_not_in_portfolio_2, role=UserDomainRole.Roles.MANAGER + ) + + # The domain invitations start off as INVITED + self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED) + + # The user domain roles exist + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_3, + ).exists() + ) + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_4, + ).exists() + ) + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_not_in_portfolio_2, + ).exists() + ) + + # retrieve the invitation + self.invitation.retrieve() + self.invitation.save() + + # Delete member (invite) + self.invitation.delete() + + # Reload the objects from the database + invite_1 = DomainInvitation.objects.get(pk=invite_1.pk) + invite_2 = DomainInvitation.objects.get(pk=invite_2.pk) + invite_3 = DomainInvitation.objects.get(pk=invite_3.pk) + + # Test that no side effects have been triggered + self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_3, + ).exists() + ) + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_4, + ).exists() + ) + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_not_in_portfolio_2, + ).exists() + ) + + @less_console_noise_decorator + def test_delete_portfolio_invitation_deletes_user_domain_roles(self): + """Deleting a portfolio invitation causes domain invitations for the same email on the same + portfolio to be canceled, also deletes any exiting user domain roles on the portfolio for the + user if the user exists.""" + + domain_in_portfolio_1, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_1.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_1, portfolio=self.portfolio + ) + invite_1, _ = DomainInvitation.objects.get_or_create(email=self.email, domain=domain_in_portfolio_1) + + domain_in_portfolio_2, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_and_invited_2.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_2, portfolio=self.portfolio + ) + invite_2, _ = DomainInvitation.objects.get_or_create(email=self.email, domain=domain_in_portfolio_2) + + domain_in_portfolio_3, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_3.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_3, portfolio=self.portfolio + ) + UserDomainRole.objects.get_or_create( + user=self.user, domain=domain_in_portfolio_3, role=UserDomainRole.Roles.MANAGER + ) + + domain_in_portfolio_4, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_and_invited_4.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_4, portfolio=self.portfolio + ) + UserDomainRole.objects.get_or_create( + user=self.user, domain=domain_in_portfolio_4, role=UserDomainRole.Roles.MANAGER + ) + + domain_not_in_portfolio_1, _ = Domain.objects.get_or_create( + name="domain_not_in_portfolio.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create(creator=self.user, domain=domain_not_in_portfolio_1) + invite_3, _ = DomainInvitation.objects.get_or_create(email=self.email, domain=domain_not_in_portfolio_1) + + domain_not_in_portfolio_2, _ = Domain.objects.get_or_create( + name="domain_not_in_portfolio_2.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create(creator=self.user, domain=domain_not_in_portfolio_2) + UserDomainRole.objects.get_or_create( + user=self.user, domain=domain_not_in_portfolio_2, role=UserDomainRole.Roles.MANAGER + ) + + # The domain invitations start off as INVITED + self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED) + + # The user domain roles exist + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_3, + ).exists() + ) + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_4, + ).exists() + ) + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_not_in_portfolio_2, + ).exists() + ) + + # Delete member (invite) + self.invitation.delete() + + # Reload the objects from the database + invite_1 = DomainInvitation.objects.get(pk=invite_1.pk) + invite_2 = DomainInvitation.objects.get(pk=invite_2.pk) + invite_3 = DomainInvitation.objects.get(pk=invite_3.pk) + + # The domain invitations to the portfolio domains have been canceled + self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.CANCELED) + self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.CANCELED) + + # Invite 3 is unaffected + self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED) + + # The user domain roles have been deleted for the domains in portfolio + self.assertFalse( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_3, + ).exists() + ) + self.assertFalse( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_4, + ).exists() + ) + + # The user domain role on the domain not in portfolio still exists + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_not_in_portfolio_2, + ).exists() + ) + class TestUserPortfolioPermission(TestCase): @less_console_noise_decorator @@ -457,6 +746,7 @@ class TestUserPortfolioPermission(TestCase): Domain.objects.all().delete() DomainInformation.objects.all().delete() DomainRequest.objects.all().delete() + DomainInvitation.objects.all().delete() UserPortfolioPermission.objects.all().delete() Portfolio.objects.all().delete() User.objects.all().delete() @@ -750,6 +1040,129 @@ class TestUserPortfolioPermission(TestCase): # Should return the forbidden permissions for member role self.assertEqual(member_only_permissions, set(member_forbidden)) + @less_console_noise_decorator + def test_delete_portfolio_permission_deletes_user_domain_roles(self): + """Deleting a user portfolio permission causes domain invitations for the same email on the same + portfolio to be canceled, also deletes any exiting user domain roles on the portfolio for the + user if the user exists.""" + + domain_in_portfolio_1, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_1.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_1, portfolio=self.portfolio + ) + invite_1, _ = DomainInvitation.objects.get_or_create(email=self.user.email, domain=domain_in_portfolio_1) + + domain_in_portfolio_2, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_and_invited_2.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_2, portfolio=self.portfolio + ) + invite_2, _ = DomainInvitation.objects.get_or_create(email=self.user.email, domain=domain_in_portfolio_2) + + domain_in_portfolio_3, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_3.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_3, portfolio=self.portfolio + ) + UserDomainRole.objects.get_or_create( + user=self.user, domain=domain_in_portfolio_3, role=UserDomainRole.Roles.MANAGER + ) + + domain_in_portfolio_4, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_and_invited_4.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_4, portfolio=self.portfolio + ) + UserDomainRole.objects.get_or_create( + user=self.user, domain=domain_in_portfolio_4, role=UserDomainRole.Roles.MANAGER + ) + + domain_not_in_portfolio_1, _ = Domain.objects.get_or_create( + name="domain_not_in_portfolio.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create(creator=self.user, domain=domain_not_in_portfolio_1) + invite_3, _ = DomainInvitation.objects.get_or_create(email=self.user.email, domain=domain_not_in_portfolio_1) + + domain_not_in_portfolio_2, _ = Domain.objects.get_or_create( + name="domain_not_in_portfolio_2.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create(creator=self.user, domain=domain_not_in_portfolio_2) + UserDomainRole.objects.get_or_create( + user=self.user, domain=domain_not_in_portfolio_2, role=UserDomainRole.Roles.MANAGER + ) + + # Create portfolio permission + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create( + portfolio=self.portfolio, user=self.user, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + + # The domain invitations start off as INVITED + self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED) + + # The user domain roles exist + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_3, + ).exists() + ) + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_4, + ).exists() + ) + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_not_in_portfolio_2, + ).exists() + ) + + # Delete member (user portfolio permission) + portfolio_permission.delete() + + # Reload the objects from the database + invite_1 = DomainInvitation.objects.get(pk=invite_1.pk) + invite_2 = DomainInvitation.objects.get(pk=invite_2.pk) + invite_3 = DomainInvitation.objects.get(pk=invite_3.pk) + + # The domain invitations to the portfolio domains have been canceled + self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.CANCELED) + self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.CANCELED) + + # Invite 3 is unaffected + self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED) + + # The user domain roles have been deleted for the domains in portfolio + self.assertFalse( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_3, + ).exists() + ) + self.assertFalse( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_4, + ).exists() + ) + + # The user domain role on the domain not in portfolio still exists + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_not_in_portfolio_2, + ).exists() + ) + class TestUser(TestCase): """Test actions that occur on user login, diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index f46e417be..2dfead13f 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -214,7 +214,7 @@ class HomeTests(TestWithUser): @less_console_noise_decorator def test_state_help_text_expired(self): """Tests if each domain state has help text when expired""" - expired_text = "This domain has expired, but it is still online. " + expired_text = "This domain has expired. " test_domain, _ = Domain.objects.get_or_create(name="expired.gov", state=Domain.State.READY) test_domain.expiration_date = date(2011, 10, 10) test_domain.save() @@ -240,7 +240,7 @@ class HomeTests(TestWithUser): """Tests if each domain state has help text when expiration date is None""" # == Test a expiration of None for state ready. This should be expired. == # - expired_text = "This domain has expired, but it is still online. " + expired_text = "This domain has expired. " test_domain, _ = Domain.objects.get_or_create(name="imexpired.gov", state=Domain.State.READY) test_domain.expiration_date = None test_domain.save() diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index ba4d4485f..831fa4e4f 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -439,15 +439,21 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): username="usertest", ) - self.domaintorenew, _ = Domain.objects.get_or_create( + self.domain_to_renew, _ = Domain.objects.get_or_create( name="domainrenewal.gov", ) - UserDomainRole.objects.get_or_create( - user=self.user, domain=self.domaintorenew, role=UserDomainRole.Roles.MANAGER + self.domain_not_expiring, _ = Domain.objects.get_or_create( + name="domainnotexpiring.gov", expiration_date=timezone.now().date() + timedelta(days=65) ) - DomainInformation.objects.get_or_create(creator=self.user, domain=self.domaintorenew) + self.domain_no_domain_manager, _ = Domain.objects.get_or_create(name="domainnodomainmanager.gov") + + UserDomainRole.objects.get_or_create( + user=self.user, domain=self.domain_to_renew, role=UserDomainRole.Roles.MANAGER + ) + + DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain_to_renew) self.portfolio, _ = Portfolio.objects.get_or_create(organization_name="Test org", creator=self.user) @@ -473,13 +479,15 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): @override_flag("domain_renewal", active=True) def test_expiring_domain_on_detail_page_as_domain_manager(self): + """If a user is a domain manager and their domain is expiring soon, + user should be able to see the "Renew to maintain access" link domain overview detail box.""" self.client.force_login(self.user) with patch.object(Domain, "is_expiring", self.custom_is_expiring), patch.object( Domain, "is_expired", self.custom_is_expired_false ): - self.assertEquals(self.domaintorenew.state, Domain.State.UNKNOWN) + self.assertEquals(self.domain_to_renew.state, Domain.State.UNKNOWN) detail_page = self.client.get( - reverse("domain", kwargs={"pk": self.domaintorenew.id}), + reverse("domain", kwargs={"pk": self.domain_to_renew.id}), ) self.assertContains(detail_page, "Expiring soon") @@ -491,6 +499,8 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): @override_flag("domain_renewal", active=True) @override_flag("organization_feature", active=True) def test_expiring_domain_on_detail_page_in_org_model_as_a_non_domain_manager(self): + """In org model: If a user is NOT a domain manager and their domain is expiring soon, + user be notified to contact a domain manager in the domain overview detail box.""" portfolio, _ = Portfolio.objects.get_or_create(organization_name="Test org", creator=self.user) non_dom_manage_user = get_user_model().objects.create( first_name="Non Domain", @@ -510,9 +520,9 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, ], ) - domaintorenew2, _ = Domain.objects.get_or_create(name="bogusdomain2.gov") + domain_to_renew2, _ = Domain.objects.get_or_create(name="bogusdomain2.gov") DomainInformation.objects.get_or_create( - creator=non_dom_manage_user, domain=domaintorenew2, portfolio=self.portfolio + creator=non_dom_manage_user, domain=domain_to_renew2, portfolio=self.portfolio ) non_dom_manage_user.refresh_from_db() self.client.force_login(non_dom_manage_user) @@ -520,38 +530,42 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): Domain, "is_expired", self.custom_is_expired_false ): detail_page = self.client.get( - reverse("domain", kwargs={"pk": domaintorenew2.id}), + reverse("domain", kwargs={"pk": domain_to_renew2.id}), ) self.assertContains(detail_page, "Contact one of the listed domain managers to renew the domain.") @override_flag("domain_renewal", active=True) @override_flag("organization_feature", active=True) def test_expiring_domain_on_detail_page_in_org_model_as_a_domain_manager(self): + """Inorg model: If a user is a domain manager and their domain is expiring soon, + user should be able to see the "Renew to maintain access" link domain overview detail box.""" portfolio, _ = Portfolio.objects.get_or_create(organization_name="Test org2", creator=self.user) - domaintorenew3, _ = Domain.objects.get_or_create(name="bogusdomain3.gov") + domain_to_renew3, _ = Domain.objects.get_or_create(name="bogusdomain3.gov") - UserDomainRole.objects.get_or_create(user=self.user, domain=domaintorenew3, role=UserDomainRole.Roles.MANAGER) - DomainInformation.objects.get_or_create(creator=self.user, domain=domaintorenew3, portfolio=portfolio) + UserDomainRole.objects.get_or_create(user=self.user, domain=domain_to_renew3, role=UserDomainRole.Roles.MANAGER) + DomainInformation.objects.get_or_create(creator=self.user, domain=domain_to_renew3, portfolio=portfolio) self.user.refresh_from_db() self.client.force_login(self.user) with patch.object(Domain, "is_expiring", self.custom_is_expiring), patch.object( Domain, "is_expired", self.custom_is_expired_false ): detail_page = self.client.get( - reverse("domain", kwargs={"pk": domaintorenew3.id}), + reverse("domain", kwargs={"pk": domain_to_renew3.id}), ) self.assertContains(detail_page, "Renew to maintain access") @override_flag("domain_renewal", active=True) def test_domain_renewal_form_and_sidebar_expiring(self): + """If a user is a domain manager and their domain is expiring soon, + user should be able to see Renewal Form on the sidebar.""" self.client.force_login(self.user) with patch.object(Domain, "is_expiring", self.custom_is_expiring), patch.object( Domain, "is_expiring", self.custom_is_expiring ): # Grab the detail page detail_page = self.client.get( - reverse("domain", kwargs={"pk": self.domaintorenew.id}), + reverse("domain", kwargs={"pk": self.domain_to_renew.id}), ) # Make sure we see the link as a domain manager @@ -561,18 +575,19 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): self.assertContains(detail_page, "Renewal form") # Grab link to the renewal page - renewal_form_url = reverse("domain-renewal", kwargs={"pk": self.domaintorenew.id}) + renewal_form_url = reverse("domain-renewal", kwargs={"pk": self.domain_to_renew.id}) self.assertContains(detail_page, f'href="{renewal_form_url}"') # Simulate clicking the link response = self.client.get(renewal_form_url) self.assertEqual(response.status_code, 200) - self.assertContains(response, f"Renew {self.domaintorenew.name}") + self.assertContains(response, f"Renew {self.domain_to_renew.name}") @override_flag("domain_renewal", active=True) def test_domain_renewal_form_and_sidebar_expired(self): - + """If a user is a domain manager and their domain is expired, + user should be able to see Renewal Form on the sidebar.""" self.client.force_login(self.user) with patch.object(Domain, "is_expired", self.custom_is_expired_true), patch.object( @@ -580,10 +595,9 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): ): # Grab the detail page detail_page = self.client.get( - reverse("domain", kwargs={"pk": self.domaintorenew.id}), + reverse("domain", kwargs={"pk": self.domain_to_renew.id}), ) - print("puglesss", self.domaintorenew.is_expired) # Make sure we see the link as a domain manager self.assertContains(detail_page, "Renew to maintain access") @@ -591,17 +605,19 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): self.assertContains(detail_page, "Renewal form") # Grab link to the renewal page - renewal_form_url = reverse("domain-renewal", kwargs={"pk": self.domaintorenew.id}) + renewal_form_url = reverse("domain-renewal", kwargs={"pk": self.domain_to_renew.id}) self.assertContains(detail_page, f'href="{renewal_form_url}"') # Simulate clicking the link response = self.client.get(renewal_form_url) self.assertEqual(response.status_code, 200) - self.assertContains(response, f"Renew {self.domaintorenew.name}") + self.assertContains(response, f"Renew {self.domain_to_renew.name}") @override_flag("domain_renewal", active=True) def test_domain_renewal_form_your_contact_info_edit(self): + """Checking that if a user is a domain manager they can edit the + Your Profile portion of the Renewal Form.""" with less_console_noise(): # Start on the Renewal page for the domain renewal_page = self.app.get(reverse("domain-renewal", kwargs={"pk": self.domain_with_ip.id})) @@ -620,6 +636,8 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): @override_flag("domain_renewal", active=True) def test_domain_renewal_form_security_email_edit(self): + """Checking that if a user is a domain manager they can edit the + Security Email portion of the Renewal Form.""" with less_console_noise(): # Start on the Renewal page for the domain renewal_page = self.app.get(reverse("domain-renewal", kwargs={"pk": self.domain_with_ip.id})) @@ -641,6 +659,8 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): @override_flag("domain_renewal", active=True) def test_domain_renewal_form_domain_manager_edit(self): + """Checking that if a user is a domain manager they can edit the + Domain Manager portion of the Renewal Form.""" with less_console_noise(): # Start on the Renewal page for the domain renewal_page = self.app.get(reverse("domain-renewal", kwargs={"pk": self.domain_with_ip.id})) @@ -658,8 +678,26 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): self.assertContains(edit_page, "Domain managers can update all information related to a domain") @override_flag("domain_renewal", active=True) - def test_ack_checkbox_not_checked(self): + def test_domain_renewal_form_not_expired_or_expiring(self): + """Checking that if the user's domain is not expired or expiring that user should not be able + to access /renewal and that it should receive a 403.""" + with less_console_noise(): + # Start on the Renewal page for the domain + renewal_page = self.client.get(reverse("domain-renewal", kwargs={"pk": self.domain_not_expiring.id})) + self.assertEqual(renewal_page.status_code, 403) + @override_flag("domain_renewal", active=True) + def test_domain_renewal_form_does_not_appear_if_not_domain_manager(self): + """If user is not a domain manager and tries to access /renewal, user should receive a 403.""" + with patch.object(Domain, "is_expired", self.custom_is_expired_true), patch.object( + Domain, "is_expired", self.custom_is_expired_true + ): + renewal_page = self.client.get(reverse("domain-renewal", kwargs={"pk": self.domain_no_domain_manager.id})) + self.assertEqual(renewal_page.status_code, 403) + + @override_flag("domain_renewal", active=True) + def test_ack_checkbox_not_checked(self): + """If user don't check the checkbox, user should receive an error message.""" # Grab the renewal URL renewal_url = reverse("domain-renewal", kwargs={"pk": self.domain_with_ip.id}) @@ -671,7 +709,8 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): @override_flag("domain_renewal", active=True) def test_ack_checkbox_checked(self): - + """If user check the checkbox and submits the form, + user should be redirected Domain Over page with an updated by 1 year expiration date""" # Grab the renewal URL with patch.object(Domain, "renew_domain", self.custom_renew_domain): renewal_url = reverse("domain-renewal", kwargs={"pk": self.domain_with_ip.id}) @@ -2909,11 +2948,11 @@ class TestDomainRenewal(TestWithUser): name="igorville.gov", expiration_date=expiring_date ) self.domain_with_expired_date, _ = Domain.objects.get_or_create( - name="domainwithexpireddate.com", expiration_date=expired_date + name="domainwithexpireddate.gov", expiration_date=expired_date ) self.domain_with_current_date, _ = Domain.objects.get_or_create( - name="domainwithfarexpireddate.com", expiration_date=expiring_date_current + name="domainwithfarexpireddate.gov", expiration_date=expiring_date_current ) UserDomainRole.objects.get_or_create( @@ -2959,7 +2998,7 @@ class TestDomainRenewal(TestWithUser): today = datetime.now() expiring_date = (today + timedelta(days=30)).strftime("%Y-%m-%d") self.domain_with_another_expiring, _ = Domain.objects.get_or_create( - name="domainwithanotherexpiringdate.com", expiration_date=expiring_date + name="domainwithanotherexpiringdate.gov", expiration_date=expiring_date ) UserDomainRole.objects.get_or_create( @@ -2995,7 +3034,7 @@ class TestDomainRenewal(TestWithUser): today = datetime.now() expiring_date = (today + timedelta(days=31)).strftime("%Y-%m-%d") self.domain_with_another_expiring_org_model, _ = Domain.objects.get_or_create( - name="domainwithanotherexpiringdate_orgmodel.com", expiration_date=expiring_date + name="domainwithanotherexpiringdate_orgmodel.gov", expiration_date=expiring_date ) UserDomainRole.objects.get_or_create( diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 464e0d2a1..24673ac4f 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -311,11 +311,39 @@ class DomainView(DomainBaseView): self._update_session_with_domain() -class DomainRenewalView(DomainView): +class DomainRenewalView(DomainBaseView): """Domain detail overview page.""" template_name = "domain_renewal.html" + def get_context_data(self, **kwargs): + """Grabs the security email information and adds security_email to the renewal form context + sets it to None if it uses a default email""" + + context = super().get_context_data(**kwargs) + + default_emails = [DefaultEmail.PUBLIC_CONTACT_DEFAULT.value, DefaultEmail.LEGACY_DEFAULT.value] + + context["hidden_security_emails"] = default_emails + + security_email = self.object.get_security_email() + context["security_email"] = security_email + return context + + def in_editable_state(self, pk): + """Override in_editable_state from DomainPermission + Allow renewal form to be accessed + returns boolean""" + requested_domain = None + if Domain.objects.filter(id=pk).exists(): + requested_domain = Domain.objects.get(id=pk) + + return ( + requested_domain + and requested_domain.is_editable() + and (requested_domain.is_expiring() or requested_domain.is_expired()) + ) + def post(self, request, pk): domain = get_object_or_404(Domain, id=pk) diff --git a/src/registrar/views/report_views.py b/src/registrar/views/report_views.py index 694d1e205..ee2c079f3 100644 --- a/src/registrar/views/report_views.py +++ b/src/registrar/views/report_views.py @@ -5,17 +5,20 @@ from django.views import View from django.shortcuts import render from django.contrib import admin from django.db.models import Avg, F + +from registrar.views.utility.mixins import DomainAndRequestsReportsPermission, PortfolioReportsPermission from .. import models import datetime from django.utils import timezone - +from django.contrib.admin.views.decorators import staff_member_required +from django.utils.decorators import method_decorator from registrar.utility import csv_export - import logging logger = logging.getLogger(__name__) +@method_decorator(staff_member_required, name="dispatch") class AnalyticsView(View): def get(self, request): thirty_days_ago = datetime.datetime.today() - datetime.timedelta(days=30) @@ -149,6 +152,7 @@ class AnalyticsView(View): return render(request, "admin/analytics.html", context) +@method_decorator(staff_member_required, name="dispatch") class ExportDataType(View): def get(self, request, *args, **kwargs): # match the CSV example with all the fields @@ -158,7 +162,7 @@ class ExportDataType(View): return response -class ExportDataTypeUser(View): +class ExportDataTypeUser(DomainAndRequestsReportsPermission, View): """Returns a domain report for a given user on the request""" def get(self, request, *args, **kwargs): @@ -169,7 +173,7 @@ class ExportDataTypeUser(View): return response -class ExportMembersPortfolio(View): +class ExportMembersPortfolio(PortfolioReportsPermission, View): """Returns a members report for a given portfolio""" def get(self, request, *args, **kwargs): @@ -197,7 +201,7 @@ class ExportMembersPortfolio(View): return response -class ExportDataTypeRequests(View): +class ExportDataTypeRequests(DomainAndRequestsReportsPermission, View): """Returns a domain requests report for a given user on the request""" def get(self, request, *args, **kwargs): @@ -208,6 +212,7 @@ class ExportDataTypeRequests(View): return response +@method_decorator(staff_member_required, name="dispatch") class ExportDataFull(View): def get(self, request, *args, **kwargs): # Smaller export based on 1 @@ -217,6 +222,7 @@ class ExportDataFull(View): return response +@method_decorator(staff_member_required, name="dispatch") class ExportDataFederal(View): def get(self, request, *args, **kwargs): # Federal only @@ -226,6 +232,7 @@ class ExportDataFederal(View): return response +@method_decorator(staff_member_required, name="dispatch") class ExportDomainRequestDataFull(View): """Generates a downloaded report containing all Domain Requests (except started)""" @@ -237,6 +244,7 @@ class ExportDomainRequestDataFull(View): return response +@method_decorator(staff_member_required, name="dispatch") class ExportDataDomainsGrowth(View): def get(self, request, *args, **kwargs): start_date = request.GET.get("start_date", "") @@ -249,6 +257,7 @@ class ExportDataDomainsGrowth(View): return response +@method_decorator(staff_member_required, name="dispatch") class ExportDataRequestsGrowth(View): def get(self, request, *args, **kwargs): start_date = request.GET.get("start_date", "") @@ -261,6 +270,7 @@ class ExportDataRequestsGrowth(View): return response +@method_decorator(staff_member_required, name="dispatch") class ExportDataManagedDomains(View): def get(self, request, *args, **kwargs): start_date = request.GET.get("start_date", "") @@ -272,6 +282,7 @@ class ExportDataManagedDomains(View): return response +@method_decorator(staff_member_required, name="dispatch") class ExportDataUnmanagedDomains(View): def get(self, request, *args, **kwargs): start_date = request.GET.get("start_date", "") diff --git a/src/registrar/views/utility/invitation_helper.py b/src/registrar/views/utility/invitation_helper.py index 7e6e53dba..18c427940 100644 --- a/src/registrar/views/utility/invitation_helper.py +++ b/src/registrar/views/utility/invitation_helper.py @@ -3,7 +3,6 @@ from django.db import IntegrityError from registrar.models import PortfolioInvitation, User, UserPortfolioPermission from registrar.utility.email import EmailSendingError import logging - from registrar.utility.errors import ( AlreadyDomainInvitedError, AlreadyDomainManagerError, @@ -61,11 +60,11 @@ def get_requested_user(email): def handle_invitation_exceptions(request, exception, email): """Handle exceptions raised during the process.""" if isinstance(exception, EmailSendingError): - logger.warning(str(exception), exc_info=True) + logger.warning(exception, exc_info=True) messages.error(request, str(exception)) elif isinstance(exception, MissingEmailError): messages.error(request, str(exception)) - logger.error(str(exception), exc_info=True) + logger.error(exception, exc_info=True) elif isinstance(exception, OutsideOrgMemberError): messages.error(request, str(exception)) elif isinstance(exception, AlreadyDomainManagerError): diff --git a/src/registrar/views/utility/mixins.py b/src/registrar/views/utility/mixins.py index 236ef8696..8a4666372 100644 --- a/src/registrar/views/utility/mixins.py +++ b/src/registrar/views/utility/mixins.py @@ -153,6 +153,48 @@ class PermissionsLoginMixin(PermissionRequiredMixin): return super().handle_no_permission() +class DomainAndRequestsReportsPermission(PermissionsLoginMixin): + """Permission mixin for domain and requests csv downloads""" + + def has_permission(self): + """Check if this user has access to this domain. + + The user is in self.request.user and the domain needs to be looked + up from the domain's primary key in self.kwargs["pk"] + """ + + if not self.request.user.is_authenticated: + return False + + if self.request.user.is_restricted(): + return False + + return True + + +class PortfolioReportsPermission(PermissionsLoginMixin): + """Permission mixin for portfolio csv downloads""" + + def has_permission(self): + """Check if this user has access to this domain. + + The user is in self.request.user and the domain needs to be looked + up from the domain's primary key in self.kwargs["pk"] + """ + + if not self.request.user.is_authenticated: + return False + + if self.request.user.is_restricted(): + return False + + portfolio = self.request.session.get("portfolio") + if not self.request.user.has_view_members_portfolio_permission(portfolio): + return False + + return self.request.user.is_org_user(self.request) + + class DomainPermission(PermissionsLoginMixin): """Permission mixin that redirects to domain if user has access, otherwise 403""" @@ -192,7 +234,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):