From 9a256741d5a0fe3e9c52860a8f5ea9e1b30408d3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Mar 2026 11:26:52 +0000 Subject: [PATCH] fix(auth): address review comments - improve debug logging, use SimpleNamespace, fix session cleanup Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- app/utils/user_scope.py | 2 +- tests/test_multi_user.py | 22 ++++++++++++++-------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/app/utils/user_scope.py b/app/utils/user_scope.py index fbec10cf..a2d169c7 100644 --- a/app/utils/user_scope.py +++ b/app/utils/user_scope.py @@ -84,7 +84,7 @@ def get_current_owner_id(request: Request) -> str | None: request.state.api_token_user = resolved return _owner_id_from_user(resolved) except Exception: - logger.debug("Bearer token resolution failed in get_current_owner_id") + logger.debug("Bearer token resolution failed in get_current_owner_id", exc_info=True) return None diff --git a/tests/test_multi_user.py b/tests/test_multi_user.py index c85ee799..49456647 100644 --- a/tests/test_multi_user.py +++ b/tests/test_multi_user.py @@ -189,6 +189,8 @@ class TestGetCurrentOwnerId: @pytest.mark.unit def test_resolves_bearer_token_directly(self, mu_engine, mu_session): """get_current_owner_id should resolve a Bearer token when no session exists.""" + from types import SimpleNamespace + from app.api.api_tokens import generate_api_token, hash_token from app.utils.user_scope import get_current_owner_id @@ -205,16 +207,18 @@ class TestGetCurrentOwnerId: mu_session.add(db_token) mu_session.commit() - # Build a mock request with Bearer header but no session + # Build a mock request with Bearer header but no session. + # SimpleNamespace starts with no attributes so getattr(..., None) works. request = MagicMock() request.session = {} - request.state = MagicMock(spec=[]) # no api_token_user attr + request.state = SimpleNamespace() request.headers = {"authorization": f"Bearer {plaintext}"} request.client.host = "127.0.0.1" - with patch("app.database.SessionLocal", return_value=mu_session): - # Prevent the session from being closed since it's shared with the test - mu_session.close = MagicMock() + # Provide the test session and make close() a no-op so the shared + # session is not torn down prematurely. + noop_close = MagicMock() + with patch("app.database.SessionLocal", return_value=mu_session), patch.object(mu_session, "close", noop_close): result = get_current_owner_id(request) assert result == "bearer-owner" @@ -224,16 +228,18 @@ class TestGetCurrentOwnerId: @pytest.mark.unit def test_returns_none_for_invalid_bearer_token(self, mu_engine, mu_session): """get_current_owner_id should return None for an invalid Bearer token.""" + from types import SimpleNamespace + from app.utils.user_scope import get_current_owner_id request = MagicMock() request.session = {} - request.state = MagicMock(spec=[]) # no api_token_user attr + request.state = SimpleNamespace() request.headers = {"authorization": "Bearer de_invalid_token_value"} request.client.host = "127.0.0.1" - with patch("app.database.SessionLocal", return_value=mu_session): - mu_session.close = MagicMock() + noop_close = MagicMock() + with patch("app.database.SessionLocal", return_value=mu_session), patch.object(mu_session, "close", noop_close): result = get_current_owner_id(request) assert result is None