From 8851866aad5b3826f0a3d569405707bffa593c80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathieu=20Gagne=CC=81?= Date: Wed, 31 Oct 2018 22:24:31 -0400 Subject: [PATCH] Fix django.contrib.auth.middleware monkey patching The "request" attribute is not available in openstack_auth.backend.KeystoneBackend.get_user when session data is restored and it's the first request to happen after a server restart. As stated by the function document, the "request" attribute needs to be monkey-patched by openstack_auth.utils.patch_middleware_get_user for this function to work properly. This should happen in openstack_auth.urls at import time. But there is nowhere in Horizon where this module is imported at startup. It's only introspected by openstack_dashboard.urls due to AUTHENTICATION_URLS setting. Without this monkey-patching, the whole authentication mechanism falls back to "AnonymousUser" and you will get redirected to the login page due to horizon.exceptions.NotAuthenticated being raised by horizon.decorators.require_auth as request.user.is_authenticated will be False. But if a user requests a page under auth/, it will have the side-effect of monkey-patching django.contrib.auth.middleware as expected. This means that once this request is completed, all following requests to pages other than the ones under auth/ will have there sessions properly restored and you will be properly authenticated. Therefore this change introduces a dummy middleware which sole purpose is to perform this monkey-patching as early as possible. There is also some cleanup to get rid of the previous attempts at monkeypatching. Closes-bug: #1764622 Conflicts: openstack_dashboard/settings.py openstack_dashboard/test/helpers.py Change-Id: Ib9912090a87b716e7f5710f6f360b0df168ec2e3 (cherry picked from commit 0d163613265e036818fe567793a4fc88fe140d4a) --- horizon/test/settings.py | 1 + openstack_auth/middleware.py | 25 +++++++++++++++++++++++++ openstack_auth/tests/settings.py | 1 + openstack_auth/tests/urls.py | 4 ---- openstack_auth/urls.py | 2 -- openstack_auth/utils.py | 4 ++-- openstack_dashboard/settings.py | 1 + openstack_dashboard/test/helpers.py | 10 ++++------ 8 files changed, 34 insertions(+), 14 deletions(-) create mode 100644 openstack_auth/middleware.py diff --git a/horizon/test/settings.py b/horizon/test/settings.py index bb1f02b207..a48d7e73e4 100644 --- a/horizon/test/settings.py +++ b/horizon/test/settings.py @@ -60,6 +60,7 @@ INSTALLED_APPS = ( ) MIDDLEWARE = ( + 'openstack_auth.middleware.OpenstackAuthMonkeyPatchMiddleware', 'django.middleware.common.CommonMiddleware', 'django.middleware.csrf.CsrfViewMiddleware', 'django.contrib.sessions.middleware.SessionMiddleware', diff --git a/openstack_auth/middleware.py b/openstack_auth/middleware.py new file mode 100644 index 0000000000..b67a54e5e2 --- /dev/null +++ b/openstack_auth/middleware.py @@ -0,0 +1,25 @@ +# 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. + +from openstack_auth import utils + +# NOTE: The main role of this middleware is to call this. +utils.patch_middleware_get_user() + + +class OpenstackAuthMonkeyPatchMiddleware(object): + def __init__(self, get_response): + self.get_response = get_response + + def __call__(self, request): + # Do nothing actually + return self.get_response(request) diff --git a/openstack_auth/tests/settings.py b/openstack_auth/tests/settings.py index 41c9041944..22e272e274 100644 --- a/openstack_auth/tests/settings.py +++ b/openstack_auth/tests/settings.py @@ -28,6 +28,7 @@ INSTALLED_APPS = [ ] MIDDLEWARE = [ + 'openstack_auth.middleware.OpenstackAuthMonkeyPatchMiddleware', 'django.middleware.common.CommonMiddleware', 'django.middleware.csrf.CsrfViewMiddleware', 'django.contrib.sessions.middleware.SessionMiddleware', diff --git a/openstack_auth/tests/urls.py b/openstack_auth/tests/urls.py index b93725a2a2..0a32b08418 100644 --- a/openstack_auth/tests/urls.py +++ b/openstack_auth/tests/urls.py @@ -15,13 +15,9 @@ from django.conf.urls import include from django.conf.urls import url from django.views import generic -from openstack_auth import utils from openstack_auth import views -utils.patch_middleware_get_user() - - urlpatterns = [ url(r"", include('openstack_auth.urls')), url(r"^websso/$", views.websso, name='websso'), diff --git a/openstack_auth/urls.py b/openstack_auth/urls.py index 12d7cfb0b3..bfa95c2ac2 100644 --- a/openstack_auth/urls.py +++ b/openstack_auth/urls.py @@ -16,8 +16,6 @@ from django.conf.urls import url from openstack_auth import utils from openstack_auth import views -utils.patch_middleware_get_user() - urlpatterns = [ url(r"^login/$", views.login, name='login'), diff --git a/openstack_auth/utils.py b/openstack_auth/utils.py index d7223a2373..006fc0aef8 100644 --- a/openstack_auth/utils.py +++ b/openstack_auth/utils.py @@ -37,8 +37,8 @@ We need the request object to get the user, so we'll slightly modify the existing django.contrib.auth.get_user method. To do so we update the auth middleware to point to our overridden method. -Calling the "patch_middleware_get_user" method somewhere like our urls.py -file takes care of hooking it in appropriately. +Calling "patch_middleware_get_user" is done in our custom middleware at +"openstack_auth.middleware" to monkeypatch the code in before it is needed. """ diff --git a/openstack_dashboard/settings.py b/openstack_dashboard/settings.py index 2701527ac7..6ef5d25a77 100644 --- a/openstack_dashboard/settings.py +++ b/openstack_dashboard/settings.py @@ -110,6 +110,7 @@ OPENSTACK_IMAGE_BACKEND = { } MIDDLEWARE = ( + 'openstack_auth.middleware.OpenstackAuthMonkeyPatchMiddleware', 'django.middleware.common.CommonMiddleware', 'django.middleware.csrf.CsrfViewMiddleware', 'django.contrib.sessions.middleware.SessionMiddleware', diff --git a/openstack_dashboard/test/helpers.py b/openstack_dashboard/test/helpers.py index 052ab654c0..ae5b349f85 100644 --- a/openstack_dashboard/test/helpers.py +++ b/openstack_dashboard/test/helpers.py @@ -496,7 +496,6 @@ class APITestCase(TestCase): LOG.warning("APITestCase has been deprecated in favor of mock usage " "and will be removed at the beginning of 'Stein' release. " "Please convert your to use APIMockTestCase instead.") - utils.patch_middleware_get_user() def fake_keystoneclient(request, admin=False): """Returns the stub keystoneclient. @@ -616,11 +615,10 @@ class APITestCase(TestCase): return self.swiftclient -class APIMockTestCase(TestCase): - - def setUp(self): - super(APIMockTestCase, self).setUp() - utils.patch_middleware_get_user() +# NOTE(adriant): APIMockTestCase was only needed for some openstack_auth +# monkeypatching. With the new monkeypatch middleware from openstack_auth this +# is not needed. This class is used by horizon plugins, so we cannot drop it. +APIMockTestCase = TestCase # Need this to test both Glance API V1 and V2 versions