Use organization role to check for edit and delete perms of service instances #148

Closed
tobru wants to merge 1 commit from instance-edit-permi-from-org into main
2 changed files with 29 additions and 8 deletions

View file

@ -3,6 +3,7 @@ from django.utils.translation import gettext_lazy as _
from django.views.generic import CreateView, DetailView
from rules.contrib.views import AutoPermissionRequiredMixin
from servala.core.rules import is_organization_admin
from servala.core.models import (
BillingEntity,
Organization,
@ -75,10 +76,10 @@ class OrganizationDashboardView(
)
recent_instances = service_instances.order_by("-created_at")[:5]
has_admin_permission = is_organization_admin(self.request.user, organization)
for instance in recent_instances:
instance.has_change_permission = self.request.user.has_perm(
"core.change_serviceinstance", instance
)
instance.has_change_permission = has_admin_permission
tobru marked this conversation as resolved

You could instead remove the is_staff permission from https://servala.app.codey.ch/servala/servala-portal/src/branch/main/src/servala/core/models/organization.py#L121

If you prefer to do the permission checks like this, I think we should remove the whole rules_permissions approach, because otherwise, it’ll be really unclear which places (views, middlewares, etc) will use the django-rules provided rules, and which places use methods like is_organization_admin directly.

You could instead remove the `is_staff` permission from https://servala.app.codey.ch/servala/servala-portal/src/branch/main/src/servala/core/models/organization.py#L121 If you prefer to do the permission checks like this, I think we should remove the whole `rules_permissions` approach, because otherwise, it’ll be really unclear which places (views, middlewares, etc) will use the `django-rules` provided rules, and which places use methods like `is_organization_admin` directly.

May I assign this PR to you to clean up as proposed?

May I assign this PR to you to clean up as proposed?
user_membership = OrganizationMembership.objects.filter(
user=self.request.user, organization=organization

View file

@ -6,6 +6,7 @@ from django.utils.functional import cached_property
from django.utils.translation import gettext_lazy as _
from django.views.generic import DetailView, ListView, UpdateView
from servala.core.rules import is_organization_admin
from servala.core.crd import deslugify
from servala.core.models import (
ControlPlaneCRD,
@ -219,12 +220,13 @@ class ServiceInstanceDetailView(
and self.object.spec
):
context["spec_fieldsets"] = self.get_nested_spec()
context["has_change_permission"] = self.request.user.has_perm(
ServiceInstance.get_perm("change"), self.object
)
context["has_delete_permission"] = self.request.user.has_perm(
ServiceInstance.get_perm("delete"), self.object
has_admin_permission = is_organization_admin(
self.request.user, self.object.organization
)
context["has_change_permission"] = has_admin_permission
context["has_delete_permission"] = has_admin_permission
return context
def get_nested_spec(self):
@ -338,6 +340,15 @@ class ServiceInstanceUpdateView(
template_name = "frontend/organizations/service_instance_update.html"
permission_type = "change"
def has_permission(self):
"""Override to use organization role-based permissions."""
# First check if user has organization access
if not self.has_organization_permission():
return False
# Then check if user has admin or owner role
return is_organization_admin(self.request.user, self.object.organization)
def get_form_class(self):
return self.object.context.model_form_class
@ -420,6 +431,15 @@ class ServiceInstanceDeleteView(
form_class = ServiceInstanceDeleteForm
permission_type = "delete"
def has_permission(self):
"""Override to use organization role-based permissions."""
# First check if user has organization access
if not self.has_organization_permission():
return False
# Then check if user has admin or owner role
return is_organization_admin(self.request.user, self.object.organization)
def form_valid(self, form):
try:
self.object.delete_instance(user=self.request.user)