From 8e6a8b792f160dab7ac0ac1902af0c0bb77adf51 Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Wed, 8 Jul 2026 15:10:22 -0700 Subject: [PATCH] docs(ui): make user OAuth flow's intentional token-drop explicit useUserMcpOAuthFlow's onSuccess is () => void while useToolsOAuthFlow's is (accessToken) => void. The base hook calls onSuccess with the access token, and the user wrapper previously dropped it implicitly via structural typing. Make that explicit (onSuccess: () => onSuccess()) and document why each flow differs: the tools caller holds the token in component state to list tools, whereas the user flow persists it server-side and deliberately exposes none. Adds an assertion that the user flow's onSuccess receives no token. --- .../src/hooks/useMcpOAuthPkceFlow.test.tsx | 2 ++ ui/litellm-dashboard/src/hooks/useToolsOAuthFlow.tsx | 5 +++++ ui/litellm-dashboard/src/hooks/useUserMcpOAuthFlow.tsx | 10 +++++++++- 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/ui/litellm-dashboard/src/hooks/useMcpOAuthPkceFlow.test.tsx b/ui/litellm-dashboard/src/hooks/useMcpOAuthPkceFlow.test.tsx index 5fe550f669f..f5a26b34bf5 100644 --- a/ui/litellm-dashboard/src/hooks/useMcpOAuthPkceFlow.test.tsx +++ b/ui/litellm-dashboard/src/hooks/useMcpOAuthPkceFlow.test.tsx @@ -126,6 +126,8 @@ describe("MCP OAuth PKCE flow wrappers", () => { }); expect(setTokenSpy).not.toHaveBeenCalled(); expect(onSuccess).toHaveBeenCalledTimes(1); + // The user flow keeps the token server-side: onSuccess must receive no token. + expect(onSuccess).toHaveBeenCalledWith(); }); it("does not consume a result written under the tools flow's key", async () => { diff --git a/ui/litellm-dashboard/src/hooks/useToolsOAuthFlow.tsx b/ui/litellm-dashboard/src/hooks/useToolsOAuthFlow.tsx index fcd9ab8a498..4d51b200206 100644 --- a/ui/litellm-dashboard/src/hooks/useToolsOAuthFlow.tsx +++ b/ui/litellm-dashboard/src/hooks/useToolsOAuthFlow.tsx @@ -31,6 +31,11 @@ interface UseToolsOAuthFlowOptions { userId?: string | null; scopes?: string[]; clientId?: string | null; + /** + * Invoked after the token is stored in sessionStorage. Receives the access + * token because the caller holds it in component state to list tools; contrast + * useUserMcpOAuthFlow, which persists the token server-side and exposes none. + */ onSuccess: (accessToken: string) => void; } diff --git a/ui/litellm-dashboard/src/hooks/useUserMcpOAuthFlow.tsx b/ui/litellm-dashboard/src/hooks/useUserMcpOAuthFlow.tsx index 58fb4955b82..3f89fdb7cc2 100644 --- a/ui/litellm-dashboard/src/hooks/useUserMcpOAuthFlow.tsx +++ b/ui/litellm-dashboard/src/hooks/useUserMcpOAuthFlow.tsx @@ -27,6 +27,12 @@ interface UseUserMcpOAuthFlowOptions { scopes?: string[]; /** Pre-configured client_id if the MCP server record has one. */ clientId?: string | null; + /** + * Invoked after the credential is persisted server-side. Receives no token by + * design: unlike useToolsOAuthFlow, this flow stores the token in the backend + * (storeMCPOAuthUserCredential) rather than handing it to the client, so the + * caller only needs a "done" signal to refresh UI state. + */ onSuccess: () => void; } @@ -73,7 +79,9 @@ export const useUserMcpOAuthFlow = ({ expires_in: token.expires_in, scopes: flowState.scopes, }), - onSuccess, + // Explicitly drop the base hook's access_token argument: this flow keeps the + // token server-side, so the public onSuccess deliberately exposes none. + onSuccess: () => onSuccess(), }; return useMcpOAuthPkceFlow(config); };