diff --git a/app/models/pydantic/datasets.py b/app/models/pydantic/datasets.py index 7ecdd8c72..8ebc84670 100644 --- a/app/models/pydantic/datasets.py +++ b/app/models/pydantic/datasets.py @@ -1,6 +1,6 @@ from typing import List, Optional, Union -from pydantic import Field, BaseModel +from pydantic import BaseModel, Field from .base import BaseRecord, StrictBaseModel from .metadata import DatasetMetadata, DatasetMetadataOut, DatasetMetadataUpdate @@ -27,6 +27,7 @@ class DatasetCreateIn(StrictBaseModel): class DatasetUpdateIn(StrictBaseModel): is_downloadable: Optional[bool] metadata: Optional[DatasetMetadataUpdate] + owner_id: Optional[str] class DatasetResponse(Response): diff --git a/app/routes/datasets/dataset.py b/app/routes/datasets/dataset.py index c11eaa363..fef1d5b98 100644 --- a/app/routes/datasets/dataset.py +++ b/app/routes/datasets/dataset.py @@ -21,6 +21,7 @@ ) from ...routes import dataset_dependency from ...settings.globals import READER_USERNAME +from ...utils.rw_api import get_rw_user router = APIRouter() @@ -28,16 +29,22 @@ async def get_owner( dataset: str = Depends(dataset_dependency), user: User = Depends(get_manager) ) -> User: - """Returns the User making the request as long as that user is an admin or - the owner of the dataset, otherwise raises a 401.""" + """Retrieves the user object of the one making the request if that user + either owns the dataset or is an ADMIN, otherwise raises a 401.""" if user.role == "ADMIN": return user dataset_row: ORMDataset = await datasets.get_dataset(dataset) - owner: str = dataset_row.owner_id - if owner != user.id: - raise HTTPException(status_code=401, detail=f"Unauthorized write access to dataset {dataset} (or its versions/assets) by a user who is not an admin or owner of the dataset") + owner_id: str = dataset_row.owner_id + + if owner_id != user.id: + owner = await get_rw_user(owner_id) + raise HTTPException( + status_code=401, + detail=f"Unauthorized write access to dataset {dataset} (or its versions/assets) by a user who is not an admin or owner of the dataset. Please contact the dataset owner ({owner.email}) or an admin to modify the dataset.", + ) + return user @@ -109,16 +116,19 @@ async def update_dataset( *, dataset: str = Depends(dataset_dependency), request: DatasetUpdateIn, - is_authorized: User = Depends(get_owner), + user: User = Depends(get_owner), ) -> DatasetResponse: - """Partially update a dataset. + """Update metadata, accessibility or ownership of a dataset.""" + input_data: Dict = request.dict(exclude_none=True, by_alias=True) - Only metadata field can be updated. All other fields will be - ignored. + if request.owner_id is not None: + new_owner = await get_rw_user(request.owner_id) + if new_owner.role != "ADMIN" and new_owner.role != "MANAGER": + raise HTTPException( + status_code=400, + detail="New owner must be a valid ADMIN or MANAGER.", + ) - Only the dataset owner or a user with `ADMIN` user role can do this operation. - """ - input_data: Dict = request.dict(exclude_none=True, by_alias=True) row: ORMDataset = await datasets.update_dataset(dataset, **input_data) return await _dataset_response(dataset, row) diff --git a/app/utils/rw_api.py b/app/utils/rw_api.py index d580b7a69..f891c9605 100644 --- a/app/utils/rw_api.py +++ b/app/utils/rw_api.py @@ -14,7 +14,7 @@ ) from ..models.pydantic.authentication import User from ..models.pydantic.geostore import Geometry, GeostoreCommon -from ..settings.globals import RW_API_URL +from ..settings.globals import RW_API_URL, SERVICE_ACCOUNT_TOKEN @alru_cache(maxsize=128) @@ -84,6 +84,37 @@ async def who_am_i(token) -> Response: return response +async def get_rw_user(user_id: str) -> User: + """Call RW API to get a user from RW API.""" + + headers = {"Authorization": f"Bearer {SERVICE_ACCOUNT_TOKEN}"} + url = f"{RW_API_URL}/auth/user/{user_id}" + + try: + async with AsyncClient() as client: + response: HTTPXResponse = await client.get( + url, headers=headers, timeout=10.0 + ) + except ReadTimeout: + raise HTTPException( + status_code=500, + detail="Call to user service timed-out. Please try again.", + ) + + if response.status_code == 404: + raise HTTPException(status_code=401, detail=f"User ID invalid: {user_id}") + + if response.status_code != 200: + logger.warning( + f"Failed to authorize user. Server responded with response code: {response.status_code} and message: {response.text}" + ) + raise HTTPException( + status_code=500, detail="Call to user service server failed" + ) + + return User(**response.json()) + + async def login(user_name: str, password: str) -> str: """Obtain a token form RW API using given user name and password.""" diff --git a/tests/__init__.py b/tests/__init__.py index 734822885..18395582b 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -306,6 +306,10 @@ async def is_admin_mocked(): return True +async def not_admin_mocked(): + return False + + async def is_service_account_mocked(): return True @@ -314,17 +318,36 @@ async def get_api_key_mocked() -> Tuple[Optional[str], Optional[str]]: return str(uuid.uuid4()), "localhost" +MANAGER = User( + id="mr_manager123", + name="Mr. Manager", + email="mr_manager@management.com", + createdAt="2021-06-13T03:18:23.000Z", + role="MANAGER", + provider="local", + providerId="123", + extraUserData={}, +) + + +NEW_OWNER = User( + id="new_owner_id123", + name="New Owner", + email="new_owner@owner.com", + createdAt="2021-06-13T03:18:23.000Z", + role="MANAGER", + provider="local", + providerId="1234", + extraUserData={}, +) + + async def get_manager_mocked() -> User: - return User( - id="mr_manager123", - name="Mr. Manager", - email="mr_manager@management.com", - createdAt="2021-06-13T03:18:23.000Z", - role="MANAGER", - provider="local", - providerId="123", - extraUserData={}, - ) + return MANAGER + + +async def get_new_owner_mocked() -> User: + return NEW_OWNER def setup_clients(ec2_client, iam_client): diff --git a/tests/routes/datasets/test_datasets.py b/tests/routes/datasets/test_datasets.py index 5e149ec1e..f88f279a8 100644 --- a/tests/routes/datasets/test_datasets.py +++ b/tests/routes/datasets/test_datasets.py @@ -3,6 +3,9 @@ import pytest from app.application import ContextEngine, db +from app.authentication.token import get_manager +from app.models.pydantic.authentication import User +from tests import MANAGER, NEW_OWNER, get_manager_mocked, get_new_owner_mocked from tests.utils import create_default_asset, dataset_metadata payload = {"metadata": dataset_metadata} @@ -70,16 +73,55 @@ async def test_datasets(async_client): assert len(rows) == 1 assert rows[0][0] == "mr_manager123" - new_payload = {"metadata": {"title": "New Title"}} - response = await async_client.patch(f"/dataset/{dataset}", json=new_payload) - assert response.status_code == 200 - assert response.json()["data"]["metadata"] != payload["metadata"] - assert response.json()["data"]["metadata"]["title"] == "New Title" - assert ( - response.json()["data"]["metadata"]["data_language"] - == payload["metadata"]["data_language"] + with patch("app.routes.datasets.dataset.get_rw_user", return_value=NEW_OWNER): + new_payload = { + "metadata": {"title": "New Title"}, + "owner_id": "new_owner_id123", + } + response = await async_client.patch(f"/dataset/{dataset}", json=new_payload) + assert response.status_code == 200 + assert response.json()["data"]["metadata"] != payload["metadata"] + assert response.json()["data"]["metadata"]["title"] == "New Title" + assert ( + response.json()["data"]["metadata"]["data_language"] + == payload["metadata"]["data_language"] + ) + + async with ContextEngine("READ"): + rows = await db.all( + f"SELECT owner_id FROM datasets WHERE dataset = '{dataset}';" + ) + + assert len(rows) == 1 + assert rows[0][0] == "new_owner_id123" + + # assign original owner back + with patch("app.routes.datasets.dataset.get_rw_user", return_value=MANAGER): + from app.main import app + + app.dependency_overrides[get_manager] = get_new_owner_mocked + new_payload = {"owner_id": "mr_manager123"} + response = await async_client.patch(f"/dataset/{dataset}", json=new_payload) + assert response.status_code == 200 + + app.dependency_overrides[get_manager] = get_manager_mocked + + bad_user = User( + id="bad_user123", + name="Bad User", + email="bad_user@bad.com", + createdAt="2021-06-13T03:18:23.000Z", + role="USER", + provider="local", + providerId="1234", + extraUserData={}, ) + with patch("app.routes.datasets.dataset.get_rw_user", return_value=bad_user): + new_payload = {"owner_id": "bad_user123"} + response = await async_client.patch(f"/dataset/{dataset}", json=new_payload) + assert response.status_code == 400 + response = await async_client.delete(f"/dataset/{dataset}") assert response.status_code == 200 assert response.json()["data"]["dataset"] == "test"