Conversation
resolves elastic#106770 Initial commit generated with Cursor <details><summary>cursor prompt</summary> Add two new Kibana config settings, `xpack.actions.proxyUser` and `xpack.actions.proxyPassword` in the actions plugin. These will be for the authentication of the proxy itself. The file `src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker` should be updated with these config values. The documentation `docs/reference/configuration-reference/alerting-settings.md` should be updated. These values should be used when the axios code in the actions plugin, when a proxy is used and has these values set. Jest unit tests should be added. Function tests should be added. </details>
🔍 Preview links for changed docs |
✅ Vale Linting ResultsNo issues found on modified lines! The Vale linter checks documentation changes against the Elastic Docs style guide. To use Vale locally or report issues, refer to Elastic style guide for Vale. |
| ); | ||
| let proxyAuth: string | undefined; | ||
| if (hasUrlAuth) { | ||
| proxyAuth = `${decodeURIComponent(proxyUrl.username)}:${decodeURIComponent(proxyUrl.password)}`; |
There was a problem hiding this comment.
I was surprised to learn that the username/password from URL are encoded!
$ node -p 'new URL("https://elas<tic:changeme@example.com")'
URL {
...
username: 'elas%3Ctic',
...
}| const withAuth = new URL(proxySettings.proxyUrl); | ||
| withAuth.username = proxySettings.proxyUser as string; | ||
| withAuth.password = proxySettings.proxyPassword as string; | ||
| httpProxyAgentUrl = withAuth.toString(); |
There was a problem hiding this comment.
This adds the auth info to the URL. Which the http agents we use check for explicitly (I don't remember them doing that when we first started using it), and send the appropriate auth headers for.
You'll also see we set the auth option for the HttpsProxyAgent, to the same value. It looks like the HttpProxyAgent also accepts options now (I think it didn't use to, except maybe timeout?). So seems like these should be aligned.
There was a problem hiding this comment.
Yeah, should we align them with this PR?
Because proxyAuth is decoded but httpProxyAgentUrl is not, they may send different credentials.
|
Pinging @elastic/response-ops (Team:ResponseOps) |
💔 Build Failed
Failed CI StepsTest Failures
Metrics [docs]Public APIs missing comments
Any counts in public APIs
Unknown metric groupsAPI count
ESLint disabled line counts
Total ESLint disabled count
History
|
florent-leborgne
left a comment
There was a problem hiding this comment.
LGTM for docs - just some quick additions needed to specify the version in which these become available
|
|
||
| $$$action-config-proxy-user$$$ | ||
|
|
||
| `xpack.actions.proxyUser`  |
There was a problem hiding this comment.
| `xpack.actions.proxyUser`  | |
| `xpack.actions.proxyUser`  {applies_to}`stack: ga 9.4+` |
|
|
||
| $$$action-config-proxy-password$$$ | ||
|
|
||
| `xpack.actions.proxyPassword`  |
There was a problem hiding this comment.
| `xpack.actions.proxyPassword`  | |
| `xpack.actions.proxyPassword`  {applies_to}`stack: ga 9.4+` |
| If your proxy is using the https protocol (vs the http protocol), the setting `xpack.actions.ssl.proxyVerificationMode: none` will likely be needed, unless your proxy’s certificates are signed using a publicly available certificate authority. | ||
|
|
||
| There is currently no support for using basic authentication with a proxy (authentication for the proxy itself, not the URL being requested through the proxy). | ||
| You can supply proxy credentials in the URL (`http://user:password@proxy-host:8080`) or use [`xpack.actions.proxyUser`](#action-config-proxy-user) and [`xpack.actions.proxyPassword`](#action-config-proxy-password). If the URL already includes a username and password, those take precedence over the separate settings. |
There was a problem hiding this comment.
| You can supply proxy credentials in the URL (`http://user:password@proxy-host:8080`) or use [`xpack.actions.proxyUser`](#action-config-proxy-user) and [`xpack.actions.proxyPassword`](#action-config-proxy-password). If the URL already includes a username and password, those take precedence over the separate settings. | |
| {applies_to}`stack: ga 9.4+` You can supply proxy credentials in the URL (`http://user:password@proxy-host:8080`) or use [`xpack.actions.proxyUser`](#action-config-proxy-user) and [`xpack.actions.proxyPassword`](#action-config-proxy-password). If the URL already includes a username and password, those take precedence over the separate settings. |
ersin-erdal
left a comment
There was a problem hiding this comment.
Overall looks good but I asked some question, if we think they are OK or we can address with separate PRs I can approve.
| headers: proxySettings.proxyHeaders, | ||
| ...(proxyUrl.username && | ||
| proxyUrl.password && { auth: `${proxyUrl.username}:${proxyUrl.password}` }), | ||
| ...(proxyAuth && { auth: proxyAuth }), |
There was a problem hiding this comment.
Could this be a breaking change for the existing users if they have special characters in their creds?
| proxySettings.proxyUser && | ||
| proxySettings.proxyPassword && | ||
| proxySettings.proxyUser !== '' && | ||
| proxySettings.proxyPassword !== '' |
There was a problem hiding this comment.
Nit: Boolean('') is already false.
| expect(proxyAuthHeader).to.eql(BasicAuthHeader); | ||
| }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Maybe a negative test too?
| proxyServer.web(req, res); | ||
| }); | ||
| server.listen(proxyPort); | ||
| return server as unknown as httpProxy; |
There was a problem hiding this comment.
Would changing the return type to httpProxy | http.Server fix this?
| const withAuth = new URL(proxySettings.proxyUrl); | ||
| withAuth.username = proxySettings.proxyUser as string; | ||
| withAuth.password = proxySettings.proxyPassword as string; | ||
| httpProxyAgentUrl = withAuth.toString(); |
There was a problem hiding this comment.
Yeah, should we align them with this PR?
Because proxyAuth is decoded but httpProxyAgentUrl is not, they may send different credentials.
|
We have another proxy package upgrade in the works, so will pick this up again when it merges: #267282 |
|
Not moving, turning into draft to stop getting the review reminders |
|
Support for this was added in #256491 , and there is still a doc issue open to document the new capability: elastic/docs-content#5731 (comment) |
resolves #106770
Release Note
Adds support for proxy-authorization for connectors with Kibana config settings
xpack.actions.proxyUserandxpack.actions.proxyPassword.Summary
Initial commit generated with Cursor
cursor prompt
Add two new Kibana config settings,
xpack.actions.proxyUserandxpack.actions.proxyPasswordin the actions plugin. These will be for the authentication of the proxy itself. The filesrc/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-dockershould be updated with these config values. The documentationdocs/reference/configuration-reference/alerting-settings.mdshould be updated. These values should be used when the axios code in the actions plugin, when a proxy is used and has these values set. Jest unit tests should be added. Function tests should be added.Checklist
Check the PR satisfies following conditions.
Reviewers should verify this PR satisfies this list as well.
release_note:*label is applied per the guidelinesbackport:*labels.