mirror of
https://github.com/cisagov/manage.get.gov.git
synced 2025-07-28 05:26:28 +02:00
Merge pull request #3682 from cisagov/ab/3599-permissions-tests
#3599: New permissions tests - [AB]
This commit is contained in:
commit
51248c4f6e
4 changed files with 302 additions and 0 deletions
|
@ -171,6 +171,7 @@ urlpatterns = [
|
||||||
path(
|
path(
|
||||||
"admin/logout/",
|
"admin/logout/",
|
||||||
RedirectView.as_view(pattern_name="logout", permanent=False),
|
RedirectView.as_view(pattern_name="logout", permanent=False),
|
||||||
|
name="logout",
|
||||||
),
|
),
|
||||||
path(
|
path(
|
||||||
"admin/analytics/export_data_type/",
|
"admin/analytics/export_data_type/",
|
||||||
|
|
229
src/registrar/permissions.py
Normal file
229
src/registrar/permissions.py
Normal file
|
@ -0,0 +1,229 @@
|
||||||
|
"""
|
||||||
|
Centralized permissions management for the registrar.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from django.urls import URLResolver, get_resolver, URLPattern
|
||||||
|
from registrar.decorators import (
|
||||||
|
HAS_PORTFOLIO_DOMAIN_REQUESTS_ANY_PERM,
|
||||||
|
IS_STAFF,
|
||||||
|
IS_DOMAIN_MANAGER,
|
||||||
|
IS_DOMAIN_MANAGER_AND_NOT_PORTFOLIO_MEMBER,
|
||||||
|
IS_PORTFOLIO_MEMBER_AND_DOMAIN_MANAGER,
|
||||||
|
IS_CISA_ANALYST,
|
||||||
|
IS_OMB_ANALYST,
|
||||||
|
IS_FULL_ACCESS,
|
||||||
|
IS_DOMAIN_REQUEST_CREATOR,
|
||||||
|
IS_STAFF_MANAGING_DOMAIN,
|
||||||
|
IS_PORTFOLIO_MEMBER,
|
||||||
|
HAS_PORTFOLIO_DOMAINS_ANY_PERM,
|
||||||
|
HAS_PORTFOLIO_DOMAINS_VIEW_ALL,
|
||||||
|
HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT,
|
||||||
|
HAS_PORTFOLIO_DOMAIN_REQUESTS_VIEW_ALL,
|
||||||
|
HAS_PORTFOLIO_MEMBERS_EDIT,
|
||||||
|
HAS_PORTFOLIO_MEMBERS_ANY_PERM,
|
||||||
|
HAS_PORTFOLIO_MEMBERS_VIEW,
|
||||||
|
ALL,
|
||||||
|
)
|
||||||
|
|
||||||
|
# Define permissions for each URL pattern by name
|
||||||
|
URL_PERMISSIONS = {
|
||||||
|
# Home & general pages
|
||||||
|
"home": [ALL],
|
||||||
|
"health": [ALL], # Intentionally no decorator
|
||||||
|
# Domain management
|
||||||
|
"domain": [HAS_PORTFOLIO_DOMAINS_VIEW_ALL, IS_DOMAIN_MANAGER, IS_STAFF_MANAGING_DOMAIN],
|
||||||
|
"domain-dns": [IS_DOMAIN_MANAGER, IS_STAFF_MANAGING_DOMAIN],
|
||||||
|
"domain-dns-nameservers": [IS_DOMAIN_MANAGER, IS_STAFF_MANAGING_DOMAIN],
|
||||||
|
"domain-dns-dnssec": [IS_DOMAIN_MANAGER, IS_STAFF_MANAGING_DOMAIN],
|
||||||
|
"domain-dns-dnssec-dsdata": [IS_DOMAIN_MANAGER, IS_STAFF_MANAGING_DOMAIN],
|
||||||
|
"domain-org-name-address": [IS_DOMAIN_MANAGER, IS_STAFF_MANAGING_DOMAIN],
|
||||||
|
"domain-suborganization": [IS_PORTFOLIO_MEMBER_AND_DOMAIN_MANAGER, IS_STAFF_MANAGING_DOMAIN],
|
||||||
|
"domain-senior-official": [IS_DOMAIN_MANAGER_AND_NOT_PORTFOLIO_MEMBER, IS_STAFF_MANAGING_DOMAIN],
|
||||||
|
"domain-security-email": [IS_DOMAIN_MANAGER, IS_STAFF_MANAGING_DOMAIN],
|
||||||
|
"domain-renewal": [IS_DOMAIN_MANAGER, IS_STAFF_MANAGING_DOMAIN],
|
||||||
|
"domain-users": [IS_DOMAIN_MANAGER, IS_STAFF_MANAGING_DOMAIN],
|
||||||
|
"domain-users-add": [IS_DOMAIN_MANAGER, IS_STAFF_MANAGING_DOMAIN],
|
||||||
|
"domain-user-delete": [IS_DOMAIN_MANAGER, IS_STAFF_MANAGING_DOMAIN],
|
||||||
|
# Portfolio management
|
||||||
|
"domains": [HAS_PORTFOLIO_DOMAINS_ANY_PERM],
|
||||||
|
"no-portfolio-domains": [IS_PORTFOLIO_MEMBER],
|
||||||
|
"no-organization-domains": [IS_PORTFOLIO_MEMBER],
|
||||||
|
"members": [HAS_PORTFOLIO_MEMBERS_ANY_PERM],
|
||||||
|
"member": [HAS_PORTFOLIO_MEMBERS_ANY_PERM],
|
||||||
|
"member-delete": [HAS_PORTFOLIO_MEMBERS_EDIT],
|
||||||
|
"member-permissions": [HAS_PORTFOLIO_MEMBERS_EDIT],
|
||||||
|
"member-domains": [HAS_PORTFOLIO_MEMBERS_ANY_PERM],
|
||||||
|
"member-domains-edit": [HAS_PORTFOLIO_MEMBERS_EDIT],
|
||||||
|
"invitedmember": [HAS_PORTFOLIO_MEMBERS_ANY_PERM],
|
||||||
|
"invitedmember-delete": [HAS_PORTFOLIO_MEMBERS_EDIT],
|
||||||
|
"invitedmember-permissions": [HAS_PORTFOLIO_MEMBERS_EDIT],
|
||||||
|
"invitedmember-domains": [HAS_PORTFOLIO_MEMBERS_ANY_PERM],
|
||||||
|
"invitedmember-domains-edit": [HAS_PORTFOLIO_MEMBERS_EDIT],
|
||||||
|
"new-member": [HAS_PORTFOLIO_MEMBERS_EDIT],
|
||||||
|
"domain-requests": [HAS_PORTFOLIO_DOMAIN_REQUESTS_ANY_PERM],
|
||||||
|
"no-portfolio-requests": [IS_PORTFOLIO_MEMBER],
|
||||||
|
"organization": [IS_PORTFOLIO_MEMBER],
|
||||||
|
"senior-official": [IS_PORTFOLIO_MEMBER],
|
||||||
|
# Domain requests
|
||||||
|
"domain-request-status": [HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT, IS_DOMAIN_REQUEST_CREATOR],
|
||||||
|
"domain-request-status-viewonly": [HAS_PORTFOLIO_DOMAIN_REQUESTS_VIEW_ALL],
|
||||||
|
"domain-request-withdraw-confirmation": [HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT, IS_DOMAIN_REQUEST_CREATOR],
|
||||||
|
"domain-request-withdrawn": [HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT, IS_DOMAIN_REQUEST_CREATOR],
|
||||||
|
"domain-request-delete": [HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT, IS_DOMAIN_REQUEST_CREATOR],
|
||||||
|
"edit-domain-request": [HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT, IS_DOMAIN_REQUEST_CREATOR],
|
||||||
|
# Admin functions
|
||||||
|
"analytics": [IS_CISA_ANALYST, IS_FULL_ACCESS],
|
||||||
|
"export_data_type": [IS_CISA_ANALYST, IS_FULL_ACCESS],
|
||||||
|
"export_data_full": [IS_CISA_ANALYST, IS_FULL_ACCESS],
|
||||||
|
"export_data_domain_requests_full": [IS_CISA_ANALYST, IS_FULL_ACCESS],
|
||||||
|
"export_data_federal": [IS_CISA_ANALYST, IS_FULL_ACCESS],
|
||||||
|
"export_domains_growth": [IS_CISA_ANALYST, IS_FULL_ACCESS],
|
||||||
|
"export_requests_growth": [IS_CISA_ANALYST, IS_FULL_ACCESS],
|
||||||
|
"export_managed_domains": [IS_CISA_ANALYST, IS_FULL_ACCESS],
|
||||||
|
"export_unmanaged_domains": [IS_CISA_ANALYST, IS_FULL_ACCESS],
|
||||||
|
"transfer_user": [IS_CISA_ANALYST, IS_FULL_ACCESS],
|
||||||
|
# Analytics
|
||||||
|
"all-domain-metadata": [IS_STAFF],
|
||||||
|
"current-full": [IS_STAFF],
|
||||||
|
"all-domain-requests-metadata": [IS_STAFF],
|
||||||
|
"domain-growth": [IS_STAFF],
|
||||||
|
"request-growth": [IS_STAFF],
|
||||||
|
"managed-domains": [IS_STAFF],
|
||||||
|
"unmanaged-domains": [IS_STAFF],
|
||||||
|
# Reports
|
||||||
|
"export-user-domains-as-csv": [IS_STAFF],
|
||||||
|
"export-portfolio-members-as-csv": [IS_STAFF],
|
||||||
|
"export_members_portfolio": [HAS_PORTFOLIO_MEMBERS_VIEW],
|
||||||
|
"export_data_type_user": [ALL],
|
||||||
|
# API endpoints
|
||||||
|
"get-senior-official-from-federal-agency-json": [IS_CISA_ANALYST, IS_FULL_ACCESS, IS_OMB_ANALYST],
|
||||||
|
"get-portfolio-json": [IS_CISA_ANALYST, IS_FULL_ACCESS, IS_OMB_ANALYST],
|
||||||
|
"get-suborganization-list-json": [IS_CISA_ANALYST, IS_FULL_ACCESS, IS_OMB_ANALYST],
|
||||||
|
"get-federal-and-portfolio-types-from-federal-agency-json": [IS_CISA_ANALYST, IS_FULL_ACCESS, IS_OMB_ANALYST],
|
||||||
|
"get-action-needed-email-for-user-json": [IS_CISA_ANALYST, IS_FULL_ACCESS, IS_OMB_ANALYST],
|
||||||
|
"get-rejection-email-for-user-json": [IS_CISA_ANALYST, IS_FULL_ACCESS, IS_OMB_ANALYST],
|
||||||
|
"get_domains_json": [ALL],
|
||||||
|
"get_domain_requests_json": [ALL],
|
||||||
|
"get_portfolio_members_json": [HAS_PORTFOLIO_MEMBERS_ANY_PERM],
|
||||||
|
"get_member_domains_json": [HAS_PORTFOLIO_MEMBERS_ANY_PERM],
|
||||||
|
# User profile
|
||||||
|
"finish-user-profile-setup": [ALL],
|
||||||
|
"user-profile": [ALL],
|
||||||
|
# Invitation
|
||||||
|
"invitation-cancel": [IS_DOMAIN_MANAGER, IS_STAFF_MANAGING_DOMAIN],
|
||||||
|
# DNS Hosting
|
||||||
|
"prototype-domain-dns": [IS_STAFF],
|
||||||
|
# Domain request wizard
|
||||||
|
"start": [HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT, IS_DOMAIN_REQUEST_CREATOR],
|
||||||
|
"finished": [HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT, IS_DOMAIN_REQUEST_CREATOR],
|
||||||
|
"generic_org_type": [HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT, IS_DOMAIN_REQUEST_CREATOR],
|
||||||
|
"tribal_government": [HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT, IS_DOMAIN_REQUEST_CREATOR],
|
||||||
|
"organization_federal": [HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT, IS_DOMAIN_REQUEST_CREATOR],
|
||||||
|
"organization_election": [HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT, IS_DOMAIN_REQUEST_CREATOR],
|
||||||
|
"organization_contact": [HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT, IS_DOMAIN_REQUEST_CREATOR],
|
||||||
|
"about_your_organization": [HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT, IS_DOMAIN_REQUEST_CREATOR],
|
||||||
|
"senior_official": [HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT, IS_DOMAIN_REQUEST_CREATOR],
|
||||||
|
"current_sites": [HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT, IS_DOMAIN_REQUEST_CREATOR],
|
||||||
|
"dotgov_domain": [HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT, IS_DOMAIN_REQUEST_CREATOR],
|
||||||
|
"purpose": [HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT, IS_DOMAIN_REQUEST_CREATOR],
|
||||||
|
"other_contacts": [HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT, IS_DOMAIN_REQUEST_CREATOR],
|
||||||
|
"additional_details": [HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT, IS_DOMAIN_REQUEST_CREATOR],
|
||||||
|
"requirements": [HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT, IS_DOMAIN_REQUEST_CREATOR],
|
||||||
|
"review": [HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT, IS_DOMAIN_REQUEST_CREATOR],
|
||||||
|
"portfolio_requesting_entity": [HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT, IS_DOMAIN_REQUEST_CREATOR],
|
||||||
|
"portfolio_additional_details": [HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT, IS_DOMAIN_REQUEST_CREATOR],
|
||||||
|
}
|
||||||
|
|
||||||
|
UNCHECKED_URLS = [
|
||||||
|
"health",
|
||||||
|
"openid/",
|
||||||
|
"get-current-federal",
|
||||||
|
"get-current-full",
|
||||||
|
"available",
|
||||||
|
"rdap",
|
||||||
|
"todo",
|
||||||
|
"logout",
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
|
def verify_all_urls_have_permissions():
|
||||||
|
"""
|
||||||
|
Utility function to verify that all URLs in the application have defined permissions
|
||||||
|
in the permissions mapping.
|
||||||
|
"""
|
||||||
|
|
||||||
|
resolver = get_resolver()
|
||||||
|
missing_permissions = []
|
||||||
|
missing_names = []
|
||||||
|
|
||||||
|
# Collect all URL pattern names
|
||||||
|
for pattern in resolver.url_patterns:
|
||||||
|
# Skip URLResolver objects (like admin.site.urls)
|
||||||
|
if isinstance(pattern, URLResolver):
|
||||||
|
continue
|
||||||
|
|
||||||
|
if hasattr(pattern, "name") and pattern.name:
|
||||||
|
if pattern.name not in URL_PERMISSIONS and pattern.name not in UNCHECKED_URLS:
|
||||||
|
missing_permissions.append(pattern.name)
|
||||||
|
else:
|
||||||
|
raise ValueError(f"URL pattern {pattern} has no name")
|
||||||
|
|
||||||
|
if missing_names:
|
||||||
|
raise ValueError(f"The following URL patterns have no name: {missing_names}")
|
||||||
|
|
||||||
|
return missing_permissions
|
||||||
|
|
||||||
|
|
||||||
|
def validate_permissions(): # noqa: C901
|
||||||
|
"""
|
||||||
|
Validates that all URL patterns have consistent permission rules between
|
||||||
|
the centralized mapping and view decorators.
|
||||||
|
|
||||||
|
Returns a dictionary of issues found.
|
||||||
|
"""
|
||||||
|
|
||||||
|
resolver = get_resolver()
|
||||||
|
issues = {
|
||||||
|
"missing_in_mapping": [], # URLs with decorators but not in mapping
|
||||||
|
"missing_decorator": [], # URLs in mapping but missing decorators
|
||||||
|
"permission_mismatch": [], # URLs with different permissions
|
||||||
|
}
|
||||||
|
|
||||||
|
def check_url_pattern(pattern, parent_path=""):
|
||||||
|
if isinstance(pattern, URLPattern):
|
||||||
|
view_func = pattern.callback
|
||||||
|
path = f"{parent_path}/{pattern.pattern}"
|
||||||
|
url_name = pattern.name
|
||||||
|
|
||||||
|
if url_name:
|
||||||
|
# Skip check for endpoints that intentionally have no decorator
|
||||||
|
if url_name in UNCHECKED_URLS:
|
||||||
|
return
|
||||||
|
|
||||||
|
# Check if view has decorator but missing from mapping
|
||||||
|
if getattr(view_func, "has_explicit_access", False) and url_name not in URL_PERMISSIONS:
|
||||||
|
issues["missing_in_mapping"].append((url_name, path))
|
||||||
|
|
||||||
|
# Check if view is in mapping but missing decorator
|
||||||
|
elif url_name in URL_PERMISSIONS and not getattr(view_func, "has_explicit_access", False):
|
||||||
|
issues["missing_decorator"].append((url_name, path))
|
||||||
|
|
||||||
|
# Check if permissions match (more complex, may need refinement)
|
||||||
|
elif getattr(view_func, "has_explicit_access", False) and url_name in URL_PERMISSIONS:
|
||||||
|
view_permissions = getattr(view_func, "_access_rules", set())
|
||||||
|
mapping_permissions = set(URL_PERMISSIONS[url_name])
|
||||||
|
|
||||||
|
if view_permissions != mapping_permissions:
|
||||||
|
issues["permission_mismatch"].append((url_name, path, view_permissions, mapping_permissions))
|
||||||
|
|
||||||
|
elif isinstance(pattern, URLResolver):
|
||||||
|
# Handle included URL patterns (nested)
|
||||||
|
new_parent = f"{parent_path}/{pattern.pattern}"
|
||||||
|
for p in pattern.url_patterns:
|
||||||
|
check_url_pattern(p, new_parent)
|
||||||
|
|
||||||
|
# Check all URL patterns
|
||||||
|
for pattern in resolver.url_patterns:
|
||||||
|
check_url_pattern(pattern)
|
||||||
|
|
||||||
|
return issues
|
70
src/registrar/tests/test_permissions.py
Normal file
70
src/registrar/tests/test_permissions.py
Normal file
|
@ -0,0 +1,70 @@
|
||||||
|
"""
|
||||||
|
Tests for validating the permissions system consistency.
|
||||||
|
|
||||||
|
These tests ensure that:
|
||||||
|
1. All URLs have permissions defined in the centralized mapping
|
||||||
|
2. All views with permission decorators are in the mapping
|
||||||
|
3. The permissions in the decorators match those in the mapping
|
||||||
|
"""
|
||||||
|
|
||||||
|
from django.test import TestCase
|
||||||
|
from registrar.permissions import verify_all_urls_have_permissions, validate_permissions
|
||||||
|
|
||||||
|
|
||||||
|
class TestPermissionsMapping(TestCase):
|
||||||
|
"""Test the centralized permissions mapping for completeness and consistency."""
|
||||||
|
|
||||||
|
def test_all_urls_have_permissions(self):
|
||||||
|
"""Verify that all URL patterns in the application have permissions defined in the mapping."""
|
||||||
|
missing_urls = verify_all_urls_have_permissions()
|
||||||
|
|
||||||
|
# Format URLs for better readability in case of failure
|
||||||
|
if missing_urls:
|
||||||
|
formatted_urls = "\n".join([f" - {url}" for url in missing_urls])
|
||||||
|
self.fail(
|
||||||
|
f"The following URL patterns are missing from URL_PERMISSIONS mapping:\n{formatted_urls}\n"
|
||||||
|
f"Please add them to the URL_PERMISSIONS dictionary in registrar/permissions.py"
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_permission_decorator_consistency(self):
|
||||||
|
"""
|
||||||
|
Test that all views have consistent permission rules between
|
||||||
|
the centralized mapping and view decorators.
|
||||||
|
"""
|
||||||
|
issues = validate_permissions()
|
||||||
|
|
||||||
|
error_messages = []
|
||||||
|
|
||||||
|
if issues["missing_in_mapping"]:
|
||||||
|
urls = "\n".join([f" - {name} (at {path})" for name, path in issues["missing_in_mapping"]])
|
||||||
|
error_messages.append(
|
||||||
|
f"The following URLs have permission decorators but are missing from the mapping:\n{urls}\n"
|
||||||
|
"Add these URLs to the URL_PERMISSIONS dictionary in registrar/permissions.py"
|
||||||
|
)
|
||||||
|
|
||||||
|
if issues["missing_decorator"]:
|
||||||
|
urls = "\n".join([f" - {name} (at {path})" for name, path in issues["missing_decorator"]])
|
||||||
|
error_messages.append(
|
||||||
|
f"The following URLs are in the mapping but missing @grant_access decorators:\n{urls}\n"
|
||||||
|
"Add appropriate @grant_access decorators to these views"
|
||||||
|
)
|
||||||
|
|
||||||
|
if issues["permission_mismatch"]:
|
||||||
|
mismatches = []
|
||||||
|
for name, path, view_perms, mapping_perms in issues["permission_mismatch"]:
|
||||||
|
view_perms_str = ", ".join(sorted(str(p) for p in view_perms))
|
||||||
|
mapping_perms_str = ", ".join(sorted(str(p) for p in mapping_perms))
|
||||||
|
mismatches.append(
|
||||||
|
f" - {name} (at {path}):\n"
|
||||||
|
f" Decorator: [{view_perms_str}]\n"
|
||||||
|
f" Mapping: [{mapping_perms_str}]"
|
||||||
|
)
|
||||||
|
|
||||||
|
error_messages.append(
|
||||||
|
f"The following URLs have mismatched permissions between decorators and mapping:\n"
|
||||||
|
f"{chr(10).join(mismatches)}\n"
|
||||||
|
"Update either the decorator or the mapping to ensure consistency"
|
||||||
|
)
|
||||||
|
|
||||||
|
if error_messages:
|
||||||
|
self.fail("\n\n".join(error_messages))
|
|
@ -15,6 +15,7 @@ from registrar.decorators import (
|
||||||
IS_DOMAIN_MANAGER,
|
IS_DOMAIN_MANAGER,
|
||||||
IS_DOMAIN_MANAGER_AND_NOT_PORTFOLIO_MEMBER,
|
IS_DOMAIN_MANAGER_AND_NOT_PORTFOLIO_MEMBER,
|
||||||
IS_PORTFOLIO_MEMBER_AND_DOMAIN_MANAGER,
|
IS_PORTFOLIO_MEMBER_AND_DOMAIN_MANAGER,
|
||||||
|
IS_STAFF,
|
||||||
IS_STAFF_MANAGING_DOMAIN,
|
IS_STAFF_MANAGING_DOMAIN,
|
||||||
grant_access,
|
grant_access,
|
||||||
)
|
)
|
||||||
|
@ -707,6 +708,7 @@ class PrototypeDomainDNSRecordForm(forms.Form):
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@grant_access(IS_STAFF)
|
||||||
class PrototypeDomainDNSRecordView(DomainFormBaseView):
|
class PrototypeDomainDNSRecordView(DomainFormBaseView):
|
||||||
template_name = "prototype_domain_dns.html"
|
template_name = "prototype_domain_dns.html"
|
||||||
form_class = PrototypeDomainDNSRecordForm
|
form_class = PrototypeDomainDNSRecordForm
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue