Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#30702 closed Bug (invalid)

Form leaks all objects of model

Reported by: Kevin Olbrich Owned by: nobody
Component: Forms Version: 2.2
Severity: Normal Keywords: form, forms, queryset
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Hi!

During development I had to filter the possible values in my form based on the logged in user:

class DomainRegisterForm(UserKwargModelFormMixin, forms.Form):
    domain = forms.CharField(label='Domain', max_length=100)
    customer = forms.ModelChoiceField(queryset=MdatCustomers.objects.none())

    def __init__(self, *args, **kwargs):
        super(DomainRegisterForm, self).__init__(*args, **kwargs)
        self.fields['customer'].queryset = MdatCustomers.objects.filter(mdatmultitenancyusers__user=self.user)

UserKwargModelFormMixin is from django-braces and simply does pull the user from the request:

class UserKwargModelFormMixin(object):
    """
    Generic model form mixin for popping user out of the kwargs and
    attaching it to the instance.

    This mixin must precede forms.ModelForm/forms.Form. The form is not
    expecting these kwargs to be passed in, so they must be popped off before
    anything else is done.
    """
    def __init__(self, *args, **kwargs):
        self.user = kwargs.pop("user", None)  # Pop the user off the
                                              # passed in kwargs.
        super(UserKwargModelFormMixin, self).__init__(*args, **kwargs)

The values are shown correctly on first load of the form. I can choose the value and send it.

I've met two issues with this:
1) After submit of the form, I get an "invalid_choice" error. The value I'd chosen is then filtered from the queryset even if it was valid.
2) The queryset is not filtered anymore, it shows ALL values (like: MdatCustomers.objects.all() ).

As I use this to give a user access to multiple companies, I absolutely don't want them to browse all ;-)

I did some research in the code but was unable to spot where this happens.

Kind regards
Kevin

Change History (8)

comment:1 by Carlton Gibson, 5 years ago

Resolution: invalid
Status: newclosed

Sorry, this isn't an Issue Report as it stands. (It's a usage question.) Please see TicketClosingReasons/UseSupportChannels.

comment:2 by Kevin Olbrich, 5 years ago

Actually I think this is a serious bug.
Do you expect Django to leak all model data on an error?
You can try it yourself, all model data will be rendered instead of the query sets filtered data.

comment:3 by Claude Paroz, 5 years ago

I guess Carlton closed your report because it sounds like an error in your code. Your report does not contain any hint that Django is at fault. Please explore a little more what's happening in your case.

comment:4 by Kevin Olbrich, 5 years ago

I've attached some demo code in my initial description. That code is enough to reproduce the issue.
I was unable to spot where in the code it decides to drop all filters.

comment:5 by Claude Paroz, 5 years ago

Sorry, but for us this code is not sufficient. Maybe upload a sample project?

comment:6 by Kevin Olbrich, 5 years ago

Sure, np.

Project:
https://bitbucket.org/code-orange/django-playground-project
(recursive clone!)

Steps to reproduce:

  1. Clone, pip, migrate, etc.
  2. Create user (initial superadmin is sufficient)
  3. Create customer 1 "Allowed Customer Example"
  4. Create customer 2 "Super Secret Customer"
  5. Add user + "Allowed Customer Example" relation in multitenancy table
  6. Access website under /playground/forms/register
  7. Enter a domain and choose customer (only "Allowed Customer Example" will be visible)
  8. Submit form
  9. Error will be displayed, Customer only shows "Super Secret Customer" ("Allowed Customer Example" missing!).

I can confirm this for SQLite and MySQL.

The user should never be able to see other customers, as the filter is:

MdatCustomers.objects.filter(mdatmultitenancyusers__user=self.user)
Last edited 5 years ago by Kevin Olbrich (previous) (diff)

comment:7 by Carlton Gibson, 5 years ago

This is an issue with your usage of the django-braces mixin, not Django. You need to ask for help on the support channels for django-braces, or django-users or StackOverflow, rather than here.

It's not a bug with Django. To see this add an assert self.user is not None in your form's __init__() after the super() call. Follow the traceback when that fails.

In general, a bug is 99% likely to be in your code and only 1% likely (if that for something like ORM filtering which is as exercised as can be) to be in the framework. So, please, before opening an issue pursue support channels to help you pin down exactly what's going on. To show a fault in Django, at the least you need to remove additional code such as django-braces.

Sorry if that seems harsh, but we can't offer usage support here. We just don't have the capacity. I hope that makes sense to you.

comment:8 by Kevin Olbrich, 5 years ago

I've got the point and you are right: AssertionError.
Thank you very much for pointing me in the right direction!

Note: See TracTickets for help on using tickets.
Back to Top