Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#5447 closed (fixed)

has_{add,change,delete}_permission not working correctly

Reported by: anonymous Owned by: anonymous
Component: contrib.admin Version: newforms-admin
Severity: Keywords:
Cc: django@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Using "def has_add_permission(*args):return False" in the model options only raises a permission denied when trying to add an object but it doesn't hide the "add object" button in change_list.html and index.html (and they seem to use different ways to check if the object has the add permission: change_list.html uses "{% if has_add_permission %}" while index.html uses {% if model.perms.add %})
This also applies to "has_change_permission" and "has_delete_permission".
Moreover I think that when has_change_permission returns False, it should't allow to modify (save modified) objects, while currently it doesn't allow to see the list of objects.

Attachments (2)

add_delete_change_permission.diff (7.3 KB) - added by anonymous 8 years ago.
fix-admin-permissions.diff (5.3 KB) - added by jkocherhans 8 years ago.
Updated to work with [6784]

Download all attachments as: .zip

Change History (23)

comment:1 Changed 8 years ago by apollo13

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to apollo13
  • Patch needs improvement unset
  • Status changed from new to assigned
  • Triage Stage changed from Unreviewed to Accepted

I'll look over it as soon as I am at home (in 2hours, if someone can fix this before, just reassign it)

comment:2 Changed 8 years ago by apollo13

  • Triage Stage changed from Accepted to Unreviewed

comment:3 follow-up: Changed 8 years ago by apollo13

  • Resolution set to worksforme
  • Status changed from assigned to closed
  • Triage Stage changed from Unreviewed to Accepted

I got time to test it now, it works for me (revision 6183 and 6036). So I'll close this as worksforme, please reopen it, if you still have problems with a head checkout.

comment:4 Changed 8 years ago by apollo13

  • Cc django@… added

comment:5 in reply to: ↑ 3 Changed 8 years ago by anonymous

Replying to apollo13:

I got time to test it now, it works for me (revision 6183 and 6036). So I'll close this as worksforme, please reopen it, if you still have problems with a head checkout.

Could you please explain briefly how you use this option? I've added it into the ModelAdmin subclass registered with the model but it doesn't work for me.
Thanks

comment:6 Changed 8 years ago by apollo13

  • Resolution worksforme deleted
  • Status changed from closed to reopened

Ok, I must have missed something, but I am going home now, I will check it there :)

comment:7 follow-up: Changed 8 years ago by apollo13

  • Triage Stage changed from Accepted to Design decision needed

I am going to change this to design decission needed, as I think you shouldn't be able to see objects you don't have a change/delete permission for. (Well maybe, now with newfoms-admin we can limit the available list to your own etc... objetcs, so maybe it would be an idea to just disable the save/delete buttons as you suggested [even more save_as_new makes only sense if you have change permission, otherwise you can't view the page, as you said])

The add link works fine for me (no special options...)

comment:8 in reply to: ↑ 7 ; follow-up: Changed 8 years ago by anonymous

Replying to apollo13:

The add link works fine for me (no special options...)

here i've made a little testcase (i'm using r6184)
here the output of $find . -type f -print -exec cat {} \;

./settings.py
# Django settings for testprj project.

DEBUG = True
TEMPLATE_DEBUG = DEBUG

ADMINS = (
    # ('Your Name', 'your_email@domain.com'),
)

MANAGERS = ADMINS

DATABASE_ENGINE = 'sqlite3'           # 'postgresql_psycopg2', 'postgresql', 'mysql', 'sqlite3' or 'oracle'.
DATABASE_NAME = '/tmp/test.db'             # Or path to database file if using sqlite3.
DATABASE_USER = ''             # Not used with sqlite3.
DATABASE_PASSWORD = ''         # Not used with sqlite3.
DATABASE_HOST = ''             # Set to empty string for localhost. Not used with sqlite3.
DATABASE_PORT = ''             # Set to empty string for default. Not used with sqlite3.

# Local time zone for this installation. Choices can be found here:
# http://en.wikipedia.org/wiki/List_of_tz_zones_by_name
# although not all choices may be avilable on all operating systems.
# If running in a Windows environment this must be set to the same as your
# system time zone.
TIME_ZONE = 'America/Chicago'

# Language code for this installation. All choices can be found here:
# http://www.w3.org/TR/REC-html40/struct/dirlang.html#langcodes
LANGUAGE_CODE = 'en-us'

SITE_ID = 1

# If you set this to False, Django will make some optimizations so as not
# to load the internationalization machinery.
USE_I18N = True

# Absolute path to the directory that holds media.
# Example: "/home/media/media.lawrence.com/"
MEDIA_ROOT = ''

# URL that handles the media served from MEDIA_ROOT. Make sure to use a
# trailing slash if there is a path component (optional in other cases).
# Examples: "http://media.lawrence.com", "http://example.com/media/"
MEDIA_URL = ''

# URL prefix for admin media -- CSS, JavaScript and images. Make sure to use a
# trailing slash.
# Examples: "http://foo.com/media/", "/media/".
ADMIN_MEDIA_PREFIX = '/media/'

# Make this unique, and don't share it with anybody.
SECRET_KEY = 'o(!r546$hlf97$5no@ie+1@@#0ufwf!9e@l!j*ewqr)49vmb$3'

# List of callables that know how to import templates from various sources.
TEMPLATE_LOADERS = (
    'django.template.loaders.filesystem.load_template_source',
    'django.template.loaders.app_directories.load_template_source',
#     'django.template.loaders.eggs.load_template_source',
)

MIDDLEWARE_CLASSES = (
    'django.middleware.common.CommonMiddleware',
    'django.contrib.sessions.middleware.SessionMiddleware',
    'django.contrib.auth.middleware.AuthenticationMiddleware',
    'django.middleware.doc.XViewMiddleware',
)

ROOT_URLCONF = 'testprj.urls'

TEMPLATE_DIRS = (
    # Put strings here, like "/home/html/django_templates" or "C:/www/django/templates".
    # Always use forward slashes, even on Windows.
    # Don't forget to use absolute paths, not relative paths.
)

INSTALLED_APPS = (
    'django.contrib.auth',
    'django.contrib.contenttypes',
    'django.contrib.sessions',
    'django.contrib.sites',
    'django.contrib.admin',
    'testprj.testapp'
)
./manage.py
#!/usr/bin/env python
from django.core.management import execute_manager
try:
    import settings # Assumed to be in the same directory.
except ImportError:
    import sys
    sys.stderr.write("Error: Can't find the file 'settings.py' in the directory containing %r. It appears you've customized things.\nYou'll have to run django-admin.py, passing it your settings module.\n(If the file settings.py does indeed exist, it's causing an ImportError somehow.)\n" % __file__)
    sys.exit(1)

if __name__ == "__main__":
    execute_manager(settings)
./urls.py
from django.conf.urls.defaults import *

# Uncomment this for admin:
from django.contrib import admin

urlpatterns = patterns('',
    # Example:
    # (r'^testprj/', include('testprj.foo.urls')),

    # Uncomment this for admin docs:
    #(r'^admin/doc/', include('django.contrib.admindocs.urls')),

    # Uncomment this for admin:
    ('^admin/(.*)', admin.site.root),
)
./__init__.py
./testapp/__init__.py
./testapp/models.py
from django.db import models
from django.contrib import admin

# Create your models here.
class TestModel(models.Model):
    notes=models.TextField()

class TestModelOptions(admin.ModelAdmin):
    def has_add_permission(self,req):
        return False

admin.site.register(TestModel,TestModelOptions)
./testapp/views.py
# Create your views here.

comment:9 in reply to: ↑ 8 Changed 8 years ago by anonymous

Replying to anonymous:

Replying to apollo13:

The add link works fine for me (no special options...)

here i've made a little testcase (i'm using r6184)
here the output of $find . -type f -print -exec cat {} \;

Forgot to say that it is a testcase demonstrating "has_add_permission" not working for me.

comment:10 Changed 8 years ago by anonymous

Now I see your point, I'll start working on it now!

comment:11 Changed 8 years ago by anonymous

  • Triage Stage changed from Design decision needed to Accepted

comment:12 follow-up: Changed 8 years ago by apollo13

  • Owner changed from apollo13 to noone
  • Status changed from reopened to new

Hmm I can't find a way round this for now, if someone else thinks so, feel free to assign :(

comment:13 Changed 8 years ago by anonymous

  • Owner changed from noone to anonymous
  • Status changed from new to assigned

comment:14 in reply to: ↑ 12 ; follow-up: Changed 8 years ago by anonymous

Replying to apollo13:

Hmm I can't find a way round this for now, if someone else thinks so, feel free to assign :(

Did you manage to reproduce this?

comment:15 in reply to: ↑ 14 Changed 8 years ago by anonymous

Replying to anonymous:

Replying to apollo13:

Hmm I can't find a way round this for now, if someone else thinks so, feel free to assign :(

Did you manage to reproduce this?

Yeah

Changed 8 years ago by anonymous

comment:16 follow-up: Changed 8 years ago by anonymous

I've produced the patch with svn diff but it doesn't get displayed. Ideas?

comment:17 in reply to: ↑ 16 Changed 8 years ago by mtredinnick

Replying to anonymous:

I've produced the patch with svn diff but it doesn't get displayed. Ideas?

Don't worry about it. The file can still be downloaded in the original format, so it can still be read and reviewed,

comment:18 Changed 8 years ago by jkocherhans

I've added a new patch based on the previous one, fix-admin-permissions.diff, that should do the trick for everything except the add button on ForeignKey and ManyToManyField. I haven't figured out a clean way to do that yet. The previous patch doesn't really fix that either, it always displays the link. I've tested this a little with a project I'm working on, but I'd appreciate it if anyone else interested in the problem could test it out too. I can't think of anything else to look for, but that doesn't mean there isn't anything.

comment:19 Changed 8 years ago by Petr Marhoun <petr.marhoun@…>

I use this patch but I have two notes:

  • Button save_as_new can be showed in change stage so add_permission should be checked.
  • Context variable show_delete is not used anywhere so it can be removed.

Also my comment comment:ticket:5520:1 could be relevant.

Changed 8 years ago by jkocherhans

Updated to work with [6784]

comment:20 Changed 8 years ago by jkocherhans

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [6797]) newforms-admin: Fixed #5447. has_X_permission now works correctly.

comment:21 Changed 8 years ago by jkocherhans

The are probably a few remaining issues related to this ticket. Please open new tickets for those issues.

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