From 815187370549eeb95c4dd2393b027c54a20b7a19 Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Wed, 8 Jul 2026 15:47:34 -0700 Subject: [PATCH] refactor(ui): extract OAuth call-arg objects to named consts, trim test comments Clears the local/no-large-inline-object-arg warnings in the three OAuth hook files by hoisting the register/authorize/exchange/persist payloads into named consts, and drops the now-redundant explanatory comments from the flow test. The metric ratchets from 513 (base) to 509. --- ui/litellm-dashboard/eslint-metrics.json | 2 +- .../src/hooks/useMcpOAuthPkceFlow.test.tsx | 3 --- .../src/hooks/useMcpOAuthPkceFlow.ts | 23 +++++++++++-------- .../src/hooks/useToolsOAuthFlow.tsx | 20 ++++++++-------- .../src/hooks/useUserMcpOAuthFlow.tsx | 8 ++++--- 5 files changed, 28 insertions(+), 28 deletions(-) diff --git a/ui/litellm-dashboard/eslint-metrics.json b/ui/litellm-dashboard/eslint-metrics.json index 6cc64598c0b..7cf578ac1c7 100644 --- a/ui/litellm-dashboard/eslint-metrics.json +++ b/ui/litellm-dashboard/eslint-metrics.json @@ -1,7 +1,7 @@ { "@typescript-eslint/no-explicit-any": 1988, "complexity": 127, - "local/no-large-inline-object-arg": 514, + "local/no-large-inline-object-arg": 509, "local/no-long-condition-chain": 233, "max-depth": 59, "no-console": 15 diff --git a/ui/litellm-dashboard/src/hooks/useMcpOAuthPkceFlow.test.tsx b/ui/litellm-dashboard/src/hooks/useMcpOAuthPkceFlow.test.tsx index f5a26b34bf5..1475f02efbf 100644 --- a/ui/litellm-dashboard/src/hooks/useMcpOAuthPkceFlow.test.tsx +++ b/ui/litellm-dashboard/src/hooks/useMcpOAuthPkceFlow.test.tsx @@ -24,7 +24,6 @@ const TOOLS_RESULT_KEY = "litellm-tools-mcp-oauth-result"; const USER_FLOW_STATE_KEY = "litellm-user-mcp-oauth-flow-state"; const USER_RESULT_KEY = "litellm-user-mcp-oauth-result"; -/** Seed the storage for a completed IdP redirect, keyed for a specific flow. */ function seedCompletedRedirect(flowStateKey: string, resultKey: string, extra: Record = {}) { setSecureItem(resultKey, JSON.stringify({ state: "state-1", code: "code-1" })); setSecureItem( @@ -84,7 +83,6 @@ describe("MCP OAuth PKCE flow wrappers", () => { useToolsOAuthFlow({ accessToken: "user-token", serverId: "server-1", onSuccess }), ); - // Give the on-mount resume a chance to (not) run. await Promise.resolve(); expect(networking.exchangeMcpOAuthToken).not.toHaveBeenCalled(); expect(result.current.status).toBe("idle"); @@ -126,7 +124,6 @@ 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(); }); diff --git a/ui/litellm-dashboard/src/hooks/useMcpOAuthPkceFlow.ts b/ui/litellm-dashboard/src/hooks/useMcpOAuthPkceFlow.ts index 35e21e338c3..367df3b8348 100644 --- a/ui/litellm-dashboard/src/hooks/useMcpOAuthPkceFlow.ts +++ b/ui/litellm-dashboard/src/hooks/useMcpOAuthPkceFlow.ts @@ -102,13 +102,14 @@ export const useMcpOAuthPkceFlow = ({ let clientSecret: string | undefined; if (!clientId) { + const registration = { + client_name: serverAlias || serverId, + grant_types: ["authorization_code", "refresh_token"], + response_types: ["code"], + token_endpoint_auth_method: "none", + }; try { - const reg = await registerMcpOAuthClient(accessToken, serverId, { - client_name: serverAlias || serverId, - grant_types: ["authorization_code", "refresh_token"], - response_types: ["code"], - token_endpoint_auth_method: "none", - }); + const reg = await registerMcpOAuthClient(accessToken, serverId, registration); clientId = reg?.client_id; clientSecret = reg?.client_secret; } catch (_) { @@ -122,14 +123,15 @@ export const useMcpOAuthPkceFlow = ({ const redirectUri = buildCallbackUrl(); const scopeString = scopes?.filter((s) => s.trim()).join(" "); - const authorizeUrl = buildMcpOAuthAuthorizeUrl({ + const authorizeParams = { serverId, clientId, redirectUri, state, codeChallenge: challenge, scope: scopeString, - }); + }; + const authorizeUrl = buildMcpOAuthAuthorizeUrl(authorizeParams); const flowState: McpOAuthFlowState = { state, @@ -202,7 +204,7 @@ export const useMcpOAuthPkceFlow = ({ } setStatus("exchanging"); - const token: McpOAuthTokenResult = await exchangeMcpOAuthToken({ + const exchangeParams = { serverId: flowState.serverId, code: payload.code as string, clientId: flowState.clientId, @@ -210,7 +212,8 @@ export const useMcpOAuthPkceFlow = ({ codeVerifier: flowState.codeVerifier, redirectUri: flowState.redirectUri, accessToken, - }); + }; + const token: McpOAuthTokenResult = await exchangeMcpOAuthToken(exchangeParams); await callbacksRef.current.persistToken(token, flowState); diff --git a/ui/litellm-dashboard/src/hooks/useToolsOAuthFlow.tsx b/ui/litellm-dashboard/src/hooks/useToolsOAuthFlow.tsx index 4d51b200206..282c95e02a1 100644 --- a/ui/litellm-dashboard/src/hooks/useToolsOAuthFlow.tsx +++ b/ui/litellm-dashboard/src/hooks/useToolsOAuthFlow.tsx @@ -70,17 +70,15 @@ export const useToolsOAuthFlow = ({ // Return to the current page (Tools tab) after the OAuth redirect. buildReturnUrl: () => window.location.href, // Store in sessionStorage only — no backend DB write. - persistToken: (token, flowState) => - setToken( - flowState.serverId, - { - access_token: token.access_token, - expires_in: token.expires_in, - refresh_token: token.refresh_token, - token_type: token.token_type, - }, - userId, - ), + persistToken: (token, flowState) => { + const stored = { + access_token: token.access_token, + expires_in: token.expires_in, + refresh_token: token.refresh_token, + token_type: token.token_type, + }; + setToken(flowState.serverId, stored, userId); + }, onSuccess, }; return useMcpOAuthPkceFlow(config); diff --git a/ui/litellm-dashboard/src/hooks/useUserMcpOAuthFlow.tsx b/ui/litellm-dashboard/src/hooks/useUserMcpOAuthFlow.tsx index 3f89fdb7cc2..6e24fc66b86 100644 --- a/ui/litellm-dashboard/src/hooks/useUserMcpOAuthFlow.tsx +++ b/ui/litellm-dashboard/src/hooks/useUserMcpOAuthFlow.tsx @@ -72,13 +72,15 @@ export const useUserMcpOAuthFlow = ({ }, // Persist the token for this user via the backend. // accessToken comes from props — it is never stored in sessionStorage. - persistToken: (token, flowState) => - storeMCPOAuthUserCredential(accessToken, flowState.serverId, { + persistToken: (token, flowState) => { + const credential = { access_token: token.access_token, refresh_token: token.refresh_token, expires_in: token.expires_in, scopes: flowState.scopes, - }), + }; + return storeMCPOAuthUserCredential(accessToken, flowState.serverId, credential); + }, // 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(),