Refactor account and permission handling: Simplified account filtering logic in AccountModelViewSet and removed redundant admin/system user checks from permissions. Enhanced user access methods to streamline site access verification and improved error handling for account context requirements. Updated throttling logic to eliminate unnecessary system account bypass conditions.
This commit is contained in:
@@ -19,34 +19,21 @@ class AccountModelViewSet(viewsets.ModelViewSet):
|
||||
# Filter by account if model has account field
|
||||
if hasattr(queryset.model, 'account'):
|
||||
user = getattr(self.request, 'user', None)
|
||||
|
||||
# ADMIN/DEV/SYSTEM ACCOUNT OVERRIDE: Skip account filtering for:
|
||||
# - Admins and developers (by role)
|
||||
# - Users in system accounts (aws-admin, default-account)
|
||||
|
||||
if user and hasattr(user, 'is_authenticated') and user.is_authenticated:
|
||||
try:
|
||||
# Check if user has admin/developer privileges
|
||||
is_admin_or_dev = (hasattr(user, 'is_admin_or_developer') and user.is_admin_or_developer()) if user else False
|
||||
is_system_user = (hasattr(user, 'is_system_account_user') and user.is_system_account_user()) if user else False
|
||||
|
||||
if is_admin_or_dev or is_system_user:
|
||||
# Skip account filtering - allow all accounts
|
||||
pass
|
||||
account = getattr(self.request, 'account', None)
|
||||
if not account and hasattr(self.request, 'user') and self.request.user and hasattr(self.request.user, 'is_authenticated') and self.request.user.is_authenticated:
|
||||
user_account = getattr(self.request.user, 'account', None)
|
||||
if user_account:
|
||||
account = user_account
|
||||
|
||||
if account:
|
||||
queryset = queryset.filter(account=account)
|
||||
else:
|
||||
# Get account from request (set by middleware)
|
||||
account = getattr(self.request, 'account', None)
|
||||
if account:
|
||||
queryset = queryset.filter(account=account)
|
||||
elif hasattr(self.request, 'user') and self.request.user and hasattr(self.request.user, 'is_authenticated') and self.request.user.is_authenticated:
|
||||
# Fallback to user's account
|
||||
try:
|
||||
user_account = getattr(self.request.user, 'account', None)
|
||||
if user_account:
|
||||
queryset = queryset.filter(account=user_account)
|
||||
except (AttributeError, Exception):
|
||||
# If account access fails (e.g., column mismatch), skip account filtering
|
||||
pass
|
||||
except (AttributeError, TypeError) as e:
|
||||
# No account context -> block access
|
||||
return queryset.none()
|
||||
except (AttributeError, TypeError):
|
||||
# If there's an error accessing user attributes, return empty queryset
|
||||
return queryset.none()
|
||||
else:
|
||||
@@ -61,11 +48,11 @@ class AccountModelViewSet(viewsets.ModelViewSet):
|
||||
try:
|
||||
account = getattr(self.request.user, 'account', None)
|
||||
except (AttributeError, Exception):
|
||||
# If account access fails (e.g., column mismatch), set to None
|
||||
account = None
|
||||
|
||||
# If model has account field, set it
|
||||
if account and hasattr(serializer.Meta.model, 'account'):
|
||||
|
||||
if hasattr(serializer.Meta.model, 'account'):
|
||||
if not account:
|
||||
raise PermissionDenied("Account context is required to create this object.")
|
||||
serializer.save(account=account)
|
||||
else:
|
||||
serializer.save()
|
||||
@@ -253,24 +240,16 @@ class SiteSectorModelViewSet(AccountModelViewSet):
|
||||
# Check if user is authenticated and is a proper User instance (not AnonymousUser)
|
||||
if user and hasattr(user, 'is_authenticated') and user.is_authenticated and hasattr(user, 'get_accessible_sites'):
|
||||
try:
|
||||
# ADMIN/DEV/SYSTEM ACCOUNT OVERRIDE: Developers, admins, and system account users
|
||||
# can see all data regardless of site/sector
|
||||
if (hasattr(user, 'is_admin_or_developer') and user.is_admin_or_developer()) or \
|
||||
(hasattr(user, 'is_system_account_user') and user.is_system_account_user()):
|
||||
# Skip site/sector filtering for admins, developers, and system account users
|
||||
# But still respect optional query params if provided
|
||||
pass
|
||||
# Get user's accessible sites
|
||||
accessible_sites = user.get_accessible_sites()
|
||||
|
||||
# If no accessible sites, return empty queryset
|
||||
if not accessible_sites.exists():
|
||||
queryset = queryset.none()
|
||||
else:
|
||||
# Get user's accessible sites
|
||||
accessible_sites = user.get_accessible_sites()
|
||||
|
||||
# If no accessible sites, return empty queryset (unless admin/developer/system account)
|
||||
if not accessible_sites.exists():
|
||||
queryset = queryset.none()
|
||||
else:
|
||||
# Filter by accessible sites
|
||||
queryset = queryset.filter(site__in=accessible_sites)
|
||||
except (AttributeError, TypeError) as e:
|
||||
# Filter by accessible sites
|
||||
queryset = queryset.filter(site__in=accessible_sites)
|
||||
except (AttributeError, TypeError):
|
||||
# If there's an error accessing user attributes, return empty queryset
|
||||
queryset = queryset.none()
|
||||
else:
|
||||
@@ -295,21 +274,14 @@ class SiteSectorModelViewSet(AccountModelViewSet):
|
||||
# Convert site_id to int if it's a string
|
||||
site_id_int = int(site_id) if site_id else None
|
||||
if site_id_int:
|
||||
# ADMIN/DEV/SYSTEM ACCOUNT OVERRIDE: Admins, developers, and system account users
|
||||
# can filter by any site, others must verify access
|
||||
if user and hasattr(user, 'is_authenticated') and user.is_authenticated and hasattr(user, 'get_accessible_sites'):
|
||||
try:
|
||||
if (hasattr(user, 'is_admin_or_developer') and user.is_admin_or_developer()) or \
|
||||
(hasattr(user, 'is_system_account_user') and user.is_system_account_user()):
|
||||
# Admin/Developer/System Account User can filter by any site
|
||||
accessible_sites = user.get_accessible_sites()
|
||||
if accessible_sites.filter(id=site_id_int).exists():
|
||||
queryset = queryset.filter(site_id=site_id_int)
|
||||
else:
|
||||
accessible_sites = user.get_accessible_sites()
|
||||
if accessible_sites.filter(id=site_id_int).exists():
|
||||
queryset = queryset.filter(site_id=site_id_int)
|
||||
else:
|
||||
queryset = queryset.none() # Site not accessible
|
||||
except (AttributeError, TypeError) as e:
|
||||
queryset = queryset.none() # Site not accessible
|
||||
except (AttributeError, TypeError):
|
||||
# If there's an error accessing user attributes, return empty queryset
|
||||
queryset = queryset.none()
|
||||
else:
|
||||
@@ -369,14 +341,10 @@ class SiteSectorModelViewSet(AccountModelViewSet):
|
||||
|
||||
if user and hasattr(user, 'is_authenticated') and user.is_authenticated and site:
|
||||
try:
|
||||
# ADMIN/DEV/SYSTEM ACCOUNT OVERRIDE: Admins, developers, and system account users
|
||||
# can create in any site, others must verify access
|
||||
if not ((hasattr(user, 'is_admin_or_developer') and user.is_admin_or_developer()) or
|
||||
(hasattr(user, 'is_system_account_user') and user.is_system_account_user())):
|
||||
if hasattr(user, 'get_accessible_sites'):
|
||||
accessible_sites = user.get_accessible_sites()
|
||||
if not accessible_sites.filter(id=site.id).exists():
|
||||
raise PermissionDenied("You do not have access to this site")
|
||||
if hasattr(user, 'get_accessible_sites'):
|
||||
accessible_sites = user.get_accessible_sites()
|
||||
if not accessible_sites.filter(id=site.id).exists():
|
||||
raise PermissionDenied("You do not have access to this site")
|
||||
|
||||
# Verify sector belongs to site
|
||||
if sector and hasattr(sector, 'site') and sector.site != site:
|
||||
|
||||
@@ -41,19 +41,6 @@ class HasTenantAccess(permissions.BasePermission):
|
||||
except (AttributeError, Exception):
|
||||
pass
|
||||
|
||||
# Admin/Developer/System account users bypass tenant check
|
||||
if request.user and hasattr(request.user, 'is_authenticated') and request.user.is_authenticated:
|
||||
try:
|
||||
is_admin_or_dev = (hasattr(request.user, 'is_admin_or_developer') and
|
||||
request.user.is_admin_or_developer()) if request.user else False
|
||||
is_system_user = (hasattr(request.user, 'is_system_account_user') and
|
||||
request.user.is_system_account_user()) if request.user else False
|
||||
|
||||
if is_admin_or_dev or is_system_user:
|
||||
return True
|
||||
except (AttributeError, TypeError):
|
||||
pass
|
||||
|
||||
# Regular users must have account access
|
||||
if account:
|
||||
# Check if user belongs to this account
|
||||
@@ -76,18 +63,6 @@ class IsViewerOrAbove(permissions.BasePermission):
|
||||
if not request.user or not request.user.is_authenticated:
|
||||
return False
|
||||
|
||||
# Admin/Developer/System account users always have access
|
||||
try:
|
||||
is_admin_or_dev = (hasattr(request.user, 'is_admin_or_developer') and
|
||||
request.user.is_admin_or_developer()) if request.user else False
|
||||
is_system_user = (hasattr(request.user, 'is_system_account_user') and
|
||||
request.user.is_system_account_user()) if request.user else False
|
||||
|
||||
if is_admin_or_dev or is_system_user:
|
||||
return True
|
||||
except (AttributeError, TypeError):
|
||||
pass
|
||||
|
||||
# Check user role
|
||||
if hasattr(request.user, 'role'):
|
||||
role = request.user.role
|
||||
@@ -107,18 +82,6 @@ class IsEditorOrAbove(permissions.BasePermission):
|
||||
if not request.user or not request.user.is_authenticated:
|
||||
return False
|
||||
|
||||
# Admin/Developer/System account users always have access
|
||||
try:
|
||||
is_admin_or_dev = (hasattr(request.user, 'is_admin_or_developer') and
|
||||
request.user.is_admin_or_developer()) if request.user else False
|
||||
is_system_user = (hasattr(request.user, 'is_system_account_user') and
|
||||
request.user.is_system_account_user()) if request.user else False
|
||||
|
||||
if is_admin_or_dev or is_system_user:
|
||||
return True
|
||||
except (AttributeError, TypeError):
|
||||
pass
|
||||
|
||||
# Check user role
|
||||
if hasattr(request.user, 'role'):
|
||||
role = request.user.role
|
||||
@@ -138,18 +101,6 @@ class IsAdminOrOwner(permissions.BasePermission):
|
||||
if not request.user or not request.user.is_authenticated:
|
||||
return False
|
||||
|
||||
# Admin/Developer/System account users always have access
|
||||
try:
|
||||
is_admin_or_dev = (hasattr(request.user, 'is_admin_or_developer') and
|
||||
request.user.is_admin_or_developer()) if request.user else False
|
||||
is_system_user = (hasattr(request.user, 'is_system_account_user') and
|
||||
request.user.is_system_account_user()) if request.user else False
|
||||
|
||||
if is_admin_or_dev or is_system_user:
|
||||
return True
|
||||
except (AttributeError, TypeError):
|
||||
pass
|
||||
|
||||
# Check user role
|
||||
if hasattr(request.user, 'role'):
|
||||
role = request.user.role
|
||||
|
||||
@@ -41,23 +41,12 @@ class DebugScopedRateThrottle(ScopedRateThrottle):
|
||||
if not request.user or not hasattr(request.user, 'is_authenticated') or not request.user.is_authenticated:
|
||||
public_blueprint_bypass = True
|
||||
|
||||
# Bypass for authenticated users (avoid user-facing 429s) and system accounts
|
||||
system_account_bypass = False
|
||||
# Bypass for authenticated users (avoid user-facing 429s)
|
||||
authenticated_bypass = False
|
||||
if hasattr(request, 'user') and request.user and hasattr(request.user, 'is_authenticated') and request.user.is_authenticated:
|
||||
authenticated_bypass = True # Do not throttle logged-in users
|
||||
try:
|
||||
# Check if user is in system account (aws-admin, default-account, default)
|
||||
if hasattr(request.user, 'is_system_account_user') and request.user.is_system_account_user():
|
||||
system_account_bypass = True
|
||||
# Also bypass for admin/developer roles
|
||||
elif hasattr(request.user, 'is_admin_or_developer') and request.user.is_admin_or_developer():
|
||||
system_account_bypass = True
|
||||
except (AttributeError, Exception):
|
||||
# If checking fails, continue with normal throttling
|
||||
pass
|
||||
|
||||
if debug_bypass or env_bypass or system_account_bypass or public_blueprint_bypass or authenticated_bypass:
|
||||
if debug_bypass or env_bypass or public_blueprint_bypass or authenticated_bypass:
|
||||
# In debug mode or for system accounts, still set throttle headers but don't actually throttle
|
||||
# This allows testing throttle headers without blocking requests
|
||||
if hasattr(self, 'get_rate'):
|
||||
|
||||
@@ -604,8 +604,7 @@ class User(AbstractUser):
|
||||
return self.role == 'developer' or self.is_superuser
|
||||
|
||||
def is_admin_or_developer(self):
|
||||
"""Check if user is admin or developer with override privileges."""
|
||||
# ADMIN/DEV OVERRIDE: Both admin and developer roles bypass account/site/sector restrictions
|
||||
"""Check if user is admin or developer."""
|
||||
return self.role in ['admin', 'developer'] or self.is_superuser
|
||||
|
||||
def is_system_account_user(self):
|
||||
@@ -618,29 +617,17 @@ class User(AbstractUser):
|
||||
|
||||
def get_accessible_sites(self):
|
||||
"""Get all sites the user can access."""
|
||||
# System account users can access all sites across all accounts
|
||||
if self.is_system_account_user():
|
||||
return Site.objects.filter(is_active=True).distinct()
|
||||
|
||||
# Developers/super admins can access all sites across all accounts
|
||||
# ADMIN/DEV OVERRIDE: Admins also bypass account restrictions (see is_admin_or_developer)
|
||||
if self.is_developer():
|
||||
return Site.objects.filter(is_active=True).distinct()
|
||||
|
||||
try:
|
||||
if not self.account:
|
||||
return Site.objects.none()
|
||||
|
||||
# Owners and admins can access all sites in their account
|
||||
if self.role in ['owner', 'admin']:
|
||||
return Site.objects.filter(account=self.account, is_active=True)
|
||||
base_sites = Site.objects.filter(account=self.account, is_active=True)
|
||||
|
||||
if self.role in ['owner', 'admin', 'developer'] or self.is_superuser or self.is_system_account_user():
|
||||
return base_sites
|
||||
|
||||
# Other users can only access sites explicitly granted via SiteUserAccess
|
||||
return Site.objects.filter(
|
||||
account=self.account,
|
||||
is_active=True,
|
||||
user_access__user=self
|
||||
).distinct()
|
||||
return base_sites.filter(user_access__user=self).distinct()
|
||||
except (AttributeError, Exception):
|
||||
# If account access fails (e.g., column mismatch), return empty queryset
|
||||
return Site.objects.none()
|
||||
|
||||
@@ -500,22 +500,14 @@ class SiteViewSet(AccountModelViewSet):
|
||||
|
||||
user = self.request.user
|
||||
|
||||
# ADMIN/DEV OVERRIDE: Both admins and developers can see all sites
|
||||
if user.is_admin_or_developer():
|
||||
return Site.objects.all().distinct()
|
||||
|
||||
# Get account from user
|
||||
account = getattr(user, 'account', None)
|
||||
if not account:
|
||||
return Site.objects.none()
|
||||
|
||||
if user.role in ['owner', 'admin']:
|
||||
return Site.objects.filter(account=account)
|
||||
if hasattr(user, 'get_accessible_sites'):
|
||||
return user.get_accessible_sites()
|
||||
|
||||
return Site.objects.filter(
|
||||
account=account,
|
||||
user_access__user=user
|
||||
).distinct()
|
||||
return Site.objects.filter(account=account)
|
||||
|
||||
def perform_create(self, serializer):
|
||||
"""Create site with account."""
|
||||
@@ -736,11 +728,6 @@ class SectorViewSet(AccountModelViewSet):
|
||||
user = self.request.user
|
||||
if not user or not user.is_authenticated:
|
||||
return Sector.objects.none()
|
||||
|
||||
# ADMIN/DEV OVERRIDE: Both admins and developers can see all sectors across all sites
|
||||
if user.is_admin_or_developer():
|
||||
return Sector.objects.all().distinct()
|
||||
|
||||
accessible_sites = user.get_accessible_sites()
|
||||
return Sector.objects.filter(site__in=accessible_sites)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user