Skip to content

Commit

Permalink
fix: provent user to triger a downlaod csv task if task details api i…
Browse files Browse the repository at this point in the history
…s inaccessible
  • Loading branch information
tehreem-sadat committed Nov 28, 2024
1 parent e4e3a89 commit 554cc3c
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 20 deletions.
7 changes: 7 additions & 0 deletions futurex_openedx_extensions/helpers/export_mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,13 @@ def list(self, request: Request, *args: Any, **kwargs: Any) -> Response:
permitted_tenant_ids = self.request.fx_permission_info[ # type: ignore[attr-defined]
'view_allowed_tenant_ids_any_access'
]

if not self.request.fx_permission_info['download_allowed']: # type: ignore[attr-defined]
return Response(
{'detail': 'You are not permitted to use the "download" parameter'},
status=http_status.HTTP_403_FORBIDDEN
)

if len(permitted_tenant_ids) > 1:
return Response(
{'detail': 'Download CSV functionality is not implemented for multiple tenants!'},
Expand Down
9 changes: 9 additions & 0 deletions futurex_openedx_extensions/helpers/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,17 +117,26 @@ def has_permission(self, request: Any, view: Any) -> bool:

system_staff_user_flag = is_system_staff_user(request.user)
user_roles: dict = get_user_course_access_roles(request.user.id)['roles']

download_allowed = bool(
set(user_roles.keys()) & set(view.get_view_user_roles_mapping(
view_name='exported_files_data', user=request.user
))
)

request.fx_permission_info = {
'user': request.user,
'user_roles': user_roles,
'is_system_staff_user': system_staff_user_flag,
'view_allowed_roles': view_allowed_roles,
'view_allowed_tenant_ids_any_access': tenant_ids,
'download_allowed': download_allowed,
}

if system_staff_user_flag:
request.fx_permission_info.update({
'user_roles': {},
'download_allowed': True,
})

if system_staff_user_flag or (
Expand Down
55 changes: 35 additions & 20 deletions tests/test_helpers/test_export_mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@ def export_csv_mixin(user, tenant): # pylint: disable=redefined-outer-name
view_instance = TestView()
request = APIRequestFactory().get('/')
request.user = user
request.fx_permission_info = {'user': user, 'view_allowed_tenant_ids_any_access': [tenant.id]}
request.fx_permission_info = {
'user': user,
'view_allowed_tenant_ids_any_access': [tenant.id],
'download_allowed': True,
}
view_instance.request = request
return view_instance

Expand Down Expand Up @@ -61,13 +65,15 @@ def test_get_filtered_query_params(export_csv_mixin): # pylint: disable=redefin
assert export_csv_mixin.get_filtered_query_params() == expected_params


def test_get_serialized_fx_permission_info(export_csv_mixin, user, tenant): # pylint: disable=redefined-outer-name
def test_get_serialized_fx_permission_info(export_csv_mixin, user): # pylint: disable=redefined-outer-name
"""Test get_serialized_fx_permission_info for user"""
assert isinstance(export_csv_mixin.request.fx_permission_info.get('user'), get_user_model()) is True
serialized_fx_info = export_csv_mixin.get_serialized_fx_permission_info()
expected_serialized_fx_info = {
'user_id': user.id, 'user': None, 'view_allowed_tenant_ids_any_access': [tenant.id]
}
expected_serialized_fx_info = export_csv_mixin.request.fx_permission_info
expected_serialized_fx_info.update({
'user': None,
'user_id': user.id,
})
assert serialized_fx_info == expected_serialized_fx_info


Expand All @@ -81,14 +87,14 @@ def test_generate_csv_url_response(
mocked_export_data_to_csv_task,
export_csv_mixin,
user,
tenant
tenant,
): # pylint: disable=redefined-outer-name, too-many-arguments
"""Test the generate_csv_url_response method."""
filename = 'mocked_file.csv'
fake_url = 'http://example.com/view'
export_csv_mixin.request.query_params = {'download': 'csv'}
serialized_fx_permission_info = {
'user_id': user.id, 'user': None, 'view_allowed_tenant_ids_any_access': [tenant.id]
'user_id': user.id, 'user': None, 'view_allowed_tenant_ids_any_access': [tenant.id], 'download_allowed': True
}
fake_query_params = {}
view_params = {'query_params': fake_query_params, 'kwargs': export_csv_mixin.kwargs, 'path': '/'}
Expand Down Expand Up @@ -193,21 +199,30 @@ def test_list_with_csv_download(


@pytest.mark.django_db
def test_list_with_csv_download_for_multiple_tenants(export_csv_mixin): # pylint: disable=redefined-outer-name
"""Test the list method for multiple tenants in fx permision info"""
export_csv_mixin.request.fx_permission_info['view_allowed_tenant_ids_any_access'] = [1, 2]
export_csv_mixin.request.query_params = {'download': 'csv'}
response = export_csv_mixin.list(export_csv_mixin.request)
assert response.status_code == http_status.HTTP_400_BAD_REQUEST


@pytest.mark.django_db
def test_list_with_csv_download_for_no_tenant(export_csv_mixin): # pylint: disable=redefined-outer-name
"""Test the list method for no tenant in fx info permissions."""
export_csv_mixin.request.fx_permission_info['view_allowed_tenant_ids_any_access'] = []
@pytest.mark.parametrize(
'view_allowed_tenant_ids, download_allowed, expected_status, usecase',
[
([1], False, http_status.HTTP_403_FORBIDDEN, 'Download access denied'),
([1, 2], True, http_status.HTTP_400_BAD_REQUEST, 'Multiple tenants in fx permission info'),
([], True, http_status.HTTP_403_FORBIDDEN, 'No tenant in fx permission info'),
([1], True, http_status.HTTP_200_OK, 'Task innitiated successfully'),
]
)
@patch('futurex_openedx_extensions.helpers.export_mixins.ExportCSVMixin.generate_csv_url_response')
def test_list_with_csv_download_for_different_http_statuses(
mocked_generate_response,
export_csv_mixin,
view_allowed_tenant_ids,
download_allowed,
expected_status,
usecase
): # pylint: disable=unused-argument, too-many-arguments, redefined-outer-name
"""Test the list method for multiple scenarios and return statuses."""
export_csv_mixin.request.fx_permission_info['view_allowed_tenant_ids_any_access'] = view_allowed_tenant_ids
export_csv_mixin.request.fx_permission_info['download_allowed'] = download_allowed
export_csv_mixin.request.query_params = {'download': 'csv'}
response = export_csv_mixin.list(export_csv_mixin.request)
assert response.status_code == http_status.HTTP_403_FORBIDDEN
assert response.status_code == expected_status, f'Failed for usecase: {usecase}'


@pytest.mark.django_db
Expand Down
32 changes: 32 additions & 0 deletions tests/test_helpers/test_permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class DummyView:
def __init__(self):
self.result_of_method = {
'dummyView': ['staff', 'admin'],
'exported_files_data': ['instructor']
}

def get_allowed_roles_all_views(self):
Expand Down Expand Up @@ -165,9 +166,39 @@ def test_fx_base_authenticated_permission_staff_always_allowed(
'view_allowed_tenant_ids_any_access': [1, 2, 3, 7, 8],
'view_allowed_tenant_ids_full_access': [1, 2, 3, 7, 8],
'view_allowed_tenant_ids_partial_access': [],
'download_allowed': True,
}


@pytest.mark.django_db
@pytest.mark.parametrize('view_allowed_roles, exported_files_data_roles, user_roles, download_allowed, usecase', [
(['staff'], ['instructor'], ['staff'], False, 'user does not have download access'),
(['staff'], ['instructor'], ['staff', 'instructor'], True, 'user have download access'),
])
def test_fx_base_authenticated_permission_for_download_allowed(
base_data, dummy_view, permission, support_user, view_allowed_roles,
exported_files_data_roles, user_roles, download_allowed, usecase
): # pylint: disable=unused-argument, redefined-outer-name, too-many-arguments
"""Verify that FXBaseAuthenticatedPermission fills fx_permission_info correctly for global users."""
request = APIRequestFactory().generic('GET', '/dummy/')
set_user(request, support_user.id)
roles_default_data = {
'orgs_full_access': [],
'tenant_ids_full_access': [],
'course_limited_access': [],
'orgs_of_courses': [],
'tenant_ids': []
}
dummy_view.result_of_method['dummyView'] = view_allowed_roles
dummy_view.result_of_method['exported_files_data'] = exported_files_data_roles
with patch('futurex_openedx_extensions.helpers.permissions.get_user_course_access_roles') as mocked_user_roles:
mocked_user_roles.return_value = {
'roles': {role: roles_default_data for role in user_roles}
}
permission.has_permission(request, dummy_view)
assert request.fx_permission_info['download_allowed'] == download_allowed, usecase


@pytest.mark.django_db
@pytest.mark.parametrize('view_allowed_roles, allowed_tenant_ids', [
(['staff', 'no-support-in-the-list'], []),
Expand Down Expand Up @@ -207,6 +238,7 @@ def test_fx_base_authenticated_permission_global_role_allow_all_tenants(
'view_allowed_tenant_ids_any_access': allowed_tenant_ids,
'view_allowed_tenant_ids_full_access': allowed_tenant_ids,
'view_allowed_tenant_ids_partial_access': [],
'download_allowed': False # user has support role while data export view allowed role is instructor.
},
ignore_order=True
)
Expand Down

0 comments on commit 554cc3c

Please sign in to comment.