Skip to content

Only use path-based discovery URLs from the authorization server to discover metadata #1070

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 3, 2025
+11 −21
Merged

Only use path-based discovery URLs from the authorization server to discover metadata #1070

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
71b5460
Use path-based discovery URLs before attempting to fall back on the r…
roadmapper Nov 3, 2025
994a8e3
Remove fallback URL for discovery
roadmapper Nov 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 8 additions & 12 deletions src/client/auth.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -712,16 +712,12 @@ describe('OAuth Authorization', () => {
it('generates correct URLs for server with path', () => {
const urls = buildDiscoveryUrls('https://auth.example.com/tenant1');

expect(urls).toHaveLength(4);
expect(urls).toHaveLength(3);
expect(urls.map(u => ({ url: u.url.toString(), type: u.type }))).toEqual([
{
url: 'https://auth.example.com/.well-known/oauth-authorization-server/tenant1',
type: 'oauth'
},
{
url: 'https://auth.example.com/.well-known/oauth-authorization-server',
type: 'oauth'
},
{
url: 'https://auth.example.com/.well-known/openid-configuration/tenant1',
type: 'oidc'
Expand All @@ -736,7 +732,7 @@ describe('OAuth Authorization', () => {
it('handles URL object input', () => {
const urls = buildDiscoveryUrls(new URL('https://auth.example.com/tenant1'));

expect(urls).toHaveLength(4);
expect(urls).toHaveLength(3);
expect(urls[0].url.toString()).toBe('https://auth.example.com/.well-known/oauth-authorization-server/tenant1');
});
});
Expand All @@ -763,28 +759,28 @@ describe('OAuth Authorization', () => {
};

it('tries URLs in order and returns first successful metadata', async () => {
// First OAuth URL fails with 404
// First OAuth URL (path before well-known) fails with 404
mockFetch.mockResolvedValueOnce({
ok: false,
status: 404
});

// Second OAuth URL (root) succeeds
// Second OIDC URL (path before well-known) succeeds
mockFetch.mockResolvedValueOnce({
ok: true,
status: 200,
json: async () => validOAuthMetadata
json: async () => validOpenIdMetadata
});

const metadata = await discoverAuthorizationServerMetadata('https://auth.example.com/tenant1');

expect(metadata).toEqual(validOAuthMetadata);
expect(metadata).toEqual(validOpenIdMetadata);

// Verify it tried the URLs in the correct order
const calls = mockFetch.mock.calls;
expect(calls.length).toBe(2);
expect(calls[0][0].toString()).toBe('https://auth.example.com/.well-known/oauth-authorization-server/tenant1');
expect(calls[1][0].toString()).toBe('https://auth.example.com/.well-known/oauth-authorization-server');
expect(calls[1][0].toString()).toBe('https://auth.example.com/.well-known/openid-configuration/tenant1');
});

it('continues on 4xx errors', async () => {
Expand Down Expand Up @@ -878,7 +874,7 @@ describe('OAuth Authorization', () => {
expect(metadata).toBeUndefined();

// Verify that all discovery URLs were attempted
expect(mockFetch).toHaveBeenCalledTimes(8); // 4 URLs × 2 attempts each (with and without headers)
expect(mockFetch).toHaveBeenCalledTimes(6); // 3 URLs × 2 attempts each (with and without headers)
});
});

Expand Down
12 changes: 3 additions & 9 deletions src/client/auth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -669,8 +669,7 @@ export async function discoverOAuthMetadata(
* Builds a list of discovery URLs to try for authorization server metadata.
* URLs are returned in priority order:
* 1. OAuth metadata at the given URL
* 2. OAuth metadata at root (if URL has path)
* 3. OIDC metadata endpoints
* 2. OIDC metadata endpoints at the given URL
*/
export function buildDiscoveryUrls(authorizationServerUrl: string | URL): { url: URL; type: 'oauth' | 'oidc' }[] {
const url = typeof authorizationServerUrl === 'string' ? new URL(authorizationServerUrl) : authorizationServerUrl;
Expand Down Expand Up @@ -706,18 +705,13 @@ export function buildDiscoveryUrls(authorizationServerUrl: string | URL): { url:
type: 'oauth'
});

// Root path: https://example.com/.well-known/oauth-authorization-server
Copy link
Member

@pcarleton pcarleton Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left a comment on the issue #1069

Let's change the path-based URL list to just be:

  • /.well-known/oauth-authorization-server${pathname}
  • /.well-known/openid-configuration/${pathname}
  • ${pathname}/.well-known/openid-configuration

And not have the root fallbacks.

urlsToTry.push({
url: new URL('/.well-known/oauth-authorization-server', url.origin),
type: 'oauth'
});

// 3. OIDC metadata endpoints
// 2. OIDC metadata endpoints
// RFC 8414 style: Insert /.well-known/openid-configuration before the path
urlsToTry.push({
url: new URL(`/.well-known/openid-configuration${pathname}`, url.origin),
type: 'oidc'
});

// OIDC Discovery 1.0 style: Append /.well-known/openid-configuration after the path
urlsToTry.push({
url: new URL(`${pathname}/.well-known/openid-configuration`, url.origin),
Expand Down