Skip to content

Commit 0f4b666

Browse files
committed
Refactoring OAuthErrors
This makes it possible to parse them from JSON, using OAUTH_ERRORS Invalidating credentials & retrying when server OAuth errors occur Updated existing tests Added some initial test coverage refactored to avoid recursion as recommended
1 parent 1b14bd7 commit 0f4b666

File tree

6 files changed

+505
-83
lines changed

6 files changed

+505
-83
lines changed

src/client/auth.test.ts

Lines changed: 29 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
auth,
1111
type OAuthClientProvider,
1212
} from "./auth.js";
13+
import {ServerError} from "../server/auth/errors.js";
1314

1415
// Mock fetch globally
1516
const mockFetch = jest.fn();
@@ -427,25 +428,23 @@ describe("OAuth Authorization", () => {
427428
});
428429

429430
it("throws on non-404 errors", async () => {
430-
mockFetch.mockResolvedValueOnce({
431-
ok: false,
432-
status: 500,
433-
});
431+
mockFetch.mockResolvedValueOnce(new Response(null, { status: 500 }));
434432

435433
await expect(
436434
discoverOAuthMetadata("https://auth.example.com")
437435
).rejects.toThrow("HTTP 500");
438436
});
439437

440438
it("validates metadata schema", async () => {
441-
mockFetch.mockResolvedValueOnce({
442-
ok: true,
443-
status: 200,
444-
json: async () => ({
445-
// Missing required fields
446-
issuer: "https://auth.example.com",
447-
}),
448-
});
439+
mockFetch.mockResolvedValueOnce(
440+
Response.json(
441+
{
442+
// Missing required fields
443+
issuer: "https://auth.example.com",
444+
},
445+
{ status: 200 }
446+
)
447+
);
449448

450449
await expect(
451450
discoverOAuthMetadata("https://auth.example.com")
@@ -666,10 +665,12 @@ describe("OAuth Authorization", () => {
666665
});
667666

668667
it("throws on error response", async () => {
669-
mockFetch.mockResolvedValueOnce({
670-
ok: false,
671-
status: 400,
672-
});
668+
mockFetch.mockResolvedValueOnce(
669+
Response.json(
670+
new ServerError("Token exchange failed").toResponseObject(),
671+
{ status: 400 }
672+
)
673+
);
673674

674675
await expect(
675676
exchangeAuthorization("https://auth.example.com", {
@@ -769,10 +770,12 @@ describe("OAuth Authorization", () => {
769770
});
770771

771772
it("throws on error response", async () => {
772-
mockFetch.mockResolvedValueOnce({
773-
ok: false,
774-
status: 400,
775-
});
773+
mockFetch.mockResolvedValueOnce(
774+
Response.json(
775+
new ServerError("Token refresh failed").toResponseObject(),
776+
{ status: 400 }
777+
)
778+
);
776779

777780
await expect(
778781
refreshAuthorization("https://auth.example.com", {
@@ -857,10 +860,12 @@ describe("OAuth Authorization", () => {
857860
});
858861

859862
it("throws on error response", async () => {
860-
mockFetch.mockResolvedValueOnce({
861-
ok: false,
862-
status: 400,
863-
});
863+
mockFetch.mockResolvedValueOnce(
864+
Response.json(
865+
new ServerError("Dynamic client registration failed").toResponseObject(),
866+
{ status: 400 }
867+
)
868+
);
864869

865870
await expect(
866871
registerClient("https://auth.example.com", {

src/client/auth.ts

Lines changed: 90 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,24 @@
11
import pkceChallenge from "pkce-challenge";
22
import { LATEST_PROTOCOL_VERSION } from "../types.js";
3-
import type { OAuthClientMetadata, OAuthClientInformation, OAuthTokens, OAuthMetadata, OAuthClientInformationFull, OAuthProtectedResourceMetadata } from "../shared/auth.js";
3+
import {
4+
OAuthClientMetadata,
5+
OAuthClientInformation,
6+
OAuthTokens,
7+
OAuthMetadata,
8+
OAuthClientInformationFull,
9+
OAuthProtectedResourceMetadata,
10+
OAuthErrorResponseSchema
11+
} from "../shared/auth.js";
412
import { OAuthClientInformationFullSchema, OAuthMetadataSchema, OAuthProtectedResourceMetadataSchema, OAuthTokensSchema } from "../shared/auth.js";
513
import { checkResourceAllowed, resourceUrlFromServerUrl } from "../shared/auth-utils.js";
14+
import {
15+
InvalidClientError,
16+
InvalidGrantError,
17+
OAUTH_ERRORS,
18+
OAuthError,
19+
ServerError,
20+
UnauthorizedClientError
21+
} from "../server/auth/errors.js";
622

723
/**
824
* Implements an end-to-end OAuth client to be used with one MCP server.
@@ -81,6 +97,13 @@ export interface OAuthClientProvider {
8197
* Implementations must verify the returned resource matches the MCP server.
8298
*/
8399
validateResourceURL?(serverUrl: string | URL, resource?: string): Promise<URL | undefined>;
100+
101+
/**
102+
* If implemented, provides a way for the client to invalidate (e.g. delete) the specified
103+
* credentials, in the case where the server has indicated that they are no longer valid.
104+
* This avoids requiring the user to intervene manually.
105+
*/
106+
invalidateCredentials?(scope: 'all' | 'client' | 'tokens' | 'verifier'): void | Promise<void>;
84107
}
85108

86109
export type AuthResult = "AUTHORIZED" | "REDIRECT";
@@ -91,13 +114,65 @@ export class UnauthorizedError extends Error {
91114
}
92115
}
93116

117+
/**
118+
* Parses an OAuth error response from a string or Response object.
119+
*
120+
* If the input is a standard OAuth2.0 error response, it will be parsed according to the spec
121+
* and an instance of the appropriate OAuthError subclass will be returned.
122+
* If parsing fails, it falls back to a generic ServerError that includes
123+
* the response status (if available) and original content.
124+
*
125+
* @param input - A Response object or string containing the error response
126+
* @returns A Promise that resolves to an OAuthError instance
127+
*/
128+
export async function parseErrorResponse(input: Response | string): Promise<OAuthError> {
129+
const statusCode = input instanceof Response ? input.status : undefined;
130+
const body = input instanceof Response ? await input.text() : input;
131+
132+
try {
133+
const result = OAuthErrorResponseSchema.parse(JSON.parse(body));
134+
const { error, error_description, error_uri } = result;
135+
const errorClass = OAUTH_ERRORS[error] || ServerError;
136+
return new errorClass(error_description || '', error_uri);
137+
} catch (error) {
138+
// Not a valid OAuth error response, but try to inform the user of the raw data anyway
139+
const errorMessage = `${statusCode ? `HTTP ${statusCode}: ` : ''}Invalid OAuth error response: ${error}. Raw body: ${body}`;
140+
return new ServerError(errorMessage);
141+
}
142+
}
143+
94144
/**
95145
* Orchestrates the full auth flow with a server.
96146
*
97147
* This can be used as a single entry point for all authorization functionality,
98148
* instead of linking together the other lower-level functions in this module.
99149
*/
100150
export async function auth(
151+
provider: OAuthClientProvider,
152+
options: {
153+
serverUrl: string | URL;
154+
authorizationCode?: string;
155+
scope?: string;
156+
resourceMetadataUrl?: URL }): Promise<AuthResult> {
157+
158+
try {
159+
return await authInternal(provider, options);
160+
} catch (error) {
161+
// Handle recoverable error types by invalidating credentials and retrying
162+
if (error instanceof InvalidClientError || error instanceof UnauthorizedClientError) {
163+
await provider.invalidateCredentials?.('all');
164+
return await authInternal(provider, options);
165+
} else if (error instanceof InvalidGrantError) {
166+
await provider.invalidateCredentials?.('tokens');
167+
return await authInternal(provider, options);
168+
}
169+
170+
// Throw otherwise
171+
throw error
172+
}
173+
}
174+
175+
async function authInternal(
101176
provider: OAuthClientProvider,
102177
{ serverUrl,
103178
authorizationCode,
@@ -157,7 +232,7 @@ export async function auth(
157232
});
158233

159234
await provider.saveTokens(tokens);
160-
return "AUTHORIZED";
235+
return "AUTHORIZED"
161236
}
162237

163238
const tokens = await provider.tokens();
@@ -174,9 +249,15 @@ export async function auth(
174249
});
175250

176251
await provider.saveTokens(newTokens);
177-
return "AUTHORIZED";
178-
} catch {
179-
// Could not refresh OAuth tokens
252+
return "AUTHORIZED"
253+
} catch (error) {
254+
// If this is a ServerError, or an unknown type, log it out and try to continue. Otherwise, escalate so we can fix things and retry.
255+
if (!(error instanceof OAuthError) || error instanceof ServerError) {
256+
// Could not refresh OAuth tokens
257+
} else {
258+
// Refresh failed for another reason, re-throw
259+
throw error;
260+
}
180261
}
181262
}
182263

@@ -194,7 +275,7 @@ export async function auth(
194275

195276
await provider.saveCodeVerifier(codeVerifier);
196277
await provider.redirectToAuthorization(authorizationUrl);
197-
return "REDIRECT";
278+
return "REDIRECT"
198279
}
199280

200281
export async function selectResourceURL(serverUrl: string| URL, provider: OAuthClientProvider, resourceMetadata?: OAuthProtectedResourceMetadata): Promise<URL | undefined> {
@@ -523,7 +604,7 @@ export async function exchangeAuthorization(
523604
});
524605

525606
if (!response.ok) {
526-
throw new Error(`Token exchange failed: HTTP ${response.status}`);
607+
throw await parseErrorResponse(response);
527608
}
528609

529610
return OAuthTokensSchema.parse(await response.json());
@@ -587,7 +668,7 @@ export async function refreshAuthorization(
587668
body: params,
588669
});
589670
if (!response.ok) {
590-
throw new Error(`Token refresh failed: HTTP ${response.status}`);
671+
throw await parseErrorResponse(response);
591672
}
592673

593674
return OAuthTokensSchema.parse({ refresh_token: refreshToken, ...(await response.json()) });
@@ -627,7 +708,7 @@ export async function registerClient(
627708
});
628709

629710
if (!response.ok) {
630-
throw new Error(`Dynamic client registration failed: HTTP ${response.status}`);
711+
throw await parseErrorResponse(response);
631712
}
632713

633714
return OAuthClientInformationFullSchema.parse(await response.json());

0 commit comments

Comments
 (0)