Add abstract views that enforce permissions.

This commit is contained in:
Neil Martinsen-Burrell 2023-05-24 15:28:52 -05:00
parent ace9a8e6a7
commit 10185553de
No known key found for this signature in database
GPG key ID: 6A3C818CC10D0184
8 changed files with 194 additions and 44 deletions

View file

@ -59,12 +59,12 @@ urlpatterns = [
),
path(
"application/<int:pk>/withdraw",
views.ApplicationWithdraw.as_view(),
views.ApplicationWithdrawConfirmation.as_view(),
name="application-withdraw-confirmation",
),
path(
"application/<int:pk>/withdrawconfirmed",
views.ApplicationWithdraw.updatestatus,
views.ApplicationWithdrawn.as_view(),
name="application-withdrawn",
),
path("health/", views.health),

View file

@ -31,7 +31,7 @@
<p> <b class="review__step__name">Last updated:</b> {{domainapplication.updated_at|date:"F j, Y"}}<br>
<b class="review__step__name">Request #:</b> {{domainapplication.id}}</p>
<p>{% include "includes/domain_application.html" %}</p>
<p><a href="{% url 'application-withdraw-confirmation' domainapplication.id %}" class="usa-button usa-button--outline withdraw_outline">
<p><a href="{% url 'application-withdraw-confirmation' pk=domainapplication.id %}" class="usa-button usa-button--outline withdraw_outline">
Withdraw request</a>
</p>
</div>

View file

@ -1136,7 +1136,7 @@ class TestDomainDetail(TestWithDomainPermissions, WebTest):
self.assertContains(response, "Add another user")
def test_domain_user_add_form(self):
"""Adding a user works."""
"""Adding an existing user works."""
other_user, _ = get_user_model().objects.get_or_create(
email="mayor@igorville.gov"
)
@ -1219,6 +1219,22 @@ class TestDomainDetail(TestWithDomainPermissions, WebTest):
with self.assertRaises(DomainInvitation.DoesNotExist):
DomainInvitation.objects.get(id=invitation.id)
def test_domain_invitation_cancel_no_permissions(self):
"""Posting to the delete view as a different user should fail."""
EMAIL = "mayor@igorville.gov"
invitation, _ = DomainInvitation.objects.get_or_create(
domain=self.domain, email=EMAIL
)
other_user = User()
other_user.save()
self.client.force_login(other_user)
with less_console_noise(): # permission denied makes console errors
result = self.client.post(
reverse("invitation-delete", kwargs={"pk": invitation.id})
)
self.assertEqual(result.status_code, 403)
@boto3_mocking.patching
def test_domain_invitation_flow(self):
"""Send an invitation to a new user, log in and load the dashboard."""
@ -1330,6 +1346,7 @@ class TestApplicationStatus(TestWithUser, WebTest):
def setUp(self):
super().setUp()
self.app.set_user(self.user.username)
self.client.force_login(self.user)
def _completed_application(
self,
@ -1443,3 +1460,24 @@ class TestApplicationStatus(TestWithUser, WebTest):
)
home_page = self.app.get("/")
self.assertContains(home_page, "Withdrawn")
def test_application_status_no_permissions(self):
"""Can't access applications without being the creator."""
application = self._completed_application()
other_user = User()
other_user.save()
application.creator = other_user
application.save()
# PermissionDeniedErrors make lots of noise in test output
with less_console_noise():
for url_name in [
"application-status",
"application-withdraw-confirmation",
"application-withdrawn",
]:
with self.subTest(url_name=url_name):
page = self.client.get(
reverse(url_name, kwargs={"pk": application.pk})
)
self.assertEqual(page.status_code, 403)

View file

@ -14,7 +14,7 @@ from registrar.models import DomainApplication
from registrar.utility import StrEnum
from registrar.views.utility import StepsHelper
from .utility import DomainPermission
from .utility import DomainApplicationPermissionView
logger = logging.getLogger(__name__)
@ -478,29 +478,31 @@ class Finished(ApplicationWizard):
return render(self.request, self.template_name, context)
class ApplicationStatus(generic.DetailView):
model = DomainApplication
class ApplicationStatus(DomainApplicationPermissionView):
template_name = "application_status.html"
def get_context_data(self, **kwargs):
"""Get context details to process information from application"""
context = super(ApplicationStatus, self).get_context_data(**kwargs)
return context
class ApplicationWithdrawConfirmation(DomainApplicationPermissionView):
"""This page will ask user to confirm if they want to withdraw
class ApplicationWithdraw(LoginRequiredMixin, generic.DetailView, DomainPermission):
model = DomainApplication
template_name = "application_withdraw_confirmation.html"
""" The page above will display asking user to confirm if they want to withdraw;
Note it uses "DomainPermission" from Domain to ensure that the person who
applied only have access to withdraw the request
The DomainApplicationPermissionView restricts access so that only the
`creator` of the application may withdraw it.
"""
def updatestatus(request, pk):
"""If user click on withdraw confirm button, it will be updated to withdraw
and send back to homepage"""
application = DomainApplication.objects.get(id=pk)
template_name = "application_withdraw_confirmation.html"
class ApplicationWithdrawn(DomainApplicationPermissionView):
# this view renders no template
template_name = None
def get(self, *args, **kwargs):
"""View class that does the actual withdrawing.
If user click on withdraw confirm button, this view updates the status to withdraw
and send back to homepage.
"""
application = DomainApplication.objects.get(id=self.kwargs["pk"])
application.status = "withdrawn"
application.save()
return HttpResponseRedirect(reverse("home"))

View file

@ -1,4 +1,9 @@
"""View for a single Domain."""
"""Views for a single Domain.
Authorization is handled by the `DomainPermissionView`. To ensure that only
authorized users can see information on a domain, every view here should
inherit from `DomainPermissionView` (or DomainInvitationPermissionDeleteView).
"""
import logging
@ -14,28 +19,24 @@ from registrar.models import Domain, DomainInvitation, User, UserDomainRole
from ..forms import DomainAddUserForm, NameserverFormset, DomainSecurityEmailForm
from ..utility.email import send_templated_email, EmailSendingError
from .utility import DomainPermission
from .utility import DomainPermissionView, DomainInvitationPermissionDeleteView
logger = logging.getLogger(__name__)
class DomainView(DomainPermission, DetailView):
class DomainView(DomainPermissionView):
"""Domain detail overview page."""
model = Domain
template_name = "domain_detail.html"
context_object_name = "domain"
class DomainNameserversView(DomainPermission, FormMixin, DetailView):
class DomainNameserversView(DomainPermissionView, FormMixin):
"""Domain nameserver editing view."""
model = Domain
template_name = "domain_nameservers.html"
context_object_name = "domain"
form_class = NameserverFormset
def get_initial(self):
@ -96,13 +97,11 @@ class DomainNameserversView(DomainPermission, FormMixin, DetailView):
return super().form_valid(formset)
class DomainSecurityEmailView(DomainPermission, FormMixin, DetailView):
class DomainSecurityEmailView(DomainPermissionView, FormMixin):
"""Domain security email editing view."""
model = Domain
template_name = "domain_security_email.html"
context_object_name = "domain"
form_class = DomainSecurityEmailForm
def get_initial(self):
@ -141,16 +140,14 @@ class DomainSecurityEmailView(DomainPermission, FormMixin, DetailView):
return redirect(self.get_success_url())
class DomainUsersView(DomainPermission, DetailView):
class DomainUsersView(DomainPermissionView):
"""User management page in the domain details."""
model = Domain
template_name = "domain_users.html"
context_object_name = "domain"
class DomainAddUserView(DomainPermission, FormMixin, DetailView):
class DomainAddUserView(DomainPermissionView, FormMixin):
"""Inside of a domain's user management, a form for adding users.
@ -159,7 +156,6 @@ class DomainAddUserView(DomainPermission, FormMixin, DetailView):
"""
template_name = "domain_add_user.html"
model = Domain
form_class = DomainAddUserForm
def get_success_url(self):
@ -239,10 +235,7 @@ class DomainAddUserView(DomainPermission, FormMixin, DetailView):
return redirect(self.get_success_url())
class DomainInvitationDeleteView(SuccessMessageMixin, DeleteView):
model = DomainInvitation
object: DomainInvitation # workaround for type mismatch in DeleteView
class DomainInvitationDeleteView(DomainInvitationPermissionDeleteView):
def get_success_url(self):
return reverse("domain-users", kwargs={"pk": self.object.domain.id})

View file

@ -1,3 +1,8 @@
from .steps_helper import StepsHelper
from .always_404 import always_404
from .mixins import DomainPermission
from .permission_views import (
DomainPermissionView,
DomainApplicationPermissionView,
DomainInvitationPermissionDeleteView,
)

View file

@ -2,7 +2,7 @@
from django.contrib.auth.mixins import PermissionRequiredMixin
from registrar.models import UserDomainRole
from registrar.models import UserDomainRole, DomainApplication, DomainInvitation
class PermissionsLoginMixin(PermissionRequiredMixin):
@ -35,3 +35,48 @@ class DomainPermission(PermissionsLoginMixin):
# if we need to check more about the nature of role, do it here.
return True
class DomainApplicationPermission(PermissionsLoginMixin):
"""Does the logged-in user have access to this domain application?"""
def has_permission(self):
"""Check if this user has access to this domain application.
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"]
"""
if not self.request.user.is_authenticated:
return False
# user needs to be the creator of the application
# this query is empty if there isn't a domain application with this
# id and this user as creator
if not DomainApplication.objects.filter(
creator=self.request.user, id=self.kwargs["pk"]
).exists():
return False
return True
class DomainInvitationPermission(PermissionsLoginMixin):
"""Does the logged-in user have access to this domain invitation?
A user has access to a domain invitation if they have a role on the
associated domain.
"""
def has_permission(self):
"""Check if this user has a role on the domain of this invitation."""
if not self.request.user.is_authenticated:
return False
if not DomainInvitation.objects.filter(
id=self.kwargs["pk"], domain__permissions__user=self.request.user
).exists():
return False
return True

View file

@ -0,0 +1,67 @@
"""View classes that enforce authorization."""
import abc # abstract base class
from django.views.generic import DetailView, DeleteView
from registrar.models import Domain, DomainApplication, DomainInvitation
from .mixins import (
DomainPermission,
DomainApplicationPermission,
DomainInvitationPermission,
)
class DomainPermissionView(DomainPermission, DetailView, abc.ABC):
"""Abstract base view for domains that enforces permissions.
This abstract view cannot be instantiated. Actual views must specify `template_name`.
"""
# DetailView property for what model this is viewing
model = Domain
# variable name in template context for the model object
context_object_name = "domain"
# Abstract property enforces NotImplementedError on an attribute.
@property
@abc.abstractmethod
def template_name(self):
raise NotImplementedError
class DomainApplicationPermissionView(DomainApplicationPermission, DetailView, abc.ABC):
"""Abstract base view for domain applications that enforces permissions
This abstract view cannot be instantiated. Actual views must specify `template_name`.
"""
# DetailView property for what model this is viewing
model = DomainApplication
# variable name in template context for the model object
context_object_name = "domainapplication"
# Abstract property enforces NotImplementedError on an attribute.
@property
@abc.abstractmethod
def template_name(self):
raise NotImplementedError
class DomainInvitationPermissionDeleteView(
DomainInvitationPermission, DeleteView, abc.ABC
):
"""Abstract view for deleting a domain invitation.
This one is fairly specialized, but this is the only thing that we do
right now with domain invitations. We still have the full
`DomainInvitationPermission` class, but here we just pair it with a
DeleteView.
"""
model = DomainInvitation
object: DomainInvitation # workaround for type mismatch in DeleteView