Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #27231 from edx/feanil/more_safe_sessions_updates

Browse Source 
fix: Reduce safe-sessions false alarms.
This commit is contained in:
Feanil Patel
2021-04-02 14:45:22 -04:00
committed by GitHub
parent d2389fb7fb 4f725aa152
commit ba4d98898e
6 changed files with 84 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

49
lms/djangoapps/courseware/tests/test_masquerade.py
View File

@@ -6,11 +6,12 @@ Unit tests for masquerade.
import json
import pickle
from datetime import datetime
from importlib import import_module
from unittest.mock import patch
import pytest
import ddt
from django.conf import settings
from django.test import TestCase
from django.test import TestCase, RequestFactory
from django.urls import reverse
from edx_toggles.toggles.testutils import override_waffle_flag
from pytz import UTC
@@ -18,9 +19,11 @@ from xblock.runtime import DictKeyValueStore
from capa.tests.response_xml_factory import OptionResponseXMLFactory
from lms.djangoapps.courseware.masquerade import (
MASQUERADE_SETTINGS_KEY,
CourseMasquerade,
MasqueradingKeyValueStore,
get_masquerading_user_group
get_masquerading_user_group,
setup_masquerade,
)
from lms.djangoapps.courseware.tests.factories import StaffFactory
from lms.djangoapps.courseware.tests.helpers import LoginEnrollmentTestCase, MasqueradeMixin, masquerade_as_group_member
@@ -535,3 +538,45 @@ class CourseMasqueradeTest(TestCase):
pickled_cmasq = pickle.dumps(cmasq)
unpickled_cmasq = pickle.loads(pickled_cmasq)
assert unpickled_cmasq.user_name is None
class SetupMasqueradeTests(SharedModuleStoreTestCase,):
"""
Tests for the setup_masquerade function.
"""
def setUp(self):
super().setUp()
self.course = CourseFactory.create(number='setup-masquerade-test', metadata={'start': datetime.now(UTC)})
self.request = RequestFactory().request()
self.staff = StaffFactory(course_key=self.course.id)
self.student = UserFactory()
CourseEnrollment.enroll(self.student, self.course.id)
session_key = "abcdef"
self.request.user = self.staff
self.request.session = import_module(settings.SESSION_ENGINE).SessionStore(session_key)
def test_setup_masquerade(self):
masquerade_settings = {
self.course.id: CourseMasquerade(
course_key=self.course.id,
role='student',
user_name=self.student.username
)
}
self.request.session[MASQUERADE_SETTINGS_KEY] = masquerade_settings
course_masquerade, masquerade_user = setup_masquerade(
self.request,
self.course.id,
staff_access=True
)
# Warning: the SafeSessions middleware relies on the `real_user` attribute to see if a
# user is masquerading as another user. If the name of this attribute is changing, please update
# the check in SafeSessionMiddleware._verify_user as well.
assert masquerade_user.real_user == self.staff
assert masquerade_user == self.student
assert self.request.user.masquerade_settings == masquerade_settings
assert course_masquerade == masquerade_settings[self.course.id]

3
lms/djangoapps/support/tests/test_views.py
View File

@@ -81,6 +81,9 @@ class SupportViewManageUserTests(SupportViewTestCase):
"""
Tests password assistance
"""
# Ensure that user is not logged in if they need
# password assistance.
self.client.logout()
url = '/password_assistance'
response = self.client.get(url)
assert response.status_code == 200

18
openedx/core/djangoapps/safe_sessions/middleware.py
View File

@@ -387,21 +387,21 @@ class SafeSessionMiddleware(SessionMiddleware, MiddlewareMixin):
# If a view overrode the request.user with a masqueraded user, this will
# revert/clean-up that change during response processing.
request.user = request.user.real_user
if request.safe_cookie_verified_user_id != request.user.id:
# The user at response time is expected to be None when the user
# is logging out. To prevent extra noise in the logs,
# conditionally set the log level.
log_func = log.debug if request.user.id is None else log.warning
log_func(
# The user at response time is expected to be None when the user
# is logging out. We won't log that.
if request.safe_cookie_verified_user_id != request.user.id and request.user.id is not None:
log.warning(
(
"SafeCookieData user at request '{0}' does not match user at response: '{1}' "
"for request path '{2}'"
).format(
).format( # pylint: disable=logging-format-interpolation
request.safe_cookie_verified_user_id, request.user.id, request.path,
),
)
set_custom_attribute("safe_sessions.user_mismatch", "request-response-mismatch")
if request.safe_cookie_verified_user_id != userid_in_session:
# The user session at response time is expected to be None when the user
# is logging out. We won't log that.
if request.safe_cookie_verified_user_id != userid_in_session and userid_in_session is not None:
log.warning(
(
"SafeCookieData user at request '{0}' does not match user in session: '{1}' "
@@ -535,7 +535,7 @@ def log_request_user_changes(request):
if name == 'user':
stack = inspect.stack()
# Written this way in case you need more of the stack for debugging.
location = "\n".join("%30s : %s:%d" % (t[3], t[1], t[2]) for t in stack[0:6])
location = "\n".join("%30s : %s:%d" % (t[3], t[1], t[2]) for t in stack[0:12])
if not hasattr(request, name):
original_user = value

3
openedx/core/djangoapps/safe_sessions/tests/test_middleware.py
View File

@@ -213,8 +213,7 @@ class TestSafeSessionProcessResponse(TestSafeSessionsLogMixin, TestCase):
self.request.user = AnonymousUser()
self.request.session[SESSION_KEY] = self.user.id
with self.assert_no_error_logged():
with self.assert_logged_for_request_user_mismatch(self.user.id, None, 'debug', self.request.path):
self.assert_response(set_request_user=False, set_session_cookie=True)
self.assert_response(set_request_user=False, set_session_cookie=True)
def test_update_cookie_data_at_step_3(self):
self.assert_response(set_request_user=True, set_session_cookie=True)

12
openedx/core/djangoapps/user_authn/views/login_form.py
View File

@@ -23,7 +23,7 @@ from openedx.core.djangoapps.user_api.accounts.utils import (
is_secondary_email_feature_enabled
)
from openedx.core.djangoapps.user_api.helpers import FormDescription
from openedx.core.djangoapps.user_authn.cookies import are_logged_in_cookies_set
from openedx.core.djangoapps.user_authn.cookies import set_logged_in_cookies
from openedx.core.djangoapps.user_authn.toggles import should_redirect_to_authn_microfrontend
from openedx.core.djangoapps.user_authn.views.password_reset import get_password_reset_form
from openedx.core.djangoapps.user_authn.views.registration_form import RegistrationFormFactory
@@ -150,11 +150,13 @@ def login_and_registration_form(request, initial_mode="login"):
redirect_to = get_next_url_for_login_page(request)
# If we're already logged in, redirect to the dashboard
# Note: We check for the existence of login-related cookies in addition to is_authenticated
# Note: If the session is valid, we update all logged_in cookies(in particular JWTs)
# since Django's SessionAuthentication middleware auto-updates session cookies but not
# the other login-related cookies. See ARCH-282.
if request.user.is_authenticated and are_logged_in_cookies_set(request):
return redirect(redirect_to)
# the other login-related cookies. See ARCH-282 and ARCHBOM-1718
if request.user.is_authenticated:
response = redirect(redirect_to)
response = set_logged_in_cookies(request, response, request.user)
return response
# Retrieve the form descriptions from the user API
form_descriptions = _get_form_descriptions(request)

17
openedx/core/djangoapps/user_authn/views/tests/test_logistration.py
View File

@@ -27,7 +27,9 @@ from common.djangoapps.course_modes.models import CourseMode
from lms.djangoapps.branding.api import get_privacy_url
from openedx.core.djangoapps.site_configuration.tests.mixins import SiteMixin
from openedx.core.djangoapps.theming.tests.test_util import with_comprehensive_theme_context
from openedx.core.djangoapps.user_authn.cookies import JWT_COOKIE_NAMES
from openedx.core.djangoapps.user_authn.toggles import REDIRECT_TO_AUTHN_MICROFRONTEND
from openedx.core.djangoapps.user_authn.tests.utils import setup_login_oauth_client
from openedx.core.djangoapps.user_authn.views.login_form import login_and_registration_form
from openedx.core.djangolib.js_utils import dump_js_escaped_json
from openedx.core.djangolib.markup import HTML, Text
@@ -145,8 +147,10 @@ class LoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMixin, ModuleSto
response = self.client.get(login_url)
assert response.status_code == 200
@mock.patch.dict("django.conf.settings.FEATURES", {"DISABLE_SET_JWT_COOKIES_FOR_TESTS": False})
@ddt.data("signin_user", "register_user")
def test_login_and_registration_form_already_authenticated(self, url_name):
setup_login_oauth_client()
# call the account registration api that sets the login cookies
url = reverse('user_api_registration')
request_data = {
@@ -167,6 +171,19 @@ class LoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMixin, ModuleSto
response = self.client.get(reverse(url_name))
self.assertRedirects(response, reverse("dashboard"))
# Refresh login even if JWT cookies are expired.
# (Give precedence to the session.)
for name in JWT_COOKIE_NAMES:
del self.client.cookies[name]
# Verify that we're still redirected to the dashboard
response = self.client.get(reverse(url_name))
self.assertRedirects(response, reverse("dashboard"))
# Verify that we got new JWT cookies.
for name in JWT_COOKIE_NAMES:
assert name in self.client.cookies
@ddt.data(
(None, "signin_user"),
(None, "register_user"),