Allow reassigning dataset owner (#505)

Allow reassigning data owner.

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):

app/routes/datasets/dataset.py

@@ -21,23 +21,30 @@
 )
 from ...routes import dataset_dependency
 from ...settings.globals import READER_USERNAME
+from ...utils.rw_api import get_rw_user
 
 router = APIRouter()
 
 
 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)

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."""
 

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="[email protected]",
+    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="[email protected]",
+    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="[email protected]",
-        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):

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="[email protected]",
+        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"