[Django] #30702: Form leaks all objects of model

11 views
Skip to first unread message

Django

unread,
Aug 12, 2019, 5:58:21 AM8/12/19
to django-...@googlegroups.com
#30702: Form leaks all objects of model
-------------------------------------+-------------------------------------
Reporter: Kevin | Owner: nobody
Olbrich |
Type: Bug | Status: new
Component: Forms | Version: 2.2
Severity: Normal | Keywords: form, forms,
Triage Stage: | queryset
Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
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

--
Ticket URL: <https://code.djangoproject.com/ticket/30702>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Aug 12, 2019, 6:25:43 AM8/12/19
to django-...@googlegroups.com
#30702: Form leaks all objects of model
-------------------------------------+-------------------------------------
Reporter: Kevin Olbrich | Owner: nobody
Type: Bug | Status: closed
Component: Forms | Version: 2.2
Severity: Normal | Resolution: invalid

Keywords: form, forms, | Triage Stage:
queryset | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Carlton Gibson):

* status: new => closed
* resolution: => invalid


Comment:

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

--
Ticket URL: <https://code.djangoproject.com/ticket/30702#comment:1>

Django

unread,
Aug 12, 2019, 8:25:18 AM8/12/19
to django-...@googlegroups.com
#30702: Form leaks all objects of model
-------------------------------------+-------------------------------------
Reporter: Kevin Olbrich | Owner: nobody
Type: Bug | Status: closed
Component: Forms | Version: 2.2
Severity: Normal | Resolution: invalid

Keywords: form, forms, | Triage Stage:
queryset | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Kevin Olbrich):

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.

--
Ticket URL: <https://code.djangoproject.com/ticket/30702#comment:2>

Django

unread,
Aug 12, 2019, 10:29:43 AM8/12/19
to django-...@googlegroups.com
#30702: Form leaks all objects of model
-------------------------------------+-------------------------------------
Reporter: Kevin Olbrich | Owner: nobody
Type: Bug | Status: closed
Component: Forms | Version: 2.2
Severity: Normal | Resolution: invalid

Keywords: form, forms, | Triage Stage:
queryset | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Claude Paroz):

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.

--
Ticket URL: <https://code.djangoproject.com/ticket/30702#comment:3>

Django

unread,
Aug 12, 2019, 10:50:39 AM8/12/19
to django-...@googlegroups.com
#30702: Form leaks all objects of model
-------------------------------------+-------------------------------------
Reporter: Kevin Olbrich | Owner: nobody
Type: Bug | Status: closed
Component: Forms | Version: 2.2
Severity: Normal | Resolution: invalid

Keywords: form, forms, | Triage Stage:
queryset | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Kevin Olbrich):

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.

--
Ticket URL: <https://code.djangoproject.com/ticket/30702#comment:4>

Django

unread,
Aug 12, 2019, 11:27:20 AM8/12/19
to django-...@googlegroups.com
#30702: Form leaks all objects of model
-------------------------------------+-------------------------------------
Reporter: Kevin Olbrich | Owner: nobody
Type: Bug | Status: closed
Component: Forms | Version: 2.2
Severity: Normal | Resolution: invalid

Keywords: form, forms, | Triage Stage:
queryset | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Claude Paroz):

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

--
Ticket URL: <https://code.djangoproject.com/ticket/30702#comment:5>

Django

unread,
Aug 12, 2019, 3:53:40 PM8/12/19
to django-...@googlegroups.com
#30702: Form leaks all objects of model
-------------------------------------+-------------------------------------
Reporter: Kevin Olbrich | Owner: nobody
Type: Bug | Status: closed
Component: Forms | Version: 2.2
Severity: Normal | Resolution: invalid

Keywords: form, forms, | Triage Stage:
queryset | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Kevin Olbrich):

Sure, np.

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

Steps to reproduce:
0. Clone, pip, migrate, etc.
1. Create user (initial superadmin is sufficient)
2. Create customer #1 "Allowed Customer Example"
3. Create customer #2 "Super Secret Customer"
4. Add user + "Allowed Customer Example" relation in multitenancy table
5. Access website under /playground/forms/register
6. Enter a domain and choose customer (only "Allowed Customer Example"
will be visible)
7. Submit form
8. 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)".

--
Ticket URL: <https://code.djangoproject.com/ticket/30702#comment:6>

Django

unread,
Aug 13, 2019, 3:26:53 AM8/13/19
to django-...@googlegroups.com
#30702: Form leaks all objects of model
-------------------------------------+-------------------------------------
Reporter: Kevin Olbrich | Owner: nobody
Type: Bug | Status: closed
Component: Forms | Version: 2.2
Severity: Normal | Resolution: invalid

Keywords: form, forms, | Triage Stage:
queryset | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Carlton Gibson):

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.

--
Ticket URL: <https://code.djangoproject.com/ticket/30702#comment:7>

Django

unread,
Aug 13, 2019, 3:53:26 AM8/13/19
to django-...@googlegroups.com
#30702: Form leaks all objects of model
-------------------------------------+-------------------------------------
Reporter: Kevin Olbrich | Owner: nobody
Type: Bug | Status: closed
Component: Forms | Version: 2.2
Severity: Normal | Resolution: invalid

Keywords: form, forms, | Triage Stage:
queryset | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Kevin Olbrich):

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

--
Ticket URL: <https://code.djangoproject.com/ticket/30702#comment:8>

Reply all
Reply to author
Forward
0 new messages