Skip to content
This repository has been archived by the owner on Apr 16, 2024. It is now read-only.

User permission checking #36

Closed
kmmbvnr opened this issue May 13, 2014 · 4 comments
Closed

User permission checking #36

kmmbvnr opened this issue May 13, 2014 · 4 comments

Comments

@kmmbvnr
Copy link
Collaborator

kmmbvnr commented May 13, 2014

There was several tries to add parameters passing to trahsition conditions. And every time pull requesters clain the only one use case for that - user permission checking.

Doing that in conditions is basically wrong. And it makes get_all_available_transition method ugly. B/c you can't mix conditions with user param and without, or have to use **kwargs everywhere.

At the end if user have no permission TransitionNotAllowed would be raisen that makes no sence.

The right solution should allow to make model transition without user (in case of background celery task or admin console when user are not available) And checking user permission should raise PermissionDenied exception.

So, i think right solution should be follows:

  1. Add permission arg to transition method that accepts django permission string name or callable that accepts user
  2. Provide additional method for perm check on transition, Ex
      @transition(field=status, source='NEW', target='PENDING',
                  permission='myapp.can_start_entry')
     def start(self):
          pass

Usage

     def start(request, entry_pk):
         entry = get_object_or_404(Entry, pk=entry_pk)
         if not enty.start.has_perm(request.user): 
               raise PermissionDenied
         if request.POST:
               entry.start()
               entry.save()
               return redirect('/')
         return render(request, {'entry': entry})
This was referenced May 13, 2014
@mbertheau
Copy link

This is an interesting approach, however for our use case it falls short. We have an entity, and some users can publish it directly (have can_publish), others have to go through a review process (don't have can_publish). So we have the states NEW, REVIEW and PUBLISHED. Now I'd need to forbid the NEW -> REVIEW transition for users that can publish directly, so I'd need something like permission='!can_publish'.

In another place (we don't use django-fsm there) we have to check for a combination of permissions like can_publish AND !is_trainer.

@kmmbvnr
Copy link
Collaborator Author

kmmbvnr commented May 13, 2014

In this case permission arg would be accept callable where yo can do any check

@mbertheau
Copy link

Ah, okay, that'd work.

Would get_all_available_transitions take an optional user argument?

@kmmbvnr
Copy link
Collaborator Author

kmmbvnr commented May 15, 2014

I think better to have separate gel_user_available_transitions method

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

No branches or pull requests

2 participants