Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#30040 closed Cleanup/optimization (fixed)

Use a default permission name in docs examples to avoid confusion

Reported by: Adrian Samatan Owned by: Hasan Ramezani
Component: Documentation Version: 2.1
Severity: Normal Keywords: template permissions documentation
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Following https://docs.djangoproject.com/el/1.11/topics/auth/default/#permissions, it's suggested that permissions should be checked on templates like this: {% if perms.foo.can_vote %}

However, while running an application on production (DEBUG = False on settings.py), it seems like the proper way to do this is {% if perms.foo.vote %}, without the can_ prefix, because the perms dictionary doesn't have any item with can_ and it fails otherwise.

I think I tested this on a local environment and it worked fine with the can_ prefix, so not sure what the deal is here.

Change History (11)

comment:1 by Claude Paroz, 5 years ago

I think can_vote here is to be considered as any custom permission codename added to some model, it does not refer to a default permission name automatically created for models. The default names are add_foo, change_foo, delete_foo (and since Django 2.1, view_foo).

So the question is: should we use a standard permission codename in the examples instead of an arbitrary permission codename? I have no strong opinion on this.

comment:2 by Claude Paroz, 5 years ago

Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

Now I see that a confusion may happen because the standard permission verbose name is "Can view foo". So I would suggest to either use a standard add/change/delete/view permission codename, or use a custom name that doesn't have can in its name (or anything else which might suggest it is a standard permission).

comment:5 by 2w3, 5 years ago

Owner: set to 2w3
Status: newassigned

comment:6 by 2w3, 5 years ago

https://github.com/django/django/pull/10757

can_vote is not django standard permission expression.
so, fixed to can_vote to add_vote in permission detail expression.

Last edited 5 years ago by 2w3 (previous) (diff)

comment:7 by Claude Paroz, 5 years ago

I think that generally when the Django documentation talks about the polls app, it refers to the tutorial app, with models Question and Choice, where votes is a field of Choice. In that context, using add_vote is at least as much confusing as the can_vote perm name. If we want to stick with default-created permissions, we should probably use add_choice.

comment:8 by Tim Graham, 5 years ago

Has patch: set
Patch needs improvement: set
Summary: Documentation on permissions on templates is wrongUse a default permission name in docs examples to avoid confusion

comment:9 by James Pic, 5 years ago

I kind of agree with everybody here. But from a higher perspective, it seems to me that the stake is rather to teach the user how to add a backend that supports can vote, ie.:

"Now, create a polls/backend.py file with the following:

class AuthenticationBackend:
    def authenticate(self, *args):
        """
        Always return ``None`` to prevent authentication within this backend.
        """
        return None

    def has_perm(self, user_obj, perm, obj=None):
        if not perm.startswith('polls.'):
            return False

        # only allow authenticated users to vote for example
        if not user_obj.is_authenticated:
            return False
       
        if 'voters' in user_obj.groups.values_list('name', flat=True)
            return True
)

Then add it to settings:

AUTHENTICATION_BACKENDS = [
    'django.contrib.auth.backends.ModelBackend',
    'polls.backends.AuthenticationBackend',
]

IMHO, teaching how to hack the permission system to their liking is what's a stake here, rather than the name of the permission. Otherwise, users might do manual permission checking in the template, instead of going through the permission abstraction layer, and their permission refactoring will be a lot harder - only tests can save them.

Sorry my text isn't good/worked out so that you can just reuse it, but I hope that should give you some ideas.

comment:10 by Hasan Ramezani, 4 years ago

Owner: changed from 2w3 to Hasan Ramezani
Patch needs improvement: unset
Last edited 4 years ago by Mariusz Felisiak (previous) (diff)

comment:11 by Carlton Gibson, 4 years ago

Triage Stage: AcceptedReady for checkin

comment:12 by Carlton Gibson <carlton@…>, 4 years ago

Resolution: fixed
Status: assignedclosed

In b7795d76:

Fixed #30040 -- Used default permission name in docs examples to avoid confusion.

comment:13 by Carlton Gibson <carlton.gibson@…>, 4 years ago

In a73489f1:

[3.0.x] Fixed #30040 -- Used default permission name in docs examples to avoid confusion.

Backport of b7795d7673f51daf288ac80616ef69b05918ca6b from master

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