-
Notifications
You must be signed in to change notification settings - Fork 8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ResponseOps] change axios usage to send auth in headers, vs using the auth
property
#182391
Comments
Pinging @elastic/response-ops (Team:ResponseOps) |
Would it makes sense to perform the manual testing portion of this work after the |
@jeramysoucy If we should upgrade the |
I opened this draft PR #183162 on top of @jeramysoucy PR ( |
Framework changes
JiraCommit: All ServiceNow connectorsCommit: IBM ResilientIBM Resilient already uses the basic auth headers. PR #180561 added this functionality. The connector was manually tested when reviewing the PR. In WebhookCommit: Cases webhookCommit: xMattersCommit: Connectors that do not use the
|
Tested below connectors to make sure they are working fine. Testing details: Set up: Deployed a cloud stateful and stateless image with build artifacts from Christo's PR - #183162 Testing in stateful on production To test: Bootstrap rac, run rules which will create alerts, and then verify that you saw them in third party connectors
|
…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]>
…th` property of `axios` (elastic#183162) ## Summary Fixes: elastic#182391 ## Framework changes - Utils to construct basic header from username and password: [`fad6bde` (elastic#183162)](elastic@fad6bde), [`b10d103` (elastic#183162)](elastic@b10d103) - Automatically convert `auth` to basic auth header in the sub-actions framework: [`ee27353` (elastic#183162)](elastic@ee27353) - Automatically convert `auth` to basic auth header in axios utils: [`94753a7` (elastic#183162)](elastic@94753a7) ## Jira Commit: [`c366163` (elastic#183162)](elastic@c366163) ## All ServiceNow connectors Commit: [`4324d93` (elastic#183162)](elastic@4324d93) ## IBM Resilient IBM Resilient already uses the basic auth headers. PR elastic#180561 added this functionality. The connector was manually tested when reviewing the PR. In [`7d9edab` (elastic#183162)](elastic@7d9edab) I updated the connector to use the new util function. ## Webhook Commit: [`1a62c77` (elastic#183162)](elastic@1a62c77) ## Cases webhook Commit: [`104f881` (elastic#183162)](elastic@104f881) ## xMatters Commit: [`ea7be2b` (elastic#183162)](elastic@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]> (cherry picked from commit 4b7d014) # Conflicts: # x-pack/plugins/stack_connectors/server/connector_types/resilient/resilient.ts
…the `auth` property of `axios` (#183162) (#184091) # Backport This will backport the following commits from `main` to `8.14`: - [Change all connectors to use the basic auth header instead of the `auth` property of `axios` (#183162)](#183162) <!--- Backport version: 8.9.8 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Christos Nasikas","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-05-17T11:18:01Z","message":"Change all connectors to use the basic auth header instead of the `auth` property of `axios` (#183162)\n\n## Summary\r\n\r\nFixes: #182391 Framework changes\r\n\r\n- Utils to construct basic header from username and password: [`fad6bde`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/fad6bde6afb1302c8629da47173abcdf41a1a602),\r\n[`b10d103`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/b10d103bd92fd17c77b97a6e014832ac1c6a9bdc)\r\n- Automatically convert `auth` to basic auth header in the sub-actions\r\nframework: [`ee27353`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/ee2735305142cbb84a930e3dc36e7a6d9116bab6)\r\n- Automatically convert `auth` to basic auth header in axios utils:\r\n[`94753a7`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/94753a7342595c0e2ee33c8f6620661d73526990)\r\n\r\n## Jira\r\n\r\nCommit: [`c366163`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/c366163486a516f1d2d62b63d3bde171b6643e19)\r\n\r\n## All ServiceNow connectors\r\n\r\nCommit: [`4324d93`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/4324d931f7bcacfd8b2d7b4eaa9c40562dc22c52)\r\n\r\n## IBM Resilient\r\n\r\nIBM Resilient already uses the basic auth headers. PR\r\nhttps://github.com//pull/180561 added this functionality.\r\nThe connector was manually tested when reviewing the PR.\r\n\r\nIn [`7d9edab`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/7d9edabd6e8454ebc2190bfc7f565c8c345f47f5)\r\nI updated the connector to use the new util function.\r\n\r\n## Webhook\r\n\r\nCommit: [`1a62c77`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/1a62c77d46dd40e9529ae102a6db3fc1513c494e)\r\n\r\n## Cases webhook\r\n\r\nCommit: [`104f881`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/104f8812515f5944bc103fa0142e55a0b0350e84)\r\n\r\n## xMatters\r\n\r\nCommit: [`ea7be2b`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/ea7be2bbee89b71c3855769fc480a013e4020732)\r\n\r\n## Connectors that do not use the `axios` `auth` property\r\n\r\n- D3Security\r\n- Email\r\n- Microsoft Teams\r\n- OpenAI\r\n- Opsgenie\r\n- PagerDuty\r\n- Sentinel One\r\n- Slack\r\n- Slack API\r\n- Swimlane\r\n- Tines\r\n- Torq\r\n\r\n### Checklist\r\n\r\nDelete any items that are not applicable to this PR.\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\r\n- [ ] [Flaky Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\r\nused on any tests changed\r\n\r\n### Risk Matrix\r\n\r\nDelete this section if it is not applicable to this PR.\r\n\r\nBefore closing this PR, invite QA, stakeholders, and other developers to\r\nidentify risks that should be tested prior to the change/feature\r\nrelease.\r\n\r\nWhen forming the risk matrix, consider some of the following examples\r\nand how they may potentially impact the change:\r\n\r\n| Risk | Probability | Severity | Mitigation/Notes |\r\n\r\n|---------------------------|-------------|----------|-------------------------|\r\n| Connectors not working correctly | Low | High | Unit test and manual\r\ntesting of all connectors affected |\r\n\r\n\r\n### For maintainers\r\n\r\n- [x] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by: “jeramysoucy” <[email protected]>\r\nCo-authored-by: Kibana Machine <[email protected]>","sha":"4b7d0145827925a297d1e1728e4447ecc4673554","branchLabelMapping":{"^v8.15.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","backport:skip","Team:ResponseOps","Feature:Actions/ConnectorTypes","ci:build-cloud-image","ci:build-serverless-image","v8.15.0"],"number":183162,"url":"#183162 all connectors to use the basic auth header instead of the `auth` property of `axios` (#183162)\n\n## Summary\r\n\r\nFixes: #182391 Framework changes\r\n\r\n- Utils to construct basic header from username and password: [`fad6bde`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/fad6bde6afb1302c8629da47173abcdf41a1a602),\r\n[`b10d103`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/b10d103bd92fd17c77b97a6e014832ac1c6a9bdc)\r\n- Automatically convert `auth` to basic auth header in the sub-actions\r\nframework: [`ee27353`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/ee2735305142cbb84a930e3dc36e7a6d9116bab6)\r\n- Automatically convert `auth` to basic auth header in axios utils:\r\n[`94753a7`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/94753a7342595c0e2ee33c8f6620661d73526990)\r\n\r\n## Jira\r\n\r\nCommit: [`c366163`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/c366163486a516f1d2d62b63d3bde171b6643e19)\r\n\r\n## All ServiceNow connectors\r\n\r\nCommit: [`4324d93`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/4324d931f7bcacfd8b2d7b4eaa9c40562dc22c52)\r\n\r\n## IBM Resilient\r\n\r\nIBM Resilient already uses the basic auth headers. PR\r\nhttps://github.com//pull/180561 added this functionality.\r\nThe connector was manually tested when reviewing the PR.\r\n\r\nIn [`7d9edab`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/7d9edabd6e8454ebc2190bfc7f565c8c345f47f5)\r\nI updated the connector to use the new util function.\r\n\r\n## Webhook\r\n\r\nCommit: [`1a62c77`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/1a62c77d46dd40e9529ae102a6db3fc1513c494e)\r\n\r\n## Cases webhook\r\n\r\nCommit: [`104f881`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/104f8812515f5944bc103fa0142e55a0b0350e84)\r\n\r\n## xMatters\r\n\r\nCommit: [`ea7be2b`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/ea7be2bbee89b71c3855769fc480a013e4020732)\r\n\r\n## Connectors that do not use the `axios` `auth` property\r\n\r\n- D3Security\r\n- Email\r\n- Microsoft Teams\r\n- OpenAI\r\n- Opsgenie\r\n- PagerDuty\r\n- Sentinel One\r\n- Slack\r\n- Slack API\r\n- Swimlane\r\n- Tines\r\n- Torq\r\n\r\n### Checklist\r\n\r\nDelete any items that are not applicable to this PR.\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\r\n- [ ] [Flaky Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\r\nused on any tests changed\r\n\r\n### Risk Matrix\r\n\r\nDelete this section if it is not applicable to this PR.\r\n\r\nBefore closing this PR, invite QA, stakeholders, and other developers to\r\nidentify risks that should be tested prior to the change/feature\r\nrelease.\r\n\r\nWhen forming the risk matrix, consider some of the following examples\r\nand how they may potentially impact the change:\r\n\r\n| Risk | Probability | Severity | Mitigation/Notes |\r\n\r\n|---------------------------|-------------|----------|-------------------------|\r\n| Connectors not working correctly | Low | High | Unit test and manual\r\ntesting of all connectors affected |\r\n\r\n\r\n### For maintainers\r\n\r\n- [x] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by: “jeramysoucy” <[email protected]>\r\nCo-authored-by: Kibana Machine <[email protected]>","sha":"4b7d0145827925a297d1e1728e4447ecc4673554"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.15.0","labelRegex":"^v8.15.0$","isSourceBranch":true,"state":"MERGED","url":"#183162 all connectors to use the basic auth header instead of the `auth` property of `axios` (#183162)\n\n## Summary\r\n\r\nFixes: #182391 Framework changes\r\n\r\n- Utils to construct basic header from username and password: [`fad6bde`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/fad6bde6afb1302c8629da47173abcdf41a1a602),\r\n[`b10d103`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/b10d103bd92fd17c77b97a6e014832ac1c6a9bdc)\r\n- Automatically convert `auth` to basic auth header in the sub-actions\r\nframework: [`ee27353`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/ee2735305142cbb84a930e3dc36e7a6d9116bab6)\r\n- Automatically convert `auth` to basic auth header in axios utils:\r\n[`94753a7`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/94753a7342595c0e2ee33c8f6620661d73526990)\r\n\r\n## Jira\r\n\r\nCommit: [`c366163`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/c366163486a516f1d2d62b63d3bde171b6643e19)\r\n\r\n## All ServiceNow connectors\r\n\r\nCommit: [`4324d93`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/4324d931f7bcacfd8b2d7b4eaa9c40562dc22c52)\r\n\r\n## IBM Resilient\r\n\r\nIBM Resilient already uses the basic auth headers. PR\r\nhttps://github.com//pull/180561 added this functionality.\r\nThe connector was manually tested when reviewing the PR.\r\n\r\nIn [`7d9edab`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/7d9edabd6e8454ebc2190bfc7f565c8c345f47f5)\r\nI updated the connector to use the new util function.\r\n\r\n## Webhook\r\n\r\nCommit: [`1a62c77`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/1a62c77d46dd40e9529ae102a6db3fc1513c494e)\r\n\r\n## Cases webhook\r\n\r\nCommit: [`104f881`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/104f8812515f5944bc103fa0142e55a0b0350e84)\r\n\r\n## xMatters\r\n\r\nCommit: [`ea7be2b`\r\n(#183162)](https://github.com/elastic/kibana/pull/183162/commits/ea7be2bbee89b71c3855769fc480a013e4020732)\r\n\r\n## Connectors that do not use the `axios` `auth` property\r\n\r\n- D3Security\r\n- Email\r\n- Microsoft Teams\r\n- OpenAI\r\n- Opsgenie\r\n- PagerDuty\r\n- Sentinel One\r\n- Slack\r\n- Slack API\r\n- Swimlane\r\n- Tines\r\n- Torq\r\n\r\n### Checklist\r\n\r\nDelete any items that are not applicable to this PR.\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\r\n- [ ] [Flaky Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\r\nused on any tests changed\r\n\r\n### Risk Matrix\r\n\r\nDelete this section if it is not applicable to this PR.\r\n\r\nBefore closing this PR, invite QA, stakeholders, and other developers to\r\nidentify risks that should be tested prior to the change/feature\r\nrelease.\r\n\r\nWhen forming the risk matrix, consider some of the following examples\r\nand how they may potentially impact the change:\r\n\r\n| Risk | Probability | Severity | Mitigation/Notes |\r\n\r\n|---------------------------|-------------|----------|-------------------------|\r\n| Connectors not working correctly | Low | High | Unit test and manual\r\ntesting of all connectors affected |\r\n\r\n\r\n### For maintainers\r\n\r\n- [x] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by: “jeramysoucy” <[email protected]>\r\nCo-authored-by: Kibana Machine <[email protected]>","sha":"4b7d0145827925a297d1e1728e4447ecc4673554"}}]}] BACKPORT-->
To work around an
axios
bug with auth in a redirection, we would like to change our usage ofaxios
to useheaders
to send basic auth values, instead of theauth
property. This is based on a report that indicates this fixes the redirected auth issue.This affects most of our connectors, however there are some common functions wrapping
axios
that most connectors use, so I'm expecting a small number of code changes to the production code. Not clear how badly tests would be affected, but I'm guessing few of them are looking at axios-specific stuff (but could be).We should then run some manual tests against all connectors, with real services, to ensure they still work with auth.
So, two lists. The first indicates whether the connectors code path now uses
headers
instead ofauth
. The second indicates the connector has been manually tested. Indicate also in the comments which connectors' code path has been changed, and manually tested.I've added Server Log and Index has already done, as they don't use
axios
at all ...code path changed to use
headers
manual test of connector with auth
The text was updated successfully, but these errors were encountered: