Default to client_secret_basic when server omits token_endpoint_auth_methods_supported (#1594)
Co-authored-by: Matt <77928207+mattzcarey@users.noreply.github.com>
This commit is contained in:
bhosmer-ant
GitHub
parent
52186d1884
commit
9a8f37b366
3 changed files with 80 additions and 26 deletions
|
|
@ -257,21 +257,25 @@ const AUTHORIZATION_CODE_CHALLENGE_METHOD = 'S256';
|
|||
export function selectClientAuthMethod(clientInformation: OAuthClientInformationMixed, supportedMethods: string[]): ClientAuthMethod {
|
||||
const hasClientSecret = clientInformation.client_secret !== undefined;
|
||||
|
||||
// If server doesn't specify supported methods, use RFC 6749 defaults
|
||||
if (supportedMethods.length === 0) {
|
||||
return hasClientSecret ? 'client_secret_post' : 'none';
|
||||
}
|
||||
|
||||
// Prefer the method returned by the server during client registration if valid and supported
|
||||
// Prefer the method returned by the server during client registration, if valid.
|
||||
// When server metadata is present we also require the method to be listed as supported;
|
||||
// when supportedMethods is empty (metadata omitted the field) the DCR hint stands alone.
|
||||
if (
|
||||
'token_endpoint_auth_method' in clientInformation &&
|
||||
clientInformation.token_endpoint_auth_method &&
|
||||
isClientAuthMethod(clientInformation.token_endpoint_auth_method) &&
|
||||
supportedMethods.includes(clientInformation.token_endpoint_auth_method)
|
||||
(supportedMethods.length === 0 || supportedMethods.includes(clientInformation.token_endpoint_auth_method))
|
||||
) {
|
||||
return clientInformation.token_endpoint_auth_method;
|
||||
}
|
||||
|
||||
// If server metadata omits token_endpoint_auth_methods_supported, RFC 8414 §2 says the
|
||||
// default is client_secret_basic. RFC 6749 §2.3.1 also requires servers to support HTTP
|
||||
// Basic authentication for clients with a secret, making it the safest default.
|
||||
if (supportedMethods.length === 0) {
|
||||
return hasClientSecret ? 'client_secret_basic' : 'none';
|
||||
}
|
||||
|
||||
// Try methods in priority order (most secure first)
|
||||
if (hasClientSecret && supportedMethods.includes('client_secret_basic')) {
|
||||
return 'client_secret_basic';
|
||||
|
|
|
|||
|
|
@ -1276,6 +1276,27 @@ describe('OAuth Authorization', () => {
|
|||
const authMethod = selectClientAuthMethod(clientInfo, supportedMethods);
|
||||
expect(authMethod).toBe('none');
|
||||
});
|
||||
it('defaults to client_secret_basic when server omits token_endpoint_auth_methods_supported (RFC 8414 §2)', () => {
|
||||
// RFC 8414 §2: if omitted, the default is client_secret_basic.
|
||||
// RFC 6749 §2.3.1: servers MUST support HTTP Basic for clients with a secret.
|
||||
const clientInfo = { client_id: 'test-client-id', client_secret: 'test-client-secret' };
|
||||
const authMethod = selectClientAuthMethod(clientInfo, []);
|
||||
expect(authMethod).toBe('client_secret_basic');
|
||||
});
|
||||
it('defaults to none for public clients when server omits token_endpoint_auth_methods_supported', () => {
|
||||
const clientInfo = { client_id: 'test-client-id' };
|
||||
const authMethod = selectClientAuthMethod(clientInfo, []);
|
||||
expect(authMethod).toBe('none');
|
||||
});
|
||||
it('honors DCR-returned token_endpoint_auth_method even when server metadata omits supported methods', () => {
|
||||
const clientInfo = {
|
||||
client_id: 'test-client-id',
|
||||
client_secret: 'test-client-secret',
|
||||
token_endpoint_auth_method: 'client_secret_post'
|
||||
};
|
||||
const authMethod = selectClientAuthMethod(clientInfo, []);
|
||||
expect(authMethod).toBe('client_secret_post');
|
||||
});
|
||||
});
|
||||
|
||||
describe('startAuthorization', () => {
|
||||
|
|
@ -1492,8 +1513,10 @@ describe('OAuth Authorization', () => {
|
|||
expect(body.get('grant_type')).toBe('authorization_code');
|
||||
expect(body.get('code')).toBe('code123');
|
||||
expect(body.get('code_verifier')).toBe('verifier123');
|
||||
expect(body.get('client_id')).toBe('client123');
|
||||
expect(body.get('client_secret')).toBe('secret123');
|
||||
// Default auth method is client_secret_basic when no metadata provided (RFC 8414 §2)
|
||||
expect(body.get('client_id')).toBeNull();
|
||||
expect(body.get('client_secret')).toBeNull();
|
||||
expect(options.headers.get('Authorization')).toBe('Basic ' + btoa('client123:secret123'));
|
||||
expect(body.get('redirect_uri')).toBe('http://localhost:3000/callback');
|
||||
expect(body.get('resource')).toBe('https://api.example.com/mcp-server');
|
||||
});
|
||||
|
|
@ -1531,8 +1554,10 @@ describe('OAuth Authorization', () => {
|
|||
expect(body.get('grant_type')).toBe('authorization_code');
|
||||
expect(body.get('code')).toBe('code123');
|
||||
expect(body.get('code_verifier')).toBe('verifier123');
|
||||
expect(body.get('client_id')).toBe('client123');
|
||||
expect(body.get('client_secret')).toBe('secret123');
|
||||
// Default auth method is client_secret_basic when no metadata provided (RFC 8414 §2)
|
||||
expect(body.get('client_id')).toBeNull();
|
||||
expect(body.get('client_secret')).toBeNull();
|
||||
expect(options.headers.get('Authorization')).toBe('Basic ' + btoa('client123:secret123'));
|
||||
expect(body.get('redirect_uri')).toBe('http://localhost:3000/callback');
|
||||
expect(body.get('resource')).toBe('https://api.example.com/mcp-server');
|
||||
});
|
||||
|
|
@ -1656,8 +1681,10 @@ describe('OAuth Authorization', () => {
|
|||
expect(body.get('grant_type')).toBe('authorization_code');
|
||||
expect(body.get('code')).toBe('code123');
|
||||
expect(body.get('code_verifier')).toBe('verifier123');
|
||||
expect(body.get('client_id')).toBe('client123');
|
||||
expect(body.get('client_secret')).toBe('secret123');
|
||||
// Default auth method is client_secret_basic when no metadata provided (RFC 8414 §2)
|
||||
expect(body.get('client_id')).toBeNull();
|
||||
expect(body.get('client_secret')).toBeNull();
|
||||
expect((options.headers as Headers).get('Authorization')).toBe('Basic ' + btoa('client123:secret123'));
|
||||
expect(body.get('redirect_uri')).toBe('http://localhost:3000/callback');
|
||||
expect(body.get('resource')).toBe('https://api.example.com/mcp-server');
|
||||
});
|
||||
|
|
@ -1716,8 +1743,10 @@ describe('OAuth Authorization', () => {
|
|||
const body = mockFetch.mock.calls[0]![1].body as URLSearchParams;
|
||||
expect(body.get('grant_type')).toBe('refresh_token');
|
||||
expect(body.get('refresh_token')).toBe('refresh123');
|
||||
expect(body.get('client_id')).toBe('client123');
|
||||
expect(body.get('client_secret')).toBe('secret123');
|
||||
// Default auth method is client_secret_basic when no metadata provided (RFC 8414 §2)
|
||||
expect(body.get('client_id')).toBeNull();
|
||||
expect(body.get('client_secret')).toBeNull();
|
||||
expect(headers.get('Authorization')).toBe('Basic ' + btoa('client123:secret123'));
|
||||
expect(body.get('resource')).toBe('https://api.example.com/mcp-server');
|
||||
});
|
||||
|
||||
|
|
@ -3076,7 +3105,7 @@ describe('OAuth Authorization', () => {
|
|||
expect(body.get('client_secret')).toBeNull();
|
||||
});
|
||||
|
||||
it('defaults to client_secret_post when no auth methods specified', async () => {
|
||||
it('defaults to client_secret_basic when no auth methods specified (RFC 8414 §2)', async () => {
|
||||
mockFetch.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
status: 200,
|
||||
|
|
@ -3093,13 +3122,15 @@ describe('OAuth Authorization', () => {
|
|||
expect(tokens).toEqual(validTokens);
|
||||
const request = mockFetch.mock.calls[0]![1];
|
||||
|
||||
// Check headers
|
||||
expect(request.headers.get('Content-Type')).toBe('application/x-www-form-urlencoded');
|
||||
expect(request.headers.get('Authorization')).toBeNull();
|
||||
// RFC 8414 §2: when token_endpoint_auth_methods_supported is omitted,
|
||||
// the default is client_secret_basic (HTTP Basic auth, not body params)
|
||||
const authHeader = request.headers.get('Authorization');
|
||||
const expected = 'Basic ' + btoa('client123:secret123');
|
||||
expect(authHeader).toBe(expected);
|
||||
|
||||
const body = request.body as URLSearchParams;
|
||||
expect(body.get('client_id')).toBe('client123');
|
||||
expect(body.get('client_secret')).toBe('secret123');
|
||||
expect(body.get('client_id')).toBeNull();
|
||||
expect(body.get('client_secret')).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -11,6 +11,21 @@ import type { OAuthClientProvider } from '../../src/client/auth.js';
|
|||
import { UnauthorizedError } from '../../src/client/auth.js';
|
||||
import { SSEClientTransport } from '../../src/client/sse.js';
|
||||
|
||||
/**
|
||||
* Parses HTTP Basic auth from a request's Authorization header.
|
||||
* Returns the decoded client_id and client_secret, or undefined if the header is absent or malformed.
|
||||
* client_secret_basic is the default client auth method when server metadata omits
|
||||
* token_endpoint_auth_methods_supported (RFC 8414 §2).
|
||||
*/
|
||||
function parseBasicAuth(req: IncomingMessage): { clientId: string; clientSecret: string } | undefined {
|
||||
const auth = req.headers.authorization;
|
||||
if (!auth || !auth.startsWith('Basic ')) return undefined;
|
||||
const decoded = Buffer.from(auth.slice(6), 'base64').toString('utf8');
|
||||
const sep = decoded.indexOf(':');
|
||||
if (sep === -1) return undefined;
|
||||
return { clientId: decoded.slice(0, sep), clientSecret: decoded.slice(sep + 1) };
|
||||
}
|
||||
|
||||
describe('SSEClientTransport', () => {
|
||||
let resourceServer: Server;
|
||||
let authServer: Server;
|
||||
|
|
@ -673,11 +688,12 @@ describe('SSEClientTransport', () => {
|
|||
});
|
||||
req.on('end', () => {
|
||||
const params = new URLSearchParams(body);
|
||||
const basicAuth = parseBasicAuth(req);
|
||||
if (
|
||||
params.get('grant_type') === 'refresh_token' &&
|
||||
params.get('refresh_token') === 'refresh-token' &&
|
||||
params.get('client_id') === 'test-client-id' &&
|
||||
params.get('client_secret') === 'test-client-secret'
|
||||
basicAuth?.clientId === 'test-client-id' &&
|
||||
basicAuth?.clientSecret === 'test-client-secret'
|
||||
) {
|
||||
res.writeHead(200, { 'Content-Type': 'application/json' });
|
||||
res.end(
|
||||
|
|
@ -801,11 +817,12 @@ describe('SSEClientTransport', () => {
|
|||
});
|
||||
req.on('end', () => {
|
||||
const params = new URLSearchParams(body);
|
||||
const basicAuth = parseBasicAuth(req);
|
||||
if (
|
||||
params.get('grant_type') === 'refresh_token' &&
|
||||
params.get('refresh_token') === 'refresh-token' &&
|
||||
params.get('client_id') === 'test-client-id' &&
|
||||
params.get('client_secret') === 'test-client-secret'
|
||||
basicAuth?.clientId === 'test-client-id' &&
|
||||
basicAuth?.clientSecret === 'test-client-secret'
|
||||
) {
|
||||
res.writeHead(200, { 'Content-Type': 'application/json' });
|
||||
res.end(
|
||||
|
|
@ -1234,10 +1251,12 @@ describe('SSEClientTransport', () => {
|
|||
});
|
||||
req.on('end', () => {
|
||||
const params = new URLSearchParams(body);
|
||||
const basicAuth = parseBasicAuth(req);
|
||||
if (
|
||||
params.get('grant_type') === 'authorization_code' &&
|
||||
params.get('code') === 'test-auth-code' &&
|
||||
params.get('client_id') === 'test-client-id'
|
||||
basicAuth?.clientId === 'test-client-id' &&
|
||||
basicAuth?.clientSecret === 'test-client-secret'
|
||||
) {
|
||||
res.writeHead(200, { 'Content-Type': 'application/json' });
|
||||
res.end(
|
||||
|
|
|
|||
Loading…
Reference in a new issue