Skip to content

Commit a5cab16

Browse files
ochafikclaude
ochafik
and
claude
committed
test: simplify resource matching validation tests to bare minimum
Reduced the RFC 9728 resource matching validation test group to only test the core functionality from commit 5b63dd6: - Test for accepting matching resources - Test for rejecting mismatched resources - Test for rejecting missing resource field - Test for fragment canonicalization during comparison Removed extensive edge case testing to focus on the essential validation logic. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent eec52f5 commit a5cab16

File tree

1 file changed

+229
-12
lines changed

1 file changed

+229
-12
lines changed

‎src/client/auth.test.ts‎

Lines changed: 229 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ describe("OAuth Authorization", () => {
271271
let callCount = 0;
272272

273273
// Mock implementation that changes behavior based on call count
274-
mockFetch.mockImplementation((_url, options) => {
274+
mockFetch.mockImplementation((_url, _options) => {
275275
callCount++;
276276

277277
if (callCount === 1) {
@@ -477,7 +477,7 @@ describe("OAuth Authorization", () => {
477477
let callCount = 0;
478478

479479
// Mock implementation that changes behavior based on call count
480-
mockFetch.mockImplementation((_url, options) => {
480+
mockFetch.mockImplementation((_url, _options) => {
481481
callCount++;
482482

483483
if (callCount === 1) {
@@ -1162,7 +1162,7 @@ describe("OAuth Authorization", () => {
11621162
);
11631163

11641164
const redirectCall = (mockProvider.redirectToAuthorization as jest.Mock).mock.calls[0];
1165-
const authUrl: URL = redirectCall[0];
1165+
const authUrl = redirectCall[0] as URL;
11661166
expect(authUrl.searchParams.get("resource")).toBe("https://api.example.com/mcp-server");
11671167
});
11681168

@@ -1210,7 +1210,7 @@ describe("OAuth Authorization", () => {
12101210
);
12111211

12121212
const redirectCall = (mockProvider.redirectToAuthorization as jest.Mock).mock.calls[0];
1213-
const authUrl: URL = redirectCall[0];
1213+
const authUrl = redirectCall[0] as URL;
12141214
expect(authUrl.searchParams.get("resource")).toBe("https://api.example.com/mcp-server");
12151215
});
12161216

@@ -1374,7 +1374,7 @@ describe("OAuth Authorization", () => {
13741374

13751375
// Verify that resource parameter is always included (derived from serverUrl)
13761376
const redirectCall = (mockProvider.redirectToAuthorization as jest.Mock).mock.calls[0];
1377-
const authUrl: URL = redirectCall[0];
1377+
const authUrl = redirectCall[0] as URL;
13781378
expect(authUrl.searchParams.has("resource")).toBe(true);
13791379
expect(authUrl.searchParams.get("resource")).toBe("https://api.example.com/mcp-server");
13801380
});
@@ -1417,7 +1417,7 @@ describe("OAuth Authorization", () => {
14171417

14181418
// Verify the resource is properly canonicalized (everything after first # removed)
14191419
const redirectCall = (mockProvider.redirectToAuthorization as jest.Mock).mock.calls[0];
1420-
const authUrl: URL = redirectCall[0];
1420+
const authUrl = redirectCall[0] as URL;
14211421
expect(authUrl.searchParams.get("resource")).toBe("https://api.example.com/mcp-server");
14221422
});
14231423

@@ -1519,10 +1519,227 @@ describe("OAuth Authorization", () => {
15191519

15201520
// Verify query parameters are preserved (only fragment is removed)
15211521
const redirectCall = (mockProvider.redirectToAuthorization as jest.Mock).mock.calls[0];
1522-
const authUrl: URL = redirectCall[0];
1522+
const authUrl = redirectCall[0] as URL;
15231523
expect(authUrl.searchParams.get("resource")).toBe("https://api.example.com/mcp-server?param=value&another=test");
15241524
});
15251525

1526+
describe("resource matching validation (RFC 9728)", () => {
1527+
// Setup common mocks for resource validation tests
1528+
beforeEach(() => {
1529+
// Mock provider methods for all resource validation tests
1530+
(mockProvider.clientInformation as jest.Mock).mockResolvedValue({
1531+
client_id: "test-client",
1532+
client_secret: "test-secret",
1533+
});
1534+
(mockProvider.tokens as jest.Mock).mockResolvedValue(undefined);
1535+
(mockProvider.saveCodeVerifier as jest.Mock).mockResolvedValue(undefined);
1536+
(mockProvider.redirectToAuthorization as jest.Mock).mockResolvedValue(undefined);
1537+
});
1538+
1539+
it("accepts when protected resource metadata returns matching resource", async () => {
1540+
// Mock console.warn to verify no warnings are logged
1541+
const originalWarn = console.warn;
1542+
const mockWarn = jest.fn();
1543+
console.warn = mockWarn;
1544+
1545+
mockFetch.mockImplementation((url) => {
1546+
const urlString = url.toString();
1547+
if (urlString.includes("/.well-known/oauth-protected-resource")) {
1548+
// Protected resource metadata returns EXACT match
1549+
return Promise.resolve({
1550+
ok: true,
1551+
status: 200,
1552+
json: async () => ({
1553+
resource: "https://api.example.com/mcp-server",
1554+
authorization_servers: ["https://auth.example.com"],
1555+
}),
1556+
});
1557+
} else if (urlString.includes("/.well-known/oauth-authorization-server")) {
1558+
return Promise.resolve({
1559+
ok: true,
1560+
status: 200,
1561+
json: async () => ({
1562+
issuer: "https://auth.example.com",
1563+
authorization_endpoint: "https://auth.example.com/authorize",
1564+
token_endpoint: "https://auth.example.com/token",
1565+
response_types_supported: ["code"],
1566+
code_challenge_methods_supported: ["S256"],
1567+
}),
1568+
});
1569+
}
1570+
return Promise.resolve({ ok: false, status: 404 });
1571+
});
1572+
1573+
const result = await auth(mockProvider, {
1574+
serverUrl: "https://api.example.com/mcp-server",
1575+
});
1576+
1577+
expect(result).toBe("REDIRECT");
1578+
1579+
// Verify NO fallback warning was logged (resource matched)
1580+
expect(mockWarn).not.toHaveBeenCalled();
1581+
1582+
// Restore console.warn
1583+
console.warn = originalWarn;
1584+
});
1585+
1586+
it("throws error when protected resource metadata returns different resource", async () => {
1587+
// Mock console.warn to verify fallback behavior
1588+
const originalWarn = console.warn;
1589+
const mockWarn = jest.fn();
1590+
console.warn = mockWarn;
1591+
1592+
let callCount = 0;
1593+
mockFetch.mockImplementation((url) => {
1594+
callCount++;
1595+
const urlString = url.toString();
1596+
1597+
if (callCount === 1 && urlString.includes("/.well-known/oauth-protected-resource")) {
1598+
// First call: Protected resource metadata returns WRONG resource
1599+
return Promise.resolve({
1600+
ok: true,
1601+
status: 200,
1602+
json: async () => ({
1603+
resource: "https://WRONG.example.com/different-server", // MISMATCH
1604+
authorization_servers: ["https://malicious.example.com"],
1605+
}),
1606+
});
1607+
} else if (callCount === 2 && urlString.includes("/.well-known/oauth-authorization-server")) {
1608+
// Second call: Traditional discovery method
1609+
return Promise.resolve({
1610+
ok: true,
1611+
status: 200,
1612+
json: async () => ({
1613+
issuer: "https://auth.example.com",
1614+
authorization_endpoint: "https://auth.example.com/authorize",
1615+
token_endpoint: "https://auth.example.com/token",
1616+
response_types_supported: ["code"],
1617+
code_challenge_methods_supported: ["S256"],
1618+
}),
1619+
});
1620+
}
1621+
return Promise.resolve({ ok: false, status: 404 });
1622+
});
1623+
1624+
const result = await auth(mockProvider, {
1625+
serverUrl: "https://api.example.com/mcp-server",
1626+
});
1627+
1628+
expect(result).toBe("REDIRECT");
1629+
expect(mockFetch).toHaveBeenCalledTimes(2);
1630+
1631+
// Verify warning was logged about resource mismatch
1632+
expect(mockWarn).toHaveBeenCalledWith(
1633+
expect.stringContaining("Could not load OAuth Protected Resource metadata"),
1634+
expect.any(Error)
1635+
);
1636+
1637+
// Verify the error message specifically mentions resource mismatch
1638+
const errorArg = mockWarn.mock.calls[0][1];
1639+
expect(errorArg.message).toContain("doesn't match the expected resource");
1640+
1641+
// Restore console.warn
1642+
console.warn = originalWarn;
1643+
});
1644+
1645+
it("throws error when protected resource metadata is missing resource field", async () => {
1646+
// Mock console.warn to verify fallback behavior
1647+
const originalWarn = console.warn;
1648+
const mockWarn = jest.fn();
1649+
console.warn = mockWarn;
1650+
1651+
let callCount = 0;
1652+
mockFetch.mockImplementation((url) => {
1653+
callCount++;
1654+
const urlString = url.toString();
1655+
1656+
if (callCount === 1 && urlString.includes("/.well-known/oauth-protected-resource")) {
1657+
// First call: Protected resource metadata missing resource field
1658+
return Promise.resolve({
1659+
ok: true,
1660+
status: 200,
1661+
json: async () => ({
1662+
// Missing resource field entirely
1663+
authorization_servers: ["https://auth.example.com"],
1664+
}),
1665+
});
1666+
} else if (callCount === 2 && urlString.includes("/.well-known/oauth-authorization-server")) {
1667+
// Second call: Traditional discovery method
1668+
return Promise.resolve({
1669+
ok: true,
1670+
status: 200,
1671+
json: async () => ({
1672+
issuer: "https://auth.example.com",
1673+
authorization_endpoint: "https://auth.example.com/authorize",
1674+
token_endpoint: "https://auth.example.com/token",
1675+
response_types_supported: ["code"],
1676+
code_challenge_methods_supported: ["S256"],
1677+
}),
1678+
});
1679+
}
1680+
return Promise.resolve({ ok: false, status: 404 });
1681+
});
1682+
1683+
const result = await auth(mockProvider, {
1684+
serverUrl: "https://api.example.com/mcp-server",
1685+
});
1686+
1687+
expect(result).toBe("REDIRECT");
1688+
expect(mockFetch).toHaveBeenCalledTimes(2);
1689+
1690+
// Verify warning was logged
1691+
expect(mockWarn).toHaveBeenCalledWith(
1692+
expect.stringContaining("Could not load OAuth Protected Resource metadata"),
1693+
expect.any(Error)
1694+
);
1695+
1696+
// Restore console.warn
1697+
console.warn = originalWarn;
1698+
});
1699+
1700+
it("compares resources after fragment removal", async () => {
1701+
mockFetch.mockImplementation((url) => {
1702+
const urlString = url.toString();
1703+
if (urlString.includes("/.well-known/oauth-protected-resource")) {
1704+
// Server returns canonical resource (no fragment)
1705+
return Promise.resolve({
1706+
ok: true,
1707+
status: 200,
1708+
json: async () => ({
1709+
resource: "https://api.example.com/mcp-server", // No fragment
1710+
authorization_servers: ["https://auth.example.com"],
1711+
}),
1712+
});
1713+
} else if (urlString.includes("/.well-known/oauth-authorization-server")) {
1714+
return Promise.resolve({
1715+
ok: true,
1716+
status: 200,
1717+
json: async () => ({
1718+
issuer: "https://auth.example.com",
1719+
authorization_endpoint: "https://auth.example.com/authorize",
1720+
token_endpoint: "https://auth.example.com/token",
1721+
response_types_supported: ["code"],
1722+
code_challenge_methods_supported: ["S256"],
1723+
}),
1724+
});
1725+
}
1726+
return Promise.resolve({ ok: false, status: 404 });
1727+
});
1728+
1729+
// Client requests with fragment - should be canonicalized
1730+
const result = await auth(mockProvider, {
1731+
serverUrl: "https://api.example.com/mcp-server#some-fragment",
1732+
});
1733+
1734+
expect(result).toBe("REDIRECT");
1735+
1736+
// Verify resource was canonicalized in authorization URL
1737+
const redirectCall = (mockProvider.redirectToAuthorization as jest.Mock).mock.calls[0];
1738+
const authUrl = redirectCall[0] as URL;
1739+
expect(authUrl.searchParams.get("resource")).toBe("https://api.example.com/mcp-server");
1740+
});
1741+
});
1742+
15261743
// Protocol Version Propagation Tests for main auth function
15271744
it("propagates protocolVersion through entire auth flow", async () => {
15281745
const customProtocolVersion = "2024-11-05";
@@ -1826,7 +2043,7 @@ describe("OAuth Authorization", () => {
18262043
const mockWarn = jest.fn();
18272044
console.warn = mockWarn;
18282045

1829-
mockFetch.mockImplementation((url, options) => {
2046+
mockFetch.mockImplementation((url, _options) => {
18302047
const urlString = url.toString();
18312048

18322049
if (urlString.includes("/.well-known/oauth-protected-resource")) {
@@ -2143,7 +2360,7 @@ describe("OAuth Authorization", () => {
21432360

21442361
// Verify final authorization includes scope and resource
21452362
const redirectCall = (mockProvider.redirectToAuthorization as jest.Mock).mock.calls[0];
2146-
const authUrl: URL = redirectCall[0];
2363+
const authUrl = redirectCall[0] as URL;
21472364
expect(authUrl.searchParams.get("resource")).toBe("https://api.example.com/mcp-server");
21482365
expect(authUrl.searchParams.get("scope")).toBe("read write admin");
21492366
});
@@ -2158,7 +2375,7 @@ describe("OAuth Authorization", () => {
21582375
];
21592376

21602377
for (const version of edgeCaseVersions) {
2161-
mockFetch.mockImplementation((url, options) => {
2378+
mockFetch.mockImplementation((url, _options) => {
21622379
const urlString = url.toString();
21632380
if (urlString.includes("/.well-known/oauth-protected-resource")) {
21642381
return Promise.resolve({
@@ -2224,7 +2441,7 @@ describe("OAuth Authorization", () => {
22242441
];
22252442

22262443
for (const customUrl of customUrls) {
2227-
mockFetch.mockImplementation((url, options) => {
2444+
mockFetch.mockImplementation((url, _options) => {
22282445
const urlString = url.toString();
22292446
if (urlString === customUrl) {
22302447
return Promise.resolve({
@@ -2457,7 +2674,7 @@ describe("OAuth Authorization", () => {
24572674
const customProtocolVersion = "2024-11-05";
24582675
let callCount = 0;
24592676

2460-
mockFetch.mockImplementation((_url, options) => {
2677+
mockFetch.mockImplementation((_url, _options) => {
24612678
callCount++;
24622679
const urlString = _url.toString();
24632680

0 commit comments

Comments
 (0)