Opened 9 years ago

Closed 9 years ago

#25075 closed Bug (invalid)

Change is_authenticated() that depended on is_active

Reported by: Grigoriy Kramarenko Owned by: nobody
Component: contrib.auth Version: 1.8
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

I propose to change the following:

@python_2_unicode_compatible
class AbstractBaseUser(models.Model):
    ...

    is_active = True

    ...

    def is_authenticated(self):
        """
        Always return True if user is active. This is a way to tell if the user has been
        authenticated in templates. Also this used in `login_required` decorator.
        """
        return self.is_active

Change History (6)

comment:1 by Tim Graham, 9 years ago

Why?

comment:2 by Grigoriy Kramarenko, 9 years ago

This has always been so in earlier versions. If the user is working with the site, then he has session_key. In that moment, when switched off it administrator (is_active=False), user will continue to have access to views under the decorator login_required until continues the action session_key.
I'm not sure that this behavior will not resume in new versions or in a custom User model or in different from default session backends.

If I'm wrong, let's close the ticket.

Last edited 9 years ago by Grigoriy Kramarenko (previous) (diff)

comment:3 by Tim Graham, 9 years ago

Resolution: invalid
Status: newclosed

The login_required() decorator is documented as not checking the is_active flag. If you can provide more information about how the behavior changed in recent versions of Django, please reopen with those details.

comment:4 by Grigoriy Kramarenko, 9 years ago

Needs documentation: set
Resolution: invalid
Status: closednew
Type: UncategorizedBug

In the documentation I have seen (on link) only vague: "If the user is logged in, execute the view normally. The view code is free to assume the user is logged in."

The documentation should be written clearly, that: "Note, if the administrator has turned off the user by using "is_active", the user will have access to all the views that are under the control of the decorator until the action of the session key."
Now it is not mentioned in the documentation.

I'll show an example (stable 1.8.3):

$ mkdir test
$ django-admin startproject project test
$ cd test/project/
$ ../manage.py migrate 
Operations to perform:
  Synchronize unmigrated apps: staticfiles, messages
  Apply all migrations: admin, contenttypes, auth, sessions
Synchronizing apps without migrations:
  Creating tables...
    Running deferred SQL...
  Installing custom SQL...
Running migrations:
  Rendering model states... DONE
  Applying contenttypes.0001_initial... OK
  Applying auth.0001_initial... OK
  Applying admin.0001_initial... OK
  Applying contenttypes.0002_remove_content_type_name... OK
  Applying auth.0002_alter_permission_name_max_length... OK
  Applying auth.0003_alter_user_email_max_length... OK
  Applying auth.0004_alter_user_username_opts... OK
  Applying auth.0005_alter_user_last_login_null... OK
  Applying auth.0006_require_contenttypes_0002... OK
  Applying sessions.0001_initial... OK

$ ../manage.py createsuperuser
Username (leave blank to use 'djbaldey'): admin
Email address: admin@example.org
Password: 
Password (again): 
Superuser created successfully.

$ nano views.py

Listing views.py:

from django.contrib.auth.decorators import login_required
from django.http import HttpResponse

@login_required
def test_access(request):
    return HttpResponse('access')
$ nano urls.py

Listing urls.py:

from django.conf.urls import include, url
from django.contrib import admin
from .views import test_access

urlpatterns = [
    url(r'^admin/', include(admin.site.urls)),
    url(r'^test/', test_access),
]
$ ../manage.py runserver

Open browser on http://localhost:8000/admin/ and sign.
And open http://localhost:8000/test/ in new tab after.
Disable user in the admin panel. You will be thrown out from page.
But, on page /test/ you will continue to have access to.

While in the documentation is not written this sudden behavior, we can assume this is a bug.

P.S.:
Help text for is_active contains "Designates whether this user should be treated as active. Unselect this instead of deleting accounts."
But: is_authenticated() != is_active
Where is logic?
To distinguish anonymous users from real users have a method is_anonymous().

Last edited 9 years ago by Grigoriy Kramarenko (previous) (diff)

comment:5 by Tim Graham, 9 years ago

The admin uses the staff_member_required decorator which does require is_active=True. I don't think changing the behavior of either decorator is an option, but I'll be happy to review a documentation patch if you could provide one.

comment:6 by Shai Berger, 9 years ago

Resolution: invalid
Status: newclosed

The linked documentation has a note that says is_active is not checked, at the bottom of the decorator's description. This has been the case for quite some time.

Note: See TracTickets for help on using tickets.
Back to Top