From e633f9deb19faa349968b6d95fe1708a19099f2d Mon Sep 17 00:00:00 2001 From: Fata Nugraha Date: Fri, 20 Jul 2018 14:28:15 +0700 Subject: [PATCH] Add Import Export Permissioning #608 (#804) * Add safeguard permission to admin * remove button if not authorized * Add Author * fix tests * add more tests * Update docs --- AUTHORS | 3 +- docs/installation.rst | 12 ++ import_export/admin.py | 51 +++++++ .../change_list_export_item.html | 2 + .../change_list_import_item.html | 2 + tests/core/tests/test_permissions.py | 125 ++++++++++++++++++ 6 files changed, 194 insertions(+), 1 deletion(-) create mode 100644 tests/core/tests/test_permissions.py diff --git a/AUTHORS b/AUTHORS index 7048c07..02dbd07 100644 --- a/AUTHORS +++ b/AUTHORS @@ -106,4 +106,5 @@ The following is a list of much appreciated contributors: * andrew-bro (Andrei Loskutov) * toivomattila (Toivo Mattila) * ZuluPro (Anthony Monthe) -* kunal15595 (Kunal Khandelwal) \ No newline at end of file +* kunal15595 (Kunal Khandelwal) +* fatanugraha (Fata Nugraha) diff --git a/docs/installation.rst b/docs/installation.rst index dec3874..6808c7e 100644 --- a/docs/installation.rst +++ b/docs/installation.rst @@ -56,7 +56,19 @@ You can use the following directives in your settings file: is checked first, which defaults to ``None``. If not found, this global option is used. Default is ``TempFolderStorage``. +``IMPORT_EXPORT_IMPORT_PERMISSION_CODE`` + Global setting for defining user permission that is required for + users/groups to execute import action. Django builtin permissions + are ``change``, ``add``, and ``delete``. It is possible to add + your own permission code. Default is ``None`` which means + everybody can execute import action. +``IMPORT_EXPORT_EXPORT_PERMISSION_CODE`` + Global setting for defining user permission that is required for + users/groups to execute export action. Django builtin permissions + are ``change``, ``add``, and ``delete``. It is possible to add + your own permission code. Default is ``None`` which means + everybody can execute export action. Example app =========== diff --git a/import_export/admin.py b/import_export/admin.py index a108dec..3ebdd56 100644 --- a/import_export/admin.py +++ b/import_export/admin.py @@ -5,6 +5,7 @@ from datetime import datetime import importlib import django from django.contrib import admin +from django.contrib.auth import get_permission_codename from django.utils import six from django.utils.translation import ugettext_lazy as _ from django.conf.urls import url @@ -12,6 +13,7 @@ from django.template.response import TemplateResponse from django.contrib import messages from django.contrib.admin.models import LogEntry, ADDITION, CHANGE, DELETION from django.contrib.contenttypes.models import ContentType +from django.core.exceptions import PermissionDenied from django.http import HttpResponseRedirect, HttpResponse try: from django.urls import reverse @@ -44,6 +46,8 @@ except ImportError: SKIP_ADMIN_LOG = getattr(settings, 'IMPORT_EXPORT_SKIP_ADMIN_LOG', False) TMP_STORAGE_CLASS = getattr(settings, 'IMPORT_EXPORT_TMP_STORAGE_CLASS', TempFolderStorage) + + if isinstance(TMP_STORAGE_CLASS, six.string_types): try: # Nod to tastypie's use of importlib. @@ -97,6 +101,18 @@ class ImportMixin(ImportExportMixinBase): else: return self.tmp_storage_class + def has_import_permission(self, request): + """ + Returns whether a request has import permission. + """ + IMPORT_PERMISSION_CODE = getattr(settings, 'IMPORT_EXPORT_IMPORT_PERMISSION_CODE', None) + if IMPORT_PERMISSION_CODE is None: + return True + + opts = self.opts + codename = get_permission_codename(IMPORT_PERMISSION_CODE, opts) + return request.user.has_perm("%s.%s" % (opts.app_label, codename)) + def get_urls(self): urls = super(ImportMixin, self).get_urls() info = self.get_model_info() @@ -139,6 +155,8 @@ class ImportMixin(ImportExportMixinBase): """ Perform the actual import action (after the user has confirmed the import) """ + if not self.has_import_permission(request): + raise PermissionDenied confirm_form = ConfirmImportForm(request.POST) if confirm_form.is_valid(): @@ -234,6 +252,9 @@ class ImportMixin(ImportExportMixinBase): uploaded file to a local temp file that will be used by 'process_import' for the actual import. ''' + if not self.has_import_permission(request): + raise PermissionDenied + resource = self.get_import_resource_class()(**self.get_import_resource_kwargs(request, *args, **kwargs)) context = self.get_import_context_data() @@ -289,6 +310,12 @@ class ImportMixin(ImportExportMixinBase): return TemplateResponse(request, [self.import_template_name], context) + def changelist_view(self, request, extra_context=None): + if extra_context is None: + extra_context = {} + extra_context['has_import_permission'] = self.has_import_permission(request) + return super(ImportMixin, self).changelist_view(request, extra_context) + class ExportMixin(ImportExportMixinBase): """ @@ -314,6 +341,18 @@ class ExportMixin(ImportExportMixinBase): ] return my_urls + urls + def has_export_permission(self, request): + """ + Returns whether a request has export permission. + """ + EXPORT_PERMISSION_CODE = getattr(settings, 'IMPORT_EXPORT_EXPORT_PERMISSION_CODE', None) + if EXPORT_PERMISSION_CODE is None: + return True + + opts = self.opts + codename = get_permission_codename(EXPORT_PERMISSION_CODE, opts) + return request.user.has_perm("%s.%s" % (opts.app_label, codename)) + def get_resource_kwargs(self, request, *args, **kwargs): return {} @@ -372,6 +411,9 @@ class ExportMixin(ImportExportMixinBase): Returns file_format representation for given queryset. """ request = kwargs.pop("request") + if not self.has_export_permission(request): + raise PermissionDenied + resource_class = self.get_export_resource_class() data = resource_class(**self.get_export_resource_kwargs(request)).export(queryset, *args, **kwargs) export_data = file_format.export_data(data) @@ -384,6 +426,9 @@ class ExportMixin(ImportExportMixinBase): return {} def export_action(self, request, *args, **kwargs): + if not self.has_export_permission(request): + raise PermissionDenied + formats = self.get_export_formats() form = ExportForm(formats, request.POST or None) if form.is_valid(): @@ -413,6 +458,12 @@ class ExportMixin(ImportExportMixinBase): return TemplateResponse(request, [self.export_template_name], context) + def changelist_view(self, request, extra_context=None): + if extra_context is None: + extra_context = {} + extra_context['has_export_permission'] = self.has_export_permission(request) + return super(ExportMixin, self).changelist_view(request, extra_context) + class ImportExportMixin(ImportMixin, ExportMixin): """ diff --git a/import_export/templates/admin/import_export/change_list_export_item.html b/import_export/templates/admin/import_export/change_list_export_item.html index 83b336a..b95d667 100644 --- a/import_export/templates/admin/import_export/change_list_export_item.html +++ b/import_export/templates/admin/import_export/change_list_export_item.html @@ -1,4 +1,6 @@ {% load i18n %} {% load admin_urls %} +{% if has_export_permission %}
  • {% trans "Export" %}
    • +{% endif %} diff --git a/import_export/templates/admin/import_export/change_list_import_item.html b/import_export/templates/admin/import_export/change_list_import_item.html index 85733b6..93c8dc5 100644 --- a/import_export/templates/admin/import_export/change_list_import_item.html +++ b/import_export/templates/admin/import_export/change_list_import_item.html @@ -1,4 +1,6 @@ {% load i18n %} {% load admin_urls %} +{% if has_import_permission %}
  • {% trans "Import" %}
    • +{% endif %} diff --git a/tests/core/tests/test_permissions.py b/tests/core/tests/test_permissions.py new file mode 100644 index 0000000..f366c71 --- /dev/null +++ b/tests/core/tests/test_permissions.py @@ -0,0 +1,125 @@ +from __future__ import unicode_literals + +import os.path + +from django.contrib.auth import get_permission_codename +from django.contrib.auth.models import Permission +from django.test.utils import override_settings +from django.test.testcases import TestCase +from django.contrib.auth.models import User +from django.core.files.uploadedfile import SimpleUploadedFile +from django.utils.translation import ugettext_lazy as _ +from django.contrib.admin.models import LogEntry +from tablib import Dataset + +from core.admin import BookAdmin, AuthorAdmin, BookResource +from core.models import Category, Parent, Book + + +class ImportExportPermissionTest(TestCase): + + def setUp(self): + user = User.objects.create_user('admin', 'admin@example.com', + 'password') + user.is_staff = True + user.is_superuser = False + user.save() + + self.user = user + self.client.login(username='admin', password='password') + + def set_user_book_model_permission(self, action): + permission = Permission.objects.get(codename="%s_book" % action) + self.user.user_permissions.add(permission) + + @override_settings(IMPORT_EXPORT_IMPORT_PERMISSION_CODE='change') + def test_import(self): + # user has no permission to import + response = self.client.get('/admin/core/book/import/') + self.assertEqual(response.status_code, 403) + + # POST the import form + input_format = '0' + filename = os.path.join( + os.path.dirname(__file__), + os.path.pardir, + 'exports', + 'books.csv') + + with open(filename, "rb") as f: + data = { + 'input_format': input_format, + 'import_file': f, + } + + response = self.client.post('/admin/core/book/import/', data) + self.assertEqual(response.status_code, 403) + + response = self.client.post('/admin/core/book/process_import/', {}) + self.assertEqual(response.status_code, 403) + + # user has sufficient permission to import + self.set_user_book_model_permission('change') + + response = self.client.get('/admin/core/book/import/') + self.assertEqual(response.status_code, 200) + + # POST the import form + input_format = '0' + filename = os.path.join( + os.path.dirname(__file__), + os.path.pardir, + 'exports', + 'books.csv') + + with open(filename, "rb") as f: + data = { + 'input_format': input_format, + 'import_file': f, + } + + response = self.client.post('/admin/core/book/import/', data) + self.assertEqual(response.status_code, 200) + confirm_form = response.context['confirm_form'] + + data = confirm_form.initial + response = self.client.post('/admin/core/book/process_import/', data) + self.assertEqual(response.status_code, 302) + + + @override_settings(IMPORT_EXPORT_EXPORT_PERMISSION_CODE='change') + def test_import_with_permission_set(self): + response = self.client.get('/admin/core/book/export/') + self.assertEqual(response.status_code, 403) + + data = {'file_format': '0'} + response = self.client.post('/admin/core/book/export/', data) + self.assertEqual(response.status_code, 403) + + self.set_user_book_model_permission('change') + response = self.client.get('/admin/core/book/export/') + self.assertEqual(response.status_code, 200) + + data = {'file_format': '0'} + response = self.client.post('/admin/core/book/export/', data) + self.assertEqual(response.status_code, 200) + + @override_settings(IMPORT_EXPORT_EXPORT_PERMISSION_CODE='add') + def test_check_export_button(self): + self.set_user_book_model_permission('change') + + response = self.client.get('/admin/core/book/') + widget = "import_link" + self.assertIn(widget, response.content.decode()) + widget = "export_link" + self.assertNotIn(widget, response.content.decode()) + + @override_settings(IMPORT_EXPORT_IMPORT_PERMISSION_CODE='add') + def test_check_import_button(self): + self.set_user_book_model_permission('change') + + response = self.client.get('/admin/core/book/') + widget = "import_link" + self.assertNotIn(widget, response.content.decode()) + widget = "export_link" + self.assertIn(widget, response.content.decode())