diff --git a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py index f9c5db76fa0..2994f98f309 100644 --- a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py +++ b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py @@ -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, diff --git a/litellm/proxy/client/cli/commands/auth.py b/litellm/proxy/client/cli/commands/auth.py index 4eb23a547b6..31583dec978 100644 --- a/litellm/proxy/client/cli/commands/auth.py +++ b/litellm/proxy/client/cli/commands/auth.py @@ -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.") diff --git a/litellm/proxy/client/cli/commands/pkce_login.py b/litellm/proxy/client/cli/commands/pkce_login.py index a0db1ba2db7..93f5cfea21b 100644 --- a/litellm/proxy/client/cli/commands/pkce_login.py +++ b/litellm/proxy/client/cli/commands/pkce_login.py @@ -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 diff --git a/tests/test_litellm/proxy/client/cli/test_auth_commands.py b/tests/test_litellm/proxy/client/cli/test_auth_commands.py index f6123b5c690..c494cf8b45b 100644 --- a/tests/test_litellm/proxy/client/cli/test_auth_commands.py +++ b/tests/test_litellm/proxy/client/cli/test_auth_commands.py @@ -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"}), diff --git a/tests/test_litellm/proxy/client/cli/test_pkce_login.py b/tests/test_litellm/proxy/client/cli/test_pkce_login.py index 712c7e83236..70f481d5cfa 100644 --- a/tests/test_litellm/proxy/client/cli/test_pkce_login.py +++ b/tests/test_litellm/proxy/client/cli/test_pkce_login.py @@ -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"