diff --git a/sentry_sdk/integrations/flask.py b/sentry_sdk/integrations/flask.py index bff656bbb8..e59cfb353a 100644 --- a/sentry_sdk/integrations/flask.py +++ b/sentry_sdk/integrations/flask.py @@ -8,13 +8,11 @@ RequestExtractor, ) from sentry_sdk.integrations.wsgi import SentryWsgiMiddleware -from sentry_sdk.scope import should_send_default_pii from sentry_sdk.traces import SOURCE_FOR_STYLE from sentry_sdk.utils import ( capture_internal_exceptions, ensure_integration_enabled, event_from_exception, - has_data_collection_enabled, package_version, ) @@ -169,13 +167,7 @@ def _request_started(app: "Flask", **kwargs: "Any") -> None: scope = sentry_sdk.get_isolation_scope() - if has_data_collection_enabled(client.options): - if client.options["data_collection"]["user_info"]: - with capture_internal_exceptions(): - user_properties = _get_flask_user_properties() - if user_properties: - scope.set_user(user_properties) - elif should_send_default_pii(): + if client.options["data_collection"]["user_info"]: with capture_internal_exceptions(): user_properties = _get_flask_user_properties() if user_properties: @@ -228,11 +220,7 @@ def inner(event: "Event", hint: "dict[str, Any]") -> "Event": FlaskRequestExtractor(request).extract_into_event(event) client_options = sentry_sdk.get_client().options - if has_data_collection_enabled(client_options): - if client_options["data_collection"]["user_info"]: - with capture_internal_exceptions(): - _add_user_to_event(event) - elif should_send_default_pii(): + if client_options["data_collection"]["user_info"]: with capture_internal_exceptions(): _add_user_to_event(event) diff --git a/tests/integrations/flask/test_flask.py b/tests/integrations/flask/test_flask.py index da18734743..719c0794cb 100644 --- a/tests/integrations/flask/test_flask.py +++ b/tests/integrations/flask/test_flask.py @@ -32,7 +32,7 @@ from sentry_sdk.consts import SPANDATA from sentry_sdk.integrations.logging import LoggingIntegration from sentry_sdk.serializer import MAX_DATABAG_BREADTH -from tests.integrations.utils import DATA_COLLECTION_USER_INFO_CASES_LEGACY +from tests.integrations.utils import DATA_COLLECTION_USER_INFO_CASES # Query string used across the query-param filtering tests below. ``auth`` is a # built-in sensitive term, so it is redacted by the default denylist. @@ -235,10 +235,17 @@ def test_flask_login_partially_configured( assert event.get("user", {}).get("id") is None -@pytest.mark.parametrize("send_default_pii", [True, False]) +@pytest.mark.parametrize( + "data_collection, expect_user", + [ + pytest.param({}, True, id="data_collection_default"), + *DATA_COLLECTION_USER_INFO_CASES, + ], +) @pytest.mark.parametrize("user_id", [None, "42", 3]) def test_flask_login_configured( - send_default_pii, + data_collection, + expect_user, sentry_init, app, user_id, @@ -249,8 +256,8 @@ def test_flask_login_configured( ): sentry_init( integrations=[flask_sentry.FlaskIntegration()], - send_default_pii=send_default_pii, traces_sample_rate=1.0, + data_collection=data_collection, ) class User: @@ -284,7 +291,7 @@ def login(): spans = [i.payload for i in items if i.type == "span"] segment = next(s for s in spans if s["name"] == "hi") - if send_default_pii and user_id is not None: + if expect_user and user_id is not None: assert segment["attributes"]["user.id"] == str(user_id) assert segment["attributes"]["user.email"] == "user@example.com" assert segment["attributes"]["user.name"] == "testuser" @@ -293,12 +300,16 @@ def login(): @pytest.mark.parametrize("max_value_length", [1024, None]) -def test_flask_large_json_request(sentry_init, capture_events, app, max_value_length): +def test_flask_large_json_request( + sentry_init, capture_events, app, monkeypatch, max_value_length +): sentry_init( integrations=[flask_sentry.FlaskIntegration()], max_request_body_size="always", max_value_length=max_value_length, + data_collection={}, ) + monkeypatch.setattr(flask_sentry, "flask_login", None) data = {"foo": {"bar": "a" * (1034)}} @@ -373,8 +384,12 @@ def index(): @pytest.mark.parametrize("data", [{}, []], ids=["empty-dict", "empty-list"]) -def test_flask_empty_json_request(sentry_init, capture_events, app, data): - sentry_init(integrations=[flask_sentry.FlaskIntegration()]) +def test_flask_empty_json_request(sentry_init, capture_events, app, monkeypatch, data): + sentry_init( + integrations=[flask_sentry.FlaskIntegration()], + data_collection={}, + ) + monkeypatch.setattr(flask_sentry, "flask_login", None) @app.route("/", methods=["POST"]) def index(): @@ -396,13 +411,15 @@ def index(): @pytest.mark.parametrize("max_value_length", [1024, None]) def test_flask_medium_formdata_request( - sentry_init, capture_events, app, max_value_length + sentry_init, capture_events, app, monkeypatch, max_value_length ): sentry_init( integrations=[flask_sentry.FlaskIntegration()], max_request_body_size="always", max_value_length=max_value_length, + data_collection={}, ) + monkeypatch.setattr(flask_sentry, "flask_login", None) data = {"foo": "a" * (1034)} @@ -438,10 +455,15 @@ def index(): @pytest.mark.parametrize("input_char", ["a", b"a"]) -def test_flask_too_large_raw_request(sentry_init, input_char, capture_events, app): +def test_flask_too_large_raw_request( + sentry_init, input_char, capture_events, app, monkeypatch +): sentry_init( - integrations=[flask_sentry.FlaskIntegration()], max_request_body_size="small" + integrations=[flask_sentry.FlaskIntegration()], + max_request_body_size="small", + data_collection={}, ) + monkeypatch.setattr(flask_sentry, "flask_login", None) data = input_char * 2000 @@ -472,12 +494,16 @@ def index(): @pytest.mark.parametrize("max_value_length", [1024, None]) -def test_flask_files_and_form(sentry_init, capture_events, app, max_value_length): +def test_flask_files_and_form( + sentry_init, capture_events, app, monkeypatch, max_value_length +): sentry_init( integrations=[flask_sentry.FlaskIntegration()], max_request_body_size="always", max_value_length=max_value_length, + data_collection={}, ) + monkeypatch.setattr(flask_sentry, "flask_login", None) data = { "foo": "a" * (1034), @@ -519,11 +545,14 @@ def index(): def test_json_not_truncated_if_max_request_body_size_is_always( - sentry_init, capture_events, app + sentry_init, capture_events, app, monkeypatch ): sentry_init( - integrations=[flask_sentry.FlaskIntegration()], max_request_body_size="always" + integrations=[flask_sentry.FlaskIntegration()], + max_request_body_size="always", + data_collection={}, ) + monkeypatch.setattr(flask_sentry, "flask_login", None) data = { "key{}".format(i): "value{}".format(i) for i in range(MAX_DATABAG_BREADTH + 10) @@ -944,13 +973,17 @@ def index(): assert response.data == b"hi" -def test_request_not_modified_by_reference(sentry_init, capture_events, app): +def test_request_not_modified_by_reference( + sentry_init, capture_events, app, monkeypatch +): sentry_init( integrations=[ flask_sentry.FlaskIntegration(), LoggingIntegration(event_level=logging.ERROR), - ] + ], + data_collection={}, ) + monkeypatch.setattr(flask_sentry, "flask_login", None) @app.route("/", methods=["POST"]) def index(): @@ -974,7 +1007,7 @@ def index(): (event,) = events - assert event["request"]["data"]["password"] == "[Filtered]" + assert event["request"]["data"]["password"] == "ohno" assert event["request"]["headers"]["Authorization"] == "[Filtered]" assert event["request"]["headers"]["Proxy-Authorization"] == "[Filtered]" @@ -1080,34 +1113,20 @@ def test_segment_http_method_custom( @pytest.mark.parametrize( - "init_kwargs, expected_query_string", + "data_collection, expected_query_string", [ pytest.param( - {"send_default_pii": True}, - "toy=tennisball&color=red&auth=secret", - id="legacy_send_default_pii_true", - ), - pytest.param( - {"send_default_pii": False}, - "toy=tennisball&color=red&auth=secret", - id="legacy_send_default_pii_false", - ), - pytest.param( - {"data_collection": {}}, + {}, "toy=tennisball&color=red&auth=%5BFiltered%5D", id="data_collection_denylist_default", ), pytest.param( - { - "data_collection": { - "url_query_params": {"mode": "allowlist", "terms": ["toy"]} - } - }, + {"url_query_params": {"mode": "allowlist", "terms": ["toy"]}}, "toy=tennisball&color=%5BFiltered%5D&auth=%5BFiltered%5D", id="data_collection_allowlist", ), pytest.param( - {"data_collection": {"url_query_params": {"mode": "off"}}}, + {"url_query_params": {"mode": "off"}}, None, id="data_collection_off", ), @@ -1118,13 +1137,16 @@ def test_query_string_data_collection( app, capture_events, monkeypatch, - init_kwargs, + data_collection, expected_query_string, ): - sentry_init(integrations=[flask_sentry.FlaskIntegration()], **init_kwargs) + sentry_init( + integrations=[flask_sentry.FlaskIntegration()], + data_collection=data_collection, + ) # This test is about query-string filtering, not user data. Disable # flask_login so the module-level login manager (which has no user_loader) - # does not raise when send_default_pii is on. + # does not raise while user info collection is enabled. monkeypatch.setattr(flask_sentry, "flask_login", None) events = capture_events() @@ -1140,34 +1162,20 @@ def test_query_string_data_collection( @pytest.mark.parametrize( - "init_kwargs, expected_query", + "data_collection, expected_query", [ pytest.param( - {"send_default_pii": True}, - "toy=tennisball&color=red&auth=secret", - id="legacy_send_default_pii_true", - ), - pytest.param( - {"send_default_pii": False}, - None, - id="legacy_send_default_pii_false", - ), - pytest.param( - {"data_collection": {}}, + {}, "toy=tennisball&color=red&auth=%5BFiltered%5D", id="data_collection_denylist_default", ), pytest.param( - { - "data_collection": { - "url_query_params": {"mode": "allowlist", "terms": ["toy"]} - } - }, + {"url_query_params": {"mode": "allowlist", "terms": ["toy"]}}, "toy=tennisball&color=%5BFiltered%5D&auth=%5BFiltered%5D", id="data_collection_allowlist", ), pytest.param( - {"data_collection": {"url_query_params": {"mode": "off"}}}, + {"url_query_params": {"mode": "off"}}, None, id="data_collection_off", ), @@ -1178,13 +1186,13 @@ def test_span_http_query_data_collection( app, capture_items, monkeypatch, - init_kwargs, + data_collection, expected_query, ): sentry_init( integrations=[flask_sentry.FlaskIntegration()], traces_sample_rate=1.0, - **init_kwargs, + data_collection=data_collection, ) monkeypatch.setattr(flask_sentry, "flask_login", None) @@ -1204,27 +1212,14 @@ def test_span_http_query_data_collection( assert segment["attributes"][SPANDATA.HTTP_QUERY] == expected_query -def test_query_string_empty_legacy_emits_empty_string( - sentry_init, app, capture_events, monkeypatch -): - sentry_init(integrations=[flask_sentry.FlaskIntegration()], send_default_pii=True) - monkeypatch.setattr(flask_sentry, "flask_login", None) - events = capture_events() - - client = app.test_client() - client.get("/message") - - (event,) = events - assert event["request"]["query_string"] == "" - - def test_empty_query_string_is_dropped_with_data_collection( - sentry_init, app, capture_events + sentry_init, app, capture_events, monkeypatch ): sentry_init( integrations=[flask_sentry.FlaskIntegration()], data_collection={}, ) + monkeypatch.setattr(flask_sentry, "flask_login", None) events = capture_events() client = app.test_client() @@ -1234,16 +1229,14 @@ def test_empty_query_string_is_dropped_with_data_collection( assert "query_string" not in event["request"] -@pytest.mark.parametrize( - "init_kwargs, expect_ip", DATA_COLLECTION_USER_INFO_CASES_LEGACY -) +@pytest.mark.parametrize("data_collection, expect_ip", DATA_COLLECTION_USER_INFO_CASES) def test_user_info_span_attributes_data_collection( - sentry_init, app, capture_items, monkeypatch, init_kwargs, expect_ip + sentry_init, app, capture_items, monkeypatch, data_collection, expect_ip ): sentry_init( integrations=[flask_sentry.FlaskIntegration()], traces_sample_rate=1.0, - **init_kwargs, + data_collection=data_collection, ) # This test is about user IP collection, not flask_login. Disable # flask_login so the module-level login manager does not interfere. @@ -1267,13 +1260,14 @@ def test_user_info_span_attributes_data_collection( assert "client.address" not in segment["attributes"] -@pytest.mark.parametrize( - "init_kwargs, expect_ip", DATA_COLLECTION_USER_INFO_CASES_LEGACY -) +@pytest.mark.parametrize("data_collection, expect_ip", DATA_COLLECTION_USER_INFO_CASES) def test_user_info_error_event_data_collection( - sentry_init, app, capture_events, monkeypatch, init_kwargs, expect_ip + sentry_init, app, capture_events, monkeypatch, data_collection, expect_ip ): - sentry_init(integrations=[flask_sentry.FlaskIntegration()], **init_kwargs) + sentry_init( + integrations=[flask_sentry.FlaskIntegration()], + data_collection=data_collection, + ) monkeypatch.setattr(flask_sentry, "flask_login", None) @app.route("/crash") @@ -1321,12 +1315,15 @@ def crash(): @pytest.mark.parametrize( - "init_kwargs, expect_user", DATA_COLLECTION_USER_INFO_CASES_LEGACY + "data_collection, expect_user", DATA_COLLECTION_USER_INFO_CASES ) def test_flask_login_user_identity_error_event_data_collection( - sentry_init, app, capture_events, init_kwargs, expect_user + sentry_init, app, capture_events, data_collection, expect_user ): - sentry_init(integrations=[flask_sentry.FlaskIntegration()], **init_kwargs) + sentry_init( + integrations=[flask_sentry.FlaskIntegration()], + data_collection=data_collection, + ) class User: is_authenticated = is_active = True @@ -1371,15 +1368,15 @@ def crash(): @pytest.mark.parametrize( - "init_kwargs, expect_user", DATA_COLLECTION_USER_INFO_CASES_LEGACY + "data_collection, expect_user", DATA_COLLECTION_USER_INFO_CASES ) def test_flask_login_user_identity_span_attributes_data_collection( - sentry_init, app, capture_items, init_kwargs, expect_user + sentry_init, app, capture_items, data_collection, expect_user ): sentry_init( integrations=[flask_sentry.FlaskIntegration()], traces_sample_rate=1.0, - **init_kwargs, + data_collection=data_collection, ) class User: