fix(auth): resolve Bearer token in get_current_owner_id for mobile API auth
get_current_owner_id() only checked the session for authenticated users. When the mobile app sends Authorization: Bearer <token>, there is no session cookie, so the Depends(_get_owner_id) dependency raised HTTP 401 before the @require_login wrapper could resolve the Bearer token. The function now checks three sources in order: 1. Session user dict (existing behavior) 2. request.state.api_token_user (cached by require_login or prior call) 3. Direct Bearer token resolution via _resolve_bearer_user (new) Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
This commit is contained in:
+56
-8
@@ -20,13 +20,31 @@ from app.models import FileRecord
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def _owner_id_from_user(user: dict) -> str | None:
|
||||
"""Extract the owner identifier from a user dict.
|
||||
|
||||
Priority: ``sub`` (OAuth subject) → ``preferred_username`` → ``email`` → ``id``.
|
||||
"""
|
||||
return user.get("sub") or user.get("preferred_username") or user.get("email") or user.get("id")
|
||||
|
||||
|
||||
def get_current_owner_id(request: Request) -> str | None:
|
||||
"""Extract the owner identifier for the current authenticated user.
|
||||
|
||||
The owner ID is derived from the user's session data. It uses the
|
||||
``sub`` claim (OAuth subject) when available, falling back to
|
||||
``preferred_username`` or ``email``. Returns ``None`` when no user
|
||||
is authenticated.
|
||||
The owner ID is derived from the user's session data or, when no session
|
||||
is present, from a valid Bearer API token in the ``Authorization`` header.
|
||||
This ensures that both browser-based (session cookie) and mobile/API
|
||||
(Bearer token) requests are correctly identified.
|
||||
|
||||
Priority for user resolution:
|
||||
|
||||
1. Session ``user`` dict (set by OAuth or local login).
|
||||
2. ``request.state.api_token_user`` (set by ``require_login`` or an
|
||||
earlier call to this function during the same request).
|
||||
3. Direct Bearer token look-up against the database.
|
||||
|
||||
Within the resolved user dict the owner ID is chosen as:
|
||||
``sub`` → ``preferred_username`` → ``email`` → ``id``.
|
||||
|
||||
Args:
|
||||
request: The current FastAPI request with session data.
|
||||
@@ -34,11 +52,41 @@ def get_current_owner_id(request: Request) -> str | None:
|
||||
Returns:
|
||||
A stable string identifier for the user, or ``None``.
|
||||
"""
|
||||
# 1. Session-based auth (most common for web UI)
|
||||
user = request.session.get("user")
|
||||
if not user or not isinstance(user, dict):
|
||||
return None
|
||||
# Prefer 'sub' (OAuth subject), then 'preferred_username', then 'email', then 'id'
|
||||
return user.get("sub") or user.get("preferred_username") or user.get("email") or user.get("id")
|
||||
if user and isinstance(user, dict):
|
||||
return _owner_id_from_user(user)
|
||||
|
||||
# 2. Already-resolved API token user (cached by require_login or a
|
||||
# prior dependency call during this request)
|
||||
api_user = getattr(request.state, "api_token_user", None)
|
||||
if isinstance(api_user, dict):
|
||||
return _owner_id_from_user(api_user)
|
||||
|
||||
# 3. Direct Bearer token resolution – necessary when this function is
|
||||
# invoked as a FastAPI dependency (via Depends) which runs *before*
|
||||
# the @require_login decorator wrapper has had a chance to resolve
|
||||
# the token and populate request.state.api_token_user.
|
||||
auth_header = request.headers.get("authorization", "")
|
||||
if isinstance(auth_header, str) and auth_header.startswith("Bearer "):
|
||||
try:
|
||||
from app.auth import _resolve_bearer_user
|
||||
from app.database import SessionLocal
|
||||
|
||||
db = SessionLocal()
|
||||
try:
|
||||
resolved = _resolve_bearer_user(request, db)
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
if resolved:
|
||||
# Cache so subsequent calls (and require_login) skip the DB
|
||||
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")
|
||||
|
||||
return None
|
||||
|
||||
|
||||
def apply_owner_filter(query: Query, request: Request) -> Query:
|
||||
|
||||
+111
-1
@@ -18,7 +18,7 @@ from sqlalchemy.pool import StaticPool
|
||||
|
||||
from app.config import settings
|
||||
from app.database import Base
|
||||
from app.models import FileRecord
|
||||
from app.models import ApiToken, FileRecord
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Fixtures
|
||||
@@ -162,6 +162,116 @@ class TestGetCurrentOwnerId:
|
||||
request.session = {}
|
||||
assert get_current_owner_id(request) is None
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_resolves_from_api_token_user_state(self):
|
||||
"""get_current_owner_id should resolve from request.state.api_token_user."""
|
||||
from app.utils.user_scope import get_current_owner_id
|
||||
|
||||
request = MagicMock()
|
||||
request.session = {}
|
||||
request.state.api_token_user = {
|
||||
"id": "tok-owner",
|
||||
"preferred_username": "tok-owner",
|
||||
"email": "tok-owner",
|
||||
}
|
||||
assert get_current_owner_id(request) == "tok-owner"
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_session_takes_precedence_over_api_token_user(self):
|
||||
"""Session auth should take precedence over api_token_user in state."""
|
||||
from app.utils.user_scope import get_current_owner_id
|
||||
|
||||
request = MagicMock()
|
||||
request.session = {"user": {"sub": "session-sub", "email": "session@example.com"}}
|
||||
request.state.api_token_user = {"id": "tok-owner"}
|
||||
assert get_current_owner_id(request) == "session-sub"
|
||||
|
||||
@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 app.api.api_tokens import generate_api_token, hash_token
|
||||
from app.utils.user_scope import get_current_owner_id
|
||||
|
||||
# Create a token in the DB
|
||||
plaintext = generate_api_token()
|
||||
token_hash = hash_token(plaintext)
|
||||
db_token = ApiToken(
|
||||
owner_id="bearer-owner",
|
||||
name="Test Bearer",
|
||||
token_hash=token_hash,
|
||||
token_prefix=plaintext[:12],
|
||||
is_active=True,
|
||||
)
|
||||
mu_session.add(db_token)
|
||||
mu_session.commit()
|
||||
|
||||
# Build a mock request with Bearer header but no session
|
||||
request = MagicMock()
|
||||
request.session = {}
|
||||
request.state = MagicMock(spec=[]) # no api_token_user attr
|
||||
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()
|
||||
result = get_current_owner_id(request)
|
||||
|
||||
assert result == "bearer-owner"
|
||||
# Verify the resolved user was cached in request.state
|
||||
assert request.state.api_token_user["id"] == "bearer-owner"
|
||||
|
||||
@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 app.utils.user_scope import get_current_owner_id
|
||||
|
||||
request = MagicMock()
|
||||
request.session = {}
|
||||
request.state = MagicMock(spec=[]) # no api_token_user attr
|
||||
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()
|
||||
result = get_current_owner_id(request)
|
||||
|
||||
assert result is None
|
||||
|
||||
|
||||
class TestOwnerIdFromUser:
|
||||
"""Tests for the _owner_id_from_user helper."""
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_prefers_sub(self):
|
||||
from app.utils.user_scope import _owner_id_from_user
|
||||
|
||||
assert _owner_id_from_user({"sub": "s", "preferred_username": "u", "email": "e"}) == "s"
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_falls_back_to_preferred_username(self):
|
||||
from app.utils.user_scope import _owner_id_from_user
|
||||
|
||||
assert _owner_id_from_user({"preferred_username": "u", "email": "e"}) == "u"
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_falls_back_to_email(self):
|
||||
from app.utils.user_scope import _owner_id_from_user
|
||||
|
||||
assert _owner_id_from_user({"email": "e"}) == "e"
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_falls_back_to_id(self):
|
||||
from app.utils.user_scope import _owner_id_from_user
|
||||
|
||||
assert _owner_id_from_user({"id": "i"}) == "i"
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_returns_none_for_empty_dict(self):
|
||||
from app.utils.user_scope import _owner_id_from_user
|
||||
|
||||
assert _owner_id_from_user({}) is None
|
||||
|
||||
|
||||
class TestApplyOwnerFilter:
|
||||
"""Tests for apply_owner_filter()."""
|
||||
|
||||
Reference in New Issue
Block a user