From b2cdf82b787440d074a7ef68bf56f70fb0d4b6b8 Mon Sep 17 00:00:00 2001 From: Ryan Petrello Date: Tue, 26 Jun 2018 22:02:43 -0400 Subject: [PATCH 1/2] make settings.SESSIONS_PER_USER work see: https://github.com/ansible/tower/issues/2209 --- awx/main/signals.py | 27 ++++++-- awx/main/tests/functional/test_session.py | 65 ++++++++++--------- .../src/shared/socket/socket.service.js | 20 ++++-- docs/websockets.md | 2 +- 4 files changed, 71 insertions(+), 43 deletions(-) diff --git a/awx/main/signals.py b/awx/main/signals.py index 4bf8e38c8b..108300dd89 100644 --- a/awx/main/signals.py +++ b/awx/main/signals.py @@ -6,6 +6,7 @@ import contextlib import logging import threading import json +import pkg_resources import sys # Django @@ -18,6 +19,7 @@ from django.db.models.signals import ( ) from django.dispatch import receiver from django.contrib.auth import SESSION_KEY +from django.contrib.sessions.models import Session from django.utils import timezone from django.utils.translation import ugettext_lazy as _ @@ -29,7 +31,6 @@ import six # AWX from awx.main.models import * # noqa -from django.contrib.sessions.models import Session from awx.api.serializers import * # noqa from awx.main.constants import TOKEN_CENSOR from awx.main.utils import model_instance_diff, model_to_dict, camelcase_to_underscore, get_current_apps @@ -600,6 +601,16 @@ def delete_inventory_for_org(sender, instance, **kwargs): @receiver(post_save, sender=Session) def save_user_session_membership(sender, **kwargs): session = kwargs.get('instance', None) + if pkg_resources.get_distribution('channels').version >= '2': + # If you get into this code block, it means we upgraded channels, but + # didn't make the settings.SESSIONS_PER_USER feature work + raise RuntimeError( + 'save_user_session_membership must be updated for channels>=2: ' + 'http://channels.readthedocs.io/en/latest/one-to-two.html#requirements' + ) + if 'runworker' in sys.argv: + # don't track user session membership for websocket per-channel sessions + return if not session: return user = session.get_decoded().get(SESSION_KEY, None) @@ -608,13 +619,15 @@ def save_user_session_membership(sender, **kwargs): user = User.objects.get(pk=user) if UserSessionMembership.objects.filter(user=user, session=session).exists(): return - UserSessionMembership.objects.create(user=user, session=session, created=timezone.now()) - for membership in UserSessionMembership.get_memberships_over_limit(user): + UserSessionMembership(user=user, session=session, created=timezone.now()).save() + expired = UserSessionMembership.get_memberships_over_limit(user) + for membership in expired: + Session.objects.filter(session_key__in=[membership.session_id]).delete() + membership.delete() + if len(expired): consumers.emit_channel_notification( - 'control-limit_reached', - dict(group_name='control', - reason=unicode(_('limit_reached')), - session_key=membership.session.session_key) + 'control-limit_reached_{}'.format(user.pk), + dict(group_name='control', reason=unicode(_('limit_reached'))) ) diff --git a/awx/main/tests/functional/test_session.py b/awx/main/tests/functional/test_session.py index 352581ae1e..5ce9bfffd6 100644 --- a/awx/main/tests/functional/test_session.py +++ b/awx/main/tests/functional/test_session.py @@ -1,14 +1,14 @@ +from importlib import import_module import pytest -from datetime import timedelta import re -from django.utils.timezone import now as tz_now +from django.conf import settings from django.test.utils import override_settings from django.contrib.sessions.middleware import SessionMiddleware from django.contrib.sessions.models import Session from django.contrib.auth import SESSION_KEY +import mock -from awx.main.models import UserSessionMembership from awx.api.versioning import reverse @@ -63,33 +63,40 @@ def test_session_create_delete(admin, post, get): assert not Session.objects.filter(session_key=session_key).exists() -@pytest.mark.skip(reason="Needs Update - CA") @pytest.mark.django_db -def test_session_overlimit(admin, post): - AlwaysPassBackend.user = admin - with override_settings( - AUTHENTICATION_BACKENDS=(AlwaysPassBackend.get_backend_path(),), - SESSION_COOKIE_NAME='session_id', SESSIONS_PER_USER=3 - ): - sessions_to_deprecate = [] - for _ in range(5): - response = post( - '/api/login/', - data={'username': admin.username, 'password': admin.password, 'next': '/api/'}, - expect=302, middleware=SessionMiddleware(), format='multipart' - ) - session_key = re.findall( - r'session_id=[a-zA-z0-9]+', - str(response.cookies['session_id']) - )[0][len('session_id=') :] - sessions_to_deprecate.append(Session.objects.get(session_key=session_key)) - sessions_to_deprecate[0].expire_date = tz_now() - timedelta(seconds=1000) - sessions_to_deprecate[0].save() - sessions_overlimit = [x.session for x in UserSessionMembership.get_memberships_over_limit(admin)] - assert sessions_to_deprecate[0] not in sessions_overlimit - assert sessions_to_deprecate[1] in sessions_overlimit - for session in sessions_to_deprecate[2 :]: - assert session not in sessions_overlimit +@mock.patch('awx.main.consumers.emit_channel_notification') +def test_sessions_unlimited(emit, admin): + assert Session.objects.count() == 0 + for i in range(5): + store = import_module(settings.SESSION_ENGINE).SessionStore() + store.create_model_instance({SESSION_KEY: admin.pk}).save() + assert Session.objects.count() == i + 1 + assert emit.call_count == 0 + + +@pytest.mark.django_db +@mock.patch('awx.main.consumers.emit_channel_notification') +def test_session_overlimit(emit, admin, alice): + # If SESSIONS_PER_USER=3, only persist the three most recently created sessions + assert Session.objects.count() == 0 + with override_settings(SESSIONS_PER_USER=3): + created = [] + for i in range(5): + store = import_module(settings.SESSION_ENGINE).SessionStore() + session = store.create_model_instance({SESSION_KEY: admin.pk}) + session.save() + created.append(session.session_key) + assert [s.pk for s in Session.objects.all()] == created[-3:] + assert emit.call_count == 2 # 2 of 5 sessions were evicted + emit.assert_called_with( + 'control-limit_reached_{}'.format(admin.pk), + {'reason': 'limit_reached', 'group_name': 'control'} + ) + + # Allow sessions for a different user to be saved + store = import_module(settings.SESSION_ENGINE).SessionStore() + store.create_model_instance({SESSION_KEY: alice.pk}).save() + assert Session.objects.count() == 4 @pytest.mark.skip(reason="Needs Update - CA") diff --git a/awx/ui/client/src/shared/socket/socket.service.js b/awx/ui/client/src/shared/socket/socket.service.js index f8db80b977..da3d471576 100644 --- a/awx/ui/client/src/shared/socket/socket.service.js +++ b/awx/ui/client/src/shared/socket/socket.service.js @@ -130,13 +130,20 @@ export default else if(data.group_name==="inventory_update_events"){ str = `ws-${data.group_name}-${data.inventory_update}`; } - else if(data.group_name==="control"){ - // As of v. 3.1.0, there is only 1 "control" - // message, which is for expiring the session if the - // session limit is breached. + else if(data.group_name==="control" && data.reason=="limit_reached"){ + // If we got a `limit_reached_` message, determine + // if the current session is still valid (it may have been + // invalidated) + // If so, log the user out and show a meaningful error $log.debug(data.reason); - $rootScope.sessionTimer.expireSession('session_limit'); - $state.go('signOut'); + self.$.ajax({ + url: '/api/v2/me/' + }).error(function(resp, status) { + if (resp.status == 401) { + $rootScope.sessionTimer.expireSession('session_limit'); + $state.go('signOut'); + } + }); } else { // The naming scheme is "ws" then a @@ -158,6 +165,7 @@ export default // listen for specific messages. A subscription object could // look like {"groups":{"jobs": ["status_changed", "summary"]}. // This is used by all socket-enabled $states + state.data.socket.groups['control'] = ['limit_reached_' + $rootScope.current_user.id]; this.emit(JSON.stringify(state.data.socket)); this.setLast(state); }, diff --git a/docs/websockets.md b/docs/websockets.md index ef0bbaedbb..42b456bbc5 100644 --- a/docs/websockets.md +++ b/docs/websockets.md @@ -27,7 +27,7 @@ Once you've connected, you are not subscribed to any event groups. You subscribe 'project_update_events': [ids...], 'inventory_update_events': [ids...], 'system_job_events': [ids...], - 'control': ['limit_reached'], + 'control': ['limit_reached_'], } These map to the event group and event type you are interested in. Sending in a new groups dictionary will clear all of your previously From aa76f6ca39d3d934e42235d7c632a66196b319d4 Mon Sep 17 00:00:00 2001 From: Jared Tabor Date: Wed, 27 Jun 2018 10:45:23 -0700 Subject: [PATCH 2/2] Adjustments to Socket Service for session-limit PR --- .../client/src/shared/socket/socket.service.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/awx/ui/client/src/shared/socket/socket.service.js b/awx/ui/client/src/shared/socket/socket.service.js index da3d471576..afae6fb36f 100644 --- a/awx/ui/client/src/shared/socket/socket.service.js +++ b/awx/ui/client/src/shared/socket/socket.service.js @@ -5,8 +5,8 @@ *************************************************/ import ReconnectingWebSocket from 'reconnectingwebsocket'; export default -['$rootScope', '$location', '$log','$state', '$q', 'i18n', - function ($rootScope, $location, $log, $state, $q, i18n) { +['$rootScope', '$location', '$log','$state', '$q', 'i18n', 'GetBasePath', 'Rest', + function ($rootScope, $location, $log, $state, $q, i18n, GetBasePath, Rest) { var needsResubscribing = false, socketPromise = $q.defer(), needsRefreshAfterBlur; @@ -130,16 +130,16 @@ export default else if(data.group_name==="inventory_update_events"){ str = `ws-${data.group_name}-${data.inventory_update}`; } - else if(data.group_name==="control" && data.reason=="limit_reached"){ + else if(data.group_name === "control" && data.reason === "limit_reached"){ // If we got a `limit_reached_` message, determine // if the current session is still valid (it may have been // invalidated) // If so, log the user out and show a meaningful error $log.debug(data.reason); - self.$.ajax({ - url: '/api/v2/me/' - }).error(function(resp, status) { - if (resp.status == 401) { + let url = GetBasePath('me'); + Rest.get(url) + .catch(function(resp) { + if (resp.status === 401) { $rootScope.sessionTimer.expireSession('session_limit'); $state.go('signOut'); } @@ -165,7 +165,7 @@ export default // listen for specific messages. A subscription object could // look like {"groups":{"jobs": ["status_changed", "summary"]}. // This is used by all socket-enabled $states - state.data.socket.groups['control'] = ['limit_reached_' + $rootScope.current_user.id]; + state.data.socket.groups.control = ['limit_reached_' + $rootScope.current_user.id]; this.emit(JSON.stringify(state.data.socket)); this.setLast(state); },