diff --git a/horizon/forms/views.py b/horizon/forms/views.py index 8169405d6f..ca337ce6fd 100644 --- a/horizon/forms/views.py +++ b/horizon/forms/views.py @@ -20,6 +20,7 @@ from django import http from django.utils.translation import gettext_lazy as _ from horizon import exceptions +from horizon.utils import http as http_utils from horizon import views @@ -59,7 +60,7 @@ class ModalBackdropMixin(object): class ModalFormMixin(ModalBackdropMixin): def get_template_names(self): - if self.request.is_ajax(): + if http_utils.is_ajax(self.request): if not hasattr(self, "ajax_template_name"): # Transform standard template name to ajax name (leading "_") bits = list(os.path.split(self.template_name)) @@ -74,7 +75,7 @@ class ModalFormMixin(ModalBackdropMixin): def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) - if self.request.is_ajax(): + if http_utils.is_ajax(self.request): context['hide'] = True if ADD_TO_FIELD_HEADER in self.request.META: context['add_to_field'] = self.request.META[ADD_TO_FIELD_HEADER] diff --git a/horizon/messages.py b/horizon/messages.py index 9e07b035ea..ea0c0e9b41 100644 --- a/horizon/messages.py +++ b/horizon/messages.py @@ -22,10 +22,12 @@ from django.contrib.messages import constants from django.utils.encoding import force_str from django.utils.safestring import SafeData +from horizon.utils import http as http_utils + def horizon_message_already_queued(request, message): _message = force_str(message) - if request.is_ajax(): + if http_utils.is_ajax(request): for tag, msg, extra in request.horizon['async_messages']: if _message == msg: return True @@ -39,7 +41,7 @@ def horizon_message_already_queued(request, message): def add_message(request, level, message, extra_tags='', fail_silently=False): """Attempts to add a message to the request using the 'messages' app.""" if not horizon_message_already_queued(request, message): - if request.is_ajax(): + if http_utils.is_ajax(request): tag = constants.DEFAULT_TAGS[level] # if message is marked as safe, pass "safe" tag as extra_tags so # that client can skip HTML escape for the message when rendering diff --git a/horizon/middleware/base.py b/horizon/middleware/base.py index da55707da6..d5a806fecc 100644 --- a/horizon/middleware/base.py +++ b/horizon/middleware/base.py @@ -36,6 +36,7 @@ from django.utils import timezone from horizon import exceptions from horizon.utils import functions as utils +from horizon.utils import http as http_utils LOG = logging.getLogger(__name__) @@ -77,7 +78,7 @@ class HorizonMiddleware(object): session_time = min(timeout, int(token_life.total_seconds())) request.session.set_expiry(session_time) - if request.is_ajax(): + if http_utils.is_ajax(request): # if the request is Ajax we do not want to proceed, as clients can # 1) create pages with constant polling, which can create race # conditions when a page navigation occurs @@ -140,7 +141,7 @@ class HorizonMiddleware(object): return shortcuts.render(request, 'not_authorized.html', status=403) - if request.is_ajax(): + if http_utils.is_ajax(request): response_401 = http.HttpResponse(status=401) response_401['X-Horizon-Location'] = response['location'] return response_401 @@ -166,7 +167,7 @@ class HorizonMiddleware(object): This is to allow ajax request to redirect url. """ - if request.is_ajax() and hasattr(request, 'horizon'): + if http_utils.is_ajax(request) and hasattr(request, 'horizon'): queued_msgs = request.horizon['async_messages'] if type(response) == http.HttpResponseRedirect: # Drop our messages back into the session as per usual so they diff --git a/horizon/tables/base.py b/horizon/tables/base.py index 8de69623cd..4427d5383f 100644 --- a/horizon/tables/base.py +++ b/horizon/tables/base.py @@ -46,6 +46,7 @@ from horizon.tables.actions import BatchAction from horizon.tables.actions import FilterAction from horizon.tables.actions import LinkAction from horizon.utils import html +from horizon.utils import http as http_utils from horizon.utils import settings as utils_settings @@ -1684,7 +1685,7 @@ class DataTable(object, metaclass=DataTableMetaclass): except Exception: datum = None error = exceptions.handle(request, ignore=True) - if request.is_ajax(): + if http_utils.is_ajax(request): if not error: return HttpResponse(new_row.render()) return HttpResponse(status=error.status_code) @@ -1744,7 +1745,7 @@ class DataTable(object, metaclass=DataTableMetaclass): except Exception: datum = None error = exceptions.handle(request, ignore=True) - if request.is_ajax(): + if http_utils.is_ajax(request): if not error: return HttpResponse(cell.render()) return HttpResponse(status=error.status_code) diff --git a/horizon/tabs/views.py b/horizon/tabs/views.py index 7dc93cb039..99f2ae2b17 100644 --- a/horizon/tabs/views.py +++ b/horizon/tabs/views.py @@ -15,6 +15,7 @@ from django import http from horizon import exceptions from horizon import tables from horizon.tabs.base import TableTab +from horizon.utils import http as http_utils from horizon import views @@ -60,7 +61,7 @@ class TabView(views.HorizonTemplateView): Otherwise renders the response as normal. """ - if self.request.is_ajax(): + if http_utils.is_ajax(self.request): if tab_group.selected: return http.HttpResponse(tab_group.selected.render()) return http.HttpResponse(tab_group.render()) diff --git a/horizon/utils/http.py b/horizon/utils/http.py new file mode 100644 index 0000000000..fbfa3b05ff --- /dev/null +++ b/horizon/utils/http.py @@ -0,0 +1,28 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + + +def is_ajax(request): + """Check if the request is AJAX-based. + + :param request: django.http.HttpRequest object + :return: True if the request is AJAX-based. + """ + # NOTE: Django 3.1 or later deprecates request.is_ajax() as it relied + # on a jQuery-specific way of signifying AJAX calls, + # but at the moment checking X-Requested-With header works in horizon. + # If we adopt modern frameworks with JavaScript Fetch API, + # we need to consider checking Accepts header as suggested in the + # Django 3.1 release notes. + # https://docs.djangoproject.com/en/4.0/releases/3.1/#id2 + # https://docs.djangoproject.com/en/3.1/ref/request-response/#django.http.HttpRequest.is_ajax + return request.headers.get('x-requested-with') == 'XMLHttpRequest' diff --git a/horizon/workflows/base.py b/horizon/workflows/base.py index 4a553ead19..69003e26e8 100644 --- a/horizon/workflows/base.py +++ b/horizon/workflows/base.py @@ -35,6 +35,7 @@ from horizon import base from horizon import exceptions from horizon.templatetags.horizon import has_permissions from horizon.utils import html +from horizon.utils import http as http_utils LOG = logging.getLogger(__name__) @@ -904,7 +905,7 @@ class Workflow(html.HTMLElement, metaclass=WorkflowMetaclass): """Renders the workflow.""" workflow_template = template.loader.get_template(self.template_name) extra_context = {"workflow": self} - if self.request.is_ajax(): + if http_utils.is_ajax(self.request): extra_context['modal'] = True return workflow_template.render(extra_context, self.request) diff --git a/horizon/workflows/views.py b/horizon/workflows/views.py index 365de3f0f3..99eb1dac04 100644 --- a/horizon/workflows/views.py +++ b/horizon/workflows/views.py @@ -25,6 +25,7 @@ from horizon import exceptions from horizon.forms import views as hz_views from horizon.forms.views import ADD_TO_FIELD_HEADER from horizon import messages +from horizon.utils import http as http_utils class WorkflowView(hz_views.ModalBackdropMixin, generic.TemplateView): @@ -115,7 +116,7 @@ class WorkflowView(hz_views.ModalBackdropMixin, generic.TemplateView): The returned classes are determied based on the workflow characteristics. """ - if self.request.is_ajax(): + if http_utils.is_ajax(self.request): layout = ['modal', ] else: layout = ['static_page', ] @@ -127,7 +128,7 @@ class WorkflowView(hz_views.ModalBackdropMixin, generic.TemplateView): def get_template_names(self): """Returns the template name to use for this request.""" - if self.request.is_ajax(): + if http_utils.is_ajax(self.request): template = self.ajax_template_name else: template = self.template_name diff --git a/openstack_auth/views.py b/openstack_auth/views.py index a342478744..3013289505 100644 --- a/openstack_auth/views.py +++ b/openstack_auth/views.py @@ -71,6 +71,13 @@ def set_logout_reason(res, msg): res.set_cookie('logout_reason', msg, max_age=10) +def is_ajax(request): + # See horizon.utils.http.is_ajax() for more detail. + # NOTE: openstack_auth does not import modules from horizon to avoid + # import loops, so we copy the logic from horizon.utils.http. + return request.headers.get('x-requested-with') == 'XMLHttpRequest' + + # TODO(stephenfin): Migrate to CBV @sensitive_post_parameters() @csrf_protect @@ -102,7 +109,7 @@ def login(request): url = utils.get_websso_url(request, auth_url, auth_type) return shortcuts.redirect(url) - if not request.is_ajax(): + if not is_ajax(request): # If the user is already authenticated, redirect them to the # dashboard straight away, unless the 'next' parameter is set as it # usually indicates requesting access to a page that requires different @@ -143,7 +150,7 @@ def login(request): 'logout_status': logout_status, } - if request.is_ajax(): + if is_ajax(request): template_name = 'auth/_login.html' extra_context['hide'] = True else: diff --git a/openstack_dashboard/api/rest/utils.py b/openstack_dashboard/api/rest/utils.py index 8073ffa4a0..5eb067999e 100644 --- a/openstack_dashboard/api/rest/utils.py +++ b/openstack_dashboard/api/rest/utils.py @@ -21,6 +21,7 @@ from django import http from oslo_serialization import jsonutils from horizon import exceptions +from horizon.utils import http as http_utils LOG = logging.getLogger(__name__) @@ -107,7 +108,7 @@ def ajax(authenticated=True, data_required=False, def _wrapped(self, request, *args, **kw): if authenticated and not request.user.is_authenticated: return JSONResponse('not logged in', 401) - if not request.is_ajax(): + if not http_utils.is_ajax(request): return JSONResponse('request must be AJAX', 400) # decode the JSON body if present diff --git a/openstack_dashboard/dashboards/project/volumes/views.py b/openstack_dashboard/dashboards/project/volumes/views.py index 8ac0f164b8..d1a2027d20 100644 --- a/openstack_dashboard/dashboards/project/volumes/views.py +++ b/openstack_dashboard/dashboards/project/volumes/views.py @@ -33,6 +33,7 @@ from horizon import exceptions from horizon import forms from horizon import tables from horizon import tabs +from horizon.utils import http as http_utils from horizon.utils import memoized from openstack_dashboard.api import cinder @@ -566,7 +567,7 @@ class EditAttachmentsView(tables.DataTableView, forms.ModalFormView): else: context['show_attach'] = False context['volume'] = volume - if self.request.is_ajax(): + if http_utils.is_ajax(self.request): context['hide'] = True return context diff --git a/openstack_dashboard/test/helpers.py b/openstack_dashboard/test/helpers.py index 5f0d72926d..9fcc49ff45 100644 --- a/openstack_dashboard/test/helpers.py +++ b/openstack_dashboard/test/helpers.py @@ -389,7 +389,6 @@ class TestCase(horizon_helpers.TestCase): def mock_rest_request(**args): mock_args = { 'user.is_authenticated': True, - 'is_ajax.return_value': True, 'policy.check.return_value': True, 'body': '' } @@ -483,6 +482,12 @@ class APITestCase(TestCase): utils.patch_middleware_get_user() +class RestAPITestCase(TestCase): + def setUp(self): + super().setUp() + mock.patch('horizon.utils.http.is_ajax', return_value=True).start() + + # APIMockTestCase was introduced to support mox to mock migration smoothly # but it turns we have still users of APITestCase. # We keep both for a while. diff --git a/openstack_dashboard/test/unit/api/rest/test_cinder.py b/openstack_dashboard/test/unit/api/rest/test_cinder.py index de6d28975f..030cd29632 100644 --- a/openstack_dashboard/test/unit/api/rest/test_cinder.py +++ b/openstack_dashboard/test/unit/api/rest/test_cinder.py @@ -20,7 +20,7 @@ from openstack_dashboard.test import helpers as test from openstack_dashboard.usage import quotas -class CinderRestTestCase(test.TestCase): +class CinderRestTestCase(test.RestAPITestCase): # # Volumes diff --git a/openstack_dashboard/test/unit/api/rest/test_config.py b/openstack_dashboard/test/unit/api/rest/test_config.py index c20054db5d..230dc211a1 100644 --- a/openstack_dashboard/test/unit/api/rest/test_config.py +++ b/openstack_dashboard/test/unit/api/rest/test_config.py @@ -18,7 +18,7 @@ from openstack_dashboard import api from openstack_dashboard.test import helpers as test -class ConfigRestTestCase(test.TestCase): +class ConfigRestTestCase(test.RestAPITestCase): @mock.patch.object(api.glance, 'get_image_schemas') def test_settings_config_get(self, mock_schemas_list): diff --git a/openstack_dashboard/test/unit/api/rest/test_glance.py b/openstack_dashboard/test/unit/api/rest/test_glance.py index b79ebbdbac..c36a8a31ca 100644 --- a/openstack_dashboard/test/unit/api/rest/test_glance.py +++ b/openstack_dashboard/test/unit/api/rest/test_glance.py @@ -19,7 +19,7 @@ from openstack_dashboard.api.rest import glance from openstack_dashboard.test import helpers as test -class ImagesRestTestCase(test.ResetImageAPIVersionMixin, test.TestCase): +class ImagesRestTestCase(test.ResetImageAPIVersionMixin, test.RestAPITestCase): def setUp(self): super().setUp() diff --git a/openstack_dashboard/test/unit/api/rest/test_keystone.py b/openstack_dashboard/test/unit/api/rest/test_keystone.py index a786fd8513..f9eb121b6e 100644 --- a/openstack_dashboard/test/unit/api/rest/test_keystone.py +++ b/openstack_dashboard/test/unit/api/rest/test_keystone.py @@ -22,7 +22,7 @@ from openstack_dashboard.api.rest import keystone from openstack_dashboard.test import helpers as test -class KeystoneRestTestCase(test.TestCase): +class KeystoneRestTestCase(test.RestAPITestCase): # # Version diff --git a/openstack_dashboard/test/unit/api/rest/test_network.py b/openstack_dashboard/test/unit/api/rest/test_network.py index 4b2b9f6b61..9482854e38 100644 --- a/openstack_dashboard/test/unit/api/rest/test_network.py +++ b/openstack_dashboard/test/unit/api/rest/test_network.py @@ -18,7 +18,7 @@ from openstack_dashboard.api.rest import network from openstack_dashboard.test import helpers as test -class RestNetworkApiSecurityGroupTests(test.TestCase): +class RestNetworkApiSecurityGroupTests(test.RestAPITestCase): @test.create_mocks({api.neutron: ['security_group_list']}) def test_security_group_detailed(self): @@ -34,7 +34,7 @@ class RestNetworkApiSecurityGroupTests(test.TestCase): self.mock_security_group_list.assert_called_once_with(request) -class RestNetworkApiFloatingIpTests(test.TestCase): +class RestNetworkApiFloatingIpTests(test.RestAPITestCase): @test.create_mocks({api.neutron: ['tenant_floating_ip_list']}) def test_floating_ip_list(self): diff --git a/openstack_dashboard/test/unit/api/rest/test_neutron.py b/openstack_dashboard/test/unit/api/rest/test_neutron.py index 5c56eb7f25..5969e4712d 100644 --- a/openstack_dashboard/test/unit/api/rest/test_neutron.py +++ b/openstack_dashboard/test/unit/api/rest/test_neutron.py @@ -24,7 +24,7 @@ from openstack_dashboard.test import helpers as test from openstack_dashboard.usage import quotas -class NeutronNetworksTestCase(test.TestCase): +class NeutronNetworksTestCase(test.RestAPITestCase): def _dictify_network(self, network): net_dict = network.to_dict() @@ -109,7 +109,7 @@ class NeutronNetworksTestCase(test.TestCase): mock_is_service_enabled.assert_called_once_with(request, 'network') -class NeutronSubnetsTestCase(test.TestCase): +class NeutronSubnetsTestCase(test.RestAPITestCase): @mock.patch.object(api.neutron, 'subnet_list') def test_get(self, mock_subnet_list): @@ -141,7 +141,7 @@ class NeutronSubnetsTestCase(test.TestCase): network_id=network_id) -class NeutronPortsTestCase(test.TestCase): +class NeutronPortsTestCase(test.RestAPITestCase): @mock.patch.object(api.neutron, 'port_list_with_trunk_types') def test_get(self, mock_port_list_with_trunk_types): @@ -155,7 +155,7 @@ class NeutronPortsTestCase(test.TestCase): request, network_id=network_id) -class NeutronTrunkTestCase(test.TestCase): +class NeutronTrunkTestCase(test.RestAPITestCase): @mock.patch.object(api.neutron, 'trunk_delete') def test_trunk_delete(self, mock_trunk_delete): @@ -189,7 +189,7 @@ class NeutronTrunkTestCase(test.TestCase): ) -class NeutronTrunksTestCase(test.TestCase): +class NeutronTrunksTestCase(test.RestAPITestCase): @mock.patch.object(api.neutron, 'trunk_list') def test_trunks_get(self, mock_trunk_list): @@ -216,7 +216,7 @@ class NeutronTrunksTestCase(test.TestCase): port_id='1') -class NeutronExtensionsTestCase(test.TestCase): +class NeutronExtensionsTestCase(test.RestAPITestCase): @mock.patch.object(api.neutron, 'list_extensions') def test_list_extensions(self, mock_list_extensions): @@ -228,7 +228,7 @@ class NeutronExtensionsTestCase(test.TestCase): mock_list_extensions.assert_called_once_with(request) -class NeutronDefaultQuotasTestCase(test.TestCase): +class NeutronDefaultQuotasTestCase(test.RestAPITestCase): @test.create_mocks({api.base: ['is_service_enabled'], api.neutron: ['tenant_quota_get']}) @@ -268,7 +268,7 @@ class NeutronDefaultQuotasTestCase(test.TestCase): mock_is_service_enabled.assert_called_once_with(request, 'network') -class NeutronQuotaSetsTestCase(test.TestCase): +class NeutronQuotaSetsTestCase(test.RestAPITestCase): @test.create_mocks({api.base: ['is_service_enabled'], api.neutron: ['is_extension_supported', diff --git a/openstack_dashboard/test/unit/api/rest/test_nova.py b/openstack_dashboard/test/unit/api/rest/test_nova.py index 43feb97122..a7947c7193 100644 --- a/openstack_dashboard/test/unit/api/rest/test_nova.py +++ b/openstack_dashboard/test/unit/api/rest/test_nova.py @@ -39,7 +39,7 @@ class FakeFlavor(object): return {"id": self.id} -class NovaRestTestCase(test.TestCase): +class NovaRestTestCase(test.RestAPITestCase): # # Snapshots diff --git a/openstack_dashboard/test/unit/api/rest/test_policy.py b/openstack_dashboard/test/unit/api/rest/test_policy.py index 7896fe34a2..eb5b678742 100644 --- a/openstack_dashboard/test/unit/api/rest/test_policy.py +++ b/openstack_dashboard/test/unit/api/rest/test_policy.py @@ -11,6 +11,7 @@ # limitations under the License. import json +from unittest import mock from django.test.utils import override_settings @@ -18,7 +19,7 @@ from openstack_dashboard.api.rest import policy from openstack_dashboard.test import helpers as test -class PolicyRestTestCase(test.TestCase): +class PolicyRestTestCase(test.RestAPITestCase): @override_settings(POLICY_CHECK_FUNCTION='openstack_auth.policy.check') def _test_policy(self, body, expected=True): @@ -77,6 +78,14 @@ class PolicyRestTestCase(test.TestCase): class AdminPolicyRestTestCase(test.BaseAdminViewTests): + + # NOTE: BaseAdminViewTests is used by other unit tests too, + # so mock for is_ajax() is prepared explicitly here. + # It should match horizon.test.helpers.RestAPITestCase.setUp(). + def setUp(self): + super().setUp() + mock.patch('horizon.utils.http.is_ajax', return_value=True).start() + @override_settings(POLICY_CHECK_FUNCTION='openstack_auth.policy.check') def test_rule_with_target(self): body = json.dumps( diff --git a/openstack_dashboard/test/unit/api/rest/test_swift.py b/openstack_dashboard/test/unit/api/rest/test_swift.py index c0c440216a..cf997e778f 100644 --- a/openstack_dashboard/test/unit/api/rest/test_swift.py +++ b/openstack_dashboard/test/unit/api/rest/test_swift.py @@ -18,7 +18,7 @@ from openstack_dashboard.api.rest import swift from openstack_dashboard.test import helpers as test -class SwiftRestTestCase(test.TestCase): +class SwiftRestTestCase(test.RestAPITestCase): # # Version diff --git a/openstack_dashboard/test/unit/api/rest/test_utils.py b/openstack_dashboard/test/unit/api/rest/test_utils.py index a5920d3711..2d96223e27 100644 --- a/openstack_dashboard/test/unit/api/rest/test_utils.py +++ b/openstack_dashboard/test/unit/api/rest/test_utils.py @@ -16,7 +16,7 @@ from openstack_dashboard.api.rest import utils from openstack_dashboard.test import helpers as test -class RestUtilsTestCase(test.TestCase): +class RestUtilsTestCase(test.RestAPITestCase): def test_api_success(self): @utils.ajax() @@ -164,7 +164,7 @@ class RestUtilsTestCase(test.TestCase): self.assertDictEqual({}, output_filters) -class JSONEncoderTestCase(test.TestCase): +class JSONEncoderTestCase(test.RestAPITestCase): # NOTE(tsufiev): NaN numeric is "conventional" in a sense that the custom # NaNJSONEncoder encoder translates it to the same token that the standard