mirror of
https://github.com/BerriAI/litellm.git
synced 2026-08-28 05:25:59 +00:00
fix(cli): keep the login record when the proxy cannot record a logout's revocation
A 503 from POST /revoke means the proxy could not write the single-use record, so clearing the local record left a live refresh token nobody could revoke and a hint to retry with nothing left to retry. lite logout now keeps the record, exits 1, and asks to be run again shortly. A refused or unreachable revocation still clears the record and warns as before, and a re-login that replaces a record keeps its existing warning because the new record already stands
This commit is contained in:
parent
238609ea5d
commit
64e7ab1beb
5 changed files with 76 additions and 13 deletions
|
|
@ -1842,8 +1842,9 @@ async def authorize_complete(
|
|||
|
||||
@router.post("/revoke")
|
||||
async def revoke_endpoint(request: Request, token: str = Form(...), client_id: str = Form(...)) -> Response:
|
||||
"""RFC 7009 revocation for the gateway's refresh tokens (``lite logout``). Always 200
|
||||
for a known client, whatever the token's state; access tokens expire on their own."""
|
||||
"""RFC 7009 revocation for the gateway's refresh tokens (``lite logout``): 200 for a known
|
||||
client whatever the token's state, 503 when the shared single-use record cannot be written;
|
||||
access tokens expire on their own."""
|
||||
from litellm.proxy.proxy_server import ( # noqa: PLC0415 # circular import at module load
|
||||
master_key,
|
||||
user_api_key_cache,
|
||||
|
|
|
|||
|
|
@ -11,7 +11,7 @@ import click
|
|||
import requests
|
||||
from rich.console import Console
|
||||
from rich.table import Table
|
||||
from typing_extensions import NotRequired, ReadOnly, TypedDict
|
||||
from typing_extensions import NotRequired, ReadOnly, TypedDict, assert_never
|
||||
|
||||
from litellm.constants import CLI_JWT_EXPIRATION_HOURS
|
||||
from litellm.litellm_core_utils.cli_token_utils import is_cli_token_fresh
|
||||
|
|
@ -25,6 +25,7 @@ from .claude_settings import (
|
|||
from .pkce_login import (
|
||||
Http,
|
||||
PkceFailure,
|
||||
RevocationUnavailable,
|
||||
fresh_api_key,
|
||||
pkce_token_record,
|
||||
revoke_stored_credential,
|
||||
|
|
@ -805,9 +806,18 @@ def logout():
|
|||
"""Logout and clear stored authentication"""
|
||||
token_data: Final = load_token()
|
||||
revocation: Final = revoke_stored_credential(token_data, requests.Session()) if token_data is not None else None
|
||||
clear_token()
|
||||
if revocation is not None:
|
||||
click.echo(f"Could not revoke the refresh token on the proxy ({revocation.reason}); it expires on its own.")
|
||||
match revocation:
|
||||
case RevocationUnavailable(reason=reason):
|
||||
raise click.ClickException(
|
||||
f"The proxy could not record the revocation ({reason}). Nothing was cleared; run `lite logout` again shortly."
|
||||
)
|
||||
case PkceFailure(reason=reason):
|
||||
clear_token()
|
||||
click.echo(f"Could not revoke the refresh token on the proxy ({reason}); it expires on its own.")
|
||||
case None:
|
||||
clear_token()
|
||||
case _:
|
||||
assert_never(revocation)
|
||||
click.echo("Logged out successfully. Authentication token cleared.")
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -69,6 +69,11 @@ class PkceFailure:
|
|||
reason: str
|
||||
|
||||
|
||||
@dataclass(frozen=True, slots=True)
|
||||
class RevocationUnavailable:
|
||||
reason: str
|
||||
|
||||
|
||||
@dataclass(frozen=True, slots=True)
|
||||
class PkceCredential:
|
||||
access_token: str
|
||||
|
|
@ -396,7 +401,9 @@ def _token_request(
|
|||
)
|
||||
|
||||
|
||||
def revoke_credential(revocation_endpoint: str, client_id: str, refresh_token: str, http: Http) -> PkceFailure | None:
|
||||
def revoke_credential(
|
||||
revocation_endpoint: str, client_id: str, refresh_token: str, http: Http
|
||||
) -> PkceFailure | RevocationUnavailable | None:
|
||||
try:
|
||||
response: Final = http.post(
|
||||
revocation_endpoint,
|
||||
|
|
@ -409,6 +416,8 @@ def revoke_credential(revocation_endpoint: str, client_id: str, refresh_token: s
|
|||
redirected: Final = _refused_redirect("revocation request", response)
|
||||
if redirected is not None:
|
||||
return redirected
|
||||
if response.status_code == 503:
|
||||
return RevocationUnavailable(f"revocation failed with 503: {_error_detail(response)}")
|
||||
if response.status_code != 200:
|
||||
return PkceFailure(f"revocation failed with {response.status_code}: {_error_detail(response)}")
|
||||
return None
|
||||
|
|
@ -554,7 +563,9 @@ def _refresh_inputs(token_data: Mapping[str, object]) -> tuple[str, str, str, st
|
|||
return str(token_endpoint), str(revocation_endpoint), str(resource), str(client_id), str(refresh_token)
|
||||
|
||||
|
||||
def revoke_stored_credential(token_data: Mapping[str, object], http: Http) -> PkceFailure | None:
|
||||
def revoke_stored_credential(
|
||||
token_data: Mapping[str, object], http: Http
|
||||
) -> PkceFailure | RevocationUnavailable | None:
|
||||
refresh_inputs: Final = _refresh_inputs(token_data)
|
||||
if refresh_inputs is None:
|
||||
return None
|
||||
|
|
|
|||
|
|
@ -1429,24 +1429,50 @@ class TestPkceLogoutCommand:
|
|||
)
|
||||
]
|
||||
|
||||
def test_logout_still_clears_when_revocation_fails(self):
|
||||
class _FailingSession(_FakeSession):
|
||||
def test_logout_still_clears_when_the_proxy_refuses_the_revocation(self):
|
||||
class _RefusingSession(_FakeSession):
|
||||
def __init__(self):
|
||||
super().__init__()
|
||||
self.response = _FakeHttpResponse(503, {"error": "temporarily_unavailable"})
|
||||
self.response = _FakeHttpResponse(401, {"error": "invalid_client"})
|
||||
|
||||
with (
|
||||
patch("litellm.proxy.client.cli.commands.auth.load_token", return_value=_pkce_record()),
|
||||
patch("litellm.proxy.client.cli.commands.auth.clear_token") as clear,
|
||||
patch("litellm.proxy.client.cli.commands.auth.requests.Session", _FailingSession),
|
||||
patch("litellm.proxy.client.cli.commands.auth.requests.Session", _RefusingSession),
|
||||
):
|
||||
result = self.runner.invoke(logout)
|
||||
|
||||
assert result.exit_code == 0
|
||||
assert "Could not revoke the refresh token on the proxy (revocation failed with 503" in result.output
|
||||
assert (
|
||||
"Could not revoke the refresh token on the proxy (revocation failed with 401: invalid_client); "
|
||||
"it expires on its own." in result.output
|
||||
)
|
||||
assert "Logged out successfully" in result.output
|
||||
clear.assert_called_once()
|
||||
|
||||
def test_logout_keeps_the_record_when_the_proxy_cannot_record_the_revocation(self):
|
||||
class _UnavailableSession(_FakeSession):
|
||||
def __init__(self):
|
||||
super().__init__()
|
||||
self.response = _FakeHttpResponse(
|
||||
503, {"error": "temporarily_unavailable", "error_description": "the record is unavailable"}
|
||||
)
|
||||
|
||||
with (
|
||||
patch("litellm.proxy.client.cli.commands.auth.load_token", return_value=_pkce_record()),
|
||||
patch("litellm.proxy.client.cli.commands.auth.clear_token") as clear,
|
||||
patch("litellm.proxy.client.cli.commands.auth.requests.Session", _UnavailableSession),
|
||||
):
|
||||
result = self.runner.invoke(logout)
|
||||
|
||||
assert result.exit_code == 1
|
||||
assert (
|
||||
"Error: The proxy could not record the revocation (revocation failed with 503: the record is unavailable). "
|
||||
"Nothing was cleared; run `lite logout` again shortly." in result.output
|
||||
)
|
||||
assert "Logged out successfully" not in result.output
|
||||
clear.assert_not_called()
|
||||
|
||||
def test_logout_of_a_classic_token_makes_no_request(self):
|
||||
with (
|
||||
patch("litellm.proxy.client.cli.commands.auth.load_token", return_value={"key": "sk-classic"}),
|
||||
|
|
|
|||
|
|
@ -22,6 +22,7 @@ from litellm.proxy.client.cli.commands.pkce_login import (
|
|||
LoopbackServer,
|
||||
PkceCredential,
|
||||
PkceFailure,
|
||||
RevocationUnavailable,
|
||||
_error_detail,
|
||||
authorize_url,
|
||||
discover_cli_auth,
|
||||
|
|
@ -408,6 +409,20 @@ def test_revoke_failures_name_the_cause(response, expected):
|
|||
assert expected in result.reason
|
||||
|
||||
|
||||
def test_revoke_reports_a_503_as_unavailable_so_the_caller_can_retry():
|
||||
response = _FakeResponse(
|
||||
503,
|
||||
{
|
||||
"error": "temporarily_unavailable",
|
||||
"error_description": "the single-use record is unavailable right now; try again shortly",
|
||||
},
|
||||
)
|
||||
result = revoke_credential(f"{BASE}/revoke", "llm_dcrc_abc", "t", _FakeHttp({("POST", f"{BASE}/revoke"): response}))
|
||||
assert result == RevocationUnavailable(
|
||||
"revocation failed with 503: the single-use record is unavailable right now; try again shortly"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("status", [302, 307, 308])
|
||||
def test_posts_to_the_proxy_never_follow_a_redirect(status):
|
||||
elsewhere = "https://evil.example.net/oauth"
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue