Skip to content

Commit

Permalink
Merge pull request #2755 from cisagov/za/2754-fix-domain-request-bugs
Browse files Browse the repository at this point in the history
Issue #2754: Don't show the delete button when the user isn't permissioned for requests - [BACKUP]
  • Loading branch information
zandercymatics authored Sep 17, 2024
2 parents c1daa45 + ef40468 commit 39ec072
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 8 deletions.
9 changes: 6 additions & 3 deletions src/registrar/templates/portfolio_requests.html
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,11 @@
<div id="main-content">
<h1 id="domain-requests-header">Domain requests</h1>
<div class="grid-row grid-gap">
<div class="mobile:grid-col-12 tablet:grid-col-6">
<p class="margin-y-0">Domain requests can only be modified by the person who created the request.</p>
</div>

{% if has_edit_request_portfolio_permission %}
<div class="mobile:grid-col-12 tablet:grid-col-6">
<p class="margin-y-0">Domain requests can only be modified by the person who created the request.</p>
</div>
<div class="mobile:grid-col-12 tablet:grid-col-6">
{% comment %}
IMPORTANT:
Expand All @@ -29,6 +30,8 @@ <h1 id="domain-requests-header">Domain requests</h1>
</a>
</p>
</div>
{% else %}
<p class="margin-y-0">Domain requests can only be modified by the person who created the request.</p>
{% endif %}
</div>

Expand Down
103 changes: 103 additions & 0 deletions src/registrar/tests/test_views_portfolio.py
Original file line number Diff line number Diff line change
Expand Up @@ -837,3 +837,106 @@ def test_portfolio_cache_updates_when_flag_disabled_while_logged_in(self):
response = self.client.get(reverse("home"))
self.assertIsNone(self.client.session.get("portfolio"))
self.assertNotContains(response, "Hotel California")

@less_console_noise_decorator
@override_flag("organization_feature", active=True)
@override_flag("organization_requests", active=True)
def test_org_user_can_delete_own_domain_request_with_permission(self):
"""Test that an org user with edit permission can delete their own DomainRequest with a deletable status."""

# Assign the user to a portfolio with edit permission
UserPortfolioPermission.objects.get_or_create(
user=self.user,
portfolio=self.portfolio,
roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER],
additional_permissions=[UserPortfolioPermissionChoices.EDIT_REQUESTS],
)

# Create a domain request with status WITHDRAWN
domain_request = completed_domain_request(
name="test-domain.gov",
status=DomainRequest.DomainRequestStatus.WITHDRAWN,
portfolio=self.portfolio,
)
domain_request.creator = self.user
domain_request.save()

self.client.force_login(self.user)
# Perform delete
response = self.client.post(reverse("domain-request-delete", kwargs={"pk": domain_request.pk}), follow=True)

# Check that the response is 200
self.assertEqual(response.status_code, 200)

# Check that the domain request no longer exists
self.assertFalse(DomainRequest.objects.filter(pk=domain_request.pk).exists())
domain_request.delete()

@less_console_noise_decorator
@override_flag("organization_feature", active=True)
@override_flag("organization_requests", active=True)
def test_delete_domain_request_as_org_user_without_permission_with_deletable_status(self):
"""Test that an org user without edit permission cant delete their DomainRequest even if status is deletable."""

# Assign the user to a portfolio without edit permission
UserPortfolioPermission.objects.get_or_create(
user=self.user,
portfolio=self.portfolio,
roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER],
additional_permissions=[],
)

# Create a domain request with status STARTED
domain_request = completed_domain_request(
name="test-domain.gov",
status=DomainRequest.DomainRequestStatus.STARTED,
portfolio=self.portfolio,
)
domain_request.creator = self.user
domain_request.save()

self.client.force_login(self.user)
# Attempt to delete
response = self.client.post(reverse("domain-request-delete", kwargs={"pk": domain_request.pk}), follow=True)

# Check response is 403 Forbidden
self.assertEqual(response.status_code, 403)

# Check that the domain request still exists
self.assertTrue(DomainRequest.objects.filter(pk=domain_request.pk).exists())
domain_request.delete()

@less_console_noise_decorator
@override_flag("organization_feature", active=True)
@override_flag("organization_requests", active=True)
def test_org_user_cannot_delete_others_domain_requests(self):
"""Test that an org user with edit permission cannot delete DomainRequests they did not create."""

# Assign the user to a portfolio with edit permission
UserPortfolioPermission.objects.get_or_create(
user=self.user,
portfolio=self.portfolio,
roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER],
additional_permissions=[UserPortfolioPermissionChoices.EDIT_REQUESTS],
)

# Create another user and a domain request
other_user = User.objects.create(username="other_user")
domain_request = completed_domain_request(
name="test-domain.gov",
status=DomainRequest.DomainRequestStatus.STARTED,
portfolio=self.portfolio,
)
domain_request.creator = other_user
domain_request.save()

self.client.force_login(self.user)
# Perform delete as self.user
response = self.client.post(reverse("domain-request-delete", kwargs={"pk": domain_request.pk}), follow=True)

# Check response is 403 Forbidden
self.assertEqual(response.status_code, 403)

# Check that the domain request still exists
self.assertTrue(DomainRequest.objects.filter(pk=domain_request.pk).exists())
domain_request.delete()
6 changes: 6 additions & 0 deletions src/registrar/views/domain_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -796,6 +796,12 @@ def has_permission(self):
if status not in valid_statuses:
return False

# Portfolio users cannot delete their requests if they aren't permissioned to do so
if self.request.user.is_org_user(self.request):
portfolio = self.request.session.get("portfolio")
if not self.request.user.has_edit_request_portfolio_permission(portfolio):
return False

return True

def get_success_url(self):
Expand Down
18 changes: 13 additions & 5 deletions src/registrar/views/domain_requests_json.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,8 @@ def get_domain_requests_json(request):
paginator = Paginator(objects, 10)
page_number = request.GET.get("page", 1)
page_obj = paginator.get_page(page_number)

domain_requests = [
serialize_domain_request(domain_request, request.user) for domain_request in page_obj.object_list
serialize_domain_request(request, domain_request, request.user) for domain_request in page_obj.object_list
]

return JsonResponse(
Expand Down Expand Up @@ -90,13 +89,22 @@ def apply_sorting(queryset, request):
return queryset.order_by(sort_by)


def serialize_domain_request(domain_request, user):
# Determine if the request is deletable
is_deletable = domain_request.status in [
def serialize_domain_request(request, domain_request, user):

deletable_statuses = [
DomainRequest.DomainRequestStatus.STARTED,
DomainRequest.DomainRequestStatus.WITHDRAWN,
]

# Determine if the request is deletable
if not user.is_org_user(request):
is_deletable = domain_request.status in deletable_statuses
else:
portfolio = request.session.get("portfolio")
is_deletable = (
domain_request.status in deletable_statuses and user.has_edit_request_portfolio_permission(portfolio)
) and domain_request.creator == user

# Determine action label based on user permissions and request status
editable_statuses = [
DomainRequest.DomainRequestStatus.STARTED,
Expand Down

0 comments on commit 39ec072

Please sign in to comment.