From 8335d7f259c2b6defb908fd73bbcf73c362f7cb7 Mon Sep 17 00:00:00 2001 From: Jacob Lowe Date: Tue, 15 Oct 2024 16:45:53 -0500 Subject: [PATCH 01/11] WP-68: Initial update of datafiles unit tests --- .../fixtures/DataFiles.systems.fixture.js | 52 +++++++++---------- .../DataFiles/tests/DataFiles.test.jsx | 4 +- 2 files changed, 26 insertions(+), 30 deletions(-) diff --git a/client/src/components/DataFiles/fixtures/DataFiles.systems.fixture.js b/client/src/components/DataFiles/fixtures/DataFiles.systems.fixture.js index d1a019d07..02d1606c9 100644 --- a/client/src/components/DataFiles/fixtures/DataFiles.systems.fixture.js +++ b/client/src/components/DataFiles/fixtures/DataFiles.systems.fixture.js @@ -1,5 +1,29 @@ /* TODOv3 update this fixture https://jira.tacc.utexas.edu/browse/WP-68*/ +// Updated fixture changes from endpoint https://cep.test/api/datafiles/systems/list/ +/* +[ + "name", + "system", + "scheme", + "api", + "homeDir", + "icon", + "default", + "siteSearchPriority", + "readOnly", + "hideSearchBar", + "integration" +] + */ +// Removed: Hidden, keyservice +// Removed +/* + error: false, + errorMessage: null, + loading: false, +Removed: definitions array +*/ const systemsFixture = { storage: { configuration: [ @@ -9,10 +33,8 @@ const systemsFixture = { scheme: 'private', api: 'tapis', icon: null, - hidden: true, homeDir: '/home/username', default: true, - keyservice: true, }, { name: 'My Data (Frontera)', @@ -65,35 +87,9 @@ const systemsFixture = { integration: 'portal.apps.googledrive_integration', }, ], - error: false, - errorMessage: null, - loading: false, defaultHost: 'frontera.tacc.utexas.edu', defaultSystem: 'frontera', }, - definitions: { - list: [ - { - id: 'frontera.home.username', - storage: { - host: 'frontera.tacc.utexas.edu', - rootDir: '/home1/012345/username', - }, - effectiveUserId: 'username', - }, - { - id: 'longhorn.home.username', - storage: { - host: 'longhorn.tacc.utexas.edu', - rootDir: '/home/012345/username', - }, - effectiveUserId: 'username', - }, - ], - error: false, - errorMessage: null, - loading: false, - }, }; export default systemsFixture; diff --git a/client/src/components/DataFiles/tests/DataFiles.test.jsx b/client/src/components/DataFiles/tests/DataFiles.test.jsx index bdcc70118..117b5ee6f 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', () => { @@ -61,7 +61,7 @@ describe('DataFiles', () => { compress: '', }, }, - systems: { + systems: { // TODO: Remove rest of unused variables storage: { configuration: [ { From dc3bf51bd966ed475e6deab5ce8f4a3ffe85d580 Mon Sep 17 00:00:00 2001 From: Jacob Lowe Date: Wed, 16 Oct 2024 14:00:38 -0500 Subject: [PATCH 02/11] WP-68: Investigation into System Definition and State --- .../DataFilesSidebar.test.jsx | 2 +- .../DataFilesSystemSelector.test.jsx | 2 +- .../fixtures/DataFiles.systems.fixture.js | 69 ++++++++++++------- 3 files changed, 47 insertions(+), 26 deletions(-) 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 02d1606c9..bed61ad27 100644 --- a/client/src/components/DataFiles/fixtures/DataFiles.systems.fixture.js +++ b/client/src/components/DataFiles/fixtures/DataFiles.systems.fixture.js @@ -1,29 +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/ -/* -[ - "name", - "system", - "scheme", - "api", - "homeDir", - "icon", - "default", - "siteSearchPriority", - "readOnly", - "hideSearchBar", - "integration" -] - */ -// Removed: Hidden, keyservice -// Removed -/* - error: false, - errorMessage: null, - loading: false, - -Removed: definitions array -*/ +// Removed from configuration: hidden, keyservice +// Removed from storage and defintions array: errorMessage, loading const systemsFixture = { storage: { configuration: [ @@ -87,9 +65,52 @@ const systemsFixture = { integration: 'portal.apps.googledrive_integration', }, ], + /* + * 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', + storage: { + host: 'frontera.tacc.utexas.edu', + rootDir: '/home1/012345/username', + }, + effectiveUserId: 'username', + }, + { + id: 'longhorn.home.username', + storage: { + host: 'longhorn.tacc.utexas.edu', + rootDir: '/home/012345/username', + }, + effectiveUserId: 'username', + }, + ], + error: false, // Commenting this out results in an error + //errorMessage: null, + //loading: false, + }, }; export default systemsFixture; From 11310ca97a544e68a736de11cb23e82fd88e7622 Mon Sep 17 00:00:00 2001 From: Jacob Lowe Date: Mon, 4 Nov 2024 11:45:35 -0600 Subject: [PATCH 03/11] showViewPath Coverage --- .../DataFilesListing.test.jsx | 96 +++++++++++++++++++ .../DataFiles/tests/DataFiles.test.jsx | 3 +- 2 files changed, 98 insertions(+), 1 deletion(-) diff --git a/client/src/components/DataFiles/DataFilesListing/DataFilesListing.test.jsx b/client/src/components/DataFiles/DataFilesListing/DataFilesListing.test.jsx index cdec53e91..307648088 100644 --- a/client/src/components/DataFiles/DataFilesListing/DataFilesListing.test.jsx +++ b/client/src/components/DataFiles/DataFilesListing/DataFilesListing.test.jsx @@ -309,3 +309,99 @@ describe('DataFilesListing - Section Name Determination', () => { ).toBeInTheDocument(); }); }); +describe('DataFilesListing - showViewPath', () => { + 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/tests/DataFiles.test.jsx b/client/src/components/DataFiles/tests/DataFiles.test.jsx index 117b5ee6f..8436914bf 100644 --- a/client/src/components/DataFiles/tests/DataFiles.test.jsx +++ b/client/src/components/DataFiles/tests/DataFiles.test.jsx @@ -61,7 +61,8 @@ describe('DataFiles', () => { compress: '', }, }, - systems: { // TODO: Remove rest of unused variables + systems: { + // TODO: Remove rest of unused variables storage: { configuration: [ { From 86c06cf8162212e73b134664212f1b2e035302a5 Mon Sep 17 00:00:00 2001 From: Jacob Lowe Date: Wed, 13 Nov 2024 09:30:40 -0600 Subject: [PATCH 04/11] unauth init --- server/portal/apps/datafiles/views.py | 2 + .../portal/apps/datafiles/views_unit_test.py | 48 +++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/server/portal/apps/datafiles/views.py b/server/portal/apps/datafiles/views.py index 69fa02fdb..12a89d741 100644 --- a/server/portal/apps/datafiles/views.py +++ b/server/portal/apps/datafiles/views.py @@ -86,6 +86,8 @@ def get(self, request, systemId): class TapisFilesView(BaseApiView): def get(self, request, operation=None, scheme=None, system=None, path='/'): try: + logger.info(request) + logger.info(request.user.tapis_oauth.client) client = request.user.tapis_oauth.client except AttributeError: # Make sure that we only let unauth'd users see public systems diff --git a/server/portal/apps/datafiles/views_unit_test.py b/server/portal/apps/datafiles/views_unit_test.py index 85b3e11f1..a851c0917 100644 --- a/server/portal/apps/datafiles/views_unit_test.py +++ b/server/portal/apps/datafiles/views_unit_test.py @@ -204,6 +204,54 @@ def test_tapis_file_view_get_is_logged_for_metrics(mock_indexer, client, authent authenticated_user.username)) +@patch("portal.libs.agave.operations.tapis_listing_indexer") +# @patch('portal.libs.agave.utils.service_account') +@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, + authenticated_user, + mock_tapis_client, + tapis_file_listing_mock, + logging_metric_mock, +): + tapis_listing_result = [TapisResult(**f) for f in tapis_file_listing_mock] + mock_tapis_client.files.listFiles.return_value = tapis_listing_result + response = client.get( + "/api/datafiles/tapis/listing/private/frontera.home.username/test.txt/?length=1234" + ) + assert response.status_code == 200 + assert response.json() == { + "data": { + "listing": [ + { + "system": "frontera.home.username", + "type": "dir" if f.type == "dir" else "file", + "format": "folder" if f.type == "dir" else "raw", + "mimeType": f.mimeType, + "path": f.path, + "name": f.name, + "length": f.size, + "lastModified": f.lastModified, + "_links": {"self": {"href": f.url}}, + } + for f in tapis_listing_result + ], + "reachedEnd": True, + } + } + + # Ensure metric-related logging is being performed + logging_metric_mock.assert_called_with( + "user:{} op:listing api:tapis scheme:private system:frontera.home.username path:test.txt filesize:1234".format( + authenticated_user.username + ) + ) + + @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): From 8a1f74b1cbab9df7791f2adc1b88df7a0e2a6393 Mon Sep 17 00:00:00 2001 From: Jacob Lowe Date: Fri, 15 Nov 2024 15:57:28 -0600 Subject: [PATCH 05/11] attribute error test coverage --- server/portal/apps/auth/views_unit_test.py | 41 ++++++++------ server/portal/apps/datafiles/views.py | 2 - .../portal/apps/datafiles/views_unit_test.py | 53 +++++-------------- 3 files changed, 38 insertions(+), 58 deletions(-) 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 12a89d741..69fa02fdb 100644 --- a/server/portal/apps/datafiles/views.py +++ b/server/portal/apps/datafiles/views.py @@ -86,8 +86,6 @@ def get(self, request, systemId): class TapisFilesView(BaseApiView): def get(self, request, operation=None, scheme=None, system=None, path='/'): try: - logger.info(request) - logger.info(request.user.tapis_oauth.client) client = request.user.tapis_oauth.client except AttributeError: # Make sure that we only let unauth'd users see public systems diff --git a/server/portal/apps/datafiles/views_unit_test.py b/server/portal/apps/datafiles/views_unit_test.py index a851c0917..5503f6180 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 @@ -204,8 +205,7 @@ def test_tapis_file_view_get_is_logged_for_metrics(mock_indexer, client, authent authenticated_user.username)) -@patch("portal.libs.agave.operations.tapis_listing_indexer") -# @patch('portal.libs.agave.utils.service_account') +@patch('portal.libs.agave.operations.tapis_indexer') @patch( "django.conf.settings.PORTAL_DATAFILES_STORAGE_SYSTEMS", [{"scheme": "public", "system": "public.system", "homeDir": "/public/home/"}], @@ -213,43 +213,14 @@ def test_tapis_file_view_get_is_logged_for_metrics(mock_indexer, client, authent def test_tapis_file_view_get_unauthorized( mock_indexer, client, - authenticated_user, - mock_tapis_client, - tapis_file_listing_mock, - logging_metric_mock, ): - tapis_listing_result = [TapisResult(**f) for f in tapis_file_listing_mock] - mock_tapis_client.files.listFiles.return_value = tapis_listing_result - response = client.get( - "/api/datafiles/tapis/listing/private/frontera.home.username/test.txt/?length=1234" - ) - assert response.status_code == 200 - assert response.json() == { - "data": { - "listing": [ - { - "system": "frontera.home.username", - "type": "dir" if f.type == "dir" else "file", - "format": "folder" if f.type == "dir" else "raw", - "mimeType": f.mimeType, - "path": f.path, - "name": f.name, - "length": f.size, - "lastModified": f.lastModified, - "_links": {"self": {"href": f.url}}, - } - for f in tapis_listing_result - ], - "reachedEnd": True, - } - } + mock_user = MagicMock() + mock_user.tapis_oauth = 0 - # Ensure metric-related logging is being performed - logging_metric_mock.assert_called_with( - "user:{} op:listing api:tapis scheme:private system:frontera.home.username path:test.txt filesize:1234".format( - authenticated_user.username - ) - ) + 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') From 792b106833196c8135ed814f4cfb25397d57f72a Mon Sep 17 00:00:00 2001 From: Jacob Lowe Date: Fri, 15 Nov 2024 17:05:38 -0600 Subject: [PATCH 06/11] put unauth request test --- server/portal/apps/datafiles/views.py | 2 +- server/portal/apps/datafiles/views_unit_test.py | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/server/portal/apps/datafiles/views.py b/server/portal/apps/datafiles/views.py index 69fa02fdb..beb39264e 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:{} " diff --git a/server/portal/apps/datafiles/views_unit_test.py b/server/portal/apps/datafiles/views_unit_test.py index 5503f6180..4fa9bfd20 100644 --- a/server/portal/apps/datafiles/views_unit_test.py +++ b/server/portal/apps/datafiles/views_unit_test.py @@ -240,6 +240,21 @@ 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') +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, From f703817d59cb13dae2e0b43dae2f31d86e60f4c1 Mon Sep 17 00:00:00 2001 From: Jacob Lowe Date: Mon, 18 Nov 2024 11:48:03 -0600 Subject: [PATCH 07/11] post unauth and handler exception tests --- server/portal/apps/datafiles/views.py | 2 +- .../portal/apps/datafiles/views_unit_test.py | 34 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/server/portal/apps/datafiles/views.py b/server/portal/apps/datafiles/views.py index beb39264e..181115272 100644 --- a/server/portal/apps/datafiles/views.py +++ b/server/portal/apps/datafiles/views.py @@ -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 4fa9bfd20..7bf34e720 100644 --- a/server/portal/apps/datafiles/views_unit_test.py +++ b/server/portal/apps/datafiles/views_unit_test.py @@ -240,6 +240,17 @@ 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() @@ -274,6 +285,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_indexer, mock_post_handler, 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" From 6e232c063610d0cb53e344b4d0baa37eeca49226 Mon Sep 17 00:00:00 2001 From: Jacob Lowe Date: Mon, 18 Nov 2024 12:18:58 -0600 Subject: [PATCH 08/11] LinkView Exception Tests --- .../portal/apps/datafiles/views_unit_test.py | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/server/portal/apps/datafiles/views_unit_test.py b/server/portal/apps/datafiles/views_unit_test.py index 7bf34e720..7083c83f3 100644 --- a/server/portal/apps/datafiles/views_unit_test.py +++ b/server/portal/apps/datafiles/views_unit_test.py @@ -130,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") @@ -139,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( @@ -150,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) From 49139b8c99d9b4e2a98eaca39a39bc7fee10de18 Mon Sep 17 00:00:00 2001 From: Jacob Lowe Date: Mon, 18 Nov 2024 12:41:08 -0600 Subject: [PATCH 09/11] post handler exception coverage fix --- server/portal/apps/datafiles/views_unit_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/portal/apps/datafiles/views_unit_test.py b/server/portal/apps/datafiles/views_unit_test.py index 7083c83f3..85f1bf42a 100644 --- a/server/portal/apps/datafiles/views_unit_test.py +++ b/server/portal/apps/datafiles/views_unit_test.py @@ -326,7 +326,7 @@ def test_tapis_file_view_post_is_unauthorized(mock_indexer, text_file_fixture, c @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_indexer, mock_post_handler, client, authenticated_user, mock_tapis_client, +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 From d452a10c705cbdb13fba0e8eccd9266ba272d4aa Mon Sep 17 00:00:00 2001 From: Jacob Lowe Date: Wed, 27 Nov 2024 09:59:20 -0600 Subject: [PATCH 10/11] restore original mocks --- .../DataFiles/DataFilesListing/DataFilesListing.test.jsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/client/src/components/DataFiles/DataFilesListing/DataFilesListing.test.jsx b/client/src/components/DataFiles/DataFilesListing/DataFilesListing.test.jsx index ed0aa6732..5b44c62df 100644 --- a/client/src/components/DataFiles/DataFilesListing/DataFilesListing.test.jsx +++ b/client/src/components/DataFiles/DataFilesListing/DataFilesListing.test.jsx @@ -325,6 +325,9 @@ describe('DataFilesListing - Section Name Determination', () => { }); }); describe('DataFilesListing - showViewPath', () => { + afterEach(() => { + vi.restoreAllMocks(); + }) it('renders the "Path" column when showViewPath is true', () => { const testfile = { system: 'test.system', From 256901ada67430b503944576f988cbec14874835 Mon Sep 17 00:00:00 2001 From: Jacob Lowe Date: Wed, 27 Nov 2024 10:02:16 -0600 Subject: [PATCH 11/11] linting --- .../DataFiles/DataFilesListing/DataFilesListing.test.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/src/components/DataFiles/DataFilesListing/DataFilesListing.test.jsx b/client/src/components/DataFiles/DataFilesListing/DataFilesListing.test.jsx index 5b44c62df..3d4a3ea9a 100644 --- a/client/src/components/DataFiles/DataFilesListing/DataFilesListing.test.jsx +++ b/client/src/components/DataFiles/DataFilesListing/DataFilesListing.test.jsx @@ -327,7 +327,7 @@ describe('DataFilesListing - Section Name Determination', () => { describe('DataFilesListing - showViewPath', () => { afterEach(() => { vi.restoreAllMocks(); - }) + }); it('renders the "Path" column when showViewPath is true', () => { const testfile = { system: 'test.system',