#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 , 15 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
comment:2 by , 15 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 , 15 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 , 15 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 , 15 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_editable
to['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.