Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

[1.1.X] Fixed #15103 - SuspiciousOperation with limit_choices_to and …

…raw_id_fields

Thanks to natrius for the report.

This patch also fixes some unicode bugs in affected code.

Backport of [15347] from trunk. Backported to 1.1.X because this was
a regression caused by a security fix backported to 1.1.X.

git-svn-id: http://code.djangoproject.com/svn/django/branches/releases/1.1.X@15350 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit 274bd67c13f4e979b3982d275470f80cc53509d0 1 parent 703dc82
Luke Plant authored January 28, 2011
11  django/contrib/admin/options.py
@@ -173,7 +173,16 @@ def _declared_fieldsets(self):
173 173
         return None
174 174
     declared_fieldsets = property(_declared_fieldsets)
175 175
 
176  
-    def lookup_allowed(self, lookup):
  176
+    def lookup_allowed(self, lookup, value):
  177
+        model = self.model
  178
+        # Check FKey lookups that are allowed, so that popups produced by
  179
+        # ForeignKeyRawIdWidget, on the basis of ForeignKey.limit_choices_to,
  180
+        # are allowed to work.
  181
+        for l in model._meta.related_fkey_lookups:
  182
+            for k, v in widgets.url_params_from_lookup_dict(l).items():
  183
+                if k == lookup and v == value:
  184
+                    return True
  185
+
177 186
         parts = lookup.split(LOOKUP_SEP)
178 187
 
179 188
         # Last term in lookup is a query term (__exact, __startswith etc)
10  django/contrib/admin/views/main.py
@@ -184,16 +184,18 @@ def get_query_set(self):
184 184
 
185 185
             # if key ends with __in, split parameter into separate values
186 186
             if key.endswith('__in'):
187  
-                lookup_params[key] = value.split(',')
  187
+                value = value.split(',')
  188
+                lookup_params[key] = value
188 189
 
189 190
             # if key ends with __isnull, special case '' and false
190 191
             if key.endswith('__isnull'):
191 192
                 if value.lower() in ('', 'false'):
192  
-                    lookup_params[key] = False
  193
+                    value = False
193 194
                 else:
194  
-                    lookup_params[key] = True
  195
+                    value = True
  196
+                lookup_params[key] = value
195 197
 
196  
-            if not self.model_admin.lookup_allowed(key):
  198
+            if not self.model_admin.lookup_allowed(key, value):
197 199
                 raise SuspiciousOperation(
198 200
                     "Filtering by %s not allowed" % key
199 201
                 )
37  django/contrib/admin/widgets.py
@@ -97,6 +97,23 @@ def render(self, name, value, attrs=None):
97 97
         output.append(super(AdminFileWidget, self).render(name, value, attrs))
98 98
         return mark_safe(u''.join(output))
99 99
 
  100
+def url_params_from_lookup_dict(lookups):
  101
+    """
  102
+    Converts the type of lookups specified in a ForeignKey limit_choices_to
  103
+    attribute to a dictionary of query parameters
  104
+    """
  105
+    params = {}
  106
+    if lookups and hasattr(lookups, 'items'):
  107
+        items = []
  108
+        for k, v in lookups.items():
  109
+            if isinstance(v, list):
  110
+                v = u','.join([str(x) for x in v])
  111
+            else:
  112
+                v = unicode(v)
  113
+            items.append((k, v))
  114
+        params.update(dict(items))
  115
+    return params
  116
+
100 117
 class ForeignKeyRawIdWidget(forms.TextInput):
101 118
     """
102 119
     A Widget for displaying ForeignKeys in the "raw_id" interface rather than
@@ -112,33 +129,23 @@ def render(self, name, value, attrs=None):
112 129
         related_url = '../../../%s/%s/' % (self.rel.to._meta.app_label, self.rel.to._meta.object_name.lower())
113 130
         params = self.url_parameters()
114 131
         if params:
115  
-            url = '?' + '&'.join(['%s=%s' % (k, v) for k, v in params.items()])
  132
+            url = u'?' + u'&'.join([u'%s=%s' % (k, v) for k, v in params.items()])
116 133
         else:
117  
-            url = ''
  134
+            url = u''
118 135
         if not attrs.has_key('class'):
119 136
             attrs['class'] = 'vForeignKeyRawIdAdminField' # The JavaScript looks for this hook.
120 137
         output = [super(ForeignKeyRawIdWidget, self).render(name, value, attrs)]
121 138
         # TODO: "id_" is hard-coded here. This should instead use the correct
122 139
         # API to determine the ID dynamically.
123  
-        output.append('<a href="%s%s" class="related-lookup" id="lookup_id_%s" onclick="return showRelatedObjectLookupPopup(this);"> ' % \
  140
+        output.append(u'<a href="%s%s" class="related-lookup" id="lookup_id_%s" onclick="return showRelatedObjectLookupPopup(this);"> ' % \
124 141
             (related_url, url, name))
125  
-        output.append('<img src="%simg/admin/selector-search.gif" width="16" height="16" alt="%s" /></a>' % (settings.ADMIN_MEDIA_PREFIX, _('Lookup')))
  142
+        output.append(u'<img src="%simg/admin/selector-search.gif" width="16" height="16" alt="%s" /></a>' % (settings.ADMIN_MEDIA_PREFIX, _('Lookup')))
126 143
         if value:
127 144
             output.append(self.label_for_value(value))
128 145
         return mark_safe(u''.join(output))
129 146
 
130 147
     def base_url_parameters(self):
131  
-        params = {}
132  
-        if self.rel.limit_choices_to and hasattr(self.rel.limit_choices_to, 'items'):
133  
-            items = []
134  
-            for k, v in self.rel.limit_choices_to.items():
135  
-                if isinstance(v, list):
136  
-                    v = ','.join([str(x) for x in v])
137  
-                else:
138  
-                    v = str(v)
139  
-                items.append((k, v))
140  
-            params.update(dict(items))
141  
-        return params
  148
+        return url_params_from_lookup_dict(self.rel.limit_choices_to)
142 149
 
143 150
     def url_parameters(self):
144 151
         from django.contrib.admin.views.main import TO_FIELD_VAR
2  django/db/models/fields/related.py
@@ -745,6 +745,8 @@ def contribute_to_class(self, cls, name):
4  django/db/models/options.py
@@ -53,6 +53,10 @@ def __init__(self, meta, app_label=None):
53 53
         self.abstract_managers = []
54 54
         self.concrete_managers = []
55 55
 
  56
+        # List of all lookups defined in ForeignKey 'limit_choices_to' options
  57
+        # from *other* models. Needed for some admin checks. Internal use only.
  58
+        self.related_fkey_lookups = []
  59
+
56 60
     def contribute_to_class(self, cls, name):
57 61
         from django.db import connection
58 62
         from django.db.backends.util import truncate_name
23  tests/regressiontests/admin_views/models.py
@@ -146,6 +146,26 @@ def __unicode__(self):
146 146
 class ThingAdmin(admin.ModelAdmin):
147 147
     list_filter = ('color',)
148 148
 
  149
+class Actor(models.Model):
  150
+    name = models.CharField(max_length=50)
  151
+    age = models.IntegerField()
  152
+    def __unicode__(self):
  153
+        return self.name
  154
+
  155
+class Inquisition(models.Model):
  156
+    expected = models.BooleanField()
  157
+    leader = models.ForeignKey(Actor)
  158
+    def __unicode__(self):
  159
+        return self.expected
  160
+
  161
+class Sketch(models.Model):
  162
+    title = models.CharField(max_length=100)
  163
+    inquisition = models.ForeignKey(Inquisition, limit_choices_to={'leader__name': 'Palin',
  164
+                                                                   'leader__age': 27,
  165
+                                                                   })
  166
+    def __unicode__(self):
  167
+        return self.title
  168
+
149 169
 class Fabric(models.Model):
150 170
     NG_CHOICES = (
151 171
         ('Textured', (
@@ -519,6 +539,9 @@ class AlbumAdmin(admin.ModelAdmin):
519 539
 admin.site.register(ModelWithStringPrimaryKey)
520 540
 admin.site.register(Color)
521 541
 admin.site.register(Thing, ThingAdmin)
  542
+admin.site.register(Actor)
  543
+admin.site.register(Inquisition)
  544
+admin.site.register(Sketch)
522 545
 admin.site.register(Person, PersonAdmin)
523 546
 admin.site.register(Persona, PersonaAdmin)
524 547
 admin.site.register(Subscriber, SubscriberAdmin)
11  tests/regressiontests/admin_views/tests.py
@@ -300,6 +300,17 @@ def test_disallowed_filtering(self):
300 300
         except SuspiciousOperation:
301 301
             self.fail("Filters should be allowed if they involve a local field without the need to whitelist them in list_filter or date_hierarchy.")
302 302
 
  303
+    def test_allowed_filtering_15103(self):
  304
+        """
  305
+        Regressions test for ticket 15103 - filtering on fields defined in a
  306
+        ForeignKey 'limit_choices_to' should be allowed, otherwise raw_id_fields
  307
+        can break.
  308
+        """
  309
+        try:
  310
+            self.client.get("/test_admin/admin/admin_views/inquisition/?leader__name=Palin&leader__age=27")
  311
+        except SuspiciousOperation:
  312
+            self.fail("Filters should be allowed if they are defined on a ForeignKey pointing to this model")
  313
+
303 314
 class SaveAsTests(TestCase):
304 315
     fixtures = ['admin-views-users.xml','admin-views-person.xml']
305 316
 

0 notes on commit 274bd67

Please sign in to comment.
Something went wrong with that request. Please try again.