Skip to content

Commit 79f8f8e

Browse files
Vagoasdfgalvana
andcommitted
Fixing up guard access requests (#7130)
Co-authored-by: Adrian Galvan <adrian@ethyca.com>
1 parent 1868bf7 commit 79f8f8e

File tree

4 files changed

+306
-5
lines changed

4 files changed

+306
-5
lines changed

src/fides/api/service/connectors/saas_connector.py

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
from fides.api.models.connectionconfig import ConnectionConfig, ConnectionTestStatus
2323
from fides.api.models.policy import Policy
2424
from fides.api.models.privacy_request import PrivacyRequest, RequestTask
25+
from fides.api.models.privacy_request.request_task import AsyncTaskType
2526
from fides.api.schemas.consentable_item import (
2627
ConsentableItem,
2728
build_consent_item_hierarchy,
@@ -277,11 +278,20 @@ def retrieve_data(
277278

278279
# Delegate async requests
279280
with get_db() as db:
280-
# Guard clause to ensure we only run async access requests for access requests
281-
if self.guard_access_request(policy):
282-
if async_dsr_strategy := _get_async_dsr_strategy(
283-
db, request_task, query_config, ActionType.access
281+
if async_dsr_strategy := _get_async_dsr_strategy(
282+
db, request_task, query_config, ActionType.access
283+
):
284+
check_guard_access_request = self.guard_access_request(policy)
285+
# Guard clause only applies to polling requests
286+
# Callback requests should always proceed
287+
if (async_dsr_strategy.type == AsyncTaskType.polling) and (
288+
not check_guard_access_request
284289
):
290+
logger.info(
291+
f"Skipping async access request for policy: {policy.name}"
292+
)
293+
return []
294+
if check_guard_access_request:
285295
return async_dsr_strategy.async_retrieve_data(
286296
client=self.create_client(),
287297
request_task_id=request_task.id,
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
saas_config:
2+
fides_key: saas_async_polling_config
3+
name: Async Polling Example Custom Connector
4+
type: async_polling_example
5+
description: Test Async Polling Config
6+
version: 0.0.1
7+
8+
connector_params:
9+
- name: domain
10+
- name: api_token
11+
label: API token
12+
13+
client_config:
14+
protocol: http
15+
host: <domain>
16+
authentication:
17+
strategy: bearer
18+
configuration:
19+
token: <api_token>
20+
21+
test_request:
22+
method: GET
23+
path: /
24+
25+
endpoints:
26+
- name: user
27+
requests:
28+
read:
29+
method: GET
30+
path: /api/v1/user
31+
query_params:
32+
- name: query
33+
value: <email>
34+
param_values:
35+
- name: email
36+
identity: email
37+
correlation_id_path: request_id
38+
async_config:
39+
strategy: polling
40+
configuration:
41+
status_request:
42+
method: GET
43+
path: /api/v1/user/status
44+
status_path: status
45+
status_completed_value: completed
46+
result_request:
47+
method: GET
48+
path: /api/v1/user/result
49+
update:
50+
method: DELETE
51+
path: /api/v1/user/<user_id>
52+
correlation_id_path: correlation_id
53+
async_config:
54+
strategy: polling
55+
configuration:
56+
status_request:
57+
method: GET
58+
path: /api/v1/user/<correlation_id>/status
59+
status_path: status
60+
status_completed_value: completed
61+
param_values:
62+
- name: user_id
63+
references:
64+
- dataset: saas_async_polling_config
65+
field: user.id
66+
direction: from
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
dataset:
2+
- fides_key: saas_async_polling_config
3+
name: async_polling_example
4+
description: A sample dataset for async polling
5+
collections:
6+
- name: user
7+
fields:
8+
- name: id
9+
data_categories: [user.unique_id]
10+
fidesops_meta:
11+
primary_key: True
12+
- name: system_id
13+
data_categories: [system]
14+
- name: state
15+
data_categories: [user.contact.address.state]

tests/ops/service/connectors/test_saas_connector.py

Lines changed: 211 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
from starlette.status import HTTP_200_OK, HTTP_204_NO_CONTENT, HTTP_404_NOT_FOUND
1313

1414
from fides.api.common_exceptions import (
15+
AwaitingAsyncProcessing,
1516
AwaitingAsyncTask,
1617
ClientUnsuccessfulException,
1718
ConnectionException,
@@ -22,7 +23,7 @@
2223
from fides.api.graph.graph import DatasetGraph, Node
2324
from fides.api.graph.traversal import Traversal, TraversalNode
2425
from fides.api.models.consent_automation import ConsentAutomation
25-
from fides.api.models.policy import Policy
26+
from fides.api.models.policy import ActionType, Policy, Rule, RuleTarget
2627
from fides.api.models.privacy_notice import UserConsentPreference
2728
from fides.api.models.privacy_request import PrivacyRequest, RequestTask
2829
from fides.api.models.worker_task import ExecutionLogStatus
@@ -1121,6 +1122,23 @@ def async_graph(self, saas_example_async_dataset_config, db, privacy_request):
11211122
db, privacy_request, traversal_nodes, end_nodes, graph
11221123
)
11231124

1125+
@pytest.fixture(scope="function")
1126+
def async_graph_polling(
1127+
self, saas_async_polling_example_dataset_config, db, privacy_request
1128+
):
1129+
# Build proper async graph with persisted request tasks for polling tests
1130+
async_graph = saas_async_polling_example_dataset_config.get_graph()
1131+
graph = DatasetGraph(async_graph)
1132+
traversal = Traversal(graph, {"email": "customer-1@example.com"})
1133+
traversal_nodes = {}
1134+
end_nodes = traversal.traverse(traversal_nodes, collect_tasks_fn)
1135+
persist_new_access_request_tasks(
1136+
db, privacy_request, traversal, traversal_nodes, end_nodes, graph
1137+
)
1138+
persist_initial_erasure_request_tasks(
1139+
db, privacy_request, traversal_nodes, end_nodes, graph
1140+
)
1141+
11241142
@mock.patch("fides.api.service.connectors.saas_connector.AuthenticatedClient.send")
11251143
def test_read_request_expects_async_results(
11261144
self,
@@ -1306,3 +1324,195 @@ def test_callback_succeeded_mask_data(
13061324
)
13071325
== 5
13081326
)
1327+
1328+
@mock.patch(
1329+
"fides.api.service.connectors.saas_connector.SaaSConnector.create_client"
1330+
)
1331+
def test_guard_access_request_with_access_policy(
1332+
self,
1333+
mock_create_client,
1334+
privacy_request,
1335+
saas_async_polling_example_connection_config,
1336+
async_graph_polling,
1337+
):
1338+
"""
1339+
Test that guard_access_request allows async access requests to run
1340+
when the policy has access rules (access request scenario).
1341+
"""
1342+
connector: SaaSConnector = get_connector(
1343+
saas_async_polling_example_connection_config
1344+
)
1345+
mock_create_client.return_value = mock.MagicMock()
1346+
1347+
# Get access request task
1348+
request_task = privacy_request.access_tasks.filter(
1349+
RequestTask.collection_name == "user"
1350+
).first()
1351+
execution_node = ExecutionNode(request_task)
1352+
1353+
# Policy has access rules, so guard should return True and async_retrieve_data should be called
1354+
with pytest.raises(AwaitingAsyncProcessing):
1355+
connector.retrieve_data(
1356+
execution_node,
1357+
privacy_request.policy,
1358+
privacy_request,
1359+
request_task,
1360+
{},
1361+
)
1362+
1363+
@mock.patch(
1364+
"fides.api.service.connectors.saas_connector.SaaSConnector.create_client"
1365+
)
1366+
def test_guard_access_request_with_erasure_only_policy(
1367+
self,
1368+
mock_create_client,
1369+
db,
1370+
privacy_request,
1371+
saas_async_polling_example_connection_config,
1372+
async_graph_polling,
1373+
oauth_client,
1374+
):
1375+
"""
1376+
Test that guard_access_request skips async access requests
1377+
when the policy has no access rules (erasure-only request scenario).
1378+
This test ensures coverage of the logger.info and return [] lines.
1379+
Uses polling async strategy to test the guard clause.
1380+
"""
1381+
# Create an erasure-only policy (no access rules)
1382+
erasure_only_policy = Policy.create(
1383+
db=db,
1384+
data={
1385+
"name": "Erasure Only Policy",
1386+
"key": "erasure_only_policy_test",
1387+
"client_id": oauth_client.id,
1388+
},
1389+
)
1390+
1391+
erasure_rule = Rule.create(
1392+
db=db,
1393+
data={
1394+
"action_type": ActionType.erasure,
1395+
"name": "Erasure Rule",
1396+
"key": "erasure_rule_test",
1397+
"policy_id": erasure_only_policy.id,
1398+
"masking_strategy": {
1399+
"strategy": "null_rewrite",
1400+
"configuration": {},
1401+
},
1402+
"client_id": oauth_client.id,
1403+
},
1404+
)
1405+
1406+
RuleTarget.create(
1407+
db=db,
1408+
data={
1409+
"data_category": "user.name",
1410+
"rule_id": erasure_rule.id,
1411+
"client_id": oauth_client.id,
1412+
},
1413+
)
1414+
1415+
connector: SaaSConnector = get_connector(
1416+
saas_async_polling_example_connection_config
1417+
)
1418+
1419+
# Get access request task
1420+
request_task = privacy_request.access_tasks.filter(
1421+
RequestTask.collection_name == "user"
1422+
).first()
1423+
execution_node = ExecutionNode(request_task)
1424+
1425+
# Verify guard_access_request returns False for erasure-only policy
1426+
assert connector.guard_access_request(erasure_only_policy) is False
1427+
1428+
result = connector.retrieve_data(
1429+
execution_node,
1430+
erasure_only_policy,
1431+
privacy_request,
1432+
request_task,
1433+
{},
1434+
)
1435+
1436+
# Should return empty list without calling async_retrieve_data
1437+
assert result == []
1438+
1439+
@mock.patch(
1440+
"fides.api.service.connectors.saas_connector.SaaSConnector.create_client"
1441+
)
1442+
def test_callback_requests_ignore_guard_clause(
1443+
self,
1444+
mock_create_client,
1445+
db,
1446+
privacy_request,
1447+
saas_async_example_connection_config,
1448+
async_graph,
1449+
oauth_client,
1450+
):
1451+
"""
1452+
Test that callback requests ignore the guard clause entirely.
1453+
Even if guard_access_request returns False (erasure-only policy),
1454+
callback requests should still proceed and raise AwaitingAsyncTask.
1455+
"""
1456+
# Create an erasure-only policy (no access rules)
1457+
erasure_only_policy = Policy.create(
1458+
db=db,
1459+
data={
1460+
"name": "Erasure Only Policy Callback Test",
1461+
"key": "erasure_only_policy_callback_test",
1462+
"client_id": oauth_client.id,
1463+
},
1464+
)
1465+
1466+
erasure_rule = Rule.create(
1467+
db=db,
1468+
data={
1469+
"action_type": ActionType.erasure,
1470+
"name": "Erasure Rule Callback Test",
1471+
"key": "erasure_rule_callback_test",
1472+
"policy_id": erasure_only_policy.id,
1473+
"masking_strategy": {
1474+
"strategy": "null_rewrite",
1475+
"configuration": {},
1476+
},
1477+
"client_id": oauth_client.id,
1478+
},
1479+
)
1480+
1481+
RuleTarget.create(
1482+
db=db,
1483+
data={
1484+
"data_category": "user.name",
1485+
"rule_id": erasure_rule.id,
1486+
"client_id": oauth_client.id,
1487+
},
1488+
)
1489+
1490+
connector: SaaSConnector = get_connector(saas_async_example_connection_config)
1491+
# Mock the client and its send method to allow async callback flow
1492+
mock_client = mock.MagicMock()
1493+
mock_send_response = mock.MagicMock()
1494+
mock_send_response.json.return_value = {"id": "123"}
1495+
mock_client.send.return_value = mock_send_response
1496+
mock_create_client.return_value = mock_client
1497+
1498+
# Get access request task
1499+
request_task = privacy_request.access_tasks.filter(
1500+
RequestTask.collection_name == "user"
1501+
).first()
1502+
execution_node = ExecutionNode(request_task)
1503+
1504+
# Verify guard_access_request returns False for erasure-only policy
1505+
assert connector.guard_access_request(erasure_only_policy) is False
1506+
1507+
# Even though guard_access_request returns False, callback requests
1508+
# should ignore the guard and always proceed with common requests.
1509+
connector.retrieve_data(
1510+
execution_node,
1511+
erasure_only_policy,
1512+
privacy_request,
1513+
request_task,
1514+
{},
1515+
)
1516+
1517+
# Verify that the async callback flow was triggered (client.send was called)
1518+
assert mock_client.send.called

0 commit comments

Comments
 (0)