From 3ffe3002cb625adb826d0a84094517a8e295df01 Mon Sep 17 00:00:00 2001 From: Alysia Broddrick Date: Tue, 9 Jan 2024 12:01:08 -0800 Subject: [PATCH 01/13] removed bl infra --- .github/workflows/migrate.yaml | 1 - .github/workflows/reset-db.yaml | 1 - ops/manifests/manifest-bl.yaml | 32 -------------------------------- src/registrar/config/settings.py | 1 - 4 files changed, 35 deletions(-) delete mode 100644 ops/manifests/manifest-bl.yaml diff --git a/.github/workflows/migrate.yaml b/.github/workflows/migrate.yaml index 8523af013..2033ee51c 100644 --- a/.github/workflows/migrate.yaml +++ b/.github/workflows/migrate.yaml @@ -26,7 +26,6 @@ on: - rb - ko - ab - - bl - rjm - dk diff --git a/.github/workflows/reset-db.yaml b/.github/workflows/reset-db.yaml index 3848a33bd..f8730c865 100644 --- a/.github/workflows/reset-db.yaml +++ b/.github/workflows/reset-db.yaml @@ -26,7 +26,6 @@ on: - rb - ko - ab - - bl - rjm - dk diff --git a/ops/manifests/manifest-bl.yaml b/ops/manifests/manifest-bl.yaml deleted file mode 100644 index 59529278b..000000000 --- a/ops/manifests/manifest-bl.yaml +++ /dev/null @@ -1,32 +0,0 @@ ---- -applications: -- name: getgov-bl - buildpacks: - - python_buildpack - path: ../../src - instances: 1 - memory: 512M - stack: cflinuxfs4 - timeout: 180 - command: ./run.sh - health-check-type: http - health-check-http-endpoint: /health - health-check-invocation-timeout: 40 - env: - # Send stdout and stderr straight to the terminal without buffering - PYTHONUNBUFFERED: yup - # Tell Django where to find its configuration - DJANGO_SETTINGS_MODULE: registrar.config.settings - # Tell Django where it is being hosted - DJANGO_BASE_URL: https://getgov-bl.app.cloud.gov - # Tell Django how much stuff to log - DJANGO_LOG_LEVEL: INFO - # default public site location - GETGOV_PUBLIC_SITE_URL: https://beta.get.gov - # Flag to disable/enable features in prod environments - IS_PRODUCTION: False - routes: - - route: getgov-bl.app.cloud.gov - services: - - getgov-credentials - - getgov-bl-database diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 2de7e6eb2..f037d4901 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -660,7 +660,6 @@ ALLOWED_HOSTS = [ "getgov-rb.app.cloud.gov", "getgov-ko.app.cloud.gov", "getgov-ab.app.cloud.gov", - "getgov-bl.app.cloud.gov", "getgov-rjm.app.cloud.gov", "getgov-dk.app.cloud.gov", "manage.get.gov", From 510da219347f8e267a3422e6d34e0810b5acaee7 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 18 Jan 2024 19:58:53 -0500 Subject: [PATCH 02/13] Implement VIP table and fix 401 login bug --- src/djangooidc/exceptions.py | 4 ++ src/djangooidc/oidc.py | 7 ++++ src/djangooidc/tests/test_views.py | 19 ++++++++++ src/djangooidc/views.py | 8 ++++ src/registrar/admin.py | 23 ++++++++++++ .../migrations/0063_veryimportantperson.py | 37 +++++++++++++++++++ src/registrar/models/__init__.py | 3 ++ src/registrar/models/user.py | 5 +++ src/registrar/models/very_important_person.py | 36 ++++++++++++++++++ src/registrar/tests/test_admin.py | 25 +++++++++++++ src/registrar/tests/test_models.py | 9 ++++- 11 files changed, 175 insertions(+), 1 deletion(-) create mode 100644 src/registrar/migrations/0063_veryimportantperson.py create mode 100644 src/registrar/models/very_important_person.py diff --git a/src/djangooidc/exceptions.py b/src/djangooidc/exceptions.py index 260750a4d..226337f54 100644 --- a/src/djangooidc/exceptions.py +++ b/src/djangooidc/exceptions.py @@ -33,6 +33,10 @@ class AuthenticationFailed(OIDCException): friendly_message = "This login attempt didn't work." +class NoStateDefined(OIDCException): + friendly_message = "The session state is None." + + class InternalError(OIDCException): status = status.INTERNAL_SERVER_ERROR friendly_message = "The system broke while trying to log you in." diff --git a/src/djangooidc/oidc.py b/src/djangooidc/oidc.py index 91bfddc66..bff766bb4 100644 --- a/src/djangooidc/oidc.py +++ b/src/djangooidc/oidc.py @@ -183,6 +183,8 @@ class Client(oic.Client): if authn_response["state"] != session.get("state", None): # this most likely means the user's Django session vanished logger.error("Received state not the same as expected for %s" % state) + if session.get("state", None) is None: + raise o_e.NoStateDefined() raise o_e.AuthenticationFailed(locator=state) if self.behaviour.get("response_type") == "code": @@ -272,6 +274,11 @@ class Client(oic.Client): super(Client, self).store_response(resp, info) + def get_default_acr_value(self): + """returns the acr_value from settings + this helper function is called from djangooidc views""" + return self.behaviour.get("acr_value") + def get_step_up_acr_value(self): """returns the step_up_acr_value from settings this helper function is called from djangooidc views""" diff --git a/src/djangooidc/tests/test_views.py b/src/djangooidc/tests/test_views.py index 282e91e1f..63b23df96 100644 --- a/src/djangooidc/tests/test_views.py +++ b/src/djangooidc/tests/test_views.py @@ -3,6 +3,8 @@ from unittest.mock import MagicMock, patch from django.http import HttpResponse from django.test import Client, TestCase, RequestFactory from django.urls import reverse + +from djangooidc.exceptions import NoStateDefined from ..views import login_callback from .common import less_console_noise @@ -17,6 +19,9 @@ class ViewsTest(TestCase): def say_hi(*args): return HttpResponse("Hi") + def create_acr(*args): + return "any string" + def user_info(*args): return { "sub": "TEST", @@ -34,6 +39,7 @@ class ViewsTest(TestCase): callback_url = reverse("openid_login_callback") # mock mock_client.create_authn_request.side_effect = self.say_hi + mock_client.get_default_acr_value.side_effect = self.create_acr # test response = self.client.get(reverse("login"), {"next": callback_url}) # assert @@ -53,6 +59,19 @@ class ViewsTest(TestCase): self.assertTemplateUsed(response, "500.html") self.assertIn("Server error", response.content.decode("utf-8")) + def test_callback_with_no_session_state(self, mock_client): + """If the local session is None (ie the server restarted while user was logged out), + we do not throw an exception. Rather, we attempt to login again.""" + # mock + mock_client.get_default_acr_value.side_effect = self.create_acr + mock_client.callback.side_effect = NoStateDefined() + # test + with less_console_noise(): + response = self.client.get(reverse("openid_login_callback")) + # assert + self.assertEqual(response.status_code, 302) + self.assertEqual(response.url, "/") + def test_login_callback_reads_next(self, mock_client): # setup session = self.client.session diff --git a/src/djangooidc/views.py b/src/djangooidc/views.py index b5905df48..b786ed2e9 100644 --- a/src/djangooidc/views.py +++ b/src/djangooidc/views.py @@ -55,6 +55,10 @@ def error_page(request, error): def openid(request): """Redirect the user to an authentication provider (OP).""" + + # If the session reset because of a server restart, attempt to login again + request.session["acr_value"] = CLIENT.get_default_acr_value() + request.session["next"] = request.GET.get("next", "/") try: @@ -78,9 +82,13 @@ def login_callback(request): if user: login(request, user) logger.info("Successfully logged in user %s" % user) + # Double login bug? return redirect(request.session.get("next", "/")) else: raise o_e.BannedUser() + except o_e.NoStateDefined as nsd_err: + logger.debug(f"No State Defined: {nsd_err}") + return redirect(request.session.get("next", "/")) except Exception as err: return error_page(request, err) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 8d3b1d29f..9ad459a06 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1239,6 +1239,28 @@ class DraftDomainAdmin(ListHeaderAdmin): search_help_text = "Search by draft domain name." +class VeryImportantPersonAdmin(ListHeaderAdmin): + list_display = ("email", "custom_user_label", "notes", "created_at") + search_fields = ["email"] + search_help_text = "Search by email." + list_filter = [ + "user", + ] + readonly_fields = [ + "user", + ] + + def custom_user_label(self, obj): + return obj.user + + custom_user_label.short_description = "Requestor" # type: ignore + + def save_model(self, request, obj, form, change): + # Set the user field to the current admin user + obj.user = request.user if request.user.is_authenticated else None + super().save_model(request, obj, form, change) + + admin.site.unregister(LogEntry) # Unregister the default registration admin.site.register(LogEntry, CustomLogEntryAdmin) admin.site.register(models.User, MyUserAdmin) @@ -1259,3 +1281,4 @@ admin.site.register(models.Website, WebsiteAdmin) admin.site.register(models.PublicContact, AuditedAdmin) admin.site.register(models.DomainApplication, DomainApplicationAdmin) admin.site.register(models.TransitionDomain, TransitionDomainAdmin) +admin.site.register(models.VeryImportantPerson, VeryImportantPersonAdmin) diff --git a/src/registrar/migrations/0063_veryimportantperson.py b/src/registrar/migrations/0063_veryimportantperson.py new file mode 100644 index 000000000..3d56ae1ac --- /dev/null +++ b/src/registrar/migrations/0063_veryimportantperson.py @@ -0,0 +1,37 @@ +# Generated by Django 4.2.7 on 2024-01-19 00:18 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + dependencies = [ + ("registrar", "0062_alter_host_name"), + ] + + operations = [ + migrations.CreateModel( + name="VeryImportantPerson", + fields=[ + ("id", models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name="ID")), + ("created_at", models.DateTimeField(auto_now_add=True)), + ("updated_at", models.DateTimeField(auto_now=True)), + ("email", models.EmailField(blank=True, db_index=True, help_text="Email", max_length=254, null=True)), + ("notes", models.TextField(blank=True, help_text="Notes", null=True)), + ( + "user", + models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="verifiedby_user", + to=settings.AUTH_USER_MODEL, + ), + ), + ], + options={ + "abstract": False, + }, + ), + ] diff --git a/src/registrar/models/__init__.py b/src/registrar/models/__init__.py index 6afad5a5c..90cb2e286 100644 --- a/src/registrar/models/__init__.py +++ b/src/registrar/models/__init__.py @@ -13,6 +13,7 @@ from .user import User from .user_group import UserGroup from .website import Website from .transition_domain import TransitionDomain +from .very_important_person import VeryImportantPerson __all__ = [ "Contact", @@ -29,6 +30,7 @@ __all__ = [ "UserGroup", "Website", "TransitionDomain", + "VeryImportantPerson", ] auditlog.register(Contact) @@ -45,3 +47,4 @@ auditlog.register(User, m2m_fields=["user_permissions", "groups"]) auditlog.register(UserGroup, m2m_fields=["permissions"]) auditlog.register(Website) auditlog.register(TransitionDomain) +auditlog.register(VeryImportantPerson) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index d79e4c9ee..269569bfe 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -7,6 +7,7 @@ from registrar.models.user_domain_role import UserDomainRole from .domain_invitation import DomainInvitation from .transition_domain import TransitionDomain +from .very_important_person import VeryImportantPerson from .domain import Domain from phonenumber_field.modelfields import PhoneNumberField # type: ignore @@ -89,6 +90,10 @@ class User(AbstractUser): if TransitionDomain.objects.filter(username=email).exists(): return False + # New users flagged by Staff to bypass ial2 + if VeryImportantPerson.objects.filter(email=email).exists(): + return False + # A new incoming user who is being invited to be a domain manager (that is, # their email address is in DomainInvitation for an invitation that is not yet "retrieved"). invited = DomainInvitation.DomainInvitationStatus.INVITED diff --git a/src/registrar/models/very_important_person.py b/src/registrar/models/very_important_person.py new file mode 100644 index 000000000..42621c64a --- /dev/null +++ b/src/registrar/models/very_important_person.py @@ -0,0 +1,36 @@ +from django.db import models + +from .utility.time_stamped_model import TimeStampedModel + + +class VeryImportantPerson(TimeStampedModel): + + """""" + + email = models.EmailField( + null=True, + blank=True, + help_text="Email", + db_index=True, + ) + + user = models.ForeignKey( + "registrar.User", + null=True, + blank=True, + on_delete=models.SET_NULL, + related_name="verifiedby_user", + ) + + notes = models.TextField( + null=True, + blank=True, + help_text="Notes", + ) + + def __str__(self): + try: + if self.email: + return self.email + except Exception: + return "" diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index f7b1ef06e..175bef11f 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -14,9 +14,11 @@ from registrar.admin import ( ContactAdmin, DomainInformationAdmin, UserDomainRoleAdmin, + VeryImportantPersonAdmin, ) from registrar.models import Domain, DomainApplication, DomainInformation, User, DomainInvitation, Contact, Website from registrar.models.user_domain_role import UserDomainRole +from registrar.models.very_important_person import VeryImportantPerson from .common import ( MockSESClient, AuditedAdminMockData, @@ -1737,3 +1739,26 @@ class ContactAdminTest(TestCase): def tearDown(self): User.objects.all().delete() + + +class VeryImportantPersonAdminTestCase(TestCase): + def setUp(self): + self.superuser = create_superuser() + self.factory = RequestFactory() + + def test_save_model_sets_user_field(self): + # Create an instance of the admin class + admin_instance = VeryImportantPersonAdmin(model=VeryImportantPerson, admin_site=None) + + # Create a VeryImportantPerson instance + vip_instance = VeryImportantPerson(email="test@example.com", notes="Test Notes") + + # Create a request object + request = self.factory.post("/admin/yourapp/veryimportantperson/add/") + request.user = self.superuser + + # Call the save_model method + admin_instance.save_model(request, vip_instance, None, None) + + # Check that the user field is set to the request.user + self.assertEqual(vip_instance.user, self.superuser) diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 464d00dc5..ef6522747 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -15,7 +15,8 @@ from registrar.models import ( ) import boto3_mocking -from registrar.models.transition_domain import TransitionDomain # type: ignore +from registrar.models.transition_domain import TransitionDomain +from registrar.models.very_important_person import VeryImportantPerson # type: ignore from .common import MockSESClient, less_console_noise, completed_application from django_fsm import TransitionNotAllowed @@ -652,6 +653,12 @@ class TestUser(TestCase): TransitionDomain.objects.get_or_create(username=self.user.email, domain_name=self.domain_name) self.assertFalse(User.needs_identity_verification(self.user.email, self.user.username)) + def test_identity_verification_with_very_important_person(self): + """A Very Important Person should return False + when tested with class method needs_identity_verification""" + VeryImportantPerson.objects.get_or_create(email=self.user.email) + self.assertFalse(User.needs_identity_verification(self.user.email, self.user.username)) + def test_identity_verification_with_invited_user(self): """An invited user should return False when tested with class method needs_identity_verification""" From d1a80d3307d95dfd042ce841b5424ea3ca630ddf Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 19 Jan 2024 15:22:42 -0500 Subject: [PATCH 03/13] revert column header to user, truncate notes --- src/registrar/admin.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 9ad459a06..125ff2e8a 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1240,7 +1240,7 @@ class DraftDomainAdmin(ListHeaderAdmin): class VeryImportantPersonAdmin(ListHeaderAdmin): - list_display = ("email", "custom_user_label", "notes", "created_at") + list_display = ("email", "user", "truncated_notes", "created_at") search_fields = ["email"] search_help_text = "Search by email." list_filter = [ @@ -1250,10 +1250,11 @@ class VeryImportantPersonAdmin(ListHeaderAdmin): "user", ] - def custom_user_label(self, obj): - return obj.user + def truncated_notes(self, obj): + # Truncate the 'notes' field to 200 characters + return str(obj.notes)[:50] - custom_user_label.short_description = "Requestor" # type: ignore + truncated_notes.short_description = "Notes (Truncated)" # type: ignore def save_model(self, request, obj, form, change): # Set the user field to the current admin user From 512d87871a4446b5e9398bd9b161071c0f0e97b2 Mon Sep 17 00:00:00 2001 From: Michelle Rago <60157596+michelle-rago@users.noreply.github.com> Date: Tue, 23 Jan 2024 12:31:28 -0500 Subject: [PATCH 04/13] Make "domain" singular in references to .gov operating requirements (#1667) * Make "domain" singular in references to .gov operating requirements * Make "domain" singular in references to .gov operating requirements * Make "domain" singular in references to .gov operating requirements * Make "domain" singular in references to .gov operating requirements * Update label.html * Change form page title from "Apply for a..." to "Request a..." * Plural to singular --- src/registrar/admin.py | 4 ++-- src/registrar/forms/application_wizard.py | 4 ++-- src/registrar/templates/application_form.html | 2 +- src/registrar/templates/application_requirements.html | 2 +- src/registrar/templates/django/forms/label.html | 2 +- src/registrar/tests/test_forms.py | 4 ++-- src/registrar/views/application.py | 2 +- 7 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 8d3b1d29f..3c1823f83 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -610,7 +610,7 @@ class DomainInformationAdmin(ListHeaderAdmin): ), ("Anything else?", {"fields": ["anything_else"]}), ( - "Requirements for operating .gov domains", + "Requirements for operating a .gov domain", {"fields": ["is_policy_acknowledged"]}, ), ] @@ -779,7 +779,7 @@ class DomainApplicationAdmin(ListHeaderAdmin): ), ("Anything else?", {"fields": ["anything_else"]}), ( - "Requirements for operating .gov domains", + "Requirements for operating a .gov domain", {"fields": ["is_policy_acknowledged"]}, ), ] diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 36ff408c2..85ce28bb6 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -838,8 +838,8 @@ class AnythingElseForm(RegistrarForm): class RequirementsForm(RegistrarForm): is_policy_acknowledged = forms.BooleanField( - label="I read and agree to the requirements for operating .gov domains.", + label="I read and agree to the requirements for operating a .gov domain.", error_messages={ - "required": ("Check the box if you read and agree to the requirements for operating .gov domains.") + "required": ("Check the box if you read and agree to the requirements for operating a .gov domain.") }, ) diff --git a/src/registrar/templates/application_form.html b/src/registrar/templates/application_form.html index c34ddf5bc..524045fbe 100644 --- a/src/registrar/templates/application_form.html +++ b/src/registrar/templates/application_form.html @@ -1,7 +1,7 @@ {% extends 'base.html' %} {% load static form_helpers url_helpers %} -{% block title %}Apply for a .gov domain | {{form_titles|get_item:steps.current}} | {% endblock %} +{% block title %}Request a .gov domain | {{form_titles|get_item:steps.current}} | {% endblock %} {% block content %}
diff --git a/src/registrar/templates/application_requirements.html b/src/registrar/templates/application_requirements.html index d16edf963..ef0bf00f2 100644 --- a/src/registrar/templates/application_requirements.html +++ b/src/registrar/templates/application_requirements.html @@ -2,7 +2,7 @@ {% load field_helpers %} {% block form_instructions %} -

Please read this page. Check the box at the bottom to show that you agree to the requirements for operating .gov domains.

+

Please read this page. Check the box at the bottom to show that you agree to the requirements for operating a .gov domain.

The .gov domain space exists to support a broad diversity of government missions. Generally, we don’t review or audit how government organizations use their registered domains. However, misuse of a .gov domain can reflect upon the integrity of the entire .gov space. There are categories of misuse that are statutorily prohibited or abusive in nature.

diff --git a/src/registrar/templates/django/forms/label.html b/src/registrar/templates/django/forms/label.html index 18d24a7bd..545ccf781 100644 --- a/src/registrar/templates/django/forms/label.html +++ b/src/registrar/templates/django/forms/label.html @@ -10,7 +10,7 @@ {% if widget.attrs.required %} - {% if field.label == "Is your organization an election office?" or field.label == "What .gov domain do you want?" or field.label == "I read and agree to the requirements for operating .gov domains." or field.label == "Please explain why there are no other employees from your organization we can contact to help us assess your eligibility for a .gov domain." %} + {% if field.label == "Is your organization an election office?" or field.label == "What .gov domain do you want?" or field.label == "I read and agree to the requirements for operating a .gov domain." or field.label == "Please explain why there are no other employees from your organization we can contact to help us assess your eligibility for a .gov domain." %} {% else %} * {% endif %} diff --git a/src/registrar/tests/test_forms.py b/src/registrar/tests/test_forms.py index 9d553acc5..a8d84ba7b 100644 --- a/src/registrar/tests/test_forms.py +++ b/src/registrar/tests/test_forms.py @@ -338,7 +338,7 @@ class TestFormValidation(MockEppLib): form = RequirementsForm(data={}) self.assertEqual( form.errors["is_policy_acknowledged"], - ["Check the box if you read and agree to the requirements for operating .gov domains."], + ["Check the box if you read and agree to the requirements for operating a .gov domain."], ) def test_requirements_form_unchecked(self): @@ -346,7 +346,7 @@ class TestFormValidation(MockEppLib): form = RequirementsForm(data={"is_policy_acknowledged": False}) self.assertEqual( form.errors["is_policy_acknowledged"], - ["Check the box if you read and agree to the requirements for operating .gov domains."], + ["Check the box if you read and agree to the requirements for operating a .gov domain."], ) def test_tribal_government_unrecognized(self): diff --git a/src/registrar/views/application.py b/src/registrar/views/application.py index 031b93dee..a15f36ccc 100644 --- a/src/registrar/views/application.py +++ b/src/registrar/views/application.py @@ -92,7 +92,7 @@ class ApplicationWizard(ApplicationWizardPermissionView, TemplateView): Step.YOUR_CONTACT: _("Your contact information"), Step.OTHER_CONTACTS: _("Other employees from your organization"), Step.ANYTHING_ELSE: _("Anything else?"), - Step.REQUIREMENTS: _("Requirements for operating .gov domains"), + Step.REQUIREMENTS: _("Requirements for operating a .gov domain"), Step.REVIEW: _("Review and submit your domain request"), } From 52fff3b82c65dd5d7863fcf9bc6ef11eb2bce82f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 23 Jan 2024 14:58:43 -0700 Subject: [PATCH 05/13] Add verbose names --- src/registrar/models/domain_application.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/registrar/models/domain_application.py b/src/registrar/models/domain_application.py index 196449bfa..30def9cfc 100644 --- a/src/registrar/models/domain_application.py +++ b/src/registrar/models/domain_application.py @@ -431,11 +431,13 @@ class DomainApplication(TimeStampedModel): null=True, blank=True, help_text="Street address", + verbose_name="Address line 1", ) address_line2 = models.TextField( null=True, blank=True, help_text="Street address line 2 (optional)", + verbose_name="Address line 2", ) city = models.TextField( null=True, From 5d97893b92eda69cb618a87328ab05c3cb26ff71 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 23 Jan 2024 15:01:19 -0700 Subject: [PATCH 06/13] Add migration --- ...omainapplication_address_line1_and_more.py | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 src/registrar/migrations/0063_alter_domainapplication_address_line1_and_more.py diff --git a/src/registrar/migrations/0063_alter_domainapplication_address_line1_and_more.py b/src/registrar/migrations/0063_alter_domainapplication_address_line1_and_more.py new file mode 100644 index 000000000..99ffac7aa --- /dev/null +++ b/src/registrar/migrations/0063_alter_domainapplication_address_line1_and_more.py @@ -0,0 +1,24 @@ +# Generated by Django 4.2.7 on 2024-01-23 22:01 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("registrar", "0062_alter_host_name"), + ] + + operations = [ + migrations.AlterField( + model_name="domainapplication", + name="address_line1", + field=models.TextField(blank=True, help_text="Street address", null=True, verbose_name="Address line 1"), + ), + migrations.AlterField( + model_name="domainapplication", + name="address_line2", + field=models.TextField( + blank=True, help_text="Street address line 2 (optional)", null=True, verbose_name="Address line 2" + ), + ), + ] From 3f8e5ce204516eb54fda3b96e7ed6c4631808ebb Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 23 Jan 2024 18:15:56 -0500 Subject: [PATCH 07/13] Revisions on model --- src/djangooidc/views.py | 4 ++-- src/registrar/admin.py | 8 ++++---- src/registrar/migrations/0063_veryimportantperson.py | 8 ++++---- src/registrar/models/very_important_person.py | 12 ++++-------- src/registrar/tests/test_admin.py | 4 ++-- 5 files changed, 16 insertions(+), 20 deletions(-) diff --git a/src/djangooidc/views.py b/src/djangooidc/views.py index b786ed2e9..2fc2a0363 100644 --- a/src/djangooidc/views.py +++ b/src/djangooidc/views.py @@ -82,12 +82,12 @@ def login_callback(request): if user: login(request, user) logger.info("Successfully logged in user %s" % user) - # Double login bug? + # Double login bug (1507)? return redirect(request.session.get("next", "/")) else: raise o_e.BannedUser() except o_e.NoStateDefined as nsd_err: - logger.debug(f"No State Defined: {nsd_err}") + logger.warning(f"No State Defined: {nsd_err}") return redirect(request.session.get("next", "/")) except Exception as err: return error_page(request, err) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 125ff2e8a..eee808691 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1240,14 +1240,14 @@ class DraftDomainAdmin(ListHeaderAdmin): class VeryImportantPersonAdmin(ListHeaderAdmin): - list_display = ("email", "user", "truncated_notes", "created_at") + list_display = ("email", "requestor", "truncated_notes", "created_at") search_fields = ["email"] search_help_text = "Search by email." list_filter = [ - "user", + "requestor", ] readonly_fields = [ - "user", + "requestor", ] def truncated_notes(self, obj): @@ -1258,7 +1258,7 @@ class VeryImportantPersonAdmin(ListHeaderAdmin): def save_model(self, request, obj, form, change): # Set the user field to the current admin user - obj.user = request.user if request.user.is_authenticated else None + obj.requestor = request.requestor if request.requestor.is_authenticated else None super().save_model(request, obj, form, change) diff --git a/src/registrar/migrations/0063_veryimportantperson.py b/src/registrar/migrations/0063_veryimportantperson.py index 3d56ae1ac..29ea7ffdb 100644 --- a/src/registrar/migrations/0063_veryimportantperson.py +++ b/src/registrar/migrations/0063_veryimportantperson.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.7 on 2024-01-19 00:18 +# Generated by Django 4.2.7 on 2024-01-23 21:02 from django.conf import settings from django.db import migrations, models @@ -17,10 +17,10 @@ class Migration(migrations.Migration): ("id", models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name="ID")), ("created_at", models.DateTimeField(auto_now_add=True)), ("updated_at", models.DateTimeField(auto_now=True)), - ("email", models.EmailField(blank=True, db_index=True, help_text="Email", max_length=254, null=True)), - ("notes", models.TextField(blank=True, help_text="Notes", null=True)), + ("email", models.EmailField(blank=True, db_index=True, help_text="Email", max_length=254)), + ("notes", models.TextField(blank=True, help_text="Notes")), ( - "user", + "requestor", models.ForeignKey( blank=True, null=True, diff --git a/src/registrar/models/very_important_person.py b/src/registrar/models/very_important_person.py index 42621c64a..2fb53ff27 100644 --- a/src/registrar/models/very_important_person.py +++ b/src/registrar/models/very_important_person.py @@ -8,13 +8,13 @@ class VeryImportantPerson(TimeStampedModel): """""" email = models.EmailField( - null=True, + null=False, blank=True, help_text="Email", db_index=True, ) - user = models.ForeignKey( + requestor = models.ForeignKey( "registrar.User", null=True, blank=True, @@ -23,14 +23,10 @@ class VeryImportantPerson(TimeStampedModel): ) notes = models.TextField( - null=True, + null=False, blank=True, help_text="Notes", ) def __str__(self): - try: - if self.email: - return self.email - except Exception: - return "" + return self.email diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 175bef11f..139e4bc84 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1755,10 +1755,10 @@ class VeryImportantPersonAdminTestCase(TestCase): # Create a request object request = self.factory.post("/admin/yourapp/veryimportantperson/add/") - request.user = self.superuser + request.requestor = self.superuser # Call the save_model method admin_instance.save_model(request, vip_instance, None, None) # Check that the user field is set to the request.user - self.assertEqual(vip_instance.user, self.superuser) + self.assertEqual(vip_instance.requestor, self.superuser) From 52b76a79981a63b03721664c699bf57c2b6d0e5e Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 23 Jan 2024 18:40:21 -0500 Subject: [PATCH 08/13] fix error in admin.py --- src/registrar/admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index eee808691..dedc6f527 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1258,7 +1258,7 @@ class VeryImportantPersonAdmin(ListHeaderAdmin): def save_model(self, request, obj, form, change): # Set the user field to the current admin user - obj.requestor = request.requestor if request.requestor.is_authenticated else None + obj.requestor = request.user if request.user.is_authenticated else None super().save_model(request, obj, form, change) From 784b187b09add07857c1975049560be1e520e44f Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 23 Jan 2024 18:48:51 -0500 Subject: [PATCH 09/13] fix failing test by force logging in user --- src/registrar/tests/test_admin.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 139e4bc84..e0d0a6be5 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1747,6 +1747,8 @@ class VeryImportantPersonAdminTestCase(TestCase): self.factory = RequestFactory() def test_save_model_sets_user_field(self): + self.client.force_login(self.superuser) + # Create an instance of the admin class admin_instance = VeryImportantPersonAdmin(model=VeryImportantPerson, admin_site=None) From 8570a03600dc8b00ff271e527514376b1ec48370 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 23 Jan 2024 18:58:26 -0500 Subject: [PATCH 10/13] Make email and notes required, fix unit test --- src/registrar/migrations/0063_veryimportantperson.py | 6 +++--- src/registrar/models/very_important_person.py | 4 ++-- src/registrar/tests/test_admin.py | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/registrar/migrations/0063_veryimportantperson.py b/src/registrar/migrations/0063_veryimportantperson.py index 29ea7ffdb..38cfe328e 100644 --- a/src/registrar/migrations/0063_veryimportantperson.py +++ b/src/registrar/migrations/0063_veryimportantperson.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.7 on 2024-01-23 21:02 +# Generated by Django 4.2.7 on 2024-01-23 23:51 from django.conf import settings from django.db import migrations, models @@ -17,8 +17,8 @@ class Migration(migrations.Migration): ("id", models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name="ID")), ("created_at", models.DateTimeField(auto_now_add=True)), ("updated_at", models.DateTimeField(auto_now=True)), - ("email", models.EmailField(blank=True, db_index=True, help_text="Email", max_length=254)), - ("notes", models.TextField(blank=True, help_text="Notes")), + ("email", models.EmailField(db_index=True, help_text="Email", max_length=254)), + ("notes", models.TextField(help_text="Notes")), ( "requestor", models.ForeignKey( diff --git a/src/registrar/models/very_important_person.py b/src/registrar/models/very_important_person.py index 2fb53ff27..3d6430251 100644 --- a/src/registrar/models/very_important_person.py +++ b/src/registrar/models/very_important_person.py @@ -9,7 +9,7 @@ class VeryImportantPerson(TimeStampedModel): email = models.EmailField( null=False, - blank=True, + blank=False, help_text="Email", db_index=True, ) @@ -24,7 +24,7 @@ class VeryImportantPerson(TimeStampedModel): notes = models.TextField( null=False, - blank=True, + blank=False, help_text="Notes", ) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index e0d0a6be5..7c9aa8fe4 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1748,7 +1748,7 @@ class VeryImportantPersonAdminTestCase(TestCase): def test_save_model_sets_user_field(self): self.client.force_login(self.superuser) - + # Create an instance of the admin class admin_instance = VeryImportantPersonAdmin(model=VeryImportantPerson, admin_site=None) @@ -1757,7 +1757,7 @@ class VeryImportantPersonAdminTestCase(TestCase): # Create a request object request = self.factory.post("/admin/yourapp/veryimportantperson/add/") - request.requestor = self.superuser + request.user = self.superuser # Call the save_model method admin_instance.save_model(request, vip_instance, None, None) From e88c50c173a9613fa9cea2ceaf5138d48617e9fb Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 24 Jan 2024 13:11:00 -0500 Subject: [PATCH 11/13] add class description for vip --- src/registrar/admin.py | 2 +- src/registrar/models/very_important_person.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index dedc6f527..d64296e56 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1251,7 +1251,7 @@ class VeryImportantPersonAdmin(ListHeaderAdmin): ] def truncated_notes(self, obj): - # Truncate the 'notes' field to 200 characters + # Truncate the 'notes' field to 50 characters return str(obj.notes)[:50] truncated_notes.short_description = "Notes (Truncated)" # type: ignore diff --git a/src/registrar/models/very_important_person.py b/src/registrar/models/very_important_person.py index 3d6430251..9134cb893 100644 --- a/src/registrar/models/very_important_person.py +++ b/src/registrar/models/very_important_person.py @@ -5,7 +5,7 @@ from .utility.time_stamped_model import TimeStampedModel class VeryImportantPerson(TimeStampedModel): - """""" + """emails that get added to this table will bypass ial2 on login.""" email = models.EmailField( null=False, From e9160e67f8d1e510a1c20db6d373ffb829666a9b Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 24 Jan 2024 13:23:56 -0500 Subject: [PATCH 12/13] Change requester to requestor in code --- .../templates/emails/domain_invitation.txt | 2 +- src/registrar/tests/test_views.py | 12 ++++----- src/registrar/views/domain.py | 26 +++++++++---------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/registrar/templates/emails/domain_invitation.txt b/src/registrar/templates/emails/domain_invitation.txt index b9e4ba853..8896bd85f 100644 --- a/src/registrar/templates/emails/domain_invitation.txt +++ b/src/registrar/templates/emails/domain_invitation.txt @@ -1,7 +1,7 @@ {% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} Hi. -{{ requester_email }} has added you as a manager on {{ domain.name }}. +{{ requestor_email }} has added you as a manager on {{ domain.name }}. You can manage this domain on the .gov registrar . diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 60ffab416..a6cc3f0f8 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -2590,7 +2590,7 @@ class TestDomainManagers(TestDomainOverview): ) @boto3_mocking.patching - def test_domain_invitation_email_has_email_as_requester_non_existent(self): + def test_domain_invitation_email_has_email_as_requestor_non_existent(self): """Inviting a non existent user sends them an email, with email as the name.""" # make sure there is no user with this email email_address = "mayor@igorville.gov" @@ -2623,13 +2623,13 @@ class TestDomainManagers(TestDomainOverview): email_content = kwargs["Content"]["Simple"]["Body"]["Text"]["Data"] self.assertIn("info@example.com", email_content) - # Check that the requesters first/last name do not exist + # Check that the requestors first/last name do not exist self.assertNotIn("First", email_content) self.assertNotIn("Last", email_content) self.assertNotIn("First Last", email_content) @boto3_mocking.patching - def test_domain_invitation_email_has_email_as_requester(self): + def test_domain_invitation_email_has_email_as_requestor(self): """Inviting a user sends them an email, with email as the name.""" # Create a fake user object email_address = "mayor@igorville.gov" @@ -2662,13 +2662,13 @@ class TestDomainManagers(TestDomainOverview): email_content = kwargs["Content"]["Simple"]["Body"]["Text"]["Data"] self.assertIn("info@example.com", email_content) - # Check that the requesters first/last name do not exist + # Check that the requestors first/last name do not exist self.assertNotIn("First", email_content) self.assertNotIn("Last", email_content) self.assertNotIn("First Last", email_content) @boto3_mocking.patching - def test_domain_invitation_email_has_email_as_requester_staff(self): + def test_domain_invitation_email_has_email_as_requestor_staff(self): """Inviting a user sends them an email, with email as the name.""" # Create a fake user object email_address = "mayor@igorville.gov" @@ -2705,7 +2705,7 @@ class TestDomainManagers(TestDomainOverview): email_content = kwargs["Content"]["Simple"]["Body"]["Text"]["Data"] self.assertIn("help@get.gov", email_content) - # Check that the requesters first/last name do not exist + # Check that the requestors first/last name do not exist self.assertNotIn("First", email_content) self.assertNotIn("Last", email_content) self.assertNotIn("First Last", email_content) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 4d47a6f59..2d634dbf2 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -646,7 +646,7 @@ class DomainAddUserView(DomainFormBaseView): """Get an absolute URL for this domain.""" return self.request.build_absolute_uri(reverse("domain", kwargs={"pk": self.object.id})) - def _send_domain_invitation_email(self, email: str, requester: User, add_success=True): + def _send_domain_invitation_email(self, email: str, requestor: User, add_success=True): """Performs the sending of the domain invitation email, does not make a domain information object email: string- email to send to @@ -654,16 +654,16 @@ class DomainAddUserView(DomainFormBaseView): adding a success message to the view if the email sending succeeds""" # Set a default email address to send to for staff - requester_email = "help@get.gov" + requestor_email = "help@get.gov" - # Check if the email requester has a valid email address - if not requester.is_staff and requester.email is not None and requester.email.strip() != "": - requester_email = requester.email - elif not requester.is_staff: + # Check if the email requestor has a valid email address + if not requestor.is_staff and requestor.email is not None and requestor.email.strip() != "": + requestor_email = requestor.email + elif not requestor.is_staff: messages.error(self.request, "Can't send invitation email. No email is associated with your account.") logger.error( f"Can't send email to '{email}' on domain '{self.object}'." - f"No email exists for the requester '{requester.username}'.", + f"No email exists for the requestor '{requestor.username}'.", exc_info=True, ) return None @@ -676,7 +676,7 @@ class DomainAddUserView(DomainFormBaseView): context={ "domain_url": self._domain_abs_url(), "domain": self.object, - "requester_email": requester_email, + "requestor_email": requestor_email, }, ) except EmailSendingError: @@ -691,7 +691,7 @@ class DomainAddUserView(DomainFormBaseView): if add_success: messages.success(self.request, f"{email} has been invited to this domain.") - def _make_invitation(self, email_address: str, requester: User): + def _make_invitation(self, email_address: str, requestor: User): """Make a Domain invitation for this email and redirect with a message.""" invitation, created = DomainInvitation.objects.get_or_create(email=email_address, domain=self.object) if not created: @@ -701,22 +701,22 @@ class DomainAddUserView(DomainFormBaseView): f"{email_address} has already been invited to this domain.", ) else: - self._send_domain_invitation_email(email=email_address, requester=requester) + self._send_domain_invitation_email(email=email_address, requestor=requestor) return redirect(self.get_success_url()) def form_valid(self, form): """Add the specified user on this domain.""" requested_email = form.cleaned_data["email"] - requester = self.request.user + requestor = self.request.user # look up a user with that email try: requested_user = User.objects.get(email=requested_email) except User.DoesNotExist: # no matching user, go make an invitation - return self._make_invitation(requested_email, requester) + return self._make_invitation(requested_email, requestor) else: # if user already exists then just send an email - self._send_domain_invitation_email(requested_email, requester, add_success=False) + self._send_domain_invitation_email(requested_email, requestor, add_success=False) try: UserDomainRole.objects.create( From 31dc26476c007a5553565181ee0698ba5270fa75 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Thu, 25 Jan 2024 14:50:22 -0500 Subject: [PATCH 13/13] migration fix --- ...y => 0064_alter_domainapplication_address_line1_and_more.py} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename src/registrar/migrations/{0063_alter_domainapplication_address_line1_and_more.py => 0064_alter_domainapplication_address_line1_and_more.py} (93%) diff --git a/src/registrar/migrations/0063_alter_domainapplication_address_line1_and_more.py b/src/registrar/migrations/0064_alter_domainapplication_address_line1_and_more.py similarity index 93% rename from src/registrar/migrations/0063_alter_domainapplication_address_line1_and_more.py rename to src/registrar/migrations/0064_alter_domainapplication_address_line1_and_more.py index 99ffac7aa..7241c7164 100644 --- a/src/registrar/migrations/0063_alter_domainapplication_address_line1_and_more.py +++ b/src/registrar/migrations/0064_alter_domainapplication_address_line1_and_more.py @@ -5,7 +5,7 @@ from django.db import migrations, models class Migration(migrations.Migration): dependencies = [ - ("registrar", "0062_alter_host_name"), + ("registrar", "0063_veryimportantperson"), ] operations = [