Skip to content

Commit 6567674

Browse files
committed
fix(client): allow cached malformed legacy issuer fallback
1 parent 7b3b4c7 commit 6567674

2 files changed

Lines changed: 67 additions & 8 deletions

File tree

packages/client/src/client/auth.ts

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -622,20 +622,46 @@ function validateAuthorizationServerMetadataIssuer(metadata: { issuer: string }
622622
}
623623
}
624624

625+
function isLegacyFallbackDiscoveryState(cachedState: OAuthDiscoveryState, serverUrl: string | URL): boolean {
626+
if (cachedState.resourceMetadata?.authorization_servers?.length) {
627+
return false;
628+
}
629+
630+
try {
631+
const cachedIssuer = normalizeDiscoveredIssuerIdentifier(cachedState.authorizationServerUrl, 'Cached authorization server URL');
632+
const legacyFallbackIssuer = normalizeDiscoveredIssuerIdentifier(new URL('/', serverUrl), 'MCP server URL');
633+
return cachedIssuer === legacyFallbackIssuer;
634+
} catch {
635+
return false;
636+
}
637+
}
638+
639+
function hasMalformedAuthorizationServerMetadataIssuer(metadata: { issuer: string } | undefined): boolean {
640+
if (!metadata) {
641+
return false;
642+
}
643+
644+
try {
645+
normalizeDiscoveredIssuerIdentifier(metadata.issuer, 'Authorization server metadata issuer');
646+
return false;
647+
} catch {
648+
return true;
649+
}
650+
}
651+
625652
function isStaleLegacyFallbackDiscoveryState(
626653
cachedState: OAuthDiscoveryState,
627654
metadata: { issuer: string } | undefined,
628655
serverUrl: string | URL
629656
): boolean {
630-
if (!metadata || cachedState.resourceMetadata?.authorization_servers?.length) {
657+
if (!metadata || !isLegacyFallbackDiscoveryState(cachedState, serverUrl)) {
631658
return false;
632659
}
633660

634661
try {
635662
const cachedIssuer = normalizeDiscoveredIssuerIdentifier(cachedState.authorizationServerUrl, 'Cached authorization server URL');
636-
const legacyFallbackIssuer = normalizeDiscoveredIssuerIdentifier(new URL('/', serverUrl), 'MCP server URL');
637663
const metadataIssuer = normalizeDiscoveredIssuerIdentifier(metadata.issuer, 'Authorization server metadata issuer');
638-
return cachedIssuer === legacyFallbackIssuer && metadataIssuer !== cachedIssuer;
664+
return metadataIssuer !== cachedIssuer;
639665
} catch {
640666
return false;
641667
}
@@ -766,12 +792,16 @@ async function authInternal(
766792
try {
767793
validateAuthorizationServerMetadataIssuer(metadata, authorizationServerUrl);
768794
} catch (error) {
769-
if (!isStaleLegacyFallbackDiscoveryState(cachedState, metadata, serverUrl)) {
770-
throw error;
771-
}
795+
const canUseMalformedLegacyFallbackState =
796+
isLegacyFallbackDiscoveryState(cachedState, serverUrl) && hasMalformedAuthorizationServerMetadataIssuer(metadata);
797+
if (!canUseMalformedLegacyFallbackState) {
798+
if (!isStaleLegacyFallbackDiscoveryState(cachedState, metadata, serverUrl)) {
799+
throw error;
800+
}
772801

773-
await provider.invalidateCredentials?.('discovery');
774-
useCachedDiscoveryState = false;
802+
await provider.invalidateCredentials?.('discovery');
803+
useCachedDiscoveryState = false;
804+
}
775805
}
776806
}
777807

packages/client/test/client/auth.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4713,6 +4713,35 @@ describe('SEP-2468: RFC 9207 authorization response iss validation', () => {
47134713
expect(provider.saveTokens).not.toHaveBeenCalled();
47144714
});
47154715

4716+
it('uses cached legacy no-PRM fallback metadata with an invalid issuer during code exchange', async () => {
4717+
const legacyAuthMetadata: AuthorizationServerMetadata = {
4718+
...authServerMetadata,
4719+
issuer: 'auth.example.com',
4720+
authorization_endpoint: 'https://auth.example.com/oauth/authorize',
4721+
token_endpoint: 'https://auth.example.com/oauth/token'
4722+
};
4723+
const invalidateCredentials = vi.fn();
4724+
const provider: OAuthClientProvider = {
4725+
...createMockProvider(legacyAuthMetadata),
4726+
discoveryState: vi.fn().mockResolvedValue({
4727+
authorizationServerUrl: 'https://resource.example.com/',
4728+
authorizationServerMetadata: legacyAuthMetadata
4729+
}),
4730+
invalidateCredentials
4731+
};
4732+
4733+
const result = await auth(provider, {
4734+
serverUrl: 'https://resource.example.com',
4735+
authorizationCode: 'code123'
4736+
});
4737+
4738+
expect(result).toBe('AUTHORIZED');
4739+
expect(invalidateCredentials).not.toHaveBeenCalled();
4740+
expect(tokenEndpointCalls()).toHaveLength(1);
4741+
expect(tokenEndpointCalls()[0]?.[0]?.toString()).toBe(legacyAuthMetadata.token_endpoint);
4742+
expect(provider.saveTokens).toHaveBeenCalled();
4743+
});
4744+
47164745
it('rejects when the AS advertises iss support and the caller reports a response without iss (null)', async () => {
47174746
const provider = createMockProvider();
47184747

0 commit comments

Comments
 (0)