From 41094b7274adb21532c23abbd8980d5d4d86495a Mon Sep 17 00:00:00 2001 From: Adrian Galvan Date: Tue, 24 Sep 2024 09:31:47 -0700 Subject: [PATCH] Updating custom integrations to support multiple identities (#5318) --- CHANGELOG.md | 1 + .../firebase_auth_request_overrides.py | 68 +++++++------- .../oracle_responsys_request_overrides.py | 76 ++++++++-------- src/fides/api/util/saas_util.py | 18 ---- .../saas/connector_runner.py | 28 +++--- .../test_firebase_auth_task.py | 88 +++++++++++++++++++ .../saas/test_oracle_responsys_task.py | 35 ++++++++ 7 files changed, 216 insertions(+), 98 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 29ebc5085c..1c8cf13c0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ The types of changes are: ### Fixed - Ignore `400` errors from Talkable's `person` endpoint. [#5317](https://github.com/ethyca/fides/pull/5317) - Fix Email Connector logs so they use configuration key instead of name [#5286](https://github.com/ethyca/fides/pull/5286) +- Updated Responsys and Firebase Auth integrations to allow multiple identities [#5318](https://github.com/ethyca/fides/pull/5318) ## [2.45.2](https://github.com/ethyca/fides/compare/2.45.1...2.45.2) diff --git a/src/fides/api/service/saas_request/override_implementations/firebase_auth_request_overrides.py b/src/fides/api/service/saas_request/override_implementations/firebase_auth_request_overrides.py index b204270da7..f1805b0c36 100644 --- a/src/fides/api/service/saas_request/override_implementations/firebase_auth_request_overrides.py +++ b/src/fides/api/service/saas_request/override_implementations/firebase_auth_request_overrides.py @@ -16,7 +16,7 @@ ) from fides.api.util.collection_util import Row from fides.api.util.logger import Pii -from fides.api.util.saas_util import get_identity +from fides.api.util.saas_util import get_identities @register("firebase_auth_user_access", [SaaSRequestType.READ]) @@ -35,32 +35,32 @@ def firebase_auth_user_access( # pylint: disable=R0914 app = initialize_firebase(secrets) processed_data = [] - identity = get_identity(privacy_request) - user: UserRecord - if identity == "email": - emails = input_data.get("email", []) - for email in emails: - try: - user = auth.get_user_by_email(email, app=app) - processed_data.append(user_record_to_row(user)) - except UserNotFoundError: - logger.warning( - f"Could not find user with email {Pii(email)} in firebase" - ) - elif identity == "phone_number": - phone_numbers = input_data.get("phone_number", []) - for phone_number in phone_numbers: - try: - user = auth.get_user_by_phone_number(phone_number, app=app) - processed_data.append(user_record_to_row(user)) - except UserNotFoundError: - logger.warning( - f"Could not find user with phone_number {Pii(phone_number)} in firebase" - ) - else: - raise FidesopsException( - "Unsupported identity type for Firebase connector. Currently only `email` and `phone_number` are supported" - ) + for identity in get_identities(privacy_request): + user: UserRecord + if identity == "email": + emails = input_data.get("email", []) + for email in emails: + try: + user = auth.get_user_by_email(email, app=app) + processed_data.append(user_record_to_row(user)) + except UserNotFoundError: + logger.warning( + f"Could not find user with email {Pii(email)} in firebase" + ) + elif identity == "phone_number": + phone_numbers = input_data.get("phone_number", []) + for phone_number in phone_numbers: + try: + user = auth.get_user_by_phone_number(phone_number, app=app) + processed_data.append(user_record_to_row(user)) + except UserNotFoundError: + logger.warning( + f"Could not find user with phone_number {Pii(phone_number)} in firebase" + ) + else: + raise FidesopsException( + "Unsupported identity type for Firebase connector. Currently only `email` and `phone_number` are supported" + ) return processed_data @@ -168,13 +168,13 @@ def retrieve_user_record( """ Utility that erasure functions can use to retrieve a Firebase `UserRecord` """ - identity = get_identity(privacy_request) - if identity == "email": - email = row_param_values.get("email", []) - return auth.get_user_by_email(email, app=app) - if identity == "phone_number": - phone_number = row_param_values.get("phone_number", []) - return auth.get_user_by_phone_number(phone_number, app=app) + for identity in get_identities(privacy_request): + if identity == "email": + email = row_param_values.get("email", []) + return auth.get_user_by_email(email, app=app) + if identity == "phone_number": + phone_number = row_param_values.get("phone_number", []) + return auth.get_user_by_phone_number(phone_number, app=app) raise FidesopsException( "Unsupported identity type for Firebase connector. Currently only `email` and `phone_number` are supported" diff --git a/src/fides/api/service/saas_request/override_implementations/oracle_responsys_request_overrides.py b/src/fides/api/service/saas_request/override_implementations/oracle_responsys_request_overrides.py index a091fee0d3..e130895bd0 100644 --- a/src/fides/api/service/saas_request/override_implementations/oracle_responsys_request_overrides.py +++ b/src/fides/api/service/saas_request/override_implementations/oracle_responsys_request_overrides.py @@ -15,7 +15,7 @@ register, ) from fides.api.util.collection_util import Row -from fides.api.util.saas_util import get_identity +from fides.api.util.saas_util import get_identities def oracle_responsys_config_parse_profile_lists(list_restrictions: str) -> List[str]: @@ -136,42 +136,46 @@ def oracle_responsys_profile_list_recipients_read( else: list_ids = list_ids_from_api - identity = get_identity(privacy_request) - if identity == "email": - query_ids = input_data.get("email", []) - query_attribute = "e" - elif identity == "phone_number": - query_ids = input_data.get("phone_number", []) - # Oracle Responsys doesn't support the initial + in phone numbers - for idx, query_id in enumerate(query_ids): - query_ids[idx] = query_id[1:] if query_id.startswith("+") else query_id - query_attribute = "m" - else: - raise FidesopsException( - "Unsupported identity type for Oracle Responsys connector. Currently only `email` and `phone_number` are supported" - ) + for identity in get_identities(privacy_request): + if identity == "email": + query_ids = input_data.get("email", []) + query_attribute = "e" + elif identity == "phone_number": + query_ids = input_data.get("phone_number", []) + # Oracle Responsys doesn't support the initial + in phone numbers + for idx, query_id in enumerate(query_ids): + query_ids[idx] = query_id[1:] if query_id.startswith("+") else query_id + query_attribute = "m" + else: + raise FidesopsException( + "Unsupported identity type for Oracle Responsys connector. Currently only `email` and `phone_number` are supported" + ) - body = {"fieldList": ["all"], "ids": query_ids, "queryAttribute": query_attribute} - for list_id in list_ids: - members_response = client.send( - SaaSRequestParams( - method=HTTPMethod.POST, - path=f"/rest/api/v1.3/lists/{list_id}/members", - query_params={"action": "get"}, - body=json.dumps(body), - headers={"Content-Type": "application/json"}, - ), - [404], # Returns a 404 if no list member is found - ) - serialized_data = oracle_responsys_serialize_record_data(members_response) - if serialized_data: - for record in serialized_data: - # Filter out the keys with falsy values and append it - filtered_records = { - key: value for key, value in record.items() if value - } - filtered_records["profile_list_id"] = list_id - results.append(filtered_records) + body = { + "fieldList": ["all"], + "ids": query_ids, + "queryAttribute": query_attribute, + } + for list_id in list_ids: + members_response = client.send( + SaaSRequestParams( + method=HTTPMethod.POST, + path=f"/rest/api/v1.3/lists/{list_id}/members", + query_params={"action": "get"}, + body=json.dumps(body), + headers={"Content-Type": "application/json"}, + ), + [404], # Returns a 404 if no list member is found + ) + serialized_data = oracle_responsys_serialize_record_data(members_response) + if serialized_data: + for record in serialized_data: + # Filter out the keys with falsy values and append it + filtered_records = { + key: value for key, value in record.items() if value + } + filtered_records["profile_list_id"] = list_id + results.append(filtered_records) return results diff --git a/src/fides/api/util/saas_util.py b/src/fides/api/util/saas_util.py index ee07916f81..d988d7ae70 100644 --- a/src/fides/api/util/saas_util.py +++ b/src/fides/api/util/saas_util.py @@ -433,24 +433,6 @@ def map_param_values( ) -def get_identity(privacy_request: Optional[PrivacyRequest]) -> Optional[str]: - """ - Returns a single identity or raises an exception if more than one identity is defined - """ - - if not privacy_request: - return None - - identity_data: Dict[str, Any] = privacy_request.get_cached_identity_data() - # filters out keys where associated value is None or empty str - identities = list({k for k, v in identity_data.items() if v}) - if len(identities) > 1: - raise FidesopsException( - "Only one identity can be specified for SaaS connector traversal" - ) - return identities[0] if identities else None - - def get_identities(privacy_request: Optional[PrivacyRequest]) -> Set[str]: """ Returns a set of cached identity names for the provided privacy request. diff --git a/tests/ops/integration_tests/saas/connector_runner.py b/tests/ops/integration_tests/saas/connector_runner.py index ebf9104849..0f9e6e80e9 100644 --- a/tests/ops/integration_tests/saas/connector_runner.py +++ b/tests/ops/integration_tests/saas/connector_runner.py @@ -158,6 +158,7 @@ async def strict_erasure_request( erasure_policy: Policy, identities: Dict[str, Any], privacy_request_id: Optional[str] = None, + skip_access_results_check: Optional[bool] = False, ) -> Tuple[Dict, Dict]: """ Erasure request with masking_strict set to true, @@ -182,6 +183,7 @@ async def non_strict_erasure_request( erasure_policy: Policy, identities: Dict[str, Any], privacy_request_id: Optional[str] = None, + skip_access_results_check: Optional[bool] = False, ) -> Tuple[Dict, Dict]: """ Erasure request with masking_strict set to false, @@ -194,7 +196,11 @@ async def non_strict_erasure_request( CONFIG.execution.masking_strict = False access_results, erasure_results = await self._base_erasure_request( - access_policy, erasure_policy, identities, privacy_request_id + access_policy, + erasure_policy, + identities, + privacy_request_id, + skip_access_results_check, ) # reset masking_strict value @@ -295,6 +301,7 @@ async def _base_erasure_request( erasure_policy: Policy, identities: Dict[str, Any], privacy_request_id: Optional[str] = None, + skip_access_results_check: Optional[bool] = False, ) -> Tuple[Dict, Dict]: from tests.conftest import access_runner_tester, erasure_runner_tester @@ -340,15 +347,16 @@ async def _base_erasure_request( self.db, ) - if ( - ActionType.access - in SaaSConfig(**self.connection_config.saas_config).supported_actions - ): - # verify we returned at least one row for each collection in the dataset - for collection in self.dataset["collections"]: - assert len( - access_results[f"{fides_key}:{collection['name']}"] - ), f"No rows returned for collection '{collection['name']}'" + if not skip_access_results_check: + if ( + ActionType.access + in SaaSConfig(**self.connection_config.saas_config).supported_actions + ): + # verify we returned at least one row for each collection in the dataset + for collection in self.dataset["collections"]: + assert len( + access_results[f"{fides_key}:{collection['name']}"] + ), f"No rows returned for collection '{collection['name']}'" erasure_results = erasure_runner_tester( privacy_request, diff --git a/tests/ops/integration_tests/saas/request_override/test_firebase_auth_task.py b/tests/ops/integration_tests/saas/request_override/test_firebase_auth_task.py index e6c582700b..187b0a839e 100644 --- a/tests/ops/integration_tests/saas/request_override/test_firebase_auth_task.py +++ b/tests/ops/integration_tests/saas/request_override/test_firebase_auth_task.py @@ -256,6 +256,94 @@ async def test_firebase_auth_access_request_phone_number_identity( assert "photo_url" not in provider_data[1].keys() +@pytest.mark.integration_saas +@pytest.mark.asyncio +@pytest.mark.parametrize( + "dsr_version", + ["use_dsr_3_0", "use_dsr_2_0"], +) +async def test_firebase_auth_access_request_multiple_identities( + db, + policy, + dsr_version, + request, + privacy_request, + firebase_auth_connection_config, + firebase_auth_dataset_config, + firebase_auth_user: auth.ImportUserRecord, +) -> None: + """Full access request based on the Firebase Auth SaaS config using a phone number identity""" + request.getfixturevalue(dsr_version) # REQUIRED to test both DSR 3.0 and 2.0 + clear_cache_identities(privacy_request.id) + + identity = Identity( + **{ + "email": firebase_auth_user.email, + "phone_number": firebase_auth_user.phone_number, + } + ) + privacy_request.cache_identity(identity) + + dataset_name = firebase_auth_connection_config.get_saas_config().fides_key + merged_graph = firebase_auth_dataset_config.get_graph() + graph = DatasetGraph(merged_graph) + + v = access_runner_tester( + privacy_request, + policy, + graph, + [firebase_auth_connection_config], + {"phone_number": firebase_auth_user.phone_number}, + db, + ) + + assert_rows_match( + v[f"{dataset_name}:user"], + min_size=1, + keys=[ + "uid", + "email", + "display_name", + "photo_url", + "disabled", + "email_verified", + "phone_number", + ], + ) + response_user = v[f"{dataset_name}:user"][0] + assert response_user["uid"] == firebase_auth_user.uid + assert response_user["email"] == firebase_auth_user.email + assert response_user["display_name"] == firebase_auth_user.display_name + assert response_user["phone_number"] == firebase_auth_user.phone_number + assert response_user["photo_url"] == firebase_auth_user.photo_url + assert response_user["disabled"] == firebase_auth_user.disabled + assert response_user["email_verified"] == firebase_auth_user.email_verified + + provider_data = response_user["provider_data"] + assert ( + provider_data[0]["provider_id"] + == firebase_auth_user.provider_data[0].provider_id + ) + assert ( + provider_data[0]["display_name"] + == firebase_auth_user.provider_data[0].display_name + ) + assert provider_data[0]["email"] == firebase_auth_user.provider_data[0].email + assert ( + provider_data[0]["photo_url"] == firebase_auth_user.provider_data[0].photo_url + ) + assert ( + provider_data[1]["provider_id"] + == firebase_auth_user.provider_data[1].provider_id + ) + assert ( + provider_data[1]["display_name"] + == firebase_auth_user.provider_data[1].display_name + ) + assert provider_data[1]["email"] == firebase_auth_user.provider_data[1].email + assert "photo_url" not in provider_data[1].keys() + + @pytest.mark.skip( "Re-enable this test if the general config needs to test the user update functionality" ) 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 f812b4e7f1..4e30a77622 100644 --- a/tests/ops/integration_tests/saas/test_oracle_responsys_task.py +++ b/tests/ops/integration_tests/saas/test_oracle_responsys_task.py @@ -120,3 +120,38 @@ async def test_non_strict_erasure_request_by_phone_number( "oracle_responsys_instance:profile_extension": 0, "oracle_responsys_instance:profile_extension_recipient": 0, } + + @pytest.mark.parametrize( + "dsr_version", + ["use_dsr_3_0", "use_dsr_2_0"], + ) + async def test_non_strict_erasure_request_by_multiple_identities( + self, + dsr_version, + request, + oracle_responsys_runner: ConnectorRunner, + policy: Policy, + erasure_policy_string_rewrite: Policy, + oracle_responsys_identity_email: str, + oracle_responsys_erasure_identity_phone_number: str, + oracle_responsys_erasure_data, + ): + request.getfixturevalue(dsr_version) # REQUIRED to test both DSR 3.0 and 2.0 + + ( + _, + erasure_results, + ) = await oracle_responsys_runner.non_strict_erasure_request( + access_policy=policy, + erasure_policy=erasure_policy_string_rewrite, + identities={ + "email": oracle_responsys_identity_email, + "phone_number": oracle_responsys_erasure_identity_phone_number, + }, + skip_access_results_check=True, + ) + assert erasure_results == { + "oracle_responsys_instance:profile_list_recipient": 1, + "oracle_responsys_instance:profile_list": 0, + "oracle_responsys_instance:profile_extension_recipient": 0, + }