From 2df390f5591933cc7cdfa0683693f3bb396354a1 Mon Sep 17 00:00:00 2001 From: florimondmanca Date: Tue, 20 Sep 2022 11:41:25 +0200 Subject: [PATCH] Switch to POST /catalogs/:siret/datasets/ --- client/src/lib/repositories/datasets.ts | 4 +- .../src/routes/(app)/contribuer/+page.svelte | 1 + client/src/tests/e2e/fixtures.ts | 6 +- server/api/datasets/routes.py | 34 ++-- server/api/datasets/schemas.py | 3 - server/api/organizations/permissions.py | 22 +++ server/application/datasets/handlers.py | 16 +- tests/api/test_datasets.py | 145 ++++++++++++++---- tests/conftest.py | 7 +- tests/helpers.py | 8 +- 10 files changed, 179 insertions(+), 67 deletions(-) create mode 100644 server/api/organizations/permissions.py diff --git a/client/src/lib/repositories/datasets.ts b/client/src/lib/repositories/datasets.ts index 8cfdf0a8..9a859aa8 100644 --- a/client/src/lib/repositories/datasets.ts +++ b/client/src/lib/repositories/datasets.ts @@ -84,16 +84,18 @@ export const getDatasets: GetDatasets = async ({ type CreateDataset = (opts: { fetch: Fetch; apiToken: string; + siret: string; data: DatasetCreateData; }) => Promise>; export const createDataset: CreateDataset = async ({ fetch, apiToken, + siret, data, }) => { const body = JSON.stringify(toPayload(data)); - const url = `${getApiUrl()}/datasets/`; + const url = `${getApiUrl()}/catalogs/${siret}/datasets/`; const request = new Request(url, { method: "POST", headers: new Headers([ diff --git a/client/src/routes/(app)/contribuer/+page.svelte b/client/src/routes/(app)/contribuer/+page.svelte index aabe63b7..e7abf552 100644 --- a/client/src/routes/(app)/contribuer/+page.svelte +++ b/client/src/routes/(app)/contribuer/+page.svelte @@ -27,6 +27,7 @@ const dataset = await createDataset({ fetch, apiToken: $apiTokenStore, + siret: Maybe.expect(catalog, "catalog").organization.siret, data: { tagIds, ...event.detail }, }); diff --git a/client/src/tests/e2e/fixtures.ts b/client/src/tests/e2e/fixtures.ts index aeff0d7c..dfe77eb9 100644 --- a/client/src/tests/e2e/fixtures.ts +++ b/client/src/tests/e2e/fixtures.ts @@ -57,8 +57,9 @@ export const test = base.extend({ dataset: async ({ apiContext, adminApiToken }, use) => { const headers = { Authorization: `Bearer ${adminApiToken}` }; + const siret = "11004601800013"; + const data = { - organization_siret: "11004601800013", title: "Sample title", description: "Sample description", formats: ["api"], @@ -77,7 +78,8 @@ export const test = base.extend({ }, ], }; - let response = await apiContext.post("/datasets/", { + + let response = await apiContext.post(`/catalogs/${siret}/datasets/`, { data, headers, }); diff --git a/server/api/datasets/routes.py b/server/api/datasets/routes.py index 7f8a898b..409163af 100644 --- a/server/api/datasets/routes.py +++ b/server/api/datasets/routes.py @@ -2,6 +2,7 @@ from fastapi.exceptions import HTTPException from starlette.responses import Response +from server.api.organizations.permissions import MatchesUserSiret from server.application.datasets.commands import ( CreateDataset, DeleteDataset, @@ -11,24 +12,25 @@ from server.application.datasets.views import DatasetView from server.config.di import resolve from server.domain.auth.entities import UserRole +from server.domain.catalogs.exceptions import CatalogDoesNotExist from server.domain.common.pagination import Page, Pagination from server.domain.common.types import ID from server.domain.datasets.exceptions import DatasetDoesNotExist from server.domain.datasets.specifications import DatasetSpec -from server.domain.organizations.exceptions import OrganizationDoesNotExist +from server.domain.organizations.types import Siret from server.seedwork.application.messages import MessageBus from ..auth.permissions import HasRole, IsAuthenticated from . import filters from .schemas import DatasetCreate, DatasetListParams, DatasetUpdate -router = APIRouter(prefix="/datasets", tags=["datasets"]) +router = APIRouter(tags=["datasets"]) -router.include_router(filters.router) +router.include_router(filters.router, prefix="/datasets") @router.get( - "/", + "/datasets/", dependencies=[Depends(IsAuthenticated())], response_model=Pagination[DatasetView], ) @@ -56,7 +58,7 @@ async def list_datasets( @router.get( - "/{id}/", + "/datasets/{id}/", dependencies=[Depends(IsAuthenticated())], response_model=DatasetView, responses={404: {}}, @@ -72,19 +74,27 @@ async def get_dataset_by_id(id: ID) -> DatasetView: @router.post( - "/", - dependencies=[Depends(IsAuthenticated())], + "/catalogs/{siret}/datasets/", + dependencies=[ + Depends( + IsAuthenticated() + & ( + HasRole(UserRole.ADMIN) + | MatchesUserSiret(key=lambda r: r.path_params["siret"]) + ) + ) + ], response_model=DatasetView, status_code=201, ) -async def create_dataset(data: DatasetCreate) -> DatasetView: +async def create_dataset(siret: Siret, data: DatasetCreate) -> DatasetView: bus = resolve(MessageBus) - command = CreateDataset(**data.dict()) + command = CreateDataset(organization_siret=siret, **data.dict()) try: id = await bus.execute(command) - except OrganizationDoesNotExist as exc: + except CatalogDoesNotExist as exc: raise HTTPException(400, detail=str(exc)) query = GetDatasetByID(id=id) @@ -92,7 +102,7 @@ async def create_dataset(data: DatasetCreate) -> DatasetView: @router.put( - "/{id}/", + "/datasets/{id}/", dependencies=[Depends(IsAuthenticated())], response_model=DatasetView, responses={404: {}}, @@ -112,7 +122,7 @@ async def update_dataset(id: ID, data: DatasetUpdate) -> DatasetView: @router.delete( - "/{id}/", + "/datasets/{id}/", dependencies=[Depends(IsAuthenticated() & HasRole(UserRole.ADMIN))], status_code=204, response_class=Response, diff --git a/server/api/datasets/schemas.py b/server/api/datasets/schemas.py index 28e6a1d6..cde82f22 100644 --- a/server/api/datasets/schemas.py +++ b/server/api/datasets/schemas.py @@ -11,8 +11,6 @@ ) from server.domain.common.types import ID from server.domain.datasets.entities import DataFormat, UpdateFrequency -from server.domain.organizations.entities import LEGACY_ORGANIZATION -from server.domain.organizations.types import Siret class DatasetListParams: @@ -40,7 +38,6 @@ def __init__( class DatasetCreate(CreateDatasetValidationMixin, BaseModel): - organization_siret: Siret = LEGACY_ORGANIZATION.siret title: str description: str service: str diff --git a/server/api/organizations/permissions.py b/server/api/organizations/permissions.py new file mode 100644 index 00000000..2cc95dac --- /dev/null +++ b/server/api/organizations/permissions.py @@ -0,0 +1,22 @@ +from typing import Callable + +from ..auth.permissions import BasePermission +from ..types import APIRequest + + +class MatchesUserSiret(BasePermission): + def __init__(self, key: Callable[[APIRequest], str]) -> None: + super().__init__() + self._key = key + + def has_permission(self, request: APIRequest) -> bool: + if not request.user.is_authenticated: + raise RuntimeError( + "Running BelongsToOrganization but user is not authenticated. " + "Hint: use IsAuthenticated() & BelongsToOrganization(...)" + ) + + user_siret = request.user.account.organization_siret + claimed_siret = self._key(request) + + return user_siret == claimed_siret diff --git a/server/application/datasets/handlers.py b/server/application/datasets/handlers.py index f69c7d62..882099b1 100644 --- a/server/application/datasets/handlers.py +++ b/server/application/datasets/handlers.py @@ -3,13 +3,13 @@ from server.config.di import resolve from server.domain.catalog_records.entities import CatalogRecord from server.domain.catalog_records.repositories import CatalogRecordRepository +from server.domain.catalogs.exceptions import CatalogDoesNotExist +from server.domain.catalogs.repositories import CatalogRepository from server.domain.common.pagination import Pagination from server.domain.common.types import ID from server.domain.datasets.entities import DataFormat, Dataset from server.domain.datasets.exceptions import DatasetDoesNotExist from server.domain.datasets.repositories import DatasetRepository -from server.domain.organizations.exceptions import OrganizationDoesNotExist -from server.domain.organizations.repositories import OrganizationRepository from server.domain.tags.repositories import TagRepository from server.seedwork.application.messages import MessageBus @@ -20,24 +20,22 @@ async def create_dataset(command: CreateDataset, *, id_: ID = None) -> ID: repository = resolve(DatasetRepository) - organization_repository = resolve(OrganizationRepository) + catalog_repository = resolve(CatalogRepository) catalog_record_repository = resolve(CatalogRecordRepository) tag_repository = resolve(TagRepository) if id_ is None: id_ = repository.make_id() - organization = await organization_repository.get_by_siret( - siret=command.organization_siret - ) + catalog = await catalog_repository.get_by_siret(siret=command.organization_siret) - if organization is None: - raise OrganizationDoesNotExist(command.organization_siret) + if catalog is None: + raise CatalogDoesNotExist(command.organization_siret) catalog_record_id = await catalog_record_repository.insert( CatalogRecord( id=catalog_record_repository.make_id(), - organization=organization, + organization=catalog.organization, ) ) catalog_record = await catalog_record_repository.get_by_id(catalog_record_id) diff --git a/tests/api/test_datasets.py b/tests/api/test_datasets.py index 086f1211..cfe6f559 100644 --- a/tests/api/test_datasets.py +++ b/tests/api/test_datasets.py @@ -1,5 +1,5 @@ import random -from typing import Any, List +from typing import Any, List, Tuple import httpx import pytest @@ -8,6 +8,8 @@ from server.application.catalogs.commands import CreateCatalog from server.application.catalogs.queries import GetCatalogBySiret from server.application.datasets.queries import GetDatasetByID +from server.application.organizations.queries import GetOrganizationBySiret +from server.application.organizations.views import OrganizationView from server.application.tags.commands import CreateTag from server.application.tags.queries import GetTagByID from server.config.di import resolve @@ -22,8 +24,31 @@ from server.seedwork.application.messages import MessageBus from tests.factories import CreateDatasetFactory -from ..factories import CreateOrganizationFactory, UpdateDatasetFactory, fake -from ..helpers import TestPasswordUser, to_payload +from ..factories import ( + CreateOrganizationFactory, + CreatePasswordUserFactory, + UpdateDatasetFactory, + fake, +) +from ..helpers import TestPasswordUser, create_test_password_user, to_payload + + +async def _setup( + skip_catalog: bool = False, +) -> Tuple[OrganizationView, TestPasswordUser]: + bus = resolve(MessageBus) + + siret = await bus.execute(CreateOrganizationFactory.build()) + org = await bus.execute(GetOrganizationBySiret(siret=siret)) + + user = await create_test_password_user( + CreatePasswordUserFactory.build(organization_siret=siret) + ) + + if not skip_catalog: + await bus.execute(CreateCatalog(organization_siret=siret)) + + return org, user @pytest.mark.asyncio @@ -83,11 +108,14 @@ ) async def test_create_dataset_invalid( client: httpx.AsyncClient, - temp_user: TestPasswordUser, payload: dict, expected_errors_attrs: list, ) -> None: - response = await client.post("/datasets/", json=payload, auth=temp_user.auth) + org, user = await _setup() + + response = await client.post( + f"/catalogs/{org.siret}/datasets/", json=payload, auth=user.auth + ) assert response.status_code == 422 data = response.json() @@ -99,21 +127,26 @@ async def test_create_dataset_invalid( @pytest.mark.asyncio -async def test_create_dataset_invalid_organization_does_not_exist( - client: httpx.AsyncClient, temp_user: TestPasswordUser +async def test_create_dataset_invalid_catalog_does_not_exist( + client: httpx.AsyncClient, ) -> None: - siret = Siret(fake.siret()) - payload = to_payload(CreateDatasetFactory.build(organization_siret=siret)) - response = await client.post("/datasets/", json=payload, auth=temp_user.auth) + org, user = await _setup(skip_catalog=True) + + payload = to_payload(CreateDatasetFactory.build()) + response = await client.post( + f"/catalogs/{org.siret}/datasets/", json=payload, auth=user.auth + ) assert response.status_code == 400 data = response.json() - assert data["detail"] == f"Organization not found: '{siret}'" + assert data["detail"] == f"Catalog not found: '{org.siret}'" @pytest.mark.asyncio async def test_dataset_crud( - client: httpx.AsyncClient, temp_user: TestPasswordUser, admin_user: TestPasswordUser + client: httpx.AsyncClient, admin_user: TestPasswordUser ) -> None: + org, user = await _setup() + last_updated_at = fake.date_time_tz() payload = to_payload( @@ -134,7 +167,9 @@ async def test_dataset_crud( ) ) - response = await client.post("/datasets/", json=payload, auth=temp_user.auth) + response = await client.post( + f"/catalogs/{org.siret}/datasets/", json=payload, auth=user.auth + ) assert response.status_code == 201 data = response.json() @@ -145,7 +180,10 @@ async def test_dataset_crud( "id": pk, "catalog_record": { **data["catalog_record"], - "organization": LEGACY_ORGANIZATION.dict(), + "organization": { + "siret": str(org.siret), + "name": org.name, + }, }, "title": "Example title", "description": "Example description", @@ -166,24 +204,24 @@ async def test_dataset_crud( non_existing_id = id_factory() - response = await client.get(f"/datasets/{non_existing_id}/", auth=temp_user.auth) + response = await client.get(f"/datasets/{non_existing_id}/", auth=user.auth) assert response.status_code == 404 - response = await client.get(f"/datasets/{pk}/", auth=temp_user.auth) + response = await client.get(f"/datasets/{pk}/", auth=user.auth) assert response.status_code == 200 assert response.json() == data - response = await client.get("/datasets/", auth=temp_user.auth) + response = await client.get("/datasets/", auth=user.auth) assert response.status_code == 200 assert response.json()["items"] == [data] response = await client.delete(f"/datasets/{pk}/", auth=admin_user.auth) assert response.status_code == 204 - response = await client.get(f"/datasets/{pk}/", auth=temp_user.auth) + response = await client.get(f"/datasets/{pk}/", auth=user.auth) assert response.status_code == 404 - response = await client.get("/datasets/", auth=temp_user.auth) + response = await client.get("/datasets/", auth=user.auth) assert response.status_code == 200 assert response.json()["items"] == [] @@ -191,12 +229,41 @@ async def test_dataset_crud( @pytest.mark.asyncio class TestDatasetPermissions: async def test_create_not_authenticated(self, client: httpx.AsyncClient) -> None: + org, _ = await _setup() + response = await client.post( - "/datasets/", + f"/catalogs/{org.siret}/datasets/", json=to_payload(CreateDatasetFactory.build()), ) assert response.status_code == 401 + async def test_create_foreign_org_denied(self, client: httpx.AsyncClient) -> None: + bus = resolve(MessageBus) + + _, user1 = await _setup() + + siret2 = await bus.execute(CreateOrganizationFactory.build()) + await bus.execute(CreateCatalog(organization_siret=siret2)) + + payload = to_payload(CreateDatasetFactory.build()) + response = await client.post( + f"/catalogs/{siret2}/datasets/", json=payload, auth=user1.auth + ) + + assert response.status_code == 403 + + async def test_create_foreign_org_admin_ok( + self, client: httpx.AsyncClient, admin_user: TestPasswordUser + ) -> None: + org, _ = await _setup() + assert org.siret != admin_user.account.organization_siret + + payload = to_payload(CreateDatasetFactory.build()) + response = await client.post( + f"/catalogs/{org.siret}/datasets/", json=payload, auth=admin_user.auth + ) + assert response.status_code == 201 + async def test_get_not_authenticated(self, client: httpx.AsyncClient) -> None: pk = id_factory() response = await client.get(f"/datasets/{pk}/") @@ -328,29 +395,32 @@ class TestDatasetOptionalFields: async def test_optional_fields_missing_uses_defaults( self, client: httpx.AsyncClient, - temp_user: TestPasswordUser, field: str, default: Any, ) -> None: + org, user = await _setup() + payload = to_payload(CreateDatasetFactory.build()) payload.pop(field) - response = await client.post("/datasets/", json=payload, auth=temp_user.auth) + response = await client.post( + f"/catalogs/{org.siret}/datasets/", json=payload, auth=user.auth + ) assert response.status_code == 201 dataset = response.json() assert dataset[field] == default - async def test_optional_fields_invalid( - self, client: httpx.AsyncClient, temp_user: TestPasswordUser - ) -> None: + async def test_optional_fields_invalid(self, client: httpx.AsyncClient) -> None: + org, user = await _setup() + response = await client.post( - "/datasets/", + f"/catalogs/{org.siret}/datasets/", json={ **to_payload(CreateDatasetFactory.build()), "contact_emails": ["notanemail", "valid@mydomain.org"], "update_frequency": "not_in_enum", "last_updated_at": "not_a_datetime", }, - auth=temp_user.auth, + auth=user.auth, ) assert response.status_code == 422 ( @@ -638,7 +708,7 @@ async def test_tags_remove( @pytest.mark.asyncio class TestExtraFieldValues: - async def _create_extra_field_in_catalog(self) -> ID: + async def _setup(self) -> Tuple[Siret, TestPasswordUser, ID]: bus = resolve(MessageBus) siret = await bus.execute(CreateOrganizationFactory.build()) @@ -657,12 +727,17 @@ async def _create_extra_field_in_catalog(self) -> ID: ) catalog = await bus.execute(GetCatalogBySiret(siret=siret)) - return catalog.extra_fields[0].id + + user = await create_test_password_user( + CreatePasswordUserFactory.build(organization_siret=siret) + ) + + return siret, user, catalog.extra_fields[0].id async def test_create_dataset_with_extra_field_values( - self, client: httpx.AsyncClient, temp_user: TestPasswordUser + self, client: httpx.AsyncClient ) -> None: - extra_field_id = await self._create_extra_field_in_catalog() + siret, user, extra_field_id = await self._setup() payload = to_payload( CreateDatasetFactory.build( @@ -671,7 +746,9 @@ async def test_create_dataset_with_extra_field_values( ] ) ) - response = await client.post("/datasets/", json=payload, auth=temp_user.auth) + response = await client.post( + f"/catalogs/{siret}/datasets/", json=payload, auth=user.auth + ) assert response.status_code == 201 data = response.json() assert data["extra_field_values"] == [ @@ -685,7 +762,7 @@ async def test_add_extra_field_value( self, client: httpx.AsyncClient, temp_user: TestPasswordUser ) -> None: bus = resolve(MessageBus) - extra_field_id = await self._create_extra_field_in_catalog() + _, _, extra_field_id = await self._setup() command = CreateDatasetFactory.build() dataset_id = await bus.execute(command) @@ -720,7 +797,7 @@ async def test_remove_extra_field_value( self, client: httpx.AsyncClient, temp_user: TestPasswordUser ) -> None: bus = resolve(MessageBus) - extra_field_id = await self._create_extra_field_in_catalog() + _, _, extra_field_id = await self._setup() command = CreateDatasetFactory.build( extra_field_values=[ diff --git a/tests/conftest.py b/tests/conftest.py index bad40c20..5475d238 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -20,6 +20,7 @@ from server.seedwork.application.messages import MessageBus from tests.factories import CreateTagFactory +from .factories import CreatePasswordUserFactory from .helpers import TestPasswordUser, create_client, create_test_password_user if TYPE_CHECKING: @@ -111,9 +112,11 @@ async def client(app: "App") -> AsyncIterator[httpx.AsyncClient]: @pytest_asyncio.fixture(name="temp_user") async def fixture_temp_user() -> TestPasswordUser: - return await create_test_password_user(UserRole.USER) + command = CreatePasswordUserFactory.build() + return await create_test_password_user(command, role=UserRole.USER) @pytest_asyncio.fixture async def admin_user() -> TestPasswordUser: - return await create_test_password_user(UserRole.ADMIN) + command = CreatePasswordUserFactory.build() + return await create_test_password_user(command, role=UserRole.ADMIN) diff --git a/tests/helpers.py b/tests/helpers.py index 42e2d340..1073f003 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -4,13 +4,12 @@ import httpx from pydantic import BaseModel +from server.application.auth.commands import CreatePasswordUser from server.config.di import resolve from server.domain.auth.entities import PasswordUser, UserRole from server.domain.auth.repositories import PasswordUserRepository from server.seedwork.application.messages import MessageBus -from .factories import CreatePasswordUserFactory - def create_client(app: Callable) -> httpx.AsyncClient: transport = httpx.ASGITransport( @@ -46,11 +45,12 @@ def auth(self, request: httpx.Request) -> httpx.Request: return request -async def create_test_password_user(role: UserRole) -> TestPasswordUser: +async def create_test_password_user( + command: CreatePasswordUser, *, role: UserRole = UserRole.USER +) -> TestPasswordUser: bus = resolve(MessageBus) password_user_repository = resolve(PasswordUserRepository) - command = CreatePasswordUserFactory.build() await bus.execute(command, role=role) user = await password_user_repository.get_by_email(command.email)