Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bugfix/RR-1343-allow-export-wins-access-to-autocomplete #5329

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions datahub/core/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),
)
Expand Down Expand Up @@ -392,13 +392,24 @@ def pretty_source(self, obj):
return format_html('<pre>{0}</pre>', 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):
Expand All @@ -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),
)
Expand Down
45 changes: 41 additions & 4 deletions datahub/core/test/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand All @@ -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

Expand All @@ -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