Florian, now the implementation is if the backend doesn't implement the
has_perm we use continue so the code is not checked at all and return False
for a regular user.
The solutions suggested here are logging, raise an error on DEBUG = True
and return False if the permission doesn't exist (if I missed anything let
me know).
I numbered the options suggested so far for easy reference.
1. The way I see it if we use logging the user will have to check if it
got an error when working with has_perm, and if the problem started with
the programmer is not doing tests well I don't think he/she will check the
logs if they have them, and they might not have logs enabled, but it's a
nice solution that won't need anything except this change (which i can
implement as a function .
2. Raise an error on DEBUG = True is my favorite, if there is a bug it will
jump on the first time the user is working on it when running the app but
it's not backwards compatible.
3. Return False if the permission doesn't exist means that we go through
the same path as a regular user, since (at least on
auth.backends.ModelBackend) we check already if the user is superuser and
if so we return all the permissions (I suppose it's only permissions that
exist) it means we only need to remove the check at the start to see if the
user is superuser.
I don't think the performance will be that much of a problem, but since
you think it might I think i will need to check it and report the results
back unless there is a preference for one of the other solutions. either
way it will be a good thing to check.
Thanks,
Moshe
On Thu, Sep 28, 2017 at 4:10 PM, Shai Berger wrote:
> Can we define a new API on the permission backend,
> "verify_permission_exists()"
> or some such, and just call it if settings.DEBUG and it is provided? That
> doesn't seem very complex to me, and doesn't necessarily imply a huge
> performance hit (even in DEBUG).
>
> On Thursday 28 September 2017 15:50:04 Tim Graham wrote:
> > I suppose we can tentatively accept the ticket, but I looked at the code
> > briefly and agree with Florian's assessment. If someone proposes a patch,
> > we can evaluate it, however, I don't see a simple way forward that
> wouldn't
> > have a security risk or an adverse effect on performance. Given the
> > philosophy, "complexity is the enemy of security," I'd lean toward
> keeping
> > the permissions checking code simple instead of adding some other logic
> > based on DEBUG.
> >
> > On Wednesday, September 27, 2017 at 9:48:24 AM UTC-4, Florian Apolloner
> >
> > wrote:
> > > I do not think it would be feasible to check existing permissions. For
> > > one, not every backend uses the Permission class Django supplies and
> > > get_all_permissions can cause performance issues so it should be used
> > > sparingly.
> > >
> > > Cheers,
> > > Florian
> > >
> > > On Sunday, September 24, 2017 at 4:56:40 PM UTC+2, moshe nahmias wrote:
> > >> Hi,
> > >> I am a python developer and like to use Django for web development.
> > >> Since I like the framework I want to contribute back, so I looked at
> the
> > >> open tickets to find something I can start with contributing and found
> > >> ticket 28588.
> > >>
> > >> This ticket is about when checking if the user has permission for some
> > >> action if the user is super user he/she gets it all the time, even
> when
> > >> the permission doesn't exist, and this is not developer friendly
> > >> because the developer can mistakenly think that everything is fine
> even
> > >> when the permission doesn't exist.
> > >>
> > >> As I understand (and correct me if I'm wrong) there should be a
> > >> discussion about if we want to do this.
> > >>
> > >> If accepted I would like to do this, I think it's an easy enough
> change
> > >> for a new contributor like me.
> > >>
> > >> As I understand the ticket the problem is that a developer gets
> confused
> > >> on this behaviour (and it's illogical) that the super user is having a
> > >> permission that doesn't exist.
> > >>
> > >> What do you think? (I think I will discuss my solution or optional
> > >> solutions after we decide if we want to change this behaviour)
> > >>
> > >> [1] https://code.djangoproject.com/ticket/28588
>
--
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 django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
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/CACf8pw6j0sGjDEkNJuxtwDbtmrWiTHrfVJWJW4uCJLrgXTx2kA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
