diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index 12efe5a9f..56f0cfd0f 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -89,52 +89,52 @@ urlpatterns = [ name="members", ), path( - "member/", + "member/", views.PortfolioMemberView.as_view(), name="member", ), path( - "member//delete", + "member//delete", views.PortfolioMemberDeleteView.as_view(), name="member-delete", ), path( - "member//permissions", + "member//permissions", views.PortfolioMemberEditView.as_view(), name="member-permissions", ), path( - "member//domains", + "member//domains", views.PortfolioMemberDomainsView.as_view(), name="member-domains", ), path( - "member//domains/edit", + "member//domains/edit", views.PortfolioMemberDomainsEditView.as_view(), name="member-domains-edit", ), path( - "invitedmember/", + "invitedmember/", views.PortfolioInvitedMemberView.as_view(), name="invitedmember", ), path( - "invitedmember//delete", + "invitedmember//delete", views.PortfolioInvitedMemberDeleteView.as_view(), name="invitedmember-delete", ), path( - "invitedmember//permissions", + "invitedmember//permissions", views.PortfolioInvitedMemberEditView.as_view(), name="invitedmember-permissions", ), path( - "invitedmember//domains", + "invitedmember//domains", views.PortfolioInvitedMemberDomainsView.as_view(), name="invitedmember-domains", ), path( - "invitedmember//domains/edit", + "invitedmember//domains/edit", views.PortfolioInvitedMemberDomainsEditView.as_view(), name="invitedmember-domains-edit", ), diff --git a/src/registrar/decorators.py b/src/registrar/decorators.py index 7cb2792f4..b4b5c3bd2 100644 --- a/src/registrar/decorators.py +++ b/src/registrar/decorators.py @@ -1,7 +1,13 @@ +import logging import functools from django.core.exceptions import PermissionDenied from django.utils.decorators import method_decorator from registrar.models import Domain, DomainInformation, DomainInvitation, DomainRequest, UserDomainRole +from registrar.models.portfolio_invitation import PortfolioInvitation +from registrar.models.user_portfolio_permission import UserPortfolioPermission + + +logger = logging.getLogger(__name__) # Constants for clarity ALL = "all" @@ -98,24 +104,38 @@ def _user_has_permission(user, request, rules, **kwargs): if not user.is_authenticated or user.is_restricted(): return False + portfolio = request.session.get("portfolio") # Define permission checks permission_checks = [ (IS_STAFF, lambda: user.is_staff), - (IS_DOMAIN_MANAGER, lambda: _is_domain_manager(user, **kwargs)), + ( + IS_DOMAIN_MANAGER, + lambda: (not user.is_org_user(request) and _is_domain_manager(user, **kwargs)) + or ( + user.is_org_user(request) + and _is_domain_manager(user, **kwargs) + and _domain_exists_under_portfolio(portfolio, kwargs.get("domain_pk")) + ), + ), (IS_STAFF_MANAGING_DOMAIN, lambda: _is_staff_managing_domain(request, **kwargs)), (IS_PORTFOLIO_MEMBER, lambda: user.is_org_user(request)), ( HAS_PORTFOLIO_DOMAINS_VIEW_ALL, - lambda: _has_portfolio_view_all_domains(request, kwargs.get("domain_pk")), + lambda: user.is_org_user(request) + and user.has_view_all_domains_portfolio_permission(portfolio) + and _domain_exists_under_portfolio(portfolio, kwargs.get("domain_pk")), ), ( HAS_PORTFOLIO_DOMAINS_ANY_PERM, lambda: user.is_org_user(request) - and user.has_any_domains_portfolio_permission(request.session.get("portfolio")), + and user.has_any_domains_portfolio_permission(portfolio) + and _domain_exists_under_portfolio(portfolio, kwargs.get("domain_pk")), ), ( IS_PORTFOLIO_MEMBER_AND_DOMAIN_MANAGER, - lambda: _is_domain_manager(user, **kwargs) and _is_portfolio_member(request), + lambda: _is_domain_manager(user, **kwargs) + and _is_portfolio_member(request) + and _domain_exists_under_portfolio(portfolio, kwargs.get("domain_pk")), ), ( IS_DOMAIN_MANAGER_AND_NOT_PORTFOLIO_MEMBER, @@ -129,34 +149,55 @@ def _user_has_permission(user, request, rules, **kwargs): ( HAS_PORTFOLIO_DOMAIN_REQUESTS_ANY_PERM, lambda: user.is_org_user(request) - and user.has_any_requests_portfolio_permission(request.session.get("portfolio")), + and user.has_any_requests_portfolio_permission(portfolio) + and _domain_request_exists_under_portfolio(portfolio, kwargs.get("domain_request_pk")), ), ( HAS_PORTFOLIO_DOMAIN_REQUESTS_VIEW_ALL, lambda: user.is_org_user(request) - and user.has_view_all_domain_requests_portfolio_permission(request.session.get("portfolio")), + and user.has_view_all_domain_requests_portfolio_permission(portfolio) + and _domain_request_exists_under_portfolio(portfolio, kwargs.get("domain_request_pk")), ), ( HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT, - lambda: _has_portfolio_domain_requests_edit(user, request, kwargs.get("domain_request_pk")), + lambda: _has_portfolio_domain_requests_edit(user, request, kwargs.get("domain_request_pk")) + and _domain_request_exists_under_portfolio(portfolio, kwargs.get("domain_request_pk")), ), ( HAS_PORTFOLIO_MEMBERS_ANY_PERM, lambda: user.is_org_user(request) and ( - user.has_view_members_portfolio_permission(request.session.get("portfolio")) - or user.has_edit_members_portfolio_permission(request.session.get("portfolio")) + user.has_view_members_portfolio_permission(portfolio) + or user.has_edit_members_portfolio_permission(portfolio) + ) + and ( + # AND rather than OR because these functions return true if the PK is not found. + # This adds support for if the view simply doesn't have said PK. + _member_exists_under_portfolio(portfolio, kwargs.get("member_pk")) + and _member_invitation_exists_under_portfolio(portfolio, kwargs.get("invitedmember_pk")) ), ), ( HAS_PORTFOLIO_MEMBERS_EDIT, lambda: user.is_org_user(request) - and user.has_edit_members_portfolio_permission(request.session.get("portfolio")), + and user.has_edit_members_portfolio_permission(portfolio) + and ( + # AND rather than OR because these functions return true if the PK is not found. + # This adds support for if the view simply doesn't have said PK. + _member_exists_under_portfolio(portfolio, kwargs.get("member_pk")) + and _member_invitation_exists_under_portfolio(portfolio, kwargs.get("invitedmember_pk")) + ), ), ( HAS_PORTFOLIO_MEMBERS_VIEW, lambda: user.is_org_user(request) - and user.has_view_members_portfolio_permission(request.session.get("portfolio")), + and user.has_view_members_portfolio_permission(portfolio) + and ( + # AND rather than OR because these functions return true if the PK is not found. + # This adds support for if the view simply doesn't have said PK. + _member_exists_under_portfolio(portfolio, kwargs.get("member_pk")) + and _member_invitation_exists_under_portfolio(portfolio, kwargs.get("invitedmember_pk")) + ), ), ] @@ -191,6 +232,70 @@ def _is_domain_manager(user, **kwargs): return False +def _domain_exists_under_portfolio(portfolio, domain_pk): + """Checks to see if the given domain exists under the provided portfolio. + HELPFUL REMINDER: Watch for typos! Verify that the kwarg key exists before using this function. + Returns True if the pk is falsy. Otherwise, returns a bool if said object exists. + """ + # The view expects this, and the page will throw an error without this if it needs it. + # Thus, if it is none, we are not checking on a specific record and therefore there is nothing to check. + if not domain_pk: + logger.warning( + "_domain_exists_under_portfolio => Could not find domain_pk. " + "This is a non-issue if called from the right context." + ) + return True + return Domain.objects.filter(domain_info__portfolio=portfolio, id=domain_pk).exists() + + +def _domain_request_exists_under_portfolio(portfolio, domain_request_pk): + """Checks to see if the given domain request exists under the provided portfolio. + HELPFUL REMINDER: Watch for typos! Verify that the kwarg key exists before using this function. + Returns True if the pk is falsy. Otherwise, returns a bool if said object exists. + """ + # The view expects this, and the page will throw an error without this if it needs it. + # Thus, if it is none, we are not checking on a specific record and therefore there is nothing to check. + if not domain_request_pk: + logger.warning( + "_domain_request_exists_under_portfolio => Could not find domain_request_pk. " + "This is a non-issue if called from the right context." + ) + return True + return DomainRequest.objects.filter(portfolio=portfolio, id=domain_request_pk).exists() + + +def _member_exists_under_portfolio(portfolio, member_pk): + """Checks to see if the given UserPortfolioPermission exists under the provided portfolio. + HELPFUL REMINDER: Watch for typos! Verify that the kwarg key exists before using this function. + Returns True if the pk is falsy. Otherwise, returns a bool if said object exists. + """ + # The view expects this, and the page will throw an error without this if it needs it. + # Thus, if it is none, we are not checking on a specific record and therefore there is nothing to check. + if not member_pk: + logger.warning( + "_member_exists_under_portfolio => Could not find member_pk. " + "This is a non-issue if called from the right context." + ) + return True + return UserPortfolioPermission.objects.filter(portfolio=portfolio, id=member_pk).exists() + + +def _member_invitation_exists_under_portfolio(portfolio, invitedmember_pk): + """Checks to see if the given PortfolioInvitation exists under the provided portfolio. + HELPFUL REMINDER: Watch for typos! Verify that the kwarg key exists before using this function. + Returns True if the pk is falsy. Otherwise, returns a bool if said object exists. + """ + # The view expects this, and the page will throw an error without this if it needs it. + # Thus, if it is none, we are not checking on a specific record and therefore there is nothing to check. + if not invitedmember_pk: + logger.warning( + "_member_invitation_exists_under_portfolio => Could not find invitedmember_pk. " + "This is a non-issue if called from the right context." + ) + return True + return PortfolioInvitation.objects.filter(portfolio=portfolio, id=invitedmember_pk).exists() + + def _is_domain_request_creator(user, domain_request_pk): """Checks to see if the user is the creator of a domain request with domain_request_pk.""" @@ -286,15 +391,3 @@ def _is_staff_managing_domain(request, **kwargs): # the user is permissioned, # and it is in a valid status return True - - -def _has_portfolio_view_all_domains(request, domain_pk): - """Returns whether the user in the request can access the domain - via portfolio view all domains permission.""" - portfolio = request.session.get("portfolio") - if request.user.has_view_all_domains_portfolio_permission(portfolio): - if Domain.objects.filter(id=domain_pk).exists(): - domain = Domain.objects.get(id=domain_pk) - if domain.domain_info.portfolio == portfolio: - return True - return False diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py index a9c9f6f03..619caee7d 100644 --- a/src/registrar/forms/domain_request_wizard.py +++ b/src/registrar/forms/domain_request_wizard.py @@ -348,7 +348,7 @@ class OrganizationContactForm(RegistrarForm): error_messages={ "required": ("Select the state, territory, or military post where your organization is located.") }, - widget=ComboboxWidget, + widget=ComboboxWidget(attrs={"required": True}), ) zipcode = forms.CharField( label="Zip code", diff --git a/src/registrar/templates/includes/member_domains_edit_table.html b/src/registrar/templates/includes/member_domains_edit_table.html index a76f711cc..1d706f89a 100644 --- a/src/registrar/templates/includes/member_domains_edit_table.html +++ b/src/registrar/templates/includes/member_domains_edit_table.html @@ -91,9 +91,9 @@ aria-describedby="You have unsaved changes that will be lost." > {% if portfolio_permission %} - {% url 'member-domains' pk=portfolio_permission.id as url %} + {% url 'member-domains' member_pk=portfolio_permission.id as url %} {% else %} - {% url 'invitedmember-domains' pk=portfolio_invitation.id as url %} + {% url 'invitedmember-domains' invitedmember_pk=portfolio_invitation.id as url %} {% endif %} {% include 'includes/modal.html' with modal_heading="Are you sure you want to continue?" modal_description="You have unsaved changes that will be lost." modal_button_url=url modal_button_text="Continue without saving" %} diff --git a/src/registrar/templates/portfolio_member_domains.html b/src/registrar/templates/portfolio_member_domains.html index 76d48c20b..677539585 100644 --- a/src/registrar/templates/portfolio_member_domains.html +++ b/src/registrar/templates/portfolio_member_domains.html @@ -15,11 +15,11 @@ {% url 'members' as url %} {% if portfolio_permission %} - {% url 'member' pk=portfolio_permission.id as url2 %} - {% url 'member-domains-edit' pk=portfolio_permission.id as url3 %} + {% url 'member' member_pk=portfolio_permission.id as url2 %} + {% url 'member-domains-edit' member_pk=portfolio_permission.id as url3 %} {% else %} - {% url 'invitedmember' pk=portfolio_invitation.id as url2 %} - {% url 'invitedmember-domains-edit' pk=portfolio_invitation.id as url3 %} + {% url 'invitedmember' invitedmember_pk=portfolio_invitation.id as url2 %} + {% url 'invitedmember-domains-edit' invitedmember_pk=portfolio_invitation.id as url3 %} {% endif %}