middleware: Don't show setup view to non-admin users

This is an improvement over !2069, which solved #2094.

Tests:

- Install an app. Success result is shown.

- Install an app and kill the apt-get process in the middle. Error result is
shown.

- Click install on an app (email_server). Close the window without seeing the
result. Access the app page (like email_server/my_aliases) as a non-admin user.
No success/file message is shown. The page is shown properly. Access the page as
admin, success message is shown.

- Access an uninstalled/installed app page as anonymous user. User is redirected
to login page.

- Access an uninstalled/installed app page as non-admin user. Forbidden page is
shown.

- Access an uninstalled app page meant for non-admin users (such as
email_server/my_aliases) as non-admin user. Forbidden page is shown.

- Access an installed app page as admin. Success.

- Access an installed app page meant for non-admin users (such as
email_server/my_aliases) as admin. Success.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>

Reviewed-by: Fioddor Superconcentrado <fioddor@gmail.com>

plinth/middleware.py

@@ -25,6 +25,29 @@ from . import views
 logger = logging.getLogger(__name__)
 
 
+def _collect_setup_result(request, module):
+    """Show success/fail message from previous install operation."""
+    if not module.setup_helper.is_finished:
+        return
+
+    exception = module.setup_helper.collect_result()
+    if not exception:
+        if not setup._is_module_essential(module):
+            messages.success(request, _('Application installed.'))
+    else:
+        if isinstance(exception, PackageException):
+            error_string = getattr(exception, 'error_string', str(exception))
+            error_details = getattr(exception, 'error_details', '')
+            message = _('Error installing application: {string} '
+                        '{details}').format(string=error_string,
+                                            details=error_details)
+        else:
+            message = _('Error installing application: {error}') \
+                .format(error=exception)
+
+        messages.error(request, message)
+
+
 class SetupMiddleware(MiddlewareMixin):
     """Django middleware to show pre-setup message and setup progress."""
 
@@ -51,30 +74,18 @@ class SetupMiddleware(MiddlewareMixin):
         module_name = resolver_match.namespaces[0]
         module = plinth.module_loader.loaded_modules[module_name]
 
-        # Collect errors from any previous operations and show them
+        is_admin = is_user_admin(request)
-        if module.setup_helper.is_finished:
+        # Collect and show setup operation result to admins
-            exception = module.setup_helper.collect_result()
+        if is_admin:
-            if not exception:
+            _collect_setup_result(request, module)
-                if not setup._is_module_essential(module):
-                    messages.success(request, _('Application installed.'))
-            else:
-                if isinstance(exception, PackageException):
-                    error_string = getattr(exception, 'error_string',
-                                           str(exception))
-                    error_details = getattr(exception, 'error_details', '')
-                    message = _('Error installing application: {string} '
-                                '{details}').format(string=error_string,
-                                                    details=error_details)
-                else:
-                    message = _('Error installing application: {error}') \
-                        .format(error=exception)
-
-                messages.error(request, message)
 
         # Check if application is up-to-date
         if module.setup_helper.get_state() == 'up-to-date':
             return
 
+        if not is_admin:
+            raise PermissionDenied
+
         # Only allow logged-in users to access any setup page
         view = login_required(views.SetupView.as_view())
         return view(request, setup_helper=module.setup_helper)

plinth/tests/test_middleware.py

@@ -6,7 +6,7 @@ Test module for custom middleware.
 from unittest.mock import MagicMock, Mock, patch
 
 import pytest
-from django.contrib.auth.models import AnonymousUser, User
+from django.contrib.auth.models import AnonymousUser, Group, User
 from django.core.exceptions import PermissionDenied
 from django.http import HttpResponse
 from django.test.client import RequestFactory
@@ -64,6 +64,7 @@ class TestSetupMiddleware:
         loaded_modules.__getitem__.return_value = module
 
         request = RequestFactory().get('/plinth/mockapp')
+        request.user = AnonymousUser()
         response = middleware.process_view(request, **kwargs)
         assert response is None
 
@@ -72,6 +73,7 @@ class TestSetupMiddleware:
     @patch('plinth.module_loader.loaded_modules')
     @patch('django.urls.resolve')
     @patch('django.urls.reverse', return_value='users:login')
+    @pytest.mark.django_db
     def test_module_view(reverse, resolve, loaded_modules, setup_view,
                          middleware, kwargs):
         """Test that only registered users can access the setup view."""
@@ -82,16 +84,36 @@ class TestSetupMiddleware:
         view = Mock()
         setup_view.as_view.return_value = view
         request = RequestFactory().get('/plinth/mockapp')
+        request.session = MagicMock()
 
         # Verify that anonymous users cannot access the setup page
         request.user = AnonymousUser()
-        middleware.process_view(request, **kwargs)
+        with pytest.raises(PermissionDenied):
-        setup_view.as_view.assert_called_once_with()
+            middleware.process_view(request, **kwargs)
+
+        setup_view.as_view.assert_not_called()
         view.assert_not_called()
 
-        # Verify that logged-in users can access the setup page
+        # Verify that non-admin logged-in users can't access the setup page
-        request.user = User(username='johndoe')
+        user = User(username='johndoe')
+        user.save()
+        request.user = user
+        with pytest.raises(PermissionDenied):
+            middleware.process_view(request, **kwargs)
+
+        setup_view.as_view.assert_not_called()
+        view.assert_not_called()
+
+        # Verify that admin logged-in users can access the setup page
+        user = User(username='adminuser')
+        user.save()
+        group = Group(name='admin')
+        group.save()
+        user.groups.add(group)
+        user.save()
+        request.user = user
         middleware.process_view(request, **kwargs)
+        setup_view.as_view.assert_called_once_with()
         view.assert_called_once_with(request, setup_helper=module.setup_helper)
 
     @staticmethod
@@ -99,6 +121,7 @@ class TestSetupMiddleware:
     @patch('plinth.module_loader.loaded_modules')
     @patch('django.urls.resolve')
     @patch('django.urls.reverse', return_value='users:login')
+    @pytest.mark.django_db
     def test_install_result_collection(reverse, resolve, loaded_modules,
                                        messages_success, middleware, kwargs):
         """Test that module installation result is collected properly."""
@@ -110,13 +133,36 @@ class TestSetupMiddleware:
         module.setup_helper.get_state.return_value = 'up-to-date'
         loaded_modules.__getitem__.return_value = module
 
+        # Admin user can't collect result
         request = RequestFactory().get('/plinth/mockapp')
+        user = User(username='adminuser')
+        user.save()
+        group = Group(name='admin')
+        group.save()
+        user.groups.add(group)
+        user.save()
+        request.user = user
+        request.session = MagicMock()
         response = middleware.process_view(request, **kwargs)
 
         assert response is None
         assert messages_success.called
         module.setup_helper.collect_result.assert_called_once_with()
 
+        # Non-admin user can't collect result
+        messages_success.reset_mock()
+        module.setup_helper.collect_result.reset_mock()
+        request = RequestFactory().get('/plinth/mockapp')
+        user = User(username='johndoe')
+        user.save()
+        request.user = user
+        request.session = MagicMock()
+        response = middleware.process_view(request, **kwargs)
+
+        assert response is None
+        assert not messages_success.called
+        module.setup_helper.collect_result.assert_not_called()
+
 
 class TestAdminMiddleware:
     """Test cases for admin middleware."""
@@ -178,8 +224,8 @@ class TestAdminMiddleware:
         assert response is None
 
     @staticmethod
-    def test_that_public_view_is_allowed_for_normal_user(web_request,
+    def test_that_public_view_is_allowed_for_normal_user(
-                                                         middleware, kwargs):
+            web_request, middleware, kwargs):
         """Test that normal user is allowed for an public view"""
         kwargs = dict(kwargs)
         kwargs['view_func'] = public(HttpResponse)