Skip to content

Commit bc83a6c

Browse files
jopemachineclaude
andcommitted
fix(BA-7364): require the rate limit field on the auth response
A missing field defaulting to null reads as unlimited, so an incomplete payload would silently disable the limit. Null stays a valid value — it is what an unset limit means — but it now has to be sent explicitly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent b6f8820 commit bc83a6c

6 files changed

Lines changed: 142 additions & 1 deletion

File tree

src/ai/backend/common/dto/manager/auth/types.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ class AuthSuccessResponse(AuthResponse):
5858
status: str
5959
session_token: str
6060
user_id: UserID
61-
rate_limit: int | None = None
61+
rate_limit: int | None
6262
type: AuthTokenType = AuthTokenType.KEYPAIR
6363

6464
def to_dict(self) -> dict[str, Any]:

tests/unit/client_v2/test_auth_client.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ async def test_authorize(self, sample_client_type_id: UUID) -> None:
8282
"status": "active",
8383
"session_token": "test_session_token",
8484
"user_id": "12345678-1234-5678-1234-567812345678",
85+
"rate_limit": None,
8586
"type": AuthTokenType.KEYPAIR,
8687
},
8788
}

tests/unit/common/dto/manager/auth/test_auth_response.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ def test_authorize_response() -> None:
3434
status="active",
3535
session_token="test_session_token",
3636
user_id=UserID(uuid4()),
37+
rate_limit=None,
3738
type=AuthTokenType.KEYPAIR,
3839
)
3940
resp = AuthorizeResponse(data=data)

tests/unit/common/dto/manager/auth/test_auth_types.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ def test_auth_success_response_creation() -> None:
4545
status="active",
4646
session_token="test_session_token",
4747
user_id=UserID(uuid4()),
48+
rate_limit=None,
4849
)
4950
assert resp.access_key == "AKTEST"
5051
assert resp.secret_key == "SKTEST"
@@ -62,6 +63,7 @@ def test_auth_success_response_to_dict() -> None:
6263
status="active",
6364
session_token="test_session_token",
6465
user_id=UserID(uuid4()),
66+
rate_limit=None,
6567
type=AuthTokenType.JWT,
6668
)
6769
d = resp.to_dict()
@@ -123,6 +125,7 @@ def test_parse_auth_response_success() -> None:
123125
"status": "active",
124126
"session_token": "test_token",
125127
"user_id": "12345678-1234-5678-1234-567812345678",
128+
"rate_limit": None,
126129
}
127130
result = parse_auth_response(data)
128131
assert isinstance(result, AuthSuccessResponse)
@@ -159,6 +162,7 @@ def test_parse_auth_response_explicit_success() -> None:
159162
"status": "active",
160163
"session_token": "test_token",
161164
"user_id": "12345678-1234-5678-1234-567812345678",
165+
"rate_limit": None,
162166
}
163167
result = parse_auth_response(data)
164168
assert isinstance(result, AuthSuccessResponse)
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
python_tests(
2+
name="tests",
3+
)
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
"""The three input states must survive serialization as three distinct wire shapes.
2+
3+
A field the caller never mentioned must be absent from the body, an explicit ``None``
4+
must survive as a JSON null, and a value must survive as itself. Collapsing any two of
5+
these makes "leave this alone" and "clear this" indistinguishable to the server.
6+
7+
"Never mentioned" has two spellings — omitting the constructor argument and assigning
8+
``SENTINEL`` — and ``BaseRequestModel`` drops the latter from ``model_fields_set`` so
9+
that both produce the same body.
10+
"""
11+
12+
from __future__ import annotations
13+
14+
import importlib
15+
import pkgutil
16+
from dataclasses import dataclass
17+
from typing import Any, get_args
18+
19+
import pytest
20+
21+
import ai.backend.common.dto.manager.v2 as v2_dto
22+
from ai.backend.common.api_handlers import SENTINEL, BaseRequestModel, Sentinel
23+
24+
# Mirrors client/v2/base_client.py, which is what actually puts a request on the wire.
25+
_WIRE_DUMP_KWARGS: dict[str, Any] = {"mode": "json", "exclude_unset": True}
26+
27+
28+
def _sentinel_fields() -> list[tuple[type[BaseRequestModel], str]]:
29+
found: list[tuple[type[BaseRequestModel], str]] = []
30+
for module in pkgutil.iter_modules(v2_dto.__path__):
31+
try:
32+
request_module = importlib.import_module(f"{v2_dto.__name__}.{module.name}.request")
33+
except ModuleNotFoundError:
34+
continue
35+
for attribute in vars(request_module).values():
36+
if not (isinstance(attribute, type) and issubclass(attribute, BaseRequestModel)):
37+
continue
38+
if attribute.__module__ != request_module.__name__:
39+
continue
40+
for name, info in attribute.model_fields.items():
41+
if Sentinel in get_args(info.annotation):
42+
found.append((attribute, name))
43+
return found
44+
45+
46+
@dataclass(frozen=True)
47+
class _WireExpectation:
48+
key_present: bool
49+
value: Any
50+
51+
52+
@dataclass(frozen=True)
53+
class _StateCase:
54+
label: str
55+
kwargs: dict[str, Any]
56+
expected: _WireExpectation
57+
58+
59+
class TestSentinelWireProtocol:
60+
def test_every_sentinel_field_defaults_to_sentinel(self) -> None:
61+
offenders = [
62+
f"{cls.__module__.rsplit('.', 2)[-2]}.{cls.__name__}.{name}"
63+
for cls, name in _sentinel_fields()
64+
if cls.model_fields[name].default is not SENTINEL
65+
]
66+
assert offenders == [], (
67+
"Sentinel-typed fields must default to SENTINEL so that a field the caller "
68+
f"never mentioned means 'no change': {offenders}"
69+
)
70+
71+
@pytest.mark.parametrize(
72+
"case",
73+
[
74+
_StateCase(
75+
label="argument-omitted",
76+
kwargs={},
77+
expected=_WireExpectation(key_present=False, value=None),
78+
),
79+
_StateCase(
80+
label="sentinel-assigned",
81+
kwargs={"field": SENTINEL},
82+
expected=_WireExpectation(key_present=False, value=None),
83+
),
84+
_StateCase(
85+
label="explicit-null",
86+
kwargs={"field": None},
87+
expected=_WireExpectation(key_present=True, value=None),
88+
),
89+
_StateCase(
90+
label="value",
91+
kwargs={"field": "a-value"},
92+
expected=_WireExpectation(key_present=True, value="a-value"),
93+
),
94+
],
95+
ids=lambda case: case.label,
96+
)
97+
def test_state_survives_serialization(self, case: _StateCase) -> None:
98+
class _Model(BaseRequestModel):
99+
field: str | Sentinel | None = SENTINEL
100+
101+
dumped = _Model(**case.kwargs).model_dump(**_WIRE_DUMP_KWARGS)
102+
assert ("field" in dumped) is case.expected.key_present
103+
if case.expected.key_present:
104+
assert dumped["field"] == case.expected.value
105+
106+
def test_an_int_field_is_not_set_to_the_sentinels_enum_value(self) -> None:
107+
"""Why an assigned SENTINEL must not be serialized.
108+
109+
``Sentinel.TOKEN`` is ``enum.auto()``, so ``mode="json"`` would render it as
110+
``1``; an int-typed field re-parses that as a value and the column would be set
111+
to 1 instead of left alone.
112+
"""
113+
114+
class _Model(BaseRequestModel):
115+
count: int | Sentinel | None = SENTINEL
116+
117+
dumped = _Model(count=SENTINEL).model_dump(**_WIRE_DUMP_KWARGS)
118+
assert "count" not in dumped
119+
assert _Model.model_validate(dumped).count is SENTINEL
120+
121+
def test_a_nested_model_drops_its_own_sentinels(self) -> None:
122+
class _Nested(BaseRequestModel):
123+
inner: str | Sentinel | None = SENTINEL
124+
125+
class _Outer(BaseRequestModel):
126+
nested: _Nested
127+
outer: str | Sentinel | None = SENTINEL
128+
129+
dumped = _Outer(nested=_Nested(inner=SENTINEL), outer=SENTINEL).model_dump(
130+
**_WIRE_DUMP_KWARGS
131+
)
132+
assert dumped == {"nested": {}}

0 commit comments

Comments
 (0)