Merge branch 'master' into hamzawaleed01/upgrade-edx-enterprise-334c0fe

openedx/core/djangoapps/user_authn/views/login_form.py

@@ -187,7 +187,12 @@ def login_and_registration_form(request, initial_mode="login"):
             log.exception("Unknown tpa_hint provider: %s", ex)
 
     # Redirect to authn MFE if it is enabled
-    # except if user is an enterprise user with tpa_hint_provider coming from a SAML IDP.
+    # AND
+    #   user is not an enterprise user
+    # AND
+    #   tpa_hint_provider is not available
+    # AND
+    #   user is not coming from a SAML IDP.
     saml_provider = False
     running_pipeline = pipeline.get(request)
     if running_pipeline:
@@ -197,8 +202,10 @@ def login_and_registration_form(request, initial_mode="login"):
 
     enterprise_customer = enterprise_customer_for_request(request)
 
-    if should_redirect_to_authn_microfrontend() and not \
-            (enterprise_customer and tpa_hint_provider and saml_provider):
+    if should_redirect_to_authn_microfrontend() and \
+            not enterprise_customer and \
+            not tpa_hint_provider and \
+            not saml_provider:
 
         # This is to handle a case where a logged-in cookie is not present but the user is authenticated.
         # Note: If we don't handle this learner is redirected to authn MFE and then back to dashboard

openedx/core/djangoapps/user_authn/views/tests/test_logistration.py

@@ -648,75 +648,6 @@ class LoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMixin, ModuleSto
 
         assert response['Content-Language'] == 'es-es'
 
-    @ddt.data(
-        (None, None, None, True),
-        ({'name': 'Test Enterprise', 'uuid': 'test-uuid'}, None, None, True),
-        ({'name': 'Test Enterprise', 'uuid': 'test-uuid'}, 'test-provider', None, True),
-        ({'name': 'Test Enterprise', 'uuid': 'test-uuid'}, 'test-provider', True, False)
-    )
-    @ddt.unpack
-    @override_settings(FEATURES=FEATURES_WITH_AUTHN_MFE_ENABLED)
-    def test_enterprise_saml_redirection(self, enterprise_customer_data, provider_id, is_saml, should_redirect):
-        """
-        Test that authentication MFE redirection respects the enterprise + SAML provider conditions.
-        In particular, verify that if we have an enterprise customer with a SAML-based tpa_hint_provider,
-        we do NOT redirect to the MFE, but handle the request in LMS. All other combinations should
-        redirect to the MFE when it's enabled.
-        """
-        if provider_id and is_saml:
-            self.enable_saml()
-            self._configure_testshib_provider('TestShib', provider_id)
-
-        with (
-            mock.patch(
-                'openedx.core.djangoapps.user_authn.views.login_form.enterprise_customer_for_request'
-            ) as mock_ec,
-            mock.patch(
-                'openedx.core.djangoapps.user_authn.views.login_form.should_redirect_to_authn_microfrontend'
-            ) as mock_should_redirect,
-            mock.patch(
-                'openedx.core.djangoapps.user_authn.views.login_form.third_party_auth.utils.is_saml_provider'
-            ) as mock_is_saml
-        ):
-            mock_ec.return_value = enterprise_customer_data
-            mock_should_redirect.return_value = should_redirect
-            mock_is_saml.return_value = (True, None) if is_saml else (False, None)
-
-            params = {}
-            if provider_id:
-                params['tpa_hint'] = provider_id
-
-            if provider_id and is_saml:
-                pipeline_target = 'openedx.core.djangoapps.user_authn.views.login_form.third_party_auth.pipeline'
-                with mock.patch(pipeline_target + '.get') as mock_pipeline:
-                    pipeline_data = {
-                        'backend': 'tpa-saml',
-                        'kwargs': {
-                            'response': {
-                                'idp_name': provider_id
-                            },
-                            'details': {
-                                'email': 'test@example.com',
-                                'fullname': 'Test User',
-                                'username': 'testuser'
-                            }
-                        }
-                    }
-                    mock_pipeline.return_value = pipeline_data
-                    response = self.client.get(reverse('signin_user'), params)
-            else:
-                response = self.client.get(reverse('signin_user'), params)
-
-            if should_redirect:
-                self.assertRedirects(
-                    response,
-                    settings.AUTHN_MICROFRONTEND_URL + '/login' +
-                    ('?' + urlencode(params) if params else ''),
-                    fetch_redirect_response=False
-                )
-            else:
-                self.assertEqual(response.status_code, 200)
-
 
 @skip_unless_lms
 class AccountCreationTestCaseWithSiteOverrides(SiteMixin, TestCase):

xmodule/capa/safe_exec/remote_exec.py

@@ -7,7 +7,7 @@ import logging
 from importlib import import_module
 import requests
 
-from codejail.safe_exec import SafeExecException
+from codejail.safe_exec import SafeExecException, json_safe
 from django.conf import settings
 from edx_toggles.toggles import SettingToggle
 from requests.exceptions import RequestException, HTTPError
@@ -90,7 +90,21 @@ def send_safe_exec_request_v0(data):
     extra_files = data.pop("extra_files")
 
     codejail_service_endpoint = get_codejail_rest_service_endpoint()
-    payload = json.dumps(data)
+
+    # In rare cases an XBlock might introduce `bytes` objects (or other
+    # non-JSON-serializable objects) into the globals dict. The codejail service
+    # (via the codejail library) will call `json_safe` on the globals before
+    # JSON-encoding for the sandbox input, but here we need to call it earlier
+    # in the process so we can even transport the globals *to* the codejail
+    # service. Otherwise, we may get a TypeError when constructing the payload.
+    #
+    # This is a lossy operation (non-serializable objects will be dropped, and
+    # bytes converted to strings) but it is the same lossy operation that
+    # codejail will perform anyhow -- and it should be idempotent.
+    data_send = {**data}
+    data_send['globals_dict'] = json_safe(data_send['globals_dict'])
+
+    payload = json.dumps(data_send)
 
     try:
         response = requests.post(

xmodule/capa/safe_exec/tests/test_remote_exec.py

@@ -0,0 +1,32 @@
+"""
+Tests for remote codejail execution.
+"""
+
+import json
+from unittest import TestCase
+from unittest.mock import patch
+
+from django.test import override_settings
+
+from xmodule.capa.safe_exec.remote_exec import get_remote_exec
+
+
+class TestRemoteExec(TestCase):
+    """Tests for remote_exec."""
+
+    @override_settings(
+        ENABLE_CODEJAIL_REST_SERVICE=True,
+        CODE_JAIL_REST_SERVICE_HOST='http://localhost',
+    )
+    @patch('requests.post')
+    def test_json_encode(self, mock_post):
+        get_remote_exec({
+            'code': "out = 1 + 1",
+            'globals_dict': {'some_data': b'bytes', 'unusable': object()},
+            'extra_files': None,
+        })
+
+        mock_post.assert_called_once()
+        data_arg = mock_post.call_args_list[0][1]['data']
+        payload = json.loads(data_arg['payload'])
+        assert payload['globals_dict'] == {'some_data': 'bytes'}