From d268ef54b19e108bbdf8c40080fe4ca6cbe94b22 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 9 May 2024 11:42:18 -0600 Subject: [PATCH] Basic setup stuff --- src/djangooidc/backends.py | 10 ++++-- src/djangooidc/tests/test_backends.py | 10 +++--- src/djangooidc/views.py | 13 ++++++-- .../migrations/0094_user_finished_setup.py | 18 ++++++++++ src/registrar/models/user.py | 7 ++++ src/registrar/views/domain_request.py | 9 ++++- src/registrar/views/utility/mixins.py | 33 +++++++++++++++++++ .../views/utility/permission_views.py | 23 +++++++++++-- 8 files changed, 110 insertions(+), 13 deletions(-) create mode 100644 src/registrar/migrations/0094_user_finished_setup.py diff --git a/src/djangooidc/backends.py b/src/djangooidc/backends.py index 41e442f2d..8bdd44698 100644 --- a/src/djangooidc/backends.py +++ b/src/djangooidc/backends.py @@ -21,10 +21,13 @@ class OpenIdConnectBackend(ModelBackend): """ def authenticate(self, request, **kwargs): + """Returns a tuple of (User, is_new_user)""" logger.debug("kwargs %s" % kwargs) user = None + is_new_user = True + if not kwargs or "sub" not in kwargs.keys(): - return user + return user, is_new_user UserModel = get_user_model() username = self.clean_username(kwargs["sub"]) @@ -48,6 +51,7 @@ class OpenIdConnectBackend(ModelBackend): } user, created = UserModel.objects.get_or_create(**args) + is_new_user = created if not created: # If user exists, update existing user @@ -59,10 +63,10 @@ class OpenIdConnectBackend(ModelBackend): try: user = UserModel.objects.get_by_natural_key(username) except UserModel.DoesNotExist: - return None + return None, is_new_user # run this callback for a each login user.on_each_login() - return user + return user, is_new_user def update_existing_user(self, user, kwargs): """ diff --git a/src/djangooidc/tests/test_backends.py b/src/djangooidc/tests/test_backends.py index c15106fa9..7b7b963ea 100644 --- a/src/djangooidc/tests/test_backends.py +++ b/src/djangooidc/tests/test_backends.py @@ -21,7 +21,7 @@ class OpenIdConnectBackendTestCase(TestCase): """Test that authenticate creates a new user if it does not find existing user""" # Ensure that the authenticate method creates a new user - user = self.backend.authenticate(request=None, **self.kwargs) + user, _ = self.backend.authenticate(request=None, **self.kwargs) self.assertIsNotNone(user) self.assertIsInstance(user, User) self.assertEqual(user.username, "test_user") @@ -39,7 +39,7 @@ class OpenIdConnectBackendTestCase(TestCase): existing_user = User.objects.create_user(username="test_user") # Ensure that the authenticate method updates the existing user - user = self.backend.authenticate(request=None, **self.kwargs) + user, _ = self.backend.authenticate(request=None, **self.kwargs) self.assertIsNotNone(user) self.assertIsInstance(user, User) self.assertEqual(user, existing_user) # The same user instance should be returned @@ -68,7 +68,7 @@ class OpenIdConnectBackendTestCase(TestCase): # Ensure that the authenticate method updates the existing user # and preserves existing first and last names - user = self.backend.authenticate(request=None, **self.kwargs) + user, _ = self.backend.authenticate(request=None, **self.kwargs) self.assertIsNotNone(user) self.assertIsInstance(user, User) self.assertEqual(user, existing_user) # The same user instance should be returned @@ -89,7 +89,7 @@ class OpenIdConnectBackendTestCase(TestCase): # Ensure that the authenticate method updates the existing user # and preserves existing first and last names - user = self.backend.authenticate(request=None, **self.kwargs) + user, _ = self.backend.authenticate(request=None, **self.kwargs) self.assertIsNotNone(user) self.assertIsInstance(user, User) self.assertEqual(user, existing_user) # The same user instance should be returned @@ -103,5 +103,5 @@ class OpenIdConnectBackendTestCase(TestCase): 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 - user = self.backend.authenticate(request=None, **{}) + user, _ = self.backend.authenticate(request=None, **{}) self.assertIsNone(user) diff --git a/src/djangooidc/views.py b/src/djangooidc/views.py index 815df4ecf..c7a8f1bba 100644 --- a/src/djangooidc/views.py +++ b/src/djangooidc/views.py @@ -85,6 +85,7 @@ def login_callback(request): """Analyze the token returned by the authentication provider (OP).""" global CLIENT try: + request.session["is_new_user"] = False # If the CLIENT is none, attempt to reinitialize before handling the request if _client_is_none(): logger.debug("OIDC client is None, attempting to initialize") @@ -97,9 +98,9 @@ def login_callback(request): # add acr_value to request.session request.session["acr_value"] = CLIENT.get_step_up_acr_value() return CLIENT.create_authn_request(request.session) - user = authenticate(request=request, **userinfo) + user, is_new_user = authenticate(request=request, **userinfo) if user: - + 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 @@ -110,9 +111,17 @@ def login_callback(request): # 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 is_new_user: + user.finished_setup = False + should_update_user = True + + if should_update_user: user.save() login(request, user) + logger.info("Successfully logged in user %s" % user) # Clear the flag if the exception is not caught diff --git a/src/registrar/migrations/0094_user_finished_setup.py b/src/registrar/migrations/0094_user_finished_setup.py new file mode 100644 index 000000000..660f950c0 --- /dev/null +++ b/src/registrar/migrations/0094_user_finished_setup.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.10 on 2024-05-09 17:42 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0093_alter_publiccontact_unique_together"), + ] + + 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 5e4c88f63..b3fd95eb3 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -80,6 +80,13 @@ 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 + ) + def __str__(self): # this info is pulled from Login.gov if self.first_name or self.last_name: diff --git a/src/registrar/views/domain_request.py b/src/registrar/views/domain_request.py index f93976138..6b0ef7223 100644 --- a/src/registrar/views/domain_request.py +++ b/src/registrar/views/domain_request.py @@ -14,7 +14,7 @@ from registrar.models.contact import Contact from registrar.models.user import User from registrar.utility import StrEnum from registrar.views.utility import StepsHelper -from registrar.views.utility.permission_views import DomainRequestPermissionDeleteView +from registrar.views.utility.permission_views import DomainRequestPermissionDeleteView, ContactPermissionView from .utility import ( DomainRequestPermissionView, @@ -819,3 +819,10 @@ class DomainRequestDeleteView(DomainRequestPermissionDeleteView): duplicates = [item for item, count in object_dict.items() if count > 1] return duplicates + + +class FinishContactProfileSetupView(ContactPermissionView): + """This view forces the user into providing additional details that + we may have missed from Login.gov""" + template_name = "domain_request_your_contact.html" + forms = [forms.YourContactForm] \ No newline at end of file diff --git a/src/registrar/views/utility/mixins.py b/src/registrar/views/utility/mixins.py index c7083ce48..4fdba113d 100644 --- a/src/registrar/views/utility/mixins.py +++ b/src/registrar/views/utility/mixins.py @@ -8,6 +8,7 @@ from registrar.models import ( DomainInvitation, DomainInformation, UserDomainRole, + Contact, ) import logging @@ -324,6 +325,38 @@ class UserDeleteDomainRolePermission(PermissionsLoginMixin): return True +class ContactPermission(PermissionsLoginMixin): + """Permission mixin for UserDomainRole if user + has access, otherwise 403""" + + def has_permission(self): + """Check if this user has access to this domain request. + + 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"] + """ + + # Check if the user is authenticated + if not self.request.user.is_authenticated: + return False + + user_pk = self.kwargs["pk"] + + # Check if the user has an associated contact + associated_contacts = Contact.objects.filter(user=user_pk) + associated_contacts_length = len(associated_contacts) + + if associated_contacts_length == 0: + # This means that the user trying to access this page + # is a different user than the contact holder. + return False + elif associated_contacts_length > 1: + # TODO - change this + raise ValueError("User has multiple connected contacts") + else: + return True + + class DomainRequestPermissionWithdraw(PermissionsLoginMixin): """Permission mixin that redirects to withdraw action on domain request if user has access, otherwise 403""" diff --git a/src/registrar/views/utility/permission_views.py b/src/registrar/views/utility/permission_views.py index f2752c3b5..c626367fe 100644 --- a/src/registrar/views/utility/permission_views.py +++ b/src/registrar/views/utility/permission_views.py @@ -3,8 +3,7 @@ import abc # abstract base class from django.views.generic import DetailView, DeleteView, TemplateView -from registrar.models import Domain, DomainRequest, DomainInvitation -from registrar.models.user_domain_role import UserDomainRole +from registrar.models import Domain, DomainRequest, DomainInvitation, UserDomainRole, Contact from .mixins import ( DomainPermission, @@ -13,6 +12,7 @@ from .mixins import ( DomainInvitationPermission, DomainRequestWizardPermission, UserDeleteDomainRolePermission, + ContactPermission, ) import logging @@ -142,3 +142,22 @@ class UserDomainRolePermissionDeleteView(UserDeleteDomainRolePermission, DeleteV # variable name in template context for the model object context_object_name = "userdomainrole" + + +class ContactPermissionView(ContactPermission, DetailView, abc.ABC): + """Abstract base view for domain requests that enforces permissions + + This abstract view cannot be instantiated. Actual views must specify + `template_name`. + """ + + # DetailView property for what model this is viewing + model = Contact + # variable name in template context for the model object + context_object_name = "Contact" + + # Abstract property enforces NotImplementedError on an attribute. + @property + @abc.abstractmethod + def template_name(self): + raise NotImplementedError \ No newline at end of file