From cb20628e7b1fd45c6b8f3cc8e1a9db05bc0a1d39 Mon Sep 17 00:00:00 2001 From: Ciprian Radulescu Date: Thu, 16 Apr 2026 13:43:19 +0300 Subject: [PATCH] refactor(users): migrate remaining Python is_staff checks to is_staff_user (#174) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - decorators.py: can_manage_financial_data now gates on is_staff_user so staff-by-role users (staff_role="admin" with is_staff=False) are correctly recognized as staff for financial data management - context_processors.py: can_access_admin uses is_staff_user with getattr guard for AnonymousUser safety - permissions.py: can_deploy_nodes uses is_staff_user so support agents with deploy perms are no longer locked out - orders/views.py: _validate_manual_price_override uses is_staff_user and cleaned up hasattr guards to getattr pattern - Update permission_matrix test to reflect that staff_role="admin" users are now correctly classified as staff regardless of is_staff flag Role-specific checks (staff_role in _REVIEW_APPROVE_ROLES, billing_staff_required, support_staff_required) are intentionally kept — they gate specific roles, not generic staff access. Audit metadata fields (is_staff in audit/services.py) and API response fields (is_staff in api/users/views.py) are intentionally unchanged as they record the raw Django flag, not the computed property. Addresses #174. Signed-off-by: Ciprian Radulescu --- services/platform/apps/common/context_processors.py | 2 +- services/platform/apps/common/decorators.py | 2 +- services/platform/apps/infrastructure/permissions.py | 2 +- services/platform/apps/orders/views.py | 4 ++-- services/platform/tests/orders/test_orders_security.py | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/services/platform/apps/common/context_processors.py b/services/platform/apps/common/context_processors.py index 77e1c405..6747e7ee 100644 --- a/services/platform/apps/common/context_processors.py +++ b/services/platform/apps/common/context_processors.py @@ -58,7 +58,7 @@ def user_permissions(request: HttpRequest) -> dict[str, Any]: "can_view_audit": request.user.has_perm("audit.view_auditlog"), }, "user_role": getattr(request.user, "role", "user"), - "can_access_admin": request.user.is_staff, + "can_access_admin": getattr(request.user, "is_staff_user", False), } diff --git a/services/platform/apps/common/decorators.py b/services/platform/apps/common/decorators.py index 6d97c1dc..6a4e522c 100644 --- a/services/platform/apps/common/decorators.py +++ b/services/platform/apps/common/decorators.py @@ -202,7 +202,7 @@ def can_manage_financial_data(user: User) -> bool: if user.is_superuser: return True - if not user.is_staff: + if not user.is_staff_user: return False allowed_roles = ["admin", "billing", "manager"] diff --git a/services/platform/apps/infrastructure/permissions.py b/services/platform/apps/infrastructure/permissions.py index faf93440..d87a7c80 100644 --- a/services/platform/apps/infrastructure/permissions.py +++ b/services/platform/apps/infrastructure/permissions.py @@ -87,7 +87,7 @@ def can_deploy_nodes(user: User) -> bool: return True # Staff with deploy permission - if user.is_staff and user.has_perm(PERM_DEPLOY_NODES): + if user.is_staff_user and user.has_perm(PERM_DEPLOY_NODES): return True return user.has_perm(PERM_DEPLOY_NODES) diff --git a/services/platform/apps/orders/views.py b/services/platform/apps/orders/views.py index 332f41ef..b6d3ce4d 100644 --- a/services/platform/apps/orders/views.py +++ b/services/platform/apps/orders/views.py @@ -144,14 +144,14 @@ def _validate_manual_price_override( manual_price_cents: int, product_price_cents: int, user: User, context: str = "" ) -> tuple[bool, str]: """🔒 Validate manual price override for security""" - if not hasattr(user, "is_staff") or not user.is_staff: + if not getattr(user, "is_staff_user", False): logger.warning( f"⛔ [Orders] Price Security: Unauthorized price override attempt by user {getattr(user, 'id', 'Unknown')} ({getattr(user, 'email', 'Unknown')}) in context: {context}" ) return False, "Insufficient permissions for price override" # Check for specific financial permissions (staff role required) - if not (user.is_superuser or (hasattr(user, "staff_role") and user.staff_role in ["admin", "billing"])): + if not (user.is_superuser or getattr(user, "staff_role", "") in ["admin", "billing"]): logger.warning( f"⛔ [Orders] Price Security: Staff user {getattr(user, 'id', 'Unknown')} ({getattr(user, 'email', 'Unknown')}) lacks financial permissions for price override in context: {context}" ) diff --git a/services/platform/tests/orders/test_orders_security.py b/services/platform/tests/orders/test_orders_security.py index 60ad6497..833ba26a 100644 --- a/services/platform/tests/orders/test_orders_security.py +++ b/services/platform/tests/orders/test_orders_security.py @@ -588,7 +588,7 @@ def test_permission_system_consistency(self) -> None: (True, False, ""): False, # Staff with no role (True, False, None): False, # Staff with None role (False, True, ""): True, # Superuser overrides everything - (False, False, "admin"): False, # Non-staff with admin role should be False + (False, False, "admin"): True, # staff_role="admin" is staff via is_staff_user } for (is_staff, is_superuser, staff_role), expected in permission_matrix.items():