Skip to content

Commit

Permalink
Updating masking strict check to use config proxy (#5512)
Browse files Browse the repository at this point in the history
  • Loading branch information
galvana authored Nov 19, 2024
1 parent c7e645f commit 2321052
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 21 deletions.
4 changes: 3 additions & 1 deletion src/fides/api/service/connectors/saas_connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,9 @@ def mask_data(

self.set_privacy_request_state(privacy_request, node, request_task)
query_config = self.query_config(node)
masking_request = query_config.get_masking_request()

session = Session.object_session(privacy_request)
masking_request = query_config.get_masking_request(session)
if not masking_request:
raise Exception(
f"Either no masking request configured or no valid masking request for {node.address.collection}. "
Expand Down
10 changes: 6 additions & 4 deletions src/fides/api/service/connectors/saas_query_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import pydash
from fideslang.models import FidesDatasetReference
from loguru import logger
from sqlalchemy.orm import Session

from fides.api.common_exceptions import FidesopsException
from fides.api.graph.config import ScalarField
Expand Down Expand Up @@ -43,7 +44,7 @@
unflatten_dict,
)
from fides.common.api.v1.urn_registry import REQUEST_TASK_CALLBACK, V1_URL_PREFIX
from fides.config import CONFIG
from fides.config.config_proxy import ConfigProxy

T = TypeVar("T")

Expand Down Expand Up @@ -131,7 +132,7 @@ def get_erasure_request_by_action(
)
return request

def get_masking_request(self) -> Optional[SaaSRequest]:
def get_masking_request(self, db: Session) -> Optional[SaaSRequest]:
"""
Returns a tuple of the preferred action and SaaSRequest to use for masking.
An update request is preferred, but we can use a gdpr delete endpoint or
Expand All @@ -142,7 +143,7 @@ def get_masking_request(self) -> Optional[SaaSRequest]:
gdpr_delete: Optional[SaaSRequest] = None
delete: Optional[SaaSRequest] = None

if not CONFIG.execution.masking_strict:
if not ConfigProxy(db).execution.masking_strict:
gdpr_delete = self.data_protection_request
delete = self.get_erasure_request_by_action("delete")

Expand Down Expand Up @@ -372,7 +373,8 @@ def generate_update_stmt(
The fields in the row are masked according to the policy and added to the request body
if specified by the body field of the masking request.
"""
current_request: SaaSRequest = self.get_masking_request() # type: ignore
session = Session.object_session(request)
current_request: SaaSRequest = self.get_masking_request(session) # type: ignore
param_values: Dict[str, Any] = self.generate_update_param_values(
row, policy, request, current_request
)
Expand Down
1 change: 1 addition & 0 deletions src/fides/config/config_proxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ class ExecutionSettingsProxy(ConfigProxyBase):
subject_identity_verification_required: bool
disable_consent_identity_verification: bool
require_manual_request_approval: bool
masking_strict: bool

def __getattribute__(self, name: str) -> Any:
"""
Expand Down
12 changes: 8 additions & 4 deletions tests/ops/service/connectors/test_saas_connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,11 @@ def test_missing_grouped_inputs_input_values(

@mock.patch("fides.api.service.connectors.saas_connector.AuthenticatedClient.send")
def test_skip_missing_param_values_masking(
self, mock_send: Mock, saas_example_config, saas_example_connection_config
self,
mock_send: Mock,
privacy_request,
saas_example_config,
saas_example_connection_config,
):
"""
Verifies skip_missing_param_values behavior for Connector.mask_data.
Expand Down Expand Up @@ -424,7 +428,7 @@ def test_skip_missing_param_values_masking(
connector.mask_data(
execution_node,
Policy(),
PrivacyRequest(id="123"),
privacy_request,
request_task,
[{"customer_id": 1}],
)
Expand All @@ -441,7 +445,7 @@ def test_skip_missing_param_values_masking(
connector.mask_data(
execution_node,
Policy(),
PrivacyRequest(id="123"),
privacy_request,
request_task,
[{"customer_id": 1}],
)
Expand All @@ -454,7 +458,7 @@ def test_skip_missing_param_values_masking(
connector.mask_data(
execution_node,
Policy(),
PrivacyRequest(id="123"),
privacy_request,
request_task,
[{"customer_id": 1}],
)
Expand Down
30 changes: 18 additions & 12 deletions tests/ops/service/connectors/test_saas_queryconfig.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from fides.api.graph.config import CollectionAddress
from fides.api.graph.graph import DatasetGraph
from fides.api.graph.traversal import Traversal
from fides.api.models.application_config import ApplicationConfig
from fides.api.models.connectionconfig import ConnectionConfig
from fides.api.models.datasetconfig import DatasetConfig
from fides.api.models.privacy_request import PrivacyRequest
Expand All @@ -22,8 +23,6 @@
from fides.config import CONFIG
from tests.ops.graph.graph_test_util import generate_node

privacy_request = PrivacyRequest(id="234544")


@pytest.mark.unit_saas
class TestSaaSQueryConfig:
Expand Down Expand Up @@ -189,6 +188,7 @@ def test_generate_requests(

def test_generate_update_stmt(
self,
privacy_request,
erasure_policy_string_rewrite,
combined_traversal,
saas_example_connection_config,
Expand Down Expand Up @@ -224,6 +224,7 @@ def test_generate_update_stmt(

def test_generate_update_stmt_custom_http_method(
self,
privacy_request,
erasure_policy_string_rewrite,
combined_traversal,
saas_example_connection_config,
Expand Down Expand Up @@ -262,6 +263,7 @@ def test_generate_update_stmt_custom_http_method(

def test_generate_update_stmt_with_request_body(
self,
privacy_request,
erasure_policy_string_rewrite,
combined_traversal,
saas_example_connection_config,
Expand Down Expand Up @@ -336,6 +338,7 @@ def test_generate_update_stmt_with_request_body(

def test_generate_update_stmt_with_url_encoded_body(
self,
privacy_request,
erasure_policy_string_rewrite,
combined_traversal,
saas_example_connection_config,
Expand Down Expand Up @@ -430,7 +433,7 @@ def test_get_read_requests_by_identity(
assert len(saas_requests) == 2

def test_get_masking_request(
self, combined_traversal, saas_example_connection_config
self, db, combined_traversal, saas_example_connection_config
):
saas_config: Optional[SaaSConfig] = (
saas_example_connection_config.get_saas_config()
Expand All @@ -448,19 +451,19 @@ def test_get_masking_request(
]

query_config = SaaSQueryConfig(member, endpoints, {})
saas_request = query_config.get_masking_request()
saas_request = query_config.get_masking_request(db)

# Assert we pulled the update method off of the member collection
assert saas_request.method == "PUT"
assert saas_request.path == "/3.0/lists/<list_id>/members/<subscriber_hash>"

# No update methods defined on other collections
query_config = SaaSQueryConfig(conversations, endpoints, {})
saas_request = query_config.get_masking_request()
saas_request = query_config.get_masking_request(db)
assert saas_request is None

query_config = SaaSQueryConfig(messages, endpoints, {})
saas_request = query_config.get_masking_request()
saas_request = query_config.get_masking_request(db)
assert saas_request is None

# Define delete request on conversations endpoint
Expand All @@ -471,15 +474,16 @@ def test_get_masking_request(
assert CONFIG.execution.masking_strict is True

query_config = SaaSQueryConfig(conversations, endpoints, {})
saas_request = query_config.get_masking_request()
saas_request = query_config.get_masking_request(db)
assert saas_request is None

# Override masking_strict to False
masking_strict = CONFIG.execution.masking_strict
original_value = CONFIG.execution.masking_strict
CONFIG.execution.masking_strict = False
ApplicationConfig.update_config_set(db, CONFIG)

# Now delete endpoint is selected as conversations masking request
saas_request: SaaSRequest = query_config.get_masking_request()
saas_request: SaaSRequest = query_config.get_masking_request(db)
assert saas_request.path == "/api/0/<conversation>/<conversation_id>/"
assert saas_request.method == "DELETE"

Expand All @@ -490,12 +494,13 @@ def test_get_masking_request(
)

# Assert GDPR Delete takes priority over Delete
saas_request: SaaSRequest = query_config.get_masking_request()
saas_request: SaaSRequest = query_config.get_masking_request(db)
assert saas_request.path == "/api/0/gdpr_delete"
assert saas_request.method == "PUT"

# Reset
CONFIG.execution.masking_strict = masking_strict
CONFIG.notifications.enable_property_specific_messaging = original_value
ApplicationConfig.update_config_set(db, CONFIG)
del endpoints["conversations"].requests.delete

def test_list_param_values(
Expand Down Expand Up @@ -651,6 +656,7 @@ def test_custom_privacy_request_fields(
self,
mock_identity_data: Mock,
mock_custom_privacy_request_fields: Mock,
privacy_request,
policy,
consent_policy,
erasure_policy_string_rewrite,
Expand All @@ -676,7 +682,7 @@ def test_custom_privacy_request_fields(
internal_information,
endpoints,
{},
privacy_request=PrivacyRequest(id="123"),
privacy_request,
)

read_request, param_value_map = config.generate_requests(
Expand Down

0 comments on commit 2321052

Please sign in to comment.