From c94ebba0b3eec20555cd1582124cef144e85b43f Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Fri, 27 Sep 2019 10:10:03 -0400 Subject: [PATCH 1/3] Saving user session checks if User exists - Check that model User object exists with id=user_id before attempting to save to database - UserSessionMembership saves to the database using foreign key, User - However, User with matching id might not exist if browser sends request with stale cookies - Change made in regards to issue #4334 --- awx/main/signals.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/awx/main/signals.py b/awx/main/signals.py index 9846b11dd3..157db6515b 100644 --- a/awx/main/signals.py +++ b/awx/main/signals.py @@ -20,6 +20,7 @@ from django.db.models.signals import ( ) from django.dispatch import receiver from django.contrib.auth import SESSION_KEY +from django.contrib.auth.models import User from django.contrib.sessions.models import Session from django.utils import timezone @@ -684,7 +685,8 @@ def save_user_session_membership(sender, **kwargs): return if UserSessionMembership.objects.filter(user=user_id, session=session).exists(): return - UserSessionMembership(user_id=user_id, session=session, created=timezone.now()).save() + if User.objects.filter(id=int(user_id)).exists(): + UserSessionMembership(user_id=user_id, session=session, created=timezone.now()).save() expired = UserSessionMembership.get_memberships_over_limit(user_id) for membership in expired: Session.objects.filter(session_key__in=[membership.session_id]).delete() From 94fa745859e40270b4c672c5d39dc443c8c4159c Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Fri, 27 Sep 2019 11:47:44 -0400 Subject: [PATCH 2/3] removed duplicate import of User --- awx/main/signals.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx/main/signals.py b/awx/main/signals.py index 157db6515b..bbe75281d4 100644 --- a/awx/main/signals.py +++ b/awx/main/signals.py @@ -20,7 +20,6 @@ from django.db.models.signals import ( ) from django.dispatch import receiver from django.contrib.auth import SESSION_KEY -from django.contrib.auth.models import User from django.contrib.sessions.models import Session from django.utils import timezone @@ -685,6 +684,7 @@ def save_user_session_membership(sender, **kwargs): return if UserSessionMembership.objects.filter(user=user_id, session=session).exists(): return + # check if user_id from session has an id match in User before saving if User.objects.filter(id=int(user_id)).exists(): UserSessionMembership(user_id=user_id, session=session, created=timezone.now()).save() expired = UserSessionMembership.get_memberships_over_limit(user_id) From b6ffde75ef298e60b8c4696b25c49cc08d030aab Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Fri, 27 Sep 2019 12:31:10 -0400 Subject: [PATCH 3/3] check expired sessions only if User exists - Indent rest of code into the conditional that checks for expired sessions of that User - If user doesn't exist, no need to check --- awx/main/signals.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/awx/main/signals.py b/awx/main/signals.py index bbe75281d4..accd00e367 100644 --- a/awx/main/signals.py +++ b/awx/main/signals.py @@ -687,15 +687,15 @@ def save_user_session_membership(sender, **kwargs): # check if user_id from session has an id match in User before saving if User.objects.filter(id=int(user_id)).exists(): UserSessionMembership(user_id=user_id, session=session, created=timezone.now()).save() - expired = UserSessionMembership.get_memberships_over_limit(user_id) - 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_{}'.format(user_id), - dict(group_name='control', reason='limit_reached') - ) + expired = UserSessionMembership.get_memberships_over_limit(user_id) + 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_{}'.format(user_id), + dict(group_name='control', reason='limit_reached') + ) @receiver(post_save, sender=OAuth2AccessToken)