From 3b23f78bdc8b571cee64e9a0c1ac3818b17d340f Mon Sep 17 00:00:00 2001 From: lispyclouds Date: Wed, 8 Feb 2017 11:33:05 +0530 Subject: [PATCH] Implement middleware for admin views - Add AdminMiddleware to deny non admin users - Add decorator to mark views as "non admin" --- plinth/__main__.py | 1 + plinth/middleware.py | 23 +++++++- plinth/modules/users/urls.py | 4 +- plinth/tests/test_middleware.py | 94 ++++++++++++++++++++++++++++++++- plinth/utils.py | 9 +++- 5 files changed, 125 insertions(+), 6 deletions(-) diff --git a/plinth/__main__.py b/plinth/__main__.py index eeb92811b..9af5cf4a6 100644 --- a/plinth/__main__.py +++ b/plinth/__main__.py @@ -234,6 +234,7 @@ def configure_django(): 'django.contrib.auth.middleware.AuthenticationMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', 'django.middleware.clickjacking.XFrameOptionsMiddleware', + 'plinth.middleware.AdminMiddleware', 'stronghold.middleware.LoginRequiredMiddleware', 'plinth.modules.first_boot.middleware.FirstBootMiddleware', 'plinth.middleware.SetupMiddleware', diff --git a/plinth/middleware.py b/plinth/middleware.py index 4552efe65..b23cebc41 100644 --- a/plinth/middleware.py +++ b/plinth/middleware.py @@ -16,16 +16,19 @@ # """ -Django middleware to show pre-setup message and setup progress. +Common Django middleware. """ from django import urls from django.conf import settings from django.contrib import messages from django.contrib.auth.decorators import login_required +from django.core.exceptions import PermissionDenied from django.utils.translation import ugettext_lazy as _ import logging +from stronghold.utils import is_view_func_public + import plinth from plinth.package import PackageException from . import views @@ -35,7 +38,9 @@ logger = logging.getLogger(__name__) class SetupMiddleware(object): - """Show setup page or progress if setup is neccessary or running.""" + """ + Django middleware to show pre-setup message and setup progress. + """ @staticmethod def process_view(request, view_func, view_args, view_kwargs): @@ -86,3 +91,17 @@ class SetupMiddleware(object): # 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) + + +class AdminMiddleware(object): + """ + Django middleware for authenticating requests for admin areas + """ + + @staticmethod + def process_view(request, view_func, view_args, view_kwargs): + if is_view_func_public(view_func) or hasattr(view_func, 'IS_NON_ADMIN'): + return + + if not request.user.groups.filter(name='admin').exists(): + raise PermissionDenied diff --git a/plinth/modules/users/urls.py b/plinth/modules/users/urls.py index 442f5e32a..fca202851 100644 --- a/plinth/modules/users/urls.py +++ b/plinth/modules/users/urls.py @@ -37,9 +37,9 @@ urlpatterns = [ url(r'^sys/users/(?P[\w.@+-]+)/change_password/$', views.UserChangePassword.as_view(), name='change_password'), # Add Django's login/logout urls - url(r'^accounts/login/$', auth_views.login, + url(r'^accounts/login/$', public(auth_views.login), {'template_name': 'login.html'}, name='login'), - url(r'^accounts/logout/$', auth_views.logout, + url(r'^accounts/logout/$', public(auth_views.logout), {'next_page': reverse_lazy('index')}, name='logout'), url(r'^users/firstboot/$', public(views.FirstBootView.as_view()), name='firstboot'), diff --git a/plinth/tests/test_middleware.py b/plinth/tests/test_middleware.py index 5ac0881ee..4520aaee8 100644 --- a/plinth/tests/test_middleware.py +++ b/plinth/tests/test_middleware.py @@ -22,12 +22,14 @@ Test module for Plinth's custom middleware. from unittest.mock import Mock, patch from django.contrib.auth.models import AnonymousUser, User +from django.core.exceptions import PermissionDenied from django.http import HttpResponse from django.test import TestCase from django.test.client import RequestFactory +from stronghold.decorators import public from plinth import cfg -from plinth.middleware import SetupMiddleware +from plinth.middleware import SetupMiddleware, AdminMiddleware class TestSetupMiddleware(TestCase): @@ -131,3 +133,93 @@ class TestSetupMiddleware(TestCase): self.assertIsNone(response) assert messages_success.called module.setup_helper.collect_result.assert_called_once_with() + + +class TestAdminMiddleware(TestCase): + """Test cases for admin middleware.""" + @classmethod + def setUpClass(cls): + """Setup all the test cases.""" + super(TestAdminMiddleware, cls).setUpClass() + + cfg.read() + + def setUp(self): + """Setup each test case before execution.""" + super(TestAdminMiddleware, self).setUp() + + self.middleware = AdminMiddleware() + + @patch('django.contrib.messages.success') + @patch('plinth.module_loader.loaded_modules') + @patch('django.urls.resolve') + @patch('django.urls.reverse', return_value='users:login') + def test_that_admin_view_is_denied_for_usual_user(self, reverse, resolve, loaded_modules, messages_success): + """Test that normal user is denied for an admin view""" + + self.kwargs = { + 'view_func': HttpResponse, + 'view_args': [], + 'view_kwargs': {}, + } + + resolve.return_value.namespaces = ['mockapp'] + loaded_modules.__getitem__.return_value = Mock() + + request = RequestFactory().get('/plinth/mockapp') + request.user = Mock() + exists_fn = Mock() + exists_fn.exists = Mock(return_value=False) + request.user.groups.filter.return_value = exists_fn + + self.assertRaises(PermissionDenied, self.middleware.process_view, request, **self.kwargs) + + @patch('django.contrib.messages.success') + @patch('plinth.module_loader.loaded_modules') + @patch('django.urls.resolve') + @patch('django.urls.reverse', return_value='users:login') + def test_that_admin_view_is_allowed_for_admin_user(self, reverse, resolve, loaded_modules, messages_success): + """Test that admin user is allowed for an admin view""" + + self.kwargs = { + 'view_func': HttpResponse, + 'view_args': [], + 'view_kwargs': {}, + } + + resolve.return_value.namespaces = ['mockapp'] + loaded_modules.__getitem__.return_value = Mock() + + request = RequestFactory().get('/plinth/mockapp') + request.user = Mock() + exists_fn = Mock() + exists_fn.exists = Mock(return_value=True) + request.user.groups.filter.return_value = exists_fn + + response = self.middleware.process_view(request, **self.kwargs) + self.assertIsNone(response) + + @patch('django.contrib.messages.success') + @patch('plinth.module_loader.loaded_modules') + @patch('django.urls.resolve') + @patch('django.urls.reverse', return_value='users:login') + def test_that_public_view_is_allowed_for_normal_user(self, reverse, resolve, loaded_modules, messages_success): + """Test that normal user is allowed for an public view""" + + self.kwargs = { + 'view_func': public(HttpResponse), + 'view_args': [], + 'view_kwargs': {}, + } + + resolve.return_value.namespaces = ['mockapp'] + loaded_modules.__getitem__.return_value = Mock() + + request = RequestFactory().get('/plinth/mockapp') + request.user = Mock() + exists_fn = Mock() + exists_fn.exists = Mock(return_value=False) + request.user.groups.filter.return_value = exists_fn + + response = self.middleware.process_view(request, **self.kwargs) + self.assertIsNone(response) diff --git a/plinth/utils.py b/plinth/utils.py index a8995332b..58fb7d89f 100644 --- a/plinth/utils.py +++ b/plinth/utils.py @@ -16,7 +16,7 @@ # """ -Miscelleneous utility method. +Miscellaneous utility methods. """ import importlib @@ -44,3 +44,10 @@ def _format_lazy(string, *args, **kwargs): format_lazy = lazy(_format_lazy, str) + + +def non_admin_view(func): + """Decorator to mark a view as non admin.""" + + setattr(func, "IS_NON_ADMIN", True) + return func