diff --git a/datahub/core/admin.py b/datahub/core/admin.py index 8bb274e92..168e331ec 100644 --- a/datahub/core/admin.py +++ b/datahub/core/admin.py @@ -80,35 +80,35 @@ def has_change_permission(self, request, obj=None): class ExportWinsAdminMixin(admin.ModelAdmin): def has_module_permission(self, request): return handle_export_wins_admin_permissions( - request.user, + request, self.opts.app_label, super().has_module_permission(request), ) def has_view_permission(self, request, obj=None): return handle_export_wins_admin_permissions( - request.user, + request, self.opts.app_label, super().has_view_permission(request, obj), ) def has_add_permission(self, request): return handle_export_wins_admin_permissions( - request.user, + request, self.opts.app_label, super().has_add_permission(request), ) def has_delete_permission(self, request, obj=None): return handle_export_wins_admin_permissions( - request.user, + request, self.opts.app_label, super().has_delete_permission(request, obj), ) def has_change_permission(self, request, obj=None): return handle_export_wins_admin_permissions( - request.user, + request, self.opts.app_label, super().has_change_permission(request, obj), ) @@ -392,13 +392,24 @@ def pretty_source(self, obj): return format_html('
{0}
', json.dumps(value, indent=2)) -def handle_export_wins_admin_permissions(user, app_label, function): +def handle_export_wins_admin_permissions(request, app_label, check_permission_function): + + # The autocomplete fields in django admin have their own permission check on the models + # referenced by that field. As we have explicitly denied export win admins permission to + # access anything other than export win models, they will receive errors using these + # autocomplete fields even though they do have permission via the team role they have been + # assigned + if request.path.startswith('/admin/autocomplete'): + return check_permission_function + + user = request.user + if not user.is_superuser and user.groups.filter(name=EXPORT_WIN_GROUP_NAME).exists(): if app_label == 'export_win': return True return False - return function + return check_permission_function def _make_admin_permission_getter(codename): @@ -408,7 +419,7 @@ def _has_permission(self, request, obj=None): qualified_name = f'{app_label}.{codename}' return handle_export_wins_admin_permissions( - request.user, + request, app_label, request.user.has_perm(qualified_name), ) diff --git a/datahub/core/test/test_admin.py b/datahub/core/test/test_admin.py index 6f41dcbed..73928906f 100644 --- a/datahub/core/test/test_admin.py +++ b/datahub/core/test/test_admin.py @@ -346,13 +346,32 @@ def test_admin_account_lock_out_after_too_many_attempts(self): @pytest.mark.django_db class TestHandleExportWinsAdminPermissions: + + def test_request_path_is_autocomplete(self): + expected_response = 'ABC' + + request = Mock() + request.path = '/admin/autocomplete' + + result = handle_export_wins_admin_permissions( + request, '', check_permission_function=expected_response, + ) + + assert result == expected_response + def test_user_is_not_superuser_not_in_export_wins_group(self): permission_group = GroupFactory(name='Group') user = AdviserFactory(is_superuser=False) user.groups.add(permission_group) expected_response = 'ABC' - result = handle_export_wins_admin_permissions(user, '', function=expected_response) + request = Mock() + request.user = user + request.path = '' + + result = handle_export_wins_admin_permissions( + request, '', check_permission_function=expected_response, + ) assert result == expected_response @@ -362,7 +381,13 @@ def test_user_is_superuser_in_export_wins_group(self): user.groups.add(permission_group) expected_response = 'ABC' - result = handle_export_wins_admin_permissions(user, '', function=expected_response) + request = Mock() + request.user = user + request.path = '' + + result = handle_export_wins_admin_permissions( + request, '', check_permission_function=expected_response, + ) assert result == expected_response @@ -371,7 +396,13 @@ def test_user_is_not_superuser_in_export_wins_group_accessing_export_win_module( user = AdviserFactory(is_superuser=False) user.groups.add(permission_group) - result = handle_export_wins_admin_permissions(user, 'export_win', function=None) + request = Mock() + request.user = user + request.path = '' + + result = handle_export_wins_admin_permissions( + request, 'export_win', check_permission_function=None, + ) assert result is True @@ -380,6 +411,12 @@ def test_user_is_not_superuser_in_export_wins_group_accessing_company_module(sel user = AdviserFactory(is_superuser=False) user.groups.add(permission_group) - result = handle_export_wins_admin_permissions(user, 'company', function=None) + request = Mock() + request.user = user + request.path = '' + + result = handle_export_wins_admin_permissions( + request, 'company', check_permission_function=None, + ) assert result is False