Skip to content

Commit 7065c38

Browse files
pcarletonclaude
andcommitted
refactor: revise discovery order for consistency
Changes discovery sequence to be more logical and consistent: 1. Path-aware OAuth discovery 2. Root OAuth discovery (if path exists) 3. OIDC discovery methods This ensures OAuth methods are exhausted before trying OIDC, providing more predictable fallback behavior. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent 4f6b5ad commit 7065c38

File tree

2 files changed

+76
-39
lines changed

2 files changed

+76
-39
lines changed

src/client/auth.test.ts

Lines changed: 65 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -727,7 +727,7 @@ describe("OAuth Authorization", () => {
727727
});
728728
});
729729

730-
it("falls back to OpenID Connect discovery when OAuth discovery fails", async () => {
730+
it("falls back to OpenID Connect discovery when OAuth discovery fails (no path component)", async () => {
731731
// First call (OAuth) returns 404
732732
mockFetch.mockResolvedValueOnce({
733733
ok: false,
@@ -781,43 +781,64 @@ describe("OAuth Authorization", () => {
781781
status: 404,
782782
});
783783

784-
// Second call (OIDC with path insertion) returns 404
784+
// Second call should be OAuth root fallback
785+
mockFetch.mockResolvedValueOnce({
786+
ok: true,
787+
status: 200,
788+
json: async () => validOAuthMetadata,
789+
});
790+
791+
const metadata = await discoverAuthorizationServerMetadata(
792+
"https://mcp.example.com",
793+
"https://auth.example.com/tenant1"
794+
);
795+
796+
expect(metadata).toEqual(validOAuthMetadata);
797+
const calls = mockFetch.mock.calls;
798+
expect(calls.length).toBe(2);
799+
800+
// Should try OAuth with path first
801+
expect(calls[0][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server/tenant1");
802+
803+
// Should fall back to OAuth root discovery
804+
expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server");
805+
});
806+
807+
it("should try OIDC discovery after OAuth attempts fail", async () => {
808+
// First call (OAuth with path) returns 404
785809
mockFetch.mockResolvedValueOnce({
786810
ok: false,
787811
status: 404,
788812
});
789813

790-
// Third call (OIDC with path appending) returns 404
814+
// Second call (OAuth root) returns 404
791815
mockFetch.mockResolvedValueOnce({
792816
ok: false,
793817
status: 404,
794818
});
795819

796-
// Fourth call should be OAuth root fallback
820+
// Third call (OIDC with path insertion) succeeds
797821
mockFetch.mockResolvedValueOnce({
798822
ok: true,
799823
status: 200,
800-
json: async () => validOAuthMetadata,
824+
json: async () => validOpenIdMetadata,
801825
});
802826

803827
const metadata = await discoverAuthorizationServerMetadata(
804828
"https://mcp.example.com",
805829
"https://auth.example.com/tenant1"
806830
);
807831

808-
expect(metadata).toEqual(validOAuthMetadata);
832+
expect(metadata).toEqual(validOpenIdMetadata);
809833
const calls = mockFetch.mock.calls;
810-
expect(calls.length).toBe(4);
811-
812-
// Should try OAuth with path first
834+
expect(calls.length).toBe(3);
835+
836+
// Should try OAuth attempts first
813837
expect(calls[0][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server/tenant1");
814-
815-
// Should try OIDC discoveries next
816-
expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/openid-configuration/tenant1");
817-
expect(calls[2][0].toString()).toBe("https://auth.example.com/tenant1/.well-known/openid-configuration");
818-
819-
// Should finally fall back to OAuth root discovery
820-
expect(calls[3][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server");
838+
expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server");
839+
840+
// Then try OIDC discovery
841+
expect(calls[2][0].toString()).toBe("https://auth.example.com/.well-known/openid-configuration/tenant1");
821842
});
822843

823844
it("handles authorization server URL with path in OAuth discovery", async () => {
@@ -840,7 +861,13 @@ describe("OAuth Authorization", () => {
840861
});
841862

842863
it("handles authorization server URL with path in OpenID Connect discovery", async () => {
843-
// OAuth discovery fails
864+
// OAuth discovery with path fails
865+
mockFetch.mockResolvedValueOnce({
866+
ok: false,
867+
status: 404,
868+
});
869+
870+
// OAuth discovery at root fails
844871
mockFetch.mockResolvedValueOnce({
845872
ok: false,
846873
status: 404,
@@ -860,17 +887,26 @@ describe("OAuth Authorization", () => {
860887

861888
expect(metadata).toEqual(validOpenIdMetadata);
862889
const calls = mockFetch.mock.calls;
863-
expect(calls.length).toBe(2);
890+
expect(calls.length).toBe(3);
864891

865892
// First call should be OAuth with path
866893
expect(calls[0][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server/tenant1");
867894

868-
// Second call should be OpenID Connect with path insertion
869-
expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/openid-configuration/tenant1");
895+
// Second call should be OAuth at root
896+
expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server");
897+
898+
// Third call should be OpenID Connect with path insertion
899+
expect(calls[2][0].toString()).toBe("https://auth.example.com/.well-known/openid-configuration/tenant1");
870900
});
871901

872902
it("tries multiple OpenID Connect endpoints when path is present", async () => {
873-
// OAuth discovery fails
903+
// OAuth discovery with path fails
904+
mockFetch.mockResolvedValueOnce({
905+
ok: false,
906+
status: 404,
907+
});
908+
909+
// OAuth discovery at root fails
874910
mockFetch.mockResolvedValueOnce({
875911
ok: false,
876912
status: 404,
@@ -896,16 +932,19 @@ describe("OAuth Authorization", () => {
896932

897933
expect(metadata).toEqual(validOpenIdMetadata);
898934
const calls = mockFetch.mock.calls;
899-
expect(calls.length).toBe(3);
935+
expect(calls.length).toBe(4);
900936

901937
// First call should be OAuth with path
902938
expect(calls[0][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server/tenant1");
903939

904-
// Second call should be OpenID Connect with path insertion
905-
expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/openid-configuration/tenant1");
940+
// Second call should be OAuth at root
941+
expect(calls[1][0].toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server");
942+
943+
// Third call should be OpenID Connect with path insertion
944+
expect(calls[2][0].toString()).toBe("https://auth.example.com/.well-known/openid-configuration/tenant1");
906945

907-
// Third call should be OpenID Connect with path prepending
908-
expect(calls[2][0].toString()).toBe("https://auth.example.com/tenant1/.well-known/openid-configuration");
946+
// Fourth call should be OpenID Connect with path prepending
947+
expect(calls[3][0].toString()).toBe("https://auth.example.com/tenant1/.well-known/openid-configuration");
909948
});
910949

911950
it("throws error when OIDC provider does not support S256 PKCE", async () => {

src/client/auth.ts

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -710,26 +710,24 @@ export async function discoverAuthorizationServerMetadata(
710710
return oauthMetadata;
711711
}
712712

713-
const oidcMetadata = await retrieveOpenIdProviderMetadataFromAuthorizationServer(authorizationServerUrl, {
714-
fetchFn,
715-
protocolVersion,
716-
});
717-
718-
if (oidcMetadata) {
719-
return oidcMetadata;
720-
}
721-
722-
// If both path-aware discoveries failed and the issuer has a path component,
723-
// try OAuth discovery at the root as a final fallback for compatibility
724713
if (hasPath) {
725714
const rootUrl = new URL(url.origin);
726-
return retrieveOAuthMetadataFromAuthorizationServer(rootUrl, {
715+
const rootOauthMetadata = await retrieveOAuthMetadataFromAuthorizationServer(rootUrl, {
727716
fetchFn,
728717
protocolVersion,
729718
});
719+
720+
if (rootOauthMetadata) {
721+
return rootOauthMetadata;
722+
}
730723
}
731724

732-
return undefined;
725+
const oidcMetadata = await retrieveOpenIdProviderMetadataFromAuthorizationServer(authorizationServerUrl, {
726+
fetchFn,
727+
protocolVersion,
728+
});
729+
730+
return oidcMetadata;
733731
}
734732

735733
/**

0 commit comments

Comments
 (0)