Merge branch 'main' into bob/2416-portfolio-admin-emails

src/djangooidc/backends.py

@@ -21,48 +21,65 @@ class OpenIdConnectBackend(ModelBackend):
     """
 
     def authenticate(self, request, **kwargs):
-        logger.debug("kwargs %s" % kwargs)
+        logger.debug("kwargs %s", kwargs)
-        user = None
+
-        if not kwargs or "sub" not in kwargs.keys():
+        if not kwargs or "sub" not in kwargs:
-            return user
+            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 = {
+            user = self.get_or_create_user(UserModel, username, openid_data, kwargs)
-                UserModel.USERNAME_FIELD: username,
+        else:
-                # defaults _will_ be updated, these are not fallbacks
+            user = self.get_user_by_username(UserModel, username)
-                "defaults": openid_data,
+
+        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", ""),
         }
 
-            user, created = UserModel.objects.get_or_create(**args)
+    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 created:
+        if not user and openid_data["email"]:
-                # If user exists, update existing user
+            user = self.get_user_by_email(UserModel, openid_data["email"])
-                self.update_existing_user(user, args["defaults"])
+            if user:
-            else:
+                # if found by email, update the username
-                # If user is created, configure the user
+                setattr(user, UserModel.USERNAME_FIELD, username)
-                user = self.configure_user(user, **kwargs)
+
-        else:
+        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:
-                user = UserModel.objects.get_by_natural_key(username)
+            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
-        # run this callback for a each login
-        user.on_each_login()
-        return user
 
     def update_existing_user(self, user, kwargs):
         """

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

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

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) => {
+  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,9 +510,10 @@ export class BaseTable {
 
   // Add event listeners to table headers for sorting
   initializeTableHeaders() {
-    this.tableHeaders.forEach(header => {
+    this.tableHeaderSortButtons.forEach(tableHeader => {
-      header.addEventListener('click', event => {
+      tableHeader.addEventListener('click', event => {
-        let button = header.querySelector('.usa-table__header__button')
+        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
@@ -517,12 +523,8 @@ export class BaseTable {
           }
           // 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
+        } else {
-        // for USWDS listener on the button to execute.
+          console.warn('Issue with DOM');
-        // Check first to see if click occurs outside of the button
-        if (!button.contains(event.target)) {
-            // Simulate a button click
-            button.click();
         }
       });
     });
@@ -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 = '';

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;
     }
   };
 }

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

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

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

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

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 = (
+            help_text = "This domain has expired. Complete the online renewal process to maintain access."
-                "This domain has expired, but it is still online. " "To renew this domain, contact help@get.gov."
-            )
         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)
 

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)

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
 

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)

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

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 %}
-                <a href="{{ url }}">Renew to maintain access.</a>
+                <a href="{{ url }}" class="usa-link">Renew to maintain access.</a>
               {% 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 %}
-                <a href="{{ url }}">Renew to maintain access.</a>
+                <a href="{{ url }}" class="usa-link">Renew to maintain access.</a>
               {% 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 %}

src/registrar/templates/domain_renewal.html

@@ -38,11 +38,11 @@
   {{ block.super }}
   <div class="margin-top-4 tablet:grid-col-10">
     <h2 class="domain-name-wrap">Confirm the following information for accuracy</h2>
-    <p>Review these details below. We <a href="https://get.gov/domains/requirements/#what-.gov-domain-registrants-must-do" class="usa-link">
+    <p>Review the details below. We <a href="https://get.gov/domains/requirements/#what-.gov-domain-registrants-must-do" class="usa-link" target="_blank">
       require</a> 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.
     </p>
-    <p>If you would like to retire your domain instead, please <a href="https://get.gov/contact/" class="usa-link">
+    <p>If you would like to retire your domain instead, please <a href="https://get.gov/contact/" class="usa-link" target="_blank">
       contact us</a>. </p>
     <p><em>Required fields are marked with an asterisk (<abbr class="usa-hint usa-hint--required" title="required">*</abbr>).</em>
     </p>
@@ -98,7 +98,7 @@
             {% if form.is_policy_acknowledged.errors %}
             {% for error in form.is_policy_acknowledged.errors %}
             <div class="usa-error-message display-flex" role="alert">
-              <svg class="usa-icon usa-icon--large" focusable="true" role="img" aria-label="Error">
+              <svg class="usa-icon usa-icon--large" focusable="true" role="img" aria-label="Error: Check the box if you read and agree to the requirements for operating a .gov domain.">
                 <use xlink:href="{%static 'img/sprite.svg'%}#error"></use>
               </svg>
               <span class="margin-left-05">{{ error }}</span>
@@ -119,10 +119,8 @@
             >
             <label class="usa-checkbox__label" for="renewal-checkbox">
               I read and agree to the 
-              <a href="https://get.gov/domains/requirements/" class="usa-link">
+              <a href="https://get.gov/domains/requirements/" class="usa-link" target="_blank">
-              requirements for operating a .gov domain
+              requirements for operating a .gov domain</a>.<abbr class="usa-hint usa-hint--required" title="required">*</abbr>
-              </a>.
-              <abbr class="usa-hint usa-hint--required" title="required">*</abbr>
             </label>
           </div>
 
@@ -131,7 +129,7 @@
             name="submit_button"
             value="next"
             class="usa-button margin-top-3"
-          > Submit
+          > Submit and renew
           </button>
         </form>
       </fieldset>

src/registrar/templates/includes/domain_dates.html

@@ -1,7 +1,7 @@
 {% if domain.expiration_date or domain.created_at %}
   <p>
     {% if domain.expiration_date %}
-    <strong class="text-primary-dark">Expires:</strong> 
+    <strong class="text-primary-dark">Date of expiration:</strong> 
     {{ domain.expiration_date|date }}
     {% if domain.is_expired %} <span class="text-error"><strong>(expired)</strong></span>{% endif %}
     <br/>

src/registrar/templates/includes/domain_requests_table.html

@@ -57,7 +57,7 @@
                 <!----------------------------------------------------------------------
                    This link is commented out because we intend to add it back in later.
                 ------------------------------------------------------------------------->
-              <!-- <a href="{% url 'export_data_type_requests' %}" class="usa-button usa-button--unstyled usa-button--with-icon usa-button--justify-right" role="button">
+              <!-- <a href="{% url 'export_data_type_requests' %}" class="usa-button usa-button--unstyled usa-button--with-icon usa-button--justify-right">
                   <svg class="usa-icon usa-icon--large" aria-hidden="true" focusable="false" role="img" width="24" height="24">
                       <use xlink:href="{% static 'img/sprite.svg' %}#file_download"></use>
                   </svg>Export as CSV
@@ -78,6 +78,7 @@
                     id="domain-requests__usa-button--filter"
                     aria-expanded="false"
                     aria-controls="filter-status"
+                    aria-label="Status, list 7 items"
                 >
                     <span class="text-bold display-none" id="domain-requests__filter-indicator"></span> Status
                     <svg class="usa-icon top-2px" aria-hidden="true" focusable="false" role="img" width="24">

src/registrar/templates/includes/domains_table.html

@@ -10,14 +10,14 @@
 
 <!-- Org model banner (org manager can view, domain manager can edit)  -->
 {% if has_domain_renewal_flag and num_expiring_domains > 0 and has_any_domains_portfolio_permission %}
-<section class="usa-site-alert usa-site-alert--info margin-bottom-2 {% if add_class %}{{ add_class }}{% endif %}" aria-label="Site alert">
+<section class="usa-site-alert--slim usa-site-alert--info margin-bottom-2 {% if add_class %}{{ add_class }}{% endif %}" aria-label="Site alert">
   <div class="usa-alert">
-    <div class="usa-alert__body usa-alert__body--widescreen">
+    <div class="usa-alert__body">
       <p class="usa-alert__text maxw-none">
         {% if num_expiring_domains == 1%}  
-        One domain will expire soon. Go to "Manage" to renew the domain. <a href="#" id="link-expiring-domains" class="usa-link">Show expiring domain.</a>
+        One domain will expire soon. Go to "Manage" to renew the domain. <a href="#" id="link-expiring-domains" class="usa-link" tabindex="0" aria-label="Show expiring domains. This will filter the Domains table to only show the expiring domain.">Show expiring domain.</a>
         {% else%}
-        Multiple domains will expire soon. Go to "Manage" to renew the domains. <a href="#" id="link-expiring-domains" class="usa-link">Show expiring domains.</a>
+        Multiple domains will expire soon. Go to "Manage" to renew the domains. <a href="#" id="link-expiring-domains" class="usa-link" tabindex="0" aria-label="Show expiring domains. This will filter the Domains table to only show the expiring domains.">Show expiring domains.</a>
         {% endif %}
       </p>
     </div>
@@ -64,7 +64,7 @@
     {% if user_domain_count and user_domain_count > 0 %}
     <div class="section-outlined__utility-button mobile-lg:padding-right-105 {% if portfolio %} mobile:grid-col-12 desktop:grid-col-6 desktop:padding-left-3{% endif %}">
       <section aria-label="Domains report component" class="margin-top-205">
-        <a href="{% url 'export_data_type_user' %}" class="usa-button usa-button--unstyled usa-button--with-icon usa-button--justify-right" role="button">
+        <a href="{% url 'export_data_type_user' %}" class="usa-button usa-button--unstyled usa-button--with-icon usa-button--justify-right">
           <svg class="usa-icon usa-icon--large" aria-hidden="true" focusable="false" role="img" width="24" height="24">
             <use xlink:href="{%static 'img/sprite.svg'%}#file_download"></use>
           </svg>Export as CSV
@@ -76,14 +76,14 @@
 
   <!-- Non org model banner -->
   {% if has_domain_renewal_flag and num_expiring_domains > 0 and not portfolio %}
-  <section class="usa-site-alert usa-site-alert--info margin-bottom-2 {% if add_class %}{{ add_class }}{% endif %}" aria-label="Site alert">
+  <section class="usa-site-alert--slim usa-site-alert--info margin-bottom-2 {% if add_class %}{{ add_class }}{% endif %}" aria-label="Site alert">
     <div class="usa-alert">
-      <div class="usa-alert__body usa-alert__body--widescreen">
+      <div class="usa-alert__body">
         <p class="usa-alert__text maxw-none">
           {% if num_expiring_domains == 1%}  
-          One domain will expire soon. Go to "Manage" to renew the domain. <a href="#" id="link-expiring-domains" class="usa-link">Show expiring domain.</a>
+          One domain will expire soon. Go to "Manage" to renew the domain. <a href="#" id="link-expiring-domains" class="usa-link" tabindex="0" aria-label="Show expiring domains. This will filter the Domains table to only show the expiring domain.">Show expiring domain.</a>
           {% else%}
-          Multiple domains will expire soon. Go to "Manage" to renew the domains. <a href="#" id="link-expiring-domains" class="usa-link">Show expiring domains.</a>
+          Multiple domains will expire soon. Go to "Manage" to renew the domains. <a href="#" id="link-expiring-domains" class="usa-link" tabindex="0" aria-label="Show expiring domains. This will filter the Domains table to only show the expiring domains.">Show expiring domains.</a>
           {% endif %}
         </p>
       </div>
@@ -101,6 +101,7 @@
           id="domains__usa-button--filter"
           aria-expanded="false"
           aria-controls="filter-status"
+          aria-label="Status, list 5 items"
         >
           <span class="text-bold display-none" id="domains__filter-indicator"></span> Status
           <svg class="usa-icon top-2px" aria-hidden="true" focusable="false" role="img" width="24">

src/registrar/templates/includes/members_table.html

@@ -38,7 +38,7 @@
       </div>
       <div class="section-outlined__utility-button mobile-lg:padding-right-105 {% if portfolio %} mobile:grid-col-12 desktop:grid-col-6 desktop:padding-left-3{% endif %}">
         <section aria-label="Domains report component" class="margin-top-205">
-          <a href="{% url 'export_members_portfolio' %}" class="usa-button usa-button--unstyled usa-button--with-icon usa-button--justify-right" role="button">
+          <a href="{% url 'export_members_portfolio' %}" class="usa-button usa-button--unstyled usa-button--with-icon usa-button--justify-right">
             <svg class="usa-icon usa-icon--large" aria-hidden="true" focusable="false" role="img" width="24" height="24">
               <use xlink:href="{%static 'img/sprite.svg'%}#file_download"></use>
             </svg>Export as CSV

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,

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

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(
+        self.domain_not_expiring, _ = Domain.objects.get_or_create(
-            user=self.user, domain=self.domaintorenew, role=UserDomainRole.Roles.MANAGER
+            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)
+        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=domaintorenew3, portfolio=portfolio)
+        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(

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)

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", "")

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

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