Skip to content

Commit 3c72721

Browse files
committed
refactor(xaa): consolidate save methods into OAuthDiscoveryState
Remove individual saveAuthorizationServerUrl/saveResourceUrl methods from OAuthClientProvider in favor of adding resourceUrl to OAuthDiscoveryState. CrossAppAccessProvider now reads both URLs from the discovery state, eliminating redundant save calls in auth(). The single saveDiscoveryState call is moved after selectResourceURL so it can include the resolved resource URL.
1 parent 689aa96 commit 3c72721

3 files changed

Lines changed: 32 additions & 113 deletions

File tree

packages/client/src/client/auth.ts

Lines changed: 12 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -184,50 +184,6 @@ export interface OAuthClientProvider {
184184
*/
185185
prepareTokenRequest?(scope?: string): URLSearchParams | Promise<URLSearchParams | undefined> | undefined;
186186

187-
/**
188-
* Saves the authorization server URL after RFC 9728 discovery.
189-
* This method is called by {@linkcode auth} after successful discovery of the
190-
* authorization server via protected resource metadata.
191-
*
192-
* Providers implementing Cross-App Access or other flows that need access to
193-
* the discovered authorization server URL should implement this method.
194-
*
195-
* @param authorizationServerUrl - The authorization server URL discovered via RFC 9728
196-
*/
197-
saveAuthorizationServerUrl?(authorizationServerUrl: string): void | Promise<void>;
198-
199-
/**
200-
* Returns the previously saved authorization server URL, if available.
201-
*
202-
* Providers implementing Cross-App Access can use this to access the
203-
* authorization server URL discovered during the OAuth flow.
204-
*
205-
* @returns The authorization server URL, or `undefined` if not available
206-
*/
207-
authorizationServerUrl?(): string | undefined | Promise<string | undefined>;
208-
209-
/**
210-
* Saves the resource URL after RFC 9728 discovery.
211-
* This method is called by {@linkcode auth} after successful discovery of the
212-
* resource metadata.
213-
*
214-
* Providers implementing Cross-App Access or other flows that need access to
215-
* the discovered resource URL should implement this method.
216-
*
217-
* @param resourceUrl - The resource URL discovered via RFC 9728
218-
*/
219-
saveResourceUrl?(resourceUrl: string): void | Promise<void>;
220-
221-
/**
222-
* Returns the previously saved resource URL, if available.
223-
*
224-
* Providers implementing Cross-App Access can use this to access the
225-
* resource URL discovered during the OAuth flow.
226-
*
227-
* @returns The resource URL, or `undefined` if not available
228-
*/
229-
resourceUrl?(): string | undefined | Promise<string | undefined>;
230-
231187
/**
232188
* Saves the OAuth discovery state after RFC 9728 and authorization server metadata
233189
* discovery. Providers can persist this state to avoid redundant discovery requests
@@ -267,6 +223,9 @@ export interface OAuthClientProvider {
267223
export interface OAuthDiscoveryState extends OAuthServerInfo {
268224
/** The URL at which the protected resource metadata was found, if available. */
269225
resourceMetadataUrl?: string;
226+
227+
/** The resolved resource URL from {@linkcode selectResourceURL}, if available. */
228+
resourceUrl?: string;
270229
}
271230

272231
export type AuthResult = 'AUTHORIZED' | 'REDIRECT';
@@ -489,6 +448,7 @@ async function authInternal(
489448
let resourceMetadata: OAuthProtectedResourceMetadata | undefined;
490449
let authorizationServerUrl: string | URL;
491450
let metadata: AuthorizationServerMetadata | undefined;
451+
let needsDiscoveryStateSave = false;
492452

493453
// If resourceMetadataUrl is not provided, try to load it from cached state
494454
// This handles browser redirects where the URL was saved before navigation
@@ -518,43 +478,31 @@ async function authInternal(
518478
}
519479

520480
// Re-save if we enriched the cached state with missing metadata
521-
if (metadata !== cachedState.authorizationServerMetadata || resourceMetadata !== cachedState.resourceMetadata) {
522-
await provider.saveDiscoveryState?.({
523-
authorizationServerUrl: String(authorizationServerUrl),
524-
resourceMetadataUrl: effectiveResourceMetadataUrl?.toString(),
525-
resourceMetadata,
526-
authorizationServerMetadata: metadata
527-
});
528-
}
481+
needsDiscoveryStateSave = metadata !== cachedState.authorizationServerMetadata || resourceMetadata !== cachedState.resourceMetadata;
529482
} else {
530483
// Full discovery via RFC 9728
531484
const serverInfo = await discoverOAuthServerInfo(serverUrl, { resourceMetadataUrl: effectiveResourceMetadataUrl, fetchFn });
532485
authorizationServerUrl = serverInfo.authorizationServerUrl;
533486
metadata = serverInfo.authorizationServerMetadata;
534487
resourceMetadata = serverInfo.resourceMetadata;
488+
needsDiscoveryStateSave = true;
489+
}
535490

536-
// Persist discovery state for future use
491+
const resource: URL | undefined = await selectResourceURL(serverUrl, provider, resourceMetadata);
492+
493+
if (needsDiscoveryStateSave) {
537494
// TODO: resourceMetadataUrl is only populated when explicitly provided via options
538495
// or loaded from cached state. The URL derived internally by
539496
// discoverOAuthProtectedResourceMetadata() is not captured back here.
540497
await provider.saveDiscoveryState?.({
541498
authorizationServerUrl: String(authorizationServerUrl),
542499
resourceMetadataUrl: effectiveResourceMetadataUrl?.toString(),
543500
resourceMetadata,
544-
authorizationServerMetadata: metadata
501+
authorizationServerMetadata: metadata,
502+
resourceUrl: resource?.toString()
545503
});
546504
}
547505

548-
// Save authorization server URL for providers that need it (e.g., CrossAppAccessProvider)
549-
await provider.saveAuthorizationServerUrl?.(String(authorizationServerUrl));
550-
551-
const resource: URL | undefined = await selectResourceURL(serverUrl, provider, resourceMetadata);
552-
553-
// Save resource URL for providers that need it (e.g., CrossAppAccessProvider)
554-
if (resource) {
555-
await provider.saveResourceUrl?.(String(resource));
556-
}
557-
558506
// Apply scope selection strategy (SEP-835):
559507
// 1. WWW-Authenticate scope (passed via `scope` param)
560508
// 2. PRM scopes_supported

packages/client/src/client/authExtensions.ts

Lines changed: 8 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
import type { FetchLike, OAuthClientInformation, OAuthClientMetadata, OAuthTokens } from '@modelcontextprotocol/core';
99
import type { CryptoKey, JWK } from 'jose';
1010

11-
import type { AddClientAuthentication, OAuthClientProvider } from './auth.js';
11+
import type { AddClientAuthentication, OAuthClientProvider, OAuthDiscoveryState } from './auth.js';
1212

1313
/**
1414
* Helper to produce a `private_key_jwt` client authentication function.
@@ -545,8 +545,7 @@ export class CrossAppAccessProvider implements OAuthClientProvider {
545545
private _clientMetadata: OAuthClientMetadata;
546546
private _assertionCallback: AssertionCallback;
547547
private _fetchFn: FetchLike;
548-
private _authorizationServerUrl?: string;
549-
private _resourceUrl?: string;
548+
private _discoveryState?: OAuthDiscoveryState;
550549
private _scope?: string;
551550

552551
constructor(options: CrossAppAccessProviderOptions) {
@@ -600,40 +599,17 @@ export class CrossAppAccessProvider implements OAuthClientProvider {
600599
throw new Error('codeVerifier is not used for jwt-bearer flow');
601600
}
602601

603-
/**
604-
* Saves the authorization server URL discovered during OAuth flow.
605-
* This is called by the auth() function after RFC 9728 discovery.
606-
*/
607-
saveAuthorizationServerUrl?(authorizationServerUrl: string): void {
608-
this._authorizationServerUrl = authorizationServerUrl;
609-
}
610-
611-
/**
612-
* Returns the cached authorization server URL if available.
613-
*/
614-
authorizationServerUrl?(): string | undefined {
615-
return this._authorizationServerUrl;
602+
saveDiscoveryState(state: OAuthDiscoveryState): void {
603+
this._discoveryState = state;
616604
}
617605

618-
/**
619-
* Saves the resource URL discovered during OAuth flow.
620-
* This is called by the auth() function after RFC 9728 discovery.
621-
*/
622-
saveResourceUrl?(resourceUrl: string): void {
623-
this._resourceUrl = resourceUrl;
624-
}
625-
626-
/**
627-
* Returns the cached resource URL if available.
628-
*/
629-
resourceUrl?(): string | undefined {
630-
return this._resourceUrl;
606+
discoveryState(): OAuthDiscoveryState | undefined {
607+
return this._discoveryState;
631608
}
632609

633610
async prepareTokenRequest(scope?: string): Promise<URLSearchParams> {
634-
// Get the authorization server URL and resource URL from cached state
635-
const authServerUrl = this._authorizationServerUrl;
636-
const resourceUrl = this._resourceUrl;
611+
const authServerUrl = this._discoveryState?.authorizationServerUrl;
612+
const resourceUrl = this._discoveryState?.resourceUrl;
637613

638614
if (!authServerUrl) {
639615
throw new Error('Authorization server URL not available. Ensure auth() has been called first.');

packages/client/test/client/authExtensions.test.ts

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -467,38 +467,33 @@ describe('CrossAppAccessProvider', () => {
467467
clientSecret: 'secret'
468468
});
469469

470-
// Manually set authorization server URL but not resource URL
471-
provider.saveAuthorizationServerUrl?.(AUTH_SERVER_URL);
470+
// Save discovery state without resourceUrl
471+
provider.saveDiscoveryState({
472+
authorizationServerUrl: AUTH_SERVER_URL
473+
});
472474

473475
await expect(provider.prepareTokenRequest()).rejects.toThrow(
474476
'Resource URL not available — server may not implement RFC 9728 Protected Resource Metadata'
475477
);
476478
});
477479

478-
it('stores and retrieves authorization server URL', () => {
480+
it('stores and retrieves discovery state', () => {
479481
const provider = new CrossAppAccessProvider({
480482
assertion: async () => 'jwt-grant',
481483
clientId: 'client',
482484
clientSecret: 'secret'
483485
});
484486

485-
expect(provider.authorizationServerUrl?.()).toBeUndefined();
486-
487-
provider.saveAuthorizationServerUrl?.(AUTH_SERVER_URL);
488-
expect(provider.authorizationServerUrl?.()).toBe(AUTH_SERVER_URL);
489-
});
487+
expect(provider.discoveryState()).toBeUndefined();
490488

491-
it('stores and retrieves resource URL', () => {
492-
const provider = new CrossAppAccessProvider({
493-
assertion: async () => 'jwt-grant',
494-
clientId: 'client',
495-
clientSecret: 'secret'
489+
provider.saveDiscoveryState({
490+
authorizationServerUrl: AUTH_SERVER_URL,
491+
resourceUrl: RESOURCE_SERVER_URL
496492
});
497493

498-
expect(provider.resourceUrl?.()).toBeUndefined();
499-
500-
provider.saveResourceUrl?.(RESOURCE_SERVER_URL);
501-
expect(provider.resourceUrl?.()).toBe(RESOURCE_SERVER_URL);
494+
const state = provider.discoveryState();
495+
expect(state?.authorizationServerUrl).toBe(AUTH_SERVER_URL);
496+
expect(state?.resourceUrl).toBe(RESOURCE_SERVER_URL);
502497
});
503498

504499
it('has correct client metadata', () => {

0 commit comments

Comments
 (0)