From fda86d4bb367b38b5d500ed5f2a58b3b40da8500 Mon Sep 17 00:00:00 2001 From: Vagoasdf Date: Tue, 16 Dec 2025 12:14:55 -0300 Subject: [PATCH 01/15] Fixing up guard access requests --- src/fides/api/service/connectors/saas_connector.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/fides/api/service/connectors/saas_connector.py b/src/fides/api/service/connectors/saas_connector.py index 8b53545ac30..1211418c93b 100644 --- a/src/fides/api/service/connectors/saas_connector.py +++ b/src/fides/api/service/connectors/saas_connector.py @@ -277,17 +277,19 @@ def retrieve_data( # Delegate async requests with get_db() as db: - # Guard clause to ensure we only run async access requests for access requests - if self.guard_access_request(policy): - if async_dsr_strategy := _get_async_dsr_strategy( - db, request_task, query_config, ActionType.access - ): + if async_dsr_strategy := _get_async_dsr_strategy( + db, request_task, query_config, ActionType.access + ): + if self.guard_access_request(policy): return async_dsr_strategy.async_retrieve_data( client=self.create_client(), request_task_id=request_task.id, query_config=query_config, input_data=input_data, ) + else: + logger.info(f"Skipping async access request for policy: {policy}") + return [] rows: List[Row] = [] for read_request in read_requests: From 241f815a39ea3d1dbfefa5a712c7b81d929e9947 Mon Sep 17 00:00:00 2001 From: Vagoasdf Date: Tue, 16 Dec 2025 12:41:41 -0300 Subject: [PATCH 02/15] Add testing for the guard --- .../service/connectors/test_saas_connector.py | 117 +++++++++++++++++- 1 file changed, 116 insertions(+), 1 deletion(-) diff --git a/tests/ops/service/connectors/test_saas_connector.py b/tests/ops/service/connectors/test_saas_connector.py index 3741cccb36c..f913f97b431 100644 --- a/tests/ops/service/connectors/test_saas_connector.py +++ b/tests/ops/service/connectors/test_saas_connector.py @@ -22,7 +22,7 @@ from fides.api.graph.graph import DatasetGraph, Node from fides.api.graph.traversal import Traversal, TraversalNode from fides.api.models.consent_automation import ConsentAutomation -from fides.api.models.policy import Policy +from fides.api.models.policy import ActionType, Policy, Rule, RuleTarget from fides.api.models.privacy_notice import UserConsentPreference from fides.api.models.privacy_request import PrivacyRequest, RequestTask from fides.api.models.worker_task import ExecutionLogStatus @@ -1306,3 +1306,118 @@ def test_callback_succeeded_mask_data( ) == 5 ) + + @mock.patch("fides.api.service.connectors.saas_connector.AuthenticatedClient.send") + def test_guard_access_request_with_access_policy( + self, + mock_send, + privacy_request, + saas_async_example_connection_config, + async_graph, + ): + """ + Test that guard_access_request allows async access requests to run + when the policy has access rules (access request scenario). + """ + connector: SaaSConnector = get_connector(saas_async_example_connection_config) + mock_send().json.return_value = {"results_pending": True} + + # Get access request task + request_task = privacy_request.access_tasks.filter( + RequestTask.collection_name == "user" + ).first() + execution_node = ExecutionNode(request_task) + + # Policy has access rules, so guard should return True and async_retrieve_data should be called + # This will raise AwaitingAsyncTask + with pytest.raises(AwaitingAsyncTask): + connector.retrieve_data( + execution_node, + privacy_request.policy, + privacy_request, + request_task, + {}, + ) + + @mock.patch( + "fides.api.service.connectors.saas_connector._get_async_dsr_strategy" + ) + def test_guard_access_request_with_erasure_only_policy( + self, + mock_get_async_strategy, + db, + privacy_request, + saas_async_example_connection_config, + async_graph, + oauth_client, + ): + """ + Test that guard_access_request skips async access requests + when the policy has no access rules (erasure-only request scenario). + """ + # Create an erasure-only policy (no access rules) + erasure_only_policy = Policy.create( + db=db, + data={ + "name": "Erasure Only Policy", + "key": "erasure_only_policy_test", + "client_id": oauth_client.id, + }, + ) + + erasure_rule = Rule.create( + db=db, + data={ + "action_type": ActionType.erasure, + "name": "Erasure Rule", + "key": "erasure_rule_test", + "policy_id": erasure_only_policy.id, + "masking_strategy": { + "strategy": "null_rewrite", + "configuration": {}, + }, + "client_id": oauth_client.id, + }, + ) + + RuleTarget.create( + db=db, + data={ + "data_category": "user.name", + "rule_id": erasure_rule.id, + "client_id": oauth_client.id, + }, + ) + + connector: SaaSConnector = get_connector(saas_async_example_connection_config) + + # Mock async strategy to verify it's not called + mock_async_strategy = Mock() + mock_get_async_strategy.return_value = mock_async_strategy + + # Get access request task + request_task = privacy_request.access_tasks.filter( + RequestTask.collection_name == "user" + ).first() + execution_node = ExecutionNode(request_task) + + # Policy has no access rules, so guard should return False + # async_retrieve_data should NOT be called, and we should get empty list + result = connector.retrieve_data( + execution_node, + erasure_only_policy, + privacy_request, + request_task, + {}, + ) + + # Should return empty list without calling async_retrieve_data + assert result == [] + mock_async_strategy.async_retrieve_data.assert_not_called() + + # Cleanup + try: + erasure_rule.delete(db) + erasure_only_policy.delete(db) + except Exception: + pass From bf39bb7b3689df447e1c0f769a692222001e40ae Mon Sep 17 00:00:00 2001 From: Vagoasdf Date: Tue, 16 Dec 2025 12:49:29 -0300 Subject: [PATCH 03/15] running static checks --- tests/ops/service/connectors/test_saas_connector.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/ops/service/connectors/test_saas_connector.py b/tests/ops/service/connectors/test_saas_connector.py index f913f97b431..aef1df98d69 100644 --- a/tests/ops/service/connectors/test_saas_connector.py +++ b/tests/ops/service/connectors/test_saas_connector.py @@ -1339,9 +1339,7 @@ def test_guard_access_request_with_access_policy( {}, ) - @mock.patch( - "fides.api.service.connectors.saas_connector._get_async_dsr_strategy" - ) + @mock.patch("fides.api.service.connectors.saas_connector._get_async_dsr_strategy") def test_guard_access_request_with_erasure_only_policy( self, mock_get_async_strategy, From fb061750e264eeed9b99034ec6fe08a5cdf49a3d Mon Sep 17 00:00:00 2001 From: Vagoasdf Date: Tue, 16 Dec 2025 12:56:53 -0300 Subject: [PATCH 04/15] removing unnecesary else --- src/fides/api/service/connectors/saas_connector.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/fides/api/service/connectors/saas_connector.py b/src/fides/api/service/connectors/saas_connector.py index 1211418c93b..e41f5e11222 100644 --- a/src/fides/api/service/connectors/saas_connector.py +++ b/src/fides/api/service/connectors/saas_connector.py @@ -287,9 +287,8 @@ def retrieve_data( query_config=query_config, input_data=input_data, ) - else: - logger.info(f"Skipping async access request for policy: {policy}") - return [] + logger.info(f"Skipping async access request for policy: {policy.name}") + return [] rows: List[Row] = [] for read_request in read_requests: From f37ac0389032ab936c277f676e51222f2da7f57f Mon Sep 17 00:00:00 2001 From: Vagoasdf Date: Tue, 16 Dec 2025 15:11:33 -0300 Subject: [PATCH 05/15] Update patch to comply code coverage --- .../ops/service/connectors/test_saas_connector.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/tests/ops/service/connectors/test_saas_connector.py b/tests/ops/service/connectors/test_saas_connector.py index aef1df98d69..0f0b20d0953 100644 --- a/tests/ops/service/connectors/test_saas_connector.py +++ b/tests/ops/service/connectors/test_saas_connector.py @@ -1339,10 +1339,10 @@ def test_guard_access_request_with_access_policy( {}, ) - @mock.patch("fides.api.service.connectors.saas_connector._get_async_dsr_strategy") + @mock.patch("fides.api.service.connectors.saas_connector.SaaSConnector.create_client") def test_guard_access_request_with_erasure_only_policy( self, - mock_get_async_strategy, + mock_create_client, db, privacy_request, saas_async_example_connection_config, @@ -1352,6 +1352,7 @@ def test_guard_access_request_with_erasure_only_policy( """ Test that guard_access_request skips async access requests when the policy has no access rules (erasure-only request scenario). + This test ensures coverage of the logger.info and return [] lines. """ # Create an erasure-only policy (no access rules) erasure_only_policy = Policy.create( @@ -1389,18 +1390,15 @@ def test_guard_access_request_with_erasure_only_policy( connector: SaaSConnector = get_connector(saas_async_example_connection_config) - # Mock async strategy to verify it's not called - mock_async_strategy = Mock() - mock_get_async_strategy.return_value = mock_async_strategy - # Get access request task request_task = privacy_request.access_tasks.filter( RequestTask.collection_name == "user" ).first() execution_node = ExecutionNode(request_task) - # Policy has no access rules, so guard should return False - # async_retrieve_data should NOT be called, and we should get empty list + # Verify guard_access_request returns False for erasure-only policy + assert connector.guard_access_request(erasure_only_policy) is False + result = connector.retrieve_data( execution_node, erasure_only_policy, @@ -1411,7 +1409,6 @@ def test_guard_access_request_with_erasure_only_policy( # Should return empty list without calling async_retrieve_data assert result == [] - mock_async_strategy.async_retrieve_data.assert_not_called() # Cleanup try: From 17a4eda02b586fb86a987758566684b51c2043cc Mon Sep 17 00:00:00 2001 From: Vagoasdf Date: Tue, 16 Dec 2025 15:21:04 -0300 Subject: [PATCH 06/15] static checks running again --- tests/ops/service/connectors/test_saas_connector.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/ops/service/connectors/test_saas_connector.py b/tests/ops/service/connectors/test_saas_connector.py index 0f0b20d0953..3a5b036bcb8 100644 --- a/tests/ops/service/connectors/test_saas_connector.py +++ b/tests/ops/service/connectors/test_saas_connector.py @@ -1339,7 +1339,9 @@ def test_guard_access_request_with_access_policy( {}, ) - @mock.patch("fides.api.service.connectors.saas_connector.SaaSConnector.create_client") + @mock.patch( + "fides.api.service.connectors.saas_connector.SaaSConnector.create_client" + ) def test_guard_access_request_with_erasure_only_policy( self, mock_create_client, From c56dea883320e750a4a462da242c8f23b15c913b Mon Sep 17 00:00:00 2001 From: Vagoasdf Date: Tue, 16 Dec 2025 16:21:21 -0300 Subject: [PATCH 07/15] Updating guard so it wont interfire with Callback --- src/fides/api/service/connectors/saas_connector.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/fides/api/service/connectors/saas_connector.py b/src/fides/api/service/connectors/saas_connector.py index e41f5e11222..38ddf8c5462 100644 --- a/src/fides/api/service/connectors/saas_connector.py +++ b/src/fides/api/service/connectors/saas_connector.py @@ -39,6 +39,7 @@ ConsentPropagationStatus, SaaSRequestParams, ) +from fides.api.models.privacy_request.request_task import AsyncTaskType from fides.api.service.async_dsr.strategies.async_dsr_strategy import AsyncDSRStrategy from fides.api.service.async_dsr.strategies.async_dsr_strategy_factory import ( get_strategy, @@ -280,15 +281,18 @@ def retrieve_data( if async_dsr_strategy := _get_async_dsr_strategy( db, request_task, query_config, ActionType.access ): - if self.guard_access_request(policy): + # Guard clause only applies to polling requests + # Callback requests should always proceed + if async_dsr_strategy.type == AsyncTaskType.polling: + if not self.guard_access_request(policy): + logger.info(f"Skipping async access request for policy: {policy.name}") + return [] return async_dsr_strategy.async_retrieve_data( client=self.create_client(), request_task_id=request_task.id, query_config=query_config, input_data=input_data, ) - logger.info(f"Skipping async access request for policy: {policy.name}") - return [] rows: List[Row] = [] for read_request in read_requests: From 9f2d549ee29484face92522fb023559bb1da5fc4 Mon Sep 17 00:00:00 2001 From: Vagoasdf Date: Tue, 16 Dec 2025 16:28:09 -0300 Subject: [PATCH 08/15] Running static checks --- src/fides/api/service/connectors/saas_connector.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/fides/api/service/connectors/saas_connector.py b/src/fides/api/service/connectors/saas_connector.py index 38ddf8c5462..ca84b766cd6 100644 --- a/src/fides/api/service/connectors/saas_connector.py +++ b/src/fides/api/service/connectors/saas_connector.py @@ -22,6 +22,7 @@ from fides.api.models.connectionconfig import ConnectionConfig, ConnectionTestStatus from fides.api.models.policy import Policy from fides.api.models.privacy_request import PrivacyRequest, RequestTask +from fides.api.models.privacy_request.request_task import AsyncTaskType from fides.api.schemas.consentable_item import ( ConsentableItem, build_consent_item_hierarchy, @@ -39,7 +40,6 @@ ConsentPropagationStatus, SaaSRequestParams, ) -from fides.api.models.privacy_request.request_task import AsyncTaskType from fides.api.service.async_dsr.strategies.async_dsr_strategy import AsyncDSRStrategy from fides.api.service.async_dsr.strategies.async_dsr_strategy_factory import ( get_strategy, @@ -285,7 +285,9 @@ def retrieve_data( # Callback requests should always proceed if async_dsr_strategy.type == AsyncTaskType.polling: if not self.guard_access_request(policy): - logger.info(f"Skipping async access request for policy: {policy.name}") + logger.info( + f"Skipping async access request for policy: {policy.name}" + ) return [] return async_dsr_strategy.async_retrieve_data( client=self.create_client(), From 44275907457bc9ffd8e5eb3043e2e261a7349afa Mon Sep 17 00:00:00 2001 From: Vagoasdf Date: Wed, 17 Dec 2025 09:53:07 -0300 Subject: [PATCH 09/15] setting up async poplling test config and dataset --- .../test_data/saas_async_polling_config.yml | 66 +++++++++++++++++++ .../test_data/saas_async_polling_dataset.yml | 15 +++++ 2 files changed, 81 insertions(+) create mode 100644 tests/fixtures/saas/test_data/saas_async_polling_config.yml create mode 100644 tests/fixtures/saas/test_data/saas_async_polling_dataset.yml diff --git a/tests/fixtures/saas/test_data/saas_async_polling_config.yml b/tests/fixtures/saas/test_data/saas_async_polling_config.yml new file mode 100644 index 00000000000..4c26bb4d47d --- /dev/null +++ b/tests/fixtures/saas/test_data/saas_async_polling_config.yml @@ -0,0 +1,66 @@ +saas_config: + fides_key: saas_async_polling_config + name: Async Polling Example Custom Connector + type: async_polling_example + description: Test Async Polling Config + version: 0.0.1 + + connector_params: + - name: domain + - name: api_token + label: API token + + client_config: + protocol: http + host: + authentication: + strategy: bearer + configuration: + token: + + test_request: + method: GET + path: / + + endpoints: + - name: user + requests: + read: + method: GET + path: /api/v1/user + query_params: + - name: query + value: + param_values: + - name: email + identity: email + correlation_id_path: request_id + async_config: + strategy: polling + configuration: + status_request: + method: GET + path: /api/v1/user/status + status_path: status + status_completed_value: completed + result_request: + method: GET + path: /api/v1/user/result + update: + method: DELETE + path: /api/v1/user/ + correlation_id_path: correlation_id + async_config: + strategy: polling + configuration: + status_request: + method: GET + path: /api/v1/user//status + status_path: status + status_completed_value: completed + param_values: + - name: user_id + references: + - dataset: saas_async_polling_config + field: user.id + direction: from diff --git a/tests/fixtures/saas/test_data/saas_async_polling_dataset.yml b/tests/fixtures/saas/test_data/saas_async_polling_dataset.yml new file mode 100644 index 00000000000..f83d391a92b --- /dev/null +++ b/tests/fixtures/saas/test_data/saas_async_polling_dataset.yml @@ -0,0 +1,15 @@ +dataset: + - fides_key: saas_async_polling_config + name: async_polling_example + description: A sample dataset for async polling + collections: + - name: user + fields: + - name: id + data_categories: [user.unique_id] + fidesops_meta: + primary_key: True + - name: system_id + data_categories: [system] + - name: state + data_categories: [user.contact.address.state] From fa10183319003f251cc14313a98e230632f4f85d Mon Sep 17 00:00:00 2001 From: Vagoasdf Date: Wed, 17 Dec 2025 10:20:32 -0300 Subject: [PATCH 10/15] Fixing Guard and tests calls --- .../api/service/connectors/saas_connector.py | 24 ++++++------ .../service/connectors/test_saas_connector.py | 38 +++++++++++++------ 2 files changed, 40 insertions(+), 22 deletions(-) diff --git a/src/fides/api/service/connectors/saas_connector.py b/src/fides/api/service/connectors/saas_connector.py index ca84b766cd6..2e425654c60 100644 --- a/src/fides/api/service/connectors/saas_connector.py +++ b/src/fides/api/service/connectors/saas_connector.py @@ -283,18 +283,17 @@ def retrieve_data( ): # Guard clause only applies to polling requests # Callback requests should always proceed - if async_dsr_strategy.type == AsyncTaskType.polling: - if not self.guard_access_request(policy): - logger.info( - f"Skipping async access request for policy: {policy.name}" - ) - return [] - return async_dsr_strategy.async_retrieve_data( - client=self.create_client(), - request_task_id=request_task.id, - query_config=query_config, - input_data=input_data, + if ( async_dsr_strategy.type == AsyncTaskType.polling ) and ( not self.guard_access_request(policy) ): + logger.info( + f"Skipping async access request for policy: {policy.name}" ) + return [] + return async_dsr_strategy.async_retrieve_data( + client=self.create_client(), + request_task_id=request_task.id, + query_config=query_config, + input_data=input_data, + ) rows: List[Row] = [] for read_request in read_requests: @@ -362,6 +361,9 @@ def guard_access_request(self, policy: Policy) -> bool: Guard clause to ensure we only run async access requests if the access request is enabled and we are in an Access Request """ + logger.info(f"Guard clause to ensure we only run async access requests if the access request is enabled and we are in an Access Request") + logger.info(f"Enabled actions: {self.configuration.enabled_actions}") + logger.info(f"Policy rules for access: {policy.get_rules_for_action(ActionType.access)}") if ( self.configuration.enabled_actions is None or ActionType.access in self.configuration.enabled_actions diff --git a/tests/ops/service/connectors/test_saas_connector.py b/tests/ops/service/connectors/test_saas_connector.py index 3a5b036bcb8..7a62d42e42a 100644 --- a/tests/ops/service/connectors/test_saas_connector.py +++ b/tests/ops/service/connectors/test_saas_connector.py @@ -12,6 +12,7 @@ from starlette.status import HTTP_200_OK, HTTP_204_NO_CONTENT, HTTP_404_NOT_FOUND from fides.api.common_exceptions import ( + AwaitingAsyncProcessing, AwaitingAsyncTask, ClientUnsuccessfulException, ConnectionException, @@ -1121,6 +1122,21 @@ def async_graph(self, saas_example_async_dataset_config, db, privacy_request): db, privacy_request, traversal_nodes, end_nodes, graph ) + @pytest.fixture(scope="function") + def async_graph_polling(self, saas_async_polling_example_dataset_config, db, privacy_request): + # Build proper async graph with persisted request tasks for polling tests + async_graph = saas_async_polling_example_dataset_config.get_graph() + graph = DatasetGraph(async_graph) + traversal = Traversal(graph, {"email": "customer-1@example.com"}) + traversal_nodes = {} + end_nodes = traversal.traverse(traversal_nodes, collect_tasks_fn) + persist_new_access_request_tasks( + db, privacy_request, traversal, traversal_nodes, end_nodes, graph + ) + persist_initial_erasure_request_tasks( + db, privacy_request, traversal_nodes, end_nodes, graph + ) + @mock.patch("fides.api.service.connectors.saas_connector.AuthenticatedClient.send") def test_read_request_expects_async_results( self, @@ -1307,20 +1323,20 @@ def test_callback_succeeded_mask_data( == 5 ) - @mock.patch("fides.api.service.connectors.saas_connector.AuthenticatedClient.send") + @mock.patch("fides.api.service.connectors.saas_connector.SaaSConnector.create_client") def test_guard_access_request_with_access_policy( self, - mock_send, + mock_create_client, privacy_request, - saas_async_example_connection_config, - async_graph, + saas_async_polling_example_connection_config, + async_graph_polling, ): """ Test that guard_access_request allows async access requests to run when the policy has access rules (access request scenario). """ - connector: SaaSConnector = get_connector(saas_async_example_connection_config) - mock_send().json.return_value = {"results_pending": True} + connector: SaaSConnector = get_connector(saas_async_polling_example_connection_config) + mock_create_client.return_value = mock.MagicMock() # Get access request task request_task = privacy_request.access_tasks.filter( @@ -1329,8 +1345,7 @@ def test_guard_access_request_with_access_policy( execution_node = ExecutionNode(request_task) # Policy has access rules, so guard should return True and async_retrieve_data should be called - # This will raise AwaitingAsyncTask - with pytest.raises(AwaitingAsyncTask): + with pytest.raises(AwaitingAsyncProcessing): connector.retrieve_data( execution_node, privacy_request.policy, @@ -1347,14 +1362,15 @@ def test_guard_access_request_with_erasure_only_policy( mock_create_client, db, privacy_request, - saas_async_example_connection_config, - async_graph, + saas_async_polling_example_connection_config, + async_graph_polling, oauth_client, ): """ Test that guard_access_request skips async access requests when the policy has no access rules (erasure-only request scenario). This test ensures coverage of the logger.info and return [] lines. + Uses polling async strategy to test the guard clause. """ # Create an erasure-only policy (no access rules) erasure_only_policy = Policy.create( @@ -1390,7 +1406,7 @@ def test_guard_access_request_with_erasure_only_policy( }, ) - connector: SaaSConnector = get_connector(saas_async_example_connection_config) + connector: SaaSConnector = get_connector(saas_async_polling_example_connection_config) # Get access request task request_task = privacy_request.access_tasks.filter( From b2495a27f8e643be571e6ecd95472cab9010f239 Mon Sep 17 00:00:00 2001 From: Vagoasdf Date: Wed, 17 Dec 2025 10:23:23 -0300 Subject: [PATCH 11/15] Running static checks, removing logs --- .../api/service/connectors/saas_connector.py | 7 +++---- .../service/connectors/test_saas_connector.py | 16 ++++++++++++---- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/fides/api/service/connectors/saas_connector.py b/src/fides/api/service/connectors/saas_connector.py index 2e425654c60..02040e6532c 100644 --- a/src/fides/api/service/connectors/saas_connector.py +++ b/src/fides/api/service/connectors/saas_connector.py @@ -283,7 +283,9 @@ def retrieve_data( ): # Guard clause only applies to polling requests # Callback requests should always proceed - if ( async_dsr_strategy.type == AsyncTaskType.polling ) and ( not self.guard_access_request(policy) ): + if (async_dsr_strategy.type == AsyncTaskType.polling) and ( + not self.guard_access_request(policy) + ): logger.info( f"Skipping async access request for policy: {policy.name}" ) @@ -361,9 +363,6 @@ def guard_access_request(self, policy: Policy) -> bool: Guard clause to ensure we only run async access requests if the access request is enabled and we are in an Access Request """ - logger.info(f"Guard clause to ensure we only run async access requests if the access request is enabled and we are in an Access Request") - logger.info(f"Enabled actions: {self.configuration.enabled_actions}") - logger.info(f"Policy rules for access: {policy.get_rules_for_action(ActionType.access)}") if ( self.configuration.enabled_actions is None or ActionType.access in self.configuration.enabled_actions diff --git a/tests/ops/service/connectors/test_saas_connector.py b/tests/ops/service/connectors/test_saas_connector.py index 7a62d42e42a..723e37f90b7 100644 --- a/tests/ops/service/connectors/test_saas_connector.py +++ b/tests/ops/service/connectors/test_saas_connector.py @@ -1123,7 +1123,9 @@ def async_graph(self, saas_example_async_dataset_config, db, privacy_request): ) @pytest.fixture(scope="function") - def async_graph_polling(self, saas_async_polling_example_dataset_config, db, privacy_request): + def async_graph_polling( + self, saas_async_polling_example_dataset_config, db, privacy_request + ): # Build proper async graph with persisted request tasks for polling tests async_graph = saas_async_polling_example_dataset_config.get_graph() graph = DatasetGraph(async_graph) @@ -1323,7 +1325,9 @@ def test_callback_succeeded_mask_data( == 5 ) - @mock.patch("fides.api.service.connectors.saas_connector.SaaSConnector.create_client") + @mock.patch( + "fides.api.service.connectors.saas_connector.SaaSConnector.create_client" + ) def test_guard_access_request_with_access_policy( self, mock_create_client, @@ -1335,7 +1339,9 @@ def test_guard_access_request_with_access_policy( Test that guard_access_request allows async access requests to run when the policy has access rules (access request scenario). """ - connector: SaaSConnector = get_connector(saas_async_polling_example_connection_config) + connector: SaaSConnector = get_connector( + saas_async_polling_example_connection_config + ) mock_create_client.return_value = mock.MagicMock() # Get access request task @@ -1406,7 +1412,9 @@ def test_guard_access_request_with_erasure_only_policy( }, ) - connector: SaaSConnector = get_connector(saas_async_polling_example_connection_config) + connector: SaaSConnector = get_connector( + saas_async_polling_example_connection_config + ) # Get access request task request_task = privacy_request.access_tasks.filter( From ba621de4417caf368f369b2d035f3dafac738e54 Mon Sep 17 00:00:00 2001 From: Vagoasdf Date: Wed, 17 Dec 2025 10:31:10 -0300 Subject: [PATCH 12/15] Adding tests for callback --- .../service/connectors/test_saas_connector.py | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/tests/ops/service/connectors/test_saas_connector.py b/tests/ops/service/connectors/test_saas_connector.py index 723e37f90b7..18bd9316567 100644 --- a/tests/ops/service/connectors/test_saas_connector.py +++ b/tests/ops/service/connectors/test_saas_connector.py @@ -1442,3 +1442,85 @@ def test_guard_access_request_with_erasure_only_policy( erasure_only_policy.delete(db) except Exception: pass + + @mock.patch( + "fides.api.service.connectors.saas_connector.SaaSConnector.create_client" + ) + def test_callback_requests_ignore_guard_clause( + self, + mock_create_client, + db, + privacy_request, + saas_async_example_connection_config, + async_graph, + oauth_client, + ): + """ + Test that callback requests ignore the guard clause entirely. + Even if guard_access_request returns False (erasure-only policy), + callback requests should still proceed and raise AwaitingAsyncTask. + """ + # Create an erasure-only policy (no access rules) + erasure_only_policy = Policy.create( + db=db, + data={ + "name": "Erasure Only Policy Callback Test", + "key": "erasure_only_policy_callback_test", + "client_id": oauth_client.id, + }, + ) + + erasure_rule = Rule.create( + db=db, + data={ + "action_type": ActionType.erasure, + "name": "Erasure Rule Callback Test", + "key": "erasure_rule_callback_test", + "policy_id": erasure_only_policy.id, + "masking_strategy": { + "strategy": "null_rewrite", + "configuration": {}, + }, + "client_id": oauth_client.id, + }, + ) + + RuleTarget.create( + db=db, + data={ + "data_category": "user.name", + "rule_id": erasure_rule.id, + "client_id": oauth_client.id, + }, + ) + + connector: SaaSConnector = get_connector(saas_async_example_connection_config) + mock_create_client.return_value = mock.MagicMock() + + # Get access request task + request_task = privacy_request.access_tasks.filter( + RequestTask.collection_name == "user" + ).first() + execution_node = ExecutionNode(request_task) + + # Verify guard_access_request returns False for erasure-only policy + assert connector.guard_access_request(erasure_only_policy) is False + + # Even though guard_access_request returns False, callback requests + # should ignore the guard and still proceed with async_retrieve_data + # This will raise AwaitingAsyncTask (not return empty list like polling would) + with pytest.raises(AwaitingAsyncTask): + connector.retrieve_data( + execution_node, + erasure_only_policy, + privacy_request, + request_task, + {}, + ) + + # Cleanup + try: + erasure_rule.delete(db) + erasure_only_policy.delete(db) + except Exception: + pass From 19b3d59939de5897e7c4325d98400a6a8ed82b06 Mon Sep 17 00:00:00 2001 From: Vagoasdf Date: Wed, 17 Dec 2025 15:47:33 -0300 Subject: [PATCH 13/15] Updated guard check and test to support currently tested callback path While im not 100% confident of the current callback testing path management of the access request, it is out of the scope of the current pr --- .../api/service/connectors/saas_connector.py | 16 ++++++----- .../service/connectors/test_saas_connector.py | 28 +++++++++++-------- 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/src/fides/api/service/connectors/saas_connector.py b/src/fides/api/service/connectors/saas_connector.py index 02040e6532c..7eb247d3802 100644 --- a/src/fides/api/service/connectors/saas_connector.py +++ b/src/fides/api/service/connectors/saas_connector.py @@ -281,21 +281,23 @@ def retrieve_data( if async_dsr_strategy := _get_async_dsr_strategy( db, request_task, query_config, ActionType.access ): + check_guard_access_request = self.guard_access_request(policy) # Guard clause only applies to polling requests # Callback requests should always proceed if (async_dsr_strategy.type == AsyncTaskType.polling) and ( - not self.guard_access_request(policy) + not check_guard_access_request ): logger.info( f"Skipping async access request for policy: {policy.name}" ) return [] - return async_dsr_strategy.async_retrieve_data( - client=self.create_client(), - request_task_id=request_task.id, - query_config=query_config, - input_data=input_data, - ) + if check_guard_access_request: + return async_dsr_strategy.async_retrieve_data( + client=self.create_client(), + request_task_id=request_task.id, + query_config=query_config, + input_data=input_data, + ) rows: List[Row] = [] for read_request in read_requests: diff --git a/tests/ops/service/connectors/test_saas_connector.py b/tests/ops/service/connectors/test_saas_connector.py index 18bd9316567..43dcca4e5c2 100644 --- a/tests/ops/service/connectors/test_saas_connector.py +++ b/tests/ops/service/connectors/test_saas_connector.py @@ -1495,7 +1495,12 @@ def test_callback_requests_ignore_guard_clause( ) connector: SaaSConnector = get_connector(saas_async_example_connection_config) - mock_create_client.return_value = mock.MagicMock() + # Mock the client and its send method to allow async callback flow + mock_client = mock.MagicMock() + mock_send_response = mock.MagicMock() + mock_send_response.json.return_value = {"id": "123"} + mock_client.send.return_value = mock_send_response + mock_create_client.return_value = mock_client # Get access request task request_task = privacy_request.access_tasks.filter( @@ -1507,16 +1512,17 @@ def test_callback_requests_ignore_guard_clause( assert connector.guard_access_request(erasure_only_policy) is False # Even though guard_access_request returns False, callback requests - # should ignore the guard and still proceed with async_retrieve_data - # This will raise AwaitingAsyncTask (not return empty list like polling would) - with pytest.raises(AwaitingAsyncTask): - connector.retrieve_data( - execution_node, - erasure_only_policy, - privacy_request, - request_task, - {}, - ) + # should ignore the guard and always proceed with common requests. + connector.retrieve_data( + execution_node, + erasure_only_policy, + privacy_request, + request_task, + {}, + ) + + # Verify that the async callback flow was triggered (client.send was called) + assert mock_client.send.called # Cleanup try: From 77f8f734ecadfc36a8ea87376442a977dba5a75d Mon Sep 17 00:00:00 2001 From: Vagoasdf Date: Thu, 18 Dec 2025 10:32:46 -0300 Subject: [PATCH 14/15] Cleaning up erasures --- tests/ops/service/connectors/test_saas_connector.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/tests/ops/service/connectors/test_saas_connector.py b/tests/ops/service/connectors/test_saas_connector.py index 43dcca4e5c2..62b5bd30d3d 100644 --- a/tests/ops/service/connectors/test_saas_connector.py +++ b/tests/ops/service/connectors/test_saas_connector.py @@ -1436,12 +1436,6 @@ def test_guard_access_request_with_erasure_only_policy( # Should return empty list without calling async_retrieve_data assert result == [] - # Cleanup - try: - erasure_rule.delete(db) - erasure_only_policy.delete(db) - except Exception: - pass @mock.patch( "fides.api.service.connectors.saas_connector.SaaSConnector.create_client" @@ -1523,10 +1517,3 @@ def test_callback_requests_ignore_guard_clause( # Verify that the async callback flow was triggered (client.send was called) assert mock_client.send.called - - # Cleanup - try: - erasure_rule.delete(db) - erasure_only_policy.delete(db) - except Exception: - pass From eeff6b9c405d8232f21f44c2b4758090849d7a59 Mon Sep 17 00:00:00 2001 From: Vagoasdf Date: Thu, 18 Dec 2025 10:33:28 -0300 Subject: [PATCH 15/15] Static checks, removing whitespaces --- tests/ops/service/connectors/test_saas_connector.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/ops/service/connectors/test_saas_connector.py b/tests/ops/service/connectors/test_saas_connector.py index 62b5bd30d3d..eaf70ab794a 100644 --- a/tests/ops/service/connectors/test_saas_connector.py +++ b/tests/ops/service/connectors/test_saas_connector.py @@ -1436,7 +1436,6 @@ def test_guard_access_request_with_erasure_only_policy( # Should return empty list without calling async_retrieve_data assert result == [] - @mock.patch( "fides.api.service.connectors.saas_connector.SaaSConnector.create_client" )