Merge pull request #2332 from ryanpetrello/fix-session-limit

make settings.SESSIONS_PER_USER work

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')))
         )
 
 

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")

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,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_<user_pk>` 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');
+                    let url = GetBasePath('me'); 
+                    Rest.get(url)
+                    .catch(function(resp) {
+                        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);
             },

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_<user_id>'],
     }
 
 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