diff --git a/futurex_openedx_extensions/helpers/export_mixins.py b/futurex_openedx_extensions/helpers/export_mixins.py index b001d34f..753137b6 100644 --- a/futurex_openedx_extensions/helpers/export_mixins.py +++ b/futurex_openedx_extensions/helpers/export_mixins.py @@ -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!'}, diff --git a/futurex_openedx_extensions/helpers/permissions.py b/futurex_openedx_extensions/helpers/permissions.py index 78565728..04e84e31 100644 --- a/futurex_openedx_extensions/helpers/permissions.py +++ b/futurex_openedx_extensions/helpers/permissions.py @@ -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 ( diff --git a/tests/test_helpers/test_export_mixins.py b/tests/test_helpers/test_export_mixins.py index e0b15c74..4f0683f1 100644 --- a/tests/test_helpers/test_export_mixins.py +++ b/tests/test_helpers/test_export_mixins.py @@ -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 @@ -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 @@ -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': '/'} @@ -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 diff --git a/tests/test_helpers/test_permissions.py b/tests/test_helpers/test_permissions.py index 172484a6..3b15dc1d 100644 --- a/tests/test_helpers/test_permissions.py +++ b/tests/test_helpers/test_permissions.py @@ -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): @@ -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'], []), @@ -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 )