0

Currently, I have two models, Parent and Child, with a one-to-many relationship. I am using the built-in generic class-based views for CRUD upon a Parent, where I'm using a django-guardian mixin to prevent users who do not own the Parent object from doing these operations, which works well. However, I want to be able to add Children to a Parent. This works fine using a generic CreateView and a modelform, where the pk of the parent is a kwarg passed in the url. However if the user changes the pk in the URL to another user's Parent object's pk, they can add a Child object to it. I want to use django-guardian (or some other means) to prevent a user from adding a Child to another User's Parent object. Can this be done or must it be done some other way? I have got it working by validating the Parent object belongs to the current user within get_form_kwargs in the CreateView but this seems hacky, and would prefer django-guardian.

(I am also not using the 'pk' kwarg in production, and instead using a different 'uuid' kwarg, but I'd like to fix this security hole nonetheless).

Code:

models.py

class Parent(models.Model): 
    user = models.ForeignKey(User, on_delete=models.CASCADE, null=True, blank=True) 
    name = models.CharField(max_length=200, null=True, blank=True)
    
    class Meta:
        permissions = (('manipulate', 'Manipulate'))

class Child(models.Model):
    parent = models.ForeignKey(Parent, on_delete=models.CASCADE)
    name = models.CharField(max_length=200, null=True, blank=True)

views.py

class CreateChildForm(ModelForm):
    class Meta:
        fields = ('name', 'parent')
        model = models.Child

    def __init__(self, *args, **kwargs):
        self.parent = kwargs.pop('parent')
        super().__init__(*args, **kwargs)
        self.fields['parent'].initial = self.parent
        self.fields['parent'].widget = HiddenInput()

class CreateChild(LoginRequiredMixin, CreateView):
    model = models.Child
    form_class = CreateChildForm
    success_url = reverse_lazy('home')

    def get_form_kwargs(self):
        try:
            self.parent = Parent.objects.get(uuid=self.kwargs['uuid'], user=self.request.user)
        except:
            raise PermissionDenied()

        kwargs = super().get_form_kwargs()
        kwargs['parent'] = self.parent
        return kwargs

    def form_valid(self, form):
        form.instance.parent = self.parent
        return super().form_valid(form)
buttery
  • 11
  • 3
  • Can you provide the models together with your `ModelForm` and the view? – Willem Van Onsem Sep 22 '21 at 17:34
  • Hi, sorry added theis now -- sorry if any name conflicts/clashes, names are changed from my actual code. – buttery Sep 22 '21 at 17:45
  • Normally the fact that you use `form.instance.parent = self.parent`, and the fact that `self.parent` has `user=self.request.user` should be sufficient to protect this. I would advise to omit the `parent` field for the form, but regardless, the fact that you check if the user is the owner of the parent with the given uuid, should suffice. – Willem Van Onsem Sep 22 '21 at 17:50
  • Thank you very much! I'll look into how to omit the field properly, thanks. – buttery Sep 24 '21 at 10:46

0 Answers0