Skip to content

Commit cb29460

Browse files
fix(auth): run ntlm through bruno's request pipeline (#9044)
NtlmClient is used as an adapter, so tls, proxy, redirect and timeline handling apply to ntlm requests. A redirect drops the finished message and X-retry; same origin renegotiates, another host only when forwardAuthorizationHeader is on. Adds an ntlm mock server to bruno-tests, playwright coverage for the app and the cli, and unit tests for the redirect rule.
1 parent 9c3d8ff commit cb29460

47 files changed

Lines changed: 1147 additions & 30 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

package-lock.json

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/bruno-app/src/components/ResponsePane/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ const ResponsePane = ({ item, collection }) => {
259259
</div>
260260
</>
261261
) : null}
262-
<div className="flex items-center response-pane-status">
262+
<div className="flex items-center response-pane-status" data-testid="response-pane-status">
263263
<StatusCode status={response.status} isStreaming={item.response?.stream?.running} />
264264
{item.response?.stream?.running
265265
? <ResponseStopWatch startTimestamp={item.requestSent?.timestamp} />

packages/bruno-cli/src/runner/run-single-request.js

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ const path = require('path');
1818
const { parseDataFromResponse } = require('../utils/common');
1919
const { getCookieStringForUrl, saveCookies } = require('../utils/cookies');
2020
const { createFormData } = require('../utils/form-data');
21+
const axios = require('axios');
2122
const { NtlmClient } = require('axios-ntlm');
2223
const { addDigestInterceptor, addEdgeGridInterceptor, getHttpHttpsAgents, makeAxiosInstance: makeAxiosInstanceForOauth2, applyOAuth1ToRequest } = require('@usebruno/requests');
2324
const { getCACertificates, transformProxyConfig, applySentHeadersToRequest } = require('@usebruno/requests');
@@ -397,7 +398,9 @@ const runSingleRequest = async function (
397398
const noproxy = get(options, 'noproxy', false);
398399
const cachedSystemProxy = get(options, 'cachedSystemProxy', null);
399400
const disableCache = !get(options, 'cacheSslSession', false);
400-
const httpsAgentRequestFields = {};
401+
// NTLM authenticates the connection rather than the request, so its handshake only completes
402+
// when every leg reuses one socket.
403+
const httpsAgentRequestFields = request.ntlmConfig ? { keepAlive: true } : {};
401404

402405
if (insecure) {
403406
httpsAgentRequestFields['rejectUnauthorized'] = false;
@@ -678,7 +681,7 @@ const runSingleRequest = async function (
678681
request.timeout = requestTimeout;
679682
}
680683

681-
let axiosInstance = makeAxiosInstance({
684+
const axiosInstance = makeAxiosInstance({
682685
requestMaxRedirects: requestMaxRedirects,
683686
disableCookies: options.disableCookies,
684687
followRedirects: followRedirects,
@@ -692,7 +695,8 @@ const runSingleRequest = async function (
692695
});
693696

694697
if (request.ntlmConfig) {
695-
axiosInstance = NtlmClient(request.ntlmConfig, axiosInstance.defaults);
698+
const ntlmInstance = NtlmClient(request.ntlmConfig, {});
699+
axiosInstance.defaults.adapter = (config) => ntlmInstance.request({ ...config, adapter: axios.getAdapter('http') });
696700
delete request.ntlmConfig;
697701
}
698702

packages/bruno-cli/src/utils/axios-instance.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ const { createFormData } = require('./form-data');
55
const { setupProxyAgents } = require('./proxy-util');
66
const { isSameOrigin, DEFAULT_MAX_REDIRECTS } = require('@usebruno/common').utils;
77
const { applyOmitHeaders, shouldOmitConnection } = require('@usebruno/common');
8-
const { getSentHeaders, applyOmitConnectionToAxiosConfig } = require('@usebruno/requests');
8+
const { getSentHeaders, applyOmitConnectionToAxiosConfig, handleNtlmRedirect } = require('@usebruno/requests');
99

1010
const redirectResponseCodes = [301, 302, 303, 307, 308];
1111
const METHOD_CHANGING_REDIRECTS = [301, 302, 303];
@@ -184,6 +184,8 @@ function makeAxiosInstance({
184184

185185
const requestConfig = createRedirectConfig(error, redirectUrl);
186186

187+
handleNtlmRedirect(requestConfig, error.config.url, redirectUrl, forwardAuthorizationHeader);
188+
187189
if (!isSameOrigin(error.config.url, redirectUrl)) {
188190
/* AWS SigV4 signs a request for a specific host; re-signing after a cross-origin
189191
* redirect would send a freshly valid signature to an unrelated host, regardless of

packages/bruno-electron/src/ipc/network/axios-instance.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ const { addCookieToJar, getCookieStringForUrl } = require('../../utils/cookies')
88
const { preferencesUtil } = require('../../store/preferences');
99
const { safeStringifyJSON } = require('../../utils/common');
1010
const { createFormData } = require('../../utils/form-data');
11-
const { getSentHeaders, applyOmitConnectionToAxiosConfig } = require('@usebruno/requests');
11+
const { getSentHeaders, applyOmitConnectionToAxiosConfig, handleNtlmRedirect } = require('@usebruno/requests');
1212
const { isSameOrigin, DEFAULT_MAX_REDIRECTS } = require('@usebruno/common').utils;
1313
const { applyOmitHeaders } = require('@usebruno/common');
1414

@@ -378,6 +378,8 @@ function makeAxiosInstance({
378378
}
379379
};
380380

381+
handleNtlmRedirect(requestConfig, error.config.url, redirectUrl, forwardAuthorizationHeader);
382+
381383
if (!isSameOrigin(error.config.url, redirectUrl)) {
382384
/* AWS SigV4 signs a request for a specific host; re-signing after a cross-origin
383385
* redirect would send a freshly valid signature to an unrelated host, regardless of

packages/bruno-electron/src/ipc/network/index.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ const configureRequest = async (
157157

158158
const { promptVariables = {} } = collection;
159159
let { proxyMode, proxyModeReason, proxyConfig, httpsAgentRequestFields, interpolationOptions } = certsAndProxyConfig;
160-
let axiosInstance = makeAxiosInstance({
160+
const axiosInstance = makeAxiosInstance({
161161
proxyMode,
162162
proxyModeReason,
163163
proxyConfig,
@@ -169,7 +169,8 @@ const configureRequest = async (
169169
});
170170

171171
if (request.ntlmConfig) {
172-
axiosInstance = NtlmClient(request.ntlmConfig, axiosInstance.defaults);
172+
const ntlmInstance = NtlmClient(request.ntlmConfig, {});
173+
axiosInstance.defaults.adapter = (config) => ntlmInstance.request({ ...config, adapter: axios.getAdapter('http') });
173174
delete request.ntlmConfig;
174175
}
175176

packages/bruno-requests/src/auth/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,4 @@ export { addDigestInterceptor } from './digestauth-helper';
22
export { getOAuth2Token } from './oauth2-helper';
33
export { createOAuth1Authorizer, computeBodyHash, applyOAuth1ToRequest } from './oauth1-request-authorization';
44
export { addEdgeGridInterceptor, signEdgeGridRequest } from './edgegrid-helper';
5+
export { handleNtlmRedirect } from './ntlm';
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import { isNtlmAuthHeader, handleNtlmRedirect } from './ntlm';
2+
3+
describe('isNtlmAuthHeader', () => {
4+
it.each(['NTLM TlRMTVNTUAAD', 'ntlm TlRMTVNTUAAD', ' NTLM TlRMTVNTUAAD'])(
5+
'recognises %s as a credential bound to the connection it was negotiated on',
6+
(header) => {
7+
expect(isNtlmAuthHeader(header)).toBe(true);
8+
}
9+
);
10+
11+
it.each(['Bearer abc', 'Basic dXNlcjpwYXNz', 'Negotiate YIIF', 'AWS4-HMAC-SHA256 Credential=abc', 'NTLMish token', '', undefined, null, 42])(
12+
'leaves %s alone',
13+
(header) => {
14+
expect(isNtlmAuthHeader(header)).toBe(false);
15+
}
16+
);
17+
});
18+
19+
describe('handleNtlmRedirect', () => {
20+
const ntlmHeaders = { 'Authorization': 'NTLM TlRMTVNTUAAD', 'X-retry': 'false', 'Accept': '*/*' };
21+
22+
it('drops the finished message, X-retry and the adapter when the redirect stays on the origin', () => {
23+
const requestConfig = { headers: { ...ntlmHeaders }, adapter: 'ntlm' };
24+
25+
handleNtlmRedirect(requestConfig, 'https://a.test/x', 'https://a.test/y', false);
26+
27+
expect(requestConfig.headers).toEqual({ Accept: '*/*' });
28+
expect(requestConfig.adapter).toBeUndefined();
29+
});
30+
31+
it('drops the finished message but keeps the adapter when the redirect goes to another host', () => {
32+
const requestConfig = { headers: { ...ntlmHeaders }, adapter: 'ntlm' };
33+
34+
handleNtlmRedirect(requestConfig, 'https://a.test/x', 'https://b.test/y', false);
35+
36+
expect(requestConfig.headers).toEqual({ Accept: '*/*' });
37+
expect(requestConfig.adapter).toBe('ntlm');
38+
});
39+
40+
it('drops the adapter for another host too when the request forwards Authorization on redirect', () => {
41+
const requestConfig = { headers: { ...ntlmHeaders }, adapter: 'ntlm' };
42+
43+
handleNtlmRedirect(requestConfig, 'https://a.test/x', 'https://b.test/y', true);
44+
45+
expect(requestConfig.headers).toEqual({ Accept: '*/*' });
46+
expect(requestConfig.adapter).toBeUndefined();
47+
});
48+
49+
it('leaves a request that carries any other credential alone', () => {
50+
const requestConfig = { headers: { 'Authorization': 'Bearer abc', 'X-retry': 'false' }, adapter: 'ntlm' };
51+
52+
handleNtlmRedirect(requestConfig, 'https://a.test/x', 'https://a.test/y', true);
53+
54+
expect(requestConfig.headers).toEqual({ 'Authorization': 'Bearer abc', 'X-retry': 'false' });
55+
expect(requestConfig.adapter).toBe('ntlm');
56+
});
57+
});
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import { AxiosRequestConfig } from 'axios';
2+
import { isSameOrigin } from '@usebruno/common/utils';
3+
4+
// The scheme is not in IANA's http registry; it is specified by [MS-NTHT] NTLM Over HTTP Protocol,
5+
// https://learn.microsoft.com/openspecs/windows_protocols/ms-ntht/f09cf6e1-529e-403b-a8a5-7368ee096a6a
6+
const NTLM_SCHEME = /^NTLM(\s|$)/i;
7+
8+
export const isNtlmAuthHeader = (value: unknown): boolean =>
9+
typeof value === 'string' && NTLM_SCHEME.test(value.trim());
10+
11+
const carriesNtlm = (headers: Record<string, unknown>): boolean =>
12+
Object.keys(headers).some((key) => key.toLowerCase() === 'authorization' && isNtlmAuthHeader(headers[key]));
13+
14+
export const handleNtlmRedirect = (
15+
requestConfig: AxiosRequestConfig,
16+
fromUrl: string,
17+
redirectUrl: string,
18+
forwardAuthorizationHeader: boolean
19+
): void => {
20+
const headers = requestConfig.headers ?? {};
21+
22+
if (!carriesNtlm(headers)) {
23+
return;
24+
}
25+
26+
// A finished message proves nothing on the socket a redirect opens, and X-retry
27+
// would stop the library negotiating a new one.
28+
Object.keys(headers).forEach((key) => {
29+
const lowerKey = key.toLowerCase();
30+
if (lowerKey === 'x-retry' || lowerKey === 'authorization') {
31+
delete headers[key];
32+
}
33+
});
34+
35+
// Dropping the inherited plain adapter puts the ntlm code back in the path to answer a fresh
36+
// challenge. Another host only earns that when the request forwards Authorization; otherwise the
37+
// redirect ends on its 401, as curl does by default.
38+
if (isSameOrigin(fromUrl, redirectUrl) || forwardAuthorizationHeader) {
39+
delete requestConfig.adapter;
40+
}
41+
};

packages/bruno-requests/src/index.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,12 @@
1-
export { addDigestInterceptor, getOAuth2Token, createOAuth1Authorizer, computeBodyHash, applyOAuth1ToRequest, addEdgeGridInterceptor } from './auth';
1+
export {
2+
addDigestInterceptor,
3+
getOAuth2Token,
4+
createOAuth1Authorizer,
5+
computeBodyHash,
6+
applyOAuth1ToRequest,
7+
addEdgeGridInterceptor,
8+
handleNtlmRedirect
9+
} from './auth';
210
export { GrpcClient, generateGrpcSampleMessage } from './grpc';
311
export { WsClient } from './ws/ws-client';
412
export { default as cookies } from './cookies';

0 commit comments

Comments
 (0)