From 6c3b1ea1e791dac7a44899e9e1b92856e9ba54f6 Mon Sep 17 00:00:00 2001 From: Catherine Smith Date: Mon, 2 Dec 2024 14:08:27 +0100 Subject: [PATCH] LA-168: adds support for taxonomy creation specifying a parent key (#5542) --- CHANGELOG.md | 1 + .../api/api/v1/endpoints/generic_overrides.py | 164 +++++++++++------- src/fides/api/schemas/taxonomy_extensions.py | 4 +- .../v1/endpoints/test_taxonomy_overrides.py | 43 ++++- 4 files changed, 140 insertions(+), 72 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a84410f2c9..52877ac02c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ The types of changes are: ### Fixed - Updating dataset PUT to allow deleting all datasets [#5524](https://github.com/ethyca/fides/pull/5524) +- Adds support for fides_key generation when parent_key is provided in Taxonomy create endpoints [#5542](https://github.com/ethyca/fides/pull/5542) ### Removed - Removed unnecessary debug logging from the load_file config helper [#5544](https://github.com/ethyca/fides/pull/5544) diff --git a/src/fides/api/api/v1/endpoints/generic_overrides.py b/src/fides/api/api/v1/endpoints/generic_overrides.py index 0635810f9f..e52d20bd5d 100644 --- a/src/fides/api/api/v1/endpoints/generic_overrides.py +++ b/src/fides/api/api/v1/endpoints/generic_overrides.py @@ -1,4 +1,4 @@ -from typing import Dict, List, Optional, Union +from typing import Dict, List, Optional, Type, Union from fastapi import APIRouter, Depends, HTTPException, Query, Security from fastapi_pagination import Page, Params @@ -110,6 +110,69 @@ async def list_dataset_paginated( return await async_paginate(db, filtered_query, pagination_params) +def validate_and_create_taxonomy( + db: Session, + model: Union[ + Type[DataCategoryDbModel], Type[DataUseDbModel], Type[DataSubjectDbModel] + ], + validation_schema: type, + data: Union[DataCategoryCreate, DataUseCreate, DataSubjectCreate], +) -> Dict: + """ + Validate and create a taxonomy element. + """ + validated_taxonomy = validation_schema(**data.model_dump(mode="json")) + return model.create(db=db, data=validated_taxonomy.model_dump(mode="json")) + + +def validate_and_update_taxonomy( + db: Session, + resource: Union[DataCategoryDbModel, DataUseDbModel, DataSubjectDbModel], + validation_schema: type, + data: Union[DataCategoryCreate, DataUseCreate, DataSubjectCreate], +) -> Dict: + """ + Validate and update a taxonomy element. + """ + validated_taxonomy = validation_schema(**data.model_dump(mode="json")) + return resource.update(db=db, data=validated_taxonomy.model_dump(mode="json")) + + +def create_or_update_taxonomy( + db: Session, + data: Union[DataCategoryCreate, DataUseCreate, DataSubjectCreate], + model: Union[ + Type[DataCategoryDbModel], Type[DataUseDbModel], Type[DataSubjectDbModel] + ], + validation_schema: type, +) -> Dict: + """ + Create or update a taxonomy element. If the element is disabled, it will be updated and re-enabled. + """ + if data.fides_key is None: + disabled_resource_with_name = ( + db.query(model) + .filter( + model.active.is_(False), + model.name == data.name, + ) + .first() + ) + data.fides_key = get_key_from_data( + {"key": data.fides_key, "name": data.name}, validation_schema.__name__ + ) + if data.parent_key if hasattr(data, "parent_key") else None: + data.fides_key = f"{data.parent_key}.{data.fides_key}" # type: ignore[union-attr] + if disabled_resource_with_name: + data.active = True + return validate_and_update_taxonomy( + db, disabled_resource_with_name, validation_schema, data + ) + return validate_and_create_taxonomy(db, model, validation_schema, data) + + return validate_and_create_taxonomy(db, model, validation_schema, data) + + @data_use_router.post( "/data_use", dependencies=[Security(verify_oauth_client, scopes=[DATA_USE_CREATE])], @@ -124,29 +187,18 @@ async def create_data_use( """ Create a data use. Updates existing data use if data use with name already exists and is disabled. """ - if data_use.fides_key is None: - disabled_resource_with_name = ( - db.query(DataUseDbModel) - .filter( - DataUseDbModel.active.is_(False), - DataUseDbModel.name == data_use.name, - ) - .first() + try: + return create_or_update_taxonomy(db, data_use, DataUseDbModel, DataUse) + except KeyOrNameAlreadyExists: + raise HTTPException( + status_code=HTTP_422_UNPROCESSABLE_ENTITY, + detail=f"Data use with key {data_use.fides_key} or name {data_use.name} already exists.", ) - data_use.fides_key = get_key_from_data( - {"key": data_use.fides_key, "name": data_use.name}, DataUse.__name__ + except Exception as e: + raise HTTPException( + status_code=HTTP_422_UNPROCESSABLE_ENTITY, + detail=f"Error creating data use: {e}", ) - if disabled_resource_with_name: - data_use.active = True - return disabled_resource_with_name.update(db, data=data_use.model_dump(mode="json")) # type: ignore[union-attr] - try: - return DataUseDbModel.create(db=db, data=data_use.model_dump(mode="json")) # type: ignore[union-attr] - except KeyOrNameAlreadyExists: - raise HTTPException( - status_code=HTTP_422_UNPROCESSABLE_ENTITY, - detail=f"Data use with key {data_use.fides_key} or name {data_use.name} already exists.", - ) - return DataUseDbModel.create(db=db, data=data_use.model_dump(mode="json")) @data_category_router.post( @@ -164,30 +216,20 @@ async def create_data_category( Create a data category """ - if data_category.fides_key is None: - disabled_resource_with_name = ( - db.query(DataCategoryDbModel) - .filter( - DataCategoryDbModel.active.is_(False), - DataCategoryDbModel.name == data_category.name, - ) - .first() + try: + return create_or_update_taxonomy( + db, data_category, DataCategoryDbModel, DataCategory ) - data_category.fides_key = get_key_from_data( - {"key": data_category.fides_key, "name": data_category.name}, - DataCategory.__name__, + except KeyOrNameAlreadyExists: + raise HTTPException( + status_code=HTTP_422_UNPROCESSABLE_ENTITY, + detail=f"Data category with key {data_category.fides_key} or name {data_category.name} already exists.", + ) + except Exception as e: + raise HTTPException( + status_code=HTTP_422_UNPROCESSABLE_ENTITY, + detail=f"Error creating data category: {e}", ) - if disabled_resource_with_name: - data_category.active = True - return disabled_resource_with_name.update(db, data=data_category.model_dump(mode="json")) # type: ignore[union-attr] - try: - return DataCategoryDbModel.create(db=db, data=data_category.model_dump(mode="json")) # type: ignore[union-attr] - except KeyOrNameAlreadyExists: - raise HTTPException( - status_code=HTTP_422_UNPROCESSABLE_ENTITY, - detail=f"Data category with key {data_category.fides_key} or name {data_category.name} already exists.", - ) - return DataCategoryDbModel.create(db=db, data=data_category.model_dump(mode="json")) @data_subject_router.post( @@ -205,30 +247,20 @@ async def create_data_subject( Create a data subject """ - if data_subject.fides_key is None: - disabled_resource_with_name = ( - db.query(DataSubjectDbModel) - .filter( - DataSubjectDbModel.active.is_(False), - DataSubjectDbModel.name == data_subject.name, - ) - .first() + try: + return create_or_update_taxonomy( + db, data_subject, DataSubjectDbModel, DataSubject ) - data_subject.fides_key = get_key_from_data( - {"key": data_subject.fides_key, "name": data_subject.name}, - DataSubject.__name__, + except KeyOrNameAlreadyExists: + raise HTTPException( + status_code=HTTP_422_UNPROCESSABLE_ENTITY, + detail=f"Data subject with key {data_subject.fides_key} or name {data_subject.name} already exists.", + ) + except Exception as e: + raise HTTPException( + status_code=HTTP_422_UNPROCESSABLE_ENTITY, + detail=f"Error creating data subject: {e}", ) - if disabled_resource_with_name: - data_subject.active = True - return disabled_resource_with_name.update(db, data=data_subject.model_dump(mode="json")) # type: ignore[union-attr] - try: - return DataSubjectDbModel.create(db=db, data=data_subject.model_dump(mode="json")) # type: ignore[union-attr] - except KeyOrNameAlreadyExists: - raise HTTPException( - status_code=HTTP_422_UNPROCESSABLE_ENTITY, - detail=f"Data subject with key {data_subject.fides_key} or name {data_subject.name} already exists.", - ) - return DataSubjectDbModel.create(db=db, data=data_subject.model_dump(mode="json")) GENERIC_OVERRIDES_ROUTER = APIRouter() diff --git a/src/fides/api/schemas/taxonomy_extensions.py b/src/fides/api/schemas/taxonomy_extensions.py index 2fb5614006..cb9c956a40 100644 --- a/src/fides/api/schemas/taxonomy_extensions.py +++ b/src/fides/api/schemas/taxonomy_extensions.py @@ -8,6 +8,7 @@ from fideslang.models import DataSubject as BaseDataSubject from fideslang.models import DataSubjectRights from fideslang.models import DataUse as BaseDataUse +from fideslang.models import DefaultModel from fideslang.validation import FidesKey from pydantic import BaseModel, Field @@ -28,12 +29,11 @@ class DataSubject(BaseDataSubject): active: bool = active_field -class TaxonomyCreateBase(BaseModel): +class TaxonomyCreateBase(DefaultModel, BaseModel): name: Optional[str] = None description: str active: bool = True fides_key: Optional[FidesKey] = None - is_default: bool = False tags: Optional[List[str]] = None organization_fides_key: Optional[FidesKey] = "default_organization" diff --git a/tests/ops/api/v1/endpoints/test_taxonomy_overrides.py b/tests/ops/api/v1/endpoints/test_taxonomy_overrides.py index 1de888f6cb..07c9fb0ac5 100644 --- a/tests/ops/api/v1/endpoints/test_taxonomy_overrides.py +++ b/tests/ops/api/v1/endpoints/test_taxonomy_overrides.py @@ -69,7 +69,7 @@ def test_create_data_use_incorrect_scope( response = api_client.post(url, headers=auth_header, json=payload) assert 403 == response.status_code - def test_create_data_use_with_fides_key( + def test_create_data_use_with_fides_key_and_parent_key( self, db: Session, api_client: TestClient, @@ -78,16 +78,32 @@ def test_create_data_use_with_fides_key( generate_auth_header, ): auth_header = generate_auth_header([DATA_USE_CREATE]) - payload["fides_key"] = "test_data_use" + payload["fides_key"] = "analytics.test_data_use" + payload["parent_key"] = "analytics" response = api_client.post(url, headers=auth_header, json=payload) assert 201 == response.status_code response_body = json.loads(response.text) - assert response_body["fides_key"] == "test_data_use" - data_use = db.query(DataUse).filter_by(fides_key="test_data_use")[0] + assert response_body["fides_key"] == "analytics.test_data_use" + data_use = db.query(DataUse).filter_by(fides_key="analytics.test_data_use")[0] data_use.delete(db) + def test_create_data_use_with_fides_key_and_non_matching_parent_key( + self, + db: Session, + api_client: TestClient, + payload, + url, + generate_auth_header, + ): + payload["fides_key"] = "analytics.test_data_use" + payload["parent_key"] = "invalid_parent" + auth_header = generate_auth_header([DATA_USE_CREATE]) + response = api_client.post(url, headers=auth_header, json=payload) + + assert 422 == response.status_code + def test_create_data_use_with_no_fides_key( self, db: Session, @@ -106,6 +122,25 @@ def test_create_data_use_with_no_fides_key( data_use = db.query(DataUse).filter_by(fides_key="test_data_use")[0] data_use.delete(db) + def test_create_data_use_with_no_fides_key_and_has_parent_key( + self, + db: Session, + api_client: TestClient, + payload, + url, + generate_auth_header, + ): + auth_header = generate_auth_header([DATA_USE_CREATE]) + payload["parent_key"] = "analytics" + response = api_client.post(url, headers=auth_header, json=payload) + + assert 201 == response.status_code + response_body = json.loads(response.text) + + assert response_body["fides_key"] == "analytics.test_data_use" + data_use = db.query(DataUse).filter_by(fides_key="analytics.test_data_use")[0] + data_use.delete(db) + def test_create_data_use_with_conflicting_key( self, db: Session,