diff --git a/horizon/exceptions.py b/horizon/exceptions.py index c5f2450..cf460e3 100644 --- a/horizon/exceptions.py +++ b/horizon/exceptions.py @@ -203,7 +203,7 @@ class indicating the type of exception that was encountered will be if issubclass(exc_type, UNAUTHORIZED): if ignore: return NotAuthorized - request.session.clear() + request.user_logout() if not handled: LOG.debug("Unauthorized: %s" % exc_value) # We get some pretty useless error messages back from diff --git a/horizon/middleware.py b/horizon/middleware.py index f20c1f0..20902c9 100644 --- a/horizon/middleware.py +++ b/horizon/middleware.py @@ -49,6 +49,14 @@ def process_request(self, request): Adds a :class:`~horizon.users.User` object to ``request.user``. """ + # A quick and dirty way to log users out + def user_logout(request): + del request._cached_user + # Use flush instead of clear, so we rotate session keys in + # addition to clearing all the session data + request.session.flush() + request.__class__.user_logout = user_logout + request.__class__.user = users.LazyUser() request.horizon = {'dashboard': None, 'panel': None} diff --git a/horizon/tests/auth_tests.py b/horizon/tests/auth_tests.py index ba16477..9f295d0 100644 --- a/horizon/tests/auth_tests.py +++ b/horizon/tests/auth_tests.py @@ -18,6 +18,8 @@ # License for the specific language governing permissions and limitations # under the License. +import time + from django import http from django.core.urlresolvers import reverse from keystoneclient import exceptions as keystone_exceptions @@ -220,3 +222,53 @@ def test_logout(self): self.assertRedirectsNoFollow(res, reverse('splash')) self.assertNotIn(KEY, self.client.session) + + def test_session_fixation(self): + session_ids = [] + form_data = {'method': 'Login', + 'region': 'http://localhost:5000/v2.0', + 'password': self.user.password, + 'username': self.user.name} + + self.mox.StubOutWithMock(api, 'token_create') + self.mox.StubOutWithMock(api, 'tenant_list_for_token') + self.mox.StubOutWithMock(api, 'token_create_scoped') + + aToken = self.tokens.unscoped_token + bToken = self.tokens.scoped_token + + api.token_create(IsA(http.HttpRequest), "", self.user.name, + self.user.password).AndReturn(aToken) + api.tenant_list_for_token(IsA(http.HttpRequest), + aToken.id).AndReturn([self.tenants.first()]) + api.token_create_scoped(IsA(http.HttpRequest), + self.tenant.id, + aToken.id).AndReturn(bToken) + + api.token_create(IsA(http.HttpRequest), "", self.user.name, + self.user.password).AndReturn(aToken) + api.tenant_list_for_token(IsA(http.HttpRequest), + aToken.id).AndReturn([self.tenants.first()]) + api.token_create_scoped(IsA(http.HttpRequest), + self.tenant.id, + aToken.id).AndReturn(bToken) + self.mox.ReplayAll() + + res = self.client.get(reverse('horizon:auth_login')) + self.assertEqual(res.cookies.get('sessionid'), None) + res = self.client.post(reverse('horizon:auth_login'), form_data) + session_ids.append(res.cookies['sessionid'].value) + + self.assertEquals(self.client.session['user_name'], + self.user.name) + self.client.session['foobar'] = 'MY TEST VALUE' + res = self.client.get(reverse('horizon:auth_logout')) + session_ids.append(res.cookies['sessionid'].value) + self.assertEqual(len(self.client.session.items()), 0) + # Sleep for 1 second so the session values are different if + # using the signed_cookies backend. + time.sleep(1) + res = self.client.post(reverse('horizon:auth_login'), form_data) + session_ids.append(res.cookies['sessionid'].value) + # Make sure all 3 session id values are different + self.assertEqual(len(session_ids), len(set(session_ids))) diff --git a/horizon/users.py b/horizon/users.py index f5dcde8..6e6be8e 100644 --- a/horizon/users.py +++ b/horizon/users.py @@ -59,7 +59,7 @@ def get_user_from_request(request): # If any of those keys are missing from the session it is # overwhelmingly likely that we're dealing with an outdated session. LOG.exception("Error while creating User from session.") - request.session.clear() + request.user_logout() raise exceptions.NotAuthorized(_("Your session has expired. " "Please log in again.")) diff --git a/horizon/views/auth.py b/horizon/views/auth.py index b48f24a..5120eed 100644 --- a/horizon/views/auth.py +++ b/horizon/views/auth.py @@ -96,6 +96,6 @@ def switch_tenants(request, tenant_id): def logout(request): """ Clears the session and logs the current user out. """ - request.session.clear() + request.user_logout() # FIXME(gabriel): we don't ship a view named splash return shortcuts.redirect('splash') diff --git a/horizon/views/auth_forms.py b/horizon/views/auth_forms.py index 2874486..2ebecfc 100644 --- a/horizon/views/auth_forms.py +++ b/horizon/views/auth_forms.py @@ -77,6 +77,16 @@ def __init__(self, *args, **kwargs): self.fields['region'].widget = forms.widgets.HiddenInput() def handle(self, request, data): + if 'user_name' in request.session: + if request.session['user_name'] != data['username']: + # To avoid reusing another user's session, create a + # new, empty session if the existing session + # corresponds to a different authenticated user. + request.session.flush() + # Always cycle the session key when viewing the login form to + # prevent session fixation + request.session.cycle_key() + # For now we'll allow fallback to OPENSTACK_KEYSTONE_URL if the # form post doesn't include a region. endpoint = data.get('region', None) or settings.OPENSTACK_KEYSTONE_URL @@ -116,7 +126,7 @@ def handle(self, request, data): # If we get here we don't want to show a stack trace to the # user. However, if we fail here, there may be bad session # data that's been cached already. - request.session.clear() + request.user_logout() exceptions.handle(request, message=_("An error occurred authenticating." " Please try again later."),