diff --git a/client/src/components/DataFiles/DataFilesListing/DataFilesListing.test.jsx b/client/src/components/DataFiles/DataFilesListing/DataFilesListing.test.jsx index 6a75abfd5..3d4a3ea9a 100644 --- a/client/src/components/DataFiles/DataFilesListing/DataFilesListing.test.jsx +++ b/client/src/components/DataFiles/DataFilesListing/DataFilesListing.test.jsx @@ -324,3 +324,102 @@ describe('DataFilesListing - Section Name Determination', () => { ).toBeInTheDocument(); }); }); +describe('DataFilesListing - showViewPath', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + it('renders the "Path" column when showViewPath is true', () => { + const testfile = { + system: 'test.system', + path: '/path/to/file', + name: 'testfile', + format: 'file', + length: 4096, + lastModified: '2019-06-17T15:49:53-05:00', + _links: { self: { href: 'href.test' } }, + }; + const history = createMemoryHistory(); + history.push('/workbench/data/tapis/private/test.system/'); + const store = mockStore({ + ...initialMockState, + files: { + ...initialMockState.files, + listing: { FilesListing: [testfile] }, + }, + workbench: { + config: { + viewPath: true, + }, + }, + }); + // Spy on useMemo to capture the cells array + const useMemoSpy = vi + .spyOn(React, 'useMemo') + .mockImplementation((fn) => fn()); + const { getByText } = renderComponent( + , + store, + history + ); + // Path cell is added + expect(getByText('Path')).toBeDefined(); + // Check the length of the cells array + const cellsArray = useMemoSpy.mock.results.find((result) => + Array.isArray(result.value) + ).value; + expect(cellsArray.length).toBe(6); + }); + it('does not render the "Path" column when showViewPath is false', () => { + const testfile = { + system: 'test.system', + path: '/path/to/file', + name: 'testfile', + format: 'file', + length: 4096, + lastModified: '2019-06-17T15:49:53-05:00', + _links: { self: { href: 'href.test' } }, + }; + const history = createMemoryHistory(); + history.push('/workbench/data/tapis/private/test.system/'); + const store = mockStore({ + ...initialMockState, + files: { + ...initialMockState.files, + listing: { FilesListing: [testfile] }, + }, + workbench: { + config: { + viewPath: false, + }, + }, + }); + // Spy on useMemo to capture the cells array + const useMemoSpy = vi + .spyOn(React, 'useMemo') + .mockImplementation((fn) => fn()); + const { queryByText } = renderComponent( + , + store, + history + ); + // Path should not exist as a new cell + expect(queryByText('Path')).toBeNull(); + // Check the length of the cells array + const cellsArray = useMemoSpy.mock.results.find((result) => + Array.isArray(result.value) + ).value; + expect(cellsArray.length).toBe(5); + }); +}); diff --git a/client/src/components/DataFiles/DataFilesSidebar/DataFilesSidebar.test.jsx b/client/src/components/DataFiles/DataFilesSidebar/DataFilesSidebar.test.jsx index 36eb90b2c..8cf95930b 100644 --- a/client/src/components/DataFiles/DataFilesSidebar/DataFilesSidebar.test.jsx +++ b/client/src/components/DataFiles/DataFilesSidebar/DataFilesSidebar.test.jsx @@ -62,7 +62,7 @@ describe('DataFilesSidebar', () => { ).toEqual( '/workbench/data/tapis/private/longhorn.home.username/home/username/' ); - expect(queryByText(/My Data \(Work\)/)).toBeNull(); + expect(queryByText(/My Data \(Work\)/)).toBeDefined(); }); it('disables creating new shared workspaces in read only shared workspaces', async () => { diff --git a/client/src/components/DataFiles/DataFilesSystemSelector/DataFilesSystemSelector.test.jsx b/client/src/components/DataFiles/DataFilesSystemSelector/DataFilesSystemSelector.test.jsx index 5aaaeb155..5e5312b78 100644 --- a/client/src/components/DataFiles/DataFilesSystemSelector/DataFilesSystemSelector.test.jsx +++ b/client/src/components/DataFiles/DataFilesSystemSelector/DataFilesSystemSelector.test.jsx @@ -31,7 +31,7 @@ describe('DataFilesSystemSelector', () => { store, history ); - expect(queryByText(/My Data \(Work\)/)).toBeNull(); + expect(queryByText(/My Data \(Work\)/)).toBeDefined(); expect(queryByText(/My Data \(Frontera\)/)).toBeDefined(); expect(queryByText(/My Data \(Longhorn\)/)).toBeDefined(); expect(queryByText(/Google Drive/)).toBeDefined(); diff --git a/client/src/components/DataFiles/fixtures/DataFiles.systems.fixture.js b/client/src/components/DataFiles/fixtures/DataFiles.systems.fixture.js index d1a019d07..bed61ad27 100644 --- a/client/src/components/DataFiles/fixtures/DataFiles.systems.fixture.js +++ b/client/src/components/DataFiles/fixtures/DataFiles.systems.fixture.js @@ -1,5 +1,7 @@ /* TODOv3 update this fixture https://jira.tacc.utexas.edu/browse/WP-68*/ - +// Updated fixture changes from endpoint https://cep.test/api/datafiles/systems/list/ +// Removed from configuration: hidden, keyservice +// Removed from storage and defintions array: errorMessage, loading const systemsFixture = { storage: { configuration: [ @@ -9,10 +11,8 @@ const systemsFixture = { scheme: 'private', api: 'tapis', icon: null, - hidden: true, homeDir: '/home/username', default: true, - keyservice: true, }, { name: 'My Data (Frontera)', @@ -65,13 +65,30 @@ const systemsFixture = { integration: 'portal.apps.googledrive_integration', }, ], - error: false, - errorMessage: null, - loading: false, + /* + * The following needs to be mirrored for the storage and definitions + + These are included in the datafiles reducers but pass tests without these + This means that tests need to be more comprehensive to catch this or removed + + Definitions that use variables other than list are used in: + - DataFilesTable.jsx:45 for error + + state.systems.definitions.* is not called for anything else other than error + These would need to be removed then + - errorMessage + - loading + */ + + //error: false, + //errorMessage: null, + //loading: false, defaultHost: 'frontera.tacc.utexas.edu', defaultSystem: 'frontera', }, + // This definitions is required for the tests, some can be removed. Referencing datafiles.reducers.js definitions: { + // For DataFilesTable and DataFilesShowPathModal it requires the id from this list list: [ { id: 'frontera.home.username', @@ -90,9 +107,9 @@ const systemsFixture = { effectiveUserId: 'username', }, ], - error: false, - errorMessage: null, - loading: false, + error: false, // Commenting this out results in an error + //errorMessage: null, + //loading: false, }, }; diff --git a/client/src/components/DataFiles/tests/DataFiles.test.jsx b/client/src/components/DataFiles/tests/DataFiles.test.jsx index bdcc70118..8436914bf 100644 --- a/client/src/components/DataFiles/tests/DataFiles.test.jsx +++ b/client/src/components/DataFiles/tests/DataFiles.test.jsx @@ -49,7 +49,7 @@ describe('DataFiles', () => { //); expect(getAllByText(/My Data \(Frontera\)/)).toBeDefined(); expect(getByText(/My Data \(Longhorn\)/)).toBeDefined(); - expect(queryByText(/My Data \(Work\)/)).toBeNull(); + expect(queryByText(/My Data \(Work\)/)).toBeDefined(); // Changed to defined, hidden attribute removed and would be defined by default }); it('should not render Data Files with no systems', () => { @@ -62,6 +62,7 @@ describe('DataFiles', () => { }, }, systems: { + // TODO: Remove rest of unused variables storage: { configuration: [ { diff --git a/server/portal/apps/auth/views_unit_test.py b/server/portal/apps/auth/views_unit_test.py index 5a9aa8815..22abc6f88 100644 --- a/server/portal/apps/auth/views_unit_test.py +++ b/server/portal/apps/auth/views_unit_test.py @@ -1,6 +1,7 @@ -from django.conf import settings import pytest +from django.conf import settings from django.urls import reverse + from portal.apps.auth.views import launch_setup_checks TEST_STATE = "ABCDEFG123456" @@ -9,33 +10,39 @@ def test_auth_tapis(client, mocker): - mocker.patch('portal.apps.auth.views._get_auth_state', return_value=TEST_STATE) + mocker.patch("portal.apps.auth.views._get_auth_state", return_value=TEST_STATE) response = client.get("/auth/tapis/", follow=False) - tapis_authorize = f"{settings.TAPIS_TENANT_BASEURL}/v3/oauth2/authorize" \ + tapis_authorize = ( + f"{settings.TAPIS_TENANT_BASEURL}/v3/oauth2/authorize" f"?client_id=test&redirect_uri=http://testserver/auth/tapis/callback/&response_type=code&state={TEST_STATE}" + ) assert response.status_code == 302 assert response.url == tapis_authorize - assert client.session['auth_state'] == TEST_STATE + assert client.session["auth_state"] == TEST_STATE def test_tapis_callback(client, mocker, regular_user, tapis_tokens_create_mock): - mock_authenticate = mocker.patch('portal.apps.auth.views.authenticate') - mock_tapis_token_post = mocker.patch('portal.apps.auth.views.requests.post') - mock_launch_setup_checks = mocker.patch('portal.apps.auth.views.launch_setup_checks') + mock_authenticate = mocker.patch("portal.apps.auth.views.authenticate") + mock_tapis_token_post = mocker.patch("portal.apps.auth.views.requests.post") + mock_launch_setup_checks = mocker.patch( + "portal.apps.auth.views.launch_setup_checks" + ) # add auth to session session = client.session - session['auth_state'] = TEST_STATE + session["auth_state"] = TEST_STATE session.save() mock_tapis_token_post.return_value.json.return_value = tapis_tokens_create_mock mock_tapis_token_post.return_value.status_code = 200 mock_authenticate.return_value = regular_user - response = client.get(f"/auth/tapis/callback/?state={TEST_STATE}&code=83163624a0bc41c4a376e0acb16a62f9") + response = client.get( + f"/auth/tapis/callback/?state={TEST_STATE}&code=83163624a0bc41c4a376e0acb16a62f9" + ) assert response.status_code == 302 assert response.url == settings.LOGIN_REDIRECT_URL assert mock_launch_setup_checks.call_count == 1 @@ -44,31 +51,35 @@ def test_tapis_callback(client, mocker, regular_user, tapis_tokens_create_mock): def test_tapis_callback_no_code(client): # add auth to session session = client.session - session['auth_state'] = TEST_STATE + session["auth_state"] = TEST_STATE session.save() response = client.get(f"/auth/tapis/callback/?state={TEST_STATE}") assert response.status_code == 302 - assert response.url == reverse('portal_accounts:logout') + assert response.url == reverse("portal_accounts:logout") def test_tapis_callback_mismatched_state(client): # add auth to session session = client.session - session['auth_state'] = "TEST_STATE" + session["auth_state"] = "TEST_STATE" session.save() response = client.get("/auth/tapis/callback/?state=bar") assert response.status_code == 400 def test_launch_setup_checks(regular_user, mocker): - mock_execute_setup_steps = mocker.patch('portal.apps.auth.views.execute_setup_steps') + mock_execute_setup_steps = mocker.patch( + "portal.apps.auth.views.execute_setup_steps" + ) launch_setup_checks(regular_user) - mock_execute_setup_steps.apply_async.assert_called_with(args=[regular_user.username]) + mock_execute_setup_steps.apply_async.assert_called_with( + args=[regular_user.username] + ) def test_launch_setup_checks_already_onboarded(regular_user, mocker): regular_user.profile.setup_complete = True - mock_index_allocations = mocker.patch('portal.apps.auth.views.index_allocations') + mock_index_allocations = mocker.patch("portal.apps.auth.views.index_allocations") launch_setup_checks(regular_user) mock_index_allocations.apply_async.assert_called_with(args=[regular_user.username]) diff --git a/server/portal/apps/datafiles/views.py b/server/portal/apps/datafiles/views.py index 69fa02fdb..181115272 100644 --- a/server/portal/apps/datafiles/views.py +++ b/server/portal/apps/datafiles/views.py @@ -137,7 +137,7 @@ def put(self, request, operation=None, scheme=None, try: client = request.user.tapis_oauth.client except AttributeError: - return HttpResponseForbidden + return HttpResponseForbidden("This data requires authentication to view.") try: METRICS.info("user:{} op:{} api:tapis scheme:{} " @@ -160,7 +160,7 @@ def post(self, request, operation=None, scheme=None, try: client = request.user.tapis_oauth.client except AttributeError: - return HttpResponseForbidden() + return HttpResponseForbidden("This data requires authentication to upload.") try: METRICS.info("user:{} op:{} api:tapis scheme:{} " diff --git a/server/portal/apps/datafiles/views_unit_test.py b/server/portal/apps/datafiles/views_unit_test.py index 85b3e11f1..85f1bf42a 100644 --- a/server/portal/apps/datafiles/views_unit_test.py +++ b/server/portal/apps/datafiles/views_unit_test.py @@ -1,13 +1,14 @@ -import pytest import json import logging import os -from mock import patch, MagicMock -from tapipy.errors import InternalServerError -from portal.apps.datafiles.models import Link + +import pytest from django.conf import settings +from mock import MagicMock, patch +from tapipy.errors import InternalServerError from tapipy.tapis import TapisResult +from portal.apps.datafiles.models import Link pytestmark = pytest.mark.django_db @@ -129,6 +130,21 @@ def test_link_post(postits_create, authenticated_user, client): ) +def test_link_post_already_exists(postits_create, authenticated_user, client): + result = client.post("/api/datafiles/link/tapis/system/path") + assert json.loads(result.content)["data"] == "https://tenant/uuid" + assert Link.objects.all()[0].get_uuid() == "uuid" + postits_create.assert_called_with( + systemId="system", + path="path", + allowedUses=-1, + validSeconds=31536000 + ) + result = client.post("/api/datafiles/link/tapis/system/path") + assert result.status_code == 400 + assert result.json() == {"message": "Link for this file already exists"} + + def test_link_delete(postits_create, authenticated_user, mock_tapis_client, client): mock_tapis_client.files.deletePostIt.return_value = "OK" client.post("/api/datafiles/link/tapis/system/path") @@ -138,6 +154,13 @@ def test_link_delete(postits_create, authenticated_user, mock_tapis_client, clie assert len(Link.objects.all()) == 0 +def test_link_delete_dne(authenticated_user, mock_tapis_client, client): + mock_tapis_client.files.deletePostIt.return_value = "Bad Request" + result = client.delete("/api/datafiles/link/tapis/system/path") + assert result.status_code == 400 + assert result.json() == {"message": "Post-it does not exist"} + + def test_link_put(postits_create, authenticated_user, mock_tapis_client, client): mock_tapis_client.files.deletePostIt.return_value = "OK" Link.objects.create( @@ -149,6 +172,13 @@ def test_link_put(postits_create, authenticated_user, mock_tapis_client, client) assert Link.objects.all()[0].get_uuid() == "uuid" +def test_link_put_dne(postits_create, authenticated_user, mock_tapis_client, client): + mock_tapis_client.files.deletePostIt.return_value = "Bad Request" + result = client.put("/api/datafiles/link/tapis/system/path") + assert result.status_code == 400 + assert result.json() == {"message": "Could not find pre-existing link"} + + def test_get_system(client, authenticated_user, mock_tapis_client, agave_storage_system_mock): mock_tapis_client.systems.getSystem.return_value = TapisResult(**agave_storage_system_mock) @@ -204,6 +234,24 @@ def test_tapis_file_view_get_is_logged_for_metrics(mock_indexer, client, authent authenticated_user.username)) +@patch('portal.libs.agave.operations.tapis_indexer') +@patch( + "django.conf.settings.PORTAL_DATAFILES_STORAGE_SYSTEMS", + [{"scheme": "public", "system": "public.system", "homeDir": "/public/home/"}], +) +def test_tapis_file_view_get_unauthorized( + mock_indexer, + client, +): + mock_user = MagicMock() + mock_user.tapis_oauth = 0 + + with patch('django.contrib.auth.get_user', return_value=mock_user): + response = client.get("/api/datafiles/tapis/listing/private/frontera.home.username/test.txt/?length=1234") + assert response.status_code == 403 + assert response.json() == {'message': 'This data requires authentication to view.'} + + @patch('portal.libs.agave.operations.tapis_indexer') def test_tapis_file_view_put_is_logged_for_metrics(mock_indexer, client, authenticated_user, mock_tapis_client, tapis_file_listing_mock, logging_metric_mock): @@ -221,6 +269,32 @@ def test_tapis_file_view_put_is_logged_for_metrics(mock_indexer, client, authent "system:frontera.home.username path:test.txt body:{}".format(authenticated_user.username, body)) +@patch('portal.libs.agave.operations.tapis_indexer') +@patch('portal.apps.datafiles.views.tapis_put_handler') +def test_tapis_file_view_put_is_logged_for_metrics_exception(mock_put_handler, mock_indexer, client, authenticated_user, mock_tapis_client): + mock_put_handler.side_effect = Exception("Exception in Metrics info or Tapis Put Handler views.py:142") + body = {"dest_path": "/testfol", "dest_system": "frontera.home.username"} + response = client.put("/api/datafiles/tapis/move/private/frontera.home.username/test.txt/", + content_type="application/json", + data=body) + assert response.status_code == 500 + + +@patch('portal.libs.agave.operations.tapis_indexer') +def test_tapis_file_view_put_is_unauthorized(mock_indexer, client): + mock_user = MagicMock() + mock_user.tapis_oauth = 0 + with patch('django.contrib.auth.get_user', return_value=mock_user): + body = {"dest_path": "/testfol", "dest_system": "frontera.home.username"} + response = client.put( + "/api/datafiles/tapis/move/private/frontera.home.username/test.txt/", + content_type="application/json", + data=body, + ) + assert response.status_code == 403 + assert response.content == b"This data requires authentication to view." + + @patch('portal.libs.agave.operations.tapis_indexer') def test_tapis_file_view_post_is_logged_for_metrics(mock_indexer, client, authenticated_user, mock_tapis_client, logging_metric_mock, @@ -240,6 +314,29 @@ def test_tapis_file_view_post_is_logged_for_metrics(mock_indexer, client, authen "system:frontera.home.username path:/ filename:text_file.txt".format(authenticated_user.username)) +@patch('portal.libs.agave.operations.tapis_indexer') +def test_tapis_file_view_post_is_unauthorized(mock_indexer, text_file_fixture, client): + mock_user = MagicMock() + mock_user.tapis_oauth = 0 + with patch('django.contrib.auth.get_user', return_value=mock_user): + response = client.post("/api/datafiles/tapis/upload/private/frontera.home.username/", data={"uploaded_file": text_file_fixture}) + assert response.status_code == 403 + assert response.content == b"This data requires authentication to upload." + + +@patch('portal.libs.agave.operations.tapis_indexer') +@patch('portal.apps.datafiles.views.tapis_post_handler') +def test_tapis_file_view_post_is_logged_for_metrics_exception(mock_post_handler, mock_indexer, client, authenticated_user, mock_tapis_client, + logging_metric_mock, tapis_file_mock, requests_mock, text_file_fixture): + mock_post_handler.side_effect = Exception("Exception in Metrics info or Tapis Put Handler views.py:175") + mock_tapis_client.files.insert.return_value = tapis_file_mock + + response = client.post("/api/datafiles/tapis/upload/private/frontera.home.username/", + data={"uploaded_file": text_file_fixture}) + + assert response.status_code == 500 + + POSTIT_HREF = "https://tapis.example/postit/something"