Skip to content

Commit 69dbfac

Browse files
0Itsuki0itsukipcarleton
authored
Feat: Separate authorization server and resource server on client auth flow (#416)
* add type and schema for protected resource metadata * add private variable to keep track of authorization server url to avoid fetching Protected Resource Metadata every time * change auth flow to use authroization server and add function to discover protected resource metadata * add/modify tests for new added auth functions * modify test to use the authorization server * remove unused variable * add resource parameter when building auth url * Change resource to use the protected resource metadata * resolve typo. Co-authored-by: Paul Carleton <[email protected]> * Update src/client/sse.ts import. Co-authored-by: Paul Carleton <[email protected]> * remove log. Co-authored-by: Paul Carleton <[email protected]> * Update section title. Co-authored-by: Paul Carleton <[email protected]> * remove log. Co-authored-by: Paul Carleton <[email protected]> * remove resource parameter. Co-authored-by: Paul Carleton <[email protected]> * remove resource parameter. Co-authored-by: Paul Carleton <[email protected]> * remove resource parameter. Co-authored-by: Paul Carleton <[email protected]> * remove resource parameter. Co-authored-by: Paul Carleton <[email protected]> * remove resource parameter. Co-authored-by: Paul Carleton <[email protected]> * remove resource parameter. Co-authored-by: Paul Carleton <[email protected]> * remove resource parameter. Co-authored-by: Paul Carleton <[email protected]> * remove resource parameter. Co-authored-by: Paul Carleton <[email protected]> * remove resource parameter. Co-authored-by: Paul Carleton <[email protected]> * make backwards compatibile * s/resourceServerUrl/serverUrl/g for backwards compat * fix test comment --------- Co-authored-by: itsuki <[email protected]> Co-authored-by: Paul Carleton <[email protected]> Co-authored-by: Paul Carleton <[email protected]>
1 parent ac1b80a commit 69dbfac

File tree

6 files changed

+674
-171
lines changed

6 files changed

+674
-171
lines changed

src/client/auth.test.ts

Lines changed: 261 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ import {
44
exchangeAuthorization,
55
refreshAuthorization,
66
registerClient,
7+
discoverOAuthProtectedResourceMetadata,
8+
extractResourceMetadataUrl,
9+
auth,
10+
type OAuthClientProvider,
711
} from "./auth.js";
812

913
// Mock fetch globally
@@ -15,6 +19,165 @@ describe("OAuth Authorization", () => {
1519
mockFetch.mockReset();
1620
});
1721

22+
describe("extractResourceMetadataUrl", () => {
23+
it("returns resource metadata url when present", async () => {
24+
const resourceUrl = "https://resource.example.com/.well-known/oauth-protected-resource"
25+
const mockResponse = {
26+
headers: {
27+
get: jest.fn((name) => name === "WWW-Authenticate" ? `Bearer realm="mcp", resource_metadata="${resourceUrl}"` : null),
28+
}
29+
} as unknown as Response
30+
31+
expect(extractResourceMetadataUrl(mockResponse)).toEqual(new URL(resourceUrl));
32+
});
33+
34+
it("returns undefined if not bearer", async () => {
35+
const resourceUrl = "https://resource.example.com/.well-known/oauth-protected-resource"
36+
const mockResponse = {
37+
headers: {
38+
get: jest.fn((name) => name === "WWW-Authenticate" ? `Basic realm="mcp", resource_metadata="${resourceUrl}"` : null),
39+
}
40+
} as unknown as Response
41+
42+
expect(extractResourceMetadataUrl(mockResponse)).toBeUndefined();
43+
});
44+
45+
it("returns undefined if resource_metadata not present", async () => {
46+
const mockResponse = {
47+
headers: {
48+
get: jest.fn((name) => name === "WWW-Authenticate" ? `Basic realm="mcp"` : null),
49+
}
50+
} as unknown as Response
51+
52+
expect(extractResourceMetadataUrl(mockResponse)).toBeUndefined();
53+
});
54+
55+
it("returns undefined on invalid url", async () => {
56+
const resourceUrl = "invalid-url"
57+
const mockResponse = {
58+
headers: {
59+
get: jest.fn((name) => name === "WWW-Authenticate" ? `Basic realm="mcp", resource_metadata="${resourceUrl}"` : null),
60+
}
61+
} as unknown as Response
62+
63+
expect(extractResourceMetadataUrl(mockResponse)).toBeUndefined();
64+
});
65+
});
66+
67+
describe("discoverOAuthProtectedResourceMetadata", () => {
68+
const validMetadata = {
69+
resource: "https://resource.example.com",
70+
authorization_servers: ["https://auth.example.com"],
71+
};
72+
73+
it("returns metadata when discovery succeeds", async () => {
74+
mockFetch.mockResolvedValueOnce({
75+
ok: true,
76+
status: 200,
77+
json: async () => validMetadata,
78+
});
79+
80+
const metadata = await discoverOAuthProtectedResourceMetadata("https://resource.example.com");
81+
expect(metadata).toEqual(validMetadata);
82+
const calls = mockFetch.mock.calls;
83+
expect(calls.length).toBe(1);
84+
const [url] = calls[0];
85+
expect(url.toString()).toBe("https://resource.example.com/.well-known/oauth-protected-resource");
86+
});
87+
88+
it("returns metadata when first fetch fails but second without MCP header succeeds", async () => {
89+
// Set up a counter to control behavior
90+
let callCount = 0;
91+
92+
// Mock implementation that changes behavior based on call count
93+
mockFetch.mockImplementation((_url, _options) => {
94+
callCount++;
95+
96+
if (callCount === 1) {
97+
// First call with MCP header - fail with TypeError (simulating CORS error)
98+
// We need to use TypeError specifically because that's what the implementation checks for
99+
return Promise.reject(new TypeError("Network error"));
100+
} else {
101+
// Second call without header - succeed
102+
return Promise.resolve({
103+
ok: true,
104+
status: 200,
105+
json: async () => validMetadata
106+
});
107+
}
108+
});
109+
110+
// Should succeed with the second call
111+
const metadata = await discoverOAuthProtectedResourceMetadata("https://resource.example.com");
112+
expect(metadata).toEqual(validMetadata);
113+
114+
// Verify both calls were made
115+
expect(mockFetch).toHaveBeenCalledTimes(2);
116+
117+
// Verify first call had MCP header
118+
expect(mockFetch.mock.calls[0][1]?.headers).toHaveProperty("MCP-Protocol-Version");
119+
});
120+
121+
it("throws an error when all fetch attempts fail", async () => {
122+
// Set up a counter to control behavior
123+
let callCount = 0;
124+
125+
// Mock implementation that changes behavior based on call count
126+
mockFetch.mockImplementation((_url, _options) => {
127+
callCount++;
128+
129+
if (callCount === 1) {
130+
// First call - fail with TypeError
131+
return Promise.reject(new TypeError("First failure"));
132+
} else {
133+
// Second call - fail with different error
134+
return Promise.reject(new Error("Second failure"));
135+
}
136+
});
137+
138+
// Should fail with the second error
139+
await expect(discoverOAuthProtectedResourceMetadata("https://resource.example.com"))
140+
.rejects.toThrow("Second failure");
141+
142+
// Verify both calls were made
143+
expect(mockFetch).toHaveBeenCalledTimes(2);
144+
});
145+
146+
it("throws on 404 errors", async () => {
147+
mockFetch.mockResolvedValueOnce({
148+
ok: false,
149+
status: 404,
150+
});
151+
152+
await expect(discoverOAuthProtectedResourceMetadata("https://resource.example.com"))
153+
.rejects.toThrow("Resource server does not implement OAuth 2.0 Protected Resource Metadata.");
154+
});
155+
156+
it("throws on non-404 errors", async () => {
157+
mockFetch.mockResolvedValueOnce({
158+
ok: false,
159+
status: 500,
160+
});
161+
162+
await expect(discoverOAuthProtectedResourceMetadata("https://resource.example.com"))
163+
.rejects.toThrow("HTTP 500");
164+
});
165+
166+
it("validates metadata schema", async () => {
167+
mockFetch.mockResolvedValueOnce({
168+
ok: true,
169+
status: 200,
170+
json: async () => ({
171+
// Missing required fields
172+
scopes_supported: ["email", "mcp"],
173+
}),
174+
});
175+
176+
await expect(discoverOAuthProtectedResourceMetadata("https://resource.example.com"))
177+
.rejects.toThrow();
178+
});
179+
});
180+
18181
describe("discoverOAuthMetadata", () => {
19182
const validMetadata = {
20183
issuer: "https://auth.example.com",
@@ -158,6 +321,7 @@ describe("OAuth Authorization", () => {
158321
const { authorizationUrl, codeVerifier } = await startAuthorization(
159322
"https://auth.example.com",
160323
{
324+
metadata: undefined,
161325
clientInformation: validClientInfo,
162326
redirectUrl: "http://localhost:3000/callback",
163327
}
@@ -503,4 +667,101 @@ describe("OAuth Authorization", () => {
503667
).rejects.toThrow("Dynamic client registration failed");
504668
});
505669
});
670+
671+
describe("auth function", () => {
672+
const mockProvider: OAuthClientProvider = {
673+
get redirectUrl() { return "http://localhost:3000/callback"; },
674+
get clientMetadata() {
675+
return {
676+
redirect_uris: ["http://localhost:3000/callback"],
677+
client_name: "Test Client",
678+
};
679+
},
680+
clientInformation: jest.fn(),
681+
tokens: jest.fn(),
682+
saveTokens: jest.fn(),
683+
redirectToAuthorization: jest.fn(),
684+
saveCodeVerifier: jest.fn(),
685+
codeVerifier: jest.fn(),
686+
};
687+
688+
beforeEach(() => {
689+
jest.clearAllMocks();
690+
});
691+
692+
it("falls back to /.well-known/oauth-authorization-server when no protected-resource-metadata", async () => {
693+
// Setup: First call to protected resource metadata fails (404)
694+
// Second call to auth server metadata succeeds
695+
let callCount = 0;
696+
mockFetch.mockImplementation((url) => {
697+
callCount++;
698+
699+
const urlString = url.toString();
700+
701+
if (callCount === 1 && urlString.includes("/.well-known/oauth-protected-resource")) {
702+
// First call - protected resource metadata fails with 404
703+
return Promise.resolve({
704+
ok: false,
705+
status: 404,
706+
});
707+
} else if (callCount === 2 && urlString.includes("/.well-known/oauth-authorization-server")) {
708+
// Second call - auth server metadata succeeds
709+
return Promise.resolve({
710+
ok: true,
711+
status: 200,
712+
json: async () => ({
713+
issuer: "https://auth.example.com",
714+
authorization_endpoint: "https://auth.example.com/authorize",
715+
token_endpoint: "https://auth.example.com/token",
716+
registration_endpoint: "https://auth.example.com/register",
717+
response_types_supported: ["code"],
718+
code_challenge_methods_supported: ["S256"],
719+
}),
720+
});
721+
} else if (callCount === 3 && urlString.includes("/register")) {
722+
// Third call - client registration succeeds
723+
return Promise.resolve({
724+
ok: true,
725+
status: 200,
726+
json: async () => ({
727+
client_id: "test-client-id",
728+
client_secret: "test-client-secret",
729+
client_id_issued_at: 1612137600,
730+
client_secret_expires_at: 1612224000,
731+
redirect_uris: ["http://localhost:3000/callback"],
732+
client_name: "Test Client",
733+
}),
734+
});
735+
}
736+
737+
return Promise.reject(new Error(`Unexpected fetch call: ${urlString}`));
738+
});
739+
740+
// Mock provider methods
741+
(mockProvider.clientInformation as jest.Mock).mockResolvedValue(undefined);
742+
(mockProvider.tokens as jest.Mock).mockResolvedValue(undefined);
743+
mockProvider.saveClientInformation = jest.fn();
744+
745+
// Call the auth function
746+
const result = await auth(mockProvider, {
747+
serverUrl: "https://resource.example.com",
748+
});
749+
750+
// Verify the result
751+
expect(result).toBe("REDIRECT");
752+
753+
// Verify the sequence of calls
754+
expect(mockFetch).toHaveBeenCalledTimes(3);
755+
756+
// First call should be to protected resource metadata
757+
expect(mockFetch.mock.calls[0][0].toString()).toBe(
758+
"https://resource.example.com/.well-known/oauth-protected-resource"
759+
);
760+
761+
// Second call should be to oauth metadata
762+
expect(mockFetch.mock.calls[1][0].toString()).toBe(
763+
"https://resource.example.com/.well-known/oauth-authorization-server"
764+
);
765+
});
766+
});
506767
});

0 commit comments

Comments
 (0)