openstack/deb-horizon
Code Issues Proposed changes

Checking session timeout before authentication

Browse Source 
If both the keystone token and the session expired, the user was asked to login
twice. This is because the token expiration was not checked.
When a user had timed out both in session and keystone token validity, the user
was asked to log in, then the timestamp was checked, and the user logged out
again and asked to log in a second time.

We now check both the timestamp and keystone token validity before
authentication validity and force back the login page to retrieve a new
keystone token, avoiding the timeout race condition between session and token
validity which was forcing a dual login. A keystone token expiration is now
considered as a session timeout too.

Also, a page can start loading while the token is valid, and finish while it's
invalid. This was leading to errors during the page loading.
We now set a TOKEN_TIMEOUT_MARGIN period in seconds which allows defining
a margin before which we consider the token as expired.
This is a configurable parameter in the django settings because the time a page
takes to render is infra and deployment specific. This margin is preset to
ten seconds.

Requires: https://review.openstack.org/101556

Closes-Bug: 1308918

Change-Id: I0bf0d079a9dc000c1a30f0e20dcaa03b22d63e51
This commit is contained in:
Yves-Gwenael Bourhis 2014-04-17 11:18:00 +02:00
parent d614789da2
commit 4824239730
6 changed files with 110 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
horizon/middleware.py
View File

@ -28,17 +28,19 @@ from django.contrib.auth import REDIRECT_FIELD_NAME # noqa
from django.contrib.auth.views import redirect_to_login # noqa
from django.contrib import messages as django_messages
from django import http
from django.http import HttpResponseRedirect # noqa
from django import shortcuts
from django.utils.encoding import iri_to_uri # noqa
from django.utils import timezone
from django.utils.translation import ugettext_lazy as _
from openstack_auth import utils as auth_utils
from openstack_auth import views as auth_views
import six
from horizon import exceptions
from horizon.utils import functions as utils
LOG = logging.getLogger(__name__)
@ -47,25 +49,53 @@ class HorizonMiddleware(object):
logout_reason = None
def process_request(self, request):
"""Adds data necessary for Horizon to function to the request."""
def _check_has_timed_timeout(self, request):
"""Check for session timeout and return timestamp."""
has_timed_out = False
# Activate timezone handling
tz = request.session.get('django_timezone')
if tz:
timezone.activate(tz)
# Check for session timeout
try:
timeout = settings.SESSION_TIMEOUT
except AttributeError:
timeout = 1800
last_activity = request.session.get('last_activity', None)
timestamp = int(time.time())
if (
hasattr(request, "user")
and hasattr(request.user, "token")
and not auth_utils.is_token_valid(request.user.token)
):
# The user was logged in, but his keystone token expired.
has_timed_out = True
if isinstance(last_activity, int):
if (timestamp - last_activity) > timeout:
has_timed_out = True
if has_timed_out:
request.session.pop('last_activity')
return (has_timed_out, timestamp)
def _logout(self, request, login_url=None, message=None):
"""Logout a user and display a logout message."""
response = auth_views.logout(request, login_url)
if message is not None:
self.logout_reason = message
utils.add_logout_reason(request, response, message)
return response
def process_request(self, request):
"""Adds data necessary for Horizon to function to the request."""
request.horizon = {'dashboard': None,
'panel': None,
'async_messages': []}
# Check for session timeout if user is (or was) authenticated.
has_timed_out, timestamp = self._check_has_timed_timeout(request)
if has_timed_out:
return self._logout(request, request.path, _("Session timed out."))
if not hasattr(request, "user") or not request.user.is_authenticated():
# proceed no further if the current request is already known
# not to be authenticated
@ -108,15 +138,7 @@ class HorizonMiddleware(object):
'max_cookie_size': max_cookie_size,
}
)
if (isinstance(last_activity, int)
and (timestamp - last_activity) > timeout):
request.session.pop('last_activity')
response = HttpResponseRedirect(
'%s?next=%s' % (settings.LOGOUT_URL, request.path))
self.logout_reason = _("Session timed out.")
utils.add_logout_reason(request, response, self.logout_reason)
return response
# We have a valid session, so we set the timestamp
request.session['last_activity'] = timestamp
def process_exception(self, request, exception):

5
horizon/test/dummy_auth/backend.py
View File

@ -10,6 +10,8 @@
# License for the specific language governing permissions and limitations
# under the License.
import datetime
from horizon.test import utils
@ -19,7 +21,8 @@ class DummyBackend(object):
nam='test_user',
email='test@example.com',
password='password',
token='test_token',
token=utils.ObjDictWrapper(
expires=datetime.datetime.now() + datetime.timedelta(30)),
project_id='1',
enabled=True,
domain_id="1",

36
horizon/test/helpers.py
View File

@ -26,6 +26,7 @@ from django.contrib.auth.models import Permission # noqa
from django.contrib.auth.models import User # noqa
from django.contrib.contenttypes.models import ContentType # noqa
from django.contrib.messages.storage import default_storage # noqa
from django.contrib.sessions.backends.base import SessionBase # noqa
from django.core.handlers import wsgi
from django import http
from django import test as django_test
@ -58,18 +59,49 @@ from horizon import middleware
wsgi.WSGIRequest.__repr__ = lambda self: "<class 'django.http.HttpRequest'>"
class SessionStore(SessionBase):
"""Dict like object for simulating sessions in unittests."""
def load(self):
self.create()
return {}
def create(self):
self.modified = True
def save(self, must_create=False):
self._session_key = self._get_session_key()
self.modified = True
def exists(self, session_key=None):
return False
def delete(self, session_key=None):
self._session_key = ''
self._session_cache = {}
self.modified = True
def cycle_key(self):
self.save()
@classmethod
def clear_expired(cls):
pass
class RequestFactoryWithMessages(RequestFactory):
def get(self, *args, **kwargs):
req = super(RequestFactoryWithMessages, self).get(*args, **kwargs)
req.user = User()
req.session = {}
req.session = SessionStore()
req._messages = default_storage(req)
return req
def post(self, *args, **kwargs):
req = super(RequestFactoryWithMessages, self).post(*args, **kwargs)
req.user = User()
req.session = {}
req.session = SessionStore()
req._messages = default_storage(req)
return req

5
horizon/test/tests/middleware.py
View File

@ -35,9 +35,8 @@ class MiddlewareTests(test.TestCase):
self.assertRedirects(resp, url)
def test_redirect_session_timeout(self):
def test_session_timeout(self):
requested_url = '/project/instances/'
response_url = '%s?next=%s' % (settings.LOGOUT_URL, requested_url)
request = self.factory.get(requested_url)
try:
timeout = settings.SESSION_TIMEOUT
@ -47,7 +46,7 @@ class MiddlewareTests(test.TestCase):
mw = middleware.HorizonMiddleware()
resp = mw.process_request(request)
self.assertEqual(302, resp.status_code)
self.assertEqual(response_url, resp.get('Location'))
self.assertEqual(requested_url, resp.get('Location'))
def test_process_response_redirect_on_ajax_request(self):
url = settings.LOGIN_URL

27
openstack_dashboard/dashboards/settings/password/tests.py
View File

@ -12,9 +12,12 @@
# License for the specific language governing permissions and limitations
# under the License.
import django
from django.conf import settings
from django.core.urlresolvers import NoReverseMatch # noqa
from django.core.urlresolvers import reverse
from django import http
from django.utils.six.moves.urllib.parse import urlsplit # noqa
from django.utils import unittest
from mox import IsA # noqa
@ -69,3 +72,27 @@ class ChangePasswordTests(test.TestCase):
info_msg = "Password changed. Please log in again to continue."
self.assertContains(res, info_msg)
@unittest.skipUnless(django.VERSION[0] >= 1 and django.VERSION[1] >= 6,
"'HttpResponseRedirect' object has no attribute "
"'url' prior to Django 1.6")
@test.create_stubs({api.keystone: ('user_update_own_password', )})
def test_change_password_sets_logout_reason(self):
api.keystone.user_update_own_password(IsA(http.HttpRequest),
'oldpwd',
'normalpwd').AndReturn(None)
self.mox.ReplayAll()
formData = {'method': 'PasswordForm',
'current_password': 'oldpwd',
'new_password': 'normalpwd',
'confirm_password': 'normalpwd'}
res = self.client.post(INDEX_URL, formData, follow=False)
self.assertRedirectsNoFollow(res, settings.LOGOUT_URL)
self.assertIn('logout_reason', res.cookies)
self.assertEqual(res.cookies['logout_reason'].value,
"Password changed. Please log in again to continue.")
scheme, netloc, path, query, fragment = urlsplit(res.url)
redirect_response = res.client.get(path, http.QueryDict(query))
self.assertRedirectsNoFollow(redirect_response, settings.LOGIN_URL)

6
openstack_dashboard/settings.py
View File

@ -221,6 +221,12 @@ SESSION_COOKIE_HTTPONLY = True
SESSION_EXPIRE_AT_BROWSER_CLOSE = True
SESSION_COOKIE_SECURE = False
SESSION_TIMEOUT = 1800
# A token can be near the end af validity when a page starts loading, and
# invalid during the rendering which can cause errors when a page load.
# TOKEN_TIMEOUT_MARGIN defines a time in seconds we retrieve from token
# validity to avoid this issue. You can adjust this time depending on the
# performance of the infrastructure.
TOKEN_TIMEOUT_MARGIN = 10
# When using cookie-based sessions, log error when the session cookie exceeds
# the following size (common browsers drop cookies above a certain size):