LA 83 Fix Salesforce Erasure Data Flow #5452

Vagoasdf · 2024-11-04T15:09:01Z

Description Of Changes

Started as a removal of the Account endpoint since that was data from the Business, not a person. Ended up creating custom overrides requests so the validation rules for the other endpoints were passing.

currently the Requests Overrides do two things

Checks if the field is an email. If it is, changes it to masked@company.com
Checks if the field is over 40 characters after masking. If it is, truncating it

Code Changes

Request overrides for the Salesforce endpoints created

Steps to Confirm

Create a new Business Account on Salesforces from the Admin Template
Create two contacts for that Business Account
Set up the Salesforces Integration
Mask one of the Contacts by Email
The Other Contact should not be masked
The Account should not be masked

Pre-Merge Checklist

All CI Pipelines Succeeded
Issue Requirements are Met
Update CHANGELOG.md

Including Email Format Masking Including the rest of the update endpoints

vercel · 2024-11-04T15:09:07Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment

Name	Status	Preview	Comments	Updated (UTC)
fides-plus-nightly	⬜️ Ignored (Inspect)	Visit Preview		Nov 6, 2024 5:57pm

cypress · 2024-11-04T15:24:58Z

fides Run #10844

Run Properties: Passed #10844 • 30b8c335ed ℹ️: Merge f03cec6daaa9ecd221b5771380106cf5a9c36887 into 0234347960163e2b413a7f6ba5cf...

Project	`fides`
Branch Review	`refs/pull/5452/merge`
Run status	`Passed #10844`
Run duration	`00m 39s`
Commit	`30b8c335ed ℹ️: Merge f03cec6daaa9ecd221b5771380106cf5a9c36887 into 0234347960163e2b413a7f6ba5cf...`
Committer	`Bruno Gutierrez Rios`
View all properties for this run ↗︎

Test results
Failures	`0`
Flaky	`0`
Pending	`0`
Skipped	`0`
Passing	`4`
View all changes introduced in this branch ↗︎

codecov · 2024-11-04T16:07:46Z

Codecov Report

Attention: Patch coverage is 29.62963% with 38 lines in your changes missing coverage. Please review.

Project coverage is 85.30%. Comparing base (0234347) to head (f03cec6).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...de_implementations/salesforce_request_overrides.py	29.62%	38 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5452      +/-   ##
==========================================
- Coverage   85.42%   85.30%   -0.13%     
==========================================
  Files         385      386       +1     
  Lines       24152    24206      +54     
  Branches     2630     2636       +6     
==========================================
+ Hits        20633    20649      +16     
- Misses       2966     3004      +38     
  Partials      553      553

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

galvana · 2024-11-04T21:50:58Z

CHANGELOG.md

@@ -21,6 +21,9 @@ The types of changes are:
 - Added DataHub integration config [#5401](https://github.com/ethyca/fides/pull/5401)
 - Added keepalive settings to the Redshift integration [#5433](https://github.com/ethyca/fides/pull/5433)

+### Fixed
+- Updating Salesforce erasure request with overrides so it properly passes validation. Removing Account endpoint since it does not represents PII [#5452](https://github.com/ethyca/fides/pull/5452)


Suggested change

- Updating Salesforce erasure request with overrides so it properly passes validation. Removing Account endpoint since it does not represents PII [#5452](https://github.com/ethyca/fides/pull/5452)

- Updating Salesforce erasure request with overrides so it properly passes validation. Removing Account endpoint since it does not contain user data [#5452](https://github.com/ethyca/fides/pull/5452)

galvana · 2024-11-04T22:57:07Z

src/fides/api/service/saas_request/override_implementations/salesforce_request_overrides.py

+# using the privacy request id to have an unique id
+def maskEmail(masked_object_fields: Dict, email_field: str) -> Dict:
+    if email_field in masked_object_fields:
+        masked_object_fields[email_field] = "Masked@company.com"


Suggested change

masked_object_fields[email_field] = "Masked@company.com"

masked_object_fields[email_field] = "masked@company.com"

galvana · 2024-11-04T23:25:26Z

src/fides/api/service/saas_request/override_implementations/salesforce_request_overrides.py

+def truncateFieldsTo40Characters(masked_object_fields: Dict) -> Dict:
+    for key in masked_object_fields:
+        logger.info(key)
+        logger.info((masked_object_fields[key]))
+        if not isinstance(masked_object_fields[key], str):
+            continue
+        if len(masked_object_fields[key]) > 40:
+            masked_object_fields[key] = masked_object_fields[key][:40]
+    return masked_object_fields


We can simplify this a bit and only log a message when we need to truncate. The comment you added is helpful but it would make sense to add it as a doc string. Finally, make sure to use snake case since that's the convention for Python.

Suggested change

def truncateFieldsTo40Characters(masked_object_fields: Dict) -> Dict:

for key in masked_object_fields:

logger.info(key)

logger.info((masked_object_fields[key]))

if not isinstance(masked_object_fields[key], str):

continue

if len(masked_object_fields[key]) > 40:

masked_object_fields[key] = masked_object_fields[key][:40]

return masked_object_fields

def truncate_fields(masked_object_fields: Dict) -> Dict:

"""

Check if the masked field is over 40 characters long, if so truncate it to 40 characters.

"""

for key in masked_object_fields:

value = masked_object_fields[key]

if isinstance(value, str) and len(value) > 40:

logger.info("Truncating {key} field to 40 characters")

masked_object_fields[key] = value[:40]

return masked_object_fields

galvana · 2024-11-04T23:26:39Z

src/fides/api/service/saas_request/override_implementations/salesforce_request_overrides.py

+# Masking Email properly so it does not breaks validation rules
+# using the privacy request id to have an unique id
+def maskEmail(masked_object_fields: Dict, email_field: str) -> Dict:


Same as above

Suggested change

# Masking Email properly so it does not breaks validation rules

# using the privacy request id to have an unique id

def maskEmail(masked_object_fields: Dict, email_field: str) -> Dict:

def mask_email(masked_object_fields: Dict, email_field: str) -> Dict:

"""Masking email fields properly so they don't break validation rules."""

galvana · 2024-11-04T23:48:52Z

tests/ops/integration_tests/saas/test_salesforce_task.py

I know we have these tests flagged as @pytest.mark.skip(reason="Currently unable to test OAuth2 connectors") but can you try the following:

Run the tests by hard-coding the access_token in your local saas_config.toml?

Use the erasure_policy_string_rewrite_name_and_email policy fixture so we can see the email being masked.

Use the new ConnectorRunner pattern for the task tests

Verify we have multiple users in Salesforce to verify we only return results for one user

Verify that multiple entities can have the same email (the masked@company email we're using)

src/fides/api/service/saas_request/override_implementations/salesforce_request_overrides.py

Vagoasdf · 2024-11-05T19:46:31Z

By the looks of the Campaings Member API docs, it looks like the Campaing Members represents a relationship between Leads and Contacts, and as such, their fields should be updated on their respective endpoints. We already have some fields on the campaign_members dataset collection as read_only, and those that are not, are throwing an 400 error as we are not able to update them.

removing the Update endpoint on campaign member. We still want to read it, even if it's a bit of duplicate data, because its relevant to see if its on a campaign, but it should not be updated at all. As we can see on the Screenshots below, updating the Contact and Lead would update the campaing member view correctly

Campaign Members are a relationship between a campaign and a lead or a contact. Updating the latter two would update user data

galvana · 2024-11-06T17:45:12Z

tests/ops/integration_tests/saas/test_salesforce_task.py



-@pytest.mark.skip(reason="Currently unable to test OAuth2 connectors")
+#@pytest.mark.skip(reason="Currently unable to test OAuth2 connectors")


Suggested change

#@pytest.mark.skip(reason="Currently unable to test OAuth2 connectors")

@pytest.mark.skip(reason="Currently unable to test OAuth2 connectors")

cypress · 2024-11-06T18:28:20Z

fides Run #10846

Run Properties: Passed #10846 • 0e36182fd8: LA 83 Fix Salesforce Erasure Data Flow (#5452)

Project	`fides`
Branch Review	`main`
Run status	`Passed #10846`
Run duration	`00m 38s`
Commit	`0e36182fd8: LA 83 Fix Salesforce Erasure Data Flow (#5452)`
Committer	`Bruno Gutierrez Rios`
View all properties for this run ↗︎

Test results
Failures	`0`
Flaky	`0`
Pending	`0`
Skipped	`0`
Passing	`4`
View all changes introduced in this branch ↗︎

Vagoasdf added 3 commits November 4, 2024 11:59

removing the Account checks on the test

d86ca8d

Adding Base request override for Salesforce Contacts

f3b585c

Expanding request override

06f7712

Including Email Format Masking Including the rest of the update endpoints

Vagoasdf requested a review from a team as a code owner November 4, 2024 15:09

Vagoasdf changed the title ~~LA 82 Fix Salesforce Erasure Data Flow~~ LA 83 Fix Salesforce Erasure Data Flow Nov 4, 2024

Vagoasdf added 2 commits November 4, 2024 12:31

running static linters

f41869d

Updating changelog

aba978b

galvana requested changes Nov 4, 2024

View reviewed changes

galvana reviewed Nov 5, 2024

View reviewed changes

src/fides/api/service/saas_request/override_implementations/salesforce_request_overrides.py Show resolved Hide resolved

Vagoasdf added 4 commits November 5, 2024 10:14

Addressing request comments: Improving Comments and code readability

a694e43

collapsing two ifs into one

7d6b3b3

Adding Base ConnectorRunner Pattern for Salesforce Testing

38ae9b3

Passing the tests into the ConectorRunner Architecture

6022c47

Vagoasdf added 2 commits November 5, 2024 17:28

Removing Campaign Members Update

f9a4125

Campaign Members are a relationship between a campaign and a lead or a contact. Updating the latter two would update user data

Adding back campaing members to the validation of erasure results

b2efa3b

Vagoasdf requested a review from galvana November 6, 2024 16:04

Vagoasdf added 2 commits November 6, 2024 14:39

Updated after removing the Primary Key field from the dataset

03b1654

Merge branch 'main' into LA-82-Remove-salesforce-account-endpint

1e521dd

galvana approved these changes Nov 6, 2024

View reviewed changes

vercel bot deployed to Preview November 6, 2024 17:46 View deployment

Adding back the tests skip

f03cec6

Vagoasdf merged commit 0e36182 into main Nov 6, 2024
16 of 18 checks passed

Vagoasdf deleted the LA-82-Remove-salesforce-account-endpint branch November 6, 2024 18:12

Vagoasdf restored the LA-82-Remove-salesforce-account-endpint branch November 6, 2024 18:35

Vagoasdf mentioned this pull request Nov 6, 2024

Running static checks properly #5471

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LA 83 Fix Salesforce Erasure Data Flow #5452

LA 83 Fix Salesforce Erasure Data Flow #5452

Vagoasdf commented Nov 4, 2024 •

edited

Loading

vercel bot commented Nov 4, 2024 •

edited

Loading

cypress bot commented Nov 4, 2024 •

edited

Loading

codecov bot commented Nov 4, 2024 •

edited

Loading

galvana Nov 4, 2024

galvana Nov 4, 2024

galvana Nov 4, 2024

galvana Nov 4, 2024

galvana Nov 4, 2024

Vagoasdf commented Nov 5, 2024 •

edited

Loading

galvana Nov 6, 2024

cypress bot commented Nov 6, 2024

	- Updating Salesforce erasure request with overrides so it properly passes validation. Removing Account endpoint since it does not represents PII [#5452](https://github.com/ethyca/fides/pull/5452)
	- Updating Salesforce erasure request with overrides so it properly passes validation. Removing Account endpoint since it does not contain user data [#5452](https://github.com/ethyca/fides/pull/5452)

	masked_object_fields[email_field] = "Masked@company.com"
	masked_object_fields[email_field] = "masked@company.com"



		@pytest.mark.skip(reason="Currently unable to test OAuth2 connectors")
		#@pytest.mark.skip(reason="Currently unable to test OAuth2 connectors")

LA 83 Fix Salesforce Erasure Data Flow #5452

LA 83 Fix Salesforce Erasure Data Flow #5452

Conversation

Vagoasdf commented Nov 4, 2024 • edited Loading

Description Of Changes

Code Changes

Steps to Confirm

Pre-Merge Checklist

vercel bot commented Nov 4, 2024 • edited Loading

cypress bot commented Nov 4, 2024 • edited Loading

codecov bot commented Nov 4, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vagoasdf commented Nov 5, 2024 • edited Loading

Choose a reason for hiding this comment

cypress bot commented Nov 6, 2024

Vagoasdf commented Nov 4, 2024 •

edited

Loading

vercel bot commented Nov 4, 2024 •

edited

Loading

cypress bot commented Nov 4, 2024 •

edited

Loading

codecov bot commented Nov 4, 2024 •

edited

Loading

Vagoasdf commented Nov 5, 2024 •

edited

Loading