mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(auth): /auth/login 422'd — FastAPI demoted db to a query param (#580)
Caught live on the NAS: every cloud-auth login failed with
422 {"loc": ["query", "db"]} regardless of credentials. Root cause:
auth/manager.py used postponed annotations with a TYPE_CHECKING-only
AsyncSession import, so FastAPI could not resolve get_user_db's
Annotated[AsyncSession, Depends(get_db)] at runtime and silently
demoted `db` to a required query parameter.
The module now evaluates annotations eagerly (no future-annotations,
runtime imports) so an unresolvable annotation is loud instead of a
silent contract change. Regression test mounts the REAL login router —
no dependency overrides, which is exactly why the existing suite never
caught this — and asserts wrong credentials yield 400
LOGIN_BAD_CREDENTIALS, never a 422.
Co-authored-by: Renn F <rennf93@users.noreply.github.com>
This commit is contained in:
@@ -5,24 +5,26 @@ package never depends on the deps module — deps depends on auth, not the
|
|||||||
other way around.
|
other way around.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
from __future__ import annotations
|
# No `from __future__ import annotations` here, deliberately: FastAPI must
|
||||||
|
# resolve get_user_db's Annotated[AsyncSession, Depends(...)] at runtime,
|
||||||
|
# and with postponed annotations plus a TYPE_CHECKING-only AsyncSession the
|
||||||
|
# resolution failed silently — FastAPI demoted `db` to a REQUIRED QUERY
|
||||||
|
# parameter and every /auth/login 422'd (caught live on the NAS,
|
||||||
|
# 2026-07-19). Eager evaluation makes an unresolvable annotation loud.
|
||||||
|
|
||||||
from typing import TYPE_CHECKING, Annotated
|
from collections.abc import AsyncGenerator
|
||||||
|
from typing import Annotated
|
||||||
from uuid import UUID
|
from uuid import UUID
|
||||||
|
|
||||||
from fastapi import Depends
|
from fastapi import Depends
|
||||||
from fastapi_users import BaseUserManager, UUIDIDMixin
|
from fastapi_users import BaseUserManager, UUIDIDMixin
|
||||||
from fastapi_users_db_sqlalchemy import SQLAlchemyUserDatabase
|
from fastapi_users_db_sqlalchemy import SQLAlchemyUserDatabase
|
||||||
|
from sqlalchemy.ext.asyncio import AsyncSession
|
||||||
|
|
||||||
from roboco.config import settings
|
from roboco.config import settings
|
||||||
from roboco.db.base import get_db
|
from roboco.db.base import get_db
|
||||||
from roboco.db.tables import UserTable
|
from roboco.db.tables import UserTable
|
||||||
|
|
||||||
if TYPE_CHECKING:
|
|
||||||
from collections.abc import AsyncGenerator
|
|
||||||
|
|
||||||
from sqlalchemy.ext.asyncio import AsyncSession
|
|
||||||
|
|
||||||
# Placeholder used only while cloud auth is off — BaseUserManager requires a
|
# Placeholder used only while cloud auth is off — BaseUserManager requires a
|
||||||
# non-empty secret attribute, but the reset-password/verify flows it signs
|
# non-empty secret attribute, but the reset-password/verify flows it signs
|
||||||
# are non-goals (never mounted), so the value is inert unless those routes
|
# are non-goals (never mounted), so the value is inert unless those routes
|
||||||
|
|||||||
@@ -15,15 +15,16 @@ from typing import TYPE_CHECKING
|
|||||||
from unittest.mock import AsyncMock, MagicMock, patch
|
from unittest.mock import AsyncMock, MagicMock, patch
|
||||||
from uuid import UUID, uuid4
|
from uuid import UUID, uuid4
|
||||||
|
|
||||||
|
import httpx
|
||||||
import jwt as _jwt
|
import jwt as _jwt
|
||||||
import pytest
|
import pytest
|
||||||
from fastapi import HTTPException, Response
|
from fastapi import FastAPI, HTTPException, Response
|
||||||
from fastapi_users.password import PasswordHelper
|
from fastapi_users.password import PasswordHelper
|
||||||
from roboco.agents_config import CEO_AGENT_ID, issue_agent_token
|
from roboco.agents_config import CEO_AGENT_ID, issue_agent_token
|
||||||
from roboco.api import websocket as ws_module
|
from roboco.api import websocket as ws_module
|
||||||
from roboco.api.auth import revocation
|
from roboco.api.auth import revocation
|
||||||
from roboco.api.auth.backend import SESSION_COOKIE_NAME, get_jwt_strategy
|
from roboco.api.auth.backend import SESSION_COOKIE_NAME, get_jwt_strategy
|
||||||
from roboco.api.auth.routes import auth_status
|
from roboco.api.auth.routes import auth_status, mount_cloud_auth
|
||||||
from roboco.api.auth.seed import ensure_seed_user
|
from roboco.api.auth.seed import ensure_seed_user
|
||||||
from roboco.api.deps import _slide_session_cookie, get_agent_context
|
from roboco.api.deps import _slide_session_cookie, get_agent_context
|
||||||
from roboco.config import settings
|
from roboco.config import settings
|
||||||
@@ -576,3 +577,38 @@ async def test_read_token_accepts_unrevoked_jti(
|
|||||||
manager.get = AsyncMock(return_value=user)
|
manager.get = AsyncMock(return_value=user)
|
||||||
result = await get_jwt_strategy().read_token(token, manager)
|
result = await get_jwt_strategy().read_token(token, manager)
|
||||||
assert result is not None and str(result.id) == str(user.id)
|
assert result is not None and str(result.id) == str(user.id)
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Login route wire contract — the REAL router, no dependency overrides.
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
_HTTP_400 = 400
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_login_route_parses_oauth2_form_not_query_params(
|
||||||
|
monkeypatch: pytest.MonkeyPatch,
|
||||||
|
) -> None:
|
||||||
|
"""Regression (caught live on the NAS, 2026-07-19): a TYPE_CHECKING-only
|
||||||
|
AsyncSession import in auth.manager broke FastAPI's postponed-annotation
|
||||||
|
resolution for get_user_db, silently demoting `db` to a REQUIRED QUERY
|
||||||
|
parameter — every /auth/login 422'd (loc ["query", "db"]) regardless of
|
||||||
|
credentials. Mount the real router (dependency overrides would mask the
|
||||||
|
resolution path) and post the OAuth2 form: wrong credentials must yield
|
||||||
|
400 LOGIN_BAD_CREDENTIALS, never a 422 validation error.
|
||||||
|
"""
|
||||||
|
monkeypatch.setattr(settings, "cloud_auth_enabled", True)
|
||||||
|
app = FastAPI()
|
||||||
|
mount_cloud_auth(app, "/api/auth")
|
||||||
|
transport = httpx.ASGITransport(app=app)
|
||||||
|
async with httpx.AsyncClient(
|
||||||
|
transport=transport, base_url="http://testserver"
|
||||||
|
) as client:
|
||||||
|
resp = await client.post(
|
||||||
|
"/api/auth/login",
|
||||||
|
data={"username": "nobody@example.com", "password": "wrong"},
|
||||||
|
)
|
||||||
|
assert resp.status_code == _HTTP_400, resp.text
|
||||||
|
assert resp.json()["detail"] == "LOGIN_BAD_CREDENTIALS"
|
||||||
|
|||||||
Reference in New Issue
Block a user