The key of permissions model

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

The key of permissions model

Anssi Kääriäinen
We use the style of "app_label.permission_name" in multiple places of
our code to refer to given auth.models.Permission. However, there is
no unique key for that combination, the key is content_type,
permission_name. I verified that it is actually possible to hit this
problem by using the Meta.permissions.

I am not sure what we can do about this... Or if we even need to do
anything about this.

Without schema changes we can't have a DB constraint for the key. But,
maybe we could validate (as part of model validation) that there are
no overlapping permissions? If the user goes and creates overlapping
permissions despite of this, we can't do anything about that.

The reason why I am wondering about this is that I'd like to add
Permissions.objects.get_permission('app_label.permission_name')
convenience method. And, while investigating that I realized that the
app_label, permission name isn't a key...

I am likely going to add the get_permission() anyways, it will just
throw "returned more than 1" if the above problem is hit.

Thoughts?

 - Anssi

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

Reply | Threaded
Open this post in threaded view
|

Re: The key of permissions model

Donald Stufft
Can't you add the constraint in both code and in the DB. On older sites
the constraint just won't exist in the DB (Could include it in the release
notes so people can add it to existing sites if they wish).

On Wednesday, September 19, 2012 at 6:40 PM, Anssi Kääriäinen wrote:

We use the style of "app_label.permission_name" in multiple places of
our code to refer to given auth.models.Permission. However, there is
no unique key for that combination, the key is content_type,
permission_name. I verified that it is actually possible to hit this
problem by using the Meta.permissions.

I am not sure what we can do about this... Or if we even need to do
anything about this.

Without schema changes we can't have a DB constraint for the key. But,
maybe we could validate (as part of model validation) that there are
no overlapping permissions? If the user goes and creates overlapping
permissions despite of this, we can't do anything about that.

The reason why I am wondering about this is that I'd like to add
Permissions.objects.get_permission('app_label.permission_name')
convenience method. And, while investigating that I realized that the
app_label, permission name isn't a key...

I am likely going to add the get_permission() anyways, it will just
throw "returned more than 1" if the above problem is hit.

Thoughts?

- Anssi

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

--
You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to [hidden email].
To unsubscribe from this group, send email to [hidden email].
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Reply | Threaded
Open this post in threaded view
|

Re: The key of permissions model

Anssi Kääriäinen
On 20 syys, 01:47, Donald Stufft <[hidden email]> wrote:
> Can't you add the constraint in both code and in the DB. On older sites
> the constraint just won't exist in the DB (Could include it in the release
> notes so people can add it to existing sites if they wish).

Unfortunately no - the app label doesn't exist in the model. There is
only reference to ContentType, and the app label is stored in there.
So, a normal unique constraint doesn't work...

 - Anssi

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

Reply | Threaded
Open this post in threaded view
|

Re: The key of permissions model

Michael Manfre
In reply to this post by Anssi Kääriäinen
Instead of get_permission('app_label.permission_name'), why not punt on the problem
until schema migrations lands. Only provide a plural helper method that always returns
a list. The first argument could be either a string or a list of strings. This leaves it up to
the caller to determine what to do if more than one is returned when they only expected
a single result. Having all of the conflicts is a lot more useful than the "returned more than
1" exception.

Regards,
Michael Manfre

On Wednesday, September 19, 2012 6:41:25 PM UTC-4, Anssi Kääriäinen wrote:
We use the style of "app_label.permission_name" in multiple places of
our code to refer to given auth.models.Permission. However, there is
no unique key for that combination, the key is content_type,
permission_name. I verified that it is actually possible to hit this
problem by using the Meta.permissions.

I am not sure what we can do about this... Or if we even need to do
anything about this.

Without schema changes we can't have a DB constraint for the key. But,
maybe we could validate (as part of model validation) that there are
no overlapping permissions? If the user goes and creates overlapping
permissions despite of this, we can't do anything about that.

The reason why I am wondering about this is that I'd like to add
Permissions.objects.get_permission('app_label.permission_name')
convenience method. And, while investigating that I realized that the
app_label, permission name isn't a key...

I am likely going to add the get_permission() anyways, it will just
throw "returned more than 1" if the above problem is hit.

Thoughts?

 - Anssi

--
You received this message because you are subscribed to the Google Groups "Django developers" group.
To view this discussion on the web visit https://groups.google.com/d/msg/django-developers/-/_ool0iQ7kKwJ.
To post to this group, send email to [hidden email].
To unsubscribe from this group, send email to [hidden email].
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Reply | Threaded
Open this post in threaded view
|

Re: The key of permissions model

Anssi Kääriäinen
On 20 syys, 17:11, Michael Manfre <[hidden email]> wrote:

> Instead of get_permission('app_label.permission_name'), why not punt on the
> problem
> until schema migrations lands. Only provide a plural helper method that
> always returns
> a list. The first argument could be either a string or a list of strings.
> This leaves it up to
> the caller to determine what to do if more than one is returned when they
> only expected
> a single result. Having all of the conflicts is a lot more useful than the
> "returned more than
> 1" exception.

It is a bad idea to allow multiple permissions with the same key to
exists at all. I checked quickly what user.has_perm() does. It happily
reports that the user has the permission if the user has any
permission matching the asked key. This again means it is possible
that a permission check will pass for the wrong instance of
'myapp.someperm'.

Luckily this isn't too serious, as I don't believe it is common to
have overlapping permission keys. The possibility is there, and if
this does happen, then there is a possibility for security issues. We
should not encourage this pattern, but instead document that applabel,
permission_name is a key for permissions (because we already treat it
so), and try to stop overlapping permissions where possible.

For the above reasons I don't like adding APIs which encourage
duplicate app-label keys for permissions, and get_permission()
returning a list is such. If get_permission() returning a single
permission isn't acceptable currently, then I see it as better to wait
until enforced key for app_label, permission_name is implemented than
add the method.

 - Anssi

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

Reply | Threaded
Open this post in threaded view
|

Re: The key of permissions model

Hanne Moa
In reply to this post by Michael Manfre
On 20 September 2012 16:11, Michael Manfre <[hidden email]> wrote:
> The first argument could be either a string or a list of strings.

Bad pattern. Then you must test for the existence of a string in order
to avoid ('t', 'h', 'i', 's').


HM

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

Reply | Threaded
Open this post in threaded view
|

Re: The key of permissions model

Flavio Curella
I'd like to take another try at this.


My proposal is to temporarily supports two formats: the new `app_label.model_name.codename` (ref #25281) and the old `app.codename`. The old format will raise a PendingDeprecationWarning. Docs, code and tests will be updated to use the new format.

I believe that by having `app_label.model_name`. we can identify the `.content_type`. That, together with `.codename`, should be enough to uniquely identify the `Permission` instance.

There will be no model changes. Just checks and new code for all the methods that accepts the 'string api' for permissions.

Thanks,
–Flavio.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/fb1b7c22-d436-4fa0-9f05-30f151a48da0%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.