tighten logic for existence checks

This commit is contained in:
zandercymatics 2025-03-06 09:37:39 -07:00
parent b6a213b88c
commit 59932c11f3
No known key found for this signature in database
GPG key ID: FF4636ABEC9682B7
2 changed files with 78 additions and 21 deletions

View file

@ -1,3 +1,4 @@
import logging
import functools
from django.core.exceptions import PermissionDenied
from django.utils.decorators import method_decorator
@ -5,6 +6,9 @@ from registrar.models import Domain, DomainInformation, DomainInvitation, Domain
from registrar.models.portfolio_invitation import PortfolioInvitation
from registrar.models.user_portfolio_permission import UserPortfolioPermission
logger = logging.getLogger(__name__)
# Constants for clarity
ALL = "all"
IS_STAFF = "is_staff"
@ -20,6 +24,7 @@ HAS_PORTFOLIO_DOMAIN_REQUESTS_ANY_PERM = "has_portfolio_domain_requests_any_perm
HAS_PORTFOLIO_DOMAIN_REQUESTS_VIEW_ALL = "has_portfolio_domain_requests_view_all"
HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT = "has_portfolio_domain_requests_edit"
HAS_PORTFOLIO_MEMBERS_ANY_PERM = "has_portfolio_members_any_perm"
HAS_PORTFOLIO_MEMBERS_VIEW_AND_EDIT = "has_portfolio_members_view_and_edit"
HAS_PORTFOLIO_MEMBERS_EDIT = "has_portfolio_members_edit"
HAS_PORTFOLIO_MEMBERS_VIEW = "has_portfolio_members_view"
@ -109,7 +114,9 @@ def _user_has_permission(user, request, rules, **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,
@ -155,10 +162,29 @@ def _user_has_permission(user, request, rules, **kwargs):
or user.has_edit_members_portfolio_permission(portfolio)
),
),
(
# More restrictive check as compared to HAS_PORTFOLIO_MEMBERS_ANY_PERM.
# This is needed because grant_access does not apply perms in a layered fashion
# and grants access when any valid perm is found - so chaining view and edit works differently.
HAS_PORTFOLIO_MEMBERS_VIEW_AND_EDIT,
lambda: user.is_org_user(request)
and (
user.has_view_members_portfolio_permission(portfolio)
and user.has_edit_members_portfolio_permission(portfolio)
)
and (
_member_exists_under_portfolio(portfolio, kwargs.get("member_pk"))
or _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(portfolio)
and (
_member_exists_under_portfolio(portfolio, kwargs.get("member_pk"))
or _member_invitation_exists_under_portfolio(portfolio, kwargs.get("invitedmember_pk"))
),
),
(
HAS_PORTFOLIO_MEMBERS_VIEW,
@ -202,17 +228,47 @@ 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"""
"""Checks to see if the given domain exists under the provided portfolio. Returns True if the pk is None.
HELPFUL REMINDER: Watch for typos! Verify that the kwarg key exists before using this function.
"""
# 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.info("_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"""
"""Checks to see if the given domain request exists under the provided portfolio. Returns True if the pk is None.
HELPFUL REMINDER: Watch for typos! Verify that the kwarg key exists before using this function.
"""
# 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.info("_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. Returns True if the pk is None.
HELPFUL REMINDER: Watch for typos! Verify that the kwarg key exists before using this function.
"""
# 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.info("_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. Returns True if the pk is None.
HELPFUL REMINDER: Watch for typos! Verify that the kwarg key exists before using this function.
"""
# 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.info("_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):
@ -310,15 +366,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

View file

@ -13,6 +13,7 @@ from registrar.decorators import (
HAS_PORTFOLIO_MEMBERS_ANY_PERM,
HAS_PORTFOLIO_MEMBERS_EDIT,
HAS_PORTFOLIO_MEMBERS_VIEW,
HAS_PORTFOLIO_MEMBERS_VIEW_AND_EDIT,
IS_PORTFOLIO_MEMBER,
grant_access,
)
@ -117,7 +118,9 @@ class PortfolioMemberView(DetailView, View):
)
@grant_access(HAS_PORTFOLIO_MEMBERS_VIEW, HAS_PORTFOLIO_MEMBERS_EDIT)
# The parent page requires VIEW access, the child requires EDIT.
# This prevents url skimming without having to inherit the base class.
@grant_access(HAS_PORTFOLIO_MEMBERS_VIEW_AND_EDIT)
class PortfolioMemberDeleteView(View):
pk_url_kwarg = "member_pk"
@ -220,7 +223,9 @@ class PortfolioMemberDeleteView(View):
messages.warning(self.request, "Could not send email notification to existing organization admins.")
@grant_access(HAS_PORTFOLIO_MEMBERS_VIEW, HAS_PORTFOLIO_MEMBERS_EDIT)
# The parent page requires VIEW access, the child requires EDIT.
# This prevents url skimming without having to inherit the base class.
@grant_access(HAS_PORTFOLIO_MEMBERS_VIEW_AND_EDIT)
class PortfolioMemberEditView(DetailView, View):
model = Portfolio
context_object_name = "portfolio"
@ -324,7 +329,9 @@ class PortfolioMemberDomainsView(View):
)
@grant_access(HAS_PORTFOLIO_MEMBERS_VIEW, HAS_PORTFOLIO_MEMBERS_EDIT)
# The parent page requires VIEW access, the child requires EDIT.
# This prevents url skimming without having to inherit the base class.
@grant_access(HAS_PORTFOLIO_MEMBERS_VIEW_AND_EDIT)
class PortfolioMemberDomainsEditView(DetailView, View):
model = Portfolio
context_object_name = "portfolio"
@ -482,7 +489,9 @@ class PortfolioInvitedMemberView(DetailView, View):
)
@grant_access(HAS_PORTFOLIO_MEMBERS_VIEW, HAS_PORTFOLIO_MEMBERS_EDIT)
# The parent page requires VIEW access, the child requires EDIT.
# This prevents url skimming without having to inherit the base class.
@grant_access(HAS_PORTFOLIO_MEMBERS_VIEW_AND_EDIT)
class PortfolioInvitedMemberDeleteView(View):
pk_url_kwarg = "invitedmember_pk"
def post(self, request, invitedmember_pk):
@ -528,7 +537,9 @@ class PortfolioInvitedMemberDeleteView(View):
messages.warning(self.request, "Could not send email notification to existing organization admins.")
@grant_access(HAS_PORTFOLIO_MEMBERS_VIEW, HAS_PORTFOLIO_MEMBERS_EDIT)
# The parent page requires VIEW access, the child requires EDIT.
# This prevents url skimming without having to inherit the base class.
@grant_access(HAS_PORTFOLIO_MEMBERS_VIEW_AND_EDIT)
class PortfolioInvitedMemberEditView(DetailView, View):
model = Portfolio
context_object_name = "portfolio"
@ -618,7 +629,9 @@ class PortfolioInvitedMemberDomainsView(View):
)
@grant_access(HAS_PORTFOLIO_MEMBERS_VIEW, HAS_PORTFOLIO_MEMBERS_EDIT)
# The parent page requires VIEW access, the child requires EDIT.
# This prevents url skimming without having to inherit the base class.
@grant_access(HAS_PORTFOLIO_MEMBERS_VIEW_AND_EDIT)
class PortfolioInvitedMemberDomainsEditView(DetailView, View):
model = Portfolio