diff --git a/src/djangooidc/backends.py b/src/djangooidc/backends.py index 8e4eb023d..25179f7fd 100644 --- a/src/djangooidc/backends.py +++ b/src/djangooidc/backends.py @@ -50,9 +50,6 @@ class OpenIdConnectBackend(ModelBackend): 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 user exists, update existing user self.update_existing_user(user, args["defaults"]) @@ -63,8 +60,6 @@ class OpenIdConnectBackend(ModelBackend): try: user = UserModel.objects.get_by_natural_key(username) except UserModel.DoesNotExist: - if request is not None: - request.session["is_new_user"] = True return None # run this callback for a each login user.on_each_login() diff --git a/src/djangooidc/views.py b/src/djangooidc/views.py index ca91058fb..b07d3222b 100644 --- a/src/djangooidc/views.py +++ b/src/djangooidc/views.py @@ -98,11 +98,10 @@ def login_callback(request): request.session["acr_value"] = CLIENT.get_step_up_acr_value() return CLIENT.create_authn_request(request.session) user = authenticate(request=request, **userinfo) - is_new_user = request.session.get("is_new_user", False) if user: # Set login metadata about this user # (verification_type for instance) - _set_authenticated_user_metadata(user, is_new_user) + _set_authenticated_user_metadata(user) login(request, user) @@ -132,7 +131,7 @@ def login_callback(request): 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, and updates fields accordingly.""" should_update_user = False @@ -148,10 +147,6 @@ def _set_authenticated_user_metadata(user, is_new_user): user.set_user_verification_type() should_update_user = True - if is_new_user: - user.finished_setup = False - should_update_user = True - if should_update_user: user.save() diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 8cf3286ce..9905cf340 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -592,7 +592,7 @@ class MyUserAdmin(BaseUserAdmin, ImportExportModelAdmin): fieldsets = ( ( None, - {"fields": ("username", "password", "status", "finished_setup", "verification_type")}, + {"fields": ("username", "password", "status", "verification_type")}, ), ("Personal Info", {"fields": ("first_name", "last_name", "email")}), ( @@ -610,7 +610,7 @@ class MyUserAdmin(BaseUserAdmin, ImportExportModelAdmin): ("Important dates", {"fields": ("last_login", "date_joined")}), ) - readonly_fields = ("verification_type", "finished_setup") + readonly_fields = ("verification_type",) # Hide Username (uuid), Groups and Permissions # Q: Now that we're using Groups and Permissions, diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index 490b64958..de7d9585d 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -181,7 +181,7 @@ urlpatterns = [ 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/", + "finish-profile-setup", views.FinishProfileSetupView.as_view(), name="finish-user-profile-setup", ), diff --git a/src/registrar/migrations/0095_user_finished_setup.py b/src/registrar/migrations/0095_user_finished_setup.py deleted file mode 100644 index 87e247330..000000000 --- a/src/registrar/migrations/0095_user_finished_setup.py +++ /dev/null @@ -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), - ), - ] diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 975c9bf6a..f6400ccd2 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -87,12 +87,23 @@ class User(AbstractUser): help_text="The means through which this user was verified", ) - # Tracks if the user finished their profile setup or not. This is so - # we can globally enforce that new users provide additional context before proceeding. - finished_setup = models.BooleanField( - # Default to true so we don't impact existing users. We set this to false downstream. - default=True - ) + @property + def finished_setup(self): + """ + Tracks if the user finished their profile setup or not. This is so + 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): # this info is pulled from Login.gov diff --git a/src/registrar/registrar_middleware.py b/src/registrar/registrar_middleware.py index 823fa6f66..8485f25e6 100644 --- a/src/registrar/registrar_middleware.py +++ b/src/registrar/registrar_middleware.py @@ -26,7 +26,9 @@ class CheckUserProfileMiddleware: return response 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 has_profile_feature_flag = flag_is_active(request, "profile_feature") @@ -34,26 +36,24 @@ class CheckUserProfileMiddleware: if not has_profile_feature_flag: return None - # Check if setup is not finished - finished_setup = hasattr(request.user, "finished_setup") and request.user.finished_setup - if hasattr(request.user, "finished_setup"): - 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) + if request.user.is_authenticated: + if hasattr(request.user, "finished_setup") and not request.user.finished_setup: + return self._handle_setup_not_finished(request) # Continue processing the view return None 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") excluded_pages = [ setup_page, @@ -66,13 +66,15 @@ class CheckUserProfileMiddleware: # Don't redirect on excluded pages (such as the setup page itself) if not any(request.path.startswith(page) for page in excluded_pages): + # Preserve the original query parameters, and coerce them into a dict query_params = parse_qs(request.META["QUERY_STRING"]) + # Set the redirect value to our redirect location if custom_redirect is not None: - # Set the redirect value to our redirect location 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) diff --git a/src/registrar/templates/base.html b/src/registrar/templates/base.html index e7abe89f0..d1a880e0d 100644 --- a/src/registrar/templates/base.html +++ b/src/registrar/templates/base.html @@ -155,7 +155,7 @@ {% if has_profile_feature_flag %}
  • {% 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 %} diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 2492423be..6215d1fdc 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -56,14 +56,14 @@ class TestWithUser(MockEppLib): last_name = "Last" email = "info@example.com" 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" first_name_2 = "Incomplete" email_2 = "unicorn@igorville.com" 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): @@ -526,7 +526,6 @@ class FinishUserProfileTests(TestWithUser, WebTest): def setUp(self): super().setUp() self.user.title = None - self.user.finished_setup = False self.user.save() self.client.force_login(self.user) self.domain, _ = Domain.objects.get_or_create(name="sampledomain.gov", state=Domain.State.READY) diff --git a/src/registrar/views/user_profile.py b/src/registrar/views/user_profile.py index dd997a794..ea803a646 100644 --- a/src/registrar/views/user_profile.py +++ b/src/registrar/views/user_profile.py @@ -178,12 +178,6 @@ class FinishProfileSetupView(UserProfileView): # Get the current form and validate it 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: # Logic for when the 'Save' button is clicked self.redirect_type = self.RedirectType.COMPLETE_SETUP @@ -197,16 +191,6 @@ class FinishProfileSetupView(UserProfileView): else: 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): """Redirect to the nameservers page for the domain.""" redirect_url = self.get_redirect_url() diff --git a/src/registrar/views/utility/mixins.py b/src/registrar/views/utility/mixins.py index 774507296..da5606d52 100644 --- a/src/registrar/views/utility/mixins.py +++ b/src/registrar/views/utility/mixins.py @@ -395,27 +395,9 @@ class UserProfilePermission(PermissionsLoginMixin): If the user is authenticated, they have access """ + # Check if the user is authenticated if not self.request.user.is_authenticated: 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