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

Adding support for special characters in SaaS request body #5099

Merged
Merged
1 change: 1 addition & 0 deletions dev-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,5 @@ types-toml==0.10.8
types-ujson==5.4.0
types-urllib3==1.26.23
watchfiles==0.19.0
werkzeug==3.0.3
galvana marked this conversation as resolved.
Show resolved Hide resolved
xenon==0.9.0
4 changes: 4 additions & 0 deletions src/fides/api/service/connectors/saas/authenticated_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,10 @@ def send(
# extract the hostname from the complete URL and verify its safety
deny_unsafe_hosts(urlparse(prepared_request.url).netloc)

# utf-8 encode the body before sending
if isinstance(prepared_request.body, str):
prepared_request.body = prepared_request.body.encode("utf-8")
galvana marked this conversation as resolved.
Show resolved Hide resolved

response = self.session.send(prepared_request)

ignore_error = self._should_ignore_error(
Expand Down
7 changes: 6 additions & 1 deletion src/fides/api/util/logger_context_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,12 @@ def request_details(
LoggerContextKeys.url.value: prepared_request.url,
}
if CONFIG.dev_mode and prepared_request.body is not None:
details[LoggerContextKeys.body.value] = prepared_request.body
if isinstance(prepared_request.body, bytes):
details[LoggerContextKeys.body.value] = prepared_request.body.decode(
"utf-8"
)
galvana marked this conversation as resolved.
Show resolved Hide resolved
elif isinstance(prepared_request.body, str):
details[LoggerContextKeys.body.value] = prepared_request.body

if response is not None:
if CONFIG.dev_mode and response.content:
Expand Down
5 changes: 3 additions & 2 deletions tests/ops/integration_tests/saas/test_onesignal_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from tests.ops.test_helpers.saas_test_utils import poll_for_existence


@pytest.mark.skip(reason="Temporarily disabled test")
andres-torres-marroquin marked this conversation as resolved.
Show resolved Hide resolved
@pytest.mark.integration_saas
class TestOneSignalConnector:
def test_connection(self, onesignal_runner: ConnectorRunner):
Expand All @@ -24,7 +25,7 @@ async def test_access_request(
):
request.getfixturevalue(dsr_version) # REQUIRED to test both DSR 3.0 and 2.0

access_results = await onesignal_runner.access_request(
await onesignal_runner.access_request(
access_policy=policy, identities={"email": onesignal_identity_email}
)

Expand All @@ -45,7 +46,7 @@ async def test_non_strict_erasure_request(

player_id = onesignal_erasure_data
(
access_results,
_,
andres-torres-marroquin marked this conversation as resolved.
Show resolved Hide resolved
erasure_results,
) = await onesignal_runner.non_strict_erasure_request(
access_policy=policy,
Expand Down
46 changes: 45 additions & 1 deletion tests/ops/service/connectors/saas/test_authenticated_client.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
import threading
import time
import unittest.mock as mock
from email.utils import formatdate
from typing import Any, Dict
from typing import Any, Dict, Generator

import pytest
from loguru import logger
from requests import ConnectionError, Response, Session
from werkzeug.serving import make_server
from werkzeug.wrappers import Response as WerkzeugResponse

from fides.api.common_exceptions import ClientUnsuccessfulException, ConnectionException
from fides.api.models.connectionconfig import ConnectionConfig, ConnectionType
Expand Down Expand Up @@ -59,6 +63,33 @@ def test_authenticated_client(
)


@pytest.fixture
def test_http_server() -> Generator[str, None, None]:
"""
Creates a simple HTTP server for testing purposes.

This fixture sets up a Werkzeug server running on localhost with a
dynamically assigned port. The server responds to all requests with
a "Request received" message.

The server is automatically shut down after the test is complete.
"""

def simple_app(environ, start_response):
logger.info("Request received")
response = WerkzeugResponse("Request received")
return response(environ, start_response)

server = make_server("localhost", 0, simple_app)
server_thread = threading.Thread(target=server.serve_forever)
server_thread.start()

yield f"http://{server.server_address[0]}:{server.server_address[1]}"

server.shutdown()
server_thread.join()


@pytest.mark.unit_saas
class TestAuthenticatedClient:
@mock.patch.object(Session, "send")
Expand Down Expand Up @@ -145,6 +176,19 @@ def test_client_ignores_errors(
errors_to_ignore=[401],
)

def test_sending_special_characters(
self, test_authenticated_client, test_http_server
):
request_params = SaaSRequestParams(
method=HTTPMethod.POST,
path="/",
body='{"addr": "1234 Peterson’s Farm Rd."}',
headers={"Content-Type": "application/json"},
)

test_authenticated_client.uri = test_http_server
test_authenticated_client.send(request_params)
galvana marked this conversation as resolved.
Show resolved Hide resolved


@pytest.mark.unit_saas
class TestRetryAfterHeaderParsing:
Expand Down
Loading