mirror of
https://github.com/n8n-io/n8n.git
synced 2026-06-19 07:36:52 +00:00
fix(core): Negotiate token_endpoint_auth_method during MCP OAuth2 DCR (#32289)
This commit is contained in:
committed by
GitHub
parent
ec9487e9cf
commit
734af045c5
@@ -1988,7 +1988,7 @@ describe('OauthService', () => {
|
||||
token_endpoint: 'https://example.domain/oauth2/token',
|
||||
registration_endpoint: 'https://example.domain/oauth2/register',
|
||||
grant_types_supported: ['authorization_code', 'refresh_token'],
|
||||
token_endpoint_auth_methods_supported: ['client_secret_basic'],
|
||||
token_endpoint_auth_methods_supported: ['none', 'client_secret_basic'],
|
||||
code_challenge_methods_supported: ['S256'],
|
||||
scopes_supported: ['openid', 'profile'],
|
||||
},
|
||||
@@ -3224,9 +3224,11 @@ describe('OauthService', () => {
|
||||
}),
|
||||
).resolves.toBeDefined();
|
||||
|
||||
// PKCE should be selected since S256 is supported
|
||||
// The server advertises S256 but only client_secret_basic (no 'none'), so a
|
||||
// confidential authorization_code flow is selected rather than public-client PKCE.
|
||||
const callArgs = (service.encryptAndSaveData as jest.Mock).mock.calls[0];
|
||||
expect(callArgs[1]).toHaveProperty('grantType', 'pkce');
|
||||
expect(callArgs[1]).toHaveProperty('grantType', 'authorizationCode');
|
||||
expect(callArgs[1]).toHaveProperty('authentication', 'header');
|
||||
});
|
||||
|
||||
it('should not produce double /.well-known/ paths when authorization server URL already contains /.well-known/', async () => {
|
||||
@@ -3710,6 +3712,70 @@ describe('OauthService', () => {
|
||||
},
|
||||
);
|
||||
|
||||
it('should default client credentials flow to header authentication when the server omits token_endpoint_auth_methods_supported', async () => {
|
||||
const oauthCredentials = makeDcrCredentials();
|
||||
const toUpdate = {};
|
||||
|
||||
const metadata = makeMetadata({ grant_types_supported: ['client_credentials'] });
|
||||
delete (metadata as Record<string, unknown>).token_endpoint_auth_methods_supported;
|
||||
jest.mocked(axios.get).mockResolvedValueOnce({ data: metadata });
|
||||
jest.mocked(axios.post).mockResolvedValueOnce({
|
||||
data: { client_id: 'registered-client-id', client_secret: 'registered-secret' },
|
||||
});
|
||||
|
||||
await (service as any).performDynamicClientRegistration(
|
||||
oauthCredentials,
|
||||
'https://auth.example.com',
|
||||
toUpdate,
|
||||
);
|
||||
|
||||
expect(oauthCredentials.grantType).toBe('clientCredentials');
|
||||
expect(oauthCredentials.authentication).toBe('header');
|
||||
expect(toUpdate).toEqual(
|
||||
expect.objectContaining({
|
||||
grantType: 'clientCredentials',
|
||||
authentication: 'header',
|
||||
clientId: 'registered-client-id',
|
||||
clientSecret: 'registered-secret',
|
||||
}),
|
||||
);
|
||||
expect(axios.post).toHaveBeenCalledWith(
|
||||
'https://auth.example.com/oauth2/register',
|
||||
expect.objectContaining({
|
||||
grant_types: ['client_credentials'],
|
||||
token_endpoint_auth_method: 'client_secret_basic',
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should default authorization code flow to header authentication when the server omits token_endpoint_auth_methods_supported', async () => {
|
||||
const oauthCredentials = makeDcrCredentials();
|
||||
const toUpdate = {};
|
||||
|
||||
const metadata = makeMetadata({ grant_types_supported: ['authorization_code'] });
|
||||
delete (metadata as Record<string, unknown>).token_endpoint_auth_methods_supported;
|
||||
jest.mocked(axios.get).mockResolvedValueOnce({ data: metadata });
|
||||
jest.mocked(axios.post).mockResolvedValueOnce({
|
||||
data: { client_id: 'registered-client-id', client_secret: 'registered-secret' },
|
||||
});
|
||||
|
||||
await (service as any).performDynamicClientRegistration(
|
||||
oauthCredentials,
|
||||
'https://auth.example.com',
|
||||
toUpdate,
|
||||
);
|
||||
|
||||
expect(oauthCredentials.grantType).toBe('authorizationCode');
|
||||
expect(oauthCredentials.authentication).toBe('header');
|
||||
expect(axios.post).toHaveBeenCalledWith(
|
||||
'https://auth.example.com/oauth2/register',
|
||||
expect.objectContaining({
|
||||
grant_types: ['authorization_code', 'refresh_token'],
|
||||
token_endpoint_auth_method: 'client_secret_basic',
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should throw when metadata does not advertise a supported grant/authentication combination', async () => {
|
||||
jest.mocked(axios.get).mockResolvedValueOnce({
|
||||
data: makeMetadata({
|
||||
@@ -3727,6 +3793,145 @@ describe('OauthService', () => {
|
||||
).rejects.toThrow('No supported grant type and authentication method found');
|
||||
expect(axios.post).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
describe('token_endpoint_auth_method negotiation with S256 support', () => {
|
||||
it.each([
|
||||
['body', 'client_secret_post'],
|
||||
['header', 'client_secret_basic'],
|
||||
] as const)(
|
||||
'should register %s authentication when the server advertises S256 but only %s',
|
||||
async (authentication, authMethod) => {
|
||||
const oauthCredentials = makeDcrCredentials();
|
||||
const toUpdate = {};
|
||||
|
||||
jest.mocked(axios.get).mockResolvedValueOnce({
|
||||
data: makeMetadata({
|
||||
grant_types_supported: ['authorization_code'],
|
||||
token_endpoint_auth_methods_supported: [authMethod],
|
||||
code_challenge_methods_supported: ['S256'],
|
||||
}),
|
||||
});
|
||||
jest.mocked(axios.post).mockResolvedValueOnce({
|
||||
data: { client_id: 'registered-client-id', client_secret: 'registered-secret' },
|
||||
});
|
||||
|
||||
await (service as any).performDynamicClientRegistration(
|
||||
oauthCredentials,
|
||||
'https://auth.example.com',
|
||||
toUpdate,
|
||||
);
|
||||
|
||||
expect(oauthCredentials.grantType).toBe('authorizationCode');
|
||||
expect(oauthCredentials.authentication).toBe(authentication);
|
||||
expect(toUpdate).toEqual(
|
||||
expect.objectContaining({
|
||||
grantType: 'authorizationCode',
|
||||
authentication,
|
||||
clientId: 'registered-client-id',
|
||||
clientSecret: 'registered-secret',
|
||||
}),
|
||||
);
|
||||
expect(axios.post).toHaveBeenCalledWith(
|
||||
'https://auth.example.com/oauth2/register',
|
||||
expect.objectContaining({
|
||||
grant_types: ['authorization_code', 'refresh_token'],
|
||||
token_endpoint_auth_method: authMethod,
|
||||
}),
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
it('should register PKCE when the server supports both S256 and the none auth method', async () => {
|
||||
const oauthCredentials = makeDcrCredentials();
|
||||
const toUpdate = {};
|
||||
|
||||
jest.mocked(axios.get).mockResolvedValueOnce({
|
||||
data: makeMetadata({
|
||||
grant_types_supported: ['authorization_code'],
|
||||
token_endpoint_auth_methods_supported: ['none', 'client_secret_post'],
|
||||
code_challenge_methods_supported: ['S256'],
|
||||
}),
|
||||
});
|
||||
jest.mocked(axios.post).mockResolvedValueOnce({
|
||||
data: { client_id: 'registered-client-id' },
|
||||
});
|
||||
|
||||
await (service as any).performDynamicClientRegistration(
|
||||
oauthCredentials,
|
||||
'https://auth.example.com',
|
||||
toUpdate,
|
||||
);
|
||||
|
||||
expect(oauthCredentials.grantType).toBe('pkce');
|
||||
expect(axios.post).toHaveBeenCalledWith(
|
||||
'https://auth.example.com/oauth2/register',
|
||||
expect.objectContaining({
|
||||
grant_types: ['authorization_code', 'refresh_token'],
|
||||
token_endpoint_auth_method: 'none',
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should register PKCE when the server supports S256 and omits token_endpoint_auth_methods_supported', async () => {
|
||||
const oauthCredentials = makeDcrCredentials();
|
||||
const toUpdate = {};
|
||||
|
||||
const metadata = makeMetadata({
|
||||
grant_types_supported: ['authorization_code'],
|
||||
code_challenge_methods_supported: ['S256'],
|
||||
});
|
||||
delete (metadata as Record<string, unknown>).token_endpoint_auth_methods_supported;
|
||||
jest.mocked(axios.get).mockResolvedValueOnce({ data: metadata });
|
||||
jest.mocked(axios.post).mockResolvedValueOnce({
|
||||
data: { client_id: 'registered-client-id' },
|
||||
});
|
||||
|
||||
await (service as any).performDynamicClientRegistration(
|
||||
oauthCredentials,
|
||||
'https://auth.example.com',
|
||||
toUpdate,
|
||||
);
|
||||
|
||||
expect(oauthCredentials.grantType).toBe('pkce');
|
||||
expect(axios.post).toHaveBeenCalledWith(
|
||||
'https://auth.example.com/oauth2/register',
|
||||
expect.objectContaining({
|
||||
token_endpoint_auth_method: 'none',
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should fall back to PKCE when the server supports S256 but only advertises unrecognized auth methods', async () => {
|
||||
const oauthCredentials = makeDcrCredentials();
|
||||
const toUpdate = {};
|
||||
|
||||
jest.mocked(axios.get).mockResolvedValueOnce({
|
||||
data: makeMetadata({
|
||||
grant_types_supported: ['authorization_code'],
|
||||
token_endpoint_auth_methods_supported: ['private_key_jwt'],
|
||||
code_challenge_methods_supported: ['S256'],
|
||||
}),
|
||||
});
|
||||
jest.mocked(axios.post).mockResolvedValueOnce({
|
||||
data: { client_id: 'registered-client-id' },
|
||||
});
|
||||
|
||||
await (service as any).performDynamicClientRegistration(
|
||||
oauthCredentials,
|
||||
'https://auth.example.com',
|
||||
toUpdate,
|
||||
);
|
||||
|
||||
expect(oauthCredentials.grantType).toBe('pkce');
|
||||
expect(axios.post).toHaveBeenCalledWith(
|
||||
'https://auth.example.com/oauth2/register',
|
||||
expect.objectContaining({
|
||||
grant_types: ['authorization_code', 'refresh_token'],
|
||||
token_endpoint_auth_method: 'none',
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('InvalidTargetError', () => {
|
||||
|
||||
@@ -950,7 +950,7 @@ export class OauthService {
|
||||
|
||||
const { grantType, authentication } = this.selectGrantTypeAndAuthenticationMethod(
|
||||
metadataValidation.data.grant_types_supported ?? ['authorization_code', 'implicit'],
|
||||
metadataValidation.data.token_endpoint_auth_methods_supported ?? ['client_secret_basic'],
|
||||
metadataValidation.data.token_endpoint_auth_methods_supported ?? [],
|
||||
metadataValidation.data.code_challenge_methods_supported ?? [],
|
||||
);
|
||||
oauthCredentials.grantType = grantType;
|
||||
@@ -1255,8 +1255,15 @@ export class OauthService {
|
||||
tokenEndpointAuthMethods: string[],
|
||||
codeChallengeMethods: string[],
|
||||
): { grantType: OAuth2GrantType; authentication?: OAuth2AuthenticationMethod } {
|
||||
const supportsPkce = codeChallengeMethods.includes('S256');
|
||||
|
||||
if (grantTypes.includes('authorization_code')) {
|
||||
if (codeChallengeMethods.includes('S256')) {
|
||||
// Public-client PKCE only when the server allows the 'none' auth method, or
|
||||
// advertises no auth methods at all (servers that expose only PKCE metadata).
|
||||
if (
|
||||
supportsPkce &&
|
||||
(tokenEndpointAuthMethods.length === 0 || tokenEndpointAuthMethods.includes('none'))
|
||||
) {
|
||||
return { grantType: 'pkce' };
|
||||
}
|
||||
|
||||
@@ -1267,6 +1274,16 @@ export class OauthService {
|
||||
if (tokenEndpointAuthMethods.includes('client_secret_post')) {
|
||||
return { grantType: 'authorizationCode', authentication: 'body' };
|
||||
}
|
||||
|
||||
// S256 advertised alongside only unrecognized methods: fall back to public-client PKCE.
|
||||
if (supportsPkce) {
|
||||
return { grantType: 'pkce' };
|
||||
}
|
||||
|
||||
// Server omitted token_endpoint_auth_methods_supported: default to client_secret_basic (RFC 8414).
|
||||
if (tokenEndpointAuthMethods.length === 0) {
|
||||
return { grantType: 'authorizationCode', authentication: 'header' };
|
||||
}
|
||||
}
|
||||
|
||||
if (grantTypes.includes('client_credentials')) {
|
||||
@@ -1277,6 +1294,11 @@ export class OauthService {
|
||||
if (tokenEndpointAuthMethods.includes('client_secret_post')) {
|
||||
return { grantType: 'clientCredentials', authentication: 'body' };
|
||||
}
|
||||
|
||||
// Server omitted token_endpoint_auth_methods_supported: default to client_secret_basic (RFC 8414).
|
||||
if (tokenEndpointAuthMethods.length === 0) {
|
||||
return { grantType: 'clientCredentials', authentication: 'header' };
|
||||
}
|
||||
}
|
||||
|
||||
throw new BadRequestError('No supported grant type and authentication method found');
|
||||
|
||||
Reference in New Issue
Block a user