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 I created a ticket.

Twice now, I've come across a situation where the default Django
behavior for inheriting permissions is inappropriate for my security
model.

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.

Example:

class MyAppBaseModel(models.Model):
    class Meta:
        abstract = True
        permissions = (("view_%(class)s", Can view %(class)s'),)


class ChildModel(MyAppBaseModel):
    class Meta:
        permissions =  (("foobar_childmodel", "Can foobar
childmodel"),)

Two problems arise:
    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.
    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.

My solution to fix both of these issues is relatively simple,
including a total of 11 lines across two files (minus comments).

The patch against rev 10176 is as follows:

django-trunk/django/db/models/base.py
46,49d45
<             # Append any inherited permissions to any explicitly
declared ones.
<             if hasattr(meta, 'permissions') and getattr(meta,
'inherit_permissions', True):
<                 if hasattr(new_class, 'Meta') and hasattr
(new_class.Meta, 'permissions'):
<                     meta.permissions += new_class.Meta.permissions

django-trunk/django/db/models/options.py
24c24
<                  'abstract', 'managed', 'proxy',
'inherit_permissions')
---
>                  'abstract', 'managed', 'proxy')
90,101d89
<             # check for the %(class)s escape used for inherited
permissions.
<             # If present, replace it with the appropriate text based
off of
<             # the class name for both the name and codename of the
permission.
<             perms = meta_attrs.pop('permissions', getattr(self,
'permissions'))
<             translated_perms = ()
<             if perms:
<                 for codename, name in perms:
<                     codename = codename % {'class':
cls.__name__.lower()}
<                     name = name % {'class': getattr(self,
'verbose_name')}
<                     translated_perms += ((codename, name),)
<             setattr(self, 'permissions', translated_perms)

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.

As you can see, in this solution, I've added a backwards-compatible
flag to the Meta options, 'inherit_permissions'.  This flag would only
be required in the case that you wanted Django's current behavior
which is to discard base class permissions when explicit permissions
are declared on the child class.

Ex:

class MyAppBaseModel(models.Model):
    class Meta:
        abstract = True
        permissions = (("view_%(class)s", Can view %(class)s'),)


class ChildModel(MyAppBaseModel):
    class Meta:
        permissions =  (("foobar_childmodel", "Can foobar
childmodel"),)
        inherit_permissions = False

This would result in ChildModel only having the can_foobar permission
(current behavior).  If you wanted to inherit/append the view_class
permission instead (new behavior), you could set the attribute to True
or leave it out entirely.

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.

Though a small change, this is definitely one that requires a design
decision.  I would greatly appreciate it if someone with authority
would let me know whether I should open a ticket for this patch or
not, and/or make any improvements first.

Thanks!


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to