Skip to content

fix: Prefer the token_endpoint_auth_method response from DCR registration #982

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+35 −12
Open

fix: Prefer the token_endpoint_auth_method response from DCR registration #982

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 10 additions & 0 deletions src/client/auth.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1635,6 +1635,7 @@ describe("OAuth Authorization", () => {
(mockProvider.clientInformation as jest.Mock).mockResolvedValue({
client_id: "test-client",
client_secret: "test-secret",
redirect_uris: ["http://localhost:3000/callback"],
Copy link
Author

Choose a reason for hiding this comment

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

Adding the redirect_uris to all the mock objects to match the new type OAuthClientInformationFull

});
(mockProvider.tokens as jest.Mock).mockResolvedValue(undefined);
(mockProvider.saveCodeVerifier as jest.Mock).mockResolvedValue(undefined);
Expand Down Expand Up @@ -1705,6 +1706,7 @@ describe("OAuth Authorization", () => {
(mockProvider.clientInformation as jest.Mock).mockResolvedValue({
client_id: "test-client",
client_secret: "test-secret",
redirect_uris: ["http://localhost:3000/callback"],
});
(mockProvider.codeVerifier as jest.Mock).mockResolvedValue("test-verifier");
(mockProvider.saveTokens as jest.Mock).mockResolvedValue(undefined);
Expand Down Expand Up @@ -1773,6 +1775,7 @@ describe("OAuth Authorization", () => {
(mockProvider.clientInformation as jest.Mock).mockResolvedValue({
client_id: "test-client",
client_secret: "test-secret",
redirect_uris: ["http://localhost:3000/callback"],
});
(mockProvider.tokens as jest.Mock).mockResolvedValue({
access_token: "old-access",
Expand Down Expand Up @@ -1841,6 +1844,7 @@ describe("OAuth Authorization", () => {
(providerWithCustomValidation.clientInformation as jest.Mock).mockResolvedValue({
client_id: "test-client",
client_secret: "test-secret",
redirect_uris: ["http://localhost:3000/callback"],
});
(providerWithCustomValidation.tokens as jest.Mock).mockResolvedValue(undefined);
(providerWithCustomValidation.saveCodeVerifier as jest.Mock).mockResolvedValue(undefined);
Expand Down Expand Up @@ -1896,6 +1900,7 @@ describe("OAuth Authorization", () => {
(mockProvider.clientInformation as jest.Mock).mockResolvedValue({
client_id: "test-client",
client_secret: "test-secret",
redirect_uris: ["http://localhost:3000/callback"],
});
(mockProvider.tokens as jest.Mock).mockResolvedValue(undefined);
(mockProvider.saveCodeVerifier as jest.Mock).mockResolvedValue(undefined);
Expand Down Expand Up @@ -1954,6 +1959,7 @@ describe("OAuth Authorization", () => {
(mockProvider.clientInformation as jest.Mock).mockResolvedValue({
client_id: "test-client",
client_secret: "test-secret",
redirect_uris: ["http://localhost:3000/callback"],
});
(mockProvider.tokens as jest.Mock).mockResolvedValue(undefined);
(mockProvider.saveCodeVerifier as jest.Mock).mockResolvedValue(undefined);
Expand Down Expand Up @@ -2021,6 +2027,7 @@ describe("OAuth Authorization", () => {
(mockProvider.clientInformation as jest.Mock).mockResolvedValue({
client_id: "test-client",
client_secret: "test-secret",
redirect_uris: ["http://localhost:3000/callback"],
});
(mockProvider.codeVerifier as jest.Mock).mockResolvedValue("test-verifier");
(mockProvider.saveTokens as jest.Mock).mockResolvedValue(undefined);
Expand Down Expand Up @@ -2086,6 +2093,7 @@ describe("OAuth Authorization", () => {
(mockProvider.clientInformation as jest.Mock).mockResolvedValue({
client_id: "test-client",
client_secret: "test-secret",
redirect_uris: ["http://localhost:3000/callback"],
});
(mockProvider.tokens as jest.Mock).mockResolvedValue({
access_token: "old-access",
Expand Down Expand Up @@ -2149,6 +2157,7 @@ describe("OAuth Authorization", () => {
(mockProvider.clientInformation as jest.Mock).mockResolvedValue({
client_id: "test-client",
client_secret: "test-secret",
redirect_uris: ["http://localhost:3000/callback"],
});
(mockProvider.tokens as jest.Mock).mockResolvedValue(undefined);
(mockProvider.saveCodeVerifier as jest.Mock).mockResolvedValue(undefined);
Expand Down Expand Up @@ -2209,6 +2218,7 @@ describe("OAuth Authorization", () => {
clientInformation: jest.fn().mockResolvedValue({
client_id: "client123",
client_secret: "secret123",
redirect_uris: ["http://localhost:3000/callback"],
}),
tokens: jest.fn().mockResolvedValue(undefined),
saveTokens: jest.fn(),
Expand Down
26 changes: 19 additions & 7 deletions src/client/auth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import pkceChallenge from "pkce-challenge";
import { LATEST_PROTOCOL_VERSION } from "../types.js";
import {
OAuthClientMetadata,
OAuthClientInformation,
OAuthTokens,
OAuthMetadata,
OAuthClientInformationFull,
Expand Down Expand Up @@ -51,7 +50,7 @@ export interface OAuthClientProvider {
* server, or returns `undefined` if the client is not registered with the
* server.
*/
clientInformation(): OAuthClientInformation | undefined | Promise<OAuthClientInformation | undefined>;
clientInformation(): OAuthClientInformationFull | undefined | Promise<OAuthClientInformationFull | undefined>;

/**
* If implemented, this permits the OAuth client to dynamically register with
Expand Down Expand Up @@ -139,6 +138,10 @@ export class UnauthorizedError extends Error {

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

function isClientAuthMethod(method: string): method is ClientAuthMethod {
return ["client_secret_basic", "client_secret_post", "none"].includes(method);
}

/**
* Determines the best client authentication method to use based on server support and client configuration.
*
Expand All @@ -152,7 +155,7 @@ type ClientAuthMethod = 'client_secret_basic' | 'client_secret_post' | 'none';
* @returns The selected authentication method
*/
function selectClientAuthMethod(
clientInformation: OAuthClientInformation,
clientInformation: OAuthClientInformationFull,
supportedMethods: string[]
): ClientAuthMethod {
const hasClientSecret = clientInformation.client_secret !== undefined;
Expand All @@ -162,6 +165,15 @@ function selectClientAuthMethod(
return hasClientSecret ? "client_secret_post" : "none";
}

// Prefer the method returned by the server during client registration if valid and supported
if (
clientInformation.token_endpoint_auth_method &&
isClientAuthMethod(clientInformation.token_endpoint_auth_method) &&
supportedMethods.includes(clientInformation.token_endpoint_auth_method)
) {
return clientInformation.token_endpoint_auth_method;
}

Comment on lines +168 to +176
Copy link
Author

@chipgpt chipgpt Sep 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Should this throw an error if an invalid or unsupported token_endpoint_auth_method is returned? This implementation would simply skip it and fall through to the rest of the logic if it is invalid or unsupported.

On the other hand, should it even care if it's valid or supported in this context? or should it just use the token_endpoint_auth_method value without validating it at all?

// Try methods in priority order (most secure first)
if (hasClientSecret && supportedMethods.includes("client_secret_basic")) {
return "client_secret_basic";
Expand Down Expand Up @@ -195,7 +207,7 @@ function selectClientAuthMethod(
*/
function applyClientAuthentication(
method: ClientAuthMethod,
clientInformation: OAuthClientInformation,
clientInformation: OAuthClientInformationFull,
headers: Headers,
params: URLSearchParams
): void {
Expand Down Expand Up @@ -809,7 +821,7 @@ export async function startAuthorization(
resource,
}: {
metadata?: AuthorizationServerMetadata;
clientInformation: OAuthClientInformation;
clientInformation: OAuthClientInformationFull;
redirectUrl: string | URL;
scope?: string;
state?: string;
Expand Down Expand Up @@ -902,7 +914,7 @@ export async function exchangeAuthorization(
fetchFn,
}: {
metadata?: AuthorizationServerMetadata;
clientInformation: OAuthClientInformation;
clientInformation: OAuthClientInformationFull;
authorizationCode: string;
codeVerifier: string;
redirectUri: string | URL;
Expand Down Expand Up @@ -988,7 +1000,7 @@ export async function refreshAuthorization(
fetchFn,
}: {
metadata?: AuthorizationServerMetadata;
clientInformation: OAuthClientInformation;
clientInformation: OAuthClientInformationFull;
refreshToken: string;
resource?: URL;
addClientAuthentication?: OAuthClientProvider["addClientAuthentication"];
Expand Down
5 changes: 3 additions & 2 deletions src/client/sse.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ describe("SSEClientTransport", () => {
mockAuthProvider = {
get redirectUrl() { return "http://localhost/callback"; },
get clientMetadata() { return { redirect_uris: ["http://localhost/callback"] }; },
clientInformation: jest.fn(() => ({ client_id: "test-client-id", client_secret: "test-client-secret" })),
clientInformation: jest.fn(() => ({ client_id: "test-client-id", client_secret: "test-client-secret", redirect_uris: ["http://localhost/callback"] })),
tokens: jest.fn(),
saveTokens: jest.fn(),
redirectToAuthorization: jest.fn(),
Expand Down Expand Up @@ -1140,7 +1140,8 @@ describe("SSEClientTransport", () => {

const clientInfo = config.clientRegistered ? {
client_id: "test-client-id",
client_secret: "test-client-secret"
client_secret: "test-client-secret",
redirect_uris: ["http://localhost/callback"],
} : undefined;

return {
Expand Down
2 changes: 1 addition & 1 deletion src/client/streamableHttp.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ describe("StreamableHTTPClientTransport", () => {
mockAuthProvider = {
get redirectUrl() { return "http://localhost/callback"; },
get clientMetadata() { return { redirect_uris: ["http://localhost/callback"] }; },
clientInformation: jest.fn(() => ({ client_id: "test-client-id", client_secret: "test-client-secret" })),
clientInformation: jest.fn(() => ({ client_id: "test-client-id", client_secret: "test-client-secret", redirect_uris: ["http://localhost/callback"] })),
tokens: jest.fn(),
saveTokens: jest.fn(),
redirectToAuthorization: jest.fn(),
Expand Down
4 changes: 2 additions & 2 deletions src/examples/client/simpleOAuthClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { URL } from 'node:url';
import { exec } from 'node:child_process';
import { Client } from '../../client/index.js';
import { StreamableHTTPClientTransport } from '../../client/streamableHttp.js';
import { OAuthClientInformation, OAuthClientInformationFull, OAuthClientMetadata, OAuthTokens } from '../../shared/auth.js';
import { OAuthClientInformationFull, OAuthClientMetadata, OAuthTokens } from '../../shared/auth.js';
import {
CallToolRequest,
ListToolsRequest,
Expand Down Expand Up @@ -49,7 +49,7 @@ class InMemoryOAuthClientProvider implements OAuthClientProvider {
return this._clientMetadata;
}

clientInformation(): OAuthClientInformation | undefined {
clientInformation(): OAuthClientInformationFull | undefined {
return this._clientInformation;
}

Expand Down