From a4663b2e393d9abbff70a21dc32329b80bc32a1c Mon Sep 17 00:00:00 2001 From: Adrian Galvan Date: Tue, 12 Mar 2024 14:23:24 -0700 Subject: [PATCH 1/8] Multi-value support for custom privacy request fields --- .../ApprovePrivacyRequestModal.tsx | 5 +- .../privacy-requests/SubjectIdentities.tsx | 9 +++- src/fides/api/models/privacy_request.py | 51 +++++++++++++------ src/fides/api/schemas/redis_cache.py | 12 +++-- src/fides/api/util/saas_util.py | 3 +- .../saas/test_data/saas_example_config.yml | 5 +- .../test_privacy_request_endpoints.py | 42 +++++++++++++++ tests/ops/models/test_privacy_request.py | 42 +++++++++++++++ .../connectors/test_saas_queryconfig.py | 14 ++++- tests/ops/util/test_saas_util.py | 27 ++++++++++ 10 files changed, 185 insertions(+), 25 deletions(-) diff --git a/clients/admin-ui/src/features/privacy-requests/ApprovePrivacyRequestModal.tsx b/clients/admin-ui/src/features/privacy-requests/ApprovePrivacyRequestModal.tsx index 1de60d404c..ff4ef3aa30 100644 --- a/clients/admin-ui/src/features/privacy-requests/ApprovePrivacyRequestModal.tsx +++ b/clients/admin-ui/src/features/privacy-requests/ApprovePrivacyRequestModal.tsx @@ -98,7 +98,10 @@ const ApprovePrivacyRequestModal = ({ fontSize="sm" mr={2} > - {item.value} (Unverified) + {Array.isArray(item.value) + ? item.value.join(", ") + : item.value}{" "} + (Unverified) diff --git a/clients/admin-ui/src/features/privacy-requests/SubjectIdentities.tsx b/clients/admin-ui/src/features/privacy-requests/SubjectIdentities.tsx index e6f47650bb..e87e37fa39 100644 --- a/clients/admin-ui/src/features/privacy-requests/SubjectIdentities.tsx +++ b/clients/admin-ui/src/features/privacy-requests/SubjectIdentities.tsx @@ -102,7 +102,14 @@ const SubjectIdentities = ({ subjectRequest }: SubjectIdentitiesProps) => { {item.label}: - + Dict[str, Any]: prefix = f"id-{self.id}-custom-privacy-request-field-*" cache: FidesopsRedis = get_cache() keys = cache.keys(prefix) - return {key.split("-")[-1]: cache.get(key) for key in keys} + result = {} + for key in keys: + value = cache.get(key) + if value: + result[key.split("-")[-1]] = json.loads(value) + return result def get_results(self) -> Dict[str, Any]: """Retrieves all cached identity data associated with this Privacy Request""" @@ -1071,11 +1085,11 @@ def hash_value( value: str, encoding: str = "UTF-8", ) -> str: - """Utility function to hash the value with a generated salt""" - SALT = "$2b$12$UErimNtlsE6qgYf2BrI1Du" + """Utility function to hash the value(s) with a generated salt""" + salt = generate_salt() hashed_value = hash_with_salt( value.encode(encoding), - SALT.encode(encoding), + salt.encode(encoding), ) return hashed_value @@ -1146,16 +1160,23 @@ def __tablename__(self) -> str: @classmethod def hash_value( cls, - value: str, + value: CustomPrivacyRequestFieldValue, encoding: str = "UTF-8", - ) -> str: - """Utility function to hash the value with a generated salt""" - salt = generate_salt() - hashed_value = hash_with_salt( - value.encode(encoding), - salt.encode(encoding), - ) - return hashed_value + ) -> Union[str, List[str]]: + """Utility function to hash the value(s) with a generated salt""" + + def hash_single_value(value: Union[str, int]) -> str: + salt = generate_salt() + value_str = str(value) + hashed_value = hash_with_salt( + value_str.encode(encoding), + salt.encode(encoding), + ) + return hashed_value + + if isinstance(value, list): + return [hash_single_value(item) for item in value] + return hash_single_value(value) class Consent(Base): diff --git a/src/fides/api/schemas/redis_cache.py b/src/fides/api/schemas/redis_cache.py index c931ea02a9..2a8f017b99 100644 --- a/src/fides/api/schemas/redis_cache.py +++ b/src/fides/api/schemas/redis_cache.py @@ -1,7 +1,7 @@ import uuid -from typing import Any, Optional +from typing import List, Optional, Union -from pydantic import EmailStr, Extra, validator +from pydantic import EmailStr, Extra, StrictInt, StrictStr, validator from fides.api.custom_types import PhoneNumber from fides.api.schemas.base_class import FidesSchema @@ -44,8 +44,14 @@ def validate_fides_user_device_id(cls, v: Optional[str]) -> Optional[str]: return v +CustomPrivacyRequestFieldValue = Union[ + Union[StrictInt, StrictStr], List[Union[StrictInt, StrictStr]] +] + + class CustomPrivacyRequestField(FidesSchema): """Schema for custom privacy request fields.""" label: str - value: Any + # use StrictInt and StrictStr to avoid type coercion and maintain the original types + value: CustomPrivacyRequestFieldValue diff --git a/src/fides/api/util/saas_util.py b/src/fides/api/util/saas_util.py index 01afdec840..271ad17050 100644 --- a/src/fides/api/util/saas_util.py +++ b/src/fides/api/util/saas_util.py @@ -349,7 +349,8 @@ def assign_placeholders(value: Any, param_values: Dict[str, Any]) -> Optional[An # removes outer {} wrapper from body for greater flexibility in custom body config if isinstance(placeholder_value, dict): placeholder_value = json.dumps(placeholder_value)[1:-1] - + if isinstance(placeholder_value, list): + placeholder_value = json.dumps(placeholder_value) if placeholder_value is not None: value = value.replace(f"<{full_placeholder}>", str(placeholder_value)) elif is_optional: diff --git a/tests/fixtures/saas/test_data/saas_example_config.yml b/tests/fixtures/saas/test_data/saas_example_config.yml index 598acc9f89..d7b8a2575c 100644 --- a/tests/fixtures/saas/test_data/saas_example_config.yml +++ b/tests/fixtures/saas/test_data/saas_example_config.yml @@ -319,7 +319,9 @@ saas_config: body: | { "last_name": "", - "order_id": "" + "order_id": "", + "subscriber_ids": , + "account_ids": } update: method: POST @@ -342,4 +344,3 @@ saas_config: - dataset: saas_connector_example field: users.list_ids direction: from - diff --git a/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py b/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py index bf5cd6f776..159e43576a 100644 --- a/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_privacy_request_endpoints.py @@ -215,6 +215,48 @@ def test_create_privacy_request_stores_custom_fields( pr.delete(db=db) assert run_access_request_mock.called + @mock.patch( + "fides.api.service.privacy_request.request_runner_service.run_privacy_request.delay" + ) + def test_create_privacy_request_stores_multivalue_custom_fields( + self, + run_access_request_mock, + url, + db, + api_client: TestClient, + policy, + allow_custom_privacy_request_field_collection_enabled, + ): + TEST_EMAIL = "test@example.com" + TEST_CUSTOM_FIELDS = { + "first_name": {"label": "First name", "value": "John"}, + "subscriber_ids": {"label": "Subscriber IDs", "value": ["123", "456"]}, + "account_ids": {"label": "Account IDs", "value": [123, 456]}, + } + data = [ + { + "requested_at": "2021-08-30T16:09:37.359Z", + "policy_key": policy.key, + "identity": { + "email": TEST_EMAIL, + }, + "custom_privacy_request_fields": TEST_CUSTOM_FIELDS, + } + ] + resp = api_client.post(url, json=data) + assert resp.status_code == 200 + response_data = resp.json()["succeeded"] + assert len(response_data) == 1 + pr = PrivacyRequest.get(db=db, object_id=response_data[0]["id"]) + persisted_identity = pr.get_persisted_identity() + assert persisted_identity.email == TEST_EMAIL + persisted_custom_privacy_request_fields = ( + pr.get_persisted_custom_privacy_request_fields() + ) + assert persisted_custom_privacy_request_fields == TEST_CUSTOM_FIELDS + pr.delete(db=db) + assert run_access_request_mock.called + @mock.patch( "fides.api.service.privacy_request.request_runner_service.run_privacy_request.delay" ) diff --git a/tests/ops/models/test_privacy_request.py b/tests/ops/models/test_privacy_request.py index df43d802ca..c45c79dee5 100644 --- a/tests/ops/models/test_privacy_request.py +++ b/tests/ops/models/test_privacy_request.py @@ -990,11 +990,19 @@ def test_cache_custom_privacy_request_fields( label="First name", value="John" ), "last_name": CustomPrivacyRequestField(label="Last name", value="Doe"), + "subscriber_ids": CustomPrivacyRequestField( + label="Subscriber IDs", value=["123", "456"] + ), + "account_ids": CustomPrivacyRequestField( + label="Account IDs", value=[123, 456] + ), } ) assert privacy_request.get_cached_custom_privacy_request_fields() == { "first_name": "John", "last_name": "Doe", + "subscriber_ids": ["123", "456"], + "account_ids": [123, 456], } def test_cache_custom_privacy_request_fields_collection_disabled( @@ -1009,6 +1017,9 @@ def test_cache_custom_privacy_request_fields_collection_disabled( label="First name", value="John" ), "last_name": CustomPrivacyRequestField(label="Last name", value="Doe"), + "subscriber_ids": CustomPrivacyRequestField( + label="Subscriber IDs", value=["123", "456"] + ), } ) assert privacy_request.get_cached_custom_privacy_request_fields() == {} @@ -1026,6 +1037,9 @@ def test_cache_custom_privacy_request_fields_collection_enabled_use_disabled( label="First name", value="John" ), "last_name": CustomPrivacyRequestField(label="Last name", value="Doe"), + "subscriber_ids": CustomPrivacyRequestField( + label="Subscriber IDs", value=["123", "456"] + ), } ) assert privacy_request.get_cached_custom_privacy_request_fields() == {} @@ -1044,11 +1058,19 @@ def test_persist_custom_privacy_request_fields( label="First name", value="John" ), "last_name": CustomPrivacyRequestField(label="Last name", value="Doe"), + "subscriber_ids": CustomPrivacyRequestField( + label="Subscriber IDs", value=["123", "456"] + ), + "account_ids": CustomPrivacyRequestField( + label="Account IDs", value=[123, 456] + ), }, ) assert privacy_request.get_persisted_custom_privacy_request_fields() == { "first_name": {"label": "First name", "value": "John"}, "last_name": {"label": "Last name", "value": "Doe"}, + "subscriber_ids": {"label": "Subscriber IDs", "value": ["123", "456"]}, + "account_ids": {"label": "Account IDs", "value": [123, 456]}, } def test_persist_custom_privacy_request_fields_collection_disabled( @@ -1065,6 +1087,12 @@ def test_persist_custom_privacy_request_fields_collection_disabled( label="First name", value="John" ), "last_name": CustomPrivacyRequestField(label="Last name", value="Doe"), + "subscriber_ids": CustomPrivacyRequestField( + label="Subscriber IDs", value=["123", "456"] + ), + "account_ids": CustomPrivacyRequestField( + label="Account IDs", value=[123, 456] + ), }, ) assert privacy_request.get_persisted_custom_privacy_request_fields() == {} @@ -1107,11 +1135,19 @@ def test_persist_custom_privacy_request_fields( label="First name", value="John" ), "last_name": CustomPrivacyRequestField(label="Last name", value="Doe"), + "subscriber_ids": CustomPrivacyRequestField( + label="Subscriber IDs", value=["123", "456"] + ), + "account_ids": CustomPrivacyRequestField( + label="Account IDs", value=[123, 456] + ), }, ) assert consent_request.get_persisted_custom_privacy_request_fields() == { "first_name": {"label": "First name", "value": "John"}, "last_name": {"label": "Last name", "value": "Doe"}, + "subscriber_ids": {"label": "Subscriber IDs", "value": ["123", "456"]}, + "account_ids": {"label": "Account IDs", "value": [123, 456]}, } def test_persist_custom_privacy_request_fields_collection_disabled( @@ -1128,6 +1164,12 @@ def test_persist_custom_privacy_request_fields_collection_disabled( label="First name", value="John" ), "last_name": CustomPrivacyRequestField(label="Last name", value="Doe"), + "subscriber_ids": CustomPrivacyRequestField( + label="Subscriber IDs", value=["123", "456"] + ), + "account_ids": CustomPrivacyRequestField( + label="Account IDs", value=[123, 456] + ), }, ) assert consent_request.get_persisted_custom_privacy_request_fields() == {} diff --git a/tests/ops/service/connectors/test_saas_queryconfig.py b/tests/ops/service/connectors/test_saas_queryconfig.py index cfa56071b7..026aaf0e56 100644 --- a/tests/ops/service/connectors/test_saas_queryconfig.py +++ b/tests/ops/service/connectors/test_saas_queryconfig.py @@ -632,6 +632,8 @@ def test_custom_privacy_request_fields( mock_custom_privacy_request_fields.return_value = { "first_name": "John", "last_name": "Doe", + "subscriber_ids": ["123", "456"], + "account_ids": [123, 456], } connector = SaaSConnector(saas_example_connection_config) saas_config: SaaSConfig = saas_example_connection_config.get_saas_config() @@ -655,6 +657,8 @@ def test_custom_privacy_request_fields( CUSTOM_PRIVACY_REQUEST_FIELDS: { "first_name": "John", "last_name": "Doe", + "subscriber_ids": ["123", "456"], + "account_ids": [123, 456], }, }, policy, @@ -666,6 +670,8 @@ def test_custom_privacy_request_fields( assert json.loads(read_request.body) == { "last_name": "Doe", "order_id": None, + "subscriber_ids": ["123", "456"], + "account_ids": [123, 456], } update_request: SaaSRequestParams = config.generate_update_stmt( @@ -675,9 +681,13 @@ def test_custom_privacy_request_fields( assert update_request.path == "/v1/internal/" assert update_request.query_params == {} assert json.loads(update_request.body) == { - "user_info": {"first_name": "John", "last_name": "Doe"} + "user_info": { + "first_name": "John", + "last_name": "Doe", + "subscriber_ids": ["123", "456"], + "account_ids": [123, 456], + } } - opt_in_request: SaaSRequest = config.generate_consent_stmt( consent_policy, privacy_request, diff --git a/tests/ops/util/test_saas_util.py b/tests/ops/util/test_saas_util.py index c5f9e2d41e..a8e8451202 100644 --- a/tests/ops/util/test_saas_util.py +++ b/tests/ops/util/test_saas_util.py @@ -442,6 +442,33 @@ def test_replacing_with_object_values(self): == '{"age": 28, "name": "Bob"}' ) + def test_replacing_with_string_list_values(self): + assert ( + assign_placeholders( + '{"subscriber_ids": }', + {"subscriber_ids": ["123", "456"]}, + ) + == '{"subscriber_ids": ["123", "456"]}' + ) + + def test_replacing_with_integer_list_values(self): + assert ( + assign_placeholders( + '{"account_ids": }', + {"account_ids": [123, 456]}, + ) + == '{"account_ids": [123, 456]}' + ) + + def test_replacing_with_empty_list_values(self): + assert ( + assign_placeholders( + '{"subscriber_ids": }', + {"subscriber_ids": []}, + ) + == '{"subscriber_ids": []}' + ) + class TestUnflattenDict: def test_empty_dict(self): From 71b362b820b279e593461fa80fbf3e2677103e7d Mon Sep 17 00:00:00 2001 From: Adrian Galvan Date: Fri, 15 Mar 2024 10:58:51 -0700 Subject: [PATCH 2/8] Fixing tests --- src/fides/api/models/privacy_request.py | 46 +++++++++---------- .../saas/test_oracle_responsys_task.py | 1 + 2 files changed, 24 insertions(+), 23 deletions(-) diff --git a/src/fides/api/models/privacy_request.py b/src/fides/api/models/privacy_request.py index 330a080e8c..fd629f220d 100644 --- a/src/fides/api/models/privacy_request.py +++ b/src/fides/api/models/privacy_request.py @@ -783,10 +783,10 @@ def get_manual_access_input( This is for use by the *manual* connector which is integrated with the graph. """ cache: FidesopsRedis = get_cache() - cached_results: Optional[ - Dict[str, Optional[List[Row]]] - ] = cache.get_encoded_objects_by_prefix( - f"MANUAL_INPUT__{self.id}__{collection.value}" + cached_results: Optional[Dict[str, Optional[List[Row]]]] = ( + cache.get_encoded_objects_by_prefix( + f"MANUAL_INPUT__{self.id}__{collection.value}" + ) ) return list(cached_results.values())[0] if cached_results else None @@ -824,9 +824,9 @@ def cache_access_graph(self, value: GraphRepr) -> None: def get_cached_access_graph(self) -> Optional[GraphRepr]: """Fetch the graph built for the access request""" cache: FidesopsRedis = get_cache() - value_dict: Optional[ - Dict[str, Optional[GraphRepr]] - ] = cache.get_encoded_objects_by_prefix(f"ACCESS_GRAPH__{self.id}") + value_dict: Optional[Dict[str, Optional[GraphRepr]]] = ( + cache.get_encoded_objects_by_prefix(f"ACCESS_GRAPH__{self.id}") + ) return list(value_dict.values())[0] if value_dict else None def cache_data_use_map(self, value: Dict[str, Set[str]]) -> None: @@ -842,9 +842,9 @@ def get_cached_data_use_map(self) -> Optional[Dict[str, Set[str]]]: Fetch the collection -> data use map cached for this privacy request """ cache: FidesopsRedis = get_cache() - value_dict: Optional[ - Dict[str, Optional[Dict[str, Set[str]]]] - ] = cache.get_encoded_objects_by_prefix(f"DATA_USE_MAP__{self.id}") + value_dict: Optional[Dict[str, Optional[Dict[str, Set[str]]]]] = ( + cache.get_encoded_objects_by_prefix(f"DATA_USE_MAP__{self.id}") + ) return list(value_dict.values())[0] if value_dict else None def trigger_policy_webhook( @@ -992,10 +992,10 @@ def _get_manual_access_input_from_cache( """Get raw manual input uploaded to the privacy request for the given webhook from the cache without attempting to coerce into a Pydantic schema""" cache: FidesopsRedis = get_cache() - cached_results: Optional[ - Optional[Dict[str, Any]] - ] = cache.get_encoded_objects_by_prefix( - f"WEBHOOK_MANUAL_ACCESS_INPUT__{privacy_request.id}__{manual_webhook.id}" + cached_results: Optional[Optional[Dict[str, Any]]] = ( + cache.get_encoded_objects_by_prefix( + f"WEBHOOK_MANUAL_ACCESS_INPUT__{privacy_request.id}__{manual_webhook.id}" + ) ) if cached_results: return list(cached_results.values())[0] @@ -1008,10 +1008,10 @@ def _get_manual_erasure_input_from_cache( """Get raw manual input uploaded to the privacy request for the given webhook from the cache without attempting to coerce into a Pydantic schema""" cache: FidesopsRedis = get_cache() - cached_results: Optional[ - Optional[Dict[str, Any]] - ] = cache.get_encoded_objects_by_prefix( - f"WEBHOOK_MANUAL_ERASURE_INPUT__{privacy_request.id}__{manual_webhook.id}" + cached_results: Optional[Optional[Dict[str, Any]]] = ( + cache.get_encoded_objects_by_prefix( + f"WEBHOOK_MANUAL_ERASURE_INPUT__{privacy_request.id}__{manual_webhook.id}" + ) ) if cached_results: return list(cached_results.values())[0] @@ -1085,11 +1085,11 @@ def hash_value( value: str, encoding: str = "UTF-8", ) -> str: - """Utility function to hash the value(s) with a generated salt""" - salt = generate_salt() + """Utility function to hash the value with a generated salt""" + SALT = "$2b$12$UErimNtlsE6qgYf2BrI1Du" hashed_value = hash_with_salt( value.encode(encoding), - salt.encode(encoding), + SALT.encode(encoding), ) return hashed_value @@ -1166,11 +1166,11 @@ def hash_value( """Utility function to hash the value(s) with a generated salt""" def hash_single_value(value: Union[str, int]) -> str: - salt = generate_salt() + SALT = "$2b$12$UErimNtlsE6qgYf2BrI1Du" value_str = str(value) hashed_value = hash_with_salt( value_str.encode(encoding), - salt.encode(encoding), + SALT.encode(encoding), ) return hashed_value diff --git a/tests/ops/integration_tests/saas/test_oracle_responsys_task.py b/tests/ops/integration_tests/saas/test_oracle_responsys_task.py index 9ecce5631a..3e6ec43352 100644 --- a/tests/ops/integration_tests/saas/test_oracle_responsys_task.py +++ b/tests/ops/integration_tests/saas/test_oracle_responsys_task.py @@ -5,6 +5,7 @@ @pytest.mark.skip("Enterprise account only") +@pytest.mark.integration_saas class TestOracleResponsysConnector: def test_connection(self, oracle_responsys_runner: ConnectorRunner): oracle_responsys_runner.test_connection() From ff56f274a19394bfc81722ce26a467d57d16d733 Mon Sep 17 00:00:00 2001 From: Adrian Galvan Date: Fri, 15 Mar 2024 11:00:23 -0700 Subject: [PATCH 3/8] Removing unused import --- src/fides/api/models/privacy_request.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fides/api/models/privacy_request.py b/src/fides/api/models/privacy_request.py index fd629f220d..56a13fa6cc 100644 --- a/src/fides/api/models/privacy_request.py +++ b/src/fides/api/models/privacy_request.py @@ -34,7 +34,7 @@ NoCachedManualWebhookEntry, PrivacyRequestPaused, ) -from fides.api.cryptography.cryptographic_util import generate_salt, hash_with_salt +from fides.api.cryptography.cryptographic_util import hash_with_salt from fides.api.db.base_class import Base # type: ignore[attr-defined] from fides.api.db.base_class import JSONTypeOverride from fides.api.db.util import EnumColumn From 5bf2ec00ea4e077e758af6d378488e8494aa5c0b Mon Sep 17 00:00:00 2001 From: Adrian Galvan Date: Fri, 15 Mar 2024 17:05:41 -0700 Subject: [PATCH 4/8] Undoing formatting changes --- src/fides/api/models/privacy_request.py | 36 ++++++++++++------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/fides/api/models/privacy_request.py b/src/fides/api/models/privacy_request.py index 56a13fa6cc..bc0b29f090 100644 --- a/src/fides/api/models/privacy_request.py +++ b/src/fides/api/models/privacy_request.py @@ -783,10 +783,10 @@ def get_manual_access_input( This is for use by the *manual* connector which is integrated with the graph. """ cache: FidesopsRedis = get_cache() - cached_results: Optional[Dict[str, Optional[List[Row]]]] = ( - cache.get_encoded_objects_by_prefix( - f"MANUAL_INPUT__{self.id}__{collection.value}" - ) + cached_results: Optional[ + Dict[str, Optional[List[Row]]] + ] = cache.get_encoded_objects_by_prefix( + f"MANUAL_INPUT__{self.id}__{collection.value}" ) return list(cached_results.values())[0] if cached_results else None @@ -824,9 +824,9 @@ def cache_access_graph(self, value: GraphRepr) -> None: def get_cached_access_graph(self) -> Optional[GraphRepr]: """Fetch the graph built for the access request""" cache: FidesopsRedis = get_cache() - value_dict: Optional[Dict[str, Optional[GraphRepr]]] = ( - cache.get_encoded_objects_by_prefix(f"ACCESS_GRAPH__{self.id}") - ) + value_dict: Optional[ + Dict[str, Optional[GraphRepr]] + ] = cache.get_encoded_objects_by_prefix(f"ACCESS_GRAPH__{self.id}") return list(value_dict.values())[0] if value_dict else None def cache_data_use_map(self, value: Dict[str, Set[str]]) -> None: @@ -842,9 +842,9 @@ def get_cached_data_use_map(self) -> Optional[Dict[str, Set[str]]]: Fetch the collection -> data use map cached for this privacy request """ cache: FidesopsRedis = get_cache() - value_dict: Optional[Dict[str, Optional[Dict[str, Set[str]]]]] = ( - cache.get_encoded_objects_by_prefix(f"DATA_USE_MAP__{self.id}") - ) + value_dict: Optional[ + Dict[str, Optional[Dict[str, Set[str]]]] + ] = cache.get_encoded_objects_by_prefix(f"DATA_USE_MAP__{self.id}") return list(value_dict.values())[0] if value_dict else None def trigger_policy_webhook( @@ -992,10 +992,10 @@ def _get_manual_access_input_from_cache( """Get raw manual input uploaded to the privacy request for the given webhook from the cache without attempting to coerce into a Pydantic schema""" cache: FidesopsRedis = get_cache() - cached_results: Optional[Optional[Dict[str, Any]]] = ( - cache.get_encoded_objects_by_prefix( - f"WEBHOOK_MANUAL_ACCESS_INPUT__{privacy_request.id}__{manual_webhook.id}" - ) + cached_results: Optional[ + Optional[Dict[str, Any]] + ] = cache.get_encoded_objects_by_prefix( + f"WEBHOOK_MANUAL_ACCESS_INPUT__{privacy_request.id}__{manual_webhook.id}" ) if cached_results: return list(cached_results.values())[0] @@ -1008,10 +1008,10 @@ def _get_manual_erasure_input_from_cache( """Get raw manual input uploaded to the privacy request for the given webhook from the cache without attempting to coerce into a Pydantic schema""" cache: FidesopsRedis = get_cache() - cached_results: Optional[Optional[Dict[str, Any]]] = ( - cache.get_encoded_objects_by_prefix( - f"WEBHOOK_MANUAL_ERASURE_INPUT__{privacy_request.id}__{manual_webhook.id}" - ) + cached_results: Optional[ + Optional[Dict[str, Any]] + ] = cache.get_encoded_objects_by_prefix( + f"WEBHOOK_MANUAL_ERASURE_INPUT__{privacy_request.id}__{manual_webhook.id}" ) if cached_results: return list(cached_results.values())[0] From 84a28b721b56812720c4250253e17149fe913c39 Mon Sep 17 00:00:00 2001 From: Adrian Galvan Date: Mon, 18 Mar 2024 15:46:56 -0700 Subject: [PATCH 5/8] Adding integration test for custom privacy request fields --- ...s_custom_privacy_request_fields_config.yml | 52 +++++ ..._custom_privacy_request_fields_dataset.yml | 12 ++ ...tegration_custom_privacy_request_fields.py | 177 ++++++++++++++++++ .../test_request_runner_service.py | 3 + 4 files changed, 244 insertions(+) create mode 100644 tests/fixtures/saas/test_data/saas_custom_privacy_request_fields_config.yml create mode 100644 tests/fixtures/saas/test_data/saas_custom_privacy_request_fields_dataset.yml create mode 100644 tests/ops/integration_tests/test_integration_custom_privacy_request_fields.py diff --git a/tests/fixtures/saas/test_data/saas_custom_privacy_request_fields_config.yml b/tests/fixtures/saas/test_data/saas_custom_privacy_request_fields_config.yml new file mode 100644 index 0000000000..8857c5accf --- /dev/null +++ b/tests/fixtures/saas/test_data/saas_custom_privacy_request_fields_config.yml @@ -0,0 +1,52 @@ +saas_config: + fides_key: + name: Custom Privacy Request Fields + type: custom_privacy_request_fields + description: A sample schema to test custom privacy request fields + version: 0.0.1 + + connector_params: + - name: + + client_config: + protocol: https + host: localhost + authentication: + strategy: bearer + configuration: + token: + + test_request: + method: GET + path: /ping + + endpoints: + - name: user + requests: + read: + method: POST + path: /v1/user-search + param_values: + - name: placeholder + identity: email + query_params: + - name: first_name + value: + body: | + { + "last_name": "", + "order_id": "", + "subscriber_ids": , + "account_ids": + } + ignore_errors: True + update: + method: PUT + path: /v1/user + body: | + { + "user_info": { + + } + } + ignore_errors: True diff --git a/tests/fixtures/saas/test_data/saas_custom_privacy_request_fields_dataset.yml b/tests/fixtures/saas/test_data/saas_custom_privacy_request_fields_dataset.yml new file mode 100644 index 0000000000..0009dce2e2 --- /dev/null +++ b/tests/fixtures/saas/test_data/saas_custom_privacy_request_fields_dataset.yml @@ -0,0 +1,12 @@ +dataset: + - fides_key: + name: Custom Privacy Request Fields Dataset + description: A sample dataset for testing + collections: + - name: user + fields: + - name: id + data_categories: [system.operations] + fidesops_meta: + data_type: integer + primary_key: True diff --git a/tests/ops/integration_tests/test_integration_custom_privacy_request_fields.py b/tests/ops/integration_tests/test_integration_custom_privacy_request_fields.py new file mode 100644 index 0000000000..419c9f6c5c --- /dev/null +++ b/tests/ops/integration_tests/test_integration_custom_privacy_request_fields.py @@ -0,0 +1,177 @@ +import json +from unittest import mock + +import pytest +from requests import Response +from sqlalchemy.orm import Session + +from fides.api.models.connectionconfig import ( + AccessLevel, + ConnectionConfig, + ConnectionType, +) +from fides.api.models.datasetconfig import DatasetConfig +from fides.api.models.policy import Policy +from fides.api.models.sql_models import Dataset as CtlDataset +from fides.api.schemas.privacy_request import CustomPrivacyRequestField +from fides.api.util.saas_util import ( + load_config_with_replacement, + load_dataset_with_replacement, +) +from tests.ops.service.privacy_request.test_request_runner_service import ( + get_privacy_request_results, +) + + +@pytest.mark.integration_saas +class TestCustomPrivacyRequestFields: + @pytest.fixture(scope="function") + def custom_privacy_request_fields_config(self, db: Session) -> ConnectionConfig: + fides_key = "custom_privacy_request_fields_instance" + config = load_config_with_replacement( + "tests/fixtures/saas/test_data/saas_custom_privacy_request_fields_config.yml", + "", + fides_key, + ) + + connection_config = ConnectionConfig.create( + db=db, + data={ + "key": fides_key, + "name": fides_key, + "connection_type": ConnectionType.saas, + "access": AccessLevel.write, + "secrets": {"api_key": "test"}, + "saas_config": config, + }, + ) + return connection_config + + @pytest.fixture(scope="function") + def custom_privacy_request_fields_dataset( + self, + db: Session, + custom_privacy_request_fields_config: ConnectionConfig, + ) -> DatasetConfig: + fides_key = "custom_privacy_request_fields_instance" + connection_config = custom_privacy_request_fields_config + dataset = load_dataset_with_replacement( + "tests/fixtures/saas/test_data/saas_custom_privacy_request_fields_dataset.yml", + "", + fides_key, + )[0] + connection_config.name = fides_key + connection_config.key = fides_key + connection_config.save(db=db) + + ctl_dataset = CtlDataset.create_from_dataset_dict(db, dataset) + + dataset = DatasetConfig.create( + db=db, + data={ + "connection_config_id": connection_config.id, + "fides_key": fides_key, + "ctl_dataset_id": ctl_dataset.id, + }, + ) + return dataset + + @pytest.mark.usefixtures( + "custom_privacy_request_fields_config", + "custom_privacy_request_fields_dataset", + "allow_custom_privacy_request_field_collection_enabled", + "allow_custom_privacy_request_fields_in_request_execution_enabled", + ) + @mock.patch("fides.api.service.connectors.saas_connector.AuthenticatedClient.send") + def test_custom_privacy_request_fields_access( + self, mock_send, db: Session, policy: Policy, run_privacy_request_task + ) -> None: + data = { + "requested_at": "2021-08-30T16:09:37.359Z", + "policy_key": policy.key, + "identity": {"email": "customer@example.com"}, + "custom_privacy_request_fields": { + "first_name": CustomPrivacyRequestField( + label="First name", value="John" + ), + "last_name": CustomPrivacyRequestField(label="Last name", value="Doe"), + "subscriber_ids": CustomPrivacyRequestField( + label="Subscriber IDs", value=["123", "456"] + ), + "account_ids": CustomPrivacyRequestField( + label="Account IDs", value=[123, 456] + ), + }, + } + + mock_response = Response() + mock_response.status_code = 200 + mock_response._content = json.dumps([{"id": 1}]).encode() + mock_send.return_value = mock_response + + get_privacy_request_results( + db, + policy, + run_privacy_request_task, + data, + ) + + request_params = mock_send.call_args[0][0] + assert request_params.query_params == {"first_name": "John"} + assert json.loads(request_params.body) == { + "last_name": "Doe", + "order_id": None, + "subscriber_ids": ["123", "456"], + "account_ids": [123, 456], + } + + @pytest.mark.usefixtures( + "custom_privacy_request_fields_config", + "custom_privacy_request_fields_dataset", + "allow_custom_privacy_request_field_collection_enabled", + "allow_custom_privacy_request_fields_in_request_execution_enabled", + ) + @mock.patch("fides.api.service.connectors.saas_connector.AuthenticatedClient.send") + def test_custom_privacy_request_fields_erasure( + self, mock_send, db: Session, erasure_policy: Policy, run_privacy_request_task + ) -> None: + data = { + "requested_at": "2021-08-30T16:09:37.359Z", + "policy_key": erasure_policy.key, + "identity": {"email": "customer@example.com"}, + "custom_privacy_request_fields": { + "first_name": CustomPrivacyRequestField( + label="First name", value="John" + ), + "last_name": CustomPrivacyRequestField(label="Last name", value="Doe"), + "subscriber_ids": CustomPrivacyRequestField( + label="Subscriber IDs", value=["123", "456"] + ), + "account_ids": CustomPrivacyRequestField( + label="Account IDs", value=[123, 456] + ), + }, + } + + mock_response = Response() + mock_response.status_code = 200 + mock_response._content = json.dumps([{"id": 1}]).encode() + mock_send.return_value = mock_response + + get_privacy_request_results( + db, + erasure_policy, + run_privacy_request_task, + data, + ) + + request_params = mock_send.call_args[0][0] + assert request_params.query_params == {} + assert json.loads(request_params.body) == { + "user_info": { + "first_name": "John", + "last_name": "Doe", + "subscriber_ids": ["123", "456"], + "account_ids": [123, 456], + } + } diff --git a/tests/ops/service/privacy_request/test_request_runner_service.py b/tests/ops/service/privacy_request/test_request_runner_service.py index c3ccd376df..3628c5f64e 100644 --- a/tests/ops/service/privacy_request/test_request_runner_service.py +++ b/tests/ops/service/privacy_request/test_request_runner_service.py @@ -300,6 +300,9 @@ def get_privacy_request_results( pass privacy_request = PrivacyRequest.create(db=db, data=kwargs) privacy_request.cache_identity(privacy_request_data["identity"]) + privacy_request.cache_custom_privacy_request_fields( + privacy_request_data["custom_privacy_request_fields"] + ) if "encryption_key" in privacy_request_data: privacy_request.cache_encryption(privacy_request_data["encryption_key"]) From 7acf06df278c839e329ed661fb19a1bad1ee26cf Mon Sep 17 00:00:00 2001 From: Adrian Galvan Date: Mon, 18 Mar 2024 16:09:59 -0700 Subject: [PATCH 6/8] Fixing integration tests --- .../ops/service/privacy_request/test_request_runner_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ops/service/privacy_request/test_request_runner_service.py b/tests/ops/service/privacy_request/test_request_runner_service.py index 3628c5f64e..f2bb0f2155 100644 --- a/tests/ops/service/privacy_request/test_request_runner_service.py +++ b/tests/ops/service/privacy_request/test_request_runner_service.py @@ -301,7 +301,7 @@ def get_privacy_request_results( privacy_request = PrivacyRequest.create(db=db, data=kwargs) privacy_request.cache_identity(privacy_request_data["identity"]) privacy_request.cache_custom_privacy_request_fields( - privacy_request_data["custom_privacy_request_fields"] + privacy_request_data.get("custom_privacy_request_fields", None) ) if "encryption_key" in privacy_request_data: privacy_request.cache_encryption(privacy_request_data["encryption_key"]) From 91e633ef6a6fbc672a8d6d79d8c439a783249e58 Mon Sep 17 00:00:00 2001 From: Adrian Galvan Date: Wed, 20 Mar 2024 12:16:24 -0700 Subject: [PATCH 7/8] Changes based on PR feedback --- src/fides/api/models/privacy_request.py | 6 +----- .../saas_custom_privacy_request_fields_config.yml | 3 ++- ...est_integration_custom_privacy_request_fields.py | 4 ++++ tests/ops/models/test_privacy_request.py | 13 +++++++++++++ .../ops/service/connectors/test_saas_queryconfig.py | 1 + 5 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/fides/api/models/privacy_request.py b/src/fides/api/models/privacy_request.py index bc0b29f090..a071f4770a 100644 --- a/src/fides/api/models/privacy_request.py +++ b/src/fides/api/models/privacy_request.py @@ -369,11 +369,7 @@ def cache_custom_privacy_request_fields( if item is not None: cache.set_with_autoexpire( get_custom_privacy_request_field_cache_key(self.id, key), - ( - json.dumps(item.value, cls=CustomJSONEncoder) - if isinstance(item.value, list) - else f'"{item.value}"' - ), + json.dumps(item.value, cls=CustomJSONEncoder), ) else: logger.info( diff --git a/tests/fixtures/saas/test_data/saas_custom_privacy_request_fields_config.yml b/tests/fixtures/saas/test_data/saas_custom_privacy_request_fields_config.yml index 8857c5accf..dbddb56af3 100644 --- a/tests/fixtures/saas/test_data/saas_custom_privacy_request_fields_config.yml +++ b/tests/fixtures/saas/test_data/saas_custom_privacy_request_fields_config.yml @@ -37,7 +37,8 @@ saas_config: "last_name": "", "order_id": "", "subscriber_ids": , - "account_ids": + "account_ids": , + "support_id": } ignore_errors: True update: diff --git a/tests/ops/integration_tests/test_integration_custom_privacy_request_fields.py b/tests/ops/integration_tests/test_integration_custom_privacy_request_fields.py index 419c9f6c5c..acb306a07a 100644 --- a/tests/ops/integration_tests/test_integration_custom_privacy_request_fields.py +++ b/tests/ops/integration_tests/test_integration_custom_privacy_request_fields.py @@ -101,6 +101,7 @@ def test_custom_privacy_request_fields_access( "account_ids": CustomPrivacyRequestField( label="Account IDs", value=[123, 456] ), + "support_id": CustomPrivacyRequestField(label="Support ID", value=1), }, } @@ -123,6 +124,7 @@ def test_custom_privacy_request_fields_access( "order_id": None, "subscriber_ids": ["123", "456"], "account_ids": [123, 456], + "support_id": 1, } @pytest.mark.usefixtures( @@ -150,6 +152,7 @@ def test_custom_privacy_request_fields_erasure( "account_ids": CustomPrivacyRequestField( label="Account IDs", value=[123, 456] ), + "support_id": CustomPrivacyRequestField(label="Support ID", value=1), }, } @@ -173,5 +176,6 @@ def test_custom_privacy_request_fields_erasure( "last_name": "Doe", "subscriber_ids": ["123", "456"], "account_ids": [123, 456], + "support_id": 1, } } diff --git a/tests/ops/models/test_privacy_request.py b/tests/ops/models/test_privacy_request.py index c45c79dee5..c4737d3be7 100644 --- a/tests/ops/models/test_privacy_request.py +++ b/tests/ops/models/test_privacy_request.py @@ -996,6 +996,7 @@ def test_cache_custom_privacy_request_fields( "account_ids": CustomPrivacyRequestField( label="Account IDs", value=[123, 456] ), + "support_id": CustomPrivacyRequestField(label="Support ID", value=1), } ) assert privacy_request.get_cached_custom_privacy_request_fields() == { @@ -1003,6 +1004,7 @@ def test_cache_custom_privacy_request_fields( "last_name": "Doe", "subscriber_ids": ["123", "456"], "account_ids": [123, 456], + "support_id": 1, } def test_cache_custom_privacy_request_fields_collection_disabled( @@ -1020,6 +1022,10 @@ def test_cache_custom_privacy_request_fields_collection_disabled( "subscriber_ids": CustomPrivacyRequestField( label="Subscriber IDs", value=["123", "456"] ), + "account_ids": CustomPrivacyRequestField( + label="Account IDs", value=[123, 456] + ), + "support_id": CustomPrivacyRequestField(label="Support ID", value=1), } ) assert privacy_request.get_cached_custom_privacy_request_fields() == {} @@ -1040,6 +1046,10 @@ def test_cache_custom_privacy_request_fields_collection_enabled_use_disabled( "subscriber_ids": CustomPrivacyRequestField( label="Subscriber IDs", value=["123", "456"] ), + "account_ids": CustomPrivacyRequestField( + label="Account IDs", value=[123, 456] + ), + "support_id": CustomPrivacyRequestField(label="Support ID", value=1), } ) assert privacy_request.get_cached_custom_privacy_request_fields() == {} @@ -1064,6 +1074,7 @@ def test_persist_custom_privacy_request_fields( "account_ids": CustomPrivacyRequestField( label="Account IDs", value=[123, 456] ), + "support_id": CustomPrivacyRequestField(label="Support ID", value=1), }, ) assert privacy_request.get_persisted_custom_privacy_request_fields() == { @@ -1071,6 +1082,7 @@ def test_persist_custom_privacy_request_fields( "last_name": {"label": "Last name", "value": "Doe"}, "subscriber_ids": {"label": "Subscriber IDs", "value": ["123", "456"]}, "account_ids": {"label": "Account IDs", "value": [123, 456]}, + "support_id": {"label": "Support ID", "value": 1}, } def test_persist_custom_privacy_request_fields_collection_disabled( @@ -1093,6 +1105,7 @@ def test_persist_custom_privacy_request_fields_collection_disabled( "account_ids": CustomPrivacyRequestField( label="Account IDs", value=[123, 456] ), + "support_id": CustomPrivacyRequestField(label="Support ID", value=1), }, ) assert privacy_request.get_persisted_custom_privacy_request_fields() == {} diff --git a/tests/ops/service/connectors/test_saas_queryconfig.py b/tests/ops/service/connectors/test_saas_queryconfig.py index 026aaf0e56..692792a261 100644 --- a/tests/ops/service/connectors/test_saas_queryconfig.py +++ b/tests/ops/service/connectors/test_saas_queryconfig.py @@ -688,6 +688,7 @@ def test_custom_privacy_request_fields( "account_ids": [123, 456], } } + opt_in_request: SaaSRequest = config.generate_consent_stmt( consent_policy, privacy_request, From 69028da1be1572fe843d7a79ac9ff3025c3b7bed Mon Sep 17 00:00:00 2001 From: Adrian Galvan Date: Thu, 21 Mar 2024 08:59:55 -0700 Subject: [PATCH 8/8] Updating change log --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a9290ad2a..c51278c712 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ The types of changes are: ### Changed - Updated privacy notice & experience forms to hide translation UI when user doesn't have translation feature [#4728](https://github.com/ethyca/fides/pull/4728) +- Custom privacy request fields now support list values [#4686](https://github.com/ethyca/fides/pull/4686) ### Fixed - Fixed responsive issues with the buttons on the integration screen [#4729](https://github.com/ethyca/fides/pull/4729)