From a6c099ff1c2e3957aa3d11818f1f938e0b79110a Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Tue, 28 Nov 2023 17:26:34 -0600 Subject: [PATCH] WIP Refactor so instance-level hybrid properties on PrivacyDeclarations use async sessions so they can be accessed via existing System API routes. Current strategy is to stash these on the declaration under a new key since declaring the hybrid_property directly on the Privacy Declaration Schema causes coroutine issues when the PrivacyDeclaration is serialized under "serialize_response" --- src/fides/api/api/v1/endpoints/system.py | 35 ++- src/fides/api/models/sql_models.py | 47 +-- src/fides/api/schemas/system.py | 4 + tests/ctl/core/test_api.py | 279 +++++++++++++++++- .../ops/util/test_tcf_privacy_declarations.py | 156 +--------- 5 files changed, 338 insertions(+), 183 deletions(-) diff --git a/src/fides/api/api/v1/endpoints/system.py b/src/fides/api/api/v1/endpoints/system.py index 59fa3be6a7..329cd79f5b 100644 --- a/src/fides/api/api/v1/endpoints/system.py +++ b/src/fides/api/api/v1/endpoints/system.py @@ -1,4 +1,4 @@ -from typing import Dict, List, Optional +from typing import Dict, List, Optional, Union from fastapi import Depends, HTTPException, Response, Security from fastapi_pagination import Page, Params @@ -258,6 +258,10 @@ async def update( updated_system, _ = await update_system( resource, db, current_user.id if current_user else None ) + await supplement_privacy_declaration_response_with_legal_basis_override( + updated_system + ) + return updated_system @@ -353,7 +357,11 @@ async def create( Override `System` create/POST to handle `.privacy_declarations` defined inline, for backward compatibility and ease of use for API users. """ - return await create_system(resource, db, current_user.id if current_user else None) + created = await create_system( + resource, db, current_user.id if current_user else None + ) + await supplement_privacy_declaration_response_with_legal_basis_override(created) + return created @SYSTEM_ROUTER.get( @@ -371,7 +379,10 @@ async def ls( # pylint: disable=invalid-name db: AsyncSession = Depends(get_async_db), ) -> List: """Get a list of all of the resources of this type.""" - return await list_resource(System, db) + systems = await list_resource(System, db) + for system in systems: + await supplement_privacy_declaration_response_with_legal_basis_override(system) + return systems @SYSTEM_ROUTER.get( @@ -389,7 +400,10 @@ async def get( db: AsyncSession = Depends(get_async_db), ) -> Dict: """Get a resource by its fides_key.""" - return await get_resource_with_custom_fields(System, fides_key, db) + + resp = await get_resource_with_custom_fields(System, fides_key, db) + await supplement_privacy_declaration_response_with_legal_basis_override(resp) + return resp @SYSTEM_CONNECTION_INSTANTIATE_ROUTER.post( @@ -412,3 +426,16 @@ def instantiate_connection_from_template( system = get_system(db, fides_key) return instantiate_connection(db, saas_connector_type, template_values, system) + + +async def supplement_privacy_declaration_response_with_legal_basis_override(resp: Union[Dict, System]) -> None: + """At runtime, adds a "legal_basis_for_processing_override" to each PrivacyDeclaration""" + + for privacy_declaration in ( + resp.get("privacy_declarations") + if isinstance(resp, Dict) + else resp.privacy_declarations + ): + privacy_declaration.legal_basis_for_processing_override = ( + await privacy_declaration.overridden_legal_basis_for_processing + ) diff --git a/src/fides/api/models/sql_models.py b/src/fides/api/models/sql_models.py index d290fa51d8..7c88198516 100644 --- a/src/fides/api/models/sql_models.py +++ b/src/fides/api/models/sql_models.py @@ -32,10 +32,11 @@ from sqlalchemy.dialects.postgresql import ARRAY, BIGINT, BYTEA from sqlalchemy.engine import Row from sqlalchemy.exc import IntegrityError +from sqlalchemy.ext.asyncio import AsyncSession, async_object_session from sqlalchemy.ext.hybrid import hybrid_property -from sqlalchemy.orm import Session, relationship +from sqlalchemy.orm import Session, object_session, relationship from sqlalchemy.orm.attributes import InstrumentedAttribute -from sqlalchemy.sql import func +from sqlalchemy.sql import func, Select from sqlalchemy.sql.elements import Case from sqlalchemy.sql.selectable import ScalarSelect from sqlalchemy.sql.sqltypes import DateTime @@ -551,15 +552,15 @@ def purpose(cls) -> Case: ) @hybrid_property - def _publisher_override_legal_basis_join(self) -> Optional[str]: + async def _publisher_override_legal_basis_join(self) -> Optional[str]: """Returns the instance-level overridden required legal basis""" - db: Session = Session.object_session(self) - required_legal_basis: Optional[Row] = ( - db.query(TCFPublisherOverride.required_legal_basis) - .filter(TCFPublisherOverride.purpose == self.purpose) - .first() + query: Select = select([TCFPublisherOverride.required_legal_basis]).where( + TCFPublisherOverride.purpose == self.purpose ) - return required_legal_basis[0] if required_legal_basis else None + async_session: AsyncSession = async_object_session(self) + async with async_session.begin(): + result = await async_session.execute(query) + return result.scalars().first() @_publisher_override_legal_basis_join.expression def _publisher_override_legal_basis_join(cls) -> ScalarSelect: @@ -571,15 +572,15 @@ def _publisher_override_legal_basis_join(cls) -> ScalarSelect: ) @hybrid_property - def _publisher_override_is_included_join(self) -> Optional[bool]: + async def _publisher_override_is_included_join(self) -> Optional[bool]: """Returns the instance-level indication of whether the purpose should be included""" - db: Session = Session.object_session(self) - is_included: Optional[Row] = ( - db.query(TCFPublisherOverride.is_included) - .filter(TCFPublisherOverride.purpose == self.purpose) - .first() + query: Select = select([TCFPublisherOverride.is_included]).where( + TCFPublisherOverride.purpose == self.purpose ) - return is_included[0] if is_included else None + async_session: AsyncSession = async_object_session(self) + async with async_session.begin(): + result = await async_session.execute(query) + return result.scalars().first() @_publisher_override_is_included_join.expression def _publisher_override_is_included_join(cls) -> ScalarSelect: @@ -591,7 +592,7 @@ def _publisher_override_is_included_join(cls) -> ScalarSelect: ) @hybrid_property - def overridden_legal_basis_for_processing(self) -> Optional[str]: + async def overridden_legal_basis_for_processing(self) -> Optional[str]: """ Instance-level override of the legal basis for processing based on publisher preferences. @@ -602,15 +603,21 @@ def overridden_legal_basis_for_processing(self) -> Optional[str]: ): return self.legal_basis_for_processing - if self._publisher_override_is_included_join is False: + is_included: Optional[bool] = await self._publisher_override_is_included_join + + if is_included is False: # Overriding to False to match behavior of class-level override. # Class-level override of legal basis to None removes Privacy Declaration # from Experience return None + overridden_legal_basis: Optional[str] = ( + await self._publisher_override_legal_basis_join + ) + return ( - self._publisher_override_legal_basis_join - if self._publisher_override_legal_basis_join # pylint: disable=using-constant-test + overridden_legal_basis + if overridden_legal_basis # pylint: disable=using-constant-test else self.legal_basis_for_processing ) diff --git a/src/fides/api/schemas/system.py b/src/fides/api/schemas/system.py index 278fc7ea76..ddae949a37 100644 --- a/src/fides/api/schemas/system.py +++ b/src/fides/api/schemas/system.py @@ -19,6 +19,10 @@ class PrivacyDeclarationResponse(PrivacyDeclaration): ) cookies: Optional[List[Cookies]] = [] + legal_basis_for_processing_override: Optional[str] = Field( + description="Global overrides for this purpose's legal basis for processing if applicable. Defaults to the legal_basis_for_processing otherwise." + ) + class BasicSystemResponse(System): """ diff --git a/tests/ctl/core/test_api.py b/tests/ctl/core/test_api.py index ba2c1357a5..bc9eadb35e 100644 --- a/tests/ctl/core/test_api.py +++ b/tests/ctl/core/test_api.py @@ -5,10 +5,12 @@ from datetime import datetime from json import loads from typing import Dict, List +from uuid import uuid4 import pytest import requests from fideslang import DEFAULT_TAXONOMY, model_list, models, parse +from fideslang.models import PrivacyDeclaration as PrivacyDeclarationSchema from fideslang.models import System as SystemSchema from pytest import MonkeyPatch from starlette.status import ( @@ -25,10 +27,12 @@ from fides.api.api.v1.endpoints import health from fides.api.db.crud import get_resource +from fides.api.db.system import create_system from fides.api.models.connectionconfig import ConnectionConfig from fides.api.models.datasetconfig import DatasetConfig from fides.api.models.sql_models import Dataset, PrivacyDeclaration, System from fides.api.models.system_history import SystemHistory +from fides.api.models.tcf_publisher_overrides import TCFPublisherOverride from fides.api.oauth.roles import OWNER, VIEWER from fides.api.schemas.system import PrivacyDeclarationResponse, SystemResponse from fides.api.schemas.taxonomy_extensions import ( @@ -629,8 +633,8 @@ async def test_system_create( for i, decl in enumerate(system.privacy_declarations): for field in PrivacyDeclarationResponse.__fields__: - decl_val = getattr(decl, field) - if isinstance(decl_val, typing.Hashable): + decl_val = getattr(decl, field, None) + if hasattr(decl, field) and isinstance(decl_val, typing.Hashable): assert decl_val == json_results["privacy_declarations"][i][field] assert len(system.privacy_declarations) == 2 @@ -678,6 +682,10 @@ async def test_system_create( assert privacy_decl.dataset_references == ["another_system_reference"] assert privacy_decl.features == ["Link different devices"] assert privacy_decl.legal_basis_for_processing == "Public interest" + assert ( + await privacy_decl.overridden_legal_basis_for_processing + == "Public interest" + ) assert ( privacy_decl.impact_assessment_location == "https://www.example.com/impact_assessment_location" @@ -835,6 +843,10 @@ async def test_system_create_custom_metadata_saas_config( assert result.status_code == HTTP_201_CREATED assert result.json()["name"] == "Test System" assert len(result.json()["privacy_declarations"]) == 2 + assert { + decl["legal_basis_for_processing_override"] + for decl in result.json()["privacy_declarations"] + } == {"Public interest", None} assert result.json()["meta"] == { "saas_config": { "type": "stripe", @@ -996,6 +1008,70 @@ def test_data_stewards_included_in_response( assert "first_name" in steward assert "last_name" in steward + @pytest.mark.usefixtures("enable_override_vendor_purposes") + async def test_get_overridden_declaration_legal_basis( + self, db, test_config, async_session_temp + ): + resource = SystemSchema( + fides_key=str(uuid4()), + organization_fides_key="default_organization", + name=f"test_system_1_{uuid4()}", + system_type="test", + privacy_declarations=[ + PrivacyDeclarationSchema( + name="Collect data for content performance", + data_use="analytics.reporting.content_performance", + legal_basis_for_processing="Consent", + data_categories=["user"], + ) + ], + ) + + system = await create_system( + resource, async_session_temp, CONFIG.security.oauth_root_client_id + ) + + TCFPublisherOverride.create( + db, + data={ + "purpose": 8, + "is_included": True, + "required_legal_basis": "Legitimate interests", + }, + ) + + decl = next( + ( + declaration + for declaration in system.privacy_declarations + if declaration.data_use == "analytics.reporting.content_performance" + ), + None, + ) + assert decl.legal_basis_for_processing == "Consent" + assert ( + await decl.overridden_legal_basis_for_processing == "Legitimate interests" + ) + + result = _api.get( + url=test_config.cli.server_url, + headers=test_config.user.auth_header, + resource_type="system", + resource_id=system.fides_key, + ) + assert result.status_code == 200 + assert result.json()["fides_key"] == system.fides_key + assert ( + result.json()["privacy_declarations"][0]["legal_basis_for_processing"] + == "Consent" + ) + assert ( + result.json()["privacy_declarations"][0][ + "legal_basis_for_processing_override" + ] + == "Legitimate interests" + ) + @pytest.mark.unit class TestSystemUpdate: @@ -1593,8 +1669,8 @@ def test_system_update_dictionary_fields( for i, decl in enumerate(system.privacy_declarations): for field in PrivacyDeclarationResponse.__fields__: - decl_val = getattr(decl, field) - if isinstance(decl_val, typing.Hashable): + decl_val = getattr(decl, field, None) + if hasattr(decl, field) and isinstance(decl_val, typing.Hashable): assert decl_val == json_results["privacy_declarations"][i][field] def test_system_update_system_cookies( @@ -2592,3 +2668,198 @@ def test_trailing_slash(test_config: FidesConfig, endpoint_name: str) -> None: assert response.status_code == 200 response = requests.get(f"{url}/", headers=CONFIG.user.auth_header) assert response.status_code == 200 + + +class TestPrivacyDeclarationInstanceLevelHybridProperties: + """Test some instance-level hybrid properties defined on the Privacy Declaration + related to Publisher Overrides""" + + async def test_privacy_declaration_enable_override_is_false( + self, async_session_temp + ): + """Enable override is false so overridden legal basis is going to default + to the defined legal basis""" + resource = SystemSchema( + fides_key=str(uuid4()), + organization_fides_key="default_organization", + name=f"test_system_1_{uuid4()}", + system_type="test", + privacy_declarations=[ + PrivacyDeclarationSchema( + name="Collect data for content performance", + data_use="analytics.reporting.campaign_insights", + legal_basis_for_processing="Consent", + data_categories=["user"], + ) + ], + ) + + system = await create_system( + resource, async_session_temp, CONFIG.security.oauth_root_client_id + ) + pd = system.privacy_declarations[0] + + assert pd.purpose == 9 + assert await pd._publisher_override_legal_basis_join is None + assert await pd._publisher_override_is_included_join is None + assert await pd.overridden_legal_basis_for_processing == "Consent" + + @pytest.mark.usefixtures( + "enable_override_vendor_purposes", + ) + async def test_enable_override_is_true_but_no_matching_purpose( + self, async_session_temp, db + ): + """Privacy Declaration has Special Purpose not Purpose, so no overrides applicable""" + resource = SystemSchema( + fides_key=str(uuid4()), + organization_fides_key="default_organization", + name=f"test_system_1_{uuid4()}", + system_type="test", + privacy_declarations=[ + PrivacyDeclarationSchema( + name="Collect data for content performance", + data_use="essential.fraud_detection", + legal_basis_for_processing="Consent", + data_categories=["user"], + ) + ], + ) + + system = await create_system( + resource, async_session_temp, CONFIG.security.oauth_root_client_id + ) + pd = system.privacy_declarations[0] + + assert pd.purpose is None + assert await pd._publisher_override_legal_basis_join is None + assert await pd._publisher_override_is_included_join is None + assert await pd.overridden_legal_basis_for_processing == "Consent" + + @pytest.mark.usefixtures( + "enable_override_vendor_purposes", + ) + async def test_enable_override_is_true_but_purpose_is_excluded( + self, async_session_temp, db + ): + """Purpose is overridden as excluded, so legal basis returns as None, to match + class-wide override""" + resource = SystemSchema( + fides_key=str(uuid4()), + organization_fides_key="default_organization", + name=f"test_system_1_{uuid4()}", + system_type="test", + privacy_declarations=[ + PrivacyDeclarationSchema( + name="Collect data for content performance", + data_use="personalize.content.profiling", + legal_basis_for_processing="Consent", + data_categories=["user"], + ) + ], + ) + + system = await create_system( + resource, async_session_temp, CONFIG.security.oauth_root_client_id + ) + pd = system.privacy_declarations[0] + + constraint = TCFPublisherOverride.create( + db, + data={ + "purpose": 5, + "is_included": False, + }, + ) + + assert pd.purpose == 5 + assert await pd._publisher_override_legal_basis_join is None + assert await pd._publisher_override_is_included_join is False + assert await pd.overridden_legal_basis_for_processing is None + + constraint.delete(db) + + @pytest.mark.usefixtures( + "enable_override_vendor_purposes", + ) + async def test_publisher_override_defined_but_no_required_legal_basis_specified( + self, db, async_session_temp + ): + """Purpose override is defined, but no legal basis override""" + resource = SystemSchema( + fides_key=str(uuid4()), + organization_fides_key="default_organization", + name=f"test_system_1_{uuid4()}", + system_type="test", + privacy_declarations=[ + PrivacyDeclarationSchema( + name="Collect data for content performance", + data_use="analytics.reporting.campaign_insights", + legal_basis_for_processing="Consent", + data_categories=["user"], + ) + ], + ) + + system = await create_system( + resource, async_session_temp, CONFIG.security.oauth_root_client_id + ) + pd = system.privacy_declarations[0] + + constraint = TCFPublisherOverride.create( + db, + data={ + "purpose": 9, + "is_included": True, + }, + ) + + assert pd.purpose == 9 + assert await pd._publisher_override_legal_basis_join is None + assert await pd._publisher_override_is_included_join is True + assert await pd.overridden_legal_basis_for_processing == "Consent" + + constraint.delete(db) + + @pytest.mark.usefixtures( + "enable_override_vendor_purposes", + ) + async def test_publisher_override_defined_with_required_legal_basis_specified( + self, async_session_temp, db + ): + """Purpose override is defined, but no legal basis override""" + + resource = SystemSchema( + fides_key=str(uuid4()), + organization_fides_key="default_organization", + name=f"test_system_1_{uuid4()}", + system_type="test", + privacy_declarations=[ + PrivacyDeclarationSchema( + name="Collect data for content performance", + data_use="functional.service.improve", + legal_basis_for_processing="Consent", + data_categories=["user"], + ) + ], + ) + + system = await create_system( + resource, async_session_temp, CONFIG.security.oauth_root_client_id + ) + override = TCFPublisherOverride.create( + db, + data={ + "purpose": 10, + "is_included": True, + "required_legal_basis": "Legitimate interests", + }, + ) + pd = system.privacy_declarations[0] + + assert pd.purpose == 10 + assert await pd._publisher_override_legal_basis_join == "Legitimate interests" + assert await pd._publisher_override_is_included_join is True + assert await pd.overridden_legal_basis_for_processing == "Legitimate interests" + + override.delete(db) diff --git a/tests/ops/util/test_tcf_privacy_declarations.py b/tests/ops/util/test_tcf_privacy_declarations.py index 7b19dd7fb9..9d338265cf 100644 --- a/tests/ops/util/test_tcf_privacy_declarations.py +++ b/tests/ops/util/test_tcf_privacy_declarations.py @@ -1,163 +1,9 @@ -from uuid import uuid4 - import pytest -from fides.api.models.sql_models import PrivacyDeclaration from fides.api.models.tcf_publisher_overrides import TCFPublisherOverride from fides.api.util.tcf.tcf_experience_contents import get_matching_privacy_declarations -class TestPrivacyDeclarationInstanceLevelHybridProperties: - """Test some instance-level hybrid properties defined on the Privacy Declaration - related to Publisher Overrides""" - - def test_privacy_declaration_enable_override_is_false(self, system, db): - """Enable override is false so overridden legal basis is going to default - to the defined legal basis""" - pd = PrivacyDeclaration( - name=f"declaration-name-{uuid4()}", - data_categories=[], - data_use="analytics.reporting.campaign_insights", - data_subjects=[], - data_qualifier="aggregated_data", - dataset_references=[], - ingress=None, - egress=None, - legal_basis_for_processing="Consent", - system_id=system.id, - ).save(db=db) - - assert pd.purpose == 9 - assert pd._publisher_override_legal_basis_join is None - assert pd._publisher_override_is_included_join is None - assert pd.overridden_legal_basis_for_processing == "Consent" - - @pytest.mark.usefixtures( - "enable_override_vendor_purposes", - ) - def test_enable_override_is_true_but_no_matching_purpose(self, system, db): - """Privacy Declaration has Special Purpose not Purpose, so no overrides applicable""" - pd = PrivacyDeclaration( - name=f"declaration-name-{uuid4()}", - data_categories=[], - data_use="essential.fraud_detection", - data_subjects=[], - data_qualifier="aggregated_data", - dataset_references=[], - ingress=None, - egress=None, - legal_basis_for_processing="Consent", - system_id=system.id, - ).save(db) - - assert pd.purpose is None - assert pd._publisher_override_legal_basis_join is None - assert pd._publisher_override_is_included_join is None - assert pd.overridden_legal_basis_for_processing == "Consent" - - @pytest.mark.usefixtures( - "enable_override_vendor_purposes", - ) - def test_enable_override_is_true_but_purpose_is_excluded(self, db, system): - """Purpose is overridden as excluded, so legal basis returns as None, to match - class-wide override""" - TCFPublisherOverride.create( - db, - data={ - "purpose": 9, - "is_included": False, - }, - ) - - pd = PrivacyDeclaration( - name="declaration-name", - data_categories=[], - data_use="analytics.reporting.campaign_insights", - data_subjects=[], - data_qualifier="aggregated_data", - dataset_references=[], - ingress=None, - egress=None, - flexible_legal_basis_for_processing=True, - legal_basis_for_processing="Consent", - system_id=system.id, - ).save(db=db) - - assert pd.purpose == 9 - assert pd._publisher_override_legal_basis_join is None - assert pd._publisher_override_is_included_join is False - assert pd.overridden_legal_basis_for_processing is None - - @pytest.mark.usefixtures( - "enable_override_vendor_purposes", - ) - def test_publisher_override_defined_but_no_required_legal_basis_specified( - self, db, system - ): - """Purpose override is defined, but no legal basis override""" - TCFPublisherOverride.create( - db, - data={ - "purpose": 9, - "is_included": True, - }, - ) - - pd = PrivacyDeclaration( - name="declaration-name", - data_categories=[], - data_use="analytics.reporting.campaign_insights", - data_subjects=[], - data_qualifier="aggregated_data", - dataset_references=[], - ingress=None, - egress=None, - flexible_legal_basis_for_processing=True, - legal_basis_for_processing="Consent", - system_id=system.id, - ).save(db=db) - - assert pd.purpose == 9 - assert pd._publisher_override_legal_basis_join is None - assert pd._publisher_override_is_included_join is True - assert pd.overridden_legal_basis_for_processing == "Consent" - - @pytest.mark.usefixtures( - "enable_override_vendor_purposes", - ) - def test_publisher_override_defined_with_required_legal_basis_specified( - self, db, system - ): - """Purpose override is defined, but no legal basis override""" - TCFPublisherOverride.create( - db, - data={ - "purpose": 9, - "is_included": True, - "required_legal_basis": "Legitimate interests", - }, - ) - - pd = PrivacyDeclaration( - name="declaration-name", - data_categories=[], - data_use="analytics.reporting.campaign_insights", - data_subjects=[], - data_qualifier="aggregated_data", - dataset_references=[], - ingress=None, - egress=None, - flexible_legal_basis_for_processing=True, - legal_basis_for_processing="Consent", - system_id=system.id, - ).save(db=db) - - assert pd.purpose == 9 - assert pd._publisher_override_legal_basis_join == "Legitimate interests" - assert pd._publisher_override_is_included_join is True - assert pd.overridden_legal_basis_for_processing == "Legitimate interests" - - class TestMatchingPrivacyDeclarations: """Tests matching privacy declarations returned that are the basis of the TCF Experience and the "relevant_systems" that are saved for consent reporting @@ -167,7 +13,7 @@ class TestMatchingPrivacyDeclarations: "emerse_system", ) def test_get_matching_privacy_declarations_enable_purpose_override_is_false( - self, emerse_system, db + self, db ): declarations = get_matching_privacy_declarations(db)