Skip to content
This repository has been archived by the owner on Jul 21, 2021. It is now read-only.

Question - Permissions do not seem to be taking effect #81

Closed
djbettega opened this issue Apr 5, 2018 · 11 comments
Closed

Question - Permissions do not seem to be taking effect #81

djbettega opened this issue Apr 5, 2018 · 11 comments
Labels

Comments

@djbettega
Copy link

Hello,

Have posted the same question on stackoverflow in case that is a more appropriate place to post it.

Have prepared a simple test app (almost identical to the example used in the docs) to try to figure out how it works. I have read the documentation and tried to use the example app provided on this link.

The issue is when the author of an article is not able to edit/ delete the article.

The user in question has been granted all permissions in the admin section.

Key code listed below - any help much appreciated

test_app/models.py

class Article(models.Model):
    created_by = models.ForeignKey(User)
    created = models.DateField(auto_now_add=True)
    modified = models.DateField(auto_now=True)
    title = models.CharField(max_length=100)
    content = models.TextField()

    class Meta:
        app_label = 'test_app'

from permission import add_permission_logic
from permission.logics import AuthorPermissionLogic

add_permission_logic(Article, AuthorPermissionLogic(
    field_name='created_by',
    any_permission = False,
    change_permission = True,
    delete_permission = True,
))

test_app/views.py

@permission_required('change_article')
def change_article(request, *args, **kwargs):
    pk = kwargs.pop('pk')
    template = 'test_app/edit.html'
    article = models.Article.objects.get(id=pk)

    if request.method == 'POST':
        form = forms.Article_form(request.POST, instance=article)

        if form.is_valid():
            article = form.save(commit=False)

            article.created_by = request.user
            article.title = form.cleaned_data['title']
            article.content = form.cleaned_data['content']

            article.save()

            return HttpResponseRedirect('/test/')

        else:

            raise Http404

    else:
        form = forms.Article_form(instance=article)

        return render(request, template_name=template, context={'form':form})

test_app/perms.py

PERMISSION_LOGICS = (
    ('test_app.Article', AuthorPermissionLogic()),
)
@lambdalisue
Copy link
Contributor

lambdalisue commented Apr 5, 2018

Remove test_app/perms.py may solve your issue because

  1. You have already add a permission logic in models.py
  2. You have not properly configured AuthorPermissionLogic in perms.py (e.g. field_name)

@lambdalisue
Copy link
Contributor

Additionally, you probably need to use test_app.change_article instead of change_article

@lambdalisue
Copy link
Contributor

I read StackOverflow as well and commented https://stackoverflow.com/a/49677458/1273406

@djbettega
Copy link
Author

djbettega commented Apr 6, 2018

Hi - thanks for the help!

Just to clarify what I meant by the permissions in the admin section - I meant to say that I granted permissions in the built in Django Administration site to the users that I am testing with.

User 1 - has been granted all the standard django permissions in the Admin site (add/change/delete)
User 2 - has not been granted any permissions in the Admin site

I do not think this is related as User 2 is still able to edit User 1's articles (despite not having any standard Django permissions)

Have tried to fix the errors you pointed out - but am not having much luck. The changes I have made are:

  1. Deleted the perms.py file
  2. Updated the models.py file (renaming the owner field from "created_by" to "author")
class Article(models.Model):
    author = models.ForeignKey(User)
    created = models.DateField(auto_now_add=True)
    modified = models.DateField(auto_now=True)
    title = models.CharField(max_length=100)
    content = models.TextField()

    class Meta:
        app_label = 'test_app'

from permission import add_permission_logic
from permission.logics import AuthorPermissionLogic

add_permission_logic(Article, AuthorPermissionLogic(
    any_permission = False,
    change_permission = True,
    delete_permission = True,
))
  1. Corrected the views decorator in views.py

@permission_required('test_app.change_article')

@lambdalisue
Copy link
Contributor

lambdalisue commented Apr 6, 2018

Did you mean "User2 cannot change an article through admin site but User2 can change an article through your change_article view" right?

I'm not sure but your Article does not have object_id (pk) so permission_required decorator may be failed to detect a correct object. What's happen if you check the permission manually in your view like

from django.core.exceptions import PermissionDenied

def change_article(request, *args, **kwargs):
    pk = kwargs.pop('pk')
    template = 'test_app/edit.html'
    article = models.Article.objects.get(id=pk)

    # Check permission
    if not request.user.has_perm('test_app.change_article', article):
        raise PermissionDenied

    if request.method == 'POST':
        form = forms.Article_form(request.POST, instance=article)

        if form.is_valid():
            article = form.save(commit=False)

            article.created_by = request.user
            article.title = form.cleaned_data['title']
            article.content = form.cleaned_data['content']

            article.save()

            return HttpResponseRedirect('/test/')

        else:

            raise Http404

    else:
        form = forms.Article_form(instance=article)

        return render(request, template_name=template, context={'form':form})

Anyway, you should use class-based generic view instead.

@djbettega
Copy link
Author

djbettega commented Apr 6, 2018

Did you mean "User2 cannot change an article through admin site but User2 can change an article through your change_article view" right?

I meant that User 2 has not been granted any permissions via the admin site and was able to edit via the change_article view. It was just to say that the Django permissions did not seem to have any effect on the behavior.

The extra code that you sent through works exactly as I expect it to work! Great

Can you let me know what you mean by

Article does not have object_id (pk)

Is this something I can fix in the model definition so that the decorator works as expected?

Yes - I am getting the feeling more and more that I have to work out how CBVs work. All the apps I want to use/ examples I find have CBVs. It is just that I a bit of a NOOB still :)

@lambdalisue
Copy link
Contributor

lambdalisue commented Apr 6, 2018

It was just to say that the Django permissions did not seem to have any effect on the behavior.

What is your admin.py or related codes for admin sites? As long as I know, Django uses it's permission system so object permission should works in admin site as well.

Is this something I can fix in the model definition so that the decorator works as expected?

The functional decorator exists for a historical reason and it uses object_id in your query (**kwargs) to determine the pk of the object.
It seems you use pk instead of object_id in your URL conf so that django-permission failed to detect the correct object.
https://github.com/lambdalisue/django-permission/blob/master/src/permission/decorators/functionbase.py#L84

If you use class-based generic view, django-permission can get an object from a <class instance>.get_object() method so this kind of unwilling behavior does not happen.

@lambdalisue
Copy link
Contributor

lambdalisue commented Apr 6, 2018

Now I think I got why you failed to use the decorator.
Make sure that you are using correct permission_required. I think you are using django's one but it's not correct for an object permission.
https://github.com/lambdalisue/django-permission#class-method-or-function-decorator

If you've used a correct one, you would noticed object_id things by AttributeError.

@djbettega
Copy link
Author

I think we are talking about different things regarding the Admin. I think we can drop this discussion as it does not seem to be relevant for the issue that I was trying to figure out (I just mentioned it in case it was relevant).

For the decorator - I tried the following

Changed the variable used in the url configuration from "pk" to "object_id"

urls.py

urlpatterns = [
    url(r'^$', views.index, name='index'),
    url(r'^add/$', views.add_article, name='add'),
    url(r'^(?P<object_id>\d+)/$', views.detail_article, name='detail'),
    url(r'^change/(?P<object_id>\d+)/$', views.change_article, name='change'),
    url(r'^delete/(?P<object_id>\d+)/$', views.delete_article, name='delete'),
]

I made sure that the permission_required was the correct one

views.py
from permission.decorators import permission_required

Even with these changes - it does not seem to function as intended

@lambdalisue
Copy link
Contributor

Sorry for the delay. I was a bit busy.

Well, the permission_required decorator for the function is designed for old style generic view (django-permission exists from Django 1.2!).
So if you desire, read the old document and mimic that function. Probably you must pass queryset to your view function (I'm sorry I don't remember the detail).

Again, the functional based view has a lot of problem like this so I strongly recommend you to switch class-based generic view or check permission by your self like #81 (comment)

@djbettega
Copy link
Author

Thanks very much for that - will try and figure it out (or better learn CBV!). Will accept your answer in Stackoverflow and link back the discussion to this thread. Thanks again. D

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants