Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating custom integrations to support multiple identities #5318

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand All @@ -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


Expand Down Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]:
Expand Down Expand Up @@ -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":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could extract the code fragment for retrieving the query_ids and query_attributes into its own method for readability, now that we are one level deeper with a for loop. It would help readability

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


Expand Down
18 changes: 0 additions & 18 deletions src/fides/api/util/saas_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the error we're currently seeing, but this isn't accurate since we support multiple identities now

)
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.
Expand Down
28 changes: 18 additions & 10 deletions tests/ops/integration_tests/saas/connector_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick fix to avoid checking for access data. This is necessary since our test account for Responsys isn't going to return access data for every collection.

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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down
Loading
Loading