From 29911dd4a4a78d641e10128f9082d9a2b4f0e07a Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Tue, 20 Jun 2023 13:05:14 -0500 Subject: [PATCH 1/2] Only create a privacy request when saving privacy preferences if there is at least one privacy notice that has system wide enforcement. --- .../endpoints/privacy_preference_endpoints.py | 59 +++--- tests/fixtures/application_fixtures.py | 17 ++ .../test_privacy_preference_endpoints.py | 175 +++++++++++++++++- 3 files changed, 226 insertions(+), 25 deletions(-) diff --git a/src/fides/api/api/v1/endpoints/privacy_preference_endpoints.py b/src/fides/api/api/v1/endpoints/privacy_preference_endpoints.py index 50f0373bbf..444cd2d52b 100644 --- a/src/fides/api/api/v1/endpoints/privacy_preference_endpoints.py +++ b/src/fides/api/api/v1/endpoints/privacy_preference_endpoints.py @@ -38,7 +38,11 @@ from fides.api.db.seed import DEFAULT_CONSENT_POLICY from fides.api.models.fides_user import FidesUser from fides.api.models.privacy_experience import PrivacyExperience -from fides.api.models.privacy_notice import PrivacyNotice, PrivacyNoticeHistory +from fides.api.models.privacy_notice import ( + EnforcementLevel, + PrivacyNotice, + PrivacyNoticeHistory, +) from fides.api.models.privacy_preference import ( CurrentPrivacyPreference, PrivacyPreferenceHistory, @@ -341,6 +345,7 @@ def _save_privacy_preferences_for_identities( fides_user_provided_identity, ProvidedIdentityType.fides_user_device_id ) + needs_server_side_propagation: bool = False for privacy_preference in request_data.preferences: historical_preference: PrivacyPreferenceHistory = PrivacyPreferenceHistory.create( db=db, @@ -377,10 +382,16 @@ def _save_privacy_preferences_for_identities( upserted_current_preference: CurrentPrivacyPreference = ( historical_preference.current_privacy_preference ) - created_historical_preferences.append(historical_preference) upserted_current_preferences.append(upserted_current_preference) + if ( + historical_preference.privacy_notice_history.enforcement_level + == EnforcementLevel.system_wide + ): + # At least one privacy notice has expected system wide enforcement + needs_server_side_propagation = True + identity = ( request_data.browser_identity if request_data.browser_identity else Identity() ) @@ -392,30 +403,32 @@ def _save_privacy_preferences_for_identities( verified_provided_identity.encrypted_value["value"], # type:ignore[index] ) - # Privacy Request needs to be created with respect to the *historical* privacy preferences - privacy_request_results: BulkPostPrivacyRequests = create_privacy_request_func( - db=db, - config_proxy=ConfigProxy(db), - data=[ - PrivacyRequestCreate( - identity=identity, - policy_key=request_data.policy_key or DEFAULT_CONSENT_POLICY, + if needs_server_side_propagation: + # Privacy Request needs to be created with respect to the *historical* privacy preferences + privacy_request_results: BulkPostPrivacyRequests = create_privacy_request_func( + db=db, + config_proxy=ConfigProxy(db), + data=[ + PrivacyRequestCreate( + identity=identity, + policy_key=request_data.policy_key or DEFAULT_CONSENT_POLICY, + ) + ], + authenticated=True, + privacy_preferences=created_historical_preferences, + ) + + if privacy_request_results.failed or not privacy_request_results.succeeded: + raise HTTPException( + status_code=HTTP_400_BAD_REQUEST, + detail=privacy_request_results.failed[0].message, ) - ], - authenticated=True, - privacy_preferences=created_historical_preferences, - ) - if privacy_request_results.failed or not privacy_request_results.succeeded: - raise HTTPException( - status_code=HTTP_400_BAD_REQUEST, - detail=privacy_request_results.failed[0].message, - ) + if consent_request: + # If we have a verified user identity, go ahead and update the associated ConsentRequest for record keeping + consent_request.privacy_request_id = privacy_request_results.succeeded[0].id + consent_request.save(db=db) - if consent_request: - # If we have a verified user identity, go ahead and update the associated ConsentRequest for record keeping - consent_request.privacy_request_id = privacy_request_results.succeeded[0].id - consent_request.save(db=db) return upserted_current_preferences diff --git a/tests/fixtures/application_fixtures.py b/tests/fixtures/application_fixtures.py index 6b08814b99..bf925ae74d 100644 --- a/tests/fixtures/application_fixtures.py +++ b/tests/fixtures/application_fixtures.py @@ -1569,6 +1569,23 @@ def privacy_notice_us_co_provide_service_operations(db: Session) -> Generator: yield privacy_notice +@pytest.fixture(scope="function") +def privacy_experience_france_overlay( + db: Session, experience_config_overlay +) -> Generator: + privacy_experience = PrivacyExperience.create( + db=db, + data={ + "component": ComponentType.overlay, + "region": PrivacyNoticeRegion.eu_fr, + "experience_config_id": experience_config_overlay.id, + }, + ) + + yield privacy_experience + privacy_experience.delete(db) + + @pytest.fixture(scope="function") def privacy_notice_eu_fr_provide_service_frontend_only(db: Session) -> Generator: privacy_notice = PrivacyNotice.create( diff --git a/tests/ops/api/v1/endpoints/test_privacy_preference_endpoints.py b/tests/ops/api/v1/endpoints/test_privacy_preference_endpoints.py index d670e9df62..1023675c1e 100644 --- a/tests/ops/api/v1/endpoints/test_privacy_preference_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_privacy_preference_endpoints.py @@ -138,7 +138,9 @@ def test_verify_then_set_privacy_preferences( request_body, privacy_notice, ): - """Verify code and then return privacy preferences""" + """Verify code, save, and then return privacy preferences + Privacy request also queued because a notice has system wide enforcement + """ masked_ip = "12.214.31.0" # Mocking because hostname for FastAPI TestClient is "testclient" mock_anonymize.return_value = masked_ip @@ -194,9 +196,101 @@ def test_verify_then_set_privacy_preferences( == PrivacyNoticeHistorySchema.from_orm(privacy_notice.histories[0]).dict() ) - privacy_preference_history.delete(db=db) + assert privacy_preference_history.privacy_request_id is not None assert run_privacy_request_mock.called + privacy_preference_history.delete(db=db) + + @pytest.mark.usefixtures( + "subject_identity_verification_required", "automatically_approved", "system" + ) + @mock.patch( + "fides.api.service.privacy_request.request_runner_service.run_privacy_request.delay" + ) + @mock.patch( + "fides.api.api.v1.endpoints.privacy_preference_endpoints.anonymize_ip_address" + ) + def test_verify_then_set_privacy_preferences_but_no_privacy_request_created( + self, + mock_anonymize, + run_privacy_request_mock, + provided_identity_and_consent_request, + api_client, + verification_code, + db: Session, + privacy_notice_eu_fr_provide_service_frontend_only, + privacy_experience_france_overlay, + consent_policy, + ): + """Verify code, save, and then return privacy preferences + Privacy request not queued because no notice has system wide enforcement + """ + masked_ip = "12.214.31.0" # Mocking because hostname for FastAPI TestClient is "testclient" + mock_anonymize.return_value = masked_ip + + _, consent_request = provided_identity_and_consent_request + consent_request.cache_identity_verification_code(verification_code) + + response = api_client.post( + f"{V1_URL_PREFIX}{CONSENT_REQUEST_PRIVACY_PREFERENCES_VERIFY.format(consent_request_id=consent_request.id)}", + json={"code": verification_code}, + ) + assert response.status_code == 200 + # Assert no existing privacy preferences exist for this identity + assert response.json() == {"items": [], "total": 0, "page": 1, "size": 50} + + request_body = { + "browser_identity": {"ga_client_id": "test"}, + "code": verification_code, + "preferences": [ + { + "privacy_notice_history_id": privacy_notice_eu_fr_provide_service_frontend_only.histories[ + 0 + ].id, + "preference": "opt_out", + } + ], + "policy_key": consent_policy.key, + "user_geography": "eu_fr", + "privacy_experience_id": privacy_experience_france_overlay.id, + "method": "button", + } + response = api_client.patch( + f"{V1_URL_PREFIX}{CONSENT_REQUEST_PRIVACY_PREFERENCES_WITH_ID.format(consent_request_id=consent_request.id)}", + json=request_body, + ) + assert response.status_code == 200 + assert len(response.json()) == 1 + + response_json = response.json()[0] + created_privacy_preference_history_id = response_json[ + "privacy_preference_history_id" + ] + privacy_preference_history = ( + db.query(PrivacyPreferenceHistory) + .filter( + PrivacyPreferenceHistory.id == created_privacy_preference_history_id + ) + .first() + ) + assert response_json["preference"] == "opt_out" + + assert ( + response_json["privacy_notice_history"] + == PrivacyNoticeHistorySchema.from_orm( + privacy_notice_eu_fr_provide_service_frontend_only.histories[0] + ).dict() + ) + db.refresh(consent_request) + # No privacy request created here + assert not consent_request.privacy_request_id + + # Privacy request not created or queued + assert privacy_preference_history.privacy_request_id is None + assert not run_privacy_request_mock.called + + privacy_preference_history.delete(db=db) + @pytest.mark.usefixtures( "subject_identity_verification_not_required", "automatically_approved" ) @@ -1037,8 +1131,12 @@ def test_save_privacy_preferences_bad_experience_id( @mock.patch( "fides.api.api.v1.endpoints.privacy_preference_endpoints.anonymize_ip_address" ) + @mock.patch( + "fides.api.service.privacy_request.request_runner_service.run_privacy_request.delay" + ) def test_save_privacy_preferences_with_respect_to_fides_user_device_id( self, + run_privacy_request_mock, mock_anonymize, db, api_client, @@ -1118,6 +1216,79 @@ def test_save_privacy_preferences_with_respect_to_fides_user_device_id( assert privacy_preference_history.url_recorded is None assert privacy_preference_history.method == ConsentMethod.button + # Privacy request created and queued because a privacy notice has system wide enforcement + assert privacy_preference_history.privacy_request_id is not None + assert run_privacy_request_mock.called + + current_preference.delete(db) + privacy_preference_history.delete(db) + + @mock.patch( + "fides.api.api.v1.endpoints.privacy_preference_endpoints.anonymize_ip_address" + ) + @mock.patch( + "fides.api.service.privacy_request.request_runner_service.run_privacy_request.delay" + ) + def test_save_privacy_preferences_for_fides_user_device_id_no_notice_has_system_wide_enforcement( + self, + run_privacy_request_mock, + mock_anonymize, + db, + api_client, + url, + consent_policy, + privacy_notice_eu_fr_provide_service_frontend_only, + privacy_experience_france_overlay, + ): + """PrivacyPreferences and CurrentPrivacyPreferences saved for the given fides user device id + but no privacy request created to propagate preferences, because all privacy notices + have frontend only enforcement + """ + masked_ip = "12.214.31.0" + mock_anonymize.return_value = masked_ip + + request_body = { + "browser_identity": { + "ga_client_id": "test", + "fides_user_device_id": "e4e573ba-d806-4e54-bdd8-3d2ff11d4f11", + }, + "preferences": [ + { + "privacy_notice_history_id": privacy_notice_eu_fr_provide_service_frontend_only.histories[ + 0 + ].id, + "preference": "opt_out", + } + ], + "policy_key": consent_policy.key, + "user_geography": "us_ca", + "privacy_experience_id": privacy_experience_france_overlay.id, + "method": "button", + } + + response = api_client.patch( + url, json=request_body, headers={"Origin": "http://localhost:8080"} + ) + assert response.status_code == 200 + response_json = response.json()[0] + assert response_json["preference"] == "opt_out" + assert ( + response_json["privacy_notice_history"]["id"] + == privacy_notice_eu_fr_provide_service_frontend_only.histories[0].id + ) + + # Fetch current privacy preference that was updated + current_preference = CurrentPrivacyPreference.get( + db, object_id=response_json["id"] + ) + # Get corresponding historical preference that was just created + privacy_preference_history = current_preference.privacy_preference_history + + assert ( + privacy_preference_history.privacy_request_id is None + ) # Privacy request not created + assert not run_privacy_request_mock.called # Privacy Request is not queued + current_preference.delete(db) privacy_preference_history.delete(db) From 30f58e9d42fdf1452da2cb2cfeebad9bdee44bd3 Mon Sep 17 00:00:00 2001 From: Dawn Pattison Date: Tue, 20 Jun 2023 13:47:56 -0500 Subject: [PATCH 2/2] Update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8728e306b..b3c094b47f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,7 +26,7 @@ The types of changes are: ### Changed - Removed `pyodbc` in favor of `pymssql` for handling SQL Server connections [#3435](https://github.com/ethyca/fides/pull/3435) - +- Only create a PrivacyRequest when saving consent if at least one notice has system-wide enforcement [#3626](https://github.com/ethyca/fides/pull/3626) ### Fixed - Fix race condition with consent modal link rendering [#3521](https://github.com/ethyca/fides/pull/3521)