Re: [Django] #10686: Add class name interpolation in Meta.permissions codenames

Django

unread,

Jul 8, 2016, 2:51:55 PM7/8/16

to django-...@googlegroups.com

#10686: Add class name interpolation in Meta.permissions codenames
-------------------------------------+-------------------------------------
Reporter: faldridge | Owner: sergei-
| maertens
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: permissions | Triage Stage: Accepted
inheritance |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by timgraham):

* needs_better_patch: 0 => 1

Comment:

Comments for improvement on the PR.

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

Django

unread,

Jul 26, 2016, 5:10:14 PM7/26/16

to django-...@googlegroups.com

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by sergei-maertens):

* needs_better_patch: 1 => 0

Comment:

Processed the remarks, up for re-review

--
Ticket URL: <https://code.djangoproject.com/ticket/10686#comment:18>

Django

unread,

Aug 13, 2016, 9:15:55 AM8/13/16

to django-...@googlegroups.com

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by timgraham):

* needs_better_patch: 0 => 1

Comment:

Left some more comments.

--
Ticket URL: <https://code.djangoproject.com/ticket/10686#comment:19>

Django

unread,

Aug 13, 2016, 9:26:26 AM8/13/16

to django-...@googlegroups.com

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by sergei-maertens):

* needs_better_patch: 1 => 0

Comment:

Processed comments - 1 comment was replied to on Github & no change made
in docs: https://github.com/django/django/pull/6861#discussion-
diff-74686863

--
Ticket URL: <https://code.djangoproject.com/ticket/10686#comment:20>

Django

unread,

Aug 17, 2016, 10:23:48 AM8/17/16

to django-...@googlegroups.com

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by timgraham):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/10686#comment:21>

Django

unread,

Aug 17, 2016, 10:36:41 AM8/17/16

to django-...@googlegroups.com

#10686: Add class name interpolation in Meta.permissions codenames
-------------------------------------+-------------------------------------
Reporter: faldridge | Owner: sergei-
| maertens
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: permissions | Triage Stage: Accepted
inheritance |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by sergei-maertens):

Note to self: the unit test should check that the correct permissions are
created in the db, per:

https://github.com/django/django/pull/6861#issuecomment-240427426
> Worked on some cosmetic edits: http://dpaste.com/19R4PZE but I also
noticed that this doesn't seem to create the correct permission in the
database. For the pizza example in the docs I see things like
"can_deliver_base" in the database.

--
Ticket URL: <https://code.djangoproject.com/ticket/10686#comment:22>

Django

unread,

May 17, 2021, 2:45:40 PM5/17/21

to django-...@googlegroups.com

#10686: Add class name interpolation in Meta.permissions codenames
-------------------------------------+-------------------------------------

Reporter: faldridge | Owner: (none)
Type: New feature | Status: new
Component: Database layer | Version: dev

Changes (by Mariusz Felisiak):

* owner: Sergei Maertens => (none)
* status: assigned => new

--
Ticket URL: <https://code.djangoproject.com/ticket/10686#comment:23>

Django

unread,

Aug 9, 2022, 10:15:14 AM8/9/22

to django-...@googlegroups.com

#10686: Add class name interpolation in Meta.permissions codenames
-------------------------------------+-------------------------------------
Reporter: faldridge | Owner: (none)
Type: New feature | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: permissions | Triage Stage: Accepted
inheritance |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Old description:

> 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.
>
> 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:
> {{{
> #!python
> 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.
>

> 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.
>
> 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.
>
> Example:
> {{{
> #!python
> 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
> (Django's current behavior). If you wanted to inherit/append the
> view_class
> permission instead (proposed 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, I believe it requires a design decision.
>
> Thanks!

New description:

--

Comment (by jul...@pinabausch.org):

Dear community,

just picked this 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

--
Ticket URL: <https://code.djangoproject.com/ticket/10686#comment:24>

Django

unread,

Aug 9, 2022, 3:25:35 PM8/9/22

to django-...@googlegroups.com

#10686: Add class name interpolation in Meta.permissions codenames
-------------------------------------+-------------------------------------
Reporter: faldridge | Owner: (none)
Type: New feature | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: permissions | Triage Stage: Accepted
inheritance |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Description changed by Tim Graham:

Old description:

New description:

Thanks!

--

--
Ticket URL: <https://code.djangoproject.com/ticket/10686#comment:25>

Django

unread,

Aug 10, 2022, 2:19:41 AM8/10/22

to django-...@googlegroups.com

#10686: Add class name interpolation in Meta.permissions codenames
-------------------------------------+-------------------------------------
Reporter: faldridge | Owner: (none)
Type: New feature | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: permissions | Triage Stage: Accepted
inheritance |

Has patch: 1 | Needs documentation: 1

Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak):

* needs_docs: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/10686#comment:26>

Django

unread,

Aug 10, 2022, 2:20:02 AM8/10/22

to django-...@googlegroups.com

#10686: Add class name interpolation in Meta.permissions codenames
-------------------------------------+-------------------------------------
Reporter: faldridge | Owner:

| jul...@pinabausch.org

Type: New feature | Status: assigned

* owner: (none) => jul...@pinabausch.org
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/10686#comment:27>

Django

unread,

Aug 11, 2022, 2:58:16 AM8/11/22

to django-...@googlegroups.com

#10686: Add class name interpolation in Meta.permissions codenames
-------------------------------------+-------------------------------------
Reporter: faldridge | Owner:
| jul...@pinabausch.org
Type: New feature | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: permissions | Triage Stage: Accepted
inheritance |
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by jul...@pinabausch.org):

Thanks Mariusz Felisiak for being so responsive!

I’d love to hear your comment on this:

> 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)

There are quite a few nonsense commits – when you consider the main work
done, please let me know. I’d like to squash the commits and add the co-
authors before merging.

--
Ticket URL: <https://code.djangoproject.com/ticket/10686#comment:28>

Django

unread,

Aug 11, 2022, 3:34:43 AM8/11/22

to django-...@googlegroups.com

Needs tests: 1 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------