From b7a974e326601573dfc9dd4d103216cb0f798ff6 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Thu, 19 Jan 2017 18:53:02 +0530 Subject: [PATCH] setup: Fix an infinite redirect in a rare case Don't try to show setup view for login page. This happens under a rare circumstance that user does not usually face. If 'users' module has not been setup but we try to run first boot and last part of the firstboot process is not yet completed and when user is not already logged in, an infinite redirect happens. Simply don't try to show setup view for login URL under any circumstance. This is similar to how firstboot middleware itself does not meddle with login URL. --- plinth/middleware.py | 7 +++++++ plinth/tests/test_middleware.py | 15 ++++++++++----- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/plinth/middleware.py b/plinth/middleware.py index b73b83ba9..4552efe65 100644 --- a/plinth/middleware.py +++ b/plinth/middleware.py @@ -20,6 +20,7 @@ Django middleware to show pre-setup message and setup progress. """ from django import urls +from django.conf import settings from django.contrib import messages from django.contrib.auth.decorators import login_required from django.utils.translation import ugettext_lazy as _ @@ -39,6 +40,12 @@ class SetupMiddleware(object): @staticmethod def process_view(request, view_func, view_args, view_kwargs): """Handle a request as Django middleware request handler.""" + # Don't interfere with login page + user_requests_login = request.path.startswith( + urls.reverse(settings.LOGIN_URL)) + if user_requests_login: + return + # Perform a URL resolution. This is slightly inefficient as # Django will do this resolution again. try: diff --git a/plinth/tests/test_middleware.py b/plinth/tests/test_middleware.py index a30343ba7..5ac0881ee 100644 --- a/plinth/tests/test_middleware.py +++ b/plinth/tests/test_middleware.py @@ -51,7 +51,8 @@ class TestSetupMiddleware(TestCase): 'view_kwargs': {}, } - def test_404_urls(self): + @patch('django.urls.reverse', return_value='users:login') + def test_404_urls(self, reverse): """Test how middleware deals with 404 URLs.""" request = RequestFactory().get('/plinth/non-existing-url') @@ -59,7 +60,8 @@ class TestSetupMiddleware(TestCase): self.assertEqual(response, None) - def test_url_not_an_application(self): + @patch('django.urls.reverse', return_value='users:login') + def test_url_not_an_application(self, reverse): """Test that none is returned for URLs that are not applications.""" request = RequestFactory().get('/plinth/') @@ -69,7 +71,8 @@ class TestSetupMiddleware(TestCase): @patch('plinth.module_loader.loaded_modules') @patch('django.urls.resolve') - def test_module_is_up_to_date(self, resolve, loaded_modules): + @patch('django.urls.reverse', return_value='users:login') + def test_module_is_up_to_date(self, reverse, resolve, loaded_modules): """Test that none is returned when module is up-to-date.""" resolve.return_value.namespaces = ['mockapp'] module = Mock() @@ -86,7 +89,8 @@ class TestSetupMiddleware(TestCase): @patch('plinth.views.SetupView') @patch('plinth.module_loader.loaded_modules') @patch('django.urls.resolve') - def test_module_view(self, resolve, loaded_modules, setup_view): + @patch('django.urls.reverse', return_value='users:login') + def test_module_view(self, reverse, resolve, loaded_modules, setup_view): """Test that only registered users can access the setup view.""" resolve.return_value.namespaces = ['mockapp'] module = Mock() @@ -110,7 +114,8 @@ class TestSetupMiddleware(TestCase): @patch('django.contrib.messages.success') @patch('plinth.module_loader.loaded_modules') @patch('django.urls.resolve') - def test_install_result_collection(self, resolve, loaded_modules, + @patch('django.urls.reverse', return_value='users:login') + def test_install_result_collection(self, reverse, resolve, loaded_modules, messages_success): """Test that module installation result is collected properly.""" resolve.return_value.namespaces = ['mockapp']