Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

How to prevent deletion of Django model from Django Admin, unless part of a cascade

Tags:

python

django

I have a project using Django 2.2.4.

I have a Django model called Company.

I use a post_save signal to make sure that as soon as a new Company is created, a new model instance called "Billing" is created, which is associated with that Company. This contains the company's billing information. This works great.

Since my Billing object is associated with a Company, and I use on_delete=models.CASCADE, as soon as the Company is deleted, the Billing object associated with that company is automatically deleted as well. This works great too.

Since the Billing object for each company is now automatically created and deleted along with the Company, there's no need for admins using the Django Admin web interface to ever have to manually create, or delete Billing objects. I want to hide this functionality from them.

Normally, the common way to prevent Django Admin from allowing someone to add or delete an object is by adding this to that model's ModelAdmin in admin.py:

class BillingAdmin(admin.ModelAdmin):
    ...

    # Prevent deletion from admin portal
    def has_delete_permission(self, request, obj=None):
        return False

    # Prevent adding from admin portal
    def has_add_permission(self, request, obj=None):
        return False

This works, and does indeed hide the ability for Admins to create or delete instances of the Billing object manually. It does however have one negative side-effect: Django Admin users can no longer delete a company. When deleting a company, Django does a lookup for all associated objects that need to also be deleted, notices that the user isn't allowed to delete the associated Billing object, and prevents the user from deleting the Company.

While I don't want Django Admin users to be able to manually create or delete instances of the Billing model, I still want them to be able to delete an entire Company, which will result in the deletion of the instance of the Billing model associated with that Company.

In my case, preventing users from deleting an instance of the Billing model isn't so much a security feature, as it is intended to prevent confusion, by not letting the database end up in a state where a Company exists, but no billing object exists for it. Django would obviously not have a problem with this, but it would confuse users.

Is there a workaround for this?

Update:

With the has_delete_permission set, if you try to delete a company through Django Admin, you get this:

enter image description here

No exceptions are thrown. At least none that aren't caught, and appear in the Django logs.

My Models look like this:

class Company(Group):
    ...

class Billing(models.Model):
    company = AutoOneToOneField('Company', on_delete=models.CASCADE, blank=False, null=False, related_name="billing")
    monthly_rate = models.DecimalField(max_digits=10, decimal_places=2, default=0, blank=False, null=False)

# Create billing object for a company when it is first created
@receiver(post_save, sender=Company)
def create_billing_for_company(sender, instance, created, *args, **kwargs):
    if created:
        Billing.objects.create(company=instance)

The AutoOneToOneField is part of django-annoying. It makes sure that if you run MyCompany.billing, and an associated billing object doesn't exist yet, one will be created automatically, rather than an exception being raised. May not be required here, since I automatically create the object when the company is created, but it can't hurt, and ensures my code never needs to worry about the associated object not existing.

Also note that I have NOT overridden my Billing model's delete function.

like image 325
John Avatar asked Aug 26 '19 15:08

John


2 Answers

Another option is to override specifically designed for this method get_deleted_objects in main Company ModelAdmin - to allow deleting all related objects when deleting company from admin web.

class CompanyAdmin(admin.ModelAdmin):
    def get_deleted_objects(self, objs, request):
        """
        Allow deleting related objects if their model is present in admin_site
        and user does not have permissions to delete them from admin web
        """
        deleted_objects, model_count, perms_needed, protected = \
            super().get_deleted_objects(objs, request)
        return deleted_objects, model_count, set(), protected

Here we replace perms_needed with empty set() - which is a set of permissions user failed to satisfy for deleting related objects via admin site.


When deleting objects via django admin it:

  • checks if user has permissions to delete main object
  • calculates list of other related objects that should be deleted as well
  • for these related objects if their model is registered in admin_site, django performs additional permission check
  • if the user has admin site permissions to delete these related objects as well
  • if user does not have permissions to delete related objects - these required permissions are added to list and shown as error page

To get the list of related objects to be deleted with main one utility method is used - get_deleted_objects

And since Django 2.1 there is more comfortable way to override it directly from ModelAdmin instance: get_deleted_objects

like image 71
Oleg Russkin Avatar answered Sep 20 '22 19:09

Oleg Russkin


After a little digging, it does appear that the ModelAdmin will simply call delete() on the object, meaning that it shouldn't look at your billing permissions for admin specifically. Looking at the model delete also confirms that it does not care what the admin permissions are.

I got curious, and wondered if maybe the has_delete_permission function looks at related objects. That also didn't appear to be the case. At this point, I'm curious if you have overridden your Billing model's delete function? That would prevent deletion, and if you have CASCADE set as your on_delete for the relation, it would not allow you to finish deleting the Company at that point because it was unable to cascade delete.

If you have a stack trace or explicit error message, please share it.


With that said, I don't know if I agree with the approach to this. I think it would make more sense to enforce this at the model level of Billing. When attempting a delete, you could check if there are no other Billing objects for the Company, and if so, raise a validation error notifying the user that a Company must have at least one Billing. I don't know your models since they aren't posted, so if it's a one-to-one relation, please ignore this. Here's a rough idea of how I would expect it to look otherwise:

def delete(self):
    other_billing = Billing.objects.filter(company_id=self.company.id).exclude(id=self.id).first()
    if not other_billing:
        raise ValidationError({"message": "A company must have at least one Billing."})
    super().delete()

Edit: Here's a method using ModelAdmin.delete_model() which won't raise an exception.

def delete_model(self, request, billing):
    other_billing = Billing.objects.filter(company_id=billing.company.id).exclude(id=billing.id).first()
    if not other_billing:
        # from django.contrib import messages
        messages.error(request, "A company must have at least one Billing.")
    else:
        super().delete_model(request, billing)

EDIT: I did find that you have access to the request, which seems to be the only reliable way via has_delete_permissions() to check whether you're on the admin change page for your model or not. For the record I think this way is hacky and I don't recommend it. However, it would allow for cascading deletes while not allowing delete via the change page (it will hide the button):

def has_delete_permissions(self, request, obj=None):
    # If we have an object, it's been fetched for deletion or to check permission against it.
    if isinstance(obj, Billing):
        if request.path == reverse("admin:<APP_NAME>_billing_change", args=[obj.id]):
            return False

    return True
like image 34
tredzko Avatar answered Sep 24 '22 19:09

tredzko