#13331 closed (invalid)
Strange behavior of lists of objects with overriden changelist_view in admin after re-login.
| Reported by: | minder | Owned by: | nobody |
|---|---|---|---|
| Component: | contrib.admin | Version: | 1.1 |
| Severity: | Keywords: | ||
| Cc: | Triage Stage: | Unreviewed | |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
I spotted a strange behavior in the admin:
I've made an Admin for my model. It looks like this:
class EntryAdmin(admin.ModelAdmin):
(...)
def changelist_view(self, request, extra_context=None):
if request.user.has_perm('blog.can_approve_entry'):
self.list_editable = ['is_approved']
return super(EntryAdmin, self).changelist_view(request, extra_context)
def get_form(self, request, obj=None, **kwargs):
if not request.user.has_perm('blog.can_approve_entry'):
self.exclude = ['is_approved']
return super(EntryAdmin, self).get_form(request, obj, **kwargs)
I've created two users: one with can_approve_entry permission (Alice) and the other without it (Bob). Then I created some Entries logged in as Bob. Entry list had no list_editable items, just nice pictures showing that his posts are not yet approved. Then I logged in as Alice and the list containted nice tickboxes and a Save button on bottom. So far, so good. Then I logged Alice out and logged back in as Bob. The list contained tickboxes instead of pictures. Now Bob could change the status of all Entries. I think showing these tickboxes is a security hole. Or maybe I'm doing something wrong? This shows up both on test server and Apache (mod_wsgi).
Change History (5)
comment:1 by , 16 years ago
| Resolution: | → invalid |
|---|---|
| Status: | new → closed |
comment:2 by , 16 years ago
| Resolution: | invalid |
|---|---|
| Status: | closed → reopened |
After setting 'list_editable' and 'exclude' to empty lists, this still shows up after editing an object. I have no idea how to fix this.
Complete ModelAdmin:
class EntryAdmin(admin.ModelAdmin):
prepopulated_fields={'url': ('title',)}
save_on_top = True
date_hierarchy = 'publish_date'
search_fields = ['title']
list_display = ['title', 'unit', 'is_approved']
list_filter = ['unit',]
list_editable = []
exclude = []
def formfield_for_foreignkey(self, db_field, request, **kwargs):
if db_field.name == 'unit':
if request.user.is_superuser:
kwargs['queryset'] = Unit.objects.all()
else:
kwargs['queryset'] = Unit.objects.filter(editors__in=request.user.groups.all())
return db_field.formfield(**kwargs)
return super(EntryAdmin, self).formfield_for_foreignkey(db_field, request, **kwargs)
def changelist_view(self, request, extra_context=None):
if request.user.has_perm('blog.can_approve_entry'):
self.list_editable = ['is_approved']
return super(EntryAdmin, self).changelist_view(request, extra_context)
def get_form(self, request, obj=None, **kwargs):
if not request.user.has_perm('blog.can_approve_entry'):
self.exclude = ['is_approved']
return super(EntryAdmin, self).get_form(request, obj, **kwargs)
def queryset(self, request):
qs = super(EntryAdmin, self).queryset(request)
if not request.user.is_superuser:
units = Unit.objects.filter(editors__in=request.user.groups.all())
qs = qs.filter(unit__in=units)
return qs
def save_model(self, request, obj, form, change):
if not change:
obj.author = request.user
if not request.user.has_perm('blog.can_approve_entry'):
obj.is_approved = False
if not bool(obj.approved_by) and obj.is_approved:
obj.approved_by = request.user
obj.save()
comment:3 by , 16 years ago
Just don't do this. It's complete un-thread safe and therefore a terrible idea. We should add a get_list_editable function which takes request and such.
follow-up: 5 comment:4 by , 16 years ago
| Resolution: | → invalid |
|---|---|
| Status: | reopened → closed |
I'm gonna third the "please don't do this." The bug you're seeing is the inevitable consequence of doing something that's not thread-safe. It's not meant to be done.
Re-closing as invalid as per kmtracey above.
comment:5 by , 16 years ago
Replying to gabrielhurley:
I'm gonna third the "please don't do this." The bug you're seeing is the inevitable consequence of doing something that's not thread-safe. It's not meant to be done.
Re-closing as invalid as per kmtracey above.
Using function methods as middlewares in a subclass such as ModelAdmin has nothing to do with thread-safety, sorry. Its fully supported by Django to use
classes methods as generic views. So this sounds more like yet another Django weird admin-policy-design-issue thing, rather than a limitation in the framework
itself.
Good luck! ;)
You are doing something wrong. Note a single ModelAdmin instance may get used for handling multiple requests, from different users. So if you set
self.list_editableto['is_approved']when you get a request from a user authorized to change that, you had best take care to set it to an empty list when you get called to handle a request for a user that is not authorized to change that, otherwise your previous setting for an authorized user may still be in effect.