Skip to content

Commit

Permalink
Add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
jterry64 committed May 10, 2024
2 parents 7db3eab + 0c3f5f8 commit 41db670
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 26 deletions.
2 changes: 1 addition & 1 deletion app/authentication/token.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ async def get_manager(user: User = Depends(get_user)) -> User:
"""Get the details for authenticated MANAGER for data-api application or
ADMIN user."""

if user.role != "ADMIN" or user.role != "MANAGER":
if user.role != "ADMIN" and user.role != "MANAGER":
raise HTTPException(status_code=401, detail="Unauthorized")

return user
3 changes: 2 additions & 1 deletion app/models/pydantic/authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ class User(StrictBaseModel):
email: EmailStr
createdAt: datetime
role: str
applications: List[str]
provider: str
providerId: Optional[str]
extraUserData: Dict[str, Any]


Expand Down
11 changes: 7 additions & 4 deletions app/routes/datasets/dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,13 @@ async def get_owner(
"""Retrieves the user object that owns the dataset, or ADMIN user, if that
user is the one making the request, 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 and owner.role != "ADMIN":
if owner != user.id:
raise HTTPException(status_code=401, detail="Unauthorized")
return user

Expand Down Expand Up @@ -120,10 +123,10 @@ async def update_dataset(
input_data: Dict = request.dict(exclude_none=True, by_alias=True)

if request.owner_id is not None:
new_owner = get_rw_user(request.owner_id)
if new_owner.role != "ADMIN" or new_owner.role != "MANAGER":
new_owner = await get_rw_user(request.owner_id)
if new_owner.role != "ADMIN" and new_owner.role != "MANAGER":
raise HTTPException(
status_code=401,
status_code=400,
detail="New owner must be a valid ADMIN or MANAGER.",
)

Expand Down
2 changes: 1 addition & 1 deletion app/utils/rw_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ async def get_rw_user(user_id: str) -> User:
status_code=500, detail="Call to user service server failed"
)

return User(**response)
return User(**response.json())


async def login(user_name: str, password: str) -> str:
Expand Down
19 changes: 18 additions & 1 deletion tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -325,11 +325,28 @@ async def get_manager_mocked() -> User:
email="[email protected]",
createdAt="2021-06-13T03:18:23.000Z",
role="MANAGER",
applications=[],
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_new_owner_mocked() -> User:
return NEW_OWNER


def setup_clients(ec2_client, iam_client):
"""
Do prerequisite setup
Expand Down
63 changes: 48 additions & 15 deletions tests/routes/datasets/test_datasets.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 NEW_OWNER, get_manager_mocked, get_new_owner_mocked
from tests.utils import create_default_asset, dataset_metadata

payload = {"metadata": dataset_metadata}
Expand Down Expand Up @@ -70,23 +73,53 @@ async def test_datasets(async_client):
assert len(rows) == 1
assert rows[0][0] == "mr_manager123"

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}';"
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"]
)

assert len(rows) == 1
assert rows[0][0] == "new_owner_id123"
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
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
Expand Down
9 changes: 6 additions & 3 deletions tests_v2/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ async def get_user_mocked() -> User:
email="[email protected]",
createdAt="2021-06-13T03:18:23.000Z",
role="USER",
applications=[],
provider="local",
providerId="1234",
extraUserData={},
)

Expand All @@ -52,7 +53,8 @@ async def get_admin_mocked() -> User:
email="[email protected]",
createdAt="2021-06-13T03:18:23.000Z",
role="ADMIN",
applications=[],
provider="google",
providerId="1234",
extraUserData={},
)

Expand All @@ -64,7 +66,8 @@ async def get_manager_mocked() -> User:
email="[email protected]",
createdAt="2021-06-13T03:18:23.000Z",
role="MANAGER",
applications=[],
provider="local",
providerId="1234",
extraUserData={},
)

Expand Down

0 comments on commit 41db670

Please sign in to comment.