mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-27 01:22:18 +00:00
feat(oauth): refuse /start when STORE_MODEL_IN_DB is not set
Without STORE_MODEL_IN_DB=True the proxy still writes OAuth credentials
to LiteLLM_CredentialsTable, but ``proxy_config.get_credentials`` — the
DB → ``litellm.credential_list`` reload run at startup — lives inside
``if store_model_in_db is True:`` in proxy_server.py. Result: silent
data loss on restart and request-time ``api_key: oauth:<name>`` failures
because the name is no longer in the in-memory cache.
Gate the /start endpoints (both ChatGPT and Copilot) behind
``get_secret_bool("STORE_MODEL_IN_DB", False)``. Admins get a clear 400
up front instead of sitting through the 15-minute device-code poll only
to discover the flow silently doesn't persist.
Tests: setenv STORE_MODEL_IN_DB=True in the autouse fixture so the
success-path tests don't each have to opt in, plus one new test per
provider asserting 400 + message when the env var is absent. 167 cases
pass locally.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
392400fe28
commit
9167f9725b
4 changed files with 85 additions and 2 deletions
|
|
@ -29,6 +29,7 @@ from pydantic import BaseModel
|
|||
|
||||
from litellm._logging import verbose_proxy_logger
|
||||
from litellm.litellm_core_utils.credential_accessor import CredentialAccessor
|
||||
from litellm.secret_managers.main import get_secret_bool
|
||||
from litellm.llms.chatgpt.authenticator import Authenticator
|
||||
from litellm.llms.chatgpt.common_utils import (
|
||||
CHATGPT_DEVICE_VERIFY_URL,
|
||||
|
|
@ -79,6 +80,32 @@ class RefreshResponse(BaseModel):
|
|||
expires_at: Optional[int] = None
|
||||
|
||||
|
||||
def _require_store_model_in_db() -> None:
|
||||
"""
|
||||
Fail fast if ``STORE_MODEL_IN_DB`` is not set.
|
||||
|
||||
OAuth tokens get encrypted + upserted into ``LiteLLM_CredentialsTable``
|
||||
by this flow, but the proxy only reloads that table into
|
||||
``litellm.credential_list`` on startup when ``STORE_MODEL_IN_DB=True``
|
||||
(see ``proxy_config.get_credentials`` wiring in ``proxy_server.py``).
|
||||
Without the env var the write succeeds but nothing reads it back — the
|
||||
credential appears to vanish on the next restart, and request-time
|
||||
``api_key: oauth:<name>`` resolution fails because the name is no
|
||||
longer in the in-memory cache. Refuse up front so admins don't sit
|
||||
through the 15-minute device-code poll only to hit silent data loss.
|
||||
"""
|
||||
if not get_secret_bool("STORE_MODEL_IN_DB", False):
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail=(
|
||||
"OAuth sign-in requires STORE_MODEL_IN_DB=True so the "
|
||||
"proxy can reload credentials from the database on "
|
||||
"restart. Set the env var (or equivalent in your "
|
||||
"deployment config) and restart before trying again."
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
def _require_admin(user_api_key_dict: UserAPIKeyAuth) -> None:
|
||||
# These endpoints write to LiteLLM_CredentialsTable (start → insert,
|
||||
# refresh → rotate). PROXY_ADMIN_VIEW_ONLY must not reach them.
|
||||
|
|
@ -121,6 +148,7 @@ async def start_oauth(
|
|||
user_api_key_dict: UserAPIKeyAuth = Depends(user_api_key_auth),
|
||||
) -> StartResponse:
|
||||
_require_admin(user_api_key_dict)
|
||||
_require_store_model_in_db()
|
||||
_purge_expired_sessions()
|
||||
|
||||
# Atomically reserve a slot so concurrent callers cannot all pass the
|
||||
|
|
|
|||
|
|
@ -26,6 +26,7 @@ from pydantic import BaseModel
|
|||
|
||||
from litellm._logging import verbose_proxy_logger
|
||||
from litellm.litellm_core_utils.credential_accessor import CredentialAccessor
|
||||
from litellm.secret_managers.main import get_secret_bool
|
||||
from litellm.llms.github_copilot.authenticator import Authenticator
|
||||
from litellm.llms.github_copilot.common_utils import GithubCopilotError
|
||||
from litellm.llms.github_copilot.db_authenticator import (
|
||||
|
|
@ -73,6 +74,26 @@ class RefreshResponse(BaseModel):
|
|||
api_key_expires_at: Optional[int] = None
|
||||
|
||||
|
||||
def _require_store_model_in_db() -> None:
|
||||
"""
|
||||
Fail fast if ``STORE_MODEL_IN_DB`` is not set — see the chatgpt
|
||||
endpoints for the full rationale. Without this env var the OAuth
|
||||
credential write succeeds but the proxy won't reload it on restart,
|
||||
causing silent data loss after the user sits through the device-code
|
||||
poll.
|
||||
"""
|
||||
if not get_secret_bool("STORE_MODEL_IN_DB", False):
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail=(
|
||||
"OAuth sign-in requires STORE_MODEL_IN_DB=True so the "
|
||||
"proxy can reload credentials from the database on "
|
||||
"restart. Set the env var (or equivalent in your "
|
||||
"deployment config) and restart before trying again."
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
def _require_admin(user_api_key_dict: UserAPIKeyAuth) -> None:
|
||||
# These endpoints write to LiteLLM_CredentialsTable (start → insert,
|
||||
# refresh → rotate). PROXY_ADMIN_VIEW_ONLY must not reach them.
|
||||
|
|
@ -115,6 +136,7 @@ async def start_oauth(
|
|||
user_api_key_dict: UserAPIKeyAuth = Depends(user_api_key_auth),
|
||||
) -> StartResponse:
|
||||
_require_admin(user_api_key_dict)
|
||||
_require_store_model_in_db()
|
||||
_purge_expired_sessions()
|
||||
|
||||
# Atomically reserve a slot so concurrent callers cannot all pass the
|
||||
|
|
|
|||
|
|
@ -42,7 +42,11 @@ def _view_only_admin() -> UserAPIKeyAuth:
|
|||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _clear_sessions():
|
||||
def _clear_sessions(monkeypatch):
|
||||
# The /start guard refuses to run without STORE_MODEL_IN_DB — set it
|
||||
# here so the success-path tests in TestStartOAuth don't all have to
|
||||
# opt in individually. The dedicated guard test overrides it.
|
||||
monkeypatch.setenv("STORE_MODEL_IN_DB", "True")
|
||||
with _sessions_lock:
|
||||
_sessions.clear()
|
||||
yield
|
||||
|
|
@ -70,6 +74,23 @@ class TestStartOAuth:
|
|||
)
|
||||
assert exc_info.value.status_code == 403
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_rejects_when_store_model_in_db_unset(self, monkeypatch):
|
||||
"""
|
||||
Without STORE_MODEL_IN_DB the proxy won't reload credentials from
|
||||
LiteLLM_CredentialsTable on restart, so the OAuth write would be
|
||||
silently lost. Refuse up front rather than have the admin sit
|
||||
through the 15-minute device-code poll.
|
||||
"""
|
||||
monkeypatch.delenv("STORE_MODEL_IN_DB", raising=False)
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await start_oauth(
|
||||
StartRequest(credential_name="c"),
|
||||
user_api_key_dict=_admin(),
|
||||
)
|
||||
assert exc_info.value.status_code == 400
|
||||
assert "STORE_MODEL_IN_DB" in exc_info.value.detail
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_rejects_when_session_cap_reached(self):
|
||||
from litellm.proxy.chatgpt_oauth_endpoints.endpoints import SESSIONS_MAX_SIZE
|
||||
|
|
|
|||
|
|
@ -40,7 +40,8 @@ def _view_only_admin() -> UserAPIKeyAuth:
|
|||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _clear_sessions():
|
||||
def _clear_sessions(monkeypatch):
|
||||
monkeypatch.setenv("STORE_MODEL_IN_DB", "True")
|
||||
with _sessions_lock:
|
||||
_sessions.clear()
|
||||
DBAuthenticator._api_key_cache.clear()
|
||||
|
|
@ -68,6 +69,17 @@ class TestStartOAuth:
|
|||
)
|
||||
assert exc_info.value.status_code == 403
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_rejects_when_store_model_in_db_unset(self, monkeypatch):
|
||||
monkeypatch.delenv("STORE_MODEL_IN_DB", raising=False)
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await start_oauth(
|
||||
StartRequest(credential_name="c"),
|
||||
user_api_key_dict=_admin(),
|
||||
)
|
||||
assert exc_info.value.status_code == 400
|
||||
assert "STORE_MODEL_IN_DB" in exc_info.value.detail
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_rejects_when_session_cap_reached(self):
|
||||
from litellm.proxy.copilot_oauth_endpoints.endpoints import SESSIONS_MAX_SIZE
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue