diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index eb095c5ca..27ad01dc8 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -296,56 +296,56 @@ urlpatterns = [ lambda r: always_404(r, "We forgot to include this link, sorry."), name="todo", ), - path("domain/", views.DomainView.as_view(), name="domain"), - path("domain//prototype-dns", views.PrototypeDomainDNSRecordView.as_view(), name="prototype-domain-dns"), - path("domain//users", views.DomainUsersView.as_view(), name="domain-users"), + path("domain/", views.DomainView.as_view(), name="domain"), + path("domain//prototype-dns", views.PrototypeDomainDNSRecordView.as_view(), name="prototype-domain-dns"), + path("domain//users", views.DomainUsersView.as_view(), name="domain-users"), path( - "domain//dns", + "domain//dns", views.DomainDNSView.as_view(), name="domain-dns", ), path( - "domain//dns/nameservers", + "domain//dns/nameservers", views.DomainNameserversView.as_view(), name="domain-dns-nameservers", ), path( - "domain//dns/dnssec", + "domain//dns/dnssec", views.DomainDNSSECView.as_view(), name="domain-dns-dnssec", ), path( - "domain//dns/dnssec/dsdata", + "domain//dns/dnssec/dsdata", views.DomainDsDataView.as_view(), name="domain-dns-dnssec-dsdata", ), path( - "domain//org-name-address", + "domain//org-name-address", views.DomainOrgNameAddressView.as_view(), name="domain-org-name-address", ), path( - "domain//suborganization", + "domain//suborganization", views.DomainSubOrganizationView.as_view(), name="domain-suborganization", ), path( - "domain//senior-official", + "domain//senior-official", views.DomainSeniorOfficialView.as_view(), name="domain-senior-official", ), path( - "domain//security-email", + "domain//security-email", views.DomainSecurityEmailView.as_view(), name="domain-security-email", ), path( - "domain//renewal", + "domain//renewal", views.DomainRenewalView.as_view(), name="domain-renewal", ), path( - "domain//users/add", + "domain//users/add", views.DomainAddUserView.as_view(), name="domain-users-add", ), @@ -370,7 +370,7 @@ urlpatterns = [ name="domain-request-delete", ), path( - "domain//users//delete", + "domain//users//delete", views.DomainDeleteUserView.as_view(http_method_names=["post"]), name="domain-user-delete", ), @@ -392,6 +392,7 @@ urlpatterns = [ # This way, we can share a view for djangooidc, and other pages as we see fit. handler500 = "registrar.views.utility.error_views.custom_500_error_view" handler403 = "registrar.views.utility.error_views.custom_403_error_view" +handler404 = "registrar.views.utility.error_views.custom_404_error_view" # we normally would guard these with `if settings.DEBUG` but tests run with # DEBUG = False even when these apps have been loaded because settings.DEBUG diff --git a/src/registrar/decorators.py b/src/registrar/decorators.py index 08214f89c..177dfab62 100644 --- a/src/registrar/decorators.py +++ b/src/registrar/decorators.py @@ -1,15 +1,14 @@ -from functools import wraps -from django.http import JsonResponse -from django.core.exceptions import ObjectDoesNotExist - -from registrar.models.domain import Domain -from registrar.models.user_domain_role import UserDomainRole +import functools +from django.core.exceptions import PermissionDenied +from django.utils.decorators import method_decorator +from registrar.models import DomainInformation, DomainRequest, UserDomainRole # Constants for clarity ALL = "all" IS_SUPERUSER = "is_superuser" IS_STAFF = "is_staff" IS_DOMAIN_MANAGER = "is_domain_manager" +IS_STAFF_MANAGING_DOMAIN = "is_staff_managing_domain" def grant_access(*rules): """ @@ -20,53 +19,128 @@ def grant_access(*rules): @grant_access(IS_DOMAIN_MANAGER) """ - def decorator(view_func): - view_func.has_explicit_access = True # Mark as explicitly access-controlled - existing_rules = getattr(view_func, "_access_rules", set()) - existing_rules.update(rules) # Support multiple rules in one call - view_func._access_rules = existing_rules # Store rules on the function + def decorator(view): + if isinstance(view, type): # If decorating a class-based view (CBV) + original_dispatch = view.dispatch # save original dispatch method - @wraps(view_func) - def wrapper(request, *args, **kwargs): - user = request.user - - # Skip authentication if @login_not_required is applied - if getattr(view_func, "login_not_required", False): - return view_func(request, *args, **kwargs) - - # Allow everyone if `ALL` is in rules - if ALL in view_func._access_rules: - return view_func(request, *args, **kwargs) + @method_decorator(grant_access(*rules)) # apply the decorator to dispatch + def wrapped_dispatch(self, request, *args, **kwargs): + if not _user_has_permission(request.user, request, rules, **kwargs): + raise PermissionDenied + return original_dispatch(self, request, *args, **kwargs) - # Ensure user is authenticated - if not user.is_authenticated: - return JsonResponse({"error": "Authentication required"}, status=403) + view.dispatch = wrapped_dispatch # replace dispatch with wrapped version + return view - conditions_met = [] + else: # If decorating a function-based view (FBV) + view.has_explicit_access = True + existing_rules = getattr(view, "_access_rules", set()) + existing_rules.update(rules) + view._access_rules = existing_rules - if IS_STAFF in view_func._access_rules: - conditions_met.append(user.is_staff) - - if not any(conditions_met) and IS_SUPERUSER in view_func._access_rules: - conditions_met.append(user.is_superuser) - - if not any(conditions_met) and IS_DOMAIN_MANAGER in view_func._access_rules: - domain_id = kwargs.get('pk') or kwargs.get('domain_id') - if not domain_id: - return JsonResponse({"error": "Domain ID missing"}, status=400) - try: - domain = Domain.objects.get(pk=domain_id) - has_permission = UserDomainRole.objects.filter( - user=user, domain=domain - ).exists() - conditions_met.append(has_permission) - except ObjectDoesNotExist: - return JsonResponse({"error": "Invalid Domain"}, status=404) - - if not any(conditions_met): - return JsonResponse({"error": "Access Denied"}, status=403) - - return view_func(request, *args, **kwargs) - - return wrapper + @functools.wraps(view) + def wrapper(request, *args, **kwargs): + if not _user_has_permission(request.user, request, rules, **kwargs): + raise PermissionDenied + return view(request, *args, **kwargs) + + return wrapper + return decorator + + +def _user_has_permission(user, request, rules, **kwargs): + """ + Checks if the user meets the permission requirements. + """ + + # Skip authentication if @login_not_required is applied + if getattr(request, "login_not_required", False): + return True + + # Allow everyone if `ALL` is in rules + if ALL in rules: + return True + + # Ensure user is authenticated + if not user.is_authenticated: + return False + + conditions_met = [] + + if IS_STAFF in rules: + conditions_met.append(user.is_staff) + + if not any(conditions_met) and IS_SUPERUSER in rules: + conditions_met.append(user.is_superuser) + + if not any(conditions_met) and IS_DOMAIN_MANAGER in rules: + domain_id = kwargs.get('domain_pk') + # Check UserDomainRole directly instead of fetching Domain + has_permission = UserDomainRole.objects.filter(user=user, domain_id=domain_id).exists() + conditions_met.append(has_permission) + + if not any(conditions_met) and IS_STAFF_MANAGING_DOMAIN in rules: + domain_id = kwargs.get('domain_pk') + has_permission = _can_access_other_user_domains(request, domain_id) + conditions_met.append(has_permission) + + return any(conditions_met) + + +def _can_access_other_user_domains(request, domain_pk): + """Checks to see if an authorized user (staff or superuser) + can access a domain that they did not create or were invited to. + """ + + # Check if the request user is permissioned... + user_is_analyst_or_superuser = request.user.has_perm( + "registrar.analyst_access_permission" + ) or request.user.has_perm("registrar.full_access_permission") + + if not user_is_analyst_or_superuser: + return False + + # Check if the user is attempting a valid edit action. + # In other words, if the analyst/admin did not click + # the 'Manage Domain' button in /admin, + # then they cannot access this page. + session = request.session + can_do_action = ( + "analyst_action" in session + and "analyst_action_location" in session + and session["analyst_action_location"] == domain_pk + ) + + if not can_do_action: + return False + + # Analysts may manage domains, when they are in these statuses: + valid_domain_statuses = [ + DomainRequest.DomainRequestStatus.APPROVED, + DomainRequest.DomainRequestStatus.IN_REVIEW, + DomainRequest.DomainRequestStatus.REJECTED, + DomainRequest.DomainRequestStatus.ACTION_NEEDED, + # Edge case - some domains do not have + # a status or DomainInformation... aka a status of 'None'. + # It is necessary to access those to correct errors. + None, + ] + + requested_domain = DomainInformation.objects.filter(domain_id=domain_pk).first() + + # if no domain information or domain request exist, the user + # should be able to manage the domain; however, if domain information + # and domain request exist, and domain request is not in valid status, + # user should not be able to manage domain + if ( + requested_domain + and requested_domain.domain_request + and requested_domain.domain_request.status not in valid_domain_statuses + ): + return False + + # Valid session keys exist, + # the user is permissioned, + # and it is in a valid status + return True diff --git a/src/registrar/registrar_middleware.py b/src/registrar/registrar_middleware.py index c23fb0515..e3dcbe788 100644 --- a/src/registrar/registrar_middleware.py +++ b/src/registrar/registrar_middleware.py @@ -6,9 +6,9 @@ import logging import re from urllib.parse import parse_qs from django.conf import settings +from django.core.exceptions import PermissionDenied from django.urls import reverse from django.http import HttpResponseRedirect -from django.http import JsonResponse from django.urls import resolve from registrar.models import User from waffle.decorators import flag_is_active @@ -182,7 +182,7 @@ class RestrictAccessMiddleware: def __init__(self, get_response): self.get_response = get_response self.ignored_paths = [re.compile(pattern) for pattern in getattr(settings, "LOGIN_REQUIRED_IGNORE_PATHS", [])] - + def __call__(self, request): # Allow requests that match LOGIN_REQUIRED_IGNORE_PATHS if any(pattern.match(request.path) for pattern in self.ignored_paths): @@ -194,7 +194,7 @@ class RestrictAccessMiddleware: view_func = resolver_match.func app_name = resolver_match.app_name # Get app name of resolved view except Exception: - return JsonResponse({"error": "Not Found"}, status=404) + return self.get_response(request) # Auto-allow Django's built-in admin views (but NOT custom /admin/* views) if app_name == "admin": @@ -206,6 +206,6 @@ class RestrictAccessMiddleware: # Enforce explicit access fules for other views if not getattr(view_func, "has_explicit_access", False): - return JsonResponse({"error": "Access Denied"}, status=403) + raise PermissionDenied return self.get_response(request) \ No newline at end of file diff --git a/src/registrar/templates/domain_add_user.html b/src/registrar/templates/domain_add_user.html index 04565f61e..abc549a82 100644 --- a/src/registrar/templates/domain_add_user.html +++ b/src/registrar/templates/domain_add_user.html @@ -16,10 +16,10 @@ Domains
  • - {{ domain.name }} + {{ domain.name }}
  • - Domain managers + Domain managers
  • Add a domain manager @@ -27,7 +27,7 @@ {% else %} - {% url 'domain-users' pk=domain.id as url %} + {% url 'domain-users' domain_pk=domain.id as url %}