Skip to content

Commit

Permalink
Change all connectors to use the basic auth header instead of the `au…
Browse files Browse the repository at this point in the history
…th` property of `axios` (#183162)

## Summary

Fixes: #182391

## Framework changes

- Utils to construct basic header from username and password: [`fad6bde`
(#183162)](fad6bde),
[`b10d103`
(#183162)](b10d103)
- Automatically convert `auth` to basic auth header in the sub-actions
framework: [`ee27353`
(#183162)](ee27353)
- Automatically convert `auth` to basic auth header in axios utils:
[`94753a7`
(#183162)](94753a7)

## Jira

Commit: [`c366163`
(#183162)](c366163)

## All ServiceNow connectors

Commit: [`4324d93`
(#183162)](4324d93)

## IBM Resilient

IBM Resilient already uses the basic auth headers. PR
#180561 added this functionality.
The connector was manually tested when reviewing the PR.

In [`7d9edab`
(#183162)](7d9edab)
I updated the connector to use the new util function.

## Webhook

Commit: [`1a62c77`
(#183162)](1a62c77)

## Cases webhook

Commit: [`104f881`
(#183162)](104f881)

## xMatters

Commit: [`ea7be2b`
(#183162)](ea7be2b)

## Connectors that do not use the `axios` `auth` property

- D3Security
- Email
- Microsoft Teams
- OpenAI
- Opsgenie
- PagerDuty
- Sentinel One
- Slack
- Slack API
- Swimlane
- Tines
- Torq

### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed

### Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to
identify risks that should be tested prior to the change/feature
release.

When forming the risk matrix, consider some of the following examples
and how they may potentially impact the change:

| Risk | Probability | Severity | Mitigation/Notes |

|---------------------------|-------------|----------|-------------------------|
| Connectors not working correctly | Low | High | Unit test and manual
testing of all connectors affected |


### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: “jeramysoucy” <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
3 people authored May 17, 2024
1 parent 621c5bc commit 4b7d014
Show file tree
Hide file tree
Showing 22 changed files with 361 additions and 58 deletions.
4 changes: 1 addition & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,7 @@
"**/@langchain/core": "0.1.53",
"**/@types/node": "20.10.5",
"**/@typescript-eslint/utils": "5.62.0",
"**/axios": "1.6.3",
"**/chokidar": "^3.5.3",
"**/follow-redirects": "1.15.2",
"**/globule/minimatch": "^3.1.2",
"**/hoist-non-react-statics": "^3.3.2",
"**/isomorphic-fetch/node-fetch": "^2.6.7",
Expand Down Expand Up @@ -959,7 +957,7 @@
"archiver": "^5.3.1",
"async": "^3.2.3",
"aws4": "^1.12.0",
"axios": "1.6.3",
"axios": "^1.6.8",
"base64-js": "^1.3.1",
"bitmap-sdf": "^1.0.3",
"blurhash": "^2.0.1",
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/actions/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export {
asSavedObjectExecutionSource,
asHttpRequestExecutionSource,
asNotificationExecutionSource,
getBasicAuthHeader,
} from './lib';
export { ACTION_SAVED_OBJECT_TYPE } from './constants/saved_objects';

Expand Down
49 changes: 49 additions & 0 deletions x-pack/plugins/actions/server/lib/axios_utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,55 @@ describe('request', () => {
`"Do not use \\"baseURL\\" in the creation of your axios instance because you will mostly break proxy"`
);
});

test('it converts the auth property to basic auth header', async () => {
await request({
axios,
url: '/test',
logger,
configurationUtilities,
auth: { username: 'username', password: 'password' },
});

expect(axiosMock).toHaveBeenCalledWith('/test', {
method: 'get',
httpAgent: undefined,
httpsAgent: expect.any(HttpsAgent),
proxy: false,
maxContentLength: 1000000,
timeout: 360000,
headers: { Authorization: `Basic ${Buffer.from('username:password').toString('base64')}` },
});
});

test('it does not override an authorization header if provided', async () => {
await request({
axios,
url: '/test',
logger,
configurationUtilities,
auth: { username: 'username', password: 'password' },
headers: {
'Content-Type': 'application/json',
'X-Test-Header': 'test',
Authorization: 'Bearer my_token',
},
});

expect(axiosMock).toHaveBeenCalledWith('/test', {
method: 'get',
httpAgent: undefined,
httpsAgent: expect.any(HttpsAgent),
proxy: false,
maxContentLength: 1000000,
timeout: 360000,
headers: {
'Content-Type': 'application/json',
'X-Test-Header': 'test',
Authorization: 'Bearer my_token',
},
});
});
});

describe('patch', () => {
Expand Down
13 changes: 11 additions & 2 deletions x-pack/plugins/actions/server/lib/axios_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { Logger } from '@kbn/core/server';
import { getCustomAgents } from './get_custom_agents';
import { ActionsConfigurationUtilities } from '../actions_config';
import { SSLSettings } from '../types';
import { combineHeadersWithBasicAuthHeader } from './get_basic_auth_header';

export const request = async <T = unknown>({
axios,
Expand Down Expand Up @@ -55,10 +56,18 @@ export const request = async <T = unknown>({
const { maxContentLength, timeout: settingsTimeout } =
configurationUtilities.getResponseSettings();

const { auth, ...restConfig } = config;

const headersWithBasicAuth = combineHeadersWithBasicAuthHeader({
username: auth?.username,
password: auth?.password,
headers,
});

return await axios(url, {
...config,
...restConfig,
method,
headers,
headers: headersWithBasicAuth,
...(data ? { data } : {}),
// use httpAgent and httpsAgent and set axios proxy: false, to be able to handle fail on invalid certs
httpAgent,
Expand Down
69 changes: 69 additions & 0 deletions x-pack/plugins/actions/server/lib/get_basic_auth_header.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { combineHeadersWithBasicAuthHeader, getBasicAuthHeader } from './get_basic_auth_header';

describe('get_basic_auth_header', () => {
describe('getBasicAuthHeader', () => {
it('constructs the basic auth header correctly', () => {
expect(getBasicAuthHeader({ username: 'test', password: 'foo' })).toEqual({
Authorization: `Basic ${Buffer.from('test:foo').toString('base64')}`,
});
});
});

describe('combineHeadersWithBasicAuthHeader', () => {
it('constructs the basic auth header correctly', () => {
expect(combineHeadersWithBasicAuthHeader({ username: 'test', password: 'foo' })).toEqual({
Authorization: `Basic ${Buffer.from('test:foo').toString('base64')}`,
});
});

it('adds extra headers correctly', () => {
expect(
combineHeadersWithBasicAuthHeader({
username: 'test',
password: 'foo',
headers: { 'X-token': 'foo' },
})
).toEqual({
Authorization: `Basic ${Buffer.from('test:foo').toString('base64')}`,
'X-token': 'foo',
});
});

it('does not overrides the auth header if provided', () => {
expect(
combineHeadersWithBasicAuthHeader({
username: 'test',
password: 'foo',
headers: { Authorization: 'Bearer my_token' },
})
).toEqual({
Authorization: 'Bearer my_token',
});
});

it('returns only the headers if auth is undefined', () => {
expect(
combineHeadersWithBasicAuthHeader({
headers: { 'X-token': 'foo' },
})
).toEqual({
'X-token': 'foo',
});
});

it('returns undefined with no arguments', () => {
expect(combineHeadersWithBasicAuthHeader()).toEqual(undefined);
});

it('returns undefined when headers are null', () => {
expect(combineHeadersWithBasicAuthHeader({ headers: null })).toEqual(undefined);
});
});
});
33 changes: 33 additions & 0 deletions x-pack/plugins/actions/server/lib/get_basic_auth_header.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import type { AxiosHeaderValue } from 'axios';

interface GetBasicAuthHeaderArgs {
username: string;
password: string;
}

type CombineHeadersWithBasicAuthHeader = Partial<GetBasicAuthHeaderArgs> & {
headers?: Record<string, AxiosHeaderValue> | null;
};

export const getBasicAuthHeader = ({ username, password }: GetBasicAuthHeaderArgs) => {
const header = `Basic ${Buffer.from(`${username}:${password}`).toString('base64')}`;

return { Authorization: header };
};

export const combineHeadersWithBasicAuthHeader = ({
username,
password,
headers,
}: CombineHeadersWithBasicAuthHeader = {}) => {
return username != null && password != null
? { ...getBasicAuthHeader({ username, password }), ...headers }
: headers ?? undefined;
};
1 change: 1 addition & 0 deletions x-pack/plugins/actions/server/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,4 @@ export {
export { validateEmptyStrings } from './validate_empty_strings';
export { parseDate } from './parse_date';
export type { RelatedSavedObjects } from './related_saved_objects';
export { getBasicAuthHeader, combineHeadersWithBasicAuthHeader } from './get_basic_auth_header';
12 changes: 12 additions & 0 deletions x-pack/plugins/actions/server/sub_action_framework/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,18 @@ export class TestSubActionConnector extends SubActionConnector<TestConfig, TestS

return res;
}

public async testAuth({ headers }: { headers?: Record<string, unknown> } = {}) {
const res = await this.request({
url: 'https://example.com',
data: {},
auth: { username: 'username', password: 'password' },
headers: { 'X-Test-Header': 'test', ...headers },
responseSchema: schema.object({ status: schema.string() }),
});

return res;
}
}

export class TestNoSubActions extends SubActionConnector<TestConfig, TestSecrets> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,5 +210,43 @@ describe('SubActionConnector', () => {
'Request to external service failed. Connector Id: test-id. Connector type: .test. Method: get. URL: https://example.com'
);
});

it('converts auth axios property to a basic auth header if provided', async () => {
await service.testAuth();

expect(requestMock).toHaveBeenCalledTimes(1);
expect(requestMock).toBeCalledWith({
axios: axiosInstanceMock,
configurationUtilities: mockedActionsConfig,
logger,
method: 'get',
data: {},
headers: {
'Content-Type': 'application/json',
'X-Test-Header': 'test',
Authorization: `Basic ${Buffer.from('username:password').toString('base64')}`,
},
url: 'https://example.com',
});
});

it('does not override an authorization header if provided', async () => {
await service.testAuth({ headers: { Authorization: 'Bearer my_token' } });

expect(requestMock).toHaveBeenCalledTimes(1);
expect(requestMock).toBeCalledWith({
axios: axiosInstanceMock,
configurationUtilities: mockedActionsConfig,
logger,
method: 'get',
data: {},
headers: {
'Content-Type': 'application/json',
'X-Test-Header': 'test',
Authorization: 'Bearer my_token',
},
url: 'https://example.com',
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import axios, {
AxiosRequestHeaders,
AxiosHeaders,
AxiosHeaderValue,
AxiosBasicCredentials,
} from 'axios';
import { SavedObjectsClientContract } from '@kbn/core-saved-objects-api-server';
import { ElasticsearchClient } from '@kbn/core-elasticsearch-server';
Expand All @@ -29,6 +30,7 @@ import { SubAction, SubActionRequestParams } from './types';
import { ServiceParams } from './types';
import * as i18n from './translations';
import { request } from '../lib/axios_utils';
import { combineHeadersWithBasicAuthHeader } from '../lib/get_basic_auth_header';

const isObject = (value: unknown): value is Record<string, unknown> => {
return isPlainObject(value);
Expand Down Expand Up @@ -87,8 +89,17 @@ export abstract class SubActionConnector<Config, Secrets> {
}
}

private getHeaders(headers?: AxiosRequestHeaders): Record<string, AxiosHeaderValue> {
return { 'Content-Type': 'application/json', ...headers };
private getHeaders(
auth?: AxiosBasicCredentials,
headers?: AxiosRequestHeaders
): Record<string, AxiosHeaderValue> {
const headersWithBasicAuth = combineHeadersWithBasicAuthHeader({
username: auth?.username,
password: auth?.password,
headers,
});

return { 'Content-Type': 'application/json', ...headersWithBasicAuth };
}

private validateResponse(responseSchema: Type<unknown>, data: unknown) {
Expand Down Expand Up @@ -137,15 +148,17 @@ export abstract class SubActionConnector<Config, Secrets> {
`Request to external service. Connector Id: ${this.connector.id}. Connector type: ${this.connector.type} Method: ${method}. URL: ${normalizedURL}`
);

const { auth, ...restConfig } = config;

const res = await request({
...config,
...restConfig,
axios: this.axiosInstance,
url: normalizedURL,
logger: this.logger,
method,
data: this.normalizeData(data),
configurationUtilities: this.configurationUtilities,
headers: this.getHeaders(headers as AxiosHeaders),
headers: this.getHeaders(auth, headers as AxiosHeaders),
timeout,
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { CasesWebhookMethods, CasesWebhookPublicConfigurationType, ExternalServi
import { Logger } from '@kbn/core/server';
import { loggingSystemMock } from '@kbn/core/server/mocks';
import { actionsConfigMock } from '@kbn/actions-plugin/server/actions_config.mock';
import { getBasicAuthHeader } from '@kbn/actions-plugin/server/lib';
const logger = loggingSystemMock.create().get() as jest.Mocked<Logger>;

jest.mock('@kbn/actions-plugin/server/lib/axios_utils', () => {
Expand Down Expand Up @@ -124,6 +125,43 @@ describe('Cases webhook service', () => {
)
).not.toThrow();
});

test('uses the basic auth header for authentication', () => {
createExternalService(
actionId,
{
config,
secrets: { user: 'username', password: 'password' },
},
logger,
configurationUtilities
);

expect(axios.create).toHaveBeenCalledWith({
headers: {
...getBasicAuthHeader({ username: 'username', password: 'password' }),
'content-type': 'application/json',
},
});
});

test('does not add the basic auth header for authentication if hasAuth=false', () => {
createExternalService(
actionId,
{
config: { ...config, hasAuth: false },
secrets: { user: 'username', password: 'password' },
},
logger,
configurationUtilities
);

expect(axios.create).toHaveBeenCalledWith({
headers: {
'content-type': 'application/json',
},
});
});
});

describe('getIncident', () => {
Expand Down
Loading

0 comments on commit 4b7d014

Please sign in to comment.