fix(auth): address review comments - improve debug logging, use SimpleNamespace, fix session cleanup
Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
This commit is contained in:
@@ -84,7 +84,7 @@ def get_current_owner_id(request: Request) -> str | None:
|
|||||||
request.state.api_token_user = resolved
|
request.state.api_token_user = resolved
|
||||||
return _owner_id_from_user(resolved)
|
return _owner_id_from_user(resolved)
|
||||||
except Exception:
|
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
|
return None
|
||||||
|
|
||||||
|
|||||||
@@ -189,6 +189,8 @@ class TestGetCurrentOwnerId:
|
|||||||
@pytest.mark.unit
|
@pytest.mark.unit
|
||||||
def test_resolves_bearer_token_directly(self, mu_engine, mu_session):
|
def test_resolves_bearer_token_directly(self, mu_engine, mu_session):
|
||||||
"""get_current_owner_id should resolve a Bearer token when no session exists."""
|
"""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.api.api_tokens import generate_api_token, hash_token
|
||||||
from app.utils.user_scope import get_current_owner_id
|
from app.utils.user_scope import get_current_owner_id
|
||||||
|
|
||||||
@@ -205,16 +207,18 @@ class TestGetCurrentOwnerId:
|
|||||||
mu_session.add(db_token)
|
mu_session.add(db_token)
|
||||||
mu_session.commit()
|
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 = MagicMock()
|
||||||
request.session = {}
|
request.session = {}
|
||||||
request.state = MagicMock(spec=[]) # no api_token_user attr
|
request.state = SimpleNamespace()
|
||||||
request.headers = {"authorization": f"Bearer {plaintext}"}
|
request.headers = {"authorization": f"Bearer {plaintext}"}
|
||||||
request.client.host = "127.0.0.1"
|
request.client.host = "127.0.0.1"
|
||||||
|
|
||||||
with patch("app.database.SessionLocal", return_value=mu_session):
|
# Provide the test session and make close() a no-op so the shared
|
||||||
# Prevent the session from being closed since it's shared with the test
|
# session is not torn down prematurely.
|
||||||
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)
|
result = get_current_owner_id(request)
|
||||||
|
|
||||||
assert result == "bearer-owner"
|
assert result == "bearer-owner"
|
||||||
@@ -224,16 +228,18 @@ class TestGetCurrentOwnerId:
|
|||||||
@pytest.mark.unit
|
@pytest.mark.unit
|
||||||
def test_returns_none_for_invalid_bearer_token(self, mu_engine, mu_session):
|
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."""
|
"""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
|
from app.utils.user_scope import get_current_owner_id
|
||||||
|
|
||||||
request = MagicMock()
|
request = MagicMock()
|
||||||
request.session = {}
|
request.session = {}
|
||||||
request.state = MagicMock(spec=[]) # no api_token_user attr
|
request.state = SimpleNamespace()
|
||||||
request.headers = {"authorization": "Bearer de_invalid_token_value"}
|
request.headers = {"authorization": "Bearer de_invalid_token_value"}
|
||||||
request.client.host = "127.0.0.1"
|
request.client.host = "127.0.0.1"
|
||||||
|
|
||||||
with patch("app.database.SessionLocal", return_value=mu_session):
|
noop_close = MagicMock()
|
||||||
mu_session.close = MagicMock()
|
with patch("app.database.SessionLocal", return_value=mu_session), patch.object(mu_session, "close", noop_close):
|
||||||
result = get_current_owner_id(request)
|
result = get_current_owner_id(request)
|
||||||
|
|
||||||
assert result is None
|
assert result is None
|
||||||
|
|||||||
Reference in New Issue
Block a user