#10609: Permissions on admin actions
-------------------------------+------------------------------------
     Reporter:  leitjohn       |                    Owner:  leitjohn
         Type:  New feature    |                   Status:  assigned
    Component:  contrib.admin  |                  Version:  master
     Severity:  Normal         |               Resolution:
     Keywords:                 |             Triage Stage:  Accepted
    Has patch:  1              |      Needs documentation:  0
  Needs tests:  1              |  Patch needs improvement:  1
Easy pickings:  0              |                    UI/UX:  0
-------------------------------+------------------------------------
Changes (by ptone):

 * needs_better_patch:  0 => 1
 * stage:  Design decision needed => Accepted


Comment:

 Thank you for your work on this feature.

 A couple administrative related bits:



 First it would be a stretch to land this in 1.5 - there are a couple
 issues in the current patch - and the feature freeze has already
 theoretically passed.



 Second - design decision needed should be used sparingly and only when a
 tough architectural design decision is required



 https://docs.djangoproject.com/en/dev/internals/contributing/triaging-
 tickets/#design-decision-needed


 In this case, having non-matching function signatures doesn't mean the
 concept is not accepted.



 OK - now on to the patch



 There should be no carryover of the redirect to login view or raising 403
 - those are view related and don't apply in this context- we know we are
 already logged into the admin when we are gathering actions.



 Likewise, what we are testing, ends up determining what actions are
 presented to the user, not whether they will run.



 Including the test function a second time inside the wrapped function
 doesn't make sense.  If you can't see it, you can't run it - this is
 currently insured in the response_action method, which would raise a
 KeyError at this line:



 {{{func, name, description = self.get_actions(request)[action]}}}





 with regard to the naming - I can't imagine a place where you would use
 these in the same file as the view decorators - so they can share the same
 name, the decorator should be descriptive and should make sense in context
 - having admin in the name is redundant when used in code that is clearly
 enough admin related.



 Should this feature really be implemented as decorators at all?  We really
 don't intend to change what the action does - only assign a test_func
 attribute to it.



 Finally (ok, this could be a design decision) we should at least explore
 whether it would be feasible to do this at a per-object permission level.



 Per object permission are a standard part of the permission system - the
 contract is that a permission check should get the object they are making
 the decision about - but don't have to pay attention to it.



 If I've added a permission check that does check objects - I would want
 that to be utilized here.



 So here is a rough outline of how that might work. We specify something,
 perhaps a method on the modeladmin that handles the permssion check like:

 
https://docs.djangoproject.com/en/dev/ref/contrib/admin/#django.contrib.admin.ModelAdmin.has_delete_permission



 This is used with obj=None when gathering the list of actions to display,
 and is then used when an action is invoked to create an intermediate page
 displaying what objects you do or don't have permission for - maybe with
 some option to show only if you don't have permission to act on some
 objects.  The full QS is then filtered down to those you do have
 permission for - and passed to the original, simple, action function.



 Like I said that is rough - but should be explored for this feature, even
 if it is a bit harder to implement - I don't think we can just skip on
 trying to do this while respecting obj level permissions.



 I'm marking back to accepted, as the feature is accepted - it is mostly a
 question of implementation.

-- 
Ticket URL: <https://code.djangoproject.com/ticket/10609#comment:16>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to