diff --git a/src/agents/mcp-oauth.test.ts b/src/agents/mcp-oauth.test.ts index b24b2dc36ed0..aa1e9f74129c 100644 --- a/src/agents/mcp-oauth.test.ts +++ b/src/agents/mcp-oauth.test.ts @@ -169,6 +169,34 @@ describe("MCP OAuth provider", () => { afterEach(() => closeOpenClawStateDatabaseForTest()); + it("reuses a valid stored session without persisting an authorization redirect", async () => { + await withTempHome( + async () => { + const provider = createMcpOAuthClientProvider({ identity: REMOTE_IDENTITY }); + await provider.saveTokens({ + access_token: "stored-access", + refresh_token: "stored-refresh", + token_type: "Bearer", + expires_in: 3600, + }); + const before = readMcpOAuthStore(REMOTE_IDENTITY.storeKey); + authMock.mockImplementationOnce(async (loginProvider) => + (await loginProvider.tokens()) ? "AUTHORIZED" : await persistRedirect(loginProvider), + ); + + await expect( + startMcpOAuthAuthorization(REMOTE_IDENTITY, resolvedOAuthConfig(REMOTE_IDENTITY), {}), + ).resolves.toEqual({ status: "authorized" }); + expect(readMcpOAuthStore(REMOTE_IDENTITY.storeKey)).toEqual(before); + }, + { + prefix: "openclaw-mcp-oauth-existing-session-", + skipSessionCleanup: true, + env: { OPENCLAW_CONFIG_PATH: undefined, OPENCLAW_STATE_DIR: undefined }, + }, + ); + }); + it("preserves insufficient scope and forces the next login through authorization", async () => { await withTempHome( async () => { @@ -231,7 +259,7 @@ describe("MCP OAuth provider", () => { }); await expect( startMcpOAuthAuthorization(REMOTE_IDENTITY, resolvedOAuthConfig(REMOTE_IDENTITY), {}), - ).resolves.toMatchObject({ state: "state-1234567890" }); + ).resolves.toMatchObject({ status: "redirect", state: "state-1234567890" }); expect(provider.tokens()).toMatchObject({ access_token: "decoy-token" }); authMock.mockImplementationOnce(async (loginProvider) => { @@ -749,6 +777,9 @@ describe("MCP OAuth provider", () => { resolvedOAuthConfig(CALENDLY_IDENTITY), {}, ); + if (session.status !== "redirect") { + throw new Error("expected MCP OAuth redirect"); + } expect(session.redirectUrl).toBe("http://localhost:8989/oauth/callback"); expect(authMock.mock.calls[1]?.[0]?.clientMetadata.redirect_uris).toEqual([ @@ -903,6 +934,9 @@ describe("MCP OAuth provider", () => { }; try { const first = await startMcpOAuthAuthorization(identity, config, {}); + if (first.status !== "redirect") { + throw new Error("expected first MCP OAuth redirect"); + } expect(readMcpOAuthStore(identity.storeKey)).toMatchObject({ codeVerifier: expect.any(String), lastAuthorizationUrl: first.authorizationUrl, @@ -918,6 +952,9 @@ describe("MCP OAuth provider", () => { expect(readMcpOAuthStore(identity.storeKey)).not.toHaveProperty("codeVerifier"); const second = await startMcpOAuthAuthorization(identity, config, {}); + if (second.status !== "redirect") { + throw new Error("expected second MCP OAuth redirect"); + } await expect( completeMcpOAuthAuthorization(identity, config, { code: "wrong-code" }), ).rejects.toThrow(); @@ -928,6 +965,9 @@ describe("MCP OAuth provider", () => { }); const third = await startMcpOAuthAuthorization(identity, config, {}); + if (third.status !== "redirect") { + throw new Error("expected third MCP OAuth redirect"); + } expect(third.authorizationUrl).not.toBe(second.authorizationUrl); await expect( completeMcpOAuthAuthorization(identity, config, { diff --git a/src/agents/mcp-oauth.ts b/src/agents/mcp-oauth.ts index 1c07b489b390..890e2eaba13e 100644 --- a/src/agents/mcp-oauth.ts +++ b/src/agents/mcp-oauth.ts @@ -34,6 +34,10 @@ type ResolvedHttpMcpTransportConfig = Extract< { kind: "http" } >; +type McpOAuthAuthorizationStartResult = + | { status: "authorized" } + | { status: "redirect"; authorizationUrl: string; redirectUrl: string; state: string }; + /** Persisted OAuth credential presence and authorization state for one MCP server. */ export type McpOAuthCredentialsStatus = { hasTokens: boolean; @@ -333,7 +337,7 @@ async function runMcpOAuthAuthorizationAttempt( suppressStoredTokens?: boolean; }, lease: OpenClawStateLeaseContext, -): Promise { +): Promise<"authorized" | "redirect"> { const provider = createMcpOAuthClientProvider({ identity: params.identity, config: params.config, @@ -341,7 +345,7 @@ async function runMcpOAuthAuthorizationAttempt( suppressStoredTokens: params.suppressStoredTokens, lease, }); - await auth(provider, { + const result = await auth(provider, { serverUrl: params.identity.serverUrl, authorizationCode: normalizeOptionalString(params.authorizationCode), resourceMetadataUrl: params.resourceMetadataUrl, @@ -349,13 +353,14 @@ async function runMcpOAuthAuthorizationAttempt( fetchFn: withMcpOAuthLeaseSignal(params.fetchFn, lease.signal), }); lease.assertOwned(); + return result === "AUTHORIZED" ? "authorized" : "redirect"; } export async function startMcpOAuthAuthorization( identity: McpOAuthIdentity, config: ResolvedHttpMcpTransportConfig, opts: { redirectUrl?: string }, -): Promise<{ authorizationUrl: string; redirectUrl: string; state: string }> { +): Promise { const storeKey = identity.storeKey; return await withMcpOAuthLease(storeKey, async (lease) => { const store = readMcpOAuthStore(storeKey); @@ -376,17 +381,18 @@ export async function startMcpOAuthAuthorization( ? new URL(pendingChallenge.resourceMetadataUrl) : undefined, scope: normalizeOptionalString(pendingChallenge?.scope), - suppressStoredTokens: true, + suppressStoredTokens: pendingChallenge?.requiresAuthorization === true, }; + let result: "authorized" | "redirect"; try { - await runMcpOAuthAuthorizationAttempt(attempt, lease); + result = await runMcpOAuthAuthorizationAttempt(attempt, lease); } catch (error) { if ( !normalizeOptionalString(opts.redirectUrl) && !normalizeOptionalString(config.oauth?.redirectUrl) && isMcpOAuthRedirectRegistrationError(error) ) { - await runMcpOAuthAuthorizationAttempt( + result = await runMcpOAuthAuthorizationAttempt( { ...attempt, config: { ...config.oauth, redirectUrl: LOCALHOST_REDIRECT_URL }, @@ -397,13 +403,16 @@ export async function startMcpOAuthAuthorization( throw error; } } + if (result === "authorized") { + return { status: "authorized" }; + } const pending = readMcpOAuthStore(storeKey); const authorizationUrl = pending.lastAuthorizationUrl; const state = authorizationUrl ? new URL(authorizationUrl).searchParams.get("state") : null; if (!authorizationUrl || !pending.codeVerifier || !pending.redirectUrl || !state) { throw new Error("MCP OAuth authorization session was not persisted."); } - return { authorizationUrl, redirectUrl: pending.redirectUrl, state }; + return { status: "redirect", authorizationUrl, redirectUrl: pending.redirectUrl, state }; }); } diff --git a/src/cli/mcp-cli.login-loopback.test.ts b/src/cli/mcp-cli.login-loopback.test.ts index 4fed82dd49bf..c1e3b405c2cd 100644 --- a/src/cli/mcp-cli.login-loopback.test.ts +++ b/src/cli/mcp-cli.login-loopback.test.ts @@ -80,6 +80,7 @@ function mockRedirectFlow(redirectUrl: string): void { authorizationUrl.searchParams.set("redirect_uri", redirectUrl); authorizationUrl.searchParams.set("state", "state-1234567890"); mocks.startMcpOAuthAuthorization.mockResolvedValue({ + status: "redirect", authorizationUrl: authorizationUrl.toString(), redirectUrl, state: "state-1234567890", @@ -140,6 +141,21 @@ describe("mcp login loopback callback", () => { }); }); + it("reports an existing session without starting the loopback", async () => { + await withTempHome("openclaw-cli-mcp-loopback-home-", async () => { + await configureServer(); + mocks.startMcpOAuthAuthorization.mockResolvedValue({ status: "authorized" }); + + await program.parseAsync(["mcp", "login", "docs"], { from: "user" }); + + expect(mocks.runtime.log).toHaveBeenCalledWith('MCP OAuth credentials saved for "docs".'); + expect(mocks.completeMcpOAuthAuthorization).not.toHaveBeenCalled(); + expect( + mocks.runtime.log.mock.calls.some(([line]) => String(line).includes("Open this URL")), + ).toBe(false); + }); + }); + it("falls back immediately to the printed manual command when binding fails", async () => { await withTempHome("openclaw-cli-mcp-loopback-home-", async () => { await configureServer(); diff --git a/src/cli/mcp-cli.ts b/src/cli/mcp-cli.ts index 61d49f042a7b..bca8620d3737 100644 --- a/src/cli/mcp-cli.ts +++ b/src/cli/mcp-cli.ts @@ -1333,6 +1333,10 @@ export function registerMcpCli(program: Command) { const manualCommand = formatCliCommand(`openclaw mcp login ${name} --code `); try { const session = await startMcpOAuthAuthorization(identity, resolved, {}); + if (session.status === "authorized") { + defaultRuntime.log(`MCP OAuth credentials saved for "${name}".`); + return; + } if (session.state.length >= 16) { try { callbackServer = await startOAuthLoopbackCallbackServer({