Skip to content

Commit e2dd162

Browse files
authored
fix(starlette): Don't overwrite user set during request in AuthenticationMiddleware (#6760)
The AuthenticationMiddleware patch added the request-derived user after calling the wrapped handler, silently overwriting any user set by the application during the request (e.g. via `sentry_sdk.set_user`). Add the user before invoking the handler so app-set user data takes precedence. Fixes PY-2596 Fixes #6756
1 parent 85ba59f commit e2dd162

2 files changed

Lines changed: 43 additions & 2 deletions

File tree

sentry_sdk/integrations/starlette.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -408,8 +408,8 @@ async def _sentry_authenticationmiddleware_call(
408408
receive: "Callable[[], Awaitable[Dict[str, Any]]]",
409409
send: "Callable[[Dict[str, Any]], Awaitable[None]]",
410410
) -> None:
411-
await old_call(self, scope, receive, send)
412411
_add_user_to_sentry_scope(scope)
412+
await old_call(self, scope, receive, send)
413413

414414
middleware_class.__call__ = _sentry_authenticationmiddleware_call
415415

@@ -422,7 +422,6 @@ def patch_middlewares() -> None:
422422
old_middleware_init = Middleware.__init__
423423

424424
not_yet_patched = "_sentry_middleware_init" not in str(old_middleware_init)
425-
426425
if not_yet_patched:
427426

428427
def _sentry_middleware_init(

tests/integrations/starlette/test_starlette.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -705,6 +705,48 @@ def test_user_information_transaction_no_pii(sentry_init, capture_events):
705705
assert "user" not in transaction_event
706706

707707

708+
def test_user_information_does_not_clobber_app_set_user(sentry_init, capture_events):
709+
"""
710+
Regression test for PY-2596/GH-6756: the AuthenticationMiddleware patch must add the
711+
request-derived user *before* the request handler runs, so that a user the app
712+
sets during the request (via ``sentry_sdk.set_user``) is preserved rather than
713+
overwritten. Starlette's ``SimpleUser`` only carries a username, so an app-set
714+
``id``/``email`` would otherwise be lost.
715+
"""
716+
sentry_init(
717+
traces_sample_rate=1.0,
718+
send_default_pii=True,
719+
integrations=[StarletteIntegration()],
720+
)
721+
722+
async def _set_user(request):
723+
sentry_sdk.set_user(
724+
{
725+
"id": "user_42",
726+
"email": "ada@beans.com",
727+
"username": "Ada",
728+
}
729+
)
730+
return starlette.responses.JSONResponse({"status": "ok"})
731+
732+
app = starlette.applications.Starlette(
733+
routes=[starlette.routing.Route("/set_user", _set_user)],
734+
middleware=[Middleware(AuthenticationMiddleware, backend=BasicAuthBackend())],
735+
)
736+
737+
events = capture_events()
738+
739+
client = TestClient(app, raise_server_exceptions=False)
740+
client.get("/set_user", auth=("Ada", "hello123"))
741+
742+
(transaction_event,) = events
743+
user = transaction_event.get("user", None)
744+
assert user
745+
assert user["username"] == "Ada"
746+
assert user["id"] == "user_42"
747+
assert user["email"] == "ada@beans.com"
748+
749+
708750
@pytest.mark.parametrize("span_streaming", [True, False])
709751
def test_middleware_spans(sentry_init, capture_events, capture_items, span_streaming):
710752
sentry_init(

0 commit comments

Comments
 (0)