Set finished_setup as property, remove id

This commit is contained in:
zandercymatics 2024-05-21 14:37:53 -06:00
parent c1d9cb1b13
commit 47de6f17a7
No known key found for this signature in database
GPG key ID: FF4636ABEC9682B7
11 changed files with 45 additions and 95 deletions

View file

@ -50,9 +50,6 @@ class OpenIdConnectBackend(ModelBackend):
user, created = UserModel.objects.get_or_create(**args) user, created = UserModel.objects.get_or_create(**args)
if created and request is not None:
request.session["is_new_user"] = True
if not created: if not created:
# If user exists, update existing user # If user exists, update existing user
self.update_existing_user(user, args["defaults"]) self.update_existing_user(user, args["defaults"])
@ -63,8 +60,6 @@ class OpenIdConnectBackend(ModelBackend):
try: try:
user = UserModel.objects.get_by_natural_key(username) user = UserModel.objects.get_by_natural_key(username)
except UserModel.DoesNotExist: except UserModel.DoesNotExist:
if request is not None:
request.session["is_new_user"] = True
return None return None
# run this callback for a each login # run this callback for a each login
user.on_each_login() user.on_each_login()

View file

@ -98,11 +98,10 @@ def login_callback(request):
request.session["acr_value"] = CLIENT.get_step_up_acr_value() request.session["acr_value"] = CLIENT.get_step_up_acr_value()
return CLIENT.create_authn_request(request.session) return CLIENT.create_authn_request(request.session)
user = authenticate(request=request, **userinfo) user = authenticate(request=request, **userinfo)
is_new_user = request.session.get("is_new_user", False)
if user: if user:
# Set login metadata about this user # Set login metadata about this user
# (verification_type for instance) # (verification_type for instance)
_set_authenticated_user_metadata(user, is_new_user) _set_authenticated_user_metadata(user)
login(request, user) login(request, user)
@ -132,7 +131,7 @@ def login_callback(request):
return error_page(request, err) return error_page(request, err)
def _set_authenticated_user_metadata(user, is_new_user): def _set_authenticated_user_metadata(user):
"""Does checks on the recieved authenticated user from login_callback, """Does checks on the recieved authenticated user from login_callback,
and updates fields accordingly.""" and updates fields accordingly."""
should_update_user = False should_update_user = False
@ -148,10 +147,6 @@ def _set_authenticated_user_metadata(user, is_new_user):
user.set_user_verification_type() user.set_user_verification_type()
should_update_user = True should_update_user = True
if is_new_user:
user.finished_setup = False
should_update_user = True
if should_update_user: if should_update_user:
user.save() user.save()

View file

@ -592,7 +592,7 @@ class MyUserAdmin(BaseUserAdmin, ImportExportModelAdmin):
fieldsets = ( fieldsets = (
( (
None, None,
{"fields": ("username", "password", "status", "finished_setup", "verification_type")}, {"fields": ("username", "password", "status", "verification_type")},
), ),
("Personal Info", {"fields": ("first_name", "last_name", "email")}), ("Personal Info", {"fields": ("first_name", "last_name", "email")}),
( (
@ -610,7 +610,7 @@ class MyUserAdmin(BaseUserAdmin, ImportExportModelAdmin):
("Important dates", {"fields": ("last_login", "date_joined")}), ("Important dates", {"fields": ("last_login", "date_joined")}),
) )
readonly_fields = ("verification_type", "finished_setup") readonly_fields = ("verification_type",)
# Hide Username (uuid), Groups and Permissions # Hide Username (uuid), Groups and Permissions
# Q: Now that we're using Groups and Permissions, # Q: Now that we're using Groups and Permissions,

View file

@ -181,7 +181,7 @@ urlpatterns = [
path( path(
# We embed the current user ID here, but we have a permission check # We embed the current user ID here, but we have a permission check
# that ensures the user is who they say they are. # that ensures the user is who they say they are.
"finish-profile-setup/<int:pk>", "finish-profile-setup",
views.FinishProfileSetupView.as_view(), views.FinishProfileSetupView.as_view(),
name="finish-user-profile-setup", name="finish-user-profile-setup",
), ),

View file

@ -1,18 +0,0 @@
# Generated by Django 4.2.10 on 2024-05-09 17:42
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
("registrar", "0094_create_groups_v12"),
]
operations = [
migrations.AddField(
model_name="user",
name="finished_setup",
field=models.BooleanField(default=True),
),
]

View file

@ -87,12 +87,23 @@ class User(AbstractUser):
help_text="The means through which this user was verified", help_text="The means through which this user was verified",
) )
# Tracks if the user finished their profile setup or not. This is so @property
# we can globally enforce that new users provide additional context before proceeding. def finished_setup(self):
finished_setup = models.BooleanField( """
# Default to true so we don't impact existing users. We set this to false downstream. Tracks if the user finished their profile setup or not. This is so
default=True we can globally enforce that new users provide additional account information before proceeding.
) """
# Change this to self once the user and contact objects are merged.
# For now, since they are linked, lets test on the underlying contact object.
user_info = self.contact # noqa
user_values = [
user_info.first_name,
user_info.last_name,
user_info.title,
user_info.phone,
]
return None not in user_values
def __str__(self): def __str__(self):
# this info is pulled from Login.gov # this info is pulled from Login.gov

View file

@ -26,7 +26,9 @@ class CheckUserProfileMiddleware:
return response return response
def process_view(self, request, view_func, view_args, view_kwargs): def process_view(self, request, view_func, view_args, view_kwargs):
"""Runs pre-processing logic for each view. Checks for the
finished_setup flag on the current user. If they haven't done so,
then we redirect them to the finish setup page."""
# Check that the user is "opted-in" to the profile feature flag # Check that the user is "opted-in" to the profile feature flag
has_profile_feature_flag = flag_is_active(request, "profile_feature") has_profile_feature_flag = flag_is_active(request, "profile_feature")
@ -34,26 +36,24 @@ class CheckUserProfileMiddleware:
if not has_profile_feature_flag: if not has_profile_feature_flag:
return None return None
# Check if setup is not finished if request.user.is_authenticated:
finished_setup = hasattr(request.user, "finished_setup") and request.user.finished_setup if hasattr(request.user, "finished_setup") and not request.user.finished_setup:
if hasattr(request.user, "finished_setup"): return self._handle_setup_not_finished(request)
user_values = [
request.user.contact.first_name,
request.user.contact.last_name,
request.user.contact.title,
request.user.contact.phone,
]
if None in user_values:
finished_setup = False
if request.user.is_authenticated and not finished_setup:
return self._handle_setup_not_finished(request)
# Continue processing the view # Continue processing the view
return None return None
def _handle_setup_not_finished(self, request): def _handle_setup_not_finished(self, request):
setup_page = reverse("finish-user-profile-setup", kwargs={"pk": request.user.contact.pk}) """Redirects the given user to the finish setup page.
We set the "redirect" query param equal to where the user wants to go.
If the user wants to go to '/request/', then we set that
information in the query param.
Otherwise, we assume they want to go to the home page.
"""
setup_page = reverse("finish-user-profile-setup")
logout_page = reverse("logout") logout_page = reverse("logout")
excluded_pages = [ excluded_pages = [
setup_page, setup_page,
@ -66,13 +66,15 @@ class CheckUserProfileMiddleware:
# Don't redirect on excluded pages (such as the setup page itself) # 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 excluded_pages):
# Preserve the original query parameters, and coerce them into a dict # Preserve the original query parameters, and coerce them into a dict
query_params = parse_qs(request.META["QUERY_STRING"]) query_params = parse_qs(request.META["QUERY_STRING"])
# Set the redirect value to our redirect location
if custom_redirect is not None: if custom_redirect is not None:
# Set the redirect value to our redirect location
query_params["redirect"] = custom_redirect query_params["redirect"] = custom_redirect
# Add our new query param, while preserving old ones
if query_params: if query_params:
setup_page = replace_url_queryparams(setup_page, query_params) setup_page = replace_url_queryparams(setup_page, query_params)

View file

@ -155,7 +155,7 @@
{% if has_profile_feature_flag %} {% if has_profile_feature_flag %}
<li class="usa-nav__primary-item"> <li class="usa-nav__primary-item">
{% url 'user-profile' as user_profile_url %} {% url 'user-profile' as user_profile_url %}
{% url 'finish-user-profile-setup' pk=request.user.contact.pk as finish_setup_url %} {% url 'finish-user-profile-setup' as finish_setup_url %}
<a class="usa-nav-link {% if request.path == user_profile_url or request.path == finish_setup_url %}usa-current{% endif %}" href="{{ user_profile_url }}"> <a class="usa-nav-link {% if request.path == user_profile_url or request.path == finish_setup_url %}usa-current{% endif %}" href="{{ user_profile_url }}">
<span class="text-primary">Your profile</span> <span class="text-primary">Your profile</span>
</a> </a>

View file

@ -56,14 +56,14 @@ class TestWithUser(MockEppLib):
last_name = "Last" last_name = "Last"
email = "info@example.com" email = "info@example.com"
self.user = get_user_model().objects.create( self.user = get_user_model().objects.create(
username=username, first_name=first_name, last_name=last_name, email=email, finished_setup=True username=username, first_name=first_name, last_name=last_name, email=email
) )
username_incomplete = "test_user_incomplete" username_incomplete = "test_user_incomplete"
first_name_2 = "Incomplete" first_name_2 = "Incomplete"
email_2 = "unicorn@igorville.com" email_2 = "unicorn@igorville.com"
self.incomplete_user = get_user_model().objects.create( self.incomplete_user = get_user_model().objects.create(
username=username_incomplete, first_name=first_name_2, email=email_2, finished_setup=False username=username_incomplete, first_name=first_name_2, email=email_2
) )
def tearDown(self): def tearDown(self):
@ -526,7 +526,6 @@ class FinishUserProfileTests(TestWithUser, WebTest):
def setUp(self): def setUp(self):
super().setUp() super().setUp()
self.user.title = None self.user.title = None
self.user.finished_setup = False
self.user.save() self.user.save()
self.client.force_login(self.user) self.client.force_login(self.user)
self.domain, _ = Domain.objects.get_or_create(name="sampledomain.gov", state=Domain.State.READY) self.domain, _ = Domain.objects.get_or_create(name="sampledomain.gov", state=Domain.State.READY)

View file

@ -178,12 +178,6 @@ class FinishProfileSetupView(UserProfileView):
# Get the current form and validate it # Get the current form and validate it
if form.is_valid(): if form.is_valid():
completed_states = [self.RedirectType.TO_SPECIFIC_PAGE, self.RedirectType.HOME]
if self.redirect_type in completed_states:
self.request.user.finished_setup = True
self.request.user.save()
if "contact_setup_save_button" in request.POST: if "contact_setup_save_button" in request.POST:
# Logic for when the 'Save' button is clicked # Logic for when the 'Save' button is clicked
self.redirect_type = self.RedirectType.COMPLETE_SETUP self.redirect_type = self.RedirectType.COMPLETE_SETUP
@ -197,16 +191,6 @@ class FinishProfileSetupView(UserProfileView):
else: else:
return self.form_invalid(form) return self.form_invalid(form)
def form_valid(self, form):
"""Saves the current contact to the database, and if the user is complete
with their setup, then we mark user.finished_setup to True."""
completed_states = [self.RedirectType.TO_SPECIFIC_PAGE, self.RedirectType.HOME]
if self.redirect_type in completed_states:
self.request.user.finished_setup = True
self.request.user.save()
return super().form_valid(form)
def get_success_url(self): def get_success_url(self):
"""Redirect to the nameservers page for the domain.""" """Redirect to the nameservers page for the domain."""
redirect_url = self.get_redirect_url() redirect_url = self.get_redirect_url()

View file

@ -395,27 +395,9 @@ class UserProfilePermission(PermissionsLoginMixin):
If the user is authenticated, they have access If the user is authenticated, they have access
""" """
# Check if the user is authenticated # Check if the user is authenticated
if not self.request.user.is_authenticated: if not self.request.user.is_authenticated:
return False return False
# If we are given a pk in the request, do checks on it
given_contact_pk = self.kwargs.get("pk", None)
if given_contact_pk:
# Grab the user in the DB to do a full object comparision, not just on ids
current_user = self.request.user
# Compare the PK that was passed in to the user currently logged in
if current_user.contact.pk != given_contact_pk:
# Don't allow users to modify other users profiles
return False
# Check if the object at the id we're searching on actually exists
requested_user_exists = User.objects.filter(pk=current_user.pk).exists()
requested_contact_exists = Contact.objects.filter(user=current_user.pk, pk=given_contact_pk).exists()
if not requested_user_exists or not requested_contact_exists:
return False
return True return True