Changes between Initial Version and Version 24 of Ticket #10686


Ignore:
Timestamp:
Aug 9, 2022, 9:15:11 AM (21 months ago)
Author:
julian@pinabausch.org
Comment:

Dear community,

just picked this up while figuring out how to build permission names dynamically.

Here’s a new PR: https://github.com/django/django/pull/15936

Open issues:

  • Tim Graham commented that the code may be [generating faulty permissions](https://github.com/django/django/pull/6861#issuecomment-240427426). I checked the behavior in a dummy project, but cannot confirm. I struggled to write a test for this, because it needs real migrations, real data etc. in the database. Please let me know if there’s an example in the existing tests that I can check (the advanced testing docs don’t mention “defining a real model in a testcase” either)
  • Authors and version added are still missing – I’ll add this if it has a chance to get merged.
  • (Changed to class argument to model_name – I believe this is more consistent with content types & auth)

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #10686

    • Property Triage Stage UnreviewedAccepted
    • Property SeverityNormal
    • Property TypeNew feature
    • Property UI/UX unset
    • Property Easy pickings unset
    • Property Owner nobody removed
    • Property Patch needs improvement set
    • Property Summary 2 simple improvements to permission inheritance.Add class name interpolation in Meta.permissions codenames
  • Ticket #10686 – Description

    initial v24  
    1 I've got a patch for a slight behavior modification that I needed and that might be useful for others, and I wanted to collect some thoughts on it before writing up the regression tests and documentation changes.
    2 
    3 Twice now, I've come across a situation where the default Django behavior for inheriting permissions is inappropriate for my security model.
    4 
    5 Here's the situation: I have a permission on an abstract base model class that I want all child classes to inherit, and I want to then append specific permission(s) to one or more of the children.
    6 
    7 Example:
    8 {{{
    9 #!python
    10 class MyAppBaseModel(models.Model):
    11     class Meta:
    12         abstract = True
    13         permissions = (("view_%(class)s", "Can view %(class)s"),)
    14 
    15 class ChildModel(MyAppBaseModel):
    16     class Meta:
    17         permissions =  (("foobar_childmodel", "Can foobar childmodel"),)
    18 }}}
    19 
    20 Two problems arise:
    21     1.  Although permissions currently may be inherited, the Options class does not currently implement %(class)s replacement like the RelatedField class does, so my permissions end up actually being stored in the database with %(class)s in the name and codename.
    22     2.  The way Meta attributes are currently processed in the ModelBase metaclass causes inherited permissions to be completely replaced if any explicit permissions are defined on the child class.  So instead of can_view and can_foobar on ChildModel, I only get can_foobar.
    23 
    24 
    25 This patch changes Django's behavior such that any explicit child class permissions would be appended to the inherited ones, rather than completely replacing them.
    26 
    27 Also, I've added a backwards-compatible flag to the Meta options, 'inherit_permissions'.  This flag would only be required in the case that one wanted Django's current behavior which is to discard base class permissions when explicit permissions are declared on the child class.
    28 
    29 Example:
    30 {{{
    31 #!python
    32 class MyAppBaseModel(models.Model):
    33     class Meta:
    34         abstract = True
    35         permissions = (("view_%(class)s", "Can view %(class)s"),)
    36 
    37 class ChildModel(MyAppBaseModel):
    38     class Meta:
    39         permissions =  (("foobar_childmodel", "Can foobar childmodel"),)
    40         inherit_permissions = False
    41 }}}
    42 
    43 This would result in ChildModel only having the can_foobar permission (Django's current behavior).  If you wanted to inherit/append the view_class
    44 permission instead (proposed behavior), you could set the attribute to True or leave it out entirely.
    45 
    46 This, of course, assumes that my desired behavior is what most other people would want.  I suspect, but am not certain that this is the case.
    47 
    48 Though a small change, I believe it requires a design decision.
    49 
    50 Thanks!
Back to Top