From 71a9fcb56a0da32fc62dc5c2a9ccdee4cd27b6ef Mon Sep 17 00:00:00 2001 From: Mateo Wang <277851410+mateo-berri@users.noreply.github.com> Date: Tue, 10 Nov 2026 02:54:15 +0000 Subject: [PATCH] fix(sso): read the SSO client secret before the refresh-token DB check (#50773) * fix(sso): read the SSO client secret before the refresh-token DB check so a concurrent env sync cannot split the client id from its secret Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(sso): type the secret-race test and read the provider from the IdP boundary mock Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- litellm/proxy/management_endpoints/ui_sso.py | 15 +++--- .../proxy/management_endpoints/test_ui_sso.py | 54 +++++++++++++++++++ 2 files changed, 63 insertions(+), 6 deletions(-) diff --git a/litellm/proxy/management_endpoints/ui_sso.py b/litellm/proxy/management_endpoints/ui_sso.py index 80df6a95cc2..33e24efb77b 100644 --- a/litellm/proxy/management_endpoints/ui_sso.py +++ b/litellm/proxy/management_endpoints/ui_sso.py @@ -3330,7 +3330,6 @@ class SSOAuthenticationHandler: Returns: RedirectResponse: The redirect response from the SSO provider. """ - asks_for_refresh_token: Final = await login_asks_for_refresh_token(retention_enabled) # Google SSO Auth if google_client_id is not None: google_client_secret: Final = os.getenv("GOOGLE_CLIENT_SECRET", None) @@ -3351,11 +3350,11 @@ class SSOAuthenticationHandler: redirect_url, google_client_id, ) + google_params: Final = ( + dict(GOOGLE_REFRESH_TOKEN_PARAMS) if await login_asks_for_refresh_token(retention_enabled) else None + ) with google_sso: - return await google_sso.get_login_redirect( - state=state, - params=dict(GOOGLE_REFRESH_TOKEN_PARAMS) if asks_for_refresh_token else None, - ) + return await google_sso.get_login_redirect(state=state, params=google_params) # Microsoft SSO Auth elif microsoft_client_id is not None: microsoft_client_secret: Final = os.getenv("MICROSOFT_CLIENT_SECRET", None) @@ -3373,7 +3372,11 @@ class SSOAuthenticationHandler: tenant=microsoft_tenant, redirect_uri=redirect_url, allow_insecure_http=True, - scope=[*CustomMicrosoftSSO.scope, MICROSOFT_REFRESH_TOKEN_SCOPE] if asks_for_refresh_token else None, + scope=( + [*CustomMicrosoftSSO.scope, MICROSOFT_REFRESH_TOKEN_SCOPE] + if await login_asks_for_refresh_token(retention_enabled) + else None + ), ) with microsoft_sso: return await microsoft_sso.get_login_redirect(state=state) diff --git a/tests/unit/proxy/management_endpoints/test_ui_sso.py b/tests/unit/proxy/management_endpoints/test_ui_sso.py index 72416908c8d..34a3298001b 100644 --- a/tests/unit/proxy/management_endpoints/test_ui_sso.py +++ b/tests/unit/proxy/management_endpoints/test_ui_sso.py @@ -10213,6 +10213,60 @@ async def test_microsoft_login_asks_for_offline_access_only_while_an_id_jag_serv assert ("offline_access" in seen["scope"]) is asks_offline_access, seen["scope"] +@pytest.mark.asyncio +@pytest.mark.parametrize( + "client_id_kwarg, environment, upstream_redirect", + [ + ( + "google_client_id", + {"GOOGLE_CLIENT_ID": "google-cid", "GOOGLE_CLIENT_SECRET": "google-secret"}, + "fastapi_sso.sso.google.GoogleSSO.get_login_redirect", + ), + ( + "microsoft_client_id", + {"MICROSOFT_CLIENT_ID": "ms-cid", "MICROSOFT_CLIENT_SECRET": "ms-secret", "MICROSOFT_TENANT": "tenant"}, + "fastapi_sso.sso.microsoft.MicrosoftSSO.get_login_redirect", + ), + ], +) +async def test_login_redirect_keeps_the_client_secret_read_with_its_client_id_when_a_sync_removes_both( + client_id_kwarg: str, environment: Mapping[str, str], upstream_redirect: str +) -> None: + """A config sync can remove an SSO client's environment variables while a sign-in that already read + the client id is waiting on the refresh-token check. The redirect must use the secret that belonged + to that client id instead of failing because the secret is gone by then.""" + from fastapi.responses import RedirectResponse + + from litellm.proxy.management_endpoints.ui_sso import SSOAuthenticationHandler + + client_id: Final = next(value for key, value in environment.items() if key.endswith("_CLIENT_ID")) + client_secret: Final = next(value for key, value in environment.items() if key.endswith("_SECRET")) + + async def sync_removes_the_client() -> bool: + for key in environment: + os.environ.pop(key, None) + return False + + with ( + patch.dict(os.environ, environment), + patch( # test-quality-ok: stubs the upstream IdP authorization boundary, where the request leaves + upstream_redirect, + autospec=True, + return_value=RedirectResponse(url="https://idp.example/authorize"), + ) as idp_redirect, + ): + response: Final = await SSOAuthenticationHandler.get_sso_login_redirect( + redirect_url="http://localhost:4000/sso/callback", + retention_enabled=sync_removes_the_client, + **{client_id_kwarg: client_id}, + ) + + assert response is not None + idp_redirect.assert_awaited_once() + provider: Final = idp_redirect.await_args.args[0] + assert (provider.client_id, provider.client_secret) == (client_id, client_secret) + + @pytest.mark.asyncio async def test_cli_completion_persists_assertion_under_db_user_id(): """The CLI funnel persists the captured assertion under the DB-resolved user_id."""