#26988 closed Cleanup/optimization (fixed)
User is_authenticated callable property is confusing to check
| Reported by: | Mark Tranchant | Owned by: | nobody | 
|---|---|---|---|
| Component: | contrib.auth | Version: | 1.10 | 
| Severity: | Release blocker | Keywords: | user is_authenticated property test | 
| Cc: | Triage Stage: | Accepted | |
| Has patch: | yes | Needs documentation: | no | 
| Needs tests: | no | Patch needs improvement: | no | 
| Easy pickings: | no | UI/UX: | no | 
Description (last modified by )
Just upgraded to 1.10, converted all is_authenticated() methods into is_authenticated properties as per the Release Notes and a test in my test suite failed. 
It turns out I was checking for a logged in user with if request.user.is_authenticated is False:, but the is_authenticated property is actually a CallableBool which cannot be tested with {{== False}}, {{is False}}, {{== True}} or {{is True}}.
Checking this property only gives logical results with direct if user.is_authenticated or if not user.is_authenticated. This is very misleading and non-intuitive behaviour (at odds with PEP8 (bottom of linked section)) and should be fixed or strongly called out in the documentation. Example:
In [1]: from django.contrib.auth.models import AnonymousUser, AbstractBaseUser In [2]: a = AnonymousUser() In [3]: b = AbstractBaseUser() In [4]: a.is_authenticated Out[4]: CallableBool(False) In [5]: b.is_authenticated Out[5]: CallableBool(True) In [6]: a.is_authenticated is False Out[6]: False In [7]: a.is_authenticated == False Out[7]: False In [8]: not a.is_authenticated Out[8]: True In [9]: not b.is_authenticated Out[9]: False In [10]: b.is_authenticated == False Out[10]: False In [11]: b.is_authenticated == True Out[11]: False
Change History (9)
comment:1 by , 9 years ago
| Summary: | User is_authenticated callable property confusing if used with "is False" → User is_authenticated callable property is confusing to check | 
|---|
comment:2 by , 9 years ago
| Description: | modified (diff) | 
|---|---|
| Easy pickings: | set | 
comment:3 by , 9 years ago
| Description: | modified (diff) | 
|---|
comment:4 by , 9 years ago
| Easy pickings: | unset | 
|---|---|
| Triage Stage: | Unreviewed → Accepted | 
| Type: | Uncategorized → Cleanup/optimization | 
comment:5 by , 9 years ago
| Has patch: | set | 
|---|
comment:6 by , 9 years ago
Thanks, looks like the best we can do for a transitional period. In your release notes update:
Allowed User.is_authenticated() and User.is_anonymous() properties
you should not include the brackets.
comment:9 by , 9 years ago
I think it's better to add a remark to the code, because someone only read the code
if request.user.is_authenticated is True:  # won't work
    ...
I think it's impossible to fix the
iscomparison, but we can fix==and!=as well as improve the release note. PR.