Redundant is_active check in auth.backends.ModelBackend

2019-03-23 Thread Tobias Bengfort
See https://github.com/django/django/pull/11037

User.is_active is checked in contrib.auth.ModelBackend on all of these
methods:

- `get_user_permissions()`
- `get_group_permissions()`
- `get_all_permissions()`
- `has_perm()`
- `has_module_perms()`

I think the last three are redundant and should be removed.

Tim Graham has concerns though:

> I'm unsure about removing the "redundant" is_active checks. It might
> be that some ModelBackend sublcasses rely on them. For example, if 
> you subclass get_all_permissions() and omitting the is_active check 
> (which wasn't there prior to Django 1.8 c33447a)... then your 
> application would still have is_active checks in the other methods. 
> This needs careful consideration and perhaps a discussion on the 
> mailing list as to whether the benefits are worth the possible
> security issues.

My opinion is exactly the opposite: If a ModelBackend subclass does not
check `is_active` in `get_all_permissions()` that is a bug and
potentially even a security issue. The redundant checks hide these
issues and therefore make it harder to find them.

I also think that the different methods should be consistent: If a
permission is returned by `get_all_permissions()`, then checking that
permission with `has_perm()` should return `True`. The only reason to do
anything special in `has_perm()` is for performance optimizations.

tobias

-- 
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/5400a61f-6fd5-5c9f-15b5-97ab657c2e7a%40posteo.de.
For more options, visit https://groups.google.com/d/optout.


Re: Fellow Reports - March 2019

2019-03-23 Thread Tim Graham


Week ending March 23, 2019

Triaged

---

https://code.djangoproject.com/ticket/30270 - .all() for related manager 
always returns empty list (invalid)

https://code.djangoproject.com/ticket/30277 - Django Advanced Tutorial Link 
doesn't lead to the intended page (fixed)

https://code.djangoproject.com/ticket/30276 - reverse DeleteModel migration 
with a primary key OneToOneField fails in MySQL (fixed)

https://code.djangoproject.com/ticket/29539 - Cannot use Aggregation 
function in Model.Meta.ordering (wontfix)

https://code.djangoproject.com/ticket/30284 - Redundant is_active check in 
auth.backends.ModelBackend (wontfix)

Reviewed/committed

--

https://code.djangoproject.com/ticket/30172 - Fixed #30172 -- Prevented 
Meta constraints from being removed in other migration operations.

https://github.com/django/django/pull/11083 - Fixed #30253 -- Doc'd how to 
order nulls in QuerySet.order_by().

https://github.com/django/django/pull/11093 - Fixed #30263 -- Doc'd changes 
to form Media sorting (refs #30179).

https://github.com/django/django/pull/11091 - Fixed #28738 -- Added the 
GeometryDistance function.

https://github.com/django/django/pull/11030 - Fixed #29471 -- Added 'Vary: 
Cookie' to invalid/empty session cookie responses.

https://github.com/django/django/pull/11030 - Fixed #30158 -- Avoided 
unnecessary subquery group by on aggregation.

https://github.com/django/django/pull/11109 - Fixed #30280 -- Restored 
Model.get_FIELD_display()'s coercion of lazy strings.

https://github.com/django/django/pull/11099 - Fixed #30257 -- Made 
UsernameValidators prohibit trailing newlines. 

https://github.com/django/django/pull/11062 - Subquery resolving refactor 
and bug fixing.

https://github.com/django/django/pull/2 - Removed redundant model field 
choices tests.
https://github.com/django/django/pull/3 - Fixed #30283 -- Fixed 
shellcheck warnings in django_bash_completion.

-- 
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/c588615f-109b-4bd6-a386-2bf7221df6f3%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.