Skip to content

Commit

Permalink
Adding support for special characters in SaaS request body (#5099)
Browse files Browse the repository at this point in the history
Co-authored-by: Andres Torres <[email protected]>
  • Loading branch information
galvana and andres-torres-marroquin authored Jul 28, 2024
1 parent 7259f7d commit 7977269
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ The types of changes are:
### Added
- Add AWS Tags in the meta field for Fides system when using `fides generate` [#4998](https://github.com/ethyca/fides/pull/4998)
- Added access and erasure support for Checkr integration [#5121](https://github.com/ethyca/fides/pull/5121)
- Added support for special characters in SaaS request payloads [#5099](https://github.com/ethyca/fides/pull/5099)

### Changed
- Moving Privacy Center endpoint logging behind debug flag [#5103](https://github.com/ethyca/fides/pull/5103)
Expand Down
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
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")

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"
)
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")
@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,
_,
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)


@pytest.mark.unit_saas
class TestRetryAfterHeaderParsing:
Expand Down

0 comments on commit 7977269

Please sign in to comment.