Skip to content

Commit 74d3787

Browse files
committed
fix(auth): preserve resource URI without trailing slash (#1968)
When handling RFC 9728 protected resource metadata, `selectResourceURL` routed the metadata's `resource` value through `new URL(...).href`. For bare-origin URIs that round trip appends a trailing slash: new URL("https://example.com").href === "https://example.com/" The resulting `resource` parameter no longer matches what the server published in PRM, which breaks providers that require an exact match. Microsoft Entra ID rejects the request with AADSTS9010010 when the `resource` parameter does not match the audience of the requested scope. Return the original metadata string verbatim from `selectResourceURL` and serialize it with `String(resource)` instead of `URL.href` in the authorization and token request paths. The validation step still parses the value as a URL via `checkResourceAllowed`. Also adjusted the cached discovery-state test to expect the un-normalized resource value, and added a regression test for the bare-domain case. Fixes #1968
1 parent bf1e022 commit 74d3787

2 files changed

Lines changed: 72 additions & 13 deletions

File tree

src/client/auth.ts

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,7 @@ async function authInternal(
503503
});
504504
}
505505

506-
const resource: URL | undefined = await selectResourceURL(serverUrl, provider, resourceMetadata);
506+
const resource: URL | string | undefined = await selectResourceURL(serverUrl, provider, resourceMetadata);
507507

508508
// Apply scope selection strategy (SEP-835):
509509
// 1. WWW-Authenticate scope (passed via `scope` param)
@@ -633,7 +633,7 @@ export async function selectResourceURL(
633633
serverUrl: string | URL,
634634
provider: OAuthClientProvider,
635635
resourceMetadata?: OAuthProtectedResourceMetadata
636-
): Promise<URL | undefined> {
636+
): Promise<URL | string | undefined> {
637637
const defaultResource = resourceUrlFromServerUrl(serverUrl);
638638

639639
// If provider has custom validation, delegate to it
@@ -650,8 +650,12 @@ export async function selectResourceURL(
650650
if (!checkResourceAllowed({ requestedResource: defaultResource, configuredResource: resourceMetadata.resource })) {
651651
throw new Error(`Protected resource ${resourceMetadata.resource} does not match expected ${defaultResource} (or origin)`);
652652
}
653-
// Prefer the resource from metadata since it's what the server is telling us to request
654-
return new URL(resourceMetadata.resource);
653+
// Prefer the resource from metadata since it's what the server is telling us to request.
654+
// Return the original string verbatim so we don't re-serialize through `URL.href`, which
655+
// appends a trailing slash to bare-origin URIs (e.g. `https://example.com` becomes
656+
// `https://example.com/`) and breaks providers that require an exact match against the
657+
// configured resource indicator (RFC 8707), such as Microsoft Entra ID.
658+
return resourceMetadata.resource;
655659
}
656660

657661
/**
@@ -1126,7 +1130,7 @@ export async function startAuthorization(
11261130
redirectUrl: string | URL;
11271131
scope?: string;
11281132
state?: string;
1129-
resource?: URL;
1133+
resource?: URL | string;
11301134
}
11311135
): Promise<{ authorizationUrl: URL; codeVerifier: string }> {
11321136
let authorizationUrl: URL;
@@ -1174,7 +1178,7 @@ export async function startAuthorization(
11741178
}
11751179

11761180
if (resource) {
1177-
authorizationUrl.searchParams.set('resource', resource.href);
1181+
authorizationUrl.searchParams.set('resource', String(resource));
11781182
}
11791183

11801184
return { authorizationUrl, codeVerifier };
@@ -1222,7 +1226,7 @@ async function executeTokenRequest(
12221226
tokenRequestParams: URLSearchParams;
12231227
clientInformation?: OAuthClientInformationMixed;
12241228
addClientAuthentication?: OAuthClientProvider['addClientAuthentication'];
1225-
resource?: URL;
1229+
resource?: URL | string;
12261230
fetchFn?: FetchLike;
12271231
}
12281232
): Promise<OAuthTokens> {
@@ -1234,7 +1238,7 @@ async function executeTokenRequest(
12341238
});
12351239

12361240
if (resource) {
1237-
tokenRequestParams.set('resource', resource.href);
1241+
tokenRequestParams.set('resource', String(resource));
12381242
}
12391243

12401244
if (addClientAuthentication) {
@@ -1287,7 +1291,7 @@ export async function exchangeAuthorization(
12871291
authorizationCode: string;
12881292
codeVerifier: string;
12891293
redirectUri: string | URL;
1290-
resource?: URL;
1294+
resource?: URL | string;
12911295
addClientAuthentication?: OAuthClientProvider['addClientAuthentication'];
12921296
fetchFn?: FetchLike;
12931297
}
@@ -1329,7 +1333,7 @@ export async function refreshAuthorization(
13291333
metadata?: AuthorizationServerMetadata;
13301334
clientInformation: OAuthClientInformationMixed;
13311335
refreshToken: string;
1332-
resource?: URL;
1336+
resource?: URL | string;
13331337
addClientAuthentication?: OAuthClientProvider['addClientAuthentication'];
13341338
fetchFn?: FetchLike;
13351339
}
@@ -1388,7 +1392,7 @@ export async function fetchToken(
13881392
fetchFn
13891393
}: {
13901394
metadata?: AuthorizationServerMetadata;
1391-
resource?: URL;
1395+
resource?: URL | string;
13921396
/** Authorization code for the default authorization_code grant flow */
13931397
authorizationCode?: string;
13941398
fetchFn?: FetchLike;

test/client/auth.test.ts

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1153,11 +1153,12 @@ describe('OAuth Authorization', () => {
11531153
);
11541154
expect(discoveryCalls).toHaveLength(0);
11551155

1156-
// Verify the token request includes the resource parameter from cached metadata
1156+
// Verify the token request includes the resource parameter from cached metadata,
1157+
// preserved verbatim (no trailing slash added — see #1968).
11571158
const tokenCall = mockFetch.mock.calls.find(call => call[0].toString().includes('/token'));
11581159
expect(tokenCall).toBeDefined();
11591160
const body = tokenCall![1].body as URLSearchParams;
1160-
expect(body.get('resource')).toBe('https://resource.example.com/');
1161+
expect(body.get('resource')).toBe('https://resource.example.com');
11611162
});
11621163

11631164
it('re-saves enriched state when partial cache is supplemented with fetched metadata', async () => {
@@ -2562,6 +2563,60 @@ describe('OAuth Authorization', () => {
25622563
expect(authUrl.searchParams.get('resource')).toBe('https://api.example.com/');
25632564
});
25642565

2566+
it('preserves bare-domain resource URI from PRM without adding a trailing slash', async () => {
2567+
// Regression test for #1968: `new URL("https://example.com").href` adds a trailing
2568+
// slash, which breaks providers (e.g. Microsoft Entra ID) that require an exact
2569+
// match between the OAuth `resource` parameter and the configured resource indicator.
2570+
mockFetch.mockImplementation(url => {
2571+
const urlString = url.toString();
2572+
2573+
if (urlString.includes('/.well-known/oauth-protected-resource')) {
2574+
return Promise.resolve({
2575+
ok: true,
2576+
status: 200,
2577+
json: async () => ({
2578+
// Bare-origin resource URI with no trailing slash
2579+
resource: 'https://api.example.com',
2580+
authorization_servers: ['https://auth.example.com']
2581+
})
2582+
});
2583+
} else if (urlString.includes('/.well-known/oauth-authorization-server')) {
2584+
return Promise.resolve({
2585+
ok: true,
2586+
status: 200,
2587+
json: async () => ({
2588+
issuer: 'https://auth.example.com',
2589+
authorization_endpoint: 'https://auth.example.com/authorize',
2590+
token_endpoint: 'https://auth.example.com/token',
2591+
response_types_supported: ['code'],
2592+
code_challenge_methods_supported: ['S256']
2593+
})
2594+
});
2595+
}
2596+
2597+
return Promise.resolve({ ok: false, status: 404 });
2598+
});
2599+
2600+
(mockProvider.clientInformation as Mock).mockResolvedValue({
2601+
client_id: 'test-client',
2602+
client_secret: 'test-secret'
2603+
});
2604+
(mockProvider.tokens as Mock).mockResolvedValue(undefined);
2605+
(mockProvider.saveCodeVerifier as Mock).mockResolvedValue(undefined);
2606+
(mockProvider.redirectToAuthorization as Mock).mockResolvedValue(undefined);
2607+
2608+
const result = await auth(mockProvider, {
2609+
serverUrl: 'https://api.example.com/mcp-server/endpoint'
2610+
});
2611+
2612+
expect(result).toBe('REDIRECT');
2613+
2614+
const redirectCall = (mockProvider.redirectToAuthorization as Mock).mock.calls[0];
2615+
const authUrl: URL = redirectCall[0];
2616+
// Resource indicator must round-trip exactly as published in PRM (no trailing slash)
2617+
expect(authUrl.searchParams.get('resource')).toBe('https://api.example.com');
2618+
});
2619+
25652620
it('excludes resource parameter when Protected Resource Metadata is not present', async () => {
25662621
// Mock metadata discovery where protected resource metadata is not available (404)
25672622
// but authorization server metadata is available

0 commit comments

Comments
 (0)