diff --git a/src/djangooidc/views.py b/src/djangooidc/views.py index b07d3222b..bbf708d1c 100644 --- a/src/djangooidc/views.py +++ b/src/djangooidc/views.py @@ -99,9 +99,18 @@ def login_callback(request): return CLIENT.create_authn_request(request.session) user = authenticate(request=request, **userinfo) if user: - # Set login metadata about this user - # (verification_type for instance) - _set_authenticated_user_metadata(user) + + # Fixture users kind of exist in a superposition of verification types, + # because while the system "verified" them, if they login, + # we don't know how the user themselves was verified through login.gov until + # they actually try logging in. This edge-case only matters in non-production environments. + fixture_user = User.VerificationTypeChoices.FIXTURE_USER + is_fixture_user = user.verification_type and user.verification_type == fixture_user + + # Set the verification type if it doesn't already exist or if its a fixture user + if not user.verification_type or is_fixture_user: + user.set_user_verification_type() + user.save() login(request, user) @@ -131,26 +140,6 @@ def login_callback(request): return error_page(request, err) -def _set_authenticated_user_metadata(user): - """Does checks on the recieved authenticated user from login_callback, - and updates fields accordingly.""" - should_update_user = False - # Fixture users kind of exist in a superposition of verification types, - # because while the system "verified" them, if they login, - # we don't know how the user themselves was verified through login.gov until - # they actually try logging in. This edge-case only matters in non-production environments. - fixture_user = User.VerificationTypeChoices.FIXTURE_USER - is_fixture_user = user.verification_type and user.verification_type == fixture_user - - # Set the verification type if it doesn't already exist or if its a fixture user - if not user.verification_type or is_fixture_user: - user.set_user_verification_type() - should_update_user = True - - if should_update_user: - user.save() - - def _requires_step_up_auth(userinfo): """if User.needs_identity_verification and step_up_acr_value not in ial returned from callback, return True""" diff --git a/src/registrar/assets/sass/_theme/_base.scss b/src/registrar/assets/sass/_theme/_base.scss index 4d5b9b822..cd2de3a71 100644 --- a/src/registrar/assets/sass/_theme/_base.scss +++ b/src/registrar/assets/sass/_theme/_base.scss @@ -171,7 +171,6 @@ abbr[title] { cursor: pointer; } -// todo this class should ideally be renamed .input-with-edit-button { svg.usa-icon { width: 1.5em !important; @@ -208,4 +207,4 @@ abbr[title] { background-color: white; padding-right: 8px; } -} \ No newline at end of file +} diff --git a/src/registrar/assets/sass/_theme/_forms.scss b/src/registrar/assets/sass/_theme/_forms.scss index bc8f631f5..46d41059a 100644 --- a/src/registrar/assets/sass/_theme/_forms.scss +++ b/src/registrar/assets/sass/_theme/_forms.scss @@ -31,7 +31,6 @@ } .usa-form-readonly { - // todo update border-top: 2px #{$dhs-dark-gray-15} solid; .bold-usa-label label.usa-label{ @@ -53,6 +52,12 @@ margin-top: unset; } +@media (min-width: 35em) { + .usa-form--largest { + max-width: 35rem; + } +} + .usa-form-group--unstyled-error { margin-left: 0; padding-left: 0; @@ -66,13 +71,6 @@ legend.float-left-tablet + button.float-right-tablet { } } -@media (min-width: 35em) { - .usa-form--largest { - max-width: 35rem; - } -} - - // Custom style for disabled inputs @media (prefers-color-scheme: light) { .usa-input:disabled, .usa-select:disabled, .usa-textarea:disabled { diff --git a/src/registrar/assets/sass/_theme/_headers.scss b/src/registrar/assets/sass/_theme/_headers.scss index 36868fdd3..2e3992e5a 100644 --- a/src/registrar/assets/sass/_theme/_headers.scss +++ b/src/registrar/assets/sass/_theme/_headers.scss @@ -11,4 +11,4 @@ .usa-logo button.usa-button--unstyled.disabled-button:hover{ color: #{$dhs-dark-gray-85}; -} \ No newline at end of file +} diff --git a/src/registrar/assets/sass/_theme/_tooltips.scss b/src/registrar/assets/sass/_theme/_tooltips.scss index 38b2f5dbb..3ab630dc0 100644 --- a/src/registrar/assets/sass/_theme/_tooltips.scss +++ b/src/registrar/assets/sass/_theme/_tooltips.scss @@ -27,4 +27,4 @@ #extended-logo .usa-tooltip__body { font-weight: 400 !important; -} \ No newline at end of file +} diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index de7d9585d..17edb4148 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -179,8 +179,6 @@ urlpatterns = [ name="domain-users-add", ), path( - # We embed the current user ID here, but we have a permission check - # that ensures the user is who they say they are. "finish-profile-setup", views.FinishProfileSetupView.as_view(), name="finish-user-profile-setup", diff --git a/src/registrar/fixtures_users.py b/src/registrar/fixtures_users.py index d87438bc9..c31acacfd 100644 --- a/src/registrar/fixtures_users.py +++ b/src/registrar/fixtures_users.py @@ -126,11 +126,11 @@ class UserFixture: "last_name": "Osos-Analyst", "email": "kosos@truss.works", }, - # { - # "username": "2cc0cde8-8313-4a50-99d8-5882e71443e8", - # "first_name": "Zander-Analyst", - # "last_name": "Adkinson-Analyst", - # }, + { + "username": "2cc0cde8-8313-4a50-99d8-5882e71443e8", + "first_name": "Zander-Analyst", + "last_name": "Adkinson-Analyst", + }, { "username": "57ab5847-7789-49fe-a2f9-21d38076d699", "first_name": "Paul-Analyst", diff --git a/src/registrar/models/contact.py b/src/registrar/models/contact.py index 0f55ed863..a5a6ff16c 100644 --- a/src/registrar/models/contact.py +++ b/src/registrar/models/contact.py @@ -1,6 +1,7 @@ from django.db import models from .utility.time_stamped_model import TimeStampedModel + from phonenumber_field.modelfields import PhoneNumberField # type: ignore diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 0b2162710..6137b4c4b 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -272,6 +272,7 @@ class CreateOrUpdateOrganizationTypeHelper: def replace_url_queryparams(url_to_modify: str, query_params: dict[Any, list]): """ Replaces the query parameters of a given URL. + Because this replaces them, this can be used to either add, delete, or modify. Args: url_to_modify (str): The URL whose query parameters need to be modified. diff --git a/src/registrar/registrar_middleware.py b/src/registrar/registrar_middleware.py index eb5ec7df0..f9921513b 100644 --- a/src/registrar/registrar_middleware.py +++ b/src/registrar/registrar_middleware.py @@ -10,6 +10,23 @@ from waffle.decorators import flag_is_active from registrar.models.utility.generic_helper import replace_url_queryparams +class NoCacheMiddleware: + """ + Middleware to add Cache-control: no-cache to every response. + + Used to force Cloudfront caching to leave us alone while we develop + better caching responses. + """ + + def __init__(self, get_response): + self.get_response = get_response + + def __call__(self, request): + response = self.get_response(request) + response["Cache-Control"] = "no-cache" + return response + + class CheckUserProfileMiddleware: """ Checks if the current user has finished_setup = False. @@ -20,8 +37,14 @@ class CheckUserProfileMiddleware: def __init__(self, get_response): self.get_response = get_response + self.setup_page = reverse("finish-user-profile-setup") + self.logout_page = reverse("logout") + self.excluded_pages = [ + self.setup_page, + self.logout_page, + ] + def __call__(self, request): - """Code that gets executed on each request before the view is called""" response = self.get_response(request) return response @@ -53,19 +76,13 @@ class CheckUserProfileMiddleware: Otherwise, we assume they want to go to the home page. """ - setup_page = reverse("finish-user-profile-setup") - logout_page = reverse("logout") - excluded_pages = [ - setup_page, - logout_page, - ] # In some cases, we don't want to redirect to home. This handles that. # Can easily be generalized if need be, but for now lets keep this easy to read. custom_redirect = "domain-request:" if request.path == "/request/" else None # Don't redirect on excluded pages (such as the setup page itself) - if not any(request.path.startswith(page) for page in excluded_pages): + if not any(request.path.startswith(page) for page in self.excluded_pages): # Preserve the original query parameters, and coerce them into a dict query_params = parse_qs(request.META["QUERY_STRING"]) @@ -75,28 +92,9 @@ class CheckUserProfileMiddleware: query_params["redirect"] = custom_redirect # Add our new query param, while preserving old ones - if query_params: - setup_page = replace_url_queryparams(setup_page, query_params) + new_setup_page = replace_url_queryparams(self.setup_page, query_params) if query_params else self.setup_page - # Redirect to the setup page - return HttpResponseRedirect(setup_page) + return HttpResponseRedirect(new_setup_page) else: # Process the view as normal return None - - -class NoCacheMiddleware: - """ - Middleware to add Cache-control: no-cache to every response. - - Used to force Cloudfront caching to leave us alone while we develop - better caching responses. - """ - - def __init__(self, get_response): - self.get_response = get_response - - def __call__(self, request): - response = self.get_response(request) - response["Cache-Control"] = "no-cache" - return response diff --git a/src/registrar/templates/includes/finish_profile_form.html b/src/registrar/templates/includes/finish_profile_form.html index 8b1de87b3..1d520b64a 100644 --- a/src/registrar/templates/includes/finish_profile_form.html +++ b/src/registrar/templates/includes/finish_profile_form.html @@ -84,4 +84,4 @@ -{% endblock profile_form %} \ No newline at end of file +{% endblock profile_form %} diff --git a/src/registrar/templates/includes/label_with_edit_button.html b/src/registrar/templates/includes/label_with_edit_button.html index b1175ab64..7f835cbeb 100644 --- a/src/registrar/templates/includes/label_with_edit_button.html +++ b/src/registrar/templates/includes/label_with_edit_button.html @@ -11,4 +11,4 @@ Edit - \ No newline at end of file + diff --git a/src/registrar/templates/includes/profile_form.html b/src/registrar/templates/includes/profile_form.html index 378bb66a4..ce337b4d5 100644 --- a/src/registrar/templates/includes/profile_form.html +++ b/src/registrar/templates/includes/profile_form.html @@ -48,4 +48,4 @@ -{% endblock profile_form %} \ No newline at end of file +{% endblock profile_form %} diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 6215d1fdc..045641ef9 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -523,6 +523,10 @@ class HomeTests(TestWithUser): class FinishUserProfileTests(TestWithUser, WebTest): """A series of tests that target the finish setup page for user profile""" + # csrf checks do not work well with WebTest. + # We disable them here. + csrf_checks = False + def setUp(self): super().setUp() self.user.title = None @@ -556,7 +560,8 @@ class FinishUserProfileTests(TestWithUser, WebTest): """Tests that a new user is redirected to the profile setup page when profile_feature is on""" self.app.set_user(self.incomplete_user.username) with override_flag("profile_feature", active=True): - # This will redirect the user to the setup page + # This will redirect the user to the setup page. + # Follow implicity checks if our redirect is working. finish_setup_page = self.app.get(reverse("home")).follow() self._set_session_cookie() @@ -578,10 +583,14 @@ class FinishUserProfileTests(TestWithUser, WebTest): finish_setup_form["phone"] = "(201) 555-0123" finish_setup_form["title"] = "CEO" finish_setup_form["last_name"] = "example" - completed_setup_page = self._submit_form_webtest(finish_setup_page.form, follow=True) + save_page = self._submit_form_webtest(finish_setup_form, follow=True) - self.assertEqual(completed_setup_page.status_code, 200) - # Assert that we're on the home page + self.assertEqual(save_page.status_code, 200) + self.assertContains(save_page, "Your profile has been updated.") + + # Try to navigate back to the home page. + # This is the same as clicking the back button. + completed_setup_page = self.app.get(reverse("home")) self.assertContains(completed_setup_page, "Manage your domain") @less_console_noise_decorator