Skip to content

Commit

Permalink
[8.10] [RAM] fix Slack API proxy (#169171) (#169550)
Browse files Browse the repository at this point in the history
# Backport

This will backport the following commits from `main` to `8.11`:
- [[RAM] fix Slack API proxy
(#169171)](#169171)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Xavier
Mouligneau","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-10-18T18:03:47Z","message":"[RAM]
fix Slack API proxy (#169171)\n\n## Summary\r\n\r\nFIX ->
https://github.com/elastic/kibana/issues/168701\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"7df3f964ce31285162dd8bc7d3850691872d01d5","branchLabelMapping":{"^v8.12.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:ResponseOps","v8.11.0","v8.12.0","v8.10.4"],"number":169171,"url":"https://github.com/elastic/kibana/pull/169171","mergeCommit":{"message":"[RAM]
fix Slack API proxy (#169171)\n\n## Summary\r\n\r\nFIX ->
https://github.com/elastic/kibana/issues/168701\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"7df3f964ce31285162dd8bc7d3850691872d01d5"}},"sourceBranch":"main","suggestedTargetBranches":["8.11","8.10"],"targetPullRequestStates":[{"branch":"8.11","label":"v8.11.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.12.0","labelRegex":"^v8.12.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/169171","number":169171,"mergeCommit":{"message":"[RAM]
fix Slack API proxy (#169171)\n\n## Summary\r\n\r\nFIX ->
https://github.com/elastic/kibana/issues/168701\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"7df3f964ce31285162dd8bc7d3850691872d01d5"}},{"branch":"8.10","label":"v8.10.4","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
  • Loading branch information
XavierM authored Oct 23, 2023
1 parent 6a92eab commit 86e52f1
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 14 deletions.
21 changes: 20 additions & 1 deletion x-pack/plugins/actions/server/lib/axios_utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import axios from 'axios';
import axios, { AxiosInstance } from 'axios';
import { Agent as HttpsAgent } from 'https';
import HttpProxyAgent from 'http-proxy-agent';
import { HttpsProxyAgent } from 'https-proxy-agent';
Expand Down Expand Up @@ -260,6 +260,25 @@ describe('request', () => {
data: { incidentId: '123' },
});
});

test('throw an error if you use baseUrl in your axios instance', async () => {
await expect(async () => {
await request({
axios: {
...axios,
defaults: {
...axios.defaults,
baseURL: 'https://here-we-go.com',
},
} as unknown as AxiosInstance,
url: '/test',
logger,
configurationUtilities,
});
}).rejects.toThrowErrorMatchingInlineSnapshot(
`"Do not use \\"baseURL\\" in the creation of your axios instance because you will mostly break proxy"`
);
});
});

describe('patch', () => {
Expand Down
5 changes: 5 additions & 0 deletions x-pack/plugins/actions/server/lib/axios_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ export const request = async <T = unknown>({
headers?: Record<string, AxiosHeaderValue>;
sslOverrides?: SSLSettings;
} & AxiosRequestConfig): Promise<AxiosResponse> => {
if (!isEmpty(axios?.defaults?.baseURL ?? '')) {
throw new Error(
`Do not use "baseURL" in the creation of your axios instance because you will mostly break proxy`
);
}
const { httpAgent, httpsAgent } = getCustomAgents(
configurationUtilities,
logger,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ const requestMock = utils.request as jest.Mock;

const services: Services = actionsMock.createServices();
const mockedLogger: jest.Mocked<Logger> = loggerMock.create();
const headers = {
Authorization: 'Bearer some token',
'Content-type': 'application/json; charset=UTF-8',
};

let connectorType: SlackApiConnectorType;
let configurationUtilities: jest.Mocked<ActionsConfigurationUtilities>;
Expand Down Expand Up @@ -244,9 +248,10 @@ describe('execute', () => {
expect(requestMock).toHaveBeenCalledWith({
axios,
configurationUtilities,
headers,
logger: mockedLogger,
method: 'post',
url: 'chat.postMessage',
url: 'https://slack.com/api/chat.postMessage',
data: { channel: 'general', text: 'some text' },
});

Expand Down Expand Up @@ -294,9 +299,10 @@ describe('execute', () => {
expect(requestMock).toHaveBeenCalledWith({
axios,
configurationUtilities,
headers,
logger: mockedLogger,
method: 'get',
url: 'conversations.list?exclude_archived=true&types=public_channel,private_channel&limit=1000',
url: 'https://slack.com/api/conversations.list?exclude_archived=true&types=public_channel,private_channel&limit=1000',
});

expect(response).toEqual({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,14 @@ describe('Slack API service', () => {
await service.getChannels();
expect(requestMock).toHaveBeenCalledWith({
axios,
headers: {
Authorization: 'Bearer token',
'Content-type': 'application/json; charset=UTF-8',
},
logger,
configurationUtilities,
method: 'get',
url: 'conversations.list?exclude_archived=true&types=public_channel,private_channel&limit=1000',
url: 'https://slack.com/api/conversations.list?exclude_archived=true&types=public_channel,private_channel&limit=1000',
});
});

Expand All @@ -155,10 +159,14 @@ describe('Slack API service', () => {
expect(requestMock).toHaveBeenCalledTimes(1);
expect(requestMock).toHaveBeenNthCalledWith(1, {
axios,
headers: {
Authorization: 'Bearer token',
'Content-type': 'application/json; charset=UTF-8',
},
logger,
configurationUtilities,
method: 'post',
url: 'chat.postMessage',
url: 'https://slack.com/api/chat.postMessage',
data: { channel: 'general', text: 'a message' },
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,11 @@ export const createExternalService = (
throw Error(`[Action][${SLACK_CONNECTOR_NAME}]: Wrong configuration.`);
}

const axiosInstance = axios.create({
baseURL: SLACK_URL,
headers: {
Authorization: `Bearer ${token}`,
'Content-type': 'application/json; charset=UTF-8',
},
});
const axiosInstance = axios.create();
const headers = {
Authorization: `Bearer ${token}`,
'Content-type': 'application/json; charset=UTF-8',
};

const getChannels = async (): Promise<
ConnectorTypeExecutorResult<GetChannelsResponse | void>
Expand All @@ -131,9 +129,10 @@ export const createExternalService = (
return request<GetChannelsResponse>({
axios: axiosInstance,
configurationUtilities,
headers,
logger,
method: 'get',
url: `conversations.list?exclude_archived=true&types=public_channel,private_channel&limit=${LIMIT}${
url: `${SLACK_URL}conversations.list?exclude_archived=true&types=public_channel,private_channel&limit=${LIMIT}${
cursor.length > 0 ? `&cursor=${cursor}` : ''
}`,
});
Expand Down Expand Up @@ -188,7 +187,8 @@ export const createExternalService = (
const result: AxiosResponse<PostMessageResponse> = await request({
axios: axiosInstance,
method: 'post',
url: 'chat.postMessage',
url: `${SLACK_URL}chat.postMessage`,
headers,
logger,
data: { channel: channels[0], text },
configurationUtilities,
Expand Down

0 comments on commit 86e52f1

Please sign in to comment.