Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating masking strict check to use config proxy #5512

Merged
merged 3 commits into from
Nov 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading