Skip to content
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

[Stack Connectors][Microsoft Defender] Improve management of OAuth token to Microsoft's API #208120

Conversation

paul-tavares
Copy link
Contributor

@paul-tavares paul-tavares commented Jan 23, 2025

Summary

  • Add re-try logic to requests to Microsoft's API so that if a 401 (Unauthorized) is encountered, the existing cached access token is regenerated and the request is tried again.
    • This change should handle (edge) cases where the connector's access settings could be updated and connector would continue to use the cached token

Checklist

@paul-tavares paul-tavares added release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) v8.18.0 labels Jan 23, 2025
@paul-tavares paul-tavares self-assigned this Jan 23, 2025
@paul-tavares
Copy link
Contributor Author

/ci

@paul-tavares paul-tavares marked this pull request as ready for review January 24, 2025 17:11
@paul-tavares paul-tavares requested a review from a team as a code owner January 24, 2025 17:11
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-defend-workflows (Team:Defend Workflows)

@paul-tavares paul-tavares requested review from ashokaditya and tomsonpl and removed request for pzl and szwarckonrad January 24, 2025 17:11
Copy link
Member

@ashokaditya ashokaditya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes. Just a nit but okay to ship as is.

@@ -52,6 +53,7 @@ export class MicrosoftDefenderEndpointConnector extends SubActionConnector<
params: ServiceParams<MicrosoftDefenderEndpointConfig, MicrosoftDefenderEndpointSecrets>
) {
super(params);
this.instanceId = Math.random().toString(36).substr(2);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe use slice(2) instead of substr as it's deprecated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ohhh. actually. this was supposed to be deleted. I added it only while trying to understand how the instances of these connectors are created by the sub-actions framework. Thanks for point it out.


// If error was a 401, then for some reason the token used was not valid (ex. perhaps the connector's credentials
// were updated). IN this case, we will try again by ensuring a new token is re-generated
if (err.message.includes('Status code: 401')) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How sure are we that 401 is returned only in this specific case?
Or on second though - it doesn't hurt us to retry anyway :)
Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We return this error - its built by the .getResponseErrorMessage() method of this class. Unfortunately the connector framework does not return the original error (Axios) from the .request(), thus why we are relying on the .message property

@paul-tavares paul-tavares enabled auto-merge (squash) January 28, 2025 15:24
@paul-tavares paul-tavares merged commit 83e2a16 into elastic:main Jan 28, 2025
8 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/13019549591

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

cc @paul-tavares

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jan 28, 2025
…ken to Microsoft's API (elastic#208120)

## Summary

- Add re-try logic to requests to Microsoft's API so that if a `401`
(Unauthorized) is encountered, the existing cached access token is
regenerated and the request is tried again.
- This change should handle (edge) cases where the connector's access
settings could be updated and connector would continue to use the cached
token

(cherry picked from commit 83e2a16)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Jan 28, 2025
…uth token to Microsoft&#x27;s API (#208120) (#208633)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Stack Connectors][Microsoft Defender] Improve management of OAuth
token to Microsoft&#x27;s API
(#208120)](#208120)

<!--- Backport version: 9.4.3 -->

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

<!--BACKPORT [{"author":{"name":"Paul
Tavares","email":"56442535+paul-tavares@users.noreply.github.com"},"sourceCommit":{"committedDate":"2025-01-28T21:06:28Z","message":"[Stack
Connectors][Microsoft Defender] Improve management of OAuth token to
Microsoft's API (#208120)\n\n## Summary\r\n\r\n- Add re-try logic to
requests to Microsoft's API so that if a `401`\r\n(Unauthorized) is
encountered, the existing cached access token is\r\nregenerated and the
request is tried again.\r\n- This change should handle (edge) cases
where the connector's access\r\nsettings could be updated and connector
would continue to use the
cached\r\ntoken","sha":"83e2a16bf8889a97dd3a1460c5fbcba34726197a","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:Defend
Workflows","backport:prev-minor","v8.18.0"],"title":"[Stack
Connectors][Microsoft Defender] Improve management of OAuth token to
Microsoft's
API","number":208120,"url":"https://github.com/elastic/kibana/pull/208120","mergeCommit":{"message":"[Stack
Connectors][Microsoft Defender] Improve management of OAuth token to
Microsoft's API (#208120)\n\n## Summary\r\n\r\n- Add re-try logic to
requests to Microsoft's API so that if a `401`\r\n(Unauthorized) is
encountered, the existing cached access token is\r\nregenerated and the
request is tried again.\r\n- This change should handle (edge) cases
where the connector's access\r\nsettings could be updated and connector
would continue to use the
cached\r\ntoken","sha":"83e2a16bf8889a97dd3a1460c5fbcba34726197a"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/208120","number":208120,"mergeCommit":{"message":"[Stack
Connectors][Microsoft Defender] Improve management of OAuth token to
Microsoft's API (#208120)\n\n## Summary\r\n\r\n- Add re-try logic to
requests to Microsoft's API so that if a `401`\r\n(Unauthorized) is
encountered, the existing cached access token is\r\nregenerated and the
request is tried again.\r\n- This change should handle (edge) cases
where the connector's access\r\nsettings could be updated and connector
would continue to use the
cached\r\ntoken","sha":"83e2a16bf8889a97dd3a1460c5fbcba34726197a"}},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Paul Tavares <56442535+paul-tavares@users.noreply.github.com>
@paul-tavares paul-tavares deleted the task/olm-microsoft-connector-token-improvements branch January 29, 2025 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants