Skip to content

Commit 3f9500f

Browse files
committed
feat: QoL improvements for OAuth Callback
1 parent 4053aa1 commit 3f9500f

File tree

4 files changed

+205
-50
lines changed

4 files changed

+205
-50
lines changed

client/src/App.tsx

Lines changed: 15 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,13 @@ import {
1717
Tool,
1818
LoggingLevel,
1919
} from "@modelcontextprotocol/sdk/types.js";
20-
import React, { Suspense, useEffect, useRef, useState } from "react";
20+
import React, {
21+
Suspense,
22+
useCallback,
23+
useEffect,
24+
useRef,
25+
useState,
26+
} from "react";
2127
import { useConnection } from "./lib/hooks/useConnection";
2228
import { useDraggablePane } from "./lib/hooks/useDraggablePane";
2329
import { StdErrNotification } from "./lib/notificationTypes";
@@ -49,14 +55,10 @@ import {
4955
getMCPProxyAddress,
5056
getMCPServerRequestTimeout,
5157
} from "./utils/configUtils";
52-
import { useToast } from "@/hooks/use-toast";
5358

54-
const params = new URLSearchParams(window.location.search);
5559
const CONFIG_LOCAL_STORAGE_KEY = "inspectorConfig_v1";
5660

5761
const App = () => {
58-
const { toast } = useToast();
59-
// Handle OAuth callback route
6062
const [resources, setResources] = useState<Resource[]>([]);
6163
const [resourceTemplates, setResourceTemplates] = useState<
6264
ResourceTemplate[]
@@ -205,31 +207,15 @@ const App = () => {
205207
localStorage.setItem(CONFIG_LOCAL_STORAGE_KEY, JSON.stringify(config));
206208
}, [config]);
207209

208-
const hasProcessedRef = useRef(false);
209-
// Auto-connect if serverUrl is provided in URL params (e.g. after OAuth callback)
210-
useEffect(() => {
211-
if (hasProcessedRef.current) {
212-
// Only try to connect once
213-
return;
214-
}
215-
const serverUrl = params.get("serverUrl");
216-
if (serverUrl) {
210+
// Auto-connect to previously saved serverURL after OAuth callback
211+
const onOAuthConnect = useCallback(
212+
(serverUrl: string) => {
217213
setSseUrl(serverUrl);
218214
setTransportType("sse");
219-
// Remove serverUrl from URL without reloading the page
220-
const newUrl = new URL(window.location.href);
221-
newUrl.searchParams.delete("serverUrl");
222-
window.history.replaceState({}, "", newUrl.toString());
223-
// Show success toast for OAuth
224-
toast({
225-
title: "Success",
226-
description: "Successfully authenticated with OAuth",
227-
});
228-
hasProcessedRef.current = true;
229-
// Connect to the server
230-
connectMcpServer();
231-
}
232-
}, [connectMcpServer, toast]);
215+
void connectMcpServer();
216+
},
217+
[connectMcpServer],
218+
);
233219

234220
useEffect(() => {
235221
fetch(`${getMCPProxyAddress(config)}/config`)
@@ -453,7 +439,7 @@ const App = () => {
453439
);
454440
return (
455441
<Suspense fallback={<div>Loading...</div>}>
456-
<OAuthCallback />
442+
<OAuthCallback onConnect={onOAuthConnect} />
457443
</Suspense>
458444
);
459445
}

client/src/components/OAuthCallback.tsx

Lines changed: 47 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,18 @@ import { useEffect, useRef } from "react";
22
import { authProvider } from "../lib/auth";
33
import { SESSION_KEYS } from "../lib/constants";
44
import { auth } from "@modelcontextprotocol/sdk/client/auth.js";
5+
import { useToast } from "@/hooks/use-toast.ts";
6+
import {
7+
generateOAuthErrorDescription,
8+
parseOAuthCallbackParams,
9+
} from "@/utils/oauthUtils.ts";
510

6-
const OAuthCallback = () => {
11+
interface OAuthCallbackProps {
12+
onConnect: (serverUrl: string) => void;
13+
}
14+
15+
const OAuthCallback = ({ onConnect }: OAuthCallbackProps) => {
16+
const { toast } = useToast();
717
const hasProcessedRef = useRef(false);
818

919
useEffect(() => {
@@ -14,37 +24,53 @@ const OAuthCallback = () => {
1424
}
1525
hasProcessedRef.current = true;
1626

17-
const params = new URLSearchParams(window.location.search);
18-
const code = params.get("code");
19-
const serverUrl = sessionStorage.getItem(SESSION_KEYS.SERVER_URL);
27+
const notifyError = (description: string) =>
28+
void toast({
29+
title: "OAuth Authorization Error",
30+
description,
31+
variant: "destructive",
32+
});
2033

21-
if (!code || !serverUrl) {
22-
console.error("Missing code or server URL");
23-
window.location.href = "/";
24-
return;
34+
const params = parseOAuthCallbackParams(window.location.search);
35+
if (!params.successful) {
36+
return notifyError(generateOAuthErrorDescription(params));
37+
}
38+
39+
const serverUrl = sessionStorage.getItem(SESSION_KEYS.SERVER_URL);
40+
if (!serverUrl) {
41+
return notifyError("Missing Server URL");
2542
}
2643

44+
let result;
2745
try {
28-
const result = await auth(authProvider, {
46+
result = await auth(authProvider, {
2947
serverUrl,
30-
authorizationCode: code,
48+
authorizationCode: params.code,
3149
});
32-
if (result !== "AUTHORIZED") {
33-
throw new Error(
34-
`Expected to be authorized after providing auth code, got: ${result}`,
35-
);
36-
}
37-
38-
// Redirect back to the main app with server URL to trigger auto-connect
39-
window.location.href = `/?serverUrl=${encodeURIComponent(serverUrl)}`;
4050
} catch (error) {
4151
console.error("OAuth callback error:", error);
42-
window.location.href = "/";
52+
return notifyError(`Unexpected error occurred: ${error}`);
4353
}
54+
55+
if (result !== "AUTHORIZED") {
56+
return notifyError(
57+
`Expected to be authorized after providing auth code, got: ${result}`,
58+
);
59+
}
60+
61+
// Finally, trigger auto-connect
62+
toast({
63+
title: "Success",
64+
description: "Successfully authenticated with OAuth",
65+
variant: "default",
66+
});
67+
onConnect(serverUrl);
4468
};
4569

46-
void handleCallback();
47-
}, []);
70+
handleCallback().finally(() => {
71+
window.history.replaceState({}, document.title, "/");
72+
});
73+
}, [toast, onConnect]);
4874

4975
return (
5076
<div className="flex items-center justify-center h-screen">
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
import {
2+
generateOAuthErrorDescription,
3+
parseOAuthCallbackParams,
4+
} from "@/utils/oauthUtils.ts";
5+
6+
describe("parseOAuthCallbackParams", () => {
7+
it("Returns successful: true and code when present", () => {
8+
expect(parseOAuthCallbackParams("?code=fake-code")).toEqual({
9+
successful: true,
10+
code: "fake-code",
11+
});
12+
});
13+
it("Returns successful: false and error when error is present", () => {
14+
expect(parseOAuthCallbackParams("?error=access_denied")).toEqual({
15+
successful: false,
16+
error: "access_denied",
17+
error_description: null,
18+
error_uri: null,
19+
});
20+
});
21+
it("Returns optional error metadata fields when present", () => {
22+
const search =
23+
"?error=access_denied&" +
24+
"error_description=User%20Denied%20Request&" +
25+
"error_uri=https%3A%2F%2Fexample.com%2Ferror-docs";
26+
expect(parseOAuthCallbackParams(search)).toEqual({
27+
successful: false,
28+
error: "access_denied",
29+
error_description: "User Denied Request",
30+
error_uri: "https://example.com/error-docs",
31+
});
32+
});
33+
it("Returns error when nothing present", () => {
34+
expect(parseOAuthCallbackParams("?")).toEqual({
35+
successful: false,
36+
error: "invalid_request",
37+
error_description: "Missing code or error in response",
38+
error_uri: null,
39+
});
40+
});
41+
});
42+
43+
describe("generateOAuthErrorDescription", () => {
44+
it("When only error is present", () => {
45+
expect(
46+
generateOAuthErrorDescription({
47+
successful: false,
48+
error: "invalid_request",
49+
error_description: null,
50+
error_uri: null,
51+
}),
52+
).toBe("Error: invalid_request.");
53+
});
54+
it("When error description is present", () => {
55+
expect(
56+
generateOAuthErrorDescription({
57+
successful: false,
58+
error: "invalid_request",
59+
error_description: "The request could not be completed as dialed",
60+
error_uri: null,
61+
}),
62+
).toEqual(
63+
"Error: invalid_request.\nDetails: The request could not be completed as dialed.",
64+
);
65+
});
66+
it("When all fields present", () => {
67+
expect(
68+
generateOAuthErrorDescription({
69+
successful: false,
70+
error: "invalid_request",
71+
error_description: "The request could not be completed as dialed",
72+
error_uri: "https://example.com/error-docs",
73+
}),
74+
).toEqual(
75+
"Error: invalid_request.\nDetails: The request could not be completed as dialed.\nMore info: https://example.com/error-docs.",
76+
);
77+
});
78+
});

client/src/utils/oauthUtils.ts

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
// The parsed query parameters returned by the Authorization Server
2+
// representing either a valid authorization_code or an error
3+
// ref: https://datatracker.ietf.org/doc/html/draft-ietf-oauth-v2-1-12#section-4.1.2
4+
type CallbackParams =
5+
| {
6+
successful: true;
7+
// The authorization code is generated by the authorization server.
8+
code: string;
9+
}
10+
| {
11+
successful: false;
12+
// The OAuth 2.1 Error Code.
13+
// Usually one of:
14+
// ```
15+
// invalid_request, unauthorized_client, access_denied, unsupported_response_type,
16+
// invalid_scope, server_error, temporarily_unavailable
17+
// ```
18+
error: string;
19+
// Human-readable ASCII text providing additional information, used to assist the
20+
// developer in understanding the error that occurred.
21+
error_description: string | null;
22+
// A URI identifying a human-readable web page with information about the error,
23+
// used to provide the client developer with additional information about the error.
24+
error_uri: string | null;
25+
};
26+
27+
export const parseOAuthCallbackParams = (location: string): CallbackParams => {
28+
const params = new URLSearchParams(location);
29+
30+
const code = params.get("code");
31+
if (code) {
32+
return { successful: true, code };
33+
}
34+
35+
const error = params.get("error");
36+
const error_description = params.get("error_description");
37+
const error_uri = params.get("error_uri");
38+
39+
if (error) {
40+
return { successful: false, error, error_description, error_uri };
41+
}
42+
43+
return {
44+
successful: false,
45+
error: "invalid_request",
46+
error_description: "Missing code or error in response",
47+
error_uri: null,
48+
};
49+
};
50+
51+
export const generateOAuthErrorDescription = (
52+
params: Extract<CallbackParams, { successful: false }>,
53+
): string => {
54+
const error = params.error;
55+
const errorDescription = params.error_description;
56+
const errorUri = params.error_uri;
57+
58+
return [
59+
`Error: ${error}.`,
60+
errorDescription ? `Details: ${errorDescription}.` : "",
61+
errorUri ? `More info: ${errorUri}.` : "",
62+
]
63+
.filter(Boolean)
64+
.join("\n");
65+
};

0 commit comments

Comments
 (0)