From caad00df188983273e8628d7e677e0e162cf1720 Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Thu, 26 Sep 2024 10:24:29 -0700 Subject: [PATCH 01/22] Block users invited to other orgs from being domain managers --- src/registrar/views/domain.py | 56 +++++++++++++++++++++++++++-------- 1 file changed, 43 insertions(+), 13 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index db0572bb3..dae3b60cd 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -21,8 +21,10 @@ from registrar.models import ( DomainRequest, DomainInformation, DomainInvitation, + PortfolioInvitation, User, UserDomainRole, + UserPortfolioPermission, PublicContact, ) from registrar.utility.enums import DefaultEmail @@ -38,6 +40,7 @@ from registrar.utility.errors import ( ) from registrar.models.utility.contact_error import ContactError from registrar.views.utility.permission_views import UserDomainRolePermissionDeleteView +from registrar.utility.waffle import flag_is_active_for_user from ..forms import ( SeniorOfficialContactForm, @@ -778,7 +781,14 @@ 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, requestor: User, add_success=True): + def _is_member_of_different_org(self, email, org): + """Verifies if an email belongs to a different organization as a member or invited member.""" + # Check if user is a member of a different organization + existing_org_permission = UserPortfolioPermission.objects.get(email=email) + print("Existing org permission: ", existing_org_permission) + return True + + def _send_domain_invitation_email(self, email: str, requestor: User, requested_user=None, add_success=True): """Performs the sending of the domain invitation email, does not make a domain information object email: string- email to send to @@ -803,6 +813,26 @@ class DomainAddUserView(DomainFormBaseView): ) return None + # Check is user is a member or invited member of a different org from this domain's org + print("org feature flag is active: ", flag_is_active_for_user(requestor, "organization_feature")) + if flag_is_active_for_user(requestor, "organization_feature"): + # Check if invited user is a member from a different org from this domain's org + existing_org_permission = UserPortfolioPermission.objects.filter(user=requested_user).first() + print("Existing org permission for requested email: ", existing_org_permission) + + existing_org_invitation = PortfolioInvitation.objects.filter(email=email).first() + requestor_org = UserPortfolioPermission.objects.get(user=requestor).portfolio + print("Requestor org: ", requestor_org) + if (existing_org_permission and existing_org_permission.portfolio != requestor_org) or \ + (existing_org_invitation and existing_org_invitation.portfolio != requestor_org): + add_success=False + messages.error( + self.request, + f"That email is already a member of another .gov organization.", + ) + raise Exception + + # Check to see if an invite has already been sent try: invite = DomainInvitation.objects.get(email=email, domain=self.object) @@ -868,7 +898,7 @@ class DomainAddUserView(DomainFormBaseView): else: # if user already exists then just send an email try: - self._send_domain_invitation_email(requested_email, requestor, add_success=False) + self._send_domain_invitation_email(requested_email, requestor, requested_user=requested_user, add_success=False) except EmailSendingError: logger.warn( "Could not send email invitation (EmailSendingError)", @@ -883,17 +913,17 @@ class DomainAddUserView(DomainFormBaseView): exc_info=True, ) messages.warning(self.request, "Could not send email invitation.") - - try: - UserDomainRole.objects.create( - user=requested_user, - domain=self.object, - role=UserDomainRole.Roles.MANAGER, - ) - except IntegrityError: - messages.warning(self.request, f"{requested_email} is already a manager for this domain") - else: - messages.success(self.request, f"Added user {requested_email}.") + else: + try: + UserDomainRole.objects.create( + user=requested_user, + domain=self.object, + role=UserDomainRole.Roles.MANAGER, + ) + except IntegrityError: + messages.warning(self.request, f"{requested_email} is already a manager for this domain") + else: + messages.success(self.request, f"Added user {requested_email}.") return redirect(self.get_success_url()) From 42de7f2bb79e76b9efcb242da853ed036c3959bc Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Thu, 26 Sep 2024 10:40:36 -0700 Subject: [PATCH 02/22] Add domain manager breadcrumb nav --- src/registrar/templates/domain_add_user.html | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/registrar/templates/domain_add_user.html b/src/registrar/templates/domain_add_user.html index b2f9fef24..320404fa9 100644 --- a/src/registrar/templates/domain_add_user.html +++ b/src/registrar/templates/domain_add_user.html @@ -4,6 +4,19 @@ {% block title %}Add a domain manager | {% endblock %} {% block domain_content %} + {% block breadcrumb %} + {% url 'domain-users' pk=domain.id as url %} + + {% endblock breadcrumb %}

Add a domain manager

You can add another user to help manage your domain. They will need to sign From 8b61eb1275f2d340c458898d22baccb1a7c53b4a Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Thu, 26 Sep 2024 10:43:32 -0700 Subject: [PATCH 03/22] Update add domain manager page content --- src/registrar/templates/domain_add_user.html | 4 ++-- src/registrar/views/domain.py | 12 +++++++----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/registrar/templates/domain_add_user.html b/src/registrar/templates/domain_add_user.html index 320404fa9..e95bacd76 100644 --- a/src/registrar/templates/domain_add_user.html +++ b/src/registrar/templates/domain_add_user.html @@ -19,8 +19,8 @@ {% endblock breadcrumb %}

Add a domain manager

-

You can add another user to help manage your domain. They will need to sign - in to the .gov registrar with their Login.gov account. +

You can add another user to help manage your domain. If they aren't an organization member they will + need to sign in to the .gov registrar with their Login.gov account.

diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index dae3b60cd..c3bbe037a 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -823,15 +823,15 @@ class DomainAddUserView(DomainFormBaseView): existing_org_invitation = PortfolioInvitation.objects.filter(email=email).first() requestor_org = UserPortfolioPermission.objects.get(user=requestor).portfolio print("Requestor org: ", requestor_org) - if (existing_org_permission and existing_org_permission.portfolio != requestor_org) or \ - (existing_org_invitation and existing_org_invitation.portfolio != requestor_org): - add_success=False + if (existing_org_permission and existing_org_permission.portfolio != requestor_org) or ( + existing_org_invitation and existing_org_invitation.portfolio != requestor_org + ): + add_success = False messages.error( self.request, f"That email is already a member of another .gov organization.", ) raise Exception - # Check to see if an invite has already been sent try: @@ -898,7 +898,9 @@ class DomainAddUserView(DomainFormBaseView): else: # if user already exists then just send an email try: - self._send_domain_invitation_email(requested_email, requestor, requested_user=requested_user, add_success=False) + self._send_domain_invitation_email( + requested_email, requestor, requested_user=requested_user, add_success=False + ) except EmailSendingError: logger.warn( "Could not send email invitation (EmailSendingError)", From b0fe698af2e1d6ae5bf586ff2345b48f7a75f98c Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Thu, 26 Sep 2024 14:45:10 -0700 Subject: [PATCH 04/22] Add error code for outside org members being added --- src/registrar/utility/errors.py | 8 ++++ src/registrar/views/domain.py | 66 ++++++++++++++++----------------- 2 files changed, 41 insertions(+), 33 deletions(-) diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index 8cb83c0ee..6a75091a6 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -23,6 +23,14 @@ class InvalidDomainError(ValueError): pass +class OutsideOrgMemberError(ValueError): + """ + Error raised when an org member tries adding a user from a different .gov org. + To be deleted when users can be members of multiple orgs. + """ + + pass + class ActionNotAllowed(Exception): """User accessed an action that is not allowed by the current state""" diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index c3bbe037a..92da57e06 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -35,6 +35,7 @@ from registrar.utility.errors import ( NameserverErrorCodes as nsErrorCodes, DsDataError, DsDataErrorCodes, + OutsideOrgMemberError, SecurityEmailError, SecurityEmailErrorCodes, ) @@ -781,12 +782,15 @@ class DomainAddUserView(DomainFormBaseView): """Get an absolute URL for this domain.""" return self.request.build_absolute_uri(reverse("domain", kwargs={"pk": self.object.id})) - def _is_member_of_different_org(self, email, org): + def _is_member_of_different_org(self, email, requested_user, org): """Verifies if an email belongs to a different organization as a member or invited member.""" - # Check if user is a member of a different organization - existing_org_permission = UserPortfolioPermission.objects.get(email=email) - print("Existing org permission: ", existing_org_permission) - return True + # Check if user is a already member of a different organization than the given org + existing_org_permission = UserPortfolioPermission.objects.filter(user=requested_user).first() + existing_org_invitation = PortfolioInvitation.objects.filter(email=email).first() + + return (existing_org_permission and existing_org_permission.portfolio != org) or ( + existing_org_invitation and existing_org_invitation.portfolio != org + ) def _send_domain_invitation_email(self, email: str, requestor: User, requested_user=None, add_success=True): """Performs the sending of the domain invitation email, @@ -814,24 +818,16 @@ class DomainAddUserView(DomainFormBaseView): return None # Check is user is a member or invited member of a different org from this domain's org - print("org feature flag is active: ", flag_is_active_for_user(requestor, "organization_feature")) if flag_is_active_for_user(requestor, "organization_feature"): # Check if invited user is a member from a different org from this domain's org - existing_org_permission = UserPortfolioPermission.objects.filter(user=requested_user).first() - print("Existing org permission for requested email: ", existing_org_permission) - - existing_org_invitation = PortfolioInvitation.objects.filter(email=email).first() requestor_org = UserPortfolioPermission.objects.get(user=requestor).portfolio - print("Requestor org: ", requestor_org) - if (existing_org_permission and existing_org_permission.portfolio != requestor_org) or ( - existing_org_invitation and existing_org_invitation.portfolio != requestor_org - ): + if self._is_member_of_different_org(email, requested_user, requestor_org): add_success = False messages.error( self.request, - f"That email is already a member of another .gov organization.", + "That email is already a member of another .gov organization.", ) - raise Exception + raise OutsideOrgMemberError # Check to see if an invite has already been sent try: @@ -847,10 +843,7 @@ class DomainAddUserView(DomainFormBaseView): add_success = False # else if it has been sent but not accepted messages.warning(self.request, f"{email} has already been invited to this domain") - except Exception: - logger.error("An error occured") - try: send_templated_email( "emails/domain_invitation.txt", "emails/domain_invitation_subject.txt", @@ -861,6 +854,11 @@ class DomainAddUserView(DomainFormBaseView): "requestor_email": requestor_email, }, ) + + if add_success: + messages.success(self.request, f"{email} has been invited to this domain.") + except Exception: + logger.error("An error occured") except EmailSendingError as exc: logger.warn( "Could not sent email invitation to %s for domain %s", @@ -869,9 +867,6 @@ class DomainAddUserView(DomainFormBaseView): exc_info=True, ) raise EmailSendingError("Could not send email invitation.") from exc - else: - if add_success: - messages.success(self.request, f"{email} has been invited to this domain.") def _make_invitation(self, email_address: str, requestor: User): """Make a Domain invitation for this email and redirect with a message.""" @@ -901,6 +896,14 @@ class DomainAddUserView(DomainFormBaseView): self._send_domain_invitation_email( requested_email, requestor, requested_user=requested_user, add_success=False ) + + UserDomainRole.objects.create( + user=requested_user, + domain=self.object, + role=UserDomainRole.Roles.MANAGER, + ) + + messages.success(self.request, f"Added user {requested_email}.") except EmailSendingError: logger.warn( "Could not send email invitation (EmailSendingError)", @@ -908,6 +911,12 @@ class DomainAddUserView(DomainFormBaseView): exc_info=True, ) messages.warning(self.request, "Could not send email invitation.") + except OutsideOrgMemberError: + logger.warn( + "Could not send email invitation to a user in a different org.", + self.object, + exc_info=True, + ) except Exception: logger.warn( "Could not send email invitation (Other Exception)", @@ -915,17 +924,8 @@ class DomainAddUserView(DomainFormBaseView): exc_info=True, ) messages.warning(self.request, "Could not send email invitation.") - else: - try: - UserDomainRole.objects.create( - user=requested_user, - domain=self.object, - role=UserDomainRole.Roles.MANAGER, - ) - except IntegrityError: - messages.warning(self.request, f"{requested_email} is already a manager for this domain") - else: - messages.success(self.request, f"Added user {requested_email}.") + except IntegrityError: + messages.warning(self.request, f"{requested_email} is already a manager for this domain") return redirect(self.get_success_url()) From d71069606b77bfb5e593d2f01659c89ffb732f7c Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Thu, 26 Sep 2024 14:51:50 -0700 Subject: [PATCH 05/22] Fix linting. Revert to original email sending logic --- src/registrar/utility/errors.py | 1 + src/registrar/views/domain.py | 56 +++++++++++++++++++++++---------- 2 files changed, 40 insertions(+), 17 deletions(-) diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index 6a75091a6..f12aba221 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -31,6 +31,7 @@ class OutsideOrgMemberError(ValueError): pass + class ActionNotAllowed(Exception): """User accessed an action that is not allowed by the current state""" diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 92da57e06..77c02d990 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -35,9 +35,9 @@ from registrar.utility.errors import ( NameserverErrorCodes as nsErrorCodes, DsDataError, DsDataErrorCodes, - OutsideOrgMemberError, SecurityEmailError, SecurityEmailErrorCodes, + OutsideOrgMemberError, ) from registrar.models.utility.contact_error import ContactError from registrar.views.utility.permission_views import UserDomainRolePermissionDeleteView @@ -859,6 +859,10 @@ class DomainAddUserView(DomainFormBaseView): messages.success(self.request, f"{email} has been invited to this domain.") except Exception: logger.error("An error occured") + except OutsideOrgMemberError: + logger.error( + "Could not send email. Can not invite member of a .gov organization to a different organization." + ) except EmailSendingError as exc: logger.warn( "Could not sent email invitation to %s for domain %s", @@ -868,6 +872,29 @@ class DomainAddUserView(DomainFormBaseView): ) raise EmailSendingError("Could not send email invitation.") from exc + try: + send_templated_email( + "emails/domain_invitation.txt", + "emails/domain_invitation_subject.txt", + to_address=email, + context={ + "domain_url": self._domain_abs_url(), + "domain": self.object, + "requestor_email": requestor_email, + }, + ) + except EmailSendingError as exc: + logger.warn( + "Could not sent email invitation to %s for domain %s", + email, + self.object, + exc_info=True, + ) + raise EmailSendingError("Could not send email invitation.") from exc + else: + if add_success: + messages.success(self.request, f"{email} has been invited to this domain.") + def _make_invitation(self, email_address: str, requestor: User): """Make a Domain invitation for this email and redirect with a message.""" try: @@ -896,14 +923,6 @@ class DomainAddUserView(DomainFormBaseView): self._send_domain_invitation_email( requested_email, requestor, requested_user=requested_user, add_success=False ) - - UserDomainRole.objects.create( - user=requested_user, - domain=self.object, - role=UserDomainRole.Roles.MANAGER, - ) - - messages.success(self.request, f"Added user {requested_email}.") except EmailSendingError: logger.warn( "Could not send email invitation (EmailSendingError)", @@ -911,12 +930,6 @@ class DomainAddUserView(DomainFormBaseView): exc_info=True, ) messages.warning(self.request, "Could not send email invitation.") - except OutsideOrgMemberError: - logger.warn( - "Could not send email invitation to a user in a different org.", - self.object, - exc_info=True, - ) except Exception: logger.warn( "Could not send email invitation (Other Exception)", @@ -924,8 +937,17 @@ class DomainAddUserView(DomainFormBaseView): exc_info=True, ) messages.warning(self.request, "Could not send email invitation.") - except IntegrityError: - messages.warning(self.request, f"{requested_email} is already a manager for this domain") + else: + try: + UserDomainRole.objects.create( + user=requested_user, + domain=self.object, + role=UserDomainRole.Roles.MANAGER, + ) + except IntegrityError: + messages.warning(self.request, f"{requested_email} is already a manager for this domain") + else: + messages.success(self.request, f"Added user {requested_email}.") return redirect(self.get_success_url()) From d66ff330572280fd7b6f935dcca32317e23ca2db Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Thu, 26 Sep 2024 14:52:59 -0700 Subject: [PATCH 06/22] Readd outside org member error handling --- src/registrar/views/domain.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 77c02d990..02019c601 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -930,6 +930,12 @@ class DomainAddUserView(DomainFormBaseView): exc_info=True, ) messages.warning(self.request, "Could not send email invitation.") + except OutsideOrgMemberError: + logger.warn( + "Could not send email. Can not invite member of a .gov organization to a different organization.", + self.object, + exc_info=True, + ) except Exception: logger.warn( "Could not send email invitation (Other Exception)", From 3d1781c4f657aa1152ad88b0df15a1c7a99c34d0 Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Thu, 26 Sep 2024 15:07:50 -0700 Subject: [PATCH 07/22] Fix linting --- src/registrar/views/domain.py | 59 ++++++++++------------------------- 1 file changed, 16 insertions(+), 43 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 02019c601..c2ca65bab 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -782,14 +782,15 @@ class DomainAddUserView(DomainFormBaseView): """Get an absolute URL for this domain.""" return self.request.build_absolute_uri(reverse("domain", kwargs={"pk": self.object.id})) - def _is_member_of_different_org(self, email, requested_user, org): + def _is_member_of_different_org(self, email, requestor, requested_user): """Verifies if an email belongs to a different organization as a member or invited member.""" - # Check if user is a already member of a different organization than the given org + # Check if user is a already member of a different organization than the requestor's org + requestor_org = UserPortfolioPermission.objects.get(user=requestor).portfolio existing_org_permission = UserPortfolioPermission.objects.filter(user=requested_user).first() existing_org_invitation = PortfolioInvitation.objects.filter(email=email).first() - return (existing_org_permission and existing_org_permission.portfolio != org) or ( - existing_org_invitation and existing_org_invitation.portfolio != org + return (existing_org_permission and existing_org_permission.portfolio != requestor_org) or ( + existing_org_invitation and existing_org_invitation.portfolio != requestor_org ) def _send_domain_invitation_email(self, email: str, requestor: User, requested_user=None, add_success=True): @@ -818,16 +819,15 @@ class DomainAddUserView(DomainFormBaseView): return None # Check is user is a member or invited member of a different org from this domain's org - if flag_is_active_for_user(requestor, "organization_feature"): - # Check if invited user is a member from a different org from this domain's org - requestor_org = UserPortfolioPermission.objects.get(user=requestor).portfolio - if self._is_member_of_different_org(email, requested_user, requestor_org): - add_success = False - messages.error( - self.request, - "That email is already a member of another .gov organization.", - ) - raise OutsideOrgMemberError + if flag_is_active_for_user(requestor, "organization_feature") and self._is_member_of_different_org( + email, requestor, requested_user + ): + add_success = False + messages.error( + self.request, + "That email is already a member of another .gov organization.", + ) + raise OutsideOrgMemberError # Check to see if an invite has already been sent try: @@ -843,34 +843,8 @@ class DomainAddUserView(DomainFormBaseView): add_success = False # else if it has been sent but not accepted messages.warning(self.request, f"{email} has already been invited to this domain") - - send_templated_email( - "emails/domain_invitation.txt", - "emails/domain_invitation_subject.txt", - to_address=email, - context={ - "domain_url": self._domain_abs_url(), - "domain": self.object, - "requestor_email": requestor_email, - }, - ) - - if add_success: - messages.success(self.request, f"{email} has been invited to this domain.") except Exception: logger.error("An error occured") - except OutsideOrgMemberError: - logger.error( - "Could not send email. Can not invite member of a .gov organization to a different organization." - ) - except EmailSendingError as exc: - logger.warn( - "Could not sent email invitation to %s for domain %s", - email, - self.object, - exc_info=True, - ) - raise EmailSendingError("Could not send email invitation.") from exc try: send_templated_email( @@ -883,6 +857,8 @@ class DomainAddUserView(DomainFormBaseView): "requestor_email": requestor_email, }, ) + if add_success: + messages.success(self.request, f"{email} has been invited to this domain.") except EmailSendingError as exc: logger.warn( "Could not sent email invitation to %s for domain %s", @@ -891,9 +867,6 @@ class DomainAddUserView(DomainFormBaseView): exc_info=True, ) raise EmailSendingError("Could not send email invitation.") from exc - else: - if add_success: - messages.success(self.request, f"{email} has been invited to this domain.") def _make_invitation(self, email_address: str, requestor: User): """Make a Domain invitation for this email and redirect with a message.""" From 117900cfb9f12d4ca65f007d212d5337acc4d2ce Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Mon, 30 Sep 2024 11:24:58 -0700 Subject: [PATCH 08/22] Revert to try else catch --- src/registrar/views/domain.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index c2ca65bab..5d7a840c7 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -857,8 +857,6 @@ class DomainAddUserView(DomainFormBaseView): "requestor_email": requestor_email, }, ) - if add_success: - messages.success(self.request, f"{email} has been invited to this domain.") except EmailSendingError as exc: logger.warn( "Could not sent email invitation to %s for domain %s", @@ -867,6 +865,9 @@ class DomainAddUserView(DomainFormBaseView): exc_info=True, ) raise EmailSendingError("Could not send email invitation.") from exc + else: + if add_success: + messages.success(self.request, f"{email} has been invited to this domain.") def _make_invitation(self, email_address: str, requestor: User): """Make a Domain invitation for this email and redirect with a message.""" From 09944e4ce00155104f0214ba14f122bad3c83e47 Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Tue, 8 Oct 2024 11:10:51 -0700 Subject: [PATCH 09/22] Fix form valid logic --- .../fixtures_user_portfolio_permissions.py | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/src/registrar/fixtures/fixtures_user_portfolio_permissions.py b/src/registrar/fixtures/fixtures_user_portfolio_permissions.py index 3c64eb6b5..6b6e137cd 100644 --- a/src/registrar/fixtures/fixtures_user_portfolio_permissions.py +++ b/src/registrar/fixtures/fixtures_user_portfolio_permissions.py @@ -1,4 +1,5 @@ import logging +import random from faker import Faker from django.db import transaction @@ -51,22 +52,23 @@ class UserPortfolioPermissionFixture: user_portfolio_permissions_to_create = [] for user in users: - for portfolio in portfolios: - try: - if not UserPortfolioPermission.objects.filter(user=user, portfolio=portfolio).exists(): - user_portfolio_permission = UserPortfolioPermission( - user=user, - portfolio=portfolio, - roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], - ) - user_portfolio_permissions_to_create.append(user_portfolio_permission) - else: - logger.info( - f"Permission exists for user '{user.username}' " - f"on portfolio '{portfolio.organization_name}'." - ) - except Exception as e: - logger.warning(e) + # Assign a random portfolio to a user + portfolio = random.choice(portfolios) # nosec + try: + if not UserPortfolioPermission.objects.filter(user=user, portfolio=portfolio).exists(): + user_portfolio_permission = UserPortfolioPermission( + user=user, + portfolio=portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + ) + user_portfolio_permissions_to_create.append(user_portfolio_permission) + else: + logger.info( + f"Permission exists for user '{user.username}' " + f"on portfolio '{portfolio.organization_name}'." + ) + except Exception as e: + logger.warning(e) # Bulk create permissions cls._bulk_create_permissions(user_portfolio_permissions_to_create) From 467b7a90f5f398c693ded053427b92d4e631a73e Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Tue, 8 Oct 2024 12:14:15 -0700 Subject: [PATCH 10/22] Readd try block --- src/registrar/views/domain.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 5d7a840c7..1bc537891 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -917,17 +917,16 @@ class DomainAddUserView(DomainFormBaseView): exc_info=True, ) messages.warning(self.request, "Could not send email invitation.") + try: + UserDomainRole.objects.create( + user=requested_user, + domain=self.object, + role=UserDomainRole.Roles.MANAGER, + ) + except IntegrityError: + messages.warning(self.request, f"{requested_email} is already a manager for this domain") else: - try: - UserDomainRole.objects.create( - user=requested_user, - domain=self.object, - role=UserDomainRole.Roles.MANAGER, - ) - except IntegrityError: - messages.warning(self.request, f"{requested_email} is already a manager for this domain") - else: - messages.success(self.request, f"Added user {requested_email}.") + messages.success(self.request, f"Added user {requested_email}.") return redirect(self.get_success_url()) From 17b5f36fbc3e85492b82d995c2c8e12806eff622 Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Tue, 8 Oct 2024 12:14:54 -0700 Subject: [PATCH 11/22] Fix indent --- src/registrar/views/domain.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 1bc537891..a30db761c 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -917,16 +917,16 @@ class DomainAddUserView(DomainFormBaseView): exc_info=True, ) messages.warning(self.request, "Could not send email invitation.") - try: - UserDomainRole.objects.create( - user=requested_user, - domain=self.object, - role=UserDomainRole.Roles.MANAGER, - ) - except IntegrityError: - messages.warning(self.request, f"{requested_email} is already a manager for this domain") - else: - messages.success(self.request, f"Added user {requested_email}.") + try: + UserDomainRole.objects.create( + user=requested_user, + domain=self.object, + role=UserDomainRole.Roles.MANAGER, + ) + except IntegrityError: + messages.warning(self.request, f"{requested_email} is already a manager for this domain") + else: + messages.success(self.request, f"Added user {requested_email}.") return redirect(self.get_success_url()) From e396534b3805c65b2a96d687e61cd9ef91cf6ebd Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Tue, 8 Oct 2024 12:15:15 -0700 Subject: [PATCH 12/22] Fix indent --- src/registrar/views/domain.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index a30db761c..86a22be0f 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -917,6 +917,7 @@ class DomainAddUserView(DomainFormBaseView): exc_info=True, ) messages.warning(self.request, "Could not send email invitation.") + try: UserDomainRole.objects.create( user=requested_user, From 9425d4c35f462eabe7824c7cfc02ca1f156ac28f Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Tue, 8 Oct 2024 15:26:16 -0700 Subject: [PATCH 13/22] Isolate user domain role create --- src/registrar/views/domain.py | 38 ++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 86a22be0f..a2d287b69 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -823,10 +823,6 @@ class DomainAddUserView(DomainFormBaseView): email, requestor, requested_user ): add_success = False - messages.error( - self.request, - "That email is already a member of another .gov organization.", - ) raise OutsideOrgMemberError # Check to see if an invite has already been sent @@ -880,6 +876,18 @@ class DomainAddUserView(DomainFormBaseView): DomainInvitation.objects.get_or_create(email=email_address, domain=self.object) return redirect(self.get_success_url()) + def _create_user_domain_role(self, requested_user, requested_email, domain, role): + """Assign a user to a domain as a specified role""" + try: + UserDomainRole.objects.create( + user=requested_user, + domain=self.object, + role=UserDomainRole.Roles.MANAGER, + ) + messages.success(self.request, f"Added user {requested_email}.") + except IntegrityError: + messages.warning(self.request, f"{requested_email} is already a manager for this domain") + def form_valid(self, form): """Add the specified user on this domain. Throws EmailSendingError.""" @@ -890,7 +898,8 @@ class DomainAddUserView(DomainFormBaseView): requested_user = User.objects.get(email=requested_email) except User.DoesNotExist: # no matching user, go make an invitation - return self._make_invitation(requested_email, requestor) + requested_user = self._make_invitation(requested_email, requestor) + self._create_user_domain_role(requested_user, requested_email, self.object, UserDomainRole.Roles.MANAGER) else: # if user already exists then just send an email try: @@ -910,6 +919,10 @@ class DomainAddUserView(DomainFormBaseView): self.object, exc_info=True, ) + messages.error( + self.request, + "That email is already a member of another .gov organization.", + ) except Exception: logger.warn( "Could not send email invitation (Other Exception)", @@ -917,17 +930,10 @@ class DomainAddUserView(DomainFormBaseView): exc_info=True, ) messages.warning(self.request, "Could not send email invitation.") - - try: - UserDomainRole.objects.create( - user=requested_user, - domain=self.object, - role=UserDomainRole.Roles.MANAGER, - ) - except IntegrityError: - messages.warning(self.request, f"{requested_email} is already a manager for this domain") - else: - messages.success(self.request, f"Added user {requested_email}.") + else: + self._create_user_domain_role( + requested_user, requested_email, self.object, UserDomainRole.Roles.MANAGER + ) return redirect(self.get_success_url()) From 2f2c4e1951701c59ce4e6b9741b87b907b638b31 Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Tue, 8 Oct 2024 15:27:36 -0700 Subject: [PATCH 14/22] Simplify try catch --- src/registrar/views/domain.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index a2d287b69..c857e7dd5 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -906,6 +906,9 @@ class DomainAddUserView(DomainFormBaseView): self._send_domain_invitation_email( requested_email, requestor, requested_user=requested_user, add_success=False ) + self._create_user_domain_role( + requested_user, requested_email, self.object, UserDomainRole.Roles.MANAGER + ) except EmailSendingError: logger.warn( "Could not send email invitation (EmailSendingError)", @@ -930,10 +933,6 @@ class DomainAddUserView(DomainFormBaseView): exc_info=True, ) messages.warning(self.request, "Could not send email invitation.") - else: - self._create_user_domain_role( - requested_user, requested_email, self.object, UserDomainRole.Roles.MANAGER - ) return redirect(self.get_success_url()) From 6a01e5646d969b315761a150b9f931505c31befc Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Tue, 8 Oct 2024 15:39:38 -0700 Subject: [PATCH 15/22] Debug email bug --- src/registrar/views/domain.py | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index c857e7dd5..433ae1230 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -876,39 +876,26 @@ class DomainAddUserView(DomainFormBaseView): DomainInvitation.objects.get_or_create(email=email_address, domain=self.object) return redirect(self.get_success_url()) - def _create_user_domain_role(self, requested_user, requested_email, domain, role): - """Assign a user to a domain as a specified role""" - try: - UserDomainRole.objects.create( - user=requested_user, - domain=self.object, - role=UserDomainRole.Roles.MANAGER, - ) - messages.success(self.request, f"Added user {requested_email}.") - except IntegrityError: - messages.warning(self.request, f"{requested_email} is already a manager for this domain") - def form_valid(self, form): """Add the specified user on this domain. Throws EmailSendingError.""" requested_email = form.cleaned_data["email"] requestor = self.request.user + email_success = False # 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 - requested_user = self._make_invitation(requested_email, requestor) - self._create_user_domain_role(requested_user, requested_email, self.object, UserDomainRole.Roles.MANAGER) + email_success = True + return self._make_invitation(requested_email, requestor) else: # if user already exists then just send an email try: self._send_domain_invitation_email( requested_email, requestor, requested_user=requested_user, add_success=False ) - self._create_user_domain_role( - requested_user, requested_email, self.object, UserDomainRole.Roles.MANAGER - ) + email_success = True except EmailSendingError: logger.warn( "Could not send email invitation (EmailSendingError)", @@ -933,6 +920,17 @@ class DomainAddUserView(DomainFormBaseView): exc_info=True, ) messages.warning(self.request, "Could not send email invitation.") + if email_success: + try: + UserDomainRole.objects.create( + user=requested_user, + domain=self.object, + role=UserDomainRole.Roles.MANAGER, + ) + messages.success(self.request, f"Added user {requested_email}.") + except IntegrityError: + messages.warning(self.request, f"{requested_email} is already a manager for this domain") + return redirect(self.get_success_url()) From 889c0a25db9bd2867b5aca571b2d6c1b11267f20 Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Tue, 8 Oct 2024 16:10:27 -0700 Subject: [PATCH 16/22] Secure portfolio check on requestor org --- src/registrar/views/domain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 433ae1230..332af5978 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -785,7 +785,7 @@ class DomainAddUserView(DomainFormBaseView): def _is_member_of_different_org(self, email, requestor, requested_user): """Verifies if an email belongs to a different organization as a member or invited member.""" # Check if user is a already member of a different organization than the requestor's org - requestor_org = UserPortfolioPermission.objects.get(user=requestor).portfolio + requestor_org = UserPortfolioPermission.objects.filter(user=requestor).first().portfolio existing_org_permission = UserPortfolioPermission.objects.filter(user=requested_user).first() existing_org_invitation = PortfolioInvitation.objects.filter(email=email).first() From aec1a4f0d439e9892160fd4ee6620a4cb1c5c1d0 Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Wed, 9 Oct 2024 15:15:45 -0700 Subject: [PATCH 17/22] Reverse email send check --- src/registrar/views/domain.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 332af5978..95fa78417 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -881,13 +881,12 @@ class DomainAddUserView(DomainFormBaseView): Throws EmailSendingError.""" requested_email = form.cleaned_data["email"] requestor = self.request.user - email_success = False + email_success = True # 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 - email_success = True return self._make_invitation(requested_email, requestor) else: # if user already exists then just send an email @@ -895,7 +894,6 @@ class DomainAddUserView(DomainFormBaseView): self._send_domain_invitation_email( requested_email, requestor, requested_user=requested_user, add_success=False ) - email_success = True except EmailSendingError: logger.warn( "Could not send email invitation (EmailSendingError)", @@ -904,6 +902,7 @@ class DomainAddUserView(DomainFormBaseView): ) messages.warning(self.request, "Could not send email invitation.") except OutsideOrgMemberError: + email_send = False logger.warn( "Could not send email. Can not invite member of a .gov organization to a different organization.", self.object, From 76fc71382da59740384a84b11fd2f464ae1f9df4 Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Wed, 9 Oct 2024 15:56:20 -0700 Subject: [PATCH 18/22] Re-revert email_success check --- src/registrar/views/domain.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 95fa78417..332af5978 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -881,12 +881,13 @@ class DomainAddUserView(DomainFormBaseView): Throws EmailSendingError.""" requested_email = form.cleaned_data["email"] requestor = self.request.user - email_success = True + email_success = False # 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 + email_success = True return self._make_invitation(requested_email, requestor) else: # if user already exists then just send an email @@ -894,6 +895,7 @@ class DomainAddUserView(DomainFormBaseView): self._send_domain_invitation_email( requested_email, requestor, requested_user=requested_user, add_success=False ) + email_success = True except EmailSendingError: logger.warn( "Could not send email invitation (EmailSendingError)", @@ -902,7 +904,6 @@ class DomainAddUserView(DomainFormBaseView): ) messages.warning(self.request, "Could not send email invitation.") except OutsideOrgMemberError: - email_send = False logger.warn( "Could not send email. Can not invite member of a .gov organization to a different organization.", self.object, From dd29cbf8cadc168ee9e38641725015dd36eb68d7 Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Wed, 9 Oct 2024 16:21:37 -0700 Subject: [PATCH 19/22] Add domain manager after email sending error --- src/registrar/views/domain.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 332af5978..3865bfc36 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -903,6 +903,7 @@ class DomainAddUserView(DomainFormBaseView): exc_info=True, ) messages.warning(self.request, "Could not send email invitation.") + email_success = True except OutsideOrgMemberError: logger.warn( "Could not send email. Can not invite member of a .gov organization to a different organization.", From 4393b5a1d7eb6ac4e57931b2536b2ac87ac130b8 Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Tue, 15 Oct 2024 16:15:09 -0700 Subject: [PATCH 20/22] Add domain manager page content updates --- src/registrar/templates/domain_add_user.html | 13 ++++++++++--- src/registrar/templates/domain_users.html | 4 ++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/registrar/templates/domain_add_user.html b/src/registrar/templates/domain_add_user.html index e95bacd76..81b6678af 100644 --- a/src/registrar/templates/domain_add_user.html +++ b/src/registrar/templates/domain_add_user.html @@ -18,10 +18,17 @@ {% endblock breadcrumb %}

Add a domain manager

- -

You can add another user to help manage your domain. If they aren't an organization member they will - need to sign in to the .gov registrar with their Login.gov account. +{% if has_organization_feature %} +

+ You can add another user to help manage your domain. Users can only be a member of one .gov organization, + and they'll need to sign in with their Login.gov account.

+{% else %} +

+ You can add another user to help manage your domain. If they aren't an organization member they will + need to sign in to the .gov registrar with their Login.gov account. +

+{% endif %} {% csrf_token %} diff --git a/src/registrar/templates/domain_users.html b/src/registrar/templates/domain_users.html index 412f4ee73..7125f9cb2 100644 --- a/src/registrar/templates/domain_users.html +++ b/src/registrar/templates/domain_users.html @@ -8,8 +8,7 @@

Domain managers can update all information related to a domain within the - .gov registrar, including contact details, senior official, security - email, and DNS name servers. + .gov registrar, including including security email and DNS name servers.

From 961a289e663769155efebfa98cc85020400f2dfb Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Tue, 15 Oct 2024 16:22:16 -0700 Subject: [PATCH 21/22] Update domain manager page content --- src/registrar/templates/domain_add_user.html | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/templates/domain_add_user.html b/src/registrar/templates/domain_add_user.html index 81b6678af..fa3f8e821 100644 --- a/src/registrar/templates/domain_add_user.html +++ b/src/registrar/templates/domain_add_user.html @@ -18,15 +18,15 @@ {% endblock breadcrumb %}

Add a domain manager

-{% if has_organization_feature %} +{% if has_organization_feature_flag %}

You can add another user to help manage your domain. Users can only be a member of one .gov organization, and they'll need to sign in with their Login.gov account.

{% else %}

- You can add another user to help manage your domain. If they aren't an organization member they will - need to sign in to the .gov registrar with their Login.gov account. + You can add another user to help manage your domain. They will need to sign in to the .gov registrar with + their Login.gov account.

{% endif %} From 3cb341da595f15f471a2a2f5ecc486ae4b4de2f7 Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Wed, 16 Oct 2024 14:46:39 -0700 Subject: [PATCH 22/22] Add content updates --- src/registrar/templates/domain_users.html | 2 +- src/registrar/views/domain.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/templates/domain_users.html b/src/registrar/templates/domain_users.html index 7125f9cb2..c41902c6f 100644 --- a/src/registrar/templates/domain_users.html +++ b/src/registrar/templates/domain_users.html @@ -17,7 +17,7 @@ instructions on how to set up an account.
  • All domain managers must keep their contact information updated and be responsive if contacted by the .gov team.
  • All domain managers will be notified when updates are made to this domain.
  • -
  • Domains must have at least one domain manager. You can’t remove yourself as a domain manager if you’re the only one assigned to this domain. Add another domain manager before you remove yourself from this domain.
  • +
  • Domains must have at least one domain manager. You can’t remove yourself as a domain manager if you’re the only one assigned to this domain.
  • {% if domain.permissions %} diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 3865bfc36..de156598a 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -912,7 +912,7 @@ class DomainAddUserView(DomainFormBaseView): ) messages.error( self.request, - "That email is already a member of another .gov organization.", + f"{requested_email} is already a member of another .gov organization.", ) except Exception: logger.warn(