Skip to content

Commit 5e85388

Browse files
chipgptdclark27
authored andcommitted
fix: Prefer the token_endpoint_auth_method response from DCR registration (modelcontextprotocol#1022)
1 parent a5a885b commit 5e85388

4 files changed

Lines changed: 47 additions & 11 deletions

File tree

src/client/auth.test.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ import {
1010
discoverOAuthProtectedResourceMetadata,
1111
extractResourceMetadataUrl,
1212
auth,
13-
type OAuthClientProvider
13+
type OAuthClientProvider,
14+
selectClientAuthMethod
1415
} from './auth.js';
1516
import { ServerError } from '../server/auth/errors.js';
1617
import { AuthorizationServerMetadata } from '../shared/auth.js';
@@ -881,6 +882,25 @@ describe('OAuth Authorization', () => {
881882
});
882883
});
883884

885+
describe('selectClientAuthMethod', () => {
886+
it('selects the correct client authentication method from client information', () => {
887+
const clientInfo = {
888+
client_id: 'test-client-id',
889+
client_secret: 'test-client-secret',
890+
token_endpoint_auth_method: 'client_secret_basic'
891+
};
892+
const supportedMethods = ['client_secret_post', 'client_secret_basic', 'none'];
893+
const authMethod = selectClientAuthMethod(clientInfo, supportedMethods);
894+
expect(authMethod).toBe('client_secret_basic');
895+
});
896+
it('selects the correct client authentication method from supported methods', () => {
897+
const clientInfo = { client_id: 'test-client-id' };
898+
const supportedMethods = ['client_secret_post', 'client_secret_basic', 'none'];
899+
const authMethod = selectClientAuthMethod(clientInfo, supportedMethods);
900+
expect(authMethod).toBe('none');
901+
});
902+
});
903+
884904
describe('startAuthorization', () => {
885905
const validMetadata = {
886906
issuer: 'https://auth.example.com',

src/client/auth.ts

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { LATEST_PROTOCOL_VERSION } from '../types.js';
33
import {
44
OAuthClientMetadata,
55
OAuthClientInformation,
6+
OAuthClientInformationMixed,
67
OAuthTokens,
78
OAuthMetadata,
89
OAuthClientInformationFull,
@@ -58,7 +59,7 @@ export interface OAuthClientProvider {
5859
* server, or returns `undefined` if the client is not registered with the
5960
* server.
6061
*/
61-
clientInformation(): OAuthClientInformation | undefined | Promise<OAuthClientInformation | undefined>;
62+
clientInformation(): OAuthClientInformationMixed | undefined | Promise<OAuthClientInformationMixed | undefined>;
6263

6364
/**
6465
* If implemented, this permits the OAuth client to dynamically register with
@@ -68,7 +69,7 @@ export interface OAuthClientProvider {
6869
* This method is not required to be implemented if client information is
6970
* statically known (e.g., pre-registered).
7071
*/
71-
saveClientInformation?(clientInformation: OAuthClientInformationFull): void | Promise<void>;
72+
saveClientInformation?(clientInformation: OAuthClientInformationMixed): void | Promise<void>;
7273

7374
/**
7475
* Loads any existing OAuth tokens for the current session, or returns
@@ -151,6 +152,10 @@ export class UnauthorizedError extends Error {
151152

152153
type ClientAuthMethod = 'client_secret_basic' | 'client_secret_post' | 'none';
153154

155+
function isClientAuthMethod(method: string): method is ClientAuthMethod {
156+
return ['client_secret_basic', 'client_secret_post', 'none'].includes(method);
157+
}
158+
154159
const AUTHORIZATION_CODE_RESPONSE_TYPE = 'code';
155160
const AUTHORIZATION_CODE_CHALLENGE_METHOD = 'S256';
156161

@@ -166,14 +171,24 @@ const AUTHORIZATION_CODE_CHALLENGE_METHOD = 'S256';
166171
* @param supportedMethods - Authentication methods supported by the authorization server
167172
* @returns The selected authentication method
168173
*/
169-
function selectClientAuthMethod(clientInformation: OAuthClientInformation, supportedMethods: string[]): ClientAuthMethod {
174+
export function selectClientAuthMethod(clientInformation: OAuthClientInformationMixed, supportedMethods: string[]): ClientAuthMethod {
170175
const hasClientSecret = clientInformation.client_secret !== undefined;
171176

172177
// If server doesn't specify supported methods, use RFC 6749 defaults
173178
if (supportedMethods.length === 0) {
174179
return hasClientSecret ? 'client_secret_post' : 'none';
175180
}
176181

182+
// Prefer the method returned by the server during client registration if valid and supported
183+
if (
184+
'token_endpoint_auth_method' in clientInformation &&
185+
clientInformation.token_endpoint_auth_method &&
186+
isClientAuthMethod(clientInformation.token_endpoint_auth_method) &&
187+
supportedMethods.includes(clientInformation.token_endpoint_auth_method)
188+
) {
189+
return clientInformation.token_endpoint_auth_method;
190+
}
191+
177192
// Try methods in priority order (most secure first)
178193
if (hasClientSecret && supportedMethods.includes('client_secret_basic')) {
179194
return 'client_secret_basic';
@@ -796,7 +811,7 @@ export async function startAuthorization(
796811
resource
797812
}: {
798813
metadata?: AuthorizationServerMetadata;
799-
clientInformation: OAuthClientInformation;
814+
clientInformation: OAuthClientInformationMixed;
800815
redirectUrl: string | URL;
801816
scope?: string;
802817
state?: string;
@@ -879,7 +894,7 @@ export async function exchangeAuthorization(
879894
fetchFn
880895
}: {
881896
metadata?: AuthorizationServerMetadata;
882-
clientInformation: OAuthClientInformation;
897+
clientInformation: OAuthClientInformationMixed;
883898
authorizationCode: string;
884899
codeVerifier: string;
885900
redirectUri: string | URL;
@@ -958,7 +973,7 @@ export async function refreshAuthorization(
958973
fetchFn
959974
}: {
960975
metadata?: AuthorizationServerMetadata;
961-
clientInformation: OAuthClientInformation;
976+
clientInformation: OAuthClientInformationMixed;
962977
refreshToken: string;
963978
resource?: URL;
964979
addClientAuthentication?: OAuthClientProvider['addClientAuthentication'];

src/examples/client/simpleOAuthClient.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { URL } from 'node:url';
66
import { exec } from 'node:child_process';
77
import { Client } from '../../client/index.js';
88
import { StreamableHTTPClientTransport } from '../../client/streamableHttp.js';
9-
import { OAuthClientInformation, OAuthClientInformationFull, OAuthClientMetadata, OAuthTokens } from '../../shared/auth.js';
9+
import { OAuthClientInformationMixed, OAuthClientMetadata, OAuthTokens } from '../../shared/auth.js';
1010
import { CallToolRequest, ListToolsRequest, CallToolResultSchema, ListToolsResultSchema } from '../../types.js';
1111
import { OAuthClientProvider, UnauthorizedError } from '../../client/auth.js';
1212

@@ -20,7 +20,7 @@ const CALLBACK_URL = `http://localhost:${CALLBACK_PORT}/callback`;
2020
* In production, you should persist tokens securely
2121
*/
2222
class InMemoryOAuthClientProvider implements OAuthClientProvider {
23-
private _clientInformation?: OAuthClientInformationFull;
23+
private _clientInformation?: OAuthClientInformationMixed;
2424
private _tokens?: OAuthTokens;
2525
private _codeVerifier?: string;
2626

@@ -46,11 +46,11 @@ class InMemoryOAuthClientProvider implements OAuthClientProvider {
4646
return this._clientMetadata;
4747
}
4848

49-
clientInformation(): OAuthClientInformation | undefined {
49+
clientInformation(): OAuthClientInformationMixed | undefined {
5050
return this._clientInformation;
5151
}
5252

53-
saveClientInformation(clientInformation: OAuthClientInformationFull): void {
53+
saveClientInformation(clientInformation: OAuthClientInformationMixed): void {
5454
this._clientInformation = clientInformation;
5555
}
5656

src/shared/auth.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,7 @@ export type OAuthErrorResponse = z.infer<typeof OAuthErrorResponseSchema>;
226226
export type OAuthClientMetadata = z.infer<typeof OAuthClientMetadataSchema>;
227227
export type OAuthClientInformation = z.infer<typeof OAuthClientInformationSchema>;
228228
export type OAuthClientInformationFull = z.infer<typeof OAuthClientInformationFullSchema>;
229+
export type OAuthClientInformationMixed = OAuthClientInformation | OAuthClientInformationFull;
229230
export type OAuthClientRegistrationError = z.infer<typeof OAuthClientRegistrationErrorSchema>;
230231
export type OAuthTokenRevocationRequest = z.infer<typeof OAuthTokenRevocationRequestSchema>;
231232
export type OAuthProtectedResourceMetadata = z.infer<typeof OAuthProtectedResourceMetadataSchema>;

0 commit comments

Comments
 (0)