From 65d0d05652de1e79751dda9e492a5a1ed6370516 Mon Sep 17 00:00:00 2001 From: Jarosenb Date: Tue, 11 Jan 2022 22:24:59 -0600 Subject: [PATCH] Refactor and enable downloads for private systems --- server/main.py | 51 ++++++++-------- server/tests/test_utils.py | 122 +++++++++++++++++++++++++++++++++++++ server/tests/util_test.py | 63 ------------------- 3 files changed, 147 insertions(+), 89 deletions(-) create mode 100644 server/tests/test_utils.py delete mode 100644 server/tests/util_test.py diff --git a/server/main.py b/server/main.py index c786164..1cbf52a 100644 --- a/server/main.py +++ b/server/main.py @@ -53,7 +53,7 @@ def raise_for_size(size: int, max_size: int = 2e9) -> None: class Archive(TypedDict): fs: str # Represents the absolute path to a file on the host machine. - n: str # Represents the path to a file relative to the zip archive root. + n: str # Represents the path to a file within the zip archive. def walk_archive_paths(base_path: str, file_paths: List[str]) -> List[Archive]: @@ -82,6 +82,24 @@ def walk_archive_paths(base_path: str, file_paths: List[str]) -> List[Archive]: return zip_paths +def check_system_access(system: str, paths: List[str], token: str) -> None: + """ + Confirm a user's READ access to files in a system by using their Tapis access token + to perform a listing at the files' common path. + """ + try: + common_path = os.path.commonpath(map(lambda p: p.strip("/"), paths)) + listing_url = ( + f"{TAPIS_BASE_URL}" + "/files/v2/listings/system/" + f"{system}/{common_path}/?limit=1" + ) + resp = requests.get(listing_url, headers={"Authorization": f"Bearer {token}"}) + resp.raise_for_status() + except HTTPError: + raise HTTPException(status_code=resp.status_code, detail=resp.reason) + + class CheckResponse(BaseModel): key: str @@ -97,33 +115,14 @@ def check_downloadable( auth: http.HTTPAuthorizationCredentials = Depends(HTTPBearer(auto_error=False)), ): PUBLIC_SYSTEMS = ["designsafe.storage.community", "designsafe.storage.published"] - if request.system not in PUBLIC_SYSTEMS: - - # Limit API to public systems for initial release. + if not auth and request.system not in PUBLIC_SYSTEMS: raise HTTPException( - status_code=403, detail="Private systems are currently disabled." + status_code=401, + detail="This resource cannot be accessed without Tapis credentials.", ) - """ - if not auth: - raise HTTPException( - status_code=401, - detail="Attempt to access private resource without auth credentials.", - ) - try: - listing_url = ( - f"{TAPIS_BASE_URL}" - "/files/v2/listings/system/" - f"{request.system}/{request.path.strip('/')}" - ) - resp = requests.get( - listing_url, headers={"Authorization": f"Bearer {auth.credentials}"} - ) - resp.raise_for_status() - return {"message": "success"} - except HTTPError: - print(resp.content) - raise HTTPException(status_code=403, detail=resp.json()) - """ + elif request.system not in PUBLIC_SYSTEMS: + check_system_access(request.system, request.paths, auth.credentials) + system_root = get_system_root(request.system) paths = walk_archive_paths(system_root, request.paths) diff --git a/server/tests/test_utils.py b/server/tests/test_utils.py new file mode 100644 index 0000000..fd4b412 --- /dev/null +++ b/server/tests/test_utils.py @@ -0,0 +1,122 @@ +import pytest +import os +from fastapi.exceptions import HTTPException +from unittest.mock import MagicMock, create_autospec +from server.main import get_system_root, walk_archive_paths, check_system_access + + +def test_get_system_root(): + assert ( + get_system_root("designsafe.storage.default") + == "/corral-repl/tacc/NHERI/shared" + ) + assert ( + get_system_root("designsafe.storage.community") + == "/corral-repl/tacc/NHERI/community" + ) + assert ( + get_system_root("designsafe.storage.published") + == "/corral-repl/tacc/NHERI/published" + ) + assert ( + get_system_root("project-7448086614930166251-242ac113-0001-012") + == "/corral-repl/tacc/NHERI/projects/7448086614930166251-242ac113-0001-012" + ) + with pytest.raises(HTTPException): + get_system_root("not-a-real-system") + + +def test_walk_archive(tmp_path): + """ + Generate and walk the following directory structure at tmp_path: + . + ├── f1.txt + └── sub_path/ + ├── f2.txt + └── sub_path2/ + └── f3.txt + """ + base = tmp_path + + f1 = base / "f1.txt" + f1.write_text("CONTENT 1") + + sub = base / "sub_path" + os.mkdir(sub) + f2 = sub / "f2.txt" + f2.write_text("CONTENT 2") + + sub2 = sub / "sub_path2" + os.mkdir(sub2) + f3 = sub2 / "f3.txt" + f3.write_text("CONTENT 3") + + requested_files = ["f1.txt", "sub_path"] + walk_result = walk_archive_paths(base, requested_files) + + assert walk_result == [ + {"fs": str(base / "f1.txt"), "n": "f1.txt"}, + {"fs": str(base / "sub_path" / "f2.txt"), "n": "sub_path/f2.txt"}, + { + "fs": str(base / "sub_path" / "sub_path2" / "f3.txt"), + "n": "sub_path/sub_path2/f3.txt", + }, + ] + + +@pytest.fixture +def mock_get_success(monkeypatch, request): + import requests + + mock_fn = create_autospec(requests.get) + mock_response = requests.Response() + mock_response.status_code = 200 + + mock_fn.return_value = mock_response + monkeypatch.setattr(requests, "get", mock_fn) + yield mock_fn + + +@pytest.fixture +def mock_get_error(monkeypatch): + import requests + + mock_fn = create_autospec(requests.get) + mock_response = requests.Response() + mock_response.status_code = 404 + + mock_fn.return_value = mock_response + monkeypatch.setattr(requests, "get", mock_fn) + yield mock_fn + + +def test_check_system_access_success(mock_get_success, monkeypatch): + from server import main + + monkeypatch.setattr(main, "TAPIS_BASE_URL", "tapis.test") + try: + check_system_access( + "test.system", ["/tmp/file1.txt", "/tmp/tmp2/file2.txt"], "ABC123" + ) + except Exception: + assert 0 + + mock_get_success.assert_called_with( + "tapis.test/files/v2/listings/system/test.system/tmp/?limit=1", + headers={"Authorization": f"Bearer {'ABC123'}"}, + ) + + +def test_check_system_access_failure(mock_get_error, monkeypatch): + from server import main + + monkeypatch.setattr(main, "TAPIS_BASE_URL", "tapis.test") + with pytest.raises(HTTPException): + check_system_access( + "test.system", ["/tmp/file1.txt", "/tmp/tmp2/file2.txt"], "ABC123" + ) + + mock_get_error.assert_called_with( + "tapis.test/files/v2/listings/system/test.system/tmp/?limit=1", + headers={"Authorization": f"Bearer {'ABC123'}"}, + ) diff --git a/server/tests/util_test.py b/server/tests/util_test.py deleted file mode 100644 index 8c71986..0000000 --- a/server/tests/util_test.py +++ /dev/null @@ -1,63 +0,0 @@ -import pytest -import os -from fastapi.exceptions import HTTPException -from server.main import get_system_root, walk_archive_paths - - -def test_get_system_root(): - assert ( - get_system_root("designsafe.storage.default") - == "/corral-repl/tacc/NHERI/shared" - ) - assert ( - get_system_root("designsafe.storage.community") - == "/corral-repl/tacc/NHERI/community" - ) - assert ( - get_system_root("designsafe.storage.published") - == "/corral-repl/tacc/NHERI/published" - ) - assert ( - get_system_root("project-7448086614930166251-242ac113-0001-012") - == "/corral-repl/tacc/NHERI/projects/7448086614930166251-242ac113-0001-012" - ) - with pytest.raises(HTTPException): - get_system_root("not-a-real-system") - - -def test_walk_archive(tmp_path): - """ - Generate and walk the following directory structure at tmp_path: - . - ├── f1.txt - └── sub_path/ - ├── f2.txt - └── sub_path2/ - └── f3.txt - """ - base = tmp_path - - f1 = base / "f1.txt" - f1.write_text("CONTENT 1") - - sub = base / "sub_path" - os.mkdir(sub) - f2 = sub / "f2.txt" - f2.write_text("CONTENT 2") - - sub2 = sub / "sub_path2" - os.mkdir(sub2) - f3 = sub2 / "f3.txt" - f3.write_text("CONTENT 3") - - requested_files = ["f1.txt", "sub_path"] - walk_result = walk_archive_paths(base, requested_files) - - assert walk_result == [ - {"fs": str(base / "f1.txt"), "n": "f1.txt"}, - {"fs": str(base / "sub_path" / "f2.txt"), "n": "sub_path/f2.txt"}, - { - "fs": str(base / "sub_path" / "sub_path2" / "f3.txt"), - "n": "sub_path/sub_path2/f3.txt", - }, - ]