Skip to content

Commit

Permalink
Ignore export win admin override on autocomplete (#5329)
Browse files Browse the repository at this point in the history
  • Loading branch information
chopkinsmade authored Apr 4, 2024
1 parent 7d04233 commit 57fa0bc
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 12 deletions.
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

0 comments on commit 57fa0bc

Please sign in to comment.