From f2be4de5443d37801554f67b5f6d70ea13db27a0 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Tue, 26 Mar 2019 17:22:16 -0400 Subject: [PATCH 1/2] Use Django's own logic to invalidate sessions of users when changing passwords The key is django.contrib.auth.update_session_auth_hash(), which knows how to inject a recalculated session hash back into the session if the requesting user is changing their own password, in order to keep that user logged in. --- awx/api/serializers.py | 9 +++++++-- awx/main/management/commands/expire_sessions.py | 6 +++--- awx/main/middleware.py | 4 ++-- awx/main/models/organization.py | 6 ------ awx/main/tests/functional/api/test_user.py | 16 ++++++++++------ 5 files changed, 22 insertions(+), 19 deletions(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index a571190199..975e5f87b8 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -16,6 +16,7 @@ from oauthlib.common import generate_token # Django from django.conf import settings +from django.contrib.auth import update_session_auth_hash from django.contrib.auth.models import User from django.contrib.contenttypes.models import ContentType from django.core.exceptions import ObjectDoesNotExist, ValidationError as DjangoValidationError @@ -933,8 +934,12 @@ class UserSerializer(BaseSerializer): if new_password: obj.set_password(new_password) obj.save(update_fields=['password']) - if self.context['request'].user != obj: - UserSessionMembership.clear_session_for_user(obj) + + # Cycle the session key, but if the requesting user is the same + # as the modified user then inject a session key derived from + # the updated user to prevent logout. This is the logic used by + # the Django admin's own user_change_password view. + update_session_auth_hash(self.context['request'], obj) elif not obj.password: obj.set_unusable_password() obj.save(update_fields=['password']) diff --git a/awx/main/management/commands/expire_sessions.py b/awx/main/management/commands/expire_sessions.py index 2053b483f6..a0275eb1ea 100644 --- a/awx/main/management/commands/expire_sessions.py +++ b/awx/main/management/commands/expire_sessions.py @@ -29,9 +29,9 @@ class Command(BaseCommand): # with consideration for timezones. start = timezone.now() sessions = Session.objects.filter(expire_date__gte=start).iterator() - request = HttpRequest() for session in sessions: user_id = session.get_decoded().get('_auth_user_id') if (user is None) or (user_id and user.id == int(user_id)): - request.session = import_module(settings.SESSION_ENGINE).SessionStore(session.session_key) - logout(request) + session = import_module(settings.SESSION_ENGINE).SessionStore(session.session_key) + # Log out the session, but without the need for a request object. + session.flush() diff --git a/awx/main/middleware.py b/awx/main/middleware.py index ed1b867c80..e2dcd9c1da 100644 --- a/awx/main/middleware.py +++ b/awx/main/middleware.py @@ -127,8 +127,8 @@ class SessionTimeoutMiddleware(object): def process_response(self, request, response): should_skip = 'HTTP_X_WS_SESSION_QUIET' in request.META - req_session = getattr(request, 'session', None) - if req_session and not req_session.is_empty() and should_skip is False: + # Only update the session if it hasn't been flushed by being forced to log out. + if request.session and not request.session.is_empty() and not should_skip: expiry = int(settings.SESSION_COOKIE_AGE) request.session.set_expiry(expiry) response['Session-Timeout'] = expiry diff --git a/awx/main/models/organization.py b/awx/main/models/organization.py index ac7b41bda9..f8aacfb995 100644 --- a/awx/main/models/organization.py +++ b/awx/main/models/organization.py @@ -183,12 +183,6 @@ class UserSessionMembership(BaseModel): non_expire_memberships = [x for x in query_set if x.session.expire_date > now] return non_expire_memberships[settings.SESSIONS_PER_USER:] - @staticmethod - def clear_session_for_user(user): - query_set = UserSessionMembership.objects.select_related('session').filter(user=user) - sessions_to_delete = [obj.session.pk for obj in query_set] - Session.objects.filter(pk__in=sessions_to_delete).delete() - # Add get_absolute_url method to User model if not present. if not hasattr(User, 'get_absolute_url'): diff --git a/awx/main/tests/functional/api/test_user.py b/awx/main/tests/functional/api/test_user.py index 80fe4783b2..b9653503fe 100644 --- a/awx/main/tests/functional/api/test_user.py +++ b/awx/main/tests/functional/api/test_user.py @@ -1,5 +1,8 @@ import pytest +from django.contrib.sessions.middleware import SessionMiddleware +from django.test import Client + from awx.api.versioning import reverse @@ -19,7 +22,7 @@ EXAMPLE_USER_DATA = { @pytest.mark.django_db def test_user_create(post, admin): - response = post(reverse('api:user_list'), EXAMPLE_USER_DATA, admin) + response = post(reverse('api:user_list'), EXAMPLE_USER_DATA, admin, middleware=SessionMiddleware()) assert response.status_code == 201 assert not response.data['is_superuser'] assert not response.data['is_system_auditor'] @@ -27,21 +30,22 @@ def test_user_create(post, admin): @pytest.mark.django_db def test_fail_double_create_user(post, admin): - response = post(reverse('api:user_list'), EXAMPLE_USER_DATA, admin) + response = post(reverse('api:user_list'), EXAMPLE_USER_DATA, admin, middleware=SessionMiddleware()) assert response.status_code == 201 - response = post(reverse('api:user_list'), EXAMPLE_USER_DATA, admin) + response = post(reverse('api:user_list'), EXAMPLE_USER_DATA, admin, middleware=SessionMiddleware()) assert response.status_code == 400 @pytest.mark.django_db def test_create_delete_create_user(post, delete, admin): - response = post(reverse('api:user_list'), EXAMPLE_USER_DATA, admin) + response = post(reverse('api:user_list'), EXAMPLE_USER_DATA, admin, middleware=SessionMiddleware()) assert response.status_code == 201 - response = delete(reverse('api:user_detail', kwargs={'pk': response.data['id']}), admin) + response = delete(reverse('api:user_detail', kwargs={'pk': response.data['id']}), admin, + middleware=SessionMiddleware()) assert response.status_code == 204 - response = post(reverse('api:user_list'), EXAMPLE_USER_DATA, admin) + response = post(reverse('api:user_list'), EXAMPLE_USER_DATA, admin, middleware=SessionMiddleware()) print(response.data) assert response.status_code == 201 From efb4fb6fd0d6a49c35a186375a1652c1442f0dc5 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Wed, 27 Mar 2019 09:53:44 -0400 Subject: [PATCH 2/2] Remove some no longer used imports --- awx/api/serializers.py | 10 +++++----- awx/main/management/commands/expire_sessions.py | 2 -- awx/main/tests/functional/api/test_user.py | 1 - 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 975e5f87b8..297ba85e83 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -51,11 +51,11 @@ from awx.main.models import ( CredentialType, CustomInventoryScript, Fact, Group, Host, Instance, InstanceGroup, Inventory, InventorySource, InventoryUpdate, InventoryUpdateEvent, Job, JobEvent, JobHostSummary, JobLaunchConfig, - JobTemplate, Label, Notification, NotificationTemplate, OAuth2AccessToken, - OAuth2Application, Organization, Project, ProjectUpdate, - ProjectUpdateEvent, RefreshToken, Role, Schedule, SystemJob, - SystemJobEvent, SystemJobTemplate, Team, UnifiedJob, UnifiedJobTemplate, - UserSessionMembership, V1Credential, WorkflowJob, WorkflowJobNode, + JobTemplate, Label, Notification, NotificationTemplate, + OAuth2AccessToken, OAuth2Application, Organization, Project, + ProjectUpdate, ProjectUpdateEvent, RefreshToken, Role, Schedule, + SystemJob, SystemJobEvent, SystemJobTemplate, Team, UnifiedJob, + UnifiedJobTemplate, V1Credential, WorkflowJob, WorkflowJobNode, WorkflowJobTemplate, WorkflowJobTemplateNode, StdoutMaxBytesExceeded ) from awx.main.models.base import VERBOSITY_CHOICES, NEW_JOB_TYPE_CHOICES diff --git a/awx/main/management/commands/expire_sessions.py b/awx/main/management/commands/expire_sessions.py index a0275eb1ea..04abd492db 100644 --- a/awx/main/management/commands/expire_sessions.py +++ b/awx/main/management/commands/expire_sessions.py @@ -4,8 +4,6 @@ from importlib import import_module # Django from django.utils import timezone from django.conf import settings -from django.contrib.auth import logout -from django.http import HttpRequest from django.core.management.base import BaseCommand, CommandError from django.contrib.auth.models import User from django.contrib.sessions.models import Session diff --git a/awx/main/tests/functional/api/test_user.py b/awx/main/tests/functional/api/test_user.py index b9653503fe..f0a4ffea84 100644 --- a/awx/main/tests/functional/api/test_user.py +++ b/awx/main/tests/functional/api/test_user.py @@ -1,7 +1,6 @@ import pytest from django.contrib.sessions.middleware import SessionMiddleware -from django.test import Client from awx.api.versioning import reverse