diff --git a/.gitignore b/.gitignore index ea7fde3d4..1b524e6ca 100644 --- a/.gitignore +++ b/.gitignore @@ -22,4 +22,4 @@ plinth/tests/coverage/report/ *.mo .vagrant/ .idea/ -**\.DS_Store +.DS_Store diff --git a/plinth/__main__.py b/plinth/__main__.py index de5aba07e..886928f68 100644 --- a/plinth/__main__.py +++ b/plinth/__main__.py @@ -227,7 +227,7 @@ def configure_django(): 'django.contrib.auth.middleware.AuthenticationMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', 'django.middleware.clickjacking.XFrameOptionsMiddleware', - 'plinth.middleware.AdminMiddleware', + 'plinth.middleware.AdminRequiredMiddleware', 'stronghold.middleware.LoginRequiredMiddleware', 'plinth.modules.first_boot.middleware.FirstBootMiddleware', 'plinth.middleware.SetupMiddleware', diff --git a/plinth/middleware.py b/plinth/middleware.py index b23cebc41..562efd0e1 100644 --- a/plinth/middleware.py +++ b/plinth/middleware.py @@ -38,9 +38,7 @@ logger = logging.getLogger(__name__) class SetupMiddleware(object): - """ - Django middleware to show pre-setup message and setup progress. - """ + """Django middleware to show pre-setup message and setup progress.""" @staticmethod def process_view(request, view_func, view_args, view_kwargs): @@ -93,14 +91,14 @@ class SetupMiddleware(object): return view(request, setup_helper=module.setup_helper) -class AdminMiddleware(object): - """ - Django middleware for authenticating requests for admin areas - """ +class AdminRequiredMiddleware(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'): + """Reject non-admin access to views that are private and not marked.""" + if is_view_func_public(view_func) or \ + hasattr(view_func, 'IS_NON_ADMIN'): return if not request.user.groups.filter(name='admin').exists(): diff --git a/plinth/modules/openvpn/urls.py b/plinth/modules/openvpn/urls.py index 2ecd82851..11571b531 100644 --- a/plinth/modules/openvpn/urls.py +++ b/plinth/modules/openvpn/urls.py @@ -28,5 +28,6 @@ from . import views urlpatterns = [ url(r'^apps/openvpn/$', views.index, name='index'), url(r'^apps/openvpn/setup/$', views.setup, name='setup'), - url(r'^apps/openvpn/profile/$', non_admin_view(views.profile), name='profile'), + url(r'^apps/openvpn/profile/$', non_admin_view(views.profile), + name='profile'), ] diff --git a/plinth/modules/users/urls.py b/plinth/modules/users/urls.py index 79d16d852..5e3377268 100644 --- a/plinth/modules/users/urls.py +++ b/plinth/modules/users/urls.py @@ -31,12 +31,13 @@ from . import views urlpatterns = [ url(r'^sys/users/$', views.UserList.as_view(), name='index'), url(r'^sys/users/create/$', views.UserCreate.as_view(), name='create'), - url(r'^sys/users/(?P[\w.@+-]+)/edit/$', non_admin_view(views.UserUpdate.as_view()), - name='edit'), + url(r'^sys/users/(?P[\w.@+-]+)/edit/$', + non_admin_view(views.UserUpdate.as_view()), name='edit'), url(r'^sys/users/(?P[\w.@+-]+)/delete/$', views.UserDelete.as_view(), name='delete'), url(r'^sys/users/(?P[\w.@+-]+)/change_password/$', - non_admin_view(views.UserChangePassword.as_view()), name='change_password'), + non_admin_view(views.UserChangePassword.as_view()), + name='change_password'), # Add Django's login/logout urls url(r'^accounts/login/$', public(auth_views.login), {'template_name': 'login.html'}, name='login'), diff --git a/plinth/modules/users/views.py b/plinth/modules/users/views.py index f431a02eb..66358c1e9 100644 --- a/plinth/modules/users/views.py +++ b/plinth/modules/users/views.py @@ -82,11 +82,12 @@ class UserUpdate(ContextMixin, SuccessMessageMixin, UpdateView): title = ugettext_lazy('Edit User') def dispatch(self, request, *args, **kwargs): + """Handle a request and return a HTTP response.""" if self.request.user.get_username() != self.kwargs['slug'] \ - and not self.request.user.groups.filter(name='admin').exists(): + and not self.request.user.groups.filter(name='admin').exists(): raise PermissionDenied - return super(UserUpdate, self).dispatch(request, *args, **kwargs) + return super().dispatch(request, *args, **kwargs) def get_form_kwargs(self): """Make the requst object available to the form.""" @@ -152,11 +153,12 @@ class UserChangePassword(ContextMixin, SuccessMessageMixin, FormView): success_message = ugettext_lazy('Password changed successfully.') def dispatch(self, request, *args, **kwargs): + """Handle a request and return a HTTP response.""" if self.request.user.get_username() != self.kwargs['slug'] \ - and not self.request.user.groups.filter(name='admin').exists(): + and not self.request.user.groups.filter(name='admin').exists(): raise PermissionDenied - return super(UserChangePassword, self).dispatch(request, *args, **kwargs) + return super().dispatch(request, *args, **kwargs) def get_form_kwargs(self): """Make the user object available to the form.""" diff --git a/plinth/tests/test_middleware.py b/plinth/tests/test_middleware.py index 4520aaee8..7ddfd0285 100644 --- a/plinth/tests/test_middleware.py +++ b/plinth/tests/test_middleware.py @@ -29,7 +29,7 @@ from django.test.client import RequestFactory from stronghold.decorators import public from plinth import cfg -from plinth.middleware import SetupMiddleware, AdminMiddleware +from plinth.middleware import SetupMiddleware, AdminRequiredMiddleware class TestSetupMiddleware(TestCase): @@ -148,7 +148,6 @@ class TestAdminMiddleware(TestCase): """Setup each test case before execution.""" super(TestAdminMiddleware, self).setUp() - self.middleware = AdminMiddleware() @patch('django.contrib.messages.success') @patch('plinth.module_loader.loaded_modules') @@ -157,6 +156,7 @@ class TestAdminMiddleware(TestCase): 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.middleware = AdminRequiredMiddleware() self.kwargs = { 'view_func': HttpResponse, 'view_args': [], diff --git a/plinth/utils.py b/plinth/utils.py index 58fb7d89f..9f6cd97ac 100644 --- a/plinth/utils.py +++ b/plinth/utils.py @@ -47,7 +47,6 @@ 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) + """Decorator to mark a view as accesible by non-admin users.""" + setattr(func, 'IS_NON_ADMIN', True) return func diff --git a/plinth/views.py b/plinth/views.py index 437a0d211..2f0db2375 100644 --- a/plinth/views.py +++ b/plinth/views.py @@ -38,12 +38,13 @@ def index(request): shortcuts = frontpage.get_shortcuts() selection = request.GET.get('selected') - details, details_label, configure_url= None, None, None + details, details_label, configure_url = None, None, None if selection in frontpage.shortcuts: details = frontpage.shortcuts[selection]['details'] details_label = frontpage.shortcuts[selection]['label'] configure_url = frontpage.shortcuts[selection]['configure_url'] + user_is_admin = request.user.groups.filter(name='admin').exists() return TemplateResponse(request, 'index.html', {'title': _('FreedomBox'), 'shortcuts': shortcuts, @@ -51,7 +52,7 @@ def index(request): 'details': details, 'details_label': details_label, 'configure_url': configure_url, - 'user_is_admin': request.user.groups.filter(name='admin').exists()}) + 'user_is_admin': user_is_admin}) class ServiceView(FormView):