mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-24 00:52:24 +00:00
fix(chatgpt): DBAuthenticator must never fall through to _login_device_code
Reported symptom: Test Model fails immediately with GetLLMProvider Exception - Failed to request device code: Client error '403 Forbidden' for url 'https://auth.openai.com/api/accounts/deviceauth/usercode' No 15-minute hang this time — it comes back instantly. Root cause: the ChatGPT ``DBAuthenticator`` inherited ``get_access_token`` from the filesystem ``Authenticator``, whose last line is: tokens = self._login_device_code() That's the right fall-through for the CLI (no tokens on disk → start a new login). In the proxy's DB-backed context it is catastrophic: the server tries to initiate an unattended device-code request, OpenAI returns 403 on the ``usercode`` endpoint (no browser to walk through consent), and the admin sees the cryptic error above. Copilot's ``DBAuthenticator`` already overrides ``get_access_token`` and raises ``GetAccessTokenError`` cleanly — this commit mirrors that in the ChatGPT side: - If no credential is loaded in ``litellm.credential_list``, raise a 401 pointing the admin at STORE_MODEL_IN_DB and the UI sign-in. - If the token is expired and refresh fails, raise a 401 quoting the IdP's refresh error. - Never call ``_login_device_code`` from the proxy path. Added regression tests that mock ``_login_device_code`` and assert it is never called across the three failure modes above (missing credential, happy path, refresh-error). 173 tests pass; Black + Ruff clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
cef21a087d
commit
503df1478e
2 changed files with 139 additions and 0 deletions
|
|
@ -18,6 +18,7 @@ from litellm.litellm_core_utils.credential_accessor import CredentialAccessor
|
|||
from litellm.types.utils import CredentialItem
|
||||
|
||||
from .authenticator import Authenticator
|
||||
from .common_utils import GetAccessTokenError, RefreshAccessTokenError
|
||||
|
||||
CREDENTIAL_TYPE = "chatgpt_oauth"
|
||||
|
||||
|
|
@ -54,6 +55,61 @@ class DBAuthenticator(Authenticator):
|
|||
return None
|
||||
return _unpack_auth_record(values)
|
||||
|
||||
def get_access_token(self) -> str:
|
||||
"""
|
||||
DB-backed token lookup with **no** fall-through to a server-side
|
||||
device-code login. The parent ``Authenticator.get_access_token``
|
||||
calls ``_login_device_code()`` as a last resort — that's the right
|
||||
behaviour for the CLI but catastrophic in the proxy: it hits
|
||||
OpenAI's ``/api/accounts/deviceauth/usercode`` unattended, gets
|
||||
an instant 403, and the admin sees a cryptic ``GetLLMProvider``
|
||||
error on Test Model. Instead, raise a clear
|
||||
:class:`GetAccessTokenError` pointing the admin back to the UI
|
||||
sign-in flow.
|
||||
"""
|
||||
auth_data = self._read_auth_file()
|
||||
if not auth_data:
|
||||
raise GetAccessTokenError(
|
||||
message=(
|
||||
f"No ChatGPT OAuth credential named "
|
||||
f"'{self.credential_name}' is loaded in the proxy's "
|
||||
"credential cache. Verify STORE_MODEL_IN_DB=True is "
|
||||
"set (so credentials survive restarts), and that the "
|
||||
"model's ``api_key`` matches the stored credential "
|
||||
"name — or re-run the Sign in with ChatGPT flow in "
|
||||
"the UI."
|
||||
),
|
||||
status_code=401,
|
||||
)
|
||||
|
||||
access_token = auth_data.get("access_token")
|
||||
if access_token and not self._is_token_expired(auth_data, access_token):
|
||||
return access_token
|
||||
|
||||
refresh_token = auth_data.get("refresh_token")
|
||||
if refresh_token:
|
||||
try:
|
||||
refreshed = self._refresh_tokens(refresh_token)
|
||||
return refreshed["access_token"]
|
||||
except RefreshAccessTokenError as exc:
|
||||
raise GetAccessTokenError(
|
||||
message=(
|
||||
f"ChatGPT OAuth credential '{self.credential_name}' "
|
||||
f"is expired and refresh failed: {exc.message}. "
|
||||
"Re-run Sign in with ChatGPT in the UI."
|
||||
),
|
||||
status_code=401,
|
||||
)
|
||||
|
||||
raise GetAccessTokenError(
|
||||
message=(
|
||||
f"ChatGPT OAuth credential '{self.credential_name}' has no "
|
||||
"valid access_token and no refresh_token. Re-run Sign in "
|
||||
"with ChatGPT in the UI."
|
||||
),
|
||||
status_code=401,
|
||||
)
|
||||
|
||||
def _write_auth_file(self, data: Dict[str, Any]) -> None:
|
||||
credential_values = _pack_auth_record(data)
|
||||
item = CredentialItem(
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import json
|
||||
import time
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
|
@ -197,6 +198,88 @@ class _AsyncNone:
|
|||
return _coro().__await__()
|
||||
|
||||
|
||||
class TestGetAccessTokenNeverFallsThroughToDeviceCode:
|
||||
"""
|
||||
Regression: the parent ``Authenticator.get_access_token`` calls
|
||||
``_login_device_code`` as a last resort. That's the right fall-through
|
||||
for the CLI but **not** for the DB-backed proxy path: it would hit
|
||||
OpenAI's device-auth endpoint unattended and get an instant 403.
|
||||
|
||||
``DBAuthenticator`` overrides the method so every failure raises a
|
||||
clear :class:`GetAccessTokenError` instead of attempting a login.
|
||||
"""
|
||||
|
||||
def test_raises_when_no_credential_is_loaded(self):
|
||||
from litellm.llms.chatgpt.common_utils import GetAccessTokenError
|
||||
from litellm.llms.chatgpt.db_authenticator import DBAuthenticator
|
||||
|
||||
auth = DBAuthenticator(credential_name="not-there")
|
||||
with patch.object(DBAuthenticator, "_login_device_code") as login_device_code:
|
||||
with pytest.raises(GetAccessTokenError) as exc_info:
|
||||
auth.get_access_token()
|
||||
login_device_code.assert_not_called()
|
||||
assert "not-there" in exc_info.value.message
|
||||
|
||||
def test_returns_stored_token_when_not_expired(self):
|
||||
from litellm.llms.chatgpt.db_authenticator import (
|
||||
CREDENTIAL_TYPE,
|
||||
DBAuthenticator,
|
||||
)
|
||||
|
||||
future = int(time.time()) + 3600
|
||||
litellm.credential_list = [
|
||||
CredentialItem(
|
||||
credential_name="test",
|
||||
credential_values={
|
||||
"access_token": "live-token",
|
||||
"refresh_token": "r",
|
||||
"expires_at": str(future),
|
||||
},
|
||||
credential_info={"type": CREDENTIAL_TYPE},
|
||||
)
|
||||
]
|
||||
auth = DBAuthenticator(credential_name="test")
|
||||
assert auth.get_access_token() == "live-token"
|
||||
|
||||
def test_refresh_path_raises_clean_error_on_failure(self):
|
||||
from litellm.llms.chatgpt.common_utils import (
|
||||
GetAccessTokenError,
|
||||
RefreshAccessTokenError,
|
||||
)
|
||||
from litellm.llms.chatgpt.db_authenticator import (
|
||||
CREDENTIAL_TYPE,
|
||||
DBAuthenticator,
|
||||
)
|
||||
|
||||
past = int(time.time()) - 10
|
||||
litellm.credential_list = [
|
||||
CredentialItem(
|
||||
credential_name="test",
|
||||
credential_values={
|
||||
"access_token": "expired",
|
||||
"refresh_token": "r1",
|
||||
"expires_at": str(past),
|
||||
},
|
||||
credential_info={"type": CREDENTIAL_TYPE},
|
||||
)
|
||||
]
|
||||
auth = DBAuthenticator(credential_name="test")
|
||||
with (
|
||||
patch.object(
|
||||
DBAuthenticator,
|
||||
"_refresh_tokens",
|
||||
side_effect=RefreshAccessTokenError(
|
||||
status_code=400, message="refresh token revoked"
|
||||
),
|
||||
),
|
||||
patch.object(DBAuthenticator, "_login_device_code") as login_device_code,
|
||||
):
|
||||
with pytest.raises(GetAccessTokenError) as exc_info:
|
||||
auth.get_access_token()
|
||||
login_device_code.assert_not_called()
|
||||
assert "refresh token revoked" in exc_info.value.message
|
||||
|
||||
|
||||
class TestPersistScheduling:
|
||||
def test_schedule_starts_background_thread(self, monkeypatch):
|
||||
from litellm.llms.chatgpt import db_authenticator as mod
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue